Skip to content

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

Open
wants to merge 42 commits into
base: trunk
Choose a base branch
from
Open

Delete subscriber #21989

wants to merge 42 commits into from

Conversation

nbradbury
Copy link
Contributor

@nbradbury nbradbury commented Jul 3, 2025

This PR adds the ability to delete a subscriber from the subscriber detail screen.

To test

  • In a debug build enable the experimental subscribers feature
  • My Site > Subscribers
  • Tap a subscriber that's your account
  • Scroll down and tap to delete
  • After confirmation, note that you're returned to the list and the deleted subscriber no longer appears

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

@nbradbury nbradbury added unit-tests-exemption Subscribers View and manage newsletter subscribers labels Jul 3, 2025
@nbradbury nbradbury requested a review from Copilot July 3, 2025 16:10
@dangermattic
Copy link
Collaborator

dangermattic commented Jul 3, 2025

2 Warnings
⚠️ strings.xml files should only be updated on release branches, when the translations are downloaded by our automation.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

Copilot

This comment was marked as outdated.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jul 3, 2025

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr21989-e99baa0
Commite99baa0
Direct Downloadwordpress-prototype-build-pr21989-e99baa0.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jul 3, 2025

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr21989-e99baa0
Commite99baa0
Direct Downloadjetpack-prototype-build-pr21989-e99baa0.apk
Note: Google Login is not supported on these builds.

@nbradbury nbradbury requested a review from Copilot July 3, 2025 19:06
Copilot

This comment was marked as outdated.

@nbradbury nbradbury requested a review from Copilot July 3, 2025 19:09
Copilot

This comment was marked as outdated.

Copy link

codecov bot commented Jul 3, 2025

Codecov Report

Attention: Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.

Project coverage is 39.02%. Comparing base (2927acb) to head (e99baa0).

Files with missing lines Patch % Lines
...s/android/ui/subscribers/SubscriberDetailScreen.kt 0.00% 6 Missing ⚠️
...ess/android/ui/subscribers/SubscribersViewModel.kt 0.00% 5 Missing ⚠️
...ress/android/ui/subscribers/SubscribersActivity.kt 0.00% 2 Missing ⚠️
...wordpress/android/ui/dataview/DataViewViewModel.kt 0.00% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nbradbury nbradbury requested a review from Copilot July 7, 2025 13:38
Copy link
Contributor

@Copilot Copilot AI left a 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 in SubscribersViewModel.
  • Wires up a Material dialog and toast in SubscribersActivity and composes a delete button in SubscriberDetailScreen.

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.

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.

@wordpress-mobile wordpress-mobile deleted a comment from Copilot AI Jul 7, 2025
val params = AddSubscribersParams(
emails = emails
)
private suspend fun addSubscribers(emails: List<String>) = runCatching {
Copy link
Contributor Author

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.

Copy link

sonarqubecloud bot commented Jul 8, 2025

@nbradbury nbradbury requested a review from adalpari July 8, 2025 20:16
@nbradbury nbradbury marked this pull request as ready for review July 8, 2025 20:16
@nbradbury nbradbury requested a review from a team as a code owner July 8, 2025 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Subscribers View and manage newsletter subscribers unit-tests-exemption
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants