Page MenuHomePhabricator

Refactor LastEditInfo to hold UserIdentity instead of separate userId and userName pair
Closed, ResolvedPublic

Description

While working on refactoring how AbuseFilter triggers Echo notification, I noticed that AbuseFilter heavily operates on $userId and $userName pairs instead of using UserIdentity.
The LasteditInfo class keeps the User information as two separate variables ($userId and $userName). We should use the UserIdentity instead to ensure that these two are always updated together, as they represent one entity.

Core migrates away from using ints to pass Users, more components require UserIdentity as a user representation. (example Linker::userLink( $userId, $userName ... ) is deprecated in favour of UserLinkRenderer::userLink( UserIdentity $targetUser ...) .

How to Refactor:

Instead of expecting and passing around

	/**
	 * @param int $userID
	 * @param string $userName
	 * @param string $timestamp
	 */
	public function __construct( int $userID, string $userName, string $timestamp ) {
		$this->userID = $userID;
		$this->userName = $userName;
		$this->timestamp = $timestamp;
	}

We should depend on UserIdentity:

	/**
	 * @param UserIdentity $identity
	 * @param string $timestamp
	 */
	public function __construct( UserIdentity $identity, string $timestamp ) {
		$this->identity = $identity;
		$this->timestamp = $timestamp;
	}

The callers should retrieve user information by calling $editInfo->getUserIdentity() instead of operating on getUserId() and getUserName().

Note: User object already implements UserIdentity, therefore it will be enough just to pass the user. And when working on raw database results
it should be enough to call UserIdentityValue::newRegistered() to get a UserIdentity based on the userId and userName.

Event Timeline

Thank you for tagging this task with good first task for Wikimedia newcomers!

Newcomers often may not be aware of things that may seem obvious to seasoned contributors, so please take a moment to reflect on how this task might look to somebody who has never contributed to Wikimedia projects.

A good first task is a self-contained, non-controversial task with a clear approach. It should be well-described with pointers to help a completely new contributor, for example it should clearly pointed to the codebase URL and provide clear steps to help a contributor get setup for success. We've included some guidelines at https://phabricator.wikimedia.org/tag/good_first_task/ !

Thank you for helping us drive new contributions to our projects <3

Vanshika subscribed.

Hello!

I am a (very) new contributor.
Ive created a patch that can be found here: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/AbuseFilter/+/1183259
Please review at your earliest convenience and let me know if I've done anything horribly wrong.

Thanks,

  • Nicolas

Change #1183259 had a related patch set uploaded (by Pppery; author: Nicolasmichel):

[mediawiki/extensions/AbuseFilter@master] Refactored LastEditInfo to user UserIdentity rather than UserID Username pairs.

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

Change #1183259 had a related patch set uploaded (by Nicolasmichel; author: Nicolasmichel):

[mediawiki/extensions/AbuseFilter@master] Refactor LastEditInfo to user UserIdentity rather than UserID Username pairs.

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

Hi all, are there any further changes that are required? Or is the most recent patch set good to merge?

Change #1183259 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Refactor LastEditInfo to user UserIdentity rather than UserID Username pairs.

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