Update cohort management module to use info banners#3546
Merged
Baalmart merged 10 commits intoMay 28, 2026
Merged
Conversation
…rredBanner in assign/unassign dialogs
Contributor
📝 WalkthroughWalkthroughMigrate toast/setTimeout notifications to InfoBanner: add useBanner and useBannerWithDelay usages; refactor cohort mutation hooks to accept optional onSuccess/onError callbacks and forward AxiosError; update cohort, grid, and device components to show scoped dialog errors and deferred global success banners. ChangesCohort Management Banner Migration
Sequence Diagram(s) sequenceDiagram
participant User as User
participant Dialog as Dialog/Component
participant Hook as useCohorts Hook
participant API as Cohorts API
participant Banner as InfoBanner
User->>Dialog: trigger action (create/assign/update)
Dialog->>Hook: call mutation with { onSuccess, onError }
Hook->>API: execute request
alt Success
API-->>Hook: success response
Hook->>Dialog: invoke onSuccess(variables/data)
Dialog->>Banner: showBannerWithDelay({ scoped:false, message })
else Error
API-->>Hook: AxiosError
Hook->>Dialog: invoke onError(error)
Dialog->>Banner: showBanner({ scoped:true, message: getApiErrorMessage(error) })
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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
Author
|
@copilot review. |
![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: 1
🧹 Nitpick comments (2)
src/vertex/components/features/cohorts/edit-cohort-details-modal.tsx (1)
128-134: ⚡ Quick winUse
useDeferredBannerinstead of a raw timeout for post-close success messaging.This keeps deferred banner behavior consistent across dialogs and provides timer lifecycle cleanup.
🤖 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/vertex/components/features/cohorts/edit-cohort-details-modal.tsx` around lines 128 - 134, Replace the ad-hoc setTimeout call that invokes showBanner in edit-cohort-details-modal.tsx with the standardized useDeferredBanner hook: import and call useDeferredBanner in the component, get the deferred banner function (e.g., deferredShow or showDeferred), and invoke that with the same payload ('severity: success', 'Cohort details updated successfully', scoped: false) instead of using setTimeout; remove the setTimeout and ensure the deferred hook is called after the modal closes (where setTimeout currently sits) so the hook manages the timer lifecycle and cleanup for you.src/vertex/components/features/cohorts/cohort-detail-card.tsx (1)
63-69: ⚡ Quick winReplace inline success
setTimeoutwith the shared deferred banner hook.Using raw timers here duplicates logic and skips cleanup; adopting
useDeferredBannerkeeps behavior consistent with assign/unassign dialogs and avoids stale delayed banners.Also applies to: 83-89
🤖 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/vertex/components/features/cohorts/cohort-detail-card.tsx` around lines 63 - 69, The inline setTimeouts in cohort-detail-card.tsx that call showBanner (the blocks at lines around the setTimeout using message `Cohort is now ${targetVisibility ? 'public' : 'private'}` and the similar block at 83-89) should be replaced with the shared useDeferredBanner hook; remove the raw setTimeout calls, import and call useDeferredBanner from the shared hooks, and use its API to schedule the same success banner (passing the same payload: severity:'success', message:`Cohort is now ${...}`, scoped:false). Locate the setTimeout usages and replace them with the hook’s deferred show invocation (e.g., const { deferBanner } = useDeferredBanner(); deferBanner(() => showBanner({...})) or the hook’s documented showDeferredBanner({ ... })) so cleanup and consistent behavior match the assign/unassign dialogs.
🤖 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/vertex/core/hooks/useCohorts.ts`:
- Around line 314-317: The invalidateQueries calls use a different key shape
than useCohorts (they pass ['cohorts', activeNetwork?.net_name] while useCohorts
registers keys as ['cohorts', { ... }]), so update the three invalidation sites
(the queryClient.invalidateQueries calls at the blocks around lines 314-317,
424-427, and 466-468) to use the same second-element object shape as useCohorts;
e.g., replace ['cohorts', activeNetwork?.net_name] with ['cohorts',
activeNetwork] or ['cohorts', { net_name: activeNetwork?.net_name }] (matching
the exact object used by useCohorts) so the cohort summary queries are properly
invalidated.
---
Nitpick comments:
In `@src/vertex/components/features/cohorts/cohort-detail-card.tsx`:
- Around line 63-69: The inline setTimeouts in cohort-detail-card.tsx that call
showBanner (the blocks at lines around the setTimeout using message `Cohort is
now ${targetVisibility ? 'public' : 'private'}` and the similar block at 83-89)
should be replaced with the shared useDeferredBanner hook; remove the raw
setTimeout calls, import and call useDeferredBanner from the shared hooks, and
use its API to schedule the same success banner (passing the same payload:
severity:'success', message:`Cohort is now ${...}`, scoped:false). Locate the
setTimeout usages and replace them with the hook’s deferred show invocation
(e.g., const { deferBanner } = useDeferredBanner(); deferBanner(() =>
showBanner({...})) or the hook’s documented showDeferredBanner({ ... })) so
cleanup and consistent behavior match the assign/unassign dialogs.
In `@src/vertex/components/features/cohorts/edit-cohort-details-modal.tsx`:
- Around line 128-134: Replace the ad-hoc setTimeout call that invokes
showBanner in edit-cohort-details-modal.tsx with the standardized
useDeferredBanner hook: import and call useDeferredBanner in the component, get
the deferred banner function (e.g., deferredShow or showDeferred), and invoke
that with the same payload ('severity: success', 'Cohort details updated
successfully', scoped: false) instead of using setTimeout; remove the setTimeout
and ensure the deferred hook is called after the modal closes (where setTimeout
currently sits) so the hook manages the timer lifecycle and cleanup for you.
🪄 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: 23542b5d-805b-43e9-8b50-d5a83ae7dee1
📒 Files selected for processing (9)
src/vertex/app/changelog.mdsrc/vertex/components/features/cohorts/assign-cohort-devices.tsxsrc/vertex/components/features/cohorts/cohort-detail-card.tsxsrc/vertex/components/features/cohorts/cohort-measurements-api-card.tsxsrc/vertex/components/features/cohorts/create-cohort.tsxsrc/vertex/components/features/cohorts/device-name-parser.tsxsrc/vertex/components/features/cohorts/edit-cohort-details-modal.tsxsrc/vertex/components/features/cohorts/unassign-cohort-devices.tsxsrc/vertex/core/hooks/useCohorts.ts
Codebmk
reviewed
May 28, 2026
| message: `${variables.deviceIds.length} device(s) assigned to cohort successfully`, | ||
| scoped: false, | ||
| }); | ||
| }, |
Member
There was a problem hiding this comment.
I should have asked this in the previous PR. What prompted you to create showDeferredBanner yet showBanner covered both global and "local" ie within the dialog, banner instances? @BwanikaRobert
Contributor
Author
There was a problem hiding this comment.
The fix is a 100 ms delay wait for the dialog to fully unmount, then show the banner. Without that delay, the success message silently disappears on every action that closes a dialog.
useDeferredBanner just encapsulates that pattern the useRef to hold the timer, the useEffect to cancel it on unmount, and the 100 ms delay so we don't copy and paste the same bannerTimerRef + setTimeout + cleanup block into every component that needs post-dialog feedback. showBanner alone can't express this.
It was one of the changes @copilot requested in the prevoius PR @Codebmk
Member
There was a problem hiding this comment.
Okay, that was a great change! Let's rename it to showBannerWithDelay and useBannerWithDelay where it is much easier to remember what they are for
Member
There was a problem hiding this comment.
Kindly replace all parts in the codebase that use the new banner but apply the timer approach manually, they are few. @BwanikaRobert
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/vertex/components/features/devices/device-assignment-modal.tsx (1)
63-67: ⚡ Quick winMake delayed success banner scope explicit.
At Line 63, add
scoped: falseexplicitly so this post-close success message is guaranteed to render globally and stays aligned with the rest of this migration.Suggested patch
showBannerWithDelay({ severity: 'success', title: 'Success', message: `${data.device.name} has been assigned successfully.`, + scoped: false, }, 300);🤖 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/vertex/components/features/devices/device-assignment-modal.tsx` around lines 63 - 67, The success banner shown after device assignment uses showBannerWithDelay but doesn't set the banner scope; update the call to showBannerWithDelay (in device-assignment-modal component where the success message is constructed) to include scoped: false in the options object so the post-close banner renders globally (e.g., add scoped: false alongside severity, title, message, and delay).
🤖 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/vertex/components/features/devices/device-assignment-modal.tsx`:
- Around line 63-67: The success banner shown after device assignment uses
showBannerWithDelay but doesn't set the banner scope; update the call to
showBannerWithDelay (in device-assignment-modal component where the success
message is constructed) to include scoped: false in the options object so the
post-close banner renders globally (e.g., add scoped: false alongside severity,
title, message, and delay).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e839ebf-ac03-4518-ac81-c644e84464d0
📒 Files selected for processing (8)
src/vertex/app/changelog.mdsrc/vertex/components/features/devices/add-maintenance-log-modal.tsxsrc/vertex/components/features/devices/create-device-modal.tsxsrc/vertex/components/features/devices/deploy-device-component.tsxsrc/vertex/components/features/devices/device-assignment-modal.tsxsrc/vertex/components/features/devices/import-device-modal.tsxsrc/vertex/components/features/devices/recall-device-dialog.tsxsrc/vertex/core/hooks/useBannerWithDelay.ts
✅ Files skipped from review due to trivial changes (1)
- src/vertex/app/changelog.md
7 tasks
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
3 participants
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.
Summary of Changes
Migrates the Cohort Management module from
ReusableToastto the centralizedInfoBannersystem (useBannerhook). All user feedback is now surfaced at the component layer rather than inside hooks, following the architecture established by the Grid Management module refactor.Hook changes (
useCohorts.ts)ReusableToastcalls from the hook layeronSuccess/onErrorcallback options to all mutation hooks:useCreateCohort,useCreateCohortWithDevices,useCreateCohortFromCohorts,useAssignDevicesToCohort,useUnassignDevicesFromCohort,useAssignCohortsToGroup,useAssignCohortsToUser,useUpdateCohortDetails,useUpdateCohortNameComponent changes
cohort-detail-card.tsx— replaced toasts with scoped/global banners; switched tomutateAsyncfor visibility and tags updatescohort-measurements-api-card.tsx— replaced toast withuseBannerfor clipboard feedbackcreate-cohort.tsx— replaced toasts with banners; consolidatedonSuccess/onErrorto hook-level (removed unreliable per-call callbacks)device-name-parser.tsx— replaced all toast calls with scoped bannersedit-cohort-details-modal.tsx— replaced toasts with banners; addedsetTimeoutsuccess banner pattern after dialog closeassign-cohort-devices.tsx/unassign-cohort-devices.tsx— replaced toasts withuseDeferredBannerfor post-close success bannersShared utilities used from Grid Management PR
useDeferredBannerhook — handles timer ref, cleanup on unmount, andAFTER_DIALOG_CLOSE_MSdelaygetApiErrorMessage— now guards against empty/whitespace and raw HTML error responsesStatus of maturity
How should this be manually tested?
What are the relevant tickets?
Screenshots
Summary by CodeRabbit
Refactor
Chore