From 6be3b9e1297c409efea5e6ebe3f2faf7e28bfba0 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste PENRATH Date: Wed, 6 May 2026 19:39:07 +0200 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(frontend)=20refactor=20threa?= =?UTF-8?q?d=20query=20cache=20management=20(#642)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The thread query is an infinite one and the frontend logic is based on the structuralSharing concept of react-query to optimiscally update the react query cache on thread mutation in order to improve ux. This part is a tricky one and it's easy to introduce regression, that's why refactor it by moving the corresponding logic into a mailbox-cache module, use a better naming (pin instead of optimistic) and battle test it. --- docs/frontend-threads-cache.md | 184 +++++++ src/backend/core/api/openapi.json | 12 +- src/backend/core/api/serializers.py | 13 +- src/backend/core/api/viewsets/label.py | 4 +- .../src/features/api/gen/labels/labels.ts | 2 +- .../src/features/api/gen/models/label.ts | 1 + .../message-importer/index.tsx | 4 +- .../forms/components/message-form/index.tsx | 34 +- .../components/labels-widget/index.tsx | 57 +- .../components/label-form-modal/index.tsx | 4 +- .../components/label-item/index.tsx | 98 ++-- .../components/thread-panel-header.tsx | 18 +- .../thread-accesses-widget/index.tsx | 27 +- .../components/thread-action-bar/index.tsx | 30 +- .../components/thread-message/index.tsx | 18 +- .../thread-message/thread-message-actions.tsx | 7 +- .../src/features/message/use-add-label.tsx | 42 ++ .../src/features/message/use-archive.tsx | 15 +- .../src/features/message/use-delete-label.tsx | 52 ++ .../message/use-delete-thread-access.tsx | 43 ++ .../src/features/message/use-flag.tsx | 22 +- .../src/features/message/use-mention-read.tsx | 3 +- .../src/features/message/use-read.tsx | 89 ++- .../src/features/message/use-spam.tsx | 8 +- .../src/features/message/use-split-thread.tsx | 6 +- .../src/features/message/use-starred.tsx | 29 +- .../src/features/message/use-trash.tsx | 8 +- .../features/providers/mailbox-cache.test.ts | 507 ++++++++++++++++++ .../src/features/providers/mailbox-cache.ts | 182 +++++++ .../src/features/providers/mailbox.test.ts | 32 ++ .../src/features/providers/mailbox.tsx | 451 ++++++---------- .../src/features/providers/sent-box/index.tsx | 4 +- .../ui/components/label-badge/index.tsx | 48 +- 33 files changed, 1533 insertions(+), 521 deletions(-) create mode 100644 docs/frontend-threads-cache.md create mode 100644 src/frontend/src/features/message/use-add-label.tsx create mode 100644 src/frontend/src/features/message/use-delete-label.tsx create mode 100644 src/frontend/src/features/message/use-delete-thread-access.tsx create mode 100644 src/frontend/src/features/providers/mailbox-cache.test.ts create mode 100644 src/frontend/src/features/providers/mailbox-cache.ts create mode 100644 src/frontend/src/features/providers/mailbox.test.ts diff --git a/docs/frontend-threads-cache.md b/docs/frontend-threads-cache.md new file mode 100644 index 00000000..63aeb73c --- /dev/null +++ b/docs/frontend-threads-cache.md @@ -0,0 +1,184 @@ +# Threads list cache — design & mutation contract + +This document explains how the **frontend threads list cache** is managed, +why it pins certain threads, and what every mutation that touches threads +must do to keep the UI consistent. + +It is required reading before adding or refactoring any hook that mutates +threads (read/unread, starred, archived, trashed, spam, draft delete/send, +labels, etc.). + +The relevant code lives in: + +- `src/frontend/src/features/providers/mailbox.tsx` — `MailboxProvider` +- `src/frontend/src/features/providers/mailbox-cache.ts` — pure cache helpers +- `src/frontend/src/features/message/use-*.tsx` — mutation hooks consuming the API + + +## 1. Server state vs. client state + +There is **no global client state** for threads. The list comes from a +React Query infinite query (`useThreadsListInfinite`) keyed per mailbox and +per filter variant. The cache **is** the source of truth on the client; we +do not duplicate it into a Zustand/Redux store. + +Two implications: + +1. Every mutation reconciles by either **patching** the cache directly or + **invalidating** it (which triggers a refetch). +2. The cache is keyed by URL search params (filter, search, label, etc.). + The same thread can live in multiple cache variants simultaneously. + +The query key is built by `getMailboxThreadsListQueryKey(mailboxId, searchParams)`: + +```ts +['threads', , 'list' | 'search', ] +``` + +`'list'` vs `'search'` lets us target the whole search subtree by prefix +without enumerating filter combinations. + + +## 2. Why pinning exists + +Mutations like **mark-as-read** or **toggle-starred** flip a property that +the server uses to filter the list: + +- Mark a thread as read while viewing the **"unread"** filter — the server + will drop it on the next refetch. +- Unstar a thread while viewing the **"starred"** filter — same problem. + +Without protection, the thread would disappear from under the user's cursor +the moment a refetch lands (polling, `invalidateQueries`, window focus). +That is jarring: the user wants to *see* the action they just performed. + +**Pinning** is the protection mechanism: + +1. The mutation calls `pinThreads(ids, patcher)`. This: + - patches the thread(s) in every cached list variant via + `patchThreadsInCache` (so the UI reflects the new state immediately); + - records the thread ids in `pinnedThreadIdsRef` (a `useRef>`). +2. The infinite query's `structuralSharing` callback runs `mergePinnedThreads` + on every refetch. Threads that are pinned but missing from the new server + data are **re-injected** at the index they previously occupied within + their original page. +3. When the server returns a pinned thread on its own, the pin is **inert**: + `mergePinnedThreads` only re-injects threads missing from the response, + so the server's data wins by default. The pin entry stays in the set but + has no effect until the thread disappears again. +4. Pinned ids are cleared in bulk when the user changes mailbox or filter + (a `useEffect` watches `selectedMailbox.id` and `searchParams`). + + +### Pinning rules + +- Pin **only** when the mutation flips a property the server filters on AND + the user should still see the thread under the current view. +- Always pair the pin with a cache patch — pinning a stale thread is worse + than dropping it, because it shows wrong data. +- Re-insertion preserves **per-page semantics**. Pinned threads never move + to page 0 if they originally lived on page 1; otherwise flattening the + pages would yield duplicates. + + +## 3. Why **unpinning** matters + +A pin survives until `unpinThreads` is called or the user changes filter / +mailbox. When a mutation **moves the thread out of the active view** +(archive, spam, trash, draft delete, draft send), the server will *never* +return it under that filter again — so the pin would persist and +`mergePinnedThreads` would re-inject the thread on every subsequent +refetch. The user sees a "ghost" thread that ignores their action. + +Every mutation that intentionally removes a thread from the current view +**must call** `unpinThreads(ids)` before invalidating the list. + +```ts +const { unpinThreads, invalidateMailbox } = useMailboxContext(); + +onSuccess: (data) => { + unpinThreads(data.thread_ids ?? []); + invalidateMailbox(); +} +``` + +Order matters less than presence: as long as the pin is gone before the +refetch settles, `structuralSharing` will let the thread disappear. + + +## 4. Mutation playbook + +Every thread mutation falls into one of three categories. Pick the right +playbook and stick to it. + +### A. "Stay visible" mutations (read/unread, starred/unstarred) + +These flip a server-filterable property but the thread should remain +visible in the current view until the user navigates away. + +```ts +onSuccess: (data) => { + pinThreads(data.thread_ids ?? [], (thread) => ({ + ...thread, + // recompute server-derived flags so the cached row matches reality + has_unread: deriveThreadHasUnread(thread.messaged_at, data.read_at ?? null), + accesses: thread.accesses.map((access) => ...), + })); + invalidateThreadsStats(); +} +``` + +The patcher encodes the **domain semantics** (recomputing `has_unread`, +flipping `has_starred`, mutating `accesses`, etc.) — `mailbox-cache.ts` +does not know about them. + +### B. "Leave the view" mutations (archive, spam, trash, draft delete/send) + +These remove the thread from the active filter. We do not patch — we +unpin and let the next refetch reconcile. + +```ts +onSuccess: (data) => { + unpinThreads(data.thread_ids ?? []); + invalidateMailbox(); + invalidateThreadsStats(); +} +``` + +### C. Per-thread message mutations (draft body, message read, etc.) + +Patch the **per-thread messages cache** (`['messages', threadId]`) via +`patchMessagesInCache` / `removeMessagesFromCache`. These do not touch the +threads list. Invalidate `invalidateThreadMessages()` if a refetch is +warranted. + + +## 5. Invalidation helpers exposed by `MailboxProvider` + +| Helper | When to call it | +|------------------------------|----------------------------------------------------------------------| +| `pinThreads(ids, patcher)` | "Stay visible" mutations (category A) | +| `unpinThreads(ids)` | "Leave the view" mutations (category B), before invalidating | +| `patchMessages(threadId, p)` | Per-thread message mutations (category C) | +| `removeMessages(...)` | Drop messages from a thread's cache (e.g. draft deletion) | +| `invalidateThreadList()` | Force refetch of every list variant of the current mailbox | +| `invalidateThreadMessages()` | Force refetch of the selected thread's messages | +| `invalidateMailbox()` | Shorthand for both above | +| `invalidateThreadEvents()` | Refetch events of the selected thread | +| `invalidateThreadsStats()` | Refetch sidebar counters (excludes per-label stats) | +| `invalidateLabels()` | Refetch the labels list | + + +## 6. Decision flow when adding a new mutation + +1. **Does the mutation flip a property the server filters on?** + - No → category C (messages-only) or just invalidate. + - Yes → step 2. +2. **Should the thread remain in the current view after the mutation?** + - Yes → category A: `pinThreads(ids, patcher)` + invalidate stats only. + - No → category B: `unpinThreads(ids)` + `invalidateMailbox()`. +3. **Are there per-thread side effects (messages, events)?** + - Yes → also patch / invalidate the relevant per-thread caches. + +When in doubt, write a unit test against `mergePinnedThreads` reproducing +the user-visible scenario before changing behaviour. diff --git a/src/backend/core/api/openapi.json b/src/backend/core/api/openapi.json index e7f17dd4..4b036782 100644 --- a/src/backend/core/api/openapi.json +++ b/src/backend/core/api/openapi.json @@ -1860,14 +1860,11 @@ "content": { "application/json": { "schema": { - "type": "array", - "items": { - "$ref": "#/components/schemas/TreeLabel" - } + "$ref": "#/components/schemas/Label" } } }, - "description": "Created labels in hierarchical structure" + "description": "Created label" }, "400": { "content": { @@ -6933,6 +6930,10 @@ "description": "Color of the label in hex format (e.g. #FF0000)", "maxLength": 7 }, + "display_name": { + "type": "string", + "readOnly": true + }, "mailbox": { "type": "string", "format": "uuid", @@ -6958,6 +6959,7 @@ } }, "required": [ + "display_name", "id", "mailbox", "name", diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 028cbcaa..6c79201f 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -539,7 +539,7 @@ class AttachmentSerializer(serializers.ModelSerializer): class ThreadLabelSerializer(serializers.ModelSerializer): """Serializer to get labels details for a thread.""" - display_name = serializers.SerializerMethodField(read_only=True) + display_name = serializers.CharField(source="get_display_name", read_only=True) class Meta: model = models.Label @@ -554,10 +554,6 @@ class ThreadLabelSerializer(serializers.ModelSerializer): ] read_only_fields = ["id", "slug", "display_name"] - def get_display_name(self, instance): - """Return the display name of the label.""" - return instance.name.split("/")[-1] - class TreeLabelSerializer(serializers.ModelSerializer): """Serializer for tree label response structure (OpenAPI purpose only...).""" @@ -566,7 +562,7 @@ class TreeLabelSerializer(serializers.ModelSerializer): name = serializers.CharField(read_only=True) slug = serializers.CharField(read_only=True) color = serializers.CharField(read_only=True) - display_name = serializers.CharField(read_only=True) + display_name = serializers.CharField(source="get_display_name", read_only=True) children = serializers.SerializerMethodField(read_only=True) description = serializers.CharField(read_only=True) is_auto = serializers.BooleanField(read_only=True) @@ -598,6 +594,8 @@ class TreeLabelSerializer(serializers.ModelSerializer): class LabelSerializer(CreateOnlyFieldsMixin, serializers.ModelSerializer): """Serializer for Label model.""" + display_name = serializers.CharField(source="get_display_name", read_only=True) + class Meta: model = models.Label fields = [ @@ -605,12 +603,13 @@ class LabelSerializer(CreateOnlyFieldsMixin, serializers.ModelSerializer): "name", "slug", "color", + "display_name", "mailbox", "threads", "description", "is_auto", ] - read_only_fields = ["id", "slug"] + read_only_fields = ["id", "slug", "display_name"] create_only_fields = ["mailbox"] def validate_mailbox(self, value): diff --git a/src/backend/core/api/viewsets/label.py b/src/backend/core/api/viewsets/label.py index 3d6b0915..0e915294 100644 --- a/src/backend/core/api/viewsets/label.py +++ b/src/backend/core/api/viewsets/label.py @@ -198,8 +198,8 @@ class LabelViewSet( request=serializers.LabelSerializer, responses={ 201: OpenApiResponse( - response=serializers.TreeLabelSerializer(many=True), - description="Created labels in hierarchical structure", + response=serializers.LabelSerializer, + description="Created label", ), 400: OpenApiResponse( response={"detail": "Validation error"}, diff --git a/src/frontend/src/features/api/gen/labels/labels.ts b/src/frontend/src/features/api/gen/labels/labels.ts index bd527cbb..d099970f 100644 --- a/src/frontend/src/features/api/gen/labels/labels.ts +++ b/src/frontend/src/features/api/gen/labels/labels.ts @@ -213,7 +213,7 @@ export function useLabelsList< * View and manage labels */ export type labelsCreateResponse201 = { - data: TreeLabel[]; + data: Label; status: 201; }; diff --git a/src/frontend/src/features/api/gen/models/label.ts b/src/frontend/src/features/api/gen/models/label.ts index e4df4513..d3644f43 100644 --- a/src/frontend/src/features/api/gen/models/label.ts +++ b/src/frontend/src/features/api/gen/models/label.ts @@ -27,6 +27,7 @@ export interface Label { * @maxLength 7 */ color?: string; + readonly display_name: string; /** Mailbox that owns this label */ mailbox: string; /** Threads that have this label */ diff --git a/src/frontend/src/features/controlled-modals/message-importer/index.tsx b/src/frontend/src/features/controlled-modals/message-importer/index.tsx index b18eea63..dfc3b1c7 100644 --- a/src/frontend/src/features/controlled-modals/message-importer/index.tsx +++ b/src/frontend/src/features/controlled-modals/message-importer/index.tsx @@ -23,7 +23,7 @@ export type IMPORT_STEP = 'idle' | 'uploading' | 'importing' | 'completed'; * - completed : Importing completed once the task is SUCCESS */ export const ModalMessageImporter = () => { - const { invalidateThreadMessages, invalidateThreadsStats, invalidateLabels,refetchMailboxes, selectedMailbox } = useMailboxContext(); + const { invalidateMailbox, invalidateThreadsStats, invalidateLabels, refetchMailboxes, selectedMailbox } = useMailboxContext(); const { t } = useTranslation(); const modals = useModals(); const taskImportCacheHelper = new TaskImportCacheHelper(selectedMailbox?.id); @@ -66,7 +66,7 @@ export const ModalMessageImporter = () => { await Promise.all([ refetchMailboxes(), invalidateThreadsStats(), - invalidateThreadMessages(), + invalidateMailbox(), invalidateLabels(), ]); } diff --git a/src/frontend/src/features/forms/components/message-form/index.tsx b/src/frontend/src/features/forms/components/message-form/index.tsx index e8a76bba..ce9350c4 100644 --- a/src/frontend/src/features/forms/components/message-form/index.tsx +++ b/src/frontend/src/features/forms/components/message-form/index.tsx @@ -6,7 +6,7 @@ import { FormProvider, useForm, useWatch } from "react-hook-form"; import { useTranslation } from "react-i18next"; import z from "zod"; import { zodResolver } from "@hookform/resolvers/zod"; -import { Attachment, DraftMessageRequestRequest, Message, sendCreateResponse200, useDraftCreate, useDraftUpdate2, useMessagesDestroy, useSendCreate } from "@/features/api/gen"; +import { Attachment, DraftMessageRequestRequest, draftCreateResponse200, Message, sendCreateResponse200, useDraftCreate, useDraftUpdate2, useMessagesDestroy, useSendCreate } from "@/features/api/gen"; import { MessageComposer, MessageComposerHandle, QuoteType } from "@/features/forms/components/message-composer"; import { useMailboxContext } from "@/features/providers/mailbox"; import MailHelper from "@/features/utils/mail-helper"; @@ -97,7 +97,7 @@ export const MessageForm = ({ const autoSaveTimerRef = useRef(null); const saveDraftRef = useRef<() => void>(() => {}); const quoteType: QuoteType | undefined = mode !== "new" ? (mode === "forward" ? "forward" : "reply") : undefined; - const { selectedMailbox, selectedThread, mailboxes, invalidateThreadMessages, invalidateThreadsStats, unselectThread } = useMailboxContext(); + const { selectedMailbox, selectedThread, mailboxes, removeMessages, invalidateMailbox, invalidateThreadsStats, unselectThread, unpinThreads, pinThreads } = useMailboxContext(); const hideSubjectField = Boolean(draftMessage?.parent_id ?? parentMessage); const defaultSenderId = mailboxes?.find((mailbox) => { if (draft?.sender) return draft.sender.email === mailbox.email; @@ -286,7 +286,21 @@ export const MessageForm = ({ const draftCreateMutation = useDraftCreate({ mutation: { - onSuccess: () => { + onSuccess: (response) => { + const message = (response as draftCreateResponse200).data; + // Patch + pin so a thread that just acquired a draft stays + // accurate even when filtered out of the next refetch (e.g. + // marked-as-read while viewing "unread"): mergePinnedThreads + // would otherwise re-insert the cached version with + // `has_draft: false` and the drafts filter would miss it. + if (message.thread_id) { + pinThreads([message.thread_id], (thread) => ({ + ...thread, + has_draft: true, + draft_messaged_at: message.created_at, + })); + } + invalidateMailbox(); invalidateThreadsStats(); handleDraftMutationSuccess(); } @@ -318,7 +332,14 @@ export const MessageForm = ({ onSuccess: () => { onClose?.(); setDraft(undefined); - invalidateThreadMessages({ type: 'delete', metadata: { ids: [messageId] } }); + if (selectedThread) { + removeMessages(selectedThread.id, [messageId]); + // The thread may exit the active filter (e.g. drafts) once + // its only draft is gone. Drop any pin so the next refetch + // is authoritative. + unpinThreads([selectedThread.id]); + } + invalidateMailbox(); invalidateThreadsStats(); // Unselect the thread if we are in the draft view if (searchParams.get('has_draft') === '1') { @@ -511,6 +532,11 @@ export const MessageForm = ({ const { htmlBody, textBody } = await composerRef.current.exportContent(); stopAutoSave(); + // Send (and "send and archive") moves the thread out of the drafts + // filter — and possibly out of the inbox when archived. Drop the + // pin upfront so the eventual refetch is authoritative. + const draftThreadId = draft?.thread_id ?? selectedThread?.id; + if (draftThreadId) unpinThreads([draftThreadId]); messageMutation.mutate({ data: { messageId, diff --git a/src/frontend/src/features/layouts/components/labels-widget/index.tsx b/src/frontend/src/features/layouts/components/labels-widget/index.tsx index 318e7fa8..ed91c78e 100644 --- a/src/frontend/src/features/layouts/components/labels-widget/index.tsx +++ b/src/frontend/src/features/layouts/components/labels-widget/index.tsx @@ -1,4 +1,4 @@ -import { ThreadLabel, TreeLabel, useLabelsAddThreadsCreate, useLabelsList, useLabelsRemoveThreadsCreate } from "@/features/api/gen"; +import { Label, ThreadLabel, TreeLabel, useLabelsList } from "@/features/api/gen"; import { Thread } from "@/features/api/gen/models"; import { Icon, IconType, Spinner } from "@gouvfr-lasuite/ui-kit"; import { Button, Checkbox, Input, Tooltip } from "@gouvfr-lasuite/cunningham-react"; @@ -10,6 +10,8 @@ import StringHelper from "@/features/utils/string-helper"; import useAbility, { Abilities } from "@/hooks/use-ability"; import { usePopupPosition } from "@/hooks/use-popup-position"; import { LabelModal } from "@/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-form-modal"; +import useDeleteLabel from "@/features/message/use-delete-label"; +import useAddLabel from "@/features/message/use-add-label"; type LabelsWidgetProps = { threadIds: string[]; @@ -23,9 +25,30 @@ type CreateModalState = { initialName: string; } +// Project either a TreeLabel (from the labels list) or a Label (from the +// create endpoint) onto the ThreadLabel shape stored on `Thread.labels`. +const toThreadLabel = (label: TreeLabel | Label): ThreadLabel => ({ + id: label.id, + name: label.name, + slug: label.slug, + color: label.color, + display_name: label.display_name, + description: label.description, + is_auto: label.is_auto, +}); + +const findTreeLabelById = (labels: readonly TreeLabel[], id: string): TreeLabel | undefined => { + for (const label of labels) { + if (label.id === id) return label; + const found = findTreeLabelById(label.children, id); + if (found) return found; + } + return undefined; +}; + export const LabelsWidget = ({ threadIds, initialLabels }: LabelsWidgetProps) => { const { t } = useTranslation(); - const { selectedMailbox, threads, invalidateThreadMessages } = useMailboxContext(); + const { selectedMailbox, threads } = useMailboxContext(); const canManageLabels = useAbility(Abilities.CAN_MANAGE_MAILBOX_LABELS, selectedMailbox); const { data: labelsList, isLoading: isLoadingLabelsList } = useLabelsList( { mailbox_id: selectedMailbox!.id }, @@ -35,24 +58,16 @@ export const LabelsWidget = ({ threadIds, initialLabels }: LabelsWidgetProps) => const [createModal, setCreateModal] = useState({ isOpen: false, initialName: '' }); const anchorRef = useRef(null); - const addLabelMutation = useLabelsAddThreadsCreate({ - mutation: { onSuccess: () => invalidateThreadMessages() } - }); - const deleteLabelMutation = useLabelsRemoveThreadsCreate({ - mutation: { onSuccess: () => invalidateThreadMessages() } - }); + const { addLabel } = useAddLabel(); + const { deleteLabel } = useDeleteLabel(); const handleAddLabel = (labelId: string) => { - addLabelMutation.mutate({ - id: labelId, - data: { thread_ids: threadIds }, - }); + const treeLabel = findTreeLabelById(labelsList?.data ?? [], labelId); + if (!treeLabel) return; + addLabel({ label: toThreadLabel(treeLabel), threadIds }); } - const handleDeleteLabel = (labelId: string) => { - deleteLabelMutation.mutate({ - id: labelId, - data: { thread_ids: threadIds }, - }); + const handleDeleteLabel = (labelId: string, labelSlug: string) => { + deleteLabel({ labelId, labelSlug, threadIds }); } const labelCounts = useMemo(() => { @@ -125,7 +140,7 @@ export const LabelsWidget = ({ threadIds, initialLabels }: LabelsWidgetProps) => isOpen={createModal.isOpen} onClose={() => setCreateModal((s) => ({ ...s, isOpen: false }))} label={{ display_name: createModal.initialName }} - onSuccess={(label) => handleAddLabel(label.id)} + onSuccess={(label) => addLabel({ label: toThreadLabel(label), threadIds })} /> ); @@ -138,7 +153,7 @@ export type LabelsPopupProps = { anchorRef: RefObject; onClose: () => void; onAddLabel: (labelId: string) => void; - onDeleteLabel: (labelId: string) => void; + onDeleteLabel: (labelId: string, labelSlug: string) => void; onCreateLabel: (initialName: string) => void; // Set to false when a modal stacked above should own Escape — otherwise // the popup's capture-phase listener races with the modal's and both close. @@ -148,6 +163,7 @@ export type LabelsPopupProps = { type LabelOption = { label: string; value: string; + slug: string; checked: boolean; indeterminate: boolean; } @@ -198,6 +214,7 @@ export const LabelsPopup = ({ return [{ label: label.name, value: label.id, + slug: label.slug, checked, indeterminate, }, ...children]; @@ -218,7 +235,7 @@ export const LabelsPopup = ({ const handleToggle = (option: LabelOption) => { if (option.checked) { - onDeleteLabel(option.value); + onDeleteLabel(option.value, option.slug); } else { onAddLabel(option.value); } diff --git a/src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-form-modal/index.tsx b/src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-form-modal/index.tsx index 70ec4f21..c9ba5701 100644 --- a/src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-form-modal/index.tsx +++ b/src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-form-modal/index.tsx @@ -19,7 +19,7 @@ export type SubLabelCreation = Partial void; - onSuccess?: (label: TreeLabel) => void; + onSuccess?: (label: Label) => void; label?: TreeLabel | SubLabelCreation } @@ -111,7 +111,7 @@ export const LabelModal = ({ isOpen, onClose, label, onSuccess }: LabelModalProp newSearchParams.set('label_slug', (data.data as Label).slug); router.push(`${pathname}?${newSearchParams.toString()}`); } - onSuccess?.(data.data as TreeLabel); + onSuccess?.(data.data as Label); handleClose(); } }); diff --git a/src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-item/index.tsx b/src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-item/index.tsx index dd21cc69..6c30e106 100644 --- a/src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-item/index.tsx +++ b/src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-labels/components/label-item/index.tsx @@ -1,6 +1,8 @@ -import { TreeLabel, ThreadsStatsRetrieveStatsFields, useLabelsDestroy, useLabelsList, useThreadsStatsRetrieve, ThreadsStatsRetrieve200, useLabelsAddThreadsCreate, useLabelsRemoveThreadsCreate, useLabelsPartialUpdate, useFlagCreate } from "@/features/api/gen"; -import { FlagEnum } from "@/features/api/gen/models"; +import { TreeLabel, ThreadsStatsRetrieveStatsFields, useLabelsDestroy, useLabelsList, useThreadsStatsRetrieve, ThreadsStatsRetrieve200, useLabelsPartialUpdate } from "@/features/api/gen"; import { getThreadsStatsQueryKey, useMailboxContext } from "@/features/providers/mailbox"; +import useArchive from "@/features/message/use-archive"; +import useDeleteLabel from "@/features/message/use-delete-label"; +import useAddLabel from "@/features/message/use-add-label"; import { DropdownMenu, Icon, IconSize, IconType } from "@gouvfr-lasuite/ui-kit"; import { Button, useModals } from "@gouvfr-lasuite/cunningham-react"; import clsx from "clsx"; @@ -32,7 +34,7 @@ type LabelItemProps = TreeLabel & { } export const LabelItem = ({ level = 0, onEdit, canManage, defaultFoldState, ...label }: LabelItemProps) => { - const { selectedMailbox, invalidateThreadMessages, invalidateThreadsStats } = useMailboxContext(); + const { selectedMailbox, invalidateThreadsStats } = useMailboxContext(); const modals = useModals(); const [isDropdownOpen, setIsDropdownOpen] = useState(false); const [isDragOver, setIsDragOver] = useState(false); @@ -62,7 +64,9 @@ export const LabelItem = ({ level = 0, onEdit, canManage, defaultFoldState, ...l const foldTimeoutRef = useRef(null); const shouldAutoArchive = !ViewHelper.isArchivedView() && !ViewHelper.isSpamView() && !ViewHelper.isTrashedView() && !ViewHelper.isDraftsView(); - const { mutate: flagMutate } = useFlagCreate(); + // Suppress the default archive toast: we render our own combined + // "Label assigned + N archived" toast below. + const { markAsArchived, markAsUnarchived } = useArchive({ showToast: false }); const unfoldIfNeeded = useEffectEvent(() => { if (isFolded) { @@ -100,23 +104,18 @@ export const LabelItem = ({ level = 0, onEdit, canManage, defaultFoldState, ...l toggle(); } - const deleteThreadMutation = useLabelsRemoveThreadsCreate({ - mutation: { - onSuccess: (_, variables) => { - invalidateThreadMessages(); - toast.dismiss(JSON.stringify(variables)); - }, - }, - }); - - const addThreadMutation = useLabelsAddThreadsCreate({ - mutation: { - onSuccess: () => { - invalidateThreadMessages(); - invalidateThreadsStats(); - }, - }, - }); + const { deleteLabel } = useDeleteLabel(); + const { addLabel } = useAddLabel(); + // ThreadLabel shape (no `children`) for the local cache patcher in `addLabel`. + const threadLabel = useMemo(() => ({ + id: label.id, + name: label.name, + slug: label.slug, + color: label.color, + display_name: label.display_name, + description: label.description, + is_auto: label.is_auto, + }), [label]); const handleDragStart = (e: React.DragEvent) => { e.dataTransfer.setData('application/json', JSON.stringify({ @@ -178,47 +177,35 @@ export const LabelItem = ({ level = 0, onEdit, canManage, defaultFoldState, ...l const doArchive = !shiftKeyHeld && shouldAutoArchive && transferData.hasEditable === true; const toastId = `label-assign-${label.id}-${Date.now()}`; - addThreadMutation.mutate({ - id: label.id, - data: { thread_ids: threadIds }, - }, { + addLabel({ + label: threadLabel, + threadIds, onSuccess: () => { + invalidateThreadsStats(); if (doArchive) { - flagMutate({ - data: { flag: FlagEnum.archived, value: true, thread_ids: threadIds }, - }, { - onSuccess: (response) => { - invalidateThreadMessages(); - invalidateThreadsStats(); - - // Mirror the `useFlag` toast pattern: label assignment is - // fully successful under the current permission model (we - // relaxed the per-thread edit check; all dragged threads - // belong to the label's mailbox), so partial/none status - // is driven by the archive mutation alone. - const responseData = response.data as Record; - const archivedCount = typeof responseData.updated_threads === 'number' - ? responseData.updated_threads - : threadIds.length; + markAsArchived({ + threadIds, + onSuccess: (_, updatedCount) => { + // Label assignment is fully successful under the current + // permission model (all dragged threads belong to the + // label's mailbox), so partial/none status is driven by + // the archive mutation alone. + const archivedCount = updatedCount ?? threadIds.length; const submittedCount = threadIds.length; const isNone = archivedCount === 0; const isPartial = archivedCount > 0 && archivedCount < submittedCount; const toastType = isNone ? 'error' : isPartial ? 'warning' : 'info'; const undo = () => { - deleteThreadMutation.mutate({ - id: label.id, - data: { thread_ids: threadIds }, + deleteLabel({ + labelId: label.id, + labelSlug: label.slug, + threadIds, }); if (archivedCount > 0) { - flagMutate({ - data: { flag: FlagEnum.archived, value: false, thread_ids: threadIds }, - }, { - onSuccess: () => { - invalidateThreadMessages(); - invalidateThreadsStats(); - toast.dismiss(toastId); - }, + markAsUnarchived({ + threadIds, + onSuccess: () => toast.dismiss(toastId), }); } else { toast.dismiss(toastId); @@ -256,9 +243,10 @@ export const LabelItem = ({ level = 0, onEdit, canManage, defaultFoldState, ...l }); } else { const undo = () => { - deleteThreadMutation.mutate({ - id: label.id, - data: { thread_ids: threadIds }, + deleteLabel({ + labelId: label.id, + labelSlug: label.slug, + threadIds, }); toast.dismiss(toastId); }; diff --git a/src/frontend/src/features/layouts/components/thread-panel/components/thread-panel-header.tsx b/src/frontend/src/features/layouts/components/thread-panel/components/thread-panel-header.tsx index dac06fb1..275d1624 100644 --- a/src/frontend/src/features/layouts/components/thread-panel/components/thread-panel-header.tsx +++ b/src/frontend/src/features/layouts/components/thread-panel/components/thread-panel-header.tsx @@ -194,13 +194,15 @@ const ThreadPanelTitle = ({ selectedThreadIds, isAllSelected, isSomeSelected, is