Fixed duplicate subscriber role in Invitation screen #21772
Conversation
nbradbury
changed the title
Generated by 🚫 Danger |
|
| App Name |  WordPress |
|
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr21772-1293b83 | |
| Commit | 1293b83 | |
| Direct Download | wordpress-prototype-build-pr21772-1293b83.apk |
|
| App Name |  Jetpack |
|
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr21772-1293b83 | |
| Commit | 1293b83 | |
| Direct Download | jetpack-prototype-build-pr21772-1293b83.apk |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #21772 +/- ##
=======================================
Coverage 39.29% 39.29%
=======================================
Files 2125 2125
Lines 99862 99862
Branches 15386 15386
=======================================
Hits 39240 39240
Misses 57144 57144
Partials 3478 3478 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@nbradbury I'm failing to reproduce the original issue in the current production release—please see the screen recording below. I attempted with Simple and Atomic sites. Am I overlooking steps? Screen recording
Screen_Recording_20250326_085108_Jetpack.mp4 |
@dcalhoun Interesting. That looks like the Subscriber role isn't being returned by the backend when the app requests a list of roles (the Subscriber role you're seeing in the app is the one added here). If you go to the web app and view user roles what do you see? I see this:
|
|
@nbradbury below is what I see in the web WP Admin. Simple site: Atomic site: |
|
@dcalhoun Hmm, well it looks like the Subscriber role is missing from the simple site but not the Atomic site. But both do look like the backend isn't returning the Subscriber role to the app, but in my case it is. I'm wondering if the real fix here is to revert the changes in this PR, and then only add the Subscriber role programmatically if it's missing from the backend response? I'm curious what you see for these sites in the iOS app. |
|
@nbradbury below is what iOS displays for the referenced sites. Simple site
Atomic site
|
|
@dcalhoun I think I see the problem here. "Subscriber" can mean two different things. There are subscribers who are "Followers," as they used to be called, who receive your posts via email and/or the reader. The API expects us to use "follower" as the role name parameter for these users. But then there are sites (like mine) which are using something like WooCommerce Subscriptions, which also show a "Subscriber" role. The API expects us to use "subscriber" as the role name parameter for these users. 
So I believe this PR is correct. Previously WPAndroid would add a role which used "follower" as the name parameter, but "Subscriber" as the display name. This would cause confusion with sites that already have a subscriber role because "Subscriber" would appear twice. By changing the display name to "Email subscriber" for the added role, this PR avoids this seeming duplication. The alternative would be to use "Follower" as the display name, as iOS does, but on Slack ( p1742906414374599/1742834018.185719-slack-C04PWEZSYFL ) we decided "Follower" was outdated, which is why we use "Email subscriber" here. |
|
@nbradbury thank you for taking the time to explain this so clearly—super helpful!
I'm not sure "Email subscriber" will always be an accurate description. My understanding is that "Subscribers" on WordPress.com can be a subscriber without an email subscription. See pe7F0s-2ul-p2 and 68a5341b7aaa-linear-project. |
|
@dcalhoun Based on pe7F0s-2ul-p2#comment-3354 I'm wondering if the best thing to do here is use the phrase "Follower" to match iOS. The whole discussion about subscriber types is beyond the scope of this simple PR, which tries only to fix the apparent duplicate "Subscriber" roles. |
|
@nbradbury I agree the referenced post goes beyond the scope of fixing a duplicative "subscriber" label, but my primary point remains true: labeling all subscribers with an "email subscriber" label can be inaccurate, as a subscriber doesn't always have an active email subscription. My perception is that the latest label proposal (see pe7F0s-2ul-p2#comment-3380) relevant to this specific PR is "Reader subscriber." Since we do not present UI related to emails or paid subscriptions in the app, we can ignore "Email subscriber" and "Paid subscriber" for the time being. WDYT? |
|
Yes, I imagine so. |
|
@nbradbury one additional thought: Is this app's label we are discussing used for both WordPress.com sites and self-hosted sites? If so, I wonder if "Reader subscriber" becomes inaccurate for the latter, which I believe has no relation/connection to the Reader feature. Should the self-hosted sites utilize "Subscriber" instead—even if it might create a duplicate collision with a plugin? |
Oof, who knew such a seemingly simple PR would be so complicated :) For self-hosted sites, the "Subscriber" role uses the slug "subscriber". For this PR, we're dealing with the "follower" slug. So perhaps we simply shouldn't add this role for self-hosted sites? |
|
Where does the My high-level thought is that we should not discard or change WordPress core's label—which I believe I understand to be "Subscriber"—solely because a plugin introduces a label with the same name. While I agree it's a confusing outcome, I perceive it to be an issue the plugin should resolve; the plugin should use a unique label of its own—e.g., "Subscriber (Woo)", "Woo subscriber," "Store subscriber". For WordPress.com sites, as we've discussed, the web platform plans to clearly communicate the nuance between Reader and email subscribers. It doesn't necessarily need to be done in this PR, but I believe we should update our WordPress.com-specific label on Android and iOS to match the web platform's labels of "Reader subscriber." That said, I'm not confident we need to address either of those problems right now. If anything, updating iOS from "Follower" to "Subscriber" feels like the only priority change to address inconsistencies. |
I'm inclined to agree with you. We can close this. |
Fixes #21768
We previously had duplicate "Subscriber" roles under People -> Invite People:
This PR corrects this by changing the duplicate "Subscriber" to "Email subscriber," as per this Slack thread:p1742834018185719-slack-C04PWEZSYFL
To test: