From 50559b64f017d26f03622d8d643553c70875fa9a Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Tue, 15 Nov 2016 11:43:54 -0500 Subject: [PATCH 1/7] [PERF-321] Move heavy JS files for WordCloud out of lms-modules.js --- .../word_cloud/public/js}/d3.layout.cloud.js | 0 .../word_cloud/public/js}/d3.min.js | 0 .../word_cloud/public/js}/word_cloud.js | 0 .../word_cloud/public/js}/word_cloud_main.js | 0 .../lib/xmodule/xmodule/word_cloud_module.py | 19 +++++++++++++------ lms/templates/word_cloud.html | 4 ++++ 6 files changed, 17 insertions(+), 6 deletions(-) rename common/lib/xmodule/xmodule/{js/src/word_cloud => assets/word_cloud/public/js}/d3.layout.cloud.js (100%) rename common/lib/xmodule/xmodule/{js/src/word_cloud => assets/word_cloud/public/js}/d3.min.js (100%) rename common/lib/xmodule/xmodule/{js/src/word_cloud => assets/word_cloud/public/js}/word_cloud.js (100%) rename common/lib/xmodule/xmodule/{js/src/word_cloud => assets/word_cloud/public/js}/word_cloud_main.js (100%) diff --git a/common/lib/xmodule/xmodule/js/src/word_cloud/d3.layout.cloud.js b/common/lib/xmodule/xmodule/assets/word_cloud/public/js/d3.layout.cloud.js similarity index 100% rename from common/lib/xmodule/xmodule/js/src/word_cloud/d3.layout.cloud.js rename to common/lib/xmodule/xmodule/assets/word_cloud/public/js/d3.layout.cloud.js diff --git a/common/lib/xmodule/xmodule/js/src/word_cloud/d3.min.js b/common/lib/xmodule/xmodule/assets/word_cloud/public/js/d3.min.js similarity index 100% rename from common/lib/xmodule/xmodule/js/src/word_cloud/d3.min.js rename to common/lib/xmodule/xmodule/assets/word_cloud/public/js/d3.min.js diff --git a/common/lib/xmodule/xmodule/js/src/word_cloud/word_cloud.js b/common/lib/xmodule/xmodule/assets/word_cloud/public/js/word_cloud.js similarity index 100% rename from common/lib/xmodule/xmodule/js/src/word_cloud/word_cloud.js rename to common/lib/xmodule/xmodule/assets/word_cloud/public/js/word_cloud.js diff --git a/common/lib/xmodule/xmodule/js/src/word_cloud/word_cloud_main.js b/common/lib/xmodule/xmodule/assets/word_cloud/public/js/word_cloud_main.js similarity index 100% rename from common/lib/xmodule/xmodule/js/src/word_cloud/word_cloud_main.js rename to common/lib/xmodule/xmodule/assets/word_cloud/public/js/word_cloud_main.js diff --git a/common/lib/xmodule/xmodule/word_cloud_module.py b/common/lib/xmodule/xmodule/word_cloud_module.py index b2fd468692..5d1fe444c6 100644 --- a/common/lib/xmodule/xmodule/word_cloud_module.py +++ b/common/lib/xmodule/xmodule/word_cloud_module.py @@ -93,10 +93,6 @@ class WordCloudModule(WordCloudFields, XModule): js = { 'js': [ resource_string(__name__, 'js/src/javascript_loader.js'), - resource_string(__name__, 'js/src/word_cloud/d3.min.js'), - resource_string(__name__, 'js/src/word_cloud/d3.layout.cloud.js'), - resource_string(__name__, 'js/src/word_cloud/word_cloud.js'), - resource_string(__name__, 'js/src/word_cloud/word_cloud_main.js'), ], } css = {'scss': [resource_string(__name__, 'css/word_cloud/display.scss')]} @@ -238,7 +234,17 @@ class WordCloudModule(WordCloudFields, XModule): }) def get_html(self): - """Template rendering.""" + """ + Template rendering. + """ + + js_includes = [ + self.runtime.local_resource_url(self, 'public/js/d3.min.js'), + self.runtime.local_resource_url(self, 'public/js/d3.layout.cloud.js'), + self.runtime.local_resource_url(self, 'public/js/word_cloud.js'), + self.runtime.local_resource_url(self, 'public/js/word_cloud_main.js'), + ] + context = { 'ajax_url': self.system.ajax_url, 'display_name': self.display_name, @@ -247,6 +253,7 @@ class WordCloudModule(WordCloudFields, XModule): 'element_id': self.location.html_id(), 'num_inputs': self.num_inputs, 'submitted': self.submitted, + 'js_includes': js_includes, } self.content = self.system.render_template('word_cloud.html', context) return self.content @@ -255,5 +262,5 @@ class WordCloudModule(WordCloudFields, XModule): class WordCloudDescriptor(WordCloudFields, MetadataOnlyEditingDescriptor, EmptyDataRawDescriptor): """Descriptor for WordCloud Xmodule.""" module_class = WordCloudModule - resources_dir = None + resources_dir = 'assets/word_cloud' template_dir_name = 'word_cloud' diff --git a/lms/templates/word_cloud.html b/lms/templates/word_cloud.html index 2b1f5d148f..a6c8a40254 100644 --- a/lms/templates/word_cloud.html +++ b/lms/templates/word_cloud.html @@ -7,6 +7,10 @@ data-ajax-url="${ajax_url}" > + % for js_include in js_includes: + + % endfor + % if display_name:

