Page MenuHomePhabricator

Actually display filter creations as such on Special:Log/abusefilter
Closed, ResolvedPublic

Description

When creating a new filter, the Special:Log/abusefilter displays that User:Admin modified filter ###.

It'd be good that instead a new message could be added for when a filter is created which said that User:Admin created filter ###.

Similarly, when a filter is disabled or marked as deleted, that should appear with proper messages in the Special:Log/abusefilter.

That way we will have properly documented actions and maybe we could filter the logs later as it happens on Special:Log/block to show only creations, modifications, disablements and deletions.


Edit: Given that we need to resolve some things first before making the logging of the disable and marking as deleted possible, I'm reducing the scope of this task to log only the creation of abusefilters.

Event Timeline

MarcoAurelio renamed this task from New message for when creating a new filter to Amend Special:Log/abusefilter messages to properly display and document creations, modifications, filter disablements and filter deletions.Oct 16 2017, 11:00 AM

Change 384496 had a related patch set (by MarcoAurelio) published:
[mediawiki/extensions/AbuseFilter@master] Messages for each AbuseFilter action for the logs

https://gerrit.wikimedia.org/r/384496

I just did the i18n part (create the new messages). Unfortunatelly I won't be able to do the AbuseFilter code use each message for each action.

@MarcoAurelio let me look at the code to see if I can take care of the latter part (or at least give you directions)

Thanks @Huji. If you can take care of the second part, feel free to. My PHP is so scarce that I won't even bother in giving directions to myself :-)

I looked into this and have some good and bad news:

It'd be good that instead a new message could be added for when a filter is created which said that User:Admin created filter ###.

This is possible, I amended the patch with this change.

Similarly, when a filter is disabled or marked as deleted, that should appear with proper messages in the Special:Log/abusefilter.

Might be possible but will need much work.

we could filter the logs later as it happens on Special:Log/block to show only creations, modifications, disablements and deletions.

I'm afraid this won't be possible with the current database schema since we don't have an efficient way to join logging with abuse_filter_history. Even if this was possible, abuse_filter_history doesn't directly store this information.

@matej_suchanek So for now we can just have the messages display created/modified/disabled/marked as deleted and later work on that, maybe on a separate task. Does that sound right?

Depends on what is common practise. Do we ever leave unused messages without implementation across MW because we expect the code to be implemented at some point? To be honest, I do not personally expect the second part (disabled/deleted) to be (but do encourage everyone to prove the opposite).

Sorry. I don't understand. In the modification of the patch it seems you made possible to use abusefilter-log-create for when we create a new filter, the abusefilter-log-modify is already there and we only lack support for the .*deleted/.*disabled ones, right? If so, what needs to be done for when a filter is disabled or deleted instead of the "modified" message the "marked as deleted" or "disabled" message appears? Is that possible at all?

What I was refering to discuss in the second task is to amend the DB schema so the second part of this task (filter the logs) could be achieved.

Thanks for your help and sorry for my questioning.

Is that possible at all?

Hm, your question made me rethink the approach and it seems now it will be possible to fulfill your wish completely. However, older entries will stay with text "modified" (unless someone rebuilds the database). I will try to complete the patch as soon as possible.

The issue is that "delete" or "disable" actions can co-occur with other actions. For instance, I can go to a filter, and both modify its rules and mark it as deleted and then press save.

This means we need to either leave it as two possibilities ("create" and "modify"), or define a rule for precedence of various types of changes (e.g. first check if deleted/undeleted, then check if disabled/enabled, then check if modified). In any case, I think we should modify the current patch to pick the low-hanging fruit (i.e. restrict this patch to only create and modify) and continue the discussion about the rest here in the task.

@Huji Maybe AbuseFilter should not let us modify a filter and disallow it or mark it as deleted in the same action.

@Huji I've modified the patch so only filter creation messages are added. We should work on doing the others in the future maybe.

MarcoAurelio renamed this task from Amend Special:Log/abusefilter messages to properly display and document creations, modifications, filter disablements and filter deletions to Actually display filter creations as such on Special:Log/abusefilter.Dec 13 2017, 10:58 AM
MarcoAurelio updated the task description. (Show Details)

Might be possible but will need much work.

I'll create a new task for this.

we could filter the logs later as it happens on Special:Log/block to show only creations, modifications, disablements and deletions.

I'm afraid this won't be possible with the current database schema since we don't have an efficient way to join logging with abuse_filter_history. Even if this was possible, abuse_filter_history doesn't directly store this information.

Shall we file an Schema-change task? (not sure how to request that as DBA people would surely want some directions and I'm not privy to the inners of how AF is designed).

I revoke my suggestion about changing database schema and suggest to decline T182762. Now I imagine this could badly influence queries for example on RecentChanges which are already very problematic. The only way to go seems to be using log_action column (although older logs will always be "modified").

I revoke my suggestion about changing database schema and suggest to decline T182762. Now I imagine this could badly influence queries for example on RecentChanges which are already very problematic. The only way to go seems to be using log_action column (although older logs will always be "modified").

Noted, +1'ed!

Change 384496 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Actually mark abusefilter creations as such in the AbuseFilter log

https://gerrit.wikimedia.org/r/384496