From fd49e6159cae709e1b0e5500a955a6489917b905 Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Mon, 12 Oct 2026 14:26:06 +0200 Subject: [PATCH 01/10] feat(usage): show the business owner by name in the Applications in use list The business owner column showed the contact person's uuid. A contact person in OpenRegister carries only a contactsUid; the name is in Nextcloud Contacts (the CMDB import puts its owners in the importing admin's "Stackiq CMDB owners" address book). GET /api/contact-names?ids[]= gives the names for a signed-in user: it reads the contact persons with the caller's own OpenRegister rights and looks each contactsUid up in the address books Nextcloud gives the caller. A name is only returned to someone who can already read both the contact person and the contact. There is no public route and nothing is stored, so the owner stays unreadable for anonymous visitors, OpenCatalogi and portaliq. The new contactName cell widget batches the cells of one page into one request and keeps the uuid when the caller has no name for it. Co-Authored-By: Claude Opus 5.5 (1M context) --- appinfo/routes.php | 2 + l10n/en.js | 4 +- l10n/en.json | 4 +- l10n/nl.js | 4 +- l10n/nl.json | 4 +- lib/Controller/ContactNamesController.php | 249 ++++++++++++++++++ src/App.vue | 4 + src/cellWidgets.js | 14 + src/components/cells/ContactNameCell.vue | 69 +++++ src/manifest.d/usages.json | 2 +- src/utils/contactNames.js | 106 ++++++++ .../Controller/ContactNamesControllerTest.php | 219 +++++++++++++++ tests/vitest/contactNames.spec.js | 111 ++++++++ 13 files changed, 787 insertions(+), 5 deletions(-) create mode 100644 lib/Controller/ContactNamesController.php create mode 100644 src/cellWidgets.js create mode 100644 src/components/cells/ContactNameCell.vue create mode 100644 src/utils/contactNames.js create mode 100644 tests/Unit/Controller/ContactNamesControllerTest.php create mode 100644 tests/vitest/contactNames.spec.js diff --git a/appinfo/routes.php b/appinfo/routes.php index 9fc761db5..0eab59877 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -168,6 +168,8 @@ // User Profile endpoint ['name' => 'contactpersonen#getMe', 'url' => '/api/me', 'verb' => 'GET'], + // Display names of contact persons from the caller's own address books (owner columns). + ['name' => 'contactNames#index', 'url' => '/api/contact-names', 'verb' => 'GET'], // Contactpersonen Management endpoints ['name' => 'contactpersonen#getContactpersonen', 'url' => '/api/contactpersonen/organisation/{organisationId}', 'verb' => 'GET'], diff --git a/l10n/en.js b/l10n/en.js index 8dd84a2f4..a02d4aabf 100644 --- a/l10n/en.js +++ b/l10n/en.js @@ -1132,7 +1132,9 @@ OC.L10N.register( "The workbook holds more than {count} different texts, the most the import reads.": "The workbook holds more than {count} different texts, the most the import reads.", "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.": "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.", "In development since": "In development since", - "In use since": "In use since" + "In use since": "In use since", + "Not signed in": "Not signed in", + "At most %s contact persons per request": "At most %s contact persons per request" }, "nplurals=2; plural=(n != 1);" ) diff --git a/l10n/en.json b/l10n/en.json index 313f968ea..02c4b18b2 100644 --- a/l10n/en.json +++ b/l10n/en.json @@ -1131,6 +1131,8 @@ "The workbook holds more than {count} different texts, the most the import reads.": "The workbook holds more than {count} different texts, the most the import reads.", "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.": "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.", "In development since": "In development since", - "In use since": "In use since" + "In use since": "In use since", + "Not signed in": "Not signed in", + "At most %s contact persons per request": "At most %s contact persons per request" } } diff --git a/l10n/nl.js b/l10n/nl.js index 3151813d6..927c85f6a 100644 --- a/l10n/nl.js +++ b/l10n/nl.js @@ -1200,7 +1200,9 @@ OC.L10N.register( "The workbook holds more than {count} different texts, the most the import reads.": "De werkmap bevat meer dan {count} verschillende teksten, het maximum dat de import leest.", "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.": "Samen verwijzen de cellen van de werkmap naar meer dan {size} gedeelde tekst, het maximum dat de import leest.", "In development since": "In ontwikkeling sinds", - "In use since": "In gebruik sinds" + "In use since": "In gebruik sinds", + "Not signed in": "Niet aangemeld", + "At most %s contact persons per request": "Ten hoogste %s contactpersonen per verzoek" }, "nplurals=2; plural=(n != 1);" ) diff --git a/l10n/nl.json b/l10n/nl.json index 680f6f853..d97f06ce7 100644 --- a/l10n/nl.json +++ b/l10n/nl.json @@ -1199,6 +1199,8 @@ "The workbook holds more than {count} different texts, the most the import reads.": "De werkmap bevat meer dan {count} verschillende teksten, het maximum dat de import leest.", "Together, the cells of the workbook reference more than {size} of shared text, the most the import reads.": "Samen verwijzen de cellen van de werkmap naar meer dan {size} gedeelde tekst, het maximum dat de import leest.", "In development since": "In ontwikkeling sinds", - "In use since": "In gebruik sinds" + "In use since": "In gebruik sinds", + "Not signed in": "Niet aangemeld", + "At most %s contact persons per request": "Ten hoogste %s contactpersonen per verzoek" } } diff --git a/lib/Controller/ContactNamesController.php b/lib/Controller/ContactNamesController.php new file mode 100644 index 000000000..086a6d400 --- /dev/null +++ b/lib/Controller/ContactNamesController.php @@ -0,0 +1,249 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://github.com/ConductionNL/stackiq + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Controller; + +use OCA\OpenRegister\Contract\ObjectServiceInterface; +use OCA\Stackiq\Service\SettingsService; +use OCA\Stackiq\Service\StackiqContactSyncService; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\NoAdminRequired; +use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\JSONResponse; +use OCP\IL10N; +use OCP\IRequest; +use OCP\IUserSession; +use Psr\Log\LoggerInterface; +use Throwable; + +/** + * The display names of contact persons, for a signed-in user. + * + * A contact person in OpenRegister carries only a `contactsUid`; the name + * lives in Nextcloud Contacts (the CMDB import puts its owners in the + * importing admin's "Stackiq CMDB owners" address book). This endpoint reads + * the contact persons with the caller's own OpenRegister rights, then looks + * each `contactsUid` up in the address books Nextcloud gives the caller. A + * name is therefore returned only to someone who can already read both the + * contact person and the contact. Nothing is stored, and there is no public + * route. + * + * @category Controller + * @package OCA\Stackiq\Controller + * + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + */ +class ContactNamesController extends Controller { + + /** + * The most contact persons one request may name; a list page shows at most this many rows. + */ + public const MAX_IDS = 100; + + /** + * Constructor for ContactNamesController. + * + * @param string $appName The app name. + * @param IRequest $request The request object. + * @param IUserSession $userSession The user session. + * @param IL10N $l10n Translations for the error messages. + * @param SettingsService $settingsService Gives the register and contact person schema. + * @param StackiqContactSyncService $contactSync Reads Nextcloud Contacts as the signed-in user. + * @param ObjectServiceInterface $objectService OpenRegister object access, with the caller's rights. + * @param LoggerInterface $logger The logger. + */ + public function __construct( + string $appName, + IRequest $request, + private readonly IUserSession $userSession, + private readonly IL10N $l10n, + private readonly SettingsService $settingsService, + private readonly StackiqContactSyncService $contactSync, + private readonly ObjectServiceInterface $objectService, + private readonly LoggerInterface $logger, + ) { + parent::__construct(appName: $appName, request: $request); + }//end __construct() + + /** + * The display names of the given contact persons. + * + * API Endpoint: GET /api/contact-names?ids[]= + * + * Answers `{"names": {"": ""}}`. The per-object + * check is OpenRegister's: the contact persons are read with the caller's + * RBAC and organisation scope, so one the caller may not read is left out, + * as is one whose contact is not in the caller's address books. + * + * @return JSONResponse 200 with the names, 400 for more than MAX_IDS ids, 401 when not signed in. + * + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + */ + #[NoAdminRequired] + #[NoCSRFRequired] + public function index(): JSONResponse { + if ($this->userSession->getUser() === null) { + return new JSONResponse( + ['message' => $this->l10n->t('Not signed in'), 'error' => 'not-authenticated'], + Http::STATUS_UNAUTHORIZED + ); + } + + $ids = $this->requestedIds(); + if (count($ids) > self::MAX_IDS) { + return new JSONResponse( + ['message' => $this->l10n->t('At most %s contact persons per request', [self::MAX_IDS]), 'error' => 'too-many-ids'], + Http::STATUS_BAD_REQUEST + ); + } + + if ($ids === [] || $this->contactSync->isAvailable() === false) { + return new JSONResponse(['names' => (object)[]], Http::STATUS_OK); + } + + $names = []; + foreach ($this->contactsUidsOf(ids: $ids) as $uuid => $contactsUid) { + $contact = $this->contactSync->findContactByUid(uid: $contactsUid); + $name = self::firstValue(value: ($contact['FN'] ?? '')); + if ($name !== '') { + $names[$uuid] = $name; + } + } + + return new JSONResponse(['names' => (object)$names], Http::STATUS_OK); + }//end index() + + /** + * The distinct, well-formed uuids in the `ids` query parameter, lower case. + * + * @return array + */ + private function requestedIds(): array { + $raw = $this->request->getParam('ids', []); + if (is_string($raw) === true) { + $raw = [$raw]; + } + + if (is_array($raw) === false) { + return []; + } + + $ids = []; + foreach ($raw as $id) { + if (is_string($id) === true && preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i', $id) === 1) { + $ids[strtolower($id)] = true; + } + } + + return array_keys($ids); + }//end requestedIds() + + /** + * The `contactsUid` of each requested contact person the caller may read. + * + * Fails closed: without a configured register and contact person schema, + * or when OpenRegister cannot be read, no contact person is resolved. + * + * @param array $ids Contact person uuids, lower case. + * + * @return array uuid => contactsUid. + */ + private function contactsUidsOf(array $ids): array { + $config = $this->settingsService->getVoorzieningenConfig(); + $register = ($config['register'] ?? null); + $schema = ($config[SettingsService::LEGACY_SCHEMA_KEY['contactPerson']] ?? null); + if (empty($register) === true || empty($schema) === true) { + $this->logger->warning('[ContactNames] The register or the contact person schema is not configured; no names are given'); + return []; + } + + try { + // RBAC and multitenancy stay at their defaults: the caller's own rights. + $result = $this->objectService->searchObjectsPaginated( + query: [ + '@self' => ['register' => (int)$register, 'schema' => (int)$schema], + '_limit' => self::MAX_IDS, + ], + ids: $ids + ); + } catch (Throwable $e) { + $this->logger->warning('[ContactNames] The contact persons could not be read', ['exception' => $e]); + return []; + } + + $uids = []; + foreach (($result['results'] ?? []) as $entry) { + $data = self::objectData(entry: $entry); + $uuid = strtolower((string)($data['id'] ?? ($data['@self']['id'] ?? ''))); + $contactsUid = trim((string)($data['contactsUid'] ?? '')); + // The ids filter is OpenRegister's; a record it did not ask for is never named. + if ($contactsUid !== '' && in_array($uuid, $ids, true) === true) { + $uids[$uuid] = $contactsUid; + } + } + + return $uids; + }//end contactsUidsOf() + + /** + * The data of one search result, an entity or an array. + * + * @param mixed $entry The search result. + * + * @return array + */ + private static function objectData(mixed $entry): array { + if (is_array($entry) === true) { + return $entry; + } + + if (is_object($entry) === true && method_exists($entry, 'jsonSerialize') === true) { + $data = $entry->jsonSerialize(); + if (is_array($data) === true) { + return $data; + } + } + + return []; + }//end objectData() + + /** + * The first value of a vCard property, which the contacts manager may give as a list. + * + * @param mixed $value The property value. + * + * @return string + */ + private static function firstValue(mixed $value): string { + if (is_array($value) === true) { + $value = (reset($value) ?? ''); + } + + if (is_scalar($value) === false) { + return ''; + } + + return trim((string)$value); + }//end firstValue() +}//end class diff --git a/src/App.vue b/src/App.vue index 89ee4fb9e..c7db8a038 100644 --- a/src/App.vue +++ b/src/App.vue @@ -21,6 +21,7 @@ :manifest="manifest" :customComponents="customComponents" :formatters="formatters" + :cellWidgets="cellWidgets" :registry="registry" :pageTypes="pageTypes" appId="stackiq" @@ -75,6 +76,7 @@ import { reactive } from 'vue' import OrganisationSwitcher from './components/organisations/OrganisationSwitcher.vue' import Dialogs from './dialogs/Dialogs.vue' import Modals from './modals/Modals.vue' +import appCellWidgets from './cellWidgets.js' import { setActiveOrganisationUuid } from './composables/orClient.js' import appFormatters from './formatters.js' import { settingsStore } from './store/store.js' @@ -152,6 +154,8 @@ export default { return { // App cell formatters for manifest columns (`columns[].formatter`). formatters: appFormatters, + // App cell widgets for manifest columns (`columns[].widget`). + cellWidgets: appCellWidgets, objectSidebarState: reactive({ active: false, open: true, diff --git a/src/cellWidgets.js b/src/cellWidgets.js new file mode 100644 index 000000000..de491599a --- /dev/null +++ b/src/cellWidgets.js @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: EUPL-1.2 +// Copyright (C) 2026 Conduction B.V. +// +// App cell widgets, handed to CnAppRoot as `cellWidgets` and resolved by id +// from a manifest column's `widget`. The library's own widgets (badge, +// fkResolve, link, avatar, date) are handled before this registry, so an id +// here must not equal one of theirs. + +import ContactNameCell from './components/cells/ContactNameCell.vue' + +export default { + // The owner column of the usage list (landscape-usage-registration). + contactName: ContactNameCell, +} diff --git a/src/components/cells/ContactNameCell.vue b/src/components/cells/ContactNameCell.vue new file mode 100644 index 000000000..06a7ec2f4 --- /dev/null +++ b/src/components/cells/ContactNameCell.vue @@ -0,0 +1,69 @@ + + + + + diff --git a/src/manifest.d/usages.json b/src/manifest.d/usages.json index 66990ad7d..df2b3d22e 100644 --- a/src/manifest.d/usages.json +++ b/src/manifest.d/usages.json @@ -23,7 +23,7 @@ { "key": "module", "label": "Application", "widget": "fkResolve", "widgetProps": { "register": "@resolve:voorzieningen_register", "schema": "module", "labelField": "name" } }, { "key": "moduleVersion", "label": "Version", "widget": "fkResolve", "widgetProps": { "register": "@resolve:voorzieningen_register", "schema": "moduleVersion", "labelField": "version" } }, "status", - "businessOwner", + { "key": "businessOwner", "label": "Business owner", "widget": "contactName" }, "technicalOwner", "timeClassification", "serviceDeskRecordId" diff --git a/src/utils/contactNames.js b/src/utils/contactNames.js new file mode 100644 index 000000000..6051639f4 --- /dev/null +++ b/src/utils/contactNames.js @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: EUPL-1.2 +// Copyright (C) 2026 Conduction B.V. + +/** + * Display names of contact persons, for the owner column of the usage list. + * + * A contact person in OpenRegister carries only a `contactsUid`; its name is + * in Nextcloud Contacts. `GET /api/contact-names` gives the names the + * signed-in user can read in their own address books. The cells of one page + * ask in the same tick, so they are batched into one request, and a name is + * asked for once per page load. + * + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + */ + +import axios from '@nextcloud/axios' +import { generateUrl } from '@nextcloud/router' + +/** The endpoint's own limit (`ContactNamesController::MAX_IDS`). */ +export const MAX_IDS_PER_REQUEST = 100 + +/** uuid -> Promise; null when no name could be read. */ +const cache = new Map() + +/** uuid -> { resolve } for the ids of the batch being collected. */ +let pending = new Map() + +/** + * The contact person uuid a cell value refers to. + * + * @param {string|object|null} value A uuid, or a related object with `id` or `uuid`. + * @return {string} The uuid, or ''. + */ +export function contactPersonId(value) { + if (typeof value === 'string') { + return value.trim() + } + if (value && typeof value === 'object') { + return String(value.id ?? value.uuid ?? '').trim() + } + return '' +} + +/** + * Ask the endpoint for the names of the collected ids, in chunks it accepts. + * + * @return {Promise} + */ +async function flush() { + const batch = pending + pending = new Map() + const ids = [...batch.keys()] + for (let start = 0; start < ids.length; start += MAX_IDS_PER_REQUEST) { + const chunk = ids.slice(start, start + MAX_IDS_PER_REQUEST) + let names = {} + try { + const { data } = await axios.get( + generateUrl('/apps/stackiq/api/contact-names'), + { params: { ids: chunk } }, + ) + names = data?.names ?? {} + } catch { + names = {} + } + chunk.forEach((id) => { + const name = typeof names[id] === 'string' ? names[id] : '' + batch.get(id).resolve(name !== '' ? name : null) + }) + } +} + +/** + * The display name of a contact person, or null when the signed-in user + * cannot read one. + * + * @param {string} id The contact person uuid. + * @return {Promise} The name. + */ +export function contactName(id) { + const key = String(id ?? '').toLowerCase() + if (key === '') { + return Promise.resolve(null) + } + if (!cache.has(key)) { + cache.set( + key, + new Promise((resolve) => { + if (pending.size === 0) { + Promise.resolve().then(flush) + } + pending.set(key, { resolve }) + }), + ) + } + return cache.get(key) +} + +/** + * Forget every name read so far (tests). + * + * @return {void} + */ +export function clearContactNames() { + cache.clear() + pending = new Map() +} diff --git a/tests/Unit/Controller/ContactNamesControllerTest.php b/tests/Unit/Controller/ContactNamesControllerTest.php new file mode 100644 index 000000000..0860873de --- /dev/null +++ b/tests/Unit/Controller/ContactNamesControllerTest.php @@ -0,0 +1,219 @@ + + * @copyright 2026 Conduction B.V. + * @license EUPL-1.2 https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * @link https://github.com/ConductionNL/stackiq + * + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + */ + +declare(strict_types=1); + +namespace OCA\Stackiq\Tests\Unit\Controller; + +use OCA\OpenRegister\Contract\ObjectServiceInterface; +use OCA\Stackiq\Controller\ContactNamesController; +use OCA\Stackiq\Service\SettingsService; +use OCA\Stackiq\Service\StackiqContactSyncService; +use OCP\IL10N; +use OCP\IRequest; +use OCP\IUser; +use OCP\IUserSession; +use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; + +/** + * The names endpoint answers only a signed-in user, reads the contact persons + * with that user's rights, and names only what it was asked for. + * + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + */ +class ContactNamesControllerTest extends TestCase { + + private const A = '00000000-0000-0000-0000-00000000000a'; + private const B = '00000000-0000-0000-0000-00000000000b'; + private const C = '00000000-0000-0000-0000-00000000000c'; + + /** @var ObjectServiceInterface&MockObject */ + private ObjectServiceInterface&MockObject $objectService; + + /** @var StackiqContactSyncService&MockObject */ + private StackiqContactSyncService&MockObject $contactSync; + + /** @var IUserSession&MockObject */ + private IUserSession&MockObject $userSession; + + /** @var SettingsService&MockObject */ + private SettingsService&MockObject $settingsService; + + /** + * Signed in, Contacts available, register and schema configured. + * + * @return void + */ + protected function setUp(): void { + $this->objectService = $this->createMock(ObjectServiceInterface::class); + $this->contactSync = $this->createMock(StackiqContactSyncService::class); + $this->contactSync->method('isAvailable')->willReturn(true); + $this->userSession = $this->createMock(IUserSession::class); + $this->userSession->method('getUser')->willReturn($this->createMock(IUser::class)); + $this->settingsService = $this->createMock(SettingsService::class); + $this->settingsService->method('getVoorzieningenConfig')->willReturn( + ['register' => '20', 'contactpersoon_schema' => '7'] + ); + }//end setUp() + + /** + * A controller for a request with these ids. + * + * @param mixed $ids The `ids` query parameter. + * + * @return ContactNamesController + */ + private function controller(mixed $ids): ContactNamesController { + $request = $this->createMock(IRequest::class); + $request->method('getParam')->willReturnCallback( + static fn (string $key, mixed $default = null): mixed => ($key === 'ids' ? $ids : $default) + ); + $l10n = $this->createMock(IL10N::class); + $l10n->method('t')->willReturnArgument(0); + + return new ContactNamesController( + appName: 'stackiq', + request: $request, + userSession: $this->userSession, + l10n: $l10n, + settingsService: $this->settingsService, + contactSync: $this->contactSync, + objectService: $this->objectService, + logger: $this->createMock(LoggerInterface::class) + ); + }//end controller() + + /** + * A contact person's name comes from its contact, read with the caller's rights. + * + * @return void + */ + public function testNamesTheReadableContactPersons(): void { + $this->objectService->expects($this->once()) + ->method('searchObjectsPaginated') + ->with( + $this->callback(static fn (array $query): bool => $query['@self'] === ['register' => 20, 'schema' => 7]), + // The caller's own RBAC and organisation scope. + true, + true, + false, + [self::A, self::B] + ) + ->willReturn( + [ + 'results' => [ + ['id' => self::A, 'contactsUid' => 'uid-a'], + ['id' => self::B, 'contactsUid' => 'uid-b'], + ], + ] + ); + $this->contactSync->method('findContactByUid')->willReturnMap( + [ + ['uid-a', ['UID' => 'uid-a', 'FN' => 'Teamleider Applicatiebeheer']], + ['uid-b', null], + ] + ); + + $response = $this->controller(ids: [self::A, strtoupper(self::B), 'not-a-uuid', self::A])->index(); + + $this->assertSame(200, $response->getStatus()); + $this->assertEquals(['names' => (object)[self::A => 'Teamleider Applicatiebeheer']], $response->getData()); + }//end testNamesTheReadableContactPersons() + + /** + * A record OpenRegister returns but that was not asked for is never named. + * + * @return void + */ + public function testNamesOnlyTheRequestedIds(): void { + $this->objectService->method('searchObjectsPaginated')->willReturn( + ['results' => [['id' => self::C, 'contactsUid' => 'uid-c']]] + ); + $this->contactSync->expects($this->never())->method('findContactByUid'); + + $response = $this->controller(ids: [self::A])->index(); + + $this->assertEquals(['names' => (object)[]], $response->getData()); + }//end testNamesOnlyTheRequestedIds() + + /** + * Without a signed-in user nothing is read. + * + * @return void + */ + public function testRefusesAnonymousCallers(): void { + $this->userSession = $this->createMock(IUserSession::class); + $this->userSession->method('getUser')->willReturn(null); + $this->objectService->expects($this->never())->method('searchObjectsPaginated'); + + $response = $this->controller(ids: [self::A])->index(); + + $this->assertSame(401, $response->getStatus()); + $this->assertSame('not-authenticated', $response->getData()['error']); + }//end testRefusesAnonymousCallers() + + /** + * More ids than one list page shows are refused. + * + * @return void + */ + public function testRefusesMoreThanOnePageOfIds(): void { + $ids = []; + for ($i = 0; $i <= ContactNamesController::MAX_IDS; $i++) { + $ids[] = sprintf('00000000-0000-0000-0000-%012d', $i); + } + + $this->objectService->expects($this->never())->method('searchObjectsPaginated'); + + $response = $this->controller(ids: $ids)->index(); + + $this->assertSame(400, $response->getStatus()); + }//end testRefusesMoreThanOnePageOfIds() + + /** + * Without a configured contact person schema no contact person is resolved. + * + * @return void + */ + public function testFailsClosedWithoutTheContactPersonSchema(): void { + $this->settingsService = $this->createMock(SettingsService::class); + $this->settingsService->method('getVoorzieningenConfig')->willReturn(['register' => '20']); + $this->objectService->expects($this->never())->method('searchObjectsPaginated'); + + $response = $this->controller(ids: [self::A])->index(); + + $this->assertSame(200, $response->getStatus()); + $this->assertEquals(['names' => (object)[]], $response->getData()); + }//end testFailsClosedWithoutTheContactPersonSchema() + + /** + * An OpenRegister failure gives no names, not a 500. + * + * @return void + */ + public function testAnUnreadableRegisterGivesNoNames(): void { + $this->objectService->method('searchObjectsPaginated')->willThrowException(new \RuntimeException('down')); + + $response = $this->controller(ids: self::A)->index(); + + $this->assertSame(200, $response->getStatus()); + $this->assertEquals(['names' => (object)[]], $response->getData()); + }//end testAnUnreadableRegisterGivesNoNames() +}//end class diff --git a/tests/vitest/contactNames.spec.js b/tests/vitest/contactNames.spec.js new file mode 100644 index 000000000..f301c045f --- /dev/null +++ b/tests/vitest/contactNames.spec.js @@ -0,0 +1,111 @@ +// @vitest-environment jsdom + +/** + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * The owner column of the usage list: contact persons shown by the name of + * their Nextcloud contact, read in one batched request, with the uuid kept + * when the signed-in user has no name for it. + * + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + */ + +import { flushPromises, mount } from '@vue/test-utils' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const http = vi.hoisted(() => ({ get: vi.fn() })) + +vi.mock('@nextcloud/axios', () => ({ default: http })) +vi.mock('@nextcloud/router', () => ({ generateUrl: (url) => url })) + +const { clearContactNames, contactName, contactPersonId, MAX_IDS_PER_REQUEST } = + await import('../../src/utils/contactNames.js') +const ContactNameCell = ( + await import('../../src/components/cells/ContactNameCell.vue') +).default +const cellWidgets = (await import('../../src/cellWidgets.js')).default + +const A = '00000000-0000-0000-0000-00000000000a' +const B = '00000000-0000-0000-0000-00000000000b' + +describe('contactNames', () => { + beforeEach(() => { + clearContactNames() + http.get.mockReset() + }) + + it('asks once for every id of the same tick and answers each', async () => { + http.get.mockResolvedValue({ data: { names: { [A]: 'Teamleider' } } }) + + const [a, b, again] = await Promise.all([ + contactName(A), + contactName(B), + contactName(A), + ]) + + expect(http.get).toHaveBeenCalledTimes(1) + expect(http.get).toHaveBeenCalledWith('/apps/stackiq/api/contact-names', { + params: { ids: [A, B] }, + }) + expect(a).toBe('Teamleider') + expect(again).toBe('Teamleider') + expect(b).toBeNull() + + await contactName(A) + expect(http.get).toHaveBeenCalledTimes(1) + }) + + it('splits a batch at the endpoint limit', async () => { + http.get.mockResolvedValue({ data: { names: {} } }) + const ids = Array.from( + { length: MAX_IDS_PER_REQUEST + 1 }, + (_, i) => `00000000-0000-0000-0000-${String(i).padStart(12, '0')}`, + ) + + await Promise.all(ids.map((id) => contactName(id))) + + expect(http.get).toHaveBeenCalledTimes(2) + expect(http.get.mock.calls[0][1].params.ids).toHaveLength( + MAX_IDS_PER_REQUEST, + ) + expect(http.get.mock.calls[1][1].params.ids).toHaveLength(1) + }) + + it('gives no name when the request fails', async () => { + http.get.mockRejectedValue(new Error('offline')) + expect(await contactName(A)).toBeNull() + }) + + it('reads the uuid from a string or a related object', () => { + expect(contactPersonId(` ${A} `)).toBe(A) + expect(contactPersonId({ id: A })).toBe(A) + expect(contactPersonId({ uuid: B })).toBe(B) + expect(contactPersonId(null)).toBe('') + }) +}) + +describe('ContactNameCell', () => { + beforeEach(() => { + clearContactNames() + http.get.mockReset() + }) + + it('is registered as the contactName cell widget', () => { + expect(cellWidgets.contactName).toBe(ContactNameCell) + }) + + it('shows the contact name instead of the uuid', async () => { + http.get.mockResolvedValue({ data: { names: { [A]: 'Teamleider' } } }) + const wrapper = mount(ContactNameCell, { props: { value: A } }) + await flushPromises() + expect(wrapper.text()).toBe('Teamleider') + }) + + it('keeps the uuid when the signed-in user has no name for it', async () => { + http.get.mockResolvedValue({ data: { names: {} } }) + const wrapper = mount(ContactNameCell, { props: { value: B } }) + await flushPromises() + expect(wrapper.text()).toBe(B) + }) +}) From 9aaacca22189c11e3f47b89f5131a1bb87d0d6e6 Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Mon, 12 Oct 2026 15:43:17 +0200 Subject: [PATCH 02/10] fix(usage): look owner names up only in the caller's own address books, and by the requested ids Review of #1292: - The name lookup used IManager::search with its defaults, which also searches the system address book (one card per local user, UID = user id). A contactsUid is free text on a contactPerson, so whoever may write one could have the endpoint name any local user. The new StackiqContactSyncService::findOwnContactByUid() passes enumeration and fullmatch false (the contacts manager then skips the system address book) and strict_search (exact UID, no substring scan). - OpenRegister's single register+schema search path ignores the ids argument and reads `_ids` from the query, so only the first 100 readable contact persons could ever be named. The ids now go in the query as `_ids`. - #[UserRateLimit(limit: 120, period: 60)] on the endpoint. - A failed request no longer pins "no name" for the page: its ids are dropped from the cache, so the next cell asks again. - @spec tags on the five frontend methods gate 16 named, and a scenario under REQ-UAP-003 for who sees a name. PHPUnit ContactNamesControllerTest 6 and StackiqContactSyncServiceTest 5 pass, vitest contactNames.spec.js 7 pass; removing the `_ids` key or the cache.delete turns a test red. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/Controller/ContactNamesController.php | 22 ++++++++----- lib/Service/StackiqContactSyncService.php | 32 +++++++++++++++++++ .../specs/application-usage-pages/spec.md | 9 ++++++ src/components/cells/ContactNameCell.vue | 2 ++ src/utils/contactNames.js | 12 +++++++ .../Controller/ContactNamesControllerTest.php | 13 +++++--- .../Service/StackiqContactSyncServiceTest.php | 31 ++++++++++++++++++ tests/vitest/contactNames.spec.js | 8 +++-- 8 files changed, 115 insertions(+), 14 deletions(-) diff --git a/lib/Controller/ContactNamesController.php b/lib/Controller/ContactNamesController.php index 086a6d400..51bb9524a 100644 --- a/lib/Controller/ContactNamesController.php +++ b/lib/Controller/ContactNamesController.php @@ -30,6 +30,7 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\NoAdminRequired; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\UserRateLimit; use OCP\AppFramework\Http\JSONResponse; use OCP\IL10N; use OCP\IRequest; @@ -44,10 +45,11 @@ * lives in Nextcloud Contacts (the CMDB import puts its owners in the * importing admin's "Stackiq CMDB owners" address book). This endpoint reads * the contact persons with the caller's own OpenRegister rights, then looks - * each `contactsUid` up in the address books Nextcloud gives the caller. A - * name is therefore returned only to someone who can already read both the - * contact person and the contact. Nothing is stored, and there is no public - * route. + * each `contactsUid` up, by exact UID, in the caller's own and shared address + * books; the system address book is never searched, so a contactsUid set to a + * user id names nobody. A name is therefore returned only to someone who can + * already read both the contact person and the contact. Nothing is stored, + * and there is no public route. * * @category Controller * @package OCA\Stackiq\Controller @@ -94,7 +96,8 @@ public function __construct( * Answers `{"names": {"": ""}}`. The per-object * check is OpenRegister's: the contact persons are read with the caller's * RBAC and organisation scope, so one the caller may not read is left out, - * as is one whose contact is not in the caller's address books. + * as is one whose contact is not in the caller's own or shared address + * books. At most 120 requests a minute per user (one per list page). * * @return JSONResponse 200 with the names, 400 for more than MAX_IDS ids, 401 when not signed in. * @@ -102,6 +105,7 @@ public function __construct( */ #[NoAdminRequired] #[NoCSRFRequired] + #[UserRateLimit(limit: 120, period: 60)] public function index(): JSONResponse { if ($this->userSession->getUser() === null) { return new JSONResponse( @@ -124,7 +128,7 @@ public function index(): JSONResponse { $names = []; foreach ($this->contactsUidsOf(ids: $ids) as $uuid => $contactsUid) { - $contact = $this->contactSync->findContactByUid(uid: $contactsUid); + $contact = $this->contactSync->findOwnContactByUid(uid: $contactsUid); $name = self::firstValue(value: ($contact['FN'] ?? '')); if ($name !== '') { $names[$uuid] = $name; @@ -180,12 +184,14 @@ private function contactsUidsOf(array $ids): array { try { // RBAC and multitenancy stay at their defaults: the caller's own rights. + // The ids go in the query as `_ids`: OpenRegister's single + // register+schema path reads that key and ignores the `ids` argument. $result = $this->objectService->searchObjectsPaginated( query: [ '@self' => ['register' => (int)$register, 'schema' => (int)$schema], + '_ids' => $ids, '_limit' => self::MAX_IDS, - ], - ids: $ids + ] ); } catch (Throwable $e) { $this->logger->warning('[ContactNames] The contact persons could not be read', ['exception' => $e]); diff --git a/lib/Service/StackiqContactSyncService.php b/lib/Service/StackiqContactSyncService.php index 2c09cd6fb..dc100c01f 100644 --- a/lib/Service/StackiqContactSyncService.php +++ b/lib/Service/StackiqContactSyncService.php @@ -470,6 +470,38 @@ public function findContactByUid(string $uid): ?array { return null; }//end findContactByUid() + /** + * Find a contact by its exact UID in the signed-in user's own and shared address books only. + * + * The system address book (one card per local user) is skipped, and the + * UID is matched exactly, not as a substring. This is the lookup for a + * name shown to a user: a contactsUid is free text on a contactPerson, + * so a lookup that included the system address book would name any + * local user for whoever may write a contactPerson. + * + * @param string $uid The contact UID. + * + * @return ?array The contact, or null when not found there. + * + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + */ + public function findOwnContactByUid(string $uid): ?array { + if ($uid === '' || $this->isAvailable() === false) { + return null; + } + + // With enumeration and fullmatch both off, the contacts manager skips the + // system address book; strict_search turns the substring match off. + $options = ['limit' => 5, 'strict_search' => true, 'enumeration' => false, 'fullmatch' => false]; + foreach ($this->contactsManager->search($uid, ['UID'], $options) as $result) { + if (($result['UID'] ?? '') === $uid) { + return $result; + } + } + + return null; + }//end findOwnContactByUid() + /** * Find a Nextcloud contact matching a relationship record's identity, by * e-mail first and — for organisations — by CBS/KvK code as a fallback. diff --git a/openspec/specs/application-usage-pages/spec.md b/openspec/specs/application-usage-pages/spec.md index 8804ce59d..17ab52e66 100644 --- a/openspec/specs/application-usage-pages/spec.md +++ b/openspec/specs/application-usage-pages/spec.md @@ -46,6 +46,15 @@ A usage SHALL carry a business owner and a technical owner, each picked from the - **WHEN** the supplier opens that usage - **THEN** the owner contact persons do not open for the supplier +#### Scenario: The usage list names an owner only for who can read the contact +@e2e exclude Needs a Contacts address book per user; tests/Unit/Controller/ContactNamesControllerTest.php asserts the read with the caller's rights and the 401, and tests/Unit/Service/StackiqContactSyncServiceTest.php testANameLookupSkipsTheSystemAddressBookAndMatchesExactly asserts the lookup skips the system address book. + +- **GIVEN** a usage whose business owner is a contact person linked to a contact in the information manager's own address book +- **WHEN** the information manager opens Applications in use +- **THEN** the Business owner column shows the contact's name +- **AND** a user without that contact in their own or shared address books sees the contact person's id, as before +- **AND** a request that is not signed in gets 401 and no name, and a contactsUid that is a user id names nobody + ### Requirement: REQ-UAP-004 A usage moves through its lifecycle from its page The usage schema SHALL declare its lifecycle on the status values its rows hold, so the detail page offers Plan, Go live, Phase out and Retire from the matching status. diff --git a/src/components/cells/ContactNameCell.vue b/src/components/cells/ContactNameCell.vue index 06a7ec2f4..e8ef534dc 100644 --- a/src/components/cells/ContactNameCell.vue +++ b/src/components/cells/ContactNameCell.vue @@ -41,6 +41,7 @@ export default { computed: { /** * @return {string} The contact person uuid, or ''. + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner */ id() { return contactPersonId(this.value) @@ -55,6 +56,7 @@ export default { * * @param {string} id The contact person uuid. * @return {Promise} + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner */ async handler(id) { this.name = null diff --git a/src/utils/contactNames.js b/src/utils/contactNames.js index 6051639f4..ed9513e46 100644 --- a/src/utils/contactNames.js +++ b/src/utils/contactNames.js @@ -30,6 +30,7 @@ let pending = new Map() * * @param {string|object|null} value A uuid, or a related object with `id` or `uuid`. * @return {string} The uuid, or ''. + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner */ export function contactPersonId(value) { if (typeof value === 'string') { @@ -44,7 +45,11 @@ export function contactPersonId(value) { /** * Ask the endpoint for the names of the collected ids, in chunks it accepts. * + * A chunk whose request fails answers "no name" for now and is forgotten, + * so the next cell that asks for one of its ids asks the endpoint again. + * * @return {Promise} + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner */ async function flush() { const batch = pending @@ -53,6 +58,7 @@ async function flush() { for (let start = 0; start < ids.length; start += MAX_IDS_PER_REQUEST) { const chunk = ids.slice(start, start + MAX_IDS_PER_REQUEST) let names = {} + let failed = false try { const { data } = await axios.get( generateUrl('/apps/stackiq/api/contact-names'), @@ -61,9 +67,13 @@ async function flush() { names = data?.names ?? {} } catch { names = {} + failed = true } chunk.forEach((id) => { const name = typeof names[id] === 'string' ? names[id] : '' + if (failed) { + cache.delete(id) + } batch.get(id).resolve(name !== '' ? name : null) }) } @@ -75,6 +85,7 @@ async function flush() { * * @param {string} id The contact person uuid. * @return {Promise} The name. + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner */ export function contactName(id) { const key = String(id ?? '').toLowerCase() @@ -99,6 +110,7 @@ export function contactName(id) { * Forget every name read so far (tests). * * @return {void} + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner */ export function clearContactNames() { cache.clear() diff --git a/tests/Unit/Controller/ContactNamesControllerTest.php b/tests/Unit/Controller/ContactNamesControllerTest.php index 0860873de..7ff34f662 100644 --- a/tests/Unit/Controller/ContactNamesControllerTest.php +++ b/tests/Unit/Controller/ContactNamesControllerTest.php @@ -109,12 +109,17 @@ public function testNamesTheReadableContactPersons(): void { $this->objectService->expects($this->once()) ->method('searchObjectsPaginated') ->with( - $this->callback(static fn (array $query): bool => $query['@self'] === ['register' => 20, 'schema' => 7]), + // The ids go in the query as `_ids`: OpenRegister's single + // register+schema path ignores the `ids` argument. + $this->callback( + static fn (array $query): bool => $query['@self'] === ['register' => 20, 'schema' => 7] + && $query['_ids'] === [self::A, self::B] + ), // The caller's own RBAC and organisation scope. true, true, false, - [self::A, self::B] + null ) ->willReturn( [ @@ -124,7 +129,7 @@ public function testNamesTheReadableContactPersons(): void { ], ] ); - $this->contactSync->method('findContactByUid')->willReturnMap( + $this->contactSync->method('findOwnContactByUid')->willReturnMap( [ ['uid-a', ['UID' => 'uid-a', 'FN' => 'Teamleider Applicatiebeheer']], ['uid-b', null], @@ -146,7 +151,7 @@ public function testNamesOnlyTheRequestedIds(): void { $this->objectService->method('searchObjectsPaginated')->willReturn( ['results' => [['id' => self::C, 'contactsUid' => 'uid-c']]] ); - $this->contactSync->expects($this->never())->method('findContactByUid'); + $this->contactSync->expects($this->never())->method('findOwnContactByUid'); $response = $this->controller(ids: [self::A])->index(); diff --git a/tests/Unit/Service/StackiqContactSyncServiceTest.php b/tests/Unit/Service/StackiqContactSyncServiceTest.php index 7b0228a0a..62c26fc2a 100644 --- a/tests/Unit/Service/StackiqContactSyncServiceTest.php +++ b/tests/Unit/Service/StackiqContactSyncServiceTest.php @@ -113,6 +113,37 @@ static function (array $contact) use ($pattern, $properties): bool { return new StackiqContactSyncService(contactsManager: $manager, logger: new NullLogger(), container: $container, userSession: $session); }//end service() + /** + * A name lookup skips the system address book and matches the UID exactly. + * + * With enumeration and fullmatch both off, Nextcloud's contacts manager + * skips the system address book (one card per local user, UID = user id), + * so a contactsUid set to a user id names nobody. + * + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + * + * @return void + */ + public function testANameLookupSkipsTheSystemAddressBookAndMatchesExactly(): void { + $manager = $this->createMock(IManager::class); + $manager->method('isEnabled')->willReturn(true); + $manager->expects($this->exactly(2)) + ->method('search') + ->with( + $this->anything(), + ['UID'], + ['limit' => 5, 'strict_search' => true, 'enumeration' => false, 'fullmatch' => false] + ) + ->willReturnCallback( + static fn (string $uid): array => ($uid === 'uid-a' ? [['UID' => 'uid-a-longer', 'FN' => 'Other'], ['UID' => 'uid-a', 'FN' => 'Owner']] : []) + ); + $service = new StackiqContactSyncService(contactsManager: $manager, logger: new NullLogger()); + + $this->assertSame('Owner', $service->findOwnContactByUid(uid: 'uid-a')['FN']); + $this->assertNull($service->findOwnContactByUid(uid: 'admin')); + $this->assertNull($service->findOwnContactByUid(uid: '')); + }//end testANameLookupSkipsTheSystemAddressBookAndMatchesExactly() + /** * The first contact creates the address book; the second reuses it; neither touches another address book. * diff --git a/tests/vitest/contactNames.spec.js b/tests/vitest/contactNames.spec.js index f301c045f..e303e85db 100644 --- a/tests/vitest/contactNames.spec.js +++ b/tests/vitest/contactNames.spec.js @@ -72,9 +72,13 @@ describe('contactNames', () => { expect(http.get.mock.calls[1][1].params.ids).toHaveLength(1) }) - it('gives no name when the request fails', async () => { - http.get.mockRejectedValue(new Error('offline')) + it('gives no name when the request fails, and asks again next time', async () => { + http.get.mockRejectedValueOnce(new Error('offline')) expect(await contactName(A)).toBeNull() + + http.get.mockResolvedValueOnce({ data: { names: { [A]: 'Teamleider' } } }) + expect(await contactName(A)).toBe('Teamleider') + expect(http.get).toHaveBeenCalledTimes(2) }) it('reads the uuid from a string or a related object', () => { From 36a095968b94ab0eeb404db746875143db230a4e Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Mon, 12 Oct 2026 15:49:52 +0200 Subject: [PATCH 03/10] fix(usage): count a name only from the caller's own and shared address books Pre-push check of the previous commit: with enumeration and fullmatch off the contacts manager skips the local system address book, but it still searches the other books of the system principal, which include the books synced from trusted servers (one card per remote user, UID = user id). A contactsUid set to a remote user id could still name that user. findOwnContactByUid() now keeps a result only when its addressbook-key is one of the books the CardDAV backend lists for the user's principal (own and shared), and finds nothing without a backend or a signed-in user. The controller docblock and the REQ-UAP-003 scenario say "local or remote user id". StackiqContactSyncServiceTest 5 and ContactNamesControllerTest 6 pass; dropping the address-book check turns the service test red. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/Controller/ContactNamesController.php | 5 +- lib/Service/StackiqContactSyncService.php | 57 ++++++++++++++++--- .../specs/application-usage-pages/spec.md | 2 +- .../Service/StackiqContactSyncServiceTest.php | 35 ++++++++++-- 4 files changed, 84 insertions(+), 15 deletions(-) diff --git a/lib/Controller/ContactNamesController.php b/lib/Controller/ContactNamesController.php index 51bb9524a..ed91f9d13 100644 --- a/lib/Controller/ContactNamesController.php +++ b/lib/Controller/ContactNamesController.php @@ -46,8 +46,9 @@ * importing admin's "Stackiq CMDB owners" address book). This endpoint reads * the contact persons with the caller's own OpenRegister rights, then looks * each `contactsUid` up, by exact UID, in the caller's own and shared address - * books; the system address book is never searched, so a contactsUid set to a - * user id names nobody. A name is therefore returned only to someone who can + * books only; the system principal's books (the local system address book + * and the books synced from trusted servers) never count, so a contactsUid + * set to a local or remote user id names nobody. A name is therefore returned only to someone who can * already read both the contact person and the contact. Nothing is stored, * and there is no public route. * diff --git a/lib/Service/StackiqContactSyncService.php b/lib/Service/StackiqContactSyncService.php index dc100c01f..7ebebdbca 100644 --- a/lib/Service/StackiqContactSyncService.php +++ b/lib/Service/StackiqContactSyncService.php @@ -473,11 +473,15 @@ public function findContactByUid(string $uid): ?array { /** * Find a contact by its exact UID in the signed-in user's own and shared address books only. * - * The system address book (one card per local user) is skipped, and the - * UID is matched exactly, not as a substring. This is the lookup for a - * name shown to a user: a contactsUid is free text on a contactPerson, - * so a lookup that included the system address book would name any - * local user for whoever may write a contactPerson. + * A result counts only when it comes from an address book the CardDAV + * backend lists for the user's principal (own and shared); every + * address book of the system principal is left out, the local system + * address book (one card per local user) and the books synced from + * trusted servers alike. The UID is matched exactly, not as a substring. + * This is the lookup for a name shown to a user: a contactsUid is free + * text on a contactPerson, so a lookup that read the system books would + * name any local or federated user for whoever may write a contactPerson. + * Without a CardDAV backend or a signed-in user nothing is found. * * @param string $uid The contact UID. * @@ -490,11 +494,17 @@ public function findOwnContactByUid(string $uid): ?array { return null; } + $keys = $this->userAddressBookKeys(); + if ($keys === []) { + return null; + } + // With enumeration and fullmatch both off, the contacts manager skips the - // system address book; strict_search turns the substring match off. + // local system address book; strict_search turns the substring match off. $options = ['limit' => 5, 'strict_search' => true, 'enumeration' => false, 'fullmatch' => false]; foreach ($this->contactsManager->search($uid, ['UID'], $options) as $result) { - if (($result['UID'] ?? '') === $uid) { + $key = (string)($result['addressbook-key'] ?? ''); + if (($result['UID'] ?? '') === $uid && in_array($key, $keys, true) === true) { return $result; } } @@ -502,6 +512,39 @@ public function findOwnContactByUid(string $uid): ?array { return null; }//end findOwnContactByUid() + /** + * The keys of the signed-in user's own and shared address books. + * + * @return array CardDAV address book ids as strings; empty without a backend or a user. + */ + private function userAddressBookKeys(): array { + try { + $backend = $this->container?->get(static::CARDDAV_BACKEND_CLASS); + $uid = $this->userSession?->getUser()?->getUID(); + if (is_object($backend) === false || $uid === null) { + return []; + } + + $books = $backend->getAddressBooksForUser('principals/users/' . $uid); + } catch (Throwable $e) { + $this->logger->warning('[StackiqContactSync] The address books of the user could not be read', ['exception' => get_class($e)]); + return []; + } + + if (is_array($books) === false) { + return []; + } + + $keys = []; + foreach ($books as $book) { + if (is_array($book) === true && isset($book['id']) === true) { + $keys[] = (string)$book['id']; + } + } + + return $keys; + }//end userAddressBookKeys() + /** * Find a Nextcloud contact matching a relationship record's identity, by * e-mail first and — for organisations — by CBS/KvK code as a fallback. diff --git a/openspec/specs/application-usage-pages/spec.md b/openspec/specs/application-usage-pages/spec.md index 17ab52e66..e022b84f7 100644 --- a/openspec/specs/application-usage-pages/spec.md +++ b/openspec/specs/application-usage-pages/spec.md @@ -53,7 +53,7 @@ A usage SHALL carry a business owner and a technical owner, each picked from the - **WHEN** the information manager opens Applications in use - **THEN** the Business owner column shows the contact's name - **AND** a user without that contact in their own or shared address books sees the contact person's id, as before -- **AND** a request that is not signed in gets 401 and no name, and a contactsUid that is a user id names nobody +- **AND** a request that is not signed in gets 401 and no name, and a contactsUid that is a local or remote user id names nobody ### Requirement: REQ-UAP-004 A usage moves through its lifecycle from its page diff --git a/tests/Unit/Service/StackiqContactSyncServiceTest.php b/tests/Unit/Service/StackiqContactSyncServiceTest.php index 62c26fc2a..670fe6cfa 100644 --- a/tests/Unit/Service/StackiqContactSyncServiceTest.php +++ b/tests/Unit/Service/StackiqContactSyncServiceTest.php @@ -117,8 +117,10 @@ static function (array $contact) use ($pattern, $properties): bool { * A name lookup skips the system address book and matches the UID exactly. * * With enumeration and fullmatch both off, Nextcloud's contacts manager - * skips the system address book (one card per local user, UID = user id), - * so a contactsUid set to a user id names nobody. + * skips the local system address book (one card per local user, UID = + * user id); a result from any book outside the user's own and shared + * ones (such as a book synced from a trusted server) is dropped, so a + * contactsUid set to a local or remote user id names nobody. * * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner * @@ -127,7 +129,7 @@ static function (array $contact) use ($pattern, $properties): bool { public function testANameLookupSkipsTheSystemAddressBookAndMatchesExactly(): void { $manager = $this->createMock(IManager::class); $manager->method('isEnabled')->willReturn(true); - $manager->expects($this->exactly(2)) + $manager->expects($this->exactly(3)) ->method('search') ->with( $this->anything(), @@ -135,13 +137,36 @@ public function testANameLookupSkipsTheSystemAddressBookAndMatchesExactly(): voi ['limit' => 5, 'strict_search' => true, 'enumeration' => false, 'fullmatch' => false] ) ->willReturnCallback( - static fn (string $uid): array => ($uid === 'uid-a' ? [['UID' => 'uid-a-longer', 'FN' => 'Other'], ['UID' => 'uid-a', 'FN' => 'Owner']] : []) + static fn (string $uid): array => match ($uid) { + // Book 7 is the user's own; 99 is a book synced from a trusted server. + 'uid-a' => [ + ['UID' => 'uid-a-longer', 'FN' => 'Other', 'addressbook-key' => '7'], + ['UID' => 'uid-a', 'FN' => 'Owner', 'addressbook-key' => '7'], + ], + 'remote-user' => [['UID' => 'remote-user', 'FN' => 'Remote User', 'addressbook-key' => '99']], + default => [], + } ); - $service = new StackiqContactSyncService(contactsManager: $manager, logger: new NullLogger()); + $backend = new class { + public function getAddressBooksForUser(string $principal): array { + return $principal === 'principals/users/admin' ? [['id' => 7, 'uri' => 'contacts'], ['id' => 8, 'uri' => 'shared_by_bob']] : []; + } + }; + $container = $this->createMock(ContainerInterface::class); + $container->method('get')->willReturn($backend); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('admin'); + $session = $this->createMock(IUserSession::class); + $session->method('getUser')->willReturn($user); + $service = new StackiqContactSyncService(contactsManager: $manager, logger: new NullLogger(), container: $container, userSession: $session); $this->assertSame('Owner', $service->findOwnContactByUid(uid: 'uid-a')['FN']); + $this->assertNull($service->findOwnContactByUid(uid: 'remote-user')); $this->assertNull($service->findOwnContactByUid(uid: 'admin')); $this->assertNull($service->findOwnContactByUid(uid: '')); + + $noUser = new StackiqContactSyncService(contactsManager: $manager, logger: new NullLogger(), container: $container); + $this->assertNull($noUser->findOwnContactByUid(uid: 'uid-a')); }//end testANameLookupSkipsTheSystemAddressBookAndMatchesExactly() /** From 756d4d97f9e2c9ab68fced038f9aa2c56664af56 Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Tue, 13 Oct 2026 18:31:13 +0200 Subject: [PATCH 04/10] fix(usage): load the settings before the risk signals read their objects Opened straight from a link, the usage page can mount the risk signals before the app has read its settings. No object type resolved then, the first read threw and the widget showed its error. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/components/portfolio/UsageRiskSignals.vue | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/components/portfolio/UsageRiskSignals.vue b/src/components/portfolio/UsageRiskSignals.vue index 424cf2586..de23c8c9e 100644 --- a/src/components/portfolio/UsageRiskSignals.vue +++ b/src/components/portfolio/UsageRiskSignals.vue @@ -141,6 +141,14 @@ export default { this.loading = true this.error = false try { + // Opened straight from a link, this widget can mount before the + // app has read its settings; without them no type resolves. + if ( + !objectStore.settings + && typeof objectStore.fetchSettings === 'function' + ) { + await objectStore.fetchSettings() + } this.ensureType('usage') this.ensureType('moduleVersion') this.ensureType('vulnerability') From 2ed63b6c84356dec6fde941f85970ba937f6da3c Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Tue, 13 Oct 2026 18:31:13 +0200 Subject: [PATCH 05/10] feat(contacts): show a contact person's name on its page and in the organisation's list A contact person holds only a contactsUid; its name lives in Nextcloud Contacts. The contact person page now opens with the name, and the organisation page's contact person list has a Name column, both read through /api/contact-names, so only someone who can read the contact sees it. Co-Authored-By: Claude Opus 5.5 (1M context) --- l10n/en.js | 4 +- l10n/en.json | 4 +- l10n/nl.js | 4 +- l10n/nl.json | 4 +- .../organisations/ContactPersonName.vue | 115 ++++++++++++++++++ src/customComponents.js | 6 + src/manifest.json | 5 +- 7 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 src/components/organisations/ContactPersonName.vue diff --git a/l10n/en.js b/l10n/en.js index a02d4aabf..238aa4aa3 100644 --- a/l10n/en.js +++ b/l10n/en.js @@ -1134,7 +1134,9 @@ OC.L10N.register( "In development since": "In development since", "In use since": "In use since", "Not signed in": "Not signed in", - "At most %s contact persons per request": "At most %s contact persons per request" + "At most %s contact persons per request": "At most %s contact persons per request", + "Loading the name": "Loading the name", + "The name is kept in Nextcloud Contacts, in an address book you cannot read.": "The name is kept in Nextcloud Contacts, in an address book you cannot read." }, "nplurals=2; plural=(n != 1);" ) diff --git a/l10n/en.json b/l10n/en.json index 02c4b18b2..7337c9fbb 100644 --- a/l10n/en.json +++ b/l10n/en.json @@ -1133,6 +1133,8 @@ "In development since": "In development since", "In use since": "In use since", "Not signed in": "Not signed in", - "At most %s contact persons per request": "At most %s contact persons per request" + "At most %s contact persons per request": "At most %s contact persons per request", + "Loading the name": "Loading the name", + "The name is kept in Nextcloud Contacts, in an address book you cannot read.": "The name is kept in Nextcloud Contacts, in an address book you cannot read." } } diff --git a/l10n/nl.js b/l10n/nl.js index 927c85f6a..d49692144 100644 --- a/l10n/nl.js +++ b/l10n/nl.js @@ -1202,7 +1202,9 @@ OC.L10N.register( "In development since": "In ontwikkeling sinds", "In use since": "In gebruik sinds", "Not signed in": "Niet aangemeld", - "At most %s contact persons per request": "Ten hoogste %s contactpersonen per verzoek" + "At most %s contact persons per request": "Ten hoogste %s contactpersonen per verzoek", + "Loading the name": "De naam wordt geladen", + "The name is kept in Nextcloud Contacts, in an address book you cannot read.": "De naam staat in Nextcloud Contacten, in een adresboek dat je niet kunt lezen." }, "nplurals=2; plural=(n != 1);" ) diff --git a/l10n/nl.json b/l10n/nl.json index d97f06ce7..edd44c807 100644 --- a/l10n/nl.json +++ b/l10n/nl.json @@ -1201,6 +1201,8 @@ "In development since": "In ontwikkeling sinds", "In use since": "In gebruik sinds", "Not signed in": "Niet aangemeld", - "At most %s contact persons per request": "Ten hoogste %s contactpersonen per verzoek" + "At most %s contact persons per request": "Ten hoogste %s contactpersonen per verzoek", + "Loading the name": "De naam wordt geladen", + "The name is kept in Nextcloud Contacts, in an address book you cannot read.": "De naam staat in Nextcloud Contacten, in een adresboek dat je niet kunt lezen." } } diff --git a/src/components/organisations/ContactPersonName.vue b/src/components/organisations/ContactPersonName.vue new file mode 100644 index 000000000..5e2ddc3ea --- /dev/null +++ b/src/components/organisations/ContactPersonName.vue @@ -0,0 +1,115 @@ + + + + + + diff --git a/src/customComponents.js b/src/customComponents.js index f6ec30d97..da401ac5e 100644 --- a/src/customComponents.js +++ b/src/customComponents.js @@ -23,6 +23,7 @@ import OrganisatieCard from './components/cards/OrganisatieCard.vue' import ApplicationContractsPanel from './components/contracts/ApplicationContractsPanel.vue' import ContractApprovalPanel from './components/contracts/ContractApprovalPanel.vue' import ContractSeatsPanel from './components/contracts/ContractSeatsPanel.vue' +import ContactPersonName from './components/organisations/ContactPersonName.vue' import OrganisationMergePanel from './components/organisations/OrganisationMergePanel.vue' import UsageRiskSignals from './components/portfolio/UsageRiskSignals.vue' import ReviewsPanel from './components/reviews/ReviewsPanel.vue' @@ -107,6 +108,11 @@ export default { // and the vulnerabilities, which no built-in widget does. UsageRiskSignals, + // The name of a contact person on its detail page (landscape-usage-registration): + // the name lives in Nextcloud Contacts, read through /api/contact-names, + // which no built-in widget does. + ContactPersonName, + // AI Act evidence per tag on the AI system page (landscape-ai-system-inventory): // it reads the object's files and their tags, which no built-in widget lists per tag. AiActChecklist, diff --git a/src/manifest.json b/src/manifest.json index 22b321684..ec9a337cf 100644 --- a/src/manifest.json +++ b/src/manifest.json @@ -423,7 +423,7 @@ { "id": "org-diensten", "type": "object-list", "title": "Services", "icon": "HandshakeOutline", "content": { "register": "@resolve:voorzieningen_register", "schema": "catalogService", "filter": { "provider": "@objectId" }, "columns": [ { "key": "type", "label": "Type" } ], "limit": 25, "emptyText": "No services registered for this organisation yet" } }, { "id": "org-modules", "type": "object-list", "title": "Applications", "icon": "Package", "content": { "register": "@resolve:voorzieningen_register", "schema": "module", "filter": { "provider": "@objectId" }, "columns": [ { "key": "name", "label": "Name" }, { "key": "licentietype", "label": "License type" }, { "key": "bbnLevel", "label": "BBN level" } ], "limit": 25, "rowRoute": "ModuleDetail", "viewAllRoute": "Modules", "viewAllQuery": { "provider": "@objectId" }, "emptyText": "No applications registered for this organisation yet" } }, { "id": "org-usages", "type": "object-list", "title": "Applications in use", "icon": "OfficeBuilding", "content": { "register": "@resolve:voorzieningen_register", "schema": "usage", "filter": { "consumer": "@objectId" }, "columns": [ { "key": "module", "label": "Application", "widget": "fkResolve", "widgetProps": { "register": "@resolve:voorzieningen_register", "schema": "module", "labelField": "name" } }, { "key": "moduleVersion", "label": "Version", "widget": "fkResolve", "widgetProps": { "register": "@resolve:voorzieningen_register", "schema": "moduleVersion", "labelField": "version" } }, { "key": "status", "label": "Status" } ], "limit": 25, "rowRoute": "GebruikDetail", "viewAllRoute": "Gebruik", "viewAllQuery": { "consumer": "@objectId" }, "allowCreate": false, "emptyText": "No applications in use recorded for this organisation yet" } }, - { "id": "org-contactpersonen", "type": "object-list", "title": "Contact persons", "icon": "AccountGroup", "content": { "register": "@resolve:voorzieningen_register", "schema": "contactPerson", "filter": { "organization": "@objectId" }, "columns": [ { "key": "role", "label": "Function" }, { "key": "roles", "label": "Roles" } ], "limit": 25, "rowRoute": "ContactpersoonDetail", "emptyText": "No contact persons linked to this organisation yet" } } + { "id": "org-contactpersonen", "type": "object-list", "title": "Contact persons", "icon": "AccountGroup", "content": { "register": "@resolve:voorzieningen_register", "schema": "contactPerson", "filter": { "organization": "@objectId" }, "columns": [ { "key": "id", "label": "Name", "widget": "contactName" }, { "key": "role", "label": "Function" }, { "key": "roles", "label": "Roles" } ], "limit": 25, "rowRoute": "ContactpersoonDetail", "emptyText": "No contact persons linked to this organisation yet" } } ], "layout": [ { "id": "1", "widgetId": "org-data", "gridX": 0, "gridY": 0, "gridWidth": 8, "gridHeight": 5 }, @@ -486,6 +486,9 @@ { "id": "1", "widgetId": "cp-data", "gridX": 0, "gridY": 0, "gridWidth": 8, "gridHeight": 4 }, { "id": "2", "widgetId": "cp-related", "gridX": 8, "gridY": 0, "gridWidth": 4, "gridHeight": 4 } ], + "bodyWidgets": [ + { "id": "cp-name", "component": "ContactPersonName", "props": { "objectId": "@objectId" }, "placement": "before-body", "colSpan": 12 } + ], "sidebar": { "enabled": true, "showMetadata": true, From cb48f556b109ca3ede6f80fab4d2a7da6ef285b7 Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Tue, 13 Oct 2026 18:38:51 +0200 Subject: [PATCH 06/10] fix(usage): keep the risk signal labels clear of their values A page-wide style gives every dt a fixed width of 130px, aligned to the end, so "Known vulnerabilities" and "End of support" ran into their values. The labels now take their own width and start-align. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/components/portfolio/UsageRiskSignals.vue | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/components/portfolio/UsageRiskSignals.vue b/src/components/portfolio/UsageRiskSignals.vue index de23c8c9e..bad7e81ed 100644 --- a/src/components/portfolio/UsageRiskSignals.vue +++ b/src/components/portfolio/UsageRiskSignals.vue @@ -224,6 +224,10 @@ export default { } .usage-risk-signals__list dt { + /* Undo the page-wide fixed width and right alignment of a dt, which made + a long label run into its value. */ + width: auto; + text-align: start; font-weight: bold; } From 4fd91944972ec373b69611dd0a82b4431d8cfbeb Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Tue, 13 Oct 2026 20:42:08 +0200 Subject: [PATCH 07/10] docs(spec): cover the contact person page and the list Name column in REQ-UAP-005 Closes review finding f2: the two new name surfaces had no scenario and their @spec tags pointed at REQ-UAP-003 (usage owners). REQ-UAP-005 states who sees the name and the empty states; ContactNameCell is retagged. Class sweep: every @spec tag of ContactPersonName, contactNames.js (new export) and ContactNameCell now targets REQ-UAP-005 or REQ-UAP-003 by surface. Co-Authored-By: Claude Opus 5.5 --- .../specs/application-usage-pages/spec.md | 22 +++++++++++++++++++ src/components/cells/ContactNameCell.vue | 6 ++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/openspec/specs/application-usage-pages/spec.md b/openspec/specs/application-usage-pages/spec.md index e022b84f7..36dee9624 100644 --- a/openspec/specs/application-usage-pages/spec.md +++ b/openspec/specs/application-usage-pages/spec.md @@ -65,3 +65,25 @@ The usage schema SHALL declare its lifecycle on the status values its rows hold, - **GIVEN** a usage with status Planned - **WHEN** the information manager opens it and clicks Go live - **THEN** its status reads In production + +### Requirement: REQ-UAP-005 A contact person's name shows only to who can read the contact + +The contact person page and the Name column of an organisation's contact person list SHALL show the name of the linked Nextcloud contact only to a user who can read that contact in their own or shared address books, and SHALL say why there is no name when there is none. + +#### Scenario: A reader of the contact sees the name +@e2e exclude Needs a Contacts address book per user; tests/vitest/contactNames.spec.js asserts the name on the contact person page and in the list column. + +- **GIVEN** a contact person linked to a contact in the information manager's own address book +- **WHEN** the information manager opens the contact person, or the organisation it belongs to +- **THEN** the contact person page opens with the contact's name +- **AND** the Name column of the organisation's contact person list shows it + +#### Scenario: Without a name the page says why +@e2e exclude Needs a Contacts address book per user; tests/vitest/contactNames.spec.js asserts each empty state. + +- **GIVEN** a contact person whose name the signed-in user does not get +- **WHEN** the user opens the contact person page +- **THEN** a contact person without a contactsUid reads that it is not linked to a Nextcloud contact +- **AND** a contact in an address book the user cannot read reads that the name is kept in Nextcloud Contacts, in an address book the user cannot read +- **AND** a request that failed (for example rate limited) reads that the name could not be loaded, to reload the page +- **AND** the list column keeps the contact person's id in each of these cases diff --git a/src/components/cells/ContactNameCell.vue b/src/components/cells/ContactNameCell.vue index e8ef534dc..5bd2f487a 100644 --- a/src/components/cells/ContactNameCell.vue +++ b/src/components/cells/ContactNameCell.vue @@ -9,7 +9,7 @@ Nextcloud contact instead of its uuid. The name comes from the signed-in user's own address books (see `utils/contactNames.js`); when there is none, the uuid stays, as before. -@spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner +@spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-005-a-contact-persons-name-shows-only-to-who-can-read-the-contact --> @@ -37,7 +35,7 @@ import { CnIcon, CnWidgetWrapper } from '@conduction/nextcloud-vue' import { translate as t } from '@nextcloud/l10n' import { NcLoadingIcon } from '@nextcloud/vue' -import { contactName } from '../../utils/contactNames.js' +import { contactNameResult } from '../../utils/contactNames.js' /** * The name of a contact person, on its detail page. @@ -47,7 +45,7 @@ import { contactName } from '../../utils/contactNames.js' * signed-in user's own and shared address books, so the name is shown to whoever * can already read the contact, and to nobody else. * - * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-005-a-contact-persons-name-shows-only-to-who-can-read-the-contact */ export default { name: 'ContactPersonName', @@ -64,15 +62,49 @@ export default { type: String, required: true, }, + + /** The Nextcloud contact the person is linked to; empty when not linked. */ + contactsUid: { + type: String, + default: '', + }, }, data() { return { loading: true, name: null, + failed: false, } }, + computed: { + /** + * Why there is no name to show. + * + * @return {string} The hint. + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-005-a-contact-persons-name-shows-only-to-who-can-read-the-contact + */ + hint() { + if (this.failed) { + return t( + 'stackiq', + 'The name could not be loaded. Reload the page to try again.', + ) + } + if (!this.contactsUid) { + return t( + 'stackiq', + 'This contact person is not linked to a Nextcloud contact.', + ) + } + return t( + 'stackiq', + 'The name is kept in Nextcloud Contacts, in an address book you cannot read.', + ) + }, + }, + watch: { objectId: { immediate: true, @@ -81,14 +113,16 @@ export default { * * @param {string} id The contact person uuid. * @return {Promise} - * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-005-a-contact-persons-name-shows-only-to-who-can-read-the-contact */ async handler(id) { this.loading = true this.name = null - const name = await contactName(id) + this.failed = false + const result = await contactNameResult(id) if (id === this.objectId) { - this.name = name + this.name = result.name + this.failed = result.failed this.loading = false } }, diff --git a/src/manifest.json b/src/manifest.json index ec9a337cf..a51036bfb 100644 --- a/src/manifest.json +++ b/src/manifest.json @@ -487,7 +487,7 @@ { "id": "2", "widgetId": "cp-related", "gridX": 8, "gridY": 0, "gridWidth": 4, "gridHeight": 4 } ], "bodyWidgets": [ - { "id": "cp-name", "component": "ContactPersonName", "props": { "objectId": "@objectId" }, "placement": "before-body", "colSpan": 12 } + { "id": "cp-name", "component": "ContactPersonName", "props": { "objectId": "@objectId", "contactsUid": "@object.contactsUid" }, "placement": "before-body", "colSpan": 12 } ], "sidebar": { "enabled": true, diff --git a/src/utils/contactNames.js b/src/utils/contactNames.js index ed9513e46..125e2880a 100644 --- a/src/utils/contactNames.js +++ b/src/utils/contactNames.js @@ -19,7 +19,7 @@ import { generateUrl } from '@nextcloud/router' /** The endpoint's own limit (`ContactNamesController::MAX_IDS`). */ export const MAX_IDS_PER_REQUEST = 100 -/** uuid -> Promise; null when no name could be read. */ +/** uuid -> Promise<{name: string|null, failed: boolean}>. */ const cache = new Map() /** uuid -> { resolve } for the ids of the batch being collected. */ @@ -74,23 +74,24 @@ async function flush() { if (failed) { cache.delete(id) } - batch.get(id).resolve(name !== '' ? name : null) + batch.get(id).resolve({ name: name !== '' ? name : null, failed }) }) } } /** - * The display name of a contact person, or null when the signed-in user - * cannot read one. + * What the endpoint made of a contact person: its name, and whether the + * request failed (rate limit, network, server error) as opposed to answering + * without a name. * * @param {string} id The contact person uuid. - * @return {Promise} The name. - * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + * @return {Promise<{name: string|null, failed: boolean}>} The name, or why there is none. + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-005-a-contact-persons-name-shows-only-to-who-can-read-the-contact */ -export function contactName(id) { +export function contactNameResult(id) { const key = String(id ?? '').toLowerCase() if (key === '') { - return Promise.resolve(null) + return Promise.resolve({ name: null, failed: false }) } if (!cache.has(key)) { cache.set( @@ -106,6 +107,18 @@ export function contactName(id) { return cache.get(key) } +/** + * The display name of a contact person, or null when the signed-in user + * cannot read one (or the request failed; see contactNameResult). + * + * @param {string} id The contact person uuid. + * @return {Promise} The name. + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-003-a-usage-names-a-business-owner-and-a-technical-owner + */ +export async function contactName(id) { + return (await contactNameResult(id)).name +} + /** * Forget every name read so far (tests). * From 7ca89b332664bbbcb36306a1ce712188e21e36b2 Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Tue, 13 Oct 2026 20:42:08 +0200 Subject: [PATCH 09/10] test(contacts): cover the contact person empty states and the risk signals settings wait Closes review finding f3: ContactPersonName (name, not linked, no readable address book, failed request), contactNameResult, and UsageRiskSignals reading the settings before its first object read. Co-Authored-By: Claude Opus 5.5 --- tests/vitest/contactNames.spec.js | 17 +++++- tests/vitest/contactPersonName.spec.js | 79 +++++++++++++++++++++++++ tests/vitest/usageRiskSignals.spec.js | 80 ++++++++++++++++++++++++++ 3 files changed, 174 insertions(+), 2 deletions(-) create mode 100644 tests/vitest/contactPersonName.spec.js create mode 100644 tests/vitest/usageRiskSignals.spec.js diff --git a/tests/vitest/contactNames.spec.js b/tests/vitest/contactNames.spec.js index e303e85db..7a1012844 100644 --- a/tests/vitest/contactNames.spec.js +++ b/tests/vitest/contactNames.spec.js @@ -19,8 +19,13 @@ const http = vi.hoisted(() => ({ get: vi.fn() })) vi.mock('@nextcloud/axios', () => ({ default: http })) vi.mock('@nextcloud/router', () => ({ generateUrl: (url) => url })) -const { clearContactNames, contactName, contactPersonId, MAX_IDS_PER_REQUEST } = - await import('../../src/utils/contactNames.js') +const { + clearContactNames, + contactName, + contactNameResult, + contactPersonId, + MAX_IDS_PER_REQUEST, +} = await import('../../src/utils/contactNames.js') const ContactNameCell = ( await import('../../src/components/cells/ContactNameCell.vue') ).default @@ -81,6 +86,14 @@ describe('contactNames', () => { expect(http.get).toHaveBeenCalledTimes(2) }) + it('tells a failed request from an answer without a name', async () => { + http.get.mockRejectedValueOnce(new Error('429')) + expect(await contactNameResult(A)).toEqual({ name: null, failed: true }) + + http.get.mockResolvedValueOnce({ data: { names: {} } }) + expect(await contactNameResult(B)).toEqual({ name: null, failed: false }) + }) + it('reads the uuid from a string or a related object', () => { expect(contactPersonId(` ${A} `)).toBe(A) expect(contactPersonId({ id: A })).toBe(A) diff --git a/tests/vitest/contactPersonName.spec.js b/tests/vitest/contactPersonName.spec.js new file mode 100644 index 000000000..b956421b6 --- /dev/null +++ b/tests/vitest/contactPersonName.spec.js @@ -0,0 +1,79 @@ +// @vitest-environment jsdom + +/** + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * The name on a contact person's page, and what it says when there is none: + * not linked, in an address book the user cannot read, or a failed request. + * + * @spec openspec/specs/application-usage-pages/spec.md#requirement-req-uap-005-a-contact-persons-name-shows-only-to-who-can-read-the-contact + */ + +import { flushPromises, mount } from '@vue/test-utils' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const http = vi.hoisted(() => ({ get: vi.fn() })) + +vi.mock('@nextcloud/axios', () => ({ default: http })) +vi.mock('@nextcloud/router', () => ({ generateUrl: (url) => url })) +vi.mock('@nextcloud/l10n', () => ({ translate: (app, text) => text })) +vi.mock('@nextcloud/vue', () => ({ + NcLoadingIcon: { template: '' }, +})) +vi.mock('@conduction/nextcloud-vue', () => ({ + CnIcon: { template: '' }, + CnWidgetWrapper: { template: '
' }, +})) + +const { clearContactNames } = await import('../../src/utils/contactNames.js') +const ContactPersonName = ( + await import('../../src/components/organisations/ContactPersonName.vue') +).default + +const A = '00000000-0000-0000-0000-00000000000a' +async function mountIt(props) { + const wrapper = mount(ContactPersonName, { props: { objectId: A, ...props } }) + await flushPromises() + return wrapper +} + +describe('ContactPersonName', () => { + beforeEach(() => { + clearContactNames() + http.get.mockReset() + }) + + it('shows the name', async () => { + http.get.mockResolvedValue({ data: { names: { [A]: 'Anna' } } }) + const wrapper = await mountIt({ contactsUid: 'uid-1' }) + expect(wrapper.text()).toContain('Anna') + expect( + wrapper.find('[data-testid="contact-person-name-hint"]').exists(), + ).toBe(false) + }) + + it('says the contact person is not linked when there is no contactsUid', async () => { + http.get.mockResolvedValue({ data: { names: {} } }) + const wrapper = await mountIt({}) + const hint = wrapper.get('[data-testid="contact-person-name-hint"]').text() + expect(hint).toBe( + 'This contact person is not linked to a Nextcloud contact.', + ) + }) + + it('names the address book only after an answer without a name', async () => { + http.get.mockResolvedValue({ data: { names: {} } }) + const wrapper = await mountIt({ contactsUid: 'uid-1' }) + const hint = wrapper.get('[data-testid="contact-person-name-hint"]').text() + expect(hint).toContain('in an address book you cannot read') + }) + + it('says the name could not be loaded when the request failed', async () => { + http.get.mockRejectedValue(new Error('429')) + const wrapper = await mountIt({ contactsUid: 'uid-1' }) + const hint = wrapper.get('[data-testid="contact-person-name-hint"]').text() + expect(hint).toContain('could not be loaded') + expect(hint).not.toContain('address book') + }) +}) diff --git a/tests/vitest/usageRiskSignals.spec.js b/tests/vitest/usageRiskSignals.spec.js new file mode 100644 index 000000000..6ba69e658 --- /dev/null +++ b/tests/vitest/usageRiskSignals.spec.js @@ -0,0 +1,80 @@ +// @vitest-environment jsdom + +/** + * SPDX-FileCopyrightText: 2026 Conduction B.V. + * SPDX-License-Identifier: EUPL-1.2 + * + * The risk signals widget, opened straight from a link: it reads the app + * settings first, so the object types resolve before the first read. + * + * @spec openspec/specs/application-value-assessment/spec.md#requirement-req-ava-002-the-usage-page-shows-the-risk-signals-next-to-the-risk-score + */ + +import { flushPromises, mount } from '@vue/test-utils' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const order = vi.hoisted(() => []) +const store = vi.hoisted(() => ({ + settings: null, + objectTypeRegistry: {}, + fetchSettings: null, + registerObjectType: null, + getSchemaConfig: null, + fetchObject: null, + fetchCollectionForOptions: null, +})) + +vi.mock('../../src/store/store.js', () => ({ objectStore: store })) +vi.mock('@nextcloud/l10n', () => ({ + translate: (app, text) => text, + translatePlural: (app, singular) => singular, +})) +vi.mock('@nextcloud/vue', () => ({ + NcLoadingIcon: { template: '' }, + NcNoteCard: { template: '
' }, +})) +vi.mock('@conduction/nextcloud-vue', () => ({ + CnIcon: { template: '' }, + CnWidgetWrapper: { template: '
' }, +})) + +const UsageRiskSignals = ( + await import('../../src/components/portfolio/UsageRiskSignals.vue') +).default + +describe('UsageRiskSignals', () => { + beforeEach(() => { + order.length = 0 + store.settings = null + store.objectTypeRegistry = {} + store.fetchSettings = vi.fn(async () => { + order.push('fetchSettings') + store.settings = { loaded: true } + }) + store.getSchemaConfig = vi.fn(() => ({ register: 1, schema: 2 })) + store.registerObjectType = vi.fn() + store.fetchObject = vi.fn(async () => { + order.push('fetchObject') + return { riskScore: 3 } + }) + store.fetchCollectionForOptions = vi.fn(async () => []) + }) + + it('reads the settings before its first object read', async () => { + const wrapper = mount(UsageRiskSignals, { props: { objectId: 'u1' } }) + await flushPromises() + + expect(store.fetchSettings).toHaveBeenCalledTimes(1) + expect(order).toEqual(['fetchSettings', 'fetchObject']) + expect(wrapper.find('[data-testid="usage-risk-score"]').text()).toBe('3 / 5') + }) + + it('does not read the settings again when the app already has them', async () => { + store.settings = { loaded: true } + mount(UsageRiskSignals, { props: { objectId: 'u1' } }) + await flushPromises() + + expect(store.fetchSettings).not.toHaveBeenCalled() + expect(store.fetchObject).toHaveBeenCalled() + }) +}) From afd1c6fbf201e1e0eed8d34a4c16340743b6dc7d Mon Sep 17 00:00:00 2001 From: WilcoLouwerse Date: Tue, 13 Oct 2026 20:45:23 +0200 Subject: [PATCH 10/10] fix(contacts): keep the spinner until the contact person is loaded CnDetailPage passes no object while it loads and CnBodySections drops the unresolved @object.contactsUid, so the 'not linked' hint flashed before the object arrived. The page now also takes @object.id (loadedId) and shows the spinner until it is there. Also names the right test files in the REQ-UAP-005 scenario exclude reasons. Co-Authored-By: Claude Opus 5.5 --- .../specs/application-usage-pages/spec.md | 4 ++-- .../organisations/ContactPersonName.vue | 22 ++++++++++++++++++- src/manifest.json | 2 +- tests/vitest/contactPersonName.spec.js | 18 ++++++++++++++- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/openspec/specs/application-usage-pages/spec.md b/openspec/specs/application-usage-pages/spec.md index 36dee9624..5e34fe5df 100644 --- a/openspec/specs/application-usage-pages/spec.md +++ b/openspec/specs/application-usage-pages/spec.md @@ -71,7 +71,7 @@ The usage schema SHALL declare its lifecycle on the status values its rows hold, The contact person page and the Name column of an organisation's contact person list SHALL show the name of the linked Nextcloud contact only to a user who can read that contact in their own or shared address books, and SHALL say why there is no name when there is none. #### Scenario: A reader of the contact sees the name -@e2e exclude Needs a Contacts address book per user; tests/vitest/contactNames.spec.js asserts the name on the contact person page and in the list column. +@e2e exclude Needs a Contacts address book per user; tests/vitest/contactPersonName.spec.js asserts the name on the contact person page and tests/vitest/contactNames.spec.js the list column cell. - **GIVEN** a contact person linked to a contact in the information manager's own address book - **WHEN** the information manager opens the contact person, or the organisation it belongs to @@ -79,7 +79,7 @@ The contact person page and the Name column of an organisation's contact person - **AND** the Name column of the organisation's contact person list shows it #### Scenario: Without a name the page says why -@e2e exclude Needs a Contacts address book per user; tests/vitest/contactNames.spec.js asserts each empty state. +@e2e exclude Needs a Contacts address book per user; tests/vitest/contactPersonName.spec.js asserts each empty state, and that none shows before the object has loaded; tests/vitest/contactNames.spec.js asserts a failed request apart from an empty answer. - **GIVEN** a contact person whose name the signed-in user does not get - **WHEN** the user opens the contact person page diff --git a/src/components/organisations/ContactPersonName.vue b/src/components/organisations/ContactPersonName.vue index ab7ead2c6..4170a25f9 100644 --- a/src/components/organisations/ContactPersonName.vue +++ b/src/components/organisations/ContactPersonName.vue @@ -13,7 +13,7 @@