Allow trash option in comment notification emails

[nacin]

wp-admin/comment.php currently doesn't support the ability to trash a comment via the AYS form.

Additionally, wp_notify_postauthor() and wp_notify_moderator() should replace the "Delete it:" links with "Trash it:" links, if trash is enabled.

I do find this important for 2.9 - at the very least, wp-admin/comment.php should gain the ability to trash a post, even if the links are not added to the pluggable functions. That said, it does require up to three new strings...

Working on a patch, any thoughts?

[nacin]

First pass...

[scribu]

+1

[caesarsgrunt]

Oops, I forgot about the email when implementing Trash.

I notice that in your patch you have used the string "Delete" if Trash is disabled, however in the rest of the interface we use "Delete Permanently".

I don't know if this can be implemented after the string freeze, but the only alternative is really to remove the delete option from the email altogether.

[caesarsgrunt]

Actually, we probably don't need an AYS page for Trash; that's part of the point of Trash. That said, we don't use AYS for Approve anywhere else in the interface, but we do use it for the email link...

[caesarsgrunt]

action=cdc&dt=trash

We don't do this for trash anywhere else; it is an action of its own.

Actually, Spam is also now its own action in the comments page. I've just noticed that it still uses the old action=delete&dt=spam in the dashboard, which I'd better go and open a new ticket for...

[nacin]

In general, I went as simple as possible with the patch to see to a 2.9 commit.

The AYS intention check is necessary because you're coming from e-mail.

I'm not sure about "Delete Permanently." I suppose it can be changed to "Permanently Delete Comment." This patch is already requiring some string changes, so it can't hurt.

mac, cdc, and dt are all artifacts. A few years ago, they stood for moderateapprovecomment, confirmdeletecomment, and delete_type. Realistically we should convert these to action=(approve|delete|trash|spam).

But since mac/cdc/dt still need to be maintained for back compat (particularly since the notify functions are pluggable), I decided against it to keep this simple for 2.9.

I will work up a second patch to clean this up some more. Xref - #11432.

[nacin]

Second pass.

[nacin]

Second pass is more ambitious. Does what the ticket is fundamentally for, plus:

• Changes mac, cdc, dt to approve, delete, spam, trash. Adds back compat check.
• DRYs up a lot of the comment.php code. (Though it could be taken further.)
• Adds query args where missing to allow for standardizes messages after an action.

New strings:

 __('Trash it: %s')
 __('You are about to move the following comment to the Trash:');
 __('Trash Comment');
 __('Permanently Delete Comment');

[nacin]

Third pass.

[nacin]

Should have read:

• Adds query args where missing to allow for standardized messages after an action.

Third patch also converts the "No" cancel button from an <input type="button"> with an onclick JavaScript location redirect, to <a class="button"> with an href. Cause that was just driving me crazy.

[nacin]

Okay, I thought about an alternative. The following patch has no string changes, but it is less than ideal.

Nutshell:

• action=trash uses "You are about to delete the following comment:", but then gives you two action buttons: "Move to Trash" or "Delete Permanently".
• If trash is enabled, then use the action=trash link in the email, which would reveal the two buttons. The string would still be "Delete it: (link)".

[nacin]

Alternative has no string changes.

[nacin]

Removes duplicated check_admin_referer call.

[caesarsgrunt]

If we're going to rename the actions used when coming from the email, why not use the same actions that we already use elsewhere? And we could add another parameter to show that we'd come from an email and so needed an AYS - eg &email=true or &confirm=true. Still don't think AYS is needed, but that's a different ticket really and too late for 2.9.

[nacin]

Removes duplicated check_admin_referer call.

[nacin]

Removes duplicated check_admin_referer call.

[nacin]

The AYS screen is needed. If it's removed, you'd get tripped up by check_admin_referer and you'd have to answer wp_nonce_ays. Not exactly user-friendly. And removing check_admin_referer, obviously, is backwards in terms of security/intent.

Adding &email=true or &confirm=true is just another way to skin a cat -- the code would largely be the same -- except that it also unnecessarily lengthens the URL by something like 18+ characters.

---

Speaking of check_admin_referer, I left one in there that needed to be removed. Most recent patches are ​11426.alternative.2.diff and ​11426.4.2.diff (sorry about the name -- left out pluggable.php in the diff and forgot to click the "Replace file" checkbox).

[caesarsgrunt]

Oh, of course. I hadn't thought of that...

Regarding the names of the actions, I don't see any point in renaming them at all if the new names are still going to be different from the ones used in other places. There's nothing really wrong with cdc and mac - and if it's shortness you're going for, you can't beat them! (Unless you were to have like t for trash, s for spam, and a for approve... actually, there might be something to be said for that...)

[nacin]

Replying to caesarsgrunt:

Regarding the names of the actions, I don't see any point in renaming them at all if the new names are still going to be different from the ones used in other places. There's nothing really wrong with cdc and mac - and if it's shortness you're going for, you can't beat them! (Unless you were to have like t for trash, s for spam, and a for approve... actually, there might be something to be said for that...)

Here's my thought. It really isn't much to ask that the links should be short, readable, and distinguishable. All three options are short enough. However, I didn't have a clue what 'mac' or 'cdc' was until I looked back in the changesets when they were shortened 3 years ago. They're not understandable to an end user, but why shouldn't they be?

The one-character strings could work, but they're not readable or easily distinguishable.

The point of renaming them is simple. We're already creating two new actions, spam and trash, and getting rid of dt. So we might as well standardize 'approve' and 'delete' as well. They're only different because they perform a unique AYS function. And FWIW, adding "comment" to the action generates the post-AYS action, so this standardizes it and makes them alike, not different. And because of that, the code is easier, too.

[nacin]

Alternative route (no new strings) -- fix for redirect in (un)approvecomment

[nacin]

Standard route (4 new strings) -- Fix for redirect

[azaozz]

(In [12402]) Add 'trash' in comment moderation emails, props nacin, fixes #11426