Centralize TierLimitReachErrorModal with shared redux state#10268
Open
sheronjay wants to merge 23 commits into
Open
Centralize TierLimitReachErrorModal with shared redux state#10268sheronjay wants to merge 23 commits into
sheronjay wants to merge 23 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR centralizes tier-limit-reached modal handling: adds Redux types/actions/reducer, wires the reducer into the root store, renders the modal from app root, and refactors many wizards to dispatch showTierLimitReachedModal() and close instead of using local modal state. ChangesUnified Tier-Limit Modal System
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Centralizes the TierLimitReachErrorModal so it’s rendered once at the Console app root and controlled via a dedicated Redux slice, replacing per-flow local modal state and duplicated JSX across multiple “create” wizards/pages.
Changes:
- Added a new Redux slice (
tierLimitModal) with actions + reducer to control tier-limit modal visibility and content. - Updated multiple create flows (roles, groups, apps, connections, orgs, API resources, onboarding) to dispatch
showTierLimitReachedModal(...)on tier-limit errors and then navigate/close the relevant wizard/page. - Rendered the centralized
TierLimitReachErrorModalonce inapps/console/src/app.tsx, wired to Redux state and a close action.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| features/admin.roles.v2/pages/create-role-wizard.tsx | Dispatch centralized tier-limit modal and redirect instead of local modal state/rendering. |
| features/admin.roles.v2/components/wizard-updated/application-role-wizard.tsx | Dispatch centralized tier-limit modal and close wizard instead of local modal state/rendering. |
| features/admin.organizations.v1/components/add-organization-modal.tsx | Replace local tier-limit modal handling with centralized modal dispatch (incl. org-level variant). |
| features/admin.onboarding.v1/components/onboarding-wizard.tsx | Dispatch centralized tier-limit modal and redirect instead of rendering local modal. |
| features/admin.groups.v1/components/wizard/create-group-wizard.tsx | Dispatch centralized tier-limit modal and close wizard instead of local modal state/rendering. |
| features/admin.core.v1/store/reducers/tier-limit-modal.ts | Introduce reducer for centralized tier-limit modal state. |
| features/admin.core.v1/store/combine-reducers.ts | Register tierLimitModal reducer in root reducer. |
| features/admin.core.v1/store/actions/types/tier-limit-modal.ts | Define action types and TS interfaces for tier-limit modal actions. |
| features/admin.core.v1/store/actions/tier-limit-modal.ts | Add showTierLimitReachedModal/hideTierLimitReachedModal action creators. |
| features/admin.core.v1/models/reducer-state.ts | Add TierLimitModalReducerStateInterface to shared reducer-state models. |
| features/admin.connections.v1/components/wizards/trusted-token-issuer-create-wizard.tsx | Dispatch centralized tier-limit modal and close wizard instead of local modal state/rendering. |
| features/admin.connections.v1/components/wizards/organization-enterprise/organization-enterprise-connection-create-wizard.tsx | Dispatch centralized tier-limit modal and close wizard instead of local modal state/rendering. |
| features/admin.connections.v1/components/wizards/expert-mode/expert-mode-authentication-provider-create-wizard.tsx | Dispatch centralized tier-limit modal and close wizard instead of local modal state/rendering. |
| features/admin.connections.v1/components/create/add-connection-wizard.tsx | Dispatch centralized tier-limit modal and close wizard instead of local modal state/rendering. |
| features/admin.applications.v1/components/wizard/minimal-application-create-wizard.tsx | Dispatch centralized tier-limit modal and close wizard instead of local modal state/rendering. |
| features/admin.applications.v1/components/try-it/try-it-create-wizard.tsx | Dispatch centralized tier-limit modal and close wizard instead of local modal state/rendering. |
| features/admin.application-templates.v1/components/application-create-wizard.tsx | Dispatch centralized tier-limit modal and close wizard instead of returning local modal. |
| features/admin.api-resources.v2/components/wizard/add-api-resource.tsx | Dispatch centralized tier-limit modal and close wizard instead of local modal state/rendering. |
| apps/console/src/app.tsx | Render centralized TierLimitReachErrorModal once and wire it to Redux state + hide action. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 6
🧹 Nitpick comments (1)
features/admin.core.v1/store/reducers/tier-limit-modal.ts (1)
47-50: ⚡ Quick winReturn state directly instead of spreading.
The default case creates a new object reference by spreading state, which causes unnecessary re-renders when unrelated actions are dispatched. Return the existing state reference to maintain Redux best practices.
⚡ Proposed fix
default: - return { - ...state - }; + return state;🤖 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 `@features/admin.core.v1/store/reducers/tier-limit-modal.ts` around lines 47 - 50, In the reducer handling actions for the tier limit modal, the switch default currently returns a new object via "return { ...state }" which creates a new reference and triggers unnecessary re-renders; change the default branch to return the existing state reference ("return state") so the reducer preserves identity for unrelated actions (update the switch default in the tier-limit-modal reducer accordingly).
🤖 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 `@apps/console/src/app.tsx`:
- Around line 199-200: The variable tierLimitReachedModal is missing an explicit
type annotation; update its declaration to explicitly type it as
TierLimitModalReducerStateInterface | undefined (since the selector uses
optional chaining) by annotating the const (referencing tierLimitReachedModal,
TierLimitModalReducerStateInterface, useSelector and AppState) so the result of
useSelector((state: AppState) => state?.tierLimitModal) is explicitly typed
rather than inferred.
- Around line 673-680: The TierLimitReachErrorModal usage is missing the
required data-componentid attribute; update the JSX for TierLimitReachErrorModal
to include a data-componentid prop (e.g.,
data-componentid={tierLimitReachedModal?.componentId ??
"tier-limit-reach-error-modal"}) so it implements
IdentifiableComponentInterface; ensure the attribute is passed alongside
actionLabel, handleModalClose, header, description, message and openModal props.
- Around line 223-225: The handler handleTierLimitReachedModalClose should have
an explicit function type and be stabilized with React.useCallback: import
useCallback, change the declaration to a const with a full signature (e.g. const
handleTierLimitReachedModalClose: () => void) and assign it using useCallback(()
=> { dispatch(hideTierLimitReachedModal()); }, [dispatch]); ensuring
hideTierLimitReachedModal and dispatch are in the dependency array as
appropriate.
In
`@features/admin.connections.v1/components/wizards/expert-mode/expert-mode-authentication-provider-create-wizard.tsx`:
- Around line 171-173: In ExpertModeAuthenticationProviderCreateWizard, guard
access to error.response before reading status: change the condition that
currently uses error.response.status to use optional chaining or an explicit
existence check (e.g., error?.response?.status or error?.response &&
error.response.status) so the clause becomes something like checking
error?.response?.status === 403 && error?.response?.data?.code ===
identityAppsError.getErrorCode(); update any similar checks in the same
conditional to consistently use optional chaining on error.response and
error.response.data to avoid TypeError on network failures.
In
`@features/admin.connections.v1/components/wizards/organization-enterprise/organization-enterprise-connection-create-wizard.tsx`:
- Around line 168-170: The check that reads if (error.response.status === 403 &&
error?.response?.data?.code === identityAppsError.getErrorCode()) can throw when
error.response is undefined; update the condition to use optional chaining for
status (and for data.code for safety) so it becomes if (error?.response?.status
=== 403 && error?.response?.data?.code === identityAppsError.getErrorCode()) in
the organization-enterprise-connection-create-wizard (the if that references
error.response.status and identityAppsError.getErrorCode()) to ensure
network/timeouts won't cause an exception and the tier-limit handling still
runs.
In `@features/admin.onboarding.v1/components/onboarding-wizard.tsx`:
- Around line 466-473: The modal is using hardcoded English strings in the
dispatch(showTierLimitReachedModal(...)) call; replace these literals with
translations resolved via react-i18next: import and call useTranslation() in the
component, then replace actionLabel, description, header and message with
t('yourNamespace:path.to.actionLabel'), t('yourNamespace:path.to.description'),
etc. Ensure keys follow the "<namespace>:<path.to.key>" format and pass the
translated values into showTierLimitReachedModal so the modal uses t(...)
results instead of raw strings.
---
Nitpick comments:
In `@features/admin.core.v1/store/reducers/tier-limit-modal.ts`:
- Around line 47-50: In the reducer handling actions for the tier limit modal,
the switch default currently returns a new object via "return { ...state }"
which creates a new reference and triggers unnecessary re-renders; change the
default branch to return the existing state reference ("return state") so the
reducer preserves identity for unrelated actions (update the switch default in
the tier-limit-modal reducer accordingly).
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a7a02650-749e-454a-8cdc-1a72e16363cc
📒 Files selected for processing (19)
apps/console/src/app.tsxfeatures/admin.api-resources.v2/components/wizard/add-api-resource.tsxfeatures/admin.application-templates.v1/components/application-create-wizard.tsxfeatures/admin.applications.v1/components/try-it/try-it-create-wizard.tsxfeatures/admin.applications.v1/components/wizard/minimal-application-create-wizard.tsxfeatures/admin.connections.v1/components/create/add-connection-wizard.tsxfeatures/admin.connections.v1/components/wizards/expert-mode/expert-mode-authentication-provider-create-wizard.tsxfeatures/admin.connections.v1/components/wizards/organization-enterprise/organization-enterprise-connection-create-wizard.tsxfeatures/admin.connections.v1/components/wizards/trusted-token-issuer-create-wizard.tsxfeatures/admin.core.v1/models/reducer-state.tsfeatures/admin.core.v1/store/actions/tier-limit-modal.tsfeatures/admin.core.v1/store/actions/types/tier-limit-modal.tsfeatures/admin.core.v1/store/combine-reducers.tsfeatures/admin.core.v1/store/reducers/tier-limit-modal.tsfeatures/admin.groups.v1/components/wizard/create-group-wizard.tsxfeatures/admin.onboarding.v1/components/onboarding-wizard.tsxfeatures/admin.organizations.v1/components/add-organization-modal.tsxfeatures/admin.roles.v2/components/wizard-updated/application-role-wizard.tsxfeatures/admin.roles.v2/pages/create-role-wizard.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10268 +/- ##
=======================================
Coverage 55.70% 55.70%
=======================================
Files 42 42
Lines 1016 1016
Branches 246 231 -15
=======================================
Hits 566 566
- Misses 416 450 +34
+ Partials 34 0 -34
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Member
|
Hi @sheronjay, apologies for the delay in reviewing. I’ve been a bit busy over the past few weeks for WSO2Con. I’ll review over the weekend and let you know if there are any improvements needed. |
Contributor
Author
|
@pavinduLakshan no worries, take your time. I’m in the middle of exams too. If you’d like me to squash or clean up any extra commits just let me know. ;) |
sheronjay
force-pushed
the
tier-limit-modal
branch
from
36b1ffc to
f07ecbf
Compare
Contributor
Author
|
@pavinduLakshan I've addressed the review comments. there are a couple of places where the i18n strings exceed 120 characters so I left them as it is. also |
Member
pavinduLakshan
left a comment
pavinduLakshan
left a comment
There was a problem hiding this comment.
Let's add the changset by running pnpm changeset from the project root
Member
Thanks, I will take a look.
will check and get back. |
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.
Purpose
Centralize
TierLimitReachErrorModalso it is rendered once and controlled through redux instead of being managed independently in each page/component.This removes duplicated local modal state, close handlers, and repeated modal JSX across the affected create flows. Resource specific modal content is still preserved by storing the modal visibility and display content in Redux and dispatching it from each feature’s tier limit error path.
A dedicated Redux slice was used for this instead of extending the existing
globalreduce.Related Issues
wso2/product-is#27714
Related PRs
Checklist
Security checks
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.