-
Notifications
You must be signed in to change notification settings - Fork 172
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
fix: use correct course_key in case of a key_for_rerun in RCM #4411
Conversation
918f5d3
to
002081d
Compare
@@ -202,6 +202,8 @@ def create_course_run(self, course, body): | |||
def get_or_create_course(self, body): | |||
course_run_key = CourseKey.from_string(body['id']) | |||
course_key = self.get_course_key_from_course_run_key(course_run_key) | |||
course_key = (Course.objects.filter(key_for_reruns=course_key).values_list('key', flat=True).first() or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: There should be a comment explaining this scenario in the code. Or better, have an internal or util function to return the course key that considers both scenarios.
datum = mock_data.COURSES_API_BODY_ORIGINAL | ||
self.mock_api([datum]) | ||
course_type = CourseType.objects.get(slug=CourseType.AUDIT) | ||
course_key = '{org}+{number}'.format(org=datum['org'], number=datum['number']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use f-string, no need to use format anymore.
course_type = CourseType.objects.get(slug=CourseType.AUDIT) | ||
course_key = '{org}+{number}'.format(org=datum['org'], number=datum['number']) | ||
course_key_with_key_for_rerun = 'test+key_for_rerun' | ||
Course.objects.create(partner=self.partner, key=course_key, title='Title', type=course_type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should the first course be created here? Shouldn't only the course with key_for_reruns be created only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scenario we're testing should have a courseA and courseB having key_for_reruns = courseA.key
f28d391
to
35d76af
Compare
35d76af
to
c2bf436
Compare
PROD-4133
Updates the logic behind bringing the courses from studio in case a course is created in Studio and not in Discovery. Previous logic used to create a course_run from Studio into Discovery with whatever was present in the key. In case a course has a value in
key_for_reruns
, it used to assign those course_runs to the key equal tokey_for_reruns
.Now, it finds if a course has a
key_for_reruns
, it will assign this course_run to the correct course that has the customkey_for_reruns
value.