Skip to content

Commit ecbb2fa

Browse files
committed
🐛(backend) make message subject optional
- Add null=True, blank=True to Message and Thread subject fields - Add comprehensive tests for empty/null/missing subjects - Update migration 0018 to alter subject field constraints - Fix #161: allow messages without subject field
1 parent 4391190 commit ecbb2fa

File tree

5 files changed

+368
-10
lines changed

5 files changed

+368
-10
lines changed

src/backend/core/mda/inbound.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ def deliver_inbound_message( # pylint: disable=too-many-branches, too-many-stat
330330
snippet = "(No snippet available)" # Absolute fallback
331331

332332
thread = models.Thread.objects.create(
333-
subject=parsed_email.get("subject", "(no subject)"),
333+
subject=parsed_email.get("subject"),
334334
snippet=snippet,
335335
)
336336
# Create a thread access for the sender mailbox
@@ -441,7 +441,7 @@ def deliver_inbound_message( # pylint: disable=too-many-branches, too-many-stat
441441
message = models.Message.objects.create(
442442
thread=thread,
443443
sender=sender_contact,
444-
subject=parsed_email.get("subject", "(no subject)"),
444+
subject=parsed_email.get("subject"),
445445
raw_mime=raw_data,
446446
mime_id=parsed_email.get("messageId", parsed_email.get("message_id"))
447447
or None,
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Generated by Django 5.1.8 on 2025-07-01 09:27
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
('core', '0017_remove_thread_count_draft_and_more'),
10+
]
11+
12+
operations = [
13+
migrations.AlterModelOptions(
14+
name='label',
15+
options={'ordering': ['slug'], 'verbose_name': 'label', 'verbose_name_plural': 'labels'},
16+
),
17+
migrations.AlterField(
18+
model_name='message',
19+
name='subject',
20+
field=models.CharField(blank=True, null=True, max_length=255, verbose_name='subject'),
21+
),
22+
migrations.AlterField(
23+
model_name='thread',
24+
name='subject',
25+
field=models.CharField(blank=True, null=True, max_length=255, verbose_name='subject'),
26+
),
27+
]

src/backend/core/models.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ def __str__(self):
325325
class Thread(BaseModel):
326326
"""Thread model to group messages."""
327327

328-
subject = models.CharField(_("subject"), max_length=255)
328+
subject = models.CharField(_("subject"), max_length=255, null=True, blank=True)
329329
snippet = models.TextField(_("snippet"), blank=True)
330330
has_unread = models.BooleanField(_("has unread"), default=False)
331331
has_trashed = models.BooleanField(_("has trashed"), default=False)
@@ -344,7 +344,7 @@ class Meta:
344344
verbose_name_plural = _("threads")
345345

346346
def __str__(self):
347-
return self.subject
347+
return str(self.subject) if self.subject else "(no subject)"
348348

349349
def update_stats(self):
350350
"""Update the denormalized stats of the thread."""
@@ -693,7 +693,7 @@ class Message(BaseModel):
693693
thread = models.ForeignKey(
694694
Thread, on_delete=models.CASCADE, related_name="messages"
695695
)
696-
subject = models.CharField(_("subject"), max_length=255)
696+
subject = models.CharField(_("subject"), max_length=255, null=True, blank=True)
697697
sender = models.ForeignKey("Contact", on_delete=models.CASCADE)
698698
parent = models.ForeignKey(
699699
"Message", on_delete=models.SET_NULL, null=True, blank=True
@@ -733,7 +733,7 @@ class Meta:
733733
ordering = ["-created_at"]
734734

735735
def __str__(self):
736-
return self.subject
736+
return str(self.subject) if self.subject else "(no subject)"
737737

738738
def get_parsed_data(self) -> Dict[str, Any]:
739739
"""Parse raw_mime using parser and cache the result."""

src/backend/core/tests/api/test_messages_create.py

Lines changed: 158 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -476,16 +476,170 @@ def test_draft_message_unauthorized(self):
476476
"senderId": uuid.uuid4(),
477477
"subject": "test",
478478
"draftBody": "<p>test</p> or test",
479-
"to": ["pierre@example.com"],
479+
"to": ["pierre@external.com"],
480480
},
481481
format="json",
482482
)
483483
# Assert the response is unauthorized
484484
assert response.status_code == status.HTTP_401_UNAUTHORIZED
485485

486-
# TODO: implement this test
487-
# def test_send_message_unauthorized(self, mailbox, authenticated_user, send_url):
488-
# """Test send message unauthorized."""
486+
def test_draft_message_with_empty_subject(self, mailbox, authenticated_user):
487+
"""Test create draft message with empty subject."""
488+
factories.MailboxAccessFactory(
489+
mailbox=mailbox,
490+
user=authenticated_user,
491+
role=enums.MailboxRoleChoices.EDITOR,
492+
)
493+
494+
client = APIClient()
495+
client.force_authenticate(user=authenticated_user)
496+
497+
draft_response = client.post(
498+
reverse("draft-message"),
499+
{
500+
"senderId": mailbox.id,
501+
"subject": "", # Empty subject should be allowed
502+
"draftBody": "Test content",
503+
"to": ["[email protected]"],
504+
},
505+
format="json",
506+
)
507+
508+
assert draft_response.status_code == status.HTTP_201_CREATED
509+
draft_message_id = draft_response.data["id"]
510+
511+
# Verify the message was created with empty subject
512+
draft_message = models.Message.objects.get(id=draft_message_id)
513+
assert draft_message.subject == ""
514+
assert draft_message.is_draft is True
515+
assert str(draft_message) == "(no subject)"
516+
517+
# Verify the thread also has empty subject
518+
assert draft_message.thread.subject == ""
519+
assert str(draft_message.thread) == "(no subject)"
520+
521+
def test_draft_message_without_subject(self, mailbox, authenticated_user):
522+
"""Test create draft message without subject field."""
523+
factories.MailboxAccessFactory(
524+
mailbox=mailbox,
525+
user=authenticated_user,
526+
role=enums.MailboxRoleChoices.EDITOR,
527+
)
528+
529+
client = APIClient()
530+
client.force_authenticate(user=authenticated_user)
531+
532+
draft_response = client.post(
533+
reverse("draft-message"),
534+
{
535+
"senderId": mailbox.id,
536+
# No subject field - should default to None
537+
"draftBody": "Test content",
538+
"to": ["[email protected]"],
539+
},
540+
format="json",
541+
)
542+
543+
assert draft_response.status_code == status.HTTP_201_CREATED
544+
draft_message_id = draft_response.data["id"]
545+
546+
# Verify the message was created with None subject
547+
draft_message = models.Message.objects.get(id=draft_message_id)
548+
assert draft_message.subject is None
549+
assert draft_message.is_draft is True
550+
assert str(draft_message) == "(no subject)"
551+
552+
# Verify the thread also has None subject
553+
assert draft_message.thread.subject is None
554+
assert str(draft_message.thread) == "(no subject)"
555+
556+
@patch("core.mda.outbound.send_outbound_message")
557+
def test_send_message_with_empty_subject(
558+
self, mock_send_outbound_message, mailbox, authenticated_user, send_url
559+
):
560+
"""Test sending a message with empty subject (migration 0018)."""
561+
mock_send_outbound_message.side_effect = lambda recipient_emails, message: {
562+
recipient_email: {
563+
"delivered": True,
564+
"error": None,
565+
}
566+
for recipient_email in recipient_emails
567+
}
568+
569+
factories.MailboxAccessFactory(
570+
mailbox=mailbox,
571+
user=authenticated_user,
572+
role=enums.MailboxRoleChoices.EDITOR,
573+
)
574+
575+
client = APIClient()
576+
client.force_authenticate(user=authenticated_user)
577+
578+
# Create draft with empty subject
579+
draft_response = client.post(
580+
reverse("draft-message"),
581+
{
582+
"senderId": mailbox.id,
583+
"subject": "", # Empty subject
584+
"draftBody": "Test content",
585+
"to": ["[email protected]"],
586+
},
587+
format="json",
588+
)
589+
590+
assert draft_response.status_code == status.HTTP_201_CREATED
591+
draft_message_id = draft_response.data["id"]
592+
593+
# Send the message
594+
send_response = client.post(
595+
send_url,
596+
{
597+
"messageId": draft_message_id,
598+
"senderId": mailbox.id,
599+
},
600+
format="json",
601+
)
602+
603+
assert send_response.status_code == status.HTTP_200_OK
604+
605+
# Verify the message was sent with empty subject
606+
sent_message = models.Message.objects.get(id=draft_message_id)
607+
assert sent_message.subject == ""
608+
assert sent_message.is_draft is False
609+
assert sent_message.sent_at is not None
610+
assert str(sent_message) == "(no subject)"
611+
612+
# Verify the thread also has empty subject
613+
assert sent_message.thread.subject == ""
614+
assert str(sent_message.thread) == "(no subject)"
615+
616+
def test_draft_message_with_very_long_subject(self, mailbox, authenticated_user):
617+
"""Test create draft message with subject exceeding max_length."""
618+
factories.MailboxAccessFactory(
619+
mailbox=mailbox,
620+
user=authenticated_user,
621+
role=enums.MailboxRoleChoices.EDITOR,
622+
)
623+
624+
client = APIClient()
625+
client.force_authenticate(user=authenticated_user)
626+
627+
# Create a subject that exceeds max_length (255)
628+
long_subject = "A" * 256
629+
630+
draft_response = client.post(
631+
reverse("draft-message"),
632+
{
633+
"senderId": mailbox.id,
634+
"subject": long_subject,
635+
"draftBody": "Test content",
636+
"to": ["[email protected]"],
637+
},
638+
format="json",
639+
)
640+
641+
# Should fail due to max_length constraint
642+
assert draft_response.status_code == status.HTTP_400_BAD_REQUEST
489643

490644
def test_send_nonexistent_message(self, mailbox, authenticated_user, send_url):
491645
"""Test sending a message that does not exist."""

0 commit comments

Comments
 (0)