Add logging on move xblock

Add i18n to backend error messages in case they appear to end user
Remove success banner when trying to publish or discard changes on container page

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.