Skip to content
Merged
Show file tree
Hide file tree
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
4 changes: 1 addition & 3 deletions lms/djangoapps/ccx/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1156,9 +1156,7 @@ def setUp(self):

def check_ccx_tab(self, course, user):
"""Helper function for verifying the ccx tab."""
request = RequestFactory().request()
request.user = user
all_tabs = get_course_tab_list(request, course)
all_tabs = get_course_tab_list(user, course)
return any(tab.type == 'ccx_coach' for tab in all_tabs)

@ddt.data(
Expand Down
4 changes: 3 additions & 1 deletion lms/djangoapps/course_api/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,13 @@ def course_detail(request, username, course_key):
`CourseOverview` object representing the requested course
"""
user = get_effective_user(request.user, username)
return get_course_overview_with_access(
overview = get_course_overview_with_access(
user,
get_permission_for_course_about(),
course_key,
)
overview.effective_user = user
return overview


def _filter_by_role(course_queryset, user, roles):
Expand Down
1 change: 1 addition & 0 deletions lms/djangoapps/course_api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from django.core.exceptions import ValidationError
from edx_django_utils.monitoring import set_custom_metric

from edx_rest_framework_extensions.paginators import NamespacedPageNumberPagination
from rest_framework.generics import ListAPIView, RetrieveAPIView
from rest_framework.throttling import UserRateThrottle
Expand Down
3 changes: 1 addition & 2 deletions lms/djangoapps/course_wiki/tests/test_tab.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ def setUp(self):
def get_wiki_tab(self, user, course):
"""Returns true if the "Wiki" tab is shown."""
request = RequestFactory().request()
request.user = user
all_tabs = get_course_tab_list(request, course)
all_tabs = get_course_tab_list(user, course)
wiki_tabs = [tab for tab in all_tabs if tab.name == 'Wiki']
return wiki_tabs[0] if len(wiki_tabs) == 1 else None

Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/courseware/entrance_exams.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ def course_has_entrance_exam(course):
"""
if not is_entrance_exams_enabled():
return False
if not course.entrance_exam_enabled:
entrance_exam_enabled = getattr(course, 'entrance_exam_enabled', None)
if not entrance_exam_enabled:
return False
if not course.entrance_exam_id:
return False
Expand Down
3 changes: 1 addition & 2 deletions lms/djangoapps/courseware/tabs.py
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,10 @@ def to_json(self):
raise NotImplementedError('SingleTextbookTab should not be serialized.')


def get_course_tab_list(request, course):
def get_course_tab_list(user, course):
"""
Retrieves the course tab list from xmodule.tabs and manipulates the set as necessary
"""
user = request.user
xmodule_tab_list = CourseTabList.iterate_displayable(course, user=user)

# Now that we've loaded the tabs for this course, perform the Entrance Exam work.
Expand Down
23 changes: 8 additions & 15 deletions lms/djangoapps/courseware/tests/test_tabs.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,6 @@ def test_get_course_tabs_list_entrance_exam_enabled(self):
'description': 'Testing Courseware Tabs'
}
self.user.is_staff = False
request = get_mock_request(self.user)
self.course.entrance_exam_enabled = True
self.course.entrance_exam_id = six.text_type(entrance_exam.location)
milestone = add_milestone(milestone)
Expand All @@ -400,7 +399,7 @@ def test_get_course_tabs_list_entrance_exam_enabled(self):
self.relationship_types['FULFILLS'],
milestone
)
course_tab_list = get_course_tab_list(request, self.course)
course_tab_list = get_course_tab_list(self.user, self.course)
self.assertEqual(len(course_tab_list), 1)
self.assertEqual(course_tab_list[0]['tab_id'], 'courseware')
self.assertEqual(course_tab_list[0]['name'], 'Entrance Exam')
Expand All @@ -425,8 +424,7 @@ def test_get_course_tabs_list_skipped_entrance_exam(self):
# log in again as student
self.client.logout()
self.login(self.email, self.password)
request = get_mock_request(self.user)
course_tab_list = get_course_tab_list(request, self.course)
course_tab_list = get_course_tab_list(self.user, self.course)
self.assertEqual(len(course_tab_list), 4)

def test_course_tabs_list_for_staff_members(self):
Expand All @@ -438,8 +436,7 @@ def test_course_tabs_list_for_staff_members(self):
self.client.logout()
staff_user = StaffFactory(course_key=self.course.id)
self.client.login(username=staff_user.username, password='test')
request = get_mock_request(staff_user)
course_tab_list = get_course_tab_list(request, self.course)
course_tab_list = get_course_tab_list(staff_user, self.course)
self.assertEqual(len(course_tab_list), 4)


Expand Down Expand Up @@ -480,8 +477,7 @@ def test_pdf_textbook_tabs(self):
"""
type_to_reverse_name = {'textbook': 'book', 'pdftextbook': 'pdf_book', 'htmltextbook': 'html_book'}
self.addCleanup(set_current_request, None)
request = get_mock_request(self.user)
course_tab_list = get_course_tab_list(request, self.course)
course_tab_list = get_course_tab_list(self.user, self.course)
num_of_textbooks_found = 0
for tab in course_tab_list:
# Verify links of all textbook type tabs.
Expand Down Expand Up @@ -706,8 +702,7 @@ def test_course_tabs_staff_only(self):

user = self.create_mock_user(is_staff=False, is_enrolled=True)
self.addCleanup(set_current_request, None)
request = get_mock_request(user)
course_tab_list = get_course_tab_list(request, self.course)
course_tab_list = get_course_tab_list(user, self.course)
name_list = [x.name for x in course_tab_list]
self.assertIn('Static Tab Free', name_list)
self.assertNotIn('Static Tab Instructors Only', name_list)
Expand All @@ -716,8 +711,7 @@ def test_course_tabs_staff_only(self):
self.client.logout()
staff_user = StaffFactory(course_key=self.course.id)
self.client.login(username=staff_user.username, password='test')
request = get_mock_request(staff_user)
course_tab_list_staff = get_course_tab_list(request, self.course)
course_tab_list_staff = get_course_tab_list(staff_user, self.course)
name_list_staff = [x.name for x in course_tab_list_staff]
self.assertIn('Static Tab Free', name_list_staff)
self.assertIn('Static Tab Instructors Only', name_list_staff)
Expand Down Expand Up @@ -775,18 +769,17 @@ class CourseInfoTabTestCase(TabTestCase):
def setUp(self):
self.user = self.create_mock_user()
self.addCleanup(set_current_request, None)
self.request = get_mock_request(self.user)

@override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=False)
def test_default_tab(self):
# Verify that the course info tab is the first tab
tabs = get_course_tab_list(self.request, self.course)
tabs = get_course_tab_list(self.user, self.course)
self.assertEqual(tabs[0].type, 'course_info')

@override_waffle_flag(UNIFIED_COURSE_TAB_FLAG, active=True)
def test_default_tab_for_new_course_experience(self):
# Verify that the unified course experience hides the course info tab
tabs = get_course_tab_list(self.request, self.course)
tabs = get_course_tab_list(self.user, self.course)
self.assertEqual(tabs[0].type, 'courseware')

# TODO: LEARNER-611 - remove once course_info is removed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1248,8 +1248,7 @@ def setUp(self):
def discussion_tab_present(self, user):
""" Returns true if the user has access to the discussion tab. """
request = RequestFactory().request()
request.user = user
all_tabs = get_course_tab_list(request, self.course)
all_tabs = get_course_tab_list(user, self.course)
return any(tab.type == 'discussion' for tab in all_tabs)

def test_tab_access(self):
Expand Down
4 changes: 1 addition & 3 deletions lms/djangoapps/edxnotes/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1003,9 +1003,7 @@ def test_edxnotes_tab(self):
"""
def has_notes_tab(user, course):
"""Returns true if the "Notes" tab is shown."""
request = RequestFactory().request()
request.user = user
tabs = get_course_tab_list(request, course)
tabs = get_course_tab_list(user, course)
return len([tab for tab in tabs if tab.type == 'edxnotes']) == 1

self.assertFalse(has_notes_tab(self.user, self.course))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ def test_instructor_tab(self):
"""
def has_instructor_tab(user, course):
"""Returns true if the "Instructor" tab is shown."""
request = RequestFactory().request()
request.user = user
tabs = get_course_tab_list(request, course)
tabs = get_course_tab_list(user, course)
return len([tab for tab in tabs if tab.name == 'Instructor']) == 1

self.assertTrue(has_instructor_tab(self.instructor, self.course))
Expand Down
2 changes: 1 addition & 1 deletion lms/templates/courseware/course_navigation.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

% if disable_tabs is UNDEFINED or not disable_tabs:
<%
tab_list = get_course_tab_list(request, course)
tab_list = get_course_tab_list(request.user, course)
%>
% if uses_bootstrap:
<nav class="navbar course-tabs pb-0 navbar-expand" aria-label="${_('Course')}">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.27 on 2020-01-16 17:23
from __future__ import unicode_literals

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('course_overviews', '0018_add_start_end_in_CourseOverview'),
]

operations = [
migrations.AddField(
model_name='courseoverviewtab',
name='course_staff_only',
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name='courseoverviewtab',
name='name',
field=models.TextField(null=True),
),
migrations.AddField(
model_name='courseoverviewtab',
name='type',
field=models.CharField(max_length=50, null=True),
),
migrations.AlterField(
model_name='courseoverviewtab',
name='course_overview',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='tab_set', to='course_overviews.CourseOverview'),
),
]
77 changes: 73 additions & 4 deletions openedx/core/djangoapps/content/course_overviews/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from django.db.utils import IntegrityError
from django.template import defaultfilters
from django.utils.encoding import python_2_unicode_compatible
from django.utils.functional import cached_property
from model_utils.models import TimeStampedModel
from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField
from six import text_type # pylint: disable=ungrouped-imports
Expand All @@ -31,6 +32,8 @@
from xmodule.course_module import DEFAULT_START_DATE, CourseDescriptor
from xmodule.error_module import ErrorDescriptor
from xmodule.modulestore.django import modulestore
from xmodule.tabs import CourseTab


log = logging.getLogger(__name__)

Expand All @@ -54,7 +57,7 @@ class Meta(object):
app_label = 'course_overviews'

# IMPORTANT: Bump this whenever you modify this model and/or add a migration.
VERSION = 7
VERSION = 8

# Cache entry versioning.
version = IntegerField()
Expand Down Expand Up @@ -247,7 +250,12 @@ def load_from_module_store(cls, course_id):
# Remove and recreate all the course tabs
CourseOverviewTab.objects.filter(course_overview=course_overview).delete()
CourseOverviewTab.objects.bulk_create([
CourseOverviewTab(tab_id=tab.tab_id, course_overview=course_overview)
CourseOverviewTab(
tab_id=tab.tab_id,
type=tab.type,
name=tab.name,
course_staff_only=tab.course_staff_only,
course_overview=course_overview)
for tab in course.tabs
])
# Remove and recreate course images
Expand Down Expand Up @@ -629,13 +637,25 @@ def is_discussion_tab_enabled(self):
"""
Returns True if course has discussion tab and is enabled
"""
tabs = self.tabs.all()
tabs = self.tab_set.all()
# creates circular import; hence explicitly referenced is_discussion_enabled
for tab in tabs:
if tab.tab_id == "discussion" and django_comment_client.utils.is_discussion_enabled(self.id):
return True
return False

@property
def tabs(self):
"""
Returns an iterator of CourseTabs.
"""
for tab_dict in self.tab_set.all().values():
tab = CourseTab.from_json(tab_dict)
if tab is None:
log.warning("Can't instantiate CourseTab from %r", tab_dict)
else:
yield tab

@property
def image_urls(self):
"""
Expand Down Expand Up @@ -733,6 +753,49 @@ def _apply_cdn_to_url(self, url, base_url):

return urlunparse(('', base_url, path, params, query, fragment))

@cached_property
def _original_course(self):

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.

The intention is to get all of the required information from the CourseOverview, without having to hit the modulestore. But some of the machinery for instantiating tabs checks other attributes on the course. We could decide whether to add these to the overview, or proxy to the course object.

"""
Returns the course from the modulestore.
"""
log.warning('Falling back on modulestore to get course information for %s', self.id)
return modulestore().get_course(self.id)

@property
def allow_public_wiki_access(self):
"""
TODO: move this to the model.
"""
return self._original_course.allow_public_wiki_access

@property
def textbooks(self):
"""
TODO: move this to the model.
"""
return self._original_course.textbooks

@property
def hide_progress_tab(self):
"""
TODO: move this to the model.
"""
return self._original_course.hide_progress_tab

@property
def edxnotes(self):
"""
TODO: move this to the model.
"""
return self._original_course.edxnotes

@property
def enable_ccx(self):
"""
TODO: move this to the model.
"""
return self._original_course.enable_ccx

def __str__(self):
"""Represent ourselves with the course key."""
return six.text_type(self.id)
Expand All @@ -745,7 +808,13 @@ class CourseOverviewTab(models.Model):
.. no_pii:
"""
tab_id = models.CharField(max_length=50)
course_overview = models.ForeignKey(CourseOverview, db_index=True, related_name="tabs", on_delete=models.CASCADE)
course_overview = models.ForeignKey(CourseOverview, db_index=True, related_name="tab_set", on_delete=models.CASCADE)
type = models.CharField(max_length=50, null=True)
name = models.TextField(null=True)
course_staff_only = models.BooleanField(default=False)

def __str__(self):
return self.tab_id


@python_2_unicode_compatible
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ def get_seconds_since_epoch(date_time):

# test tabs for both cached miss and cached hit courses
for course_overview in [course_overview_cache_miss, course_overview_cache_hit]:
course_overview_tabs = course_overview.tabs.all()
course_overview_tabs = course_overview.tab_set.all()
course_resp_tabs = {tab.tab_id for tab in course_overview_tabs}
self.assertEqual(self.COURSE_OVERVIEW_TABS, course_resp_tabs)

Expand Down Expand Up @@ -1089,7 +1089,7 @@ def test_tabs_deletion_rollback_on_integrity_error(self, modulestore_type):
"""
course = CourseFactory.create(default_store=modulestore_type)
course_overview = CourseOverview.get_from_id(course.id)
expected_tabs = {tab.tab_id for tab in course_overview.tabs.all()}
expected_tabs = {tab.tab_id for tab in course_overview.tab_set.all()}

with mock.patch(
'openedx.core.djangoapps.content.course_overviews.models.CourseOverviewTab.objects.bulk_create'
Expand All @@ -1105,6 +1105,6 @@ def test_tabs_deletion_rollback_on_integrity_error(self, modulestore_type):

# Asserts that the tabs deletion is properly rolled back to a save point and
# the course overview is not updated.
actual_tabs = {tab.tab_id for tab in course_overview.tabs.all()}
actual_tabs = {tab.tab_id for tab in course_overview.tab_set.all()}
self.assertEqual(actual_tabs, expected_tabs)
self.assertNotEqual(course_overview.display_name, course.display_name)
Empty file.
Loading