Merge pull request #2337 from LibreSign/feature/unify-list-and-validate-response

Unify list and validate response

appinfo/routes/routesPageController.php

@@ -10,7 +10,7 @@ return [
 		['name' => 'page#index',                      'url' => '/', 'verb' => 'GET'],
 		['name' => 'page#indexF',                     'url' => '/f/', 'verb' => 'GET'],
 		['name' => 'page#indexFPath',                 'url' => '/f/{path}', 'verb' => 'GET', 'requirements' => ['path' => '.+'], 'postfix' => 'front'],
-		['name' => 'page#getPdfUser',                 'url' => '/pdf/user/{uuid}', 'verb' => 'GET'],
+		['name' => 'page#getPdfAccountFile',          'url' => '/pdf/user/{uuid}', 'verb' => 'GET'],
 		['name' => 'page#resetPassword',              'url' => '/reset-password', 'verb' => 'GET'],
 		// Pages - public
 		['name' => 'page#sign',                       'url' => '/p/sign/{uuid}', 'verb' => 'GET'],

lib/Controller/AccountController.php

@@ -108,7 +108,7 @@ class AccountController extends ApiController implements ISignatureUuid {
 				'message' => $this->l10n->t('Success'),
 				'action' => JSActions::ACTION_SIGN,
 				'pdf' => [
-					'url' => $this->urlGenerator->linkToRoute('libresign.page.getPdfUser', ['uuid' => $uuid])
+					'url' => $this->urlGenerator->linkToRoute('libresign.page.getPdfAccountFile', ['uuid' => $uuid])
 				],
 				'filename' => $fileToSign['fileData']->getName(),
 				'description' => $signRequest->getDescription()

lib/Controller/FileController.php

@@ -136,7 +136,9 @@ class FileController extends Controller {
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
 	public function list($page = null, $length = null): JSONResponse {
-		$return = $this->fileService->listAssociatedFilesOfSignFlow($this->userSession->getUser(), $page, $length);
+		$return = $this->fileService
+			->setMe($this->userSession->getUser())
+			->listAssociatedFilesOfSignFlow($page, $length);
 		return new JSONResponse($return, Http::STATUS_OK);
 	}
 

lib/Controller/PageController.php

@@ -286,13 +286,10 @@ class PageController extends AEnvironmentPageAwareController {
 	#[PublicPage]
 	#[RequireSetupOk]
 	#[AnonRateLimit(limit: 30, period: 60)]
-	public function getPdfUser($uuid) {
+	public function getPdfAccountFile($uuid) {
 		$this->throwIfValidationPageNotAccessible();
 		$resp = new FileDisplayResponse($this->getNextcloudFile());
 		$resp->addHeader('Content-Type', 'application/pdf');
-		$csp = new ContentSecurityPolicy();
-		$csp->addAllowedFrameDomain('\'self\'');
-		$resp->setContentSecurityPolicy($csp);
 
 		return $resp;
 	}

lib/Db/AccountFileMapper.php

@@ -97,7 +97,7 @@ class AccountFileMapper extends QBMapper {
 		$pagination->setCurrentPage($page);
 		$currentPageResults = $pagination->getCurrentPageResults();
 
-		$url = $this->urlGenerator->linkToRoute('libresign.page.getPdfUser', ['uuid' => '_replace_']);
+		$url = $this->urlGenerator->linkToRoute('libresign.page.getPdfAccountFile', ['uuid' => '_replace_']);
 		$url = str_replace('_replace_', '', $url);
 
 		$data = [];

lib/Db/SignRequestMapper.php

@@ -33,7 +33,9 @@ use OCP\AppFramework\Db\QBMapper;
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\IDBConnection;
 use OCP\IL10N;
+use OCP\IURLGenerator;
 use OCP\IUser;
+use OCP\IUserManager;
 
 /**
  * Class SignRequestMapper
@@ -50,6 +52,8 @@ class SignRequestMapper extends QBMapper {
 		IDBConnection $db,
 		protected IL10N $l10n,
 		protected FileMapper $fileMapper,
+		private IUserManager $userManager,
+		private IURLGenerator $urlGenerator,
 	) {
 		parent::__construct($db, 'libresign_sign_request');
 	}
@@ -312,7 +316,6 @@ class SignRequestMapper extends QBMapper {
 	 */
 	public function getFilesAssociatedFilesWithMeFormatted(
 		IUser $user,
-		string $url,
 		int $page = null,
 		int $length = null
 	): array {
@@ -326,7 +329,7 @@ class SignRequestMapper extends QBMapper {
 
 		foreach ($currentPageResults as $row) {
 			$fileIds[] = $row['id'];
-			$data[] = $this->formatListRow($row, $url);
+			$data[] = $this->formatListRow($row);
 		}
 		$signers = $this->getByMultipleFileId($fileIds);
 		$identifyMethods = $this->getIdentifyMethodsFromSigners($signers);
@@ -371,11 +374,11 @@ class SignRequestMapper extends QBMapper {
 		$qb = $this->db->getQueryBuilder();
 		$qb->select(
 			'f.id',
+			'f.node_id',
+			'f.user_id',
 			'f.uuid',
 			'f.name',
-			'f.callback',
-			'f.status',
-			'f.node_id'
+			'f.status'
 		)
 			->selectAlias('f.created_at', 'request_date')
 			->from('libresign_file', 'f')
@@ -383,10 +386,11 @@ class SignRequestMapper extends QBMapper {
 			->leftJoin('f', 'libresign_identify_method', 'im', $qb->expr()->eq('sr.id', 'im.sign_request_id'))
 			->groupBy(
 				'f.id',
+				'f.node_id',
+				'f.user_id',
 				'f.uuid',
 				'f.name',
-				'f.callback',
-				'f.node_id',
+				'f.status',
 				'f.created_at',
 			);
 
@@ -451,13 +455,7 @@ class SignRequestMapper extends QBMapper {
 						'request_sign_date' => (new \DateTime())
 							->setTimestamp($signer->getCreatedAt())
 							->format('Y-m-d H:i:s'),
-						'sign_date' => null,
-						'uid' => array_reduce($identifyMethodsOfSigner, function (string $carry, IdentifyMethod $identifyMethod): string {
-							if ($identifyMethod->getIdentifierKey() === IdentifyMethodService::IDENTIFY_ACCOUNT) {
-								return $identifyMethod->getIdentifierValue();
-							}
-							return $carry;
-						}, ''),
+						'signed' => null,
 						'signRequestId' => $signer->getId(),
 						'me' => array_reduce($identifyMethodsOfSigner, function (bool $carry, IdentifyMethod $identifyMethod) use ($user): bool {
 							if ($identifyMethod->getIdentifierKey() === IdentifyMethodService::IDENTIFY_ACCOUNT) {
@@ -477,8 +475,8 @@ class SignRequestMapper extends QBMapper {
 						'identifyMethods' => array_map(function (IdentifyMethod $identifyMethod) use ($signer): array {
 							return [
 								'method' => $identifyMethod->getIdentifierKey(),
+								'value' => $identifyMethod->getIdentifierValue(),
 								'mandatory' => $identifyMethod->getMandatory(),
-								'identifiedAtDate' => $identifyMethod->getIdentifiedAtDate()
 							];
 						}, array_values($identifyMethodsOfSigner)),
 					];
@@ -488,11 +486,12 @@ class SignRequestMapper extends QBMapper {
 					}
 
 					if ($signer->getSigned()) {
-						$data['sign_date'] = (new \DateTime())
+						$data['signed'] = (new \DateTime())
 							->setTimestamp($signer->getSigned())
 							->format('Y-m-d H:i:s');
 						$totalSigned++;
 					}
+					ksort($data);
 					$files[$key]['signers'][] = $data;
 					unset($signers[$signerKey]);
 				}
@@ -504,21 +503,31 @@ class SignRequestMapper extends QBMapper {
 				$files[$key]['statusText'] = $this->fileMapper->getTextOfStatus((int) $files[$key]['status']);
 			}
 			unset($files[$key]['id']);
+			ksort($files[$key]);
 		}
 		return $files;
 	}
 
-	private function formatListRow(array $row, string $url): array {
+	private function formatListRow(array $row): array {
 		$row['id'] = (int) $row['id'];
 		$row['status'] = (int) $row['status'];
+		$row['statusText'] = $this->fileMapper->getTextOfStatus($row['status']);
+		$row['nodeId'] = (int) $row['node_id'];
+		$row['uuid'] = $row['uuid'];
+		$row['name'] = $row['name'];
+		$row['requested_by'] = [
+			'uid' => $row['user_id'],
+			'displayName' => $this->userManager->get($row['user_id'])->getDisplayName(),
+		];
 		$row['request_date'] = (new \DateTime())
 			->setTimestamp((int) $row['request_date'])
 			->format('Y-m-d H:i:s');
-		$row['type'] = 'pdf';
-		$row['url'] = $url . $row['uuid'];
+		$row['file'] = $this->urlGenerator->linkToRoute('libresign.page.getPdf', ['uuid' => $row['uuid']]);
+		$row['url'] = $this->urlGenerator->linkToRoute('libresign.page.getPdfAccountFile', ['uuid' => $row['uuid']]);
 		$row['nodeId'] = (int) $row['node_id'];
 		$row['uuid'] = $row['uuid'];
 		unset(
+			$row['user_id'],
 			$row['node_id'],
 		);
 		return $row;

lib/Service/FileService.php

@@ -181,14 +181,24 @@ class FileService {
 		$signers = $this->signRequestMapper->getByFileId($this->file->getId());
 		foreach ($signers as $signer) {
 			$signatureToShow = [
-				'signed' => $signer->getSigned(),
+				'signed' => null,
 				'displayName' => $signer->getDisplayName(),
 				'me' => false,
 				'signRequestId' => $signer->getId(),
+				'description' => $signer->getDescription(),
 				'identifyMethods' => $this->identifyMethodService->getIdentifyMethodsFromSignRequestId($signer->getId()),
+				'request_sign_date' => (new \DateTime())
+					->setTimestamp($signer->getCreatedAt())
+					->format('Y-m-d H:i:s'),
 			];
+			if ($signer->getSigned()) {
+				$data['sign_date'] = (new \DateTime())
+					->setTimestamp($signer->getSigned())
+					->format('Y-m-d H:i:s');
+			}
 			// @todo refactor this code
 			if ($this->me || $this->identifyMethodId) {
+				$signatureToShow['sign_uuid'] = $signer->getUuid();
 				$identifyMethodServices = $signatureToShow['identifyMethods'];
 				// Identifi if I'm file owner
 				if ($this->me?->getUID() === $this->file->getUserId()) {
@@ -197,7 +207,7 @@ class FileService {
 							$carry = $identifyMethod->getEntity()->getIdentifierValue();
 						}
 						return $carry;
-					});
+					}, '');
 					$signatureToShow['email'] = $email;
 					$user = $this->userManager->getByEmail($email);
 					if ($user && count($user) === 1) {
@@ -226,10 +236,12 @@ class FileService {
 					$carry[] = [
 						'method' => $identifyMethod->getEntity()->getIdentifierKey(),
 						'value' => $identifyMethod->getEntity()->getIdentifierValue(),
+						'mandatory' => $identifyMethod->getEntity()->getMandatory(),
 					];
 				}
 				return $carry;
 			}, []);
+			ksort($signatureToShow);
 			$this->signers[] = $signatureToShow;
 		}
 		return $this->signers;
@@ -349,20 +361,21 @@ class FileService {
 		if (!$this->file) {
 			return $return;
 		}
-		$return['status'] = $this->file->getStatus();
-		$return['statusText'] = $this->fileMapper->getTextOfStatus($this->file->getStatus());
-		$return['nodeId'] = $this->file->getNodeId();
 		$return['uuid'] = $this->file->getUuid();
 		$return['name'] = $this->file->getName();
-		$return['file'] = $this->urlGenerator->linkToRoute('libresign.page.getPdf', ['uuid' => $this->file->getUuid()]);
+		$return['status'] = $this->file->getStatus();
+		$return['request_date'] = (new \DateTime())
+			->setTimestamp($this->file->getCreatedAt())
+			->format('Y-m-d H:i:s');
+		$return['statusText'] = $this->fileMapper->getTextOfStatus($this->file->getStatus());
+		$return['nodeId'] = $this->file->getNodeId();
 
 		$return['requested_by'] = [
 			'uid' => $this->file->getUserId(),
 			'displayName' => $this->userManager->get($this->file->getUserId())->getDisplayName(),
 		];
-		$return['request_date'] = (new \DateTime())
-			->setTimestamp($this->file->getCreatedAt())
-			->format('Y-m-d H:i:s');
+		$return['file'] = $this->urlGenerator->linkToRoute('libresign.page.getPdf', ['uuid' => $this->file->getUuid()]);
+		$return['url'] = $this->urlGenerator->linkToRoute('libresign.page.getPdfAccountFile', ['uuid' => $this->file->getUuid()]);
 		if ($this->showSigners) {
 			$return['signers'] = $this->getSigners();
 		}
@@ -372,6 +385,7 @@ class FileService {
 				$return['visibleElements'] = $visibleElements;
 			}
 		}
+		ksort($return);
 		return $return;
 	}
 
@@ -427,16 +441,12 @@ class FileService {
 	 *
 	 * @psalm-return array{data: array, pagination: array}
 	 */
-	public function listAssociatedFilesOfSignFlow(IUser $user, $page = null, $length = null): array {
+	public function listAssociatedFilesOfSignFlow($page = null, $length = null): array {
 		$page = $page ?? 1;
 		$length = $length ?? (int) $this->appConfig->getAppValue('length_of_page', '100');
 
-		$url = $this->urlGenerator->linkToRoute('libresign.page.getPdfUser', ['uuid' => '_replace_']);
-		$url = str_replace('_replace_', '', $url);
-
 		$data = $this->signRequestMapper->getFilesAssociatedFilesWithMeFormatted(
-			$user,
-			$url,
+			$this->me,
 			$page,
 			$length
 		);

lib/Service/SignFileService.php

@@ -675,7 +675,7 @@ class SignFileService {
 					$this->accountFileMapper->getByFileId($fileEntity->getId());
 					$url = ['url' => $this->urlGenerator->linkToRoute('libresign.page.getPdf', ['uuid' => $uuid])];
 				} catch (DoesNotExistException $e) {
-					$url = ['url' => $this->urlGenerator->linkToRoute('libresign.page.getPdfUser', ['uuid' => $uuid])];
+					$url = ['url' => $this->urlGenerator->linkToRoute('libresign.page.getPdfAccountFile', ['uuid' => $uuid])];
 				}
 				break;
 			case 'nodeId':

src/Components/Request/IdentifySigner.vue

@@ -2,7 +2,7 @@
 	<div class="identifySigner">
 		<AccountOrEmail v-if="methods.account.enabled || methods.email.enabled"
 			:required="methods.account.required || methods.email.required"
-			:signer="methods.account.value || methods.email.value"
+			:signer="signer"
 			:placeholder="placeholder"
 			@update:account="updateAccount"
 			@update:email="updateEmail"
@@ -68,17 +68,36 @@ export default {
 				account: {
 					enabled: false,
 					required: false,
-					value: {},
+					value: '',
 				},
 				email: {
 					enabled: false,
 					required: false,
-					value: {},
+					value: '',
 				},
 			},
 		}
 	},
 	computed: {
+		signer() {
+			if (this.methods.account.value.length > 0) {
+				return {
+					id: this.methods.account.value,
+					icon: 'icon-user',
+					isNoUser: false,
+					displayName: this.signerToEdit.displayName,
+				}
+			} else if (this.methods.email.value.length > 0) {
+				return {
+					id: this.methods.email.value,
+					icon: 'icon-mail',
+					isNoUser: true,
+					displayName: this.signerToEdit.displayName,
+				}
+			} else {
+				return {}
+			}
+		},
 		isNewSigner() {
 			return this.id === null || this.id === undefined
 		},
@@ -98,21 +117,16 @@ export default {
 	beforeMount() {
 		this.displayName = ''
 		this.identify = ''
-		this.methods.account.value = {}
-		this.methods.email.value = {}
+		this.methods.account.value = ''
+		this.methods.email.value = ''
 		if (Object.keys(this.signerToEdit).length > 0) {
-			this.name = this.signerToEdit.displayName
 			this.identify = this.signerToEdit.identify ?? this.signerToEdit.signRequestId
+			this.updateDisplayName(this.signerToEdit.displayName)
 			this.signerToEdit.identifyMethods.forEach(method => {
-				this.updateName(method.value?.displayName ?? this.name)
 				if (method.method === 'email') {
-					this.methods.email.value = method.value ?? this.signerToEdit.email
+					this.methods.email.value = method.value
 				} else if (method.method === 'account') {
-					this.updateName(this.signerToEdit.displayName ?? this.name)
-					this.methods.account.value = method.value ?? {
-						account: this.signerToEdit.uid,
-						displayName: this.signerToEdit.displayName,
-					}
+					this.methods.account.value = method.value
 				}
 			})
 		}
@@ -162,24 +176,21 @@ export default {
 			this.filesStore.disableIdentifySigner()
 		},
 		updateDisplayName(name) {
-			this.displayName = name
+			this.displayName = name ?? ''
 		},
 		updateEmail(email) {
-			if (typeof email === 'object' && Object.hasOwn(email, 'displayName')) {
-				email = email.displayName
-			} else if (typeof email === 'boolean') {
-				email = ''
+			if (typeof email !== 'object') {
+				this.methods.email.value = ''
+			} else {
+				this.methods.email.value = email.id
 			}
-			this.methods.email.value = email
 		},
 		updateAccount(account) {
 			if (typeof account !== 'object') {
-				account = {}
+				this.methods.account.value = ''
+			} else {
+				this.methods.account.value = account.id
 			}
-			this.methods.account.value = account
-		},
-		updateName(name) {
-			this.displayName = name
 		},
 		onNameChange() {
 			const name = this.displayName.trim()

src/Components/Request/RequestSignature.vue

@@ -12,7 +12,7 @@
 		<Signers :signers="dataSigners"
 			event="libresign:edit-signer">
 			<template #actions="{signer}">
-				<NcActionButton v-if="canRequestSign && !signer.sign_date"
+				<NcActionButton v-if="canRequestSign && !signer.signed"
 					aria-label="Delete"
 					:close-after-click="true"
 					@click="filesStore.deleteSigner(signer)">
@@ -21,7 +21,7 @@
 					</template>
 					{{ t('libresign', 'Delete') }}
 				</NcActionButton>
-				<NcActionButton v-if="canRequestSign && !signer.sign_date && signer.signRequestId && !signer.me"
+				<NcActionButton v-if="canRequestSign && !signer.signed && signer.signRequestId && !signer.me"
 					icon="icon-comment"
 					:close-after-click="true"
 					@click="sendNotify(signer)">

src/Components/Signers/Signer.vue

@@ -64,7 +64,7 @@ export default {
 			return this.signer.identifyMethods.map(method => method.method)
 		},
 		statusColor() {
-			if (this.signer.sign_date) {
+			if (this.signer.signed) {
 				return '#008000'
 			}
 			// Pending
@@ -75,9 +75,9 @@ export default {
 			return '#dbdbdb'
 		},
 		statusText() {
-			if (this.signer.sign_date) {
+			if (this.signer.signed) {
 				return t('libresign', 'signed at {date}', {
-					date: Moment(this.signer.request_sign_date).format('LLL'),
+					date: Moment(this.signer.request_signed).format('LLL'),
 				})
 			}
 			// Pending
@@ -96,7 +96,7 @@ export default {
 			if (this.event.length === 0) {
 				return
 			}
-			if (this.signer.sign_date) {
+			if (this.signer.signed) {
 				return
 			}
 			emit(this.event, this.signer)

src/store/files.js

@@ -59,19 +59,19 @@ export const useFilesStore = defineStore('files', {
 			if (!Object.hasOwn(this.getFile(), 'signers')) {
 				return false
 			}
-			return this.files[this.selectedNodeId].signers.filter(signer => signer.sign_date?.length > 0).length > 0
+			return this.files[this.selectedNodeId].signers.filter(signer => signer.signed?.length > 0).length > 0
 		},
 		getSubtitle() {
 			if (this.selectedNodeId === 0) {
 				return ''
 			}
 			const file = this.files[this.selectedNodeId]
-			if ((file?.requested_by?.uid ?? '').length === 0 || file?.requestDate.length === 0) {
+			if ((file?.requested_by?.uid ?? '').length === 0 || file?.request_date.length === 0) {
 				return ''
 			}
 			return t('libresign', 'Requested by {name}, at {date}', {
 				name: file.requested_by.uid,
-				date: Moment(Date.parse(file.requestDate)).format('LL LTS'),
+				date: Moment(Date.parse(file.request_date)).format('LL LTS'),
 			})
 		},
 		async hydrateFile(nodeId) {

tests/integration/features/page/sign_identify_account.feature

@@ -23,15 +23,13 @@ Feature: page/sign_identify_account
       | key        | value                   |
       | uuid       | <IGNORE>                |
       | name       | document                |
-      | callback   |                         |
       | status     | 1                       |
       | statusText | available for signature |
     And the signer contains
       | key | value |
-      | uid | signer1 |
       | email | |
       | me | true |
-      | identifyMethods | [{"method":"account","mandatory":1,"identifiedAtDate":null}] |
+      | identifyMethods | [{"method":"account","value":"signer1","mandatory":1}] |
     # invalid UUID, need to be the signer UUID
     When as user "signer1"
     And sending "get" to "/apps/libresign/p/sign/<FILE_UUID>"
@@ -78,15 +76,13 @@ Feature: page/sign_identify_account
       | key        | value                   |
       | uuid       | <IGNORE>                |
       | name       | document                |
-      | callback   |                         |
       | status     | 1                       |
       | statusText | available for signature |
     And the signer contains
       | key | value |
-      | uid | signer1 |
       | email | |
       | me | true |
-      | identifyMethods | [{"method":"account","mandatory":1,"identifiedAtDate":null}] |
+      | identifyMethods | [{"method":"account","value":"signer1","mandatory":1}] |
     When as user "signer1"
     And sending "get" to "/apps/libresign/p/sign/<SIGN_UUID>"
     And the response should contain the initial state "libresign-action" with the following values: