Skip to content

Development#653

Merged
jbpenrath merged 8 commits into
mainfrom
development
May 20, 2026
Merged

Development#653
jbpenrath merged 8 commits into
mainfrom
development

Conversation

@jbpenrath

@jbpenrath jbpenrath commented May 5, 2026

Copy link
Copy Markdown
Contributor

Purpose

Intermediate branch to merge heavy features to be able to validate all of them before releasing.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added thread assignment functionality—assign users to threads and track who is assigned.
    • Added "Assigned to me" filter to easily find threads assigned to you.
    • Added visual assignee badges and quick-assign popover for managing assignments.
    • Added assignment history displayed as system events in thread timelines.
    • Added assignment statistics to thread filtering and stats endpoints.
    • Enhanced sharing modal with assignment management alongside access controls.
  • Improvements

    • Improved thread sidebar with visibility rules for "Assigned to me" and "Unassigned" folders based on mailbox type.
    • Improved API response shapes for thread and mailbox data to expose shared/identity status and assigned user information.
    • Refined IM mention and assignment event validation for better data consistency.
  • Documentation

    • Expanded permissions and data model documentation with event types and design principles for assignments and mentions.

@coderabbitai

coderabbitai Bot commented May 5, 2026

Copy link
Copy Markdown

Note

Reviews paused

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR implements thread user assignment and unassignment workflows across the full stack. It introduces ASSIGN/UNASSIGN ThreadEvent types with per-user UserEvent records, an undo-window mechanism that absorbs recent reversal attempts, automatic access-change cleanup, frontend components for managing assignees, thread filtering and statistics, and comprehensive test coverage.

Changes

Thread Assignment Feature

Layer / File(s) Summary
Data Models & Enums
src/backend/core/enums.py, src/backend/core/models.py
Added ThreadEventTypeChoices.ASSIGN/.UNASSIGN, UserEventTypeChoices.ASSIGN, MAILBOX_ROLES_CAN_BE_ASSIGNED constant. ThreadEvent schema validation refactored to classmethod validate_data(). UserEvent gains conditional uniqueness constraint on (user, thread) for type='assign'. ThreadAccessQuerySet adds editor_user_ids() and viewer_user_ids() helpers.
Database Migrations
src/backend/core/migrations/0026_userevent_usrevt_user_thread_assign_uniq.py
Conditional uniqueness constraint added to enforce at most one active ASSIGN per (user, thread).
Core Service Implementation
src/backend/core/services/thread_events.py
New service module with transactional handlers: assign_users(), unassign_users() (with 120s undo-window absorption), sync_im_mentions(), revoke_thread_access(), downgrade_thread_access(), revoke_mailbox_access(), downgrade_mailbox_access(). Handles validation, idempotency, cascading cleanup, and system-event generation.
API Serializers & Permissions
src/backend/core/api/serializers.py, src/backend/core/api/permissions.py
ThreadEventSerializer validates data via ThreadEvent.validate_data(), routes IM/ASSIGN/UNASSIGN schemas to PolymorphicProxySerializer. ThreadMentionableUserSerializer adds can_post_comments field. MailboxSerializer adds computed is_shared field and access counts cache. ThreadSerializer adds assigned_users field with prefetch optimization. ThreadAccessSerializer adds users computed field. Centralized non-author mutation guard via _is_thread_event_mutation_by_non_author().
API Viewsets
src/backend/core/api/viewsets/thread_event.py, src/backend/core/api/viewsets/thread_access.py, src/backend/core/api/viewsets/mailbox_access.py, src/backend/core/api/viewsets/thread.py, src/backend/core/api/viewsets/thread_user.py
ThreadEventViewSet.create() atomically locks thread, delegates ASSIGN/UNASSIGN to service (returns 204 on idempotency), syncs IM mentions post-update. ThreadAccessViewSet disables pagination, adds atomic update/delete with cleanup. MailboxAccessViewSet adds atomic update/delete with cleanup. ThreadViewSet adds has_assigned_to_me/has_unassigned filtering and stats. ThreadUserViewSet adds can_post_comments annotation.
OpenAPI Schema
src/backend/core/api/openapi.json
Query parameters added: has_assigned_to_me, has_unassigned on thread list/stats. Response schemas updated: Thread adds assigned_users field, ThreadAccess adds users field, Mailbox adds is_shared field. ThreadEvent.data remapped to named components (ThreadEventIMData, ThreadEventAssigneesData). New ThreadMentionableUser schema.
Admin Interface
src/backend/core/admin.py
MailboxAccessAdmin, ThreadAccessAdmin registered with explicit save/delete handlers routing to cleanup service. MailboxAdmin and ThreadAdmin gain save_formset overrides for inline edit cleanup. ThreadEventInline marked read-only.
Signal Cleanup Migration
src/backend/core/signals.py
Removed post_save mention sync logic (moved to service-layer sync_im_mentions()).
Factory Test Support
src/backend/core/factories.py
ThreadEventFactory adds @factory.post_generation hook to sync UserEvent rows, mirroring production assign/unassign/mention behaviors.
Utility Helpers
src/backend/core/utils.py
Added validate_json_schema() helper for consistent Django ValidationError raising.
Backend Tests
src/backend/core/tests/api/test_thread_event.py, src/backend/core/tests/api/test_thread_filter_assignment.py, src/backend/core/tests/api/test_thread_access.py, src/backend/core/tests/api/test_thread_user.py, src/backend/core/tests/services/test_thread_events.py, src/backend/core/tests/models/test_thread_event.py, src/backend/core/tests/models/test_user_event.py, src/backend/core/tests/api/test_threads_list.py, src/backend/core/tests/api/test_mailboxes.py, src/backend/core/tests/api/test_provisioning_mailbox.py
Comprehensive test coverage for ASSIGN/UNASSIGN creation/idempotency/undo-window, access cleanup cascading, mention sync, assignment filtering, stats computation, prefetch query optimization, and is_shared field logic.
Frontend Hooks & Context
src/frontend/src/features/message/use-assigned-users.ts, src/frontend/src/features/message/use-thread-assignment.ts, src/frontend/src/features/providers/mailbox.tsx, src/frontend/src/hooks/use-is-shared-context.ts
useAssignedUsers hook derives current assignees from event-sourced ThreadEvents. useThreadAssignment hook manages assignment mutations and per-user mutating state. Mailbox context adds invalidateThreadsList(). useIsSharedContext determines shared vs personal context.
Frontend UI Components
src/frontend/src/features/layouts/components/thread-view/components/assignees-widget/index.tsx, src/frontend/src/features/layouts/components/thread-view/components/assignees-widget/quick-assign-popover/index.tsx, src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/..., src/frontend/src/features/ui/components/assignees-avatar-group/index.tsx
AssigneesWidget displays current assignees, opens QuickAssignPopover. QuickAssignPopover autocompletes and toggles user assignments. AssigneesAvatarGroup renders overlapping avatar stack with overflow counter. ShareModal fork-extended with assignment footer, role dropdown, and per-mailbox user lists. New ShareMemberItem, AccessUsersList, AssignedUsersSection, AccessRoleDropdown, InvitationUserSelectorList components. ThreadAccessesWidget enriched with assignment integration and mutation state tracking.
Thread Timeline Rendering
src/frontend/src/features/layouts/components/thread-view/components/thread-event/..., src/frontend/src/features/layouts/components/thread-view/index.tsx
Added system-event rendering (assignment-message builder, SystemEventLine, CollapsedEventsGroup). groupSystemEvents groups consecutive non-IM system events into collapsible buckets. ThreadView refactored to use grouped render items and shared-context gating for IM input.
Thread Filtering & Display
src/frontend/src/features/layouts/components/thread-panel/..., src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-list/index.tsx
Thread-panel filters expanded with has_assigned_to_me. Filter labels, header counts, and folder visibility predicates updated. Mailbox sidebar folders use isVisible predicates for conditional rendering.
Styling
src/frontend/src/features/layouts/components/.../...scss, src/frontend/src/styles/main.scss
New SCSS for assignees-widget, quick-assign-popover, share-modal-extensions, assignees-avatar-group, system-event timeline, collapsed-events toggle, thread-view container queries, and thread action-bar layout.
Utility Functions
src/frontend/src/features/utils/date-helper.ts
Added bucketDate() and formatEventTimestamp() for relative date categorization and event timestamp formatting.
Frontend Tests
src/e2e/src/__tests__/thread-event.spec.ts, src/frontend/src/features/ui/components/assignees-avatar-group/index.test.tsx, src/frontend/src/features/layouts/components/thread-view/components/thread-event/assignment-message.test.ts, src/frontend/src/features/layouts/components/thread-view/components/thread-event/group-system-events.test.ts
E2E tests for self-assign/unassign UI and "Assigned to me" folder. Component tests for avatar group overflow, assignment message localization, and system-event grouping.
Localization
src/frontend/public/locales/common/en-US.json, src/frontend/public/locales/common/fr-FR.json
Added i18n keys for assign/unassign messages (with pronoun/plural variants), assignment UI labels, "assigned to you" search result text, sharing terminology, and localized permission explanations.
Documentation
docs/permissions.md
Updated entity-relationship diagram, key models table, and role definitions. Expanded "Key Design Principles" with comprehensive rules for permission composition, per-mailbox access, event authorization, undo window, cascading cleanup, mention reconciliation, and denormalized thread stats.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Frontend
    participant ThreadEventAPI as Thread Event API
    participant Service as thread_events Service
    participant DB as Database
    
    User->>Frontend: Assign user to thread
    Frontend->>ThreadEventAPI: POST /threads/{id}/events/<br/>(type: "assign", data: {assignees: [user_id]})
    ThreadEventAPI->>ThreadEventAPI: Lock thread row
    ThreadEventAPI->>Service: assign_users(thread, author, assignees)
    Service->>Service: Validate editor rights for assignees
    Service->>DB: Create ThreadEvent (type=assign)
    Service->>DB: Bulk-create UserEvent ASSIGN rows
    Service-->>ThreadEventAPI: Return created ThreadEvent
    ThreadEventAPI-->>Frontend: 201 + ThreadEvent payload
    Frontend->>Frontend: Update assigned users UI
    
    User->>Frontend: Unassign user within 120s
    Frontend->>ThreadEventAPI: POST /threads/{id}/events/<br/>(type: "unassign", data: {assignees: [user_id]})
    ThreadEventAPI->>ThreadEventAPI: Lock thread row
    ThreadEventAPI->>Service: unassign_users(thread, author, assignees)
    Service->>Service: Check undo window (recent ASSIGN by same author)
    Service->>DB: Delete absorbed ASSIGN's assignee data
    Service->>DB: Delete matching UserEvent ASSIGN rows
    Service-->>ThreadEventAPI: Return 204 (absorbed)
    ThreadEventAPI-->>Frontend: 204 No Content
    Frontend->>Frontend: Update timeline (no new events shown)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • sylvinus

