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 2024: Ratings #2376

Merged
merged 10 commits into from
Nov 1, 2024
Merged

Playback 2024: Ratings #2376

merged 10 commits into from
Nov 1, 2024

Conversation

bjtitus
Copy link
Contributor

@bjtitus bjtitus commented Nov 1, 2024

📘 Part of: #2250

Fixes #2353

Ratings.mov
RatingsSmall.mov
Simulator.Screen.Recording.-.iPhone.16.-.2024-10-31.at.22.05.03.mp4
Simulator.Screen.Recording.-.iPhone.6s.-.2024-10-31.at.22.03.56.mp4

Loading Ratings

  • Loads Ratings using the API endpoint
  • Caches loaded ratings on DataManager object
  • Adds check for loaded ratings in when loading story

Ratings Screen

  • Adds Rating Chart
  • Adds Empty view when there are no ratings with Learn More button
  • Adds SafariView to present load button and handles dismissal to pause & then resume playback on the story

To test

Ratings Chart

  • Use an account which has left several ratings
  • Enable endOfYear2024
  • Restart app
  • Go to Profile and tap "Playback 2024"
  • Navigate until you get to the Ratings screen

Ratings Chart

  • Use an account which has never left ratings
  • Enable endOfYear2024
  • Restart app
  • Go to Profile and tap "Playback 2024"
  • Navigate until you get to the Ratings screen
  • Ensure that the "Oopsies" screen shows
  • Tap the "Learn about ratings" button
  • Verify that the web view shows and that sitting here for 5+ seconds doesn't continue the story progression in the background
  • Dismiss web view and verify that story resumes

Checklist

  • I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
  • I have considered adding unit tests for my changes.
  • I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

@bjtitus bjtitus added the [Project] End of Year 2024 End of Year project for 2024 label Nov 1, 2024
@bjtitus bjtitus added this to the 7.77 milestone Nov 1, 2024
# Conflicts:
#	podcasts/End of Year/End of Year 2024/EndOfYear2024StoriesModel.swift
#	podcasts/en.lproj/Localizable.strings
@bjtitus bjtitus marked this pull request as ready for review November 1, 2024 04:08
@bjtitus bjtitus requested a review from a team as a code owner November 1, 2024 04:08
@bjtitus bjtitus requested review from danielebogo and removed request for a team November 1, 2024 04:08
Copy link
Contributor

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

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

Hey @bjtitus I get a loading error while runnig this branch. I think something happens when calling the EndOfYearStoriesBuilder.build().
I also left a comment about one function name.

Comment on lines 112 to 114
func hasLoadedData(in dataManager: DataManager) -> Bool {
dataManager.ratings.ratings == nil
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bit confusing. If I call hasLoadedData I'm not expecting ratings to be nil. Can we find a better name? If it's only related to ratings I would specify that in the function name.

@bjtitus bjtitus requested a review from danielebogo November 1, 2024 15:08
Copy link
Contributor

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

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

Thanks Brandon! Now it's working fine!

@danielebogo danielebogo enabled auto-merge November 1, 2024 15:21
@danielebogo danielebogo merged commit c8e34b6 into trunk Nov 1, 2024
4 of 6 checks passed
@danielebogo danielebogo deleted the bjtitus/playback-2024/ratings branch November 1, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Project] End of Year 2024 End of Year project for 2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ratings Chart screen
2 participants