From 537fcd4b280baf954d86d37f2e47661d9ac0374a Mon Sep 17 00:00:00 2001 From: Muhammad Afaq Shuaib Date: Fri, 13 Sep 2024 23:33:45 +0500 Subject: [PATCH] feat: add capabilty in queryset method to exclude hidden runs (#4443) --- .github/workflows/ci.yml | 5 +- .../commands/populate_product_catalog.py | 2 +- .../tests/test_populate_product_catalog.py | 103 ++++++++++++++---- .../apps/course_metadata/query.py | 13 ++- 4 files changed, 98 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c3d1c25878..38d29a9e5a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,7 +63,10 @@ jobs: uses: actions/download-artifact@v4 - name: Run coverage run: make ci_coverage - - uses: codecov/codecov-action@v1 + - uses: codecov/codecov-action@v4 + with: + token: ${{ secrets.CODECOV_TOKEN }} + fail_ci_if_error: true quality: runs-on: ubuntu-latest diff --git a/course_discovery/apps/course_metadata/management/commands/populate_product_catalog.py b/course_discovery/apps/course_metadata/management/commands/populate_product_catalog.py index 769b6e9e8f..0a35e1466b 100644 --- a/course_discovery/apps/course_metadata/management/commands/populate_product_catalog.py +++ b/course_discovery/apps/course_metadata/management/commands/populate_product_catalog.py @@ -74,7 +74,7 @@ def get_products(self, product_type, product_source): ] if product_type in ['executive_education', 'bootcamp', 'ocm_course']: - queryset = Course.objects.available().select_related('partner', 'type') + queryset = Course.objects.available(exclude_hidden_runs=True).select_related('partner', 'type') if product_type == 'ocm_course': queryset = queryset.filter(type__slug__in=ocm_course_catalog_types) diff --git a/course_discovery/apps/course_metadata/management/commands/tests/test_populate_product_catalog.py b/course_discovery/apps/course_metadata/management/commands/tests/test_populate_product_catalog.py index 557cfb1dbf..fd8c31d736 100644 --- a/course_discovery/apps/course_metadata/management/commands/tests/test_populate_product_catalog.py +++ b/course_discovery/apps/course_metadata/management/commands/tests/test_populate_product_catalog.py @@ -41,6 +41,12 @@ def setUp(self): self.course_run_2 = CourseRunFactory.create_batch( 2, course=Course.objects.all()[1] ) + self.course_run_3 = CourseRunFactory( + course=Course.objects.all()[1], + status=CourseRunStatus.Published, + hidden=True, + ) + self.hidden_run_seat = SeatFactory.create(course_run=self.course_run_3) self.program_type = ProgramTypeFactory.create(slug=ProgramType.MICROMASTERS) self.degrees = DegreeFactory.create_batch( 2, @@ -52,32 +58,89 @@ def setUp(self): card_image=factory.django.ImageField() ) + def _execute_populate_product_catalog(self, product_type, output_csv, product_source, gspread_client_flag=False): + """ + Helper method to execute populate_product_catalog command + """ + call_command( + "populate_product_catalog", + product_type=product_type, + output_csv=output_csv, + product_source=product_source, + gspread_client_flag=gspread_client_flag, + ) + with open(output_csv, "r") as output_csv_file: + return list(csv.DictReader(output_csv_file)) + + def _assert_row_data(self, rows, expected_uuids, should_exist=True): + """ + Helper method to assert row data in the CSV + + Args: + rows (list): List of CSV rows + expected_uuids (list): List of Course UUIDs to be expected in the CSV + should_exist (bool, optional): Whether the expected UUIDs should exist in the CSV. Defaults to True. + """ + for uuid in expected_uuids: + matching_rows = [row for row in rows if row["UUID"] == str(uuid.hex)] + if should_exist: + self.assertEqual(len(matching_rows), 1) + else: + self.assertEqual(len(matching_rows), 0, f"UUID {uuid} shouldn't be in the CSV") + def test_populate_product_catalog(self): """ Test populate_product_catalog command and verify data has been populated successfully """ with NamedTemporaryFile() as output_csv: - call_command( - "populate_product_catalog", - product_type="ocm_course", - output_csv=output_csv.name, - product_source="edx", - gspread_client_flag=False, + rows = self._execute_populate_product_catalog( + product_source="edx", product_type="ocm_course", output_csv=output_csv.name ) - with open(output_csv.name, "r") as output_csv_file: - csv_reader = csv.DictReader(output_csv_file) - for row in csv_reader: - self.assertIn("UUID", row) - self.assertIn("Title", row) - self.assertIn("Organizations Name", row) - self.assertIn("Organizations Logo", row) - self.assertIn("Organizations Abbr", row) - self.assertIn("Languages", row) - self.assertIn("Subjects", row) - self.assertIn("Subjects Spanish", row) - self.assertIn("Marketing URL", row) - self.assertIn("Marketing Image", row) + for row in rows: + self.assertIn("UUID", row) + self.assertIn("Title", row) + self.assertIn("Organizations Name", row) + self.assertIn("Organizations Logo", row) + self.assertIn("Organizations Abbr", row) + self.assertIn("Languages", row) + self.assertIn("Subjects", row) + self.assertIn("Subjects Spanish", row) + self.assertIn("Marketing URL", row) + self.assertIn("Marketing Image", row) + + def test_populate_product_catalog_for_courses_with_hidden_and_non_hidden_published_runs(self): + """ + Test populate_product_catalog command for course having hidden and non-hidden published runs + and verify data has been populated successfully. + """ + hidden_course_run = CourseRunFactory( + course=Course.objects.all()[1], + status=CourseRunStatus.Published, + hidden=True, + ) + SeatFactory.create(course_run=hidden_course_run) + + with NamedTemporaryFile() as output_csv: + rows = self._execute_populate_product_catalog( + product_source="edx", product_type="ocm_course", output_csv=output_csv.name + ) + self._assert_row_data(rows, [self.courses[0].uuid], should_exist=True) + self._assert_row_data(rows, [self.courses[1].uuid], should_exist=False) + + non_hidden_course_run = CourseRunFactory( + course=Course.objects.all()[1], + status=CourseRunStatus.Published, + hidden=False, + ) + SeatFactory.create(course_run=non_hidden_course_run) + + with NamedTemporaryFile() as output_csv: + rows = self._execute_populate_product_catalog( + product_source="edx", product_type="ocm_course", output_csv=output_csv.name + ) + self._assert_row_data(rows, [self.courses[0].uuid], should_exist=True) + self._assert_row_data(rows, [self.courses[1].uuid], should_exist=True) def test_populate_product_catalog_for_degrees(self): """ @@ -197,7 +260,7 @@ def test_populate_product_catalog_excludes_non_marketable_degrees(self): # Check that non-marketable degrees are not in the CSV for degree in non_marketable_degrees: with self.subTest(degree=degree): - matching_rows = [row for row in rows if row["UUID"] == str(degree.uuid)] + matching_rows = [row for row in rows if row["UUID"] == str(degree.uuid.hex)] self.assertEqual( len(matching_rows), 0, f"Non-marketable degree '{degree.title}' should not be in the CSV" ) diff --git a/course_discovery/apps/course_metadata/query.py b/course_discovery/apps/course_metadata/query.py index 1a191fb817..6168bb732f 100644 --- a/course_discovery/apps/course_metadata/query.py +++ b/course_discovery/apps/course_metadata/query.py @@ -8,16 +8,19 @@ class CourseQuerySet(models.QuerySet): - def available(self): + def available(self, exclude_hidden_runs=False): """ A Course is considered to be "available" if it contains at least one CourseRun that can be enrolled in immediately, is ongoing or yet to start, and appears on the marketing site. + + Args: + exclude_hidden_runs (bool): Whether to exclude hidden course runs from the query """ now = datetime.datetime.now(pytz.UTC) - # A CourseRun is "enrollable" if its enrollment start date has passed, # is now, or is None, and its enrollment end date is in the future or is None. + enrollable = ( ( Q(course_runs__enrollment_start__lte=now) | @@ -54,7 +57,11 @@ def available(self): # By itself, the query performs a join across several tables and would return # the id of the same course multiple times (a separate copy for each available # seat in each available run). - ids = self.filter(enrollable & not_ended & marketable).values('id').distinct() + if exclude_hidden_runs: + non_hidden = ~Q(course_runs__hidden=True) + ids = self.filter(enrollable & not_ended & marketable & non_hidden).values('id').distinct() + else: + ids = self.filter(enrollable & not_ended & marketable).values('id').distinct() # Now return the full object for each of the selected ids return self.filter(id__in=ids)