-
Notifications
You must be signed in to change notification settings - Fork 0
π :: (#23) connect account api #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Defines the `AccountApi` interface using Retrofit for interacting with the account-related endpoints. This includes a method for retrieving the user's account information.
Adds the `MyAccountResponseDto` to represent the structure of the account information received from the remote API. This DTO includes fields for ID, KRW balance, and USD balance, using `freezed` for immutable data classes and `json_serializable` for JSON conversion. Also creates empty directories with `.gitkeep` for local request/response DTOs and remote request DTOs.
Implements the account data source interface. This retrieves user account information from the API.
Defines an abstract AccountRepository and its implementation, AccountRepositoryImpl, to handle fetching user account details. This setup provides a clear separation of concerns for data access and allows for easier testing and future modifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR connects the Account API to the app, updates DI, and enforces new lint rules.
- Adds
flutter_lintsand updatesanalysis_options.yaml - Introduces
AccountApi, its data source, DTO, and DI registrations - Refactors controllers to return success booleans, updates navigation calls, and cleans up imports & const usage
Reviewed Changes
Copilot reviewed 40 out of 43 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pubspec.yaml | Added flutter_lints |
| analysis_options.yaml | Enabled strict const and lint rules |
| lib/data/account/service/account_api.dart | New Retrofit API interface for account |
| lib/data/account/dto/remote/response/my_account_response_dto.dart | DTO for account response |
| lib/data/account/data_sources/account_data_source_impl.dart | Implementation of AccountDataSource |
| lib/data/account/data_sources/account_data_source.dart | AccountDataSource abstraction |
| lib/core/config/di/dependencies.dart | Registered AccountApi and AccountDataSource |
| lib/presentation/sign_up/controller/*.dart | Refactored start/sendVerificationCode to return bool |
| lib/presentation/sign_in/controller/sign_in_controller.dart | Refactored signIn to return bool |
| (multiple) | Various const/coding-style cleanups |
Comments suppressed due to low confidence (4)
pubspec.yaml:59
- The flutter_lints package is intended for development only and should be moved to
dev_dependenciesto avoid shipping linter code in production builds.
flutter_lints: ^6.0.0
lib/data/account/service/account_api.dart:1
- New account API endpoints have been added but no tests are included. Please add unit tests for
AccountApi.getMyAccount()(e.g., using a mock Dio) to ensure correct request formation and error handling.
import 'package:dio/dio.dart';
lib/core/config/di/dependencies.dart:42
- [nitpick] When registering
AccountApi, consider passing a namedbaseUrlor using the same instantiation pattern as other APIs to avoid inconsistencies if multiple base URLs are needed.
di.registerLazySingleton<AccountApi>(() => AccountApi(di.get<Dio>()));
analysis_options.yaml:1
- [nitpick] Enabled
prefer_const_declarationsandprefer_const_constructors, but many widgets lackconst. Consider runningdart fix --applyto update codebase and satisfy the new lint rules.
include: package:flutter_lints/flutter.yaml
| } | ||
|
|
||
| void signIn(BuildContext context) { | ||
| bool signIn() { |
Copilot
AI
Jul 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to other controllers, signIn returns false before the async sign-in completes. Consider making this method async and await the use case so that the returned boolean reflects the real outcome.
| bool signIn() { | |
| Future<bool> signIn() async { |
aiden30015
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
μκ³ νμ ¨μ΅λλ€..
Co-authored-by: Copilot <[email protected]>
bluemoon983
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
μκ³ νμ ¨μ΅λλ€
π‘ κ°μ
π μμ λ΄μ©
π λ³κ²½μ¬ν
πββοΈ μ§λ¬Έμ¬ν
π΄ μ¬μ©λ°©λ²
πΈ κΈ°ν