diff --git a/components/admin/org/sections/shared-tags-section.tsx b/components/admin/org/sections/shared-tags-section.tsx index 4bd0c35d5..8b19486e1 100644 --- a/components/admin/org/sections/shared-tags-section.tsx +++ b/components/admin/org/sections/shared-tags-section.tsx @@ -15,7 +15,7 @@ export function SharedTagsSection({ draft }: { draft: OrgPolicyDraft }) {
{restrictionKeysForPage('org-sharing') diff --git a/components/settings/__tests__/shared-tag-settings.test.tsx b/components/settings/__tests__/shared-tag-settings.test.tsx index 114a61cb4..6d582d9c9 100644 --- a/components/settings/__tests__/shared-tag-settings.test.tsx +++ b/components/settings/__tests__/shared-tag-settings.test.tsx @@ -1,23 +1,31 @@ -import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Names with their tag in them: the global mock returns the bare key. +vi.mock('next-intl', () => ({ + useTranslations: () => (key: string, params?: { name?: string }) => (params?.name ? `${key}:${params.name}` : key), + useLocale: () => 'en', +})); + import { SharedTagSettings, maySetSharedTags } from '../shared-tag-settings'; import { DEFAULT_POLICY } from '@/lib/admin/types'; import { useAuthStore } from '@/stores/auth-store'; import { useEmailStore } from '@/stores/email-store'; import { usePolicyStore } from '@/stores/policy-store'; import { useSettingsStore } from '@/stores/settings-store'; -import { resetSharedTags } from '@/stores/shared-tags-store'; +import { resetSharedTags, sharedTagsKey, useSharedTagsStore } from '@/stores/shared-tags-store'; // Settings > Folders & tags > Shared tags: the editor of each group // mailbox's tags, read-only when the admin locked them, and a note instead of -// an editor where the server keeps no metadata. +// an editor where the server keeps no metadata or couldn't be read. -const URGENT = { id: 'urgent', label: 'Urgent', color: 'red' }; +const URGENT = { label: 'Urgent', color: 'red' }; const RIGHTS = { mayReadItems: true, mayAddItems: true, mayRemoveItems: true, maySetSeen: true, maySetKeywords: true, mayCreateChild: true, mayRename: true, mayDelete: true, maySubmit: true }; -let stored: Record; +let stored: Record | null; let keywords: Record; -let metadata: boolean; +let advertised: boolean; +let principalType: string; const client = { getAccountId: () => 'me', getAccountInfo: (id: string) => ({ isPersonal: id === 'me' }), @@ -25,12 +33,21 @@ const client = { { id: 'me', name: 'org-alice@example.org', isPrimary: true, capabilities: { mail: true } }, { id: 'team', name: 'org-team@example.org', isPrimary: false, capabilities: { mail: true } }, ], - getPrincipalTypes: async () => ({ team: 'group' }), - supportsBulwarkMetadata: () => metadata, - getBulwarkMetadata: vi.fn(async () => stored), - setBulwarkMetadata: vi.fn(async (_id: string | undefined, key: string, value: unknown) => { stored = { ...stored, [key]: value }; }), + getPrincipalTypes: async () => ({ team: principalType }), + advertisesBulwarkMetadata: () => advertised, + supportsBulwarkMetadata: () => advertised, + getBulwarkMetadata: vi.fn(async () => (stored ? { tags: stored } : null)), + patchBulwarkMetadata: vi.fn(async (_id: string | undefined, _key: string, patches: Array<{ path: string[]; value: unknown }>) => { + const tags = { ...((stored?.tags ?? {}) as Record) }; + for (const { path, value } of patches) { + if (value === null) delete tags[path[1]]; + else tags[path[1]] = value; + } + stored = { tags }; + return { newState: 'sX' }; + }), discoverKeywords: async () => ({ keywords, scanned: 0, total: 0, complete: true }), - getMailboxes: async () => [], + getMailboxes: vi.fn(async () => []), }; function setRights(rights: Record) { @@ -43,11 +60,13 @@ function setLocked(locked: boolean) { beforeEach(() => { resetSharedTags(); - stored = { tags: { list: [URGENT] } }; + stored = { tags: { urgent: { ...URGENT } } }; keywords = {}; - metadata = true; + advertised = true; + principalType = 'group'; client.getBulwarkMetadata.mockClear(); - client.setBulwarkMetadata.mockClear(); + client.patchBulwarkMetadata.mockClear(); + client.getMailboxes.mockClear(); useSettingsStore.setState({ emailKeywords: [], nestedTags: false }); useAuthStore.setState({ client: client as never, @@ -60,26 +79,53 @@ beforeEach(() => { afterEach(() => resetSharedTags()); describe('SharedTagSettings', () => { - it('lets a member who may change the Inbox add a shared tag', async () => { + it('lets a member who may change the Inbox add a shared tag, as one patch', async () => { render(); expect(await screen.findByText('Urgent')).toBeInTheDocument(); fireEvent.click(screen.getByText('add')); fireEvent.change(screen.getByLabelText('name'), { target: { value: 'Waiting' } }); fireEvent.click(screen.getByText('save')); - await waitFor(() => expect(client.setBulwarkMetadata).toHaveBeenCalledWith('team', 'tags', { - list: [URGENT, { id: 'waiting', label: 'Waiting', color: 'blue' }], - })); + await waitFor(() => expect(client.patchBulwarkMetadata).toHaveBeenCalledWith('team', 'tags', [ + { path: ['tags', 'waiting'], value: { label: 'Waiting', color: 'blue' } }, + ])); expect(await screen.findByText('Waiting')).toBeInTheDocument(); }); + it('names the buttons after their tag and puts focus back on the edit button after saving', async () => { + render(); + fireEvent.click(await screen.findByLabelText('edit_tag:Urgent')); + fireEvent.change(screen.getByLabelText('name'), { target: { value: 'Very urgent' } }); + fireEvent.click(screen.getByText('save')); + // The color wasn't picked, so it isn't sent: whatever is stored stays. + await waitFor(() => expect(client.patchBulwarkMetadata).toHaveBeenCalledWith('team', 'tags', [ + { path: ['tags', 'urgent'], value: { label: 'Very urgent', color: 'red' } }, + ])); + await waitFor(() => expect(document.activeElement).toBe(screen.getByLabelText('edit_tag:Very urgent'))); + }); + + it('warns that deleting is for everyone, in words, before it deletes', async () => { + render(); + fireEvent.click(await screen.findByLabelText('delete_tag:Urgent')); + const confirm = screen.getByTestId('shared-tag-delete-confirm'); + const warning = screen.getByText('delete_confirm'); + expect(confirm).toContainElement(warning); + // Shown at every width, not only from the sm breakpoint up. + expect(warning.className).not.toMatch(/(^|\s)hidden(\s|$)/); + fireEvent.click(screen.getByText('delete')); + await waitFor(() => expect(client.patchBulwarkMetadata).toHaveBeenCalledWith('team', 'tags', [{ path: ['tags', 'urgent'], value: null }])); + }); + it('shows the tags read-only with "Set by your organisation" when the admin locked them', async () => { setLocked(true); + keywords = { '$label:found': 1 }; const { container } = render(); expect(await screen.findByText('Urgent')).toBeInTheDocument(); + await screen.findByText('found_on_messages'); expect(container.querySelector('[data-org-locked]')).not.toBeNull(); expect(screen.queryByText('add')).not.toBeInTheDocument(); - expect(screen.queryByLabelText('edit')).not.toBeInTheDocument(); - expect(screen.queryByLabelText('delete')).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/^edit_tag/)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/^delete_tag/)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/^name_it/)).not.toBeInTheDocument(); }); it('is read-only with a note for a member without the right to change the Inbox', async () => { @@ -89,23 +135,50 @@ describe('SharedTagSettings', () => { expect(screen.queryByText('add')).not.toBeInTheDocument(); }); + it('asks for no rights of a mailbox a person shared', async () => { + principalType = 'individual'; + useEmailStore.setState({ mailboxes: [] }); + render(); + await waitFor(() => expect(useSharedTagsStore.getState().entries[sharedTagsKey('login-1', 'team')]?.status).toBe('personal')); + expect(client.getMailboxes).not.toHaveBeenCalled(); + expect(screen.queryByTestId('shared-tags-account')).not.toBeInTheDocument(); + }); + it('says in one line that the server keeps no shared tags, with no editor', async () => { - metadata = false; + advertised = false; render(); expect(await screen.findByTestId('shared-tags-unsupported')).toHaveTextContent('unsupported'); expect(screen.queryByText('add')).not.toBeInTheDocument(); expect(client.getBulwarkMetadata).not.toHaveBeenCalled(); }); + it('says so in one line when the shared tags couldn\'t be read', async () => { + stored = null; + render(); + expect(await screen.findByTestId('shared-tags-unavailable')).toHaveTextContent('unavailable'); + expect(screen.queryByText('add')).not.toBeInTheDocument(); + }); + + it('keeps a change the server didn\'t take and says it will try again', async () => { + render(); + await screen.findByText('Urgent'); + client.patchBulwarkMetadata.mockRejectedValueOnce(new TypeError('Failed to fetch')); + fireEvent.click(screen.getByText('add')); + fireEvent.change(screen.getByLabelText('name'), { target: { value: 'Waiting' } }); + await act(async () => { fireEvent.click(screen.getByText('save')); }); + expect(await screen.findByTestId('shared-tags-pending')).toHaveTextContent('save_pending'); + expect(screen.getByText('Waiting')).toBeInTheDocument(); + }); + it('offers to name a tag found on the messages, keeping its id', async () => { keywords = { '$label:waiting-for-customer': 2 }; render(); - fireEvent.click(await screen.findByLabelText('name_it')); + fireEvent.click(await screen.findByLabelText('name_it:waiting-for-customer')); fireEvent.change(screen.getByLabelText('name'), { target: { value: 'Customer' } }); fireEvent.click(screen.getByText('save')); - await waitFor(() => expect(client.setBulwarkMetadata).toHaveBeenCalledWith('team', 'tags', { - list: [URGENT, { id: 'waiting-for-customer', label: 'Customer', color: expect.any(String) }], - })); + await waitFor(() => expect(client.patchBulwarkMetadata).toHaveBeenCalledWith('team', 'tags', [ + { path: ['tags', 'waiting-for-customer'], value: { label: 'Customer', color: expect.any(String) } }, + ])); }); }); diff --git a/components/settings/shared-tag-settings.tsx b/components/settings/shared-tag-settings.tsx index 1301b5259..7b7da3701 100644 --- a/components/settings/shared-tag-settings.tsx +++ b/components/settings/shared-tag-settings.tsx @@ -1,6 +1,6 @@ "use client"; -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useTranslations } from "next-intl"; import { Check, Loader2, Pencil, Plus, Trash2, Users, X } from "@/components/icons"; import { TagBadge } from "@/components/email/tag-badge"; @@ -9,12 +9,12 @@ import { useRestriction } from "@/hooks/use-restriction"; import { MetadataTooLargeError } from "@/lib/jmap/bulwark-metadata"; import type { MailboxRights } from "@/lib/jmap/types"; import { composeKeywordId, MAX_KEYWORD_ID_LENGTH } from "@/lib/keyword-nesting"; -import { MAX_SHARED_TAG_LABEL, groupAccountTags, removeSharedTag, upsertSharedTag } from "@/lib/shared-tags"; +import { MAX_SHARED_TAG_LABEL, groupAccountTags, type SharedTagChange } from "@/lib/shared-tags"; import { cn } from "@/lib/utils"; import { useAuthStore } from "@/stores/auth-store"; import { useEmailStore } from "@/stores/email-store"; -import { useSettingsStore, type KeywordDefinition } from "@/stores/settings-store"; -import { saveSharedTags, sharedTagsKey, useSharedTagsStore } from "@/stores/shared-tags-store"; +import { KEYWORD_PALETTE, useSettingsStore, type KeywordDefinition } from "@/stores/settings-store"; +import { SharedTagsSaveDeferredError, saveSharedTags, sharedTagsKey, useSharedTagsStore } from "@/stores/shared-tags-store"; import { toast } from "@/stores/toast-store"; import { toUnicodeEmail } from "@/lib/idn"; import { KeywordColorPicker } from "./keyword-settings"; @@ -28,13 +28,16 @@ export function maySetSharedTags(rights: Partial | null | undefin return typeof rights.maySetMetadata === "boolean" ? rights.maySetMetadata : rights.mayRename === true; } -/** The user's rights on the Inbox of a group account, from the folder list or asked once. */ -function useInboxRights(scope: TagScope): Partial | null | undefined { +/** + * The user's rights on the Inbox of a confirmed group account, from the + * folder list or asked once. Nothing is asked for any other account. + */ +function useInboxRights(scope: TagScope, enabled: boolean): Partial | null | undefined { const fromStore = useEmailStore((state) => state.mailboxes.find((mailbox) => mailbox.accountId === scope.accountId && mailbox.role === "inbox")?.myRights); const [fetched, setFetched] = useState | null | undefined>(undefined); useEffect(() => { - if (fromStore) return; + if (!enabled || fromStore) return; let cancelled = false; const client = clientForLogin(scope.loginId) as unknown as { getMailboxes?: (accountId?: string) => Promise> } | null; if (!client?.getMailboxes) return; @@ -42,8 +45,8 @@ function useInboxRights(scope: TagScope): Partial | null | undefi .then((mailboxes) => { if (!cancelled) setFetched(mailboxes.find((m) => m.role === "inbox")?.myRights ?? null); }) .catch(() => { if (!cancelled) setFetched(null); }); return () => { cancelled = true; }; - }, [fromStore, scope.loginId, scope.accountId]); - return fromStore ?? fetched; + }, [enabled, fromStore, scope.loginId, scope.accountId]); + return enabled ? fromStore ?? fetched : undefined; } function SharedTagForm({ @@ -59,17 +62,22 @@ function SharedTagForm({ fixedId?: string; takenIds: string[]; saving: boolean; - onSave: (tag: KeywordDefinition) => void; + /** `color` only when it was picked (or the tag is new), so a color this version doesn't know survives a rename. */ + onSave: (id: string, value: { label: string; color?: string }) => void; onCancel: () => void; }) { const t = useTranslations("shared_tags"); const [label, setLabel] = useState(initial?.label ?? ""); const [color, setColor] = useState(initial?.color ?? "blue"); + const [colorPicked, setColorPicked] = useState(false); const id = fixedId ?? composeKeywordId(null, label); const duplicate = !fixedId && id.length > 0 && takenIds.includes(id.toLowerCase()); const tooLong = !fixedId && id.length > MAX_KEYWORD_ID_LENGTH; const valid = label.trim().length > 0 && id.length > 0 && !duplicate && !tooLong; - const save = () => { if (valid && !saving) onSave({ id, label: label.trim(), color }); }; + const keepsColor = !!initial && !colorPicked; + const save = () => { + if (valid && !saving) onSave(id, keepsColor ? { label: label.trim() } : { label: label.trim(), color }); + }; return (
@@ -94,14 +102,14 @@ function SharedTagForm({ {tooLong &&

{t("too_long")}

}
-

{t("colour")}

- +

{t("color")}

+ { setColor(next); setColorPicked(true); }} />
-
@@ -235,10 +272,11 @@ function SharedAccountTagEditor({ scope, name }: { scope: TagScope; name: string
@@ -246,9 +284,9 @@ function SharedAccountTagEditor({ scope, name }: { scope: TagScope; name: string @@ -268,6 +306,9 @@ function SharedAccountTagEditor({ scope, name }: { scope: TagScope; name: string )} {shared.map((tag) => row(tag, false))} {discovered.map((tag) => row(tag, true))} + {(entry?.pending.length ?? 0) > 0 && ( +

{t("save_pending")}

+ )} {locked ? ( ) : !canWrite ? ( @@ -279,14 +320,14 @@ function SharedAccountTagEditor({ scope, name }: { scope: TagScope; name: string takenIds={takenIds} saving={saving} onCancel={() => setEditing(null)} - onSave={async (next) => { if (await save((list) => upsertSharedTag(list, next))) setEditing(null); }} + onSave={async (id, value) => { if (await save({ id, value })) { setEditing(null); setFocusId(id); } }} /> ) : (