Hello!

As of 7.x-2.3, the Mollom module allows forms to be configured such that "unsure" posts can either be presented with a CAPTCHA or retained for manual moderation. One of my customers would like to take this further —they would like a third option that would allow all "unsure" comments to be published immediately without further validation.

Patch forthcoming; thanks!
Adam

Comments

jazzslider’s picture

Status: Active » Needs review
StatusFileSize
new3.63 KB

I've attached a patch that should take care of the basic use case here, though I have not yet had time to run any tests on it; I'd appreciate any community review!

Thanks,
Adam

Status: Needs review » Needs work

The last submitted patch, mollom-allow_unsure-1834860-1.patch, failed testing.

jazzslider’s picture

Status: Needs work » Needs review

Hmm —that's a lot of test failures, but on reviewing them I'm not sure they have anything at all to do with the code I added. It seems as though the Drupal testing bot isn't able to reach the Mollom testing server, and that's causing a lot of processes to fail unexpectedly.

I'm putting this back into "needs review," because I think the automated tests will always fail based on the above observations —so it needs manual tests, then :)

Thanks!
Adam

jazzslider’s picture

Interestingly, I'm getting the same Internal Server Errors when I attempt to test this locally (with both "Plugin Developer Mode" enabled at mollom.com, and "test mode" enabled in the module itself). Perhaps there's an issue with dev.mollom.com at the moment?

Thanks!
Adam

sun’s picture

Version: 7.x-2.3 » 7.x-2.x-dev

That's odd — I've informed the Mollom backend engineers about those errors.

Will review and comment about the proposed change shortly.

jazzslider’s picture

#1: mollom-allow_unsure-1834860-1.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, mollom-allow_unsure-1834860-1.patch, failed testing.

jazzslider’s picture

StatusFileSize
new3.72 KB

Hello!

Looks like I patched against the wrong version (7.x-2.3 instead of 7.x-2.x, the development branch). I've updated the patch so that it should apply against 7.x-2.x, and also changed the verbiage from "allow" to "accept" per the comment I found in the code about adding these new actions.

Thanks!
Adam

jazzslider’s picture

Status: Needs work » Needs review

Whoops, forgot to change to "needs review"; fixing.

jazzslider’s picture

OK, looks like we passed the automated tests. I've also tested this out manually on my local machine, and verified that it implements the requested behavior correctly. Can anyone else confirm?

Thanks!
Adam

sun’s picture

Sorry for not replying earlier.

So, hm. Originally, this behavior was rather foreseen to be toggled through the native "unsure"/binary mode of the Mollom API. The native unsure/binary mode essentially disables the possibility for a "unsure" response in the first place, which in turn means that posts are literally treated as either ham or spam, because only these two classification results are possible.

As originally mentioned in #1817446: Allow to retain and moderate unsure posts instead of showing CAPTCHA, the precursor for the new unsure setting was actually that issue, #1096412: Allow to disable fall-back CAPTCHA for text analysis.

So the essential difference is that with the unsure/binary mode, the Drupal client/module never receives a "unsure" response in the first place, which in turn means that all of the code paths for "unsure" responses inherently cannot be reached either. At the same time, the classification result is always ham or spam, but never unsure, so one wouldn't be able to see which posts were classified as unsure after the fact. Off-hand, I cannot see a reason for why anyone would want to see that, but OTOH, it might be useful.

jazzslider’s picture

Hello!

I think either solution would satisfy my customer's use case; however, I think that following the options added in #1817446, the approach taken here would be a more predictable UX —if users are already looking at a "When text analysis is unsure" field, I think that's where users would first think to look for this.

Regarding the unsure/binary mode —that sounds like a viable approach too. However, I do think it's useful for site owners to know how Mollom makes its decisions, and being able to see whether something was "unsure" can help along those lines. One of the most common criticisms I've heard about any spam protection service I've used is that it's not easy to see how and why certain posts got rejected —transparency about that is very helpful in creating trust in the service.

Either way, though —I'd definitely like to see this capability incorporated into the module by one means or another; thanks for your help!

sun’s picture

Title: Add option to allow "unsure" posts through without further validation » Allow to accept "unsure" posts as ham without CAPTCHA or further validation (binary mode)
StatusFileSize
new4.03 KB

Alright. Attached patch combines this patch with the original #1096412: Allow to disable fall-back CAPTCHA for text analysis.

Status: Needs review » Needs work

The last submitted patch, mollom.binary.13.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
StatusFileSize
new6.87 KB

Hm. The new configuration option is not presented very well yet:

mollom-binary.13.png

1) IMHO, the new option is the least favorable, so it should come last.

2) We might want to clarify that the actual action is more fine-grained and can also mean that a post is discarded, if it's more "spammy" than "hammy". Although I'm not sure whether that's necessarily needed/noteworthy.

sun’s picture

StatusFileSize
new8.79 KB
new4.17 KB

Please ignore the test failures for now.

Only moved the option down:

mollom-binary.16.png

I think this is acceptable and ready to go.

Status: Needs review » Needs work

The last submitted patch, mollom.binary.16.patch, failed testing.

sun’s picture

Status: Needs work » Needs review

#16: mollom.binary.16.patch queued for re-testing.

sun’s picture

Status: Needs review » Fixed

Thanks for reporting, reviewing, and testing! Committed to 7.x-2.x.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

sun’s picture

Version: 7.x-2.x-dev » 6.x-2.x-dev
Status: Closed (fixed) » Patch (to be ported)
sun’s picture

Status: Patch (to be ported) » Needs review
StatusFileSize
new4.67 KB

Status: Needs review » Needs work

The last submitted patch, 0001-1834860-by-jazzslider-sun-Allow-to-accept-unsure-pos.patch, failed testing.

sun’s picture

Status: Needs work » Needs review
sun’s picture

Status: Needs review » Fixed

Committed and pushed to 6.x-2.x.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit 300acad on 7.x-2.x, 8.x-2.x, fbajs, actions by sun:
    - #1834860 by jazzslider, sun: Allow to accept 'unsure' posts as ham...

  • Commit 300acad on 7.x-2.x, 8.x-2.x, fbajs, actions by sun:
    - #1834860 by jazzslider, sun: Allow to accept 'unsure' posts as ham...