Skip to content

Conversation

@ChristopherChudzicki
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki commented Jul 26, 2024

What are the relevant tickets?

Fixes a bug exposed during #1323

Description (What does it do?)

Starting with #1275, some of our topic names have ampersands. The current channel creation code was creating search filters like

{
   "search_filter": "topic=Education & Teaching"
}

The frontend expects search_filter to be parseable as a URLSearchParams, so ampersands were being interpreted as separate parameters: topic=Education&topic=Teaching.

This PR ensures that the search filter is urlencoded .

How can this be tested?

  1. Make sure that you've run the data migration from Release 0.14.3 #1323 and re-run the ETL pipelines to associate resources with topics.
  2. Suggested: On main, go to /topics on the frontend. Click any topic that has an ampersand in its title. You will be navigated to the topic detail page.
  3. Now on this branch (restart docker, or at least celery!), run ./manage.py backpopulate_resource_channels --topic --overwrite
    • Note: this command only creates a channel if some resource uses that topic. So you need to make sure you're testing a topic that actually has resources associated with it.
  4. Check the topic detail page from (2) again. It should have search results.

Additional Context

@ChristopherChudzicki ChristopherChudzicki added the Needs Review An open Pull Request that is ready for review label Jul 26, 2024
@ChristopherChudzicki ChristopherChudzicki marked this pull request as ready for review July 26, 2024 20:47
@jkachel jkachel self-assigned this Jul 26, 2024
Copy link
Contributor

@jkachel jkachel left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Tested with "Business & Management" (as that had courses in it for me locally) - on main the filter has a & but in the branch it is properly urlencoded, and the topic channel page works.

Copy link
Contributor

@jkachel jkachel left a comment

Choose a reason for hiding this comment

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

misclicked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review An open Pull Request that is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants