Fix part of oppia#6732: Migrate visualizations to webpack (oppia#6885)

* migrate visualizations to webpack * fix linting * fix linting * fix linting * test coverage * fix tests
saketringsia · Jun 12, 2019 · f9e9922 · f9e9922
1 parent 5803780
commit f9e9922
Show file tree

Hide file tree

Showing 14 changed files with 32 additions and 42 deletions.
diff --git a/core/controllers/editor.py b/core/controllers/editor.py
@@ -38,7 +38,6 @@
 from core.domain import stats_domain
 from core.domain import stats_services
 from core.domain import user_services
-from core.domain import visualization_registry
 from core.platform import models
 import feconf
 import utils
@@ -94,8 +93,6 @@ def get(self, exploration_id):
         exploration_rights = rights_manager.get_exploration_rights(
             exploration_id)
 
-        visualizations_html = visualization_registry.Registry.get_full_html()
-
         interaction_ids = (
             interaction_registry.Registry.get_all_interaction_ids())
 
@@ -137,7 +134,6 @@ def get(self, exploration_id):
             'interaction_templates': jinja2.utils.Markup(
                 interaction_templates),
             'meta_description': feconf.CREATE_PAGE_DESCRIPTION,
-            'visualizations_html': jinja2.utils.Markup(visualizations_html),
             'INVALID_PARAMETER_NAMES': feconf.INVALID_PARAMETER_NAMES,
             'SHOW_TRAINABLE_UNRESOLVED_ANSWERS': (
                 feconf.SHOW_TRAINABLE_UNRESOLVED_ANSWERS),

diff --git a/core/domain/visualization_registry.py b/core/domain/visualization_registry.py
@@ -17,11 +17,8 @@
 """Registry for visualizations."""
 
 import inspect
-import os
 
 from extensions.visualizations import models
-import feconf
-import utils
 
 
 class Registry(object):
@@ -46,19 +43,6 @@ def _refresh_registry(cls):
             if 'BaseVisualization' in ancestor_names:
                 cls.visualizations_dict[clazz.__name__] = clazz
 
