Merge pull request #18270 from nextcloud/wiswedel/transferOwnership/polishing

Transfer ownership function polishing

apps/files/lib/Notification/Notifier.php

@@ -127,7 +127,7 @@ class Notifier implements INotifier, IDismissableNotifier {
 			);
 
 		$disapproveAction = $notification->createAction()
-			->setParsedLabel($l->t('Decline'))
+			->setParsedLabel($l->t('Reject'))
 			->setPrimary(false)
 			->setLink(
 				$this->urlGenerator->getAbsoluteURL(
@@ -142,7 +142,7 @@ class Notifier implements INotifier, IDismissableNotifier {
 		$notification->addParsedAction($approveAction)
 			->addParsedAction($disapproveAction)
 			->setRichSubject(
-				$l->t('Incoming file transfer from {user}'),
+				$l->t('Incoming ownership transfer from {user}'),
 				[
 					'user' => [
 						'type' => 'user',
@@ -150,9 +150,9 @@ class Notifier implements INotifier, IDismissableNotifier {
 						'name' => $param['sourceUser'],
 					],
 				])
-			->setParsedSubject(str_replace('{user}', $param['sourceUser'], $l->t('Incoming file transfer from {user}')))
+			->setParsedSubject(str_replace('{user}', $param['sourceUser'], $l->t('Incoming ownership transfer from {user}')))
 			->setRichMessage(
-				$l->t('Do you want to accept {path}?'),
+				$l->t("Do you want to accept {path}?\n\nNote: The transfer process after accepting may take up to 1 hour."),
 				[
 					'path' => [
 						'type' => 'highlight',
@@ -160,7 +160,7 @@ class Notifier implements INotifier, IDismissableNotifier {
 						'name' => $param['nodeName'],
 					]
 				])
-			->setParsedMessage(str_replace('{path}', $param['nodeName'], $l->t('Do you want to accept {path}?')));
+			->setParsedMessage(str_replace('{path}', $param['nodeName'], $l->t("Do you want to accept {path}?\n\nNote: The transfer process after accepting may take up to 1 hour.")));
 
 		return $notification;
 	}
@@ -169,11 +169,11 @@ class Notifier implements INotifier, IDismissableNotifier {
 		$l = $this->l10nFactory->get('files', $languageCode);
 		$param = $notification->getSubjectParameters();
 
-		$notification->setRichSubject($l->t('File transfer failed'))
-			->setParsedSubject($l->t('File transfer failed'))
+		$notification->setRichSubject($l->t('Ownership transfer failed'))
+			->setParsedSubject($l->t('Ownership transfer failed'))
 
 			->setRichMessage(
-				$l->t('Your transfer of {path} to {user} failed.'),
+				$l->t('Your ownership transfer of {path} to {user} failed.'),
 				[
 					'path' => [
 						'type' => 'highlight',
@@ -186,7 +186,7 @@ class Notifier implements INotifier, IDismissableNotifier {
 						'name' => $param['targetUser'],
 					],
 				])
-			->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['targetUser']], $l->t('Your transfer of {path} to {user} failed.')));
+			->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['targetUser']], $l->t('Your ownership transfer of {path} to {user} failed.')));
 		return $notification;
 	}
 
@@ -194,11 +194,11 @@ class Notifier implements INotifier, IDismissableNotifier {
 		$l = $this->l10nFactory->get('files', $languageCode);
 		$param = $notification->getSubjectParameters();
 
-		$notification->setRichSubject($l->t('File transfer failed'))
-			->setParsedSubject($l->t('File transfer failed'))
+		$notification->setRichSubject($l->t('Ownership transfer failed'))
+			->setParsedSubject($l->t('Ownership transfer failed'))
 
 			->setRichMessage(
-				$l->t('The transfer of {path} from {user} failed.'),
+				$l->t('The ownership transfer of {path} from {user} failed.'),
 				[
 					'path' => [
 						'type' => 'highlight',
@@ -211,7 +211,7 @@ class Notifier implements INotifier, IDismissableNotifier {
 						'name' => $param['sourceUser'],
 					],
 				])
-			->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['sourceUser']], $l->t('The transfer of {path} from {user} failed.')));
+			->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['sourceUser']], $l->t('The ownership transfer of {path} from {user} failed.')));
 
 		return $notification;
 	}
@@ -220,11 +220,11 @@ class Notifier implements INotifier, IDismissableNotifier {
 		$l = $this->l10nFactory->get('files', $languageCode);
 		$param = $notification->getSubjectParameters();
 
-		$notification->setRichSubject($l->t('File transfer done'))
-			->setParsedSubject($l->t('File transfer done'))
+		$notification->setRichSubject($l->t('Ownership transfer done'))
+			->setParsedSubject($l->t('Ownership transfer done'))
 
 			->setRichMessage(
-				$l->t('Your transfer of {path} to {user} has completed.'),
+				$l->t('Your ownership transfer of {path} to {user} has completed.'),
 				[
 					'path' => [
 						'type' => 'highlight',
@@ -237,7 +237,7 @@ class Notifier implements INotifier, IDismissableNotifier {
 						'name' => $param['targetUser'],
 					],
 				])
-			->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['targetUser']], $l->t('Your transfer of {path} to {user} has completed.')));
+			->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['targetUser']], $l->t('Your ownership transfer of {path} to {user} has completed.')));
 
 		return $notification;
 	}
@@ -246,11 +246,11 @@ class Notifier implements INotifier, IDismissableNotifier {
 		$l = $this->l10nFactory->get('files', $languageCode);
 		$param = $notification->getSubjectParameters();
 
-		$notification->setRichSubject($l->t('File transfer done'))
-			->setParsedSubject($l->t('File transfer done'))
+		$notification->setRichSubject($l->t('Ownership transfer done'))
+			->setParsedSubject($l->t('Ownership transfer done'))
 
 			->setRichMessage(
-				$l->t('The transfer of {path} from {user} has completed.'),
+				$l->t('The ownership transfer of {path} from {user} has completed.'),
 				[
 					'path' => [
 						'type' => 'highlight',
@@ -263,7 +263,7 @@ class Notifier implements INotifier, IDismissableNotifier {
 						'name' => $param['sourceUser'],
 					],
 				])
-			->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['sourceUser']], $l->t('The transfer of {path} from {user} has completed.')));
+			->setParsedMessage(str_replace(['{path}', '{user}'], [$param['nodeName'], $param['sourceUser']], $l->t('The ownership transfer of {path} from {user} has completed.')));
 
 		return $notification;
 	}

apps/files/lib/Service/OwnershipTransferService.php

