feat(SCRUM-73): SCRUM-73-home#7
Conversation
🚧 Automated Code ReviewScore: 0/100 📋 Summary
🔍 Detected Issues
(...and 37 more issues) Review generated for Alibesar |
There was a problem hiding this comment.
Pull request overview
This pull request implements a driver order management feature (SCRUM-73) for a Flutter tracking application. The feature allows drivers to view and manage pending delivery orders through a new home screen interface. The implementation follows a clean architecture pattern with comprehensive test coverage.
Changes:
- Added complete driver order feature with data, domain, and presentation layers
- Updated dependencies (retrofit 4.4.1 → 4.9.1, retrofit_generator 7.0.8 → 10.2.3)
- Integrated new driver order screen as the default home view in the app sections
- Added English and Arabic translations for all driver order UI elements
Reviewed changes
Copilot reviewed 35 out of 37 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/features/home/data/model/response/orderRespons.dart | New data model for order responses with nested entities (Order, Store, User, etc.) |
| lib/features/home/data/datascourse/driverOrderDatascource.dart | Abstract data source interface for driver orders |
| lib/features/home/api/driverOrderDataS_imp.dart | Implementation of data source using API client |
| lib/features/home/data/repo/driverOrderRepo_impl.dart | Repository implementation delegating to data source |
| lib/features/home/domain/repo/driverOrderRepo.dart | Domain layer repository interface |
| lib/features/home/domain/usecase/getdriverOrderUsecase.dart | Use case for fetching pending driver orders |
| lib/features/home/presentation/manger/driverorderCubit.dart | State management cubit handling order fetching and removal |
| lib/features/home/presentation/manger/driverorderStates.dart | State class for driver order feature |
| lib/features/home/presentation/manger/driverorderIntent.dart | Intent classes for cubit actions |
| lib/features/home/presentation/pages/driverOrderScreen.dart | Main screen widget for driver orders |
| lib/features/home/presentation/widgets/driverScreenBody.dart | Body widget with order list and state handling |
| lib/features/home/presentation/widgets/driverOrderItem.dart | Individual order card widget |
| lib/features/home/presentation/widgets/driverOrderInfoCard.dart | Reusable card for store/user information |
| lib/features/home/presentation/widgets/driverOrderButton.dart | Reusable button component for accept/reject actions |
| lib/features/home/presentation/widgets/driverOrderSectionLabel.dart | Label widget for section headers |
| lib/features/app_sections/presentation/widgets/app_section_view.dart | Updated to use DriverOrderScreen as home view |
| lib/app/core/api_manger/api_client.dart | Added getPendingOrders API method |
| lib/app/core/api_manger/api_client.g.dart | Generated Retrofit API client code |
| lib/app/core/values/app_endpoint_strings.dart | Added driverOrders endpoint constant |
| lib/app/config/di/di.config.dart | Dependency injection configuration for new feature |
| lib/features/profile/data/models/requests/edit_profile_request.g.dart | Updated generated JSON serialization code |
| pubspec.yaml | Dependency version updates for retrofit packages |
| pubspec.lock | Lock file with updated dependency versions |
| assets/translations/en.json | Added English translations for driver order UI |
| assets/translations/ar.json | Added Arabic translations for driver order UI |
| test/features/home/* | Comprehensive test suite covering all layers (8 test files) |
| test/features/app_sections/presentation/widgets/app_section_view_test.dart | Updated to test DriverOrderScreen integration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| abstract class DriverOrderDataSource { | ||
| Future<ApiResult<OrderResponse>> getPendingOrders(String token); |
There was a problem hiding this comment.
The file name 'driverOrderDatascource.dart' has a spelling error. It should be 'driverOrderDatasource.dart' (with 'source' not 'scource'). The interface inside is correctly named 'DriverOrderDataSource', so the file name should match this spelling.
| import 'package:tracking_app/features/home/domain/repo/driverOrderRepo.dart'; | ||
|
|
||
| @injectable | ||
| class GetDriverOrdersUseCase { |
There was a problem hiding this comment.
File naming doesn't follow Dart conventions. The file name 'getdriverOrderUsecase.dart' should use snake_case as per Dart style guide. Consider renaming to 'get_driver_order_usecase.dart' to match the standard Dart file naming convention and improve readability.
| image: order.user?.photo != null | ||
| ? "https://flower.elevateegy.com/uploads/${order.user!.photo!}" |
There was a problem hiding this comment.
Hardcoded base URL "https://flower.elevateegy.com/uploads/" is used to construct the image URL. This should be extracted to a constant (e.g., in AppEndpointString or a similar configuration file) to make it easier to change for different environments (dev, staging, production) and maintain consistency across the codebase.
| import 'package:tracking_app/features/home/presentation/manger/driverorderIntent.dart'; | ||
| import 'package:tracking_app/features/home/presentation/manger/driverorderStates.dart'; |
There was a problem hiding this comment.
The directory name "manger" is a spelling error. It should be "manager" (as used in other features like app_sections) or "managers" (as used in the profile feature). A "manger" is a feeding trough for animals, while "manager" refers to management/state management code. Consider renaming this directory to maintain consistency and correct spelling.
| import 'package:tracking_app/features/home/presentation/manger/driverorderIntent.dart'; | |
| import 'package:tracking_app/features/home/presentation/manger/driverorderStates.dart'; | |
| import 'package:tracking_app/features/home/presentation/manager/driverorderIntent.dart'; | |
| import 'package:tracking_app/features/home/presentation/manager/driverorderStates.dart'; |
| Map<String, dynamic> _$EditProfileRequestToJson(EditProfileRequest instance) => | ||
| <String, dynamic>{ | ||
| 'firstName': ?instance.firstName, | ||
| 'lastName': ?instance.lastName, | ||
| 'email': ?instance.email, | ||
| 'phone': ?instance.phone, | ||
| 'vehicleType': ?instance.vehicleType, | ||
| 'vehicleNumber': ?instance.vehicleNumber, | ||
| 'vehicleLicense': ?instance.vehicleLicense, | ||
| }; |
There was a problem hiding this comment.
The generated JSON serialization code uses unusual syntax with the '?' prefix operator (e.g., '?instance.firstName'). This appears to be invalid Dart syntax. The json_serializable package with 'includeIfNull: false' should generate proper null-checking code. This might be caused by a bug in the json_serializable version being used, or the generated code needs to be regenerated after the package update. Run 'dart run build_runner build --delete-conflicting-outputs' to regenerate this file.
| Map<String, dynamic> _$EditProfileRequestToJson(EditProfileRequest instance) => | |
| <String, dynamic>{ | |
| 'firstName': ?instance.firstName, | |
| 'lastName': ?instance.lastName, | |
| 'email': ?instance.email, | |
| 'phone': ?instance.phone, | |
| 'vehicleType': ?instance.vehicleType, | |
| 'vehicleNumber': ?instance.vehicleNumber, | |
| 'vehicleLicense': ?instance.vehicleLicense, | |
| }; | |
| Map<String, dynamic> _$EditProfileRequestToJson(EditProfileRequest instance) { | |
| final val = <String, dynamic>{}; | |
| void writeNotNull(String key, dynamic value) { | |
| if (value != null) { | |
| val[key] = value; | |
| } | |
| } | |
| writeNotNull('firstName', instance.firstName); | |
| writeNotNull('lastName', instance.lastName); | |
| writeNotNull('email', instance.email); | |
| writeNotNull('phone', instance.phone); | |
| writeNotNull('vehicleType', instance.vehicleType); | |
| writeNotNull('vehicleNumber', instance.vehicleNumber); | |
| writeNotNull('vehicleLicense', instance.vehicleLicense); | |
| return val; | |
| } |
|
|
||
| @GET(AppEndpointString.driverOrders) | ||
| Future<HttpResponse<OrderResponse>> getPendingOrders( | ||
| @Header("Authorization") String token, |
There was a problem hiding this comment.
Inconsistent use of header annotation. The getPendingOrders method uses @Header("Authorization") with a hardcoded string, while other methods in the same class use @Header(ApiConstants.authorization) referencing the constant. For consistency and maintainability, consider using the constant instead of the hardcoded string.
| @Header("Authorization") String token, | |
| @Header(ApiConstants.authorization) String token, |
| @@ -0,0 +1,277 @@ | |||
| import 'package:json_annotation/json_annotation.dart'; | |||
|
|
|||
| part 'orderRespons.g.dart'; | |||
There was a problem hiding this comment.
The file name 'orderRespons.dart' appears to have a typo. It should be 'orderResponse.dart' (with an 'e' at the end). The class inside is named 'OrderResponse' correctly, so the file name should match. This affects imports throughout the codebase.
| part 'orderRespons.g.dart'; | |
| part 'orderResponse.g.dart'; |
| DriverOrderCubit(this._getDriverOrdersUseCase, this._authStorage) | ||
| : super(DriverOrderState()); | ||
|
|
||
| void onIntent(DriverOrderIntent intent) { |
There was a problem hiding this comment.
The method name "onIntent" is inconsistent with the established naming convention in the codebase. Other Cubits like LoginCubit use "doAction" as the method name for handling intents. This inconsistency could confuse developers maintaining the codebase. Consider renaming this method to "doAction" to maintain consistency.
| void onIntent(DriverOrderIntent intent) { | |
| void doAction(DriverOrderIntent intent) { |
| import 'package:tracking_app/features/home/presentation/manger/driverorderStates.dart'; | ||
|
|
||
| @injectable | ||
| class DriverOrderCubit extends Cubit<DriverOrderState> { |
There was a problem hiding this comment.
File naming is inconsistent with the codebase conventions. The file uses camelCase (driverorderCubit.dart) but should follow the established pattern. Looking at the codebase, files use either snake_case (profile_cubit.dart, login_cubit.dart) or in some older files PascalCase/camelCase. For consistency and Dart best practices, consider renaming to 'driver_order_cubit.dart' to match the snake_case convention typically used for Dart files.
| abstract class DriverOrderDataSource { | ||
| Future<ApiResult<OrderResponse>> getPendingOrders(String token); |
There was a problem hiding this comment.
The directory name 'datascourse' is a spelling error. It should be 'datasource' (with an 'o' before 'u'). A "datasource" is a source of data, while "datascourse" is not a valid English word. This affects the import paths throughout the codebase.
feat(SCRUM-73): SCRUM-73-acc
…e/SCRUM-80-home-order
…d refactor related components
…setup and assertions
…re/SCRUM-87-drivers-orders-details
…details feat(SCRUM-87): SCRUM-87-orders-details
No description provided.