@jbpenrath

Copy link
Copy Markdown
Contributor Author

@coderabbitai help

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 20

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
src/backend/core/api/viewsets/thread.py (1)

432-444: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

has_assigned_to_me_unread / has_unassigned_unread will FieldError at aggregation time.

has_assigned_to_me and has_unassigned are added to valid_base_fields but not to annotation_fields. As a result, a request for has_assigned_to_me_unread (or has_unassigned_unread) passes the validation in lines 450‑462, then falls into the generic _unread branch at lines 501‑506, which builds Q(has_assigned_to_me=True) — a non-existent model field/annotation (the actual annotation is _has_assigned_to_me). The aggregation will raise FieldError: Cannot resolve keyword 'has_assigned_to_me' and surface as a 500.

These are annotations like has_mention/has_unread_mention, so they belong in annotation_fields to be rejected at validation time.

🛡️ Proposed fix
-        # Base fields that cannot be combined with the "_unread" suffix because
-        # they are annotations (not real model columns) and their unread variant
-        # is either already exposed (has_unread_mention) or meaningless.
-        annotation_fields = {"has_mention", "has_unread_mention"}
+        # Base fields that cannot be combined with the "_unread" suffix because
+        # they are annotations (not real model columns) and their unread variant
+        # is either already exposed (has_unread_mention) or meaningless.
+        annotation_fields = {
+            "has_mention",
+            "has_unread_mention",
+            "has_assigned_to_me",
+            "has_unassigned",
+        }
🤖 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/viewsets/thread.py` around lines 432 - 444, Validation
currently allows requests like has_assigned_to_me_unread because
has_assigned_to_me and has_unassigned are in valid_base_fields but not in
annotation_fields; update the annotation_fields set in thread.py to include
"has_assigned_to_me" and "has_unassigned" (alongside "has_mention" and
"has_unread_mention") so those annotated flags are rejected during validation
and do not fall through to the generic "_unread" Q-building branch that expects
real model fields.
src/backend/core/api/serializers.py (1)

392-455: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Eliminate the N+1 query for count_accesses on mailbox list endpoints.

Each mailbox in the list triggers instance.accesses.count() in _get_cached_counts(), issuing a fresh COUNT query. Although prefetch_related("accesses__user", "domain") caches the accesses, .count() still queries the database separately—a Django gotcha. This adds one query per mailbox on top of the existing two (thread_accesses.aggregate(...) and UserEvent count).

get_is_shared short-circuits for non-identity mailboxes, but identity mailboxes are the common case for personal users, so the cost impacts the typical hot path. Annotate accesses_count in MailboxViewSet.get_queryset() using Count("accesses", distinct=True) and read it from the annotation with a fallback to .count() for non-annotated paths—matching the pattern already in place for _can_edit in ThreadSerializer.get_abilities().

Suggested implementation
-            counts["count_accesses"] = instance.accesses.count()
+            counts["count_accesses"] = getattr(
+                instance,
+                "_count_accesses",
+                None,
+            )
+            if counts["count_accesses"] is None:
+                counts["count_accesses"] = instance.accesses.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` around lines 392 - 455, The mailbox list
is causing an N+1 because _get_cached_counts() calls instance.accesses.count();
change this to prefer an annotation named accesses_count (add Count("accesses",
distinct=True) in MailboxViewSet.get_queryset()) and in _get_cached_counts() /
get_is_shared() read instance.accesses_count if present (fallback to
instance.accesses.count() only when the annotation is missing) so annotated list
endpoints use the cached value and avoid per-instance COUNT queries.
src/backend/core/api/openapi.json (2)

5576-5622: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

threads_events_create documents a 204 path but does not declare it in responses.

The description on Line 5576 explicitly says idempotent ASSIGN/UNASSIGN no-op cases return 204, but only 201 is declared. Generated clients may treat valid 204 responses as unexpected failures.

