Skip to content

Fix: extra filter dropdown on tickler manager page#2409

Merged
lacarmen merged 1 commit into
developfrom
regression/extra-filter-dropdown-tickler-manager
Apr 14, 2026
Merged

Fix: extra filter dropdown on tickler manager page#2409
lacarmen merged 1 commit into
developfrom
regression/extra-filter-dropdown-tickler-manager

Conversation

@LiamStanziani

@LiamStanziani LiamStanziani commented Apr 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

A regression was found relating to a missing tickler manager change in this PR: #2268, where an added conditional around the patient-specific tickler manager UI filter dropdown was removed, resulting in a duplicate filter dropdown that should only show in patient-specific tickler manager pages.

image

Fixes #2392

Problem

A conditional check for a patient-specific tickler manager dropdown has been removed, causing the dropdown to always show on the page, this is an issue as there is already a filter option for non-patients AND this duplicate filter doesn't work in that context, causing potential user confusion as to why one of the filter dropdowns is broken.

Solution

Re-add the missing conditional that conditionally shows the filter dropdown on patient-specific tickler manager pages

Manual testing steps

  1. Login to the EMR
  2. Click the "Tickler" option at the top section
  3. If testing the branches behaviour: Observe that there is only 1 filter dropdown available, and that the dropdown works
  4. If testing the develop behaviour: Observe that there is 2 filter dropdowns, and that the filter dropdown closer to the tickler entries does not work inside of the main tickler manager page

Summary by Sourcery

Bug Fixes:

  • Prevent duplicate, non-functional filter dropdown from appearing on non–patient-specific tickler manager pages by gating it on the patient context parameter.

Summary by cubic

Fixes a regression that showed a duplicate filter dropdown on the Tickler Manager. Re-wraps the "ticklerview" select in ticklerMain.jsp with <c:if test="${not empty param.demoview}"> so it only renders on patient-specific pages.

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

Summary by Sourcery

Bug Fixes:

  • Prevent duplicate, non-functional filter dropdown from rendering on non–patient-specific tickler manager pages by conditioning it on the patient context parameter.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed filter control display logic to render conditionally based on specific conditions, improving UI consistency and preventing the filter from appearing in unintended scenarios.

@LiamStanziani LiamStanziani requested a review from Copilot April 13, 2026 13:50
@LiamStanziani LiamStanziani self-assigned this Apr 13, 2026
@coderabbitai

coderabbitai Bot commented Apr 13, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The Tickler Manager page's "Filter" dropdown is now conditionally rendered based on the demoview parameter. Previously, an unconditional <div> block caused the filter to display when it shouldn't, creating a regression. The change wraps this UI element in a JSTL <c:if> condition.

Changes

Cohort / File(s) Summary
Tickler Filter Conditional Rendering
src/main/webapp/tickler/ticklerMain.jsp
Wrapped the "Filter" (ticklerview <select>) UI block in a JSTL condition (<c:if test="${not empty param.demoview}">) to prevent it from rendering unconditionally, fixing the extra dropdown regression.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 A filter that filtered too much, we say,
Now hides away on a conditional day,
When demoview calls, it will cheerfully show,
But vanish quite nicely when it shouldn't flow! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix: extra filter dropdown on tickler manager page' accurately and concisely describes the main change: fixing a duplicate filter dropdown issue on the Tickler Manager page.
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements from issue #2392: it restores the missing conditional to gate the filter dropdown on patient context, preventing it from rendering unconditionally on non-patient-specific pages.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the regression: the modification wraps the filter dropdown in a JSTL conditional without introducing unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch regression/extra-filter-dropdown-tickler-manager

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.

@LiamStanziani

LiamStanziani commented Apr 13, 2026

Copy link
Copy Markdown
Collaborator Author

@SourceryAI review

@sourcery-ai

sourcery-ai Bot commented Apr 13, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Reintroduces a JSP conditional so the tickler filter dropdown only renders on patient-specific Tickler Manager views, preventing a duplicate/non-functional filter on general views.

Flow diagram for conditional rendering of ticklerview filter in JSP

flowchart TD
    A[Request ticklerMain.jsp] --> B{param.demoview is present and not empty}
    B -- No --> C[Render page without ticklerview dropdown]
    B -- Yes --> D[Render ticklerview dropdown
label Filter
options Active Completed Deleted]
    D --> E[Render rest of ticklerMain.jsp]
    C --> E
Loading

File-Level Changes

Change Details Files
Gate the tickler filter dropdown rendering behind the patient-specific view parameter.
  • Wrap the existing ticklerview filter label/select block in a JSTL <c:if> that checks ${not empty param.demoview}.
  • Ensure the filter dropdown markup remains unchanged so existing behavior on patient-specific pages is preserved.
  • Limit the filter dropdown to patient-specific Tickler Manager pages to avoid a duplicate, broken filter on non-patient views.
src/main/webapp/tickler/ticklerMain.jsp

Assessment against linked issues

Issue Objective Addressed Explanation
https://github.com/openo-beta/Open-O/issues/2392 Ensure the extra ticklerview "Filter" dropdown only appears on patient-specific Tickler Manager pages (and not on the general/non-patient Tickler Manager page), resolving the regression that introduced a duplicate filter.

Possibly linked issues

  • #2392: The PR reintroduces the patient-specific conditional, removing the extra Filter dropdown exactly as the issue reports.

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. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the 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 exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

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

@github-actions

Copy link
Copy Markdown

Dependency Review

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

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 3739743.
Ensure 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 Files

None

@LiamStanziani

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Apr 13, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes a UI regression in OpenO EMR’s Tickler Manager page where an extra (non-functional) “Filter” dropdown was appearing in the non-patient (main) tickler view by restoring the patient-context-only conditional rendering.

Changes:

  • Wrap the secondary “Filter” dropdown (ticklerview) in a ${not empty param.demoview} conditional so it only renders on patient-specific tickler pages.

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

Copy link
Copy Markdown

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 modifies 'ticklerMain.jsp' to conditionally display the tickler filter dropdown based on the presence of the 'demoview' parameter. The review feedback identifies redundant bundle declarations and code duplication within the filter options, recommending a more efficient and readable implementation.

Comment thread src/main/webapp/tickler/ticklerMain.jsp
@LiamStanziani LiamStanziani marked this pull request as ready for review April 13, 2026 14:02
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Fix duplicate filter dropdown on tickler manager page

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Re-adds missing conditional check for patient-specific tickler manager
• Wraps filter dropdown with <c:if test="${not empty param.demoview}"> condition
• Prevents duplicate non-functional filter from appearing on non-patient pages
• Fixes regression from PR #2268 that removed the conditional logic
Diagram
flowchart LR
  A["Tickler Manager Page"] --> B{"Patient-specific<br/>demoview param?"}
  B -->|Yes| C["Show Filter Dropdown"]
  B -->|No| D["Hide Filter Dropdown"]
  C --> E["Single functional filter"]
  D --> E
Loading

Grey Divider

File Changes

1. src/main/webapp/tickler/ticklerMain.jsp 🐞 Bug fix +10/-8

Add conditional wrapper to filter dropdown

• Wrapped filter dropdown div with <c:if test="${not empty param.demoview}"> conditional
• Filter now only renders when demoview parameter is present in request
• Prevents duplicate filter from appearing on non-patient-specific tickler manager pages
• Restores conditional logic that was accidentally removed in previous PR

src/main/webapp/tickler/ticklerMain.jsp


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Apr 13, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)
🐞\ ≡ Correctness (1)

Grey Divider


Remediation recommended

1. Wrong demoview condition 🐞
Description
ticklerMain.jsp renders the patient-only ticklerview dropdown when param.demoview is merely
non-empty, which includes demoview=0 even though the backend treats "0" as the
non-patient/global view. This causes routes that open ticklerMain.jsp?demoview=0 to skip the
global filter controls and still render the patient-context dropdown, contradicting the intended
gating.
Code

src/main/webapp/tickler/ticklerMain.jsp[744]

+            <c:if test="${not empty param.demoview}">
Evidence
Backend logic normalizes missing/empty demoview to "0" and uses "0" as the global/non-patient
sentinel, while the UI currently uses empty/not empty param.demoview which misclassifies
demoview=0 as patient context. There is an existing entry point that explicitly opens ticklerMain
with demoview=0, so this condition will be hit in production.

src/main/webapp/tickler/ticklerMain.jsp[88-97]
src/main/webapp/tickler/ticklerMain.jsp[152-154]
src/main/webapp/tickler/ticklerMain.jsp[836-842]
src/main/webapp/tickler/ticklerMain.jsp[602-603]
src/main/webapp/tickler/ticklerMain.jsp[744-745]
src/main/webapp/provider/appointmentprovideradminday.jsp[1897-1904]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The ticklerMain.jsp UI uses `${empty param.demoview}` vs `${not empty param.demoview}` to decide between the global filter controls and the patient-specific ticklerview dropdown. This incorrectly classifies `demoview=0` as “patient context”, even though the backend treats `"0"` as the global/non-patient sentinel.

### Issue Context
- Backend normalizes missing/empty `demoview` to `"0"` and uses `"0"`/`>0` to decide whether the request is patient-specific.
- There is at least one entry point that calls `ticklerMain.jsp?demoview=0` (schedule).

### Fix Focus Areas
- Update the global-controls condition to include `demoview == '0'`, e.g.:
 - `${empty param.demoview || param.demoview eq '0'}`
- Update the patient-dropdown condition to exclude `demoview == '0'`, e.g.:
 - `${not empty param.demoview && param.demoview ne '0'}`
- Verify there is exactly one `#ticklerview` element in both contexts.

- src/main/webapp/tickler/ticklerMain.jsp[598-606]
- src/main/webapp/tickler/ticklerMain.jsp[744-753]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • The param.demoview check is a bit of a magic condition here; consider centralizing the notion of "patient-specific tickler manager page" (e.g., via a helper or a clearly named flag) so this logic is easier to understand and reuse across the JSP.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `param.demoview` check is a bit of a magic condition here; consider centralizing the notion of "patient-specific tickler manager page" (e.g., via a helper or a clearly named flag) so this logic is easier to understand and reuse across the JSP.

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@LiamStanziani LiamStanziani requested a review from D3V41 April 13, 2026 14:20
@lacarmen lacarmen merged commit ce8663e into develop Apr 14, 2026
34 of 37 checks passed
lacarmen added a commit that referenced this pull request Apr 29, 2026
…down-tickler-manager

Fix: extra filter dropdown on tickler manager page
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.

[Regression]: Extra filter dropdown on tickler manager page

4 participants