fix: prevent multiple submissions in NewSyncProfileCubit - #96
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds an early isSubmitting guard and reorders isSubmitting toggling in submit() of new_sync_profile_cubit to prevent concurrent submissions and ensure isSubmitting resets on validation failure. Also increments app version from 0.2.2 to 0.2.3 in pubspec.yaml. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant C as NewSyncProfileCubit
participant S as Backend/Service
U->>C: submit()
alt Already submitting
C->>C: Check state.isSubmitting == true
Note right of C #F9F6EE: Early return (no-op)
else Not submitting
C->>C: Emit isSubmitting = true
C->>C: canSubmit() validation
alt Invalid
C->>C: Emit isSubmitting = false and submitError
C-->>U: Emit validation error state
else Valid
C->>S: Perform submission
S-->>C: Response
C->>C: Emit isSubmitting = false and result state
C-->>U: Emit success or error state
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
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 |
Summary of ChangesHello @SuperMuel, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a crucial fix to prevent users from inadvertently triggering multiple submissions within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to prevent multiple submissions in NewSyncProfileCubit by adding a guard clause at the beginning of the submit method. While this is a good intention, the current implementation is not fully protected against race conditions. I've left a comment with a suggestion to make the fix more robust by ensuring the isSubmitting state is updated immediately. This will prevent multiple submissions from being processed if the submit method is invoked multiple times in quick succession.
| if (state.isSubmitting) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This check is a good step towards preventing multiple submissions. However, a race condition is still possible. If submit() is called multiple times in quick succession (e.g., due to rapid UI taps before the button is disabled), both calls might pass this check before the state is updated with isSubmitting: true.
To make this fully robust against race conditions, you should emit a state with isSubmitting: true immediately after this guard clause, and before any asynchronous operations (await).
For example:
Future<void> submit() async {
if (state.isSubmitting) {
return;
}
emit(state.copyWith(isSubmitting: true));
if (!state.canSubmit()) {
emit(state.copyWith(
isSubmitting: false, // Also reset here
submitError: 'Please fill all the required fields.',
));
return;
}
try {
// ... async submission logic ...
emit(state.copyWith(isSubmitting: false, submittedSuccessfully: true));
} catch (e) {
emit(state.copyWith(isSubmitting: false, submitError: e.toString()));
}
}By setting isSubmitting: true synchronously at the beginning, you effectively "lock" the submission process and prevent any concurrent executions from starting.
|
Visit the preview URL for this PR (updated for commit 9a924ef): https://syncademic-36c18--pr96-feature-prevent-doub-397dkx9c.web.app (expires Sun, 28 Sep 2025 16:45:20 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 1427d2ca23ba5ec56b241548ca0f0f1eb2be2ea1 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
syncademic_app/lib/screens/new_sync_profile/cubit/new_sync_profile_cubit.dart (2)
100-112: Listener callback should not beasync/Future
providerAccountSelectedreturnsFuture<void>but is used as aStream.listencallback (expectsvoid Function(T)). Make it synchronous.- Future<void> providerAccountSelected( - ProviderAccount? providerAccount) async => + void providerAccountSelected( + ProviderAccount? providerAccount) => emit( providerAccount == null ? state.copyWith( providerAccount: null, providerAccountError: 'No provider account selected.', ) : state.copyWith( providerAccount: providerAccount, providerAccountError: null, ), );
241-289: Reset isSubmitting in a finally block and clear stale submit flags when starting submitExceptions thrown while building the payload (before the current try/catch) can leave isSubmitting true — clear submitError/submittedSuccessfully at start and guarantee isSubmitting is reset in a finally.
File: syncademic_app/lib/screens/new_sync_profile/cubit/new_sync_profile_cubit.dart (submit method)
- At submit start: emit(state.copyWith(isSubmitting: true, submitError: null, submittedSuccessfully: false)).
- Wrap payload construction + service call in try { … } finally { emit(state.copyWith(isSubmitting: false)); } — move success/error emits into the inner try/catch and avoid setting isSubmitting there.
- Add tests: simulate an exception between start and service call to assert isSubmitting → false; test double-submit (first call pending) to assert single service invocation and proper state transitions.
🧹 Nitpick comments (1)
syncademic_app/lib/screens/new_sync_profile/cubit/new_sync_profile_cubit.dart (1)
148-178: Async method should returnFuture<void>
void authorizeBackend() asyncprevents callers from awaiting completion or catching errors. PreferFuture<void>.- void authorizeBackend() async { + Future<void> authorizeBackend() async {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
syncademic_app/lib/screens/new_sync_profile/cubit/new_sync_profile_cubit.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: build_and_preview
🔇 Additional comments (1)
syncademic_app/lib/screens/new_sync_profile/cubit/new_sync_profile_cubit.dart (1)
241-253: Early re-entrancy guard + immediate lock: good fixThe guard and the immediate
isSubmitting: trueemit eliminate concurrent submissions on rapid taps.
1 participant
Summary by CodeRabbit
Bug Fixes
Chores