📌 Proposed OpenAPI fix
                 "responses": {
                     "201": {
                         "content": {
                             "application/json": {
                                 "schema": {
                                     "$ref": "#/components/schemas/ThreadEvent"
                                 }
                             }
                         },
                         "description": ""
+                    },
+                    "204": {
+                        "description": "No response body"
                     }
                 }
🤖 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/openapi.json` around lines 5576 - 5622, The OpenAPI
operation that creates ThreadEvent (referenced by the
ThreadEventRequest/ThreadEvent schemas and the "thread-events" tag) documents
that idempotent ASSIGN/UNASSIGN no-ops return 204 but only declares a 201
response; add a "204" entry to the operation's "responses" object (alongside the
existing "201") with an appropriate description (e.g., "No Content — idempotent
no-op") and no response body (or empty content) so generated clients treat 204
as a valid response for the threads_events_create endpoint.

6032-6047: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

stats_fields schema contradicts its own CSV description/examples.

On Line 6034, stats_fields is modeled as a single enum string, but Line 6046 describes comma-separated multi-values and _unread variants (for example has_starred_unread) that are not in the enum. This will generate incorrect client types and client-side validation.

Please align contract and docs: either model this as multi-value input (array with style=form, explode=false) and include all supported values, or constrain the description/examples to single enum values.

🤖 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/openapi.json` around lines 6032 - 6047, The OpenAPI
schema for the parameter "stats_fields" currently defines a single string enum
but its description and examples describe comma-separated multi-values and
_unread variants; update the "stats_fields" parameter to be an array schema
(type: array, items: {type: string, enum: [...]}) with style=form and
explode=false so it models CSV input, include all supported token values (e.g.,
all, all_unread, has_assigned_to_me, has_delivery_failed, has_delivery_pending,
has_mention, has_unread_mention, has_unassigned, plus all boolean flag tokens
and their _unread variants) in the items.enum, and adjust the description to no
longer claim “single enum string” but to document the CSV/array semantics and
examples (or alternatively, if you intend single values only, remove CSV wording
and _unread examples and keep the enum as-is).
🤖 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/admin.py`:
- Around line 777-866: The current bug is that instances added to
instances_to_revoke come from sub_form.instance (which may include unsaved
changes when DELETE is ticked) so revoke_* is applied to the wrong principal; in
both _cleanup_thread_access_formset and _cleanup_mailbox_access_formset replace
appending sub_form.instance for deleted forms with a DB snapshot (e.g. load
models.ThreadAccess.objects.filter(pk=sub_form.instance.pk).first() and
models.MailboxAccess.objects.filter(pk=sub_form.instance.pk).first()) so
revoke_thread_access/revoke_mailbox_access receive the actual pre-deletion row;
keep the rest of the flow (formset.save(), then revoke and downgrade calls)
unchanged.
- Around line 642-652: The deletion and subsequent cleanup in delete_queryset
must run in a single DB transaction: wrap the call to
super().delete_queryset(request, queryset) and the following per-row cleanup
loop that calls thread_events_service.revoke_mailbox_access(mailbox_access=obj)
inside a django.db.transaction.atomic() block so the delete and revoke occur
atomically; do the same for the other delete_queryset variant that calls
thread_events_service.revoke_thread_access(...). Ensure django.db.transaction is
imported if not already.

In `@src/backend/core/api/openapi.json`:
- Around line 9297-9306: The OpenAPI schemas for ThreadEventAssigneesData and
ThreadEventAssigneesDataRequest currently define "assignees" as an array of
ThreadEventUser but lack the non-empty constraint; update both schemas (look for
the schemas named ThreadEventAssigneesData and ThreadEventAssigneesDataRequest
and the "assignees" property) to add "minItems": 1 to the array definition so
the contract enforces at least one assignee, matching backend validation.
- Around line 5278-5281: Add a migration note entry documenting the
contract-breaking schema changes to the OpenAPI spec: describe the old→new
shapes for the affected schemas (Mailbox, Thread, ThreadAccess) and the
paginated ThreadAccess response change (paginated structure → array) and include
concrete example request/response snippets for both old and new forms, state the
target release/version (e.g., vX.Y.0) and migration guidance for clients; place
this note near the OpenAPI top-level "info" or in a new "x-migration-notes"
extension and reference the exact schema names ("Mailbox", "Thread",
"ThreadAccess") and the response location that changed from a paginated
container to an array so downstream consumers can find and adapt their
integrations.

In `@src/backend/core/api/viewsets/thread_event.py`:
- Around line 79-88: The update currently calls serializer.save() before
thread_events_service.sync_im_mentions(), risking a partial commit if syncing
fails; wrap the save and the mention reconciliation in a single DB transaction
so both succeed or both roll back: import and use Django's transaction.atomic()
inside perform_update (around serializer.save() and
thread_events_service.sync_im_mentions(thread_event=thread_event)), keep the
existing editability check and PermissionDenied raise, and ensure thread_event
is assigned inside the atomic block so any exception during sync will rollback
the saved ThreadEvent and its associated UserEvent changes.

In `@src/backend/core/services/thread_events.py`:
- Around line 316-423: The race occurs because assign_users and unassign_users
create ThreadEvent before the actual UserEvent insert/delete is serialized,
allowing concurrent requests to emit timeline events that don't reflect real DB
changes; fix by serializing on the assignment state (e.g. acquire a DB lock or
perform the mutation first) and only create the ThreadEvent if at least one
UserEvent row actually changed. Concretely: in assign_users, call and complete
the insertion logic in _create_user_event_assigns (or use
select_for_update/transactional lock on the relevant UserEvent rows or
ThreadAccess editor_user_ids) to determine which rows were newly inserted, then
create the ThreadEvent with those actual new_assignees (return None if nothing
changed). In unassign_users, perform the check-and-delete (including
_absorb_unassign_in_undo_window and _delete_user_event_assigns) under a lock or
before persisting ThreadEvent and only emit the UNASSIGN ThreadEvent when
deletes removed at least one row; adjust the code paths in assign_users,
unassign_users, _create_user_event_assigns, and _delete_user_event_assigns
accordingly.

In `@src/backend/core/tests/api/test_threads_list.py`:
- Around line 1898-1923: Add an absolute upper-bound assertion to both
test_base_fields_are_prefetched and test_labels_and_assignees_prefetched to
catch increases in the constant query cost: after computing queries_1 (via
_count_queries(api_client, user_1, mailbox_1, url)) assert it is below a chosen
MAX_QUERIES (or use pytest.assume for a soft check), e.g. assert queries_1 <
MAX_QUERIES, and keep the existing equality check against queries_5; define
MAX_QUERIES as a small constant near the current observed baseline and document
why that value was chosen.

In `@src/e2e/src/__tests__/thread-event.spec.ts`:
- Around line 609-635: The test's single `unassign` POST only changes current
assignment state but leaves prior ThreadEvent rows, so later global
toHaveCount(0) checks can fail on reruns; fix by creating and using a fresh
thread for this test or explicitly clearing the thread's event history before
asserting absence: call the thread creation or thread-events cleanup API (the
same endpoint pattern used in the test, /api/v1.0/threads/${threadId}/events/
referenced via API_URL and threadId from threadMatch) to either create a new
thread and use its id instead of the shared seed thread, or fetch and
delete/reset existing events for threadId so earlier ASSIGN/UNASSIGN rows are
removed prior to clicking Refresh and running the toHaveCount(0) assertions.

In `@src/frontend/public/locales/common/fr-FR.json`:
- Line 639: The translation for the key "Search results" is currently singular
("Résultat de la recherche"); update the value for the "Search results" JSON key
to a plural French phrase such as "Résultats de la recherche" so the UI displays
the correct plural wording for search results.

In
`@src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-list/index.tsx`:
- Around line 245-253: The useState initializer for expandedFolders only runs on
mount so changes to selectedMailbox (or its is_shared flag) aren’t reflected;
add a useEffect that watches selectedMailbox?.id and selectedMailbox?.is_shared
and recomputes the intended default (const defaultState = { inbox:
selectedMailbox?.is_shared ?? true }) then reads EXPANDED_FOLDERS_KEY from
localStorage and merges/preserves any saved overrides, and call
setExpandedFolders only when the resulting state differs from the current
expandedFolders; reference the expandedFolders/setExpandedFolders state,
EXPANDED_FOLDERS_KEY constant, and selectedMailbox in the effect to keep the
Inbox disclosure synced without clobbering user overrides.

In
`@src/frontend/src/features/layouts/components/thread-view/components/assignees-widget/index.tsx`:
- Around line 49-64: The branch that renders an interactive Button when
!canManageThreadAccess must not produce a focusable control if no onClick
handler is provided; update the rendering logic in the assignees-widget branch
(the block using canManageThreadAccess, assignedUsers, onClick,
AssigneesAvatarGroup, Button, Tooltip) to check for onClick and, if it's
missing, render a non-interactive wrapper (for example a div/span with the same
styling and aria-label/Tooltip but not focusable or clickable) instead of the
Button; if onClick exists keep the current Button behavior.

In
`@src/frontend/src/features/layouts/components/thread-view/components/assignees-widget/quick-assign-popover/_index.scss`:
- Around line 23-28: The Stylelint error is caused by missing an empty line
before the double-slash comment in the quick-assign-popover styles; open the
_index.scss block containing "flex: 1;" and the comment that starts "// Without
min-height:0 ..." and insert a single blank line immediately before that
double-slash comment so the scss/double-slash-comment-empty-line-before rule is
satisfied while keeping the existing "min-height: 0;" and related declarations
unchanged.

In
`@src/frontend/src/features/layouts/components/thread-view/components/assignees-widget/quick-assign-popover/index.tsx`:
- Around line 229-245: The empty-state branch conflates backend errors with
legitimately empty results; update the renderEmptyState in this component (the
renderEmptyState prop in index.tsx of the quick-assign-popover) to explicitly
check accessesQuery.isError before falling back to “No matching users”: if
accessesQuery.isError render a distinct error UI (e.g., a message from t('Failed
to load users') plus an actionable retry that calls accessesQuery.refetch), keep
the existing loading UI when accessesQuery.isLoading (Spinner and t('Loading
users')), and only show t('No matching users') when neither loading nor error is
true; reference accessesQuery, renderEmptyState, Spinner, t and use
accessesQuery.refetch for retry.

In
`@src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/_index.scss`:
- Line 49: Remove the empty single-line SCSS comments (`//`) used as paragraph
separators at the two occurrences and remove the stray blank line immediately
before the `position: absolute;` declaration inside the `&::before` rule in the
share-modal-extensions stylesheet; specifically, delete the bare `//` lines (the
ones causing scss/comment-no-empty) and ensure there is no blank line before
`position: absolute;` in the `&::before` block so declaration-empty-line-before
is satisfied.

