Feature: eForm auto add tickler window popup#2320
Open
LiamStanziani wants to merge 12 commits into
Open
Conversation
Reviewer's GuideIntegrates tickler creation workflows into eForm toolbar actions by wrapping existing toolbar functions with a stateful tickler prompt/auto-flow, adds jQuery UI dialog infrastructure for tickler confirmation/success/error handling, pre-fills tickler messages via window.opener, returns created tickler IDs from the REST API, and exposes providerNo in eForm pages for use in tickler defaults and payloads. Sequence diagram for eForm toolbar action with tickler auto-open flowsequenceDiagram
actor User
participant Toolbar as eFormToolbar_JS
participant TicklerInt as TicklerIntegration
participant Popup as TicklerAdd_popup
User->>Toolbar: click remotePrint/remoteSave/etc
Toolbar->>TicklerInt: hasTicklerTags() and getTicklerMode()
TicklerInt-->>Toolbar: mode autoOpen
Toolbar->>TicklerInt: wrapWithTicklerCheck(originalFn)
TicklerInt->>TicklerInt: set _ticklerState inProgress
TicklerInt->>TicklerInt: promptTicklerAutoOpen(proceedCallback)
TicklerInt->>TicklerInt: collectTicklerData()
TicklerInt->>TicklerInt: initTicklerDialogs()
TicklerInt->>User: show ticklerConfirmDialog
User-->>TicklerInt: clicks Yes
TicklerInt->>Popup: openTicklerPopup()
TicklerInt->>Popup: URL ticklerAdd.jsp with params
TicklerInt->>Popup: set window._eformTicklerPrefillMessage
TicklerInt->>TicklerInt: showTicklerWaitingOverlay()
Popup->>Popup: DOMContentLoaded
Popup->>Popup: applyOpenerTicklerPrefill()
Popup-->User: user fills tickler and saves
Popup-->>Popup: window.close()
TicklerInt->>TicklerInt: poll popup.closed
TicklerInt->>TicklerInt: hideTicklerWaitingOverlay()
TicklerInt->>User: showProceedDialog
User-->>TicklerInt: clicks Yes, proceed
TicklerInt->>Toolbar: proceedCallback()
TicklerInt->>TicklerInt: set _ticklerState proceeding
TicklerInt->>Toolbar: call originalFn()
Toolbar-->>User: completes original toolbar action
TicklerInt->>TicklerInt: resetHandled() set _ticklerState idle
Sequence diagram for eForm toolbar action with tickler auto-save REST flowsequenceDiagram
actor User
participant Toolbar as eFormToolbar_JS
participant TicklerInt as TicklerIntegration
participant REST as TicklerWebService_addTickler
participant DB as TicklerManager
User->>Toolbar: click remoteSave/remotePrint/etc
Toolbar->>TicklerInt: hasTicklerTags() and getTicklerMode()
TicklerInt-->>Toolbar: mode autoSave
TicklerInt->>TicklerInt: set _ticklerState inProgress
TicklerInt->>TicklerInt: promptTicklerAutoSave(proceedCallback)
TicklerInt->>TicklerInt: collectTicklerData()
TicklerInt->>TicklerInt: build payload with serviceDateWithTz
TicklerInt->>REST: POST /ws/rs/tickler/add payload
REST->>DB: ticklerManager.addTickler(loggedInInfo, tickler)
DB-->>REST: created boolean and tickler.id
REST->>REST: setSuccess(created)
alt created is true
REST->>REST: setMessage(tickler.id)
else created is false
REST->>REST: setMessage(error detail)
end
REST-->>TicklerInt: JSON {success, message}
alt success true
TicklerInt->>TicklerInt: build viewUrl with ticklerId
TicklerInt->>User: show ticklerAutoSaveSuccessDialog with View link
User-->>TicklerInt: clicks OK
TicklerInt->>Toolbar: proceedCallback()
TicklerInt->>TicklerInt: set _ticklerState proceeding then resetHandled()
Toolbar-->>User: completes original toolbar action
else success false
TicklerInt->>User: show ticklerAutoSaveErrorDialog
User-->>TicklerInt: clicks OK (proceed anyway)
TicklerInt->>Toolbar: proceedCallback()
TicklerInt->>TicklerInt: set _ticklerState proceeding then resetHandled()
Toolbar-->>User: completes original toolbar action
end
Updated class diagram for TicklerWebService addTickler REST responseclassDiagram
class TicklerWebService {
+GenericRESTResponse addTickler(Tickler tickler)
}
class GenericRESTResponse {
-boolean success
-String message
+void setSuccess(boolean success)
+void setMessage(String message)
+boolean isSuccess()
+String getMessage()
}
class TicklerManager {
+boolean addTickler(LoggedInInfo loggedInInfo, Tickler tickler)
}
class Tickler {
+Long id
+Long getId()
+void setId(Long id)
+void setProgramId(int programId)
}
class LoggedInInfo {
+String getLoggedInProviderNo()
}
TicklerWebService --> TicklerManager : uses
TicklerWebService --> GenericRESTResponse : returns
TicklerWebService --> Tickler : parameter
TicklerManager --> Tickler : creates/updates
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Collaborator
Author
|
@coderabbitai review |
Summary of ChangesHello, 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 introduces a significant feature to automate tickler creation directly from eForms. It streamlines workflows by allowing eForms to be configured to either prompt the user to create a tickler via an interactive popup or silently create one using a REST API, based on predefined eForm tags. This enhancement aims to improve efficiency and ensure timely follow-ups related to eForm submissions. Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
✅ Actions performedReview triggered.
|
Collaborator
Author
|
@SourceryAI review |
📝 WalkthroughWalkthroughThe PR adds tickler-aware gating to the eForm floating toolbar (JS), pre-populates tickler message from opener state in the tickler add JSP, refactors LoggedInInfo initialization in two JSPs to add a hidden Changes
Sequence DiagramsequenceDiagram
participant User
participant Toolbar as Floating Toolbar
participant TicklerUI as Tickler Dialog UI
participant TicklerAPI as Tickler REST API
participant RemoteAction as Original Remote Action
User->>Toolbar: Click remote action (e.g., Save)
activate Toolbar
Toolbar->>Toolbar: Check for tickler tags
alt Tickler Found
Toolbar->>TicklerUI: Init/show dialog
activate TicklerUI
alt autoOpen Mode
TicklerUI->>User: Show confirm dialog
User->>TicklerUI: Confirm
TicklerUI->>TicklerUI: Open popup (ticklerAdd.jsp)
User->>TicklerUI: Complete popup and close
TicklerUI->>User: Show proceed dialog
User->>TicklerUI: Confirm proceed
else autoSave Mode
TicklerUI->>TicklerAPI: POST /ws/rs/tickler/add
activate TicklerAPI
TicklerAPI->>TicklerAPI: addTickler
TicklerAPI->>TicklerUI: Return response (success/error)
deactivate TicklerAPI
alt Success
TicklerUI->>User: Show success dialog
else Error
TicklerUI->>User: Show error dialog (allow proceed)
end
User->>TicklerUI: Acknowledge
end
deactivate TicklerUI
TicklerUI->>Toolbar: Resume (proceeding)
end
deactivate Toolbar
Toolbar->>RemoteAction: Execute original action
activate RemoteAction
RemoteAction->>User: Complete action
deactivate RemoteAction
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
📝 WalkthroughWalkthroughThe pull request integrates tickler functionality into eForms by adding provider number tracking fields to form submissions, implementing a comprehensive tickler workflow in the toolbar that gates form actions behind optional tickler creation prompts, and enhancing the tickler response message with ticket identifiers. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Toolbar as Floating Toolbar
participant Dialog as Tickler Dialog
participant API as Tickler REST API
participant Action as Form Action
User->>Toolbar: Click Save/Print/etc
Toolbar->>Toolbar: Check hasTicklerTags()
alt Has Tickler Tags
Toolbar->>Dialog: Collect form data & open dialog
alt AutoOpen Mode (Interactive)
Dialog->>User: Display tickler confirmation
User->>Dialog: Enter tickler details
User->>Dialog: Submit
Dialog->>API: POST tickler with form data
API-->>Dialog: Success with tickler ID
Dialog->>User: Show success message
User->>Dialog: Proceed/View Tickler
else AutoSave Mode (Silent)
Dialog->>API: POST tickler silently
API-->>Dialog: Success with tickler ID
Dialog->>User: Notify creation (optional)
end
Dialog->>Action: Proceed with original action
else No Tickler Tags
Toolbar->>Action: Skip tickler flow
end
Action->>Action: Execute (Save/Print/Download/etc)
Action-->>User: Complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![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 4 issues, and left some high level feedback:
- The tickler integration adds a lot of global functions and state onto
window(e.g.resetTicklerHandled,_ticklerDialogButtonClicked,hasTicklerTagshelpers) insideeform_floating_toolbar.js; consider encapsulating these under a single namespaced object or module to reduce risk of collisions with other scripts on the page. - In
styleTicklerDialogCloseButtonthe comment says the close button is styled "with an X character" but the implementation only swaps the class tobtn-closeand removes contents; if Bootstrap-specific styling is relied on, this could render as an empty button in non-Bootstrap contexts—either add explicit text/aria-label or adjust the comment/implementation so they match. - The various dialog texts in
initTicklerDialogs(proceed, success, error) are currently hardcoded; sinceticklerPromptMessageis already configurable via a hidden field, you may want to make the other user-facing strings similarly configurable to better support localization and site-specific wording.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The tickler integration adds a lot of global functions and state onto `window` (e.g. `resetTicklerHandled`, `_ticklerDialogButtonClicked`, `hasTicklerTags` helpers) inside `eform_floating_toolbar.js`; consider encapsulating these under a single namespaced object or module to reduce risk of collisions with other scripts on the page.
- In `styleTicklerDialogCloseButton` the comment says the close button is styled "with an X character" but the implementation only swaps the class to `btn-close` and removes contents; if Bootstrap-specific styling is relied on, this could render as an empty button in non-Bootstrap contexts—either add explicit text/aria-label or adjust the comment/implementation so they match.
- The various dialog texts in `initTicklerDialogs` (proceed, success, error) are currently hardcoded; since `ticklerPromptMessage` is already configurable via a hidden field, you may want to make the other user-facing strings similarly configurable to better support localization and site-specific wording.
## Individual Comments
### Comment 1
<location path="src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js" line_range="49-58" />
<code_context>
+ if (ticklerMode) {
+ initTicklerDialogs();
+
+ // _ticklerHandled stays true during one action chain (e.g. Print→Save)
+ // so Save doesn't re-trigger the dialog when called internally by Print.
+ // resetTicklerHandled() clears it after the chain completes or is canceled.
+ var _ticklerHandled = false;
+ window.resetTicklerHandled = function() { _ticklerHandled = false; };
+
+ var _origSave = remoteSave;
+ var _origPrint = remotePrint;
+ var _origDownload = remoteDownload;
+ var _origFax = remoteFax;
+ var _origEmail = remoteEmail;
+ var _origEdocument = remoteEdocument;
+
+ function wrapWithTicklerCheck(originalFn) {
+ return function() {
+ if (_ticklerHandled) {
</code_context>
<issue_to_address>
**issue (bug_risk):** _ticklerHandled is never reset after a successful action chain, so future actions may skip tickler prompts unintentionally.
`resetTicklerHandled()` is only called on cancel/close (X/Escape), not on successful completion of the flow. In successful paths (e.g., auto-open Yes → popup → proceed, or auto-save success), `_ticklerHandled` stays `true`, so subsequent toolbar actions on the same page will skip tickler entirely. If the goal is “once per page load,” the code is correct but the comment should say that; if the goal is to skip tickler only for internal chained calls (e.g., Print → Save), you’ll need to reset the flag after the outermost operation completes (e.g., in `proceedCallback` or the success dialog OK handler).
</issue_to_address>
### Comment 2
<location path="src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js" line_range="705-706" />
<code_context>
+ * default icon sprite. Resolves the Bootstrap/jQuery UI CSS conflict where
+ * the close icon renders as an empty square.
+ */
+function styleTicklerDialogCloseButton(dialogSelector) {
+ jQuery(dialogSelector).on("dialogopen", function() {
+ var closeBtn = jQuery(this).parent().find(".ui-dialog-titlebar-close");
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Removing all classes from the dialog close button may interfere with jQuery UI styling/behavior.
`removeClass()` with no arguments removes all classes, including `ui-dialog-titlebar-close` and other jQuery UI hooks. This can break styling or other code that relies on those classes. Instead, only remove the icon-related classes and add the Bootstrap class, e.g. `closeBtn.removeClass('ui-icon ui-icon-closethick').addClass('btn-close');` so you resolve the icon conflict without dropping the dialog’s base classes.
```suggestion
var closeBtn = jQuery(this).parent().find(".ui-dialog-titlebar-close");
closeBtn
.empty()
.removeClass("ui-button-icon-only ui-button-icon ui-icon ui-icon-closethick")
.addClass("btn-close")
.css({
```
</issue_to_address>
### Comment 3
<location path="src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js" line_range="742" />
<code_context>
+ * opener window (which would destroy our poll timer and proceed callback).
+ * Polls for popup close, then shows the proceed dialog.
+ */
+function openTicklerPopup(proceedCallback) {
+ var data = collectTicklerData();
+ var contextPath = getTicklerFieldValue("context", "");
</code_context>
<issue_to_address>
**suggestion:** Popup-blocked scenario is treated as a closed popup, which can confuse users.
If `window.open` is blocked, `popup` is `null`, so the first poll treats this as a closed popup and shows the “tickler window has closed…” dialog even though it never opened. Consider checking `!popup` immediately after `window.open` and handling it separately (e.g., a specific popup-blocked message or going straight to `proceedCallback` without referencing a closed window).
</issue_to_address>
### Comment 4
<location path="src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js" line_range="44" />
<code_context>
showSuccessAlert(remoteClose);
}
+
+ // Tickler integration: wrap toolbar actions if eForm has tickler tags
+ var ticklerMode = hasTicklerTags();
+ if (ticklerMode) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider encapsulating the tickler state and dialog behavior in a small controller and shared dialog helper to avoid scattered globals and repeated flag plumbing.
You can keep all existing behavior but reduce the mental overhead by encapsulating the tickler state and dialog wiring into a small controller, and by centralizing the repetitive dialog button/close logic.
### 1. Encapsulate `_ticklerHandled` & `_ticklerDialogButtonClicked`
Instead of exposing `resetTicklerHandled` and `_ticklerDialogButtonClicked` on `window`, keep them inside a controller and expose only the minimal API you actually need:
```js
// create once when ticklerMode is detected
var TicklerController = (function (ticklerMode) {
var handled = false;
var dialogButtonClicked = false;
function resetHandled() {
handled = false;
}
function markDialogButton() {
dialogButtonClicked = true;
}
function onDialogClose() {
if (!dialogButtonClicked) {
resetHandled();
}
dialogButtonClicked = false;
}
function wrapWithGate(originalFn) {
return function () {
if (handled) return originalFn.apply(this, arguments);
handled = true;
var args = arguments, self = this;
var proceed = function () { originalFn.apply(self, args); };
if (ticklerMode === "autoOpen") {
promptTicklerAutoOpen(proceed);
} else if (ticklerMode === "autoSave") {
promptTicklerAutoSave(proceed);
}
};
}
return {
wrapWithGate: wrapWithGate,
resetHandled: resetHandled,
markDialogButton: markDialogButton,
onDialogClose: onDialogClose
};
}(ticklerMode));
```
Then replace the global usage:
```js
// instead of window.resetTicklerHandled / window._ticklerDialogButtonClicked
// wrap toolbar actions
window.remoteSave = TicklerController.wrapWithGate(_origSave);
window.remotePrint = TicklerController.wrapWithGate(_origPrint);
window.remoteDownload = TicklerController.wrapWithGate(_origDownload);
window.remoteFax = TicklerController.wrapWithGate(_origFax);
window.remoteEmail = TicklerController.wrapWithGate(_origEmail);
window.remoteEdocument = TicklerController.wrapWithGate(_origEdocument);
```
…and in dialog-related code:
```js
// initTicklerDialogs
function initTicklerDialogs() {
// ...
jQuery("#ticklerConfirmDialog").dialog({
autoOpen: false,
modal: true,
width: 420,
buttons: {},
close: TicklerController.onDialogClose
});
// same for other tickler dialogs
}
```
```js
// inside any button handler
"Yes": function() {
TicklerController.markDialogButton();
jQuery(this).dialog("close");
// ...
}
// when you need to cancel the chain
"Cancel": function() {
TicklerController.markDialogButton();
jQuery(this).dialog("close");
TicklerController.resetHandled();
}
```
This removes the cross‑cutting globals (`_ticklerHandled`, `resetTicklerHandled`, `_ticklerDialogButtonClicked`) and makes the tickler flow self-contained.
### 2. Centralize the repetitive dialog button pattern
Most dialog flows follow: set buttons → each button sets “button clicked” → close → optionally `proceedCallback()` or reset. You can centralize that:
```js
function openTicklerDialog(dialogId, buttonsConfig) {
var mappedButtons = {};
Object.keys(buttonsConfig).forEach(function (label) {
mappedButtons[label] = function () {
TicklerController.markDialogButton();
jQuery(this).dialog("close");
buttonsConfig[label](); // delegate to caller-specific behavior
};
});
jQuery(dialogId).dialog("option", "buttons", mappedButtons).dialog("open");
}
```
Then your specific flows collapse to the intent, without repeating the flag plumbing:
```js
function promptTicklerAutoOpen(proceedCallback) {
openTicklerDialog("#ticklerConfirmDialog", {
"Yes": function () { openTicklerPopup(proceedCallback); },
"No": function () { proceedCallback(); TicklerController.resetHandled(); }
});
}
function showProceedDialog(proceedCallback) {
openTicklerDialog("#ticklerProceedDialog", {
"Yes, proceed": function () { proceedCallback(); },
"No, cancel": function () { TicklerController.resetHandled(); }
});
}
function showTicklerError(detail, proceedCallback) {
jQuery("#ticklerErrorDetail").text(detail);
openTicklerDialog("#ticklerAutoSaveErrorDialog", {
"OK (proceed anyway)": function () { proceedCallback(); },
"Cancel": function () { TicklerController.resetHandled(); }
});
}
```
This keeps all current behavior (including the “X/Escape cancels the chain” semantics) but:
- removes global flags,
- makes the state transitions explicit via `TicklerController`,
- and shrinks the duplicated dialog boilerplate into a single helper.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/webapp/eform/efmformadd_data.jsp (1)
148-152: Use the same provider source across eForm entry flows.Line 148 uses
provider_no, while Line 151 already resolvesLoggedInInfo. UsingloggedInInfo.getLoggedInProviderNo()forproviderNokeeps behavior aligned withefmshowform_data.jsp.Suggested refactor
- thisEForm.addHiddenInputElement("providerNo", provider_no); + LoggedInInfo loggedInInfo = LoggedInInfo.getLoggedInInfoFromSession(request); + thisEForm.addHiddenInputElement("providerNo", loggedInInfo.getLoggedInProviderNo()); // Add email consent properties - LoggedInInfo loggedInInfo = LoggedInInfo.getLoggedInInfoFromSession(request); addHiddenEmailProperties(loggedInInfo, thisEForm, demographic_no);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/eform/efmformadd_data.jsp` around lines 148 - 152, Replace use of the standalone provider_no when adding the providerNo hidden field with the provider from the resolved session user to keep flows consistent: inside the block where LoggedInInfo loggedInInfo = LoggedInInfo.getLoggedInInfoFromSession(request); call loggedInInfo.getLoggedInProviderNo() and pass that value into thisEForm.addHiddenInputElement("providerNo", ...) instead of provider_no; ensure null-safety if getLoggedInProviderNo() can be null to match existing behavior in efmshowform_data.jsp.src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js (1)
798-833: Convert this auto-save request to async/await.The AJAX request uses callback-based handlers (
successanderrorfunctions), which violates the coding guideline that requires async/await for promises in JavaScript files. Converting to async/await will improve error handling, reduce nesting, and simplify control flow maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js` around lines 798 - 833, Make the AJAX call to add the tickler async by turning the surrounding function into an async function and awaiting jQuery.ajax(...) (jqXHR is thenable) instead of using success/error callbacks: call await jQuery.ajax({url: contextPath + "/ws/rs/tickler/add", type: "POST", contentType: "application/json", data: JSON.stringify(payload), dataType: "json"}); then wrap it in try/catch—on success check resp && resp.success, extract ticklerId and build viewUrl, set jQuery("#ticklerViewLink").attr("href", viewUrl), configure and open jQuery("#ticklerAutoSaveSuccessDialog") with the same OK handler that sets window._ticklerDialogButtonClicked and calls proceedCallback; in the catch block reproduce the existing error parsing logic (inspect xhr.responseText, JSON.parse if possible, fallback to "HTTP " + xhr.status or default detail) and call showTicklerError(detail, proceedCallback); ensure variable names payload, contextPath, proceedCallback, ticklerViewLink, ticklerAutoSaveSuccessDialog, and showTicklerError are used so the refactor maps to the original behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/ca/openosp/openo/webserv/rest/TicklerWebService.java`:
- Around line 402-404: In TicklerWebService (the method that calls
ticklerManager.addTickler and sets the response), only set
response.setMessage(String.valueOf(tickler.getId())) when the creation
succeeded; i.e., capture the boolean result from
ticklerManager.addTickler(getLoggedInInfo(), tickler) (or check
response.isSuccess()) and call response.setMessage(...) inside the success
branch so message is not populated when success is false.
In `@src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js`:
- Around line 62-72: The wrapper function wrapWithTicklerCheck sets the
module-level flag _ticklerHandled true before showing prompts but never resets
it after successful action chains, causing later toolbar actions to skip
prompts; update the proceedCallback and any other success continuation (the
places that call proceed() / originalFn.apply like proceedCallback and the other
proceed handlers) to reset _ticklerHandled = false once the originalFn has been
invoked (or in a finally-style continuation after originalFn completes) so the
flag is cleared for subsequent independent toolbar actions while preserving the
single-prompt behavior during the prompt sequence.
---
Nitpick comments:
In `@src/main/webapp/eform/efmformadd_data.jsp`:
- Around line 148-152: Replace use of the standalone provider_no when adding the
providerNo hidden field with the provider from the resolved session user to keep
flows consistent: inside the block where LoggedInInfo loggedInInfo =
LoggedInInfo.getLoggedInInfoFromSession(request); call
loggedInInfo.getLoggedInProviderNo() and pass that value into
thisEForm.addHiddenInputElement("providerNo", ...) instead of provider_no;
ensure null-safety if getLoggedInProviderNo() can be null to match existing
behavior in efmshowform_data.jsp.
In `@src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js`:
- Around line 798-833: Make the AJAX call to add the tickler async by turning
the surrounding function into an async function and awaiting jQuery.ajax(...)
(jqXHR is thenable) instead of using success/error callbacks: call await
jQuery.ajax({url: contextPath + "/ws/rs/tickler/add", type: "POST", contentType:
"application/json", data: JSON.stringify(payload), dataType: "json"}); then wrap
it in try/catch—on success check resp && resp.success, extract ticklerId and
build viewUrl, set jQuery("#ticklerViewLink").attr("href", viewUrl), configure
and open jQuery("#ticklerAutoSaveSuccessDialog") with the same OK handler that
sets window._ticklerDialogButtonClicked and calls proceedCallback; in the catch
block reproduce the existing error parsing logic (inspect xhr.responseText,
JSON.parse if possible, fallback to "HTTP " + xhr.status or default detail) and
call showTicklerError(detail, proceedCallback); ensure variable names payload,
contextPath, proceedCallback, ticklerViewLink, ticklerAutoSaveSuccessDialog, and
showTicklerError are used so the refactor maps to the original behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dcffae9f-3cef-40e5-ba58-a6dbd4f33b38
📒 Files selected for processing (5)
src/main/java/ca/openosp/openo/webserv/rest/TicklerWebService.javasrc/main/webapp/eform/efmformadd_data.jspsrc/main/webapp/eform/efmshowform_data.jspsrc/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.jssrc/main/webapp/tickler/ticklerAdd.jsp
![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 a feature for eForms to automatically prompt for or create ticklers. The changes are implemented across the backend REST service, JSPs, and client-side JavaScript. My review focuses on improving the robustness, maintainability, and consistency of the new code. I've provided suggestions to enhance API design clarity, ensure safer data handling in JSPs (with one comment specifically referencing consistency rules), and modernize the JavaScript implementation by using appropriate variable scopes and browser APIs.
- Use session-based LoggedInInfo for providerNo in both JSP files - Only set tickler ID in REST response on successful creation - Remove redundant LoggedInInfo call in efmshowform_data.jsp - Scope _dialogButtonClicked to closure instead of window global - Use targeted removeClass for close button styling - Convert all tickler var declarations to const/let - Reset _ticklerHandled after successful action chains - Handle popup blocker with clear message instead of false closed dialog
… improve accessibility
2 tasks
Collaborator
|
@LiamStanziani just want to confirm: is this PR is still being worked on? |
Collaborator
Author
|
@sebastian-j-ibanez This PR is waiting for testing by Keith, not currently working it at the moment but I might need to update it later on if something comes up. Since this is in draft I will probably need to update it somewhat when setting it ready to review for AI comments, but I plan to do that after Keith has approved the feature behaviour. |
Collaborator
|
Okay, sounds good! Just wanted to make sure this PR should be kept open. |
…d-tickler-window-popup Conflicts: src/main/webapp/tickler/ticklerAdd.jsp
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 |
Collaborator
Author
|
@SourceryAI review |
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?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:
- In
eform_floating_toolbar.js, the tickler integration logic is quite large and tightly coupled to the toolbar initialization; consider extracting the tickler-related helpers (dialog setup, data collection, auto-open/auto-save flows) into a separate module/file to keep the toolbar script easier to follow and maintain. - The JSDoc for
createDialogDiv(“Creates a DOM element with text content and optional child elements.”) no longer reflects its actual behavior (it only sets id and title); update the comment to avoid confusion for future readers. - In the auto-save flow, when the REST call returns
success=falseyou currently show a generic “Server returned unsuccessful response.” message; ifresp.messageis present but success is false, consider surfacing that in the error dialog so users get more actionable feedback.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `eform_floating_toolbar.js`, the tickler integration logic is quite large and tightly coupled to the toolbar initialization; consider extracting the tickler-related helpers (dialog setup, data collection, auto-open/auto-save flows) into a separate module/file to keep the toolbar script easier to follow and maintain.
- The JSDoc for `createDialogDiv` (“Creates a DOM element with text content and optional child elements.”) no longer reflects its actual behavior (it only sets id and title); update the comment to avoid confusion for future readers.
- In the auto-save flow, when the REST call returns `success=false` you currently show a generic “Server returned unsuccessful response.” message; if `resp.message` is present but success is false, consider surfacing that in the error dialog so users get more actionable feedback.
## Individual Comments
### Comment 1
<location path="src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js" line_range="57-58" />
<code_context>
+ // Minimal API for dialog handlers to interact with tickler state.
+ // Single window property to avoid polluting the global namespace.
+ // Must be defined before initTicklerDialogs() which references it.
+ window._ticklerIntegration = {
+ resetHandled: function() { _ticklerState = "idle"; },
+ markDialogButton: null // set by initTicklerDialogs closure
+ };
</code_context>
<issue_to_address>
**issue (bug_risk):** Use a consistent reference to the tickler integration object to avoid `ReferenceError` in `initTicklerDialogs` and other helpers.
Here you assign the object as `window._ticklerIntegration`, but `initTicklerDialogs` and other helpers use `_ticklerIntegration` without the `window.` prefix (e.g. `_ticklerIntegration.markDialogButton = ...`, `_ticklerIntegration.resetHandled()`). Since `_ticklerIntegration` isn’t otherwise declared, those reads will throw `ReferenceError`. Either declare `var _ticklerIntegration = window._ticklerIntegration;` in this scope or consistently use `window._ticklerIntegration` everywhere.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js (1)
860-872: CodeQL flag on Line 864 appears to be a false positive, but consider validatingresp.messageshape.CodeQL’s "DOM text reinterpreted as HTML" here is driven by
resp.messageflowing into anhref. In practice the value is the integer id set server-side inTicklerWebService.addTickler(String.valueOf(tickler.getId())), is passed throughencodeURIComponent, and is concatenated after a fixedcontextPath + "/tickler/ticklerEdit.jsp?tickler_no="prefix, so ajavascript:URL is not reachable.For defense-in-depth (and to silence the scanner), consider asserting the id is numeric before building the URL:
🛡️ Optional hardening
- const ticklerId = resp.message; - const viewUrl = contextPath + "/tickler/ticklerEdit.jsp?tickler_no=" + encodeURIComponent(ticklerId); - jQuery("#ticklerViewLink").attr("href", viewUrl); + const ticklerId = resp.message; + if (!/^\d+$/.test(String(ticklerId))) { + showTicklerError("Unexpected tickler id in response.", proceedCallback); + return; + } + const viewUrl = contextPath + "/tickler/ticklerEdit.jsp?tickler_no=" + encodeURIComponent(ticklerId); + jQuery("#ticklerViewLink").attr("href", viewUrl);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js` around lines 860 - 872, The success handler currently trusts resp.message when building viewUrl (ticklerId → viewUrl → jQuery("#ticklerViewLink").attr), so add a defensive validation step in the success function to ensure resp.message is a numeric id (e.g., test resp.message against a /^\d+$/ regex or parseInt+Number.isFinite) before assigning ticklerId and constructing viewUrl; if validation fails, log the unexpected value and skip/abort setting the href and opening the ticklerAutoSaveSuccessDialog (or use a safe fallback), leaving proceedCallback behavior unchanged for valid IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js`:
- Around line 87-91: The proceedCallback currently sets _ticklerState =
"proceeding" then calls originalFn.apply(self, args) and afterwards calls
_ticklerIntegration.resetHandled(); change proceedCallback so that
originalFn.apply(self, args) is executed inside a try block and the state reset
logic (calling _ticklerIntegration.resetHandled() and restoring or clearing
_ticklerState) is performed in a finally block to guarantee reset on both
success and exception; update the proceedCallback function to ensure
_ticklerState is cleared (or set back to its prior value) in the finally so the
tickler dialog state machine cannot get stuck in "proceeding".
---
Nitpick comments:
In `@src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js`:
- Around line 860-872: The success handler currently trusts resp.message when
building viewUrl (ticklerId → viewUrl → jQuery("#ticklerViewLink").attr), so add
a defensive validation step in the success function to ensure resp.message is a
numeric id (e.g., test resp.message against a /^\d+$/ regex or
parseInt+Number.isFinite) before assigning ticklerId and constructing viewUrl;
if validation fails, log the unexpected value and skip/abort setting the href
and opening the ticklerAutoSaveSuccessDialog (or use a safe fallback), leaving
proceedCallback behavior unchanged for valid IDs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 75cedf6e-6f03-414d-be77-c47c278d4c90
📒 Files selected for processing (5)
src/main/java/ca/openosp/openo/webserv/rest/TicklerWebService.javasrc/main/webapp/eform/efmformadd_data.jspsrc/main/webapp/eform/efmshowform_data.jspsrc/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.jssrc/main/webapp/tickler/ticklerAdd.jsp
…hen migrating to develop updates
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The global overlay style injected in
initTicklerDialogs(.ui-widget-overlay { ... !important; }) will affect all jQuery UI dialogs across the app; consider scoping this to tickler-specific overlays or using a more targeted selector to avoid unintended visual changes elsewhere. - The tickler integration adds several globals (
window._ticklerIntegration,window._eformTicklerPrefillMessage, direct reassignment ofremoteSave/remotePrint/etc.); consider wrapping this in a dedicated namespace or module to reduce the risk of collisions and make the integration easier to reason about. - Dialog initialization in
initTicklerDialogsis quite verbose and repetitive (four dialogs with similar setup and button wiring); extracting a helper to create/configure dialogs with parameters (id, title, width, button config) would simplify maintenance and make behavior changes more consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The global overlay style injected in `initTicklerDialogs` (`.ui-widget-overlay { ... !important; }`) will affect all jQuery UI dialogs across the app; consider scoping this to tickler-specific overlays or using a more targeted selector to avoid unintended visual changes elsewhere.
- The tickler integration adds several globals (`window._ticklerIntegration`, `window._eformTicklerPrefillMessage`, direct reassignment of `remoteSave`/`remotePrint`/etc.); consider wrapping this in a dedicated namespace or module to reduce the risk of collisions and make the integration easier to reason about.
- Dialog initialization in `initTicklerDialogs` is quite verbose and repetitive (four dialogs with similar setup and button wiring); extracting a helper to create/configure dialogs with parameters (id, title, width, button config) would simplify maintenance and make behavior changes more consistent.
## Individual Comments
### Comment 1
<location path="src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js" line_range="71-76" />
<code_context>
+ const _origEmail = remoteEmail;
+ const _origEdocument = remoteEdocument;
+
+ function wrapWithTicklerCheck(originalFn) {
+ return function() {
+ if (_ticklerState === "proceeding") {
+ return originalFn.apply(this, arguments);
+ }
+ if (_ticklerState === "inProgress") {
+ return; // block — tickler dialog is still open
+ }
</code_context>
<issue_to_address>
**suggestion:** Consider providing user feedback when actions are blocked while the tickler dialog is in progress.
Right now, when `_ticklerState === "inProgress"`, the wrapper just returns and the user gets no indication that their action was blocked. If a toolbar action is clicked while the tickler dialog is open, it’s silently ignored. Consider instead focusing the existing dialog or showing a brief message/toast so users understand they must finish or cancel the tickler flow before running other actions.
Suggested implementation:
```javascript
function wrapWithTicklerCheck(originalFn) {
return function() {
if (_ticklerState === "proceeding") {
return originalFn.apply(this, arguments);
}
if (_ticklerState === "inProgress") {
// Tickler dialog is already open — bring it to the user's attention
const activeDialog =
document.querySelector(".tickler-dialog, .tickler-modal, .tickler-dialog-container");
if (activeDialog && typeof activeDialog.focus === "function") {
activeDialog.focus();
} else if (window.showTransientMessage) {
// Prefer a non-blocking toast/message if the app provides one
window.showTransientMessage(
"Please complete or cancel the tickler action before continuing."
);
} else {
// Fallback so the user still receives feedback
window.alert("Please complete or cancel the tickler action before continuing.");
}
return; // block — tickler dialog is still open
}
// Check mode at action time so dynamically toggled tags are respected
const currentMode = getTicklerMode();
if (!currentMode) {
return originalFn.apply(this, arguments);
}
_ticklerState = "inProgress";
const args = arguments;
```
1. Replace the CSS selectors in `document.querySelector(".tickler-dialog, .tickler-modal, .tickler-dialog-container")` with the actual class or ID used by your tickler dialog so `focus()` targets the correct element.
2. If your application has an existing toast/notification utility (for example `showToast`, `notifyUser`, or similar), replace the `window.showTransientMessage(...)` and `window.alert(...)` calls with that preferred mechanism to keep UX consistent with the rest of the app.
3. If the tickler dialog content (rather than the container) should receive focus for accessibility (e.g., a close button or heading), adjust the `querySelector` to point to the most appropriate focusable element inside the dialog.
</issue_to_address>
### Comment 2
<location path="src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js" line_range="52" />
<code_context>
+ // - "proceeding": dialog completed, allow the original action through
+ // (e.g. Print→Save chain where Save is called internally by Print)
+ // Resets to "idle" on cancellation. Page reload naturally resets via new JS context.
+ let _ticklerState = "idle";
+
+ // Minimal API for dialog handlers to interact with tickler state.
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the tickler state handling by replacing the tri-state global machine with a boolean in-progress flag plus a local guard flag inside the wrapper closure.
You can simplify the tickler state machine without changing behavior by removing the `"proceeding"` state and `window._ticklerIntegration.resetHandled`, and instead using a short‑lived guard flag scoped to the `wrapWithTicklerCheck` closure.
That keeps the semantics (nested toolbar calls during `proceedCallback` bypass tickler) but avoids the tri‑state and cross‑dialog coupling.
### Before (tri‑state + global reset)
```js
let _ticklerState = "idle";
window._ticklerIntegration = {
resetHandled: function() { _ticklerState = "idle"; },
markDialogButton: null
};
function wrapWithTicklerCheck(originalFn) {
return function() {
if (_ticklerState === "proceeding") {
return originalFn.apply(this, arguments);
}
if (_ticklerState === "inProgress") {
return;
}
const currentMode = getTicklerMode();
if (!currentMode) {
return originalFn.apply(this, arguments);
}
_ticklerState = "inProgress";
const args = arguments;
const self = this;
const proceedCallback = function() {
_ticklerState = "proceeding";
try {
originalFn.apply(self, args);
} finally {
window._ticklerIntegration.resetHandled();
}
};
if (currentMode === "autoOpen") {
promptTicklerAutoOpen(proceedCallback);
} else if (currentMode === "autoSave") {
promptTicklerAutoSave(proceedCallback);
}
};
}
```
### After (boolean state + local guard)
You can keep `initTicklerDialogs` unchanged except for calling a local `resetTicklerState` instead of the global `resetHandled`. The key piece is simplifying the wrapper:
```js
if (hasTicklerTags()) {
let _ticklerInProgress = false;
let _ticklerGuardActive = false;
function resetTicklerState() {
_ticklerInProgress = false;
_ticklerGuardActive = false;
}
window._ticklerIntegration = {
// keep the same surface area but delegate to local fn
resetHandled: resetTicklerState,
markDialogButton: null
};
initTicklerDialogs();
const _origSave = remoteSave;
const _origPrint = remotePrint;
// ... other originals ...
function wrapWithTicklerCheck(originalFn) {
return function() {
// Nested calls inside proceedCallback bypass tickler
if (_ticklerGuardActive) {
return originalFn.apply(this, arguments);
}
// Block while any tickler dialog flow is active
if (_ticklerInProgress) {
return;
}
const currentMode = getTicklerMode();
if (!currentMode) {
return originalFn.apply(this, arguments);
}
_ticklerInProgress = true;
const args = arguments;
const self = this;
const proceedCallback = function() {
_ticklerGuardActive = true;
try {
originalFn.apply(self, args);
} finally {
// Always reset state after the original action completes
resetTicklerState();
}
};
if (currentMode === "autoOpen") {
promptTicklerAutoOpen(proceedCallback);
} else if (currentMode === "autoSave") {
promptTicklerAutoSave(proceedCallback);
}
};
}
window.remoteSave = wrapWithTicklerCheck(_origSave);
window.remotePrint = wrapWithTicklerCheck(_origPrint);
// ... remaining wrappers ...
}
```
This keeps all existing behavior:
- Dialogs still call `window._ticklerIntegration.markDialogButton()` so `initTicklerDialogs` logic doesn’t need to change.
- Closing a dialog with X/Escape still routes through `resetTicklerState` (via existing `resetHandled`), cancelling the flow.
- Nested toolbar actions invoked during `proceedCallback` still bypass the tickler flow, but that rule is now explicit (`_ticklerGuardActive`) and local to the wrapper, instead of encoded as a `"proceeding"` state shared across modules.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js`:
- Around line 903-915: The success handler currently assumes resp.message is a
valid tickler ID; add a guard that checks resp && resp.success && typeof
resp.message === "string" && resp.message.trim() !== "" before assigning
ticklerId/viewUrl and wiring jQuery("#ticklerViewLink"). If the check fails, do
not set the href or open the success dialog—instead open or set an error/failure
dialog (or reuse jQuery("#ticklerAutoSaveSuccessDialog") with an error message),
avoid calling proceedCallback(), and ensure
window._ticklerIntegration.markDialogButton() still runs where appropriate;
update the code around the success function, ticklerId, viewUrl,
jQuery("#ticklerViewLink"), and jQuery("#ticklerAutoSaveSuccessDialog") to
implement this validation and failure path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d2cbe1ee-001f-4f4a-848c-f0db20da6e1e
📒 Files selected for processing (2)
src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.jssrc/main/webapp/tickler/ticklerAdd.jsp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/webapp/tickler/ticklerAdd.jsp
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
D3V41
approved these changes
Apr 23, 2026
Collaborator
|
This feature looks good after testing on QA. The corresponding MHI PR (MagentaHealth#127) has been merged. This PR will merged after the feature is tested at MHI. |
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
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.
Summary
Adds tickler integration tags to eForms so authors can configure a form to prompt for or silently create a tickler when any toolbar action runs (Save, Print, Download, Fax, Email, Add to Documents). Supports an interactive Auto-Open mode (opens the existing Add Tickler window pre-filled) and an Auto-Save mode (creates the tickler via REST and links to it from a confirmation dialog).
Closes #2314.
User-facing changes
ticklerAutoOpen(true/false) — opens the Add Tickler window pre-filledticklerAutoSave(true/false) — silently creates the tickler via RESTticklerServiceDate,ticklerPriority,ticklerAssignedTo,ticklerMessage,ticklerPromptMessage— optional pre-population/ws/rs/tickler/add→ success dialog with "Click here to view" deep link → original action runs.setAtickler()) are auto-detected and skipped to avoid colliding with the Generator's existing tickler mechanism.Confirm dialog (Auto-Open)
Tickler Window Closed dialog (Auto-Open)
Tickler Created dialog (Auto-Save)
Tickler Error dialog (Auto-Save)
Implementation approach
Toolbar JS — wrapping pattern
eform_floating_toolbar.jswraps the 6remoteXtoolbar functions withwrapWithTicklerCheck. The wrapper uses a closure-scoped tri-state machine (idle/inProgress/proceeding) so the Print → Save chain (where Print internally calls Save) only triggers the dialog once. State reset is wrapped intry/finallyaround the original action call so an unhandled exception cannot leave the toolbar stuck.REST contract — tickler id in response
TicklerWebService.addTicklernow returns the new tickler's id in themessagefield of the standardGenericRESTResponseon success. The auto-save success dialog reads this id to build the "Click here to view" deep link, with a guard for missing/empty values. On failure the JS readsresp.messagefor the actual server error rather than showing a generic string.eForm display JSPs — provider context injection
efmformadd_data.jspandefmshowform_data.jspinjectproviderNo(the logged-in provider) as a hidden input. The toolbar uses it as the defaulttaskAssignedTowhen the eForm author hasn't setticklerAssignedToexplicitly.Tickler add JSP — opener-based message prefill
ticklerAdd.jspreads the prefill message fromwindow.opener._eformTicklerPrefillMessageafterDOMContentLoadedand immediately deletes it (single-use). The toolbar stashes the message onwindowbefore opening the popup; the URL itself never carries it.Dialog UX — scoped overlay and styled close button
The four jQuery UI dialogs (confirm, proceed, success, error) inject a
.tickler-overlay-scoped style so the dim background only applies to tickler dialogs, not every modal app-wide. A manual overlay div bridges the gap between the confirm dialog closing and the popup window closing so the parent page stays visually locked. The titlebar X is hand-rendered as a×text character so it renders consistently after the Bootstrap 3 → 5.3.3 migration that landed mid-development.Privacy and PHI considerations
ticklerMessagemay contain clinical content (the documented use case is "follow up on lab results" and similar). To keep it out of Tomcat access logs and browser history, the toolbar passes the message viawindow.opener._eformTicklerPrefillMessagerather than as a URL query parameter. The popup reads it client-side after load and deletes it. The remaining URL parameters (demographic_no,taskTo,priority,xml_appointment_date) are not PHI in the same sense and stay in the URL.Auto-Save uses POST with the payload in the body, so no PHI ends up in a URL there either.
Testing
Automated
This PR is predominantly client-side (~470 lines of JavaScript orchestration). OpenO has no JS test harness, so a unit/integration suite for the toolbar wrapper would require introducing one — out of scope here. The Java change is a 2-line response field addition; its contract is exercised by the manual auto-save tests below. Recommendation: revisit if a JS test framework lands in the project.
Manual smoke tests performed
resp.message✓ticklerMessage(quotes, ampersands,</>, newlines) round-trip verbatim ✓Design decisions worth noting
ticklerAutoOpenandticklerAutoSavearetrue, auto-open wins (safer; provider reviews before saving).Summary by Sourcery
Integrate tickler creation workflows into eForm toolbar actions, including auto-open and auto-save modes, and wire through supporting data and REST responses.
New Features:
Bug Fixes:
Enhancements:
Summary by cubic
Adds tickler integration to eForms with optional auto-open or auto-save and accessible dialogs. Prompts before Save/Print/Download/Fax/Email/eDocument, links to the new tickler, and skips built‑in generator tickler flows.
New Features
ticklerAdd.jsp) or auto-save (REST) with a “View” link on success; skips integration whensetAtickleris present.providerNofrom session; REST returns the tickler ID only on success;ticklerAdd.jsppre-fills the message via the opener to avoid PHI in URLs.Bug Fixes
updateParent=false,parentAjaxId) and keep a scoped overlay visible while the popup is open.Written for commit 3ae2832. Summary will update on new commits.
Summary by Sourcery
Integrate tickler creation workflows into eForm toolbar actions and backend services, enabling optional auto-open or auto-save ticklers when eForm tags are present.
New Features:
Bug Fixes:
Enhancements:
Summary by CodeRabbit
New Features
Improvements