[_]: feat/add mail EPs and refresh user avatar#31
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCentralized error exports into Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 1-3: Reorder the environment variable entries so they are
alphabetically consistent: place VITE_MAIL_API_URL before VITE_PAYMENTS_API_URL
(keeping VITE_DRIVE_API_URL where appropriate), i.e., update the .env.example
listing to alphabetic order referencing the variables VITE_MAIL_API_URL,
VITE_PAYMENTS_API_URL, and VITE_DRIVE_API_URL so the linter warning is resolved.
In `@sonar-project.properties`:
- Line 7: The PR description/messaging incorrectly states that `**/*.errors.ts`
was replaced; update the summary to accurately say that the
`sonar.coverage.exclusions` property now adds `errors/**` in addition to the
existing `**/*.errors.ts` pattern (key: sonar.coverage.exclusions in
sonar-project.properties), and optionally note that `**/*.errors.ts` can be
removed later once migration is complete.
In `@src/store/slices/user/thunks/refreshAvatarThunk/index.ts`:
- Around line 9-14: The thunk currently calls
UserService.instance.refreshUserAvatar() before ensuring a user exists; update
the async thunk (the function using getState and dispatch) to first read
getState().user.user and if no user return undefined, then call
UserService.instance.refreshUserAvatar() and proceed; in other words, move the
user existence guard above the call to refreshUserAvatar() so the API is only
invoked when a user is present.
In `@src/store/slices/user/thunks/refreshAvatarThunk/refreshAvatarThunk.test.ts`:
- Around line 31-43: The test currently stubs
UserService.instance.refreshUserAvatar but doesn't assert it wasn't invoked;
update the test in refreshAvatarThunk.test.ts to assert that
UserService.instance.refreshUserAvatar was not called when the store's user.user
is undefined by adding an expectation like
expect(UserService.instance.refreshUserAvatar).not.toHaveBeenCalled() after
dispatching refreshAvatarThunk(); keep the existing spy creation
(vi.spyOn(...).mockResolvedValue(null)), the store setup with user: { user:
undefined }, and the existing payload/state assertions.
🪄 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: 7f98ff57-19f9-4697-80d4-dc58d6a933c0
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (29)
.env.examplepackage.jsonsonar-project.propertiessrc/App.tsxsrc/errors/config/index.tssrc/errors/index.tssrc/errors/navigation/index.tssrc/errors/oauth/index.tssrc/errors/shared/index.tssrc/errors/storage/index.tssrc/main.tsxsrc/services/config/config.service.test.tssrc/services/config/index.tssrc/services/navigation/index.tssrc/services/navigation/navigation.service.test.tssrc/services/oauth/oauth.service.test.tssrc/services/oauth/oauth.service.tssrc/services/sdk/index.tssrc/services/sdk/mail/index.tssrc/services/sdk/mail/mail.service.test.tssrc/services/user/user.service.tssrc/store/queries/storage/storage.query.test.tssrc/store/queries/storage/storage.query.tssrc/store/slices/user/index.tssrc/store/slices/user/thunks/index.tssrc/store/slices/user/thunks/refreshAvatarThunk/index.tssrc/store/slices/user/thunks/refreshAvatarThunk/refreshAvatarThunk.test.tssrc/store/slices/user/userSlice.test.tsvite.config.ts
src/store/slices/user/thunks/refreshAvatarThunk/refreshAvatarThunk.test.ts
Show resolved
Hide resolved
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: 1
♻️ Duplicate comments (1)
src/store/slices/user/thunks/refreshAvatarThunk/index.ts (1)
13-20:⚠️ Potential issue | 🟠 MajorMissing validation for
userAvatarbefore dispatch.Per the context snippet from
user.service.ts,refreshUserAvatar()can returnnullorundefined. The current code dispatchessetUserunconditionally, which would overwrite any existing avatar withnull/undefinedwhen the refresh fails to return a valid avatar.The AI summary also indicates test expectations of "no state change when it resolves to null", suggesting this validation was intended.
🔧 Proposed fix
const userAvatar = await UserService.instance.refreshUserAvatar(); + if (!userAvatar) return undefined; + dispatch( userActions.setUser({ ...user, avatar: userAvatar, }), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/slices/user/thunks/refreshAvatarThunk/index.ts` around lines 13 - 20, The thunk currently calls UserService.instance.refreshUserAvatar() and unconditionally dispatches userActions.setUser with avatar: userAvatar, which can overwrite the existing avatar with null/undefined; update the logic in the refreshAvatarThunk so that after calling refreshUserAvatar() you validate the result (check userAvatar !== null && userAvatar !== undefined) and only call dispatch(userActions.setUser({...user, avatar: userAvatar})) when a truthy/valid avatar is returned; otherwise do nothing (or return early) so the existing user.avatar remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/store/slices/user/thunks/refreshAvatarThunk/index.ts`:
- Around line 7-22: The thunk refreshAvatarThunk currently calls
UserService.instance.refreshUserAvatar() without error handling; wrap the async
call in a try-catch inside the createAsyncThunk payload so network or API errors
are caught, dispatch userActions.setUser only on success, and return/propagate a
controlled rejection (e.g., via thunkAPI.rejectWithValue or undefined) in the
catch block so callers receive a predictable failure shape; ensure you reference
refreshAvatarThunk, UserService.instance.refreshUserAvatar, and
userActions.setUser when making the changes.
---
Duplicate comments:
In `@src/store/slices/user/thunks/refreshAvatarThunk/index.ts`:
- Around line 13-20: The thunk currently calls
UserService.instance.refreshUserAvatar() and unconditionally dispatches
userActions.setUser with avatar: userAvatar, which can overwrite the existing
avatar with null/undefined; update the logic in the refreshAvatarThunk so that
after calling refreshUserAvatar() you validate the result (check userAvatar !==
null && userAvatar !== undefined) and only call
dispatch(userActions.setUser({...user, avatar: userAvatar})) when a truthy/valid
avatar is returned; otherwise do nothing (or return early) so the existing
user.avatar remains unchanged.
🪄 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: de57e1d4-27d7-43ae-b55b-1e96a487e671
📒 Files selected for processing (1)
src/store/slices/user/thunks/refreshAvatarThunk/index.ts
| export const refreshAvatarThunk = createAsyncThunk<UserSettings | undefined, void, { state: RootState }>( | ||
| 'user/refreshAvatar', | ||
| async (_, { getState, dispatch }) => { | ||
| const user = getState().user.user; | ||
| if (!user) return undefined; | ||
|
|
||
| const userAvatar = await UserService.instance.refreshUserAvatar(); | ||
|
|
||
| dispatch( | ||
| userActions.setUser({ | ||
| ...user, | ||
| avatar: userAvatar, | ||
| }), | ||
| ); | ||
| }, | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding error handling for the API call.
If UserService.instance.refreshUserAvatar() throws (e.g., network failure), the thunk will reject without graceful handling. Depending on how this thunk is consumed, you may want to wrap the call in a try-catch to handle failures gracefully or ensure callers handle the rejected promise appropriately.
♻️ Optional: Add try-catch for resilience
export const refreshAvatarThunk = createAsyncThunk<UserSettings | undefined, void, { state: RootState }>(
'user/refreshAvatar',
async (_, { getState, dispatch }) => {
const user = getState().user.user;
if (!user) return undefined;
- const userAvatar = await UserService.instance.refreshUserAvatar();
+ let userAvatar: string | null | undefined;
+ try {
+ userAvatar = await UserService.instance.refreshUserAvatar();
+ } catch {
+ return undefined;
+ }
if (!userAvatar) return undefined;
dispatch(
userActions.setUser({
...user,
avatar: userAvatar,
}),
);
},
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/store/slices/user/thunks/refreshAvatarThunk/index.ts` around lines 7 -
22, The thunk refreshAvatarThunk currently calls
UserService.instance.refreshUserAvatar() without error handling; wrap the async
call in a try-catch inside the createAsyncThunk payload so network or API errors
are caught, dispatch userActions.setUser only on success, and return/propagate a
controlled rejection (e.g., via thunkAPI.rejectWithValue or undefined) in the
catch block so callers receive a predictable failure shape; ensure you reference
refreshAvatarThunk, UserService.instance.refreshUserAvatar, and
userActions.setUser when making the changes.
|
2 participants
New EPs added:
Refactors: