Review copy/move/rename methods

docs/CHANGES.rst

@@ -38,7 +38,8 @@ Changes
 - Make api.content.create() also print out the underlying error, refs #118.
   [winston88]
 
-- Fix api.content.copy() without target parameter, refs #115.
+- Fix api.content copy/move/rename functions to return the object after they
+  change content, refs #115.
   [rodfersou]
 
 - Make Travis IRC notification message to be one-line instead of three-lines.

src/plone/api/content.py

@@ -160,6 +160,7 @@ def move(source=None, target=None, id=None, safe_id=False):
         conflicting with another object in the target container, raise a
         InvalidParameterError. When True, choose a new, non-conflicting id.
     :type safe_id: boolean
+    :returns: Content object that was moved to the target location
     :raises:
         KeyError
         ValueError
@@ -175,7 +176,9 @@ def move(source=None, target=None, id=None, safe_id=False):
         target = source
 
     if id:
-        rename(obj=target[source_id], new_id=id, safe_id=safe_id)
+        return rename(obj=target[source_id], new_id=id, safe_id=safe_id)
+    else:
+        return target[source_id]
 
 
 @required_parameters('obj', 'new_id')
@@ -190,6 +193,7 @@ def rename(obj=None, new_id=None, safe_id=False):
         conflicting with another object in the container, raise a
         InvalidParameterError. When True, choose a new, non-conflicting id.
     :type safe_id: boolean
