Agent screen#26
Closed
Khallad1424 wants to merge 3 commits intomainfrom
Closed
Conversation
…e database integration
…tabase integration and map picker support
…lite dependencies
📝 WalkthroughWalkthroughThe PR implements comprehensive user authentication and registration infrastructure by introducing SQLite database persistence with user credential storage, converting registration screens to validated forms with location and document selection capabilities, integrating map-based location picking with reverse geocoding, and implementing login flow with database-backed credential validation across mobile and web platforms. Changes
Sequence DiagramssequenceDiagram
participant User
participant RegForm as Registration Form
participant MapPicker as Map Picker Screen
participant Geocoder as Geocoding Service
participant DB as Database (SQLite)
User->>RegForm: Fill form & tap "تأكيد الموقع"
RegForm->>MapPicker: Navigate to location selection
User->>MapPicker: Pan/select location on map
User->>MapPicker: Tap "تأكيد الموقع"
MapPicker->>Geocoder: reverse-geocode coordinates
Geocoder-->>MapPicker: Return address placemark
MapPicker->>MapPicker: Build Arabic address string
MapPicker-->>RegForm: Return coordinates & address
User->>RegForm: Fill remaining fields & submit
RegForm->>RegForm: Validate all inputs
RegForm->>DB: registerUser(userData map)
DB-->>RegForm: User inserted with ID
RegForm-->>User: Success snackbar & navigate to login
sequenceDiagram
participant User
participant LoginForm as Login Form
participant DB as Database (SQLite)
participant Router as Navigation
User->>LoginForm: Enter phone & password
User->>LoginForm: Tap Login button
LoginForm->>LoginForm: Validate form fields
LoginForm->>LoginForm: Set loading = true
LoginForm->>DB: loginUser(phone, password)
DB->>DB: Query users table by phone & password
DB-->>LoginForm: Return user record or null
alt User found
LoginForm->>Router: Navigate to /customer_home
Router-->>User: Display customer home screen
else User not found or error
LoginForm->>User: Show error snackbar
end
LoginForm->>LoginForm: Set loading = false
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
brightcleanproject/pubspec.yaml (1)
1-1:⚠️ Potential issue | 🟡 MinorTypo in package name:
brightcleanprojet.
name: brightcleanprojetis missing ac(likely intendedbrightcleanproject). This name becomes the import prefix for the package and any downstream import paths — fixing it later will require updates to everypackage:brightcleanproject/...import. Consider correcting now while the project is still small.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brightcleanproject/pubspec.yaml` at line 1, The package name value "brightcleanprojet" in the pubspec (the YAML key/value "name: brightcleanprojet") is misspelled; change it to "brightcleanproject" and then run the package manager (flutter pub get / dart pub get) to refresh locks; also search the repo for any existing imports using the old package prefix "package:brightcleanprojet/" and update them to "package:brightcleanproject/" so all package import paths remain consistent.
🧹 Nitpick comments (8)
brightcleanproject/web/index.html (1)
36-48: Useconsole.errorfor the failure branch and be aware of the load/bootstrap race.Two small things on the inline registration script:
- The catch handler logs registration failures via
console.log; usingconsole.errorsurfaces them at the right severity in DevTools and any log aggregation, which matters becausedatabase_helper.dartrelies on this worker and only fails after a 15s timeout — making early visibility important.flutter_bootstrap.jsis loaded withasync, so Flutter (and therefore_initDatabase) can start before theload-gatedserviceWorker.register(...)resolves. The 15s timeout indatabase_helper.dartmasks this, but on slow connections the very first DB call may still race. Consider registering the SW eagerly (outside theloadlistener) so registration starts as early as possible.♻️ Suggested tweak
- <script> - // تسجيل الخدمة اليدوية لقاعدة البيانات - window.addEventListener('load', function() { - if ('serviceWorker' in navigator) { - navigator.serviceWorker.register('sqflite_sw.js') - .then(function(registration) { - console.log('SQFLite Service Worker registered successfully:', registration.scope); - }).catch(function(error) { - console.log('SQFLite ServiceWorker registration failed:', error); - }); - } - }); - </script> + <script> + // Register the sqflite service worker as early as possible so the + // Flutter bootstrap (loaded async below) can use it without racing. + if ('serviceWorker' in navigator) { + navigator.serviceWorker.register('sqflite_sw.js') + .then(function(registration) { + console.log('SQFLite Service Worker registered:', registration.scope); + }) + .catch(function(error) { + console.error('SQFLite Service Worker registration failed:', error); + }); + } + </script>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brightcleanproject/web/index.html` around lines 36 - 48, The registration failure path uses console.log and delays SW startup until window.load, which hides errors and can race with Flutter's _initDatabase in database_helper.dart; change the catch to use console.error when calling navigator.serviceWorker.register('sqflite_sw.js') and move the register call out of window.addEventListener('load') so registration starts immediately (still guard with 'serviceWorker' in navigator) to reduce bootstrap race with flutter_bootstrap.js and _initDatabase.brightcleanproject/lib/core/widgets/map_picker_screen.dart (2)
14-19: Hard-coded default center will be far from many users.Defaulting to Jeddah is reasonable for an MVP, but if the OS gives you location permission you'll want to center on the user's actual position. Even without permission, you may want to expose this default as a constant (or read from config) so it's obvious where to tweak it for other regions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brightcleanproject/lib/core/widgets/map_picker_screen.dart` around lines 14 - 19, The hard-coded Jeddah LatLng in _MapPickerScreenState (_currentCenter) should be replaced so the map centers on the user's actual position when available and the default is configurable; implement location fetching and permission checks in initState (or a helper like _initLocation) to request permission, get the device location and update _currentCenter and _mapController accordingly, and if the location is unavailable fall back to a named constant (e.g., kDefaultMapCenter) or a value read from config so the default isn’t buried in the field initializer.
30-53: Web users will always get raw coordinates, never a human-readable address.The
kIsWebbranch unconditionally falls back to the lat/lng-formatted string. For a customer/agent registering on the web build, the saved "address" will literally readموقع محدد (21.5433, 39.1728), which is not useful for fulfillment/dispatch later. Consider calling a free reverse-geocoding HTTP API (e.g. Nominatim with a proper user agent and rate-limit caching, or Google/Mapbox if licensed) on web so both platforms produce comparable data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brightcleanproject/lib/core/widgets/map_picker_screen.dart` around lines 30 - 53, The web branch currently bypasses reverse geocoding (kIsWeb) and leaves addressName as raw coordinates; implement a web-side reverse-geocoding call instead of the debug fallback by adding an HTTP request to a free geocoding API (e.g., Nominatim) when kIsWeb is true: use _currentCenter.latitude and _currentCenter.longitude to query the API, parse the returned address components into the same elements list format used in the placemarkFromCoordinates branch, then set addressName accordingly (joining elements with '، '); ensure you include a proper User-Agent, basic error handling/fallback to the coordinate string, and consider simple caching/rate-limit handling to avoid excessive requests (modify the kIsWeb branch around placemarkFromCoordinates and addressName).brightcleanproject/lib/features/auth/presentation/customer_registration_screen.dart (2)
1-9: Consider usingSafeAreaand aresizeToAvoidBottomInset-aware layout to satisfy issue#4acceptance criterion 3.Issue
#4explicitly calls out "improve form scrolling on small screens to prevent the keyboard from covering form fields."SingleChildScrollViewalready helps here, but for the agent screen (which has many more fields than the customer one) you'll want to:
- wrap the
Scaffoldbodyin aSafeArea,- ensure
Scaffold.resizeToAvoidBottomInset: true(the default) is preserved,- add
padding: EdgeInsets.only(bottom: MediaQuery.of(context).viewInsets.bottom + 24)to theSingleChildScrollViewso the focused field can scroll above the keyboard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brightcleanproject/lib/features/auth/presentation/customer_registration_screen.dart` around lines 1 - 9, Wrap the Scaffold body in a SafeArea and ensure Scaffold.resizeToAvoidBottomInset remains true, then update the SingleChildScrollView (used in customer registration) to include padding: EdgeInsets.only(bottom: MediaQuery.of(context).viewInsets.bottom + 24) so form fields can scroll above the keyboard; locate the Scaffold and its SingleChildScrollView in customer_registration_screen.dart and apply these changes to improve form scrolling on small screens.
191-205: Password validator has redundant/contradictory checks.Line 198 already requires
[A-Z], and line 201 requires[a-zA-Z\u0600-\u06FF]or[0-9](note the||) — but with the prior uppercase requirement the letter half is always satisfied, so 201 effectively only enforces "must contain a digit". Either drop line 201 or rewrite it as a positive "must contain digit" check, mirroring the agent screen (_validatePasswordlines 254‑260) which is cleaner.♻️ Proposed fix
- if (!RegExp(r'[a-zA-Z\u0600-\u06FF]').hasMatch(value) || !RegExp(r'[0-9]').hasMatch(value)) { - return 'يجب أن تحتوي على حرف ورقم على الأقل'; - } + if (!RegExp(r'[0-9]').hasMatch(value)) { + return 'يجب أن تحتوي على رقم واحد على الأقل'; + }Consider extracting the validators shared with
agent_registration_screen.dartinto a singlecore/validators.dartto remove the duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brightcleanproject/lib/features/auth/presentation/customer_registration_screen.dart` around lines 191 - 205, The _validatePassword function currently has a redundant/contradictory check: after requiring an uppercase letter with RegExp(r'[A-Z]'), the subsequent condition uses RegExp(r'[a-zA-Z\u0600-\u06FF]') || RegExp(r'[0-9]') which makes the letter-side always true and thus only enforces a digit; replace that combined condition with an explicit digit-only check (e.g. validate using RegExp(r'[0-9]')) so the validator clearly enforces: non-empty, min length 8, at least one uppercase, and at least one digit; optionally refactor this logic into a shared validator (e.g., core/validators.dart) matching the agent screen's _validatePassword implementation to remove duplication.brightcleanproject/lib/features/auth/presentation/login_screen.dart (1)
33-33: UseFuture<void>instead ofvoidfor async handlers.
void _handleLogin() asyncmakes the function fire-and-forget; any thrown exception that escapes thetry/catchbecomes an unawaitable error and analysis tools (e.g.avoid_void_async/unawaited_futures) can't reason about callers. PreferFuture<void> _handleLogin() async.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brightcleanproject/lib/features/auth/presentation/login_screen.dart` at line 33, Change the async handler declaration from void to Future<void> for _handleLogin so callers and static analysis can await it; update the signature to Future<void> _handleLogin() async in login_screen.dart and ensure any call sites (e.g., button onPressed or other invocations) either await the call or explicitly handle the returned Future so exceptions are not fire-and-forget.brightcleanproject/lib/features/auth/presentation/agent_registration_screen.dart (2)
96-109:initialDateof year 2000 may exceed allowed range and there's no minimum-age check.
showDatePickerrequiresinitialDateto lie within[firstDate, lastDate]; today (DateTime.now()) easily satisfies that, but a future maintainer changinglastDatecould trigger an assertion. More importantly, agents are presumably adults, but the picker allows selecting any date up to today — consider clampinglastDateto e.g.DateTime(now.year - 18, now.month, now.day)to enforce a minimum age at the picker level (and mirror the same incustomer_registration_screen.dartlines 50‑55).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brightcleanproject/lib/features/auth/presentation/agent_registration_screen.dart` around lines 96 - 109, The _selectDOB method uses showDatePicker with initialDate fixed to DateTime(2000) and lastDate set to DateTime.now(), which can break the date-picker assertion and doesn't enforce a minimum age; update _selectDOB to compute now = DateTime.now(), compute an adultCutoff = DateTime(now.year - 18, now.month, now.day) and use adultCutoff as lastDate (so users must be >=18), then clamp initialDate to lie between firstDate (e.g., DateTime(1900)) and adultCutoff before passing it to showDatePicker, and keep setting _dobController.text as before; apply the same change pattern to the customer_registration_screen.dart date-picker implementation referenced in the comment.
132-175: Document placeholder lacks a "remove/replace" affordance and an accessibility label.Once a file is selected, the only way to change it is to re-tap and pick again; there is no way to clear/remove the selection. Also,
GestureDetector+Containeris not announced as a button by screen readers — wrap it withSemantics(button: true, label: title)(or useInkWell/OutlinedButton.icon) so assistive tech can describe the upload action.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@brightcleanproject/lib/features/auth/presentation/agent_registration_screen.dart` around lines 132 - 175, The upload tile built with GestureDetector/Container (using isUploaded, imageFile, title, onTap) lacks an accessibility role and a clear way to remove/replace the file; wrap or replace the GestureDetector with a Semantics widget (e.g., Semantics(button: true, label: title)) or use an interactive Material widget (InkWell/OutlinedButton.icon) so screen readers announce it as a button, and when isUploaded is true render a visible remove/replace affordance (e.g., a trailing IconButton or small overlay “remove” icon) that calls a new clear callback (or reuses onTap with a mode) to clear imageFile and reset isUploaded so users can remove the selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@brightcleanproject/lib/core/database/database_helper.dart`:
- Around line 67-80: The users table schema in _createDB must mark phone as
UNIQUE to prevent duplicate accounts—update the CREATE TABLE statement in
_createDB to add UNIQUE to the phone column; then update registerUser to catch
DatabaseException (inspect exception code for SQLITE_CONSTRAINT) and return a
friendly "phone already registered" error instead of a generic failure. Also
modify loginUser to query WHERE phone = ? first and then verify the password
hash against the returned record (rather than relying on results.first), so
authentication is deterministic and safe.
- Around line 61-65: The _upgradeDB function currently runs `DROP TABLE IF
EXISTS users` which will erase all accounts on any schema version bump; update
_upgradeDB (and ensure callers via _initDatabase) to avoid destructive drops in
production by either wrapping the DROP logic behind a debug-only guard (use
assert or kDebugMode) or replace it with proper non-destructive migrations
(ALTER TABLE / data-preserving steps) that handle oldVersion→newVersion
upgrades. Locate the `_upgradeDB` function and change its behavior so production
runs perform safe migrations while debug/dev runs may still drop+recreate for
convenience.
- Around line 36-39: Replace the current .timeout(... onTimeout: ...) usage in
database_helper.dart with a try/catch around the await that calls timeout so you
can log with debugPrint and preserve/rethrow the original error: remove the
onTimeout handler, wrap the future call that uses .timeout(const
Duration(seconds: 15)) in a try { ... } on TimeoutException catch (e, st) {
debugPrint("Service worker initialization timed out: $e\n$st"); rethrow; } and
for other exceptions use catch (e, st) { debugPrint("Service worker
initialization failed: $e\n$st"); rethrow; } — this keeps debugPrint (from
material.dart) instead of print and preserves the original exception and stack
trace for functions/blocks around the timeout call.
- Around line 13-17: The lazy getter DatabaseHelper.instance.database has a race
where multiple callers can call _initDatabase concurrently; fix it by
introducing a single-flight Future/Completer (e.g. a private Future<Database>?
_initializingDatabase or Completer<Database>? _dbCompleter) that you set before
calling _initDatabase, have subsequent callers await that future instead of
calling _initDatabase again, and on completion assign _database and clear the
single-flight field (and complete/completeError the completer on success/error);
update the getter to check _database first, then the single-flight future, and
only call _initDatabase when neither exists (also handle clearing the future on
errors to allow retries).
- Around line 67-93: Schema and auth currently store plaintext passwords (see
_createDB creating users.password and seed insert, and places using db.insert in
registerUser and WHERE phone = ? AND password = ? in loginUser); change to store
Argon2id hashes and salts: alter the users schema in _createDB to add
password_salt TEXT NOT NULL, remove plaintext seeds or replace password with a
hashed value, and in registerUser generate a secure random salt, derive an
Argon2id key from the provided password (using the cryptography package
Argon2id), base64-encode and store both the hash and salt via db.insert; in
loginUser query by phone only, retrieve password_salt and stored password hash,
derive the Argon2id key from the supplied password and the stored salt,
base64-encode and compare to the stored hash to authenticate.
In `@brightcleanproject/lib/core/widgets/map_picker_screen.dart`:
- Around line 56-67: The finally block calls setState(() { _isLoading = false;
}) without checking mounted, which can throw after dispose; update the finally
to check mounted before calling setState (e.g. if (mounted) setState(() {
_isLoading = false; });) and keep the existing mounted check for Navigator.pop
that uses _currentCenter and addressName, so both state update and navigation
only run when mounted.
- Around line 86-91: onPositionChanged is calling setState on every camera frame
causing excessive rebuilds; remove the setState call inside the
onPositionChanged callback and update the _currentCenter field directly (e.g.,
_currentCenter = camera.center) so the map/Stack/FlutterMap aren't rebuilt
continuously, and only call setState when the user commits the selection (e.g.,
inside _confirmLocation) if UI needs to reflect the confirmed center; also
ensure the fixed center pin is a const Center (not driven by _currentCenter) so
it doesn't rely on rebuilds.
- Around line 93-96: The TileLayer call currently uses a placeholder
userAgentPackageName ('com.example.brightcleanprojet'); update the TileLayer's
userAgentPackageName to your real production bundle identifier (the Android
applicationId or iOS CFBundleIdentifier) so OSM tiles receive a valid User-Agent
(e.g., change in the TileLayer widget to 'com.brightclean.app' or your actual
app id); ensure the value set in userAgentPackageName in TileLayer matches the
app's configured applicationId/CFBundleIdentifier.
In
`@brightcleanproject/lib/features/auth/presentation/agent_registration_screen.dart`:
- Around line 437-446: The form currently allows submission without uploaded
documents because _submitForm does not validate _commercialRegImage or _idImage;
update _submitForm to include non-null checks for _commercialRegImage and
_idImage (same pattern used for _selectedAddress) and prevent submission if
either is null by setting state (e.g., set _hasAttemptedSubmit = true and
return) so validation runs. Then surface inline errors under each placeholder in
the UI by either extending _buildFileUploadPlaceholder to accept an optional
errorText or by rendering a Text error widget directly below each
_buildFileUploadPlaceholder when _hasAttemptedSubmit && imageFile == null (use
_commercialRegImage and _idImage to decide which error to show). Ensure error
messaging appears only after an attempted submit and follows existing styling
conventions.
- Around line 363-367: The validator on CustomTextField using
_businessNameController currently allows whitespace-only and unbounded names;
update the validator for the business name field (the CustomTextField using
_businessNameController) to check value.trim().isEmpty and enforce a sane
maximum length (e.g. 100 chars) — return the existing "يرجى إدخال اسم المغسلة"
for empty/whitespace-only and a new message for too-long values; also add a
maxLength/inputFormatter to the CustomTextField if it supports it to prevent
oversized input at the UI level.
- Around line 246-252: The email validator in _validateEmail uses a regex that
limits TLD length to {2,4}, rejecting valid longer TLDs; replace the pattern
RegExp(r'^[\w-\.]+@([\w-]+\.)+[\w-]{2,4}$') with a more permissive one such as
RegExp(r'^[\w\-.]+@([\w\-]+\.)+[\w\-]{2,}$') (or switch to a vetted
email-validation package) and update the identical occurrence in the other
_validateEmail implementation (customer registration) so both screens use the
new pattern; keep the existing null/empty checks and Arabic messages unchanged.
- Around line 268-300: _submitForm currently only validates and shows a snackbar
but never persists the agent to the database or navigates, so implement
persistence and UX state: call DatabaseHelper.instance.registerUser(...) with
the constructed user/agent payload (include agent-specific fields like
businessName, selectedServices, document paths, coordinates) inside _submitForm
after successful validation, wrap the operation with a loading state boolean
(e.g., _isSubmitting) set with setState before/after the await to disable UI and
show a progress indicator, handle exceptions to show an error SnackBar on
failure, and on success navigate away (Navigator.pop or pushReplacement to
LoginScreen or a success screen) and show a success SnackBar; update
database_helper.dart schema as needed to store agent fields and ensure
registerUser returns a meaningful result to branch success vs failure.
In
`@brightcleanproject/lib/features/auth/presentation/customer_registration_screen.dart`:
- Around line 345-349: The validation message for location is shown immediately
because there's no submit-attempt guard; add a boolean state field named
_hasAttemptedSubmit initialized to false and set it to true at the start of the
_submitForm method, then change the conditional that shows the red Text (which
currently checks _selectedAddress == null) to require both _selectedAddress ==
null && _hasAttemptedSubmit so the error only appears after a user attempts to
submit (mirroring the pattern used in agent_registration_screen.dart).
- Around line 92-148: The form currently validates but never enforces or
persists the map-picked address: add a check similar to
agent_registration_screen.dart (guarding on _selectedAddress != null) before
proceeding after _formKey.currentState!.validate(), and include the selected
address and coordinates in the userData map (add keys like 'address',
'latitude', 'longitude' populated from _selectedAddress and
_selectedCoordinates) so DatabaseHelper.registerUser receives them; update
DatabaseHelper._createDB to add address/latitude/longitude columns and adjust
registerUser to insert those fields. Ensure UI feedback matches the validation
(show SnackBar when _selectedAddress is null) and keep the existing kIsWeb
bypass logic for coordinates.
- Around line 128-139: The catch block that builds errorMessage (inside the
catch (e) where you show a SnackBar via
ScaffoldMessenger.of(context).showSnackBar) should not rely on the Arabic
substring check; instead check the exception type (e.g. use `if (e is
DatabaseException)` or inspect `e.code` if available) and set a localized
DB-specific message like 'تعذر الاتصال بقاعدة البيانات المحلية' for that branch,
otherwise set a generic localized message without interpolating the raw `$e`;
then call the existing SnackBar display with the safe message. Ensure you
import/resolve `DatabaseException` from sqflite (or the appropriate DB exception
type) and keep the mounted check and SnackBar call as-is.
In `@brightcleanproject/lib/features/auth/presentation/login_screen.dart`:
- Around line 41-42: Remove the artificial 1.5s delay by deleting the await
Future.delayed(const Duration(milliseconds: 1500)) call in login_screen.dart
(the line containing Future.delayed(const Duration(milliseconds: 1500))). This
line currently lives in the login flow (e.g., the handler that performs
authentication) and should be removed so authentication uses the actual SQLite
round-trip time; keep the surrounding authentication logic and spinner state
management intact.
- Around line 50-63: The current post-login routing always sends authenticated
users to '/customer_home' even though loginUser returns a user record with a
role; update the routing logic in the block that checks if (mounted) after login
to inspect user['role'] (from the loginUser result) and branch accordingly
(e.g., context.go('/agent_home') for role == 'agent',
context.go('/customer_home') for role == 'customer'), and handle unrecognized
roles by showing an error SnackBar or failing the login; keep the existing
mounted check and Snackbar usage (ScaffoldMessenger.of(context).showSnackBar)
and only change the branch that currently always calls
context.go('/customer_home').
In `@brightcleanproject/pubspec.yaml`:
- Line 48: Remove the unused dependency entry "file_picker: ^11.0.2" from
pubspec.yaml; search the codebase for any imports or references to "file_picker"
or "FilePicker" to confirm there are no usages (the project uses "image_picker"
instead), delete the dependency line, run flutter pub get to update lockfiles,
and if present remove any platform-specific configuration or Android/iOS
permission entries that were added only for file_picker.
In `@brightcleanproject/web/sqflite_sw.js`:
- Around line 1-2: The service worker currently imports sql.js and sqflite_sw.js
from unpkg via importScripts, which is unsafe and unreliable; instead vendor the
runtime assets locally (run dart run sqflite_common_ffi_web:setup to copy
sqflite_sw.js and the matching sqlite3.wasm/sql-wasm.js into your project's
web/), replace the external importScripts calls in sqflite_sw.js with relative
local paths to those vendored files, and ensure the vendored sql.js/wasm version
matches the version resolved in pubspec.lock (e.g., sql.js 1.8.0 / corresponding
sqlite3.wasm expected by sqflite_common_ffi_web version) so the worker and Dart
package remain compatible.
---
Outside diff comments:
In `@brightcleanproject/pubspec.yaml`:
- Line 1: The package name value "brightcleanprojet" in the pubspec (the YAML
key/value "name: brightcleanprojet") is misspelled; change it to
"brightcleanproject" and then run the package manager (flutter pub get / dart
pub get) to refresh locks; also search the repo for any existing imports using
the old package prefix "package:brightcleanprojet/" and update them to
"package:brightcleanproject/" so all package import paths remain consistent.
---
Nitpick comments:
In `@brightcleanproject/lib/core/widgets/map_picker_screen.dart`:
- Around line 14-19: The hard-coded Jeddah LatLng in _MapPickerScreenState
(_currentCenter) should be replaced so the map centers on the user's actual
position when available and the default is configurable; implement location
fetching and permission checks in initState (or a helper like _initLocation) to
request permission, get the device location and update _currentCenter and
_mapController accordingly, and if the location is unavailable fall back to a
named constant (e.g., kDefaultMapCenter) or a value read from config so the
default isn’t buried in the field initializer.
- Around line 30-53: The web branch currently bypasses reverse geocoding
(kIsWeb) and leaves addressName as raw coordinates; implement a web-side
reverse-geocoding call instead of the debug fallback by adding an HTTP request
to a free geocoding API (e.g., Nominatim) when kIsWeb is true: use
_currentCenter.latitude and _currentCenter.longitude to query the API, parse the
returned address components into the same elements list format used in the
placemarkFromCoordinates branch, then set addressName accordingly (joining
elements with '، '); ensure you include a proper User-Agent, basic error
handling/fallback to the coordinate string, and consider simple
caching/rate-limit handling to avoid excessive requests (modify the kIsWeb
branch around placemarkFromCoordinates and addressName).
In
`@brightcleanproject/lib/features/auth/presentation/agent_registration_screen.dart`:
- Around line 96-109: The _selectDOB method uses showDatePicker with initialDate
fixed to DateTime(2000) and lastDate set to DateTime.now(), which can break the
date-picker assertion and doesn't enforce a minimum age; update _selectDOB to
compute now = DateTime.now(), compute an adultCutoff = DateTime(now.year - 18,
now.month, now.day) and use adultCutoff as lastDate (so users must be >=18),
then clamp initialDate to lie between firstDate (e.g., DateTime(1900)) and
adultCutoff before passing it to showDatePicker, and keep setting
_dobController.text as before; apply the same change pattern to the
customer_registration_screen.dart date-picker implementation referenced in the
comment.
- Around line 132-175: The upload tile built with GestureDetector/Container
(using isUploaded, imageFile, title, onTap) lacks an accessibility role and a
clear way to remove/replace the file; wrap or replace the GestureDetector with a
Semantics widget (e.g., Semantics(button: true, label: title)) or use an
interactive Material widget (InkWell/OutlinedButton.icon) so screen readers
announce it as a button, and when isUploaded is true render a visible
remove/replace affordance (e.g., a trailing IconButton or small overlay “remove”
icon) that calls a new clear callback (or reuses onTap with a mode) to clear
imageFile and reset isUploaded so users can remove the selection.
In
`@brightcleanproject/lib/features/auth/presentation/customer_registration_screen.dart`:
- Around line 1-9: Wrap the Scaffold body in a SafeArea and ensure
Scaffold.resizeToAvoidBottomInset remains true, then update the
SingleChildScrollView (used in customer registration) to include padding:
EdgeInsets.only(bottom: MediaQuery.of(context).viewInsets.bottom + 24) so form
fields can scroll above the keyboard; locate the Scaffold and its
SingleChildScrollView in customer_registration_screen.dart and apply these
changes to improve form scrolling on small screens.
- Around line 191-205: The _validatePassword function currently has a
redundant/contradictory check: after requiring an uppercase letter with
RegExp(r'[A-Z]'), the subsequent condition uses RegExp(r'[a-zA-Z\u0600-\u06FF]')
|| RegExp(r'[0-9]') which makes the letter-side always true and thus only
enforces a digit; replace that combined condition with an explicit digit-only
check (e.g. validate using RegExp(r'[0-9]')) so the validator clearly enforces:
non-empty, min length 8, at least one uppercase, and at least one digit;
optionally refactor this logic into a shared validator (e.g.,
core/validators.dart) matching the agent screen's _validatePassword
implementation to remove duplication.
In `@brightcleanproject/lib/features/auth/presentation/login_screen.dart`:
- Line 33: Change the async handler declaration from void to Future<void> for
_handleLogin so callers and static analysis can await it; update the signature
to Future<void> _handleLogin() async in login_screen.dart and ensure any call
sites (e.g., button onPressed or other invocations) either await the call or
explicitly handle the returned Future so exceptions are not fire-and-forget.
In `@brightcleanproject/web/index.html`:
- Around line 36-48: The registration failure path uses console.log and delays
SW startup until window.load, which hides errors and can race with Flutter's
_initDatabase in database_helper.dart; change the catch to use console.error
when calling navigator.serviceWorker.register('sqflite_sw.js') and move the
register call out of window.addEventListener('load') so registration starts
immediately (still guard with 'serviceWorker' in navigator) to reduce bootstrap
race with flutter_bootstrap.js and _initDatabase.
🪄 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: 7dd09e1e-8518-4b86-a41f-238a0161adfe
⛔ Files ignored due to path filters (1)
brightcleanproject/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
brightcleanproject/lib/core/database/database_helper.dartbrightcleanproject/lib/core/widgets/custom_text_field.dartbrightcleanproject/lib/core/widgets/map_picker_screen.dartbrightcleanproject/lib/features/auth/presentation/agent_registration_screen.dartbrightcleanproject/lib/features/auth/presentation/customer_registration_screen.dartbrightcleanproject/lib/features/auth/presentation/login_screen.dartbrightcleanproject/linux/flutter/generated_plugin_registrant.ccbrightcleanproject/linux/flutter/generated_plugins.cmakebrightcleanproject/macos/Flutter/GeneratedPluginRegistrant.swiftbrightcleanproject/pubspec.yamlbrightcleanproject/web/index.htmlbrightcleanproject/web/sqflite_sw.jsbrightcleanproject/windows/flutter/generated_plugin_registrant.ccbrightcleanproject/windows/flutter/generated_plugins.cmake
Comment on lines
+13
to
+17
| Future<Database> get database async { | ||
| if (_database != null) return _database!; | ||
| _database = await _initDatabase('brightclean.db'); | ||
| return _database!; | ||
| } |
There was a problem hiding this comment.
Race condition in the lazy database getter.
If two callers await DatabaseHelper.instance.database concurrently before _database is initialized, both will see _database == null and both will call _initDatabase (and on web, both will reassign the global databaseFactory). Wrap the initialization in a single-flight Completer<Database>? (or a Future<Database>?) so only the first caller opens the DB and the rest await the same future.
🛠️ Proposed fix
- static Database? _database;
+ static Database? _database;
+ static Future<Database>? _opening;
DatabaseHelper._init();
Future<Database> get database async {
if (_database != null) return _database!;
- _database = await _initDatabase('brightclean.db');
- return _database!;
+ _opening ??= _initDatabase('brightclean.db').then((db) {
+ _database = db;
+ return db;
+ });
+ return _opening!;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@brightcleanproject/lib/core/database/database_helper.dart` around lines 13 -
17, The lazy getter DatabaseHelper.instance.database has a race where multiple
callers can call _initDatabase concurrently; fix it by introducing a
single-flight Future/Completer (e.g. a private Future<Database>?
_initializingDatabase or Completer<Database>? _dbCompleter) that you set before
calling _initDatabase, have subsequent callers await that future instead of
calling _initDatabase again, and on completion assign _database and clear the
single-flight field (and complete/completeError the completer on success/error);
update the getter to check _database first, then the single-flight future, and
only call _initDatabase when neither exists (also handle clearing the future on
errors to allow retries).
Comment on lines
+36
to
+39
| ).timeout(const Duration(seconds: 15), onTimeout: () { | ||
| print("ERROR: Service worker initialization timed out or failed to connect."); | ||
| throw Exception("خطأ في تهيئة قاعدة البيانات - تأكد من رسائل F12 (Service Worker Timeout)"); | ||
| }); |
There was a problem hiding this comment.
Use debugPrint instead of print, and don't lose the original error.
print lands in production logs (and gets rate-limited on Android). Also, the onTimeout swaps the underlying error for a string with no inner cause, making field debugging harder. Use debugPrint (already imported via material.dart) and include the original exception when available.
♻️ Proposed fix
- ).timeout(const Duration(seconds: 15), onTimeout: () {
- print("ERROR: Service worker initialization timed out or failed to connect.");
- throw Exception("خطأ في تهيئة قاعدة البيانات - تأكد من رسائل F12 (Service Worker Timeout)");
- });
+ ).timeout(const Duration(seconds: 15), onTimeout: () {
+ debugPrint(
+ "ERROR: Service worker initialization timed out or failed to connect.",
+ );
+ throw TimeoutException(
+ "خطأ في تهيئة قاعدة البيانات - تأكد من رسائل F12 (Service Worker Timeout)",
+ );
+ });(TimeoutException lives in dart:async.)
📝 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
| ).timeout(const Duration(seconds: 15), onTimeout: () { | |
| print("ERROR: Service worker initialization timed out or failed to connect."); | |
| throw Exception("خطأ في تهيئة قاعدة البيانات - تأكد من رسائل F12 (Service Worker Timeout)"); | |
| }); | |
| ).timeout(const Duration(seconds: 15), onTimeout: () { | |
| debugPrint( | |
| "ERROR: Service worker initialization timed out or failed to connect.", | |
| ); | |
| throw TimeoutException( | |
| "خطأ في تهيئة قاعدة البيانات - تأكد من رسائل F12 (Service Worker Timeout)", | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@brightcleanproject/lib/core/database/database_helper.dart` around lines 36 -
39, Replace the current .timeout(... onTimeout: ...) usage in
database_helper.dart with a try/catch around the await that calls timeout so you
can log with debugPrint and preserve/rethrow the original error: remove the
onTimeout handler, wrap the future call that uses .timeout(const
Duration(seconds: 15)) in a try { ... } on TimeoutException catch (e, st) {
debugPrint("Service worker initialization timed out: $e\n$st"); rethrow; } and
for other exceptions use catch (e, st) { debugPrint("Service worker
initialization failed: $e\n$st"); rethrow; } — this keeps debugPrint (from
material.dart) instead of print and preserves the original exception and stack
trace for functions/blocks around the timeout call.
Comment on lines
+61
to
+65
| Future _upgradeDB(Database db, int oldVersion, int newVersion) async { | ||
| // Drop existing table and recreate it with the new schema for development purposes | ||
| await db.execute('DROP TABLE IF EXISTS users'); | ||
| await _createDB(db, newVersion); | ||
| } |
There was a problem hiding this comment.
_upgradeDB drops the users table — every schema bump erases all accounts.
DROP TABLE IF EXISTS users is fine during early development, but the comment on line 62 ("for development purposes") suggests this is intentional. Once this code is shipped to a real device, bumping version in _initDatabase will silently delete every customer's and agent's account on next app launch. Either guard this behind a debug flag (assert/kDebugMode) or implement proper ALTER TABLE migrations now, before any production data exists.
🛡️ Suggested guard
+import 'package:flutter/foundation.dart' show kDebugMode;
+
Future _upgradeDB(Database db, int oldVersion, int newVersion) async {
- // Drop existing table and recreate it with the new schema for development purposes
- await db.execute('DROP TABLE IF EXISTS users');
- await _createDB(db, newVersion);
+ assert(kDebugMode, 'Destructive upgrade path running in release build!');
+ // TODO: replace with non-destructive ALTER TABLE migrations before launch.
+ await db.execute('DROP TABLE IF EXISTS users');
+ await _createDB(db, newVersion);
}📝 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 _upgradeDB(Database db, int oldVersion, int newVersion) async { | |
| // Drop existing table and recreate it with the new schema for development purposes | |
| await db.execute('DROP TABLE IF EXISTS users'); | |
| await _createDB(db, newVersion); | |
| } | |
| import 'package:flutter/foundation.dart' show kDebugMode; | |
| Future _upgradeDB(Database db, int oldVersion, int newVersion) async { | |
| assert(kDebugMode, 'Destructive upgrade path running in release build!'); | |
| // TODO: replace with non-destructive ALTER TABLE migrations before launch. | |
| await db.execute('DROP TABLE IF EXISTS users'); | |
| await _createDB(db, newVersion); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@brightcleanproject/lib/core/database/database_helper.dart` around lines 61 -
65, The _upgradeDB function currently runs `DROP TABLE IF EXISTS users` which
will erase all accounts on any schema version bump; update _upgradeDB (and
ensure callers via _initDatabase) to avoid destructive drops in production by
either wrapping the DROP logic behind a debug-only guard (use assert or
kDebugMode) or replace it with proper non-destructive migrations (ALTER TABLE /
data-preserving steps) that handle oldVersion→newVersion upgrades. Locate the
`_upgradeDB` function and change its behavior so production runs perform safe
migrations while debug/dev runs may still drop+recreate for convenience.
Comment on lines
+67
to
+80
| Future _createDB(Database db, int version) async { | ||
| await db.execute(''' | ||
| CREATE TABLE users ( | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| first_name TEXT, | ||
| last_name TEXT, | ||
| phone TEXT NOT NULL, | ||
| email TEXT, | ||
| password TEXT NOT NULL, | ||
| gender TEXT, | ||
| dob TEXT, | ||
| role TEXT NOT NULL | ||
| ) | ||
| '''); |
There was a problem hiding this comment.
No UNIQUE constraint on phone — duplicate accounts can be registered, and login becomes ambiguous.
phone is the de-facto login identifier, but the schema only marks it NOT NULL. Two users (or the seed user plus a new registration with phone 0500000000) can coexist; loginUser then returns results.first, which is non-deterministic and could log the user into someone else's account if passwords happen to match. Add UNIQUE to phone, handle the resulting DatabaseException (code = SQLITE_CONSTRAINT) in registerUser with a friendly "phone already registered" message, and consider WHERE phone = ? first then verifying the password hash separately.
🛡️ Proposed schema change
CREATE TABLE users (
id INTEGER PRIMARY KEY AUTOINCREMENT,
first_name TEXT,
last_name TEXT,
- phone TEXT NOT NULL,
+ phone TEXT NOT NULL UNIQUE,
email TEXT,
password TEXT NOT NULL,
gender TEXT,
dob TEXT,
role TEXT NOT NULL
)Also applies to: 102-115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@brightcleanproject/lib/core/database/database_helper.dart` around lines 67 -
80, The users table schema in _createDB must mark phone as UNIQUE to prevent
duplicate accounts—update the CREATE TABLE statement in _createDB to add UNIQUE
to the phone column; then update registerUser to catch DatabaseException
(inspect exception code for SQLITE_CONSTRAINT) and return a friendly "phone
already registered" error instead of a generic failure. Also modify loginUser to
query WHERE phone = ? first and then verify the password hash against the
returned record (rather than relying on results.first), so authentication is
deterministic and safe.
Comment on lines
+67
to
+93
| Future _createDB(Database db, int version) async { | ||
| await db.execute(''' | ||
| CREATE TABLE users ( | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| first_name TEXT, | ||
| last_name TEXT, | ||
| phone TEXT NOT NULL, | ||
| email TEXT, | ||
| password TEXT NOT NULL, | ||
| gender TEXT, | ||
| dob TEXT, | ||
| role TEXT NOT NULL | ||
| ) | ||
| '''); | ||
|
|
||
| // Insert seed data | ||
| await db.insert('users', { | ||
| 'first_name': 'عميل', | ||
| 'last_name': 'تجريبي', | ||
| 'phone': '0500000000', | ||
| 'email': 'customer@test.com', | ||
| 'password': 'password1234', | ||
| 'gender': 'M', | ||
| 'dob': '2000-01-01', | ||
| 'role': 'customer', | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Flutter recommended password hashing package 2026 argon2 bcrypt
💡 Result:
The recommended package for password hashing in Flutter as of 2026 is the cryptography package (with cryptography_flutter for Flutter), which explicitly states "Argon2id is the recommended algorithm for password hashing." It provides a modern, cross-platform cryptographic library supporting Argon2id, the current standard for password hashing due to its memory-hardness and resistance to GPU attacks. For bcrypt, the bcrypt package is available and commonly used, implementing the classic Blowfish-based algorithm. However, Argon2 (specifically Argon2id) is superior and preferred over bcrypt per modern security recommendations (e.g., password-hashing.net, recent benchmarks showing Argon2id's GPU resistance). Example usage with cryptography for Argon2id: import 'package:cryptography/cryptography.dart'; final algorithm = Argon2id( memory: 10*1000, // 10 MB parallelism: 2, iterations: 1, hashLength: 32, ); final key = await algorithm.deriveKeyFromPassword( password: SecretKey('chpassword'.codeUnits), nonce: List.filled(16, 0), // Use random salt ); final hashBytes = await key.extractBytes; Other Argon2 options include argon2 (pure Dart, multi-platform) and dargon2_flutter (native C bindings for performance), but cryptography is the most authoritative and actively maintained with the explicit recommendation.
Citations:
- 1: https://pub.dev/packages/cryptography_plus
- 2: https://pub.dev/packages/cryptography/versions/2.8.1
- 3: https://pub.dartlang.org/packages/cryptography_flutter
- 4: https://pub.dev/packages/argon2
- 5: https://pub.dev/packages/dargon2_flutter
- 6: https://pub.dev/packages/bcrypt
- 7: https://dev.to/treszyk/architecture-vs-brute-force-what-i-learned-benchmarking-kdfs-for-my-thesis-18bo
- 8: https://stackoverflow.com/questions/78456054/in-my-flutter-app-how-can-i-verify-an-entered-password-against-an-argon2id-enco
🏁 Script executed:
cat -n brightcleanproject/lib/core/database/database_helper.dart | sed -n '67,120p'Repository: Klvvr1/BrightClean
Length of output: 1598
Passwords are stored and compared in plaintext.
The schema stores password TEXT NOT NULL, the seed inserts 'password1234', registerUser writes the password from the form directly via db.insert('users', userData) without processing, and loginUser performs WHERE phone = ? AND password = ? against the raw value. Even for a local-only SQLite DB this is unsafe — anyone with file access (rooted device, leaked backup) can read every user's credential, and users routinely re-use passwords across services. Hash with Argon2id on registration and compare hashes on login.
🛡️ Suggested approach
Use the cryptography package (recommended by Flutter security standards as of 2026) with Argon2id:
// On registration
import 'package:cryptography/cryptography.dart';
final algorithm = Argon2id(
memory: 10 * 1000, // 10 MB
parallelism: 2,
iterations: 1,
hashLength: 32,
);
final salt = List<int>.filled(16, 0); // Replace with random bytes in production
final key = await algorithm.deriveKeyFromPassword(
password: SecretKey(password.codeUnits),
nonce: salt,
);
final hash = await key.extractBytes();
await db.insert('users', {
...userData,
'password': base64Encode(hash),
'password_salt': base64Encode(salt),
});
// On login
final row = await db.query('users', where: 'phone = ?', whereArgs: [phone]);
if (row.isEmpty) return null;
final key = await algorithm.deriveKeyFromPassword(
password: SecretKey(password.codeUnits),
nonce: base64Decode(row.first['password_salt']),
);
final hash = await key.extractBytes();
return base64Encode(hash) == row.first['password'] ? row.first : null;Add password_salt TEXT NOT NULL column to the schema and update both registration screens.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@brightcleanproject/lib/core/database/database_helper.dart` around lines 67 -
93, Schema and auth currently store plaintext passwords (see _createDB creating
users.password and seed insert, and places using db.insert in registerUser and
WHERE phone = ? AND password = ? in loginUser); change to store Argon2id hashes
and salts: alter the users schema in _createDB to add password_salt TEXT NOT
NULL, remove plaintext seeds or replace password with a hashed value, and in
registerUser generate a secure random salt, derive an Argon2id key from the
provided password (using the cryptography package Argon2id), base64-encode and
store both the hash and salt via db.insert; in loginUser query by phone only,
retrieve password_salt and stored password hash, derive the Argon2id key from
the supplied password and the stored salt, base64-encode and compare to the
stored hash to authenticate.
Comment on lines
+345
to
+349
| if (_selectedAddress == null) // Very basic required validation for map visually | ||
| const Padding( | ||
| padding: EdgeInsets.only(top: 8, right: 12), | ||
| child: Text('الرجاء تحديد الموقع', style: TextStyle(color: Colors.red, fontSize: 12)), | ||
| ), |
There was a problem hiding this comment.
"Please select location" error is shown on first render before the user has done anything.
Unlike the agent screen, there's no _hasAttemptedSubmit gate here, so the red الرجاء تحديد الموقع message renders the moment the screen is opened. Track an attempt flag (or only show after submit is pressed) so the form doesn't look pre-failed.
♻️ Proposed fix
Mirror the pattern from agent_registration_screen.dart lines 481‑488:
- if (_selectedAddress == null) // Very basic required validation for map visually
+ if (_hasAttemptedSubmit && _selectedAddress == null)
const Padding(
padding: EdgeInsets.only(top: 8, right: 12),
child: Text('الرجاء تحديد الموقع', style: TextStyle(color: Colors.red, fontSize: 12)),
),(Add a bool _hasAttemptedSubmit = false; to state and set it at the start of _submitForm.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@brightcleanproject/lib/features/auth/presentation/customer_registration_screen.dart`
around lines 345 - 349, The validation message for location is shown immediately
because there's no submit-attempt guard; add a boolean state field named
_hasAttemptedSubmit initialized to false and set it to true at the start of the
_submitForm method, then change the conditional that shows the red Text (which
currently checks _selectedAddress == null) to require both _selectedAddress ==
null && _hasAttemptedSubmit so the error only appears after a user attempts to
submit (mirroring the pattern used in agent_registration_screen.dart).
Comment on lines
+41
to
+42
| // Simulate a network request taking 1.5 seconds | ||
| await Future.delayed(const Duration(milliseconds: 1500)); |
There was a problem hiding this comment.
Remove the artificial 1.5 s delay before authentication.
await Future.delayed(const Duration(milliseconds: 1500)) was likely added to demo the spinner, but it now penalizes every real login by 1.5 seconds — and runs even on the failure path. The DB call is local SQLite, so the round-trip is already fast; just delete this line.
♻️ Proposed fix
try {
- // Simulate a network request taking 1.5 seconds
- await Future.delayed(const Duration(milliseconds: 1500));
-
// DB Authentication Logic
final user = await DatabaseHelper.instance.loginUser(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@brightcleanproject/lib/features/auth/presentation/login_screen.dart` around
lines 41 - 42, Remove the artificial 1.5s delay by deleting the await
Future.delayed(const Duration(milliseconds: 1500)) call in login_screen.dart
(the line containing Future.delayed(const Duration(milliseconds: 1500))). This
line currently lives in the login flow (e.g., the handler that performs
authentication) and should be removed so authentication uses the actual SQLite
round-trip time; keep the surrounding authentication logic and spinner state
management intact.
Comment on lines
+50
to
+63
| // After async gaps, check if the widget is still mounted | ||
| if (mounted) { | ||
| if (user != null) { | ||
| // If needed, check user['role'] to route appropriately | ||
| context.go('/customer_home'); | ||
| } else { | ||
| ScaffoldMessenger.of(context).showSnackBar( | ||
| const SnackBar( | ||
| content: Text('رقم الجوال أو كلمة المرور غير صحيحة'), | ||
| backgroundColor: Colors.red, | ||
| ), | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Role is fetched but ignored — agents will land on /customer_home.
loginUser returns the full user row (including role), and the comment on line 53 acknowledges this, yet every authenticated user is routed to /customer_home. With the agent registration flow being added in this PR, an agent who logs in successfully will land on the customer experience. Branch on user['role'] and route to the appropriate home (e.g. /agent_home) — or fail the login if the role isn't recognized.
🛡️ Proposed fix
if (user != null) {
- // If needed, check user['role'] to route appropriately
- context.go('/customer_home');
+ final role = user['role'] as String?;
+ switch (role) {
+ case 'customer':
+ context.go('/customer_home');
+ break;
+ case 'agent':
+ context.go('/agent_home');
+ break;
+ default:
+ ScaffoldMessenger.of(context).showSnackBar(
+ const SnackBar(content: Text('دور المستخدم غير معروف')),
+ );
+ }
} else {📝 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
| // After async gaps, check if the widget is still mounted | |
| if (mounted) { | |
| if (user != null) { | |
| // If needed, check user['role'] to route appropriately | |
| context.go('/customer_home'); | |
| } else { | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| const SnackBar( | |
| content: Text('رقم الجوال أو كلمة المرور غير صحيحة'), | |
| backgroundColor: Colors.red, | |
| ), | |
| ); | |
| } | |
| } | |
| // After async gaps, check if the widget is still mounted | |
| if (mounted) { | |
| if (user != null) { | |
| final role = user['role'] as String?; | |
| switch (role) { | |
| case 'customer': | |
| context.go('/customer_home'); | |
| break; | |
| case 'agent': | |
| context.go('/agent_home'); | |
| break; | |
| default: | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| const SnackBar(content: Text('دور المستخدم غير معروف')), | |
| ); | |
| } | |
| } else { | |
| ScaffoldMessenger.of(context).showSnackBar( | |
| const SnackBar( | |
| content: Text('رقم الجوال أو كلمة المرور غير صحيحة'), | |
| backgroundColor: Colors.red, | |
| ), | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@brightcleanproject/lib/features/auth/presentation/login_screen.dart` around
lines 50 - 63, The current post-login routing always sends authenticated users
to '/customer_home' even though loginUser returns a user record with a role;
update the routing logic in the block that checks if (mounted) after login to
inspect user['role'] (from the loginUser result) and branch accordingly (e.g.,
context.go('/agent_home') for role == 'agent', context.go('/customer_home') for
role == 'customer'), and handle unrecognized roles by showing an error SnackBar
or failing the login; keep the existing mounted check and Snackbar usage
(ScaffoldMessenger.of(context).showSnackBar) and only change the branch that
currently always calls context.go('/customer_home').
| flutter_map: ^8.3.0 | ||
| latlong2: ^0.9.1 | ||
| geocoding: ^4.0.0 | ||
| file_picker: ^11.0.2 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify file_picker is genuinely unused in the codebase
rg -nP "package:file_picker|FilePicker|file_picker" --type=dartRepository: Klvvr1/BrightClean
Length of output: 44
Remove unused dependency: file_picker: ^11.0.2
This package is not imported or used anywhere in the codebase. The registration flow uses image_picker exclusively. Unused native plugins increase app size and may introduce unnecessary Android/iOS permissions and configuration requirements—remove this entry from pubspec.yaml.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@brightcleanproject/pubspec.yaml` at line 48, Remove the unused dependency
entry "file_picker: ^11.0.2" from pubspec.yaml; search the codebase for any
imports or references to "file_picker" or "FilePicker" to confirm there are no
usages (the project uses "image_picker" instead), delete the dependency line,
run flutter pub get to update lockfiles, and if present remove any
platform-specific configuration or Android/iOS permission entries that were
added only for file_picker.
Comment on lines
+1
to
+2
| importScripts('https://unpkg.com/sql.js@1.8.0/dist/sql-wasm.js'); | ||
| importScripts('https://unpkg.com/sqflite_common_ffi_web@0.4.0/assets/sqflite_sw.js'); |
There was a problem hiding this comment.
Avoid loading the SQLite worker dependencies from a third‑party CDN at runtime.
Pulling sql.js and sqflite_common_ffi_web from unpkg.com inside a service worker has several real downsides:
- Reliability/offline: any unpkg outage or restrictive network (corporate proxies, regions where unpkg is blocked) breaks the DB layer, after the 15s timeout in
database_helper.dart. - Supply chain / security:
importScriptsdoes not support Subresource Integrity, so a compromised CDN response runs arbitrary code in your origin's service-worker scope (full access to caches, fetch interception, IndexedDB). - Privacy: every user load reveals the origin to a third party.
- Version drift risk: the
sqflite_common_ffi_web@0.4.0JS must match the Dartsqflite_common_ffi_webversion resolved inpubspec.lock; a mismatch can produce subtle DB init failures.
The recommended setup from the sqflite_common_ffi_web package is to vendor sqflite_sw.js and sqlite3.wasm (or sql-wasm.js) into web/ (e.g. via dart run sqflite_common_ffi_web:setup) and load them locally:
♻️ Suggested replacement (after vendoring assets into web/)
-importScripts('https://unpkg.com/sql.js@1.8.0/dist/sql-wasm.js');
-importScripts('https://unpkg.com/sqflite_common_ffi_web@0.4.0/assets/sqflite_sw.js');
+importScripts('sql-wasm.js');
+importScripts('sqflite_sw_helper.js'); // or whatever the setup script emitsWhat is the recommended setup for sqflite_common_ffi_web service worker on Flutter web, and what versions of sql.js / sqlite3.wasm does it expect?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@brightcleanproject/web/sqflite_sw.js` around lines 1 - 2, The service worker
currently imports sql.js and sqflite_sw.js from unpkg via importScripts, which
is unsafe and unreliable; instead vendor the runtime assets locally (run dart
run sqflite_common_ffi_web:setup to copy sqflite_sw.js and the matching
sqlite3.wasm/sql-wasm.js into your project's web/), replace the external
importScripts calls in sqflite_sw.js with relative local paths to those vendored
files, and ensure the vendored sql.js/wasm version matches the version resolved
in pubspec.lock (e.g., sql.js 1.8.0 / corresponding sqlite3.wasm expected by
sqflite_common_ffi_web version) so the worker and Dart package remain
compatible.
Owner
|
Very Distracted Changes for 15 Files and more than 1500 Lines, Although The task is for the Agent screen only. |
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.
close #4
Summary by CodeRabbit