Fix therapist registration by including email and phone in insert#193
Open
hasanravda wants to merge 2 commits intoAOSSIE-Org:mainfrom
Open
Fix therapist registration by including email and phone in insert#193hasanravda wants to merge 2 commits intoAOSSIE-Org:mainfrom
hasanravda wants to merge 2 commits intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughThe pull request adds phone number collection and validation to the therapist registration flow. Updates to the auth interface, UI form, provider layer, and repository implementation ensure phone is captured during sign-up and persisted alongside email to the database, addressing a null value constraint violation. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as PersonalDetailsScreen
participant Provider as AuthProvider
participant Repo as SupabaseAuthRepository
participant DB as Therapist Table
UI->>UI: User enters phone & submits form
UI->>Provider: storePersonalInfo(personalInfo, phone: '+1234567890')
Provider->>Repo: storePersonalInfo(personalInfo, phone: '+1234567890')
Repo->>Repo: Validate current user exists
Repo->>Repo: Build data map with id, email, phone + personalInfoEntity fields
Repo->>DB: Insert therapist record with complete data
DB-->>Repo: Success / Error
Repo-->>Provider: Return ActionResult
Provider-->>UI: Return bool success status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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.
Actionable comments posted: 3
🧹 Nitpick comments (3)
therapist/lib/core/repository/auth/auth_repository.dart (1)
29-30: Remove leftover instruction comment and add proper documentation.The comment "Add this new method" appears to be a leftover instruction rather than documentation. Consider replacing it with proper doc comments for the new method.
📝 Suggested fix
- // Add this new method + /// Checks if the user with the given [userId] is new (not yet in the therapist table). + /// + /// Returns [ActionResultSuccess] with `is_new_user` boolean, or [ActionResultFailure] on error. Future<ActionResult> checkIfUserIsNew(String userId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@therapist/lib/core/repository/auth/auth_repository.dart` around lines 29 - 30, Remove the leftover inline instruction comment "Add this new method" and replace it with a concise Dart doc comment for the method declaration checkIfUserIsNew(String userId) describing what the method does, the meaning of the returned Future<ActionResult>, and any expectations for the userId parameter (e.g., non-null/format). Locate the declaration of Future<ActionResult> checkIfUserIsNew in the auth repository interface/class and add a /// comment above it that documents behavior, inputs, and output rather than leaving an instruction-style comment.therapist/lib/presentation/auth/personal_details_screen.dart (1)
190-201: Consider adding phone number format validation.The current validation only checks for empty input. Adding basic format validation (e.g., minimum length, digits-only check) would prevent obviously invalid entries from being stored.
♻️ Suggested phone validation improvement
validator: (value) { if (value == null || value.trim().isEmpty) { return 'Please enter your phone number'; } + final digitsOnly = value.replaceAll(RegExp(r'[^0-9]'), ''); + if (digitsOnly.length < 7 || digitsOnly.length > 15) { + return 'Please enter a valid phone number'; + } return null; },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@therapist/lib/presentation/auth/personal_details_screen.dart` around lines 190 - 201, The phone field validator currently only checks for emptiness; update the validator passed to _buildTextField (where phoneController is used) to also enforce a basic phone format: trim the value, remove allowed characters (spaces, dashes, parentheses, optional leading '+'), then check that the remaining characters are all digits and meet a minimum length (e.g., >=7) and maximum (e.g., <=15); return specific error messages like "Please enter a valid phone number" when the checks fail and null when valid.therapist/lib/repository/supabase_auth_repository.dart (1)
111-111: ReplaceProduction code should use a logging framework (e.g.,
package:logging) or at minimumdebugPrintwhich is no-op in release mode. The currentAlso applies to: 126-126, 136-136, 172-172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@therapist/lib/repository/supabase_auth_repository.dart` at line 111, Replace all print(...) calls inside SupabaseAuthRepository with a proper logger: add import 'package:logging/logging.dart'; create a private Logger (e.g., final _logger = Logger('SupabaseAuthRepository')) and replace prints at the locations mentioned (around lines where methods like signIn/signUp/signOut/getCurrentUser are implemented) with appropriate log levels such as _logger.warning(...) or _logger.severe(...). Alternatively, if you prefer framework-safe no-op in release builds, use Flutter's debugPrint(...) instead of print(...); ensure consistent usage across the four occurrences referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@therapist/lib/provider/auth_provider.dart`:
- Around line 137-149: The signOut method currently swallows all exceptions and
always clears local auth state; change it so the catch captures the exception
and stack trace (e.g., catch (e, st)) and logs them (use debugPrint or your app
Logger) instead of an empty block, and only clear/reset _isAuthenticated,
_userId, _userEmail, _userPhone, _userName, and _isNewUser (and call
notifyListeners()) after a successful sign-out attempt (i.e., after await
GoogleSignIn().signOut() / await _supabaseClient.auth.signOut() complete); if
sign-out fails, log the error and keep current local state or surface the error
to the caller.
In `@therapist/lib/repository/supabase_auth_repository.dart`:
- Around line 119-124: The map construction for `data` currently spreads
`personalInfoEntity.toMap()` after explicit keys which allows its `id` to
overwrite `currentUser.id`; change the order so `...personalInfoEntity.toMap()`
comes before the explicit fields (`'id': currentUser.id`, `'email':
currentUser.email ?? ''`, `'phone': phone`) so the explicit values in the `data`
map take precedence (alternatively remove `id` from `personalInfoEntity.toMap()`
if that better fits the domain).
- Line 121: The code currently writes 'email': currentUser.email ?? '' which
silently stores an empty string and can violate data integrity; update the
method that builds the user record (e.g., the function/method that references
currentUser.email in SupabaseAuthRepository—look for the record creation/update
logic in SupabaseAuthRepository or the method name that inserts the user) to
explicitly validate currentUser.email and fail fast: if currentUser.email is
null/empty throw or return a clear error/result indicating missing email instead
of writing an empty string, and adjust callers to handle that error path
accordingly so no invalid empty-email rows are inserted.
---
Nitpick comments:
In `@therapist/lib/core/repository/auth/auth_repository.dart`:
- Around line 29-30: Remove the leftover inline instruction comment "Add this
new method" and replace it with a concise Dart doc comment for the method
declaration checkIfUserIsNew(String userId) describing what the method does, the
meaning of the returned Future<ActionResult>, and any expectations for the
userId parameter (e.g., non-null/format). Locate the declaration of
Future<ActionResult> checkIfUserIsNew in the auth repository interface/class and
add a /// comment above it that documents behavior, inputs, and output rather
than leaving an instruction-style comment.
In `@therapist/lib/presentation/auth/personal_details_screen.dart`:
- Around line 190-201: The phone field validator currently only checks for
emptiness; update the validator passed to _buildTextField (where phoneController
is used) to also enforce a basic phone format: trim the value, remove allowed
characters (spaces, dashes, parentheses, optional leading '+'), then check that
the remaining characters are all digits and meet a minimum length (e.g., >=7)
and maximum (e.g., <=15); return specific error messages like "Please enter a
valid phone number" when the checks fail and null when valid.
In `@therapist/lib/repository/supabase_auth_repository.dart`:
- Line 111: Replace all print(...) calls inside SupabaseAuthRepository with a
proper logger: add import 'package:logging/logging.dart'; create a private
Logger (e.g., final _logger = Logger('SupabaseAuthRepository')) and replace
prints at the locations mentioned (around lines where methods like
signIn/signUp/signOut/getCurrentUser are implemented) with appropriate log
levels such as _logger.warning(...) or _logger.severe(...). Alternatively, if
you prefer framework-safe no-op in release builds, use Flutter's debugPrint(...)
instead of print(...); ensure consistent usage across the four occurrences
referenced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3c6879e-cb76-4fbb-9043-c899e635ac34
📒 Files selected for processing (4)
therapist/lib/core/repository/auth/auth_repository.darttherapist/lib/presentation/auth/personal_details_screen.darttherapist/lib/provider/auth_provider.darttherapist/lib/repository/supabase_auth_repository.dart
Comment on lines
+137
to
+149
| Future<void> signOut() async { | ||
| try { | ||
| if (!kIsWeb) await GoogleSignIn().signOut(); | ||
| await _supabaseClient.auth.signOut(); | ||
| } catch (_) {} | ||
| _isAuthenticated = false; | ||
| _userId = null; | ||
| _userEmail = null; | ||
| _userPhone = ''; | ||
| _userName = ''; | ||
| _isNewUser = true; | ||
| notifyListeners(); | ||
| } |
There was a problem hiding this comment.
Empty catch block silently swallows sign-out errors.
The catch (_) {} block discards all exceptions without logging. If sign-out fails (e.g., network issue), the user won't know and local state will still be cleared, potentially causing inconsistency.
🔧 Suggested improvement
Future<void> signOut() async {
try {
if (!kIsWeb) await GoogleSignIn().signOut();
await _supabaseClient.auth.signOut();
- } catch (_) {}
+ } catch (e) {
+ debugPrint('Sign out error: $e');
+ // Continue to clear local state even on error
+ }
_isAuthenticated = false;📝 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
| Future<void> signOut() async { | |
| try { | |
| if (!kIsWeb) await GoogleSignIn().signOut(); | |
| await _supabaseClient.auth.signOut(); | |
| } catch (_) {} | |
| _isAuthenticated = false; | |
| _userId = null; | |
| _userEmail = null; | |
| _userPhone = ''; | |
| _userName = ''; | |
| _isNewUser = true; | |
| notifyListeners(); | |
| } | |
| Future<void> signOut() async { | |
| try { | |
| if (!kIsWeb) await GoogleSignIn().signOut(); | |
| await _supabaseClient.auth.signOut(); | |
| } catch (e) { | |
| debugPrint('Sign out error: $e'); | |
| // Continue to clear local state even on error | |
| } | |
| _isAuthenticated = false; | |
| _userId = null; | |
| _userEmail = null; | |
| _userPhone = ''; | |
| _userName = ''; | |
| _isNewUser = true; | |
| notifyListeners(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@therapist/lib/provider/auth_provider.dart` around lines 137 - 149, The
signOut method currently swallows all exceptions and always clears local auth
state; change it so the catch captures the exception and stack trace (e.g.,
catch (e, st)) and logs them (use debugPrint or your app Logger) instead of an
empty block, and only clear/reset _isAuthenticated, _userId, _userEmail,
_userPhone, _userName, and _isNewUser (and call notifyListeners()) after a
successful sign-out attempt (i.e., after await GoogleSignIn().signOut() / await
_supabaseClient.auth.signOut() complete); if sign-out fails, log the error and
keep current local state or surface the error to the caller.
hasanravda
changed the title
Member
Author
|
@jddeep Can you review this ! As this is solving the issue that is critical during registration. |
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
1 participant
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.
Closes #192
📝 Description
During therapist registration, the
storePersonalInfoinsert payload was missingemailandphone, both of which areNOT NULLin thetherapisttable. This caused aPostgrestExceptionon every new therapist sign-up. This PR fixes the crash by sourcingemailfrom the authenticated Google user and collectingphonevia a new required input field in the registration form.🔧 Changes Made
supabase_auth_repository.dart- addedemail(fromcurrentUser.email) andphone(from param) to the insert payload instorePersonalInfoauth_repository.dart- updatedstorePersonalInfointerface signature to require{required String phone}auth_provider.dart- forwardedphoneparameter through to the repository callpersonal_details_screen.dart- added a required Phone Number input field to the registration form✅ Checklist
Summary by CodeRabbit
Release Notes