Improve Comment moderation possibilities

[jensbrak]

Background: comment moderation is limited to pre-approved comments and admin required approval, as set in Manager Config. If the validation hook for comments is used, however, that hook will replace the admin approval since it will be applied after Post/Page Service has processed the comment. This means that the admin has no say about comments if the site uses custom comment validation. Either the admin decides or the hook does it. Not both.

The assumption here is that custom validation is most likely to determine if a comment is considered to be spam or not. Maybe there could be other scenarios for comment validation using hooks, and this would mean that naming will have to be carefully (re)considered.

Suggestion: Add a new property to the Comment model called IsSpam or something. This would allow for comments to have a new state in the Manager; even if a hook is used for spam detection, an admin can choose to have a final say about comments processed by the spam detection service.

This would affect the Approve Comments setting in Manager Config in the sense that if the hook for Validation is used, approved comments are only published automatically (Approve Comments set to True) if the hook is marking the comment as not spam. The spam marked comments would end up in a new tab under comments named Spam for admin to review (and not published of course). If the setting Approve Comments is False the comments are listed as they are today inside manager - with the addition of the Spam tab/filter for comments.

This would mean that the admin still can approve comments flagged by a spam filter, which could be useful and similar to what we see in mail clients. Piranha admins will then be able to look in spam filter if something mistakenly has ended up there. To be really flexible, one might consider adding a new setting in Manager Config to automatically purge spam marked comments after N number of days, just like you would do in Outlook or similar. Or perhaps even offer more units than time (days): number of spam messages or size on disk they take, in order to minimize the risk of filling a disk quota if a spam attack is directed to the site.

I haven't looked closely at different anti-spam API's more than briefly (and I'm focusing on Akismet personally) - but I think one might want to add more fields than IsSpam; maybe a spam score and a free text comment about the spam assessment result. Some services seem to provide more info than just "yes/no this is spam" and maybe that information might be interesting to list in the spam filter tab inside the Manager, to help admin determine things.

Another thing that seems relevant for some spam detection API's is REFERRER, so that might be good to add for relevant Piranha content types, alongside USER AGENT.

Also: comments are currently not aware of what content they belong to. The ContentId points to an unknown content type, that currently can either be a Page or a Post. It could be useful to provide information in the Comment to retrieve relevant information about the content it belongs to.

Comments are a great addition to Piranha and there are steps taken to assist taking anti-spam precautions and/or support moderation. However, it might be good to go a little further in this direction to make comment moderation and spam detection easier and more reliable.

TL;DR: Add some fields to Comment model to better support moderation and anti-spam. Also, consider using them in the Piranha Manager out of the box. To make it even more useful, combine with the possibility to do bulk-moderation as proposed in #1339

Thoughts?

[tidyui]

We’d be very happy to elaborate on this functionality once 9.0 is released. Maybe we could schedule this for 9.1?

[jensbrak]

@tidyui - At the moment the Comment class has the property IsApproved. I am not sure what would be the most flexible yet non-confusing way of extending the class. Another property for IsSpam would for sure be easy to add but then we would have states of the objects that would make little sense - like IsApproved = true and IsSpam = true.

One option would be to have an enum for the state instead, enum ModerationStatus { Pending, Rejected, Approved }. This could be a possible thing to apply to more than comments - I'd imagine a scenario where content of a site from one category of users is moderated by other users with higher privileges. Maybe this is to widen the scope too much but I feel like it's relevant to give it a thought first.

As for rejection, it might be good with a field for rejection info too. But if adding this, we have info that really is not part of the content, it's more like meta information about the state the content has. This would not be a logical part of the content itself.

[jensbrak]

No matter what, I will play around with it a little to see how it might feel.

[jensbrak]

@tidyui - I have a bit of a dilemma when it comes to the design choices of a modified Comment class. For the sake of simplicity and flexibility I believe an enum instead of two booleans are good - as we've discussed. IsValid and IsRejected (or IsSpam) vs Validation enum (or whatever it's best named).

However, looking at the other end of the use case we have the Manager and the Comment-moderation page with the CommentList. My incitament for another state is the comment validation hook I use. In that context it is obvious I'd like to have some sort of third option for a Comment List entry part from Approveand Unapprove namely Reject (or IsSpam). Moreover, that calls for a revised UI with perhaps a tri-state switch or something like that. But that is only logical if I use my module.

