diff --git a/CHANGELOG.md b/CHANGELOG.md index 5838b28d1..97976bd32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to - Bump keycloak to 26.6.3 - Bump keycloak to 26.6.4 +### Added + +- Expand thread-list rows with a chevron to show per-message summaries (sender, date, snippet, read state) and jump directly to a message from its summary + ### Fixed - πŸ›(mta-out) fix relay block indentation breaking SASL auth #733 diff --git a/src/backend/core/api/openapi.json b/src/backend/core/api/openapi.json index 869ee882d..1b1b0c532 100644 --- a/src/backend/core/api/openapi.json +++ b/src/backend/core/api/openapi.json @@ -10191,6 +10191,10 @@ "type": "integer", "readOnly": true }, + "message_count": { + "type": "integer", + "readOnly": true + }, "abilities": { "type": "object", "additionalProperties": { @@ -10230,6 +10234,7 @@ "is_spam", "is_trashed", "labels", + "message_count", "messaged_at", "messages", "sender_messaged_at", diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 7ea1873d7..207c96532 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -837,6 +837,7 @@ class ThreadSerializer(serializers.ModelSerializer): labels = serializers.SerializerMethodField() summary = serializers.CharField(read_only=True) events_count = serializers.IntegerField(read_only=True) + message_count = serializers.IntegerField(read_only=True) abilities = serializers.SerializerMethodField(read_only=True) assigned_users = serializers.SerializerMethodField(read_only=True) @@ -1019,6 +1020,7 @@ class Meta: "labels", "summary", "events_count", + "message_count", "abilities", "assigned_users", ] @@ -1075,6 +1077,36 @@ class Meta: read_only_fields = fields +class MessageSummarySerializer(serializers.ModelSerializer): + """Lightweight per-message summary for the thread-list expand dropdown. + + Reads only stored fields β€” never Message.get_parsed_data(), which hits + object storage and re-parses MIME on every call. Used behind the + ``?summary=true`` query param on MessageViewSet's list action. + """ + + sender = ContactSerializer(read_only=True) + is_unread = serializers.SerializerMethodField(read_only=True) + + @extend_schema_field(serializers.BooleanField()) + def get_is_unread(self, instance): + """Return the ``_is_unread`` annotation set by ``MessageQuerySet.with_read_state()``.""" + return getattr(instance, "_is_unread", False) + + class Meta: + model = models.Message + fields = [ + "id", + "sender", + "sent_at", + "is_unread", + "is_draft", + "has_attachments", + "snippet", + ] + read_only_fields = fields + + class MessageSerializer(serializers.ModelSerializer): """ Serialize messages, getting parsed details from the Message model. diff --git a/src/backend/core/api/viewsets/message.py b/src/backend/core/api/viewsets/message.py index a92d29a79..0689e6ed1 100644 --- a/src/backend/core/api/viewsets/message.py +++ b/src/backend/core/api/viewsets/message.py @@ -72,6 +72,16 @@ class MessageViewSet( lookup_field = "id" lookup_url_kwarg = "id" + def get_serializer_class(self): + """Use the lightweight summary serializer for ?summary=true list requests. + + Powers the thread-list expand dropdown, which only needs + sender/date/snippet/unread per message β€” never the full body. + """ + if self.action == "list" and self.request.GET.get("summary") == "true": + return serializers.MessageSummarySerializer + return super().get_serializer_class() + def get_queryset(self): """Restrict results to messages in threads accessible by the current user.""" user = self.request.user diff --git a/src/backend/core/api/viewsets/thread.py b/src/backend/core/api/viewsets/thread.py index 902318231..42d65d0c6 100644 --- a/src/backend/core/api/viewsets/thread.py +++ b/src/backend/core/api/viewsets/thread.py @@ -210,6 +210,9 @@ def _annotate_thread_permissions(queryset, user, mailbox_id): ) ), events_count=Count("events", distinct=True), + message_count=Count( + "messages", filter=Q(messages__is_draft=False), distinct=True + ), _can_edit=Exists(can_edit_qs), ).prefetch_related( # Feeds ThreadSerializer.get_assigned_users without N+1. UserEvent diff --git a/src/backend/core/mda/autoreply.py b/src/backend/core/mda/autoreply.py index d3a9d40ba..c03f37307 100644 --- a/src/backend/core/mda/autoreply.py +++ b/src/backend/core/mda/autoreply.py @@ -294,7 +294,7 @@ def send_autoreply_for_message( message.blob = models.Blob.objects.create_blob( content=signed_mime, content_type="message/rfc822" ) - message.save(update_fields=["mime_id", "blob", "has_attachments"]) + message.save(update_fields=["mime_id", "blob", "has_attachments", "snippet"]) # Trigger async send (outside transaction to avoid sending before commit) send_message_task.delay(str(message.id)) diff --git a/src/backend/core/mda/inbound_create.py b/src/backend/core/mda/inbound_create.py index f157f50df..93e97ce75 100644 --- a/src/backend/core/mda/inbound_create.py +++ b/src/backend/core/mda/inbound_create.py @@ -473,6 +473,10 @@ def _create_message_from_inbound( # pylint: disable=too-many-arguments thread=thread, sender=sender_contact, subject=subject, + snippet=thread_snippet( + parsed_email, + fallback=subject or "(No snippet available)", + ), blob=blob, mime_id=first_msgid(parsed_email.get("messageId")) or None, parent=parent_message, diff --git a/src/backend/core/mda/outbound.py b/src/backend/core/mda/outbound.py index 68209fb5a..580503d71 100644 --- a/src/backend/core/mda/outbound.py +++ b/src/backend/core/mda/outbound.py @@ -29,7 +29,7 @@ from core.mda.replies import make_forward, make_reply from core.mda.signing import sign_message_dkim, verify_message_dkim from core.mda.smtp import send_smtp_mail -from core.mda.utils import current_sent_at +from core.mda.utils import current_sent_at, thread_snippet from core.services.blob_gc import schedule_for_gc from core.services.dns.check import check_spf_status from core.services.throttle import check_and_increment_throttle @@ -211,6 +211,13 @@ def compose_and_sign_mime( "messageId": [message.mime_id] if message.mime_id else None, } + # Mutated in memory like mime_id/has_attachments above; the caller + # persists it (see _finalize_sent_message's update_fields). + message.snippet = thread_snippet( + {"textBody": mime_data["textBody"]}, + fallback=message.subject or "", + ) + # Advertise the sending application via X-Mailer (see build_xmailer_value). mime_data["headers"] = [{"name": "X-Mailer", "value": build_xmailer_value()}] @@ -346,6 +353,7 @@ def prepare_outbound_message( # unparseable input (already rejected upstream by the submit view). parsed = parse_email(raw_mime) if parsed is not None: + message.snippet = thread_snippet(parsed, fallback=message.subject or "") if not find_header(parsed, "to"): raw_mime = UNDISCLOSED_RECIPIENTS_TO_HEADER + b"\r\n" + raw_mime # Mirror the composed-body path: advertise the sending application @@ -371,7 +379,9 @@ def prepare_outbound_message( # TODO: Fetch MIME IDs of "references" from the thread # references = message.thread.messages.exclude(id=message.id).order_by("-created_at").all() - # TODO: set the thread snippet? + # Message.snippet is set inside compose_and_sign_mime, from the + # already-in-memory composed body. + # TODO: set the thread-level snippet? # Insert the validated signature validated_signature = mailbox_sender.get_validated_signature( @@ -491,6 +501,7 @@ def _finalize_sent_message( "sender_user", "draft_blob", "created_at", + "snippet", *extra_update_fields, ] message.save(update_fields=update_fields) diff --git a/src/backend/core/migrations/0033_message_snippet.py b/src/backend/core/migrations/0033_message_snippet.py new file mode 100644 index 000000000..0235e2739 --- /dev/null +++ b/src/backend/core/migrations/0033_message_snippet.py @@ -0,0 +1,18 @@ +# Generated by Django 5.2.11 on 2026-07-20 21:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0032_inboundmessage_blob_envelope'), + ] + + operations = [ + migrations.AddField( + model_name='message', + name='snippet', + field=models.TextField(blank=True, verbose_name='snippet'), + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 0ac99c279..e36fb7326 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -2013,6 +2013,7 @@ class Message(BaseModel): Thread, on_delete=models.CASCADE, related_name="messages" ) subject = models.CharField("subject", max_length=255, null=True, blank=True) + snippet = models.TextField("snippet", blank=True) sender = models.ForeignKey("Contact", on_delete=models.CASCADE) sender_user = models.ForeignKey( "User", diff --git a/src/backend/core/tests/api/test_message_summary_serializer.py b/src/backend/core/tests/api/test_message_summary_serializer.py new file mode 100644 index 000000000..2aa4bc030 --- /dev/null +++ b/src/backend/core/tests/api/test_message_summary_serializer.py @@ -0,0 +1,54 @@ +"""Test MessageSummarySerializer.""" + +import pytest + +from core import factories +from core.api.serializers import MessageSummarySerializer + + +@pytest.mark.django_db +class TestMessageSummarySerializer: + """MessageSummarySerializer must expose only the lightweight summary fields.""" + + def test_serializes_expected_fields(self): + """Only id/sender/sent_at/is_unread/has_attachments/snippet are exposed.""" + message = factories.MessageFactory( + snippet="Hello preview", + has_attachments=True, + ) + + data = MessageSummarySerializer(message).data + + assert set(data.keys()) == { + "id", + "sender", + "sent_at", + "is_unread", + "is_draft", + "has_attachments", + "snippet", + } + assert data["id"] == str(message.id) + assert data["snippet"] == "Hello preview" + assert data["has_attachments"] is True + assert data["sender"]["email"] == message.sender.email + + def test_is_unread_defaults_false_without_annotation(self): + """Falls back to False when the queryset wasn't annotated with _is_unread.""" + message = factories.MessageFactory() + + data = MessageSummarySerializer(message).data + + assert data["is_unread"] is False + + def test_does_not_touch_blob_storage(self, monkeypatch): + """Serializing must never call get_parsed_data() (blob fetch + MIME parse).""" + message = factories.MessageFactory(snippet="Already computed") + + def fail_if_called(*args, **kwargs): + raise AssertionError("MessageSummarySerializer must not parse the blob") + + monkeypatch.setattr(type(message), "get_parsed_data", fail_if_called) + + serialized = MessageSummarySerializer(message).data # must not raise + assert serialized["snippet"] == "Already computed" diff --git a/src/backend/core/tests/api/test_messages_list.py b/src/backend/core/tests/api/test_messages_list.py index fe0e2424d..a7b0eddf7 100644 --- a/src/backend/core/tests/api/test_messages_list.py +++ b/src/backend/core/tests/api/test_messages_list.py @@ -664,3 +664,62 @@ def test_is_unread_defaults_to_false_without_mailbox(self): assert response.status_code == status.HTTP_200_OK messages = {str(m["id"]): m for m in response.data} assert messages[str(msg.id)]["is_unread"] is False + + +@pytest.mark.django_db +class TestMessageListSummaryParam: + """?summary=true on the messages list endpoint returns the lightweight shape.""" + + def test_summary_true_returns_summary_fields_only(self, api_client): + """With summary=true, the response is the lightweight MessageSummarySerializer shape.""" + user = factories.UserFactory() + mailbox = factories.MailboxFactory() + factories.MailboxAccessFactory( + mailbox=mailbox, user=user, role=enums.MailboxRoleChoices.EDITOR + ) + thread = factories.ThreadFactory() + factories.ThreadAccessFactory( + mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR + ) + message = factories.MessageFactory(thread=thread, snippet="Preview text") + api_client.force_authenticate(user=user) + + response = api_client.get( + reverse("messages-list"), + {"thread_id": str(thread.id), "summary": "true"}, + ) + + assert response.status_code == status.HTTP_200_OK + payload = next(m for m in response.data if m["id"] == str(message.id)) + assert set(payload.keys()) == { + "id", + "sender", + "sent_at", + "is_unread", + "is_draft", + "has_attachments", + "snippet", + } + assert payload["snippet"] == "Preview text" + + def test_without_summary_param_returns_full_message(self, api_client): + """Without summary=true, behavior is unchanged (full MessageSerializer).""" + user = factories.UserFactory() + mailbox = factories.MailboxFactory() + factories.MailboxAccessFactory( + mailbox=mailbox, user=user, role=enums.MailboxRoleChoices.EDITOR + ) + thread = factories.ThreadFactory() + factories.ThreadAccessFactory( + mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR + ) + message = factories.MessageFactory(thread=thread) + api_client.force_authenticate(user=user) + + response = api_client.get( + reverse("messages-list"), {"thread_id": str(thread.id)} + ) + + assert response.status_code == status.HTTP_200_OK + payload = next(m for m in response.data if m["id"] == str(message.id)) + assert "textBody" in payload diff --git a/src/backend/core/tests/api/test_threads_list.py b/src/backend/core/tests/api/test_threads_list.py index b1148c2b2..93d67cfc3 100644 --- a/src/backend/core/tests/api/test_threads_list.py +++ b/src/backend/core/tests/api/test_threads_list.py @@ -2020,3 +2020,107 @@ def test_labels_and_assignees_prefetched(self, api_client, url): f"(N+1 regression on prefetch chain?), " f"got 1β†’{queries_1} vs 5β†’{queries_5}" ) + + +class TestThreadListMessageCount: + """Test that ThreadSerializer exposes message_count on the list endpoint. + + message_count drives the chevron that expands a thread's per-message + summary list. It counts non-draft messages only. + """ + + @pytest.fixture + def url(self): + """Return the URL for the list endpoint.""" + return reverse("threads-list") + + @staticmethod + def _setup_user_with_thread(user=None): + """Create a user with an admin mailbox and an editor thread access.""" + user = user or UserFactory() + mailbox = MailboxFactory() + MailboxAccessFactory( + mailbox=mailbox, + user=user, + role=enums.MailboxRoleChoices.ADMIN, + ) + thread = ThreadFactory() + ThreadAccessFactory( + mailbox=mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + return user, mailbox, thread + + def test_list_threads_message_count_zero_when_no_messages(self, api_client, url): + """A thread without any message should expose message_count == 0.""" + user, mailbox, thread = self._setup_user_with_thread() + api_client.force_authenticate(user=user) + + response = api_client.get(url, {"mailbox_id": str(mailbox.id)}) + + assert response.status_code == status.HTTP_200_OK + payload = next(t for t in response.data["results"] if t["id"] == str(thread.id)) + assert payload["message_count"] == 0 + + def test_list_threads_message_count_matches_messages(self, api_client, url): + """message_count should equal the number of non-draft messages.""" + user, mailbox, thread = self._setup_user_with_thread() + api_client.force_authenticate(user=user) + + MessageFactory(thread=thread) + MessageFactory(thread=thread) + MessageFactory(thread=thread) + + response = api_client.get(url, {"mailbox_id": str(mailbox.id)}) + + assert response.status_code == status.HTTP_200_OK + payload = next(t for t in response.data["results"] if t["id"] == str(thread.id)) + assert payload["message_count"] == 3 + + def test_list_threads_message_count_excludes_drafts(self, api_client, url): + """Drafts must not be counted in message_count.""" + user, mailbox, thread = self._setup_user_with_thread() + api_client.force_authenticate(user=user) + + MessageFactory(thread=thread) + MessageFactory(thread=thread, is_draft=True) + MessageFactory(thread=thread, is_draft=True) + + response = api_client.get(url, {"mailbox_id": str(mailbox.id)}) + + assert response.status_code == status.HTTP_200_OK + payload = next(t for t in response.data["results"] if t["id"] == str(thread.id)) + assert payload["message_count"] == 1 + + def test_list_threads_message_count_distinct_per_thread(self, api_client, url): + """message_count must use distinct counting to avoid JOIN multiplication. + + The queryset joins on accesses__mailbox, so without ``distinct=True`` the + count would be multiplied by the number of ThreadAccess rows. Guard against + regressions by creating several accesses and expecting the raw message count. + """ + user, mailbox, thread = self._setup_user_with_thread() + api_client.force_authenticate(user=user) + + for _ in range(2): + extra_mailbox = MailboxFactory() + MailboxAccessFactory( + mailbox=extra_mailbox, + user=user, + role=enums.MailboxRoleChoices.ADMIN, + ) + ThreadAccessFactory( + mailbox=extra_mailbox, + thread=thread, + role=enums.ThreadAccessRoleChoices.EDITOR, + ) + + MessageFactory(thread=thread) + MessageFactory(thread=thread) + + response = api_client.get(url, {"mailbox_id": str(mailbox.id)}) + + assert response.status_code == status.HTTP_200_OK + payload = next(t for t in response.data["results"] if t["id"] == str(thread.id)) + assert payload["message_count"] == 2 diff --git a/src/backend/core/tests/mda/test_autoreply.py b/src/backend/core/tests/mda/test_autoreply.py index 0e975f349..e938f1ca7 100644 --- a/src/backend/core/tests/mda/test_autoreply.py +++ b/src/backend/core/tests/mda/test_autoreply.py @@ -1061,3 +1061,18 @@ def test_does_not_update_sender_read_at( access.refresh_from_db() assert access.read_at is None + + @patch("core.mda.outbound_tasks.send_message_task", new_callable=MagicMock) + @patch("core.mda.outbound.sign_message_dkim", return_value=None) + def test_snippet_persisted( + self, mock_dkim, mock_send_task, mailbox, autoreply_template, inbound_message + ): + """Snippet computed by compose_and_sign_mime is persisted to the database.""" + send_autoreply_for_message(autoreply_template, mailbox, inbound_message) + + autoreply_msg = models.Message.objects.filter( + parent=inbound_message, is_sender=True + ).last() + # Reload from DB to verify persisted value + autoreply_msg.refresh_from_db() + assert autoreply_msg.snippet == "I am out of office." diff --git a/src/backend/core/tests/mda/test_inbound.py b/src/backend/core/tests/mda/test_inbound.py index 370268497..0a0e2211a 100644 --- a/src/backend/core/tests/mda/test_inbound.py +++ b/src/backend/core/tests/mda/test_inbound.py @@ -330,6 +330,7 @@ def test_basic_delivery_new_thread( assert message.sender.mailbox == target_mailbox assert message.blob.get_content() == raw_email_data assert message.mime_id == sample_parsed_email["messageId"][0] + assert message.snippet == "Test body content." # Inbound message from another sender: thread should be unread assert access.read_at is None diff --git a/src/backend/core/tests/mda/test_outbound.py b/src/backend/core/tests/mda/test_outbound.py index 6305db4c1..896fcf73b 100644 --- a/src/backend/core/tests/mda/test_outbound.py +++ b/src/backend/core/tests/mda/test_outbound.py @@ -915,6 +915,73 @@ def test_prepare_outbound_message_updates_sender_read_at(self, mailbox_sender): assert access.read_at >= message.created_at +@pytest.mark.django_db +class TestMessageSnippetOnSend: + """``prepare_outbound_message`` must persist a ``Message.snippet`` + derived from the content actually sent, for both the composed-body + and raw-MIME paths.""" + + def test_composed_body_send_sets_snippet( + self, user, mailbox_sender, mailbox_access + ): + """The composed text body becomes the persisted snippet.""" + message = factories.MessageFactory( + thread=factories.ThreadFactory(), + sender=factories.ContactFactory(mailbox=mailbox_sender), + is_draft=True, + subject="Test Message", + signature=None, + ) + factories.MessageRecipientFactory( + message=message, + contact=factories.ContactFactory( + mailbox=mailbox_sender, email="to@example.com" + ), + type=models.MessageRecipientTypeChoices.TO, + ) + text_body = "This is the composed body content used for the snippet test." + + outbound.prepare_outbound_message( + mailbox_sender, message, text_body, "

irrelevant html

", user + ) + + message.refresh_from_db() + assert message.snippet != "" + assert message.snippet.startswith(text_body[:20]) + + def test_raw_mime_send_sets_snippet(self, user, mailbox_sender, mailbox_access): + """A caller-supplied raw MIME body also produces a persisted snippet.""" + message = factories.MessageFactory( + thread=factories.ThreadFactory(), + sender=factories.ContactFactory(mailbox=mailbox_sender), + is_draft=True, + subject="Test Message", + signature=None, + ) + factories.MessageRecipientFactory( + message=message, + contact=factories.ContactFactory( + mailbox=mailbox_sender, email="to@example.com" + ), + type=models.MessageRecipientTypeChoices.TO, + ) + raw_mime = ( + b"From: sender@example.com\r\n" + b"To: to@example.com\r\n" + b"Subject: Raw MIME snippet test\r\n" + b"\r\n" + b"This is the raw MIME body used for the snippet test.\r\n" + ) + + outbound.prepare_outbound_message( + mailbox_sender, message, "", "", user, raw_mime=raw_mime + ) + + message.refresh_from_db() + assert message.snippet != "" + assert "This is the raw MIME body" in message.snippet + + @pytest.mark.django_db class TestUndisclosedRecipientsHeader: """A message with no To recipient (e.g. Bcc-only) must get an empty-group diff --git a/src/frontend/src/features/api/gen/models/thread.ts b/src/frontend/src/features/api/gen/models/thread.ts index 41dde4b62..4a4661e67 100644 --- a/src/frontend/src/features/api/gen/models/thread.ts +++ b/src/frontend/src/features/api/gen/models/thread.ts @@ -74,6 +74,7 @@ export interface Thread { readonly labels: readonly ThreadLabel[]; readonly summary: string; readonly events_count: number; + readonly message_count: number; readonly abilities: ThreadAbilities; readonly assigned_users: readonly ThreadEventUser[]; } diff --git a/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/_index.scss b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/_index.scss index 8a91e5ead..345ea5735 100644 --- a/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/_index.scss +++ b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/_index.scss @@ -1,3 +1,50 @@ +// Wraps the disclosure chevron and the thread .thread-item as +// siblings (interactive content cannot be nested inside an β€” see the +// same constraint documented on mailbox-list's .mailbox__item-row). +.thread-item-row { + display: flex; + // flex-wrap so the expanded summaries container (below) can be + // pushed onto its own line instead of squeezing in beside the + // chevron/Link as a third flex item. + flex-wrap: wrap; + align-items: flex-start; + + .thread-item { + flex: 1; + min-width: 0; + } +} + +// Container for ThreadItemMessageSummaries β€” forces it onto its own row +// below the chevron/Link pair (see flex-wrap above), spanning the full +// width of the thread-list row. +.thread-item-row > [id^="thread-item-summaries-"] { + flex-basis: 100%; + width: 100%; +} + +// Native )} - data-unread={hasUnread} - draggable - onDragStart={handleDragStart} - onDragEnd={handleDragEnd} - onClick={handleItemClick} - onFocus={onFocusItem} - tabIndex={tabIndex} - ref={itemRef} - role="option" - aria-selected={isSelected} - > -
+ +
{showCheckbox && ( // Mouse-only affordance, purely presentational: the option // itself carries the selection state (aria-selected) and a @@ -319,7 +341,16 @@ export const ThreadItem = ({ thread, isSelected, onToggle, onSelectRange, select )}
- + + {isExpanded && ( +
+ +
+ )} + {(isDragging || isExiting) && dragPreviewContainer.current && createPortal( { + if (typeof globalThis.localStorage === "undefined") { + const store = new Map() + globalThis.localStorage = { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => store.set(key, value), + removeItem: (key: string) => store.delete(key), + clear: () => store.clear(), + key: (index: number) => Array.from(store.keys())[index] ?? null, + get length() { + return store.size + }, + } as Storage + } +}) + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string) => key, + i18n: { resolvedLanguage: "en" }, + }), + // features/i18n/initI18n calls i18n.use(initReactI18next) at import + // time (transitively) β€” it just needs to exist as a valid i18next + // plugin shape, its behaviour is irrelevant to this test. + initReactI18next: { type: "3rdParty", init: () => {} }, +})) + +vi.mock("@/features/utils/date-helper", () => ({ + DateHelper: { formatDate: (date: string) => date }, +})) + +vi.mock("@tanstack/react-router", () => ({ + Link: ({ children, ...rest }: React.AnchorHTMLAttributes) => ( + {children} + ), +})) + +vi.mock("@gouvfr-lasuite/cunningham-react", () => ({ + Button: ({ children, ...rest }: React.ButtonHTMLAttributes) => ( + + ), +})) + +vi.mock("@gouvfr-lasuite/ui-kit", () => ({ + Icon: ({ name, ...rest }: { name: string } & React.HTMLAttributes) => ( + + ), + IconType: { OUTLINED: "outlined", FILLED: "filled" }, + Spinner: (props: React.HTMLAttributes) => , +})) + +vi.mock("./use-message-summaries") + +const mockUseMessageSummaries = (overrides: { + data?: MessageSummary[] + isLoading?: boolean + isError?: boolean + refetch?: () => void +}) => { + vi.mocked(useMessageSummaries).mockReturnValue({ + data: overrides.data, + isLoading: overrides.isLoading ?? false, + isError: overrides.isError ?? false, + refetch: overrides.refetch ?? vi.fn(), + } as unknown as ReturnType) +} + +const renderSummaries = () => { + const container = document.createElement("div") + document.body.appendChild(container) + const root = createRoot(container) + + act(() => { + root.render() + }) + + return { + container, + cleanup: () => { + act(() => root.unmount()) + container.remove() + }, + } +} + +describe("ThreadItemMessageSummaries", () => { + it("shows a loading state while summaries are fetching", () => { + mockUseMessageSummaries({ isLoading: true, data: undefined }) + const { container, cleanup } = renderSummaries() + + expect(container.querySelector('[role="status"]')).not.toBeNull() + + cleanup() + }) + + it("shows an inline error with a retry button on failure", () => { + const refetch = vi.fn() + mockUseMessageSummaries({ isError: true, refetch }) + const { container, cleanup } = renderSummaries() + + const button = container.querySelector("button") as HTMLButtonElement + expect(button).not.toBeNull() + act(() => { + button.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true })) + }) + + expect(refetch).toHaveBeenCalledTimes(1) + + cleanup() + }) + + it("renders one row per message with sender, date and snippet", () => { + mockUseMessageSummaries({ + data: [ + { + id: "m1", + sender: { id: "s1", name: "Alice", email: "a@x.com" }, + sent_at: "2026-07-20T10:00:00Z", + is_unread: true, + is_draft: false, + has_attachments: false, + snippet: "Hello there", + }, + ], + }) + const { container, cleanup } = renderSummaries() + + expect(container.textContent).toContain("Alice") + expect(container.textContent).toContain("Hello there") + + cleanup() + }) + + it("visually distinguishes a draft summary row", () => { + mockUseMessageSummaries({ + data: [ + { + id: "m1", + sender: { id: "s1", name: "Alice", email: "a@x.com" }, + sent_at: null, + is_unread: false, + is_draft: true, + has_attachments: false, + snippet: "", + }, + ], + }) + const { container, cleanup } = renderSummaries() + + expect(container.textContent).toContain("Draft") + expect(container.querySelector(".thread-item__summary-link--draft")).not.toBeNull() + + cleanup() + }) +}) diff --git a/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/thread-item-message-summaries.tsx b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/thread-item-message-summaries.tsx new file mode 100644 index 000000000..564817d44 --- /dev/null +++ b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/thread-item-message-summaries.tsx @@ -0,0 +1,82 @@ +import { useTranslation } from "react-i18next" +import { Link } from "@tanstack/react-router" +import clsx from "clsx" +import { Icon, IconType, Spinner } from "@gouvfr-lasuite/ui-kit" +import { Button } from "@gouvfr-lasuite/cunningham-react" +import { DateHelper } from "@/features/utils/date-helper" +import { useMessageSummaries } from "./use-message-summaries" + +type ThreadItemMessageSummariesProps = { + threadId: string + mailboxId: string +} + +/** + * Renders the per-message summary rows shown when a thread-list row is + * expanded (Task 8's chevron). Sender, date, snippet, unread weight and + * attachment icon per message; drafts are visually distinguished but not + * yet wired to open the composer on click (see Task 10 brief) β€” they + * navigate like any other row, landing on the draft's position in the + * thread view. + */ +export const ThreadItemMessageSummaries = ({ threadId, mailboxId }: ThreadItemMessageSummariesProps) => { + const { t, i18n } = useTranslation() + const { data, isLoading, isError, refetch } = useMessageSummaries(threadId, mailboxId, { enabled: true }) + + if (isLoading) { + return ( +
+ +
+ ) + } + + if (isError) { + return ( +
+ {t("Could not load messages.")} + +
+ ) + } + + return ( +
    + {(data ?? []).map((message) => ( +
  • + + {message.sender.name} + {message.is_draft && ( + {t("Draft")} + )} + {message.sent_at && ( + + {DateHelper.formatDate(message.sent_at, i18n.resolvedLanguage)} + + )} + {message.has_attachments && ( +
    • + ))} +
+ ) +} diff --git a/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/types.ts b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/types.ts new file mode 100644 index 000000000..19498a2d4 --- /dev/null +++ b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/types.ts @@ -0,0 +1,13 @@ +export type MessageSummary = { + id: string; + sender: { + id: string; + name: string; + email: string; + }; + sent_at: string | null; + is_unread: boolean; + is_draft: boolean; + has_attachments: boolean; + snippet: string; +}; diff --git a/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/use-message-summaries.test.tsx b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/use-message-summaries.test.tsx new file mode 100644 index 000000000..ee757a49f --- /dev/null +++ b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/use-message-summaries.test.tsx @@ -0,0 +1,147 @@ +import { beforeEach, describe, expect, it, vi } from "vitest" +import { createRoot } from "react-dom/client" +import { act } from "react" +import { QueryClient, QueryClientProvider } from "@tanstack/react-query" +import { fetchAPI } from "@/features/api/fetch-api" +import { useMessageSummaries } from "./use-message-summaries" + +// This repo has no @testing-library/react (confirmed absent from +// package.json/node_modules), so there is no renderHook/waitFor. Following +// the same pattern as thread-item/index.test.tsx: mount a tiny host +// component with createRoot + act and read the hook's state back off the +// DOM, polling with act(async) + a short delay instead of waitFor. +;(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +// vi.hoisted runs before every import in this file (unlike a plain +// top-level statement, which would run after use-message-summaries' own +// imports β€” including features/api/fetch-api -> features/api/api-error -> +// features/i18n/initI18n β€” have already executed and crashed on a missing +// localStorage). Node's global localStorage is experimental and not +// enabled in this project's vitest run, so polyfill it (same fix as +// thread-item/index.test.tsx). +vi.hoisted(() => { + if (typeof globalThis.localStorage === "undefined") { + const store = new Map() + globalThis.localStorage = { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => store.set(key, value), + removeItem: (key: string) => store.delete(key), + clear: () => store.clear(), + key: (index: number) => Array.from(store.keys())[index] ?? null, + get length() { + return store.size + }, + } as Storage + } +}) + +vi.mock("@/features/api/fetch-api") + +const Host = ({ + threadId, + mailboxId, + enabled, +}: { + threadId: string + mailboxId: string + enabled: boolean +}) => { + const query = useMessageSummaries(threadId, mailboxId, { enabled }) + return ( +
+ ) +} + +const renderHost = (props: { threadId: string; mailboxId: string; enabled: boolean }) => { + const container = document.createElement("div") + document.body.appendChild(container) + const root = createRoot(container) + const queryClient = new QueryClient() + + act(() => { + root.render( + + + + ) + }) + + return { + container, + cleanup: () => { + act(() => root.unmount()) + container.remove() + }, + } +} + +const waitUntil = async (check: () => boolean, timeoutMs = 2000) => { + const start = Date.now() + while (!check()) { + if (Date.now() - start > timeoutMs) { + throw new Error("waitUntil: condition not met within timeout") + } + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 5)) + }) + } +} + +describe("useMessageSummaries", () => { + // vitest doesn't clear mocks between tests by default here (no + // clearMocks in vitest.config.ts), so the first test's call count would + // otherwise leak into the second. + beforeEach(() => { + vi.clearAllMocks() + }) + + it("fetches summaries for the given thread with summary=true and mailbox_id", async () => { + const mocked = vi.mocked(fetchAPI) + mocked.mockResolvedValue({ + data: [ + { + id: "m1", + sender: { id: "s1", name: "Alice", email: "a@example.com" }, + sent_at: "2026-07-20T10:00:00Z", + is_unread: false, + is_draft: false, + has_attachments: false, + snippet: "Hi", + }, + ], + status: 200, + } as never) + + const { container, cleanup } = renderHost({ + threadId: "thread-1", + mailboxId: "mb-1", + enabled: true, + }) + + await waitUntil(() => container.querySelector('[data-testid="host"]')?.getAttribute("data-status") === "success") + + const host = container.querySelector('[data-testid="host"]') + expect(host?.getAttribute("data-snippet")).toBe("Hi") + expect(mocked).toHaveBeenCalledWith( + expect.stringContaining("/messages/"), + expect.objectContaining({ + params: { thread_id: "thread-1", mailbox_id: "mb-1", summary: "true" }, + }) + ) + + cleanup() + }) + + it("does not fetch when enabled is false", () => { + const mocked = vi.mocked(fetchAPI) + const { cleanup } = renderHost({ threadId: "thread-1", mailboxId: "mb-1", enabled: false }) + + expect(mocked).not.toHaveBeenCalled() + + cleanup() + }) +}) diff --git a/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/use-message-summaries.ts b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/use-message-summaries.ts new file mode 100644 index 000000000..37eca6920 --- /dev/null +++ b/src/frontend/src/features/layouts/components/thread-panel/components/thread-item/use-message-summaries.ts @@ -0,0 +1,36 @@ +import { useQuery } from "@tanstack/react-query"; +import { fetchAPI } from "@/features/api/fetch-api"; +import { MessageSummary } from "./types"; + +type MessagesSummaryResponse = { + data: MessageSummary[]; + status: 200; +}; + +/** + * Fetches the lightweight per-message summaries for a thread (sender, date, + * snippet, unread state) β€” used by the thread-list expand dropdown. + * Hand-written rather than via the generated Orval client: ?summary=true + * returns a different shape than the full Message type the generated + * useMessagesList hook is typed for. + * + * mailboxId is required, not optional: MessageSummarySerializer.is_unread + * reads an annotation the backend only applies when mailbox_id is present + * on the request β€” omitting it would silently make every summary read as + * "read" (is_unread always false) with no error. + */ +export const useMessageSummaries = ( + threadId: string, + mailboxId: string, + { enabled }: { enabled: boolean } +) => { + return useQuery({ + queryKey: ["messages", "summary", threadId, mailboxId], + queryFn: () => + fetchAPI("/api/v1.0/messages/", { + params: { thread_id: threadId, mailbox_id: mailboxId, summary: "true" }, + }), + select: (response) => response.data, + enabled, + }); +}; diff --git a/src/frontend/src/features/layouts/components/thread-panel/index.tsx b/src/frontend/src/features/layouts/components/thread-panel/index.tsx index f48104dd3..6cee24e1d 100644 --- a/src/frontend/src/features/layouts/components/thread-panel/index.tsx +++ b/src/frontend/src/features/layouts/components/thread-panel/index.tsx @@ -4,7 +4,7 @@ import { ThreadItem } from "./components/thread-item"; import { Spinner } from "@gouvfr-lasuite/ui-kit"; import { useTranslation } from "react-i18next"; import { Button } from "@gouvfr-lasuite/cunningham-react"; -import { useEffect, useRef, useCallback } from "react"; +import { useEffect, useRef, useCallback, useState } from "react"; import { useUrlSearchParams } from "@/hooks/use-url-search-params"; import ThreadPanelHeader from "./components/thread-panel-header"; import { useThreadSelection } from "@/features/providers/thread-selection"; @@ -40,6 +40,23 @@ export const ThreadPanel = () => { const { getItemProps, onKeyDown: handleListboxKeyDown, onBlur: handleListboxBlur } = useThreadListbox(threads?.results); + // In-memory only, by design: expansion state resets on reload but + // survives SPA navigation (unlike mailbox-list's folder-expand state, + // which persists to localStorage β€” a deliberate product decision not + // to persist this one). + const [expandedThreadIds, setExpandedThreadIds] = useState>(new Set()); + const toggleThreadExpand = useCallback((threadId: string) => { + setExpandedThreadIds((prev) => { + const next = new Set(prev); + if (next.has(threadId)) { + next.delete(threadId); + } else { + next.add(threadId); + } + return next; + }); + }, []); + const handleObserver = useCallback((entries: IntersectionObserverEntry[]) => { const target = entries[0]; if (target.isIntersecting && threads?.next && !queryStates.threads.isFetchingNextPage) { @@ -137,6 +154,8 @@ export const ThreadPanel = () => { onSelectRange={selectRange} selectedThreadIds={selectedThreadIds} isSelectionMode={isSelectionMode} + isExpanded={expandedThreadIds.has(thread.id)} + onToggleExpand={() => toggleThreadExpand(thread.id)} {...getItemProps(thread.id)} /> ))} diff --git a/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.test.tsx b/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.test.tsx new file mode 100644 index 000000000..33bc66908 --- /dev/null +++ b/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.test.tsx @@ -0,0 +1,243 @@ +import { describe, expect, it, vi, beforeEach } from "vitest" +import { createRoot, Root } from "react-dom/client" +import { act, useEffect as useEffectReal, useState as useStateReal } from "react" +import type React from "react" +import { Message } from "@/features/api/gen/models" + +// This repo has no @testing-library/react; createRoot + act (both from +// React/ReactDOM, already a dependency) is the lightest way to mount a +// component tree and observe real DOM mutations driven by effects. Follows +// the same mocking approach as thread-view/index.test.tsx and +// thread-panel/components/thread-item/index.test.tsx. +;(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +vi.hoisted(() => { + if (typeof globalThis.localStorage === "undefined") { + const store = new Map() + globalThis.localStorage = { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => store.set(key, value), + removeItem: (key: string) => store.delete(key), + clear: () => store.clear(), + key: (index: number) => Array.from(store.keys())[index] ?? null, + get length() { + return store.size + }, + } as Storage + } +}) + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string) => key, + i18n: { resolvedLanguage: "en" }, + }), + initReactI18next: { type: "3rdParty", init: () => {} }, +})) + +vi.mock("@gouvfr-lasuite/ui-kit", () => ({ + Icon: ({ name, ...rest }: { name: string } & React.HTMLAttributes) => ( + + ), + IconType: { OUTLINED: "outlined", FILLED: "filled" }, + Spinner: () =>
, +})) + +vi.mock("@/features/providers/config", () => ({ + useConfig: () => ({ MESSAGES_MANUAL_RETRY_MAX_AGE: 3600 }), +})) + +vi.mock("@/features/ui/components/banner", () => ({ + Banner: ({ children }: React.PropsWithChildren) =>
{children}
, +})) + +vi.mock("@/features/api/gen/messages/messages", () => ({ + useMessagesDeliveryStatusesPartialUpdate: () => ({ mutate: vi.fn() }), +})) + +vi.mock("@/hooks/use-ability", () => ({ + default: () => true, + Abilities: { CAN_SEND_MESSAGES: "CAN_SEND_MESSAGES", CAN_EDIT_THREAD: "CAN_EDIT_THREAD", CAN_MANAGE_THREAD_DELIVERY_STATUSES: "CAN_MANAGE_THREAD_DELIVERY_STATUSES" }, +})) + +vi.mock("@/features/utils/mail-helper", () => ({ + default: { + extractDriveAttachmentsFromHtmlBody: (content: string) => [content, []], + extractDriveAttachmentsFromTextBody: (content: string) => [content], + }, +})) + +// The real ThreadMessageBody signals readiness via `onLoad` once its iframe +// content has rendered. That readiness (not just `isFolded`) gates the +// `thread-message--folded` class, so the stub must fire `onLoad` on mount to +// keep that class meaningful for these tests. +vi.mock("./thread-message-body", () => ({ + default: function ThreadMessageBodyStub({ onLoad }: { onLoad?: () => void }) { + useEffectReal(() => { + onLoad?.() + }, [onLoad]) + return
+ }, +})) + +vi.mock("../message-reply-form", () => ({ + default: () =>
, +})) + +vi.mock("./thread-message-header", () => ({ + default: () =>
, +})) + +vi.mock("./thread-message-footer", () => ({ + default: () =>
, +})) + +vi.mock("../calendar-invite", () => ({ + CalendarInvite: () =>
, +})) + +let mailboxContextValue: Record = {} +vi.mock("@/features/providers/mailbox", () => ({ + useMailboxContext: () => mailboxContextValue, +})) + +// Reactive hash: tests drive it via `setLocationHash`, matching the +// `useLocation({ select })` pattern already used elsewhere in this codebase +// (e.g. mailbox.tsx, label-badge/index.tsx). +let currentHash = "" +const locationListeners = new Set<() => void>() +const setLocationHash = (hash: string) => { + currentHash = hash + locationListeners.forEach((listener) => listener()) +} + +vi.mock("@tanstack/react-router", () => ({ + useLocation: ({ select }: { select: (location: { hash: string }) => T }) => { + const [, forceRender] = useStateReal(0) + useEffectReal(() => { + const listener = () => forceRender((n) => n + 1) + locationListeners.add(listener) + return () => { + locationListeners.delete(listener) + } + }, []) + return select({ hash: currentHash }) + }, +})) + +const { ThreadMessage } = await import("./index") +const { default: ThreadViewProvider } = await import("../../provider") + +const buildMessage = (overrides: Partial = {}): Message => ({ + id: "m1", + subject: "Hello", + sender: { name: "", email: "" } as Message["sender"], + to: [], + cc: [], + bcc: [], + attachments: [], + htmlBody: [], + textBody: [], + is_unread: false, + is_sender: false, + is_draft: false, + is_trashed: false, + is_archived: false, + is_starred: false, + created_at: "2026-07-20T00:00:00Z", + updated_at: "2026-07-20T00:00:00Z", + parent_id: null, + thread_id: "thread-1", + ...overrides, +} as Message) + +type RenderOptions = { + hash?: string + message: Message + isLatest?: boolean +} + +const renderThreadMessage = ({ hash = "", message, isLatest = false }: RenderOptions) => { + currentHash = hash + mailboxContextValue = { + selectedMailbox: { id: "mailbox-1", email: "mailbox@example.com" }, + selectedThread: { id: "thread-1", is_spam: false }, + queryStates: { messages: { isFetching: false } }, + invalidateMailbox: vi.fn(), + invalidateThreadsStats: vi.fn(), + } + + const container = document.createElement("div") + document.body.appendChild(container) + let root: Root + act(() => { + root = createRoot(container) + root.render( + + + + ) + }) + + return { + container, + rerender: () => { + act(() => { + root.render( + + + + ) + }) + }, + cleanup: () => { + act(() => root.unmount()) + container.remove() + }, + } +} + +describe("ThreadMessage β€” hash-driven unfold", () => { + beforeEach(() => { + currentHash = "" + locationListeners.clear() + }) + + it("unfolds when the initial hash targets this message", () => { + // Not latest, read, no draft => starts folded by default. + const message = buildMessage({ id: "m1", is_unread: false }) + const { container, cleanup } = renderThreadMessage({ hash: "#thread-message-m1", message, isLatest: false }) + + const section = container.querySelector("#thread-message-m1") + expect(section).not.toBeNull() + expect(section?.classList.contains("thread-message--folded")).toBe(false) + + cleanup() + }) + + it("stays folded when the hash does not target this message", () => { + const message = buildMessage({ id: "m1", is_unread: false }) + const { container, cleanup } = renderThreadMessage({ hash: "", message, isLatest: false }) + + const section = container.querySelector("#thread-message-m1") + expect(section?.classList.contains("thread-message--folded")).toBe(true) + + cleanup() + }) + + it("unfolds when the hash changes to target this message after mount, without remounting", () => { + const message = buildMessage({ id: "m1", is_unread: false }) + const { container, rerender, cleanup } = renderThreadMessage({ hash: "", message, isLatest: false }) + + expect(container.querySelector("#thread-message-m1")?.classList.contains("thread-message--folded")).toBe(true) + + act(() => { + setLocationHash("#thread-message-m1") + }) + rerender() + + expect(container.querySelector("#thread-message-m1")?.classList.contains("thread-message--folded")).toBe(false) + + cleanup() + }) +}) diff --git a/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.tsx b/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.tsx index c5bcfec39..b353d16e1 100644 --- a/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.tsx +++ b/src/frontend/src/features/layouts/components/thread-view/components/thread-message/index.tsx @@ -1,4 +1,5 @@ import { useState, useCallback, forwardRef, useEffect, useRef, useMemo } from "react"; +import { useLocation } from "@tanstack/react-router"; import clsx from "clsx"; import { useTranslation } from "react-i18next"; import { Icon, IconType, Spinner } from "@gouvfr-lasuite/ui-kit"; @@ -142,18 +143,20 @@ export const ThreadMessage = forwardRef( // lives in the compose form), so it is ready to render immediately. const [isThreadMessageBodyLoaded, setIsThreadMessageBodyLoaded] = useState(isMessageReady || message.is_draft); const [isFolded, setIsFolded] = useState(!isLatest && !message.is_unread && !draftMessage?.is_draft); + const hash = useLocation({ select: (l) => l.hash }); // Deep-link target: unfold this message when the URL hash points to // it so the user lands on its expanded content instead of the folded - // preview. Runs once on mount because the hash is read at navigation - // time and the user can still re-fold manually afterwards. + // preview. Re-runs whenever the hash changes (not just on mount) so + // that clicking a different message's summary row of an already-open + // thread β€” which only changes the hash, without remounting this + // component β€” still unfolds the newly targeted message. The user can + // still re-fold manually afterwards. useEffect(() => { - if (typeof window === 'undefined') return; - if (window.location.hash === `#thread-message-${message.id}`) { + if (hash === `#thread-message-${message.id}`) { setIsFolded(false); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [hash, message.id]); const [replyFormMode, setReplyFormMode] = useState(() => { if (draftMessage?.is_draft) return 'reply'; if (!message.is_draft || message.is_trashed) return null; diff --git a/src/frontend/src/features/layouts/components/thread-view/index.test.tsx b/src/frontend/src/features/layouts/components/thread-view/index.test.tsx new file mode 100644 index 000000000..3d751a38d --- /dev/null +++ b/src/frontend/src/features/layouts/components/thread-view/index.test.tsx @@ -0,0 +1,393 @@ +import { describe, expect, it, vi, beforeEach } from "vitest" +import { createRoot, Root } from "react-dom/client" +import { act, useEffect as useEffectReal, useState as useStateReal } from "react" +import type React from "react" +import { MailboxRoleChoices, Message, Thread } from "@/features/api/gen/models" + +// This repo has no @testing-library/react; createRoot + act (both from +// React/ReactDOM, already a dependency) is the lightest way to mount a +// component tree and observe real DOM mutations driven by effects. +;(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +vi.hoisted(() => { + if (typeof globalThis.localStorage === "undefined") { + const store = new Map() + globalThis.localStorage = { + getItem: (key: string) => store.get(key) ?? null, + setItem: (key: string, value: string) => store.set(key, value), + removeItem: (key: string) => store.delete(key), + clear: () => store.clear(), + key: (index: number) => Array.from(store.keys())[index] ?? null, + get length() { + return store.size + }, + } as Storage + } +}) + +// ThreadView pulls in a large amount of surrounding infrastructure (mailbox +// context, i18n, feature flags, visibility observers, sub-components with +// their own heavy dependency trees). None of that matters for the +// hash-driven scroll/highlight effect under test, so every collaborator is +// stubbed to its minimal observable shape, following the same approach as +// thread-panel/components/thread-item/index.test.tsx. + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string) => key, + i18n: { resolvedLanguage: "en" }, + }), + initReactI18next: { type: "3rdParty", init: () => {} }, +})) + +vi.mock("@/features/utils/date-helper", () => ({ + DateHelper: { formatDate: () => "" }, +})) + +vi.mock("@/features/utils/view-helper", () => ({ + default: { isTrashedView: () => false }, +})) + +vi.mock("@/hooks/use-feature", () => ({ + FEATURE_KEYS: { AI_SUMMARY: "ai_summary" }, + useFeatureFlag: () => false, +})) + +vi.mock("@/features/message/use-read", () => ({ + default: () => ({ markAsReadAt: vi.fn() }), +})) + +vi.mock("@/features/message/use-mention-read", () => ({ + default: () => ({ markMentionsRead: vi.fn() }), +})) + +vi.mock("@/features/message/use-spam", () => ({ + default: () => ({ markAsNotSpam: vi.fn() }), +})) + +vi.mock("@/hooks/use-debounce-callback", () => ({ + useDebounceCallback: (fn: (...args: unknown[]) => void) => fn, +})) + +vi.mock("@/hooks/use-is-shared-context", () => ({ + useIsSharedContext: () => false, +})) + +vi.mock("@/hooks/use-visibility-observer", () => ({ + useVisibilityObserver: () => {}, +})) + +vi.mock("@/features/ui/components/banner", () => ({ + Banner: ({ children }: React.PropsWithChildren) =>
{children}
, +})) + +vi.mock("@gouvfr-lasuite/ui-kit", () => ({ + Icon: ({ name, ...rest }: { name: string } & React.HTMLAttributes) => ( + + ), + IconType: { OUTLINED: "outlined", FILLED: "filled" }, + Spinner: () =>
, +})) + +vi.mock("./components/thread-action-bar", () => ({ + ThreadActionBar: () =>
, +})) + +vi.mock("./components/thread-view-labels-list", () => ({ + ThreadViewLabelsList: () =>
, +})) + +vi.mock("./components/thread-summary", () => ({ + ThreadSummary: () =>
, +})) + +vi.mock("./components/thread-view-empty", () => ({ + ThreadViewEmpty: () =>
, +})) + +vi.mock("./components/thread-event-input", () => ({ + ThreadEventInput: () =>
, +})) + +vi.mock("./components/thread-event", () => ({ + ThreadEvent: () =>
, + CollapsedEventsGroup: () =>
, + isCondensed: () => false, + // Real implementation just re-shapes TimelineItem[] into render items; + // a message-only identity mapping is enough for these tests. + groupSystemEvents: (items: { type: string; data: unknown }[]) => + items.map((item) => + item.type === "event" ? { kind: "event", data: item.data } : { kind: "message", data: item.data } + ), +})) + +// The real ThreadMessage does a lot (delivery status, body rendering, +// reply forms…) that is irrelevant here β€” what matters is that it renders +// a DOM node with the right id, and reports readiness so the surrounding +// `isReady` gate (from the real ThreadViewProvider, used unmocked below) +// flips true, exactly like the real component does once its body loads. +vi.mock("./components/thread-message", async () => { + const React = await import("react") + const { useEffect } = React + const { useThreadViewContext } = await import("./provider") + const ThreadMessage = React.forwardRef( + ({ message }, ref) => { + const { setMessageReadiness } = useThreadViewContext() + // `setMessageReadiness` is a plain inline function on the real + // ThreadViewProvider (not memoized), so it gets a new identity + // every render β€” including it here would re-run this effect on + // every readiness update and loop forever. + useEffect(() => { + setMessageReadiness(message.id, true) + }, [message.id]) + return
+ } + ) + return { ThreadMessage } +}) + +// Reactive hash: tests drive it via `setLocationHash`, matching the +// `useLocation({ select })` pattern already used elsewhere in this codebase +// (e.g. mailbox.tsx, label-badge/index.tsx). +let currentHash = "" +const locationListeners = new Set<() => void>() +const setLocationHash = (hash: string) => { + currentHash = hash + // Some effects in this file (e.g. the trashed-message deep-link + // redirect, out of scope for this fix) still read `window.location.hash` + // directly rather than through the reactive `useLocation` hook, so keep + // the real jsdom location in sync too. + window.location.hash = hash + locationListeners.forEach((listener) => listener()) +} + +vi.mock("@tanstack/react-router", () => ({ + useLocation: ({ select }: { select: (location: { hash: string }) => T }) => { + const [, forceRender] = useStateReal(0) + useEffectReal(() => { + const listener = () => forceRender((n) => n + 1) + locationListeners.add(listener) + return () => { + locationListeners.delete(listener) + } + }, []) + return select({ hash: currentHash }) + }, +})) + +let mailboxContextValue: Record = {} +vi.mock("@/features/providers/mailbox", () => ({ + useMailboxContext: () => mailboxContextValue, + isThreadEvent: (item: { type: string } | null) => item?.type === "event", +})) + +// Imported after all vi.mock calls above so the mocked modules are in place +// before ThreadView's own module graph resolves. +const { ThreadView } = await import("./index") + +const buildMessage = (overrides: Partial = {}): Message => ({ + id: "m1", + subject: "Hello", + sender: { name: "", email: "" } as Message["sender"], + to: [], + cc: [], + bcc: [], + attachments: [], + htmlBody: [], + textBody: [], + is_unread: false, + is_sender: false, + is_draft: false, + is_trashed: false, + is_archived: false, + is_starred: false, + created_at: "2026-07-20T00:00:00Z", + updated_at: "2026-07-20T00:00:00Z", + parent_id: null, + thread_id: "thread-1", + ...overrides, +} as Message) + +const buildThread = (overrides: Partial = {}): Thread => ({ + id: "thread-1", + subject: "Hello", + snippet: "", + messages: "", + has_unread: false, + has_unread_mention: false, + has_trashed: false, + is_trashed: false, + has_archived: false, + has_draft: false, + has_starred: false, + has_attachments: false, + has_sender: true, + has_messages: true, + has_delivery_failed: false, + has_delivery_pending: false, + is_spam: false, + has_active: true, + messaged_at: null, + active_messaged_at: null, + draft_messaged_at: null, + sender_messaged_at: null, + archived_messaged_at: null, + trashed_messaged_at: null, + sender_names: [], + updated_at: "2026-07-20T00:00:00Z", + user_role: null as unknown as Thread["user_role"], + accesses: [], + labels: [], + summary: "", + events_count: 0, + message_count: 1, + abilities: {} as Thread["abilities"], + assigned_users: [], + ...overrides, +} as Thread) + +type RenderThreadViewOptions = { + hash?: string + messages: Message[] +} + +const renderThreadView = ({ hash = "", messages }: RenderThreadViewOptions) => { + currentHash = hash + window.location.hash = hash + const thread = buildThread() + mailboxContextValue = { + selectedMailbox: { id: "mailbox-1", role: MailboxRoleChoices.editor }, + selectedThread: thread, + unmountThreadViewNeeded: false, + messages, + threadItems: messages.map((m) => ({ type: "message", data: m, created_at: m.created_at })), + queryStates: { + messages: { isLoading: false }, + threadEvents: { isLoading: false }, + }, + } + + const container = document.createElement("div") + document.body.appendChild(container) + let root: Root + act(() => { + root = createRoot(container) + root.render() + }) + + return { + container, + rerender: () => { + act(() => { + root.render() + }) + }, + cleanup: () => { + act(() => root.unmount()) + container.remove() + }, + } +} + +// The hashMatch path defers scroll+highlight past a double rAF, then waits +// for either a real `scrollend` (unavailable in jsdom) or a 700ms safety +// timeout before applying the highlight class. Flushing real time (rather +// than faking it) keeps this aligned with the actual rAF/setTimeout +// scheduling used by the effect. +const flushHashEffect = async () => { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 800)) + }) +} + +describe("ThreadView β€” hash-driven scroll/highlight", () => { + beforeEach(() => { + currentHash = "" + locationListeners.clear() + }) + + it("scrolls to and highlights the message targeted by the initial hash", async () => { + const messages = [buildMessage({ id: "m1" }), buildMessage({ id: "m2" })] + const { container, cleanup } = renderThreadView({ hash: "#thread-message-m1", messages }) + await flushHashEffect() + + const target = container.querySelector("#thread-message-m1") + expect(target).not.toBeNull() + expect(target?.classList.contains("thread-view__highlight")).toBe(true) + + cleanup() + }) + + it("scrolls to a different message when the hash changes while the same thread stays open", async () => { + const messages = [buildMessage({ id: "m1" }), buildMessage({ id: "m2" })] + const { container, rerender, cleanup } = renderThreadView({ hash: "#thread-message-m1", messages }) + await flushHashEffect() + + expect(container.querySelector("#thread-message-m1")?.classList.contains("thread-view__highlight")).toBe(true) + + act(() => { + setLocationHash("#thread-message-m2") + }) + rerender() + await flushHashEffect() + + expect(container.querySelector("#thread-message-m2")?.classList.contains("thread-view__highlight")).toBe(true) + + cleanup() + }) + + it("does not re-run the once-only draft/unread/latest fallback after a later, unrelated hash change", () => { + // No hash match at all: falls back to the "latest message" heuristic, + // which scrolls the thread body via `rootRef.current.scrollTo(...)`. + // That's the actual observable side effect the once-only guard + // protects β€” asserting on it (rather than on the highlight class, + // which this no-hash path never sets: `highlightTargetId` is only + // assigned on a real hash match) is what lets this test fail if the + // guard is ever removed and the fallback re-runs on every render. + // + // jsdom never lays out elements, so `offsetTop` is always 0 and the + // effect's "am I already scrolled there" check would otherwise make + // `scrollTo` a permanent no-op regardless of the guard. Stub + // `offsetTop` so the fallback scroll actually has somewhere to go, + // and stub the (jsdom-unimplemented) `scrollTo` so calls can be + // counted instead of throwing. + const offsetTopDescriptor = Object.getOwnPropertyDescriptor(HTMLElement.prototype, "offsetTop") + Object.defineProperty(HTMLElement.prototype, "offsetTop", { + configurable: true, + get: () => 1000, + }) + const scrollTo = vi.fn() + Element.prototype.scrollTo = scrollTo + + try { + const messages = [ + buildMessage({ id: "m1", created_at: "2026-07-20T00:00:00Z" }), + buildMessage({ id: "m2", created_at: "2026-07-20T01:00:00Z" }), + ] + const { rerender, cleanup } = renderThreadView({ hash: "", messages }) + + // No hashMatch, no drafts, no unread => falls back to latestMessage + // (m2), scrolling to it exactly once. + expect(scrollTo).toHaveBeenCalledTimes(1) + + act(() => { + setLocationHash("#not-a-thread-hash") + }) + rerender() + + // hasBeenInitialized is already true and the new hash doesn't + // match the deep-link pattern, so the guard blocks the fallback + // from running again β€” no second scroll call. + expect(scrollTo).toHaveBeenCalledTimes(1) + + cleanup() + } finally { + if (offsetTopDescriptor) { + Object.defineProperty(HTMLElement.prototype, "offsetTop", offsetTopDescriptor) + } else { + delete (HTMLElement.prototype as unknown as Record).offsetTop + } + delete (Element.prototype as unknown as Record).scrollTo + } + }) +}) diff --git a/src/frontend/src/features/layouts/components/thread-view/index.tsx b/src/frontend/src/features/layouts/components/thread-view/index.tsx index 7b036bed7..043c3a4c7 100755 --- a/src/frontend/src/features/layouts/components/thread-view/index.tsx +++ b/src/frontend/src/features/layouts/components/thread-view/index.tsx @@ -1,4 +1,5 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react" +import { useLocation } from "@tanstack/react-router"; import { FEATURE_KEYS, useFeatureFlag } from "@/hooks/use-feature"; import { ThreadActionBar } from "./components/thread-action-bar" import { ThreadMessage } from "./components/thread-message" @@ -58,6 +59,7 @@ const ThreadViewComponent = ({ threadItems, mailboxId, thread, showTrashedMessag const rootRef = useRef(null); const isAISummaryEnabled = useFeatureFlag(FEATURE_KEYS.AI_SUMMARY); const { isReady, reset, hasBeenInitialized, setHasBeenInitialized } = useThreadViewContext(); + const hash = useLocation({ select: (l) => l.hash }); // Refs for all unread messages const unreadRefs = useRef>({}); // Refs for thread events with unread mentions @@ -200,108 +202,114 @@ const ThreadViewComponent = ({ threadItems, mailboxId, thread, showTrashedMessag }); useEffect(() => { - if (isReady && !hasBeenInitialized) { - // Deep-link hash takes precedence over draft/unread heuristics: - // a user arriving via a shared "#thread-message-{id}" or - // "#thread-event-{id}" URL expects to land on that exact element, - // scrolled smoothly into view with a brief highlight. - const hash = typeof window !== 'undefined' ? window.location.hash : ''; - const hashMatch = hash.match(/^#thread-(message|event)-([\w-]+)$/); - let selector = `#thread-message-${latestMessage?.id}`; - let scrollBehavior: ScrollBehavior = 'instant'; - let highlightTargetId: string | null = null; + const hashMatch = hash.match(/^#thread-(message|event)-([\w-]+)$/); - if (hashMatch) { - selector = hash; - scrollBehavior = 'smooth'; - highlightTargetId = hash.slice(1); - } else if (draftMessageIds.length > 0) { - selector = `#thread-message-${draftMessageIds[0]} > .thread-message__reply-form`; - } else { - const firstUnreadItem = threadItems.find((item) => { - if (item.type === 'message') { - return (item.data as MessageWithDraftChild).is_unread; - } - return (item.data as ThreadEventModel).has_unread_mention === true; - }); - if (firstUnreadItem) { - selector = firstUnreadItem.type === 'message' - ? `#thread-message-${firstUnreadItem.data.id}` - : `#thread-event-${firstUnreadItem.data.id}`; + // A hash-targeted jump must run every time the hash changes, even + // if this thread was already initialized (e.g. clicking a + // different message summary of the same open thread from the + // list). The draft/unread/latest fallback heuristics, by + // contrast, are meant to run only once per thread mount. + if (!isReady) return; + if (!hashMatch && hasBeenInitialized) return; + + // Deep-link hash takes precedence over draft/unread heuristics: + // a user arriving via a shared "#thread-message-{id}" or + // "#thread-event-{id}" URL expects to land on that exact element, + // scrolled smoothly into view with a brief highlight. + let selector = `#thread-message-${latestMessage?.id}`; + let scrollBehavior: ScrollBehavior = 'instant'; + let highlightTargetId: string | null = null; + + if (hashMatch) { + selector = hash; + scrollBehavior = 'smooth'; + highlightTargetId = hash.slice(1); + } else if (draftMessageIds.length > 0) { + selector = `#thread-message-${draftMessageIds[0]} > .thread-message__reply-form`; + } else { + const firstUnreadItem = threadItems.find((item) => { + if (item.type === 'message') { + return (item.data as MessageWithDraftChild).is_unread; } + return (item.data as ThreadEventModel).has_unread_mention === true; + }); + if (firstUnreadItem) { + selector = firstUnreadItem.type === 'message' + ? `#thread-message-${firstUnreadItem.data.id}` + : `#thread-event-${firstUnreadItem.data.id}`; } + } - const el = document.querySelector(selector); - if (el) { - const performScroll = (): boolean => { - const refreshed = document.querySelector(selector); - const target = refreshed ?? el; - const root = rootRef.current; - if (!root) return false; - const targetTop = Math.max(target.offsetTop - 225, 0); - const willScroll = Math.abs(targetTop - root.scrollTop) > 1; - if (willScroll) { - root.scrollTo({ top: targetTop, behavior: scrollBehavior }); - } - return willScroll; - }; + const el = document.querySelector(selector); + if (el) { + const performScroll = (): boolean => { + const refreshed = document.querySelector(selector); + const target = refreshed ?? el; + const root = rootRef.current; + if (!root) return false; + const targetTop = Math.max(target.offsetTop - 225, 0); + const willScroll = Math.abs(targetTop - root.scrollTop) > 1; + if (willScroll) { + root.scrollTo({ top: targetTop, behavior: scrollBehavior }); + } + return willScroll; + }; - const startHighlight = () => { - if (!highlightTargetId) return; - const highlightTarget = document.getElementById(highlightTargetId); - if (!highlightTarget) return; - const messagesList = rootRef.current?.querySelector('.thread-view__messages-list'); - highlightTarget.classList.add('thread-view__highlight'); - messagesList?.classList.add('thread-view__messages-list--focusing'); + const startHighlight = () => { + if (!highlightTargetId) return; + const highlightTarget = document.getElementById(highlightTargetId); + if (!highlightTarget) return; + const messagesList = rootRef.current?.querySelector('.thread-view__messages-list'); + highlightTarget.classList.add('thread-view__highlight'); + messagesList?.classList.add('thread-view__messages-list--focusing'); - // Sync cleanup to the CSS animation. The timeout - // guards prefers-reduced-motion, where no animation fires. - let cleaned = false; - const cleanup = () => { - if (cleaned) return; - cleaned = true; - highlightTarget.classList.remove('thread-view__highlight'); - messagesList?.classList.remove('thread-view__messages-list--focusing'); - highlightTarget.removeEventListener('animationend', cleanup); - }; - highlightTarget.addEventListener('animationend', cleanup); - setTimeout(cleanup, 2200); + // Sync cleanup to the CSS animation. The timeout + // guards prefers-reduced-motion, where no animation fires. + let cleaned = false; + const cleanup = () => { + if (cleaned) return; + cleaned = true; + highlightTarget.classList.remove('thread-view__highlight'); + messagesList?.classList.remove('thread-view__messages-list--focusing'); + highlightTarget.removeEventListener('animationend', cleanup); }; + highlightTarget.addEventListener('animationend', cleanup); + setTimeout(cleanup, 2200); + }; - // Defer the highlight until the smooth scroll has actually - // landed. `scrollend` doesn't emit when no scroll happens - // nor on older browsers, so the 700ms safety acts as a fallback. - const scheduleHighlight = (willScroll: boolean) => { - const root = rootRef.current; - if (!root || scrollBehavior !== 'smooth' || !willScroll) { - startHighlight(); - return; - } - let fired = false; - const fire = () => { - if (fired) return; - fired = true; - root.removeEventListener('scrollend', fire); - clearTimeout(safety); - startHighlight(); - }; - root.addEventListener('scrollend', fire); - const safety = setTimeout(fire, 700); + // Defer the highlight until the smooth scroll has actually + // landed. `scrollend` doesn't emit when no scroll happens + // nor on older browsers, so the 700ms safety acts as a fallback. + const scheduleHighlight = (willScroll: boolean) => { + const root = rootRef.current; + if (!root || scrollBehavior !== 'smooth' || !willScroll) { + startHighlight(); + return; + } + let fired = false; + const fire = () => { + if (fired) return; + fired = true; + root.removeEventListener('scrollend', fire); + clearTimeout(safety); + startHighlight(); }; + root.addEventListener('scrollend', fire); + const safety = setTimeout(fire, 700); + }; - if (hashMatch) { - requestAnimationFrame(() => requestAnimationFrame(() => { - const willScroll = performScroll(); - scheduleHighlight(willScroll); - })); - } else { + if (hashMatch) { + requestAnimationFrame(() => requestAnimationFrame(() => { const willScroll = performScroll(); scheduleHighlight(willScroll); - } - setHasBeenInitialized(true); + })); + } else { + const willScroll = performScroll(); + scheduleHighlight(willScroll); } + setHasBeenInitialized(true); } - }, [isReady]); + }, [isReady, hash]); const handleEventDelete = useCallback((eventId: string) => { if (editingEvent?.id === eventId) {