Reworked the code a bit and introduced a confirm dialog to prevent users to clone nodes by mistake.

CommentFileSizeAuthor
#5 clone_settings_5.diff6.18 KBpwolanin
#4 clone_confirm-1.patch.txt2.71 KBEgon Bianchet
clone_confirm.patch.txt2.35 KBEgon Bianchet

Comments

Egon Bianchet’s picture

Egon Bianchet commented
Status: Active » Needs review
pwolanin’s picture

pwolanin commented

Well, I think this is a generally good idea, but I think that not all admins will want to force a confirm screen. For example, on the site where I'm using it, only a very few people have the permission, and I'm using it for making lots of copies of calendar events, etc.

I think to put this in place, I'd want a settings page (maybe access based on "administer nodes" permission), where the admin can choose to require a confirm from regular users and whether to require one from users with "administer nodes" permission.

Then in the callback, check these permissions before deciding whether to display the form, or execute the cloning without confirm.

pwolanin’s picture

pwolanin commented
Status: Needs review » Needs work
Egon Bianchet’s picture

Egon Bianchet commented
Status: Needs work » Needs review
StatusFileSize
new clone_confirm-1.patch.txt2.71 KB

I've implemented it as a permission

pwolanin’s picture

pwolanin commented
Title: Code rework + confirm » new settings for node clone
StatusFileSize
new clone_settings_5.diff6.18 KB

Combining this issue with http://drupal.org/node/80452

The attached patch covers both requests. I'd rather see the "require confirm" as a setting than a permission, since otherwise there is no way that a site admin (user #1) can use this feature.

Please give me your feedback on these changes.

Egon Bianchet’s picture

Egon Bianchet commented
Status: Needs review » Needs work

it's ok for me.

Just a thought about the publishing options: IMHO they should be placed in the content type settings pages with a form_alter

pwolanin’s picture

pwolanin commented

Hmmm, I like having the setting on one page, since I think putting them on each of the node types makes it harder to set them all. Is there an official "best practice'?

Egon Bianchet’s picture

Egon Bianchet commented
Status: Needs work » Reviewed & tested by the community

I couldn't find anything about that in the handbooks ... anyway it's not something that will bother me :-) . You could commit it like this and eventually change it if someone will come up with solid rationale.

pwolanin’s picture

pwolanin commented
Status: Reviewed & tested by the community » Fixed

patch committed

pwolanin’s picture

pwolanin commented
Status: Fixed » Active
pwolanin’s picture

pwolanin commented
Status: Active » Fixed

no- fixed!

pwolanin’s picture

pwolanin commented
Status: Fixed » Closed (fixed)

  • Commit 09ce60b on 4.7.x-1.x, 4.7.x-2.x, 5.x-1.x, 5.x-2.x, 6.x-1.x, 7.x-1.x, master, 8.x-1.x by pwolanin: 
    #81882 and #80452 added new features- confirm screen and resetting of...