For anyone NOT using that kind of module, there's still only two options that are reaonable - Approved and Unapproved. I guess. So any modification to the core needs to work with this setup. Then I figured - it would be possible to have some sorts of conditional switch that is used if the validation hook is used and have the UI of the comments that flexibility.

But reasoning like that ends up in the conclusion that we then have an added state or condition of the Comment that denotes validation result. That's about it. So we're back at adding a third state of the comment that reflects validation result. Which might motivate that enum instead of another bool. Still, we basically add nothing but that enum as far as Core is concerned.

So the question then becomes what use one would have of it in the manager and how to enable it without messing with it for those who does not use validation hooks. Are you following my reasoning?

I have little hesitation about the technical aspects of modifying the core - the hard thing (which I assume you confront and balance in every Piranha decision most likely) is to find the balance between extensibility and usability (out of the box)... :)

Another thing I was tempted to add was a Rejection reason. With that, there's less focus on spam being the only reason for rejecting comments. A comment might be rejected (ie failed validation) for some reason (spam being one of them) and therefore that reason would make sense to have included.

[tidyui]

Personally I don't think it would be confusing to have a column in the admin comment section that shows validation status that can't be set from the manager. This field would only get set if you have a module using the hooks for comment validation, like the one you have built. If the validation module would classify a comment as spam it would then most likely set the validation status to false or whatever we choose here and set the flag for approved to false (or something equivalent).

For the manager it would be clearly visible that the comment failed validation but I can still manually change the state to approved if I desire, or the other way around.

I'll keep my input on a high level as comment moderation really isn't my field of expertise :)

[jensbrak]

I was hoping on just that - the high level input. What you do know is the continous challenge keeping the balance between flexibility and usability. I implemented the enum all the way in the Core just to give it a spin and that was a no brainer. It's when I started to look on what one would like to have in the Manager I started to (re) consinder what would be the best way of extending the comment class.

I am very interested in software design and architecture design but tend to get stuck on it since I love to fix like "everything". This is why I have to come to enjoy Piranha so much since the careful choices made makes so much sense. This is also why I open up a discussion like this because it's the most interesting (and challenging) part: to have a good balance between flexibility and functionality.

[jensbrak]

Manager UI "guidelines" aka "best practices". I had in mind a 'StatusReason' property of comments to be used to explain why a comment might have a certain status - for instance to be set by a validation module. This in addition to the rejection icon we've discussed earlier.

However, it feels kinda wasteful to use up space in the table for things most sites might not use. (First I added functionality for Manager and Core to also set this field (comment approved by setting = 'Config Approved', comment rejected or approved by toggle the button in manager = 'Manager Approved' or 'Manager Rejected'. However, I couldn't think of a reasonable way to deal with translations of the message set in core so I ditched the idea.) A column with the icon that lights up if a comment is rejected by a module, fine - that was even your suggestion. But another column with (most likely) empty space explaining "why" feels... not tidy. :) I am not a big fan of dynamic content, it's illogical - but otherwise an option would be to show the StatusReason column only if there are reasons to show. Don't like that idea personally. Another approach would be 'hover' over the icon but that's not good design either as far as I'm concerned. OnMouseOver is fine for detailing something already there but not provide (perhaps) relevant info if the user happen to hover over the right place.

A more complex solution and maybe the one to go for (eventually, but certainly not now) would be to let a module implementing validation to provide an api to get details about the validation and simply add a link on the icon routing to the details page. That way it would be compact and still informational. However, I don't feel that would be in the scope for the first version of the additional comment moderation feature.

Thoughts?

[tidyui]

@jensbrak I think we could add the column but no incorporate it in the first version. Another option for display would be that if the rejection reason isn't empty, a second line in the table could be added for the comment only presenting the reason in that row. But we can elaborate on this for the next version!

EDIT

Btw, I never really make these decisions on my own, I just let @NJepop decide as he's responsible for keeping the UI aligned and tidy :)

[jensbrak]

Ah, yes, of course you're not alone! :) Well, the first draft includes the column but that's rather minor thing to change. I'll do a PR and we'll take it from there.