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

Playback 2023: Skip "Plus stories" in sequence #1512

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Nov 2, 2023

📘 Part of: #1463

This PR adds a mechanism to:

  1. Set which stories are Plus or not;
  2. Skip a bunch of Plus stories in sequence for a non-paid user
  3. Skip a bunch of Plus stories in sequence for a non-paid user when going to previous stories

Basically — given the "Plus stories" will have an upsell, we don't want users to see the same story multiple times. So we just display it once, whether is going to next stories or previous one.

To test

Before testing, add override val plusOnly: Boolean = true in StoryYearOverYear and StoryCompletionRate.
Note that the first plus story "StoryYearOverYear" doesn't have an upsell yet.

Then:

  1. Run the app
  2. Login with Free account
  3. Let the stories run (don't tap to skip to the next or to the previous one)
  4. ✅ After the "YearOverYear" story is shown, it should skip to the last one
  5. Tap to go to the previous story
  6. ✅ It should go back to the "YearOverYear" (skipping CompletionRate story)
  7. On the "YearOverYear" story, tap to go next
  8. ✅ It should go to the last story
  9. Exit the stories
  10. Assign Plus plan to account
  11. Tap "Refresh now" on Profile tab
  12. Repeat the testing steps
  13. ✅ However, all stories should show (no stories will be skipped)
  14. Assign Patron plan to account
  15. Tap "Refresh now" on Profile tab
  16. Repeat the testing steps
  17. ✅ All stories should show (no stories will be skipped)

** Tests cover skip scenarios when there are multiple plus stories in between

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
  • I have considered whether it makes sense to add tests for my changes
  • All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • Any jetpack compose components I added or changed are covered by compose previews

I have tested any UI changes...

  • with different themes
  • with a landscape orientation
  • with the device set to have a large display and font size
  • for accessibility with TalkBack

@ashiagr ashiagr requested a review from a team as a code owner November 2, 2023 11:04
@ashiagr ashiagr mentioned this pull request Nov 2, 2023
25 tasks
@ashiagr ashiagr added this to the Future milestone Nov 2, 2023
@ashiagr ashiagr force-pushed the task/1463-skip-plus-stories branch from 5d8bc48 to c400891 Compare November 2, 2023 12:13
Copy link
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

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

This is working well. I imagine it was a bit tricky getting that logic just right. Thanks for adding the tests.

@mchowning mchowning merged commit 7eb4283 into main Nov 2, 2023
@mchowning mchowning deleted the task/1463-skip-plus-stories branch November 2, 2023 14:46
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.

2 participants