Skip to content

Commit

Permalink
feat: introduce new marketable field (#4493)
Browse files Browse the repository at this point in the history
* feat: introduce new marketable field

* test: handle all cases in is_marketable_external test

* refactor: code cleanup

* fix: use correct review state

* chore: revert numQueries and try prefetch
  • Loading branch information
hamzawaleed01 authored Dec 11, 2024
1 parent c66c37b commit c49675f
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 12 deletions.
7 changes: 4 additions & 3 deletions course_discovery/apps/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,7 @@ def prefetch_queryset(cls, queryset=None):
# queryset passed in happens to be empty.
queryset = queryset if queryset is not None else CourseRun.objects.all()

return queryset.select_related('course', 'type').prefetch_related(
return queryset.select_related('course', 'type', 'course__type').prefetch_related(
'_official_version',
'course__partner',
'restricted_run',
Expand All @@ -973,7 +973,7 @@ class Meta:
fields = ('key', 'uuid', 'title', 'external_key', 'fixed_price_usd', 'image', 'short_description',
'marketing_url', 'seats', 'start', 'end', 'go_live_date', 'enrollment_start', 'enrollment_end',
'weeks_to_complete', 'pacing_type', 'type', 'restriction_type', 'run_type', 'status', 'is_enrollable',
'is_marketable', 'term', 'availability', 'variant_id')
'is_marketable', 'is_marketable_external', 'term', 'availability', 'variant_id')

def get_marketing_url(self, obj):
include_archived = self.context.get('include_archived')
Expand Down Expand Up @@ -1080,6 +1080,7 @@ def prefetch_queryset(cls, queryset=None):

return queryset.select_related(
'course__level_type',
'course__type',
'course__video__image',
'course__additional_metadata',
'language',
Expand Down Expand Up @@ -1167,7 +1168,7 @@ class CourseRunWithProgramsSerializer(CourseRunSerializer):
def prefetch_queryset(cls, queryset=None):
queryset = super().prefetch_queryset(queryset=queryset)

return queryset.select_related('course').prefetch_related(
return queryset.select_related('course', 'course__type').prefetch_related(
Prefetch('course__programs', queryset=(
Program.objects.select_related('type', 'partner').prefetch_related('excluded_course_runs', 'courses')
))
Expand Down
87 changes: 78 additions & 9 deletions course_discovery/apps/api/tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
from course_discovery.apps.core.tests.mixins import ElasticsearchTestMixin, LMSAPIClientMixin
from course_discovery.apps.core.utils import serialize_datetime
from course_discovery.apps.course_metadata.choices import CourseRunStatus, ProgramStatus
from course_discovery.apps.course_metadata.models import AbstractLocationRestrictionModel, CourseReview
from course_discovery.apps.course_metadata.models import AbstractLocationRestrictionModel, CourseReview, CourseType
from course_discovery.apps.course_metadata.search_indexes.documents import (
CourseDocument, CourseRunDocument, LearnerPathwayDocument, PersonDocument, ProgramDocument
)
Expand All @@ -55,14 +55,14 @@
from course_discovery.apps.course_metadata.tests.factories import (
AdditionalMetadataFactory, AdditionalPromoAreaFactory, CertificateInfoFactory, CollaboratorFactory,
CorporateEndorsementFactory, CourseEditorFactory, CourseEntitlementFactory, CourseFactory,
CourseLocationRestrictionFactory, CourseRunFactory, CourseSkillsFactory, CurriculumCourseMembershipFactory,
CurriculumFactory, CurriculumProgramMembershipFactory, DegreeAdditionalMetadataFactory, DegreeCostFactory,
DegreeDeadlineFactory, DegreeFactory, EndorsementFactory, ExpectedLearningItemFactory, FactFactory,
IconTextPairingFactory, ImageFactory, JobOutlookItemFactory, OrganizationFactory, PathwayFactory,
PersonAreaOfExpertiseFactory, PersonFactory, PersonSocialNetworkFactory, PositionFactory, PrerequisiteFactory,
ProgramFactory, ProgramLocationRestrictionFactory, ProgramSkillFactory, ProgramSubscriptionFactory,
ProgramSubscriptionPriceFactory, ProgramTypeFactory, RankingFactory, SeatFactory, SeatTypeFactory,
SpecializationFactory, SubjectFactory, TopicFactory, VideoFactory
CourseLocationRestrictionFactory, CourseRunFactory, CourseSkillsFactory, CourseTypeFactory,
CurriculumCourseMembershipFactory, CurriculumFactory, CurriculumProgramMembershipFactory,
DegreeAdditionalMetadataFactory, DegreeCostFactory, DegreeDeadlineFactory, DegreeFactory, EndorsementFactory,
ExpectedLearningItemFactory, FactFactory, IconTextPairingFactory, ImageFactory, JobOutlookItemFactory,
OrganizationFactory, PathwayFactory, PersonAreaOfExpertiseFactory, PersonFactory, PersonSocialNetworkFactory,
PositionFactory, PrerequisiteFactory, ProgramFactory, ProgramLocationRestrictionFactory, ProgramSkillFactory,
ProgramSubscriptionFactory, ProgramSubscriptionPriceFactory, ProgramTypeFactory, RankingFactory, SeatFactory,
SeatTypeFactory, SpecializationFactory, SubjectFactory, TopicFactory, VideoFactory
)
from course_discovery.apps.course_metadata.utils import get_course_run_estimated_hours
from course_discovery.apps.ietf_language_tags.models import LanguageTag
Expand Down Expand Up @@ -636,6 +636,7 @@ def get_expected_data(cls, course_run, request):
'external_key': course_run.external_key,
'is_enrollable': course_run.is_enrollable,
'is_marketable': course_run.is_marketable,
'is_marketable_external': course_run.is_marketable_external,
'availability': course_run.availability,
'variant_id': str(course_run.variant_id),
'fixed_price_usd': str(course_run.fixed_price_usd),
Expand All @@ -647,6 +648,7 @@ def get_expected_data(cls, course_run, request):
}


@ddt.ddt
class MinimalCourseRunSerializerTests(MinimalCourseRunBaseTestSerializer):

def test_data(self):
Expand All @@ -656,6 +658,73 @@ def test_data(self):
expected = self.get_expected_data(course_run, request)
assert serializer.data == expected

@ddt.data(
{
'course_type': CourseType.EXECUTIVE_EDUCATION_2U,
'status': CourseRunStatus.Reviewed,
'is_marketable': False,
'has_future_start_date': True,
'expected_is_marketable_external': True
},
{
'course_type': CourseType.EXECUTIVE_EDUCATION_2U,
'status': CourseRunStatus.InternalReview,
'is_marketable': False,
'has_future_start_date': True,
'expected_is_marketable_external': False
},
{
'course_type': CourseType.EXECUTIVE_EDUCATION_2U,
'status': CourseRunStatus.LegalReview,
'is_marketable': False,
'has_future_start_date': True,
'expected_is_marketable_external': False
},
{
'course_type': CourseType.EXECUTIVE_EDUCATION_2U,
'status': CourseRunStatus.Unpublished,
'is_marketable': False,
'has_future_start_date': True,
'expected_is_marketable_external': False
},
{
'course_type': CourseType.PROFESSIONAL,
'status': CourseRunStatus.InternalReview,
'is_marketable': False,
'has_future_start_date': True,
'expected_is_marketable_external': False
},
{
'course_type': CourseType.VERIFIED_AUDIT,
'status': CourseRunStatus.Published,
'is_marketable': True,
'has_future_start_date': False,
'expected_is_marketable_external': True
},
)
@ddt.unpack
def test_is_marketable_external(
self, course_type, status, is_marketable,
has_future_start_date, expected_is_marketable_external
):
course_type_instance = CourseTypeFactory(slug=course_type)
current_time = datetime.datetime.now(tz=UTC)
start_date = current_time + datetime.timedelta(
days=10) if has_future_start_date else current_time - datetime.timedelta(days=10)
go_live_date = current_time + datetime.timedelta(days=5) # Assuming go_live_date is 5 days in the future

course_run = CourseRunFactory(
course=CourseFactory(type=course_type_instance),
status=status,
start=start_date,
go_live_date=go_live_date
)
seat = SeatFactory(course_run=course_run, type=SeatTypeFactory.verified())
course_run.seats.set([seat])

assert course_run.is_marketable == is_marketable
assert course_run.is_marketable_external == expected_is_marketable_external

def test_get_lms_course_url(self):
partner = PartnerFactory()
course_key = 'course-v1:testX+test1.23+2018T1'
Expand Down
21 changes: 21 additions & 0 deletions course_discovery/apps/course_metadata/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2969,6 +2969,27 @@ def is_marketable(self):
is_published = self.status == CourseRunStatus.Published
return is_published and self.seats.exists() and bool(self.marketing_url)

@property
def is_marketable_external(self):
"""
Determines if the course_run is suitable for external marketing.
If already marketable, simply return self.is_marketable.
Else, a course run is deemed suitable for external marketing if it is an
executive education (EE) course, the discovery service status is
'Reviewed', and the course go_live_date & start_date is in the future.
"""
if self.is_marketable:
return self.is_marketable
is_exec_ed_course = self.course.type.slug == CourseType.EXECUTIVE_EDUCATION_2U
if is_exec_ed_course:
current_time = datetime.datetime.now(pytz.UTC)
is_reviewed = self.status == CourseRunStatus.Reviewed
has_future_go_live_date = self.go_live_date and self.go_live_date > current_time
return is_reviewed and self.is_upcoming() and has_future_go_live_date
return False

@property
def is_active(self):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ def get_queryset(self, excluded_restriction_types=None): # pylint: disable=unus
return filter_visible_runs(
super().get_queryset()
.select_related('course')
.select_related('course__type')
.prefetch_related('seats__type')
.prefetch_related('transcript_languages')
)
Expand Down

0 comments on commit c49675f

Please sign in to comment.