-
Notifications
You must be signed in to change notification settings - Fork 172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
initial commit #4349
initial commit #4349
Conversation
@@ -81,6 +82,7 @@ def get_queryset(self): | |||
multiple: false | |||
""" | |||
q = self.request.query_params.get('q') | |||
restriction_list = self.request.query_params.get('restriction_list', '').split(',') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we would need this on courses API as well.
if course.advertised_course_run(restriction_list=[CourseRunRestrictionType.CustomB2C.value]) and course.advertised_course_run(restriction_list=[CourseRunRestrictionType.CustomB2C.value]).language: | ||
return course.advertised_course_run(restriction_list=[CourseRunRestrictionType.CustomB2C.value]).language |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
advertised_run = course.advertised_course_run(restriction_list=[CourseRunRestrictionType.CustomB2C.value])
if advertised_run and advertised_run.language:
return advertised_run.language
@@ -103,6 +107,8 @@ def should_include_course_run(course_run, params, exclude_expired): | |||
matches_parameter = True | |||
if matches_parameter: | |||
break | |||
if hasattr(course_run, 'restricted_run') and course_run.restricted_run.restriction_type in forbidden: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
won't we need this type of filtering on restricted_runs in get_languages
and get_seat_types
method.
@@ -1995,6 +2011,10 @@ def get_organization_logo_override_url(self, obj): | |||
return logo_image_override.url | |||
return None | |||
|
|||
def get_course_run_statuses(self, program): | |||
restriction_list = self.context['request'].query_params.get('restriction_list', '').split(',') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we are using restriction_list = self.context['request'].query_params.get('restriction_list', '').split(',')
in many places in this file. Maybe add a unified util/method to avoid repeating this?
@@ -95,10 +98,14 @@ def get_queryset(self): | |||
queryset = CourseEditor.editable_course_runs(self.request.user, queryset) | |||
else: | |||
queryset = self.queryset | |||
|
|||
|
|||
if self.request.method == 'GET': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q: can we use edit_mode boolean here?
restriction_list = query_params.get('restriction_list', '').split(',') | ||
forbidden = list(set(CourseRunRestrictionType.values) - set(restriction_list)) | ||
queryset = queryset.exclude('terms', restriction_type=forbidden) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: what's the difference in doing this here vs in
faceted_search_fields = { |
@@ -253,15 +255,15 @@ def active_languages(self): | |||
|
|||
@property | |||
def active_run_key(self): | |||
return getattr(self.advertised_course_run, 'key', None) | |||
return getattr(self.advertised_course_run(restriction_list=[CourseRunRestrictionType.CustomB2C.value]), 'key', None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: if advertised_course_run
is method in the same file, we can add B2C as a default value in the method instead of repeating across various lines.
@@ -2587,6 +2585,17 @@ def search(cls, query): | |||
dsl_query = ESDSLQ('query_string', query=query, analyze_wildcard=True) | |||
return queryset.query(dsl_query) | |||
|
|||
@classmethod | |||
def get_exposed_runs(cls, runs, restriction_list=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: the method name can be improved. exposed_runs does not make sense.
@@ -125,6 +126,10 @@ def prepare_seat_types(self, obj): | |||
def prepare_skill_names(self, obj): | |||
return get_product_skill_names(obj.course.key, ProductTypes.Course) | |||
|
|||
def prepare_restriction_type(self, obj): | |||
if hasattr(obj, "restricted_run"): | |||
return obj.restricted_run.restriction_type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should it be restriction_type.value? Or is that catered during indexing?
375d5b9
to
43fecbb
Compare
Closing in favor of 4356 |
PROD-4006
Expose restricted runs on non-ES Apis only when a query param asks for them