Skip to content

Conversation

@shanbady
Copy link
Contributor

@shanbady shanbady commented Oct 4, 2024

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/5502

Description (What does it do?)

This PR moves the featured resources randomization for the featured resources carousel to the frontend (since the backend results are now cached).

How can this be tested?

  1. Checkout this branch
  2. make sure you have resources loaded up. Also run python manage.py populate_featured_lists to have featured resources
  3. visit the homepage and unit pages and note that the featured resources change on each load

@shanbady shanbady marked this pull request as ready for review October 4, 2024 17:38
@mbertrand mbertrand self-assigned this Oct 7, 2024
}
return arr
}

Copy link
Member

@mbertrand mbertrand Oct 7, 2024

Choose a reason for hiding this comment

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

The backend randomizer used to randomize results within each learning path position index, so for example, assuming 5 "featured resource" learning paths:

Carousel positions 1-5 would be the 1st course from each of the 5 paths, in random order
Carousel positions 6-10 would be the 2nd course from each of the 5 paths, in random order
etc.

This ensured that all units would be equally represented, with the top courses appearing first.

This PR seems to make the order completely random, for instance the first time I tried it I got:

  1. Position 3 course from MITx
  2. Position 1 course from Professional Education
  3. Position 1 course from OCW
  4. Position 2 course from Sloan
  5. Position 3 course from Sloan
    etc

Have the specs changed so that the order can be completely random now, or should the old pseudo-random sorting still be in place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just asked Ferdi - he said try to preserve the old randomization if its easy enough so I'm going to refactor this to match what it was previously there.

Copy link
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

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

Works great! 👍

PS just a reminder, I think Chris mentioned this morning that any frontend changes to main should also be applied to the nextjs branch.

@shanbady shanbady merged commit d9ed99a into main Oct 8, 2024
11 checks passed
@shanbady shanbady deleted the shanbady/randomize-featured-resources branch October 8, 2024 13:27
shanbady added a commit that referenced this pull request Oct 8, 2024
* set default minimum score cutoff (#1642)

* add is_incomplete_or_stale to default sort (#1641)

* Release 0.20.4

* Use a partial match mode as the search mode for instructor fields(#1652)

* Release date for 0.20.4

* update og:image to use new logo (#1658)

* Release 0.21.0

* Ignore NotFoundErrors in switch_indices function (#1654)

* Release date for 0.21.0

* Shanbady/randomize featured resources (#1653)

* removing backend randomization

* removing results randomization

* moiving shuffle to frontend

* fixing tests

* test fix

* refactoring to old randomization method on frontend

* updating spec

* fixing typechecks

* updating spec and adding position field to api

* adding position to factory

* fixing test

* adding new og-image

---------

Co-authored-by: Anastasia Beglova <abeglova@mit.edu>
Co-authored-by: Doof <mitx-devops@mit.edu>
Co-authored-by: Chris Chudzicki <christopher.chudzicki@gmail.com>
Co-authored-by: Matt Bertrand <mrbertrand@gmail.com>
@odlbot odlbot mentioned this pull request Oct 8, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants