Skip to content

✨(frontend+backend) show per-thread message count badge in mailbox list #751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nicolasaunai wants to merge 1 commit into
base: main
Choose a base branch
from
Open

✨(frontend+backend) show per-thread message count badge in mailbox list #751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ and this project adheres to
- Bump keycloak to 26.6.3
- Bump keycloak to 26.6.4

### Added

- ✨(frontend+backend) show per-thread message count badge in mailbox list

### Fixed

- 🐛(mta-out) fix relay block indentation breaking SASL auth #733
Expand Down
5 changes: 5 additions & 0 deletions src/backend/core/api/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10132,6 +10132,10 @@
"type": "integer",
"readOnly": true
},
"message_count": {
"type": "integer",
"readOnly": true
},
"abilities": {
"type": "object",
"additionalProperties": {
Expand Down Expand Up @@ -10171,6 +10175,7 @@
"is_spam",
"is_trashed",
"labels",
"message_count",
"messaged_at",
"messages",
"sender_messaged_at",
Expand Down
2 changes: 2 additions & 0 deletions src/backend/core/api/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

@coderabbitai coderabbitai Bot Jul 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check if `message_count` property is defined on the Thread model.
rg 'def message_count' src/backend/core/models/

Repository: suitenumerique/messages

Length of output: 226

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate Thread model, message_count usages, and split action.
git ls-files | rg '(^|/)serializers\.py$|(^|/)models?\.py$|(^|/)views?\.py$|(^|/)viewsets?\.py$|(^|/)api/'
printf '\n--- search class Thread ---\n'
rg -n "class Thread\b|message_count|def split\b|`@action`\(.*split|split\(" src/backend -S

Repository: suitenumerique/messages

Length of output: 36523

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map likely files for Thread-related logic before reading them.
fd -a 'serializers.py|models.py|views.py|viewsets.py' src/backend 2>/dev/null | sed 's#^\./##'

Repository: suitenumerique/messages

Length of output: 262

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- src/backend/core/models.py (Thread excerpt) ---'
sed -n '1032,1135p' src/backend/core/models.py

printf '\n%s\n' '--- src/backend/core/api/serializers.py (ThreadSerializer excerpt) ---'
sed -n '780,890p' src/backend/core/api/serializers.py

printf '\n%s\n' '--- src/backend/core/api/viewsets/thread.py (split excerpt) ---'
sed -n '820,930p' src/backend/core/api/viewsets/thread.py

printf '\n%s\n' '--- tests for split endpoint ---'
sed -n '1,260p' src/backend/core/tests/api/test_thread_split.py

Repository: suitenumerique/messages

Length of output: 21021

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- search Thread.message_count definition ---'
rg -n "message_count" src/backend/core/models.py src/backend/core/api/viewsets/thread.py src/backend/core/api/serializers.py src/backend/core/tests/api/test_thread_split.py -S

printf '\n%s\n' '--- src/backend/core/models.py (later Thread methods) ---'
sed -n '1135,1245p' src/backend/core/models.py

printf '\n%s\n' '--- src/backend/core/api/viewsets/thread.py (split continuation) ---'
sed -n '930,1045p' src/backend/core/api/viewsets/thread.py

printf '\n%s\n' '--- src/backend/core/tests/api/test_thread_split.py (success tests) ---'
sed -n '260,520p' src/backend/core/tests/api/test_thread_split.py

Repository: suitenumerique/messages

Length of output: 18819

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
try:
    from rest_framework import serializers
except Exception as exc:
    print(f"IMPORT_FAIL: {type(exc).__name__}: {exc}")
    raise SystemExit(0)

class Obj:
    pass

class S(serializers.Serializer):
    x = serializers.IntegerField(read_only=True)

print(S(Obj()).data)
PY

Repository: suitenumerique/messages

Length of output: 228

🌐 Web query:

DRF read-only IntegerField missing attribute serialization AttributeError SkipField get_attribute

💡 Result:

The AttributeError combined with SkipField in Django REST Framework (DRF) typically occurs during the serialization process when the framework attempts to access an attribute on an object that does not exist [1][2][3]. ### Why this happens In DRF, the Field.get_attribute() method is responsible for retrieving the value for a field from the object instance [4][3]. To handle cases where an attribute might be missing (often due to required=False or specific model states), DRF wraps the attribute access in a try-except block that catches AttributeError and KeyError [1][2][3]. When such an exception is caught: 1. If the field is not required and has no default value, DRF raises a SkipField exception [1][3]. 2. The SkipField exception is caught by the parent serializer, causing the field to be silently omitted from the serialized output [1][5][6]. If you are seeing this error, it often means that get_attribute is encountering an AttributeError that it interprets as "the field is missing," but it may actually be masking a legitimate error in your code (such as an incorrectly named field or a property that raises an AttributeError internally) [1][2][7]. ### Common Causes and Solutions * Incorrect Source: Ensure the source argument on your serializer field correctly matches an attribute or method on your model instance [1][2]. If the path is wrong, get_attribute will fail to find it [1]. * Property Methods: If a @property on your model raises an AttributeError internally, DRF may catch it and treat the field as missing [1][2]. Ensure your property logic is robust and does not raise AttributeError for expected null or missing states [7]. * Providing a Default: If you want the field to appear in the output even if the attribute access fails (e.g., returning None instead of skipping the field), you can explicitly set default=None on the field [7]. This prevents the logic from reaching the SkipField path [3]. * Debugging: Because this behavior suppresses the original traceback, it can be difficult to debug [1][2]. If you suspect the error is not simply a missing attribute, you can temporarily override get_attribute on your field to inspect the instance and source_attrs directly, allowing you to see what is causing the AttributeError [5]. For more information, refer to the DRF documentation on serializer fields, specifically the behavior of read_only fields and get_attribute [4].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- frontend generated thread model ---'
sed -n '1,220p' src/frontend/src/features/api/gen/models/thread.ts

printf '\n%s\n' '--- any split response usage of message_count ---'
rg -n "message_count|messageCount" src/frontend/src/features/api/gen/models src/frontend/src/features/api/gen/threads src/frontend/src/features/api/gen/thread* src/frontend/src/features/api/gen -S

Repository: suitenumerique/messages

Length of output: 2999

Populate message_count on the split response. new_thread isn’t annotated here, so DRF will skip this field and the split payload will be missing message_count compared with list/retrieve. Set it before serialization or switch to a serializer method with a fallback count.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/backend/core/api/serializers.py` at line 840, Ensure the split response
serializer populates the read-only message_count field even though new_thread is
not annotated. Update the split-response construction to set message_count
before serialization, or change the serializer field implementation to compute a
fallback count when the annotation is absent, while preserving the existing
list/retrieve behavior.

abilities = serializers.SerializerMethodField(read_only=True)
assigned_users = serializers.SerializerMethodField(read_only=True)

Expand Down Expand Up @@ -1019,6 +1020,7 @@ class Meta:
"labels",
"summary",
"events_count",
"message_count",
"abilities",
"assigned_users",
]
Expand Down
3 changes: 3 additions & 0 deletions src/backend/core/api/viewsets/thread.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
104 changes: 104 additions & 0 deletions src/backend/core/tests/api/test_threads_list.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1941,6 +1941,110 @@ def test_list_threads_events_count_distinct_per_thread(self, api_client, url):
assert payload["events_count"] == 2


class TestThreadListMessageCount:
"""Test that ThreadSerializer exposes message_count on the list endpoint.

message_count drives the per-thread email-count badge in the mailbox 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


class TestThreadListQueryCount:
"""Regression guard for N+1 queries on the thread list endpoint.

Expand Down
1 change: 1 addition & 0 deletions src/frontend/src/features/api/gen/models/thread.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}
11 changes: 11 additions & 0 deletions src/frontend/src/features/layouts/components/thread-panel/components/thread-item/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,17 @@ export const ThreadItem = ({ thread, isSelected, onToggle, onSelectRange, select
<p className="thread-item__subject">{thread.subject || t('No subject')}</p>
</div>
<div className="thread-item__column thread-item__column--badges">
{thread.message_count > 1 && (
<Badge
aria-label={t('{{count}} messages', { count: thread.message_count })}
title={t('{{count}} messages', { count: thread.message_count })}
color="neutral"
variant="tertiary"
compact
>
{thread.message_count}
</Badge>
)}
{thread.has_draft && (
<Badge aria-label={t('Draft')} title={t('Draft')} color="neutral" variant="tertiary" compact>
<Icon
Expand Down