This is more or less a duplicate of #364575: Support alternative spam rejection methods than a CAPTCHA (e.g. unpublishing), but it's 1) for D6 and 2) totally simplified to the most common use-case. That other issue should focus on D7 and investigate advanced configuration scenarios, possibly integration with other modules, or not, and how to approach a user interface for them.
Here, we just want to allow: If Mollom identifies a post as spam, allow me to configure that I want to put the unpublished post into the moderation queue instead.
Quite simple, but still, quite challenging kung-fu. ;)
Comment | File | Size | Author |
---|---|---|---|
#100 | mollom-HEAD.bogus-feedback.100.patch | 20.7 KB | sun |
#96 | mollom-DRUPAL-6--1.bogus-feedback.96.patch | 8.28 KB | sun |
#95 | mollom-DRUPAL-6--1.bogus-feedback.94.patch | 6.85 KB | sun |
#93 | mollom-HEAD.bogus-feedback.92.patch | 21.05 KB | sun |
#93 | mollom-DRUPAL-6--1.bogus-feedback.93-d6.patch | 9.9 KB | sun |
Comments
Comment #1
sunOne of the key questions: After Mollom identified a post as spam, what happens if a client requests and checks a CAPTCHA afterwards?
1) Can the server use that additional data somehow or is it totally useless?
2) Or is it actually bad to additionally do a CAPTCHA, and the client should rather just accept the post, if it wants to?
3) Should Mollom servers get to know that we did NOT block the post on the client-side?
Comment #3
Dries CreditAttribution: Dries commentedAfter Mollom identified a post as spam, what happens if a client requests and checks a CAPTCHA afterwards?
Could you elaborate? I'm not sure I understand the question/scenario.
The client only gets a CAPTCHA when the post is 'unsure'. The scenario you describe might not exist.
Comment #4
Dries CreditAttribution: Dries commentedThe challenge with communicating the a score, is that it allows spammers to tweak their content until it is less spammy. They could do that on their own site, and than re-use the content on other sites. It makes more Mollom vulnerable.
Shouldn't this only be applied to comments?
Redundant?
This if-test puzzles me. We're already in 'case MOLLOM_ANALYSIS_UNSURE' so the if-test seems redundant. Either way, I don't really understand this change.
Powered by Dreditor.
Comment #5
sunAlright, let me clarify what this patch does:
If you absolutely do not feel comfortable with showing a CAPTCHA for "spam" submissions, then we can surely remove that part from the patch.
We already have a similarly special mapping property -- 'post_id' is only used internally to let our form processing know where to find the id of an entity in the submitted form values. This patch merely advances on that by saying: If the entity, which belongs to your registered form, supports a publishing status, then you use the 'post_status' property to let us know where to find (and set) that status value in the submitted form values.
Effectively, only forms that specify the 'post_status' property are able to accept (and unpublish) spam posts (instead of rejecting them).
You additionally asked for a "spam callback", which would allow (other) modules to alter Mollom's default decision (accept/reject) and possibly also do custom changes to the submitted form values. While that would certainly be possible, I'm not sure about concrete use-cases where it would be helpful. Overall, I think this idea rather relates to #364575: Support alternative spam rejection methods than a CAPTCHA (e.g. unpublishing)
Comment #6
Dries CreditAttribution: Dries commentedImplementing the option, I thought that it would be illogical to show a CAPTCHA for "unsure" submissions, but not for "spam" submissions. Compared to "unsure", I figured that "spam" can just be a slightly more confident decision, but may not be a 100% confident. If "unsure" is 50%, then "spam" can be anything between >50% and 100%. The client only gets the decision, but not the rating. When not showing a CAPTCHA for to-be-unpublished spam submissions, we would may be showing a CAPTCHA for a 50% confident rating, but not for a potential 55% rating.
I don't follow this. I'd like to see the following behavior:
I need to think more about 'post_status' -- it is not a good variable name, IMO. It feels like the code that deals with 'post_status' makes too many assumptions about how $foo->status works. What if I have a complicated workflow, and status has 5 states?
Comment #7
sunOk, let's do it without CAPTCHA then :)
For now, and even more so in Drupal 7, $entity->status is always what we expect here if it exists; a Boolean whether a post is published or not. Re-using or abusing $entity->status to implement workflows is still an open core discussion, which we'll hopefully focus on for D8 or D9, but also, the current discussion rather tends to an approach similar to Flag module, i.e., separate storage columns per workflow state.
Bottom line being for D6 and D7: If your entity supports the concept of a publishing status, then there must be a property which holds that Boolean. Most contributed modules simply follow the $entity->status property of Drupal core entities, which means that it has become an unofficial standard. By using the name 'post_status', we allow developers to directly associate it with the commonly used $entity->status property, just like we do for $entity->id (post_id). So we are purposively making assumptions about how those properties work.
Comment #8
sunIncorporated #6.
Comment #9
sun#8: mollom-DRUPAL-6--1.reject.8.patch queued for re-testing.
Comment #11
sunok, this one should pass.
Comment #12
sun#11: mollom-DRUPAL-6--1.reject.11.patch queued for re-testing.
Comment #13
Dries CreditAttribution: Dries commentedI'm still not convinced I like the post_status handling and the assumptions it makes about the status value. While it might be OK for now, it will backfire at which point we'll have to break our APIs and back out the changes. These assumptions are generally a bad idea because they are based on share coincidence -- in my mind, it doesn't really make sense to make those kind of assumptions.
For example, the next thing I'd like us to work on is more advanced moderation queue. It would be good to separate spam posts from profane posts.
Depending on the configuration, ham posts might also being moderated/unpublished. For example, I still moderate all ham posts on my blog.
This solution is a bit of a quick fix -- it doesn't provides the best user experience as there might be different kinds of unpublished posts that need different ways of interacting with them. Ideally, we'd be able to separate moderated spam posts from moderated ham posts by showing them in 2 different queues. When we show them in different queues we can also optimize the operations (e.g. show a quick 'approve' link) as well as optimize the information presented (e.g. show whether a CAPTCHA was required, whether profanity was detected).
We don't need to do that now but we'll certainly want to add this kind of functionality in the future. For any big site that gets hundreds of spam comments a day, mixed with ham comments, we can't just lump things into one queue.
Going forward, we'll also want to add features like "Automatically delete spam comments after x days". Some of our users have hundreds of spam comments a day. Having to manually delete hundreds of spam comments is a real pain.
It feels like the current approach is trying to abstract things too much whereas reality might be that we need to add comment module specific code to create the best experience.
If the form doesn't support it, I'd suggest we don't show the option.
Not sure I follow this comment. It might make too many assumptions about people's understanding of how things work.
Comment #14
Dries CreditAttribution: Dries commentedThinking about this some more from an end-user point of view.
Assumption: most new end users expect that we store spam comments in a moderation queue. So instead of
'Unpublish spam posts instead of rejecting'
maybe we should haveDelete spam comments after [ x ] days
and always store spam comments? Just thinking up loud, trying to look at this with a fresh pair of eyes from an end-user point of view.Comment #15
sunThank you for reviewing and sharing your ideas, Dries!
Trying to address all issues separately:
- Unpublish if spam
- Unpublish if unsure, after passing CAPTCHA
- Unpublish if ham
- Unpublish if profane
separately will ultimately turn into a star trek trooper cockpit of options and basically means that a proper and future-proof solution attempt follows a trigger/action workflow within a form submission context:
Implementing something like this will take some time to get right and could easily duplicate the Rules module, that's loved & hated by many. For now, we need to solve the pressing spam rejection problem for false positives at hand, and may even slightly advance on the functionality later on.
Note that with regard to Comment module only, "Unpublish if ham" is equal to configuring comments to be unpublished/moderated by default.
Comment #16
Bojhan CreditAttribution: Bojhan commentedRegarding #14. I do feel its common for users to expect spam to be stored somewhere, rather then rejected - this because its often how spam in terms of e-mail is treated as well from a user p.o.v. Especially with the introduction of profanity filter I see the need for such a listing and option to be important.
I do wish to note that the current indicator of spam is somewhat strange, unpublished sure - but when looking at a list of comments the unpublished background doesn't trigger the "ohh this is spam". Is there anything we can fix with that? Instead of adding a *new* adding *spam* indicator or so?
Comment #17
Dries CreditAttribution: Dries commented- Bojhan, that is exactly why I think we need to come up with better displays for comment moderation, and possibly different/additional status handling.
- Sun, not sure about using the Views module for that. I'd prefer not to create a dependency on Views at this point. At Acquia, we have 15.000+ Drupal 7 sites that are using Mollom -- we don't have the Views module enabled for those sites.
- Sun, I'd like to see us experiment with an alternative approach to post_status that does not rely on non-standardized publishing status values. It doesn't have to work but it would be great if we could compare and contrast code. Maybe timebox the amount of time invested to 2-3 hours or so to make sure we're not going down a rat hole.
- Here is how I think about the navigation:
Comment #18
sunI've reworked this patch to add a "reject callback" instead of a 'post_status' mapping. The behavior is almost identical: if a reject callback is defined for a protected form, then Mollom does not reject bad submissions, and instead leaves it to the defined reject callback to properly change the submitted form values in a way that it won't be published, resp., end up in a moderation queue.
Regardless of which way we choose, it always has to be an optional property in the form definition, because not all implementing modules support the concept of a publishing status.
However, for me, the reject callback design is pretty much code duplication. As mentioned in #5, I still think that a simple form value mapping ('post_status' or similar) is sufficient for most if not all modules in D6 and D7, and we can still add an alternative reject callback, if a module happens to need it. The other major concern I have is that a reject callback would make integration of other modules with Mollom module more complex, and we would be putting lots of (too much) power in the hands of other module authors, while Mollom's own API documentation clearly states that clients should normally reject posts that Mollom evaluates as bad. Thus, while Mollom module might ensure what exactly happens for rejected core entities, we'd be no longer able to ensure the same for other modules/entities, because they can technically do whatever they want in that callback. Compared to that, a simple form value mapping that is either supported or not, keeps the power of intended reject actions in Mollom module's hands.
We should discuss the moderation queue and other UI aspects like the cron purge in a separate issues, as those are entirely different topics having to touch other parts of the module.
P.S.: I think we need a new word, like "bad post" or "bad submission". "Spam", "profanity", etc. doesn't cut it anymore, if we mean a post that is spam or profane or both.
Comment #20
Dries CreditAttribution: Dries commentedI personally like the callback approach better so let's continue down that path for now. It gives us more flexibility and while a bit more verbose, it is actually easier to read and understand.
Here are some thoughts on the latest patch:
It would be good if we could explain what you mean with 'negated form widget and value handling'. The code comments are great, but they assume that the reader understands what 'negated form widget and value handling' means.
Note that this became clear after reading the remainder of the code in this patch. A bit more documentation would have eliminated the initial "WTF?", but clearly it is not the end of the world. As we all know, even good code has some WTFs. ;-)
One possible way to get rid of the negated checkbox stuff is by using radio buttons instead of a single checkbox. That could actually be a good thing for the end-user as we can explain the difference a bit more carefully. In addition, it would clean up the code a little bit and eliminate the WTF. Thoughts on that?
Is the reject callback only for 'spam', or for both 'spam', 'profanity', etc? Looking at the code, it only seems to be called for spam right now. Hence, I'd suggest renaming the callback 'spam reject callback'.
However, we probably want to have a callback for profanity too. In other words, maybe we should keep it as 'reject callback' but figure out how/when to call it for profanity. In that case, we might also need to reword the newly introduced setting, or add a second configuration setting.
I think we're really validating the 'Mollom response' so maybe instead of 'status' we should use 'response'?
Comment #21
Dave ReidWould it be possible to use the trigger/action system instead? Then this could be managed by users themselves. Mollom provides a 'Comment is analyzed as spam' trigger that could be linked to the 'Unpublish comment' action. Same for nodes.
Comment #22
sun@Dave: That's the amount of complexity I was specifically trying to avoid. #364575: Support alternative spam rejection methods than a CAPTCHA (e.g. unpublishing) is discussing that approach in detail already. Likely results in 100% duplication of Rules module.
Comment #23
Dries CreditAttribution: Dries commentedIntegration with Rules modules is beyond the scope of this issue, IMO. It also doesn't provide the kind of user experience that I envision for such a fundamental feature.
Comment #24
sunre #20:
1) Although I can understand the idea of using radio buttons, the UX principles are pretty straight here: Two radio buttons is always a checkbox.
2) The D6 module does not implement profanity checking (yet). I don't think there is a difference between spam and profanity for reject callbacks. They can check the response in $form_state, if it is important for them.
3) I've renamed the last new form validation callback into mollom_validate_post(), as that also matches its phpDoc - the validation handler is for post-validation tasks only.
Sorry for also cramming that $GLOBALS refactoring into this patch. It's required, because we'd have to ensure that the global values equal the form state values in some more locations for this issue, so we rather want to change it in the right way directly. Working on this, I recognized that the module's global variable handling can easily break - too easily, actually. Since that global variable code hasn't changed for a long time, I need to look up what actually requires it, and how we can fix it. Separate issue.
EDIT: oh, and I also revamped lots of comments ;)
Comment #25
Dries CreditAttribution: Dries commentedA checkbox usually means 'enabled' or 'disabled'. This situation might be a bit more subtle, and because of that, I think radio buttons could work. We're offering the user two mutually exclusive options (i.e. reject spam or unpublish spam) and we're expecting the user to select exactly one choice. While it could be combined into a single checkbox, it does not have to because it doesn't have to be formulated as 'enabled' or 'disabled'. One can argue that we dilute the two mutually exclusive options by 'cramping' them into a checkbox. That said, I'm OK with a checkbox if you feel strong about keeping it a checkbox.
Comment #27
Dries CreditAttribution: Dries commented#24: mollom-DRUPAL-6--1.reject.24.patch queued for re-testing.
Comment #29
sunRe-rolled.
Comment #30
sunNote, however, that this patch contains no tests yet and that we likely want to postpone a commit to after the next stable release.
Comment #31
Dries CreditAttribution: Dries commentedAlso, "0 pass(es)" so the green is not really green.
Comment #33
sunHm. So let's follow the usual procedures and do this for HEAD first; so we get proper test results.
Comment #35
sunThe automated deletion of not published posts gets too complex without #909484: Add {mollom}.form_id and store originating form_id for session values, so let's do that first.
Comment #37
Dries CreditAttribution: Dries commentedI committed #909484: Add {mollom}.form_id and store originating form_id for session values.
Comment #38
Dries CreditAttribution: Dries commented#35: mollom-HEAD.reject.34.patch queued for re-testing.
Comment #40
sunSo something like this could work. Didn't have time to test yet.
Comment #42
sunSorry, merely a update number hiccup.
Comment #44
sunComment #46
sunSo the remaining challenge to resolve is this @todo:
Comment #47
sunoh yay! This one works quite smoothly already. Now going to write some tests ;)
Comment #49
sunSo this is the first actually working patch, which includes a first stab at tests. (I purposively left trailing white-space for absolutely required @todos in this patch)
The currently proposed implementation, which we should discuss, is based on the following:
'reject callback'
, which basically is a form validation handler (like any other), but only invoked from Mollom's form validation handler, if the form was configured for delayed rejection and the post contains either spam or profanity. The reject callback's job is to manipulate the form values in a way that the post goes into a "moderation queue" (which most commonly means to set an "unpublished" flag for the post's 'status' value).mollom_data_unreject()
when an existing post is published/moderated. If this invocation fails for any reason, the post will be rejected (deleted) later on. This is the scary part of the otherwise fantastic delayed rejection idea, so let's make sure we think about ways to prevent us from incorrectly deleting valid posts.'entity delete multiple callback'
, which is very common for modules following the API design of Drupal 7 core. The function takes an array of entity ids to delete. Popular core examples are node_delete_multiple() and comment_delete_multiple().reject callback
unpublishes it, so it goes into moderation queue.mollom_data_unreject()
, which marks the post as ham/not-profane (+ ideally also sends feedback to Mollom) and clears the{mollom}.reject
flag of the stored Mollom session data.In this case, the following last step does not occur.
reject
flag set. The posts get grouped by theirentity
, and lastly, theentity delete multiple callback
is invoked with the corresponding ids.mollom_data_unreject()
needs a better name ;) Also, during writing this summary, I figured that this function can take over the entire Mollom session data handling and should just accept $entity and $id as parameters.entity delete multiple callback
is not yet validated in the registered form information; onlyreject callback
is. Both have to exist to make delayed rejection work.Working on this patch made me realize that our current binding on form_ids is not really perfect for all entity-kind of operations. If we would not support entity-less forms, then we could potentially re-use and integrate with parts of the existing entity API in D7. However, I think it is unlikely that we are going to change that, and given D7's limited entity API, it may also be not worth to consider integrating with it. But speaking macro-level, we are facing the challenge of mapping entity data to arbitrary forms and form data, on performing operations on either data type. While we were happy with just caring for form submissions in the past, we now face the need of having to perform actions in situations when no form might be involved.
I hope this summary helps. I'd love to clarify everything that needs clarification.
Comment #51
sunRe-rolled against HEAD.
Comment #53
sunOdd test failures.
Comment #55
sun#53: mollom-HEAD.reject.53.patch queued for re-testing.
Comment #57
sunSo it turns out that my PHP's error_reporting was bogus, so I didn't see some problems.
Comment #58
Dries CreditAttribution: Dries commentedMy personal preference would be to use radio buttons. I think that could make this option more explanatory as people could properly weigh the two alternatives.
It itches me that the description is inverted; i.e. instead of 'Enable' is starts with 'Disable'. This trips people up. I had to read it twice to grok it.
I wonder if there is a better name for the hook_mollom_reject.
You have to implement the reject callback in order not to reject the post. See what I mean? One would think you implement the reject callback to reject the post.
I can't think of a better name though.
Powered by Dreditor.
Comment #59
sunChanged a couple of things to use "moderation" and "moderate". The new names make much more sense now.
The last remaining potential issue I see is the administrative UI. Would love to hear some feedback on that! :)
Comment #61
sunAdded mollom.sendFeedback to the fake server implementation. Also, since we're using radio buttons now, we have to make sure that a form configuration value for 'reject' is submitted.
Comment #63
sunSo this one finally passes for me locally. However, I just realized that I still need to fill in some @todos in the test.
Comment #64
sunCompleted the tests.
Comment #66
sunStupid me :P
Comment #68
sunSorry for cluttering up this issue. This one should finally pass now.
Comment #69
Dries CreditAttribution: Dries commentedI'm still struggling with the language/terminology.
I'm not sure this makes it crystal clear.
The word 'block' is ambiguous and so is the word 'moderate'.
Maybe it should read more like:
- Discard all bad posts and avoid them being in the database
- Keep a copy of all bad posts for moderation or review
I'd also like to figure out a better title.
I also think that 'discard' might be a better name for the database field than 'reject'.
In the future I can actually see us nuke the 'discard' option, and to always go with the 'keep copies' option. With the right automated clean-up, this becomes a no-brainer IMO.
The content doesn't /need/ to be moderated. Shouldn't that be /has been/ moderated?
Comment #70
Dries CreditAttribution: Dries commentedThe more I think about it, the more I'm leaning towards always keeping a copy in the database, and just adding an auto-discard feature. How do others feel about that?
Comment #71
sunTweaked the admin UI options.
re: {mollom}.moderate, the 0/1 in this column indicates whether a post needs or should go through manual moderation. An alternative would be 'needs_moderation', but I think that is too lengthy. Everything with a 1 in there will have to show up in a moderation UI, or alternatively, will be automatically deleted after a certain time; depending on what we are going to implement next :)
Comment #72
Dries CreditAttribution: Dries commentedThe UI looks perfect now. :)
Can we name this field to 'discard' instead of 'reject', and can we update the description to use the words 'discard' and 'retain' instead of 'reject' and 'unpublish'? For example, in the future, the contact module's mails could also be 'retained' (but that isn't necessarily 'unpublished'). Retained is more accurate or more generic.
I still think this is a poor name. They don't really 'require' moderation, do they? The comments are retained for review. Should this variable be called 'retain'?
I wonder if this should be called 'retention callback'.
Comment #73
sunImproved the tests, but did not rename the 'reject' column yet.
Comment #75
sunRenamed 'reject' to 'discard'. Also, interesting test failure -- the admin UI does not pre-select the first available protection mode if the registered form does not define one. :)
Comment #77
sunSorry, I forgot that reset() returns the array value and not the array key.
Comment #78
sunDoesn't match.
But aside from that, I'm really happy with this patch now.
Powered by Dreditor.
Comment #79
Dries CreditAttribution: Dries commentedWe're very close!
We should write 'discard' and not 'block', not?
We should write 'Retain' instead of 'Unpublish'. Whether the post is unpublished is up to the module.
Comment #80
sunAwesome, thanks! Additionally changed "moderate" into "retain" in the administrative form configuration overview/list, so we see either "discard" or "retain" there. "moderate" didn't make much sense in that context.
Comment #81
Dries CreditAttribution: Dries commentedSuperb! Committed to CVS HEAD. There is important follow-up work to do, but this will/should make a lot of people happy.
Comment #82
Dries CreditAttribution: Dries commentedI'm going to re-open this issue because we didn't got it 100% right. See screenshot and attached patch.
Comment #83
Dries CreditAttribution: Dries commentedSorry, this was a non-issue.
Comment #84
sunBackported to D6, and ran all tests manually.
Comment #85
Dries CreditAttribution: Dries commentedThis looks good.
Reviewing the code, I found a couple of things, none of which should hold back a commit.
Looking at the code again after a week or so, it's still clear that the 'retain' vs 'moderation' nomenclature is a bit of a WTF. More detailed feedback below.
Should be retain instead of moderate?
See above.
We don't check 'discard' although the code comments make it sound like we will.
It feels like, in a follow-up patch, we may want to move some existing tests over to this new class?
I felt like this should have been called testRetainModeration(). Update: I was once again confused by the moderate vs retain terminology. Nevermind.
What's this?
Powered by Dreditor.
Comment #86
sunFixed all of the review issues -- except for the condition that checks for 'require_moderation', since the value of 'discard' is merely the form configuration value. We can revisit that in a follow-up though.
Comment #87
Dries CreditAttribution: Dries commentedThanks for incorporating those suggestions. Feel free to commit this patch if the testbot is happy with it. I'm at an airport and the firewall won't let me commit it.
PS: I haven't tested the patch myself.
Comment #88
sunYes, tests are passing. As always, I think we will manually test the entire code prior to publishing the next stable release.
Thanks for reporting, reviewing, and testing! Committed to D6.
A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.
Comment #89
sunI'm still seeing bogus mollom.sendFeedback attempts in my site's logs.
Attached patch adds some extensive test coverage for new, published HAM/UNSURE comments. Actually, they should fail, but they don't want to, and I cannot explain why yet. In HEAD, that is. I'll go ahead and backport + run them on D6 now, which is where I actually see the bug. Perhaps it just doesn't exist in HEAD only.
Comment #90
sunTo clarify the remaining bug that we need to fix, this is the typical sequence of log messages I see in my logs:
The
Error -32700
can be ignored - looks like the new backend still bails out on 'ham' feedback.Somehow, I think that the added tests are doing exactly the same. But I also ran them in D6 now, but don't get any failures. :(
Actually, I also understand why this happens -- because comment_save() unconditionally invokes the comment-published hook, regardless of previous comment publishing status, so our code runs for any comment that happens to be published, while we're only interested in comments that transition from unpublished to published -- but we totally need to be able to cleanly reproduce the bug in order to fix it.
Any thoughts or ideas?
Comment #91
sunThis is how I think we could solve this problem -- at least for D7/HEAD.
The key part is to copy ->status into ->original_status, and comparing that prior to marking anything as moderated.
Comment #93
sunAttached patch fixes the remaining issues with retaining bad posts. To ensure that we are catching the bug, I've backported the additional Comment module integration tests, and they fail now.
Comment #94
Dries CreditAttribution: Dries commentedI think 'xxxFakeServer' would be a slightly better name. TestServer got me initially confused with the 'test mode' or 'developer mode'. You're actually using the words 'fake server' in the documentation so why not consistently use it in the API?
Oopsie. Remove the 'x'. Happens a few times in the code.
Comment #95
sunSo we agreed on putting the patch for D7 on hold for now, as we don't know whether and how we will actually use the data yet.
For D6, we just want to remove the ->moderate flag and related functionality. Merely keeping the discard/retain option in the form configuration.
Attached patch does that. Will run tests manually now to verify.
Comment #96
sunForgot mollom_test.module. This one passes all tests in D6.
Comment #97
Dries CreditAttribution: Dries commentedCommitted to DRUPAL-6--1. Moving to Drupal 7 now...
Comment #98
mgriego CreditAttribution: mgriego commentedsub
Comment #99
sunComment #100
sunRe-rolled against Mollom and Drupal HEAD.
Comment #101
sunThis patch is back on the table now, at least for D7.
@Dries needs to decide on whether we want to "revert-revert" the changes for D6 (originally added, rolled back in #96).
Comment #103
sunComment #104
1kenthomas CreditAttribution: 1kenthomas commentedDid this get implemented or is it still pending?
The issue I have is that giving users access to Mollom results in the ability to delete posts without an audit trail / record of the posts. That is not an authority which I usually wish to grant to any user but root (if root-- still violates the "preserve all user input" principle).
Thanks/bedankt.
Comment #105
1kenthomas CreditAttribution: 1kenthomas commentedOoops, did not mean to alter version, but might as well make current -dev.
Comment #106
sunThis feature is part of 2.0 already. You can configure whether spam posts should be discarded (default) or retained (as unpublished). The option is only available for entities that actually support a notion of a unpublished status (which normally goes hand in hand with a moderation queue).
I need to check which parts of the patch are still to be considered, as it appears to me that some parts have been extracted and committed through other issues/patches in the meantime.
Comment #107
1kenthomas CreditAttribution: 1kenthomas commentedThanks again. I'll install and review.
Comment #108
sunThe essentials of the follow-up patch have been committed via other issues to 2.x already.
Therefore, we can finally close this issue as fixed. :)
Thanks all!