+    :returns: Content object that was renamed
     :Example: :ref:`content_rename_example`
     """
     obj_id = obj.getId()
@@ -202,6 +206,7 @@ def rename(obj=None, new_id=None, safe_id=False):
         new_id = chooser.chooseName(new_id, obj)
 
     obj.aq_parent.manage_renameObject(obj_id, new_id)
+    return obj.aq_parent[new_id]
 
 
 @required_parameters('source')
@@ -220,11 +225,11 @@ def copy(source=None, target=None, id=None, safe_id=False):
         - however, if the new object's id conflicts with another object in the
         target container, a suffix will be added to the new object's id.
     :type id: string
-    :returns: Content object that was created in the target location
     :param safe_id: When True, the given id will be enforced. If the id is
         conflicting with another object in the target container, raise a
         InvalidParameterError. When True, choose a new, non-conflicting id.
     :type safe_id: boolean
+    :returns: Content object that was created in the target location
     :raises:
         KeyError,
         ValueError
@@ -238,7 +243,9 @@ def copy(source=None, target=None, id=None, safe_id=False):
     target.manage_pasteObjects(source.aq_parent.manage_copyObjects(source_id))
 
     if id:
-        rename(obj=target[source_id], new_id=id, safe_id=safe_id)
+        return rename(obj=target[source_id], new_id=id, safe_id=safe_id)
+    else:
+        return target[source_id]
 
 
 @required_parameters('obj')

src/plone/api/tests/test_content.py

@@ -298,39 +298,46 @@ def test_move(self):
         container = self.portal
 
         # Move contact to the same folder (basically a rename)
-        api.content.move(source=self.contact, id='nu-contact')
-        assert container['about']['nu-contact']
+        nucontact = api.content.move(source=self.contact, id='nu-contact')
+        assert (container['about']['nu-contact'] and
+                container['about']['nu-contact'] == nucontact)
         assert 'contact' not in container['about'].keys()
 
         # Move team page to portal root
-        api.content.move(source=self.team, target=container)
-        assert container['team']
+        team = api.content.move(source=self.team, target=container)
+        assert container['team'] and container['team'] == team
         assert 'team' not in container['about'].keys()
 
         # When moving objects we can change the id
         team = container['team']
-        api.content.move(source=team, target=self.about, id='our-team')
-        assert container['about']['our-team']
+        ourteam = api.content.move(source=team,
+                                   target=self.about,
+                                   id='our-team')
+        assert (container['about']['our-team'] and
+                container['about']['our-team'] == ourteam)
         assert 'team' not in container.keys()
 
         # Test with safe_id option when moving content
         api.content.create(
             container=self.about, type='Link', id='link-to-blog')
-        api.content.move(
+        linktoblog1 = api.content.move(
             source=self.blog,
             target=self.about,
             id='link-to-blog',
             safe_id=True,
         )
-        assert container['about']['link-to-blog-1']
+        assert (container['about']['link-to-blog-1'] and
+                container['about']['link-to-blog-1'] == linktoblog1)
         assert 'link-to-blog' not in container.keys()
 
         api.content.move(source=self.conference, id='conference-renamed')
         self.assertEqual(self.conference.id, 'conference-renamed')
 
         # Move folderish object
-        api.content.move(source=container.about, target=container.events)
-        assert container['events']['about']
+        about = api.content.move(source=container.about,
+                                 target=container.events)
+        assert (container['events']['about'] and
+                container['events']['about'] == about)
 
     def test_rename_constraints(self):
         """Test the constraints for rename content."""
@@ -353,19 +360,21 @@ def test_rename(self):
         container = self.portal
 
         # Rename contact
-        api.content.rename(obj=self.contact, new_id='nu-contact')
-        assert container['about']['nu-contact']
+        nucontact = api.content.rename(obj=self.contact, new_id='nu-contact')
+        assert (container['about']['nu-contact'] and
+                container['about']['nu-contact'] == nucontact)
         assert 'contact' not in container['about'].keys()
 
         # Test with safe_id option when moving content
         api.content.create(
             container=self.about, type='Link', id='link-to-blog')
-        api.content.rename(
+        linktoblog1 = api.content.rename(
             obj=container['about']['link-to-blog'],
             new_id='link-to-blog',
             safe_id=True,
         )
-        assert container['about']['link-to-blog-1']
+        assert (container['about']['link-to-blog-1'] and
+                container['about']['link-to-blog-1'] == linktoblog1)
         assert 'link-to-blog' not in container.keys()
 
         # Rename to existing id
@@ -377,12 +386,13 @@ def test_rename(self):
             obj=container['about']['link-to-blog'],
             new_id='link-to-blog-1',
         )
-        api.content.rename(
+        linktoblog11 = api.content.rename(
             obj=container['about']['link-to-blog'],
             new_id='link-to-blog-1',
             safe_id=True,
         )
-        assert container['about']['link-to-blog-1-1']
+        assert (container['about']['link-to-blog-1-1'] and
+                container['about']['link-to-blog-1-1'] == linktoblog11)
         assert 'link-to-blog' not in container.keys()
 
     def test_copy_constraints(self):
@@ -403,13 +413,19 @@ def test_copy(self):
         container = self.portal
 
         # Copy team page to portal root
-        api.content.copy(source=self.team, target=container)
-        assert container['team']
-        assert container['about']['team']  # old content still available
+        team = api.content.copy(source=self.team, target=container)
+        assert container['team'] and container['team'] == team
+        assert (
+            container['about']['team'] and
+            container['about']['team'] != team
+        )  # old content still available
 
         # When copying objects we can change the id
-        api.content.copy(source=self.team, target=self.about, id='our-team')
-        assert container['about']['our-team']
+        ourteam = api.content.copy(source=self.team,
+                                   target=self.about,
+                                   id='our-team')
+        assert (container['about']['our-team'] and
+                container['about']['our-team'] == ourteam)
 
         # When copying whithout target parameter should take source parent
         api.content.copy(source=self.team, id='our-team-no-target')
@@ -419,17 +435,20 @@ def test_copy(self):
         api.content.create(
             container=self.about, type='Link', id='link-to-blog')
 
-        api.content.copy(
+        linktoblog1 = api.content.copy(
             source=self.blog,
             target=self.about,
             id='link-to-blog',
             safe_id=True,
         )
-        assert container['about']['link-to-blog-1']
+        assert (container['about']['link-to-blog-1'] and
+                container['about']['link-to-blog-1'] == linktoblog1)
 
         # Copy folderish content under target
-        api.content.copy(source=container.about, target=container.events)
-        assert container['events']['about']
+        about = api.content.copy(source=container.about,
+                                 target=container.events)
+        assert (container['events']['about'] and
+                container['events']['about'] == about)
 
     def test_delete_constraints(self):
         """Test the constraints for deleting content."""