Page MenuHomePhabricator

Actions are marked as red even if unchanged
Closed, ResolvedPublic

Description

I just noticed that in AF's history, actions of the same filters are marked as red even if unchanged. See for example on itwiki:

In those versions there wasn't any change to the actions.

This surely happens for disallow. I didn't test any other action, but I'm confident it happens for every one. It surely comes from some change in wmf.24, maybe this one. I'm currently investigating and will report here any progress, but if you're quicker than me, feel free to upload a patch. Also, I'm unsure whether we should also backport the fix.

Event Timeline

Daimona created this task.Mar 14 2018, 1:23 PM
Restricted Application added subscribers: Scoopfinder, Aklapper. · View Herald TranscriptMar 14 2018, 1:23 PM
Daimona triaged this task as High priority.Mar 14 2018, 1:24 PM
Daimona moved this task from Backlog to Internal bugs on the AbuseFilter board.

Change 419428 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Filter array when comparing versions

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

Daimona claimed this task.Mar 14 2018, 1:51 PM

Explaination given on gerrit. This also opens the door to a deeper investigation, while the patch itself solves the problem.

Huji added a subscriber: Huji.Mar 15 2018, 11:54 PM

I cannot reproduce this. Can you please provide a step by step process (from creating a new filter, until the step that reproduces this issue) please?

Daimona added a comment.EditedMar 20 2018, 2:21 PM

@Huji I'm sorry, I didn't notice this message! Sure, here are the steps:
1-Create a new filter with actions e.g. disallow and save it
2-Edit that filter by touching anything but actions (e.g. notes)
3-Go to filter history
->The "actions" box is red in the (2) edit, which shouldn't be since actions weren't touched.

EDIT: I just discovered that this seems to only happen for filters including "disallow" action. Further investigation coming.

Change 420729 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/AbuseFilter@master] Add missing trim() and array_filter() to parameter loading/editing

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

Change 419428 abandoned by Daimona Eaytoy:
Filter array when comparing versions

Reason:
I8eb50d38c81b4e446c0f1dc03abc27122b8fa025 is a better way to solve this.

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

Change 420729 abandoned by Thiemo Kreuz (WMDE):
Add missing trim() and array_filter() to parameter loading/editing

Reason:
Feel free to pick this up and continue working on it any way you like, e.g. copy it to your own patch and make it work. I personally don't have time I can invest here.

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

Change 423890 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Filter parameters when loading/editing them

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

Apart from the visual bug (which makes it hard to tell if actions were really changed) this also let users save the filter without any change.

Huji removed a project: Patch-For-Review.EditedApr 23 2018, 10:50 PM

913d37eba65b resolves the part of the issue that allows saving unchanged filters, but it doesn't address the issue of "Actions" being highlighted in pink even when it is not changed.

Change 423890 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Filter parameters when loading/editing them

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

Change 428560 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_31] Filter parameters when loading/editing them

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

Change 428593 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Don't use an empty string for block parameters

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

Change 428560 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@REL1_31] Filter parameters when loading/editing them

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

Huji closed this task as Resolved.Apr 26 2018, 2:13 PM
Huji removed a project: Patch-For-Review.

Change 428593 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Don't use an empty string for block parameters

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

Change 429203 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_31] Don't use an empty string for block parameters

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

Change 429203 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@REL1_31] Don't use an empty string for block parameters

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

Change 429570 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@wmf/1.32.0-wmf.1] Don't use an empty string for block parameters

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

Change 429570 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@wmf/1.32.0-wmf.1] Don't use an empty string for block parameters

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

Mentioned in SAL (#wikimedia-operations) [2018-04-30T13:30:29Z] <zfilipin@tin> Synchronized php-1.32.0-wmf.1/extensions/AbuseFilter: SWAT: [[gerrit:429570|Dont use an empty string for block parameters (T189681)]] (duration: 01m 02s)