feat(session): move the SPA session poll off the legacy CFM - #306
Open
rlorenzo wants to merge 1 commit into
Open
feat(session): move the SPA session poll off the legacy CFM#306rlorenzo wants to merge 1 commit into
rlorenzo wants to merge 1 commit into
Conversation
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
📝 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
Estimated code review effort: 3 (Moderate) | ~20 minutes 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🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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.
rlorenzo
force-pushed
the
feature/session-timeout-spa
branch
from
August 8, 2026 02:34
7c677c8 to
e956bcb
Compare
Bundle ReportChanges will increase total bundle size by 1.46kB (0.06%) ⬆️. 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 41.85% 41.84% -0.01%
====================================================================
Files 987 987
Lines 49518 49526 +8
Branches 5843 5847 +4
====================================================================
Hits 20726 20726
- Misses 27882 27890 +8
Partials 910 910
Flags with carried forward coverage won't be shown. Click here to find out more.
|
rlorenzo
force-pushed
the
feature/session-timeout-spa
branch
from
August 8, 2026 06:10
e956bcb to
d5226aa
Compare
rlorenzo
force-pushed
the
feature/session-timeout-spa
branch
2 times, most recently
from
August 8, 2026 18:08
6081219 to
281d071
Compare
rlorenzo
force-pushed
the
feature/session-timeout-spa
branch
from
August 10, 2026 03:27
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)
- 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
rlorenzo
force-pushed
the
feature/session-timeout-spa
branch
from
August 10, 2026 05:56
2f32839 to
c9cd016
Compare
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.
Migrates the Vue SPA's session timeout component off the legacy ColdFusion endpoint and onto the VIPER 2 endpoint added in #304.
Why this one matters
#304 only moved the Razor side.
SessionTimeout.vueis a parallel implementation of the same component and was still callingpublic/timeout/seconds_until_timeout_v2.cfm?id=<loginId>, the exact unauthenticated endpoint #304 exists to replace. So until this merges:With this merged, nothing in
web/orVueApp/src/references the CFM. Retiringseconds_until_timeout_v2.cfmbecomes a separate, coordinated change in the legacy repo. Note thatseconds_until_timeout.cfm(v1) must stay, since VIPER 1 uses it for its own timeout.Deliberate deviation: plain
fetch, notuseFetch()CLAUDE.mdsays use the service layer plususeFetch(). I am not doing that here, and it is worth a reviewer's attention.It is not the envelope.
fetchWrapperonly unwraps whenr.success !== undefined(ViperFetch.ts:137-141), so the envelope-less payload from the new endpoint passes through fine.useFetch()would work.The reason is error handling.
useFetch()routes every failure througherrorHandler.handleError, which writes to the global error store (ErrorHandler.ts:13). For this component that is wrong twice over:handleAuthError, which would kick in at exactly the moment our own dialog is trying to tell the user their session expired and offer them a Log in button. The two would fight.The file already used plain
fetch, so this is not a regression, but it is a conscious exception rather than an oversight. Happy to revisit if you would rather add a non-reporting variant to the service layer.Also carried over
This file had the same defects #302 fixed on the Razor side, so the same fixes apply:
StatusBanner type="error"inside the dialog, and leaves the dialog up so the user can retry.StatusBanneris the SPA equivalent of the Razorq-bannerperDESIGN.md, andtype="error"is assertive (role="alert") by default.try/catch, which could not catch an async rejection, is replaced with a real.catchthat reschedules. Previously a rejected poll scheduled no follow-up, so session checking stopped for the life of the page.extendSession's target is unchanged on purpose. It usesVITE_VIPER_HOME, which is already/2/in.env.testand.env.production, so it was never affected by the PathBase bug in #302.Verification
npm run lint -- --fix VueApp/src/components/SessionTimeout.vuenpm run verify:buildnpm run test:frontendnpm run test:backendNot verified locally. There is no existing test for this component and I did not add one; it is DOM-and-timer heavy and the valuable assertions need the real endpoint. The end-to-end path (SPA polling
/2/api/sessionTimeoutunder the real PathBase, against a real database) needs a TEST deploy. Local dev has no PathBase, so the/2prefix cannot be exercised here.