Skip to content

LNK-4658: Fix sorting when there are 10 or more query plan items - #1380

Merged
edward-miller-lcg merged 3 commits into
devfrom
LNK-4658-double-digit-query-plan
Jan 20, 2026
Merged

LNK-4658: Fix sorting when there are 10 or more query plan items#1380
edward-miller-lcg merged 3 commits into
devfrom
LNK-4658-double-digit-query-plan

Conversation

@edward-miller-lcg

@edward-miller-lcg edward-miller-lcg commented Jan 20, 2026

Copy link
Copy Markdown
Contributor

🛠️ Description of Changes

Fix sorting when there are 10 or more query plan items.

🧪 Testing Performed

Local testing and E2E.

🧑‍🔬 Unit Testing

  • I have written or updated unit tests to cover my changes

📓 Documentation Updated

N/A

Summary by CodeRabbit

  • Bug Fixes

    • Improved sorting behavior for query results with numeric keys—now sorted by numeric value (1, 2, 10, 20) instead of alphabetical order (1, 10, 2, 20).
  • Tests

    • Added unit tests to validate numeric sorting behavior for query collections.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 20, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

The PR modifies query sorting logic in PatientDataService to order InitialQueries and SupplementalQueries by numeric key values rather than lexicographic order. This addresses incorrect deferral of double-digit reference queries (e.g., key "10" was sorted before "2" alphabetically). Additionally, unit tests are added to validate numeric-based sorting behavior.

Changes

Cohort / File(s) Summary
Production Code Change
DotNet/DataAcquisition.Domain/Application/Services/PatientDataService.cs
Modified sorting logic to parse query dictionary keys as integers when possible; keys are now ordered by numeric value rather than lexicographically. Non-numeric keys are deprioritized using int.MaxValue.
Test Coverage
DotNet/ServiceTests/UnitTests/DataAcquisition/QueryPlanConverterTests.cs
Added two new unit tests: one validating numeric sorting of integer-string keys yields correct ascending order (1,2,10,20), and another demonstrating the difference between alphabetic vs. numeric sorting behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • LNK-4119: Check to ensure QP Order #1029: Modifies query dictionary ordering logic using the same numeric-key-based sorting approach for InitialQueries/SupplementalQueries to fix double-digit key handling.

Suggested reviewers

  • seanmcilvenna
  • arianamihailescu
  • dvargaslantana

Poem

🐰 Numeric rabbits hop with glee,
No more "10" before "2"!
Sorting queries, key by key,
Double-digits now in their rightful spree.

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing numeric sorting for query plan items with 10+ entries, which directly addresses the PR's primary objective.
Description check ✅ Passed The description follows the template structure with all required sections filled out: Changes, Testing, Unit Testing checkbox marked, and Documentation status provided.
Linked Issues check ✅ Passed Changes implement numeric sorting for query plan keys LNK-4658, replacing alphabetic ordering to ensure reference queries are deferred correctly and execute in intended sequence.
Out of Scope Changes check ✅ Passed All changes are directly scoped to LNK-4658: sorting logic in PatientDataService and comprehensive unit tests validating numeric vs alphabetic sorting behavior.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@edward-miller-lcg
edward-miller-lcg merged commit 6c1cacb into dev Jan 20, 2026
19 checks passed
@edward-miller-lcg
edward-miller-lcg deleted the LNK-4658-double-digit-query-plan branch January 20, 2026 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants