Page MenuHomePhabricator

AbuseFilterGenerateVarsForRecentChange hook is not working like other hooks
Open, Needs TriagePublic

Description

I realized the AbuseFilterGenerateVarsForRecentChange is working in an opposite way than it is usual.

The hook is run when we have found a row that the default generator does not understand (the "!" is important here):

extensions/AbuseFilter/includes/VariableGenerator/RCVariableGenerator.php
		} elseif (
			!$this->hookRunner->onAbuseFilterGenerateVarsForRecentChange(
				$this, $this->rc, $this->vars, $this->contextUser )
		) {
			throw new LogicException( 'Cannot understand the given recentchanges row!' );
		}

Currently, it is only used by Flow (StructuredDiscussions), so that we can retrospectively generate variables for edits done using Flow:

extensions/Flow/src/Hooks/AbuseFilterHandler.php
class AbuseFilterHandler implements AbuseFilterGenerateVarsForRecentChangeHook {

	public function onAbuseFilterGenerateVarsForRecentChange(
		RCVariableGenerator $generator,
		RecentChange $rc,
		VariableHolder $vars,
		User $contextUser
	): bool {
		if ( $rc->getAttribute( 'rc_source' ) !== RecentChangesListener::SRC_FLOW ) {
			return false;
		}
		Hooks::getAbuseFilter()->generateRecentChangesVars( $rc, $vars, $contextUser );
		return true;
	}
}

The handler method returns false if it does not recognize the row (i.e., it wasn't generated by Flow), and true otherwise.
Returning false, however, stops running the hook and gives other extensions zero chance to observe the row. (As said, there are probably no other extensions right now.)
The hook stub method correctly states @return bool|void True or no return value to continue or false to abort.