Fixes oppia#5687 : Added feature to re submission of rejected suggest…

…ions. (oppia#5730) * Fixes oppia#5687 Added a feature to resubmit rejected submission * Added backend tests. Addressed Review changes * lint and testing issues rectified * Addressing review comments * Addressing review changes * Removed unnecessary check in suggestion controller * backend test changed. * Addressing review changes * Addressing review changes * Addressing review changes * Addressing review changes
quantumferret · Oct 29, 2018 · 7dcbc2f · 7dcbc2f
1 parent 01170c4
commit 7dcbc2f
Show file tree

Hide file tree

Showing 12 changed files with 497 additions and 21 deletions.
diff --git a/core/controllers/learner_dashboard.py b/core/controllers/learner_dashboard.py
@@ -165,9 +165,9 @@ def get(self, thread_id):
             exploration = exp_services.get_exploration_by_id(exploration_id)
             current_content_html = (
                 exploration.states[
-                    suggestion.state_name].content.html)
+                    suggestion.change.state_name].content.html)
             suggestion_summary = {
-                'suggestion_html': suggestion.suggestion_html,
+                'suggestion_html': suggestion.change.new_value['html'],
                 'current_content_html': current_content_html,
                 'description': suggestion.description,
                 'author_username': authors_settings[0].username,

diff --git a/core/controllers/suggestion.py b/core/controllers/suggestion.py
@@ -83,6 +83,26 @@ def put(self, target_id, suggestion_id):
         self.render_json(self.values)
 
 
+class ReSubmitSuggestionHandler(base.BaseHandler):
+    """Handler to Reopen a rejected suggestion."""
+
+    @acl_decorators.can_resubmit_suggestion
+    def put(self, suggestion_id):
+        suggestion = suggestion_services.get_suggestion_by_id(suggestion_id)
+        if not suggestion:
+            raise self.InvalidInputException(
+                'No suggestion found with given suggestion id')
+        new_change = self.payload.get('change')
+        change_type = type(suggestion.change)
+        change_object = change_type(new_change)
+        suggestion.pre_update_validate(change_object)
+        suggestion.change = change_object
+        summary_message = self.payload.get('summary_message')
+        suggestion_services.resubmit_rejected_suggestion(
+            suggestion, summary_message, self.user_id)
+        self.render_json(self.values)
+
+
 class SuggestionToTopicActionHandler(base.BaseHandler):
     """Handles actions performed on suggestions to topics."""
 

diff --git a/core/controllers/suggestion_test.py b/core/controllers/suggestion_test.py
@@ -83,6 +83,8 @@ def setUp(self):
 
         self.new_content = state_domain.SubtitledHtml(
             'content', 'new content html').to_dict()
+        self.resubmit_change_content = state_domain.SubtitledHtml(
+            'content', 'resubmit change content html').to_dict()
 
         self.logout()
 
@@ -320,6 +322,50 @@ def test_suggestion_list_handler(self):
             )['suggestions']
         self.assertEqual(len(suggestions), 2)
 
+    def test_resubmit_rejected_suggestion(self):
+
+        self.login(self.EDITOR_EMAIL)
+        response = self.testapp.get('/explore/%s' % self.EXP_ID)
+        csrf_token = self.get_csrf_token_from_response(response)
+
+        suggestion = suggestion_services.query_suggestions(
+            [('author_id', self.author_id), ('target_id', self.EXP_ID)])[0]
+        suggestion_services.reject_suggestion(
+            suggestion, self.reviewer_id, 'reject message')
+        self.logout()
+
+        self.login(self.AUTHOR_EMAIL)
+        response = self.testapp.get('/explore/%s' % self.EXP_ID)
+        csrf_token = self.get_csrf_token_from_response(response)
+
+        self.put_json('%s/resubmit/%s' % (
+            feconf.SUGGESTION_ACTION_URL_PREFIX, suggestion.suggestion_id), {
+                'summary_message': 'summary message',
+                'action': u'resubmit',
+                'change': {
+                    'cmd': exp_domain.CMD_EDIT_STATE_PROPERTY,
+                    'property_name': exp_domain.STATE_PROPERTY_CONTENT,
+                    'state_name': 'State 1',
+                    'new_value': self.resubmit_change_content,
+                    'old_value': self.old_content
+                }
+            }, csrf_token=csrf_token)
+
+        suggestion = suggestion_services.query_suggestions(
+            [('author_id', self.author_id), ('target_id', self.EXP_ID)])[0]
+        self.assertEqual(
+            suggestion.status, suggestion_models.STATUS_IN_REVIEW)
+        self.assertEqual(
+            suggestion.change.new_value['html'],
+            self.resubmit_change_content['html'])
+        self.assertEqual(
+            suggestion.change.cmd, exp_domain.CMD_EDIT_STATE_PROPERTY)
+        self.assertEqual(
+            suggestion.change.property_name, exp_domain.STATE_PROPERTY_CONTENT)
+        self.assertEqual(
+            suggestion.change.state_name, 'State 1')
+        self.logout()
+
 
 class QuestionSuggestionTests(test_utils.GenericTestBase):
 

