Skip to content

Commit

Permalink
MIT: CCX. Code Quality Fixes
Browse files Browse the repository at this point in the history
Remove duplicated course listings template code on the student dashboard.
  • Loading branch information
cguardia authored and cewing committed Apr 11, 2015
1 parent 8ba7442 commit f9351ef
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 75 deletions.
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/course_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ class CourseFields(object):
)


class CourseModule(CourseFields, SequenceModule):
class CourseModule(CourseFields, SequenceModule): # pylint: disable=abstract-method
"""
The CourseDescriptor needs its module_class to be a SequenceModule, but some code that
expects a CourseDescriptor to have all its fields can fail if it gets a SequenceModule instead.
Expand Down
5 changes: 2 additions & 3 deletions lms/djangoapps/ccx/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# -*- coding: utf-8 -*-
# pylint: disable=invalid-name, missing-docstring, unused-argument, unused-import, line-too-long
import datetime
from south.db import db
from south.v2 import SchemaMigration
Expand Down Expand Up @@ -48,7 +49,6 @@ def forwards(self, orm):
# Adding unique constraint on 'CcxFieldOverride', fields ['ccx', 'location', 'field']
db.create_unique('ccx_ccxfieldoverride', ['ccx_id', 'location', 'field'])


def backwards(self, orm):
# Removing unique constraint on 'CcxFieldOverride', fields ['ccx', 'location', 'field']
db.delete_unique('ccx_ccxfieldoverride', ['ccx_id', 'location', 'field'])
Expand All @@ -65,7 +65,6 @@ def backwards(self, orm):
# Deleting model 'CcxFieldOverride'
db.delete_table('ccx_ccxfieldoverride')


models = {
'auth.group': {
'Meta': {'object_name': 'Group'},
Expand Down Expand Up @@ -134,4 +133,4 @@ def backwards(self, orm):
}
}

complete_apps = ['ccx']
complete_apps = ['ccx']
9 changes: 6 additions & 3 deletions lms/djangoapps/ccx/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ def tearDown(self):
"""
Undo patches.
"""
super(TestCoachDashboard, self).tearDown()
patch.stopall()

def test_not_a_coach(self):
Expand Down Expand Up @@ -419,11 +420,13 @@ def test_manage_remove_single_student(self):
)


original_get_children = XModuleMixin.get_children
GET_CHILDREN = XModuleMixin.get_children


def patched_get_children(self, usage_key_filter=None): # pylint: disable=missing-docstring
def iter_children(): # pylint: disable=missing-docstring
print self.__dict__
for child in original_get_children(self, usage_key_filter=usage_key_filter):
for child in GET_CHILDREN(self, usage_key_filter=usage_key_filter):
child._field_data_cache = {} # pylint: disable=protected-access
if not child.visible_to_staff_only:
yield child
Expand Down Expand Up @@ -492,7 +495,7 @@ def setUp(self):
for block in iter_blocks(course):
block._field_data = OverrideFieldData.wrap( # pylint: disable=protected-access
coach, block._field_data) # pylint: disable=protected-access
block._field_data_cache = {'tabs':[],'discussion_topics':[]} # pylint: disable=protected-access
block._field_data_cache = {'tabs': [], 'discussion_topics': []} # pylint: disable=protected-access

def cleanup_provider_classes():
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# -*- coding: utf-8 -*-
# pylint: disable=invalid-name, missing-docstring, unused-argument, unused-import, line-too-long

import datetime
from south.db import db
from south.v2 import SchemaMigration
Expand All @@ -19,12 +21,10 @@ def forwards(self, orm):
))
db.send_create_signal('courseware', ['StudentFieldOverride'])


def backwards(self, orm):
# Deleting model 'StudentFieldOverride'
db.delete_table('courseware_studentfieldoverride')


models = {
'auth.group': {
'Meta': {'object_name': 'Group'},
Expand Down Expand Up @@ -142,4 +142,4 @@ def backwards(self, orm):
}
}

complete_apps = ['courseware']
complete_apps = ['courseware']
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# -*- coding: utf-8 -*-
# pylint: disable=invalid-name, missing-docstring, unused-argument, unused-import, line-too-long

import datetime
from south.db import db
from south.v2 import SchemaMigration
Expand All @@ -12,12 +14,10 @@ def forwards(self, orm):
# Adding unique constraint on 'StudentFieldOverride', fields ['course_id', 'field', 'location', 'student']
db.create_unique('courseware_studentfieldoverride', ['course_id', 'field', 'location', 'student_id'])


def backwards(self, orm):
# Removing unique constraint on 'StudentFieldOverride', fields ['course_id', 'field', 'location', 'student']
db.delete_unique('courseware_studentfieldoverride', ['course_id', 'field', 'location', 'student_id'])


models = {
'auth.group': {
'Meta': {'object_name': 'Group'},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# -*- coding: utf-8 -*-
# pylint: disable=invalid-name, missing-docstring, unused-argument, unused-import, line-too-long

import datetime
from south.db import db
from south.v2 import SchemaMigration
Expand All @@ -21,7 +23,6 @@ def forwards(self, orm):
# Adding index on 'StudentFieldOverride', fields ['course_id', 'location', 'student']
db.create_index('courseware_studentfieldoverride', ['course_id', 'location', 'student_id'])


def backwards(self, orm):
# Deleting field 'StudentFieldOverride.created'
db.delete_column('courseware_studentfieldoverride', 'created')
Expand All @@ -32,7 +33,6 @@ def backwards(self, orm):
# Removing index on 'StudentFieldOverride', fields ['course_id', 'location', 'student']
db.delete_index('courseware_studentfieldoverride', ['course_id', 'location', 'student_id'])


models = {
'auth.group': {
'Meta': {'object_name': 'Group'},
Expand Down
1 change: 1 addition & 0 deletions lms/djangoapps/courseware/tests/test_field_overrides.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def setUp(self):
OverrideFieldData.provider_classes = None

def tearDown(self):
super(OverrideFieldDataTests, self).tearDown()
OverrideFieldData.provider_classes = None

def make_one(self):
Expand Down
7 changes: 3 additions & 4 deletions lms/djangoapps/instructor/tests/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
from django.utils.timezone import utc
from django.test.utils import override_settings

from courseware.models import StudentModule
from courseware.field_overrides import OverrideFieldData
from student.tests.factories import UserFactory
from courseware.field_overrides import OverrideFieldData # pylint: disable=import-error
from student.tests.factories import UserFactory # pylint: disable=import-error
from xmodule.fields import Date
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
Expand Down Expand Up @@ -222,6 +221,7 @@ def setUp(self):
user, block._field_data) # pylint: disable=protected-access

def tearDown(self):
super(TestSetDueDateExtension, self).tearDown()
OverrideFieldData.provider_classes = None

def _clear_field_data_cache(self):
Expand Down Expand Up @@ -280,7 +280,6 @@ def setUp(self):
course = CourseFactory.create()
week1 = ItemFactory.create(due=due, parent=course)
week2 = ItemFactory.create(due=due, parent=course)
week3 = ItemFactory.create(due=due, parent=course)

homework = ItemFactory.create(
parent=week1,
Expand Down
64 changes: 7 additions & 57 deletions lms/templates/dashboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ <h2 class="header-courses">${_("Current Courses")}</h2>
<% course_requirements = courses_requirements_not_met.get(course.id) %>
<%include file='dashboard/_dashboard_course_listing.html' args="course=course, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, show_refund_option = show_refund_option, is_paid_course = is_paid_course, is_course_blocked = is_course_blocked, verification_status=course_verification_status, course_requirements=course_requirements, dashboard_index=dashboard_index, share_settings=share_settings" />
% endfor

% if settings.FEATURES.get('CUSTOM_COURSES_EDX', False):
% for ccx, membership, course in ccx_membership_triplets:
<%include file='ccx/_dashboard_ccx_listing.html' args="ccx=ccx, membership=membership, course=course" />
% endfor
% endif

</ul>
% else:
<section class="empty-dashboard-message">
Expand Down Expand Up @@ -222,63 +229,6 @@ <h2 class="username-header"><span class="sr">${_("Username")}: </span><span clas
</ul>
</section>
</section>

<section id="my-courses" class="my-courses" role="main" aria-label="Content">
<header>
<h2>${_("Current Courses")}</h2>
</header>

% if len(course_enrollment_pairs) > 0:
<ul class="listing-courses">
% for course, enrollment in course_enrollment_pairs:
<% show_courseware_link = (course.id in show_courseware_links_for) %>
<% cert_status = cert_statuses.get(course.id) %>
<% show_email_settings = (course.id in show_email_settings_for) %>
<% course_mode_info = all_course_modes.get(course.id) %>
<% show_refund_option = (course.id in show_refund_option_for) %>
<% is_paid_course = (course.id in enrolled_courses_either_paid) %>
<% is_course_blocked = (course.id in block_courses) %>
<% course_verification_status = verification_status_by_course.get(course.id, {}) %>
<% course_requirements = courses_requirements_not_met.get(course.id) %>
<%include file='dashboard/_dashboard_course_listing.html' args="course=course, enrollment=enrollment, show_courseware_link=show_courseware_link, cert_status=cert_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, show_refund_option = show_refund_option, is_paid_course = is_paid_course, is_course_blocked = is_course_blocked, verification_status=course_verification_status, course_requirements=course_requirements" />
% endfor

% if settings.FEATURES.get('CUSTOM_COURSES_EDX', False):
% for ccx, membership, course in ccx_membership_triplets:
<%include file='ccx/_dashboard_ccx_listing.html' args="ccx=ccx, membership=membership, course=course" />
% endfor
% endif

</ul>
% else:
<section class="empty-dashboard-message">
% if settings.FEATURES.get('COURSES_ARE_BROWSABLE'):
<p>${_("Looks like you haven't enrolled in any courses yet.")}</p>
<a href="${marketing_link('COURSES')}">
${_("Find courses now!")}
</a>
% else:
<p>${_("Looks like you haven't enrolled in any courses yet.")}</p>
%endif
</section>
% endif

% if staff_access and len(errored_courses) > 0:
<div id="course-errors">
<h2>${_("Course-loading errors")}</h2>

% for course_dir, errors in errored_courses.items():
<h3>${course_dir | h}</h3>
<ul>
% for (msg, err) in errors:
<li>${msg}
<ul><li><pre>${err}</pre></li></ul>
</li>
% endfor
</ul>
% endfor
% endif
</section>
</section>

<section id="email-settings-modal" class="modal" aria-hidden="true">
Expand Down

0 comments on commit f9351ef

Please sign in to comment.