Feature: eForm auto add tickler window popup#2320
Draft
LiamStanziani wants to merge 9 commits intohotfix/01232026from
Draft
Feature: eForm auto add tickler window popup#2320LiamStanziani wants to merge 9 commits intohotfix/01232026from
LiamStanziani wants to merge 9 commits intohotfix/01232026from
Conversation
Reviewer's GuideAdds client-side tickler integration to the eForm floating toolbar, enabling optional popup-based or REST-based tickler creation around toolbar actions, wires it to backend tickler services, and supports pre-filling tickler messages and returning tickler IDs for linking. Sequence diagram for eForm toolbar action with tickler autoOpen modesequenceDiagram
actor User
participant Browser_eForm as Browser_eForm
participant eform_floating_toolbar_js as eform_floating_toolbar_js
participant ticklerAdd_jsp as ticklerAdd_jsp
User->>Browser_eForm: Click toolbar action (Save/Print/Download/Fax/Email/eDocument)
Browser_eForm->>eform_floating_toolbar_js: Call wrapped remoteAction
eform_floating_toolbar_js->>eform_floating_toolbar_js: hasTicklerTags() returns autoOpen
eform_floating_toolbar_js->>eform_floating_toolbar_js: _ticklerHandled = true
eform_floating_toolbar_js->>eform_floating_toolbar_js: promptTicklerAutoOpen(proceedCallback)
eform_floating_toolbar_js->>Browser_eForm: Open ticklerConfirmDialog
alt User chooses Yes
User-->>Browser_eForm: Click Yes
Browser_eForm->>eform_floating_toolbar_js: Confirm dialog Yes handler
eform_floating_toolbar_js->>eform_floating_toolbar_js: collectTicklerData()
eform_floating_toolbar_js->>ticklerAdd_jsp: window.open(ticklerAdd.jsp with query params)
loop Poll popup
eform_floating_toolbar_js->>ticklerAdd_jsp: Check popup.closed
end
eform_floating_toolbar_js->>Browser_eForm: Show ticklerProceedDialog
alt User clicks Yes, proceed
User-->>Browser_eForm: Yes, proceed
Browser_eForm->>eform_floating_toolbar_js: Proceed dialog Yes handler
eform_floating_toolbar_js->>Browser_eForm: Call original toolbar action
else User clicks No, cancel
User-->>Browser_eForm: No, cancel
Browser_eForm->>eform_floating_toolbar_js: Proceed dialog Cancel handler
eform_floating_toolbar_js->>eform_floating_toolbar_js: resetTicklerHandled()
end
else User chooses No
User-->>Browser_eForm: Click No
Browser_eForm->>eform_floating_toolbar_js: Confirm dialog No handler
eform_floating_toolbar_js->>Browser_eForm: Call original toolbar action
eform_floating_toolbar_js->>eform_floating_toolbar_js: resetTicklerHandled()
end
Sequence diagram for eForm toolbar action with tickler autoSave modesequenceDiagram
actor User
participant Browser_eForm as Browser_eForm
participant eform_floating_toolbar_js as eform_floating_toolbar_js
participant TicklerREST as TicklerREST_ws_rs_tickler_add
participant TicklerWebService as TicklerWebService
participant TicklerManager as TicklerManager
User->>Browser_eForm: Click toolbar action (Save/Print/Download/Fax/Email/eDocument)
Browser_eForm->>eform_floating_toolbar_js: Call wrapped remoteAction
eform_floating_toolbar_js->>eform_floating_toolbar_js: hasTicklerTags() returns autoSave
eform_floating_toolbar_js->>eform_floating_toolbar_js: _ticklerHandled = true
eform_floating_toolbar_js->>eform_floating_toolbar_js: promptTicklerAutoSave(proceedCallback)
eform_floating_toolbar_js->>eform_floating_toolbar_js: collectTicklerData()
eform_floating_toolbar_js->>TicklerREST: POST /ws/rs/tickler/add (JSON payload)
TicklerREST->>TicklerWebService: addTickler(Tickler)
TicklerWebService->>TicklerManager: addTickler(LoggedInInfo, Tickler)
TicklerManager-->>TicklerWebService: success flag
TicklerWebService->>TicklerWebService: set tickler.id
TicklerWebService->>TicklerWebService: response.setSuccess(...)
TicklerWebService->>TicklerWebService: response.setMessage(String.valueOf(tickler.id))
TicklerWebService-->>TicklerREST: GenericRESTResponse(success, message=ticklerId)
alt REST call success
TicklerREST-->>eform_floating_toolbar_js: JSON with success true and ticklerId
eform_floating_toolbar_js->>eform_floating_toolbar_js: Build ticklerEdit.jsp viewUrl
eform_floating_toolbar_js->>Browser_eForm: Open ticklerAutoSaveSuccessDialog with View link
User-->>Browser_eForm: Click OK
Browser_eForm->>eform_floating_toolbar_js: Success dialog OK handler
eform_floating_toolbar_js->>Browser_eForm: Call original toolbar action
else REST call error or unsuccessful response
TicklerREST-->>eform_floating_toolbar_js: HTTP error or success false
eform_floating_toolbar_js->>Browser_eForm: Open ticklerAutoSaveErrorDialog(detail)
alt User chooses OK (proceed anyway)
User-->>Browser_eForm: OK (proceed anyway)
Browser_eForm->>eform_floating_toolbar_js: Error dialog OK handler
eform_floating_toolbar_js->>Browser_eForm: Call original toolbar action
else User chooses Cancel
User-->>Browser_eForm: Cancel
Browser_eForm->>eform_floating_toolbar_js: Error dialog Cancel handler
eform_floating_toolbar_js->>eform_floating_toolbar_js: resetTicklerHandled()
end
end
Class diagram for updated TicklerWebService addTickler behaviorclassDiagram
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 Tickler {
-Long id
-Integer demographicNo
-String message
-String taskAssignedTo
-String priority
-LocalDateTime serviceDate
-String status
+Long getId()
+void setId(Long id)
+Integer getDemographicNo()
+void setDemographicNo(Integer demographicNo)
+String getMessage()
+void setMessage(String message)
+String getTaskAssignedTo()
+void setTaskAssignedTo(String taskAssignedTo)
+String getPriority()
+void setPriority(String priority)
+LocalDateTime getServiceDate()
+void setServiceDate(LocalDateTime serviceDate)
+String getStatus()
+void setStatus(String status)
}
class TicklerManager {
+boolean addTickler(LoggedInInfo loggedInInfo, Tickler tickler)
}
TicklerWebService --> TicklerManager : uses
TicklerWebService --> Tickler : parameter
TicklerWebService --> GenericRESTResponse : returns
File-Level Changes
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 |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
📝 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.
src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js
Outdated
Show resolved
Hide resolved
src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js
Outdated
Show resolved
Hide resolved
![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
src/main/java/ca/openosp/openo/webserv/rest/TicklerWebService.java
Outdated
Show resolved
Hide resolved
![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.
src/main/java/ca/openosp/openo/webserv/rest/TicklerWebService.java
Outdated
Show resolved
Hide resolved
src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js
Outdated
Show resolved
Hide resolved
src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js
Outdated
Show resolved
Hide resolved
src/main/webapp/eform/eformFloatingToolbar/eform_floating_toolbar.js
Outdated
Show resolved
Hide resolved
- 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
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.
NOTE: These changes are a work in progress and will be updated overtime, manual review of the design and testing will need to be looked at for this feature.
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, supporting dynamic mode detection and accessible dialogs. Prompts before Save/Print/Download/Fax/Email/eDocument and links to the created tickler.
New Features
Bug Fixes
Written for commit 3aad298. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Improvements