-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Delete subscriber #21989
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
base: trunk
Are you sure you want to change the base?
Delete subscriber #21989
Conversation
Co-authored-by: Copilot <[email protected]>
…ddSubscribersScreen.kt Co-authored-by: Copilot <[email protected]>
…ddSubscribersScreen.kt Co-authored-by: Copilot <[email protected]>
Generated by 🚫 Danger |
|
App Name | ![]() |
|
Flavor | Jalapeno | |
Build Type | Debug | |
Version | pr21989-e99baa0 | |
Commit | e99baa0 | |
Direct Download | wordpress-prototype-build-pr21989-e99baa0.apk |
|
App Name | ![]() |
|
Flavor | Jalapeno | |
Build Type | Debug | |
Version | pr21989-e99baa0 | |
Commit | e99baa0 | |
Direct Download | jetpack-prototype-build-pr21989-e99baa0.apk |
…ddSubscribersViewModel.kt Co-authored-by: Copilot <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #21989 +/- ##
=======================================
Coverage 39.02% 39.02%
=======================================
Files 2153 2153
Lines 101494 101494
Branches 15585 15585
=======================================
Hits 39613 39613
Misses 58384 58384
Partials 3497 3497 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…feature/delete-subscriber
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
Adds the ability to delete a subscriber from the detail screen, showing a confirmation dialog and toast feedback.
- Introduces new UI strings for delete confirmation and results.
- Implements
deleteSubscriber
API calls and one-off UI events inSubscribersViewModel
. - Wires up a Material dialog and toast in
SubscribersActivity
and composes a delete button inSubscriberDetailScreen
.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
gradle/libs.versions.toml | Bumps wordpress-rs revision for compatibility |
WordPress/src/main/res/values/strings.xml | Adds delete-subscriber confirmation and result texts |
WordPress/src/main/java/org/wordpress/android/ui/subscribers/SubscribersViewModel.kt | Implements delete logic, UI events, and public API |
WordPress/src/main/java/org/wordpress/android/ui/subscribers/SubscribersActivity.kt | Observes ViewModel events, shows dialog/toast |
WordPress/src/main/java/org/wordpress/android/ui/subscribers/SubscriberDetailScreen.kt | Adds delete button and prop to detail screen |
WordPress/src/main/java/org/wordpress/android/ui/subscribers/AddSubscribersViewModel.kt | Refactors coroutine dispatch for add flow |
Comments suppressed due to low confidence (1)
WordPress/src/main/java/org/wordpress/android/ui/subscribers/SubscribersViewModel.kt:282
- No tests cover the delete-subscriber flow or UI event emissions. Consider adding unit tests for
deleteSubscriber
and ViewModel event handling, as well as instrumentation tests for the dialog and toast behaviors.
fun deleteSubscriberConfirmed(subscriber: Subscriber, onSuccess: () -> Unit) {
@@ -55,6 +59,18 @@ class SubscribersActivity : BaseAppCompatActivity() { | |||
} | |||
} | |||
) | |||
|
|||
viewModel.uiEvent.observe(this) { event -> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
data class ShowDeleteConfirmationDialog(val subscriber: Subscriber) : UiEvent() | ||
data class ShowToast(val messageRes: Int) : UiEvent() | ||
} | ||
private val _uiEvent = MutableStateFlow<UiEvent?>(null) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
val params = AddSubscribersParams( | ||
emails = emails | ||
) | ||
private suspend fun addSubscribers(emails: List<String>) = runCatching { |
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.
This change is unrelated to deleting a subscriber, but after reading this article I thought runCatching
was a good fit for deleting a subscriber so I added it here, too.
|
This PR adds the ability to delete a subscriber from the subscriber detail screen.
To test
Note: I've noticed that it sometimes takes a bit for the subscriber to be deleted on the backend, so returning to My Site and back to Subscribers may still show the deleted subscriber. I addressed this by adding a dialog which tells the user this. but I'm open to other ideas.
delete.mp4