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.

Sign up for GitHub

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

Jump to bottom

Image Recommendations - Data improvements #4802

Merged
merged 11 commits into from
Apr 3, 2024
Merged

Image Recommendations - Data improvements #4802

merged 11 commits into from
Apr 3, 2024

Conversation

tonisevener
Copy link
Collaborator

@tonisevener tonisevener commented Mar 29, 2024 •
edited
Loading

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

Notes

This PR contains some improvements to image recommendations data fetching.

  1. The explore feed card already fetched image recommendations, because it needs that data to know whether or not to display. However, when we pushed on image recommendations, it fetched a whole new set. It's wasteful to do it twice, especially since this call seems to take a while to load. Now I am caching the image recommendations within the data controller.
  2. At the end of the cycle, I am attempting to fetch more image recommendations. This matches Android.
  3. I added underscores to the article summary title before fetching. Without them, the article summary call redirected to another endpoint with underscores.
  4. The faster initial call caused onboarding to intercept the half sheet modal. The last commit fixes that.

Test Steps

  1. Fresh install on Staging. Tap Explore feed card.
  2. Dismiss onboarding. Confirm image recommendations appear after dismissal.
  3. Tap Not Sure until the end. Confirm you see a loading state after exhausting recommendations, and then new recommendations load.

@tonisevener tonisevener requested review from a team and mazevedofs and removed request for a team March 29, 2024 21:45
@tonisevener tonisevener changed the title Image recommendations - data improvements Image Recommendations - data improvements Mar 29, 2024
@tonisevener tonisevener changed the title Image Recommendations - data improvements Image Recommendations - Data improvements Mar 29, 2024
@staykids staykids requested review from staykids and removed request for mazevedofs April 1, 2024 20:40
@staykids staykids self-assigned this Apr 1, 2024
staykids
staykids reviewed Apr 2, 2024
View reviewed changes
Copy link
Contributor

@staykids staykids left a comment

Choose a reason for hiding this comment

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

Nice – one minor note below, but happy to merge as-is if you prefer.

Components/Sources/Components/Components/Onboarding/WKOnboardingViewController.swift Outdated Show resolved Hide resolved
@staykids staykids removed their assignment Apr 2, 2024
@staykids staykids assigned staykids and unassigned staykids Apr 2, 2024
staykids
staykids approved these changes Apr 2, 2024
View reviewed changes
Copy link
Contributor

@staykids staykids left a comment

Choose a reason for hiding this comment

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

👍🏽 thanks for that change! Just a merge conflict to resolve.

@tonisevener tonisevener merged commit 8b40a9a into main Apr 3, 2024
4 checks passed
@tonisevener tonisevener deleted the image-rec-data-improvements branch April 3, 2024 21:29
@tonisevener tonisevener mentioned this pull request Apr 3, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@staykids staykids staykids approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tonisevener @staykids