Consultation table filters#2480
Open
sebastian-j-ibanez wants to merge 4 commits into
Open
Conversation
2 tasks
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 GuideAdds consultant and provider autocomplete filters to the consultations page, wires their values through the action/util/DAO layers into the DTO query, and parameterizes a legacy consultation retrieval query to avoid SQL injection. Sequence diagram for consultations search with consultant/provider filterssequenceDiagram
actor User
participant ViewConsultationRequests_jsp as ViewConsultationRequests.jsp
participant EctViewConsultationRequests2Action
participant EctViewConsultationRequestsUtil
participant ConsultationRequestDaoImpl
User->>ViewConsultationRequests_jsp: submit consultSearchForm
ViewConsultationRequests_jsp->>EctViewConsultationRequests2Action: ViewConsultation.do
EctViewConsultationRequests2Action->>ViewConsultationRequests_jsp: setAttribute consultantId, filterProviderNo
ViewConsultationRequests_jsp->>EctViewConsultationRequestsUtil: estConsultationVecByTeam(..., consultantId, filterProviderNo)
EctViewConsultationRequestsUtil->>ConsultationRequestDaoImpl: getConsultationDTOs(..., consultantId, filterProviderNo)
ConsultationRequestDaoImpl->>ConsultationRequestDaoImpl: buildConsultationDTOQuery(..., consultantId, filterProviderNo)
ConsultationRequestDaoImpl-->>EctViewConsultationRequestsUtil: List<ConsultationListDTO>
EctViewConsultationRequestsUtil-->>ViewConsultationRequests_jsp: populate consultation list
ViewConsultationRequests_jsp-->>User: render filtered consultations
Sequence diagram for loading consultant/provider autocomplete optionssequenceDiagram
actor User
participant ViewConsultationRequests_jsp as ViewConsultationRequests.jsp
participant ConsultationRequestDaoImpl
User->>ViewConsultationRequests_jsp: request consultations page
ViewConsultationRequests_jsp->>ConsultationRequestDaoImpl: getDistinctConsultants()
ConsultationRequestDaoImpl-->>ViewConsultationRequests_jsp: List<ProfessionalSpecialist>
ViewConsultationRequests_jsp->>ConsultationRequestDaoImpl: getDistinctConsultProviders()
ConsultationRequestDaoImpl-->>ViewConsultationRequests_jsp: List<Provider>
ViewConsultationRequests_jsp-->>User: render autocomplete filters (consultant, provider)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThe PR extends consultation request filtering by adding consultant ( ChangesConsultant & Provider Filtering for Consultation Requests
Sequence DiagramsequenceDiagram
participant User
participant AutocompleteJS as jQuery UI Autocomplete
participant consultSearchForm as Search Form
participant EctAction as EctViewConsultationRequests2Action
participant Util as EctViewConsultationRequestsUtil
participant DaoImpl as ConsultationRequestDaoImpl
User->>AutocompleteJS: types consultant/provider name
AutocompleteJS->>consultSearchForm: sync hidden consultantId / filterProviderNo
User->>consultSearchForm: submit (offset reset to 0)
consultSearchForm->>EctAction: POST consultantId, filterProviderNo
EctAction->>Util: estConsultationVecByTeam(..., consultantId, filterProviderNo)
Util->>DaoImpl: getConsultationDTOs(..., consultantId, filterProviderNo)
DaoImpl->>DaoImpl: buildConsultationDTOQuery with specialist.id / mrp.ProviderNo predicates
DaoImpl-->>Util: List~ConsultationListDTO~
Util-->>EctAction: populated consultation list
EctAction-->>User: filtered results rendered in JSP
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 |
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? |
![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 introduces optional consultant and patient provider (MRP) filters to the consultation requests view. It updates the DAO layer, action classes, and the JSP page to fetch distinct lists of consultants and providers and display them as searchable autocomplete dropdowns. The review comments correctly identify critical bugs in ConsultationRequestDaoImpl.java where capitalized property names are used in JPQL queries instead of camelCase, which would result in runtime QuerySyntaxExceptions.
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 found 1 issue, and left some high level feedback:
- The JSP now instantiates
ConsultationRequestDaoand runs distinct consultant/provider queries directly; consider moving these lookups (and the site/team filtering) into the action/util layer and passing the result via request attributes to keep data access out of the view and avoid duplicating privacy logic. - In
getDistinctConsultProviders(), the JPQL usesJOIN Demographic d ON d.DemographicNo = cr.demographicId JOIN Provider mrp ON mrp.ProviderNo = d.ProviderNo; it would be more robust and readable to join via mapped entity relationships and entity field names instead of column names and explicitONconditions. - The JSP adds its own
jquery-3.6.4andjquery-ui-1.12.1includes; please verify there isn't an existing global jQuery instance and, if there is, reuse it or usenoConflictto avoid version conflicts and duplicated script payload.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The JSP now instantiates `ConsultationRequestDao` and runs distinct consultant/provider queries directly; consider moving these lookups (and the site/team filtering) into the action/util layer and passing the result via request attributes to keep data access out of the view and avoid duplicating privacy logic.
- In `getDistinctConsultProviders()`, the JPQL uses `JOIN Demographic d ON d.DemographicNo = cr.demographicId JOIN Provider mrp ON mrp.ProviderNo = d.ProviderNo`; it would be more robust and readable to join via mapped entity relationships and entity field names instead of column names and explicit `ON` conditions.
- The JSP adds its own `jquery-3.6.4` and `jquery-ui-1.12.1` includes; please verify there isn't an existing global jQuery instance and, if there is, reuse it or use `noConflict` to avoid version conflicts and duplicated script payload.
## Individual Comments
### Comment 1
<location path="src/main/webapp/oscarEncounter/oscarConsultationRequest/ViewConsultationRequests.jsp" line_range="195-203" />
<code_context>
+ // privacy filter used elsewhere on this page to the provider list.
+ ConsultationRequestDao consultReqDaoForFilters = SpringUtils.getBean(ConsultationRequestDao.class);
+ List<ProfessionalSpecialist> distinctConsultants = consultReqDaoForFilters.getDistinctConsultants();
+ List<Provider> distinctConsultProviders = consultReqDaoForFilters.getDistinctConsultProviders();
+ if (isSiteAccessPrivacy || isTeamAccessPrivacy) {
+ List<Provider> filteredProviders = new ArrayList<Provider>();
+ for (Provider pv : distinctConsultProviders) {
+ if (providerMap.containsKey(pv.getProviderNo())) {
+ filteredProviders.add(pv);
+ }
+ }
+ distinctConsultProviders = filteredProviders;
+ }
+
</code_context>
<issue_to_address>
**🚨 issue (security):** Provider filter options are privacy-aware, but the server-side filter is not enforced against the same constraints.
The dropdown correctly limits visible providers via `providerMap`, but `filterProviderNo` from the request is still passed through to `estConsultationVecByTeam(...)` and the DAO without being checked against `providerMap` (or equivalent site/team constraints). This allows a crafted request with a provider outside the user’s allowed scope to potentially access unauthorized data. Please either (a) validate/reject `filterProviderNo` values that are not in `providerMap`, or (b) enforce the same site/team restrictions directly in the DAO query so authorization is guaranteed server-side.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/ca/openosp/openo/commn/dao/ConsultationRequestDao.java (1)
77-96: 💤 Low valueConsider adding
@sincetag for consistency with other methods.The existing
getConsultationDTOsoverload (line 61) andgetConsultationDTOsByDemographic(line 73) include@sincetags. Adding one here would maintain consistency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/ca/openosp/openo/commn/dao/ConsultationRequestDao.java` around lines 77 - 96, The JavaDoc comment for the getConsultationDTOs method (the overload that accepts consultantId and filterProviderNo parameters) is missing a `@since` tag for consistency with the other getConsultationDTOs overload and the getConsultationDTOsByDemographic method. Add the `@since` tag to the JavaDoc comment for this method, placing it after the `@return` tag or in the appropriate location with other documentation tags, using the same version notation as the other similar methods in the class.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/main/java/ca/openosp/openo/commn/dao/ConsultationRequestDao.java`:
- Around line 77-96: The JavaDoc comment for the getConsultationDTOs method (the
overload that accepts consultantId and filterProviderNo parameters) is missing a
`@since` tag for consistency with the other getConsultationDTOs overload and the
getConsultationDTOsByDemographic method. Add the `@since` tag to the JavaDoc
comment for this method, placing it after the `@return` tag or in the appropriate
location with other documentation tags, using the same version notation as the
other similar methods in the class.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: acae5517-c97c-4aa2-bc26-50a64d2468c6
📒 Files selected for processing (5)
src/main/java/ca/openosp/openo/commn/dao/ConsultationRequestDao.javasrc/main/java/ca/openosp/openo/commn/dao/ConsultationRequestDaoImpl.javasrc/main/java/ca/openosp/openo/encounter/oscarConsultationRequest/pageUtil/EctViewConsultationRequests2Action.javasrc/main/java/ca/openosp/openo/encounter/oscarConsultationRequest/pageUtil/EctViewConsultationRequestsUtil.javasrc/main/webapp/oscarEncounter/oscarConsultationRequest/ViewConsultationRequests.jsp
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Confidence score: 3/5
- In
src/main/webapp/oscarEncounter/oscarConsultationRequest/ViewConsultationRequests.jsp, sanitation/reset logic runs only on submit handlers, so nativeform.submit()paths used by sorting/paging can carry stale hidden filter IDs and page state, leading users to see incorrect filtered or paged results — route all submit paths through the same reset/sanitize function before merging. - In
src/main/java/ca/openosp/openo/encounter/oscarConsultationRequest/pageUtil/EctViewConsultationRequestsUtil.java, the unusedloggedInInfoparameter is low-risk but leaves intent unclear and may miss useful auth/audit context in exception handling — either remove it with caller cleanup or use it explicitly (especially in thecatchpath) to reduce maintenance risk.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
LiamStanziani
approved these changes
Jun 22, 2026
LiamStanziani
left a comment
LiamStanziani
left a comment
Collaborator
There was a problem hiding this comment.
Looks good to me! thank you.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes Made
ConsultantandProviderfilter inputs on the consultations page.ConsultationRequestDaowith an overloadedgetConsultationDTOs(), for the consultant and provider filters.getDistinctConsultants()andgetDistinctConsultProviders()queries to populate filter options.ConsultantandProviderfilters to the DAO viaEctViewConsultationRequestsUtilandEctViewConsultationRequests2Action.getConsults()to fix a SQL injection.Summary by Sourcery
Add consultant and provider filtering to the consultations view and improve safety of consultation queries.
New Features:
Bug Fixes:
Enhancements:
Summary by cubic
Adds Consultant and Provider autocomplete filters to the Consultations page, integrating with existing filters and sorting, and strengthens query safety and input validation.
New Features
consultantIdandfilterProviderNo, wired through the action and util.Bug Fixes
getConsults()to prevent SQL injection.Written for commit d507313. Summary will update on new commits.
Summary by CodeRabbit