Re: [habari-users] Re: Disabling Comment Moderation?

0 views
Skip to first unread message

Michael C. Harris

unread,
Apr 2, 2008, 4:43:08 AM4/2/08
to habari...@googlegroups.com, Habari Dev
On Wed, Apr 02, 2008 at 12:42:16AM -0700, Braden wrote:
>
> One thing about the PreApproved plugin is that it seems to cause an
> "Internal Server Error" when I activate it or (attempt to) deactivate
> it. It seems to happen due to a call to EventLog::register_type in
> its action_plugin_activation function, and also
> EventLog::unregister_type in its action_plugin_deactivation function.
> Do those need to be there...?

That's an excellent question. I've CC'd this to the developers list.
There's a comment in the code that asks if we should be unregistering
the 'PreApproved' type. I haven't looked into the implications of
unregistering a type (yet); perhaps someone can now say if it's a bad
idea? The evidence above suggests it may be ...

--
Michael C. Harris, School of CS&IT, RMIT University
http://twofishcreative.com/michael/blog

Scott Merrill

unread,
Apr 2, 2008, 11:09:47 AM4/2/08
to habari...@googlegroups.com, habar...@googlegroups.com
On Wed, Apr 2, 2008 at 4:43 AM, Michael C. Harris
<michael...@gmail.com> wrote:
>
> On Wed, Apr 02, 2008 at 12:42:16AM -0700, Braden wrote:
> >
> > One thing about the PreApproved plugin is that it seems to cause an
> > "Internal Server Error" when I activate it or (attempt to) deactivate
> > it. It seems to happen due to a call to EventLog::register_type in
> > its action_plugin_activation function, and also
> > EventLog::unregister_type in its action_plugin_deactivation function.
> > Do those need to be there...?
>
> That's an excellent question. I've CC'd this to the developers list.
> There's a comment in the code that asks if we should be unregistering
> the 'PreApproved' type. I haven't looked into the implications of
> unregistering a type (yet); perhaps someone can now say if it's a bad
> idea? The evidence above suggests it may be ...

I don't like the idea of unregistering log types when the plugin that
provides that log type is deactivated. It means that there could be
logs stored in the DB for which no log type is defined.

For the record, I also dislike the idea of removing statuses and
content type indicators when plugins that create them are deactivated.

Can someone provide a compelling argument for why it would be
beneficial to remove a log type, status type, or content type record
when the associated plugin is disabled?

Braden

unread,
Apr 4, 2008, 5:31:02 AM4/4/08
to habari...@googlegroups.com, habar...@googlegroups.com


Just to follow up here, the consequence to unregistering a log type is
that any log entries that reference that type get marked as
"Unknown". As such, I would think it is not desirable to remove the
log type when there still exist entries in the log associated with
that type.

As for adding log types, apparently they are added automatically when
writing to the log (by LogEntry::type() called by
LogEntry::insert()). So does one even need to register a log type
when activating a plugin?

BTW, the reason the PreApproved plugin was causing an internal server
error is because it passed only one argument to the two-argument
function register_type, which has the second argument ('module') set
to null by default, but the schema of the log_types table disallows
nulls for 'module' or 'type'.

- Braden

Caius Durling

unread,
Apr 4, 2008, 1:59:46 PM4/4/08
to habari-dev
The way we handle unregistering post types is to not actually delete
the type from the database, just set a flag on it to stop it being
used. So all posts of that type are still in the database, and are
still of that type.

Michael C. Harris

unread,
Apr 5, 2008, 3:14:41 AM4/5/08
to habar...@googlegroups.com
On Fri, Apr 04, 2008 at 10:59:46AM -0700, Caius Durling wrote:
>
> The way we handle unregistering post types is to not actually delete
> the type from the database, just set a flag on it to stop it being
> used. So all posts of that type are still in the database, and are
> still of that type.

In that case it seems that it's good practice to unregister. Is that
right? If so, we should document that.

Braden

unread,
Apr 5, 2008, 3:50:31 AM4/5/08
to habar...@googlegroups.com

On Apr 5, 2008, at 12:14 AM, Michael C. Harris wrote:
>
> On Fri, Apr 04, 2008 at 10:59:46AM -0700, Caius Durling wrote:
>>
>> The way we handle unregistering post types is to not actually delete
>> the type from the database, just set a flag on it to stop it being
>> used. So all posts of that type are still in the database, and are
>> still of that type.
>
> In that case it seems that it's good practice to unregister. Is that
> right? If so, we should document that.

We are talking about log types though, right? Caius was talking about
post types.

Currently, when one unregisters a log type, it simply gets deleted.
It is true that it doesn't seem right to delete the log type when
there might exist log messages of that type.

Caius, are you suggesting that log types should instead be handled the
same way as post types?

- Braden

Caius Durling

unread,
May 25, 2008, 6:15:50 PM5/25/08
to habari-dev
On Apr 5, 8:50 am, Braden <goo...@dabrado.net> wrote:
> Caius, are you suggesting that log types should instead be handled the  
> same way as post types?

Basically, yes. By setting them all to "Unknown" for example, you lose
the context of why the log entry was created, which makes the system
of having log types pointless.

I suggest we allow [un]registering, and then have the log viewer show
only registered (active) log types by default, but with the option to
filter to show unregistered (inactive) log types as well. So if I have
plugin foo that creates log entries and de-activate it, I can still go
in and manually show all entries the plugin created (and delete them
if I want to.)

Thoughts?

C
----
ca...@caius.name
+44 (0) 7960 268100
http://caius.name/
Reply all
Reply to author
Forward
0 new messages