bugfix remove course pre requisite only if is not entrance exam Fixes RED-512 - #494
Conversation
There was a problem hiding this comment.
Thanks @melvinsoft for your work on this intricate bug. You know what I'm going to say next.
Tests please 😺
OmarIthawi
left a comment
OmarIthawi
left a comment
There was a problem hiding this comment.
I've added few questions so I learn about the issue more.
98f3f58 to
64040f4
Compare
iamjazzar
left a comment
iamjazzar
left a comment
There was a problem hiding this comment.
Thanks @melvinsoft for the detailed explanation! I still remember this module, it's messy and unstable! The issue was raised on one of the tests we ran on our mobile app back then and that was a surprise how everything's linked together.
Since we're fixing a fix here. I'd second @OmarIthawi by writing some unit tests to control the behavior here.
|
@OmarIthawi @AhmedAljazzar Thank you both for the review, I replied your comments and added 1 test. I'm pretty rusty on testing, specially edX testing. I agree Ahmed, this app is really buggy and unstable. I'm not sure is only test is enough, but let me know if I'm in the right track, thanks! |
OmarIthawi
left a comment
OmarIthawi
left a comment
There was a problem hiding this comment.
@melvinsoft this is awesome! I've checked the code and it looks good and the tests fails without your fix. That's awesome!
I've added few changes after that it should be good to go.
|
@OmarIthawi Thanks for the suggestions, I'm gonna wait for the test to pass again and merge it. |
add test Update cms/djangoapps/contentstore/tests/test_course_settings.py Co-Authored-By: Omar Al-Ithawi <i@omardo.com> Update cms/djangoapps/contentstore/tests/test_course_settings.py Co-Authored-By: Omar Al-Ithawi <i@omardo.com>
caaf482 to
ed6e462
Compare
3 participants
This error is easy to reproduce in staging.
Go to or create a new course, then go to "Schedule and Details" , enable an entrance exam for the course, you can leave the default score or change it.
Go to Content -> Outline and fill the entrance exam with a couple of courses, then create a normal unit in the course.
Go to the LMS and Enroll in the course with a new learner (without any staff privilege). You will only be able to see the entrance exam.
Go to Studio again, same course and then "Schedule and Details" change the enrollment date or any other data and save. When you go back to the LMS, you will find out that the entrance exam is disabled now, and you can see all the course content, but in the "Schedule and Details" is still active.
All this is related to the Milestones app, which is pretty messy. This bug is produce by a bugfix that @AhmedAljazzar contributed upstream while ago. Is a race condition between normal course pre requisites and entrance exams, who are technically a pre requisite too. Basically if there isn't any pre requisite course set, the backend logically deletes all the pre requisites, and the entrance exam too.