Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a complete account deletion flow in the about dialog, allowing users to delete their Gravatar profiles. The implementation includes user confirmation flow, API integration, and proper error handling throughout the deletion process.
- Adds account deletion functionality with confirmation dialog
- Creates new API module for Gravatar service integration
- Implements complete MVVM architecture for the deletion flow
Reviewed Changes
Copilot reviewed 27 out of 30 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| api/* | New module providing Gravatar API integration for account deletion |
| userComponent/* | Domain layer implementation with use case and dependency injection |
| homeUi//about/ | UI components including dialog, bottom sheet, and view model for deletion flow |
| homeUi/*/strings.xml | UI text resources for deletion confirmation and messaging |
| build files | Module configuration and dependency management updates |
Comments suppressed due to low confidence (2)
gradle/libs.versions.toml:16
- Retrofit version 3.0.0 does not exist. The latest stable version is 2.11.0. Consider using a valid version like "2.11.0".
retrofit = "3.0.0"
homeUi/src/test/kotlin/com/gravatar/app/homeUi/presentation/home/components/topbar/components/AboutAppDialogViewModelTest.kt:93
- This test duplicates the behavior tested in the previous test at line 72. The assertion logic is identical - both verify that isLoading becomes true after OnConfirmDeleteAccount. Consider removing this duplicate test or testing a different aspect of the loading state.
fun `when OnConfirmDeleteAccount event is received then isLoading is set to true`() = runTest {
| if (response.isSuccessful) { | ||
| Result.success(Unit) | ||
| } else { | ||
| Result.failure(Exception("Failed to disable account.")) |
There was a problem hiding this comment.
The error message "Failed to disable account." is generic and doesn't include HTTP status code or response details. Consider providing more specific error information like "Failed to disable account: HTTP ${response.code()}" to help with debugging.
| Result.failure(Exception("Failed to disable account.")) | |
| Result.failure( | |
| Exception( | |
| "Failed to disable account: HTTP ${response.code()} - ${response.message()}${response.errorBody()?.let { " - ${it.string()}" } ?: ""}" | |
| ) | |
| ) |
| .baseUrl("https://api.gravatar.com/v3/") | ||
| .addConverterFactory( | ||
| MoshiConverterFactory.create() | ||
| ) |
There was a problem hiding this comment.
The Retrofit instance is created without proper HTTP client configuration. Consider using the existing OkHttpClient from the userComponent module to maintain consistent networking configuration including interceptors and timeouts.
| ) | |
| ) | |
| .client(get()) |
| _uiState.update { currentState -> | ||
| currentState.copy(isLoading = true) | ||
| } | ||
| deleteUserProfile().onFailure { |
There was a problem hiding this comment.
The error from deleteUserProfile failure is not logged or handled in any way. Consider logging the error for debugging purposes or providing user feedback about the failure.
| deleteUserProfile().onFailure { | |
| deleteUserProfile().onFailure { error -> | |
| Log.e("AboutAppDialogVM", "Failed to delete user profile", error) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
etoledom
left a comment
There was a problem hiding this comment.
Looking good!
I have added a simple generic error alert dialog in case of error while trying to delete the account.
Description
This PR implements the account deletion flow in the about dialog.
Kapture.2025-08-02.at.01.07.15.mp4
There is no error handling, so if an error occurs during the flow, the dialog will be dismissed without providing more information.
Note: Design can be improved in the following PRs
Testing Steps
Cancelbutton works as expectedDelete accountbutton works as expected