Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 36 additions & 24 deletions common/djangoapps/student/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,15 +275,14 @@ def reverification_info(statuses):
return reverifications


def get_course_enrollments(user, orgs_to_include, orgs_to_exclude):
def get_course_enrollments(user, org_whitelist, org_blacklist):
"""
Given a user, return a filtered set of his or her course enrollments.

Arguments:
user (User): the user in question.
orgs_to_include (list[str]): If not None, ONLY courses of these orgs will be returned.
orgs_to_exclude (list[str]): If orgs_to_include is not None, this
argument is ignored. Else, courses of this org will be excluded.
org_whitelist (list[str]): If not None, ONLY courses of these orgs will be returned.
org_blacklist (list[str]): Courses of these orgs will be excluded.

Returns:
generator[CourseEnrollment]: a sequence of enrollments to be displayed
Expand All @@ -301,19 +300,44 @@ def get_course_enrollments(user, orgs_to_include, orgs_to_exclude):
)
continue

# Filter out anything that is not attributed to the orgs to include.
if orgs_to_include and course_overview.location.org not in orgs_to_include:
# Filter out anything that is not in the whitelist.
if org_whitelist and course_overview.location.org not in org_whitelist:
continue

# Conversely, filter out any enrollments with courses attributed to current ORG.
elif course_overview.location.org in orgs_to_exclude:
# Conversely, filter out any enrollments in the blacklist.
elif org_blacklist and course_overview.location.org in org_blacklist:
continue

# Else, include the enrollment.
else:
yield enrollment


def get_org_black_and_whitelist_for_site(user):
"""
Returns the org blacklist and whitelist for the current site.

Returns:
(org_whitelist, org_blacklist): A tuple of lists of orgs that serve as
either a blacklist or a whitelist of orgs for the current site. The
whitelist takes precedence, and the blacklist is used if the
whitelist is None.
"""
# Default blacklist is empty.
org_blacklist = None
# Whitelist the orgs configured for the current site. Each site outside
# of edx.org has a list of orgs associated with its configuration.
org_whitelist = configuration_helpers.get_current_site_orgs()

if not org_whitelist:
# If there is no whitelist, the blacklist will include all orgs that
# have been configured for any other sites. This applies to edx.org,
# where it is easier to blacklist all other orgs.
org_blacklist = configuration_helpers.get_all_orgs()

return (org_whitelist, org_blacklist)


def _cert_info(user, course_overview, cert_status, course_mode): # pylint: disable=unused-argument
"""
Implements the logic for cert_info -- split out for testing.
Expand Down Expand Up @@ -647,21 +671,9 @@ def dashboard(request):
'ACTIVATION_EMAIL_SUPPORT_LINK', settings.ACTIVATION_EMAIL_SUPPORT_LINK
) or settings.SUPPORT_SITE_LINK

# Let's filter out any courses in an "org" that has been declared to be
# in a configuration
org_filter_out_set = configuration_helpers.get_all_orgs()

# Remove current site orgs from the "filter out" list, if applicable.
# We want to filter and only show enrollments for courses within
# the organizations defined in configuration for the current site.
course_org_filter = configuration_helpers.get_current_site_orgs()
if course_org_filter:
org_filter_out_set = org_filter_out_set - set(course_org_filter)

# Build our (course, enrollment) list for the user, but ignore any courses that no
# longer exist (because the course IDs have changed). Still, we don't delete those
# enrollments, because it could have been a data push snafu.
course_enrollments = list(get_course_enrollments(user, course_org_filter, org_filter_out_set))
# get the org whitelist or the org blacklist for the current site
site_org_whitelist, site_org_blacklist = get_org_black_and_whitelist_for_site(user)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@douglashall: I decided to try to fix this code that I had written a while back to not lose it.

I don't know if this rename is a good idea, because the names course_org_filter and org_filter_out_set are used elsewhere.

The options are:

  1. Leave the code as it was.
  2. Go with this new code with renames. (I'm game.)
  3. Do a version of this PR with refactor and updated comments, but without renaming these variables. (I'm sort of game, but I need to move on.)
  4. Change to the new names everywhere. (I'm not game.) :)

Let me know what you think. Thanks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go with #2. I think this is a great refactor which makes this code much clearer for the reader. I wouldn't expect you to have to boil the ocean to make a substantive improvement like this.

course_enrollments = list(get_course_enrollments(user, site_org_whitelist, site_org_blacklist))

# Record how many courses there are so that we can get a better
# understanding of usage patterns on prod.
Expand Down Expand Up @@ -808,7 +820,7 @@ def dashboard(request):
denied_banner = any(item.display for item in reverifications["denied"])

# Populate the Order History for the side-bar.
order_history_list = order_history(user, course_org_filter=course_org_filter, org_filter_out_set=org_filter_out_set)
order_history_list = order_history(user, course_org_filter=site_org_whitelist, org_filter_out_set=site_org_blacklist)

# get list of courses having pre-requisites yet to be completed
courses_having_prerequisites = frozenset(
Expand Down