feat(session): move the SPA session poll off the legacy CFM - #306
feat(session): move the SPA session poll off the legacy CFM#306rlorenzo wants to merge 1 commit into
Conversation
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughSession timeout polling now uses the API session-timeout endpoint. The component retries failed checks, handles session extensions, formats expiration times correctly, and displays extension failures without closing the dialog. ChangesSession timeout flow
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Session timeout polling now uses the API endpoint and provides retry and failure feedback without leaving stale warnings. The supplied change context identifies no remaining merge-blocking risk. Sequence Diagram(s)sequenceDiagram
participant SessionTimeoutVue
participant PollingTimer
participant SessionTimeoutAPI
PollingTimer->>SessionTimeoutVue: trigger session check
SessionTimeoutVue->>SessionTimeoutAPI: fetch session-timeout status
SessionTimeoutAPI-->>SessionTimeoutVue: return status or HTTP error
SessionTimeoutVue->>PollingTimer: schedule next check
SessionTimeoutVue->>SessionTimeoutAPI: request session extension
SessionTimeoutAPI-->>SessionTimeoutVue: return extension result
SessionTimeoutVue->>PollingTimer: reset polling after success
SessionTimeoutVue-->>SessionTimeoutVue: show error banner after failure
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@VueApp/src/components/SessionTimeout.vue`:
- Around line 32-33: The session lifecycle requests in
VueApp/src/components/SessionTimeout.vue at lines 32-33 and 58-59 both require
changes: replace the raw fetch calls in the polling and session-extension flows
with a shared silent useFetch or service-layer method for sessionTimeout and
RefreshSession. Ensure this path suppresses global error and authentication
handling while preserving the existing API response contract.
- Around line 68-70: Update the failed-session-extension state in SessionTimeout
so the open dialog includes an accessible close q-btn wired to
hideSessionTimeoutWarning, while preserving the StatusBanner text. Ensure the
Log in action is visible alongside the existing Retry/Refresh action when
sessionExtendFailed is true, and add Playwright coverage for keyboard navigation
plus both actions.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 41e27369-f9d1-41fd-ad3b-45d95b33e500
📒 Files selected for processing (1)
VueApp/src/components/SessionTimeout.vue
There was a problem hiding this comment.
Pull request overview
Updates the Vue SPA’s session-timeout polling to use the VIPER 2 /api/sessionTimeout endpoint (instead of the legacy ColdFusion seconds_until_timeout_v2.cfm), aligning the SPA with the already-migrated Razor implementation and removing the legacy dependency/security hole.
Changes:
- Replaced the legacy CFM poll URL (with
loginIdin the query string) with a VIPER 2 API poll based onVITE_API_URL. - Improved polling resiliency by rejecting non-OK responses and ensuring failures still reschedule future checks.
- Added in-dialog error UI for failed session extension attempts via
StatusBanner, and centralized the expiry time formatter.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
7c677c8 to
e956bcb
Compare
Bundle ReportChanges will increase total bundle size by 1.57kB (0.07%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: viper-frontend-esmAssets Changed:
Files in
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/session-timeout-endpoint #306 +/- ##
====================================================================
- Coverage 45.07% 45.06% -0.01%
====================================================================
Files 942 942
Lines 49202 49210 +8
Branches 6595 6598 +3
====================================================================
Hits 22177 22177
- Misses 26079 26087 +8
Partials 946 946
Flags with carried forward coverage won't be shown. Click here to find out more.
|
e956bcb to
d5226aa
Compare
6081219 to
281d071
Compare
281d071 to
2f32839
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (3)
VueApp/src/components/SessionTimeout.vue:60
fetch(viperHome + "RefreshSession")assumesVITE_VIPER_HOMEalways ends with a trailing slash. If it is configured as "/2" (no trailing slash), this becomes "/2RefreshSession" and the refresh button will never work on that environment.
fetch(viperHome + "RefreshSession")
.then((r) => (r.ok ? r.json() : Promise.reject(new Error("RefreshSession returned " + r.status))))
VueApp/src/components/SessionTimeout.vue:9
sessionTimeoutUrlis built by direct string concatenation and relies onVITE_API_URLalways ending with a trailing slash. If someone configures it as "/api" (no trailing slash), the fetch will go to "/apisessionTimeout" and silently fail/reschedule forever.
This issue also appears on line 59 of the same file.
const sessionTimeoutUrl = `${import.meta.env.VITE_API_URL}sessionTimeout`
VueApp/src/components/SessionTimeout.vue:53
- The reschedule delay uses the magic number
300000(ms). Using a seconds/minutes expression makes this easier to audit and prevents unit mixups during future edits.
sessionTimeoutCheckEventId = window.setTimeout(checkSessionTimeout, 300000)
2f32839 to
c9cd016
Compare
c9cd016 to
6c4283b
Compare
6c4283b to
253a224
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
🟡 Changes recommended
It introduces browser-compatibility risk via AbortSignal.timeout() without a fallback and adds more direct fetch usage that diverges from the app’s useFetch conventions without a wrapper-level “silent poll” option.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
VueApp/src/components/SessionTimeout.vue:57
- Same compatibility concern here:
AbortSignal.timeout()may be missing in some browsers, which would throw before the request starts. Guarding the timeout creation (or usingAbortController) avoids breaking the Refresh Session flow on unsupported clients.
fetch(viperHome + "RefreshSession", { signal: AbortSignal.timeout(10000) })
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
253a224 to
cd6babd
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The timer/poll scheduling can accumulate multiple concurrent timeouts when a refresh overlaps an in-flight poll, which can cause unnecessary extra polling over time.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
- Point the poll at the VIPER 2 endpoint via VITE_API_URL, dropping the CFM URL that passed the login id as an unauthenticated query parameter - Keep plain fetch rather than useFetch, which reports every failure through the global error store: that would banner a silent five minute poll and fire the auth handler while our dialog offers a log in - Carry over the Razor fixes, since this file had the same defects: reject non-OK responses, report a failed extend in a StatusBanner while offering both Refresh Session and Log in, reschedule after a failed poll rather than stopping for the life of the page, stand the warning down when the session is extended elsewhere, and render midnight as 12 AM rather than 0 AM
cd6babd to
572fe7f
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The change is localized to the SPA session timeout component and the updated polling/URL construction matches the repo’s VITE_API_URL and VITE_VIPER_HOME conventions without introducing verified defects.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
Moves
SessionTimeout.vueonto/api/sessionTimeoutand applies the same fixes as #302 (non-OK rejection, error banner, poll reschedule, stale-warning clear). After this, nothing inweb/orVueApp/references the CFM, soseconds_until_timeout_v2.cfmcan be retired in the legacy repo (seconds_until_timeout.cfmstays; VIPER 1 uses it).Uses plain
fetchrather thanuseFetchon purpose:useFetchpushes every failure to the global error store, which would raise a banner on each blip of a background poll and fire the auth handler while our own dialog offers Log in.No component test; the component is timer-driven and the useful assertions need the real endpoint. End-to-end under the
/2PathBase needs a TEST deploy.