Skip to content
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

Remove social subscribers #20763

Merged
merged 7 commits into from
May 7, 2024
Merged

Conversation

irfano
Copy link
Member

@irfano irfano commented May 3, 2024

Fixes #20714

This removes social subribers from the detail screen of the Total Followers card and additionally, changes the endpoint for the "Total Followers" card from /sites/$site/stats/summary/stats/subscribers.

Additionally, this PR removes FollowerTotalsUseCase, which was already unused. This card was being used only on the WordPress app but then we removed stats from WP, now it's fine to remove the usecase as well. I haven't removed InsightType.FOLLOWER_TOTALS from FLuxC for now because it may cause crashes on a case when upgraded from a very old version since selected insight cards are stored in the db.

after

To Test:

  1. Log in
  2. Enable stats_traffic_subscribers_tab flag from "Me → Debug settings"
  3. Navigate back.
  4. Open the INSIGHTS tab from "My Site → Stats".
  5. Tap the "VIEW MORE" button on the Total Followers card.

Regression Notes

  1. Potential unintended areas of impact

    • None
  2. What I did to test those areas of impact (or what existing automated tests I relied on)

    • N/A
  3. What automated tests I added (or what prevented me from doing so)

    • Updated FollowerTypesUseCaseTest and FollowerTotalsUseCaseTest

PR Submission Checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing Checklist (strike-out the not-applying and unnecessary ones):

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • Talkback.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • Large and small screen sizes. (Tablet and smaller phones)
  • Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

@irfano irfano added the Stats label May 3, 2024
@dangermattic
Copy link
Collaborator

dangermattic commented May 3, 2024

1 Warning
⚠️ strings.xml files should only be updated on release branches, when the translations are downloaded by our automation.

Generated by 🚫 Danger

@irfano irfano requested a review from aditi-bhatia May 3, 2024 15:54
@irfano irfano added this to the 24.9 milestone May 3, 2024
@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 3, 2024

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
Versionpr20763-98837a3
Commit98837a3
Direct Downloadjetpack-prototype-build-pr20763-98837a3.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 3, 2024

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
Versionpr20763-98837a3
Commit98837a3
Direct Downloadwordpress-prototype-build-pr20763-98837a3.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented May 3, 2024

Codecov Report

Attention: Patch coverage is 64.00000% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 40.65%. Comparing base (d7de297) to head (98837a3).

Files Patch % Lines
...ections/insights/usecases/TotalFollowersUseCase.kt 70.58% 2 Missing and 3 partials ⚠️
...sections/insights/usecases/FollowerTypesUseCase.kt 40.00% 0 Missing and 3 partials ⚠️
...ns/insights/management/InsightsManagementMapper.kt 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #20763      +/-   ##
==========================================
- Coverage   40.69%   40.65%   -0.05%     
==========================================
  Files        1491     1490       -1     
  Lines       68677    68579      -98     
  Branches    11357    11332      -25     
==========================================
- Hits        27949    27878      -71     
+ Misses      38191    38183       -8     
+ Partials     2537     2518      -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@irfano irfano changed the title Remove social subcribers Remove social subscribers May 6, 2024
Copy link
Contributor

@aditi-bhatia aditi-bhatia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm, works as expected 👍 I went ahead and merged in trunk since there was a conflict. Just curious, when would be considered a good time to remove InsightType.FOLLOWER_TOTALS from FluxC?

Copy link

sonarqubecloud bot commented May 7, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@irfano
Copy link
Member Author

irfano commented May 7, 2024

Thanks for resolving the conflict.

when would be considered a good time to remove InsightType.FOLLOWER_TOTALS from FluxC?

I've made a note of it for myself. I will remove it after the project by checking if it affects old versions.

@aditi-bhatia aditi-bhatia merged commit 5760de9 into trunk May 7, 2024
20 checks passed
@aditi-bhatia aditi-bhatia deleted the issue/20714-remove-social-subcribers branch May 7, 2024 09:49
@aditi-bhatia
Copy link
Contributor

I've made a note of it for myself. I will remove it after the project by checking if it affects old versions.

Sounds good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stats Insights: Remove social (publicize) subscribers
4 participants