Skip to content

Fixed some hibernate-query parameter errors, enabled the echart.#14

Merged
yingbull merged 1 commit intodevelop/bullfrogfrom
open-o-11-echart-query
Nov 11, 2024
Merged

Fixed some hibernate-query parameter errors, enabled the echart.#14
yingbull merged 1 commit intodevelop/bullfrogfrom
open-o-11-echart-query

Conversation

@kateyang1998
Copy link
Copy Markdown

@kateyang1998 kateyang1998 commented Nov 7, 2024

Summary by Sourcery

Fix hibernate query parameter indexing errors and enable echart functionality.

Bug Fixes:

  • Correct hibernate query parameter indexing to ensure proper query execution.

Enhancements:

  • Enable echart functionality in the application.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented Nov 7, 2024

Reviewer's Guide by Sourcery

This PR fixes Hibernate query parameter indexing issues in CaseManagementNoteDAOImpl.java. The changes update the parameter placeholders from '?' to indexed placeholders '?0', '?1', etc., to ensure correct parameter binding in the HQL queries.

Class diagram for CaseManagementNoteDAOImpl changes

classDiagram
    class CaseManagementNoteDAOImpl {
        +List<CaseManagementNote> getNotes(List<Long> ids)
        +CaseManagementNote getMostRecentNote(String uuid)
        +List<CaseManagementNote> getCPPNotes(String demoNo, long issueId, String uuid)
        +List<CaseManagementNote> getNotesByDemographic(String demographic_no, String uuid)
        +List<CaseManagementNote> getNotesByDemographicSince(String demographic_no, Date date)
        +long getNotesCountByDemographicId(String demographic_no)
        +List<Object[]> getRawNoteInfoByDemographic(String demographic_no)
        +List<Map<String, Object>> getRawNoteInfoMapByDemographic(String demographic_no)
        +List<Map<String, Object>> getUnsignedRawNoteInfoMapByDemographic(String demographic_no)
        +List<CaseManagementNote> searchDemographicNotes(String demographic_no, String searchString)
        +List<CaseManagementNote> getCaseManagementNoteByProgramIdAndObservationDate(Integer programId, Date minObservationDate, Date maxObservationDate)
        +List<CaseManagementNote> getMostRecentNotesByAppointmentNo(int appointmentNo)
        +List<CaseManagementNote> getMostRecentNotes(Integer demographicNo)
        +List<Integer> getNotesByFacilitySince(Date date, List<Program> programs)
        +int getNoteCountForProviderForDateRange(String providerNo, Date startDate, Date endDate)
        +int getNoteCountForProviderForDateRangeWithIssueId(String providerNo, Date startDate, Date endDate, String issueCode)
    }
    note for CaseManagementNoteDAOImpl "Updated HQL queries to use indexed placeholders for parameters."
Loading

File-Level Changes

Change Details Files
Updated HQL query parameter placeholders to use indexed parameters
  • Changed '?' placeholders to indexed placeholders (?0, ?1, etc.)
  • Fixed parameter indexing in getMostRecentNote query
  • Updated parameter placeholders in getCPPNotes query
  • Fixed parameter indexing in getNotesByDemographic queries
  • Updated parameter placeholders in getNotesCountByDemographicId query
  • Fixed parameter indexing in getRawNoteInfo queries
  • Updated parameter placeholders in getActiveNotesByDemographic queries
  • Fixed parameter indexing in SQL queries for note counting functions
src/main/java/org/oscarehr/casemgmt/dao/CaseManagementNoteDAOImpl.java

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @kateyang1998 - I've reviewed your changes - here's some feedback:

Overall Comments:

  • In getNoteCountForProviderForDateRange(), the SQL query uses ?1, ?2, ?3 but the PreparedStatement uses indices 1,2,3. This mismatch could cause errors - either change the SQL to use ?0, ?1, ?2 or adjust the PreparedStatement indices accordingly.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Override
public CaseManagementNote getMostRecentNote(String uuid) {
String hql = "select distinct cmn from CaseManagementNote cmn where cmn.uuid = ?0 and cmn.id = (select max(cmn.id) from cmn where cmn.uuid = ?0)";
String hql = "select distinct cmn from CaseManagementNote cmn where cmn.uuid = ?0 and cmn.id = (select max(cmn.id) from cmn where cmn.uuid = ?1)";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Parameter numbering inconsistency could cause bugs

The subquery should use ?0 instead of ?1 since it's referring to the same uuid parameter. Using different parameter numbers for the same value could lead to incorrect parameter binding.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of these parameters should be ?0. They use the same value.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sebastian-j-ibanez! 👋

Only authors and team members can run @sourcery-ai commands on public repos.

Copy link
Copy Markdown
Collaborator

@sebastian-j-ibanez sebastian-j-ibanez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one small change to make.

See my comment on the Sourcery comment.

@Override
public CaseManagementNote getMostRecentNote(String uuid) {
String hql = "select distinct cmn from CaseManagementNote cmn where cmn.uuid = ?0 and cmn.id = (select max(cmn.id) from cmn where cmn.uuid = ?0)";
String hql = "select distinct cmn from CaseManagementNote cmn where cmn.uuid = ?0 and cmn.id = (select max(cmn.id) from cmn where cmn.uuid = ?1)";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both of these parameters should be ?0. They use the same value.

@kateyang1998 kateyang1998 force-pushed the open-o-11-echart-query branch from 58afd75 to d0b8a41 Compare November 8, 2024 18:51
Copy link
Copy Markdown
Collaborator

@sebastian-j-ibanez sebastian-j-ibanez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@yingbull yingbull merged commit f2c37c6 into develop/bullfrog Nov 11, 2024
@kateyang1998 kateyang1998 deleted the open-o-11-echart-query branch November 19, 2024 16:16
Jdarji2130 pushed a commit to Jdarji2130/Open-O-jay that referenced this pull request Mar 23, 2025
yingbull pushed a commit that referenced this pull request May 9, 2025
yingbull pushed a commit that referenced this pull request May 9, 2025
fixed: while removing acknowledged provider, their comments were getting archived
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.

3 participants