From 1665f8f70dd86c2c49f0f23489bce0536a2f8f81 Mon Sep 17 00:00:00 2001 From: Fateme Khodayari Date: Sat, 9 Mar 2024 12:32:47 +0330 Subject: [PATCH 1/4] fix: course progress url returned based on course_home_mfe_progress_tab_is_active --- lms/djangoapps/learner_home/serializers.py | 3 ++- .../learner_home/test_serializers.py | 26 ++++++++++++++++++- lms/djangoapps/learner_home/utils.py | 16 ++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/learner_home/serializers.py b/lms/djangoapps/learner_home/serializers.py index 0ce7bf9c6977..b3471715b9dc 100644 --- a/lms/djangoapps/learner_home/serializers.py +++ b/lms/djangoapps/learner_home/serializers.py @@ -15,6 +15,7 @@ from common.djangoapps.course_modes.models import CourseMode from openedx.features.course_experience import course_home_url from xmodule.data import CertificatesDisplayBehaviors +from lms.djangoapps.learner_home.utils import course_progress_url class LiteralField(serializers.Field): @@ -116,7 +117,7 @@ def get_homeUrl(self, instance): return course_home_url(instance.course_id) def get_progressUrl(self, instance): - return reverse("progress", kwargs={"course_id": instance.course_id}) + return course_progress_url(instance.course_id) def get_unenrollUrl(self, instance): return reverse("course_run_refund_status", args=[instance.course_id]) diff --git a/lms/djangoapps/learner_home/test_serializers.py b/lms/djangoapps/learner_home/test_serializers.py index f588af58aee4..ac11a8b2990d 100644 --- a/lms/djangoapps/learner_home/test_serializers.py +++ b/lms/djangoapps/learner_home/test_serializers.py @@ -51,7 +51,7 @@ SuggestedCourseSerializer, UnfulfilledEntitlementSerializer, ) - +from lms.djangoapps.learner_home.utils import course_progress_url from lms.djangoapps.learner_home.test_utils import ( datetime_to_django_format, random_bool, @@ -224,6 +224,30 @@ def test_missing_resume_url(self): # Then the resumeUrl is None, which is allowed self.assertIsNone(output_data["resumeUrl"]) + def is_progress_url_matching_course_home_mfe_progress_tab_is_active(self): + """ + Compares the progress URL generated by CourseRunSerializer to the expected progress URL. + + :return: True if the generated progress URL matches the expected, False otherwise. + """ + input_data = self.create_test_enrollment() + input_context = self.create_test_context(input_data.course.id) + output_data = CourseRunSerializer(input_data, context=input_context).data + return output_data['progressUrl'] == course_progress_url(input_data.course.id) + + @mock.patch('lms.djangoapps.learner_home.utils.course_home_mfe_progress_tab_is_active') + def test_progress_url(self, mock_course_home_mfe_progress_tab_is_active): + """ + Tests the progress URL generated by the CourseRunSerializer. When course_home_mfe_progress_tab_is_active + is true, the generated progress URL must point to the progress page of the course home (learning) MFE. + Otherwise, it must point to the legacy progress page. + """ + mock_course_home_mfe_progress_tab_is_active.return_value = True + self.assertTrue(self.is_progress_url_matching_course_home_mfe_progress_tab_is_active()) + + mock_course_home_mfe_progress_tab_is_active.return_value = False + self.assertTrue(self.is_progress_url_matching_course_home_mfe_progress_tab_is_active()) + @ddt.ddt class TestCoursewareAccessSerializer(LearnerDashboardBaseTest): diff --git a/lms/djangoapps/learner_home/utils.py b/lms/djangoapps/learner_home/utils.py index 28e4479f9439..96af6a64452b 100644 --- a/lms/djangoapps/learner_home/utils.py +++ b/lms/djangoapps/learner_home/utils.py @@ -4,6 +4,7 @@ import logging +from django.urls import reverse from django.contrib.auth import get_user_model from django.core.exceptions import MultipleObjectsReturned from rest_framework.exceptions import PermissionDenied, NotFound @@ -11,6 +12,8 @@ from common.djangoapps.student.models import ( get_user_by_username_or_email, ) +from lms.djangoapps.course_home_api.toggles import course_home_mfe_progress_tab_is_active +from openedx.features.course_experience.url_helpers import get_learning_mfe_home_url log = logging.getLogger(__name__) User = get_user_model() @@ -54,3 +57,16 @@ def get_masquerade_user(request): ) log.info(success_msg) return masquerade_user + + +def course_progress_url(course_key) -> str: + """ + Returns the course progress page's URL for the current user. + + :param course_key: The course key for which the home url is being requested. + + :return: The course progress page URL. + """ + if course_home_mfe_progress_tab_is_active(course_key): + return get_learning_mfe_home_url(course_key, url_fragment='progress') + return reverse('progress', kwargs={'course_id': course_key}) From 3ec40ead15a34eb3f89e26ca5feb0e0d40b2eb23 Mon Sep 17 00:00:00 2001 From: Muhammad Anas <88967643+Anas12091101@users.noreply.github.com> Date: Wed, 7 Aug 2024 14:44:38 +0500 Subject: [PATCH 2/4] backport fix: disable submit button for archived courses (#34920) to quince (#35225) * fix: disable submit button for archived courses (#34920) --- xmodule/capa_block.py | 15 ++++++++++++++- xmodule/tests/test_capa_block.py | 33 +++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/xmodule/capa_block.py b/xmodule/capa_block.py index 7b58b5aa9ab8..b08da69a8e86 100644 --- a/xmodule/capa_block.py +++ b/xmodule/capa_block.py @@ -733,12 +733,25 @@ def generate_report_data(self, user_state_iterator, limit_responses=None): } yield (user_state.username, report) + @property + def course_end_date(self): + """ + Return the end date of the problem's course + """ + + try: + course_block_key = self.runtime.course_entry.structure['root'] + return self.runtime.course_entry.structure['blocks'][course_block_key].fields['end'] + except (AttributeError, KeyError): + return None + @property def close_date(self): """ Return the date submissions should be closed from. """ - due_date = self.due + + due_date = self.due or self.course_end_date if self.graceperiod is not None and due_date: return due_date + self.graceperiod diff --git a/xmodule/tests/test_capa_block.py b/xmodule/tests/test_capa_block.py index b511fe403e43..a5b564324f5d 100644 --- a/xmodule/tests/test_capa_block.py +++ b/xmodule/tests/test_capa_block.py @@ -10,7 +10,7 @@ import random import textwrap import unittest -from unittest.mock import DEFAULT, Mock, patch +from unittest.mock import DEFAULT, Mock, patch, PropertyMock import pytest import ddt @@ -648,6 +648,37 @@ def test_closed(self): due=self.yesterday_str) assert block.closed() + @patch.object(ProblemBlock, 'course_end_date', new_callable=PropertyMock) + def test_closed_for_archive(self, mock_course_end_date): + + # Utility to create a datetime object in the past + def past_datetime(days): + return (datetime.datetime.now(UTC) - datetime.timedelta(days=days)) + + # Utility to create a datetime object in the future + def future_datetime(days): + return (datetime.datetime.now(UTC) + datetime.timedelta(days=days)) + + block = CapaFactory.create(max_attempts="1", attempts="0") + + # For active courses without graceperiod + mock_course_end_date.return_value = future_datetime(10) + assert not block.closed() + + # For archive courses without graceperiod + mock_course_end_date.return_value = past_datetime(10) + assert block.closed() + + # For active courses with graceperiod + mock_course_end_date.return_value = future_datetime(10) + block.graceperiod = datetime.timedelta(days=2) + assert not block.closed() + + # For archive courses with graceperiod + mock_course_end_date.return_value = past_datetime(2) + block.graceperiod = datetime.timedelta(days=3) + assert not block.closed() + def test_parse_get_params(self): # Valid GET param dict From 493e7ef43f8d93e7aaf3ce3487632ccb84158eab Mon Sep 17 00:00:00 2001 From: Mudassir Hafeez Date: Tue, 27 Aug 2024 12:03:20 +0500 Subject: [PATCH 3/4] chore: provide logo url from backend for batch enrollment email (#35138) (#35378) * chore: provide logo url from backend for batch enrollment email --- lms/djangoapps/instructor/enrollment.py | 2 ++ .../instructor/tests/test_enrollment.py | 16 ++++++++++++++++ .../ace_common/edx_ace/common/base_body.html | 2 +- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index f8bf1dd8667e..416153a6187f 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -33,6 +33,7 @@ get_event_transaction_id, set_event_transaction_type ) +from lms.djangoapps.branding.api import get_logo_url_for_email from lms.djangoapps.courseware.models import StudentModule from lms.djangoapps.grades.api import constants as grades_constants from lms.djangoapps.grades.api import disconnect_submissions_signal_receiver @@ -456,6 +457,7 @@ def get_email_params(course, auto_enroll, secure=True, course_key=None, display_ 'contact_mailing_address': contact_mailing_address, 'platform_name': platform_name, 'site_configuration_values': configuration_helpers.get_current_site_configuration_values(), + 'logo_url': get_logo_url_for_email(), } return email_params diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 4aa14e32256b..89f24bc9a4f0 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -23,6 +23,7 @@ from common.djangoapps.student.models import CourseEnrollment, CourseEnrollmentAllowed, anonymous_id_for_user from common.djangoapps.student.roles import CourseCcxCoachRole from common.djangoapps.student.tests.factories import AdminFactory, UserFactory +from lms.djangoapps.branding.api import get_logo_url_for_email from lms.djangoapps.ccx.tests.factories import CcxFactory from lms.djangoapps.course_blocks.api import get_course_blocks from lms.djangoapps.courseware.models import StudentModule @@ -937,6 +938,7 @@ def setUpClass(cls): ) cls.course_about_url = cls.course_url + 'about' cls.registration_url = f'https://{site}/register' + cls.logo_url = get_logo_url_for_email() def test_normal_params(self): # For a normal site, what do we expect to get for the URLs? @@ -947,6 +949,7 @@ def test_normal_params(self): assert result['course_about_url'] == self.course_about_url assert result['registration_url'] == self.registration_url assert result['course_url'] == self.course_url + assert result['logo_url'] == self.logo_url def test_marketing_params(self): # For a site with a marketing front end, what do we expect to get for the URLs? @@ -959,6 +962,19 @@ def test_marketing_params(self): assert result['course_about_url'] is None assert result['registration_url'] == self.registration_url assert result['course_url'] == self.course_url + assert result['logo_url'] == self.logo_url + + @patch('lms.djangoapps.instructor.enrollment.get_logo_url_for_email', return_value='https://www.logo.png') + def test_logo_url_params(self, mock_get_logo_url_for_email): + # Verify that the logo_url is correctly set in the email params + result = get_email_params(self.course, False) + + assert result['auto_enroll'] is False + assert result['course_about_url'] == self.course_about_url + assert result['registration_url'] == self.registration_url + assert result['course_url'] == self.course_url + mock_get_logo_url_for_email.assert_called_once() + assert result['logo_url'] == 'https://www.logo.png' @ddt.ddt diff --git a/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html b/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html index 8d51b16498d7..9319217aa4cf 100644 --- a/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html +++ b/themes/red-theme/lms/templates/ace_common/edx_ace/common/base_body.html @@ -63,7 +63,7 @@ {% filter force_escape %} {% blocktrans %}Go to {{ platform_name }} Home Page{% endblocktrans %} {% endfilter %} From f70063d3d65493f68bc370c00fe698ca8c355848 Mon Sep 17 00:00:00 2001 From: bydawen Date: Mon, 9 Sep 2024 22:30:33 +0300 Subject: [PATCH 4/4] feat: course about page markup and styles improvements (#33791) * feat: course about page markup and styles improvements * test: update tests according to changes * fix: relocate course organization and return removed prerequisites info * fix: display org info above the course title --------- Co-authored-by: Eugene Dyudyunov Co-authored-by: ihor-romaniuk --- lms/djangoapps/courseware/tests/test_about.py | 10 ++++++-- .../sass/multicourse/_course_about.scss | 24 +++++++++++++++++++ lms/templates/courseware/course_about.html | 19 ++++++++++----- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py index f1f915fd02f4..9497fa2a1d68 100644 --- a/lms/djangoapps/courseware/tests/test_about.py +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -156,7 +156,10 @@ def test_pre_requisite_course(self): assert resp.status_code == 200 pre_requisite_courses = get_prerequisite_courses_display(course) pre_requisite_course_about_url = reverse('about_course', args=[str(pre_requisite_courses[0]['key'])]) - assert '{}'.format(pre_requisite_course_about_url, pre_requisite_courses[0]['display']) in resp.content.decode(resp.charset).strip('\n') # pylint: disable=line-too-long + assert ( + f'You must successfully complete ' + f'{pre_requisite_courses[0]["display"]} before you begin this course.' + ) in resp.content.decode(resp.charset).strip('\n') @patch.dict(settings.FEATURES, {'ENABLE_PREREQUISITE_COURSES': True}) def test_about_page_unfulfilled_prereqs(self): @@ -190,7 +193,10 @@ def test_about_page_unfulfilled_prereqs(self): assert resp.status_code == 200 pre_requisite_courses = get_prerequisite_courses_display(course) pre_requisite_course_about_url = reverse('about_course', args=[str(pre_requisite_courses[0]['key'])]) - assert '{}'.format(pre_requisite_course_about_url, pre_requisite_courses[0]['display']) in resp.content.decode(resp.charset).strip('\n') # pylint: disable=line-too-long + assert ( + f'You must successfully complete ' + f'{pre_requisite_courses[0]["display"]} before you begin this course.' + ) in resp.content.decode(resp.charset).strip('\n') url = reverse('about_course', args=[str(pre_requisite_course.id)]) resp = self.client.get(url) diff --git a/lms/static/sass/multicourse/_course_about.scss b/lms/static/sass/multicourse/_course_about.scss index 629b3065778e..a8a34ccec589 100644 --- a/lms/static/sass/multicourse/_course_about.scss +++ b/lms/static/sass/multicourse/_course_about.scss @@ -44,6 +44,11 @@ > div.table { display: table; width: 100%; + + @include media-breakpoint-down(sm) { + display: flex; + flex-direction: column; + } } .intro { @@ -51,6 +56,11 @@ @include clearfix(); + @include media-breakpoint-down(sm) { + width: auto; + order: 2; + } + display: table-cell; vertical-align: middle; padding: $baseline; @@ -127,6 +137,10 @@ a.add-to-cart { @include button(shiny, $button-color); + @include media-breakpoint-down(md) { + width: 100%; + } + box-sizing: border-box; border-radius: 3px; display: block; @@ -189,6 +203,11 @@ @include float(left); @include margin(1px, flex-gutter(8), 0, 0); @include transition(none); + @include media-breakpoint-down(md) { + width: 100%; + margin-right: 0; + margin-bottom: 10px; + } width: flex-grid(5, 8); } @@ -213,6 +232,11 @@ width: flex-grid(4); z-index: 2; + @include media-breakpoint-down(sm) { + width: auto; + order: 1; + } + .hero { border: 1px solid $border-color-3; height: 100%; diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index 91d7a2a28e57..eec9caeadbec 100644 --- a/lms/templates/courseware/course_about.html +++ b/lms/templates/courseware/course_about.html @@ -62,11 +62,10 @@
-

- ${course.display_name_with_default} -

+

${course.display_org_with_default}

+

${course.display_name_with_default}


- ${course.display_org_with_default} +

${get_course_about_section(request, course, 'short_description')}

@@ -160,7 +159,11 @@

<%block name="course_about_important_dates">
    -

  1. ${_("Course Number")}

    ${course.display_number_with_default}
    2. +
  2. + +

    ${_("Course Number")}

    + ${course.display_number_with_default} +
    3. % if not course.start_date_is_still_default: <% course_start_date = course.advertised_start or course.start @@ -231,7 +234,11 @@

    % endif % if get_course_about_section(request, course, "prerequisites"): -

  3. ${_("Requirements")}

    ${get_course_about_section(request, course, "prerequisites")}
    4. +
  4. + +

    ${_("Requirements")}

    + ${get_course_about_section(request, course, "prerequisites")} +
    5. % endif