Issue status update for http://drupal.org/node/21632 Project: Drupal Version: cvs Component: node system Category: feature requests Priority: normal Assigned to: puregin Reported by: puregin Updated by: Bèr Kessels Status: patch O, and thanks for taking the time to make a patch. I never meant the above post to be rude, just as some guidelines to help you make even better patches :). Bèr Kessels Previous comments: ------------------------------------------------------------------------ April 30, 2005 - 02:57 : puregin Attachment: http://drupal.org/files/issues/node_copy_patch (4.06 KB) The purpose of this patch is to enable a copy operation on nodes. The semantics of this are: given a node, a copy is saved. Only the title is changed (and of course the node will have new nid). Hence, for example, copies of book pages are siblings of the original. Q. Are there any situations where having a copy of a node will mess up a compound/structure? Q. Should anything else be modfied (created/changed dates, ...)? Could you please review carefully - I'm pretty new to this code! Please pay special attention to: - permission checks - I'm not clear on the semantics of these - generation of watchdog entries / messages The patch makes the following changes: 1. Adds a function node_copy(&$node), based on node_submit. This accepts a node or node array, and checks if the current user has view permissions and create permissions. If so, it creates a title "(Copy of node ".$node->nid.") ". $node->title It then clears the node's nid, and submits to node_save(), which creates a new node. Appropriate messages are generated to indicate success or reasons for failure. 2. Adds an item to the $items[] array in node_menu after line 710 to allow adding copying a node '$nid' by by "node/$nid/copy". This tries to follow the way "node/$nid/delete" is handled. 3. Adds an operation 'copy' => array(t('Copy the selected posts'), ''), (at line 748) to the operations array in function node_admin_nodes() to allow mass copying of nodes (a la mass deletion of nodes). Implements this operation. 4. Adds a case in function node_page() to take care of the copy operation. ------------------------------------------------------------------------ April 30, 2005 - 03:32 : syscrusher Just a question... How does what you propose differ from the EditAsNew module? http://drupal.org/node/16806 Just curious. Scott ------------------------------------------------------------------------ April 30, 2005 - 04:25 : puregin Hi Scott, This implements copy at the level of nodes, which feels right to me. That EditAsNew exist illustrates the need for this functionality. To me, having to implement this outside of node.module is awkward and leads to potential problems with concurrency. Putting this in node.module also enables mass copying of nodes, which seems like a useful thing to be able to do. It also exposes copying in a more programmatic way. Modules which would like to support making copies could offer this to users by generating a link. This is parallel to the 'Add child' link functionality presented by the 4.6 version of book.module. On the other hand, EditAsNew has some nice design ideas also - being able to selectively copy fields might be useful. Some users might prefer to see this operation expressed as a tab. It seems like it should be possible to simplify EditAsNew based on a copy function in node.module. Regards, Djun ------------------------------------------------------------------------ April 30, 2005 - 04:35 : syscrusher Thanks, Djun, for the detailed reply. I understand better now. ------------------------------------------------------------------------ April 30, 2005 - 11:31 : Bèr Kessels A quick scan of the patch and your desciption: * always use two spaces for identation * try to avoid the use of words such as "items" * never use the word "node" or show node ids to users, they are developers stuff, not meant for users. * you add an awfull lot to the watchdog, IMO. And last but not least: do we really need this? I would vote against this functionality in core, this should really live in editasnew.module (not even a new copynode.module).