From 38a99397ef40c964b8a9ebd958e26e602a29b327 Mon Sep 17 00:00:00 2001 From: Ivo Branco Date: Fri, 21 Jun 2024 22:41:51 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(rdfa)=20fix=20errors=20on=20Google?= =?UTF-8?q?=20Search=20Console?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Google Search Console requires more information for the RDFa information. Fixed errors: - Missing field 'hasCourseInstance' - Either 'courseWorkload' or 'courseSchedule' should be specified (in 'hasCourseInstance') Fix course enrollment_count shouldn't include the hidden runs. --- CHANGELOG.md | 5 +++ src/richie/apps/courses/models/course.py | 12 ++++--- .../templates/courses/cms/course_detail.html | 33 ++++++++++--------- tests/apps/courses/test_models_course.py | 20 +++++++++++ .../test_templates_course_detail_rdfa.py | 15 +++++++++ 5 files changed, 65 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcb2e62c58..ccd81da529 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ Versioning](https://semver.org/spec/v2.0.0.html). ## [Unrealeased] +### Fixed + +- Fix RDFa errors on Google Search Console +- Fix course enrollment count shouldn't include the hidden runs. + ## [2.28.1] ### Fixed diff --git a/src/richie/apps/courses/models/course.py b/src/richie/apps/courses/models/course.py index cfe1896ecd..ddb491bbe2 100644 --- a/src/richie/apps/courses/models/course.py +++ b/src/richie/apps/courses/models/course.py @@ -481,10 +481,14 @@ def course_runs_enrollment_count(self): node = self.extended_object.node current_and_descendant_nodes = node.__class__.get_tree(parent=node) - return CourseRun.objects.filter( - direct_course__extended_object__node__in=current_and_descendant_nodes, - direct_course__extended_object__publisher_is_draft=is_draft, - ).aggregate(sum=Sum("enrollment_count"))["sum"] + return ( + CourseRun.objects.filter( + direct_course__extended_object__node__in=current_and_descendant_nodes, + direct_course__extended_object__publisher_is_draft=is_draft, + ) + .exclude(catalog_visibility=CourseRunCatalogVisibility.HIDDEN) + .aggregate(sum=Sum("enrollment_count"))["sum"] + ) @cached_property def languages_display(self): diff --git a/src/richie/apps/courses/templates/courses/cms/course_detail.html b/src/richie/apps/courses/templates/courses/cms/course_detail.html index cbdae3859f..de94a5f6b7 100644 --- a/src/richie/apps/courses/templates/courses/cms/course_detail.html +++ b/src/richie/apps/courses/templates/courses/cms/course_detail.html @@ -368,22 +368,23 @@

{% blocktrans context "course_detail__title" %} {% block runs %}
{% render_model_add course "" "" "get_admin_url_to_add_run" %} - {% with runs_dict=current_page.course.course_runs_dict %} - {% with open_visible_runs=runs_dict.0|add:runs_dict.1|add:runs_dict.2|visible_on_course_page:request.toolbar.edit_mode_active %} - - {% for run in open_visible_runs %} - - - - - - - - {% endfor %} - {% endwith %} + {% with runs=current_page.course.course_runs|visible_on_course_page:request.toolbar.edit_mode_active %} + + {% for run in runs %} + {% if run.is_visible_on_course_page %} + + + + + + + + + {% endif %} + {% endfor %} {% endwith %}
{% endblock runs %} diff --git a/tests/apps/courses/test_models_course.py b/tests/apps/courses/test_models_course.py index 13ea6d177f..5933f47e61 100644 --- a/tests/apps/courses/test_models_course.py +++ b/tests/apps/courses/test_models_course.py @@ -22,6 +22,7 @@ from richie.apps.courses import defaults, factories from richie.apps.courses.cms_plugins import CoursePlugin from richie.apps.courses.models import Course, CourseRun, CourseRunTranslation, Program +from richie.apps.courses.models.course import CourseRunCatalogVisibility # pylint: disable=too-many-public-methods @@ -941,6 +942,25 @@ def test_models_course_course_runs_enrollment_count(self): enrollment_count_sum = course.public_extension.course_runs_enrollment_count self.assertEqual(enrollment_count_sum, 5) + def test_models_course_course_runs_enrollment_count_hidden_run(self): + """ + The `course_runs_enrollment_count` property should not include the hidden course runs. + """ + course = factories.CourseFactory() + + # Create random course runs for this course + factories.CourseRunFactory(enrollment_count=30, direct_course=course) + factories.CourseRunFactory(enrollment_count=20, direct_course=course) + factories.CourseRunFactory( + enrollment_count=10, + direct_course=course, + catalog_visibility=CourseRunCatalogVisibility.HIDDEN, + ) + course.extended_object.publish("en") + + enrollment_count_sum = course.public_extension.course_runs_enrollment_count + self.assertEqual(enrollment_count_sum, 50) + def test_models_course_course_runs_dict(self): """The `course_runs_dict` property should be computed on once per request.""" course = factories.CourseFactory() diff --git a/tests/apps/courses/test_templates_course_detail_rdfa.py b/tests/apps/courses/test_templates_course_detail_rdfa.py index 27a8a2d83a..80befff639 100644 --- a/tests/apps/courses/test_templates_course_detail_rdfa.py +++ b/tests/apps/courses/test_templates_course_detail_rdfa.py @@ -25,6 +25,7 @@ OrganizationFactory, PersonFactory, ) +from richie.apps.courses.models import CourseRunCatalogVisibility from richie.plugins.nesteditem.defaults import ACCORDION # pylint: disable=too-many-lines,too-many-locals,too-many-statements @@ -213,6 +214,17 @@ def test_templates_course_detail_rdfa(self): languages=["de"], enrollment_count=3000, ) + CourseRunFactory( + title="A hidden course run", + direct_course=course, + start=datetime(2010, 6, 1, tzinfo=timezone.utc), + end=datetime(2050, 7, 10, tzinfo=timezone.utc), + enrollment_start=datetime(2010, 6, 13, tzinfo=timezone.utc), + enrollment_end=datetime(2050, 6, 20, tzinfo=timezone.utc), + languages=["pt"], + enrollment_count=100, + catalog_visibility=CourseRunCatalogVisibility.HIDDEN, + ) contributor1.extended_object.publish("en") course.extended_object.publish("en") @@ -552,6 +564,9 @@ def test_templates_course_detail_rdfa(self): self.assertTrue( (course_run_subject, SDO.courseMode, Literal("online")) in graph ) + self.assertTrue( + (course_run_subject, SDO.courseWorkload, Literal("PT3H")) in graph + ) for title in ["Run 0", "Run 1"]: (course_run_subject,) = graph.subjects(SDO.name, Literal(title))