Fixed Spam cleaner actions aren't dynamic

[DragonByte Tech]

Affected version

2.0.10

It is not possible to extend the spam cleaner's actions in a sensible manner because they are hardcoded:

$actions = $this->filter([
    'action_threads' => 'bool',
    'delete_messages' => 'bool',
    'delete_conversations' => 'bool',
    'ban_user' => 'bool',
    'check_ips' => 'bool'
]);

Compare this with the bulk user actions array:

$actions = $this->filter('actions', 'array');

You have to work around this design flaw by extending public function cleanUpContent(array $actions) and filter the additional actions that way, instead of simply having an array filter like the bulk user actions.

Please consider fixing this so that adding new spam cleaning action options can be done without needing hacky extensions like that.


Fillip

 

[XF Bug Bot]

Thank you for reporting this issue. It has now been resolved and we are aiming to include it in a future XF release (2.1.0 B7/RC1).

Change log:

New method to filter spam cleaner actions. Coerce the resulting actions to ensure we at least have our expected default keys (set to false). Move the processing of the spam cleaning actions to a new cleanUp method which handles banning the user and calling cleanUpContent.

Any changes made as a result of this issue being resolved may not be rolled out here until later.

 

[Chris D]

So, this isn't quite what you asked for. This is mostly because there's not actually a "design flaw" here, rather the current approach is because we do need to ensure we actually get values for at least the default actions. If we just filtered on array then we wouldn't have a default false value for the existing boolean actions.

The other point worth noting is that we wouldn't necessarily want to filter a load of checkboxes as strings as semantically they should be boolean so we'd have to use array-bool but then that wouldn't exactly work if you actually wanted a non-boolean option/action to be passed in.

There's now a dedicated method for filtering the actions, which will mostly just replace the extensions you're currently adding to cleanUpContent. Having them in the controller makes more sense because that's where we process user input ordinarily and it also means that if there are non-content related actions, then they can be added at a single point which makes sense.

We've also added some code to essentially sanity check that we get our default values at least (so they can't be accidentally wiped out).

Finally, we've just restructured things slightly so there is now a single cleanUp method called from the controller. That takes care of banning the user and cleaning the content, and serves as an additional (and action agnostic) extension point; so a good place to perform other actions that aren't necessarily content specific, for example.