Skip to content

Commit

Permalink
API Enable comments to save spam status
Browse files Browse the repository at this point in the history
  • Loading branch information
Damian Mooyman committed Mar 23, 2015
1 parent ae391bc commit c40d120
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions code/extensions/CommentSpamProtection.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class CommentSpamProtection extends Extension {

public function alterCommentForm(&$form) {
$form->enableSpamProtection(array(
'name' => 'IsSpam',
'mapping' => array(
'Name' => 'authorName',
'Email' => 'authorEmail',
Expand Down

4 comments on commit c40d120

@wilr
Copy link
Member

@wilr wilr commented on c40d120 Jun 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tractorcow This breaks any protector that uses a Text Field for the value as the value will be saved into the database. IsSpam should be internal and not presented to the user right? For example Math Spam Protection now saves the value of the answer into the database (i.e 10) which evaluates to true and therefore makes it all as spam.

My feeling is to rename this name away from IsSpam, protectors can always write back to the form data using getForm(). If you really want to keep IsSpam then we need to update all the protectors we support.

@tractorcow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh dear, I think you're right.

Can we add a new API that separates the "Sets IsSpam flag" from the actual name of the field? (which probably should be left up to the protector).

As long as we can still save spam if using the akismet field (https://github.com/silverstripe/silverstripe-akismet#comments) I'm flexible on how that is actually done. :)

@tractorcow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest a FlagField option to specify the boolean field to save into, although I'd imagine that making this a core api in spam protector would require each of the other modules to be updated. Can it be done in a non-breaking way? (old spamprotector / new field or new spamprotector / old field?)

@tractorcow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've raised an issue at silverstripe/silverstripe-spamprotection#22

I'm really not sure if there's a clear way to approach this.

Please sign in to comment.