From b85016dab4e463c0711ef2a64c0143c877a6668e Mon Sep 17 00:00:00 2001 From: Rasmus Oscar Welander Date: Wed, 5 Aug 2026 16:29:39 +0200 Subject: [PATCH] EuroOffice mentions --- packages/web-app-external/src/App.vue | 32 ++- .../src/composables/postMessages/types.ts | 2 +- .../postMessages/useEuroOfficePostMessages.ts | 121 +++++++++- .../composables/useMentionNotifications.ts | 221 ++++++++++++++---- .../web-app-external/tests/unit/app.spec.ts | 43 +++- .../composables/postMessages/registry.spec.ts | 4 +- .../useCollaboraPostMessages.spec.ts | 3 + .../useEuroOfficePostMessages.spec.ts | 188 ++++++++++++++- .../useMentionNotifications.spec.ts | 146 +++++++++--- 9 files changed, 671 insertions(+), 89 deletions(-) diff --git a/packages/web-app-external/src/App.vue b/packages/web-app-external/src/App.vue index bc5bc2e7ab4..ce0dc411783 100644 --- a/packages/web-app-external/src/App.vue +++ b/packages/web-app-external/src/App.vue @@ -97,6 +97,36 @@ export default defineComponent({ return queryItemAsString(unref(templateIdQuery)) }) + // Anchor into the document, carried by "copy link to this comment" links and by @mention + // notifications. Only EuroOffice produces and understands these today, but forwarding it + // is app-agnostic: the parameter is only ever present on a link that app generated. + const actionLinkQuery = useRouteQuery('actionLink') + const actionLinkQueryValue = computed(() => { + return queryItemAsString(unref(actionLinkQuery)) + }) + + /** + * The editor reads its anchor off its own iframe URL (editor-wopi.ejs, `queryParams`), + * so it has to be forwarded onto the app url the backend hands back - nothing else in + * the chain carries it. Lower-cased on the way out to match the convention every other + * parameter that template reads follows (lang, thm, dchat, formsubmit...). + */ + const withActionLink = (url: string) => { + const actionLink = unref(actionLinkQueryValue) + if (!actionLink) { + return url + } + + try { + const parsed = new URL(url) + parsed.searchParams.set('actionlink', actionLink) + return parsed.toString() + } catch { + // never worth losing the document over a failed deep link + return url + } + } + const appName = computed(() => { const lowerCaseAppName = unref(route) .name.toString() @@ -334,7 +364,7 @@ export default defineComponent({ throw new Error('Error in app server response') } - appUrl.value = response.data.app_url + appUrl.value = withActionLink(response.data.app_url) method.value = response.data.method if (response.data.form_parameters) { diff --git a/packages/web-app-external/src/composables/postMessages/types.ts b/packages/web-app-external/src/composables/postMessages/types.ts index c55149d0644..a0522cf74e2 100644 --- a/packages/web-app-external/src/composables/postMessages/types.ts +++ b/packages/web-app-external/src/composables/postMessages/types.ts @@ -23,7 +23,7 @@ export interface OfficePostMessageRegistration { // only populated by MS365 today, exposed generically so App.vue can react to it isLoaded?: Ref // whether there are @mentions queued for notifyMentionedUsers that haven't been flushed yet; - // only populated by Collabora today, exposed generically so App.vue can warn before unload + // populated by Collabora and EuroOffice, exposed generically so App.vue can warn before unload hasPendingMentions?: Ref } diff --git a/packages/web-app-external/src/composables/postMessages/useEuroOfficePostMessages.ts b/packages/web-app-external/src/composables/postMessages/useEuroOfficePostMessages.ts index 67ff6b8f109..af4037d4753 100644 --- a/packages/web-app-external/src/composables/postMessages/useEuroOfficePostMessages.ts +++ b/packages/web-app-external/src/composables/postMessages/useEuroOfficePostMessages.ts @@ -1,6 +1,7 @@ import { ref, unref } from 'vue' import { useShareDialog } from '../useShareDialog' import { postMessageToIframe } from './postMessageToIframe' +import { useMentionNotifications } from '../useMentionNotifications' import { useOfficeFileOperations } from '../useOfficeFileOperations' import type { OfficePostMessage, @@ -8,12 +9,46 @@ import type { OfficePostMessageRegistration } from './types' +/** + * EuroOffice's actionLink: an anchor into the document - a comment id or a bookmark name. + * The editor produces one for every mention notification and for "copy link", and consumes + * one via editorConfig.actionLink when the document is opened, which is how a link of ours + * scrolls to the right comment. editor-wopi.ejs reads it off the `actionlink` query param. + */ +interface ActionLink { + action?: { type?: string; data?: string } +} + +const buildActionLinkUrl = (documentUrl: string, actionLink: ActionLink): string => { + if (!documentUrl || !actionLink) { + return documentUrl + } + + try { + const url = new URL(documentUrl) + url.searchParams.set('actionLink', JSON.stringify(actionLink)) + return url.toString() + } catch { + // privateLink isn't guaranteed to be absolute; a plain document link still opens the + // right file, it just won't scroll to the anchor + return documentUrl + } +} + export function useEuroOfficePostMessages( ctx: OfficePostMessageContext ): OfficePostMessageRegistration { const { space, resource, appIframeRef } = ctx const { openShareDialog } = useShareDialog() const { insertGraphic } = useOfficeFileOperations(ctx) + const { + resolveMentionUsers, + resolveUserIdsForEmails, + queueMention, + notifyMentionedUsers, + resetMentionState, + hasPendingMentions + } = useMentionNotifications(ctx) const isLoaded = ref(false) @@ -34,6 +69,79 @@ export function useEuroOfficePostMessages( }) } + /** + * The mention autocomplete asking who could be mentioned, filtered by `search`. + * + * Everything except `c: 'info'` takes an in-flight lock in Common.UI.ExternalUsers, which + * drops any further request until it's released - so those MUST be answered, an empty list + * included, or the mention dropdown stays wedged for the rest of the session. + */ + const handleRequestUsers = async (message: OfficePostMessage): Promise => { + const operation = (message.Values?.c as string) || 'mention' + const respond = (users: unknown[], values: Record = {}) => { + postMessageToIframe(appIframeRef, 'Action_SetUsers', { c: operation, users, ...values }) + } + + // 'info' asks who the ids already in the document belong to, so the comments panel can + // show author avatars. We have none, and it takes no lock, so leaving it unanswered is + // safe - the editor falls back to the names stored in the document. + if (operation === 'info') { + return + } + + // 'protect' (spreadsheet protected ranges) - nothing to offer, but the lock needs releasing + if (operation !== 'mention') { + return respond([], { isPaginated: true }) + } + + // `isPaginated` is what keeps the editor in server-side-search mode: leave it off and it + // caches the first list it gets and filters that locally forever, which for a directory the + // size of CERN's would mean shipping a useless prefix of it once and never searching again. + // `from`/`count` come back when the dropdown is scrolled to the bottom, and have to be + // honoured - the editor appends what it gets rather than replacing, so returning the whole + // result set again would show every match twice. + const from = (message.Values?.from as number) || 0 + const count = (message.Values?.count as number) || 100 + const users = await resolveMentionUsers((message.Values?.search as string) || '') + respond(users.slice(from, from + count), { isPaginated: true }) + } + + /** + * A comment containing mentions was submitted. Unlike Collabora, which reports a pick the + * moment it happens, EuroOffice only tells us once the comment is actually saved - so + * there is nothing to defer here and the queue is flushed straight away. + */ + const handleSendNotify = async (message: OfficePostMessage): Promise => { + const emails = message.Values?.emails + if (!Array.isArray(emails) || !emails.length) { + return + } + + const userIds = await resolveUserIdsForEmails(emails as string[]) + userIds.forEach(queueMention) + + await notifyMentionedUsers({ + commentText: (message.Values?.message as string) || '', + documentUrl: buildActionLinkUrl( + unref(resource).privateLink || '', + message.Values?.actionLink as ActionLink + ) + }) + } + + /** + * "Copy link to this comment/bookmark". The editor blocks on the reply - the link button + * shows nothing until Action_SetActionLink arrives - so this always answers, falling back + * to the plain document link if the anchor can't be attached. + */ + const handleMakeActionLink = (message: OfficePostMessage): void => { + const url = buildActionLinkUrl( + unref(resource).privateLink || '', + message.Values?.config as ActionLink + ) + postMessageToIframe(appIframeRef, 'Action_SetActionLink', { url }) + } + const handlePostMessage = async (event: MessageEvent): Promise => { let message: OfficePostMessage try { @@ -50,6 +158,12 @@ export function useEuroOfficePostMessages( return handleUiSharing() case 'UI_InsertGraphic': return handleUiInsertGraphic() + case 'UI_RequestUsers': + return handleRequestUsers(message) + case 'UI_SendNotify': + return handleSendNotify(message) + case 'UI_MakeActionLink': + return handleMakeActionLink(message) // case 'File_Rename': // case 'UI_Close': // case 'UI_Edit': @@ -58,5 +172,10 @@ export function useEuroOfficePostMessages( } } - return { handlePostMessage, isLoaded } + return { + handlePostMessage, + isLoaded, + onResourceChanged: resetMentionState, + hasPendingMentions + } } diff --git a/packages/web-app-external/src/composables/useMentionNotifications.ts b/packages/web-app-external/src/composables/useMentionNotifications.ts index 4512a75556f..d430119ccb0 100644 --- a/packages/web-app-external/src/composables/useMentionNotifications.ts +++ b/packages/web-app-external/src/composables/useMentionNotifications.ts @@ -1,5 +1,5 @@ import { computed, ref, unref } from 'vue' -import { call, GraphSharePermission, urlJoin } from '@ownclouders/web-client' +import { call, GraphSharePermission, isShareSpaceResource, urlJoin } from '@ownclouders/web-client' import type { User } from '@ownclouders/web-client/graph/generated' import { useClientService, @@ -8,6 +8,7 @@ import { useMessages, useRequest, useSharesStore, + useSpacesStore, useUserStore } from '@ownclouders/web-pkg' import { storeToRefs } from 'pinia' @@ -22,6 +23,28 @@ export interface MentionCandidate { label: string } +/** + * The same search results in the shape EuroOffice wants for + * setUsers. They key mentions on the email address rather than on an opaque id - the editor + * literally writes "+" into the comment text and parses the addresses back out of it + * (see Comments.js' pickEMail in Euro-Office/web-apps) - so a user without a mail address + * can't be mentioned there at all, unlike in Collabora. + */ +export interface MentionUser { + id: string + name: string + email: string + // users who can already open the file are listed above a separator in the editor's dropdown + hasAccess: boolean +} + +export interface MentionNotificationDetails { + // the comment the mention was written in, when the editor exposes it + commentText?: string + // deep link to the mention, when the editor can produce one; defaults to resource.privateLink + documentUrl?: string +} + /** * Shared, app-agnostic "notify a mentioned user" behavior - implemented once here, * translated to/from each app's own postMessage protocol by its dedicated composable. @@ -34,6 +57,7 @@ export function useMentionNotifications(ctx: OfficePostMessageContext) { const capabilityStore = useCapabilityStore() const userStore = useUserStore() const sharesStore = useSharesStore() + const spacesStore = useSpacesStore() const { collaboratorShares } = storeToRefs(sharesStore) const { makeRequest } = useRequest({ clientService }) const { showMessage } = useMessages() @@ -45,19 +69,42 @@ export function useMentionNotifications(ctx: OfficePostMessageContext) { // candidate id) when a mention is selected, not the full candidate, so this is // needed to recover the display name for error messages when granting access below. const lastSearchResults = ref(new Map()) + // Every user seen in a search this session, keyed by lowercased email: EuroOffice reports + // its mentions as email addresses, and only once the comment is submitted, so unlike + // lastSearchResults this has to survive the searches that happened in between. + const userIdsByEmail = ref(new Map()) const defaultShareRoleId = ref() const defaultShareRoleFetched = ref(false) + /** + * The drive that actually owns the permissions. For a resource shared with us the space is + * a share-jail entry whose id is not a real drive id, so listPermissions has to be pointed + * at the mount point's remote root instead - the same resolution FileSideBar.vue does + * before its own listPermissions call. Without it, mentioning someone in a document that + * was shared with you finds no roles and silently grants nothing. + */ + const resolveDriveId = async (): Promise => { + const currentSpace = unref(space) + if (!isShareSpaceResource(currentSpace)) { + return currentSpace.id + } + + const mountPoint = await spacesStore.getMountPointForSpace({ + graphClient: graphAuthenticated, + space: currentSpace + }) + return mountPoint?.root?.remoteItem?.rootId || currentSpace.id + } + const loadDefaultShareRoleId = async (): Promise => { if (unref(defaultShareRoleFetched)) { return unref(defaultShareRoleId) } - const currentSpace = unref(space) const currentResource = unref(resource) const { allowedRoles } = await graphAuthenticated.permissions.listPermissions( - currentSpace.id, + await resolveDriveId(), currentResource.fileId, sharesStore.graphRoles, {}, @@ -94,57 +141,140 @@ export function useMentionNotifications(ctx: OfficePostMessageContext) { } /** - * Searches all users, the same call the "invite people" dialog makes - * (InviteCollaboratorForm.vue's fetchRecipientsTask) - not just existing collaborators, - * so a mention can invite someone new. Deliberately scoped to individual users only - * (no groups): a @mention notifies one specific person, unlike a group share. + * Searches every user, not just existing collaborators, so a mention can invite someone + * new. Deliberately scoped to individual users only (no groups): a @mention notifies one + * specific person, unlike a group share. * - * CERN: the default search only covers primary (personal) accounts - service and - * secondary accounts aren't included unless explicitly filtered for, same as - * InviteCollaboratorForm.vue's per-role-type filters. The graph API's $filter doesn't - * support "or", so those two account types need separate calls; only issued when the - * default search comes up empty, to avoid the extra round-trips on the common case. + * CERN: `userType eq 'all'` covers primary, secondary and service accounts in one call. + * The invite dialog instead issues one call per type, because the graph $filter has no + * "or" - unnecessary here. Note this needs a reva new enough to know the 'all' user type; + * older ones reject the request with `unknown usertype: all`. */ + const findUsers = (searchText: string, signal?: AbortSignal): Promise => + graphAuthenticated.users.listUsers( + { orderBy: ['displayName'], search: `"${searchText}"`, filter: `userType eq 'all'` }, + { signal } + ) + // restartable: Collabora fires a fresh autocomplete search on every keystroke, so an // in-flight request from an earlier (now-stale) keystroke must be cancelled rather than // left to race a newer one and potentially overwrite it - same signal-cancellation pattern // as InviteCollaboratorForm.vue's fetchRecipientsTask. - const resolveMentionCandidatesTask = useTask(function* (signal, searchText: string) { - const currentResource = unref(resource) - const users = yield* call( - graphAuthenticated.users.listUsers( - { orderBy: ['displayName'], search: `"${searchText}"`, filter: `userType eq 'all'` }, - { signal } - ) - ) - - const candidates = (users || []) - .filter((user) => user.id !== userStore.user.id) - .map((user) => ({ - username: user.id, - // office apps expect a profile URL; we don't have a dedicated one, so link to the document - profile: currentResource.privateLink, - label: buildMentionLabel(user) - })) - - lastSearchResults.value = new Map( - candidates.map((candidate) => [candidate.username, candidate]) - ) - return candidates + const searchUsersTask = useTask(function* (signal, searchText: string) { + const users = yield* call(findUsers(searchText, signal)) + return (users || []).filter((user) => user.id !== userStore.user.id) }).restartable() - const resolveMentionCandidates = async (searchText: string): Promise => { + /** + * Returns null - not an empty list - when no search actually ran, so callers can tell + * "nobody matched" from "we never asked" and leave their remembered results alone. + */ + const searchUsers = async (searchText: string): Promise => { if (searchText.length < capabilityStore.sharingSearchMinLength) { - return [] + return null } try { - return (await resolveMentionCandidatesTask.perform(searchText)) || [] + return (await searchUsersTask.perform(searchText)) || [] } catch { // a newer keystroke restarted the task and cancelled this one - its result is stale, // the newer perform() call (already in flight) will produce the real answer + return null + } + } + + const toMentionCandidate = (user: User): MentionCandidate => ({ + username: user.id, + // office apps expect a profile URL; we don't have a dedicated one, so link to the document + profile: unref(resource).privateLink, + label: buildMentionLabel(user) + }) + + const rememberSearchResults = (users: User[]): void => { + lastSearchResults.value = new Map(users.map((user) => [user.id, toMentionCandidate(user)])) + // accumulated across searches, unlike lastSearchResults: EuroOffice only reports which + // users were mentioned once the comment is submitted, by which point the search that + // produced them can be many keystrokes old + users.forEach((user) => { + if (user.mail) { + unref(userIdsByEmail).set(user.mail.toLowerCase(), user.id) + } + }) + } + + /** Mention candidates in the shape Collabora's Action_Mention wants. */ + const resolveMentionCandidates = async (searchText: string): Promise => { + const users = await searchUsers(searchText) + if (!users) { + return [] + } + + rememberSearchResults(users) + return users.map(toMentionCandidate) + } + + /** + * Whether the user can already open the document. Broader than the check in + * grantAccessIfNeeded, which deliberately ignores indirect shares because it decides + * whether to create a *direct* one; here it only drives where the editor draws the + * separator in its mention dropdown, and inherited access counts just as well. + */ + const hasAccessToResource = (userId: string): boolean => + unref(collaboratorShares).some((share) => share.sharedWith?.id === userId) + + /** + * Same search as resolveMentionCandidates, in the shape EuroOffice's setUsers wants. + * Users without a mail address are dropped: the editor writes "+" into the comment + * and parses the addresses back out, so it has no way to refer to them. + */ + const resolveMentionUsers = async (searchText: string): Promise => { + const users = await searchUsers(searchText) + if (!users) { return [] } + + rememberSearchResults(users) + return users + .filter((user): user is User & { mail: string } => !!user.mail) + .map((user) => ({ + id: user.id, + name: buildMentionLabel(user), + email: user.mail, + hasAccess: hasAccessToResource(user.id) + })) + } + + /** + * Maps the email addresses EuroOffice reports back to user ids for notifyMentionedUsers. + * Addresses picked from the autocomplete are already known; one typed by hand never went + * through a search, hence the lookup fallback. + */ + const resolveUserIdsForEmails = async (emails: string[]): Promise => { + const userIds: string[] = [] + + for (const email of emails) { + const normalizedEmail = email.toLowerCase() + const knownUserId = unref(userIdsByEmail).get(normalizedEmail) + if (knownUserId) { + userIds.push(knownUserId) + continue + } + + try { + // deliberately not searchUsersTask: it is restartable, so this lookup and an + // autocomplete search running at the same time would cancel each other + const users = await findUsers(email) + const match = (users || []).find((user) => user.mail?.toLowerCase() === normalizedEmail) + if (match) { + unref(userIdsByEmail).set(normalizedEmail, match.id) + userIds.push(match.id) + } + } catch (e) { + console.error(`Error resolving mentioned user "${email}"`, e) + } + } + + return userIds } /** @@ -216,15 +346,17 @@ export function useMentionNotifications(ctx: OfficePostMessageContext) { * // Collabora @mention always * // targets one specific person * "event_id": string, // unique id for this flush - * "comment_text": string, // not exposed by Collabora's - * "anchor_text": string, // UI_Mention postMessage today - * "document_url": string, // resource.privateLink + * "comment_text": string, // details.commentText - not + * // exposed by Collabora's + * "anchor_text": string, // UI_Mention postMessage today + * "document_url": string, // details.documentUrl, else + * // resource.privateLink * "app_name": "office" * } * * Response: 202 Accepted, { accepted: [...], rejected: [...] } per-mention results. */ - const notifyMentionedUsers = async (): Promise => { + const notifyMentionedUsers = async (details: MentionNotificationDetails = {}): Promise => { if (!unref(userIdsToMention).length) { return } @@ -239,9 +371,9 @@ export function useMentionNotifications(ctx: OfficePostMessageContext) { file_id: currentResource.fileId, mentions: userIDs.map((username) => ({ type: 'user' as const, username })), event_id: uuidV4(), - comment_text: '', + comment_text: details.commentText || '', anchor_text: '', - document_url: currentResource.privateLink || '', + document_url: details.documentUrl || currentResource.privateLink || '', app_name: 'office' } }) @@ -256,12 +388,15 @@ export function useMentionNotifications(ctx: OfficePostMessageContext) { const resetMentionState = (): void => { lastSearchResults.value = new Map() + userIdsByEmail.value = new Map() defaultShareRoleId.value = undefined defaultShareRoleFetched.value = false } return { resolveMentionCandidates, + resolveMentionUsers, + resolveUserIdsForEmails, queueMention, notifyMentionedUsers, resetMentionState, diff --git a/packages/web-app-external/tests/unit/app.spec.ts b/packages/web-app-external/tests/unit/app.spec.ts index 550ce72bac0..6794ed5158d 100644 --- a/packages/web-app-external/tests/unit/app.spec.ts +++ b/packages/web-app-external/tests/unit/app.spec.ts @@ -5,7 +5,7 @@ import { nextTicks, shallowMount } from '@ownclouders/web-test-helpers' -import { AppProviderService, useRequest, useRoute } from '@ownclouders/web-pkg' +import { AppProviderService, useRequest, useRoute, useRouteQuery } from '@ownclouders/web-pkg' import { computed, ref } from 'vue' import { flushPromises } from '@vue/test-utils' @@ -18,7 +18,8 @@ import { WebThemeType } from '@ownclouders/web-pkg' vi.mock('@ownclouders/web-pkg', async (importOriginal) => ({ ...(await importOriginal()), useRequest: vi.fn(), - useRoute: vi.fn() + useRoute: vi.fn(), + useRouteQuery: vi.fn() })) vi.mock('../../src/composables', async (importOriginal) => ({ @@ -90,6 +91,40 @@ describe('The app provider extension', () => { await flushPromises() expect(wrapper.html()).toMatchSnapshot() }) + + // deep link into a comment: the editor only ever sees its own iframe url, so the anchor has + // to be forwarded onto the app url rather than left on ours + describe('actionLink forwarding', () => { + const getMakeRequest = () => + vi.fn().mockResolvedValue({ + ok: true, + status: 200, + data: providerSuccessResponseGet + }) + + it('forwards an actionLink from the route onto the app url', async () => { + const { wrapper } = createShallowMountWrapper( + getMakeRequest(), + { appNames: ['example-app'] }, + null, + mock({ isDark: false }), + { actionLink: '{"action":{"type":"comment","data":"1_1"}}' } + ) + await flushPromises() + + // lower-cased on the way out, to match what editor-wopi.ejs reads + expect(wrapper.html()).toContain('actionlink=%7B%22action%22') + }) + + it('leaves the app url alone when there is no actionLink', async () => { + const { wrapper } = createShallowMountWrapper(getMakeRequest()) + await flushPromises() + + expect(wrapper.html()).toContain(appUrl) + expect(wrapper.html()).not.toContain('actionlink') + }) + }) + describe('when the file is locked by another app', () => { it('shows a warning without a switch button when the locking app is unknown', async () => { const makeRequest = vi.fn().mockResolvedValue({ @@ -347,7 +382,8 @@ function createShallowMountWrapper( makeRequest = vi.fn().mockResolvedValue({ status: 200 }), appProviderService: Partial = { appNames: ['example-app'] }, space: SpaceResource = null, - currentTheme: WebThemeType = mock({ isDark: false }) + currentTheme: WebThemeType = mock({ isDark: false }), + routeQuery: Record = {} ) { vi.mocked(useRequest).mockImplementation(() => ({ makeRequest @@ -355,6 +391,7 @@ function createShallowMountWrapper( vi.mocked(useRoute).mockImplementation(() => ref(mock({ name: 'external-example-app-apps' })) ) + vi.mocked(useRouteQuery).mockImplementation((name: string) => ref(routeQuery[name])) const mocks = { ...defaultComponentMocks(), $appProviderService: mock(appProviderService) diff --git a/packages/web-app-external/tests/unit/composables/postMessages/registry.spec.ts b/packages/web-app-external/tests/unit/composables/postMessages/registry.spec.ts index e8b0af74737..d4c1143b7c3 100644 --- a/packages/web-app-external/tests/unit/composables/postMessages/registry.spec.ts +++ b/packages/web-app-external/tests/unit/composables/postMessages/registry.spec.ts @@ -26,9 +26,9 @@ describe('useOfficePostMessageRegistry', () => { expect(realRegistrations.some((r) => r.match('collabora-something'))).toBe(true) }) - it('has a real matcher for EuroOffice/OnlyOffice app names', () => { + it('has a real matcher for EuroOffice app names', () => { expect(realRegistrations.some((r) => r.match('EuroOffice'))).toBe(true) - expect(realRegistrations.some((r) => r.match('OnlyOffice'))).toBe(true) + expect(realRegistrations.some((r) => r.match('EuroOffice Writer'))).toBe(true) }) it('has a real matcher for MS365, exclusive to that app name', () => { diff --git a/packages/web-app-external/tests/unit/composables/postMessages/useCollaboraPostMessages.spec.ts b/packages/web-app-external/tests/unit/composables/postMessages/useCollaboraPostMessages.spec.ts index 742a3ac1695..0d42d6cef4a 100644 --- a/packages/web-app-external/tests/unit/composables/postMessages/useCollaboraPostMessages.spec.ts +++ b/packages/web-app-external/tests/unit/composables/postMessages/useCollaboraPostMessages.spec.ts @@ -28,6 +28,9 @@ describe('useCollaboraPostMessages', () => { const createMentionsMock = () => ({ resolveMentionCandidates: vi.fn().mockResolvedValue([]), + // EuroOffice-only, but mockReturnValue needs the whole composable's shape + resolveMentionUsers: vi.fn().mockResolvedValue([]), + resolveUserIdsForEmails: vi.fn().mockResolvedValue([]), queueMention: vi.fn(), notifyMentionedUsers: vi.fn().mockResolvedValue(undefined), resetMentionState: vi.fn(), diff --git a/packages/web-app-external/tests/unit/composables/postMessages/useEuroOfficePostMessages.spec.ts b/packages/web-app-external/tests/unit/composables/postMessages/useEuroOfficePostMessages.spec.ts index ebbb358b744..354b520c8e1 100644 --- a/packages/web-app-external/tests/unit/composables/postMessages/useEuroOfficePostMessages.spec.ts +++ b/packages/web-app-external/tests/unit/composables/postMessages/useEuroOfficePostMessages.spec.ts @@ -1,17 +1,21 @@ -import { ref, unref } from 'vue' +import { computed, ref, unref } from 'vue' import { mock } from 'vitest-mock-extended' import { Resource, SpaceResource } from '@ownclouders/web-client' import { useEuroOfficePostMessages } from '../../../../src/composables/postMessages/useEuroOfficePostMessages' import { useShareDialog } from '../../../../src/composables/useShareDialog' +import { useMentionNotifications } from '../../../../src/composables/useMentionNotifications' +import { useOfficeFileOperations } from '../../../../src/composables/useOfficeFileOperations' import { postMessageToIframe } from '../../../../src/composables/postMessages/postMessageToIframe' vi.mock('../../../../src/composables/useShareDialog') +vi.mock('../../../../src/composables/useMentionNotifications') +vi.mock('../../../../src/composables/useOfficeFileOperations') vi.mock('../../../../src/composables/postMessages/postMessageToIframe') describe('useEuroOfficePostMessages', () => { const getContext = () => ({ space: ref(mock()), - resource: ref(mock()), + resource: ref(mock({ privateLink: 'https://cernbox.cern.ch/f/123' })), appIframeRef: ref(null) }) @@ -20,52 +24,212 @@ describe('useEuroOfficePostMessages', () => { data: JSON.stringify({ MessageId: messageId, ...(values && { Values: values }) }) }) + const createMentionsMock = () => ({ + resolveMentionCandidates: vi.fn().mockResolvedValue([]), + resolveMentionUsers: vi.fn().mockResolvedValue([]), + resolveUserIdsForEmails: vi.fn().mockResolvedValue([]), + queueMention: vi.fn(), + notifyMentionedUsers: vi.fn().mockResolvedValue(undefined), + resetMentionState: vi.fn(), + hasPendingMentions: computed(() => false) + }) + let openShareDialogMock: ReturnType + let mentionsMock: ReturnType beforeEach(() => { openShareDialogMock = vi.fn() + mentionsMock = createMentionsMock() vi.mocked(useShareDialog).mockReturnValue({ openShareDialog: openShareDialogMock }) + vi.mocked(useMentionNotifications).mockReturnValue(mentionsMock) + vi.mocked(useOfficeFileOperations).mockReturnValue({ + saveAs: vi.fn().mockResolvedValue(undefined), + insertGraphic: vi.fn().mockResolvedValue(undefined), + insertFile: vi.fn().mockResolvedValue(undefined), + insertLink: vi.fn().mockResolvedValue(undefined) + }) }) it.each([ 'App_LoadingStatus', 'UI_Close', 'UI_Sharing', + 'UI_InsertGraphic', 'UI_FileVersions', + 'UI_RequestUsers', + 'UI_SendNotify', + 'UI_MakeActionLink', 'File_Rename', 'Edit_Notification' - ])('dispatches a %s message without throwing', (messageId) => { + ])('dispatches a %s message without throwing', async (messageId) => { const { handlePostMessage } = useEuroOfficePostMessages(getContext()) - handlePostMessage(createMessageEvent(messageId)) + await handlePostMessage(createMessageEvent(messageId)) }) - it('ignores unknown message ids', () => { + it('ignores unknown message ids', async () => { const { handlePostMessage } = useEuroOfficePostMessages(getContext()) - handlePostMessage(createMessageEvent('Some_Unknown_Message')) + await handlePostMessage(createMessageEvent('Some_Unknown_Message')) }) - it('swallows malformed message data', () => { + it('swallows malformed message data', async () => { const { handlePostMessage } = useEuroOfficePostMessages(getContext()) - handlePostMessage(mock({ data: 'not-json' })) + await handlePostMessage(mock({ data: 'not-json' })) }) - it('replies with Host_PostmessageReady and marks the app as loaded on App_LoadingStatus', () => { + it('replies with Host_PostmessageReady and marks the app as loaded on App_LoadingStatus', async () => { const ctx = getContext() const { handlePostMessage, isLoaded } = useEuroOfficePostMessages(ctx) expect(unref(isLoaded)).toBe(false) - handlePostMessage(createMessageEvent('App_LoadingStatus')) + await handlePostMessage(createMessageEvent('App_LoadingStatus')) expect(unref(isLoaded)).toBe(true) expect(postMessageToIframe).toHaveBeenCalledWith(ctx.appIframeRef, 'Host_PostmessageReady') }) - it('opens the share dialog on UI_Sharing', () => { + it('opens the share dialog on UI_Sharing', async () => { const ctx = getContext() const { handlePostMessage } = useEuroOfficePostMessages(ctx) - handlePostMessage(createMessageEvent('UI_Sharing')) + await handlePostMessage(createMessageEvent('UI_Sharing')) expect(openShareDialogMock).toHaveBeenCalledWith(ctx.space.value, ctx.resource.value) }) + + describe('UI_RequestUsers', () => { + it('answers a mention search with the resolved users, echoing c and paginating', async () => { + const ctx = getContext() + const users = [ + { id: '1', name: 'Alice', email: 'alice@cern.ch', hasAccess: true }, + { id: '2', name: 'Bob', email: 'bob@cern.ch', hasAccess: false } + ] + mentionsMock.resolveMentionUsers.mockResolvedValue(users) + const { handlePostMessage } = useEuroOfficePostMessages(ctx) + + await handlePostMessage( + createMessageEvent('UI_RequestUsers', { c: 'mention', from: 0, count: 100, search: 'al' }) + ) + + expect(mentionsMock.resolveMentionUsers).toHaveBeenCalledWith('al') + expect(postMessageToIframe).toHaveBeenCalledWith(ctx.appIframeRef, 'Action_SetUsers', { + c: 'mention', + users, + isPaginated: true + }) + }) + + it('returns the page the editor asked for', async () => { + const ctx = getContext() + mentionsMock.resolveMentionUsers.mockResolvedValue([ + { id: '1', name: 'Alice', email: 'alice@cern.ch', hasAccess: false }, + { id: '2', name: 'Bob', email: 'bob@cern.ch', hasAccess: false } + ]) + const { handlePostMessage } = useEuroOfficePostMessages(ctx) + + await handlePostMessage( + createMessageEvent('UI_RequestUsers', { c: 'mention', from: 1, count: 1, search: 'b' }) + ) + + expect(postMessageToIframe).toHaveBeenCalledWith( + ctx.appIframeRef, + 'Action_SetUsers', + expect.objectContaining({ + users: [{ id: '2', name: 'Bob', email: 'bob@cern.ch', hasAccess: false }] + }) + ) + }) + + // the editor drops any request made while one is still in flight, so an unanswered + // request it holds the lock for wedges the mention dropdown for good + it('still answers a search it cannot serve, so the editor is not left waiting', async () => { + const ctx = getContext() + const { handlePostMessage } = useEuroOfficePostMessages(ctx) + + await handlePostMessage(createMessageEvent('UI_RequestUsers', { c: 'protect' })) + + expect(postMessageToIframe).toHaveBeenCalledWith(ctx.appIframeRef, 'Action_SetUsers', { + c: 'protect', + users: [], + isPaginated: true + }) + }) + + // avatars aren't a thing in CERNBox, and 'info' takes no in-flight lock, so ignoring it + // is safe - the editor falls back to the author names stored in the document + it('ignores author-info lookups', async () => { + const ctx = getContext() + const { handlePostMessage } = useEuroOfficePostMessages(ctx) + + await handlePostMessage(createMessageEvent('UI_RequestUsers', { c: 'info', id: ['1', '2'] })) + + expect(postMessageToIframe).not.toHaveBeenCalled() + expect(mentionsMock.resolveMentionUsers).not.toHaveBeenCalled() + }) + }) + + describe('UI_MakeActionLink', () => { + it('answers with the document link carrying the anchor', async () => { + const ctx = getContext() + const { handlePostMessage } = useEuroOfficePostMessages(ctx) + + await handlePostMessage( + createMessageEvent('UI_MakeActionLink', { + config: { action: { type: 'comment', data: '1_1' } } + }) + ) + + expect(postMessageToIframe).toHaveBeenCalledWith(ctx.appIframeRef, 'Action_SetActionLink', { + url: 'https://cernbox.cern.ch/f/123?actionLink=%7B%22action%22%3A%7B%22type%22%3A%22comment%22%2C%22data%22%3A%221_1%22%7D%7D' + }) + }) + + // the editor's link button blocks until it gets a reply + it('still answers when the anchor cannot be attached', async () => { + const ctx = getContext() + ctx.resource.value.privateLink = 'not-a-url' + const { handlePostMessage } = useEuroOfficePostMessages(ctx) + + await handlePostMessage(createMessageEvent('UI_MakeActionLink', { config: {} })) + + expect(postMessageToIframe).toHaveBeenCalledWith(ctx.appIframeRef, 'Action_SetActionLink', { + url: 'not-a-url' + }) + }) + }) + + describe('UI_SendNotify', () => { + it('resolves the mentioned emails and flushes the notification', async () => { + const ctx = getContext() + mentionsMock.resolveUserIdsForEmails.mockResolvedValue(['1', '2']) + const { handlePostMessage } = useEuroOfficePostMessages(ctx) + + await handlePostMessage( + createMessageEvent('UI_SendNotify', { + emails: ['alice@cern.ch', 'bob@cern.ch'], + message: 'have a look +alice@cern.ch', + actionLink: { action: { type: 'comment', data: '1_1' } } + }) + ) + + expect(mentionsMock.resolveUserIdsForEmails).toHaveBeenCalledWith([ + 'alice@cern.ch', + 'bob@cern.ch' + ]) + expect(mentionsMock.queueMention).toHaveBeenCalledWith('1') + expect(mentionsMock.queueMention).toHaveBeenCalledWith('2') + expect(mentionsMock.notifyMentionedUsers).toHaveBeenCalledWith({ + commentText: 'have a look +alice@cern.ch', + documentUrl: + 'https://cernbox.cern.ch/f/123?actionLink=%7B%22action%22%3A%7B%22type%22%3A%22comment%22%2C%22data%22%3A%221_1%22%7D%7D' + }) + }) + + it('does nothing when no emails were mentioned', async () => { + const { handlePostMessage } = useEuroOfficePostMessages(getContext()) + + await handlePostMessage(createMessageEvent('UI_SendNotify', { emails: [] })) + + expect(mentionsMock.notifyMentionedUsers).not.toHaveBeenCalled() + }) + }) }) diff --git a/packages/web-app-external/tests/unit/composables/useMentionNotifications.spec.ts b/packages/web-app-external/tests/unit/composables/useMentionNotifications.spec.ts index 8066065d0d8..7a86728674f 100644 --- a/packages/web-app-external/tests/unit/composables/useMentionNotifications.spec.ts +++ b/packages/web-app-external/tests/unit/composables/useMentionNotifications.spec.ts @@ -19,11 +19,14 @@ describe('useMentionNotifications', () => { appIframeRef: ref(null) }) + // mock() auto-stubs any property left unset, and a stub is truthy - so every field + // that gets read as "is this set?" has to be spelled out here const createUser = (overrides: Partial = {}) => mock({ id: 'alice-id', displayName: 'Alice', onPremisesSamAccountName: undefined, + mail: undefined, ...overrides }) @@ -140,41 +143,132 @@ describe('useMentionNotifications', () => { expect(candidates).toEqual([expect.objectContaining({ label: 'Alice Smith (asmith)' })]) }) - it('falls back to separate secondary/service account searches when the default search is empty', async () => { - const { getInstance, mocks } = getWrapper() - mocks.$clientService.graphAuthenticated.users.listUsers - .mockResolvedValueOnce([]) - .mockResolvedValueOnce([ - createUser({ id: 'secondary-id', displayName: 'Secondary Account' }) - ]) - .mockResolvedValueOnce([createUser({ id: 'svc-id', displayName: 'Service Account' })]) - - const candidates = await getInstance().resolveMentionCandidates('svc') - - expect(mocks.$clientService.graphAuthenticated.users.listUsers).toHaveBeenNthCalledWith( - 2, - expect.objectContaining({ search: '"svc"', filter: "userType eq 'Secondary'" }), - expect.objectContaining({ signal: expect.anything() }) - ) - expect(mocks.$clientService.graphAuthenticated.users.listUsers).toHaveBeenNthCalledWith( - 3, - expect.objectContaining({ search: '"svc"', filter: "userType eq 'Service'" }), + // one call covering primary, secondary and service accounts - the invite dialog needs + // one per type only because it filters by share role + it("covers every account type in a single call via userType eq 'all'", async () => { + const { getInstance, mocks } = getWrapper({ + users: [createUser({ id: 'alice-id', displayName: 'Alice' })] + }) + + await getInstance().resolveMentionCandidates('ali') + + expect(mocks.$clientService.graphAuthenticated.users.listUsers).toHaveBeenCalledTimes(1) + expect(mocks.$clientService.graphAuthenticated.users.listUsers).toHaveBeenCalledWith( + expect.objectContaining({ filter: "userType eq 'all'" }), expect.objectContaining({ signal: expect.anything() }) ) - expect(candidates).toEqual([ - expect.objectContaining({ username: 'secondary-id' }), - expect.objectContaining({ username: 'svc-id' }) + }) + }) + + describe('resolveMentionUsers', () => { + it('returns the search results in the shape EuroOffice setUsers wants', async () => { + const { getInstance } = getWrapper({ + users: [ + createUser({ + id: 'alice-id', + displayName: 'Alice Smith', + onPremisesSamAccountName: 'asmith', + mail: 'alice@example.test' + }) + ] + }) + + const users = await getInstance().resolveMentionUsers('ali') + + expect(users).toEqual([ + { + id: 'alice-id', + name: 'Alice Smith (asmith)', + email: 'alice@example.test', + hasAccess: false + } ]) }) - it('does not search service/secondary accounts when the default search already found someone', async () => { + // the editor writes "+" into the comment and parses the addresses back out, so a + // user it has no address for cannot be mentioned at all + it('drops users without an email address', async () => { + const { getInstance } = getWrapper({ + users: [ + createUser({ id: 'alice-id', displayName: 'Alice', mail: undefined }), + createUser({ id: 'bob-id', displayName: 'Bob', mail: 'bob@example.test' }) + ] + }) + + const users = await getInstance().resolveMentionUsers('b') + + expect(users).toEqual([expect.objectContaining({ id: 'bob-id' })]) + }) + + it('flags users who can already open the document', async () => { + const { getInstance } = getWrapper({ + users: [createUser({ id: 'alice-id', displayName: 'Alice', mail: 'alice@example.test' })], + collaboratorShares: [ + createCollaboratorShare({ sharedWith: { id: 'alice-id', displayName: 'Alice' } }) + ] + }) + + const users = await getInstance().resolveMentionUsers('ali') + + expect(users).toEqual([expect.objectContaining({ hasAccess: true })]) + }) + + it('does not search below the sharing search minimum length', async () => { + const { getInstance, mocks } = getWrapper({ users: [createUser()], searchMinLength: 3 }) + + const users = await getInstance().resolveMentionUsers('al') + + expect(users).toEqual([]) + expect(mocks.$clientService.graphAuthenticated.users.listUsers).not.toHaveBeenCalled() + }) + }) + + describe('resolveUserIdsForEmails', () => { + it('maps addresses picked from the autocomplete without searching again', async () => { const { getInstance, mocks } = getWrapper({ - users: [createUser({ id: 'alice-id', displayName: 'Alice' })] + users: [createUser({ id: 'alice-id', displayName: 'Alice', mail: 'Alice@Example.test' })] }) + const instance = getInstance() - await getInstance().resolveMentionCandidates('ali') + await instance.resolveMentionUsers('ali') + vi.mocked(mocks.$clientService.graphAuthenticated.users.listUsers).mockClear() - expect(mocks.$clientService.graphAuthenticated.users.listUsers).toHaveBeenCalledTimes(1) + // the editor lowercases the address it writes into the comment + const userIds = await instance.resolveUserIdsForEmails(['alice@example.test']) + + expect(userIds).toEqual(['alice-id']) + expect(mocks.$clientService.graphAuthenticated.users.listUsers).not.toHaveBeenCalled() + }) + + it('looks up an address that was typed by hand', async () => { + const { getInstance, mocks } = getWrapper({ + users: [createUser({ id: 'bob-id', displayName: 'Bob', mail: 'bob@example.test' })] + }) + + const userIds = await getInstance().resolveUserIdsForEmails(['bob@example.test']) + + expect(userIds).toEqual(['bob-id']) + expect(mocks.$clientService.graphAuthenticated.users.listUsers).toHaveBeenCalled() + }) + + it('skips an address that matches nobody', async () => { + const { getInstance } = getWrapper({ + users: [createUser({ id: 'bob-id', displayName: 'Bob', mail: 'bob@example.test' })] + }) + + const userIds = await getInstance().resolveUserIdsForEmails(['nobody@example.test']) + + expect(userIds).toEqual([]) + }) + + it('swallows lookup errors', async () => { + vi.spyOn(console, 'error').mockImplementation(() => undefined) + const { getInstance, mocks } = getWrapper() + mocks.$clientService.graphAuthenticated.users.listUsers.mockRejectedValue( + new Error('network error') + ) + + await expect(getInstance().resolveUserIdsForEmails(['bob@example.test'])).resolves.toEqual([]) }) })