Skip to content

fix(medications): TAMOC-395: Fix 'Missing FROM clause' error in Patients Ongoing Medications table (hotfix 2.48)#9260

Open
rohan-bes wants to merge 2 commits intorelease/2.48from
fix/tamoc-395-from-clause-error-medications
Open

fix(medications): TAMOC-395: Fix 'Missing FROM clause' error in Patients Ongoing Medications table (hotfix 2.48)#9260
rohan-bes wants to merge 2 commits intorelease/2.48from
fix/tamoc-395-from-clause-error-medications

Conversation

@rohan-bes
Copy link
Collaborator

Changes

There was a fair bit of finagling to get cursor to do the right thing here, tbf its a pretty awkward issue. Sequelize kept moving parts of the nested include into an inner subquery to ensure it fetched the right number of records, but there were missing join clauses.

I think I've got it working right now, and added a bunch of unit tests to confirm

Deploys

  • Deploy to Tamanu Internal

Tests

  • Run E2E Tests

Review Hero

  • Run Review Hero
  • Auto-fix review suggestions Wait for Review Hero to finish, resolve any comments you disagree with or want to fix manually, then check this to auto-fix the rest.
  • Auto-fix CI failures Check this to auto-fix lint errors, test failures, and other CI issues.

Remember to...

  • ...write or update tests
  • ...add UI screenshots and testing notes to the Linear issue
  • ...add any manual upgrade steps to the Linear issue
  • ...update the config reference, settings reference, or any relevant runbook(s)
  • ...call out additions or changes to config files for the deployment team to take note of

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical database query error affecting the display of patient ongoing medications. The primary goal was to fix a 'Missing FROM clause' error that arose from complex Sequelize include operations, particularly when pagination and sensitive data permissions were involved. The changes ensure that medication lists are accurately retrieved, sensitive information is protected based on user permissions, and facility-specific drug details are correctly associated and displayed, thereby improving data integrity and user experience for medication management.

Highlights

  • Database Query Fix: Resolved a 'Missing FROM clause' error that occurred in the Patients Ongoing Medications table due to Sequelize incorrectly generating subqueries when include statements were combined with pagination and sensitive data filtering.
  • Sensitive Medication Handling: Improved the logic for filtering sensitive medications, ensuring that they are only returned if the user has the necessary 'list SensitiveMedication' permission. This was achieved by adjusting the where clause and include options to correctly exclude sensitive drugs.
  • Facility-Specific Drug Information: Enhanced the API to conditionally include referenceDrugFacility data in the response for ongoing prescriptions when a facilityId is provided in the query, allowing for facility-specific drug stock information.
  • Comprehensive Unit Tests: Added extensive unit tests for the /patient/:id/ongoing-prescriptions, /patient/:id/dispensed-medications, and /patient/:id/last-inpatient-discharge-medications endpoints to validate the correct behavior of permission checks, sensitive medication filtering, and facility-specific data inclusion.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • packages/facility-server/tests/apiv1/Patient.test.js
    • Imported DRUG_STOCK_STATUSES and disableHardcodedPermissionsForSuite for new test scenarios.
    • Added new test suites for GET /patient/:id/ongoing-prescriptions, GET /patient/:id/dispensed-medications, and GET /patient/:id/last-inpatient-discharge-medications endpoints.
    • Included tests to verify permission handling for listing medications and sensitive medications.
    • Implemented tests to confirm correct filtering of sensitive medications based on user permissions.
    • Added tests to ensure referenceDrugFacility data is correctly included or excluded based on the facilityId query parameter.
  • packages/facility-server/app/routes/apiv1/patient/patient.js
    • Imported isNil from lodash for robust null/undefined checks.
    • Parsed page and rowsPerPage query parameters more safely using isNil and parseInt.
    • Refactored medication filtering logic to dynamically apply sensitive medication checks.
    • Introduced a referenceDrugInclude object to standardize the inclusion of ReferenceDrug and ReferenceDrugFacility associations.
    • Added prescriber and discontinuingClinician user models to the prescription include list.
    • Modified the medication include to use the new referenceDrugInclude and set required: true.
    • Implemented a conditional where clause for sensitive medication filtering using Op.or.
    • Crucially, set subQuery: false in findAll options when both pagination and sensitive filtering are active to prevent Sequelize from generating incorrect subqueries and resolve the 'Missing FROM clause' error.
    • Updated the response data mapping to conditionally include referenceDrugFacility information when a facilityId is provided.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively addresses a "Missing FROM clause" error that occurred when fetching paginated ongoing prescriptions with specific filters. The core of the fix, setting subQuery: false in the Sequelize query, is a well-targeted solution to prevent problematic subquery generation. The refactoring of the query logic for filtering sensitive medications and handling facility-specific data is also a good improvement. I appreciate the comprehensive set of unit tests added, which cover various scenarios including permissions and edge cases, significantly increasing confidence in the fix. I have one minor suggestion to improve the robustness of integer parsing.

Comment on lines +521 to +522
const parsedPage = isNil(page) ? undefined : parseInt(page) || 0;
const parsedRowsPerPage = isNil(rowsPerPage) ? undefined : parseInt(rowsPerPage) || 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

It's a good practice to always specify the radix (the base in mathematical numeral systems) for the parseInt function. This prevents unexpected behavior, especially with strings that could be misinterpreted as octal or hexadecimal numbers. While modern JavaScript engines default to base 10, being explicit improves code clarity and robustness across different environments.

Suggested change
const parsedPage = isNil(page) ? undefined : parseInt(page) || 0;
const parsedRowsPerPage = isNil(rowsPerPage) ? undefined : parseInt(rowsPerPage) || 10;
const parsedPage = isNil(page) ? undefined : parseInt(page, 10) || 0;
const parsedRowsPerPage = isNil(rowsPerPage) ? undefined : parseInt(rowsPerPage, 10) || 10;

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.

1 participant