Skip to content

Commit

Permalink
Merge branch 'master' into afaq/prod-4167-3
Browse files Browse the repository at this point in the history
  • Loading branch information
hamza-56 authored Sep 16, 2024
2 parents c06627c + 537fcd4 commit 0bd4300
Show file tree
Hide file tree
Showing 7 changed files with 148 additions and 30 deletions.
9 changes: 6 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jobs:
continue-on-error: ${{ matrix.status == 'ignored' }}
- name: Upload coverage
if: matrix.db-version == 'mysql80'
uses: actions/upload-artifact@v2
uses: actions/upload-artifact@v4
with:
name: coverage${{ matrix.pytest-split-group }}
path: .coverage
Expand All @@ -60,10 +60,13 @@ jobs:
PYTHON_VERSION: 3.12
- name: Download all artifacts
# Downloads coverage1, coverage2, etc.
uses: actions/download-artifact@v2
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,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,
Expand All @@ -53,32 +59,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):
"""
Expand Down Expand Up @@ -198,7 +261,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"
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,43 @@ def test_command_with_marketable_flag(self, mock_get_translations):
[{'code': 'es', 'label': 'Spanish'}]
)

def test_command_with_marketable_and_active_flag(self, mock_get_translations):
"""Test the command with the marketable and active flag filtering both marketable and active course runs."""
mock_get_translations.return_value = {
**self.TRANSLATION_DATA,
'available_translation_languages': [{'code': 'fr', 'label': 'French'}]
}

non_active_non_marketable_course_run = CourseRunFactory(
end=now() - datetime.timedelta(days=10), translation_languages=[])
active_non_marketable_course_run = CourseRunFactory(end=now() + datetime.timedelta(days=10))

verified_and_audit_type = CourseRunType.objects.get(slug='verified-audit')
verified_and_audit_type.is_marketable = True
verified_and_audit_type.save()

marketable_non_active_course_run = CourseRunFactory(
status='published',
slug='test-course-run',
type=verified_and_audit_type,
end=now() - datetime.timedelta(days=10), translation_languages=[]
)
seat = SeatFactory(course_run=marketable_non_active_course_run)
marketable_non_active_course_run.seats.add(seat)

call_command('update_course_ai_translations', partner=self.partner.name, marketable=True, active=True)

marketable_non_active_course_run.refresh_from_db()
self.assertListEqual(
marketable_non_active_course_run.translation_languages,
[{'code': 'fr', 'label': 'French'}]
)
self.assertListEqual(
active_non_marketable_course_run.translation_languages,
[{'code': 'fr', 'label': 'French'}]
)
self.assertListEqual(non_active_non_marketable_course_run.translation_languages, [])

def test_command_no_partner(self, _):
"""Test the command raises an error if no valid partner is found."""
with self.assertRaises(CommandError):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ def handle(self, *args, **options):

course_runs = CourseRun.objects.all()

if options['active']:
if options['active'] and options['marketable']:
course_runs = course_runs.marketable().union(course_runs.active())
elif options['active']:
course_runs = course_runs.active()

if options['marketable']:
elif options['marketable']:
course_runs = course_runs.marketable()

for course_run in course_runs:
Expand Down
13 changes: 10 additions & 3 deletions course_discovery/apps/course_metadata/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -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) |
Expand Down Expand Up @@ -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)
Expand Down
7 changes: 7 additions & 0 deletions course_discovery/settings/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,10 @@
k.lower(): (v.replace("\\n", "\n") if k.lower() == "private_key" else v)
for (k, v) in GOOGLE_SERVICE_ACCOUNT_CREDENTIALS.items()
}

# IMPORTANT: With this enabled, the server must always be behind a proxy that
# strips the header X_FORWARDED_PROTO from client requests. Otherwise,
# a user can fool our server into thinking it was an https connection.
# See https://docs.djangoproject.com/en/5.1/ref/settings/#secure-proxy-ssl-header
# for other warnings.
SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https')

0 comments on commit 0bd4300

Please sign in to comment.