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

chore: remove course/course topic association #2649

Merged
merged 4 commits into from
May 30, 2023

Conversation

arslanashraf7
Copy link
Contributor

Pre-Flight checklist

  • Screenshots and design review for any changes that affect layout or styling
    • Desktop screenshots
    • Mobile width screenshots
  • Migrations
    • Migration is backwards-compatible with current production code
  • Testing
    • Code is tested
    • Changes have been manually tested
  • Settings
    • New settings are documented and present in app.json
    • New settings have reasonable development defaults, if applicable

What are the relevant tickets?

#2647

What's this PR do?

  • Removed the association between course and course topic models since we've moved that to course page, Take a look the ticket for more details
  • refactors the code that used this association

How should this be manually tested?

  • The api/courses and /api/programs should work as expected and return the proper topic values associated through CMS in the APIs
  • Make sure the frontend e.g. topic filter works fine
  • Just an overall smoke test

Where should the reviewer start?

(Optional)

Any background context you want to provide?

(Optional)

Screenshots (if appropriate)

(Optional)

What GIF best describes this PR or how it makes you feel?

(Optional)

@odlbot odlbot temporarily deployed to xpro-ci-pr-2649 May 17, 2023 12:55 Inactive
@odlbot odlbot temporarily deployed to xpro-ci-pr-2649 May 17, 2023 12:59 Inactive
@odlbot odlbot temporarily deployed to xpro-ci-pr-2649 May 19, 2023 07:59 Inactive
@arslanashraf7 arslanashraf7 force-pushed the arslan/2647-td-course-course-topic branch from 9021b59 to b583517 Compare May 19, 2023 08:06
@arslanashraf7 arslanashraf7 marked this pull request as ready for review May 19, 2023 13:11
@asadali145 asadali145 self-assigned this May 23, 2023
@arslanashraf7 arslanashraf7 force-pushed the arslan/2647-td-course-course-topic branch from b583517 to 01c16db Compare May 24, 2023 10:03
@arslanashraf7
Copy link
Contributor Author

@asadali145 I did some more cleanup in 01c16db. Could you take a look?

Copy link
Contributor

@asadali145 asadali145 left a comment

Choose a reason for hiding this comment

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

LGTM!

@arslanashraf7 arslanashraf7 force-pushed the arslan/2647-td-course-course-topic branch from 01c16db to 6eff737 Compare May 26, 2023 08:24
@arslanashraf7 arslanashraf7 merged commit d706c5c into master May 30, 2023
@arslanashraf7 arslanashraf7 deleted the arslan/2647-td-course-course-topic branch May 30, 2023 11:59
@odlbot odlbot mentioned this pull request Jun 2, 2023
11 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.

None yet

3 participants