From f9351ef830f60b8192598b499304c434feaecb74 Mon Sep 17 00:00:00 2001
From: Carlos de la Guardia <cguardia@yahoo.com>
Date: Mon, 30 Mar 2015 10:29:42 -0600
Subject: [PATCH] MIT: CCX. Code Quality Fixes
Remove duplicated course listings template code on the student dashboard.
---
common/lib/xmodule/xmodule/course_module.py | 2 +-
lms/djangoapps/ccx/migrations/0001_initial.py | 5 +-
lms/djangoapps/ccx/tests/test_views.py | 9 ++-
.../0011_add_model_StudentFieldOverride.py | 6 +-
...erride_course_id_location_student__add_.py | 4 +-
...rride_created__add_field_studentfieldov.py | 4 +-
.../courseware/tests/test_field_overrides.py | 1 +
lms/djangoapps/instructor/tests/test_tools.py | 7 +-
lms/templates/dashboard.html | 64 ++-----------------
9 files changed, 27 insertions(+), 75 deletions(-)
diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index 2bca5d3405a..e71fa5fdb87 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -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.
diff --git a/lms/djangoapps/ccx/migrations/0001_initial.py b/lms/djangoapps/ccx/migrations/0001_initial.py
index 80eb4a2cc4c..6ec2fe67cef 100644
--- a/lms/djangoapps/ccx/migrations/0001_initial.py
+++ b/lms/djangoapps/ccx/migrations/0001_initial.py
@@ -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
@@ -48,7 +49,6 @@ class Migration(SchemaMigration):
# 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'])
@@ -65,7 +65,6 @@ class Migration(SchemaMigration):
# Deleting model 'CcxFieldOverride'
db.delete_table('ccx_ccxfieldoverride')
-
models = {
'auth.group': {
'Meta': {'object_name': 'Group'},
@@ -134,4 +133,4 @@ class Migration(SchemaMigration):
}
}
- complete_apps = ['ccx']
\ No newline at end of file
+ complete_apps = ['ccx']
diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py
index 46a7d28dbb7..d3211833752 100644
--- a/lms/djangoapps/ccx/tests/test_views.py
+++ b/lms/djangoapps/ccx/tests/test_views.py
@@ -120,6 +120,7 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase):
"""
Undo patches.
"""
+ super(TestCoachDashboard, self).tearDown()
patch.stopall()
def test_not_a_coach(self):
@@ -419,11 +420,13 @@ class TestCoachDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase):
)
-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
@@ -492,7 +495,7 @@ class TestCCXGrades(ModuleStoreTestCase, LoginEnrollmentTestCase):
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():
"""
diff --git a/lms/djangoapps/courseware/migrations/0011_add_model_StudentFieldOverride.py b/lms/djangoapps/courseware/migrations/0011_add_model_StudentFieldOverride.py
index d8f5caadd62..22bc4ae6896 100644
--- a/lms/djangoapps/courseware/migrations/0011_add_model_StudentFieldOverride.py
+++ b/lms/djangoapps/courseware/migrations/0011_add_model_StudentFieldOverride.py
@@ -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
@@ -19,12 +21,10 @@ class Migration(SchemaMigration):
))
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'},
@@ -142,4 +142,4 @@ class Migration(SchemaMigration):
}
}
- complete_apps = ['courseware']
\ No newline at end of file
+ complete_apps = ['courseware']
diff --git a/lms/djangoapps/courseware/migrations/0012_auto__del_unique_studentfieldoverride_course_id_location_student__add_.py b/lms/djangoapps/courseware/migrations/0012_auto__del_unique_studentfieldoverride_course_id_location_student__add_.py
index d79f119a247..38091383734 100644
--- a/lms/djangoapps/courseware/migrations/0012_auto__del_unique_studentfieldoverride_course_id_location_student__add_.py
+++ b/lms/djangoapps/courseware/migrations/0012_auto__del_unique_studentfieldoverride_course_id_location_student__add_.py
@@ -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
@@ -12,12 +14,10 @@ class Migration(SchemaMigration):
# 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'},
diff --git a/lms/djangoapps/courseware/migrations/0013_auto__add_field_studentfieldoverride_created__add_field_studentfieldov.py b/lms/djangoapps/courseware/migrations/0013_auto__add_field_studentfieldoverride_created__add_field_studentfieldov.py
index bd672261376..28b82c84252 100644
--- a/lms/djangoapps/courseware/migrations/0013_auto__add_field_studentfieldoverride_created__add_field_studentfieldov.py
+++ b/lms/djangoapps/courseware/migrations/0013_auto__add_field_studentfieldoverride_created__add_field_studentfieldov.py
@@ -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
@@ -21,7 +23,6 @@ class Migration(SchemaMigration):
# 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')
@@ -32,7 +33,6 @@ class Migration(SchemaMigration):
# 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'},
diff --git a/lms/djangoapps/courseware/tests/test_field_overrides.py b/lms/djangoapps/courseware/tests/test_field_overrides.py
index 76cd636c917..1504be6d480 100644
--- a/lms/djangoapps/courseware/tests/test_field_overrides.py
+++ b/lms/djangoapps/courseware/tests/test_field_overrides.py
@@ -30,6 +30,7 @@ class OverrideFieldDataTests(TestCase):
OverrideFieldData.provider_classes = None
def tearDown(self):
+ super(OverrideFieldDataTests, self).tearDown()
OverrideFieldData.provider_classes = None
def make_one(self):
diff --git a/lms/djangoapps/instructor/tests/test_tools.py b/lms/djangoapps/instructor/tests/test_tools.py
index 7be040a6744..17a08c72eda 100644
--- a/lms/djangoapps/instructor/tests/test_tools.py
+++ b/lms/djangoapps/instructor/tests/test_tools.py
@@ -10,9 +10,8 @@ import unittest
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
@@ -222,6 +221,7 @@ class TestSetDueDateExtension(ModuleStoreTestCase):
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):
@@ -280,7 +280,6 @@ class TestDataDumps(ModuleStoreTestCase):
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,
diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html
index 28a743f0ff2..30fd0768165 100644
--- a/lms/templates/dashboard.html
+++ b/lms/templates/dashboard.html
@@ -94,6 +94,13 @@
<% 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">
@@ -222,63 +229,6 @@
</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">
--
GitLab