@@ -83,7 +83,7 @@ class OwnershipTransferService {
 
 		// target user has to be ready
 		if (!$this->encryptionManager->isReadyForUser($destinationUid)) {
-			throw new TransferOwnershipException("The target user is not ready to accept files. The user has at least to be logged in once.", 2);
+			throw new TransferOwnershipException("The target user is not ready to accept files. The user has at least to have logged in once.", 2);
 		}
 
 		$date = date('Y-m-d H-i-s');
@@ -156,8 +156,8 @@ class OwnershipTransferService {
 		$size = $view->getFileInfo($sourcePath, false)->getSize(false);
 		$freeSpace = $view->free_space($destinationUid . '/files/');
 		if ($size > $freeSpace) {
-			$output->writeln('<error>Target user does not have enough free space available</error>');
-			throw new \Exception('Execution terminated');
+			$output->writeln('<error>Target user does not have enough free space available.</error>');
+			throw new \Exception('Execution terminated.');
 		}
 
 		$output->writeln("Analysing files of $sourceUid ...");
@@ -185,7 +185,7 @@ class OwnershipTransferService {
 
 		// no file is allowed to be encrypted
 		if (!empty($encryptedFiles)) {
-			$output->writeln("<error>Some files are encrypted - please decrypt them first</error>");
+			$output->writeln("<error>Some files are encrypted - please decrypt them first.</error>");
 			foreach ($encryptedFiles as $encryptedFile) {
 				/** @var FileInfo $encryptedFile */
 				$output->writeln("  " . $encryptedFile->getPath());
@@ -196,7 +196,7 @@ class OwnershipTransferService {
 
 	private function collectUsersShares(string $sourceUid,
 										OutputInterface $output): array {
-		$output->writeln("Collecting all share information for files and folder of $sourceUid ...");
+		$output->writeln("Collecting all share information for files and folders of $sourceUid ...");
 
 		$shares = [];
 		$progress = new ProgressBar($output);
@@ -235,7 +235,7 @@ class OwnershipTransferService {
 			$finalTarget = $finalTarget . '/' . basename($sourcePath);
 		}
 		if ($view->rename($sourcePath, $finalTarget) === false) {
-			throw new TransferOwnershipException("Could not transfer files", 1);
+			throw new TransferOwnershipException("Could not transfer files.", 1);
 		}
 		if (!is_dir("$sourceUid/files")) {
 			// because the files folder is moved away we need to recreate it

apps/files/src/components/TransferOwnershipDialogue.vue

@@ -21,33 +21,27 @@
 
 <template>
 	<div>
-		<h3>{{ t('files', 'Transfer ownership') }} </h3>
-		<p>
-			{{ t('files', 'Here you can select a directory that is transferred to another user. It may take some time until the process is done.') }}
-		</p>
+		<h3>{{ t('files', 'Transfer ownership of a file or folder') }} </h3>
 		<form @submit.prevent="submit">
-			<ol>
-				<li>
-					<div class="step-header">
-						{{ t('files', 'Directory to move') }}
-					</div>
-					<span v-if="directory === undefined">{{ t('files', 'No directory selected') }}</span>
-					<span v-else>{{ directory }}</span>
-					<button class="primary" @click.prevent="start">
-						{{ t('files', 'Select') }}
-					</button>
-					<span class="error">{{ directoryPickerError }}</span>
-				</li>
-				<li>
-					<div class="step-header">
-						{{ t('files', 'Target user') }}
-					</div>
+			<p>
+				<span>{{ readableDirectory }}</span>
+				<button v-if="directory === undefined" @click.prevent="start">
+					{{ t('files', 'Choose file or folder to transfer') }}
+				</button>
+				<button v-else @click.prevent="start">
+					{{ t('files', 'Change') }}
+				</button>
+				<span class="error">{{ directoryPickerError }}</span>
+			</p>
+			<p>
+				<label>
+					<span>{{ t('files', 'New owner') }}</span>
 					<Multiselect
 						v-model="selectedUser"
 						:options="formatedUserSuggestions"
 						:multiple="false"
 						:searchable="true"
-						:placeholder="t('core', 'Target user …')"
+						:placeholder="t('files', 'Search users')"
 						:preselect-first="true"
 						:preserve-search="true"
 						:loading="loadingUsers"
@@ -56,16 +50,17 @@
 						:internal-search="false"
 						:clear-on-select="false"
 						:user-select="true"
-						@search-change="findUserDebounced" />
-				</li>
-				<li>
-					<input type="submit"
-						class="primary"
-						:value="t('files', 'Submit')"
-						:disabled="!canSubmit">
-					<span class="error">{{ submitError }}</span>
-				</li>
-			</ol>
+						@search-change="findUserDebounced"
+						class="middle-align" />
+				</label>
+			</p>
+			<p>
+				<input type="submit"
+					class="primary"
+					:value="submitButtonText"
+					:disabled="!canSubmit">
+				<span class="error">{{ submitError }}</span>
+			</p>
 		</form>
 	</div>
 </template>
@@ -80,7 +75,7 @@ import Vue from 'vue'
 
 import logger from '../logger'
 
-const picker = getFilePickerBuilder(t('files', 'Select directory to transfer'))
+const picker = getFilePickerBuilder(t('files', 'Choose a file or folder to transfer'))
 	.setMultiSelect(false)
 	.setModal(true)
 	.setType(1)
@@ -115,6 +110,19 @@ export default {
 					icon: 'icon-user'
 				}
 			})
+		},
+		submitButtonText() {
+			if (!this.canSubmit) {
+				return t('files', 'Transfer')
+			}
+			const components = this.readableDirectory.split('/')
+			return t('files', 'Transfer {path} to {userid}', { path: components[components.length - 1], userid: this.selectedUser.displayName })
+		},
+		readableDirectory() {
+			if (!this.directory) {
+				return ''
+			}
+			return this.directory.substring(1)
 		}
 	},
 	created() {
@@ -127,7 +135,7 @@ export default {
 			picker.pick()
 				.then(dir => dir === '' ? '/' : dir)
 				.then(dir => {
-					logger.debug(`path ${dir} selected for transfer ownership`)
+					logger.debug(`path ${dir} selected for transferring ownership`)
 					if (!dir.startsWith('/')) {
 						throw new Error(t('files', 'Invalid path selected'))
 					}
@@ -135,7 +143,7 @@ export default {
 					// /ocs/v2.php/apps/files/api/v1/transferownership
 					this.directory = dir
 				}).catch(error => {
-					logger.error(`Selecting dir for transfer aborted: ${error.message || 'Unknown error'}`, { error })
+					logger.error(`Selecting object for transfer aborted: ${error.message || 'Unknown error'}`, { error })
 
 					this.directoryPickerError = error.message || t('files', 'Unknown error')
 				})
@@ -210,5 +218,11 @@ export default {
 </script>
 
 <style scoped>
-
+.middle-align {
+	vertical-align: middle;
+}
+p {
+	margin-top: 12px;
+	margin-bottom: 12px;
+}
 </style>