In
`@src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/access-users-list.tsx`:
- Around line 66-88: The unassign flow currently disables all buttons using
mutatingUserIds.size > 0 and only the assign branch shows a Spinner, causing no
per-row feedback; update the Button disabled logic to use
mutatingUserIds.has(user.id) (so only the affected row is disabled) and add the
same row-level spinner/indicator to the unassign branch (use isMutating and
Spinner) so both onAssign and onUnassign flows render a spinner for the specific
user row; modify references in this component to canAssign, isAssigned,
onAssign, onUnassign, mutatingUserIds, isMutating, and Spinner accordingly.

In
`@src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/assigned-users-section.tsx`:
- Around line 41-45: The UserRow call currently coerces user.email to an empty
string which can cause a blank email slot to render; change the props to pass
email={user.email} (no ?? "") and make the email visibility explicit by setting
showEmail={Boolean(user.email)} (or showEmail={!!user.email}) so the component
only renders the email line when an actual email exists; update the UserRow
invocation (props fullName, email, showEmail) accordingly.

In
`@src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/share-modal.tsx`:
- Around line 230-239: Reset transient UI state when the modal is closed by
adding a useEffect that watches the modal open flag (e.g. props.isOpen or
isOpen) and clears inputValue, searchQuery and pendingInvitationUsers via
setInputValue(""), setSearchQuery(""), and setPendingInvitationUsers([])
whenever the modal transitions to closed (or whenever it opens, if you prefer
fresh state on open); update the ShareModal component (share-modal.tsx) to
include this effect so stale search text and selections are cleared between
open/close cycles.

In
`@src/frontend/src/features/layouts/components/thread-view/components/thread-accesses-widget/index.tsx`:
- Around line 180-193: The current enrichedAccesses computation drops accesses
returned by threadAccessesQuery.data if they are not found in accessesById,
causing freshly created accesses to disappear; update the logic in the
enrichedAccesses useMemo (referencing enrichedAccesses, accessesById,
threadAccessesQuery.data, and the EnrichedAccess shape) to produce a fallback
EnrichedAccess when base is missing instead of returning null — e.g., construct
an object using the access.id, access.role, access.users ?? [], and any minimal
required fields for EnrichedAccess so the new access appears immediately while
the accesses prop catches up. Ensure the fallback stays consistent with existing
objects so downstream rendering (the modal) works without requiring an immediate
refresh.

In
`@src/frontend/src/features/layouts/components/thread-view/components/thread-event/assignment-message.test.ts`:
- Around line 37-153: Add a unit test covering the system-ASSIGN edge case where
author is null so the code falls back to the legacy/Unknown author branch:
construct an event via makeEvent(ThreadEventTypeEnum.assign, null, [{ id: 'bob',
name: 'Bob' }]) and assert buildAssignmentMessage(event, SELF_ID, fakeT) returns
'Unknown assigned Bob' to pin the current behavior and prevent regressions.

---

Outside diff comments:
In `@src/backend/core/api/openapi.json`:
- Around line 5576-5622: The OpenAPI operation that creates ThreadEvent
(referenced by the ThreadEventRequest/ThreadEvent schemas and the
"thread-events" tag) documents that idempotent ASSIGN/UNASSIGN no-ops return 204
but only declares a 201 response; add a "204" entry to the operation's
"responses" object (alongside the existing "201") with an appropriate
description (e.g., "No Content — idempotent no-op") and no response body (or
empty content) so generated clients treat 204 as a valid response for the
threads_events_create endpoint.
- Around line 6032-6047: The OpenAPI schema for the parameter "stats_fields"
currently defines a single string enum but its description and examples describe
comma-separated multi-values and _unread variants; update the "stats_fields"
parameter to be an array schema (type: array, items: {type: string, enum:
[...]}) with style=form and explode=false so it models CSV input, include all
supported token values (e.g., all, all_unread, has_assigned_to_me,
has_delivery_failed, has_delivery_pending, has_mention, has_unread_mention,
has_unassigned, plus all boolean flag tokens and their _unread variants) in the
items.enum, and adjust the description to no longer claim “single enum string”
but to document the CSV/array semantics and examples (or alternatively, if you
intend single values only, remove CSV wording and _unread examples and keep the
enum as-is).

