Fix: Rx module meds list should display all meds#2477
Conversation
…ing correctly to localStorage
📝 WalkthroughWalkthroughThe PR modifies the DataTables configuration in ChangesPage-Length Persistence Configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRx drug list DataTables config is updated to use a custom page-length preference stored in localStorage, defaulting to showing all medications, and to offer an explicit "All" option along with several fixed page sizes. Sequence diagram for Rx meds DataTable page length preferencesequenceDiagram
actor User
participant DataTable as drugListDataTable
participant LocalStorage as localStorage
rect rgb(230,230,230)
Note over DataTable: Table initialization
DataTable->>LocalStorage: getItem(drugListPageLength)
LocalStorage-->>DataTable: storedLength or null
DataTable->>DataTable: pageLength getter
DataTable->>DataTable: use storedLength or -1 (All)
DataTable->>DataTable: initComplete(settings)
DataTable->>jQuery: jQuery(settings.nTable).on(length.dt)
end
rect rgb(230,230,230)
Note over User,DataTable: User changes page size
User->>DataTable: change length menu selection
DataTable->>DataTable: length.dt event(len)
DataTable->>LocalStorage: setItem(drugListPageLength, len)
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@SourceryAI review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the DataTables configuration in ListDrugs.jsp to persist the user's selected page length in localStorage. The reviewer points out potential compatibility issues if DataTables version 1.9.4 is loaded instead of 1.13.4, as the new properties and events would be ignored. They suggest using backward-compatible options like iDisplayLength, aLengthMenu, and fnDrawCallback to ensure the persistence feature works reliably across both versions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- When reading the saved page length (
parseInt(localStorage.getItem('drugListPageLength')) || -1), consider usingNumber(...)orparseInt(..., 10)with an explicitisNaNcheck to avoid relying on truthiness and to make the handling of invalid values clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- When reading the saved page length (`parseInt(localStorage.getItem('drugListPageLength')) || -1`), consider using `Number(...)` or `parseInt(..., 10)` with an explicit `isNaN` check to avoid relying on truthiness and to make the handling of invalid values clearer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
2 participants
Problem
After the DataTables upgrade (
edac1715d5, "upgrade medication list to use DataTables and eliminate the server-side sorting"), the Rx medication list stopped showing the full medication list. With no page-length configured, DataTables fell back to its built-in default of 10 rows, so the list silently truncated. Before the upgrade the module displayed the full (filtered) list.The reporter's workaround was to set the dropdown to "100" — but that still isn't "all", and the choice didn't survive a reload.
Fixes #2475
Root cause
Two separate problems in the
window.drugListTableConfigDataTables config inListDrugs.jsp:No default page length. The
aLengthMenu/iDisplayLengthlines were commented out, so DataTables used its default of 10 rows.Page-length persistence was effectively broken. The config used
bStateSavewithfnStateSave/fnStateLoadcallbacks. Those callback names are from the DataTables 1.9 API and were renamed in 1.13.x (stateSaveCallback/stateLoadCallback), so they were never invoked — the intendeddrugListTablelocalStorage key was never written. DataTables' built-in default state-save ran instead and wrote the full state blob toDataTables_Drug_table_<path>, but the drug list is destroyed and rebuilt on every load (rebuildDrugProfileinSearchDrug3.jsp), and that rebuild re-draws the table at the config defaults — overwriting the saved state back to defaults before the user ever sees their value. The net effect: neither page length nor sort order survived a reload.Fix
Replaced the broken full-state-save with a small, targeted page-length preference, mirroring the approach already used by the tickler table (the one table in the app that persists its length reliably):
pageLength: -1) when the user has no saved preference, restoring the pre-upgrade full-list behaviour.localStorage['drugListPageLength'], written on an explicitlength.dtchange (i.e. only when the user actually changes the dropdown).pageLengthgetter (rather than a static value) so the saved length is re-read from storage on every table init — important because the config object is created once but reused across the destroy/rebuild churn.lengthMenuoffers25, 50, 75, 100, All, with "All" in the last position to match the tickler and consultation tables.Resulting behaviour (exactly the requested spec):
Old
DataTables_Drug_table_*/drugListTablelocalStorage entries are simply never read anymore, so existing users land on "All" with no manual cache clear.Testing
Verified in-browser (DataTables 1.13.4, demographic with 133 meds):
drugListPageLength=50)-1)For comparison, the original version was also rebuilt and tested: a changed length (50) and sort (
[0,'asc']) were both clobbered back to defaults (length:10,order:[[1,'desc']]) on the next reload — confirming the prior persistence was non-functional, and that the customdrugListTablekey was never written.Scope / safety
src/main/webapp/oscarRx/ListDrugs.jsp(10 additions, 8 deletions).pageLengthdefault lives only indrugListTableConfig, and the tickler (ticklerPageLength, default 50) and consultation (default 10) tables use their own separate keys/pages.Summary by Sourcery
Persist and honor the user’s chosen page size for the Rx medications list, and expose an option to view all medications at once.
Bug Fixes:
Enhancements:
Summary by cubic
Fixes the Rx meds list so it can display all medications and correctly remembers the user's page size.
Written for commit 16c6db9. Summary will update on new commits.
Summary by CodeRabbit