Merge branch 'develop' into ser-up-more

oppia · Jul 17, 2019 · 0ca2eae · 0ca2eae
2 parents 6fa6c50 + ea2a0db
commit 0ca2eae
Show file tree

Hide file tree

Showing 55 changed files with 1,024 additions and 295 deletions.
diff --git a/assets/constants.js b/assets/constants.js
@@ -586,6 +586,18 @@ var constants = {
   "SITE_NAME_FOR_ANALYTICS": "",
 
   "ALLOW_YAML_FILE_UPLOAD": false,
+
+  // A regular expression for tags.
+  "TAG_REGEX": "^[a-z ]+$",
+
+  // Invalid names for parameters used in expressions.
+  "INVALID_PARAMETER_NAMES": [
+    "answer", "choices", "abs", "all", "and", "any", "else",
+    "floor", "if", "log", "or", "pow", "round", "then"
+  ],
+
+  // Unfinished features.
+  "SHOW_TRAINABLE_UNRESOLVED_ANSWERS": false,
 
   "DEV_MODE": true
 };
diff --git a/core/controllers/collection_editor.py b/core/controllers/collection_editor.py
@@ -62,7 +62,6 @@ def get(self, collection_id):
                 feconf.SHOW_COLLECTION_NAVIGATION_TAB_HISTORY),
             'SHOW_COLLECTION_NAVIGATION_TAB_STATS': (
                 feconf.SHOW_COLLECTION_NAVIGATION_TAB_STATS),
-            'TAG_REGEX': feconf.TAG_REGEX,
             'INTERACTION_SPECS': interaction_registry.Registry.get_all_specs(),
         })
 

diff --git a/core/controllers/editor.py b/core/controllers/editor.py
@@ -128,10 +128,6 @@ def get(self, exploration_id):
             'interaction_templates': jinja2.utils.Markup(
                 interaction_templates),
             'meta_description': feconf.CREATE_PAGE_DESCRIPTION,
-            'INVALID_PARAMETER_NAMES': feconf.INVALID_PARAMETER_NAMES,
-            'SHOW_TRAINABLE_UNRESOLVED_ANSWERS': (
-                feconf.SHOW_TRAINABLE_UNRESOLVED_ANSWERS),
-            'TAG_REGEX': feconf.TAG_REGEX,
         })
 
         self.render_template('dist/exploration-editor-page.mainpage.html')

diff --git a/core/domain/collection_domain.py b/core/domain/collection_domain.py
@@ -919,7 +919,7 @@ def validate(self, strict=True):
             if not tag:
                 raise utils.ValidationError('Tags should be non-empty.')
 
-            if not re.match(feconf.TAG_REGEX, tag):
+            if not re.match(constants.TAG_REGEX, tag):
                 raise utils.ValidationError(
                     'Tags should only contain lowercase letters and spaces, '
                     'received \'%s\'' % tag)
@@ -1109,7 +1109,7 @@ def validate(self):
             if not tag:
                 raise utils.ValidationError('Tags should be non-empty.')
 
-            if not re.match(feconf.TAG_REGEX, tag):
+            if not re.match(constants.TAG_REGEX, tag):
                 raise utils.ValidationError(
                     'Tags should only contain lowercase letters and spaces, '
                     'received \'%s\'' % tag)

diff --git a/core/domain/exp_domain.py b/core/domain/exp_domain.py
@@ -633,7 +633,7 @@ def validate(self, strict=False):
             if not tag:
                 raise utils.ValidationError('Tags should be non-empty.')
 
-            if not re.match(feconf.TAG_REGEX, tag):
+            if not re.match(constants.TAG_REGEX, tag):
                 raise utils.ValidationError(
                     'Tags should only contain lowercase letters and spaces, '
                     'received \'%s\'' % tag)
@@ -742,7 +742,7 @@ def validate(self, strict=False):
                 raise utils.ValidationError(
                     'No parameter named \'%s\' exists in this exploration'
                     % param_change.name)
-            if param_change.name in feconf.INVALID_PARAMETER_NAMES:
+            if param_change.name in constants.INVALID_PARAMETER_NAMES:
                 raise utils.ValidationError(
                     'The exploration-level parameter with name \'%s\' is '
                     'reserved. Please choose a different name.'
@@ -764,7 +764,7 @@ def validate(self, strict=False):
                         '\'%s\', but it does not exist in the list of '
                         'parameter specifications for this exploration.'
                         % (param_change.name, state_name))
