[PB-5978]: feat(mail)/frozen account state#53
Merged
Conversation
… downgrade messages
… mail account status handling in Sidenav
Deploying mail-web with
|
| Latest commit: |
eba51ab
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d6189205.mail-web-ea0.pages.dev |
| Branch Preview URL: | https://feat-disabled-account-state.mail-web-ea0.pages.dev |
…to version 0.1.16
jzunigax2
force-pushed
the
feat/disabled-account-state
branch
from
65b05ed to
99d313d
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a reusable ChangesSidenav data and timing utilities
Sequence Diagram(s)sequenceDiagram
participant Sidenav
participant useSidenavData
participant useGetStorageQueries
participant useGetMailMeQuery
participant getDaysUntil
Sidenav->>useSidenavData: call hook
useSidenavData->>useGetStorageQueries: query plan limit & usage
useSidenavData->>useGetMailMeQuery: query mail account
useSidenavData->>getDaysUntil: compute daysUntilDeletion(deletionAt)
useGetMailMeQuery-->>useSidenavData: mailMe (active/suspended)
useGetStorageQueries-->>useSidenavData: planLimit / planUsage
useSidenavData-->>Sidenav: { isMailDisabled, daysUntilDeletion, storagePercentage, loading flags }
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
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/Sidenav/index.tsx`:
- Around line 24-27: Extract the data fetching and derived UI state from Sidenav
into a hook (e.g., create useMailAccountState or useSidenavData) so the
component remains presentational: move the useGetMailMeQuery() call and derived
values isMailDisabled, daysUntilDeletion (derived via
getDaysUntil(mailMe?.deletionAt)), and storagePercentage (derived from planUsage
and planLimit) into that hook, have the hook return plain values/booleans
(mailMe, isMailDisabled, daysUntilDeletion, storagePercentage, planUsage,
planLimit) and any callbacks, then replace the in-component logic in Sidenav
with a single call to the new hook and pass the returned values as props to the
render layer; update any other similar logic around lines 76-85 into the same or
another focused use... hook following the same pattern.
- Around line 79-82: The CTA's onAction is currently a no-op and should invoke
the real upgrade flow; replace the empty handler assigned to onAction with a
call that opens your app's upgrade UI (for example call an existing helper like
openUpgradeModal(), openBillingPortal(), or dispatch a navigation such as
router.push('/settings/billing')), ensuring you import or access that function
inside the Sidenav component and pass any required params; update the onAction
reference (the handler on the message created with
translate('mailDowngraded.message'...)) to invoke that real upgrade function
instead of () => {} so the warning button actually starts the upgrade flow.
In `@src/utils/days-until/index.ts`:
- Around line 3-4: The getDaysUntil function currently computes days for any
string and can return NaN for unparseable dates; update getDaysUntil to first
parse the input (e.g., const ts = Date.parse(date) or new Date(date).getTime()),
check Number.isFinite(ts) or !Number.isNaN(ts), and if the parse fails return
undefined; otherwise use ts and MS_PER_DAY in the Math.max/Math.ceil calculation
so invalid date strings do not propagate NaN to callers.
🪄 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.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b187cb78-a03d-40ee-b0cf-5170ddddf4e1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
package.jsonsrc/components/Sidenav/index.tsxsrc/errors/mail/index.tssrc/hooks/navigation/useSidenavNavigation.tsxsrc/i18n/locales/en.jsonsrc/i18n/locales/es.jsonsrc/i18n/locales/fr.jsonsrc/i18n/locales/it.jsonsrc/services/sdk/mail/index.tssrc/services/sdk/mail/mail.service.test.tssrc/store/api/base.tssrc/store/api/mail/index.tssrc/store/api/mail/mail.api.test.tssrc/utils/days-until/daysUntil.test.tssrc/utils/days-until/index.ts
…enhance date handling
xabg2
reviewed
May 13, 2026
| isLoadingPlanLimit, | ||
| isLoadingPlanUsage, | ||
| storagePercentage, | ||
| } = useSidenavData(); |
xabg2
reviewed
May 13, 2026
| expect(mockMailClient.getMailAccount).toHaveBeenCalledOnce(); | ||
| }); | ||
|
|
||
| test('When fetching the mail account and it is suspended, then suspendedAt and deletionAt should be present', async () => { |
Contributor
There was a problem hiding this comment.
Avoid using technical descriptions. When fetching the mail account and it is suspended, then the date when it was suspended and it will be deleted are present
| expect(result.data).toStrictEqual(mockAccount); | ||
| }); | ||
|
|
||
| test('When fetching the mail account fails, then a FetchMailMeError should be returned', async () => { |
Contributor
There was a problem hiding this comment.
We can use an error indicating so is thrown
…ive scenarios for mail account status and storage usage
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/Sidenav/useSidenavData.test.ts (1)
127-169: ⚡ Quick winAdd a boundary test for undefined storage values.
There’s no case asserting behavior when RTK Query returns
data: undefinedfor plan values; adding it will protect fallback semantics and avoid regressions instoragePercentage.Suggested test case
describe('Storage percentage', () => { + test('When storage values are unavailable, then usage percentage is reported as zero', () => { + setupMocks({ + planLimit: undefined as unknown as number, + planUsage: undefined as unknown as number, + }); + + const { result } = renderHook(() => useSidenavData()); + + expect(result.current.planLimit).toBe(0); + expect(result.current.planUsage).toBe(0); + expect(result.current.storagePercentage).toBe(0); + }); + test('When the used storage is half of the available storage, then the reported usage is fifty percent', () => {As per coding guidelines,
src/**/*.test.{ts,tsx}: ... cover happy path, edge cases, error/rejection paths, and boundary values.🤖 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/components/Sidenav/useSidenavData.test.ts` around lines 127 - 169, Add a boundary test to ensure useSidenavData handles RTK Query returning undefined for plan values: create a new test in the "Storage percentage" suite that calls setupMocks with planUsage: undefined and planLimit: undefined (and appropriate isLoading flags false), renderHook(() => useSidenavData()), and assert that result.current.storagePercentage is 0 and that planUsage/planLimit fall back to safe values (e.g., 0) and loading flags (isLoadingPlanLimit/isLoadingPlanUsage) are false; reference the existing tests using setupMocks, useSidenavData, storagePercentage, planUsage, planLimit, isLoadingPlanLimit and isLoadingPlanUsage to place and name the test consistent with the suite.
🤖 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/Sidenav/useSidenavData.test.ts`:
- Line 3: Import of useSidenavData uses a relative path; replace the relative
import with the project path alias (e.g., use '@/...' instead of './...') so
internal imports follow the src alias convention—update the import that
references the useSidenavData symbol in useSidenavData.test.ts to use the `@/`*
alias form consistent with the repository rules.
In `@src/components/Sidenav/useSidenavData.ts`:
- Around line 6-12: The code sets planLimit default to 1 which can misrepresent
unknown/unloaded limits; change the default to 0 or undefined when destructuring
the result of useGetStorageLimitQuery (variable planLimit in useSidenavData.ts)
and adjust the storagePercentage calculation to explicitly handle missing/zero
limits (e.g., treat undefined or 0 as "unknown" and return 0% or a special
state) so storagePercentage = planLimit > 0 ? Math.min((planUsage / planLimit) *
100, 100) : 0; also ensure any UI that relies on isLoadingPlanLimit or planLimit
distinguishes loading/unknown vs actual 0 values.
---
Nitpick comments:
In `@src/components/Sidenav/useSidenavData.test.ts`:
- Around line 127-169: Add a boundary test to ensure useSidenavData handles RTK
Query returning undefined for plan values: create a new test in the "Storage
percentage" suite that calls setupMocks with planUsage: undefined and planLimit:
undefined (and appropriate isLoading flags false), renderHook(() =>
useSidenavData()), and assert that result.current.storagePercentage is 0 and
that planUsage/planLimit fall back to safe values (e.g., 0) and loading flags
(isLoadingPlanLimit/isLoadingPlanUsage) are false; reference the existing tests
using setupMocks, useSidenavData, storagePercentage, planUsage, planLimit,
isLoadingPlanLimit and isLoadingPlanUsage to place and name the test consistent
with the suite.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 411b5301-77cd-44cf-82ba-b916a13e7382
📒 Files selected for processing (7)
src/components/Sidenav/index.tsxsrc/components/Sidenav/useSidenavData.test.tssrc/components/Sidenav/useSidenavData.tssrc/services/sdk/mail/mail.service.test.tssrc/store/api/mail/mail.api.test.tssrc/utils/days-until/daysUntil.test.tssrc/utils/days-until/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/utils/days-until/daysUntil.test.ts
- src/components/Sidenav/index.tsx
- src/services/sdk/mail/mail.service.test.ts
- src/store/api/mail/mail.api.test.ts
| @@ -0,0 +1,170 @@ | |||
| import { renderHook } from '@testing-library/react'; | |||
| import { describe, test, expect, vi, beforeEach, afterEach } from 'vitest'; | |||
| import { useSidenavData } from './useSidenavData'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use @/* alias for internal imports in src.
Switch the local relative import to alias form for consistency with repository rules.
-import { useSidenavData } from './useSidenavData';
+import { useSidenavData } from '@/components/Sidenav/useSidenavData';As per coding guidelines, src/**/*.{ts,tsx}: Use the path alias @/* → src/* when importing internal modules.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { useSidenavData } from './useSidenavData'; | |
| import { useSidenavData } from '@/components/Sidenav/useSidenavData'; |
🤖 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/components/Sidenav/useSidenavData.test.ts` at line 3, Import of
useSidenavData uses a relative path; replace the relative import with the
project path alias (e.g., use '@/...' instead of './...') so internal imports
follow the src alias convention—update the import that references the
useSidenavData symbol in useSidenavData.test.ts to use the `@/`* alias form
consistent with the repository rules.
… in useSidenavData hook
|
xabg2
approved these changes
May 15, 2026
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
2 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.
Added account suspended notification to the sidebar. Locked some actions when account status is suspended