${display_name}

% endif From 842e9dce56d10e5ab283208bafae081b2e0303d1 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Tue, 15 Nov 2016 13:42:09 -0500 Subject: [PATCH 2/7] test fix - this is so stupid --- .../courseware/tests/test_word_cloud.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py index 18add8df13..69ae03c9c1 100644 --- a/lms/djangoapps/courseware/tests/test_word_cloud.py +++ b/lms/djangoapps/courseware/tests/test_word_cloud.py @@ -14,6 +14,10 @@ class TestWordCloud(BaseTestXmodule): """Integration test for word cloud xmodule.""" CATEGORY = "word_cloud" + def _get_resource_url(self, item): + display_name = self.item_descriptor.display_name.replace(' ', '_') + return "resource/i4x://{}/{}/word_cloud/{}/{}".format(self.course.id.org, self.course.id.course, display_name, item) + def _get_users_state(self): """Return current state for each user: @@ -241,7 +245,17 @@ class TestWordCloud(BaseTestXmodule): ) def test_word_cloud_constructor(self): + self.maxDiff = None + """Make sure that all parameters extracted correctly from xml""" + + js_includes = [ + self._get_resource_url('public/js/d3.min.js'), + self._get_resource_url('public/js/d3.layout.cloud.js'), + self._get_resource_url('public/js/word_cloud.js'), + self._get_resource_url('public/js/word_cloud_main.js'), + ] + fragment = self.runtime.render(self.item_descriptor, STUDENT_VIEW) expected_context = { 'ajax_url': self.item_descriptor.xmodule_runtime.ajax_url, @@ -250,6 +264,9 @@ class TestWordCloud(BaseTestXmodule): 'element_class': self.item_descriptor.location.category, 'element_id': self.item_descriptor.location.html_id(), 'num_inputs': 5, # default value - 'submitted': False, # default value + 'submitted': False, # default value, + 'js_includes': js_includes, } + + self.assertEqual(fragment.content, self.runtime.render_template('word_cloud.html', expected_context)) From f136911eaa85ccdca5595998715b8e99424915fe Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Tue, 15 Nov 2016 14:19:41 -0500 Subject: [PATCH 3/7] quality fix --- lms/djangoapps/courseware/tests/test_word_cloud.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py index 69ae03c9c1..084f255e4a 100644 --- a/lms/djangoapps/courseware/tests/test_word_cloud.py +++ b/lms/djangoapps/courseware/tests/test_word_cloud.py @@ -245,8 +245,6 @@ class TestWordCloud(BaseTestXmodule): ) def test_word_cloud_constructor(self): - self.maxDiff = None - """Make sure that all parameters extracted correctly from xml""" js_includes = [ @@ -268,5 +266,4 @@ class TestWordCloud(BaseTestXmodule): 'js_includes': js_includes, } - self.assertEqual(fragment.content, self.runtime.render_template('word_cloud.html', expected_context)) From da664ce7f881c140fcd7785192a7c5893cd59761 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Tue, 15 Nov 2016 15:03:29 -0500 Subject: [PATCH 4/7] quality fix --- lms/djangoapps/courseware/tests/test_word_cloud.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py index 084f255e4a..e7221e1543 100644 --- a/lms/djangoapps/courseware/tests/test_word_cloud.py +++ b/lms/djangoapps/courseware/tests/test_word_cloud.py @@ -15,8 +15,13 @@ class TestWordCloud(BaseTestXmodule): CATEGORY = "word_cloud" def _get_resource_url(self, item): + """ + Creates a resource URL for a given asset that is compatible with this old XModule testing stuff. + """ display_name = self.item_descriptor.display_name.replace(' ', '_') - return "resource/i4x://{}/{}/word_cloud/{}/{}".format(self.course.id.org, self.course.id.course, display_name, item) + return "resource/i4x://{}/{}/word_cloud/{}/{}".format( + self.course.id.org, self.course.id.course, display_name, item + ) def _get_users_state(self): """Return current state for each user: From ff619d9bd451882b264a7b9e18ae4e89ee1917c6 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Wed, 16 Nov 2016 13:10:52 -0500 Subject: [PATCH 5/7] Switch to dedicated student/author views. --- .../lib/xmodule/xmodule/word_cloud_module.py | 27 ++++++++++--------- lms/templates/word_cloud.html | 5 ---- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/common/lib/xmodule/xmodule/word_cloud_module.py b/common/lib/xmodule/xmodule/word_cloud_module.py index 5d1fe444c6..7355bbb281 100644 --- a/common/lib/xmodule/xmodule/word_cloud_module.py +++ b/common/lib/xmodule/xmodule/word_cloud_module.py @@ -15,6 +15,7 @@ from xmodule.editing_module import MetadataOnlyEditingDescriptor from xmodule.x_module import XModule from xblock.fields import Scope, Dict, Boolean, List, Integer, String +from xblock.fragment import Fragment log = logging.getLogger(__name__) @@ -233,19 +234,14 @@ class WordCloudModule(WordCloudFields, XModule): 'error': 'Unknown Command!' }) - def get_html(self): + def student_view(self, context): """ Template rendering. """ - js_includes = [ - self.runtime.local_resource_url(self, 'public/js/d3.min.js'), - self.runtime.local_resource_url(self, 'public/js/d3.layout.cloud.js'), - self.runtime.local_resource_url(self, 'public/js/word_cloud.js'), - self.runtime.local_resource_url(self, 'public/js/word_cloud_main.js'), - ] + fragment = Fragment() - context = { + fragment.add_content(self.system.render_template('word_cloud.html', { 'ajax_url': self.system.ajax_url, 'display_name': self.display_name, 'instructions': self.instructions, @@ -253,10 +249,17 @@ class WordCloudModule(WordCloudFields, XModule): 'element_id': self.location.html_id(), 'num_inputs': self.num_inputs, 'submitted': self.submitted, - 'js_includes': js_includes, - } - self.content = self.system.render_template('word_cloud.html', context) - return self.content + })) + + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/d3.min.js')) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/d3.layout.cloud.js')) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/word_cloud.js')) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/word_cloud_main.js')) + + return fragment + + def author_view(self, context): + return self.student_view(context) class WordCloudDescriptor(WordCloudFields, MetadataOnlyEditingDescriptor, EmptyDataRawDescriptor): diff --git a/lms/templates/word_cloud.html b/lms/templates/word_cloud.html index a6c8a40254..8cbd49fcea 100644 --- a/lms/templates/word_cloud.html +++ b/lms/templates/word_cloud.html @@ -6,11 +6,6 @@ class="${element_class}" data-ajax-url="${ajax_url}" > - - % for js_include in js_includes: - - % endfor - % if display_name:

