✨(backend) add mention read receipts - #769
Open
nicolasaunai wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthroughAdds an author-only ChangesMention read receipts
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ThreadEventSerializer
participant UserEvent
Client->>ThreadEventSerializer: Request thread events
ThreadEventSerializer->>UserEvent: Query read mention rows
UserEvent-->>ThreadEventSerializer: Return reader names and read_at values
ThreadEventSerializer-->>Client: Return mention_read_by
Suggested reviewers: 🚥 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 |
Add mention_read_by field to ThreadEventSerializer exposing which mentioned users have read the mention. Only visible to the event author. - New MentionReadByUserSerializer + get_mention_read_by on backend - Updated OpenAPI schema and regenerated TypeScript client - Frontend shows "Lu par X/Y" indicator with tooltip on author's bubbles - French and English translations for the read indicator Signed-off-by: Nicolas Aunai <nicolas.aunai@lpp.polytechnique.fr>
nicolasaunai
force-pushed
the
feat/mention-read-receipts
branch
from
July 29, 2026 20:54
225e0bd to
c41cd96
Compare
nicolasaunai
marked this pull request as ready for review
July 29, 2026 21:18
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Around line 1420-1441: Eliminate the per-event UserEvent query in
get_mention_read_by by prefetching filtered, read mention events for all thread
events in the parent events queryset, storing them under a cache attribute.
Update get_mention_read_by to consume that prefetched cache while preserving the
author-only None behavior and existing serialized fields.
In
`@src/frontend/src/features/layouts/components/thread-view/components/thread-event/index.tsx`:
- Around line 313-328: Add component-test coverage for the read-status rendering
in the thread-event component: verify an author with populated mention_read_by
renders the X/Y count and tooltip containing reader names, while non-authors and
null mention_read_by do not render the status. Extend the relevant
assignment-message and group-system-events fixtures or tests without changing
the component 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 Plus
Run ID: 74dc8d3f-6153-46ad-a1fe-b72e36d933d4
⛔ Files ignored due to path filters (3)
src/frontend/src/features/api/gen/models/index.tsis excluded by!**/gen/**src/frontend/src/features/api/gen/models/mention_read_by_user.tsis excluded by!**/gen/**src/frontend/src/features/api/gen/models/thread_event.tsis excluded by!**/gen/**
📒 Files selected for processing (9)
spec_read_receipts.mdsrc/backend/core/api/openapi.jsonsrc/backend/core/api/serializers.pysrc/frontend/public/locales/common/en-US.jsonsrc/frontend/public/locales/common/fr-FR.jsonsrc/frontend/src/features/layouts/components/thread-view/components/thread-event/_index.scsssrc/frontend/src/features/layouts/components/thread-view/components/thread-event/assignment-message.test.tssrc/frontend/src/features/layouts/components/thread-view/components/thread-event/group-system-events.test.tssrc/frontend/src/features/layouts/components/thread-view/components/thread-event/index.tsx
Comment on lines
+1420
to
+1441
| @extend_schema_field(MentionReadByUserSerializer(many=True, allow_null=True)) | ||
| def get_mention_read_by(self, obj): | ||
| """Return the list of mentioned users who have read this mention. | ||
|
|
||
| Only returned for the event author. Returns ``None`` for other users. | ||
| """ | ||
| request = self.context.get("request") | ||
| if not request or request.user != obj.author: | ||
| return None | ||
| read_mentions = models.UserEvent.objects.filter( | ||
| thread_event=obj, | ||
| type=enums.UserEventTypeChoices.MENTION, | ||
| read_at__isnull=False, | ||
| ).select_related("user") | ||
| return [ | ||
| { | ||
| "id": str(ue.user.id), | ||
| "name": ue.user.full_name or ue.user.email or "", | ||
| "read_at": ue.read_at.isoformat(), | ||
| } | ||
| for ue in read_mentions | ||
| ] |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Avoid a per-event database query in get_mention_read_by.
For GET /threads/{id}/events/, this method executes a separate UserEvent query for every authored event. select_related("user") only optimizes the user join inside each query; it does not remove the N+1 pattern. Prefetch the filtered mention rows once on the events queryset (or bulk-load them through serializer context) and read that cache here.
🤖 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` around lines 1420 - 1441, Eliminate the
per-event UserEvent query in get_mention_read_by by prefetching filtered, read
mention events for all thread events in the parent events queryset, storing them
under a cache attribute. Update get_mention_read_by to consume that prefetched
cache while preserving the author-only None behavior and existing serialized
fields.
Comment on lines
+313
to
+328
| {isAuthor && event.mention_read_by && event.mention_read_by.length > 0 && ( | ||
| <div className="thread-event__read-status"> | ||
| <Tooltip | ||
| content={event.mention_read_by.map((u) => u.name).join(', ')} | ||
| placement="bottom" | ||
| > | ||
| <span className="thread-event__read-indicator"> | ||
| <Icon type={IconType.OUTLINED} size={IconSize.X_SMALL} name="done_all" aria-hidden="true" /> | ||
| {t('read_by_count', { | ||
| count: event.mention_read_by.length, | ||
| total: (event.data as ThreadEventIMData).mentions?.length ?? 0, | ||
| })} | ||
| </span> | ||
| </Tooltip> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Add coverage for the populated read-status branch.
The changed fixtures in assignment-message.test.ts and group-system-events.test.ts only set mention_read_by to null. Add component tests covering an author with readers, the non-author/null case, and the rendered X/Y count and tooltip names.
🤖 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/frontend/src/features/layouts/components/thread-view/components/thread-event/index.tsx`
around lines 313 - 328, Add component-test coverage for the read-status
rendering in the thread-event component: verify an author with populated
mention_read_by renders the X/Y count and tooltip containing reader names, while
non-authors and null mention_read_by do not render the status. Extend the
relevant assignment-message and group-system-events fixtures or tests without
changing the component behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Add read receipts for
@mentionsin internal comments (ThreadEvent typeim). Currently the author of a comment has no way to know if a mentioned colleague has read the mention — even though theUserEvent.read_attimestamp is already recorded when the reader scrolls the comment into view.Proposal
Backend: new
MentionReadByUserSerializerandmention_read_byfield onThreadEventSerializer. Returns the list of mentioned users withread_at IS NOT NULL, ornullif the requesting user is not the event author.OpenAPI: updated schema with
MentionReadByUsercomponent + field reference.Frontend:
ThreadEventcomponent shows✓ Lu par X/Yindicator with aTooltiplisting names. Only rendered when the current user is the author and at least one mention has been read.i18n: added
read_by_countkey in en-US and fr-FR.Backend
mention_read_byfieldOpenAPI schema + generated TypeScript types
Frontend UI indicator + tooltip
Translations (en, fr)
Spec file for reference
Summary by CodeRabbit
New Features
Documentation