✨(frontend+backend) show per-thread message count badge in mailbox list#751
✨(frontend+backend) show per-thread message count badge in mailbox list#751nicolasaunai wants to merge 1 commit into
Conversation
Adds a message_count field to the ThreadSerializer (counting non-draft messages, distinct to avoid JOIN multiplication) and renders a neutral badge in the thread list when the count is greater than one. Signed-off-by: Nicolas Aunai <nicolas.aunai@lpp.polytechnique.fr>
📝 WalkthroughWalkthroughThe thread API now returns distinct non-draft message counts, and the mailbox thread item displays counts greater than one as an internationalized badge. API tests cover zero, draft, duplicate-join, and normal message-count cases. ChangesThread message count
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Mailbox as Mailbox list
participant ThreadViewSet
participant Database
participant ThreadSerializer
participant ThreadItem
Mailbox->>ThreadViewSet: request threads-list
ThreadViewSet->>Database: count distinct non-draft messages
Database-->>ThreadViewSet: annotated threads
ThreadViewSet->>ThreadSerializer: serialize thread data
ThreadSerializer-->>Mailbox: response with message_count
Mailbox->>ThreadItem: render thread
ThreadItem-->>Mailbox: show badge when message_count > 1
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/backend/core/api/serializers.py`:
- 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9777168b-a166-455b-90e3-c7e505e7bf60
⛔ Files ignored due to path filters (1)
src/frontend/src/features/api/gen/models/thread.tsis excluded by!**/gen/**
📒 Files selected for processing (6)
CHANGELOG.mdsrc/backend/core/api/openapi.jsonsrc/backend/core/api/serializers.pysrc/backend/core/api/viewsets/thread.pysrc/backend/core/tests/api/test_threads_list.pysrc/frontend/src/features/layouts/components/thread-panel/components/thread-item/index.tsx
| labels = serializers.SerializerMethodField() | ||
| summary = serializers.CharField(read_only=True) | ||
| events_count = serializers.IntegerField(read_only=True) | ||
| message_count = serializers.IntegerField(read_only=True) |
There was a problem hiding this comment.
🎯 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 -SRepository: 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.pyRepository: 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.pyRepository: 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)
PYRepository: 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:
- 1: Silent skipping of read-only fields that throw exceptions: weird! unintentional? encode/django-rest-framework#2598
- 2: get_attribute swallows tracebacks for KeyError/AttributeError encode/django-rest-framework#2884
- 3: https://www.cdrf.co/3.9/rest_framework.serializers/Serializer.html
- 4: https://www.django-rest-framework.org/api-guide/fields/
- 5: to_representation omits field if there is an error. encode/django-rest-framework#6575
- 6: SkipField Exception when serializing list openwisp/django-rest-framework-gis#97
- 7: Changes to the get_attribute method under fields.py after version 3.6.x encode/django-rest-framework#7196
🏁 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 -SRepository: 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.
2 participants
Purpose
Adds a per-thread email-count badge in the mailbox list so users can see at a glance how many messages a thread contains.
Proposal
Backend:
ThreadSerializergains amessage_countfield, annotated asCount("messages", filter=Q(messages__is_draft=False), distinct=True)on the thread list queryset to avoid JOIN multiplication from the accesses join.Frontend: a neutral
Badgerenders the count (only when > 1) in the thread list item, with an i18n aria-label/title.Tests:
TestThreadListMessageCountcovers zero, exact count, draft exclusion, and distinct-per-thread regression.signoff
signed commits
lint
tests
Summary by CodeRabbit
New Features
Documentation