${display_name}

% endif From 5e40dfa24be932d833908915d3cab872b576394c Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Wed, 16 Nov 2016 14:37:47 -0500 Subject: [PATCH 6/7] quality --- common/lib/xmodule/xmodule/word_cloud_module.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/word_cloud_module.py b/common/lib/xmodule/xmodule/word_cloud_module.py index 7355bbb281..6e12843efb 100644 --- a/common/lib/xmodule/xmodule/word_cloud_module.py +++ b/common/lib/xmodule/xmodule/word_cloud_module.py @@ -236,9 +236,8 @@ class WordCloudModule(WordCloudFields, XModule): def student_view(self, context): """ - Template rendering. + Renders the output that a student will see. """ - fragment = Fragment() fragment.add_content(self.system.render_template('word_cloud.html', { @@ -259,6 +258,9 @@ class WordCloudModule(WordCloudFields, XModule): return fragment def author_view(self, context): + """ + Renders the output that an author will see. + """ return self.student_view(context) From a49227661be74a186381d32e2c20fc26edfb2961 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Thu, 17 Nov 2016 09:49:42 -0500 Subject: [PATCH 7/7] undo the test shenanigans i was doing --- lms/djangoapps/courseware/tests/test_word_cloud.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lms/djangoapps/courseware/tests/test_word_cloud.py b/lms/djangoapps/courseware/tests/test_word_cloud.py index e7221e1543..690a8d010c 100644 --- a/lms/djangoapps/courseware/tests/test_word_cloud.py +++ b/lms/djangoapps/courseware/tests/test_word_cloud.py @@ -250,15 +250,9 @@ class TestWordCloud(BaseTestXmodule): ) def test_word_cloud_constructor(self): - """Make sure that all parameters extracted correctly from xml""" - - js_includes = [ - self._get_resource_url('public/js/d3.min.js'), - self._get_resource_url('public/js/d3.layout.cloud.js'), - self._get_resource_url('public/js/word_cloud.js'), - self._get_resource_url('public/js/word_cloud_main.js'), - ] - + """ + Make sure that all parameters extracted correctly from xml. + """ fragment = self.runtime.render(self.item_descriptor, STUDENT_VIEW) expected_context = { 'ajax_url': self.item_descriptor.xmodule_runtime.ajax_url, @@ -268,7 +262,6 @@ class TestWordCloud(BaseTestXmodule): 'element_id': self.item_descriptor.location.html_id(), 'num_inputs': 5, # default value 'submitted': False, # default value, - 'js_includes': js_includes, } self.assertEqual(fragment.content, self.runtime.render_template('word_cloud.html', expected_context))