diff --git a/core/domain/acl_decorators.py b/core/domain/acl_decorators.py
@@ -876,6 +876,34 @@ def test_can_suggest(self, **kwargs):
     return test_can_suggest
 
 
+def can_resubmit_suggestion(handler):
+    """Decorator to check whether a user can resubmit a suggestion."""
+
+    def test_can_resubmit_suggestion(self, suggestion_id, **kwargs):
+        """Checks if the use can edit the given suggestion.
+
+        Args:
+            suggestion_id: str. The ID of the suggestion.
+            **kwargs: *. keyword arguments.
+
+        Returns:
+            *. The return value of the decorated function.
+
+        Raises:
+            UnauthorizedUserException: The user does not have
+                credentials to edit this suggestion.
+        """
+        if suggestion_services.check_can_resubmit_suggestion(
+                suggestion_id, self.user_id):
+            return handler(self, suggestion_id, **kwargs)
+        else:
+            raise base.UserFacingExceptions.UnauthorizedUserException(
+                'You do not have credentials to resubmit this suggestion.')
+    test_can_resubmit_suggestion.__wrapped__ = True
+
+    return test_can_resubmit_suggestion
+
+
 def can_publish_exploration(handler):
     """Decorator to check whether user can publish exploration."""
 

diff --git a/core/domain/acl_decorators_test.py b/core/domain/acl_decorators_test.py
@@ -19,6 +19,7 @@
 from core.domain import question_services
 from core.domain import rights_manager
 from core.domain import skill_services
+from core.domain import suggestion_services
 from core.domain import topic_domain
 from core.domain import topic_services
 from core.domain import user_services
@@ -1229,6 +1230,74 @@ def test_normal_user_can_suggest_changes(self):
         self.logout()
 
 
+class ResubmitSuggestionDecoratorsTest(test_utils.GenericTestBase):
+    """Tests for can_resubmit_suggestion decorator."""
+    owner_username = 'owner'
+    owner_email = 'owner@example.com'
+    author_username = 'author'
+    author_email = 'author@example.com'
+    username = 'user'
+    user_email = 'user@example.com'
+    TARGET_TYPE = 'exploration'
+    SUGGESTION_TYPE = 'edit_exploration_state_content'
+    exploration_id = 'exp_id'
+    target_version_id = 1
+    change_dict = {
+        'cmd': 'edit_state_property',
+        'property_name': 'content',
+        'state_name': 'Introduction',
+        'new_value': ''
+    }
+
+    class MockHandler(base.BaseHandler):
+
+        GET_HANDLER_ERROR_RETURN_TYPE = feconf.HANDLER_TYPE_JSON
+
+        @acl_decorators.can_resubmit_suggestion
+        def get(self, suggestion_id):
+            self.render_json({'suggestion_id': suggestion_id})
+
+    def setUp(self):
+        super(ResubmitSuggestionDecoratorsTest, self).setUp()
+        self.signup(self.author_email, self.author_username)
+        self.signup(self.user_email, self.username)
+        self.signup(self.owner_email, self.owner_username)
+        self.author_id = self.get_user_id_from_email(self.author_email)
+        self.owner_id = self.get_user_id_from_email(self.owner_email)
+        self.mock_testapp = webtest.TestApp(webapp2.WSGIApplication(
+            [webapp2.Route('/mock/<suggestion_id>', self.MockHandler)],
+            debug=feconf.DEBUG,
+        ))
+        self.save_new_default_exploration(self.exploration_id, self.owner_id)
+        suggestion_services.create_suggestion(
+            self.SUGGESTION_TYPE, self.TARGET_TYPE,
+            self.exploration_id, self.target_version_id,
+            self.author_id,
+            self.change_dict, '', None)
+        suggestion = suggestion_services.query_suggestions(
+            [('author_id', self.author_id),
+             ('target_id', self.exploration_id)])[0]
+        self.suggestion_id = suggestion.suggestion_id
+
+
+    def test_author_can_resubmit_suggestion(self):
+        self.login(self.author_email)
+        with self.swap(self, 'testapp', self.mock_testapp):
+            response = self.get_json(
+                '/mock/%s' % self.suggestion_id, expect_errors=False,
+                expected_status_int=200)
+        self.assertEqual(response['suggestion_id'], self.suggestion_id)
+        self.logout()
+
+    def test_non_author_cannot_resubmit_suggestion(self):
+        self.login(self.user_email)
+        with self.swap(self, 'testapp', self.mock_testapp):
+            self.get_json(
+                '/mock/%s' % self.suggestion_id, expect_errors=True,
+                expected_status_int=401)
+        self.logout()
+
+
 class PublishExplorationTest(test_utils.GenericTestBase):
     """Tests for can_publish_exploration decorator."""
     private_exp_id = 'exp_0'

diff --git a/core/domain/suggestion_registry.py b/core/domain/suggestion_registry.py
@@ -213,6 +213,14 @@ def populate_old_value_of_change(self):
             'Subclasses of BaseSuggestion should implement '
             'populate_old_value_of_change.')
 
