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

[Reader IA] Refresh blogs count after subscribing/unsubscribing from search screen #20135

Conversation

RenanLukas
Copy link
Contributor

Fixes #20011


To Test:

  1. Install Jetpack and sign-in;
  2. Open Reader;
  3. Open Search;
  4. Search for a blog;
  5. Subscribe to it;
  6. Return to the Reader feed: the blogs count should be correct;
  7. Repeat steps 3-6 but this time unsubscribe from a blog.

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)

    • Manual testing
  3. What automated tests I added (or what prevented me from doing so)

    --


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:

  • 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)

@dangermattic
Copy link
Collaborator

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 7, 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
Versionpr20135-00dd473
Commit00dd473
Direct Downloadwordpress-prototype-build-pr20135-00dd473.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Feb 7, 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
Versionpr20135-00dd473
Commit00dd473
Direct Downloadjetpack-prototype-build-pr20135-00dd473.apk
Note: Google Login is not supported on these builds.

Copy link
Contributor

@daniloercoli daniloercoli left a comment

Choose a reason for hiding this comment

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

It works as expected when following the testing steps precisely. Initially, I overlooked the three-dots menu in the bottom-right corner of the results list. Instead, I tapped on the blog to open an article and attempted to subscribe from there, which led to the pill not updating correctly upon returning to the feed.

Here are the steps:

  • Search for a blog; for example, use "wec".
  • In the results list, tap on an article.
  • Subscribe to the blog.
  • Return to the Reader feed; the blog count should now reflect the new subscription.

I'm fine if you want to merge this PR and open a new ticket for it, since I guess the fix may not be super easy to implement?

@RenanLukas
Copy link
Contributor Author

Thanks for the review, @daniloercoli

Since we're targeting the release branch, I'll try to solve it in the same PR. I'll keep you posted.

@peril-wordpress-mobile
Copy link

Warnings
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

Copy link

codecov bot commented Feb 7, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (818bd93) 40.22% compared to head (00dd473) 40.04%.
Report is 4 commits behind head on release/24.2.

Files Patch % Lines
...ress/android/ui/reader/ReaderActivityLauncher.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff                @@
##           release/24.2   #20135      +/-   ##
================================================
- Coverage         40.22%   40.04%   -0.18%     
================================================
  Files              1469     1460       -9     
  Lines             67556    67200     -356     
  Branches          11183    11141      -42     
================================================
- Hits              27173    26910     -263     
+ Misses            37894    37819      -75     
+ Partials           2489     2471      -18     

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

@RenanLukas
Copy link
Contributor Author

Update: we're handling the fix of the issue @daniloercoli found separately. I'll create the issue shortly.

@RenanLukas RenanLukas merged commit 24b46d0 into release/24.2 Feb 7, 2024
20 checks passed
@RenanLukas RenanLukas deleted the issue/20011-subscriptions-chip-count-not-updated-after-subscribing-to-blog-on-search branch February 7, 2024 18:40
@RenanLukas
Copy link
Contributor Author

Issue created #20151

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

Successfully merging this pull request may close these issues.

4 participants