Feat/fix local ads archiving undo #87
Conversation
…bar functionality - Remove lastDeletedLocalAd field and related logic - Add lastPendingDeletionId and snackbarLocalAdTitle fields - Update copyWith method to accommodate new fields - Adjust equality and hashCode methods for new state structure
- Rename and restructure the _ConfirmDeleteLocalAdRequested event - Implement a new _DeletionServiceStatusChanged event to handle updates from the pending deletions service
… undo functionality - Replace local delete timer with PendingDeletionsService for managing pending deletions - Update BLoC to handle deletion events from PendingDeletionsService - Modify state to include last pending deletion ID and snackbar ad title - Refactor deletion and undo logic to use the new service - Update comments and documentation to reflect changes
…tionality - Introduce PendingDeletionsService for better undo management - Simplify snackbar logic in ArchivedLocalAdsPage - Update ArchiveLocalAdsBloc to use pending deletions service
- Remove unnecessary null assignment for 'exception' and 'restoredLocalAd' - Simplify state copying by removing redundant code
Summary of ChangesHello @fulleni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a significant refactoring of the local ads archiving feature, specifically enhancing the undo deletion mechanism. By introducing a dedicated Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the "delete forever" functionality for archived local ads by introducing a reusable PendingDeletionsService, which is a great improvement for centralizing the undo logic. The changes in the ArchiveLocalAdsBloc and its related state and event files are mostly well-executed to support this new service. My review includes a critical issue regarding potential state corruption in copyWith, a high-severity issue about a functional regression where the archived list may become stale, and several medium-severity suggestions to improve code maintainability and adhere to common architectural patterns.
lib/local_ads_management/bloc/archive_local_ads/archive_local_ads_state.dart
Show resolved
Hide resolved
lib/local_ads_management/bloc/archive_local_ads/archive_local_ads_bloc.dart
Show resolved
Hide resolved
lib/local_ads_management/bloc/archive_local_ads/archive_local_ads_bloc.dart
Show resolved
Hide resolved
lib/local_ads_management/bloc/archive_local_ads/archive_local_ads_bloc.dart
Show resolved
Hide resolved
lib/local_ads_management/bloc/archive_local_ads/archive_local_ads_bloc.dart
Show resolved
Hide resolved
- Update the ArchiveLocalAdsState.copyWith constructor to properly handle null values for exception, restoredLocalAd, lastPendingDeletionId, and snackbarLocalAdTitle properties. - This change prevents potential null dereference errors when these properties are not explicitly provided when copying the state.
- Remove direct dependency on PendingDeletionsService - Use ArchiveLocalAdsBloc to handle undo delete action - Improve adherence to business logic separation principles
- Remove unnecessary `restoredLocalAd` updates in multiple states - This change simplifies the state management and improves performance
- Remove restoredLocalAd field from ArchiveLocalAdsState - Adjust copyWith method to remove restoredLocalAd parameter - Update ArchiveLocalAdsStateProps to exclude restoredLocalAd
… error - Replace context.read in SnackBar action with direct service call - Remove unnecessary state check for restoredLocalAd - Update BlocListener to avoid redundant UI updates
Status
READY
Description
This pull request implements a significant refactoring of the local ads archiving feature, specifically enhancing the undo deletion mechanism. By introducing a dedicated PendingDeletionsService, the logic for managing temporary deletions with timed undo capabilities is centralized. This change simplifies the ArchiveLocalAdsBloc, making it more focused on state management rather than orchestrating deletion timers, and provides a more robust and consistent user experience for undoing archived ad deletions.
Type of Change