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.

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

feat(topics): Add new 'Home' topic as an option in the tool #1212

Merged
merged 5 commits into from
Sep 30, 2024
Merged

Conversation

nina-py
Copy link
Contributor

@nina-py nina-py commented Aug 2, 2024

DO NOT DEPLOY

See notes in https://mozilla-hub.atlassian.net/browse/MC-1248 for when this feature is going to production.

Goal

Allow the curators to categorise stories as belonging to the brand new "Home" category.

Implementation details

  • Note: with regenerating types, a handful of unrelated schema changes have sneaked through. I think that is fine - we'll use them in the near future anyway. It's also possible that these schema changes will be merged to main earlier than this PR is deployed, in which case the changes will be removed from this PR.

Tested manually:

  • Adding a new corpus item manually with "Home" as topic
  • Editing an existing item to set the topic to "Home"
  • Adding a prospect to the corpus with "Home" as topic
  • Scheduled item cards - topic is displayed correctly (see screenshot below)
home-article

Reference

https://mozilla-hub.atlassian.net/browse/MC-1251

@nina-py nina-py requested a review from a team as a code owner August 2, 2024 12:46
expect(topics).to.be.an('array').with.lengthOf(16);
expect(topics).to.be.an('array').with.lengthOf(17);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With most of the test updates involving updating the number of topics expected, should tests be modified to not check for the total number of topics? Should we compare them to the enums in the generated code instead?

Or is adding a topic going to be a rare enough event that it's fine to leave this - and by extension the process of adding a topic - as-is?

Copy link
Collaborator

Choose a reason for hiding this comment

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

good question and i'm not sure we know the answer yet re: how often will we get new topics. but - i think your point about avoiding testing a specific number of topics is sound. we should stop testing the number - i think testing against the enum sounds good.

expect(topics).to.be.an('array').with.lengthOf(16);
expect(topics).to.be.an('array').with.lengthOf(17);
Copy link
Collaborator

Choose a reason for hiding this comment

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

good question and i'm not sure we know the answer yet re: how often will we get new topics. but - i think your point about avoiding testing a specific number of topics is sound. we should stop testing the number - i think testing against the enum sounds good.

@nina-py nina-py merged commit 75083bf into main Sep 30, 2024
6 checks passed
@nina-py nina-py deleted the MC-1251 branch September 30, 2024 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants