Page MenuHomePhabricator

createAndPromote.php --reason missing
Open, Needs TriagePublic

Description

On includes/specialpage/LoginSignupSpecialPage.php there is

'reason' => [
 // comment for the user creation log

but on maintenance/createAndPromote.php there is no --reason option! Please add it. Else one is forced to manually use the web interface each time instead of being able to use the maintenance script when one wishes to log a reason when creating an account, just because the maintenance script lacks a --reason option.

Event Timeline

Jidanni created this task.Jun 18 2017, 2:30 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 18 2017, 2:30 PM

(I would love to not need to use the web form when creating accounts for
my users, as browsers detect name and password fields and try to help
you fill in/remember values.)

Reedy updated the task description. (Show Details)Jun 18 2017, 6:17 PM
Reedy added a project: good first task.
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptJun 18 2017, 6:17 PM
MtDu claimed this task.Jul 4 2017, 2:09 PM
MtDu added a subscriber: MtDu.

I'll take care of it.

MtDu removed MtDu as the assignee of this task.Jul 4 2017, 2:16 PM
MtDu added a subscriber: Reedy.

Not sure where I need to insert the reason itself in the maintenance script, ideas @Reedy?

Reedy added a comment.EditedJul 4 2017, 6:40 PM

Not sure where I need to insert the reason itself in the maintenance script, ideas @Reedy?

Add an option in the constructor

Get the option into a variable in execute

Unfortunately, the promotion is then just done with array_map( [ $user, 'addGroup' ], $promotions ); which doesn't take a reason

Looking at SpecialUserrights.php and specifically UserrightsPage::addLogEntry... It adds one log entry for all the changes overall

You could steal the code below, but reusing would be more sensible, I think. However, you probably shouldn't just make it static, it's used and overridden in CentralAuth and GlobalUserrights extensions.

Though, making it static... There is https://secure.php.net/manual/en/language.oop5.late-static-bindings.php

	/**
	 * Add a rights log entry for an action.
	 * @param User|UserRightsProxy $user
	 * @param array $oldGroups
	 * @param array $newGroups
	 * @param array $reason
	 * @param array $tags Change tags for the log entry
	 * @param array $oldUGMs Associative array of (group name => UserGroupMembership)
	 * @param array $newUGMs Associative array of (group name => UserGroupMembership)
	 */
	protected function addLogEntry( $user, $oldGroups, $newGroups, $reason, $tags,
		$oldUGMs, $newUGMs
	) {
		// make sure $oldUGMs and $newUGMs are in the same order, and serialise
		// each UGM object to a simplified array
		$oldUGMs = array_map( function ( $group ) use ( $oldUGMs ) {
			return isset( $oldUGMs[$group] ) ?
				self::serialiseUgmForLog( $oldUGMs[$group] ) :
				null;
		}, $oldGroups );
		$newUGMs = array_map( function ( $group ) use ( $newUGMs ) {
			return isset( $newUGMs[$group] ) ?
				self::serialiseUgmForLog( $newUGMs[$group] ) :
				null;
		}, $newGroups );

		$logEntry = new ManualLogEntry( 'rights', 'rights' );
		$logEntry->setPerformer( $this->getUser() );
		$logEntry->setTarget( $user->getUserPage() );
		$logEntry->setComment( $reason );
		$logEntry->setParameters( [
			'4::oldgroups' => $oldGroups,
			'5::newgroups' => $newGroups,
			'oldmetadata' => $oldUGMs,
			'newmetadata' => $newUGMs,
		] );
		$logid = $logEntry->insert();
		if ( count( $tags ) ) {
			$logEntry->setTags( $tags );
		}
		$logEntry->publish( $logid );
	}

So... A better option is looking how the API does userrights changes, and that's using UserrightsPage::doSaveUserGroups

Which does:

	protected function getUserRightsPage() {
		return new UserrightsPage;
	}
		$form = $this->getUserRightsPage();
		$form->setContext( $this->getContext() );
		$r['user'] = $user->getName();
		$r['userid'] = $user->getId();
		list( $r['added'], $r['removed'] ) = $form->doSaveUserGroups(
			$user, (array)$params['add'], (array)$params['remove'],
			$params['reason'], $tags, $groupExpiries
		);

I think the same should go for a --realname option. While for setting an (optional) e-mail address there is resetUserEmail.php, I could not find any other maintenance script to set the (optional) realname. Or did I just overlook something? And if it is not possible so far to add/change the realname of a user via maintenance scripts, would it be better to have a separate script to just do that (like for the resetUserEmail.php case) or should it go into the createAndPromote.php script as an option?