-                if param_change.name in feconf.INVALID_PARAMETER_NAMES:
+                if param_change.name in constants.INVALID_PARAMETER_NAMES:
                     raise utils.ValidationError(
                         'The parameter name \'%s\' is reserved. Please choose '
                         'a different name for the parameter being set in '
@@ -3319,7 +3319,7 @@ def validate(self):
             if not tag:
                 raise utils.ValidationError('Tags should be non-empty.')
 
-            if not re.match(feconf.TAG_REGEX, tag):
+            if not re.match(constants.TAG_REGEX, tag):
                 raise utils.ValidationError(
                     'Tags should only contain lowercase letters and spaces, '
                     'received \'%s\'' % tag)

diff --git a/core/domain/user_services.py b/core/domain/user_services.py
@@ -844,7 +844,7 @@ def update_subject_interests(user_id, subject_interests):
             elif not interest:
                 raise utils.ValidationError(
                     'Expected each subject interest to be non-empty.')
-            elif not re.match(feconf.TAG_REGEX, interest):
+            elif not re.match(constants.TAG_REGEX, interest):
                 raise utils.ValidationError(
                     'Expected each subject interest to consist only of '
                     'lowercase alphabetic characters and spaces.')

diff --git a/core/domain/visualization_registry_test.py b/core/domain/visualization_registry_test.py
@@ -19,6 +19,7 @@
 import importlib
 import inspect
 import os
+import re
 
 from core.domain import visualization_registry
 from core.tests import test_utils
@@ -39,6 +40,34 @@ def test_get_visualization_class_with_invalid_id_raises_error(self):
             visualization_registry.Registry.get_visualization_class(
                 'invalid_visualization_id')
 
+    def test_visualization_class_with_invalid_option_names(self):
+        bar_chart = visualization_registry.Registry.get_visualization_class(
+            'BarChart')
+        bar_chart_instance = bar_chart('AnswerFrequencies', {}, True)
+
+        with self.assertRaisesRegexp(
+            Exception,
+            re.escape(
+                'For visualization BarChart, expected option names '
+                '[\'x_axis_label\', \'y_axis_label\']; received names []')):
+            bar_chart_instance.validate()
+
+    def test_visualization_class_with_invalid_addressed_info_is_supported(self):
+        bar_chart = visualization_registry.Registry.get_visualization_class(
+            'BarChart')
+        option_names = {
+            'x_axis_label': 'Answer',
+            'y_axis_label': 'Count'
+        }
+        bar_chart_instance = bar_chart(
+            'AnswerFrequencies', option_names, 'invalid_value')
+
+        with self.assertRaisesRegexp(
+            Exception,
+            'For visualization BarChart, expected a bool value for '
+            'addressed_info_is_supported; received invalid_value'):
+            bar_chart_instance.validate()
+
 
 class VisualizationsNameTests(test_utils.GenericTestBase):
 

diff --git a/...ates/dev/head/components/state-editor/state-responses-editor/state-responses.directive.ts b/...ates/dev/head/components/state-editor/state-responses-editor/state-responses.directive.ts
@@ -105,7 +105,7 @@ oppia.directive('stateResponses', [
             INTERACTION_IDS_WITHOUT_ANSWER_DETAILS, INTERACTION_SPECS,
             PLACEHOLDER_OUTCOME_DEST, RULE_SUMMARY_WRAP_CHARACTER_COUNT) {
           $scope.SHOW_TRAINABLE_UNRESOLVED_ANSWERS = (
-            GLOBALS.SHOW_TRAINABLE_UNRESOLVED_ANSWERS);
+            constants.SHOW_TRAINABLE_UNRESOLVED_ANSWERS);
           $scope.EditabilityService = EditabilityService;
           $scope.stateName = StateEditorService.getActiveStateName();
           $scope.enableSolicitAnswerDetailsFeature = (

diff --git a/core/templates/dev/head/css/oppia.css b/core/templates/dev/head/css/oppia.css
@@ -3890,7 +3890,7 @@ span.learner-action {
   margin-bottom: 2em;
 }
 
-span.oppia-issues-learner-action {
+div.oppia-issues-learner-action {
   display: block;
   margin-bottom: 2em;
 }

diff --git a/core/templates/dev/head/domain/collection/CollectionValidationService.ts b/core/templates/dev/head/domain/collection/CollectionValidationService.ts
@@ -48,7 +48,7 @@ oppia.factory('CollectionValidationService', [
     var validateTagFormat = function(tags) {
       // Check to ensure that all tags follow the format specified in
       // TAG_REGEX.
-      var tagRegex = new RegExp(GLOBALS.TAG_REGEX);
+      var tagRegex = new RegExp(constants.TAG_REGEX);
       return tags.every(function(tag) {
         return tag.match(tagRegex);
       });

diff --git a/core/templates/dev/head/domain/statistics/FeedbackImprovementCardObjectFactory.ts b/core/templates/dev/head/domain/statistics/FeedbackImprovementCardObjectFactory.ts
@@ -16,25 +16,30 @@
  * @fileoverview Factory for creating Feedback Cards in the Improvements Tab.
  */
 
-
 require('domain/statistics/ImprovementActionButtonObjectFactory.ts');
-require('domain/utilities/UrlInterpolationService.ts');
+require(
+  'pages/exploration-editor-page/improvements-tab/services/' +
+  'improvement-modal.service.ts');
 require(
   'pages/exploration-editor-page/feedback-tab/services/thread-data.service.ts');
-
 require('domain/statistics/statistics-domain.constants.ts');
 
 var oppia = require('AppInit.ts').module;
 
 oppia.factory('FeedbackImprovementCardObjectFactory', [
-  '$q', 'ImprovementActionButtonObjectFactory', 'ThreadDataService',
-  'UrlInterpolationService', 'FEEDBACK_IMPROVEMENT_CARD_TYPE',
+  '$q', 'ImprovementActionButtonObjectFactory', 'ImprovementModalService',
+  'ThreadDataService', 'FEEDBACK_IMPROVEMENT_CARD_TYPE',
   function(
-      $q, ImprovementActionButtonObjectFactory, ThreadDataService,
-      UrlInterpolationService, FEEDBACK_IMPROVEMENT_CARD_TYPE) {
+      $q, ImprovementActionButtonObjectFactory, ImprovementModalService,
+      ThreadDataService, FEEDBACK_IMPROVEMENT_CARD_TYPE) {
     var FeedbackImprovementCard = function(feedbackThread) {
-      this._actionButtons = [];
       this._feedbackThread = feedbackThread;
+      this._actionButtons = [
+        ImprovementActionButtonObjectFactory.createNew(
+          'Review Thread', 'btn-primary', function() {
+            ImprovementModalService.openFeedbackThread(feedbackThread);
+          }),
+      ];
     };
 
     /**

diff --git a/core/templates/dev/head/domain/statistics/FeedbackImprovementCardObjectFactorySpec.ts b/core/templates/dev/head/domain/statistics/FeedbackImprovementCardObjectFactorySpec.ts
@@ -24,7 +24,9 @@ require('domain/statistics/FeedbackImprovementCardObjectFactory.ts');
 describe('FeedbackImprovementCardObjectFactory', function() {
   var $q = null;
   var $rootScope = null;
+  var $uibModal = null;
   var FeedbackImprovementCardObjectFactory = null;
+  var ImprovementModalService = null;
   var ThreadDataService = null;
   var FEEDBACK_IMPROVEMENT_CARD_TYPE = null;
 
@@ -34,12 +36,15 @@ describe('FeedbackImprovementCardObjectFactory', function() {
       'ExplorationDraftObjectFactory', new ExplorationDraftObjectFactory());
   }));
   beforeEach(angular.mock.inject(function(
-      _$q_, _$rootScope_, _FeedbackImprovementCardObjectFactory_,
-      _ThreadDataService_, _FEEDBACK_IMPROVEMENT_CARD_TYPE_) {
+      _$q_, _$rootScope_, _$uibModal_, _FeedbackImprovementCardObjectFactory_,
+      _ImprovementModalService_, _ThreadDataService_,
+      _FEEDBACK_IMPROVEMENT_CARD_TYPE_) {
     $q = _$q_;
     $rootScope = _$rootScope_;
+    $uibModal = _$uibModal_;
     FeedbackImprovementCardObjectFactory =
       _FeedbackImprovementCardObjectFactory_;
+    ImprovementModalService = _ImprovementModalService_;
     ThreadDataService = _ThreadDataService_;
     FEEDBACK_IMPROVEMENT_CARD_TYPE = _FEEDBACK_IMPROVEMENT_CARD_TYPE_;
   }));
@@ -121,8 +126,22 @@ describe('FeedbackImprovementCardObjectFactory', function() {
     });
 
     describe('.getActionButtons', function() {
-      it('is empty', function() {
-        expect(this.card.getActionButtons()).toEqual([]);
+      it('contains one button', function() {
+        expect(this.card.getActionButtons().length).toEqual(1);
+      });
+
+      describe('first button', function() {
+        beforeEach(function() {
+          this.button = this.card.getActionButtons()[0];
+        });
+
+        it('opens a thread modal', function() {
+          var spy = spyOn(ImprovementModalService, 'openFeedbackThread');
+
+          this.button.execute();
+
+          expect(spy).toHaveBeenCalledWith(this.mockThread);
+        });
       });
     });
   });

diff --git a/core/templates/dev/head/domain/statistics/ImprovementActionButtonObjectFactory.ts b/core/templates/dev/head/domain/statistics/ImprovementActionButtonObjectFactory.ts
@@ -27,27 +27,27 @@ oppia.factory('ImprovementActionButtonObjectFactory', [function() {
    * @param {string} [cssClass=btn-default] - The CSS class to render the button
    *    with.
    */
-  var ImprovementActionButton = function(text, actionFunc, cssClass) {
+  var ImprovementActionButton = function(text, cssClass, actionFunc) {
     this._text = text;
+    this._cssClass = cssClass;
     this._actionFunc = actionFunc;
-    this._cssClass = cssClass || 'btn-default';
   };
 
   /** @returns {string} - The text of the action (text rendered in button). */
   ImprovementActionButton.prototype.getText = function() {
     return this._text;
   };
 
-  /** Performs the associated action and return its result. */
-  ImprovementActionButton.prototype.execute = function() {
-    return this._actionFunc();
-  };
-
   /** Returns the CSS class of the button. */
   ImprovementActionButton.prototype.getCssClass = function() {
     return this._cssClass;
   };
 
+  /** Performs the associated action and return its result. */
+  ImprovementActionButton.prototype.execute = function() {
+    return this._actionFunc();
+  };
+
   return {
     /**
      * @returns {ImprovementActionButton}

diff --git a/core/templates/dev/head/domain/statistics/ImprovementActionButtonObjectFactorySpec.ts b/core/templates/dev/head/domain/statistics/ImprovementActionButtonObjectFactorySpec.ts
@@ -19,33 +19,28 @@
 require('domain/statistics/ImprovementActionButtonObjectFactory.ts');
 
 describe('ImprovementActionButtonObjectFactory', function() {
+  var ImprovementActionButtonObjectFactory = null;
+
   beforeEach(angular.mock.module('oppia'));
-  beforeEach(angular.mock.inject(function($injector) {
-    this.ImprovementActionButtonObjectFactory =
-      $injector.get('ImprovementActionButtonObjectFactory');
+  beforeEach(angular.mock.inject(function(
+      _ImprovementActionButtonObjectFactory_) {
+    ImprovementActionButtonObjectFactory =
+      _ImprovementActionButtonObjectFactory_;
   }));
 
   describe('.createNew', function() {
     it('stores the name and action', function() {
       var flagToSetOnCallback = false;
-      var improvementAction =
-        this.ImprovementActionButtonObjectFactory.createNew('Test', function() {
+      var improvementAction = ImprovementActionButtonObjectFactory.createNew(
+        'Test', 'btn-success', function() {
           flagToSetOnCallback = true;
-        }, 'btn-success');
+        });
 
       expect(improvementAction.getText()).toEqual('Test');
       expect(improvementAction.getCssClass()).toEqual('btn-success');
       expect(flagToSetOnCallback).toBe(false);
       improvementAction.execute();
       expect(flagToSetOnCallback).toBe(true);
     });
-
-    it('uses btn-default as class by default', function() {
-      var improvementAction =
-        this.ImprovementActionButtonObjectFactory.createNew('Test', function() {
-        });
-
-      expect(improvementAction.getCssClass()).toEqual('btn-default');
-    });
   });
 });
diff --git a/core/templates/dev/head/domain/statistics/PlaythroughImprovementCardObjectFactory.ts b/core/templates/dev/head/domain/statistics/PlaythroughImprovementCardObjectFactory.ts
@@ -49,8 +49,9 @@ oppia.factory('PlaythroughImprovementCardObjectFactory', [
             '$scope', '$uibModalInstance',
             function($scope, $uibModalInstance) {
               $scope.confirmationMessage =
-                'Are you sure you want to discard this card?';
-              $scope.confirmationButtonText = 'Discard';
+                'Marking this action as resolved will discard the ' +
+                'playthrough. Are you sure you want to proceed?';
+              $scope.confirmationButtonText = 'Mark as Resolved';
               $scope.confirmationButtonClass = 'btn-danger';
               $scope.action = function() {
                 $uibModalInstance.close();
@@ -74,7 +75,7 @@ oppia.factory('PlaythroughImprovementCardObjectFactory', [
       /** @type {ImprovementActionButton[]} */
       this._actionButtons = [
         ImprovementActionButtonObjectFactory.createNew(
-          'Discard', discardThis, 'btn-danger'),
+          'Mark as Resolved', 'btn-primary', discardThis),
       ];
       /** @type {{suggestions: string[], playthroughIds: string[]}} */
       this._directiveData = {