Page MenuHomePhabricator

Refactor anti-spam/vandalism checks out of EditPage.php
Open, LowPublic

Description

The documentation for EditPage::internalAttemptSave() has the following comment:

	 * @todo FIXME: This interface is TERRIBLE, but hard to get rid of due to
	 *   various error display idiosyncrasies. There are also lots of cases
	 *   where error metadata is set in the object and retrieved later instead
	 *   of being returned, e.g. AS_CONTENT_TOO_BIG and
	 *   AS_BLOCKED_PAGE_FOR_USER. All that stuff needs to be cleaned up some
	 * time.

We should fix this by extracting all the anti-spam/vandalism checks (plus hooks) out of EditPage, and into a pluggable interface (that ideally promotes best practices like T59026).

EditPage would call into this to determine whether the edit is OK or not, and then update the UI accordingly.

Event Timeline

Duplicate of T20654: EditPage.php needs rewrite: Separate DB and UI logic? There's a lot more technical debt there than just anti-spam checks.

Duplicate of T20654: EditPage.php needs rewrite: Separate DB and UI logic? There's a lot more technical debt there than just anti-spam checks.

A subtask. There's definitely more work to be done, but this is the scope of what I'm planning to do this quarter.

Change 365866 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] [POC] Refactor spam filter edit checks

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

Aklapper changed the task status from Stalled to Open.May 18 2020, 1:41 PM

@AMooney: I cannot see either in previous comments what/who exactly this task is stalled on ("If a report is waiting for further input (e.g. from its reporter or a third party) and can currently not be acted on"). Hence boldly resetting task status (please feel free to correct me and elaborate - thanks!)

@Aklapper, I think I accidentally set it to stalled when I was trying to do some housekeeping. I do think from the the time the task was last looked at to when I set it to stalled it had not been looked at in awhile, maybe 5 months?

Change 365866 abandoned by Legoktm:
[POC] Refactor spam filter edit checks

Reason:
Rescue after EditPage refactor

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

There's useful code in my patch, specifically the EditRequest value object, but I think this makes more sense to do after an EditPage refactor or as part of it (which I don't want to do)

There's useful code in my patch, specifically the EditRequest value object, but I think this makes more sense to do after an EditPage refactor or as part of it (which I don't want to do)

Will take a look - I have a larger plan for each of the different checks that edit page uses to possibly prevent an edit, including the spam checkr

Aklapper removed a subscriber: AMooney.

Removing task assignee due to inactivity as this open task has been assigned for more than two years. See the email sent to the task assignee on August 22nd, 2022.
Please assign this task to yourself again if you still realistically [plan to] work on this task - it would be welcome!
If this task has been resolved in the meantime, or should not be worked on ("declined"), please update its task status via "Add Action… 🡒 Change Status".
Also see https://www.mediawiki.org/wiki/Bug_management/Assignee_cleanup for tips how to best manage your individual work in Phabricator. Thanks!