Page MenuHomePhabricator

Year in Review - code cleanup and improvements for 2025
Closed, ResolvedPublic5 Estimated Story Points

Description

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
  1. 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.

  1. 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

Details

Other Assignee
GOlson-WMF

Event Timeline

HNordeenWMF moved this task from Needs Triage to Engineering Backlog on the Wikipedia-iOS-App board.
HNordeenWMF subscribed.

We should do the first thing when we pick up next year's YiR (if we do a 2025 version)

HNordeenWMF renamed this task from Year in Review - code cleanup to Year in Review - code cleanup and improvements for 2025.Jan 7 2025, 8:30 PM

Engineering notes:
if time:

  • Cleanup coordinators
  • Create a class to manage dependencies
Mazevedo set the point value for this task to 5.Jun 9 2025, 7:35 PM

We're considering 5 with the nice-to-have coordinator cleanup as well

Testing here:

  • This is mainly refactoring YIR - things will react differently due to hard coded behaviors.
  • The other ticket officially hides YIR but they should be in the same build after merge
Seddon updated Other Assignee, added: GOlson-WMF; removed: Tsevener.
Seddon added a subscriber: GOlson-WMF.
ABorbaWMF subscribed.

Appears to be correct on 7.8.1 (5741)

Tested on iPhone on iOS 26.0 and iPad on iPadOS 18.5.