+    def pre_update_validate(self, change):
+        """Performs the pre update validation. This functions need to be called
+        before updating the suggestion.
+        """
+        raise NotImplementedError(
+            'Subclasses of BaseSuggestion should implement '
+            'pre_update_validate.')
+
     @property
     def is_handled(self):
         """Returns if the suggestion has either been accepted or rejected.
@@ -338,6 +346,32 @@ def accept(self, commit_message):
             self.final_reviewer_id, self.target_id, change_list,
             commit_message, is_suggestion=True)
 
+    def pre_update_validate(self, change):
+        """Performs the pre update validation. This functions need to be called
+        before updating the suggestion.
+
+        Args:
+            change: ExplorationChange. The new change.
+
+        Raises:
+            ValidationError: Invalid new change.
+        """
+        if self.change.cmd != change.cmd:
+            raise utils.ValidationError(
+                'The new change cmd must be equal to %s' %
+                self.change.cmd)
+        elif self.change.property_name != change.property_name:
+            raise utils.ValidationError(
+                'The new change property_name must be equal to %s' %
+                self.change.property_name)
+        elif self.change.state_name != change.state_name:
+            raise utils.ValidationError(
+                'The new change state_name must be equal to %s' %
+                self.change.state_name)
+        elif self.change.new_value['html'] == change.new_value['html']:
+            raise utils.ValidationError(
+                'The new html must not match the old html')
+
 
 class SuggestionAddQuestion(BaseSuggestion):
     """Domain object for a suggestion of type SUGGESTION_TYPE_ADD_QUESTION.
@@ -476,6 +510,31 @@ def populate_old_value_of_change(self):
         pass
 
 
+    def pre_update_validate(self, change):
+        """Performs the pre update validation. This functions need to be called
+        before updating the suggestion.
+
+        Args:
+            change: QuestionChange. The new change.
+
+        Raises:
+            ValidationError: Invalid new change.
+        """
+        if self.change.cmd != change.cmd:
+            raise utils.ValidationError(
+                'The new change cmd must be equal to %s' %
+                self.change.cmd)
+        if self.change.skill_id != change.skill_id:
+            raise utils.ValidationError(
+                'The new change skill_id must be equal to %s' %
+                self.change.skill_id)
+        if self.change.question_domain == change.question_dict:
+            raise utils.ValidationError(
+                'The new change question_dict must',
+                ' not be equal to old question_dict')
+
+
+
 SUGGESTION_TYPES_TO_DOMAIN_CLASSES = {
     suggestion_models.SUGGESTION_TYPE_EDIT_STATE_CONTENT: (
         SuggestionEditStateContent),

diff --git a/core/domain/suggestion_services.py b/core/domain/suggestion_services.py
@@ -159,7 +159,7 @@ def _update_suggestion(suggestion):
 
     suggestion_model.status = suggestion.status
     suggestion_model.final_reviewer_id = suggestion.final_reviewer_id
-    suggestion_model.change = suggestion.change.to_dict()
+    suggestion_model.change_cmd = suggestion.change.to_dict()
     suggestion_model.score_category = suggestion.score_category
 
     suggestion_model.put()
@@ -277,6 +277,36 @@ def reject_suggestion(suggestion, reviewer_id, review_message):
         None, review_message)
 
 
+def resubmit_rejected_suggestion(suggestion, summary_message, author_id):
+    """Resubmit a rejected suggestion.
+
+     Args:
+        suggestion: Suggestion. The rejected suggestion.
+        summary_message: str. The message provided by the author to
+            summarize new suggestion.
+        author_id: str. The ID of the author creating the suggestion.
+
+    Raises:
+        Exception: Only rejected suggestions can be resubmitted.
+    """
+    if not summary_message:
+        raise Exception('Summary message cannot be empty.')
+    if not suggestion.is_handled:
+        raise Exception('The suggestion is not yet handled.')
+    if suggestion.status == suggestion_models.STATUS_ACCEPTED:
+        raise Exception(
+            'The suggestion was accepted.'
+            'only rejected suggestions can be resubmitted.')
+
+    suggestion.status = suggestion_models.STATUS_IN_REVIEW
+    _update_suggestion(suggestion)
+
+    thread_id = suggestion.suggestion_id
+    feedback_services.create_message(
+        thread_id, author_id, feedback_models.STATUS_CHOICES_OPEN,
+        None, summary_message)
+
+
 def get_all_suggestions_that_can_be_reviewed_by_user(user_id):
     """Returns a list of suggestions which need to be reviewed, in categories
     where the user has crossed the minimum score to review.
@@ -494,3 +524,20 @@ def update_position_in_rotation(score_category, user_id):
     """
     suggestion_models.ReviewerRotationTrackingModel.update_position_in_rotation(
         score_category, user_id)
+
+
+def check_can_resubmit_suggestion(suggestion_id, user_id):
+    """Checks whether the given user can resubmit the suggestion.
+
+    Args:
+        suggestion_id: str. The ID of the suggestion.
+        user_id: str. The ID of the user
+
+    Returns:
+        bool: A boolean indication whether
+            the user can resubmit the suggestion or not.
+    """
+
+    suggestion = get_suggestion_by_id(suggestion_id)
+
+    return suggestion.author_id == user_id