[wp-hackers] mollom feedback (was: Spam plugin (was:Deprecated functions and files))

Tue Aug 26 11:06:47 GMT 2008

Hi there!

On Mon, Aug 25, 2008 at 3:45 PM, Ozh <ozh at planetozh.com> wrote:

> >So far, I've been developing this plugin since april. It's currently in
> >public beta (version 0.6.0) Maybe you should give it a try. I'm keen to
> >receive feedback!
>
> What, what? Someone asked for nosy and snoopy unauthoritive comments?
> Sounds like a job for me!
>

Thanks! :-)

> A few comments & suggestions:
>
> - you got the translation thing wrong. It's not _e('string'), it's
> _e('string', 'domain') after you've load_plugin_textdomain('domain',
> $path_to_mo_file)
>

Well, i18n is something that's planned. I skimmed through the documentation
so I know it's faulty. As far as bugs go, as long as the core functionality
of the plugin still contains nasty bugs, translation support is not my
priority.

> - I personally hate when plugins add 15 rows in wp_options when one is
> enough. Instead of add_option('mollom_this',$molthis) and
> add_option('mollom_that',$molthat), why not
> add_option(array('mollom_this'=>$molthis,'mollom_that'=>$molthat))? Much
> cleaner on the DB.
> More on that: <
>
> http://striderweb.com/nerdaphernalia/2008/07/consolidate-options-with-arrays/
> >
>

Hm. I can relate to that. My own database dates back to 2005. I tried a lot
of (deprecated) plugins. You should see my wp_options table. A real mess.
Using an array to save and load all the parameters is a nice solution.

Then again, I can see Otto's point: MySQL can perfectly handle a table with
several of hundereds, albeit unused, records. The sting is in how much the
database gets hit. So, certain values like the private and public keys,
should be cached. I'm going to rethink when and where I call the other
parameters to get things more optimized.

Is it feasible to combine both solutions?

>
> - upon activation edit-comments.php?page=mollommanage spits "Warning:
> Division by zero on line 317" like mad.
>
> - I don't see the point of the subpage under "Plugins" (with the same
> division by zero mayhem, obviously)

Not seen that error before. You went right after activation in the Mollom
Manage panel? Perhaps it's a conflict with the 'mollom' menu item in
'plugins'. Speaking of which: I'm not to happy with it being located there.
It only contains statistics.

Suggestions for a better location? Can I hook it to the dashboard perhaps?

>
> - the default message when a message is refused for lack of configuration
> should not be "You haven't configured Mollom yet! Per the website's
> policy. We could not process your comment.", it should be something like
> "The site admin blabla and your comment blabla". Potentially less
> confusing for commenters who might think "wtf i didn't configure what??"
>
> - I'm not sure the "remove settings upon deactivation" thing is a good
> idea: deactivation != uninstall, and deactivation happens a lot (auto
> updating the plugin, deactivating plugins prior to upgrading WP, etc...).
> As this option is on by default, this is going to confuse & upset a few
> who'll lost everything quite often.
> I'd suggest a manual "Remove everything from DB" button that would be
> pressed when the user wants to uninstall, as well as some complying to the
> upcoming 2.7 uninstall feature.
> More on that:
> <http://www.santosj.name/general/wordpress-27-plugin-uninstall-methods/>
>

Well, it's a case of semantics :-) But I like the button idea! Regarding
your earlier remarks: I implemented this to clear out the database when the
plugin gets deactivated. Default this option should be 'off' in the
configuration panel. So that's something I'm going to check out. In normal
circumstances, you should be able to deactivate the plugin by default
without purging your database. Unless you really want to deïnstall the
plugin and wipe all data clean.

>
> - Since you have a cool logo & favicon, I'd suggest adding something like:
> add_filter('ozh_adminmenu_icon', 'mollom_icon')
> function mollom_icon($hook) {
> if ($hook == 'mollommanage' || $hook == 'mollom-key-config')
>        return $url_to_local_copy_of_favicon.png;
> return $hook;
> }
> That would be very cute with the awesomely awesome Ozh' Admin Drop Down
> Menu ;)
> More on that: <
>
> http://planetozh.com/blog/2008/08/admin-drop-down-menu-more-goodness-an-api/
> >
>

Going to check it out :-)

>
> - I don't get why anti spam plugins insist on creating their own table. Or
> is WP missing a comments_meta table?
> Is there nothing you can do with columns 'comment_karma' which is unused
> AFAIK or 'comment_approved', like storing custom values like
> 'mollom_unsure' ?
> There's no deep thinking behind this comment, I admit it, but I find it
> strange that every plugin needs a table where Akismet doesnt.
>

Well, I went with this remark:

http://pressedwords.com/mollom-anti-spam-plugin/

I'm not a fan of using unused WP table fields. Maybe they'll be used by WP
in the future and other plugin developers might start to use the same field.

First installments of the plugin created an extra table column on
wp_comments, but altering the default WP model isn't a clean way of doing
things. So I went for a seperate table all together.

>
> - I'd split the plugin in various files and conditionally include them:
> nothing at all should load when someone is viewing a public page, the
> require('class-IXR.php') should occur only when a comment has been
> submitted, the whole captcha stuff should load only if a captcha is
> needed, etc...
>

I'll check it out.

>
> - Similarly but more importantly to me, most of the plugin should not load
> "if (is_admin())": unless you consider that a blog owner is a potential
> spammer for their own blog, your plugin will eventually conflict with
> plugins that allow them to reply to comments from within the admin
> interface.

Well, I'm checking against logged in users all togheter in
mollom_check_comment. If a logged in user makes a comment, the plugin code
isn't even executed at all.

I can see now that this quite a primitive way of doing things...

>
>
> All in all and despite my rather numerous comments, this is a plugin I
> like. Much much much muuuuch cleaner than Defensio who, regardless of how
> efficient it is, is really messy. I'm going to give mollom a try (when you
> group all the options into a single DB entry, that is :)

Thanks!! I aim to please ;-)

With regards!

Matthias

>
> --
--
http://www.netsensei.nl