fix: prevent duplicate API calls on Sign Up button rapid taps (#612) #660
Conversation
|
🎉 Welcome @pankajbaid567!
We appreciate your contribution! 🚀 |
WalkthroughAdds an early guard in the AuthenticationController.signup to prevent concurrent sign-up calls, disables form inputs and the submit button while loading in the sign-up screen, and adds tests validating the processing-lock and isLoading lifecycle. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as SignUp Screen (UI)
participant C as AuthenticationController
participant S as Auth Service / API
UI->>C: onPressed(signup)
alt controller.isLoading == true
C-->>UI: return false (ignore)
else
UI->>UI: disable inputs & button (isLoading=true)
C->>S: perform signup API call
alt signup succeeds
S-->>C: success
C-->>UI: navigate / show success
else signup fails
S-->>C: error
C-->>UI: show error / snackbar
end
C->>UI: set isLoading=false (re-enable inputs/button)
C-->>UI: return result (true/false)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 |
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
test/controllers/authentication_controller_test.dart (1)
48-64: Test doesn't verify the stated behavior.The test name claims to verify that
isLoadingis set to true at the start, but the assertion never actually checks this. The comment on lines 61-63 acknowledges the limitation.Consider using a
Completerto pause execution mid-signup and verify the loading state:test('signup sets isLoading to true at the start', () async { // Arrange authController.emailController.text = 'test@test.com'; authController.passwordController.text = 'Password123'; + final completer = Completer<void>(); when( mockAuthStateController.signup(any, any), - ).thenAnswer((_) async {}); + ).thenAnswer((_) => completer.future); // Act expect(authController.isLoading.value, false); final signupFuture = authController.signup(_MockBuildContext()); - // Assert: isLoading should be true during the call - // Note: This is tricky to test synchronously, so we verify after completion + // Assert: isLoading should be true while waiting + expect(authController.isLoading.value, true); + + completer.complete(); await signupFuture; + expect(authController.isLoading.value, false); });lib/views/screens/signup_screen.dart (1)
238-296: Solid dual-layer protection against duplicate submissions.The combination of
signUpIsAllowed(UI-level, synchronous) andisLoading(controller-level) provides robust protection. The immediate setting ofsignUpIsAllowed = falseat line 251 before theawaitensures no gap in protection.One minor consideration: if any code between lines 256-275 throws (e.g., during navigation or snackbar),
signUpIsAllowedwon't reset. Atry-finallypattern could make this more robust, though the risk is low:emailVerifyController .signUpIsAllowed .value = false; + try { var isSignedIn = await controller.signup( context, ); if (isSignedIn) { Get.toNamed(AppRoutes.onBoarding); customSnackbar( // ... snackbar code ); SemanticsService.announce( // ... announce code ); - emailVerifyController - .signUpIsAllowed - .value = true; - } else { - emailVerifyController - .signUpIsAllowed - .value = true; } + } finally { + emailVerifyController + .signUpIsAllowed + .value = true; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/controllers/authentication_controller.dart(1 hunks)lib/views/screens/signup_screen.dart(3 hunks)test/controllers/authentication_controller_test.dart(1 hunks)
🔇 Additional comments (3)
lib/controllers/authentication_controller.dart (1)
70-72: LGTM! Clean implementation of the processing lock.The early guard correctly prevents concurrent API calls. Since Dart's event loop is single-threaded and the check-then-set happens synchronously before any
await, there's no race condition risk for UI-triggered events.test/controllers/authentication_controller_test.dart (1)
19-24: Good test setup with explicit mock assignment.The direct assignment at line 23 ensures the mock is used, which is appropriate for unit testing. The combination of
Get.put()and direct assignment provides belt-and-suspenders coverage.lib/views/screens/signup_screen.dart (1)
70-85: LGTM! Form fields correctly disabled during loading.The
Obxwrapper ensures the email field reactively updates whenisLoadingchanges, preventing user input during the API call. The same pattern is correctly applied to password fields at lines 89 and 132.
|
Closing as issue is not part of the unstoppable hackathon and already assigned to another contributor |
|
✅ PR Closed - Thank You, @pankajbaid567!
We appreciate your effort and look forward to more contributions from you! 🤝 |
3 participants
Fix: Prevent duplicate API calls on Sign Up button rapid taps
Issue
Fixes #612
Problem
When users rapidly tap the Sign Up button multiple times, the app sends multiple API requests to Appwrite. The first request successfully creates the user, but subsequent requests fail with "User already exists" error, causing poor UX and unnecessary backend load.
Solution
Implemented a controller-level processing lock combined with reactive UI updates to prevent duplicate sign-up requests.
Flow Diagram
sequenceDiagram participant User participant SignupScreen participant AuthController participant AppwriteAPI User->>SignupScreen: Tap Sign Up SignupScreen->>AuthController: signup() AuthController->>AuthController: Check isLoading alt isLoading is true AuthController-->>SignupScreen: return false (blocked) Note right of SignupScreen: Duplicate tap ignored else isLoading is false AuthController->>AuthController: Set isLoading = true SignupScreen->>SignupScreen: Disable button & form fields SignupScreen->>SignupScreen: Show loading indicator AuthController->>AppwriteAPI: Create user AppwriteAPI-->>AuthController: Response AuthController->>AuthController: Set isLoading = false (finally) SignupScreen->>SignupScreen: Re-enable button & form fields AuthController-->>SignupScreen: return result endChanges
lib/controllers/authentication_controller.dartsignup()methodisLoading.valueis alreadytrue, returnsfalseimmediately without making API calllib/views/screens/signup_screen.dartObxfor reactive state bindingisLoading.valueistrueenabled: !controller.isLoading.valueto all form fields (email, password, confirm password)test/controllers/authentication_controller_test.dartsignup()returnsfalsewhen already loadingisLoadingis reset after success/failureTesting
Screenshots/Video
Add before/after video showing the fix in action
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.