FreeScout: Leave users deleted on a source disabled when connecting them

freescout.wordpress.net/Modules/WPOrgFreeScoutImport/README.md

@@ -18,7 +18,7 @@ Copies mailboxes' conversations from other FreeScouts, like WordCamp US's at sco
 - **What doesn't:** spam, drafts, and deleted conversations; other line items, like moves and merges; saved replies, workflows, custom folders, and mailbox settings, which the API doesn't give: set those up again by hand. Senders come across with their name and email only.
 - **Conversations agents worked on here:** once a conversation has a reply, note, or change made here, importing it again only adds the new threads. Its status and assignee stay this FreeScout's. What automatic workflows note when they run doesn't count. A conversation moved to another mailbox there moves here too, unless agents worked on it.
 - **Users:** every user's replies, notes, and assignments are credited to a user here: the one chosen on the source's **Users** page, under **Manage » FreeScout Import**, or else the one with the same email, or else a new, disabled one that keeps their name. Active users get access to the mailboxes an import credits them with something in.
-  - **Connecting in bulk:** on a source's Users page, **Download CSV** lists its users with their users here. Fill in its `wporg_username` column for those who'll work here, upload or paste it, and **Check** it: that shows, for each row, the WordPress.org account's name and email next to the user's, and what connecting would do, without doing it. **Connect** then connects each one's user here to the account, creating it if they have none yet (before their mailboxes are imported, too), or switching it on, with access to the mailboxes their imported conversations are in, if an import created it disabled. Someone whose account is connected to a user already is credited to that user instead, so they don't get a second one.
+  - **Connecting in bulk:** on a source's Users page, **Download CSV** lists its users with their users here. Fill in its `wporg_username` column for those who'll work here, upload or paste it, and **Check** it: that shows, for each row, the WordPress.org account's name and email next to the user's, and what connecting would do, without doing it. **Connect** then connects each one's user here to the account, creating it if they have none yet (before their mailboxes are imported, too), or switching it on, with access to the mailboxes their imported conversations are in, if an import created it disabled. Someone deleted on the source is connected and credited, but isn't switched on: a user created for them, or one an import created disabled, stays disabled. Someone whose account is connected to a user already is credited to that user instead, so they don't get a second one.
     - Connecting lets the account's owner log in as that user. So only users an import created, or whose email is the account's, are connected in bulk, and administrators only by their email; anyone else is connected on their profile.
     - A CSV connects each user once, each account once, and each user here to one account; the check flags rows that would do more. More rows naming an account, like someone's older, deleted users there, are credited to the user its first row connects, or to the one it's connected to already, so they're one user here.
     - A row whose account has neither the user's name nor their email there is highlighted, and only connected if its box is checked.

freescout.wordpress.net/Modules/WPOrgFreeScoutImport/Resources/views/connect.blade.php

@@ -69,6 +69,9 @@
 								<strong class="text-danger">{{ $step['error'] }}</strong>
 							@else
 								{{ $actions[ $step['action'] ] }}
+								@if ( $step['deleted'] )
+									<br/><small class="text-help">{{ __('Deleted on :name: not switched on', [ 'name' => $name ]) }}</small>
+								@endif
 								@if ( $step['mismatch'] )
 									<br/><label class="checkbox-inline"><input type="checkbox" name="confirmed[]" value="{{ $step['remote_user']['id'] }}" form="wporgfreescoutimport-connect"> <strong>{{ __('Not their name or email: connect anyway') }}</strong></label>
 								@endif

freescout.wordpress.net/Modules/WPOrgFreeScoutImport/Resources/views/users.blade.php

@@ -38,7 +38,7 @@
 		<div class="panel panel-default">
 			<div class="panel-body">
 				<h4>{{ __('Connect to WordPress.org accounts') }}</h4>
-				<p>{{ __('Download :name’s users, fill in the wporg_username column for those who’ll work here, and check it here. Each one’s user here is connected to that account, created if they have none yet, and switched on if an import created them disabled: their name, email, and avatar come from WordPress.org, and they can log in with it. Someone whose account is connected to a user already is credited to that user.', [ 'name' => $name ]) }}</p>
+				<p>{{ __('Download :name’s users, fill in the wporg_username column for those who’ll work here, and check it here. Each one’s user here is connected to that account, created if they have none yet, and switched on if an import created them disabled, unless they’re deleted there: their name, email, and avatar come from WordPress.org, and once their user is on, they can log in with it. Someone whose account is connected to a user already is credited to that user.', [ 'name' => $name ]) }}</p>
 				<p><a href="{{ route( 'wporgfreescoutimport.users.export', [ 'id' => $source->id ] ) }}" class="btn btn-default">{{ __('Download CSV') }}</a></p>
 				@if ( $can_connect )
 					<form method="POST" action="{{ route( 'wporgfreescoutimport.users.connect', [ 'id' => $source->id ] ) }}" enctype="multipart/form-data">

freescout.wordpress.net/Modules/WPOrgFreeScoutImport/Services/People.php

@@ -296,7 +296,7 @@ public static function remote_user( RemoteApi $api, int $remote_user_id ): ?arra
 	}
 
 	/**
-	 * The user for a remote user who'll work here: the one they have already, or a new, active one.
+	 * The user for a remote user who'll work here: the one they have already, or a new one, disabled if they're deleted there.
 	 *
 	 * @param int   $source_id   Source.
 	 * @param array $remote_user Remote user.
@@ -305,7 +305,17 @@ public static function remote_user( RemoteApi $api, int $remote_user_id ): ?arra
 	public static function find_or_create( int $source_id, array $remote_user ): User {
 		$existing = self::existing( $source_id, $remote_user );
 
-		return $existing ? $existing['user'] : self::match_or_create( $source_id, $remote_user, true );
+		return $existing ? $existing['user'] : self::match_or_create( $source_id, $remote_user, ! self::is_deleted( $remote_user ) );
+	}
+
+	/**
+	 * Whether a remote user is deleted there: FreeScout appends a suffix and the time to a deleted user's email.
+	 *
+	 * @param array $remote_user Remote user.
+	 * @return bool
+	 */
+	public static function is_deleted( array $remote_user ): bool {
+		return 1 === preg_match( '/' . preg_quote( User::EMAIL_DELETED_SUFFIX, '/' ) . '\d{14}$/', (string) ( $remote_user['email'] ?? '' ) );
 	}
 
 	/**

freescout.wordpress.net/Modules/WPOrgFreeScoutImport/Services/WordPressOrgAccounts.php

@@ -177,7 +177,8 @@ public static function parse( string $csv ): array {
 	 * @param array[] $rows Rows, from parse().
 	 * @return array[] Each with `row`, `remote_user` (or null), `wporg_user` (WordPressOrgUser, or null), `action`
 	 *                 (one of the constants, or null), `user` (the user here it's for, or null for a new one),
-	 *                 `mismatch`, and `error` (why nothing can be done, or '').
+	 *                 `mismatch`, `deleted` (whether the remote user is deleted there, so isn't switched on), and
+	 *                 `error` (why nothing can be done, or '').
 	 */
 	public function plan( array $rows ): array {
 		$client    = Client::from_config();
@@ -268,7 +269,10 @@ function () use ( $step, $connected, $username, $id ): User {
 						}
 
 						People::choose( $this->source_id, $id, $user );
-						People::activate( $user );
+						// A deleted remote user's row doesn't switch their user on, but still gives an active one access.
+						if ( ! $step['deleted'] || User::STATUS_ACTIVE === (int) $user->status ) {
+							People::activate( $user );
+						}
 
 						return $user;
 					}
@@ -337,13 +341,16 @@ private function plan_row( array $row, Client $client ): array {
 			'action'      => null,
 			'user'        => null,
 			'mismatch'    => false,
+			'deleted'     => false,
 			'error'       => '',
 		);
 
 		if ( ! $step['remote_user'] ) {
 			return array( 'error' => __( 'No user there with that ID or email.' ) ) + $step;
 		}
 
