Picture Of The day bug fixes

[spatel]

Phabricator:
https://phabricator.wikimedia.org/T356255

---

Test Steps

1. Fresh install of the app (primary language English)
2. Background the app > add POTD widget to home screen
3. Tap the widget (it deeplinks to the POTD gallery view) ✅
4. Settings > My Languages > Add language (e.g. Spanish)
5. Change the primary language (e.g. to Spanish)
6. Close settings > refresh the Explore tab (pull-to-refresh)
7. Background the app > tap POTD widget
8. The app launches but the deeplink does nothing (it actually only works against the original primary language i.e. English) ❌

Expected behaviour: the app should launch and open the POTD image gallery view

---

Notes

I split this into 2 commits (one of which is just clang-format changes), so you only really need to review this commit

Possible root cause

I think this could be due to the fact the POTD image only gets saved once in the data store against the app's initial site URL (i.e. initial primary language). After that, the same record gets continually modified. This is deliberate (to avoid duplicate images from appearing across each language feed) and there was a comment in the code to explain why this was originally done. However, I think you may be running into issues with content URL deep-links from widgets etc because they appear to be querying data against site URL (even for POTD).

Worked example

This is based on the test steps mentioned above:

1. The "English" feed is initially loaded and it inserts a new POTD content record in the data store (against the "EN" site URL)
2. The user later switches primary language to "Spanish" and launches the app from a widget deeplink like this: wikipedia://content/picture-of-the-day/wikipedia.org/es/2024/05/01
3. The app tries processes the deeplink (and reaches [WMFAppViewController processUserActivity:animated:completion])
4. It queries the data store for a POTD (against the ES site URL) but finds no match (because it was originally stored against "EN")
5. It then falls through to here where it eventually tries to refresh all the feeds and save the POTD again
6. At this point it DOESN'T query against the site URL, so it finds the existing "EN" record and updates it.
7. Once everything is refreshed, a completion block runs & queries for the POTD (i.e. similar to step 4, but it now expects the ES record to exist) - but since it doesn't exist, nothing happens

Other observations

• At point 6 above, this is interesting. Because ALL language feeds end up overwriting the same record, the image description is overwritten (so whichever language is saved last "wins"). As a result you can end up with the language text randomly changing language (see attached video).
• Although this might fix some quirks with POTD, it's unlikely to fix all of them. After switching languages, I still found it occasionally necessary to do a manual refresh of the Explore tab before the POTD deeplinks fix themselves. I'm not sure why this was (but hopefully someone with a better knowledge of the code base may know!).
• I also noticed that if the user is offline when they originally install the widget, it's possible for the placeholder image to get cached on their home screen (which can affect deeplinking to the POTD image gallery). This is something that may need fixing separately (for example, you might want to pass something like a ?forceReloadWidget=<widget.id> query parameter with the widget URL) - but this seems to be quite "edge-casey" anyway.

---

Screenshots/Videos

As described above, here is the Explore feed's POTD image description (see bottom part of the image) flipping languages:

PictureOfTheDay-language-flip.mp4

[tonisevener]

@spatel Sorry for the delay on this review. Excellent investigative work, as always. 🙂

I found a couple of bugs, but overall I'm good with your approach in this method. Let us know if these fixes give you trouble and we can discuss alternatives. Thanks!

[spatel]

@tonisevener - thank you SO much for all the helpful comments, I knew there'd be subtleties like that which I'd missed! I have addressed all your comments (but it's probably hard to see exactly what I've done as clang-format has not made it easy for you to review them!).

But just to summarise what I've done:

(1) I've tidied up all of the "save" logic so it now does what you suggested (i.e. it's now checking against the contentLanguageCode to fix the issue with language variants, plus I'm also doing everything in one go when the primary language feed is updated - so hopefully this should fix the issue you found regarding lingering duplicates when a language is removed).

(2) The issue with the Chinese "variant" language code deeplinks not opening, seemed to be due to the wmf_languageVariantCode being nil. I've fixed that in WMFAppViewController but I'm not totally sure about that one. I can easily revert all the changes to this file if you prefer to tackle this separately.

[tonisevener]

Working great, thanks for fixing!