dominno / django-moderation

django-moderation is reusable application for Django framework, that allows to moderate any model objects.
BSD 3-Clause "New" or "Revised" License
269 stars 90 forks source link

Pass object to `is_auto_*` for more useful custom auto-moderation #15

Closed hexabits closed 14 years ago

hexabits commented 14 years ago

This will allow for custom auto-moderation methods that would allow for per-object and per-model moderation checks. The methods already exist, they just need the extra obj parameter, and to have this custom auto-approve functionality documented.

Example:

# Inside MyModelModerator, which is registered with MyModel
def is_auto_reject(self, obj, user):
    # Auto reject spam
    if akismet_spam_check(obj.body):  # Check body of object for spam
        # Body of object is spam, moderate
        return True
    super(MyModelModerator, self).is_auto_reject(obj, user)

For the exact object to pass in, I'm not sure off hand which is smarter to pass... The SerializedObject in changed_object or the original object in content_object? I guess it depends on the other logic, if it's new or changed at that moment.

dominno commented 14 years ago

Maybe pass both ? original_obj and changed_obj ? also changed object can be extracted from object: obj.moderated_object.changed_object. I think it will be more cleaner if both of them will be passed in to is_auto_* method. What do you think ?

hexabits commented 14 years ago

I only see passing both being useful if you need to make some kind of comparison between the original and the changed. But I think using original_obj.body as in my example is extra tedious and a lot to write I feel. And of course obj1 vs obj2 doesn't make sense either, so I don't know how they should be named...

How about putting changed_object into the **kwargs instead, and making them retrieve it if it is absolutely necessary? Changed example:

# Inside MyModelModerator, which is registered with MyModel
def is_auto_reject(self, obj, user, *args, **kwargs):
    ## Grab changed object only if needed
    changed_object = kwargs['changed_object']

    # Auto reject spam
    if akismet_spam_check(obj.body):  # Check body of object for spam
        # Body of object is spam, moderate
        return True
    super(MyModelModerator, self).is_auto_reject(obj, user)

Of course the user can name their arguments whatever they feel, so if the above is too much, then at least the recommended usage should be something like:

def is_auto_reject(self, user, obj, changed_obj):
    ...

And so if the user really doesn't care to do anything with changed_obj they could just do:

def is_auto_reject(self, user, obj, *args):
    ...

And ignore the other args basically. So the standard recommended way would still have the nice and short obj and changed_obj would be basically optional.

hexabits commented 14 years ago

Of course if both arguments aren't needed for any good use case, maybe if the object is NEW obj should be the original, and if the object was pre-existing then obj should actually be the object from the changed_object field on the ModeratedObject model instead.

dominno commented 14 years ago

If obj is NEW then obj == changed object so obj that will be passed can changed_object.

dominno commented 14 years ago

Added possibility of passing changed object in to is_auto* methods of GenericModerator class. This will allow more useful custom auto-moderation. Ex. auto reject if akismet spam check returns True. Closed by 2520c404bbd697395c2c57644b97d812778b3e94

dominno commented 14 years ago

I have added example how to use it in e186c2e90032fb768c51c0f0a1636c7ad331080f Is it ok ? or it needs more explanation ?