Explore page update#4
Conversation
…ce, and refactor various components
- Fetch within ~100km and group results city-first then nearby by distance - Refresh Explore reloads data only (no location reset) - ""Use my location"" clears manual selection before GPS update - Add pull-to-refresh to Search Results and snapshot query location
- New PropertyGridCard with image, price, rating, address, distance - Cleaner layout, rounded card, subtle elevation, hero transition - App bar actions placeholders for Sort and Map - Adjust grid ratio for better card proportions
- Card color set to #fff, elevation removed - 1px subtle border added with rounded corners - Multi-line description (up to 3 lines) to fit content - Maintains clean padding and grid spacing
- Drop AspectRatio; use top-only ClipRRect and SizedBox(height:150) - CachedNetworkImage with BoxFit.cover keeps image natural - Keep favorite and distance overlays intact - Column uses mainAxisSize.min to shrink-wrap content
- Fixed image height of 160 with ClipRRect (top corners) and BoxFit.cover - Removed AspectRatio; consistent visual rhythm like Airbnb/Oyo - Info section shrink-wraps (no extra whitespace) - Favorite overlay and distance badge preserved - 1px #ddd border for clean, compact appearance
- Phones: use ListView so cards shrink to dynamic height (no empty space) - Tablets/desktop: tuned grid aspect ratio to better fit content - Card paddings tightened for a more compact, polished look
- Card background set to white - Border changed to solid black (1px) - Added light shadow (elevation 2, soft shadowColor)
- Explicitly set Scaffold backgroundColor to Colors.white - Matches card styling and removes pink tint
- Added support for Hindi localization with new `hi.json` file. - Removed unused Spanish and French localization files. - Introduced `SettingsBinding`, `SettingsController`, and `ThemeController` for managing app settings and theme preferences. - Updated UI components to utilize theme extensions for consistent styling. - Refactored various views to improve localization handling and theme integration.
- enhance listing details - wishlist like feature update state well - improve ui of listing -
now properties which we want to book is now add to booking page after "book -> pay and confirm "
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (14)
lib/features/tour/controllers/tour_controller.dart (1)
32-35: Wrap async operation in try-catch block.The
onPageFinishedasync callback callsWebViewHelper.injectResponsiveStyleswithout error handling. If this fails, the error is silently swallowed. Per coding guidelines, async operations should be wrapped in try-catch blocks with AppLogger for error logging.🔧 Proposed fix
- onPageFinished: (_) async { - isLoading.value = false; - await WebViewHelper.injectResponsiveStyles(webViewController); - }, + onPageFinished: (_) async { + isLoading.value = false; + try { + await WebViewHelper.injectResponsiveStyles(webViewController); + } catch (e, stackTrace) { + AppLogger.error('Failed to inject responsive styles', e, stackTrace); + } + },Add the import at the top of the file:
import 'package:stays_app/app/utils/logger/app_logger.dart';lib/features/wishlist/controllers/wishlist_controller.dart (2)
37-42: Potential redundant API call inonReady.
loadWishlist()is already called inonInit()(line 34). Calling it again inonReady()withshowLoader: falsemay result in a race condition or redundant network requests ifonInit's call hasn't completed.Consider guarding against concurrent calls or removing the duplicate invocation if immediate refresh isn't strictly required.
286-307: Sequential deletion inclearWishlistrisks partial state on failure.The loop removes items one-by-one, and if any call fails mid-way, the wishlist ends up in an inconsistent state with some items removed and others retained. Consider:
- Collecting all IDs first, then attempting batch deletion (if API supports it).
- Or, refreshing the full list on any failure to restore consistent state.
lib/features/wishlist/views/wishlist_view.dart (1)
453-479: Fix inconsistent indentation in_buildTrailingMeta.Lines 453-465 have extra indentation that doesn't align with the rest of the method. This appears to be a formatting issue.
🔧 Suggested fix
- if (hasRating && ratingLabel != null) { - widgets.addAll([ - Icon(Icons.star_rounded, size: 14, color: Colors.amber), - const SizedBox(width: 4), - Text( - ratingLabel, - style: textStyles.bodySmall?.copyWith( - fontWeight: FontWeight.w600, - color: colors.onSurface.withValues(alpha: 0.8), - fontSize: 11, - ), - ), - ]); + if (hasRating && ratingLabel != null) { + widgets.addAll([ + Icon(Icons.star_rounded, size: 14, color: Colors.amber), + const SizedBox(width: 4), + Text( + ratingLabel, + style: textStyles.bodySmall?.copyWith( + fontWeight: FontWeight.w600, + color: colors.onSurface.withValues(alpha: 0.8), + fontSize: 11, + ), + ), + ]);lib/features/messaging/controllers/hotels_map_controller.dart (4)
272-314: Improve error handling: avoid exposing raw errors and use localization.Several issues in this method:
- Line 308 exposes raw error
$eto users — per security guidelines, sanitize error messages before displaying.- Hardcoded English strings should use
.trfor localization.- Missing
AppLogger.error()calls before showing snackbars.🔧 Suggested improvement
} catch (e) { - Get.snackbar('Error', 'Failed to get current location: $e'); + AppLogger.error('Failed to get current location', e); + Get.snackbar( + 'error.title'.tr, + 'error.location_failed'.tr, + ); _loadSampleHotels(); } finally {Apply similar changes to lines 278 and 287-290.
316-345: Add error handling for API call.The
tryblock lacks acatchclause. If_propertiesService!.explore()fails, the exception propagates unhandled. Per coding guidelines, catchApiExceptionseparately, log withAppLogger, and show user feedback.🐛 Suggested fix
Future<void> _loadHotelsNearLocation( LatLng location, { double? radiusKm, }) async { isLoadingHotels.value = true; try { if (_propertiesService == null) { _allHotels.clear(); _setHotels(const <HotelModel>[]); return; } final double radius = radiusKm ?? _activeFilters.radiusKm ?? _lastRadius; final resp = await _propertiesService!.explore( lat: location.latitude, lng: location.longitude, radiusKm: radius, limit: 50, filters: _activeFilters.toQueryParameters(), ); _lastRadius = radius; final mapped = resp.properties.map((p) => _toHotelModel(p)).toList(); _allHotels ..clear() ..addAll(mapped); _applyFilters(fromRemoteFetch: true); _refreshLocationLabel(); + } on ApiException catch (e) { + AppLogger.error('Failed to load hotels', e); + Get.snackbar('error.title'.tr, 'error.load_hotels_failed'.tr); + } catch (e, stackTrace) { + AppLogger.error('Unexpected error loading hotels', e, stackTrace); } finally { isLoadingHotels.value = false; } }
475-491: Add error handling for autocomplete API call.The method uses
try-finallywithout acatch. If the autocomplete call fails, the exception propagates and the user receives no feedback.🔧 Suggested fix
isSearching.value = true; try { final results = await _placesService!.autocomplete( q, lat: _locationService?.latitude ?? currentLocation.value.latitude, lng: _locationService?.longitude ?? currentLocation.value.longitude, ); predictions.assignAll(results); + } catch (e) { + AppLogger.warning('Autocomplete search failed', e); + predictions.clear(); } finally { isSearching.value = false; }
493-515: Add error handling for place details API call.Missing
catchblock; if_placesService!.details()throws, the exception propagates unhandled. Also, line 498 returns silently whendetails == nullwithout user feedback.🔧 Suggested fix
try { final details = await _placesService!.details(p.placeId); - if (details == null) return; + if (details == null) { + AppLogger.warning('No details found for place: ${p.placeId}'); + return; + } // ... rest of the code + } catch (e) { + AppLogger.error('Failed to get place details', e); + Get.snackbar('error.title'.tr, 'error.place_details_failed'.tr); } finally { isLoadingLocation.value = false; }lib/features/trips/controllers/trips_controller.dart (1)
279-293: Missing error logging in catch blocks.Per coding guidelines, all errors should be logged with
AppLoggerbefore showing user-facing messages. Both catch blocks here restore the snapshot and show a snackbar but don't log the errors.🔧 Proposed fix to add error logging
} on ApiException catch (error) { _restoreBookingSnapshot(snapshot); + AppLogger.error('Failed to cancel booking $bookingId', error); Get.snackbar( 'Unable to cancel', error.message, snackPosition: SnackPosition.BOTTOM, ); - } catch (_) { + } catch (error, stackTrace) { _restoreBookingSnapshot(snapshot); + AppLogger.error('Unexpected error cancelling booking $bookingId', error, stackTrace); Get.snackbar( 'Unable to cancel', 'We could not cancel this inquiry. Please try again.', snackPosition: SnackPosition.BOTTOM, ); }lib/features/listing/controllers/location_search_controller.dart (1)
59-75: Add error handling with user feedback inselectPrediction.The method has
try-finallybut nocatchblock. If_placesService.details()fails, the error propagates without user feedback, and the loading indicator may leave users confused.🐛 Proposed fix
Future<void> selectPrediction(PlacePrediction prediction) async { try { isLoading.value = true; final details = await _placesService.details(prediction.placeId); if (details == null) return; _locationService.setSelectedLocation( lat: details.lat, lng: details.lng, locationName: details.name, ); Get.back( result: {'lat': details.lat, 'lng': details.lng, 'name': details.name}, ); + } catch (e) { + AppLogger.error('Failed to get place details', e); + Get.snackbar( + 'Error', + 'Could not get location details. Please try again.', + snackPosition: SnackPosition.BOTTOM, + ); } finally { isLoading.value = false; } }lib/app/data/models/api_response_models.dart (1)
10-40: Add@JsonSerializableannotation and delegate to generated codeThese classes have manual fromJson/toJson implementations, but the generated
.g.dartfile already exists and should be utilized instead. All other models in the project (amenity, booking, hotel, etc.) use the@JsonSerializablepattern with code generation.♻️ Suggested fix
+import 'package:json_annotation/json_annotation.dart'; + +part 'api_response_models.g.dart'; + +@JsonSerializable() class NotificationSettings { final bool pushEnabled; @@ - factory NotificationSettings.fromJson(Map<String, dynamic> json) { - return NotificationSettings( - pushEnabled: json['pushEnabled'] ?? true, - emailEnabled: json['emailEnabled'] ?? true, - smsEnabled: json['smsEnabled'] ?? false, - bookingUpdates: json['bookingUpdates'] ?? true, - promotions: json['promotions'] ?? false, - messages: json['messages'] ?? true, - reminders: json['reminders'] ?? true, - ); - } + factory NotificationSettings.fromJson(Map<String, dynamic> json) => + _$NotificationSettingsFromJson(json); @@ - Map<String, dynamic> toJson() => { - 'pushEnabled': pushEnabled, - 'emailEnabled': emailEnabled, - 'smsEnabled': smsEnabled, - 'bookingUpdates': bookingUpdates, - 'promotions': promotions, - 'messages': messages, - 'reminders': reminders, - }; + Map<String, dynamic> toJson() => _$NotificationSettingsToJson(this); } +@JsonSerializable() class PrivacySettings { final bool profilePublic; @@ - factory PrivacySettings.fromJson(Map<String, dynamic> json) { - return PrivacySettings( - profilePublic: json['profilePublic'] ?? true, - showEmail: json['showEmail'] ?? false, - showPhone: json['showPhone'] ?? false, - showListings: json['showListings'] ?? true, - showReviews: json['showReviews'] ?? true, - allowMessages: json['allowMessages'] ?? true, - ); - } + factory PrivacySettings.fromJson(Map<String, dynamic> json) => + _$PrivacySettingsFromJson(json); @@ - Map<String, dynamic> toJson() => { - 'profilePublic': profilePublic, - 'showEmail': showEmail, - 'showPhone': showPhone, - 'showListings': showListings, - 'showReviews': showReviews, - 'allowMessages': allowMessages, - }; + Map<String, dynamic> toJson() => _$PrivacySettingsToJson(this); }The
.g.dartfile is already generated—just add the annotations and import statements to activate it. Also applies to the PrivacySettings class (lines 51–78).lib/features/inquiry/bindings/inquiry_binding.dart (1)
28-36: Consider defensiveTokenServiceregistration pattern used inProfileBinding.
TokenServiceis registered globally inInitialBindingviaGet.putAsync()(async initialization), butInquiryBindingretrieves it synchronously at line 32 viaGet.find<TokenService>(). WhileInitialBindingshould run at app startup before route navigation, the async initialization creates a potential race condition.ProfileBindinghandles this more defensively (lines 28-29) by registeringTokenServicelocally with anisRegisteredcheck. Consider adopting the same pattern here for robustness, or document the assumption thatInitialBindingasync initialization completes before this binding executes.lib/features/listing/controllers/listing_detail_controller.dart (1)
92-211: Guard analytics calls with try-catch to prevent Firebase exceptions from breaking user flows.The AnalyticsService Firebase calls (
logPropertyView(),logWishlistAdded(),logWishlistRemoved(),logBookingStarted()) lack error handling. While the service checksif (_analytics != null), the underlying Firebase methods can throw exceptions. The guard patternGet.isRegistered<AnalyticsService>()in the controller only verifies service existence, not runtime safety. Wrap each analytics call in try-catch and log failures to AppLogger to ensure wishlist toggles, navigation, and other critical flows remain resilient.🐛 Suggested hardening
@@ - _logPropertyView(property); + _logPropertyView(property); @@ - if (Get.isRegistered<AnalyticsService>()) { - Get.find<AnalyticsService>().logWishlistRemoved('$propertyId'); - } + if (Get.isRegistered<AnalyticsService>()) { + try { + Get.find<AnalyticsService>().logWishlistRemoved('$propertyId'); + } catch (e, st) { + AppLogger.error('Analytics logWishlistRemoved failed', e, st); + } + } @@ - if (Get.isRegistered<AnalyticsService>()) { - Get.find<AnalyticsService>().logWishlistAdded('$propertyId'); - } + if (Get.isRegistered<AnalyticsService>()) { + try { + Get.find<AnalyticsService>().logWishlistAdded('$propertyId'); + } catch (e, st) { + AppLogger.error('Analytics logWishlistAdded failed', e, st); + } + } @@ - if (Get.isRegistered<AnalyticsService>()) { - Get.find<AnalyticsService>().logBookingStarted( - '${property.id}', - property.pricePerNight, - ); - } + if (Get.isRegistered<AnalyticsService>()) { + try { + Get.find<AnalyticsService>().logBookingStarted( + '${property.id}', + property.pricePerNight, + ); + } catch (e, st) { + AppLogger.error('Analytics logBookingStarted failed', e, st); + } + } @@ - void _logPropertyView(Property property) { - if (Get.isRegistered<AnalyticsService>()) { - Get.find<AnalyticsService>().logPropertyView( - '${property.id}', - property.name, - ); - } - } + void _logPropertyView(Property property) { + if (Get.isRegistered<AnalyticsService>()) { + try { + Get.find<AnalyticsService>().logPropertyView( + '${property.id}', + property.name, + ); + } catch (e, st) { + AppLogger.error('Analytics logPropertyView failed', e, st); + } + } + }lib/features/explore/bindings/explore_binding.dart (1)
53-62: ExploreController registration should have isRegistered guard.Unlike other dependencies,
ExploreControlleris registered without anisRegisteredcheck. This could cause registration conflicts on re-navigation to the explore page.🔧 Suggested fix
// Register ExploreController AFTER all dependencies are registered + if (!Get.isRegistered<ExploreController>()) { Get.lazyPut<ExploreController>( () => ExploreController( locationService: Get.find<LocationService>(), propertiesRepository: Get.find<PropertiesRepository>(), wishlistRepository: Get.find<WishlistRepository>(), filterController: Get.find<FilterController>(), favoritesController: Get.find<FavoritesController>(), ), ); + }
🤖 Fix all issues with AI agents
In `@lib/app/data/models/message_model.dart`:
- Around line 16-23: Replace the manual Map serialization in MessageModel with
the project-standard json_serializable setup: add the import for json_annotation
and the part directive for message_model.g.dart, annotate the MessageModel class
with `@JsonSerializable`(), remove/replace the manual factory MessageModel.fromMap
and toMap with factory MessageModel.fromJson delegating to
_$MessageModelFromJson(map) and Map<String, dynamic> toJson() delegating to
_$MessageModelToJson(this); you may keep thin backwards-compatible fromMap/toMap
wrappers that call fromJson/toJson if desired; ensure DateTime parsing uses the
generated behavior (i.e., rely on DateTime.parse via the generated code) and
reference the existing message_model.g.dart for generated helpers.
In `@lib/app/ui/widgets/common/animated_toast.dart`:
- Around line 143-154: The auto-dismiss timer's Future.delayed callback and
manual close both call _dismiss(), which can lead to overlayEntry.remove() being
invoked twice; add a boolean guard (e.g., _isDismissed or _isDismissing) as a
field, check it at the start of _dismiss() and in the Future.delayed callback,
return immediately if already true, and set it to true before performing the
reverse animation and calling the code that removes the OverlayEntry (the place
that calls overlayEntry.remove() or widget.onDismiss) so the removal only
happens once.
In `@lib/app/ui/widgets/common/swipeable_item.dart`:
- Around line 127-128: The Dismissible currently uses widget.key ?? UniqueKey(),
causing unstable keys and broken dismiss behavior; change SwipeableItem to
require a stable itemKey parameter (add final Key itemKey; and a required
this.itemKey in the constructor) and replace the Dismissible key usage with
widget.itemKey so the parent provides a stable unique key (e.g., based on the
item ID) instead of generating a new UniqueKey() on every rebuild.
- Around line 302-303: The Dismissible currently uses key ?? UniqueKey() which
creates a new UniqueKey on every rebuild and resets Dismissible state; update
the widget to accept a required stable identifier (e.g., add a new parameter
itemKey or itemId) and use that to build a stable Key (for example
ValueKey(itemKey) or ValueKey(itemId)) when constructing the Dismissible instead
of UniqueKey(); locate the Dismissible constructor in the build method where key
is passed and replace the fallback UniqueKey() usage, and update
constructors/signature of the SwipeableItem widget to require and propagate the
new itemKey parameter.
🟠 Major comments (17)
lib/features/trips/controllers/trips_controller.dart-403-406 (1)
403-406: Potential null/type error ontotalAmountaccess.This code assumes
booking['totalAmount']is always a non-nullnum. However, thetotalSpentgetter (line 496-498) defensively checksif (amount is num)before calling methods on it. This inconsistency could cause a runtimeNoSuchMethodErroriftotalAmountis null or not a number.🐛 Proposed fix with defensive handling
_buildDetailRow( 'Total Amount', - '\$${booking['totalAmount'].toStringAsFixed(2)}', + _formatAmount(booking['totalAmount']), ),Add a helper method:
String _formatAmount(dynamic amount) { if (amount is num) { return '\$${amount.toStringAsFixed(2)}'; } return 'N/A'; }lib/app/ui/widgets/common/swipeable_item.dart-171-174 (1)
171-174: Animation won't display when confirming via overlay button.When the user taps "Yes",
_controller.forward()is called but_isDeletingremainsfalse. The build method (line 115) only renders theSizeTransition/FadeTransitionwhen_isDeletingis true, so the exit animation won't be visible.🐛 Proposed fix
onConfirm: () { + setState(() { + _isDeleting = true; + _isConfirmingDelete = false; + }); _controller.forward().then((_) { widget.onDelete?.call(); }); },lib/features/auth/views/phone_login_view.dart-399-403 (1)
399-403: Addawaitto asyncloginWithPhonecall.
TheloginWithPhonemethod returnsFuture<void>but is called withoutawaitin_handleLogin(). This leaves exceptions unhandled and can cause loading state to become out of sync. Compare withotp_controller.dartline 189, which correctly awaits the same call.Fix
- void _handleLogin() { - controller.loginWithPhone( - phone: _phoneController.text.trim(), - password: _passwordController.text, - ); - } + Future<void> _handleLogin() async { + await controller.loginWithPhone( + phone: _phoneController.text.trim(), + password: _passwordController.text, + ); + }lib/app/data/models/payment_model.dart-14-26 (1)
14-26: Implement json_serializable for PaymentModelPaymentModel uses manual
fromMap/toMapmethods but should follow the project's standard with@JsonSerializableannotation and generatedfromJson/toJsonmethods. AmenityModel demonstrates the correct pattern.♻️ Suggested implementation
+import 'package:json_annotation/json_annotation.dart'; + +part 'payment_model.g.dart'; + +@JsonSerializable() class PaymentModel { final String id; final num amount; final String currency; final String status; const PaymentModel({ required this.id, required this.amount, this.currency = 'USD', this.status = 'pending', }); - factory PaymentModel.fromMap(Map<String, dynamic> map) => PaymentModel( - id: map['id']?.toString() ?? '', - amount: map['amount'] as num? ?? 0, - currency: map['currency'] as String? ?? 'USD', - status: map['status'] as String? ?? 'pending', - ); - - Map<String, dynamic> toMap() => { - 'id': id, - 'amount': amount, - 'currency': currency, - 'status': status, - }; + factory PaymentModel.fromJson(Map<String, dynamic> json) => + _$PaymentModelFromJson(json); + + Map<String, dynamic> toJson() => _$PaymentModelToJson(this); }After changes, run code generation:
flutter pub run build_runner build.lib/app/ui/widgets/common/explore_hero_header.dart-9-46 (1)
9-46: Add translation keys for greeting messages and subtitleThe hardcoded strings bypass localization. Add translation keys to both
l10n/en.jsonandl10n/hi.json, then update the code to use.tr:🌐 Translation keys (add to both JSON files)
In
l10n/en.json, add to the"explore"section:"greeting": { "morning": "Good morning", "afternoon": "Good afternoon", "evening": "Good evening" }, "subtitle": "Discover stays around you"In
l10n/hi.json, add corresponding Hindi translations to the"explore"section:"greeting": { "morning": "शुभ प्रभात", "afternoon": "शुभ दोपहर", "evening": "शुभ संध्या" }, "subtitle": "आपके आसपास स्टे खोजें"📝 Code update
import 'package:flutter/material.dart'; +import 'package:get/get.dart'; import 'package:stays_app/app/ui/theme/theme_extensions.dart'; @@ static String getTimeBasedGreeting() { final hour = DateTime.now().hour; if (hour < 12) { - return 'Good morning'; + return 'explore.greeting.morning'.tr; } else if (hour < 17) { - return 'Good afternoon'; + return 'explore.greeting.afternoon'.tr; } else { - return 'Good evening'; + return 'explore.greeting.evening'.tr; } } @@ Text( - 'Discover stays around you', + 'explore.subtitle'.tr, style: textStyles.bodySmall?.copyWith(lib/app/data/models/notification_model.dart-14-29 (1)
14-29: Restore@JsonSerializableannotation and regenerate with code_runner.NotificationModel uses manual
fromMap/toMapserialization, deviating from project standards. All other models inlib/app/data/models/(Property, WishlistItem, Hotel, AmenityModel, etc.) use the@JsonSerializable()annotation with generated.g.dartfiles. Update this model to match the established pattern: add the annotation, include the necessary imports andpartdirective, then runbuild_runnerto regenerate the.g.dartfile.lib/app/data/models/trip_model.dart-25-26 (1)
25-26:DateTime.parsewill throw ifcheckInorcheckOutis null or malformed.Unlike other models in this PR that handle null dates gracefully, this code assumes the map always contains valid date strings. A null or malformed value will crash with
FormatExceptionorTypeError.🐛 Add defensive parsing
factory TripModel.fromMap(Map<String, dynamic> map) => TripModel( id: map['id']?.toString() ?? '', propertyName: map['propertyName'] as String? ?? '', - checkIn: DateTime.parse(map['checkIn'] as String), - checkOut: DateTime.parse(map['checkOut'] as String), + checkIn: _parseDate(map['checkIn']) ?? DateTime.now(), + checkOut: _parseDate(map['checkOut']) ?? DateTime.now().add(const Duration(days: 1)), status: map['status'] as String? ?? 'pending', propertyImage: map['propertyImage'] as String?, totalCost: (map['totalCost'] as num?)?.toDouble(), hostName: map['hostName'] as String?, ); + + static DateTime? _parseDate(dynamic value) { + if (value == null) return null; + if (value is String) return DateTime.tryParse(value); + return null; + }lib/app/data/models/review_model.dart-14-19 (1)
14-19: Remove manual serialization and restore@JsonSerializable()annotation.The
ReviewModelis missing the@JsonSerializable()annotation and required imports (json_annotationandpart 'review_model.g.dart'). Coding guidelines require all models inlib/app/data/models/to usejson_serializablefor JSON serialization. A.g.dartfile exists but cannot be used without the annotation.Additionally, verify the API response key format: the codebase shows inconsistency with some endpoints using snake_case (
booking_id) and others using camelCase (bookingId). If the review API returns snake_case keys, use the@JsonKey(name: 'booking_id')annotation on thebookingIdfield in the model.After adding the annotation, run code generation:
dart run build_runner build --delete-conflicting-outputslib/features/auth/controllers/auth_controller.dart-49-50 (1)
49-50: Race condition:_authPrefsmay be accessed before initialization completes.
_authPrefsis declared aslate final(line 50) and initialized asynchronously in_initializeRememberMePreference()(line 148-149). However,onInit()calls this method withunawaited()(line 62), meaning execution continues without waiting.If
setRememberMe(),_persistRememberedSession(), or_clearRememberedSession()is invoked before initialization completes, it will throwLateInitializationError.🐛 Suggested fix: Await initialization or guard access
Option 1 - Make initialization synchronous by awaiting:
`@override` void onInit() { super.onInit(); - unawaited(_initializeRememberMePreference()); + // Note: This requires making onInit async or using a Completer _initAuthStatus(); _bindAuthStateListener(); }Option 2 - Use a Completer to guard access:
final Completer<void> _authPrefsReady = Completer<void>(); Future<void> _initializeRememberMePreference() async { await GetStorage.init(_rememberMeBox); _authPrefs = GetStorage(_rememberMeBox); // ... rest of initialization _authPrefsReady.complete(); } Future<void> setRememberMe(bool value) async { await _authPrefsReady.future; // Wait for initialization rememberMe.value = value; await _authPrefs.write(_rememberMeFlagKey, value); // ... }Also applies to: 57-67
lib/features/messaging/views/chat_view.dart-85-92 (1)
85-92: Messages are added locally without backend persistence.
_controller.messages.add(text)appends to a local list but doesn't send the message to a backend or persist it. This will lose messages on app restart and won't sync with other participants.If this is intentional stub code, consider adding a TODO or implementing a proper
sendMessagemethod in the controller.Would you like me to help implement a
sendMessagemethod that integrates with a messaging API?lib/features/messaging/views/chat_view.dart-43-66 (1)
43-66: Index-based message alignment is incorrect for chat semantics.Using
i.isEvento determine message alignment doesn't reflect actual sender/receiver relationships. In a real chat, messages should be aligned based on the sender ID (current user vs. other participant), not list index.If this is placeholder/stub code, consider adding a TODO comment to clarify.
🛠️ Suggested approach
// Example: if messages were MessageModel with senderId itemBuilder: (_, i) { final message = _controller.messages[i]; final isCurrentUser = message.senderId == currentUserId; return Align( alignment: isCurrentUser ? Alignment.centerRight : Alignment.centerLeft, // ... ); }lib/main_dev.dart-64-79 (1)
64-79: Potential uninitializedlatevariables ifFuture.waitpartially fails.If any task in
Future.waitthrows (e.g.,supabaseService.initialize()fails), thelatevariablesthemeServiceandlocaleServicemay remain uninitialized, causing aLateInitializationErrorwhen accessed at lines 85-86.Consider wrapping in try-catch or initializing with default values before
Future.wait.🛠️ Suggested fix
- late ThemeService themeService; - late LocaleService localeService; - - await Future.wait([ - // Supabase initialization (critical) - supabaseService.initialize(), - // Theme service initialization - ThemeService().init().then((service) => themeService = service), - // Locale service initialization - LocaleService().init().then((service) => localeService = service), - // Orientation lock (lightweight, run in parallel) - SystemChrome.setPreferredOrientations([ - DeviceOrientation.portraitUp, - DeviceOrientation.portraitDown, - ]), - ]); + ThemeService? themeService; + LocaleService? localeService; + + try { + await Future.wait([ + supabaseService.initialize(), + ThemeService().init().then((service) => themeService = service), + LocaleService().init().then((service) => localeService = service), + SystemChrome.setPreferredOrientations([ + DeviceOrientation.portraitUp, + DeviceOrientation.portraitDown, + ]), + ]); + } catch (e, st) { + AppLogger.error('Service initialization failed', e, st); + rethrow; + } + + if (themeService == null || localeService == null) { + throw StateError('Required services failed to initialize'); + }lib/app/ui/widgets/cards/base_card.dart-207-224 (1)
207-224: Method named_buildCachedImagebut usesImage.networkwithout caching.The method name suggests caching, but it uses
Image.networkinstead ofCachedNetworkImage. Other cards in this PR (likeFeaturedPropertyCard) properly useCachedNetworkImage. This inconsistency could lead to redundant network requests.🔧 Suggested fix - use CachedNetworkImage
Add the import:
import 'package:cached_network_image/cached_network_image.dart';Then update the method:
Widget _buildCachedImage(BuildContext context) { - return Image.network( - imageUrl!, + return CachedNetworkImage( + imageUrl: imageUrl!, fit: BoxFit.cover, - loadingBuilder: (context, child, loadingProgress) { - if (loadingProgress == null) return child; - return Center( - child: CircularProgressIndicator( - value: loadingProgress.expectedTotalBytes != null - ? loadingProgress.cumulativeBytesLoaded / - loadingProgress.expectedTotalBytes! - : null, - ), - ); - }, - errorBuilder: (context, error, stackTrace) => buildPlaceholder(context), + placeholder: (context, url) => const Center( + child: CircularProgressIndicator(), + ), + errorWidget: (context, url, error) => buildPlaceholder(context), ); }lib/app/ui/widgets/common/animated_search_bar.dart-130-150 (1)
130-150: Animations recreated on every build - move toinitStateordidChangeDependencies.
_widthAnimation,_fadeAnimation, and_iconSlideAnimationare recreated on everybuild()call. This is inefficient and can cause subtle animation issues. These should be initialized once and only updated when dependencies change.🔧 Suggested fix - move animations to didChangeDependencies
class _AnimatedSearchBarState extends State<AnimatedSearchBar> with SingleTickerProviderStateMixin { late AnimationController _controller; - late Animation<double> _widthAnimation; - late Animation<double> _fadeAnimation; - late Animation<double> _iconSlideAnimation; + Animation<double>? _widthAnimation; + Animation<double>? _fadeAnimation; + Animation<double>? _iconSlideAnimation; + double? _cachedExpandedWidth; // ... existing code ... + `@override` + void didChangeDependencies() { + super.didChangeDependencies(); + _initAnimations(); + } + + void _initAnimations() { + final expandedWidth = widget.expandedWidth ?? + (MediaQuery.of(context).size.width - 32); + + if (_cachedExpandedWidth != expandedWidth) { + _cachedExpandedWidth = expandedWidth; + + _widthAnimation = Tween<double>( + begin: widget.collapsedWidth, + end: expandedWidth, + ).animate(CurvedAnimation( + parent: _controller, + curve: widget.curve, + )); + + _fadeAnimation = Tween<double>(begin: 0.0, end: 1.0).animate( + CurvedAnimation( + parent: _controller, + curve: const Interval(0.3, 1.0, curve: Curves.easeOut), + ), + ); + + _iconSlideAnimation = Tween<double>(begin: 0.0, end: -8.0).animate( + CurvedAnimation( + parent: _controller, + curve: widget.curve, + ), + ); + } + } `@override` Widget build(BuildContext context) { // ... remove animation initialization from here ... - _widthAnimation = Tween<double>(...lib/app/ui/widgets/common/animated_search_bar.dart-389-438 (1)
389-438: Width animation doesn't animate smoothly - immediate toggle instead of transition.The width calculation on lines 393-395 doesn't produce a smooth animation. When
_isExpandedis true, it immediately jumps to full width. When false, the formula56 * _expandAnimation.value + 56 * (1 - _expandAnimation.value)always equals 56 regardless of animation value.🔧 Suggested fix for smooth width animation
builder: (context, child) { + final expandedWidth = MediaQuery.of(context).size.width - 32; + final currentWidth = 56 + (expandedWidth - 56) * _expandAnimation.value; + return Container( - width: _isExpanded - ? MediaQuery.of(context).size.width - 32 - : 56 * _expandAnimation.value + 56 * (1 - _expandAnimation.value), + width: currentWidth, height: 56,lib/app/ui/theme/app_theme.dart-661-690 (1)
661-690: ChangehourMinuteColorto use a surface-based color instead ofonSurface.The
hourMinuteColorproperty controls the background color of the hour and minute header segments, so it should use a surface-based color (e.g.,colorScheme.surfaceorcolorScheme.primaryContainer) rather thancolorScheme.onSurface, which is semantically a foreground/text color.lib/app/ui/theme/page_transitions.dart-471-476 (1)
471-476: Unsafe type cast will cause runtime errors.Casting
PageRouteBuilder<void>toRoute<T>is unsafe. IfTis notvoid, this will fail at runtime. Either make the transition classes generic or restrict the extension methods to returnFuture<void>.🐛 Proposed fix — make transitions generic
-class FadeInUpTransition extends PageRouteBuilder<void> { - FadeInUpTransition({ +class FadeInUpTransition<T> extends PageRouteBuilder<T> { + FadeInUpTransition({ required this.page, // ... }) : super( - pageBuilder: (context, animation, secondaryAnimation) => page, + pageBuilder: (context, animation, secondaryAnimation) => page, // ... ); final Widget page; // ... } // Then in extension: Future<T?> fadeInUp<T>({required Widget page}) { return Navigator.push<T>( this, - FadeInUpTransition(page: page) as Route<T>, + FadeInUpTransition<T>(page: page), ); }Apply the same pattern to all transition classes.
🟡 Minor comments (16)
lib/features/splash/controllers/splash_controller.dart-220-238 (1)
220-238: Empty catch block silently swallows errors.Per coding guidelines, all errors should be logged with
AppLogger. The silent catch makes debugging token synchronization issues difficult.🐛 Proposed fix
Future<void> _syncTokenServiceFromSession( Session session, StorageService storageService, TokenService? tokenService, ) async { try { if (tokenService != null) { await tokenService.storeTokens( accessToken: session.accessToken, refreshToken: session.refreshToken, ); return; } - } catch (_) {} + } catch (e) { + AppLogger.warning('TokenService.storeTokens failed, falling back to StorageService: $e'); + } await storageService.saveTokens( accessToken: session.accessToken, refreshToken: session.refreshToken, ); }lib/features/splash/controllers/splash_controller.dart-245-259 (1)
245-259: Multiple empty catch blocks suppress potentially important errors.While the defensive approach of continuing cleanup even if one step fails is appropriate, errors should still be logged per coding guidelines. Silent failures during sign-out could mask authentication or storage issues.
🐛 Proposed fix with logging
Future<void> _signOutAndClear( StorageService storageService, TokenService? tokenService, ) async { try { await Supabase.instance.client.auth.signOut(); - } catch (_) {} + } catch (e) { + AppLogger.warning('Supabase signOut failed during cleanup: $e'); + } try { await tokenService?.clearTokens(); - } catch (_) {} + } catch (e) { + AppLogger.warning('TokenService.clearTokens failed during cleanup: $e'); + } try { await storageService.clearTokens(); await storageService.clearUserData(); - } catch (_) {} + } catch (e) { + AppLogger.warning('StorageService cleanup failed: $e'); + } }lib/app/ui/widgets/common/swipeable_item.dart-15-17 (1)
15-17: Unused callback parametersonEditandonArchive.These parameters are declared but never used in the widget implementation. Either implement the corresponding swipe actions or remove them to avoid confusing API consumers.
🧹 Suggested fix: Remove unused parameters
const SwipeableItem({ super.key, required this.child, this.onDelete, - this.onEdit, - this.onArchive, this.confirmBeforeDelete = true, this.deleteConfirmDuration = const Duration(seconds: 3), this.backgroundColor, }); final Widget child; final VoidCallback? onDelete; -final VoidCallback? onEdit; -final VoidCallback? onArchive; final bool confirmBeforeDelete;lib/app/utils/services/connectivity_service.dart-158-175 (1)
158-175: DNS-only success can mark offline hosts as online.Line 158-164:
_canReachHostreturnstrueright after DNS resolution, which can misreport online status when the port is blocked/captive portal. Consider verifying socket connectivity before returning true.✅ Suggested tightening
- try { - final result = await InternetAddress.lookup(host) - .timeout(const Duration(seconds: 3)); - if (result.isNotEmpty && result.first.rawAddress.isNotEmpty) { - return true; - } - } catch (_) {} - try { - final socket = await Socket.connect( - host, - port, - timeout: const Duration(seconds: 3), - ); - socket.destroy(); - return true; - } catch (_) {} - return false; + try { + final result = await InternetAddress.lookup(host) + .timeout(const Duration(seconds: 3)); + if (result.isEmpty || result.first.rawAddress.isEmpty) return false; + final socket = await Socket.connect( + host, + port, + timeout: const Duration(seconds: 3), + ); + socket.destroy(); + return true; + } catch (_) { + return false; + }lib/app/data/models/user_model.dart-214-238 (1)
214-238: Use@JsonSerializableannotation instead of manualtoMap()implementation and remove duplicate keys.Lines 214-239: The
toMap()method contains duplicate keys (profileImageUrlandprofile_image_urlon lines 223-224) and mixes camelCase/snake_case inconsistently. Per the coding guidelines, data models should use@JsonSerializablefromjson_annotationfor JSON serialization rather than manual implementation. This provides automatic, consistent serialization and avoids duplication errors.Run code generation after applying the annotation:
flutter pub run build_runner build --delete-conflicting-outputslib/main.dart-27-108 (1)
27-108: Set up global error handlers beforerunApp()to catch all errors including early widget tree initialization failures.The app initializes
FlutterError.onErrorandPlatformDispatcher.instance.onErrorthroughCrashReportingServiceinInitialBinding, but this is applied duringGetMaterialAppinitialization—afterrunApp()is called. This timing gap means errors during early widget tree setup won't be captured by the crash reporting handlers.Move
CrashReportingService.init()to execute in therunZonedGuardedcallback beforerunApp()(or call it directly afterWidgetsFlutterBinding.ensureInitialized()), ensuring the handlers are active before any widget code runs.lib/app/data/models/review_model.dart-17-17 (1)
17-17: Suspicious default:ratingdefaults to5when missing.Defaulting to the maximum rating (5) when data is missing could misrepresent actual ratings. Consider defaulting to
0or making it nullable to indicate missing data.🐛 Suggested fix
- rating: map['rating'] as int? ?? 5, + rating: map['rating'] as int? ?? 0,lib/main_staging.dart-26-36 (1)
26-36: Missing FlutterError.onError handler.Per coding guidelines, main files should set up global error handlers using
FlutterError.onErrorfor Flutter framework errors in addition to the zone error handler.🔧 Recommended addition after WidgetsFlutterBinding
WidgetsFlutterBinding.ensureInitialized(); + FlutterError.onError = (FlutterErrorDetails details) { + AppLogger.error('Flutter error', details.exception, details.stack); + if (Get.isRegistered<CrashReportingService>()) { + Get.find<CrashReportingService>().recordError( + details.exception, + stackTrace: details.stack, + fatal: false, + ); + } + }; await dotenv.load(fileName: '.env.staging');lib/app/ui/widgets/common/animated_favorite_button.dart-129-143 (1)
129-143: UnnecessaryAnimatedBuilderfor static content.The background circle (lines 131-143) uses
AnimatedBuilderbut theContainerinside doesn't reference any animation values (_scaleAnimationis not applied). Either apply the scale animation or remove theAnimatedBuilderwrapper.🛠️ Option 1: Apply scale animation
// Background circle if (widget.hasBackground) AnimatedBuilder( animation: _scaleAnimation, builder: (context, child) { - return Container( + return Transform.scale( + scale: _scaleAnimation.value, + child: child, + ); + }, + child: Container( width: widget.size + 12, height: widget.size + 12, decoration: BoxDecoration( shape: BoxShape.circle, color: Colors.black.withValues(alpha: 0.3), ), - ); - }, + ), ),🛠️ Option 2: Remove AnimatedBuilder if no animation needed
// Background circle if (widget.hasBackground) - AnimatedBuilder( - animation: _scaleAnimation, - builder: (context, child) { - return Container( - width: widget.size + 12, - height: widget.size + 12, - decoration: BoxDecoration( - shape: BoxShape.circle, - color: Colors.black.withValues(alpha: 0.3), - ), - ); - }, - ), + Container( + width: widget.size + 12, + height: widget.size + 12, + decoration: BoxDecoration( + shape: BoxShape.circle, + color: Colors.black.withValues(alpha: 0.3), + ), + ),lib/app/ui/widgets/common/animated_widgets.dart-10-46 (1)
10-46:staggerDelayparameter is unused.The
staggerDelayparameter (line 17, 25) is accepted in the constructor but never used in thebuildmethod. The widget animates immediately without any stagger delay based on index.Either implement the stagger logic or remove the unused parameter.
🛠️ Suggested implementation using stagger delay
class StaggeredListItem extends StatelessWidget { const StaggeredListItem({ super.key, required this.index, required this.child, this.duration = AppAnimations.listItemDuration, this.curve = AppAnimations.listItemCurve, this.staggerDelay = AppAnimations.staggerDelay, this.offset = const Offset(0, 30), }); final int index; final Widget child; final Duration duration; final Curve curve; final Duration staggerDelay; final Offset offset; `@override` Widget build(BuildContext context) { + final totalDelay = Duration( + milliseconds: staggerDelay.inMilliseconds * index, + ); return TweenAnimationBuilder<double>( tween: Tween(begin: 0.0, end: 1.0), - duration: duration, + duration: totalDelay + duration, curve: curve, builder: (context, value, child) { + // Clamp value to account for delay portion + final delayRatio = totalDelay.inMilliseconds / + (totalDelay + duration).inMilliseconds; + final animValue = ((value - delayRatio) / (1 - delayRatio)) + .clamp(0.0, 1.0); return Transform.translate( - offset: Offset(offset.dx, offset.dy * (1 - value)), + offset: Offset(offset.dx, offset.dy * (1 - animValue)), child: Opacity( - opacity: value, + opacity: animValue, child: child, ), ); }, child: child, ); } }Alternatively, consider removing the parameter if stagger logic is handled externally.
lib/app/ui/widgets/common/animated_widgets.dart-170-175 (1)
170-175:delayimplementation doesn't create an actual delay.Adding
delaytoduration(line 174) makes the animation run longer, but it doesn't delay the start. The animation begins immediately and runs fordelay + durationmilliseconds with no initial pause.For a true delay, consider using
Future.delayedor a custom animation curve that holds at 0 initially.🛠️ Suggested fix using Interval curve
`@override` Widget build(BuildContext context) { + final totalDuration = delay + duration; + final delayRatio = delay.inMilliseconds / totalDuration.inMilliseconds; return TweenAnimationBuilder<double>( tween: Tween(begin: 0.0, end: 1.0), - duration: delay + duration, - curve: curve, + duration: totalDuration, + curve: Interval(delayRatio, 1.0, curve: curve), builder: (context, value, child) { final animatedChild = Opacity( opacity: value, child: child, );lib/app/ui/widgets/cards/base_card.dart-188-199 (1)
188-199: Hook methods called multiple times - cache results in local variables.
buildOverlayContent(context)is called twice (lines 191-192) andbuildBelowImageContent(context)is called twice (lines 197-198). While likely not expensive, this is inefficient and could cause issues if the methods have side effects.🔧 Suggested fix
`@override` Widget build(BuildContext context) { final effectiveBorderRadius = borderRadius ?? BorderRadius.circular(18); final effectiveInset = overlayInset ?? AppDimensions.cardPaddingLg.toDouble(); + final overlayContent = buildOverlayContent(context); + final belowImageContent = buildBelowImageContent(context); final imageWidget = imageUrl != null && imageUrl!.isNotEmpty ? _buildCachedImage(context) : buildPlaceholder(context); // ... in the Stack ... // Overlay content - if (buildOverlayContent(context) != null) - buildOverlayContent(context)!, + if (overlayContent != null) + overlayContent, ], ), ), // Content below image - if (buildBelowImageContent(context) != null) - buildBelowImageContent(context)!, + if (belowImageContent != null) + belowImageContent,lib/app/ui/widgets/common/animated_search_bar.dart-88-99 (1)
88-99: Potential issue:Future.delayedcallback may fire after widget disposal.The
Future.delayedon line 91 doesn't check if the widget is still mounted before calling_focusNode.requestFocus(). This could cause issues if the widget is disposed during the delay.🔧 Suggested fix
if (widget.isExpanded) { _controller.forward(); // Auto-focus when expanded Future.delayed(const Duration(milliseconds: 100), () { - _focusNode.requestFocus(); + if (mounted) { + _focusNode.requestFocus(); + } }); } else {lib/app/ui/theme/page_transitions.dart-521-527 (1)
521-527: Unused parameterpageinreplaceWithTransition.The
pageparameter is never used; onlynewPageis passed to the transition.🐛 Proposed fix
- Future<T?> replaceWithTransition<T>({required Widget page, required Widget newPage}) { + Future<T?> replaceWithTransition<T>({required Widget newPage}) { return Navigator.pushReplacement<T, dynamic>( this, FadeInUpTransition(page: newPage) as Route<T>, ); }lib/features/explore/controllers/explore_controller.dart-456-465 (1)
456-465: Incomplete favorite status update —nearbyHotelslist not updated.The method only updates
popularHomesbut the comment on line 464 indicatesnearbyHotelsshould also be updated. This will cause UI inconsistency when a property from the nearby section is favorited.🐛 Proposed fix
void _updatePropertyFavoriteStatusInLists(int propertyId, bool isFavorite) { // This correctly updates the UI by creating a new instance of the Property int index = popularHomes.indexWhere((p) => p.id == propertyId); if (index != -1) { popularHomes[index] = popularHomes[index].copyWith( isFavorite: isFavorite, ); } - // Repeat for other lists like nearbyHotels if you have them + // Update nearbyHotels list as well + index = nearbyHotels.indexWhere((p) => p.id == propertyId); + if (index != -1) { + nearbyHotels[index] = nearbyHotels[index].copyWith( + isFavorite: isFavorite, + ); + } }lib/app/ui/widgets/common/premium_animated_section.dart-152-161 (1)
152-161: Stagger delay is not applied per-item.Each
PremiumAnimatedSectionreceives the samedelayvalue. To achieve a true cascading/stagger effect, the delay should be multiplied by the index.🐛 Proposed fix for stagger effect
`@override` Widget build(BuildContext context) { return Column( crossAxisAlignment: CrossAxisAlignment.start, children: [ for (int i = 0; i < itemCount; i++) PremiumAnimatedSection( key: ValueKey('item_$i'), index: i, - delay: delay, + delay: delay + Duration(milliseconds: i * 50), // Stagger by index duration: duration, curve: curve, slideOffset: slideOffset, child: itemBuilder(context, i), ), ], ); }
| factory MessageModel.fromMap(Map<String, dynamic> map) => MessageModel( | ||
| id: map['id']?.toString() ?? '', | ||
| conversationId: map['conversationId']?.toString() ?? '', | ||
| senderId: map['senderId']?.toString() ?? '', | ||
| content: map['content'] as String? ?? '', | ||
| createdAt: | ||
| DateTime.tryParse(map['createdAt'] as String? ?? '') ?? DateTime.now(), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the full message_model.dart file
cat -n lib/app/data/models/message_model.dartRepository: 360ghar/stays-app
Length of output: 1162
🏁 Script executed:
# List other model files to understand the project pattern
find lib/app/data/models -name "*_model.dart" -type fRepository: 360ghar/stays-app
Length of output: 666
🏁 Script executed:
# Check for generated .g.dart files
find lib/app/data/models -name "*.g.dart" -type fRepository: 360ghar/stays-app
Length of output: 748
🏁 Script executed:
# Search for `@JsonSerializable` usage in model files
rg "@JsonSerializable" lib/app/data/models/ -A 2 -B 2Repository: 360ghar/stays-app
Length of output: 1790
🏁 Script executed:
# Check the generated message_model.g.dart file
cat -n lib/app/data/models/message_model.g.dartRepository: 360ghar/stays-app
Length of output: 1089
🏁 Script executed:
# Check if message_model.dart has any `@JsonSerializable` annotation or part directive
head -20 lib/app/data/models/message_model.dartRepository: 360ghar/stays-app
Length of output: 646
🏁 Script executed:
# Compare with another model that uses `@JsonSerializable`
head -25 lib/app/data/models/amenity_model.dartRepository: 360ghar/stays-app
Length of output: 626
Replace manual fromMap/toMap with @JsonSerializable annotation to follow project pattern.
The MessageModel class deviates from the established pattern used in all other models (AmenityModel, PropertyModel, HotelModel, etc.). Add the required annotation and use the generated methods instead:
- Add
import 'package:json_annotation/json_annotation.dart';at the top - Add
part 'message_model.g.dart';after imports - Add
@JsonSerializable()annotation before the class - Replace manual
fromMap()/toMap()withfromJson()/toJson()that delegate to generated_$MessageModelFromJson()and_$MessageModelToJson() - Optionally add backwards-compatible
fromMap()/toMap()that callfromJson()/toJson(), similar to other models
The generated .g.dart file already exists but is unused. Additionally, the current manual implementation uses DateTime.now() as a fallback, which silently masks invalid date data—the generated code uses strict DateTime.parse() instead.
🤖 Prompt for AI Agents
In `@lib/app/data/models/message_model.dart` around lines 16 - 23, Replace the
manual Map serialization in MessageModel with the project-standard
json_serializable setup: add the import for json_annotation and the part
directive for message_model.g.dart, annotate the MessageModel class with
`@JsonSerializable`(), remove/replace the manual factory MessageModel.fromMap and
toMap with factory MessageModel.fromJson delegating to
_$MessageModelFromJson(map) and Map<String, dynamic> toJson() delegating to
_$MessageModelToJson(this); you may keep thin backwards-compatible fromMap/toMap
wrappers that call fromJson/toJson if desired; ensure DateTime parsing uses the
generated behavior (i.e., rely on DateTime.parse via the generated code) and
reference the existing message_model.g.dart for generated helpers.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/app/data/services/remember_me_service.dart (2)
57-75: Potential logic issue:persistSessionunconditionally sets remember-me flag to true.Line 66 sets
_rememberMeFlagKeytotrueregardless of user preference. IfpersistSessionis called whenisEnabledisfalse, this overrides the user's choice. Consider checkingisEnabled.valueor removing this line since callers should usesetEnabledto control the flag.🐛 Suggested fix
Future<void> persistSession({Session? session}) async { final activeSession = session ?? Supabase.instance.client.auth.currentSession; if (activeSession == null) { AppLogger.warning('Cannot persist session: no active session'); return; } - await _storage.write(_rememberMeFlagKey, true); + // Only persist tokens; the flag is managed by setEnabled() await _storage.write(_accessTokenKey, activeSession.accessToken); final refreshToken = activeSession.refreshToken; if (refreshToken != null && refreshToken.isNotEmpty) { await _storage.write(_refreshTokenKey, refreshToken); } AppLogger.info('Session persisted for remember-me'); }
10-16: Replace GetStorage with flutter_secure_storage for token persistence.Tokens are stored in plain text via
GetStorage(lines 67, 71) without encryption, violating the security guideline: "Encrypt sensitive data using proper encryption mechanisms before storing locally." Sinceflutter_secure_storageis already a dependency, use it for persisting_accessTokenKeyand_refreshTokenKeyinstead ofGetStorage, which stores data unencrypted. Alternatively, encrypt tokens before writing toGetStorageusing the availablecryptopackage.lib/app/utils/services/connectivity_service.dart (1)
63-72: Ethernet incorrectly mapped toConnectivityStatus.mobile.Line 66 maps
ConnectivityResult.ethernettoConnectivityStatus.mobile, which is semantically misleading. Consider adding anethernetvariant toConnectivityStatusor mapping it toconnectedinstead.🔧 Suggested fix
Option 1 - Add ethernet to enum:
-enum ConnectivityStatus { connected, disconnected, mobile, wifi } +enum ConnectivityStatus { connected, disconnected, mobile, wifi, ethernet }Then update the handler:
} else if (results.contains(ConnectivityResult.mobile) || results.contains(ConnectivityResult.ethernet)) { - status.value = ConnectivityStatus.mobile; + status.value = results.contains(ConnectivityResult.ethernet) + ? ConnectivityStatus.ethernet + : ConnectivityStatus.mobile; }Option 2 - Map ethernet to connected:
- } else if (results.contains(ConnectivityResult.mobile) || - results.contains(ConnectivityResult.ethernet)) { + } else if (results.contains(ConnectivityResult.ethernet)) { + status.value = ConnectivityStatus.connected; + } else if (results.contains(ConnectivityResult.mobile)) { status.value = ConnectivityStatus.mobile; }
🤖 Fix all issues with AI agents
In `@lib/app/data/services/analytics_service.dart`:
- Line 27: The singleton accessor AnalyticsService.I uses
Get.find<AnalyticsService>(), so register the service as permanent in your
initial bindings: replace the current Get.put registration with
Get.put<AnalyticsService>(AnalyticsService(enabled:
AppConfig.I.enableAnalytics), permanent: true); this ensures AnalyticsService is
not disposed and remains available for Get.find across the app (update the
registration in the InitialBinding where AnalyticsService is currently put).
- Line 35: The unawaited Firebase Analytics calls in AnalyticsService (e.g., the
call using _analytics!.setAnalyticsCollectionEnabled(true) and other unawaited
calls referencing _analytics, setUserId, setUserProperty, logEvent, etc.) need
error handling so async failures are logged; update each unawaited(...) call to
either append .catchError((e, s) => AppLogger.warning('Analytics <methodName>
failed', error: e, stack: s)) or wrap the call in a small try-catch helper
(e.g., safeAnalyticsCall(() async { await _analytics!.setUserId(...); })) that
catches exceptions and calls AppLogger.warning with the error and
stacktrace—ensure you reference the same _analytics instance and preserve
existing behavior while logging failures.
- Around line 9-14: AnalyticsService currently skips Firebase setup when enabled
is false, which lets the SDK auto-collect; always initialize Firebase Analytics
in AnalyticsService (call _initializeFirebaseAnalytics from the constructor
regardless of enabled) and after creating/assigning _analytics call
_analytics.setAnalyticsCollectionEnabled(enabled) to enforce the consent gate;
also ensure the service binding is registered permanent: true in
initial_binding.dart to match app-wide service lifetime.
In `@lib/app/data/services/supabase_service.dart`:
- Line 10: The client getter currently returns Supabase.instance.client
unconditionally and can throw if initialize() wasn't run or failed; update the
getter in SupabaseService to check the service initialization state (e.g., the
_initialized flag) and either return a nullable SupabaseClient or throw a
descriptive StateError like "SupabaseService not initialized. Call initialize()
first." Ensure the check references the existing initialize() and _initialized
symbols so callers get a clear runtime message instead of an unexpected
exception.
In `@lib/app/utils/services/connectivity_service.dart`:
- Around line 88-103: In checkConnection(), remove the redundant conditional and
ensure the call to _hasInternetAccess() is awaited; specifically, call
Connectivity().checkConnectivity() once, determine hasTransport from the result
only if you plan to early-return when no transport, otherwise simplify to always
await and return await _hasInternetAccess(); fix the catch to await
_hasInternetAccess() as well and keep AppLogger.error('Failed to check
connection', e) intact—refer to the checkConnection function and the
_hasInternetAccess helper to locate and update these calls.
- Around line 160-178: The method _canReachHost currently short-circuits to true
after InternetAddress.lookup succeeds, which can be a false positive when
offline; change the logic to perform a real Socket.connect check as the primary
connectivity verification (use Socket.connect(host, port, timeout:
Duration(seconds:3))), ensuring the socket is destroyed after success and
catching exceptions to return false, and if you still want DNS as a preliminary
check call InternetAddress.lookup only to early-fail for invalid hosts but do
not return true from the lookup result alone; keep timeouts on both lookup and
connect and preserve existing exception handling around InternetAddress.lookup
and Socket.connect.
In `@lib/app/utils/services/token_service.dart`:
- Around line 12-33: TokenInfo contains a createdAt field that's neither
persisted by StorageService nor used by token logic, and TokenInfo.fromJson
calls DateTime.parse on createdAt without null checks which will throw if
absent; either remove the createdAt field from the TokenInfo class and its
fromJson usage (and any references) or update storage and loading so createdAt
is persisted and loaded consistently (modify StorageService to save 'createdAt',
update _loadStoredTokens() to read and parse it, and add null-guarding in
TokenInfo.fromJson to handle missing values). Ensure you update TokenInfo,
TokenInfo.fromJson, StorageService save/load routines, and _loadStoredTokens()
to remain consistent.
In `@lib/app/utils/services/validation_service.dart`:
- Around line 276-278: The registration form is using the weak preset
passwordRequired; update the form validation to use the stronger preset
passwordStrong (which enforces minLength: 8 + uppercase, lowercase, numbers,
special chars) instead of passwordRequired — locate the registration rules usage
in the form validation controller (the reference to passwordRequired in the
FormValidationController) and replace that reference with passwordStrong so the
registration flow uses the stronger password rules.
🧹 Nitpick comments (16)
lib/app/data/services/push_notification_service.dart (4)
22-22: Unused constructor parameter.
StorageService _is accepted but never used in the class. Either remove the parameter or utilize it if needed for token persistence.♻️ Suggested fix
- PushNotificationService(StorageService _); + PushNotificationService();Or, if
StorageServiceis intended for future use:- PushNotificationService(StorageService _); + final StorageService _storageService; + PushNotificationService(this._storageService);
74-75: Permission result is ignored.
requestPermission()returnsNotificationSettingswhich indicates whether the user granted or denied permissions. Ignoring this means the app cannot adapt its behavior (e.g., show a prompt explaining why notifications are useful if denied).♻️ Suggested improvement
// Request permissions (iOS/macOS) - await _messaging.requestPermission(); + final settings = await _messaging.requestPermission(); + if (settings.authorizationStatus == AuthorizationStatus.denied) { + AppLogger.warning('Push notification permission denied by user'); + }
92-102: Address TODO comments for notification handling.Two TODO items remain:
- Line 95: Show local notification for foreground messages
- Line 101: Navigate to specific screen on notification tap
These are common requirements for production notification handling.
Would you like me to generate implementations for these TODOs using
flutter_local_notificationsfor foreground display and GetX navigation for deep linking?
111-114: Direct provider usage violates repository pattern.Per coding guidelines, services should use repositories for data access rather than directly calling providers. This creates tight coupling and bypasses the abstraction layer.
♻️ Suggested approach
Inject a repository (e.g.,
UsersRepository) through the constructor or GetX binding:+ final UsersRepository _usersRepository; + PushNotificationService(this._usersRepository); ... Future<void> _registerTokenWithBackend(String token) async { try { - if (!Get.isRegistered<UsersProvider>()) { - Get.put<UsersProvider>(UsersProvider()); - } - final provider = Get.find<UsersProvider>(); ... - await provider.registerDeviceToken( + await _usersRepository.registerDeviceToken( token: token, platform: platform, appVersion: appVersion, locale: locale, );Based on coding guidelines: "Implement repository pattern for data access; controllers should not directly call providers, use repositories instead."
lib/app/data/services/supabase_service.dart (1)
4-8: Constructor reordering looks good; consider extendingGetxServiceper coding guidelines.The constructor placement before field declarations is a valid style choice. However, per the coding guidelines for
**/*_service.dart, services should extendGetxServicewith@override onInit()for initialization and proper lifecycle management. This would also enable usingpermanent: truewhen registering for app-wide services.♻️ Suggested refactor to align with GetX patterns
import 'package:supabase_flutter/supabase_flutter.dart'; +import 'package:get/get.dart'; import '../../utils/logger/app_logger.dart'; -class SupabaseService { - SupabaseService({required this.url, required this.anonKey}); +class SupabaseService extends GetxService { + SupabaseService({required this.url, required this.anonKey}); final String url; final String anonKey; + + `@override` + void onInit() { + super.onInit(); + initialize(); + }Based on learnings and coding guidelines, Supabase integration should use GetxService with async initialization in bindings.
lib/app/data/services/crash_reporting_service.dart (2)
56-66: Consider adding error handling for Crashlytics API calls.The
awaitcalls to Crashlytics can throw if the SDK encounters issues. While the parentinit()method has a try-catch, explicit handling here would provide more granular error recovery and logging context.Additionally, verify that logging
api_base_urlto Crashlytics is acceptable for your security posture. While API base URLs are typically public, this custom key will be visible in crash reports.♻️ Optional: Add defensive error handling
Future<void> _initializeCrashlytics() async { if (_crashlytics == null) return; - // Enable crash collection - await _crashlytics!.setCrashlyticsCollectionEnabled(true); - - // Set custom keys for debugging - await _crashlytics!.setCustomKey('environment', AppConfig.I.environment); - await _crashlytics!.setCustomKey('api_base_url', AppConfig.I.apiBaseUrl); + try { + // Enable crash collection + await _crashlytics!.setCrashlyticsCollectionEnabled(true); + + // Set custom keys for debugging + await _crashlytics!.setCustomKey('environment', AppConfig.I.environment); + await _crashlytics!.setCustomKey('api_base_url', AppConfig.I.apiBaseUrl); + } catch (e, stack) { + AppLogger.warning('Failed to configure Crashlytics: $e'); + } }
86-96: Log message may be misleading due to fire-and-forget.Since
unawaited()makes the Crashlytics call fire-and-forget, the log on line 95 executes immediately without waiting for the recording to complete. The message "recorded to Crashlytics" implies the operation succeeded, but it may still be in progress or could fail silently.♻️ Optional: Adjust log message for accuracy
// Record as fatal error unawaited(_crashlytics!.recordFlutterFatalError(details)); - AppLogger.error('Flutter error recorded to Crashlytics', details.exception, details.stack); + AppLogger.error('Flutter error sent to Crashlytics', details.exception, details.stack);lib/app/data/services/image_prefetch_service.dart (3)
12-32: Unbounded growth of tracking sets may lead to memory pressure.The
_prefetchedUrlsand_prefetchingUrlssets can grow indefinitely as users browse more properties. For long-running sessions, this could consume significant memory.Consider adding a size limit with LRU eviction or periodic cleanup:
♻️ Suggested improvement
+ /// Maximum number of URLs to track in prefetched set + static const int _maxTrackedUrls = 500; + /// Track URLs that have been successfully prefetched final Set<String> _prefetchedUrls = {}; + + void _addToPrefetched(String url) { + if (_prefetchedUrls.length >= _maxTrackedUrls) { + // Remove oldest entries (first ~10%) + final toRemove = _prefetchedUrls.take(_maxTrackedUrls ~/ 10).toList(); + _prefetchedUrls.removeAll(toRemove); + } + _prefetchedUrls.add(url); + }Then use
_addToPrefetched(imageUrl)instead of_prefetchedUrls.add(imageUrl)in_executePrefetch.
34-38: AddonInit()override per GetxService lifecycle pattern.Per coding guidelines, services extending
GetxServiceshould overrideonInit()for initialization. While theinit()method works withGet.putAsync(), addingonInit()ensures proper lifecycle management if the service is registered differently.♻️ Suggested addition
+ `@override` + void onInit() { + super.onInit(); + AppLogger.info('ImagePrefetchService onInit'); + } + /// Initialize the service Future<ImagePrefetchService> init() async { AppLogger.info('ImagePrefetchService initialized'); return this; } + + `@override` + void onClose() { + clearPrefetchTracking(); + super.onClose(); + }Based on learnings, services should extend GetxService with
@override onInit()for initialization and implement required lifecycle methods.
153-165: Log failures inprecacheForDisplayfor observability.While silent failure is acceptable for best-effort prefetching, completely swallowing errors makes debugging difficult. Consider logging at debug level (consistent with
_executePrefetch).♻️ Suggested improvement
Future<void> precacheForDisplay(BuildContext context, String? imageUrl) async { if (imageUrl == null || imageUrl.isEmpty) return; try { await precacheImage( CachedNetworkImageProvider(imageUrl), context, ); - } catch (_) { - // Silently fail + } catch (e) { + AppLogger.debug('Failed to precache for display: ${_truncateUrl(imageUrl)}'); } }lib/app/data/services/remember_me_service.dart (3)
18-27: Consider using@override onInit()lifecycle method and add error handling.Per coding guidelines, services extending
GetxServiceshould use the@override onInit()lifecycle method for initialization. Additionally, async operations should be wrapped in try/catch blocks.♻️ Suggested refactor using GetxService lifecycle
- /// Initialize the service - Future<RememberMeService> init() async { - await GetStorage.init(_boxName); - _storage = GetStorage(_boxName); - isEnabled.value = _storage.read<bool>(_rememberMeFlagKey) ?? false; - AppLogger.info( - 'RememberMeService initialized. Enabled: ${isEnabled.value}', - ); - return this; - } + `@override` + void onInit() { + super.onInit(); + _initStorage(); + } + + Future<void> _initStorage() async { + try { + await GetStorage.init(_boxName); + _storage = GetStorage(_boxName); + isEnabled.value = _storage.read<bool>(_rememberMeFlagKey) ?? false; + AppLogger.info( + 'RememberMeService initialized. Enabled: ${isEnabled.value}', + ); + } catch (e, stackTrace) { + AppLogger.error('Failed to initialize RememberMeService', e, stackTrace); + } + }
48-55: Named parameter improves API clarity; consider adding error handling.The change to a named required parameter enhances call-site readability. However, async operations should be wrapped in try/catch per coding guidelines.
♻️ Optional: Add error handling
Future<void> setEnabled({required bool value}) async { + try { isEnabled.value = value; await _storage.write(_rememberMeFlagKey, value); if (!value) { await clearStoredSession(); } + } catch (e, stackTrace) { + AppLogger.error('Failed to update remember-me setting', e, stackTrace); + rethrow; + } }
84-92: Minor redundancy insyncAfterLogin.Line 89 writes
falseto_rememberMeFlagKey, butisEnabled.valueis alreadyfalseat this point (checked on line 86). This write is redundant ifsetEnabled({value: false})was previously called. Consider removing it for consistency, or ensure the reactive state stays in sync.♻️ Simplify syncAfterLogin
Future<void> syncAfterLogin() async { if (isEnabled.value) { await persistSession(); } else { - await _storage.write(_rememberMeFlagKey, false); await clearStoredSession(); } }lib/app/utils/services/connectivity_service.dart (2)
29-36: Consider wrapping cleanup in try-catch for diagnostic logging.Cleanup failures in
onCloseare silently swallowed. While this is often acceptable for cleanup code, adding error logging could help diagnose issues in production.♻️ Suggested improvement
`@override` void onClose() { - if (_subscription != null) { - unawaited(_subscription!.cancel()); - } - unawaited(_connectionChangeController.close()); + try { + if (_subscription != null) { + unawaited(_subscription!.cancel()); + } + unawaited(_connectionChangeController.close()); + } catch (e, stack) { + AppLogger.error('Failed to cleanup connectivity resources', e, stack); + } super.onClose(); }
123-140: Hardcodedgoogle.commay cause issues in restricted regions.Using
google.comas a fallback reachability check could fail in regions where Google services are blocked (e.g., China), and may raise privacy/compliance concerns. Consider making fallback hosts configurable viaAppConfig.♻️ Suggested improvement
Future<bool> _hasInternetAccess() async { final apiHost = _resolveApiHost(); + final fallbackHosts = AppConfig.I.connectivityCheckHosts ?? ['cloudflare.com', '1.1.1.1']; final hosts = <String>{ if (apiHost.isNotEmpty) apiHost, - 'google.com', + ...fallbackHosts, }; for (final host in hosts) { if (await _canReachHost( host, - host == apiHost ? _resolveHealthPort() : 443, + host == apiHost ? _resolveHealthPort() : 443, )) { return true; } } return false; }Add to
AppConfig:List<String>? get connectivityCheckHosts => ...;lib/app/data/services/analytics_service.dart (1)
9-14: Move initialization to GetxService.onInit() lifecycle hook.Constructor side effects can bypass GetX lifecycle hooks. Moving
_initializeFirebaseAnalytics()and the info log to anonInit()override keeps initialization consistent with GetX patterns.♻️ Suggested refactor
- AnalyticsService({required this.enabled}) { - if (enabled) { - _initializeFirebaseAnalytics(); - AppLogger.info('AnalyticsService initialized with Firebase Analytics'); - } - } + AnalyticsService({required this.enabled}); + + `@override` + void onInit() { + if (enabled) { + _initializeFirebaseAnalytics(); + AppLogger.info('AnalyticsService initialized with Firebase Analytics'); + } + super.onInit(); + }
|
|
||
| // Enable analytics collection | ||
| _analytics!.setAnalyticsCollectionEnabled(true); | ||
| unawaited(_analytics!.setAnalyticsCollectionEnabled(true)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "analytics_service.dart" -type fRepository: 360ghar/stays-app
Length of output: 107
🏁 Script executed:
cat -n lib/app/data/services/analytics_service.dart | head -350Repository: 360ghar/stays-app
Length of output: 11311
🌐 Web query:
Firebase Analytics Flutter unawaited futures error handling best practices
💡 Result:
Best practices (Flutter + firebase_analytics) for unawaited Futures & error handling
-
Default to
awaitfor “important” analytics- If logging must happen before navigation/exit (e.g., checkout, purchase),
awaitthe call and handle failures withtry/catch. FlutterFire’s own examples showawaitforlogEvent/suggested events. [1]
- If logging must happen before navigation/exit (e.g., checkout, purchase),
-
If you truly want fire-and-forget, be explicit and still handle errors
- Use
unawaited(future)to document intent and satisfy theunawaited_futureslint—but note: it does not handle errors; failures can still become unhandled asynchronous errors. [2][3] - Therefore, attach an error handler:
import 'dart:async'; import 'package:firebase_analytics/firebase_analytics.dart'; void logAnalyticsSafely(String name, Map<String, Object?> params) { unawaited( FirebaseAnalytics.instance .logEvent(name: name, parameters: params) .catchError((e, st) { // Swallow or report (Crashlytics/logger) based on your policy. }), ); }
- Use
-
Don’t rely on global handlers as your primary strategy
- Unhandled Future/Stream errors end up in the zone’s uncaught error handler (effectively “uncaught async errors”). Treat global handlers as a last-resort safety net, not routine control flow. [3]
-
Enable the right lints
- Turn on
unawaited_futures(and considerdiscarded_futures) so you’re forced to make a deliberate choice:await,.catchError, or explicitlyunawaited(...). [2]
- Turn on
Rule of thumb: Await analytics when correctness matters; otherwise fire-and-forget only with an attached error handler (or intentionally ignored errors with a documented policy). [1][2][3]
Sources
[1] FlutterFire Analytics “Log events” docs (examples use await) (firebase.flutter.dev)
[2] Dart unawaited API docs (suppresses lint; doesn’t handle errors) (api.flutter.dev)
[3] Dart Zone uncaught async error handling (handleUncaughtError) (api.flutter.dev)
Citations:
- 1: https://firebase.flutter.dev/docs/analytics/events/?utm_source=openai
- 2: https://api.flutter.dev/flutter/dart-async/unawaited.html?utm_source=openai
- 3: https://api.flutter.dev/flutter/dart-async/Zone/handleUncaughtError.html?utm_source=openai
Add error handling to unawaited Firebase Analytics calls.
Using unawaited() suppresses the lint warning but does not catch errors. Unhandled async failures can emerge at runtime. Attach .catchError() or wrap calls in a helper with try-catch and AppLogger.warning() to log failures.
🛡️ Example helper
+ Future<void> _safeFirebaseCall(Future<void> Function() call, String label) async {
+ try {
+ await call();
+ } catch (e) {
+ AppLogger.warning('Firebase Analytics $label failed: $e');
+ }
+ }
...
- unawaited(_analytics!.logScreenView(
- screenName: screenName,
- screenClass: params?['screen_class'] ?? screenName,
- ));
+ unawaited(_safeFirebaseCall(
+ () => _analytics!.logScreenView(
+ screenName: screenName,
+ screenClass: params?['screen_class'] ?? screenName,
+ ),
+ 'logScreenView',
+ ));Applies to lines 35, 43, 118–121, 134, 151–159, 172–182, 199–203, 220–223, 235–242, 264, 277, 290, 338–342.
🤖 Prompt for AI Agents
In `@lib/app/data/services/analytics_service.dart` at line 35, The unawaited
Firebase Analytics calls in AnalyticsService (e.g., the call using
_analytics!.setAnalyticsCollectionEnabled(true) and other unawaited calls
referencing _analytics, setUserId, setUserProperty, logEvent, etc.) need error
handling so async failures are logged; update each unawaited(...) call to either
append .catchError((e, s) => AppLogger.warning('Analytics <methodName> failed',
error: e, stack: s)) or wrap the call in a small try-catch helper (e.g.,
safeAnalyticsCall(() async { await _analytics!.setUserId(...); })) that catches
exceptions and calls AppLogger.warning with the error and stacktrace—ensure you
reference the same _analytics instance and preserve existing behavior while
logging failures.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
android/app/build.gradle.kts (1)
59-74: Inconsistent applicationId for dev flavor.The
devflavor uses an explicitapplicationId = "com.example.stays_app.dev"which is completely different from the base packagecom.a360ghar.stays. This creates inconsistency:
Flavor Application ID dev com.example.stays_app.devstaging com.a360ghar.stays.stagingprod com.a360ghar.staysThis can cause issues with:
- Firebase: Requires a separate Firebase app configuration for dev
- Deep links: Different schemes needed per flavor
- Testing: Behavior differences between flavors due to package mismatch
The
com.exampleprefix also suggests this may be leftover from initial project scaffolding.🔧 Suggested fix: Use suffix pattern consistently
productFlavors { create("dev") { dimension = "env" - applicationId = "com.example.stays_app.dev" + applicationIdSuffix = ".dev" resValue("string", "app_name", "360ghar stays (Dev)") }
🤖 Fix all issues with AI agents
In `@android/app/build.gradle.kts`:
- Around line 117-119: Update the coreLibraryDesugaring dependency version from
2.1.4 to 2.1.5 by changing the version string in the dependencies block where
coreLibraryDesugaring("com.android.tools:desugar_jdk_libs:2.1.4") is declared;
ensure the only change is the version number to "2.1.5" so the build uses the
latest desugar_jdk_libs.
- Migrate controllers to BaseController for consistent loading/error handling - Centralize token management through TokenService as single source of truth - Create shared helpers: parseBool() and shouldCountBookingStatus() - Replace all Get.snackbar calls with AppSnackbar for consistent styling - Extract UI code from TripsController to dedicated widget files - Mark deprecated StorageService token methods with @deprecated annotations - Remove dead code and duplicate helper methods across controllers
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
lib/features/tour/views/tour_view.dart (1)
47-67: Align share snackbar with actual clipboard behavior, and localize new strings.The success message says the link was copied, but no clipboard write occurs in this handler. Either perform the copy or change the message. Also, new user‑facing strings should use
.trfor localization.As per coding guidelines, please localize the strings.Proposed fix (copy to clipboard)
+import 'package:flutter/services.dart'; ... if (url?.isNotEmpty == true) { + Clipboard.setData(ClipboardData(text: url!)); AppSnackbar.success( title: 'Share tour', message: 'Tour link copied to clipboard.', );lib/features/auth/controllers/otp_controller.dart (1)
205-223: Missing double-submit guard and consistent error handling.Unlike
verifyOTP(), this method lacks theif (isLoading.value) return;guard clause. Rapid taps on the resend button could trigger multiple OTP requests. Additionally, the same error handling gaps apply (generic catch, noAppLogger, hardcoded strings).🔒 Proposed fix to add guard clause
Future<void> resendOTP() async { + if (isLoading.value) return; + try { isLoading.value = true;lib/features/messaging/controllers/hotels_map_controller.dart (1)
311-321: Avoid exposing raw exception details to users; add proper error logging.Per coding guidelines:
- Raw exception
$eis exposed in the user-facing snackbar, which may leak sensitive details.- Missing
AppLogger.error()call—errors should be logged with the error object and stack trace.- Consider catching specific exception types (e.g.,
PlatformException) before the generic catch.🔧 Proposed fix
} catch (e) { + AppLogger.error('Failed to get current location', e, StackTrace.current); AppSnackbar.error( title: 'Error', - message: 'Failed to get current location: $e', + message: 'Failed to get current location. Please try again.', ); _loadSampleHotels(); } finally {You'll also need to import
AppLogger:import 'package:stays_app/app/utils/logger/app_logger.dart';As per coding guidelines, use
AppLoggerfor logging and avoid exposing internal error details to users.lib/features/listing/views/listing_detail_view.dart (1)
944-975: Localize and log map-launch errors.
These error paths display hardcoded strings and don’t log failures. Please route messages through translation keys and callAppLogger.error(...)with context before showing the snackbar. As per coding guidelines, please localize strings and log errors with AppLogger.lib/features/listing/views/search_results_view.dart (1)
303-377:_PaginationBarreads reactive values withoutObxwrapper — UI won't update.The widget accesses
controller.isLoading.value,controller.isRefreshing.value,controller.currentPage.value, etc. directly inbuild(), but since this is aStatelessWidgetwithout anObxwrapper, it won't rebuild when those observables change. The parentObxat line 52 only covers theCustomScrollView, not the pagination bar added via_buildSlivers.As per coding guidelines, wrap UI in
Obx(() => ...)for reactive updates in GetX.Proposed fix
class _PaginationBar extends StatelessWidget { const _PaginationBar({required this.controller}); final ListingController controller; `@override` Widget build(BuildContext context) { + return Obx(() { final isBusy = controller.isLoading.value || controller.isRefreshing.value; final canGoPrev = controller.currentPage.value > 1 && !isBusy; final canGoNext = controller.currentPage.value < controller.totalPages.value && !isBusy; final pageSize = controller.pageSize.value; final summary = Text( 'Page ${controller.currentPage.value} of ${controller.totalPages.value}', style: const TextStyle(fontWeight: FontWeight.w600), overflow: TextOverflow.ellipsis, ); // ... rest of the method ... return LayoutBuilder( builder: (context, constraints) { // ... }, ); + }); } }lib/features/profile/controllers/edit_profile_controller.dart (1)
204-244: Remove manual error snackbar; rely on ErrorHandler for error feedback.
executeWithErrorHandlinginternally callsErrorHandler.handleError(), which displays error snackbars based on exception type. Lines 240-243 duplicate this behavior whenresultis null. Either omit the explicit error snackbar (lines 240-243) and letErrorHandlerdisplay errors, or passswallowError: trueto suppress automatic error handling if custom messaging is needed. Apply the same fix toPreferencesControllerandNotificationsController, which share this pattern.lib/app/data/repositories/auth_repository.dart (1)
132-144: Missing fallback for token persistence could leave auth state inconsistent.If
TokenServicelookup orreadyfails (lines 135-136), the catch block only logs a warning and returns. The tokens are not persisted, but the calling method continues to return aUserModel, potentially leaving the user "logged in" without persisted tokens. This inconsistency could cause issues on app restart.Consider adding a fallback to
StorageServicesimilar to thelogoutmethod:🔧 Proposed fix
Future<void> _persistTokens(ProviderAuthResult res) async { if (res.accessToken == null) return; try { final tokenService = Get.find<TokenService>(); await tokenService.ready; await tokenService.storeTokens( accessToken: res.accessToken!, refreshToken: res.refreshToken, ); } catch (e) { - AppLogger.warning('Failed to persist tokens: $e'); + AppLogger.warning('TokenService unavailable, falling back to StorageService: $e'); + try { + await _storage.saveTokens( + accessToken: res.accessToken!, + refreshToken: res.refreshToken, + ); + } catch (storageError) { + AppLogger.error('Failed to persist tokens via fallback', storageError); + rethrow; + } } }lib/app/data/services/location_service.dart (1)
85-106: Add translation keys and localize location service warning messages.The two
AppSnackbar.warning()calls use hardcoded English strings that should be routed through GetX localization (.tr) per coding guidelines. Add the missing translation keys to bothl10n/en.jsonandl10n/hi.json:{ "location": { "services_disabled_title": "Location Services", "services_disabled_message": "Please enable location services to get better recommendations", "permission_denied_title": "Location Permission", "permission_denied_message": "Location permissions are permanently denied, please enable from settings" } }Then update the snackbar calls to use
.tr:- AppSnackbar.warning( - title: 'Location Services', - message: 'Please enable location services to get better recommendations', - ); + AppSnackbar.warning( + title: 'location.services_disabled_title'.tr, + message: 'location.services_disabled_message'.tr, + );- AppSnackbar.warning( - title: 'Location Permission', - message: 'Location permissions are permanently denied, please enable from settings', - ); + AppSnackbar.warning( + title: 'location.permission_denied_title'.tr, + message: 'location.permission_denied_message'.tr, + );
🤖 Fix all issues with AI agents
In `@lib/app/ui/widgets/common/location_filter_app_bar.dart`:
- Around line 160-170: The catch block currently swallows exceptions (catch (_))
when fetching location; change it to capture the exception and stack trace
(catch (error, stackTrace)) and call AppLogger.error with a descriptive message
and those values before showing the AppSnackbar.warning; update the catch in the
same scope as the AppSnackbar calls in location_filter_app_bar.dart so the error
is logged via AppLogger.error('Failed to get current location', error,
stackTrace) prior to the user-facing warning.
In `@lib/app/utils/extensions/dynamic_extensions.dart`:
- Around line 10-13: The string parsing branch in parseBool (the block where
value is String and normalized is computed) only recognizes truthy strings and
falls back for all others; update that branch to explicitly handle common falsy
strings (e.g., 'false', '0', 'no') by returning false when normalized matches
them, return true when it matches truthy tokens ('true','1','yes'), and
otherwise return the provided fallback. Locate the code that declares normalized
= value.toLowerCase() and modify it to check falsy tokens before returning the
fallback so parseBool behaves symmetrically for truthy and falsy string inputs.
In `@lib/app/utils/helpers/error_handler.dart`:
- Around line 57-60: The _handleValidationException method can crash when
ValidationException.errors is empty or the first error list is empty; update
_handleValidationException to defensively check ValidationException.errors and
its first value list before accessing values.first.first, derive a fallback
message (e.g., a generic 'Unknown validation error' or join all messages if
present), and pass that safe message to AppSnackbar.warning so
_handleValidationException, ValidationException, and AppSnackbar.warning are
used without risking a StateError.
In `@lib/features/explore/controllers/explore_controller.dart`:
- Around line 320-324: The code currently sets isOffline.value and
errorMessage.value then uses rethrow, which can propagate unexpected exceptions
to callers like refreshData(); modify _fetchInitialData() to not rethrow after
setting the error state—replace the rethrow with a safe return (or otherwise
swallow the exception) so callers (e.g., refreshData()) receive the updated
error state without crashing; update references in the same function where
rethrow is used and ensure refreshData() and any callers of _fetchInitialData()
rely on the errorMessage/isOffline state rather than catching the propagated
exception.
- Around line 457-466: The _updatePropertyFavoriteStatusInLists method currently
only updates popularHomes; update all relevant lists (e.g., nearbyHotels and any
other property lists) by finding the index with indexWhere((p) => p.id ==
propertyId) and replacing the item with its copyWith(isFavorite: isFavorite)
just like for popularHomes; ensure you update each list variable (nearbyHotels,
popularHomes, etc.) so the favorite status stays consistent across views and
avoid mutating the object in place by assigning the copied instance back into
the list.
In `@lib/features/messaging/controllers/chat_controller.dart`:
- Around line 4-6: Change the messages field in ChatController from
RxList<String> to RxList<MessageModel> so the controller stores strongly-typed
MessageModel objects (update the declaration of messages in class ChatController
that currently reads messages = <String>[].obs to use MessageModel). Ensure
imports/reference to MessageModel are present and update any consumer code
accordingly; if message loading/sending/pagination are intentionally not
implemented yet, add a short TODO in ChatController noting that logic will be
added later.
In `@lib/features/profile/controllers/edit_profile_controller.dart`:
- Around line 155-161: In the catch block where both handleError(e) and
AppSnackbar.error(...) are invoked, remove the duplicate feedback so only one
error UX is presented: either delete the AppSnackbar.error call and rely on
handleError(e) to surface the error, or remove handleError(e) and call
AppSnackbar.error with your custom message; update the catch block containing
handleError and AppSnackbar.error to use only the chosen path (reference:
handleError and AppSnackbar.error).
In `@lib/features/profile/controllers/notifications_controller.dart`:
- Around line 110-128: The current code may show two error snackbars because
executeWithErrorHandling may already display errors; decide on a single place to
show errors and implement it: either remove the local AppSnackbar.error block in
NotificationsController (so
executeWithErrorHandling/_profileRepository.updateNotificationSettings is solely
responsible for error UI) or modify executeWithErrorHandling to accept a
parameter (e.g., showError: false) and call it here, keeping the local
AppSnackbar.error; update references to executeWithErrorHandling and
AppSnackbar.error/_profileRepository.updateNotificationSettings accordingly so
only one error snackbar is ever shown.
In `@lib/features/profile/controllers/preferences_controller.dart`:
- Around line 102-119: The explicit AppSnackbar.error after the
executeWithErrorHandling call duplicates the snackbar already shown by
ErrorHandler.handleError; update the executeWithErrorHandling invocation (the
call that wraps _profileRepository.updatePreferences in
preferences_controller.dart) to pass swallowError: true so ErrorHandler does not
show its snackbar, allowing you to keep the explicit
feedbackMessage/AppSnackbar.success and the explicit AppSnackbar.error branch;
alternatively, remove the explicit AppSnackbar.error branch (the else block that
sets the error snackbar) and keep executeWithErrorHandling as-is so
ErrorHandler.handleError handles UI error reporting.
In `@lib/features/splash/controllers/splash_controller.dart`:
- Around line 222-234: The empty catch in the token persistence branch swallows
TokenService errors; update the try/catch around tokenService.storeTokens
(referenced as tokenService and its method storeTokens) to catch the exception
and log it via AppLogger (include error and stack/exception details) before
falling back to storageService.saveTokens (referenced as storageService and
saveTokens), so failures are recorded for audit while preserving the existing
fallback behavior.
- Around line 242-256: Replace the empty catch blocks in _signOutAndClear so
failures are logged (not swallowed): for each operation
(Supabase.instance.client.auth.signOut, tokenService?.clearTokens,
storageService.clearTokens, storageService.clearUserData) catch exceptions as
catch (e, st) and call your app's structured logger to record a descriptive
message, the exception e, and stack trace st (include operation name like
"signOut", "tokenService.clearTokens", etc.) so these security-relevant cleanup
failures are auditable while still allowing the method to continue.
♻️ Duplicate comments (1)
lib/app/utils/services/token_service.dart (1)
19-28:createdAtparsing lacks null safety and field is not persisted.
TokenInfo.fromJsonat line 26 callsDateTime.parse()directly onjson['createdAt'], which will throw aFormatExceptionif the field is null or missing. While this method isn't currently called in the codebase (tokens are loaded via_loadStoredTokenswherecreatedAtis set toDateTime.now()), it's a latent bug iffromJsonis used in the future.Consider adding null handling or removing the unused
createdAtfield entirely:🔧 Proposed fix for null safety
factory TokenInfo.fromJson(Map<String, dynamic> json) { return TokenInfo( accessToken: json['accessToken'] as String, refreshToken: json['refreshToken'] as String?, expiresAt: json['expiresAt'] != null ? DateTime.parse(json['expiresAt'] as String) : null, - createdAt: DateTime.parse(json['createdAt'] as String), + createdAt: json['createdAt'] != null + ? DateTime.parse(json['createdAt'] as String) + : DateTime.now(), ); }
🧹 Nitpick comments (16)
lib/features/auth/controllers/otp_controller.dart (2)
143-182: Missing specific exception handling, logging, and repository pattern.Per coding guidelines, this method has several gaps:
- Direct Supabase call (line 162-166): Controllers should use the repository pattern rather than calling providers directly.
- Generic catch block (line 177): Should catch
AuthExceptionseparately before the generic catch to provide specific error messages for auth failures.- Missing AppLogger: Errors should be logged with
AppLogger.error()before showing user feedback.- Hardcoded strings: Error messages should use
.trfor localization.♻️ Suggested improvement for error handling
} catch (e) { + AppLogger.error('OTP verification failed', e); otpError.value = 'Invalid or expired OTP. Please try again'; } finally {Consider also extracting Supabase auth calls to an
AuthRepositoryfor consistency with the repository pattern, and using translation keys like'otp.error.invalid'.trfor localized messages.
159-161: Hardcoded country code limits internationalization.The
'+91'prefix is hardcoded in multiple places. Consider extracting this to a constant or configuration value for easier maintenance if the app expands to other regions.+// At class level or in AppConfig +static const String DEFAULT_COUNTRY_CODE = '+91'; + final formattedPhone = phoneNumber.startsWith('+') ? phoneNumber - : '+91$phoneNumber'; + : '$DEFAULT_COUNTRY_CODE$phoneNumber';lib/features/wishlist/controllers/wishlist_controller.dart (2)
183-189: Consider catchingApiExceptionseparately for better error handling.Per coding guidelines, controllers should catch specific exception types before generic exceptions to enable HTTP status code mapping and provide more specific error messages.
♻️ Suggested improvement
+ } on ApiException catch (e) { + AppLogger.error('API error adding to wishlist', e); + AppSnackbar.error( + title: 'error'.tr, + message: e.userFriendlyMessage ?? 'wishlist.add_error'.tr, + ); } catch (e) { AppLogger.error('Error adding to wishlist', e); AppSnackbar.error( - title: 'Error', - message: 'Failed to add to wishlist. Please try again.', + title: 'error'.tr, + message: 'wishlist.add_error'.tr, ); }Based on learnings, controllers should catch
ApiExceptionseparately from other exceptions and use.trfor localized error messages.
276-292: Sequential deletion may cause performance issues with large wishlists.The loop on lines 277-279 makes N sequential API calls. If the wishlist has many items, this could be slow and prone to partial failures.
💡 Consider a batch delete endpoint or parallel execution
If a batch endpoint is available:
await _wishlistRepository!.clearAll();Alternatively, for parallel execution with controlled concurrency:
final ids = wishlistItems.map((item) => item.id).toList(); await Future.wait(ids.map((id) => _wishlistRepository!.remove(id)));The snackbar migration itself is correct.
lib/features/messaging/controllers/hotels_map_controller.dart (1)
83-88: Consider logging failed service lookups instead of silently swallowing exceptions.Empty catch blocks silently swallow exceptions. While optional service lookup is valid, consider logging at debug level for troubleshooting.
♻️ Suggested improvement
try { _propertiesService = Get.find<PropertiesRepository>(); - } catch (_) {} + } catch (e) { + AppLogger.debug('PropertiesRepository not registered: $e'); + } try { _placesService = Get.find<PlacesService>(); - } catch (_) {} + } catch (e) { + AppLogger.debug('PlacesService not registered: $e'); + }lib/app/ui/widgets/common/location_filter_app_bar.dart (1)
178-186: Inconsistent snackbar usage — consider migrating toAppSnackbar.This method still uses
ScaffoldMessenger.of(context).showSnackBar()while the rest of the file has been migrated toAppSnackbar. For consistency across the codebase, consider usingAppSnackbar.warninghere as well.Proposed refactor
void _showFilterUnavailable(BuildContext context) { - final colors = Theme.of(context).colorScheme; - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - backgroundColor: colors.error, - content: const Text('Filters are unavailable on this screen'), - ), - ); + AppSnackbar.warning( + title: 'Unavailable', + message: 'Filters are unavailable on this screen', + ); }lib/features/inquiry/views/inquiry_view.dart (1)
179-190: Consider adding debug logging for silent storage failures.While the comment explains the intentional silent catch, adding
AppLogger.debugwould help with troubleshooting without affecting user experience. Per coding guidelines, useAppLoggerfor logging with appropriate levels.Proposed refactor
Future<void> _tryPrefillFromStorage() async { try { if (!Get.isRegistered<StorageService>()) return; final storage = Get.find<StorageService>(); final map = await storage.getUserData(); if (map == null) return; final cachedUser = UserModel.fromMap(map); _prefillFromUser(cachedUser); - } catch (_) { - // ignore silently; UI already has manual entry available + } catch (e) { + // Non-critical: UI has manual entry fallback + AppLogger.debug('Storage prefill skipped: $e'); } }lib/features/profile/controllers/edit_profile_controller.dart (1)
33-34: UnusedWorkerfields can be removed.The
_profileChangeWorkerand_authChangeWorkerfields are assigned viatrackWorkerbut never referenced afterward. SincetrackWorkermanages disposal automatically, these fields are unnecessary overhead.♻️ Proposed removal of unused fields
- // Listeners for profile changes - late final Worker _profileChangeWorker; - late final Worker _authChangeWorker;And update the
onInit:- _profileChangeWorker = trackWorker(ever(_profileController.user, (UserModel? user) { + trackWorker(ever(_profileController.user, (UserModel? user) { if (user != null) { _updateFieldsFromUser(user); } })); - _authChangeWorker = trackWorker(ever(_authController.currentUser, (UserModel? user) { + trackWorker(ever(_authController.currentUser, (UserModel? user) { if (user != null) { _updateFieldsFromUser(user); } }));lib/features/explore/controllers/explore_controller.dart (2)
234-276: Error message detection via string matching is fragile.Matching substrings like
'401','token','connection'inerror.toString()can produce false positives (e.g., a property named "Connection Place" or ID containing "401"). Consider:
- Using exception types (
ApiException,NetworkException) from the custom hierarchy- Checking
error.runtimeTypeor status codes directly♻️ Suggested approach using exception types
String _getUserFriendlyErrorMessage(dynamic error) { if (error is ApiException) { switch (error.statusCode) { case 401: return 'Session expired. Please log in again.'; case 403: return 'Access denied. Please log in again.'; case 404: return 'Requested resource not found.'; case >= 500: return 'Server is temporarily unavailable. Please try again later.'; } } if (error is NetworkException) { return 'No internet connection. Please check your network and try again.'; } // ... other exception types AppLogger.warning('Unhandled error type: ${error.runtimeType} - $error'); return 'Unable to load properties. Pull down to refresh.'; }
89-98: Consider cachingnearestPropertyto avoid repeated iteration.
popularInCityandnearbyStaysboth callnearestProperty, which iteratesallExplorePropertieseach time. When both getters are accessed (common in UI), this causes redundant iteration. Consider caching the result:♻️ Optional optimization
// Add a cached field final Rx<Property?> _cachedNearestProperty = Rx<Property?>(null); // Update in _updatePropertiesFromResponse after assigning lists: void _updatePropertiesFromResponse(List<Property> props) { // ... existing code ... // Cache nearest property _cachedNearestProperty.value = _computeNearestProperty(); } Property? _computeNearestProperty() { // Current nearestProperty logic } Property? get nearestProperty => _cachedNearestProperty.value;lib/app/utils/services/token_service.dart (2)
337-339: Force-unwrap of_storageServicecould throw unhelpful NPE.If
_initializefails to set_storageService(e.g.,StorageService.readythrows), the force-unwrap on lines 337-339 will throw a null-check error with no context. While the outer try-catch handles this, the error message won't indicate the root cause.🔧 Suggested improvement
+ final storage = _storageService; + if (storage == null) { + AppLogger.warning('StorageService not available, skipping token load'); + return; + } - final accessToken = await _storageService!.getAccessToken(); - final refreshToken = await _storageService!.getRefreshToken(); - final expiresAtStr = await _storageService!.getTokenExpiration(); + final accessToken = await storage.getAccessToken(); + final refreshToken = await storage.getRefreshToken(); + final expiresAtStr = await storage.getTokenExpiration();
397-402: Refresh debounce could return stale state.When
_isRefreshingis true, the method waits 1 second and returnshasValidToken. This doesn't guarantee the concurrent refresh completed—it might still be in progress, causing the caller to receive stale state.Consider using a
Completerto wait for the actual refresh result:🔧 Suggested improvement
+ Completer<bool>? _refreshCompleter; + Future<bool> _refreshTokens() async { if (_isRefreshing) { AppLogger.info('Token refresh already in progress, waiting...'); - await Future.delayed(const Duration(seconds: 1)); - return hasValidToken; + return _refreshCompleter?.future ?? hasValidToken; } _isRefreshing = true; + _refreshCompleter = Completer<bool>(); try { // ... existing refresh logic ... + final result = hasValidToken; + _refreshCompleter?.complete(result); + return result; } catch (e) { // ... existing error handling ... + _refreshCompleter?.complete(false); + return false; } finally { _isRefreshing = false; + _refreshCompleter = null; } }lib/features/trips/views/widgets/booking_details_sheet.dart (3)
17-22: Consider using theme colors instead of hardcodedColors.white.The hardcoded
Colors.whitewon't adapt to dark mode. Consider using theme-aware colors:🔧 Suggested improvement
return Container( padding: const EdgeInsets.all(24), - decoration: const BoxDecoration( - color: Colors.white, + decoration: BoxDecoration( + color: Theme.of(context).colorScheme.surface, borderRadius: BorderRadius.vertical(top: Radius.circular(20)), ),
41-44: Consider adding localization support for UI strings.Per coding guidelines, use the
.trextension method for user-facing strings to support internationalization:// Example const Text('Inquiry Details'.tr, ...) // or with translation keys const Text('inquiry.details_title'.tr, ...)This applies to strings like 'Inquiry Details', 'Inquire Again', 'Close', and label text in
_buildDetailRow.Also applies to: 75-75, 82-82
116-138: Duplicate_formatDateimplementation exists inTripsController.This date formatting logic is duplicated in
trips_controller.dart(lines 361-382). Consider extracting to a shared helper inlib/app/utils/helpers/for consistency and maintainability.lib/features/trips/controllers/trips_controller.dart (1)
361-382: Consider extracting_formatDateto a shared utility.This method duplicates the implementation in
BookingDetailsSheet. Extract to a shared date formatting helper inlib/app/utils/helpers/to maintain DRY principles.
Add _ensureRememberMePreferenceReady() to prevent race conditions when accessing GetStorage before async initialization completes. chore(prod): initialize Firebase and reformat main_prod.dart
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
lib/features/auth/controllers/auth_controller.dart (3)
394-409:⚠️ Potential issue | 🟡 MinorHardcoded
+91country code and missing error logging in catch blocks.
Line 398: The OTP success message hardcodes
+91, which breaks for any non-Indian phone number. Consider accepting the country code as a parameter or reading it from user input.Lines 401-409: Neither the
ApiExceptioncatch nor the generic catch logs the error viaAppLoggerbefore showing the snackbar.♻️ Suggested fix for logging
} on ApiException catch (e) { + AppLogger.error('Phone signup failed: ${e.message}', e); _showErrorSnackbar(title: 'Signup Failed', message: e.message); return false; } catch (e) { + AppLogger.error('Phone signup error', e); _showErrorSnackbar(As per coding guidelines: "Always wrap API calls in try-catch blocks with specific exception type handling, and log errors with AppLogger before showing user-facing error messages."
559-565:⚠️ Potential issue | 🟡 MinorMissing
AppLogger.errorinregistercatch blocks.The
loginmethod logs errors in both itsApiExceptionand generic catch blocks (lines 332, 335), butregistersilently swallows them into snackbars only. This makes post-mortem debugging harder.♻️ Suggested fix
} on ApiException catch (e) { + AppLogger.error('Registration failed: ${e.message}', e); _showErrorSnackbar(title: 'Registration Failed', message: e.message); } catch (e) { + AppLogger.error('Registration error', e); _showErrorSnackbar(As per coding guidelines: "log errors with AppLogger before showing user-facing error messages."
454-491:⚠️ Potential issue | 🟠 MajorLocal token/state cleanup is skipped if
_authRepository.logout()throws.If the server-side logout call on line 457 fails (e.g., network error), the catch block only shows an error snackbar —
_tokenService.clearTokens(),setRememberMe(false), and the state resets on lines 459–469 are all skipped. The user remains locally "authenticated" with stale tokens, which is a security concern.Consider clearing local state unconditionally (e.g., move cleanup to
finallyor before the remote call):🛡️ Suggested restructure
Future<void> logout() async { try { isLoading.value = true; + // Clear local state first — even if the server call fails the user + // should not remain authenticated locally. + await _tokenService.clearTokens(); + await setRememberMe(false); + currentUser.value = null; + isAuthenticated.value = false; + + // Best-effort server-side sign-out await _authRepository.logout(); - await _tokenService.clearTokens(); - await setRememberMe(false); - currentUser.value = null; - isAuthenticated.value = false;As per coding guidelines: "Properly handle session expiration and implement logout functionality."
🤖 Fix all issues with AI agents
In `@lib/features/auth/controllers/auth_controller.dart`:
- Around line 58-79: onInit currently calls _initAuthStatus() without unawaited
and onReady reads isAuthenticated.value immediately, causing a race where
fetchAndCacheProfile() is skipped; change the onInit call to
unawaited(_initAuthStatus()) to satisfy the lint, and move or duplicate the
fetch trigger so profile loading isn't missed by onReady—either await completion
before checking isAuthenticated in onReady or have onReady call
fetchAndCacheProfile() unconditionally (or guarded by a separate flag) and keep
_checkAuthStatus() calling fetchAndCacheProfile() as backup; ensure all
fire-and-forget calls use unawaited (e.g., _ensureRememberMePreferenceReady(),
_initAuthStatus(), _loadSavedUser()) and reference the _initAuthStatus, onReady,
isAuthenticated, fetchAndCacheProfile, and _checkAuthStatus symbols when making
the changes.
- Around line 593-603: The controller currently creates UsersProvider and
ProfileRepository inside _ensureProfileRepository(), coupling it to concrete
providers; replace this by adding a ProfileRepository constructor parameter
(like AuthRepository and TokenService), store it as a private field (e.g.
_profileRepository), remove _ensureProfileRepository() and all calls to it, and
update the auth binding to register/provide ProfileRepository so the controller
receives it via DI; ensure references to UsersProvider remain only inside the
ProfileRepository implementation, not the controller.
- Around line 161-192: The auth-state listener in _bindAuthStateListener may
read a stale rememberMe.value because _ensureRememberMePreferenceReady() is
async; inside the onAuthStateChange callback (in _bindAuthStateListener) await
readiness before using rememberMe: call await _ensureRememberMePreferenceReady()
(or otherwise await a readiness/future that _ensureRememberMePreferenceReady
exposes) at the top of the signedIn/tokenRefreshed branch, then check
rememberMe.value and conditionally call _persistRememberedSession(session:
session); this ensures _persistRememberedSession is not skipped during
cold-start session restores.
In `@lib/main_prod.dart`:
- Around line 27-28: Remove the outer async/await on main to avoid crossing zone
boundaries: change the signature and invocation so main no longer awaits
runZonedGuarded; either make main void and call runZonedGuarded(...) directly or
return the runZonedGuarded Future (e.g., Future<void> main() =>
runZonedGuarded<Future<void>>(...)); update the existing main and its
runZonedGuarded usage so the initialization code stays inside the guarded
callback and the error handler stays as the second argument without awaiting the
zone from the root zone.
🧹 Nitpick comments (8)
lib/features/auth/controllers/auth_controller.dart (5)
224-261: Misleading fallback comment and swallowed error in legacy migration.Line 237 already awaits
_tokenService.ready, so the catch on line 245 isn't guarding against "TokenService not ready" — it catches anystoreTokensfailure. The comment on line 246 is misleading. More importantly, ifstoreTokensfails for a real reason (e.g., secure storage corruption), silently falling back toStorageService(which may use less secure storage) could mask a problem.Consider logging the actual error in the inner catch:
♻️ Suggested fix
} catch (e) { - // Fallback to StorageService if TokenService not ready yet + AppLogger.warning('TokenService.storeTokens failed during migration: $e'); if (Get.isRegistered<StorageService>()) {
669-703: All user-facing strings are hardcoded English — no.trlocalization.Every snackbar title/message and error string throughout the controller (e.g.,
'Welcome Back!','Login Failed','Invalid credentials…') uses hardcoded English. The coding guidelines require using.trextension or translation keys for user-facing text and mapping HTTP status codes to translation keys.Consider extracting these to a translation file and using
'auth.login_failed'.trstyle keys throughout. This applies to_handleApiError,_showSuccessSnackbar/_showErrorSnackbarcalls inlogin,logout,register,registerWithPhone,sendForgotPasswordOTP, andresetPassword.As per coding guidelines: "Use .tr extension method on strings in Dart code to apply translations" and "Provide localized error messages for API errors by mapping HTTP status codes to translation keys."
21-42: NoTokenExpiredExceptionhandling anywhere in the controller.The coding guidelines and learnings require that
TokenExpiredExceptionbe caught separately — attempting a token refresh first, then forcing logout if refresh fails. None of the public methods (login,register,fetchAndCacheProfile,updateUserProfileData, etc.) catchTokenExpiredException.While
loginandregisterare unlikely to encounter token expiry, the profile methods (fetchAndCacheProfile,updateUserProfileData,updateUserPreferences,updateUserLocation) make authenticated API calls that can receive expired-token errors.Consider adding a shared helper, e.g.:
♻️ Suggested pattern
Future<T> _withTokenRefresh<T>(Future<T> Function() action) async { try { return await action(); } on TokenExpiredException { try { await _tokenService.refreshToken(); return await action(); } catch (_) { await logout(); rethrow; } } }Based on learnings: "For TokenExpiredException, attempt token refresh via AuthController; if refresh fails, force logout to maintain app security."
355-370: Consider catchingApiExceptionseparately for richer diagnostics.
fetchAndCacheProfilecatches a bareException(line 366). Since this is a network call, distinguishingApiException(especially 401) would allow triggering token refresh or logout instead of silently returningnullfor an expired session.
44-51:late final GetStorage _authPrefsrelies entirely on the_rememberMeReadyguard.If any future code path accesses
_authPrefswithout first awaiting_ensureRememberMePreferenceReady(), it will throw aLateInitializationErrorat runtime. Currently all paths call the guard, but this is fragile. Consider initializing_authPrefseagerly with a default or making it nullable with a null check instead.lib/main_prod.dart (3)
30-36: MissingFlutterError.onErrorandPlatformDispatcher.instance.onErrorsetup.Per project guidelines, main entry points should set up global error handlers for Flutter framework errors and platform-level errors. Without these, rendering exceptions and platform errors in production won't be routed to
CrashReportingService/AppLogger.Proposed addition after `WidgetsFlutterBinding.ensureInitialized()`
WidgetsFlutterBinding.ensureInitialized(); + + FlutterError.onError = (FlutterErrorDetails details) { + AppLogger.error('Flutter error', details.exception, details.stack); + if (Get.isRegistered<CrashReportingService>()) { + Get.find<CrashReportingService>().recordFlutterError(details); + } + }; + + PlatformDispatcher.instance.onError = (error, stack) { + AppLogger.error('Platform error', error, stack); + if (Get.isRegistered<CrashReportingService>()) { + Get.find<CrashReportingService>().recordError(error, stackTrace: stack, fatal: true); + } + return true; + }; + await dotenv.load(fileName: '.env.prod');You'll also need to add the import:
import 'dart:ui' show PlatformDispatcher;Based on learnings: "Set up global error handlers in main files: use FlutterError.onError for Flutter errors and PlatformDispatcher.instance.onError for platform errors."
63-79: Consider avoidinglate+ closure assignment fromFuture.waitfor clarity.The
latevariables assigned inside.then()callbacks work correctly withFuture.wait, but the indirection makes it harder to trace initialization order at a glance. An alternative is to useFuture.wait's indexed result list:Alternative pattern
- late ThemeService themeService; - late LocaleService localeService; - - await Future.wait([ - // Supabase initialization (critical) - supabaseService.initialize(), - // Theme service initialization - ThemeService().init().then((service) => themeService = service), - // Locale service initialization - LocaleService().init().then((service) => localeService = service), - // Orientation lock (lightweight, run in parallel) - SystemChrome.setPreferredOrientations([ - DeviceOrientation.portraitUp, - DeviceOrientation.portraitDown, - ]), - ]); + final results = await Future.wait<Object?>([ + supabaseService.initialize(), + ThemeService().init(), + LocaleService().init(), + SystemChrome.setPreferredOrientations([ + DeviceOrientation.portraitUp, + DeviceOrientation.portraitDown, + ]), + ]); + final themeService = results[1] as ThemeService; + final localeService = results[2] as LocaleService;
82-91: InconsistentisRegisteredguards — SupabaseService is guarded but ThemeService, LocaleService, and ThemeController are not.SupabaseService uses
Get.isRegistered<>()beforeGet.put()because it's also registered inInitialBindingviaGet.putAsync(). ThemeService, LocaleService, and ThemeController are only registered here (not in InitialBinding), so no duplicate registration occurs. However, the inconsistent pattern is confusing and could cause issues if these services are later moved to InitialBinding. For consistency, either guard all of them or document that only SupabaseService needs guarding due to dual registration.
- Add error handling to TokenService initialization - Use passwordStrong instead of passwordRequired for registration - Update desugar_jdk_libs to version 2.1.5 - Add error logging in location filter and splash controller - Fix parseBool asymmetric string parsing - Add guard for empty validation errors in error handler - Remove rethrow in explore controller _fetchInitialData - Update nearbyHotels list in favorite status method - Use RxList<MessageModel> instead of RxList<String> in chat controller - Remove duplicate error snackbars in profile controllers - Log TokenService and sign-out cleanup errors - Fix race condition with unawaited(_initAuthStatus()) - Fix stale rememberMe.value in auth-state listener - Inject ProfileRepository via constructor in auth controller - Fix main_prod.dart zone handling pattern - Fix binding profileRepository registration order - Update auth controller test with ProfileRepository mock
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/auth/controllers/auth_controller.dart (1)
171-185:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset controller auth state on external
signedOutevents.The
signedOutbranch clears tokens, but leavesisAuthenticated/currentUseruntouched. If sign-out happens outsidelogout()(session expiry, remote revocation), UI can remain in a stale authenticated state.Proposed fix
if (event == AuthChangeEvent.signedOut) { // Keep TokenService in sync on sign-out events await _tokenService.clearTokens(); await _clearRememberedSession(); + isAuthenticated.value = false; + currentUser.value = null; + if (Get.currentRoute != Routes.login) { + await Get.offAllNamed(Routes.login); + } return; }As per coding guidelines, "Properly handle session expiration and implement logout functionality."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/auth/controllers/auth_controller.dart` around lines 171 - 185, When handling AuthChangeEvent.signedOut inside the auth change handler in AuthController, you clear tokens and remembered session but forget to reset the controller's auth state; update the signedOut branch (where _tokenService.clearTokens() and _clearRememberedSession() are called) to also set isAuthenticated = false and currentUser = null (and any related auth flags), and ensure you invoke the same state-updating/notification path used by logout() so UI/listeners are updated consistently after external sign-outs or session expiration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/features/auth/controllers/form_validation_controller.dart`:
- Around line 39-41: The validatePassword usage currently applies
ValidationService.passwordStrong for the shared passwordKey, which will reject
legacy-but-valid passwords during login; change the validation so that
validatePassword (or the call in FormValidationController where passwordKey is
validated) uses ValidationService.passwordRequired for the shared/login path and
apply ValidationService.passwordStrong only in the registration flow (e.g., in
the registration controller or registerPassword validation function) so login
continues to accept legacy passwords while registration enforces strength.
In `@lib/features/messaging/views/chat_view.dart`:
- Around line 88-101: The view is creating MessageModel instances and mutating
_controller.messages directly; move that logic into a controller method
ChatController.sendMessage(String content): validate non-empty content, obtain
conversationId from the controller state and senderId from the auth state,
construct the MessageModel (no hardcoded '' or 'current_user'), call the
repository/API to persist the message, add the message to the controller's
messages observable, and handle/report errors; then update the view's onPressed
to call _controller.sendMessage(_input.text) and clear _input only after a
successful send (or handle rollback on failure).
In `@test/unit/controllers/auth/auth_controller_test.mocks.dart`:
- Around line 7-14: Regenerate the test mocks file
auth_controller_test.mocks.dart with the exact Mockito version locked in
pubspec.lock (5.6.1) so the generated imports and APIs match the current
generator; run the project’s mock-generation/build_runner step after ensuring
dependencies are up to date and replace the existing
auth_controller_test.mocks.dart with the newly generated file so imports like
package:mockito/mockito.dart and package:mockito/src/dummies.dart and any mock
classes match Mockito 5.6.1.
---
Outside diff comments:
In `@lib/features/auth/controllers/auth_controller.dart`:
- Around line 171-185: When handling AuthChangeEvent.signedOut inside the auth
change handler in AuthController, you clear tokens and remembered session but
forget to reset the controller's auth state; update the signedOut branch (where
_tokenService.clearTokens() and _clearRememberedSession() are called) to also
set isAuthenticated = false and currentUser = null (and any related auth flags),
and ensure you invoke the same state-updating/notification path used by logout()
so UI/listeners are updated consistently after external sign-outs or session
expiration.
🪄 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: d67e7fad-68ca-40f4-ab58-1da78abae4e2
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
android/app/build.gradle.ktslib/app/data/models/api_response_models.g.dartlib/app/data/models/booking_model.g.dartlib/app/data/models/booking_pricing_model.g.dartlib/app/data/models/location_model.g.dartlib/app/data/models/message_model.g.dartlib/app/data/models/notification_model.g.dartlib/app/data/models/payment_model.g.dartlib/app/data/models/review_model.g.dartlib/app/data/models/trip_model.g.dartlib/app/data/models/unified_property_response.g.dartlib/app/data/models/user_model.g.dartlib/app/ui/widgets/common/location_filter_app_bar.dartlib/app/utils/extensions/dynamic_extensions.dartlib/app/utils/helpers/error_handler.dartlib/features/auth/bindings/auth_binding.dartlib/features/auth/controllers/auth_controller.dartlib/features/auth/controllers/form_validation_controller.dartlib/features/explore/controllers/explore_controller.dartlib/features/home/bindings/home_binding.dartlib/features/inquiry/bindings/inquiry_binding.dartlib/features/messaging/controllers/chat_controller.dartlib/features/messaging/views/chat_view.dartlib/features/profile/bindings/profile_binding.dartlib/features/profile/controllers/edit_profile_controller.dartlib/features/profile/controllers/notifications_controller.dartlib/features/profile/controllers/preferences_controller.dartlib/features/splash/controllers/splash_controller.dartlib/main_prod.darttest/unit/controllers/auth/auth_controller_test.darttest/unit/controllers/auth/auth_controller_test.mocks.dart
💤 Files with no reviewable changes (13)
- lib/app/data/models/booking_pricing_model.g.dart
- lib/app/data/models/trip_model.g.dart
- lib/app/data/models/payment_model.g.dart
- lib/app/data/models/api_response_models.g.dart
- lib/app/data/models/message_model.g.dart
- lib/app/data/models/notification_model.g.dart
- lib/app/data/models/review_model.g.dart
- lib/app/data/models/user_model.g.dart
- lib/app/data/models/booking_model.g.dart
- lib/app/data/models/location_model.g.dart
- lib/app/data/models/unified_property_response.g.dart
- lib/features/profile/controllers/notifications_controller.dart
- lib/features/profile/controllers/edit_profile_controller.dart
| onPressed: () { | ||
| final text = _input.text.trim(); | ||
| if (text.isEmpty) return; | ||
| _controller.messages.add( | ||
| MessageModel( | ||
| id: DateTime.now().millisecondsSinceEpoch.toString(), | ||
| conversationId: '', | ||
| senderId: 'current_user', | ||
| content: text, | ||
| createdAt: DateTime.now(), | ||
| ), | ||
| ); | ||
| _input.clear(); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Business logic should not be in the view layer.
The view directly creates MessageModel instances and manipulates _controller.messages. This violates the separation of concerns - the controller should handle message creation and state updates. Additionally:
conversationId: ''is hardcoded emptysenderId: 'current_user'is hardcoded instead of fetched from auth state- No API call to persist the message
- No error handling
Move this logic to ChatController.sendMessage(String content):
Proposed refactor
In chat_controller.dart:
Future<void> sendMessage(String content) async {
if (content.trim().isEmpty) return;
// TODO: Get actual conversationId and senderId from state/auth
final message = MessageModel(
id: DateTime.now().millisecondsSinceEpoch.toString(),
conversationId: conversationId, // from controller state
senderId: currentUserId, // from auth
content: content,
createdAt: DateTime.now(),
);
messages.add(message);
// TODO: Persist via repository
}In chat_view.dart:
IconButton(
icon: const Icon(Icons.send),
onPressed: () {
final text = _input.text.trim();
if (text.isEmpty) return;
- _controller.messages.add(
- MessageModel(
- id: DateTime.now().millisecondsSinceEpoch.toString(),
- conversationId: '',
- senderId: 'current_user',
- content: text,
- createdAt: DateTime.now(),
- ),
- );
+ _controller.sendMessage(text);
_input.clear();
},
),As per coding guidelines: "Use GetX controllers for business logic in Flutter".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/features/messaging/views/chat_view.dart` around lines 88 - 101, The view
is creating MessageModel instances and mutating _controller.messages directly;
move that logic into a controller method ChatController.sendMessage(String
content): validate non-empty content, obtain conversationId from the controller
state and senderId from the auth state, construct the MessageModel (no hardcoded
'' or 'current_user'), call the repository/API to persist the message, add the
message to the controller's messages observable, and handle/report errors; then
update the view's onPressed to call _controller.sendMessage(_input.text) and
clear _input only after a successful send (or handle rollback on failure).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 137 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/app/data/services/storage_service.dart">
<violation number="1" location="lib/app/data/services/storage_service.dart:25">
P2: Changing `FlutterSecureStorage` to default iOS options can make previously stored tokens unreadable on iOS because accessibility changed from `first_unlock_this_device` to default `unlocked`.</violation>
</file>
<file name="lib/app/data/models/unified_property_response.dart">
<violation number="1" location="lib/app/data/models/unified_property_response.dart:25">
P2: `fromJson` now performs an unchecked map conversion for each `properties` item, which can throw on malformed/non-map list entries. Restore type filtering before conversion to keep parsing resilient.</violation>
</file>
<file name="lib/app/data/models/review_model.dart">
<violation number="1" location="lib/app/data/models/review_model.dart:17">
P2: `rating` deserialization is too strict and can crash when JSON provides non-`int` numeric types.</violation>
</file>
<file name="lib/app/data/repositories/auth_repository.dart">
<violation number="1" location="lib/app/data/repositories/auth_repository.dart:93">
P1: Token persistence now silently fails when `TokenService` is unavailable or not ready, with no storage fallback, which can leave users logged in without persisted credentials.</violation>
</file>
Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
| } finally { | ||
| await _storage.clearTokens(); | ||
| try { | ||
| final tokenService = Get.find<TokenService>(); |
There was a problem hiding this comment.
P1: Token persistence now silently fails when TokenService is unavailable or not ready, with no storage fallback, which can leave users logged in without persisted credentials.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/app/data/repositories/auth_repository.dart, line 93:
<comment>Token persistence now silently fails when `TokenService` is unavailable or not ready, with no storage fallback, which can leave users logged in without persisted credentials.</comment>
<file context>
@@ -89,7 +89,13 @@ class AuthRepository {
} finally {
- await _storage.clearTokens();
+ try {
+ final tokenService = Get.find<TokenService>();
+ await tokenService.ready;
+ await tokenService.clearTokens();
</file context>
| aOptions: _androidOptions, | ||
| iOptions: _iosOptions, | ||
| ); | ||
| _secureStorage = const FlutterSecureStorage(); |
There was a problem hiding this comment.
P2: Changing FlutterSecureStorage to default iOS options can make previously stored tokens unreadable on iOS because accessibility changed from first_unlock_this_device to default unlocked.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/app/data/services/storage_service.dart, line 25:
<comment>Changing `FlutterSecureStorage` to default iOS options can make previously stored tokens unreadable on iOS because accessibility changed from `first_unlock_this_device` to default `unlocked`.</comment>
<file context>
@@ -17,22 +17,12 @@ class StorageService extends GetxService {
- aOptions: _androidOptions,
- iOptions: _iosOptions,
- );
+ _secureStorage = const FlutterSecureStorage();
if (!_ready.isCompleted) {
_ready.complete();
</file context>
| _secureStorage = const FlutterSecureStorage(); | |
| _secureStorage = const FlutterSecureStorage( | |
| iOptions: IOSOptions( | |
| accessibility: KeychainAccessibility.first_unlock_this_device, | |
| ), | |
| ); |
| properties: | ||
| (json['properties'] as List?) | ||
| ?.map((e) => Property.fromJson(Map<String, dynamic>.from(e))) | ||
| .toList() ?? | ||
| [], |
There was a problem hiding this comment.
P2: fromJson now performs an unchecked map conversion for each properties item, which can throw on malformed/non-map list entries. Restore type filtering before conversion to keep parsing resilient.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/app/data/models/unified_property_response.dart, line 25:
<comment>`fromJson` now performs an unchecked map conversion for each `properties` item, which can throw on malformed/non-map list entries. Restore type filtering before conversion to keep parsing resilient.</comment>
<file context>
@@ -29,30 +20,33 @@ class UnifiedPropertyResponse {
- return [];
+ factory UnifiedPropertyResponse.fromJson(Map<String, dynamic> json) {
+ return UnifiedPropertyResponse(
+ properties:
+ (json['properties'] as List?)
+ ?.map((e) => Property.fromJson(Map<String, dynamic>.from(e)))
</file context>
| properties: | |
| (json['properties'] as List?) | |
| ?.map((e) => Property.fromJson(Map<String, dynamic>.from(e))) | |
| .toList() ?? | |
| [], | |
| properties: | |
| (json['properties'] as List?) | |
| ?.whereType<Map>() | |
| .map((e) => Property.fromJson(Map<String, dynamic>.from(e))) | |
| .toList() ?? | |
| [], |
| factory ReviewModel.fromMap(Map<String, dynamic> map) => ReviewModel( | ||
| id: map['id']?.toString() ?? '', | ||
| bookingId: map['bookingId']?.toString() ?? '', | ||
| rating: map['rating'] as int? ?? 5, |
There was a problem hiding this comment.
P2: rating deserialization is too strict and can crash when JSON provides non-int numeric types.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/app/data/models/review_model.dart, line 17:
<comment>`rating` deserialization is too strict and can crash when JSON provides non-`int` numeric types.</comment>
<file context>
@@ -18,14 +11,17 @@ class ReviewModel {
+ factory ReviewModel.fromMap(Map<String, dynamic> map) => ReviewModel(
+ id: map['id']?.toString() ?? '',
+ bookingId: map['bookingId']?.toString() ?? '',
+ rating: map['rating'] as int? ?? 5,
+ comment: map['comment'] as String? ?? '',
+ );
</file context>
- Remove unused variable 'colors' in custom_button.dart - Remove unused field '_isPressed' in premium_animated_section.dart - Remove unused method '_handleDelete' in swipeable_item.dart - Fix unnecessary null comparison in interactive_virtual_tour.dart - Remove unused imports in auth_controller.dart and user_profile_controller.dart - Remove unused fields '_filterWorker' and '_locationWorker' in controllers - Remove unused variable 'textStyles' in search_results_view.dart - Fix dead code in chat_view.dart - Fix unnecessary null assertions in locate_view.dart
There was a problem hiding this comment.
0 issues found across 11 files (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
- Remove unused fields '_profileChangeWorker' and '_authChangeWorker' in edit_profile_controller.dart - Remove unused variable 'legacyAccess' in splash_controller.dart - Remove unused method '_formatDate' in trips_controller.dart
There was a problem hiding this comment.
0 issues found across 3 files (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
- Add fallback TextStyle to ensure baseTitleStyle is non-nullable
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
- Add com.a360ghar.stays (prod) client entry - Add com.example.stays_app.dev (dev) client entry - Add com.a360ghar.stays.staging (staging) client entry - Fixes processProdReleaseGoogleServices build failure
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
- Add dontwarn rules for Play Core classes in proguard-rules.pro - Fix formatting in error_handler.dart
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
- Add com.google.android.play:core:1.10.3 to fix R8 minification errors - Flutter embedding references Play Core classes for deferred components
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
- Prevents workflow failure when GitHub token lacks PR comment permissions
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: Auto-approval blocked by 4 unresolved issues from previous reviews.
Re-trigger cubic
Critical fixes: - Fix double-dismiss crash in animated_toast.dart with _isDismissed guard - Fix unstable Dismissible keys in swipeable_item.dart (require stable keys) - Add logging for invalid date fallback in message_model.dart Major fixes: - Separate login vs registration password validation (passwordStrong only for registration) - Always init Firebase Analytics, use setAnalyticsCollectionEnabled for consent gate - Add permanent: true to AnalyticsService registration in initial_binding - Simplify redundant conditional in connectivity_service.dart checkConnection - Replace DNS lookup with socket-only check in _canReachHost Minor fixes: - Add assert guard for Supabase client getter (check initialized) - Remove unused createdAt field from TokenInfo
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/app/ui/widgets/common/swipeable_item.dart">
<violation number="1" location="lib/app/ui/widgets/common/swipeable_item.dart:96">
P1: This null-assert can crash in release builds because `SwipeableItem` still allows `key` to be omitted and the constructor assert is debug-only.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| return ClipRect( | ||
| child: Dismissible( | ||
| key: widget.key!, |
There was a problem hiding this comment.
P1: This null-assert can crash in release builds because SwipeableItem still allows key to be omitted and the constructor assert is debug-only.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/app/ui/widgets/common/swipeable_item.dart, line 96:
<comment>This null-assert can crash in release builds because `SwipeableItem` still allows `key` to be omitted and the constructor assert is debug-only.</comment>
<file context>
@@ -93,7 +93,7 @@ class _SwipeableItemState extends State<SwipeableItem>
return ClipRect(
child: Dismissible(
- key: widget.key ?? UniqueKey(),
+ key: widget.key!,
direction: DismissDirection.endToStart,
dismissThresholds: const {
</file context>
1 participant
Summary
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Behavior Changes