In `@src/backend/core/api/serializers.py`:
- Around line 392-455: The mailbox list is causing an N+1 because
_get_cached_counts() calls instance.accesses.count(); change this to prefer an
annotation named accesses_count (add Count("accesses", distinct=True) in
MailboxViewSet.get_queryset()) and in _get_cached_counts() / get_is_shared()
read instance.accesses_count if present (fallback to instance.accesses.count()
only when the annotation is missing) so annotated list endpoints use the cached
value and avoid per-instance COUNT queries.

In `@src/backend/core/api/viewsets/thread.py`:
- Around line 432-444: Validation currently allows requests like
has_assigned_to_me_unread because has_assigned_to_me and has_unassigned are in
valid_base_fields but not in annotation_fields; update the annotation_fields set
in thread.py to include "has_assigned_to_me" and "has_unassigned" (alongside
"has_mention" and "has_unread_mention") so those annotated flags are rejected
during validation and do not fall through to the generic "_unread" Q-building
branch that expects real model fields.
🪄 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: 923f61f6-3132-497a-939b-425af7c339af

📥 Commits

Reviewing files that changed from the base of the PR and between 5ca5932 and 5a5d15d.

⛔ Files ignored due to path filters (32)
  • src/frontend/src/features/api/gen/models/index.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/mailbox.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/mailbox_light.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/paginated_thread_access_list.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/patched_thread_event_request.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/patched_thread_event_request_data_one_of.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/patched_thread_event_request_data_one_of_mentions_item.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_access.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_assignees_data.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_assignees_data_request.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_data.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_data_one_of.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_data_one_of_mentions_item.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_data_request.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_im_data.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_im_data_request.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_request.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_request_data_one_of.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_type_enum.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_user.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_event_user_request.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_mentionable_user.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/thread_mentionable_user_custom_attributes.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/threads_accesses_list_params.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/threads_list_params.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/threads_stats_retrieve_params.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/models/threads_stats_retrieve_stats_fields.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/thread-access/thread-access.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/thread-events/thread-events.ts is excluded by !**/gen/**
  • src/frontend/src/features/api/gen/thread-users/thread-users.ts is excluded by !**/gen/**
📒 Files selected for processing (69)
  • docs/permissions.md
  • src/backend/core/admin.py
  • src/backend/core/api/openapi.json
  • src/backend/core/api/permissions.py
  • src/backend/core/api/serializers.py
  • src/backend/core/api/viewsets/mailbox_access.py
  • src/backend/core/api/viewsets/thread.py
  • src/backend/core/api/viewsets/thread_access.py
  • src/backend/core/api/viewsets/thread_event.py
  • src/backend/core/api/viewsets/thread_user.py
  • src/backend/core/enums.py
  • src/backend/core/factories.py
  • src/backend/core/migrations/0026_userevent_usrevt_user_thread_assign_uniq.py
  • src/backend/core/models.py
  • src/backend/core/services/thread_events.py
  • src/backend/core/signals.py
  • src/backend/core/tests/api/test_mailboxes.py
  • src/backend/core/tests/api/test_provisioning_mailbox.py
  • src/backend/core/tests/api/test_thread_access.py
  • src/backend/core/tests/api/test_thread_event.py
  • src/backend/core/tests/api/test_thread_filter_assignment.py
  • src/backend/core/tests/api/test_thread_user.py
  • src/backend/core/tests/api/test_threads_list.py
  • src/backend/core/tests/models/test_thread_event.py
  • src/backend/core/tests/models/test_user_event.py
  • src/backend/core/tests/services/test_thread_events.py
  • src/backend/core/utils.py
  • src/e2e/src/__tests__/thread-event.spec.ts
  • src/frontend/public/locales/common/en-US.json
  • src/frontend/public/locales/common/fr-FR.json
  • src/frontend/src/features/layouts/components/mailbox-panel/components/mailbox-list/index.tsx
  • src/frontend/src/features/layouts/components/thread-panel/components/thread-item/_index.scss
  • src/frontend/src/features/layouts/components/thread-panel/components/thread-item/index.tsx
  • src/frontend/src/features/layouts/components/thread-panel/components/thread-panel-filter.tsx
  • src/frontend/src/features/layouts/components/thread-panel/components/thread-panel-header.tsx
  • src/frontend/src/features/layouts/components/thread-panel/hooks/use-thread-panel-filters.ts
  • src/frontend/src/features/layouts/components/thread-view/_index.scss
  • src/frontend/src/features/layouts/components/thread-view/components/assignees-widget/_index.scss
  • src/frontend/src/features/layouts/components/thread-view/components/assignees-widget/index.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/assignees-widget/quick-assign-popover/_index.scss
  • src/frontend/src/features/layouts/components/thread-view/components/assignees-widget/quick-assign-popover/index.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/_index.scss
  • src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/access-role-dropdown.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/access-users-list.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/assigned-users-section.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/index.ts
  • src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/invitation-user-selector.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/share-member-item.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/share-modal-extensions/share-modal.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/thread-accesses-widget/index.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/thread-action-bar/_index.scss
  • src/frontend/src/features/layouts/components/thread-view/components/thread-action-bar/index.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/thread-event-input/_index.scss
  • src/frontend/src/features/layouts/components/thread-view/components/thread-event-input/index.tsx
  • src/frontend/src/features/layouts/components/thread-view/components/thread-event/_index.scss
  • src/frontend/src/features/layouts/components/thread-view/components/thread-event/assignment-message.test.ts
  • src/frontend/src/features/layouts/components/thread-view/components/thread-event/assignment-message.ts
  • src/frontend/src/features/layouts/components/thread-view/components/thread-event/group-system-events.test.ts
  • src/frontend/src/features/layouts/components/thread-view/components/thread-event/index.tsx
  • src/frontend/src/features/layouts/components/thread-view/index.tsx
  • src/frontend/src/features/message/use-assigned-users.ts
  • src/frontend/src/features/message/use-thread-assignment.ts
  • src/frontend/src/features/providers/mailbox.tsx
  • src/frontend/src/features/ui/components/assignees-avatar-group/_index.scss
  • src/frontend/src/features/ui/components/assignees-avatar-group/index.test.tsx
  • src/frontend/src/features/ui/components/assignees-avatar-group/index.tsx
  • src/frontend/src/features/utils/date-helper.ts
  • src/frontend/src/hooks/use-is-shared-context.ts
  • src/frontend/src/styles/main.scss
💤 Files with no reviewable changes (1)
  • src/backend/core/signals.py

Comment thread src/backend/core/admin.py
Comment thread src/backend/core/admin.py
Comment thread src/backend/core/api/openapi.json
Comment thread src/backend/core/api/openapi.json
Comment thread src/backend/core/api/viewsets/thread_access.py
@jbpenrath

Copy link
Copy Markdown
Contributor Author

@coderabbitai pause

@coderabbitai

coderabbitai Bot commented May 5, 2026

Copy link
Copy Markdown
✅ Actions performed

Reviews paused.

@suitenumerique suitenumerique deleted a comment from coderabbitai Bot May 5, 2026
@suitenumerique suitenumerique deleted a comment from coderabbitai Bot May 5, 2026
@jbpenrath
jbpenrath force-pushed the development branch 8 times, most recently from 50224a3 to cea5db7 Compare May 11, 2026 15:04
As a follow-up of the mention feature, we build upon ThreadEvent & UserEvent
models a feature to assign users to a thread.
We allow to filter mailboxe's inbox through assignation state (assigned to me, unassigned).
The thread share modal has been forked from ui-kit to be able to list users of each
mailbox and add a cta to assign them to the thread. A section above shows assigned users.
Latest fixes in the stdlib make it a more solid alternative for strict composition than Flanker. We keep Flanker for now for lenient inbound parsing. We add stronger tests and fuzzing to validate we didn't regress.
@jbpenrath
jbpenrath force-pushed the development branch 2 times, most recently from 2e249e2 to 8a18227 Compare May 20, 2026 18:47
sylvinus and others added 6 commits May 20, 2026 20:48
This allows to use S3-compatible object storage to offload blobs, making Postgres much lighter. We design for storing ~1B emails on a single instance.

We also take this opportunity to do model changes on blobs & attachments.

Migration 0027 is one-way, no going back after this one so check your backups.
* allow to render table in email

Improve email exporter to support table elements. We do not add
blocknote tool to add explicitly table but we allow user to
copy/paste it and render it properly.

* upgrade to blocknote 0.49.0

Remove a bug that prevent to use backspace in an empty block.
TypeCellOS/BlockNote#2610
To enable syncing a role to many users, we had to add a custom
Keycloak plugin.
Allow user to copy/paste a thread link with other mailbox users.
Currently, if the user copy the current url, the link is broken once
the thread has been moved from the origin folder. It is also possible
to target a message or internal message.
- Improve error management for pst

We get some pst that are unparsable by pypff. To help user to understand
that the issue is coming from the PST file, we improve the exception
raised by pst task and display a custom error message according to the
error format.

- Recover Exchange X.500-only senders during PST import

Sent items from shared mailboxes — and many internal Exchange messages —
expose every PR_SENDER_*/PR_SENT_REPRESENTING_* slot as an unresolvable
X.500 DN. compose_email then rejected the EML for lack of a valid From
address and pst_tasks silently dropped the message at debug level, so
entire folders disappeared from the import without a trace.

- Prevent duplicate messages on PST re-import

PST messages without transport_headers (drafts, locally composed items)
were reconstructed with no Message-ID at all, and Exchange/O365 exports
sometimes drop the header even on received items. With no mime_id to
key on, deliver_inbound_message skipped its dedup check and inserted
the same message on every import — and even twice within a single
import when the same message appeared in multiple Outlook folders.
Currently, user can delete/edit an internal message while
it is within the edit timeframe defined by
`MAX_THREAD_EVENT_EDIT_DELAY`. First feedbacks raises that
this limit is not relevant for deletion.
@jbpenrath
jbpenrath merged commit 7690d08 into main May 20, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants