-
Notifications
You must be signed in to change notification settings - Fork 0
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
PAE-845 Enhance prerequisites to allow more flexible configuration #59
base: pearson-release/juniper.stage
Are you sure you want to change the base?
Conversation
3981e47
to
a488f94
Compare
e5410e7
to
85e0961
Compare
85e0961
to
7f620a7
Compare
2e0c4f2
to
1b5d580
Compare
run_extension_point( | ||
'PEARSON_CORE_MILESTONE_PREREQUISITES_MODULE', | ||
course_key = course_key, | ||
prerequisite_course_keys = prerequisite_course_keys, | ||
) | ||
if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys): | ||
return JsonResponseBadRequest({"error": _("Invalid prerequisite course key")}) | ||
set_prerequisite_courses(course_key, prerequisite_course_keys) |
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.
What do you think about moving the run_extension_point
part after the validation? @anfbermudezme
run_extension_point( | |
'PEARSON_CORE_MILESTONE_PREREQUISITES_MODULE', | |
course_key = course_key, | |
prerequisite_course_keys = prerequisite_course_keys, | |
) | |
if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys): | |
return JsonResponseBadRequest({"error": _("Invalid prerequisite course key")}) | |
set_prerequisite_courses(course_key, prerequisite_course_keys) | |
if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys): | |
return JsonResponseBadRequest({"error": _("Invalid prerequisite course key")}) | |
run_extension_point( | |
'PEARSON_CORE_MILESTONE_PREREQUISITES_MODULE', | |
course_key = course_key, | |
prerequisite_course_keys = prerequisite_course_keys, | |
) | |
set_prerequisite_courses(course_key, prerequisite_course_keys) |
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.
What happens if run_extension_point(..
goes after set_prerequisite_courses(..
?. Does the order have any impact in the behavior? @anfbermudezme
1b5d580
to
afaef49
Compare
…ort courses_requirements_not_met
5d6b84e
to
7fbd18d
Compare
…g prerequisites from pearson_core
…ustom functionality
4dcd971
to
06d1b0f
Compare
sku_not_enrollment_in_requirement = \ | ||
sku_not_enrollment_in_requirement[course.id] if sku_not_enrollment_in_requirement else None |
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.
Please write all the code related to the 'PEARSON_CORE_STUDENT_NOT_ENROLLED_IN_REQUIREMENTS'
function in Pearson Core. @anfbermudezme
…ED function in Pearson Core
2e514c9
to
4fba153
Compare
…remove magic number un course.py.
4fba153
to
f8f73be
Compare
@@ -1161,6 +1161,13 @@ def settings_handler(request, course_key_string): | |||
if not all(is_valid_course_key(course_key) for course_key in prerequisite_course_keys): | |||
return JsonResponseBadRequest({"error": _("Invalid prerequisite course key")}) | |||
set_prerequisite_courses(course_key, prerequisite_course_keys) | |||
# add all courses with same org and number to milestones. |
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.
# add all courses with same org and number to milestones. | |
# Add all courses with the same org and number to milestones. |
@@ -802,8 +803,22 @@ def student_dashboard(request): | |||
enrollment.course_id for enrollment in course_enrollments | |||
if enrollment.course_overview.pre_requisite_courses | |||
) | |||
|
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.
Unnecessary change. @anfbermudezme
courses_requirements_not_met = get_pre_requisite_courses_not_completed(user, courses_having_prerequisites) | ||
|
||
# sort requirements for each course in course_requirements_not_met. |
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.
# sort requirements for each course in course_requirements_not_met. | |
# Sort requirements for each course in course_requirements_not_met. |
user=user, | ||
courses_requirements_not_met=courses_requirements_not_met, | ||
) | ||
# get the sku value for the requirements of each course in courses_requirements_not_met. |
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.
# get the sku value for the requirements of each course in courses_requirements_not_met. | |
# Get the SKU value for the requirements of each course in courses_requirements_not_met. |
@@ -956,6 +956,20 @@ def course_about(request, course_id): | |||
# get prerequisite courses display names | |||
pre_requisite_courses = get_prerequisite_courses_display(course) | |||
|
|||
courses_requirements_not_met = get_pre_requisite_courses_not_completed(request.user, frozenset({course.id})) | |||
# sort requirements for each course in course_requirements_not_met. |
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.
# sort requirements for each course in course_requirements_not_met. | |
# Sort requirements for each course in course_requirements_not_met. |
user=request.user, | ||
courses_requirements_not_met=courses_requirements_not_met, | ||
) | ||
# the first prerequisite from courses_requirements_not_met is taken. |
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.
I don't think this explains what you are doing here. @anfbermudezme
@@ -994,6 +1008,10 @@ def course_about(request, course_id): | |||
# context. This value is therefor explicitly set to render the appropriate header. | |||
'disable_courseware_header': True, | |||
'pre_requisite_courses': pre_requisite_courses, | |||
'course_requirements': course_requirements if course_requirements else None, |
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.
course_requirements_for_courseware_views returns {} in case things don't go as expected, so you don't need to return None. @anfbermudezme
@@ -994,6 +1008,10 @@ def course_about(request, course_id): | |||
# context. This value is therefor explicitly set to render the appropriate header. | |||
'disable_courseware_header': True, | |||
'pre_requisite_courses': pre_requisite_courses, | |||
'course_requirements': course_requirements if course_requirements else None, | |||
'student_not_enrollment_in_requirement': \ | |||
skus_not_enrollment_in_requirements.get(course.id) if skus_not_enrollment_in_requirements else None, |
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.
sku_not_enrollment_in_requirements returns {} in case things don't go as expected, so you don't need to return None. @anfbermudezme
'course_requirements': course_requirements if course_requirements else None, | ||
'student_not_enrollment_in_requirement': \ | ||
skus_not_enrollment_in_requirements.get(course.id) if skus_not_enrollment_in_requirements else None, | ||
"ecommerce_payment_page": EcommerceService().payment_page_url(), |
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.
Why do you need this? @anfbermudezme
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.
This PR is on hold down due to business decisions
Currently prerequisites operate at the course run level, and only one course run can be specified. Which means that course run dependencies are very inflexible. From the example below a learner could pass course-v1:PS+INTRODA+OCT but if course-v1:PS+DATAVIZ+NOV has a prerequisite of course-v1:PS+INTRODA+NOV then the learner could not proceed with the DATAVIZ course even though they passed an earlier run of INTRODA.
PS+INTRODA:
PS+DATAVIZ (dependent on completion of a run of PS+INTRODA):
Ideally, the PS+DATAVIZ runs would be unlocked if a learner had completed any of the PS+INTRODA runs.
It's neccesary to enable
'MILESTONES_APP':True
and'ENABLE_PREREQUISITE_COURSES': True
. Also it's neccesary pull the PR https://github.com/Pearson-Advance/pearson-core/pull/25 from pearson_core, thenrun_extension_point()
in used in the edx-platform.For this, when a prerequisite is selected in studio, It is added to the milestones_milestone table through the
course.py
module, which in this PR now adds the courses with the same org and number taken from discovery through thecourse_uuid
.For a user to enter the course, the prerequisite must be certified and this is added to a table called milestones_usermilestone, for this the function handle_course_cert_awarded is modified in
lms/djangoapps/certificates/models.py
Also, to improve the learner experience, Following the logic:
For this, add backend functionality to sort courses_requirements_not_met variable if the learner is enrolled and if the learnor is not enrolled find the sku for verified mode of the course requirement, to redirect the link:
About course if the learner is enrolled and basket sku if the learner is not enrolled, to show this in the frontend it's neccesary follow the PR Pearson-Advance/openedx-themes#189 from openedx-themes.
IMPORTANT: To active this functionality for an organization it's necessary to add it in CUSTOM_PREREQUISITES_FOR_ORG in site configurations, both in the lms and in the studio. For example: "CUSTOM_PREREQUISITES_FOR_ORG":["PX"] and enable the Feature Flag ENABLE_CUSTOM_PREREQUISITES.
Also in the course about view for the course with prerequisites the links have the same behavior of the learner dashboard, that means, About course if the learner is enrolled and basket sku if the learner is not enrolled.