Not a must for V2, but some of Year in Review is starting to feel messy, and I think an eventual refactor is in order.
Tasks
- Let's return "false" for all of the data controller "should" methods, so that YiR code doesn't run regardless of the values on the remote config.
- Remove GIFs entirely from app binary, verify that size of app has decreased to pre-YiR levels
- Remove the display persisted flag from Core Data slide objects. This field is no longer the source-of-truth on what slide will actually be seen in the feature, because we added additional display logic in YearInReviewCoordinator. I would like all display logic to be determined in YearInReviewCoordinator, for consistency. So, for example, we remove this section here, and address the logic here:
case .readCount:
if slide.evaluated == true,
let data = slide.data {
let decoder = JSONDecoder()
if let readCount = try? decoder.decode(Int.self, from: data),
let readCount >= 2 {
readCountSlide = YearInReviewSlideContent(
imageName: "read",
textOverlay: personalizedSlide1Overlay(readCount: readCount),
title: personalizedSlide1Title(readCount: readCount),
informationBubbleText: nil,
subtitle: personalizedSlide1Subtitle(readCount: readCount),
loggingID: "read_count_custom",
infoURL: aboutYIRURL,
hideDonateButton: shoudlHideDonateButton())
}
}(Note the changes are: do not check displayed flag here, only check slide.evaluted == true, and add additional business logic of let readCount >= 2.
- Better enforcement of must-have slide implementations. When we create new slides, there are several areas where we have to add logic. Missing any one of these errors causes bugs that are hard to find and understand. I propose that we move all the slide-specific logic within YearInReviewDataController into their own class (YearInReviewReadCountSlide), and each of these classes conform to a protocol (name TBD). The protocol enforces methods that must be implemented in order for the slide to work.
TE NOTES
Build: 7.8.1 5629 white label