From 35260fe7f83fb63d7163f89607d04534f01cf3db Mon Sep 17 00:00:00 2001 From: jbpenrath Date: Tue, 23 Jun 2026 19:25:32 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B(backend)=20set=20attachment=20name?= =?UTF-8?q?=20fallback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The openapi schema specifies that an attachment has always a name but we have some case where this attribute is None. To guarantee this contract, we use a fallback value `unnamed` when name is None during serialization. Also fix other issues of the same kind identified. --- src/backend/core/api/openapi.json | 30 +++++++++++-- src/backend/core/api/serializers.py | 44 ++++++++++++++++--- .../test_message_attachments_serializer.py | 42 ++++++++++++++++++ .../core/tests/swagger/test_openapi_schema.py | 26 +++++++++++ .../src/features/api/gen/models/index.ts | 5 +++ .../api/gen/models/mail_domain_admin.ts | 6 ++- ..._domain_admin_expected_dns_records_item.ts | 13 ++++++ .../src/features/api/gen/models/mailbox.ts | 5 ++- .../features/api/gen/models/mailbox_admin.ts | 5 ++- .../api/gen/models/mailbox_admin_contact.ts | 13 ++++++ .../api/gen/models/mailbox_admin_create.ts | 5 ++- .../models/mailbox_admin_create_contact.ts | 13 ++++++ .../features/api/gen/models/mailbox_role.ts | 13 ++++++ .../src/features/api/gen/models/thread.ts | 5 ++- .../api/gen/models/thread_user_role.ts | 13 ++++++ .../src/features/providers/mailbox.tsx | 2 + 16 files changed, 222 insertions(+), 18 deletions(-) create mode 100644 src/backend/core/tests/api/test_message_attachments_serializer.py create mode 100644 src/frontend/src/features/api/gen/models/mail_domain_admin_expected_dns_records_item.ts create mode 100644 src/frontend/src/features/api/gen/models/mailbox_admin_contact.ts create mode 100644 src/frontend/src/features/api/gen/models/mailbox_admin_create_contact.ts create mode 100644 src/frontend/src/features/api/gen/models/mailbox_role.ts create mode 100644 src/frontend/src/features/api/gen/models/thread_user_role.ts diff --git a/src/backend/core/api/openapi.json b/src/backend/core/api/openapi.json index 6a6a0c20..8be7b3b1 100644 --- a/src/backend/core/api/openapi.json +++ b/src/backend/core/api/openapi.json @@ -7928,7 +7928,27 @@ "description": "date and time at which a record was last updated" }, "expected_dns_records": { - "type": "string", + "type": "array", + "nullable": true, + "items": { + "type": "object", + "properties": { + "target": { + "type": "string" + }, + "type": { + "type": "string" + }, + "value": { + "type": "string" + } + }, + "required": [ + "target", + "type", + "value" + ] + }, "readOnly": true }, "mailbox_count": { @@ -8111,6 +8131,7 @@ "$ref": "#/components/schemas/MailboxRoleChoices" } ], + "nullable": true, "readOnly": true }, "count_unread_threads": { @@ -8421,7 +8442,8 @@ "$ref": "#/components/schemas/Contact" } ], - "readOnly": true + "readOnly": true, + "nullable": true }, "last_accessed_at": { "type": "string", @@ -8511,7 +8533,8 @@ "$ref": "#/components/schemas/Contact" } ], - "readOnly": true + "readOnly": true, + "nullable": true }, "last_accessed_at": { "type": "string", @@ -9975,6 +9998,7 @@ "$ref": "#/components/schemas/ThreadAccessRoleChoices" } ], + "nullable": true, "readOnly": true }, "accesses": { diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index f00c1ad2..a9f5754d 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -220,6 +220,23 @@ class IntegerChoicesField(serializers.ChoiceField): super().fail(key, **kwargs) +def nullable_choices_schema(choices_class): + """Nullable enum schema for a ``SerializerMethodField`` that may return + ``None``. + + Reuse the shared ``{choices_class.__name__}`` component (registered by the + non-null :class:`IntegerChoicesField` usages) and apply the nullability + locally via the ``allOf`` wrapper. Baking ``nullable`` into the component + itself would collide with those non-null usages; an inline enum dict would + instead make drf-spectacular extract a redundant ``…Enum`` component. This + mirrors the shape drf-spectacular emits natively for a nullable ``$ref``. + """ + return { + "allOf": [{"$ref": f"#/components/schemas/{choices_class.__name__}"}], + "nullable": True, + } + + class AbilitiesModelSerializer(serializers.ModelSerializer): """ A ModelSerializer that takes an additional `exclude` argument that @@ -380,7 +397,7 @@ class MailboxSerializer(AbilitiesModelSerializer): return instance.contact.name return None - @extend_schema_field(IntegerChoicesField(choices_class=models.MailboxRoleChoices)) + @extend_schema_field(nullable_choices_schema(models.MailboxRoleChoices)) def get_role(self, instance): """Return the allowed actions of the logged-in user on the instance.""" # Use the annotated user_role field @@ -890,9 +907,7 @@ class ThreadSerializer(serializers.ModelSerializer): cached = instance.messages.order_by("created_at") return [str(message.id) for message in cached] - @extend_schema_field( - IntegerChoicesField(choices_class=models.ThreadAccessRoleChoices) - ) + @extend_schema_field(nullable_choices_schema(models.ThreadAccessRoleChoices)) def get_user_role(self, instance): """Get current user's role for this thread, scoped to the context mailbox. @@ -1155,7 +1170,7 @@ class MessageSerializer(serializers.ModelSerializer): stripped_attachments.append( { "blobId": f"msg_{instance.id}_{index}", - "name": attachment["name"], + "name": attachment.get("name") or "unnamed", "size": attachment["size"], "type": attachment["type"], "cid": attachment.get("cid"), @@ -1444,6 +1459,21 @@ class MailDomainAdminSerializer(AbilitiesModelSerializer): """Return the abilities for the mail domain.""" return super().get_abilities(instance) + @extend_schema_field( + { + "type": "array", + "nullable": True, + "items": { + "type": "object", + "properties": { + "target": {"type": "string"}, + "type": {"type": "string"}, + "value": {"type": "string"}, + }, + "required": ["target", "type", "value"], + }, + } + ) def get_expected_dns_records(self, instance): """Return the expected DNS records for the mail domain, only in detail views.""" @@ -1552,7 +1582,9 @@ class MailboxAdminSerializer(serializers.ModelSerializer): many=True, read_only=True ) # accesses is the related_name can_reset_password = serializers.BooleanField(read_only=True) - contact = ContactSerializer(read_only=True) + # ``Mailbox.contact`` is ``SET_NULL, null=True`` — an alias mailbox (or one + # whose contact was deleted) has none, so the nested field must be nullable. + contact = ContactSerializer(read_only=True, allow_null=True) alias_of = serializers.PrimaryKeyRelatedField( required=False, allow_null=True, queryset=models.Mailbox.objects.none() ) diff --git a/src/backend/core/tests/api/test_message_attachments_serializer.py b/src/backend/core/tests/api/test_message_attachments_serializer.py new file mode 100644 index 00000000..5af5d831 --- /dev/null +++ b/src/backend/core/tests/api/test_message_attachments_serializer.py @@ -0,0 +1,42 @@ +"""Tests for ``MessageSerializer.get_attachments`` name handling.""" + +from unittest.mock import Mock + +import pytest + +from core.api.serializers import MessageSerializer + + +def _serialize_parsed_attachments(parsed_attachments): + """Run ``get_attachments`` against a non-draft message whose parsed MIME + exposes ``parsed_attachments``, bypassing DB and ``parse_email``.""" + instance = Mock() + instance.id = "00000000-0000-0000-0000-000000000000" + instance.has_attachments = True + instance.is_draft = False + instance.get_parsed_field.return_value = parsed_attachments + return MessageSerializer().get_attachments(instance) + + +def test_get_attachments_preserves_present_name(): + """A MIME part that carries a ``filename`` keeps it verbatim.""" + result = _serialize_parsed_attachments( + [{"name": "report.pdf", "size": 12, "type": "application/pdf"}] + ) + assert [a["name"] for a in result] == ["report.pdf"] + + +@pytest.mark.parametrize( + "attachment", + [ + pytest.param({"size": 1, "type": "text/plain"}, id="missing-name"), + pytest.param({"name": None, "size": 1, "type": "text/plain"}, id="none-name"), + pytest.param({"name": "", "size": 1, "type": "text/plain"}, id="empty-name"), + ], +) +def test_get_attachments_falls_back_to_unnamed(attachment): + """A MIME part with no usable ``filename`` falls back to the "unnamed" + sentinel so consumers never receive a null/empty name (regression: a null + name crashed the frontend calendar-invite download button).""" + result = _serialize_parsed_attachments([attachment]) + assert result[0]["name"] == "unnamed" diff --git a/src/backend/core/tests/swagger/test_openapi_schema.py b/src/backend/core/tests/swagger/test_openapi_schema.py index 74b0f183..3a139a67 100644 --- a/src/backend/core/tests/swagger/test_openapi_schema.py +++ b/src/backend/core/tests/swagger/test_openapi_schema.py @@ -40,3 +40,29 @@ def test_openapi_client_schema(): "core/tests/swagger/swagger.json", "r", encoding="utf-8" ) as expected_schema: assert response.json() == json.load(expected_schema) + + +@pytest.mark.parametrize( + "schema_name,field", + [ + # Mailbox.contact is a SET_NULL FK (an alias mailbox has none). + ("MailboxAdmin", "contact"), + # get_expected_dns_records returns None outside the retrieve action. + ("MailDomainAdmin", "expected_dns_records"), + # get_role / get_user_role return None when the user has no role in + # the relevant scope (e.g. no mailbox_id in context on the split action). + ("Mailbox", "role"), + ("Thread", "user_role"), + ], +) +def test_openapi_method_fields_are_nullable(schema_name, field): + """Fields whose value can legitimately be ``null`` at runtime must be + advertised as nullable in the schema, so the generated typed client does + not assume a non-null value (regression: SerializerMethodField and nested + serializers on nullable FKs don't inherit nullability automatically).""" + response = Client().get("/api/v1.0/swagger.json") + assert response.status_code == 200 + + prop = response.json()["components"]["schemas"][schema_name]["properties"][field] + + assert prop.get("nullable") is True diff --git a/src/frontend/src/features/api/gen/models/index.ts b/src/frontend/src/features/api/gen/models/index.ts index f701e535..2b654b0e 100644 --- a/src/frontend/src/features/api/gen/models/index.ts +++ b/src/frontend/src/features/api/gen/models/index.ts @@ -64,6 +64,7 @@ export * from "./labels_remove_threads_create_body"; export * from "./mail_domain_access_role_choices"; export * from "./mail_domain_admin"; export * from "./mail_domain_admin_abilities"; +export * from "./mail_domain_admin_expected_dns_records_item"; export * from "./mail_domain_admin_write"; export * from "./mail_domain_admin_write_request"; export * from "./mailbox"; @@ -73,7 +74,9 @@ export * from "./mailbox_access_read"; export * from "./mailbox_access_write"; export * from "./mailbox_access_write_request"; export * from "./mailbox_admin"; +export * from "./mailbox_admin_contact"; export * from "./mailbox_admin_create"; +export * from "./mailbox_admin_create_contact"; export * from "./mailbox_admin_create_metadata_request"; export * from "./mailbox_admin_create_metadata_type_enum"; export * from "./mailbox_admin_create_payload_request"; @@ -82,6 +85,7 @@ export * from "./mailbox_admin_mandatory_totp_response"; export * from "./mailbox_admin_reset_totp_response"; export * from "./mailbox_admin_update_metadata_request"; export * from "./mailbox_light"; +export * from "./mailbox_role"; export * from "./mailbox_role_choices"; export * from "./mailboxes_accesses_list_params"; export * from "./mailboxes_calendar_add_create400"; @@ -182,6 +186,7 @@ export * from "./thread_label"; export * from "./thread_mentionable_user"; export * from "./thread_mentionable_user_custom_attributes"; export * from "./thread_split_request_request"; +export * from "./thread_user_role"; export * from "./threads_accesses_create_params"; export * from "./threads_accesses_destroy_params"; export * from "./threads_accesses_list_params"; diff --git a/src/frontend/src/features/api/gen/models/mail_domain_admin.ts b/src/frontend/src/features/api/gen/models/mail_domain_admin.ts index b9c15dc2..1df2d834 100644 --- a/src/frontend/src/features/api/gen/models/mail_domain_admin.ts +++ b/src/frontend/src/features/api/gen/models/mail_domain_admin.ts @@ -5,6 +5,7 @@ * This is the messages API schema. * OpenAPI spec version: 1.0.0 (v1.0) */ +import type { MailDomainAdminExpectedDnsRecordsItem } from "./mail_domain_admin_expected_dns_records_item"; import type { MailDomainAdminAbilities } from "./mail_domain_admin_abilities"; /** @@ -18,7 +19,10 @@ export interface MailDomainAdmin { readonly created_at: string; /** date and time at which a record was last updated */ readonly updated_at: string; - readonly expected_dns_records: string; + /** @nullable */ + readonly expected_dns_records: + | readonly MailDomainAdminExpectedDnsRecordsItem[] + | null; readonly mailbox_count: string; /** Sync mailboxes to an identity provider. */ readonly identity_sync: boolean; diff --git a/src/frontend/src/features/api/gen/models/mail_domain_admin_expected_dns_records_item.ts b/src/frontend/src/features/api/gen/models/mail_domain_admin_expected_dns_records_item.ts new file mode 100644 index 00000000..6b42966a --- /dev/null +++ b/src/frontend/src/features/api/gen/models/mail_domain_admin_expected_dns_records_item.ts @@ -0,0 +1,13 @@ +/** + * Generated by orval 🍺 + * Do not edit manually. + * messages API + * This is the messages API schema. + * OpenAPI spec version: 1.0.0 (v1.0) + */ + +export type MailDomainAdminExpectedDnsRecordsItem = { + target: string; + type: string; + value: string; +}; diff --git a/src/frontend/src/features/api/gen/models/mailbox.ts b/src/frontend/src/features/api/gen/models/mailbox.ts index def57d97..6b28d0db 100644 --- a/src/frontend/src/features/api/gen/models/mailbox.ts +++ b/src/frontend/src/features/api/gen/models/mailbox.ts @@ -5,7 +5,7 @@ * This is the messages API schema. * OpenAPI spec version: 1.0.0 (v1.0) */ -import type { MailboxRoleChoices } from "./mailbox_role_choices"; +import type { MailboxRole } from "./mailbox_role"; import type { MailboxAbilities } from "./mailbox_abilities"; /** @@ -29,7 +29,8 @@ Drives mailbox-level UI gating for collaboration features (assignment sub-folders, mention folder) that have no purpose in a mono-user identity mailbox. */ readonly is_shared: boolean; - readonly role: MailboxRoleChoices; + /** @nullable */ + readonly role: MailboxRole; /** Return the number of threads with unread messages in the mailbox. */ readonly count_unread_threads: number; /** Return the number of threads in the mailbox. */ diff --git a/src/frontend/src/features/api/gen/models/mailbox_admin.ts b/src/frontend/src/features/api/gen/models/mailbox_admin.ts index ba4935be..5a0ba848 100644 --- a/src/frontend/src/features/api/gen/models/mailbox_admin.ts +++ b/src/frontend/src/features/api/gen/models/mailbox_admin.ts @@ -6,7 +6,7 @@ * OpenAPI spec version: 1.0.0 (v1.0) */ import type { MailboxAccessNestedUser } from "./mailbox_access_nested_user"; -import type { Contact } from "./contact"; +import type { MailboxAdminContact } from "./mailbox_admin_contact"; /** * Serialize Mailbox details for admin view, including users with access. @@ -33,7 +33,8 @@ export interface MailboxAdmin { /** date and time at which a record was last updated */ readonly updated_at: string; readonly can_reset_password: boolean; - readonly contact: Contact; + /** @nullable */ + readonly contact: MailboxAdminContact; /** * Most recent ``accessed_at`` across all mailbox accesses. * @nullable diff --git a/src/frontend/src/features/api/gen/models/mailbox_admin_contact.ts b/src/frontend/src/features/api/gen/models/mailbox_admin_contact.ts new file mode 100644 index 00000000..aee75d5b --- /dev/null +++ b/src/frontend/src/features/api/gen/models/mailbox_admin_contact.ts @@ -0,0 +1,13 @@ +/** + * Generated by orval 🍺 + * Do not edit manually. + * messages API + * This is the messages API schema. + * OpenAPI spec version: 1.0.0 (v1.0) + */ +import type { Contact } from "./contact"; + +/** + * @nullable + */ +export type MailboxAdminContact = Contact | null; diff --git a/src/frontend/src/features/api/gen/models/mailbox_admin_create.ts b/src/frontend/src/features/api/gen/models/mailbox_admin_create.ts index 05bc1c90..66da6d35 100644 --- a/src/frontend/src/features/api/gen/models/mailbox_admin_create.ts +++ b/src/frontend/src/features/api/gen/models/mailbox_admin_create.ts @@ -6,7 +6,7 @@ * OpenAPI spec version: 1.0.0 (v1.0) */ import type { MailboxAccessNestedUser } from "./mailbox_access_nested_user"; -import type { Contact } from "./contact"; +import type { MailboxAdminCreateContact } from "./mailbox_admin_create_contact"; /** * Serialize Mailbox details for create admin endpoint, including users with access and @@ -30,7 +30,8 @@ export interface MailboxAdminCreate { /** date and time at which a record was last updated */ readonly updated_at: string; readonly can_reset_password: boolean; - readonly contact: Contact; + /** @nullable */ + readonly contact: MailboxAdminCreateContact; /** * Most recent ``accessed_at`` across all mailbox accesses. * @nullable diff --git a/src/frontend/src/features/api/gen/models/mailbox_admin_create_contact.ts b/src/frontend/src/features/api/gen/models/mailbox_admin_create_contact.ts new file mode 100644 index 00000000..3477330a --- /dev/null +++ b/src/frontend/src/features/api/gen/models/mailbox_admin_create_contact.ts @@ -0,0 +1,13 @@ +/** + * Generated by orval 🍺 + * Do not edit manually. + * messages API + * This is the messages API schema. + * OpenAPI spec version: 1.0.0 (v1.0) + */ +import type { Contact } from "./contact"; + +/** + * @nullable + */ +export type MailboxAdminCreateContact = Contact | null; diff --git a/src/frontend/src/features/api/gen/models/mailbox_role.ts b/src/frontend/src/features/api/gen/models/mailbox_role.ts new file mode 100644 index 00000000..45f79426 --- /dev/null +++ b/src/frontend/src/features/api/gen/models/mailbox_role.ts @@ -0,0 +1,13 @@ +/** + * Generated by orval 🍺 + * Do not edit manually. + * messages API + * This is the messages API schema. + * OpenAPI spec version: 1.0.0 (v1.0) + */ +import type { MailboxRoleChoices } from "./mailbox_role_choices"; + +/** + * @nullable + */ +export type MailboxRole = MailboxRoleChoices | null; diff --git a/src/frontend/src/features/api/gen/models/thread.ts b/src/frontend/src/features/api/gen/models/thread.ts index 327b7103..41dde4b6 100644 --- a/src/frontend/src/features/api/gen/models/thread.ts +++ b/src/frontend/src/features/api/gen/models/thread.ts @@ -5,7 +5,7 @@ * This is the messages API schema. * OpenAPI spec version: 1.0.0 (v1.0) */ -import type { ThreadAccessRoleChoices } from "./thread_access_role_choices"; +import type { ThreadUserRole } from "./thread_user_role"; import type { ThreadAccessDetail } from "./thread_access_detail"; import type { ThreadLabel } from "./thread_label"; import type { ThreadAbilities } from "./thread_abilities"; @@ -68,7 +68,8 @@ export interface Thread { readonly sender_names: readonly string[]; /** date and time at which a record was last updated */ readonly updated_at: string; - readonly user_role: ThreadAccessRoleChoices; + /** @nullable */ + readonly user_role: ThreadUserRole; readonly accesses: readonly ThreadAccessDetail[]; readonly labels: readonly ThreadLabel[]; readonly summary: string; diff --git a/src/frontend/src/features/api/gen/models/thread_user_role.ts b/src/frontend/src/features/api/gen/models/thread_user_role.ts new file mode 100644 index 00000000..b554ce64 --- /dev/null +++ b/src/frontend/src/features/api/gen/models/thread_user_role.ts @@ -0,0 +1,13 @@ +/** + * Generated by orval 🍺 + * Do not edit manually. + * messages API + * This is the messages API schema. + * OpenAPI spec version: 1.0.0 (v1.0) + */ +import type { ThreadAccessRoleChoices } from "./thread_access_role_choices"; + +/** + * @nullable + */ +export type ThreadUserRole = ThreadAccessRoleChoices | null; diff --git a/src/frontend/src/features/providers/mailbox.tsx b/src/frontend/src/features/providers/mailbox.tsx index 2c45e59f..237e8827 100644 --- a/src/frontend/src/features/providers/mailbox.tsx +++ b/src/frontend/src/features/providers/mailbox.tsx @@ -726,6 +726,8 @@ export const MailboxProvider = ({ children }: PropsWithChildren) => { }, [selectedMailbox?.id, searchParams.toString()]); useEffect(() => { + if (!location.pathname.startsWith('/mailbox')) return; + const previousSearch = previousSearchParams?.get('search'); const currentSearch = searchParams.get('search');