Skip to content

chore: remove dead showRxChartNo preference and related code#1971

Open
yingbull wants to merge 1 commit intodevelopfrom
claude/remove-dead-showrxchartno-szjbh
Open

chore: remove dead showRxChartNo preference and related code#1971
yingbull wants to merge 1 commit intodevelopfrom
claude/remove-dead-showrxchartno-szjbh

Conversation

@yingbull
Copy link
Copy Markdown
Collaborator

@yingbull yingbull commented Apr 25, 2026

The showRxChartNo preference had no admin UI to toggle it and shipped
defaulted to false. Remove the preference from carlos.properties (both
main and devcontainer copies) along with the JSP scriptlet that read it,
the ptChartNo variable it populated, the hidden patientChartNo input
that submitted to FrmCustomedPDFServlet, and the display block in
Preview2.jsp that rendered the chart number.

Behavior is preserved: FrmCustomedPDFServlet still reads patientChartNo
from the request, defaults null to empty, and skips rendering when
empty — equivalent to the prior showRxChartNo=false default.


Summary by cubic

Removed dead showRxChartNo preference and the related chart number UI in Rx preview. Behavior remains the same: chart number is omitted unless provided to FrmCustomedPDFServlet.

  • Refactors
    • Deleted showRxChartNo from src/main/resources/carlos.properties and .devcontainer/development/config/shared/volumes/carlos.properties.
    • Removed scriptlet, ptChartNo, hidden patientChartNo field, and the chart number display block in src/main/webapp/WEB-INF/jsp/rx/Preview2.jsp.

Written for commit efba6b6. Summary will update on new commits.

The showRxChartNo preference had no admin UI to toggle it and shipped
defaulted to false. Remove the preference from carlos.properties (both
main and devcontainer copies) along with the JSP scriptlet that read it,
the ptChartNo variable it populated, the hidden patientChartNo input
that submitted to FrmCustomedPDFServlet, and the display block in
Preview2.jsp that rendered the chart number.

Behavior is preserved: FrmCustomedPDFServlet still reads patientChartNo
from the request, defaults null to empty, and skips rendering when
empty — equivalent to the prior showRxChartNo=false default.
Copilot AI review requested due to automatic review settings April 25, 2026 02:47
Copy link
Copy Markdown
Contributor

@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.

Sorry @yingbull, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 25, 2026

Warning

Rate limit exceeded

@yingbull has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 58 minutes and 3 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 58 minutes and 3 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c079f756-313b-4e47-98ed-1b745ff65727

📥 Commits

Reviewing files that changed from the base of the PR and between b2e9356 and efba6b6.

📒 Files selected for processing (3)
  • .devcontainer/development/config/shared/volumes/carlos.properties
  • src/main/resources/carlos.properties
  • src/main/webapp/WEB-INF/jsp/rx/Preview2.jsp
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/remove-dead-showrxchartno-szjbh
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/remove-dead-showrxchartno-szjbh

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.

@penify-dev
Copy link
Copy Markdown
Contributor

penify-dev Bot commented Apr 25, 2026

Failed to generate code suggestions for PR

@github-actions
Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-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.

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.
Architecture diagram
sequenceDiagram
    participant Browser as Client Browser
    participant JSP as Preview2.jsp (Server-side)
    participant Config as carlos.properties
    participant Servlet as FrmCustomedPDFServlet

    Note over Browser,Servlet: Rx Preview and PDF Generation Flow

    Browser->>JSP: GET /rx/Preview2.jsp
    
    Note over JSP: CHANGED: showRxChartNo preference logic removed
    
    JSP->>JSP: Build hidden form fields
    Note right of JSP: CHANGED: 'patientChartNo' hidden input removed
    
    JSP-->>Browser: Render Preview HTML
    
    Note over Browser: User clicks "Print" or "Save"
    
    Browser->>Servlet: POST (patientCityPostal, patientHIN, etc.)
    
    Note over Servlet: CHANGED: Request no longer contains 'patientChartNo'
    
    Servlet->>Servlet: getParameter("patientChartNo")
    
    alt patientChartNo is null or empty
        Servlet->>Servlet: Skip rendering Chart Number in PDF
    end

    Servlet-->>Browser: Return Generated PDF Document
Loading

Copy link
Copy Markdown
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 removes the showRxChartNo configuration and its associated logic from the CARLOS EMR system. The changes involve deleting the property from configuration files and removing the code responsible for retrieving and displaying the patient's chart number on the prescription preview page. I have no feedback to provide.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes a dead showRxChartNo configuration preference and the associated prescription preview/JSP logic that previously (conditionally) displayed and submitted a patient chart number.

Changes:

  • Removed showRxChartNo from carlos.properties (main + devcontainer copies).
  • Removed the showRxChartNo check, ptChartNo variable, and the patientChartNo hidden form field from Preview2.jsp.
  • Removed the chart number rendering block from the prescription preview UI.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/main/webapp/WEB-INF/jsp/rx/Preview2.jsp Deletes dead preference-driven chart number derivation, submission, and display.
src/main/resources/carlos.properties Removes unused showRxChartNo property entry.
.devcontainer/development/config/shared/volumes/carlos.properties Removes unused showRxChartNo property entry from devcontainer config.

@sonarqubecloud
Copy link
Copy Markdown

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