-    @classmethod
-    def get_full_html(cls):
-        """Returns the HTML bodies for all visualizations."""
-        js_directives = ''
-        for visualization_class in cls.get_all_visualization_ids():
-            filename = (
-                'OppiaVisualization%sDirective.js' % (visualization_class))
-            js_directives += (
-                utils.get_file_contents(os.path.join(
-                    feconf.VISUALIZATIONS_DIR_FOR_JS, filename)))
-
-        return '<script>%s</script>\n' % (js_directives)
-
     @classmethod
     def get_visualization_class(cls, visualization_id):
         """Gets a visualization class by its id (which is also its class name).

diff --git a/core/domain/visualization_registry_test.py b/core/domain/visualization_registry_test.py
@@ -33,18 +33,6 @@ def test_visualization_registry(self):
             len(visualization_registry.Registry.get_all_visualization_ids()),
             0)
 
-    def test_get_full_html(self):
-        """Check that the visualization HTML contains templates and directives
-        for all visualizations.
-        """
-
-        full_html = visualization_registry.Registry.get_full_html()
-        all_visualization_ids = (
-            visualization_registry.Registry.get_all_visualization_ids())
-
-        for visualization_id in all_visualization_ids:
-            self.assertIn('oppiaVisualization%s' % visualization_id, full_html)
-
     def test_get_visualization_class_with_invalid_id_raises_error(self):
         with self.assertRaisesRegexp(
             TypeError, 'is not a valid visualization id.'):

diff --git a/core/templates/dev/head/pages/creator-dashboard-page/creator-dashboard-page.mainpage.html b/core/templates/dev/head/pages/creator-dashboard-page/creator-dashboard-page.mainpage.html
@@ -912,6 +912,5 @@ <h2>Suggestions to review</h2>
     <% } %>
   <% } %>
   {{ interaction_templates }}
-  {{ visualizations_html }}
   {{ dependencies_html }}
 {% endblock footer_js %}
diff --git a/core/templates/dev/head/pages/exploration-editor-page/exploration-editor-page.mainpage.html b/core/templates/dev/head/pages/exploration-editor-page/exploration-editor-page.mainpage.html
@@ -162,6 +162,5 @@
     <% } %>
   <% } %>
   {{ interaction_templates }}
-  {{ visualizations_html }}
   {{ dependencies_html }}
 {% endblock footer_js %}
diff --git a/core/templates/dev/head/pages/skill-editor-page/skill-editor-page.mainpage.html b/core/templates/dev/head/pages/skill-editor-page/skill-editor-page.mainpage.html
@@ -82,6 +82,5 @@
     <% } %>
   <% } %>
   {{ interaction_templates }}
-  {{ visualizations_html }}
   {{ dependencies_html }}
 {% endblock footer_js %}
diff --git a/core/templates/dev/head/pages/topic_editor/topic_editor.html b/core/templates/dev/head/pages/topic_editor/topic_editor.html
@@ -82,6 +82,5 @@
     <% } %>
   <% } %>
   {{ interaction_templates }}
-  {{ visualizations_html }}
   {{ dependencies_html }}
 {% endblock footer_js %}
diff --git a/extensions/visualizations/OppiaVisualizationBarChartDirective.ts b/extensions/visualizations/OppiaVisualizationBarChartDirective.ts
@@ -16,10 +16,11 @@
  * @fileoverview Directive for "bar chart" visualization.
  */
 
+require('services/HtmlEscaperService.ts');
+
 // Each visualization receives three variables: 'data', 'options', and
 // 'isAddressed'. The exact format for each of these is specific to the
 // particular visualization.
-
 oppia.directive('oppiaVisualizationBarChart', [function() {
   return {
     restrict: 'E',

diff --git a/extensions/visualizations/OppiaVisualizationEnumeratedFrequencyTableDirective.ts b/extensions/visualizations/OppiaVisualizationEnumeratedFrequencyTableDirective.ts
@@ -16,10 +16,12 @@
  * @fileoverview Directive for "enumerated frequency table" visualization.
  */
 
+require('domain/utilities/UrlInterpolationService.ts');
+require('services/HtmlEscaperService.ts');
+
 // Each visualization receives three variables: 'data', 'options', and
 // 'isAddressed'. The exact format for each of these is specific to the
 // particular visualization.
-
 oppia.directive('oppiaVisualizationEnumeratedFrequencyTable', [
   'UrlInterpolationService', function(UrlInterpolationService) {
     return {

diff --git a/extensions/visualizations/OppiaVisualizationFrequencyTableDirective.ts b/extensions/visualizations/OppiaVisualizationFrequencyTableDirective.ts
@@ -16,10 +16,12 @@
  * @fileoverview Directive for the "frequency table" visualization.
  */
 
+require('domain/utilities/UrlInterpolationService.ts');
+require('services/HtmlEscaperService.ts');
+
 // Each visualization receives three variables: 'data', 'options', and
 // 'isAddressed'. The exact format for each of these is specific to the
 // particular visualization.
-
 oppia.directive('oppiaVisualizationFrequencyTable', [
   'UrlInterpolationService', function(UrlInterpolationService) {
     return {

diff --git a/extensions/visualizations/visualizationsRequires.ts b/extensions/visualizations/visualizationsRequires.ts
@@ -0,0 +1,22 @@
+// Copyright 2019 The Oppia Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS-IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+/**
+ * @fileoverview Requires for all the visualization directives.
+ */
+
+require('visualizations/OppiaVisualizationBarChartDirective.ts');
+require(
+  'visualizations/OppiaVisualizationEnumeratedFrequencyTableDirective.ts');
+require('visualizations/OppiaVisualizationFrequencyTableDirective.ts');
diff --git a/tsconfig.json b/tsconfig.json
@@ -26,7 +26,6 @@
     "extensions/classifiers",
     "extensions/interactions",
     "extensions/objects",
-    "extensions/visualizations",
     "typings"
   ]
 }
diff --git a/webpack.dev.config.ts b/webpack.dev.config.ts
@@ -35,7 +35,7 @@ module.exports = {
       include: [
         path.resolve(__dirname, 'core/templates/dev/head'),
         path.resolve(__dirname, 'extensions'),
-        path.resolve(__dirname, 'typings'),
+        path.resolve(__dirname, 'typings')
       ],
       use: [
         'cache-loader',

diff --git a/webpack.prod.config.ts b/webpack.prod.config.ts
@@ -35,7 +35,7 @@ module.exports = {
       include: [
         path.resolve(__dirname, 'core/templates/dev/head'),
         path.resolve(__dirname, 'extensions'),
-        path.resolve(__dirname, 'typings'),
+        path.resolve(__dirname, 'typings')
       ],
       use: [
         'cache-loader',