Skip to content

Commit

Permalink
Info box slow render fixes (#2304)
Browse files Browse the repository at this point in the history 
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
@cp-at-mit @pre-commit-ci
cp-at-mit and pre-commit-ci[bot] authored Jul 29, 2024
1 parent 50be6b0 commit dd29236
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 432 deletions.
16 changes: 2 additions & 14 deletions cms/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1109,18 +1109,6 @@ def get_url_parts(self, request=None):
),
)

@property
def get_current_finaid(self):
"""
Returns information about financial aid for the current learner.
If the learner has a flexible pricing(financial aid) request that's
approved, this should return the learner's adjusted price. If they
don't, this should return the Page for the applicable request form.
If they're not logged in, this should return None.
"""
raise NotImplementedError

def get_context(self, request, *args, **kwargs): # noqa: ARG002
instructors = [
member.linked_instructor_page
Expand Down Expand Up @@ -1165,7 +1153,7 @@ def product(self):
"""Gets the product associated with this page"""
return self.course

def get_current_finaid(self, request):
def _get_current_finaid(self, request):
"""
Returns information about financial aid for the current learner.
Expand Down Expand Up @@ -1216,7 +1204,7 @@ def get_context(self, request, *args, **kwargs):
and relevant_run is not None
and (relevant_run.is_in_progress or request.user.is_editor)
)
finaid_price = self.get_current_finaid(request)
finaid_price = self._get_current_finaid(request)
product = (
relevant_run.products.filter(is_active=True).first()
if relevant_run
Expand Down
22 changes: 0 additions & 22 deletions cms/models_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -618,28 +618,6 @@ def test_courseware_title_synced_with_product_page_title(test_course):
assert courseware.title == updated_title


def test_get_current_finaid_with_flex_price_for_expired_course_run(mocker):
"""
Tests that get_current_finaid returns None for a user approved for
financial aid on a course with only expired course runs.
"""
now = now_in_utc()
course_run = CourseRunFactory.create(enrollment_end=now - timedelta(days=10))
ProductFactory.create(purchasable_object=course_run)
rf = RequestFactory()
request = rf.get("/")
request.user = UserFactory.create()
patched_flexible_price_approved = mocker.patch(
"flexiblepricing.api.is_courseware_flexible_price_approved"
)
patched_flexible_price_discount = mocker.patch(
"flexiblepricing.api.determine_courseware_flexible_price_discount"
)
assert course_run.course.page.get_current_finaid(request) is None
patched_flexible_price_discount.assert_not_called()
patched_flexible_price_approved.assert_not_called()


@pytest.mark.parametrize("flex_form_for_course", [True, False])
def test_flexible_pricing_request_form_context(flex_form_for_course):
"""
Expand Down
6 changes: 3 additions & 3 deletions cms/templates/product_page.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

{% block seohead %}
{% meta_tags page %}
{% if page.thumbnail_image %}
<meta property="og:image" content="{% feature_img_src page.thumbnail_image %}" />
{% if page.feature_image %}
<meta property="og:image" content="{% feature_img_src page.feature_image %}" />
{% endif %}
{% endblock %}

Expand Down Expand Up @@ -129,7 +129,7 @@ <h2>Prerequisites</h2>
{{ page.prerequisites |richtext }}
</section>{% endif %}

{% if instructors or page.faculty_members %}
{% if instructors %}
<section class="faculty-section" id="instructors">
<div class="container">
<h2>{{ page.faculty_section_title }}</h2>
Expand Down
94 changes: 0 additions & 94 deletions courses/api.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"""API for the Courses app"""

import itertools
import logging
from collections import namedtuple
from datetime import timedelta
Expand All @@ -16,7 +15,6 @@
from mitol.common.utils.collections import (
first_or_none,
has_equal_properties,
partition,
)
from requests.exceptions import ConnectionError as RequestsConnectionError
from requests.exceptions import HTTPError
Expand Down Expand Up @@ -96,98 +94,6 @@ def get_user_relevant_program_course_run_qset(
return enrollable_run_qset.order_by("enrollment_start")


def get_relevant_course_run(
course: Course,
) -> CourseRun:
"""
For a given Course, finds the course run that is the most relevant to the user.
For anonymous users, this means the soonest enrollable course run.
For logged-in users, this means an active course run that they're enrolled in, or the soonest enrollable course run.
"""
runs = get_relevant_course_run_qset(course)
run = first_or_none(runs)
return run # noqa: RET504


def get_user_enrollments(user):
"""
Fetches a user's enrollments
If the user is enrolled in a course that belongs to a program, we count that
as an enrollment in the program (and we create an ephemeral
ProgramEnrollment for that) unless the user is already enrolled in the
program directly.
Args:
user (User): A user
Returns:
UserEnrollments: An object representing a user's program and course run enrollments
"""
course_run_enrollments = (
CourseRunEnrollment.objects.filter(user=user).order_by("run__start_date").all()
)

all_course_run_programs = [] # all the programs that the course runs belong to
filtered_course_run_programs = [] # just the programs we don't have distinct ProgramEnrollments for

for cre in course_run_enrollments:
if cre.run.course.programs:
all_course_run_programs += cre.run.course.programs
all_course_run_programs = set(all_course_run_programs)

program_enrollments = ProgramEnrollment.objects.filter(user=user).all()

for program in all_course_run_programs:
found = False

for enrollment in program_enrollments:
if enrollment.program == program:
found = True
break

if not found:
filtered_course_run_programs.append(program)

program_enrollments = list(
itertools.chain(
program_enrollments,
[
ProgramEnrollment(user=user, program=program)
for program in filtered_course_run_programs
],
)
)

program_courses = itertools.chain(
*(
program_enrollment.program.courses
for program_enrollment in program_enrollments
)
)
program_course_ids = set(course[0].id for course in program_courses) # noqa: C401
non_program_run_enrollments, program_run_enrollments = partition(
course_run_enrollments,
lambda course_run_enrollment: (
course_run_enrollment.run.course_id in program_course_ids
),
)
program_enrollments, past_program_enrollments = partition(
program_enrollments, lambda program_enrollment: program_enrollment.is_ended
)
non_program_run_enrollments, past_non_program_run_enrollments = partition(
non_program_run_enrollments,
lambda non_program_run_enrollment: non_program_run_enrollment.is_ended,
)

return UserEnrollments(
programs=program_enrollments,
past_programs=past_program_enrollments,
program_runs=program_run_enrollments,
non_program_runs=non_program_run_enrollments,
past_non_program_runs=past_non_program_run_enrollments,
)


def create_run_enrollments( # noqa: C901
user,
runs,
Expand Down
174 changes: 0 additions & 174 deletions courses/api_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
defer_enrollment,
generate_course_run_certificates,
generate_program_certificate,
get_relevant_course_run,
get_user_enrollments,
manage_course_run_certificate_access,
manage_program_certificate_access,
override_user_grade,
Expand Down Expand Up @@ -118,178 +116,6 @@ def courses_api_logs(mocker):
return mocker.patch("courses.api.log")


@pytest.mark.parametrize("is_enrolled", [True, False])
@pytest.mark.parametrize("is_live", [True, False])
def test_get_relevant_course_run(user, dates, course, is_live, is_enrolled):
"""
get_relevant_course_run should return the soonest course
run that is enrollable, even if the user is already enrolled.
"""
# One run in the near future, one run in progress with an expired enrollment period, and one run in the far future.
course_runs = CourseRunFactory.create_batch(
3,
course=course,
start_date=factory.Iterator(
[dates.future_10_days, dates.past_10_days, dates.future_30_days]
),
end_date=factory.Iterator([None, dates.future_10_days, dates.future_60_days]),
enrollment_start=factory.Iterator(
[dates.past_60_days, dates.past_10_days, dates.past_30_days]
),
enrollment_end=factory.Iterator(
[None, dates.future_30_days, dates.future_60_days]
),
)
if is_enrolled:
# Enroll in the in-progress course run
CourseRunEnrollmentFactory.create(
run=course_runs[1], user=user, edx_enrolled=True, active=True
)
returned_run = get_relevant_course_run(course=course)
assert returned_run == course_runs[0]
course_runs[0].live = is_live
course_runs[0].save()
returned_run = get_relevant_course_run(course=course)
assert returned_run == (course_runs[0] if is_live else course_runs[2])


def test_get_relevant_course_run_invalid_dates(user, dates, course):
"""
get_relevant_course_run should ignore course runs with any of the following properties when the user is not enrolled:
1) No start date or enrollment start date
2) An enrollment end date in the past
3) The course run is not live
"""
CourseRunFactory.create_batch(
3,
course=course,
start_date=factory.Iterator([None, dates.future_10_days, dates.past_30_days]),
end_date=factory.Iterator([None, dates.future_60_days, dates.past_10_days]),
enrollment_start=factory.Iterator(
[dates.future_10_days, None, dates.past_30_days]
),
enrollment_end=factory.Iterator(
[dates.future_60_days, None, dates.past_10_days]
),
)
CourseRunFactory.create(course=course, live=False)
returned_run = get_relevant_course_run(course=course)
assert returned_run is None


def test_get_user_enrollments(user, program_with_empty_requirements): # noqa: F811
"""Test that get_user_enrollments returns an object with a user's program and course enrollments"""
past_date = now_in_utc() - timedelta(days=1)
past_start_dates = [
now_in_utc() - timedelta(days=2),
now_in_utc() - timedelta(days=3),
now_in_utc() - timedelta(days=4),
]
program_course_runs = CourseRunFactory.create_batch(3)
for course_run in program_course_runs:
program_with_empty_requirements.add_requirement(course_run.course)

past_program = ProgramFactory.create()
ProgramRequirementFactory.add_root(past_program)
root_node = past_program.requirements_root
required_courses_node = root_node.add_child(
node_type=ProgramRequirementNodeType.OPERATOR,
operator=ProgramRequirement.Operator.ALL_OF,
title="Required Courses",
)
past_program_course_runs = CourseRunFactory.create_batch(
3,
start_date=factory.Iterator(past_start_dates),
end_date=past_date,
)
for course_run in past_program_course_runs:
past_program.add_requirement(course_run.course)

non_program_course_runs = CourseRunFactory.create_batch(2)
past_non_program_course_runs = CourseRunFactory.create_batch(
2,
start_date=factory.Iterator(past_start_dates),
end_date=past_date,
)
all_course_runs = (
program_course_runs
+ past_program_course_runs
+ non_program_course_runs
+ past_non_program_course_runs
)
course_run_enrollments = CourseRunEnrollmentFactory.create_batch(
len(all_course_runs), run=factory.Iterator(all_course_runs), user=user
)
program_enrollment = ProgramEnrollmentFactory.create(
program=program_with_empty_requirements, user=user
)
past_program_enrollment = ProgramEnrollmentFactory.create(
program=past_program, user=user
)
# Add a non-active enrollment so we can confirm that it isn't returned
CourseRunEnrollmentFactory.create(user=user, active=False)

def key_func(enrollment):
"""Function for sorting runs by start_date"""
return enrollment.run.start_date

user_enrollments = get_user_enrollments(user)
assert list(user_enrollments.programs) == [program_enrollment]
assert list(user_enrollments.past_programs) == [past_program_enrollment]
assert list(user_enrollments.program_runs) == sorted(
[
run_enrollment
for run_enrollment in course_run_enrollments
if run_enrollment.run in program_course_runs + past_program_course_runs
],
key=key_func,
)
assert list(user_enrollments.non_program_runs) == sorted(
[
run_enrollment
for run_enrollment in course_run_enrollments
if run_enrollment.run in non_program_course_runs
],
key=key_func,
)

assert list(user_enrollments.past_non_program_runs) == sorted(
[
run_enrollment
for run_enrollment in course_run_enrollments
if run_enrollment.run in past_non_program_course_runs
],
key=key_func,
)

# Create a separate Course and Program and then just enroll in the course
# The program ought to still show up as an enrollment
separate_program = ProgramFactory.create()
ProgramRequirementFactory.add_root(separate_program)
root_node = separate_program.requirements_root
required_courses_node = root_node.add_child( # noqa: F841
node_type=ProgramRequirementNodeType.OPERATOR,
operator=ProgramRequirement.Operator.ALL_OF,
title="Required Courses",
)
separate_courserun = CourseRunFactory.create()
separate_program.add_requirement(separate_courserun.course)
separate_courserun_enrollment = CourseRunEnrollmentFactory.create( # noqa: F841
run=separate_courserun, user=user
)

user_enrollments = get_user_enrollments(user)
assert len(list(user_enrollments.programs)) == 2

user_enrollments = get_user_enrollments(user)
enrollment_programs = [
enrollment.program for enrollment in user_enrollments.programs
]
assert program_enrollment.program in enrollment_programs
assert separate_program in enrollment_programs


@pytest.mark.parametrize(
"enrollment_mode", [EDX_DEFAULT_ENROLLMENT_MODE, EDX_ENROLLMENT_VERIFIED_MODE]
)
Expand Down
Loading

0 comments on commit dd29236

Please sign in to comment.