bugfix remove course pre requisite only if is not entrance exam

[melvinsoft]

This is a bug fix for Entrance Exams management in Schedule & Details page.

The error is easy to reproduce: You need to make sure that the following FEATURES flags are enabled:

"FEATURES":
    "MILESTONES_APP": true,
    "ENABLE_PREREQUISITE_COURSES": true,
    "ENTRANCE_EXAMS": true

• 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.

What happens in the background, is that there is a bug in the settings handlers that is there is any pre requisite course set, iterates over all pre requisites and deletes them, but the problem is Entrance Exams are a type of pre requisite since they belong to the milestones app, and is getting deleted too.

The fix check the milestone namespace, and if it's an Entrance Exam skips it.

Co-Authored-By: Omar Al-Ithawi [email protected]

[openedx-webhooks]

Thanks for the pull request, @melvinsoft! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[natabene]

jenkins run all

[natabene]

@melvinsoft Thank you for your contribution, please let me know once all tests are green.

[melvinsoft]

jenkins run all

[natabene]

jenkins run all

[natabene]

jenkins run all

[natabene]

jenkins run all

[melvinsoft]

jenkins run all

[natabene]

jenkins run all

[natabene]

@melvinsoft Can you please check the failing tests?

[OmarIthawi]

Thanks Natalia for the ping. @melvinsoft I've rebased the branch meanwhile hoping the Jenkins tests will be a bit better,

[melvinsoft]

@natabene Thanks for reaching out, and thanks @OmarIthawi I hope test are better now after the rebase, if not we'll take care of the errors.

[natabene]

jenkins run all

[OmarIthawi]

@melvinsoft I've fixed all of the quality checks that I can fix, please check the second commit to see what I have done.

There's only one that I couldn't fix quickly, which is pylint complaining about the if statement we added, too many nested if statements are not acceptable by pylint.

@natabene We could use the reviewer advice on how to refactor the code to avoid nested if statements. Could you please move this to Engineering Review?

[natabene]

@OmarIthawi Sure, I will line this up.

[melvinsoft]

@OmarIthawi Thank you for the changes, and thanks for helping moving this PR along.

[natabene]

jenkins run all

[melvinsoft]

@natabene Looks like all checks have passed now!

@OmarIthawi Thanks for pylint fixes!

[natabene]

@melvinsoft Thanks, lining this up for our review.

[marcotuts]

Can I ask for a clarification on this - What does it mean that you add courses into the entrance exam in the description? Is this meant to be adding content (videos, problems, text etc) into the entrance exam area?

[OmarIthawi]

Can I ask for a clarification on this - What does it mean that you add courses into the entrance exam in the description? Is this meant to be adding content (videos, problems, text etc) into the entrance exam area?

@melvinsoft would you mind checking here?

[edx-status-bot]

Your PR has finished running tests. The following contexts failed:

• jenkins/js
• jenkins/a11y
• jenkins/python
• jenkins/quality

[marcotuts]

One other thing to note here - the entrance exam feature is currently something we plan to deprecate though a sept ticket doesn't exist yet we have that as a task in our queue.

https://openedx.atlassian.net/browse/TNL-7046

Part of the reason here is that entrance exams would need to be added to the learning mfe, which has not been done.

It may make sense to merge this bug fix in regardless but wanted to flag the future issue of this not being in the mfe.

No plans at edX to build this currently into the mfe, though this could change.

[OmarIthawi]

One other thing to note here - the entrance exam feature is currently something we plan to deprecate though a sept ticket doesn't exist yet we have that as a task in our queue.

https://openedx.atlassian.net/browse/TNL-7046

Part of the reason here is that entrance exams would need to be added to the learning mfe, which has not been done.

It may make sense to merge this bug fix in regardless but wanted to flag the future issue of this not being in the mfe.

No plans at edX to build this currently into the mfe, though this could change.

Thanks @marcotuts. It sounds that the plan is mostly a definitive one. Would you mind creating a DEPR task even if it stays in "Backlog" for a long while? It'll be helpful for us to track it. The Entrance Exam is a feature that's extensively used by some of our course authors (customers).

[arch-bom-gocd-alerts]

📣 💥 Heads-up: You must either rebase onto master or merge master into your branch to avoid breaking the build.

We recently removed diff-quality and introduced lint-amnesty. This means that the automated quality check that has run on your branch doesn't work the same way it will on master. If you have introduced any quality failures, they might pass on the PR but then break the build on master.

This branch has been detected to not have commit 2e335653 as an ancestor. Here's how to see for yourself:

git merge-base --is-ancestor 2e335653 maxi/fix-entrance-exam-course-details && echo "You're all set" || echo "Please rebase onto master or merge master to your branch"

If you have any questions, please reach out to the Architecture team (either #edx-shared-architecture on Open edX Slack or #architecture on edX internal).

[mariajgrimaldi]

Hi there! :smile:

Can you rebase against the master branch? It's been quite some time :sweat_smile:

[mariajgrimaldi]

Hello there! There hasn't been any activity in this PR in the last year. What should we do? @natabene @OmarIthawi

[OmarIthawi]

Hello there! There hasn't been any activity in this PR in the last year. What should we do? @natabene @OmarIthawi

Thanks @mariajgrimaldi for the reminder. Let me ping @melvinsoft and see what he thinks.

[natabene]

Just checking if this is still being pursued. It has been a while.

[OmarIthawi]

Thanks for the reminder.

@shadinaif could you please take over this pull request? It sounds like a good bug fix in the platform.