Skip to content

use useProfile instead of user - #889

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

use useProfile instead of user#889
ghazwarhili wants to merge 11 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 40 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: 26de925d-25be-47e4-83c4-86230300af18

📥 Commits

Reviewing files that changed from the base of the PR and between de04fbd and 4a3387f.

⛔ 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

Adds OIDC silent-renew support by caching IdP settings in localStorage, introducing a SilentRenew component for the callback flow, and updating the entry point to route to it based on the URL path. Refactors Redux user selectors in TreeViewsContainer and DirectoryContentDialog from user to userProfile with custom equality comparators.

Changes

OIDC Silent Renew Feature

Layer / File(s) Summary
IdP settings localStorage caching
src/utils/rest-api.ts
Adds IdpSettings type import, IDP_SETTINGS_CACHE_KEY and SILENT_RENEW_CALLBACK_PATH constants, updates fetchIdpSettings to cache parsed results in localStorage, and adds getCachedIdpSettings that reads from cache with fallback to a fresh network fetch.
SilentRenew component and entry-point routing
src/components/silent-renew.tsx, src/index.tsx, src/module-core-js-array-flat-map.d.ts, src/module-typeface-roboto.d.ts
Adds SilentRenew component that initializes OIDC silent-renew auth via getCachedIdpSettings, stores the resolved UserManager, and renders SilentRenewCallbackHandler. Converts index.tsx to an async renderApp() that routes to SilentRenew when the path ends with SILENT_RENEW_CALLBACK_PATH, or dynamically imports polyfills, font, CSS, and AppWrapper otherwise. Adds ambient TypeScript declarations for the dynamically imported modules.

userProfile Selector Refactor

Layer / File(s) Summary
userProfile selector and prop updates
src/components/tree-views-container.tsx, src/components/directory-content-dialog.tsx
Replaces the user Redux selector with a userProfile selector (state.user?.profile ?? null) using a custom equality comparator in both components. Updates the rootDirectories effect guard to require non-null userProfile, and replaces user={user} with userProfile={userProfile} across all *ParametersEditionDialog render branches.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title matches one part of the changeset by describing the user to userProfile selector migration.
Description check ✅ Passed The description accurately explains the silent-renew flow and caching changes introduced in this pull request.
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.

Actionable comments posted: 3

🤖 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 `@src/components/directory-content-dialog.tsx`:
- Line 383: The ParametersEditionDialogProps interface expects a prop named
`user` of type `User | null`, but the code is passing an incorrectly named
`userProfile` prop with potentially incorrect values. In
src/components/directory-content-dialog.tsx at lines 383, 399, 414, 429, 444,
459, and 475, change the prop name from `userProfile={userProfile}` to
`user={userProfile}` at each location. Ensure that the value being passed is the
complete User object (verify that `userProfile` variable refers to the full User
object and not just the profile property extracted from state.user) so that the
ParametersEditionDialog components receive the correct prop name and complete
User data structure they expect.

In `@src/index.tsx`:
- Around line 20-24: The pathname comparison in the if condition checking for
the silent-renew callback route fails when the URL has a trailing slash (e.g.,
/silent-renew-callback/), causing the full app to render instead of
SilentRenewApp. Normalize the pathname by removing any trailing slashes before
comparing it with the expected route string. Modify the condition that checks
window.location.pathname.endsWith('/silent-renew-callback') to first normalize
the pathname using a method like replaceAll or a regex pattern to remove
trailing slashes, ensuring the comparison works consistently regardless of
whether the URL includes a trailing slash.

In `@src/utils/rest-api.ts`:
- Around line 101-120: The fetchIdpSettings and getCachedIdpSettings functions
have two vulnerabilities: (1) the fetch response is not validated for success
status before caching, allowing error responses to be poisoned into cache, and
(2) localStorage access is unguarded and can throw exceptions, crashing the flow
before fallback logic executes. In fetchIdpSettings, add a check for res.ok
before calling .json() on the response, and wrap the localStorage.setItem call
in a try-catch block to prevent storage exceptions from aborting the flow. In
getCachedIdpSettings, wrap the localStorage.getItem call in a try-catch block so
that storage access errors gracefully fall through to the fetchIdpSettings
fallback rather than crashing the silent-renew 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 25d8d0d5-8586-41f4-b6a8-86760f3dc380

📥 Commits

Reviewing files that changed from the base of the PR and between ab13f5b and 86e824b.

📒 Files selected for processing (6)
  • src/components/app.tsx
  • src/components/directory-content-dialog.tsx
  • src/components/silent-renew-app.tsx
  • src/components/tree-views-container.tsx
  • src/index.tsx
  • src/utils/rest-api.ts

Comment thread src/components/directory-content-dialog.tsx
Comment thread src/index.tsx Outdated
Comment thread src/utils/rest-api.ts

@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.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/index.tsx (1)

19-24: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Optional: parallelize independent dynamic imports.

The font, CSS, and yup-init imports are independent and could load concurrently. Keep the core-js polyfill first if app code depends on it; the rest can be awaited together via Promise.all to shave startup latency on the main app path.

♻️ Suggested change
     await import('core-js/es/array/flat-map');
-    await import('typeface-roboto');
-    await import('./index.css');
-    await import('./configure-yup-init');
-    const { default: AppWrapper } = await import('./components/app-wrapper');
+    await Promise.all([
+        import('typeface-roboto'),
+        import('./index.css'),
+        import('./configure-yup-init'),
+    ]);
+    const { default: AppWrapper } = await import('./components/app-wrapper');
     root.render(<AppWrapper />);
🤖 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 `@src/index.tsx`:
- Around line 15-24: The silent-renew entry path in src/index.tsx currently
returns before loading the Array.prototype.flatMap polyfill, so add the
core-js/es/array/flat-map import to the silent-renew branch as well, not just
the AppWrapper path. Update the startup flow around
window.location.pathname.endsWith(SILENT_RENEW_CALLBACK_PATH) and SilentRenew so
both routes load the same required polyfill before rendering.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a1c4997f-a5c1-49a7-aa0e-73406cba78d9

📥 Commits

Reviewing files that changed from the base of the PR and between 86e824b and de04fbd.

📒 Files selected for processing (7)
  • src/components/directory-content-dialog.tsx
  • src/components/silent-renew.tsx
  • src/components/tree-views-container.tsx
  • src/index.tsx
  • src/module-core-js-array-flat-map.d.ts
  • src/module-typeface-roboto.d.ts
  • src/utils/rest-api.ts
✅ Files skipped from review due to trivial changes (2)
  • src/module-typeface-roboto.d.ts
  • src/module-core-js-array-flat-map.d.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/components/tree-views-container.tsx
  • src/components/directory-content-dialog.tsx
  • src/utils/rest-api.ts

Comment thread src/index.tsx Outdated
@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