Add support for handling groups tier limit reached#10215
Add support for handling groups tier limit reached#10215Lakshan-Banneheke merged 2 commits intowso2:masterfrom
Conversation
📝 WalkthroughWalkthroughA new changesets entry coordinates synchronized patch releases. The create-group wizard now intercepts 403 responses with tier limit errors, displays a modal with localized text, and prevents wizard closure. Supporting error constants and i18n translations are added. Changes
🚥 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)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms 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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10215 +/- ##
=======================================
Coverage 56.05% 56.05%
=======================================
Files 42 42
Lines 1024 1024
Branches 254 254
=======================================
Hits 574 574
Misses 416 416
Partials 34 34
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@features/admin.groups.v1/components/wizard/create-group-wizard.tsx`:
- Around line 603-611: The TierLimitReachErrorModal invocation passes
actionLabel and header props, but the component only destructures { description,
message, openModal, handleModalClose } and ignores those two props; remove the
actionLabel and header assignments from the TierLimitReachErrorModal JSX in
create-group-wizard.tsx (i.e., drop the actionLabel={...} and header={...}
props) so the callsite matches the component's actual API and avoid passing
unused props.
🪄 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: df4bd7c8-86db-45b4-9f56-061d1411da59
📒 Files selected for processing (5)
.changeset/witty-teeth-vanish.mdfeatures/admin.groups.v1/components/wizard/create-group-wizard.tsxfeatures/admin.groups.v1/constants/group-constants.tsmodules/i18n/src/models/namespaces/groups-ns.tsmodules/i18n/src/translations/en-US/portals/groups.ts
There was a problem hiding this comment.
Since the tierLimitReached modal is going to be soon used across multiple UI components with the introduction of the trial account concept, implementing this separately in each UI page will lead to duplication and make maintenance harder.
A better approach would be to centralize the modal at a higher level, such as App.tsx or another appropriate root component, and control its visibility through a global state mechanism like Redux or React Context. With this in place, triggering the modal becomes a simple state update or dispatch call, for example directly from a catch block, without needing to replicate modal logic across components.
PS: An AI tool might be helpful in achieving this ;)
Thanks for the comment Pavindu. I agree that centralizing this would be a better approach. However, currently we already have this mechanism in 10 pages and different components get different error codes depending on the resource (This in itself is a problem which we need to improve and unify to a single error code :) ). Due to that, I think we will need to atleast retain the "if" condition logic in each component even if we move the other logic to a centralized location at present. Additionally, with the new effort we will only be introducing this to 3 new pages, so there will not be a significant increase in the usage of the However, I completely agree that the approach you are suggesting is a better approach. Shall we create an issue for this and track as an improvement? |
@coderabbitai create an issue to track this. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll create a GitHub issue to track the centralization of 🏁 Script executed: Length of output: 51
It captures the proposed improvement to centralize 🧠 Learnings used |
Purpose
This is to Add support for handling groups tier limit reached
This follows the same approach done for applications, sub organizations, connections
Related Issues
Related PRs
Checklist
Security checks
Developer Checklist (Mandatory)
product-isissue to track any behavioral change or migration impact.