Merge pull request #14659 from edx/mushtaq/remove-banner-logging

Remove success banner on container state change

cms/djangoapps/contentstore/views/item.py

@@ -729,20 +729,20 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non
 
         if (valid_move_type.get(target_parent_type, '') != source_type and
                 target_parent_type not in parent_component_types):
-            error = 'You can not move {source_type} into {target_parent_type}.'.format(
+            error = _('You can not move {source_type} into {target_parent_type}.').format(
                 source_type=source_type,
                 target_parent_type=target_parent_type,
             )
         elif source_parent.location == target_parent.location:
-            error = 'You can not move an item into the same parent.'
+            error = _('You can not move an item into the same parent.')
         elif source_item.location == target_parent.location:
-            error = 'You can not move an item into itself.'
+            error = _('You can not move an item into itself.')
         elif is_source_item_in_target_parents(source_item, target_parent):
-            error = 'You can not move an item into it\'s child.'
+            error = _('You can not move an item into it\'s child.')
         elif target_parent_type == 'split_test':
-            error = 'You can not move an item directly into content experiment.'
+            error = _('You can not move an item directly into content experiment.')
         elif source_index is None:
-            error = '{source_usage_key} not found in {parent_usage_key}.'.format(
+            error = _('{source_usage_key} not found in {parent_usage_key}.').format(
                 source_usage_key=unicode(source_usage_key),
                 parent_usage_key=unicode(source_parent.location)
             )
@@ -750,12 +750,12 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non
             try:
                 target_index = int(target_index) if target_index is not None else None
                 if len(target_parent.children) < target_index:
-                    error = 'You can not move {source_usage_key} at an invalid index ({target_index}).'.format(
+                    error = _('You can not move {source_usage_key} at an invalid index ({target_index}).').format(
                         source_usage_key=unicode(source_usage_key),
                         target_index=target_index
                     )
             except ValueError:
-                error = 'You must provide target_index ({target_index}) as an integer.'.format(
+                error = _('You must provide target_index ({target_index}) as an integer.').format(
                     target_index=target_index
                 )
         if error:
@@ -772,6 +772,14 @@ def _move_item(source_usage_key, target_parent_usage_key, user, target_index=Non
         target_parent.children.insert(insert_at, source_item.location)
         store.update_item(target_parent, user.id)
 
+        log.info(
+            'MOVE: %s moved from %s to %s at %d index',
+            unicode(source_usage_key),
+            unicode(source_parent.location),
+            unicode(target_parent_usage_key),
+            insert_at
+        )
+
         context = {
             'move_source_locator': unicode(source_usage_key),
             'parent_locator': unicode(target_parent_usage_key),

cms/djangoapps/contentstore/views/tests/test_item.py

@@ -1126,6 +1126,24 @@ class TestMoveItem(ItemTest):
         response = json.loads(response.content)
         self.assertEqual(response['error'], 'Patch request did not recognise any parameters to handle.')
 
+    @patch('contentstore.views.item.log')
+    def test_move_logging(self, mock_logger):
+        """
+        Test logging when an item is successfully moved.
+
+        Arguments:
+            mock_logger (object):  A mock logger object.
+        """
+        insert_at = 0
+        self.assert_move_item(self.html_usage_key, self.vert2_usage_key, insert_at)
+        mock_logger.info.assert_called_with(
+            'MOVE: %s moved from %s to %s at %d index',
+            unicode(self.html_usage_key),
+            unicode(self.vert_usage_key),
+            unicode(self.vert2_usage_key),
+            insert_at
+        )
+
 
 class TestDuplicateItemWithAsides(ItemTest, DuplicateHelper):
     """

cms/static/js/views/pages/container_subviews.js

@@ -2,10 +2,11 @@
  * Subviews (usually small side panels) for XBlockContainerPage.
  */
 define(['jquery', 'underscore', 'gettext', 'js/views/baseview', 'common/js/components/utils/view_utils',
-    'js/views/utils/xblock_utils'],
-    function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils) {
-        var VisibilityState = XBlockViewUtils.VisibilityState,
-            disabledCss = 'is-disabled';
+    'js/views/utils/xblock_utils', 'js/views/utils/move_xblock_utils'],
+    function($, _, gettext, BaseView, ViewUtils, XBlockViewUtils, MoveXBlockUtils) {
+        'use strict';
+
+        var disabledCss = 'is-disabled';
 
         /**
          * A view that refreshes the view when certain values in the XBlockInfo have changed
@@ -132,6 +133,8 @@ define(['jquery', 'underscore', 'gettext', 'js/views/baseview', 'common/js/compo
                         return xblockInfo.save({publish: 'make_public'}, {patch: true});
                     }).always(function() {
                         xblockInfo.set('publish', null);
+                        // Hide any move notification if present.
+                        MoveXBlockUtils.hideMovedNotification();
                     }).done(function() {
                         xblockInfo.fetch();
                     });
@@ -151,6 +154,8 @@ define(['jquery', 'underscore', 'gettext', 'js/views/baseview', 'common/js/compo
                                 return xblockInfo.save({publish: 'discard_changes'}, {patch: true});
                             }).always(function() {
                                 xblockInfo.set('publish', null);
+                                // Hide any move notification if present.
+                                MoveXBlockUtils.hideMovedNotification();
                             }).done(function() {
                                 renderPage();
                             });

common/test/acceptance/pages/studio/container.py

@@ -286,16 +286,18 @@ class ContainerPage(PageObject, HelpMixin):
         """
         return _click_edit(self, '.edit-button', '.xblock-studio_view')
 
-    def verify_confirmation_message(self, message):
+    def verify_confirmation_message(self, message, verify_hidden=False):
         """
-        Verify for confirmation message.
+        Verify for confirmation message is present or hidden.
         """
         def _verify_message():
             """ promise function to check confirmation message state """
             text = self.q(css='#page-alert .alert.confirmation #alert-confirmation-title').text
-            return text and message in text[0]
+            return text and message not in text[0] if verify_hidden else text and message in text[0]
 
-        self.wait_for(_verify_message, description='confirmation message present')
+        self.wait_for(_verify_message, description='confirmation message {status}'.format(
+            status='hidden' if verify_hidden else 'present'
+        ))
 
     def click_undo_move_link(self):
         """

common/test/acceptance/tests/studio/test_studio_container.py

@@ -17,7 +17,7 @@ from common.test.acceptance.pages.lms.staff_view import StaffPage
 from common.test.acceptance.tests.helpers import create_user_partition_json
 
 import datetime
-from bok_choy.promise import Promise, EmptyPromise
+import ddt
 from base_studio_test import ContainerBase
 from xmodule.partitions.partitions import Group
 
@@ -1131,6 +1131,7 @@ class ProblemCategoryTabsTest(ContainerBase):
 
 
 @attr(shard=1)
+@ddt.ddt
 class MoveComponentTest(ContainerBase):
     """
     Tests of moving an XBlock to another XBlock.
@@ -1220,6 +1221,30 @@ class MoveComponentTest(ContainerBase):
             component_display_names_after_operation
         )
 
+    def verify_state_change(self, unit_page, operation):
+        """
+        Verify that after state change, confirmation message is hidden.
+
+        Arguments:
+            unit_page (Object)  Unit container page.
+            operation (String)  Publish or discard changes operation.
+        """
+        # Verify unit in draft state now
+        self.container.verify_publish_title(self.DRAFT_STATUS)
+
+        # Now click publish/discard button
+        if operation == 'publish':
+            unit_page.publish_action.click()
+        else:
+            unit_page.discard_changes()
+
+        # Now verify success message is hidden
+        self.container.verify_publish_title(self.PUBLISHED_LIVE_STATUS)
+        self.container.verify_confirmation_message(
+            message=self.message_move.format(display_name=self.source_component_display_name),
+            verify_hidden=True
+        )
+
     def test_move_component_successfully(self):
         """
         Test if we can move a component successfully.
@@ -1268,6 +1293,40 @@ class MoveComponentTest(ContainerBase):
             component_display_names_after_operation=['HTML 11', 'HTML 12']
         )
 
+    @ddt.data('publish', 'discard')
+    def test_publish_discard_changes_afer_move(self, operation):
+        """
+        Test if success banner is hidden when we  discard changes or publish the unit after a move operation.
+
+        Given I am a staff user
+        And I go to unit page in first section
+        And I open the move modal
+        And I navigate to unit in second section
+        And I see move button is enabled
+        When I click on the move button
+        Then I see move operation success message
+        And When I click on publish or discard changes button
+        Then I see move operation success message is hidden.
+        """
+        unit_page = self.go_to_unit_page(unit_name='Test Unit 1')
+        components = unit_page.displayed_children
+        self.assertEqual(len(components), 2)
+
+        components[0].open_move_modal()
+        self.move_modal_view.navigate_to_category(self.source_xblock_category, self.navigation_options)
+        self.assertEqual(self.move_modal_view.is_move_button_enabled, True)
+
+        # Verify unit is in published state before move operation
+        self.container.verify_publish_title(self.PUBLISHED_LIVE_STATUS)
+
+        self.move_modal_view.click_move_button()
+        self.container.verify_confirmation_message(
+            self.message_move.format(display_name=self.source_component_display_name)
+        )
+        self.assertEqual(len(unit_page.displayed_children), 1)
+
+        self.verify_state_change(unit_page, operation)
+
     def test_content_experiment(self):
         """
         Test if we can move a component of content experiment successfully.