+		$step['deleted'] = People::is_deleted( $step['remote_user'] );
+
 		try {
 			$step['wporg_user'] = $this->lookup( $row['username'], $client );
 		} catch ( \Throwable $e ) {

freescout.wordpress.net/Modules/WPOrgFreeScoutImport/module.json

@@ -3,7 +3,7 @@
     "alias": "wporgfreescoutimport",
     "img": "/modules/wporgfreescoutimport/img/icon.svg",
     "description": "Imports other FreeScouts' mailboxes, so their history moves here with them.",
-    "version": "1.1.0",
+    "version": "1.1.1",
     "detailsUrl": "https://github.com/WordPress/wordpress.org/tree/trunk/freescout.wordpress.net/Modules/WPOrgFreeScoutImport",
     "author": "WordPress.org",
     "authorUrl": "https://wordpress.org/",

freescout.wordpress.net/Modules/WPOrgFreeScoutImport/tests/ImportTest.php

@@ -321,7 +321,8 @@ public function test_a_run_imports_every_page_and_finds_conversations_by_their_o
 
 	/**
 	 * A CSV connects users who'll work here: one an import created disabled is switched on, with access to its mailbox,
-	 * and someone new gets a user; one who was here before, under another email than the account's, isn't connected.
+	 * and someone new gets a user, disabled if they're deleted there; one who was here before, under another email than
+	 * the account's, isn't connected.
 	 *
 	 * @return void
 	 */
@@ -332,6 +333,7 @@ public function test_csv_connects_users_who_will_work_here(): void {
 				'adaagent' => array( 'Ada', 'Agent', 'ada@wordpress.example' ),
 				'noranote' => array( 'Nora', 'Note', 'nora@wordpress.example' ),
 				'bonew'    => array( 'Bo', 'New', 'bo@wordpress.example' ),
+				'ezragone' => array( 'Ezra', 'Gone', 'ezra@wordpress.example' ),
 			)
 		);
 		$this->remote->json(
@@ -342,6 +344,7 @@ public function test_csv_connects_users_who_will_work_here(): void {
 					self::user( 55, 'Ada', 'Agent', 'agent@example.org' ),
 					self::user( 66, 'Nora', 'Note', 'nora@wcus.test' ),
 					self::user( 77, 'Bo', 'New', 'bo@wcus.test' ),
+					self::user( 99, 'Ezra', 'Gone', 'ezra@wcus.test_deleted20210601184317' ),
 				)
 			)
 		);
@@ -351,11 +354,11 @@ public function test_csv_connects_users_who_will_work_here(): void {
 			->post(
 				route( 'wporgfreescoutimport.users.connect', array( 'id' => $this->source->id ) ),
 				array(
-					'csv'   => "remote_id,wporg_username\n55,adaagent\n66,noranote\n77,bonew\n",
+					'csv'   => "remote_id,wporg_username\n55,adaagent\n66,noranote\n77,bonew\n99,ezragone\n",
 					'apply' => 1,
 				)
 			)
-			->assertSessionHas( 'flash_success', 'Connected 2 users to WordPress.org accounts.' );
+			->assertSessionHas( 'flash_success', 'Connected 3 users to WordPress.org accounts.' );
 
 		$nora = User::query()->findOrFail( (int) RemoteUser::query()->where( 'source_id', $this->source->id )->where( 'remote_user_id', 66 )->value( 'user_id' ) );
 		$this->assertSame( 'noranote', Account::username_for( (int) $nora->id ) );
@@ -366,12 +369,16 @@ public function test_csv_connects_users_who_will_work_here(): void {
 		$this->assertSame( 'bonew', Account::username_for( (int) $bo->id ) );
 		$this->assertSame( User::STATUS_ACTIVE, (int) $bo->status );
 
+		$ezra = User::query()->findOrFail( (int) RemoteUser::query()->where( 'source_id', $this->source->id )->where( 'remote_user_id', 99 )->value( 'user_id' ) );
+		$this->assertSame( 'ezragone', Account::username_for( (int) $ezra->id ) );
+		$this->assertSame( User::STATUS_DISABLED, (int) $ezra->status );
+
 		$this->assertSame( '', Account::username_for( (int) $this->agent->id ) );
 	}
 
 	/**
 	 * Rows naming the same account, like someone's older, deleted users there, are all credited to the one user the
-	 * account is connected to.
+	 * account is connected to, switched on even when the row of a deleted one comes first.
 	 *
 	 * @return void
 	 */
@@ -393,7 +400,7 @@ public function test_csv_credits_rows_naming_the_same_account_to_one_user(): voi
 			->post(
 				route( 'wporgfreescoutimport.users.connect', array( 'id' => $this->source->id ) ),
 				array(
-					'csv'   => "remote_id,wporg_username\n66,noranote\n88,noranote\n",
+					'csv'   => "remote_id,wporg_username\n88,noranote\n66,noranote\n",
 					'apply' => 1,
 				)
 			)
@@ -402,6 +409,7 @@ public function test_csv_credits_rows_naming_the_same_account_to_one_user(): voi
 		$users = RemoteUser::query()->where( 'source_id', $this->source->id )->pluck( 'user_id', 'remote_user_id' )->map( 'intval' )->all();
 		$this->assertSame( $users[66], $users[88] ?? null );
 		$this->assertSame( 'noranote', Account::username_for( $users[66] ) );
+		$this->assertSame( User::STATUS_ACTIVE, (int) User::query()->findOrFail( $users[66] )->status );
 	}
 
 	/**