Page MenuHomePhabricator

Convert Special:AbuseLog to more standard delete and suppress system
Open, Needs TriagePublic

Assigned To
None
Authored By
Luke081515
Oct 14 2015, 10:04 PM
Referenced Files
None
Tokens
"Like" token, awarded by matej_suchanek."Like" token, awarded by MGChecker."Like" token, awarded by mfb."Yellow Medal" token, awarded by XenonX3."Like" token, awarded by Morten_Haan.

Description

Problem

Only Oversighters can delete a abusefilter logentry. That's an problem, because you have more stric criterias for deleting an entry. For example: A copyright violation at an abuselog entry should be deleted, but this is not a suppress criteria (at the most cases), so this entry will not be suppressed, it's mostly public visible.

Solution

Create a two steps system, like at revision deletion:

  • Sysops can hide an entry
  • Oversighters can suppress it

Current hidden entrys would remain as suppressed.

Event Timeline

Luke081515 raised the priority of this task from to Needs Triage.
Luke081515 updated the task description. (Show Details)
Luke081515 added a project: AbuseFilter.
Luke081515 subscribed.
Billinghurst renamed this task from Allow sysops to delete abusefilter logentrys to Convert Special:AbuseLog to more standard delete and suppress system.Oct 15 2015, 2:26 AM
Billinghurst set Security to None.

I'll work on this as I'll find some time, and this will have to be dependent on gerrit/467774. I'm writing down some things to do for when I'll work on it (or for whoever wants):

  1. Split user rights: instead of having abusefilter-hide-log, we should have abusefilter-delete-log and abusefilter-suppress-log (assign them on wmf wikis, add a notice to AbuseFilterHooks::onRegistration)
  2. Tweak i18n: distinct "delete" and "suppress" where necessary
  3. Add a deletion LogFormatter (like the one for suppression), again fix log-related i18n
  4. Add the "suppress" option to the form (so that, by default, entries will only be deleted), understand it in saveHideForm
  5. Add a "deleted" value to afl_deleted. Right now it only accepts 0s and 1s. We could use a bitfield or, since we only have three cases, use 0=visible, 1=deleted, 2=suppressed. However, this would require a DB patch to migrate all 1s to 2s, + some back compat stuff. Oterhwise, we could just do 1=suppressed, 2=deleted for the moment, as it'd just be for clarity. Anyway, code in showHideForm/saveHideForm should be updated to understand this new value
  6. Add the LogEventsList::showLogExtract for deletion, and conditionally show the one for suppression only if the user holds suppression rights
  7. Change SpecialAbuseLog::isHidden to return different values depending on the deletion status. Actually, that would be a nice moment to rework its return value, for instance making it bitwise

I'll work on this as I'll find some time, and this will have to be dependent on gerrit/467774. I'm writing down some things to do for when I'll work on it (or for whoever wants):
[snip]

  1. Add a "deleted" value to afl_deleted. Right now it only accepts 0s and 1s. We could use a bitfield or, since we only have three cases, use 0=visible, 1=deleted, 2=suppressed. However, this would require a DB patch to migrate all 1s to 2s, + some back compat stuff. Oterhwise, we could just do 1=suppressed, 2=deleted for the moment, as it'd just be for clarity. Anyway, code in showHideForm/saveHideForm should be updated to understand this new value

I would suggest keeping T234155 in mind - if abuse filters can use CU variables, not only should the filters be sensitive (i.e. harder to view that private) but so should the log entries generated, so that variable results can be stored. That will also depend on a better visibility field for abuse filter log entries. To avoid needing two rounds of DB patches, it may be good to plan the schema with both tasks in mind

I would suggest keeping T234155 in mind - if abuse filters can use CU variables, not only should the filters be sensitive (i.e. harder to view that private) but so should the log entries generated, so that variable results can be stored.

Yeah, that's definitely true.

That will also depend on a better visibility field for abuse filter log entries. To avoid needing two rounds of DB patches, it may be good to plan the schema with both tasks in mind

It depends... I don't think sensitiveness should be handled with the afl_deleted column. But even if we do, there should be no need to re-patch that column. We could, instead, have to add a separate column. But whether these should be done together depends on when these issues will be tackled.

Daimona added subscribers: MBH, Base.