Skip to content

use useProfile instead of user#185

Open
ghazwarhili wants to merge 9 commits into
mainfrom
cleanup-apache-config-requests
Open

use useProfile instead of user#185
ghazwarhili wants to merge 9 commits into
mainfrom
cleanup-apache-config-requests

Conversation

@ghazwarhili

@ghazwarhili ghazwarhili commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

PR Summary

Context

On every OIDC silent renew, oidc-client-ts loads /silent-renew-callback inside a
hidden iframe. Because index.tsx always rendered <AppWrapper />, the entire
application was bootstrapped inside that iframe just to end up calling
signinSilentCallback(): Redux store, IntlProvider and all translations,
AG Grid registration, MUI theme, SnackbarProvider and NotificationsProvider.
NotificationsProvider additionally fetched env.json and opened a notification
WebSocket on each renewal — all of it pure overhead repeated at every token refresh.

Changes

  • index.tsx: branch on the pathname before rendering. On /silent-renew-callback
    render a new lightweight SilentRenewApp; otherwise render <AppWrapper /> as before.
  • New components/silent-renew-app.tsx: rebuilds the UserManager via
    initializeAuthenticationProd(..., isSilentRenew=true, ...) (same settings, same
    localStorage state store, same Azure authority hack as the parent) and completes
    the flow through commons-ui's SilentRenewCallbackHandler.
  • utils/rest-api.ts: fetchIdpSettings now caches the result in localStorage;
    added getCachedIdpSettings, used only by the silent-renew path, which reads that
    cache (no network) and falls back to a real fetch if it is missing/corrupted.

Result

Inside the silent-renew iframe, requests go from a full SPA boot
(idpSettings.json + env.json + WebSocket + app bundle execution) down to a
single POST /token — the actual token exchange. No more spurious WebSocket
reconnections triggered by the renewal iframe.

Testing

  • Open the app, let a silent renew happen (or wait for token expiry).
  • In the Network tab, confirm the iframe no longer fetches env.json, no longer
    re-fetches idpSettings.json, and does not open a WebSocket — only POST /token remains.
  • Confirm the user stays logged in across renewals and that the parent app's
    notification WebSocket is not reconnected on each cycle.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@ghazwarhili, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 59 minutes and 11 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: aead2250-6a2e-4b70-8435-129f015461fa

📥 Commits

Reviewing files that changed from the base of the PR and between dfb5e49 and b935417.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • package.json
  • src/index.tsx
📝 Walkthrough

Walkthrough

The PR updates AppTopBar selector equality, adds a silent-renew authentication component, and changes app startup to route silent-renew callbacks through a dedicated entry path with cached IdP settings support.

Changes

User Profile Selector Equality

Layer / File(s) Summary
Custom user profile selector equality
src/components/App/AppTopBar.tsx
userProfile now uses an explicit equality comparator over sub, name, email, and profile, and the react-redux import no longer includes shallowEqual.

Silent Renew Startup Flow

Layer / File(s) Summary
IdP settings cache helpers
src/services/apps-metadata.ts
fetchIdpSettings now writes fetched settings to localStorage, and getCachedIdpSettings reads cached settings or falls back to fetching; the silent-renew callback path constant is also added.
SilentRenew component
silent-renew.tsx
SilentRenew initializes silent-renew authentication on mount, loads cached IdP settings, stores the UserManager in state, and forwards it to SilentRenewCallbackHandler.
Entrypoint routing and module declarations
src/index.tsx, src/module-core-js-array-flat-map.d.ts, src/module-typeface-roboto.d.ts
index.tsx now renders SilentRenew on the callback path and otherwise performs async startup imports before rendering AppWrapper; new module declarations support the added dynamic imports.

Suggested reviewers

  • flomillot
  • sBouzols
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title refers to a different refactor and does not describe the silent-renew and caching changes in this PR. Rename it to mention the OIDC silent-renew optimization and IdP settings caching.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description matches the PR’s silent-renew iframe optimization and IdP settings caching changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/services/apps-metadata.ts (1)

35-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Cached IdP settings can serve stale config until a full app reload.

getCachedIdpSettings returns the cached value without any freshness/version check, and the cache is only refreshed by fetchIdpSettings on a full app load. If idpSettings.json changes (e.g. authority/clientId rotation) while a long-lived session keeps performing silent renews without a full reload, the iframe will keep using the old settings. This matches the PR's stated design, but consider a TTL or version stamp if config can change mid-session.

🤖 Prompt for 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.

In `@src/services/apps-metadata.ts` around lines 35 - 45, `getCachedIdpSettings`
currently returns whatever is in localStorage with no freshness check, so
long-lived sessions can keep using stale IdP config. Update the caching flow in
`getCachedIdpSettings`/`fetchIdpSettings` to validate cache freshness before
reusing it, such as by storing and checking a TTL or version stamp alongside
`IDP_SETTINGS_CACHE_KEY`. If the cached metadata is missing, expired, or
version-mismatched, fall back to `fetchIdpSettings` and refresh the cache.
🤖 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.

Nitpick comments:
In `@src/services/apps-metadata.ts`:
- Around line 35-45: `getCachedIdpSettings` currently returns whatever is in
localStorage with no freshness check, so long-lived sessions can keep using
stale IdP config. Update the caching flow in
`getCachedIdpSettings`/`fetchIdpSettings` to validate cache freshness before
reusing it, such as by storing and checking a TTL or version stamp alongside
`IDP_SETTINGS_CACHE_KEY`. If the cached metadata is missing, expired, or
version-mismatched, fall back to `fetchIdpSettings` and refresh the cache.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 72b88294-7232-4e50-8b6b-019a81796e78

📥 Commits

Reviewing files that changed from the base of the PR and between 70fcbf9 and dfb5e49.

📒 Files selected for processing (5)
  • silent-renew.tsx
  • src/index.tsx
  • src/module-core-js-array-flat-map.d.ts
  • src/module-typeface-roboto.d.ts
  • src/services/apps-metadata.ts
✅ Files skipped from review due to trivial changes (2)
  • src/module-core-js-array-flat-map.d.ts
  • src/module-typeface-roboto.d.ts

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant