(thread) scope starred flag per mailbox via ThreadAccess.starred_at

The starred flag was previously global (Message.is_starred + Thread.has_starred),
meaning if one mailbox starred a thread, all mailboxes would see it as starred.

This migrates starred to ThreadAccess.starred_at (DateTimeField), following the
same per-mailbox scoping pattern already established for read_at. Each mailbox
now independently controls its own starred state.
This commit is contained in:
jbpenrath
2026-03-03 19:13:06 +01:00
parent dc52f1f99a
commit 03aac79606
26 changed files with 880 additions and 247 deletions

View File

@@ -419,7 +419,7 @@ class ThreadAccessInline(admin.TabularInline):
model = models.ThreadAccess
autocomplete_fields = ("mailbox",)
readonly_fields = ("read_at",)
readonly_fields = ("read_at", "starred_at")
@admin.register(models.Thread)
@@ -441,7 +441,6 @@ class ThreadAdmin(admin.ModelAdmin):
"has_trashed",
"has_archived",
"has_draft",
"has_starred",
"has_sender",
"has_attachments",
"has_delivery_pending",
@@ -458,7 +457,6 @@ class ThreadAdmin(admin.ModelAdmin):
"has_trashed",
"has_archived",
"has_draft",
"has_starred",
"has_sender",
"has_messages",
"has_attachments",
@@ -493,7 +491,6 @@ class ThreadAdmin(admin.ModelAdmin):
"has_trashed",
"has_archived",
"has_draft",
"has_starred",
"has_attachments",
"has_sender",
"has_messages",
@@ -593,7 +590,6 @@ class MessageAdmin(admin.ModelAdmin):
list_filter = (
"is_sender",
"is_draft",
"is_starred",
"is_trashed",
"is_spam",
"is_archived",

View File

@@ -577,7 +577,7 @@ class ThreadAccessDetailSerializer(serializers.ModelSerializer):
class Meta:
model = models.ThreadAccess
fields = ["id", "mailbox", "role", "read_at"]
fields = ["id", "mailbox", "role", "read_at", "starred_at"]
read_only_fields = fields
@@ -588,6 +588,7 @@ class ThreadSerializer(serializers.ModelSerializer):
sender_names = serializers.ListField(child=serializers.CharField(), read_only=True)
user_role = serializers.SerializerMethodField(read_only=True)
has_unread = serializers.SerializerMethodField(read_only=True)
has_starred = serializers.SerializerMethodField(read_only=True)
accesses = serializers.SerializerMethodField()
labels = serializers.SerializerMethodField()
summary = serializers.CharField(read_only=True)
@@ -601,6 +602,15 @@ class ThreadSerializer(serializers.ModelSerializer):
"""
return getattr(instance, "_has_unread", False)
@extend_schema_field(serializers.BooleanField())
def get_has_starred(self, instance):
"""Return whether the thread is starred for the current mailbox.
Requires the _has_starred annotation (set by ThreadViewSet when mailbox_id is provided).
Returns False when the annotation is absent (no mailbox context).
"""
return getattr(instance, "_has_starred", False)
@extend_schema_field(ThreadAccessDetailSerializer(many=True))
def get_accesses(self, instance):
"""Return the accesses for the thread."""
@@ -898,7 +908,6 @@ class MessageSerializer(serializers.ModelSerializer):
"is_sender",
"is_draft",
"is_unread",
"is_starred",
"is_trashed",
"is_archived",
"has_attachments",

View File

@@ -14,7 +14,7 @@ from rest_framework import serializers as drf_serializers
from rest_framework.views import APIView
from core import enums, models
from core.services.search.tasks import update_threads_unread_mailboxes_task
from core.services.search.tasks import update_threads_mailbox_flags_task
from .. import permissions
@@ -51,7 +51,7 @@ class ChangeFlagView(APIView):
),
"mailbox_id": drf_serializers.UUIDField(
required=False,
help_text="Mailbox UUID. Required when flag is 'unread'.",
help_text="Mailbox UUID. Required when flag is 'unread' or 'starred'.",
),
"read_at": drf_serializers.DateTimeField(
required=False,
@@ -62,6 +62,15 @@ class ChangeFlagView(APIView):
"null means nothing has been read (all messages unread)."
),
),
"starred_at": drf_serializers.DateTimeField(
required=False,
allow_null=True,
help_text=(
"Timestamp when the thread was starred. "
"When provided with flag='starred' and value=true, sets ThreadAccess.starred_at. "
"null or value=false removes the starred flag."
),
),
},
),
responses={
@@ -164,18 +173,17 @@ class ChangeFlagView(APIView):
status=drf.status.HTTP_400_BAD_REQUEST,
)
if flag == "unread":
if not mailbox_id:
return drf.response.Response(
{"detail": "mailbox_id is required for the 'unread' flag."},
status=drf.status.HTTP_400_BAD_REQUEST,
)
if flag in ("unread", "starred") and not mailbox_id:
return drf.response.Response(
{"detail": f"mailbox_id is required for the '{flag}' flag."},
status=drf.status.HTTP_400_BAD_REQUEST,
)
if "read_at" not in request.data:
return drf.response.Response(
{"detail": "read_at is required for the 'unread' flag."},
status=drf.status.HTTP_400_BAD_REQUEST,
)
if flag == "unread" and "read_at" not in request.data:
return drf.response.Response(
{"detail": f"read_at is required for the '{flag}' flag."},
status=drf.status.HTTP_400_BAD_REQUEST,
)
current_time = timezone.now()
updated_threads = set()
@@ -185,9 +193,8 @@ class ChangeFlagView(APIView):
mailbox__accesses__user=request.user,
).values_list("thread_id", flat=True)
# Except for unread flag, user must have EDITOR access to thread to
# modify it.
if flag != "unread":
# Unread and starred are personal actions that don't require EDITOR access.
if flag not in ('unread', 'starred'):
accessible_thread_ids_qs = accessible_thread_ids_qs.filter(
role__in=enums.THREAD_ROLES_CAN_EDIT
)
@@ -217,7 +224,15 @@ class ChangeFlagView(APIView):
accessible_thread_ids_qs,
)
# --- Non-unread flags: update Message fields as before ---
if flag == "starred":
return self._handle_starred_flag(
request,
thread_ids,
mailbox_id,
accessible_thread_ids_qs,
)
# --- Non-unread/starred flags: update Message fields as before ---
if message_ids:
messages_to_update = models.Message.objects.select_related(
"thread"
@@ -228,9 +243,7 @@ class ChangeFlagView(APIView):
if messages_to_update.exists():
batch_update_data = {"updated_at": current_time}
if flag == "starred":
batch_update_data["is_starred"] = value
elif flag == "trashed":
if flag == "trashed":
batch_update_data["is_trashed"] = value
batch_update_data["trashed_at"] = (
current_time if value else None
@@ -275,9 +288,7 @@ class ChangeFlagView(APIView):
# Prepare update data for messages within these threads
batch_update_data = {"updated_at": current_time}
if flag == "starred":
batch_update_data["is_starred"] = value
elif flag == "trashed":
if flag == "trashed":
batch_update_data["is_trashed"] = value
batch_update_data["trashed_at"] = (
current_time if value else None
@@ -354,7 +365,61 @@ class ChangeFlagView(APIView):
updated_count = accesses.update(read_at=read_at)
if thread_ids_to_sync:
update_threads_unread_mailboxes_task.delay(thread_ids_to_sync)
update_threads_mailbox_flags_task.delay(thread_ids_to_sync)
return drf.response.Response(
{"success": True, "updated_threads": updated_count}
)
def _handle_starred_flag(
self,
request,
thread_ids,
mailbox_id,
accessible_thread_ids_qs,
):
"""Handle the 'starred' flag by setting ThreadAccess.starred_at.
The caller sends value=true to star and value=false to unstar.
An optional starred_at timestamp can be provided; otherwise the
current time is used when starring.
"""
value = request.data.get("value")
starred_at = request.data.get("starred_at")
if value:
# Use provided timestamp or default to now
if starred_at is not None:
parsed = parse_datetime(str(starred_at))
if parsed is None:
return drf.response.Response(
{"detail": "starred_at must be a valid ISO 8601 datetime."},
status=drf.status.HTTP_400_BAD_REQUEST,
)
starred_at = parsed
else:
starred_at = timezone.now()
else:
starred_at = None
thread_qs = models.Thread.objects.filter(
id__in=thread_ids,
).filter(
id__in=accessible_thread_ids_qs,
)
accesses = models.ThreadAccess.objects.filter(
thread__in=thread_qs,
mailbox_id=mailbox_id,
)
with transaction.atomic():
thread_ids_to_sync = [
str(tid) for tid in accesses.values_list("thread_id", flat=True)
]
updated_count = accesses.update(starred_at=starred_at)
if thread_ids_to_sync:
update_threads_mailbox_flags_task.delay(thread_ids_to_sync)
return drf.response.Response(
{"success": True, "updated_threads": updated_count}

View File

@@ -64,10 +64,10 @@ class ThreadViewSet(
"You do not have access to this mailbox."
) from e
if mailbox_id:
queryset = queryset.annotate(
_has_unread=models.ThreadAccess.thread_unread_filter(mailbox_id),
)
queryset = queryset.annotate(
_has_unread=models.ThreadAccess.thread_unread_filter(user, mailbox_id),
_has_starred=models.ThreadAccess.thread_starred_filter(user, mailbox_id),
)
if label_slug:
# Filter threads by label slug, ensuring user has access to the label's mailbox
@@ -86,11 +86,11 @@ class ThreadViewSet(
# Apply boolean filters
# These filters operate on the Thread model's boolean fields
filter_mapping = {
"has_unread": "_has_unread" if mailbox_id else None,
"has_unread": "_has_unread",
"has_trashed": "has_trashed",
"has_archived": "has_archived",
"has_draft": "has_draft",
"has_starred": "has_starred",
"has_starred": "_has_starred",
"has_sender": "has_sender",
"has_active": "has_active",
"has_messages": "has_messages",
@@ -319,12 +319,9 @@ class ThreadViewSet(
status=drf.status.HTTP_400_BAD_REQUEST,
)
# Build unread condition from per-mailbox annotation
mailbox_id = request.query_params.get("mailbox_id")
if mailbox_id:
unread_condition = Q(_has_unread=True)
else:
unread_condition = Q(pk__isnull=True) # No unread without mailbox context
# Build unread/starred conditions from annotations (always available)
unread_condition = Q(_has_unread=True)
starred_condition = Q(_has_starred=True)
aggregations = {}
for field in requested_fields:
@@ -334,6 +331,12 @@ class ThreadViewSet(
aggregations[agg_key] = Count("pk")
elif field == "all_unread":
aggregations[agg_key] = Count("pk", filter=unread_condition)
elif field == "has_starred":
aggregations[agg_key] = Count("pk", filter=starred_condition)
elif field == "has_starred_unread":
aggregations[agg_key] = Count(
"pk", filter=starred_condition & unread_condition
)
elif field.endswith("_unread"):
base_field = field[:-7]
base_condition = Q(**{base_field: True})
@@ -496,8 +499,18 @@ class ThreadViewSet(
# Get the thread IDs from the search results
thread_ids = [thread["id"] for thread in results["threads"]]
# Retrieve the actual thread objects from the database
# Build a queryset with annotations but without the default
# exclusion filters (trashed, spam) that get_queryset() applies,
# since OpenSearch already handles those filters.
threads = models.Thread.objects.filter(id__in=thread_ids)
threads = threads.annotate(
_has_unread=models.ThreadAccess.thread_unread_filter(
request.user, mailbox_id
),
_has_starred=models.ThreadAccess.thread_starred_filter(
request.user, mailbox_id
),
)
# Order the threads in the same order as the search results
thread_dict = {str(thread.id): thread for thread in threads}

View File

@@ -362,7 +362,6 @@ def _create_message_from_inbound(
sent_at=parsed_email.get("date") or timezone.now(),
is_draft=False,
is_sender=is_sender,
is_starred=False,
is_trashed=False,
is_spam=is_spam,
has_attachments=len(parsed_email.get("attachments", [])) > 0,
@@ -372,8 +371,10 @@ def _create_message_from_inbound(
# We need to set the created_at field to the date of the message
# because the inbound message is not created at the same time as the message is received
message.created_at = parsed_email.get("date") or timezone.now()
# Extract is_unread from flags (handled via ThreadAccess.read_at)
# Extract flags handled via ThreadAccess (not Message fields)
import_is_unread = message_flags.pop("is_unread", True)
import_is_starred = message_flags.pop("_starred", False)
for flag, value in message_flags.items():
if hasattr(message, flag):
setattr(message, flag, value)
@@ -383,16 +384,22 @@ def _create_message_from_inbound(
*message_flags.keys(),
]
)
# If the imported message is read, update read_at on ThreadAccess
if not import_is_unread:
access = models.ThreadAccess.objects.filter(
thread=thread, mailbox=mailbox
).first()
if access and (
# Update ThreadAccess for read/starred state
access = models.ThreadAccess.objects.filter(
thread=thread, mailbox=mailbox
).first()
if access:
update_fields = []
if not import_is_unread and (
access.read_at is None or message.created_at > access.read_at
):
access.read_at = message.created_at
access.save(update_fields=["read_at"])
update_fields.append("read_at")
if import_is_starred and access.starred_at is None:
access.starred_at = message.created_at
update_fields.append("starred_at")
if update_fields:
access.save(update_fields=update_fields)
elif is_sender:
access = models.ThreadAccess.objects.filter(
thread=thread, mailbox=mailbox

View File

@@ -0,0 +1,45 @@
"""Add starred_at to ThreadAccess, populate from Thread.has_starred, then remove legacy fields."""
from django.db import migrations, models
from django.utils import timezone
def populate_starred_at(apps, schema_editor):
"""Populate ThreadAccess.starred_at from Thread.has_starred.
All accesses on previously starred threads inherit the flag since the old
model was global (not per-mailbox).
"""
ThreadAccess = apps.get_model("core", "ThreadAccess")
ThreadAccess.objects.filter(thread__has_starred=True).update(
starred_at=timezone.now()
)
class Migration(migrations.Migration):
dependencies = [
("core", "0019_remove_message_read_at"),
]
operations = [
# 1. Add starred_at field
migrations.AddField(
model_name="threadaccess",
name="starred_at",
field=models.DateTimeField(
blank=True, null=True, verbose_name="starred at"
),
),
# 2. Populate starred_at from has_starred
migrations.RunPython(populate_starred_at, migrations.RunPython.noop),
# 3. Remove legacy fields
migrations.RemoveField(
model_name="thread",
name="has_starred",
),
migrations.RemoveField(
model_name="message",
name="is_starred",
),
]

View File

@@ -18,7 +18,7 @@ from django.contrib.auth.base_user import AbstractBaseUser
from django.core import validators
from django.core.exceptions import ValidationError
from django.db import models, transaction
from django.db.models import Case, F, Q, Value, When
from django.db.models import Case, Exists, F, Q, Value, When
from django.db.models.fields import BooleanField
from django.utils import timezone
from django.utils.html import escape
@@ -764,7 +764,6 @@ class Thread(BaseModel):
)
has_archived = models.BooleanField("has archived", default=False)
has_draft = models.BooleanField("has draft", default=False)
has_starred = models.BooleanField("has starred", default=False)
has_sender = models.BooleanField("has sender", default=False)
has_messages = models.BooleanField("has messages", default=True)
has_attachments = models.BooleanField("has attachments", default=False)
@@ -840,7 +839,6 @@ class Thread(BaseModel):
.values(
"is_trashed",
"is_draft",
"is_starred",
"is_sender",
"is_spam",
"is_archived",
@@ -857,7 +855,6 @@ class Thread(BaseModel):
self.is_trashed = False
self.has_archived = False
self.has_draft = False
self.has_starred = False
self.has_sender = False
self.has_messages = False
self.has_attachments = False
@@ -880,9 +877,6 @@ class Thread(BaseModel):
self.has_draft = any(
msg["is_draft"] and not msg["is_trashed"] for msg in message_data
)
self.has_starred = any(
msg["is_starred"] and not msg["is_trashed"] for msg in message_data
)
self.has_sender = any(
msg["is_sender"] and not msg["is_trashed"] and not msg["is_draft"]
for msg in message_data
@@ -1021,7 +1015,6 @@ class Thread(BaseModel):
"is_trashed",
"has_archived",
"has_draft",
"has_starred",
"has_sender",
"has_messages",
"has_attachments",
@@ -1258,6 +1251,7 @@ class ThreadAccess(BaseModel):
default=ThreadAccessRoleChoices.VIEWER,
)
read_at = models.DateTimeField("read at", null=True, blank=True)
starred_at = models.DateTimeField("starred at", null=True, blank=True)
class Meta:
db_table = "messages_threadaccess"
@@ -1269,26 +1263,39 @@ class ThreadAccess(BaseModel):
return f"{self.thread} - {self.mailbox} - {self.role}"
@staticmethod
def thread_unread_filter(mailbox_id):
"""Return a `Case` expression to annotate `_has_unread` on a Thread queryset.
def thread_unread_filter(user, mailbox_id=None):
"""Return an expression to annotate `_has_unread` on a Thread queryset.
Intended for `ThreadViewSet.get_queryset()`. Uses `active_messaged_at`
so that threads are only marked unread when they contain received messages.
When `mailbox_id` is provided, scopes to that single mailbox (Case expression).
Otherwise, checks across all the user's mailboxes (Exists subquery).
"""
return Case(
When(
accesses__mailbox_id=mailbox_id,
accesses__read_at__isnull=True,
has_active=True,
then=Value(True),
),
When(
accesses__mailbox_id=mailbox_id,
active_messaged_at__gt=F("accesses__read_at"),
then=Value(True),
),
default=Value(False),
output_field=BooleanField(),
if mailbox_id:
return Case(
When(
accesses__mailbox_id=mailbox_id,
accesses__read_at__isnull=True,
has_active=True,
then=Value(True),
),
When(
accesses__mailbox_id=mailbox_id,
active_messaged_at__gt=F("accesses__read_at"),
then=Value(True),
),
default=Value(False),
output_field=BooleanField(),
)
return Exists(
ThreadAccess.objects.filter(
thread=models.OuterRef("pk"),
mailbox__accesses__user=user,
).filter(
Q(read_at__isnull=True, thread__has_active=True)
| Q(read_at__lt=F("thread__active_messaged_at"))
)
)
@staticmethod
@@ -1302,6 +1309,42 @@ class ThreadAccess(BaseModel):
read_at__lt=F("thread__active_messaged_at")
)
@staticmethod
def thread_starred_filter(user, mailbox_id=None):
"""Return an expression to annotate `_has_starred` on a Thread queryset.
Intended for `ThreadViewSet.get_queryset()`. A thread is starred when
the corresponding ThreadAccess has a non-null `starred_at`.
When `mailbox_id` is provided, scopes to that single mailbox (Case expression).
Otherwise, checks across all the user's mailboxes (Exists subquery).
"""
if mailbox_id:
return Case(
When(
accesses__mailbox_id=mailbox_id,
accesses__starred_at__isnull=False,
then=Value(True),
),
default=Value(False),
output_field=BooleanField(),
)
return Exists(
ThreadAccess.objects.filter(
thread=models.OuterRef("pk"),
mailbox__accesses__user=user,
starred_at__isnull=False,
)
)
@staticmethod
def starred_filter():
"""Return a `Q` for filtering a `ThreadAccess` queryset to starred entries.
Used by `compute_starred_mailboxes()` in the search index.
"""
return Q(starred_at__isnull=False)
class Contact(BaseModel):
"""Contact model to store contact information."""
@@ -1418,7 +1461,6 @@ class Message(BaseModel):
# Flags
is_draft = models.BooleanField("is draft", default=False)
is_sender = models.BooleanField("is sender", default=False)
is_starred = models.BooleanField("is starred", default=False)
is_trashed = models.BooleanField("is trashed", default=False)
is_spam = models.BooleanField("is spam", default=False)
is_archived = models.BooleanField("is archived", default=False)

View File

@@ -466,7 +466,13 @@ def export_mailbox_task(self, mailbox_id: str, user_id: str) -> Dict[str, Any]:
thread_id=OuterRef("thread_id"),
mailbox_id=mailbox_id,
).values("read_at")[:1]
)
),
_starred_at=Subquery(
ThreadAccess.objects.filter(
thread_id=OuterRef("thread_id"),
mailbox_id=mailbox_id,
).values("starred_at")[:1]
),
)
.select_related("blob", "thread")
.prefetch_related("thread__labels")
@@ -537,12 +543,16 @@ def export_mailbox_task(self, mailbox_id: str, user_id: str) -> Dict[str, Any]:
read_at = getattr(msg, "_read_at", None)
is_unread = read_at is None or msg.created_at > read_at
# Compute starred from ThreadAccess.starred_at
starred_at = getattr(msg, "_starred_at", None)
is_starred = starred_at is not None
# Create MBOX entry with metadata and write to stream
mbox_entry = _create_mbox_entry(
raw_content,
msg.created_at or datetime.now(timezone.utc),
is_unread=is_unread,
is_starred=msg.is_starred,
is_starred=is_starred,
is_draft=msg.is_draft,
is_sender=msg.is_sender,
labels=thread_labels,

View File

@@ -25,10 +25,10 @@ IMAP_LABEL_TO_MESSAGE_FLAG = {
"INBOX.INBOX.Sent": "is_sender",
"Archived": "is_archived",
"Messages archivés": "is_archived",
"Starred": "is_starred",
"[Gmail]/Starred": "is_starred",
"[Gmail]/Suivis": "is_starred",
"Favoris": "is_starred",
"Starred": "_starred",
"[Gmail]/Starred": "_starred",
"[Gmail]/Suivis": "_starred",
"Favoris": "_starred",
"Trash": "is_trashed",
"TRASH": "is_trashed",
"[Gmail]/Corbeille": "is_trashed",
@@ -106,7 +106,7 @@ def compute_labels_and_flags(
# Handle \\Flagged flag (follow-up / starred)
if "\\Flagged" in imap_flags:
message_flags["is_starred"] = True
message_flags["_starred"] = True
# Special case: if message is sender or draft, it should not be unread
if message_flags.get("is_sender") or message_flags.get("is_draft"):
@@ -128,10 +128,36 @@ def handle_duplicate_message(
parsed_email, imap_labels, imap_flags
)
# Extract flags handled via ThreadAccess (not Message fields)
import_is_unread = message_flags.pop("is_unread", True)
import_is_starred = message_flags.pop("_starred", False)
for flag, value in message_flags.items():
if hasattr(existing_message, flag):
setattr(existing_message, flag, value)
existing_message.save(update_fields=message_flags.keys())
if message_flags:
existing_message.save(update_fields=message_flags.keys())
# Update ThreadAccess.starred_at if the duplicate is starred
if not import_is_unread or import_is_starred:
access = models.ThreadAccess.objects.filter(
thread=existing_message.thread, mailbox=mailbox
).first()
if access:
update_fields = []
if not import_is_unread and (
access.read_at is None or existing_message.created_at > access.read_at
):
access.read_at = existing_message.created_at
update_fields.append("read_at")
if import_is_starred and (
access.starred_at is None
or existing_message.created_at > access.starred_at
):
access.starred_at = existing_message.created_at
update_fields.append("starred_at")
if update_fields:
access.save(update_fields=update_fields)
for label in labels:
try:

View File

@@ -9,7 +9,7 @@ from core.services.search.index import (
reindex_all,
reindex_mailbox,
reindex_thread,
update_thread_unread_mailboxes,
update_thread_mailbox_flags,
)
from core.services.search.mapping import MESSAGE_INDEX, MESSAGE_MAPPING
from core.services.search.parse import parse_search_query
@@ -29,7 +29,7 @@ __all__ = [
"reindex_all",
"reindex_mailbox",
"reindex_thread",
"update_thread_unread_mailboxes",
"update_thread_mailbox_flags",
# Parsing
"parse_search_query",
# Searching

View File

@@ -135,7 +135,6 @@ def index_message(message: models.Message) -> bool:
"is_trashed": message.is_trashed,
"is_archived": message.is_archived,
"is_spam": message.is_spam,
"is_starred": message.is_starred,
"is_sender": message.is_sender,
}
@@ -169,9 +168,24 @@ def compute_unread_mailboxes(thread: models.Thread) -> list:
]
def update_thread_unread_mailboxes(thread: models.Thread) -> bool:
"""Re-index the thread parent document to update unread_mailboxes.
def compute_starred_mailboxes(thread: models.Thread) -> list:
"""Return mailbox IDs for which the thread is starred.
Delegates to ``ThreadAccess.starred_filter()`` so that the definition of
"starred" stays consistent with ``ThreadViewSet``.
"""
return [
str(mid)
for mid in thread.accesses.filter(
models.ThreadAccess.starred_filter()
).values_list("mailbox_id", flat=True)
]
def update_thread_mailbox_flags(thread: models.Thread) -> bool:
"""Re-index the thread parent document to update mailbox-scoped flags.
Updates both ``unread_mailboxes`` and ``starred_mailboxes``.
Uses full document replacement (es.index) instead of partial update
(es.update) because partial updates don't work reliably with join field
documents in OpenSearch.
@@ -184,6 +198,7 @@ def update_thread_unread_mailboxes(thread: models.Thread) -> bool:
"subject": thread.subject,
"mailbox_ids": [str(mid) for mid in mailbox_ids],
"unread_mailboxes": compute_unread_mailboxes(thread),
"starred_mailboxes": compute_starred_mailboxes(thread),
}
try:
# pylint: disable=no-value-for-parameter
@@ -191,7 +206,7 @@ def update_thread_unread_mailboxes(thread: models.Thread) -> bool:
return True
# pylint: disable=broad-exception-caught
except Exception as e:
logger.error("Error updating unread_mailboxes for thread %s: %s", thread.id, e)
logger.error("Error updating mailbox flags for thread %s: %s", thread.id, e)
return False
@@ -209,6 +224,7 @@ def index_thread(thread: models.Thread) -> bool:
"subject": thread.subject,
"mailbox_ids": [str(mailbox_id) for mailbox_id in mailbox_ids],
"unread_mailboxes": compute_unread_mailboxes(thread),
"starred_mailboxes": compute_starred_mailboxes(thread),
}
try:

View File

@@ -34,6 +34,7 @@ MESSAGE_MAPPING = {
"thread_id": {"type": "keyword"},
"mailbox_ids": {"type": "keyword"},
"unread_mailboxes": {"type": "keyword"},
"starred_mailboxes": {"type": "keyword"},
# Message fields
"message_id": {"type": "keyword"},
"mime_id": {"type": "keyword"},
@@ -113,7 +114,6 @@ MESSAGE_MAPPING = {
"is_trashed": {"type": "boolean"},
"is_archived": {"type": "boolean"},
"is_spam": {"type": "boolean"},
"is_starred": {"type": "boolean"},
"is_sender": {"type": "boolean"},
},
},

View File

@@ -21,7 +21,7 @@ def parse_search_query(query: str) -> Dict[str, Any]:
- in:spam (dans:spam) - in spam
- in:sent (dans:envoyes or dans:envoyés) - sent items
- in:drafts (dans:brouillons) - drafts
- is:starred (est:suivi) - starred
- is:starred (is:starred, est:suivi) - starred
- is:read (est:lu) - read
- is:unread (est:nonlu) - unread
@@ -52,7 +52,7 @@ def parse_search_query(query: str) -> Dict[str, Any]:
"in_archives": ["in:archives", "dans:archives", "dans:archivés", "in:archief"],
"in_spam": ["in:spam", "dans:spam"],
"in_drafts": ["in:drafts", "dans:brouillons", "in:concepten"],
"is_starred": ["is:starred", "est:suivi", "is:gemarkeerd"],
"is_starred": ["is:starred", "est:suivi", "is:gevolgd"],
"is_read_true": ["is:read", "est:lu", "is:gelezen"],
"is_read_false": ["is:unread", "est:nonlu", "is:ongelezen"],
}

View File

@@ -183,9 +183,14 @@ def search_threads( # pylint: disable=too-many-branches
search_body["query"]["bool"]["filter"].append({"term": {"is_spam": False}})
# Add is: filters (starred, read, unread)
if parsed_query.get("is_starred", False):
if parsed_query.get("is_starred", False) and mailbox_ids:
search_body["query"]["bool"]["filter"].append(
{"term": {"is_starred": True}}
{
"has_parent": {
"parent_type": "thread",
"query": {"terms": {"starred_mailboxes": mailbox_ids}},
}
}
)
if parsed_query.get("is_read") is not None and mailbox_ids:
@@ -222,6 +227,26 @@ def search_threads( # pylint: disable=too-many-branches
# Add other filters if provided
if filters:
# Handle mailbox-scoped filters that use has_parent queries
# instead of legacy term fields that no longer exist in the index
filter_starred = filters.pop("is_starred", None)
if filter_starred is not None and mailbox_ids:
starred_query = {"terms": {"starred_mailboxes": mailbox_ids}}
if not filter_starred:
starred_query = {"bool": {"must_not": starred_query}}
search_body["query"]["bool"]["filter"].append(
{"has_parent": {"parent_type": "thread", "query": starred_query}}
)
filter_unread = filters.pop("is_unread", None)
if filter_unread is not None and mailbox_ids:
unread_query = {"terms": {"unread_mailboxes": mailbox_ids}}
if not filter_unread:
unread_query = {"bool": {"must_not": unread_query}}
search_body["query"]["bool"]["filter"].append(
{"has_parent": {"parent_type": "thread", "query": unread_query}}
)
for field, value in filters.items():
search_body["query"]["bool"]["filter"].append({"term": {field: value}})

View File

@@ -12,7 +12,7 @@ from core.services.search import (
delete_index,
index_message,
index_thread,
update_thread_unread_mailboxes,
update_thread_mailbox_flags,
)
from messages.celery_app import app as celery_app
@@ -115,8 +115,8 @@ def reindex_thread_task(self, thread_id):
@celery_app.task(bind=True)
def update_threads_unread_mailboxes_task(self, thread_ids):
"""Update unread_mailboxes for multiple threads in OpenSearch."""
def update_threads_mailbox_flags_task(self, thread_ids):
"""Update mailbox-scoped flags (unread_mailboxes, starred_mailboxes) in OpenSearch."""
if not settings.OPENSEARCH_INDEX_THREADS:
logger.info("OpenSearch thread indexing is disabled.")
return {"success": False, "reason": "disabled"}
@@ -127,7 +127,7 @@ def update_threads_unread_mailboxes_task(self, thread_ids):
for thread_id in thread_ids:
try:
thread = models.Thread.objects.get(id=thread_id)
success = update_thread_unread_mailboxes(thread)
success = update_thread_mailbox_flags(thread)
results.append({"thread_id": thread_id, "success": success})
except models.Thread.DoesNotExist:
logger.error("Thread %s does not exist", thread_id)

View File

@@ -4,6 +4,7 @@
import logging
from django.conf import settings
from django.db import transaction
from django.db.models.signals import post_delete, post_save, pre_delete
from django.dispatch import receiver
@@ -16,7 +17,7 @@ from core.services.search import MESSAGE_INDEX, get_opensearch_client
from core.services.search.tasks import (
index_message_task,
reindex_thread_task,
update_threads_unread_mailboxes_task,
update_threads_mailbox_flags_task,
)
from core.utils import ThreadStatsUpdateDeferrer
@@ -242,17 +243,22 @@ def delete_orphan_draft_attachments(sender, instance, **kwargs):
@receiver(post_save, sender=models.ThreadAccess)
def update_unread_mailboxes_on_access_save(sender, instance, created, **kwargs):
"""Update unread_mailboxes in OpenSearch when ThreadAccess.read_at changes."""
def update_mailbox_flags_on_access_save(sender, instance, created, **kwargs):
"""Update mailbox flags in OpenSearch when ThreadAccess read/starred state changes."""
if not getattr(settings, "OPENSEARCH_INDEX_THREADS", False):
return
update_fields = kwargs.get("update_fields")
if update_fields is not None and "read_at" not in update_fields:
if update_fields is not None and not (
{"read_at", "starred_at"} & set(update_fields)
):
return
thread_id = str(instance.thread_id)
try:
update_threads_unread_mailboxes_task.delay([str(instance.thread_id)])
transaction.on_commit(
lambda tid=thread_id: update_threads_mailbox_flags_task.delay([tid])
)
# pylint: disable=broad-exception-caught
except Exception as e:
logger.exception(
@@ -268,8 +274,11 @@ def update_unread_mailboxes_on_access_delete(sender, instance, **kwargs):
if not getattr(settings, "OPENSEARCH_INDEX_THREADS", False):
return
thread_id = str(instance.thread_id)
try:
update_threads_unread_mailboxes_task.delay([str(instance.thread_id)])
transaction.on_commit(
lambda tid=thread_id: update_threads_mailbox_flags_task.delay([tid])
)
# pylint: disable=broad-exception-caught
except Exception as e:
logger.exception(

View File

@@ -115,7 +115,7 @@ class TestApiDraftAndSendMessage:
assert draft_message.is_draft is True
assert draft_message.is_sender is True
assert draft_message.is_trashed is False
assert draft_message.is_starred is False
assert draft_message.has_attachments is False
assert draft_message.sent_at is None
assert draft_message.draft_blob.get_content().decode("utf-8") == draft_content
@@ -132,7 +132,7 @@ class TestApiDraftAndSendMessage:
assert draft_message.thread.has_messages is True
assert draft_message.thread.has_sender is False
assert draft_message.thread.has_trashed is False
assert draft_message.thread.has_starred is False
assert draft_message.thread.has_attachments is False
assert draft_message.thread.has_draft is True
assert draft_message.thread.sender_names == [draft_message.sender.name]
@@ -190,14 +190,14 @@ class TestApiDraftAndSendMessage:
assert sent_message.is_draft is False
assert sent_message.is_sender is True
assert sent_message.is_trashed is False
assert sent_message.is_starred is False
assert sent_message.sent_at is not None
# Assert the thread is updated
assert sent_message.thread.has_messages is True
assert sent_message.thread.has_sender is True
assert sent_message.thread.has_trashed is False
assert sent_message.thread.has_starred is False
assert sent_message.thread.has_draft is False
assert sent_message.thread.sender_names == [sent_message.sender.name]
assert sent_message.thread.messaged_at is not None
@@ -309,14 +309,14 @@ class TestApiDraftAndSendMessage:
assert draft_message.is_draft is True
assert draft_message.is_sender is True
assert draft_message.is_trashed is False
assert draft_message.is_starred is False
assert draft_message.has_attachments is False
assert draft_message.draft_blob.get_content().decode("utf-8") == draft_content
assert draft_message.thread.has_messages is True
assert draft_message.thread.has_sender is False
assert draft_message.thread.has_trashed is False
assert draft_message.thread.has_starred is False
assert draft_message.thread.has_attachments is False
assert draft_message.thread.has_draft is True
assert draft_message.thread.sender_names == [
@@ -352,14 +352,14 @@ class TestApiDraftAndSendMessage:
assert sent_message.is_draft is False
assert sent_message.is_sender is True
assert sent_message.is_trashed is False
assert sent_message.is_starred is False
assert sent_message.sent_at is not None
# Assert the thread is updated
assert sent_message.thread.has_messages is True
assert sent_message.thread.has_sender is True
assert sent_message.thread.has_trashed is False
assert sent_message.thread.has_starred is False
assert sent_message.thread.has_draft is False
assert sent_message.thread.sender_names == [
message.sender.name,

View File

@@ -342,9 +342,7 @@ def test_api_flag_read_at_syncs_opensearch(api_client, settings):
"read_at": timezone.now().isoformat(),
}
with patch(
"core.api.viewsets.flag.update_threads_unread_mailboxes_task"
) as mock_task:
with patch("core.api.viewsets.flag.update_threads_mailbox_flags_task") as mock_task:
response = api_client.post(API_URL, data=data, format="json")
assert response.status_code == status.HTTP_200_OK
@@ -517,71 +515,137 @@ def test_api_flag_mark_messages_invalid_requests(api_client, data):
assert response.status_code == status.HTTP_400_BAD_REQUEST
# --- Tests for Starred Flag ---
# --- Tests for Starred Flag (operates on ThreadAccess.starred_at) ---
def test_api_flag_mark_messages_starred_success(api_client):
"""Test marking messages as starred successfully."""
def test_api_flag_starred_requires_mailbox_id(api_client):
"""Test that starring requires a mailbox_id."""
user = UserFactory()
api_client.force_authenticate(user=user)
data = {
"flag": "starred",
"value": True,
"thread_ids": ["00000000-0000-0000-0000-000000000001"],
}
response = api_client.post(API_URL, data=data, format="json")
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert "mailbox_id" in response.data["detail"]
@patch("core.api.viewsets.flag.update_threads_mailbox_flags_task")
def test_api_flag_mark_thread_starred_success(mock_task, api_client):
"""Test starring a thread sets starred_at on the ThreadAccess."""
user = UserFactory()
api_client.force_authenticate(user=user)
mailbox = MailboxFactory(users_read=[user])
thread = ThreadFactory()
ThreadAccessFactory(
mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR
access = ThreadAccessFactory(
mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.VIEWER
)
msg1 = MessageFactory(thread=thread, is_starred=False)
msg2 = MessageFactory(thread=thread, is_starred=True) # Already starred
MessageFactory(thread=thread)
thread.refresh_from_db()
thread.update_stats()
assert thread.has_starred is True
assert access.starred_at is None
message_ids = [str(msg1.id)]
data = {"flag": "starred", "value": True, "message_ids": message_ids}
data = {
"flag": "starred",
"value": True,
"thread_ids": [str(thread.id)],
"mailbox_id": str(mailbox.id),
}
response = api_client.post(API_URL, data=data, format="json")
assert response.status_code == status.HTTP_200_OK
assert response.data["updated_threads"] == 1
msg1.refresh_from_db()
msg2.refresh_from_db()
assert msg1.is_starred is True
assert msg2.is_starred is True
thread.refresh_from_db()
assert thread.has_starred is True
access.refresh_from_db()
assert access.starred_at is not None
mock_task.delay.assert_called_once()
def test_api_flag_mark_messages_unstarred_success(api_client):
"""Test marking messages as unstarred successfully."""
@patch("core.api.viewsets.flag.update_threads_mailbox_flags_task")
def test_api_flag_mark_thread_unstarred_success(mock_task, api_client):
"""Test unstarring a thread clears starred_at on the ThreadAccess."""
user = UserFactory()
api_client.force_authenticate(user=user)
mailbox = MailboxFactory(users_read=[user])
thread = ThreadFactory()
ThreadAccessFactory(
mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.EDITOR
access = ThreadAccessFactory(
mailbox=mailbox,
thread=thread,
role=enums.ThreadAccessRoleChoices.VIEWER,
starred_at=timezone.now(),
)
msg1 = MessageFactory(thread=thread, is_starred=True)
msg2 = MessageFactory(thread=thread, is_starred=False) # Already unstarred
MessageFactory(thread=thread)
thread.refresh_from_db()
thread.update_stats()
assert thread.has_starred is True
assert access.starred_at is not None
message_ids = [str(msg1.id)]
data = {"flag": "starred", "value": False, "message_ids": message_ids}
data = {
"flag": "starred",
"value": False,
"thread_ids": [str(thread.id)],
"mailbox_id": str(mailbox.id),
}
response = api_client.post(API_URL, data=data, format="json")
assert response.status_code == status.HTTP_200_OK
assert response.data["updated_threads"] == 1
msg1.refresh_from_db()
msg2.refresh_from_db()
assert msg1.is_starred is False
assert msg2.is_starred is False
access.refresh_from_db()
assert access.starred_at is None
mock_task.delay.assert_called_once()
thread.refresh_from_db()
assert thread.has_starred is False
def test_api_flag_starred_scoped_to_mailbox(api_client):
"""Test that starring via mailbox A does not affect mailbox B."""
user = UserFactory()
api_client.force_authenticate(user=user)
mailbox_a = MailboxFactory(users_read=[user])
mailbox_b = MailboxFactory(users_read=[user])
thread = ThreadFactory()
access_a = ThreadAccessFactory(mailbox=mailbox_a, thread=thread)
access_b = ThreadAccessFactory(mailbox=mailbox_b, thread=thread)
MessageFactory(thread=thread)
data = {
"flag": "starred",
"value": True,
"thread_ids": [str(thread.id)],
"mailbox_id": str(mailbox_a.id),
}
response = api_client.post(API_URL, data=data, format="json")
assert response.status_code == status.HTTP_200_OK
access_a.refresh_from_db()
access_b.refresh_from_db()
assert access_a.starred_at is not None
assert access_b.starred_at is None
def test_api_flag_viewer_can_star_thread(api_client):
"""Test that a VIEWER can star a thread (personal action)."""
user = UserFactory()
api_client.force_authenticate(user=user)
mailbox = MailboxFactory(users_read=[user])
thread = ThreadFactory()
access = ThreadAccessFactory(
mailbox=mailbox, thread=thread, role=enums.ThreadAccessRoleChoices.VIEWER
)
MessageFactory(thread=thread)
data = {
"flag": "starred",
"value": True,
"thread_ids": [str(thread.id)],
"mailbox_id": str(mailbox.id),
}
response = api_client.post(API_URL, data=data, format="json")
assert response.status_code == status.HTTP_200_OK
access.refresh_from_db()
assert access.starred_at is not None
# --- Tests for Trashed Flag ---

View File

@@ -155,9 +155,11 @@ def test_api_import_labels_import_mbox_with_labels_and_flags(
assert draft_access.read_at >= draft_message.created_at
assert not models.Label.objects.filter(name="Draft").exists()
# Test starred message
starred_message = messages.filter(is_starred=True).first()
assert starred_message is not None
# Test starred message (starred is now on ThreadAccess, not Message)
starred_access = models.ThreadAccess.objects.filter(
starred_at__isnull=False, mailbox=mailbox
).first()
assert starred_access is not None
assert not models.Label.objects.filter(name="Starred").exists()
# Test archived message

View File

@@ -160,9 +160,11 @@ def test_api_import_labels_french_import_mbox_with_labels_and_flags(
assert draft_access.read_at >= draft_message.created_at
assert not models.Label.objects.filter(name="Brouillons").exists()
# Test starred message with "Favoris" label
starred_message = messages.filter(is_starred=True).first()
assert starred_message is not None
# Test starred message (starred is now on ThreadAccess, not Message)
starred_access = models.ThreadAccess.objects.filter(
starred_at__isnull=False, mailbox=mailbox
).first()
assert starred_access is not None
assert not models.Label.objects.filter(name="Favoris").exists()
# Test archived message with "Messages archivés" label

View File

@@ -204,43 +204,85 @@ def test_list_threads_filter_has_trashed(api_client):
def test_list_threads_filter_has_starred(api_client):
"""Test filtering threads by has_starred=1."""
"""Test filtering threads by has_starred=1 (mailbox-scoped via ThreadAccess.starred_at)."""
user = UserFactory()
api_client.force_authenticate(user=user)
mailbox = MailboxFactory(users_read=[user])
# Thread 1: Has starred messages
# Thread 1: Starred for this mailbox
thread1 = ThreadFactory()
ThreadAccessFactory(
mailbox=mailbox,
thread=thread1,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
MessageFactory(thread=thread1, is_starred=True)
# Thread 2: No starred messages
MessageFactory(thread=thread1)
# Thread 2: Not starred
thread2 = ThreadFactory()
ThreadAccessFactory(
mailbox=mailbox,
thread=thread2,
role=enums.ThreadAccessRoleChoices.EDITOR,
)
MessageFactory(thread=thread2, is_starred=False)
MessageFactory(thread=thread2)
thread1.update_stats()
thread2.update_stats()
response = api_client.get(API_URL, {"has_starred": "1"})
response = api_client.get(
API_URL, {"has_starred": "1", "mailbox_id": str(mailbox.id)}
)
assert response.status_code == status.HTTP_200_OK
assert response.data["count"] == 1
assert response.data["results"][0]["id"] == str(thread1.id)
response = api_client.get(API_URL, {"has_starred": "0"})
response = api_client.get(
API_URL, {"has_starred": "0", "mailbox_id": str(mailbox.id)}
)
assert response.status_code == status.HTTP_200_OK
assert response.data["count"] == 1
assert response.data["results"][0]["id"] == str(thread2.id)
# Shared thread: starred for mailbox but not for mailbox2
mailbox2 = MailboxFactory(users_read=[user])
shared_thread = ThreadFactory()
ThreadAccessFactory(
mailbox=mailbox,
thread=shared_thread,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
ThreadAccessFactory(
mailbox=mailbox2,
thread=shared_thread,
role=enums.ThreadAccessRoleChoices.EDITOR,
)
MessageFactory(thread=shared_thread)
# mailbox should see the shared thread as starred
response = api_client.get(
API_URL, {"has_starred": "1", "mailbox_id": str(mailbox.id)}
)
assert response.status_code == status.HTTP_200_OK
starred_ids = {r["id"] for r in response.data["results"]}
assert str(shared_thread.id) in starred_ids
# mailbox2 should NOT see the shared thread as starred
response = api_client.get(
API_URL, {"has_starred": "1", "mailbox_id": str(mailbox2.id)}
)
assert response.status_code == status.HTTP_200_OK
starred_ids = {r["id"] for r in response.data["results"]}
assert str(shared_thread.id) not in starred_ids
# mailbox2 should see the shared thread as unstarred
response = api_client.get(
API_URL, {"has_starred": "0", "mailbox_id": str(mailbox2.id)}
)
assert response.status_code == status.HTTP_200_OK
unstarred_ids = {r["id"] for r in response.data["results"]}
assert str(shared_thread.id) in unstarred_ids
def test_list_threads_filter_combined(api_client):
"""Test filtering threads by combining filters."""
"""Test filtering threads by combining filters (starred is mailbox-scoped)."""
user = UserFactory()
api_client.force_authenticate(user=user)
mailbox = MailboxFactory(users_read=[user])
@@ -251,36 +293,34 @@ def test_list_threads_filter_combined(api_client):
thread=thread1,
role=enums.ThreadAccessRoleChoices.EDITOR,
)
MessageFactory(thread=thread1, is_starred=False, is_trashed=False)
# Thread 2: Has trashed message (starred message is trashed, so has_starred=False)
MessageFactory(thread=thread1, is_trashed=False)
# Thread 2: Has trashed message, not starred
thread2 = ThreadFactory()
ThreadAccessFactory(
mailbox=mailbox,
thread=thread2,
role=enums.ThreadAccessRoleChoices.EDITOR,
)
MessageFactory(thread=thread2, is_starred=True, is_trashed=True)
MessageFactory(thread=thread2, is_trashed=True)
# Thread 3: Starred, not trashed
thread3 = ThreadFactory()
ThreadAccessFactory(
mailbox=mailbox,
thread=thread3,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
MessageFactory(thread=thread3, is_starred=True, is_trashed=False)
# Thread 4: Has both starred (not trashed) and trashed messages
MessageFactory(thread=thread3, is_trashed=False)
# Thread 4: Starred AND has trashed messages
thread4 = ThreadFactory()
ThreadAccessFactory(
mailbox=mailbox,
thread=thread4,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
MessageFactory(
thread=thread4, is_starred=True, is_trashed=False
) # Starred, not trashed
MessageFactory(
thread=thread4, is_starred=False, is_trashed=True
) # Not starred, trashed
MessageFactory(thread=thread4, is_trashed=False)
MessageFactory(thread=thread4, is_trashed=True)
# Thread 5 : Is spam not trashed
thread5 = ThreadFactory()
@@ -294,27 +334,37 @@ def test_list_threads_filter_combined(api_client):
for t in [thread1, thread2, thread3, thread4, thread5]:
t.update_stats()
# Filter: has_starred=1 AND has_trashed=1 (thread has both starred non-trashed AND trashed messages)
response = api_client.get(API_URL, {"has_starred": "1", "has_trashed": "1"})
params = {"mailbox_id": str(mailbox.id)}
# Filter: has_starred=1 AND has_trashed=1 (thread is starred AND has trashed messages)
response = api_client.get(
API_URL, {**params, "has_starred": "1", "has_trashed": "1"}
)
assert response.status_code == status.HTTP_200_OK
assert response.data["count"] == 1
assert response.data["results"][0]["id"] == str(thread4.id)
# Filter: has_starred=1 AND has_trashed=0 (thread has starred non-trashed messages, no trashed messages)
response = api_client.get(API_URL, {"has_starred": "1", "has_trashed": "0"})
# Filter: has_starred=1 AND has_trashed=0 (thread is starred, no trashed messages)
response = api_client.get(
API_URL, {**params, "has_starred": "1", "has_trashed": "0"}
)
assert response.status_code == status.HTTP_200_OK
assert response.data["count"] == 1
assert response.data["results"][0]["id"] == str(thread3.id)
# Filter: has_starred=0 AND has_trashed=0 (thread has no starred non-trashed messages, no trashed messages)
response = api_client.get(API_URL, {"has_starred": "0", "has_trashed": "0"})
# Filter: has_starred=0 AND has_trashed=0 (thread not starred, no trashed messages)
response = api_client.get(
API_URL, {**params, "has_starred": "0", "has_trashed": "0"}
)
assert response.status_code == status.HTTP_200_OK
assert response.data["count"] == 1
thread_ids = [t["id"] for t in response.data["results"]]
assert str(thread1.id) in thread_ids
# Filter: has_starred=0 AND has_trashed=1 (thread has no starred non-trashed messages, but has trashed messages)
response = api_client.get(API_URL, {"has_starred": "0", "has_trashed": "1"})
# Filter: has_starred=0 AND has_trashed=1 (thread not starred, has trashed messages)
response = api_client.get(
API_URL, {**params, "has_starred": "0", "has_trashed": "1"}
)
assert response.status_code == status.HTTP_200_OK
assert response.data["count"] == 1
assert response.data["results"][0]["id"] == str(thread2.id)
@@ -358,20 +408,19 @@ class TestThreadStatsAPI:
has_messages=True,
has_trashed=False,
has_draft=True,
has_starred=True,
has_sender=True,
)
ThreadAccessFactory(
mailbox=mailbox,
thread=thread1,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
thread2 = ThreadFactory(
has_messages=True,
has_trashed=True,
has_draft=False,
has_starred=False,
has_sender=True,
)
ThreadAccessFactory(
@@ -392,7 +441,8 @@ class TestThreadStatsAPI:
response = api_client.get(
url,
{
"stats_fields": "has_messages,has_trashed,has_draft,has_starred,has_sender"
"mailbox_id": str(mailbox.id),
"stats_fields": "has_messages,has_trashed,has_draft,has_starred,has_sender",
},
)
@@ -401,7 +451,7 @@ class TestThreadStatsAPI:
"has_messages": 2, # Both threads have has_messages=True
"has_trashed": 1, # Only thread2 has has_trashed=True
"has_draft": 1, # Only thread1 has has_draft=True
"has_starred": 1, # Only thread1 has has_starred=True
"has_starred": 1, # Only thread1 is starred
"has_sender": 2, # Both threads have has_sender=True
}
@@ -441,14 +491,15 @@ class TestThreadStatsAPI:
mailbox = MailboxFactory(users_read=[user])
# Starred thread
thread1 = ThreadFactory(has_starred=True, has_messages=True)
thread1 = ThreadFactory(has_messages=True)
ThreadAccessFactory(
mailbox=mailbox,
thread=thread1,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
# Not starred thread
thread2 = ThreadFactory(has_starred=False, has_messages=True)
thread2 = ThreadFactory(has_messages=True)
ThreadAccessFactory(
mailbox=mailbox,
thread=thread2,
@@ -456,7 +507,12 @@ class TestThreadStatsAPI:
)
response = api_client.get(
url, {"has_starred": "1", "stats_fields": "has_messages"}
url,
{
"has_starred": "1",
"stats_fields": "has_messages",
"mailbox_id": str(mailbox.id),
},
)
assert response.status_code == 200
@@ -560,7 +616,7 @@ class TestThreadStatsAPI:
thread1_mbx2.labels.add(label1_mbx2)
# Test stats with label_slug filter without mailbox_id
# all_unread is 0 because unread state requires a mailbox context
# all_unread checks across all user's mailboxes
response = api_client.get(
url,
{
@@ -572,7 +628,7 @@ class TestThreadStatsAPI:
assert response.status_code == 200
# Should count threads from both mailboxes since user has access to both
assert response.data["all"] == 2
assert response.data["all_unread"] == 0
assert response.data["all_unread"] == 2
# Test stats with label_slug and mailbox_id filter - should return only threads from that mailbox
response = api_client.get(
@@ -744,9 +800,7 @@ class TestThreadStatsAPI:
thread2 = ThreadFactory(
has_messages=True, has_active=True, active_messaged_at=timezone.now()
)
thread3 = ThreadFactory(
has_starred=True, has_active=True, active_messaged_at=timezone.now()
)
thread3 = ThreadFactory(has_active=True, active_messaged_at=timezone.now())
for thread in [thread1, thread2, thread3]:
ThreadAccessFactory(
@@ -778,36 +832,44 @@ class TestThreadStatsAPI:
mailbox = MailboxFactory(users_read=[user])
# Create threads with different combinations of flags and unread status
# thread1: unread (has_active + active_messaged_at, no read_at)
# thread1: unread (has_active + active_messaged_at, no read_at), starred
thread1 = ThreadFactory(
has_starred=True,
has_sender=True,
is_spam=False,
has_active=True,
active_messaged_at=timezone.now(),
)
# thread2: read (has_active + active_messaged_at, read_at set)
# thread2: read (has_active + active_messaged_at, read_at set), starred
thread2 = ThreadFactory(
has_starred=True,
has_sender=True,
is_spam=False,
has_active=True,
active_messaged_at=timezone.now(),
)
# thread3: unread (no has_active, so not unread despite no read_at)
# thread3: unread (no has_active, so not unread despite no read_at), not starred
thread3 = ThreadFactory(
has_starred=False,
has_sender=False,
is_spam=True,
has_active=False,
)
for thread in [thread1, thread2, thread3]:
ThreadAccessFactory(
mailbox=mailbox,
thread=thread,
role=enums.ThreadAccessRoleChoices.EDITOR,
)
ThreadAccessFactory(
mailbox=mailbox,
thread=thread1,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
ThreadAccessFactory(
mailbox=mailbox,
thread=thread2,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
ThreadAccessFactory(
mailbox=mailbox,
thread=thread3,
role=enums.ThreadAccessRoleChoices.EDITOR,
)
# Mark thread2 as read
access2 = thread2.accesses.get(mailbox=mailbox)
@@ -833,7 +895,7 @@ class TestThreadStatsAPI:
assert response.status_code == 200
assert response.data == {
"has_starred": 2, # thread1 and thread2 have has_starred=True
"has_starred": 2, # thread1 and thread2 are starred
"has_starred_unread": 1, # Only thread1 is starred AND unread
"has_sender": 2, # thread1 and thread2 have has_sender=True
"has_sender_unread": 1, # Only thread1 is sender AND unread
@@ -850,34 +912,42 @@ class TestThreadStatsAPI:
mailbox = MailboxFactory(users_read=[user])
# Create threads with different combinations
# thread1: unread
# thread1: unread, starred
thread1 = ThreadFactory(
has_starred=True,
has_sender=True,
has_active=True,
active_messaged_at=timezone.now(),
)
# thread2: read
# thread2: read, starred
thread2 = ThreadFactory(
has_starred=True,
has_sender=True,
has_active=True,
active_messaged_at=timezone.now(),
)
# thread3: unread
# thread3: unread, not starred
thread3 = ThreadFactory(
has_starred=False,
has_sender=True,
has_active=True,
active_messaged_at=timezone.now(),
)
for thread in [thread1, thread2, thread3]:
ThreadAccessFactory(
mailbox=mailbox,
thread=thread,
role=enums.ThreadAccessRoleChoices.EDITOR,
)
ThreadAccessFactory(
mailbox=mailbox,
thread=thread1,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
ThreadAccessFactory(
mailbox=mailbox,
thread=thread2,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
ThreadAccessFactory(
mailbox=mailbox,
thread=thread3,
role=enums.ThreadAccessRoleChoices.EDITOR,
)
# Mark thread2 as read
access2 = thread2.accesses.get(mailbox=mailbox)

View File

@@ -418,12 +418,11 @@ def test_export_includes_status_headers(mailbox_fixture, admin_user, cleanup_exp
"""Test that exported messages include Status/X-Status headers for flags."""
# Create a read, starred message
msg = create_test_message(mailbox_fixture, "Starred Message", "Important content")
msg.is_starred = True # Starred
msg.save()
# Mark as read via ThreadAccess.read_at
# Mark as read and starred via ThreadAccess
access = ThreadAccess.objects.get(thread=msg.thread, mailbox=mailbox_fixture)
access.read_at = timezone.now()
access.save(update_fields=["read_at"])
access.starred_at = timezone.now()
access.save(update_fields=["read_at", "starred_at"])
mock_task = MagicMock()
@@ -454,6 +453,82 @@ def test_export_includes_status_headers(mailbox_fixture, admin_user, cleanup_exp
assert b"X-Status: F" in mbox_content
@pytest.mark.django_db
def test_export_starred_flag_is_mailbox_scoped(
mailbox_fixture, domain, admin_user, cleanup_exports
):
"""Test that the starred flag in export is scoped per mailbox.
When a thread is starred in one mailbox but not another, only the
export for the starred mailbox should contain X-Status: F.
"""
# Create a message tied to mailbox_fixture
msg = create_test_message(mailbox_fixture, "Shared Thread", "Shared content")
# Mark as read and starred for mailbox_fixture
access = ThreadAccess.objects.get(thread=msg.thread, mailbox=mailbox_fixture)
access.read_at = timezone.now()
access.starred_at = timezone.now()
access.save(update_fields=["read_at", "starred_at"])
# Create another mailbox sharing the same thread, without starred_at
other_mailbox = Mailbox.objects.create(local_part="other", domain=domain)
ThreadAccess.objects.create(
thread=msg.thread, mailbox=other_mailbox, read_at=timezone.now()
)
mock_task = MagicMock()
# Export mailbox_fixture — should contain X-Status: F
with (
patch.object(export_mailbox_task, "update_state", mock_task.update_state),
patch(
"core.services.exporter.tasks.deliver_inbound_message", return_value=True
),
):
result_starred = export_mailbox_task(
str(mailbox_fixture.id), str(admin_user.id)
)
assert result_starred["status"] == "SUCCESS"
cleanup_exports.append(result_starred["result"]["s3_key"])
# Export other_mailbox — should NOT contain X-Status: F
with (
patch.object(export_mailbox_task, "update_state", mock_task.update_state),
patch(
"core.services.exporter.tasks.deliver_inbound_message", return_value=True
),
):
result_not_starred = export_mailbox_task(
str(other_mailbox.id), str(admin_user.id)
)
assert result_not_starred["status"] == "SUCCESS"
cleanup_exports.append(result_not_starred["result"]["s3_key"])
storage = storages["message-imports"]
s3_client = storage.connection.meta.client
# Verify starred mailbox export includes X-Status: F
response = s3_client.get_object(
Bucket=storage.bucket_name, Key=result_starred["result"]["s3_key"]
)
with gzip.open(BytesIO(response["Body"].read()), "rb") as f:
starred_content = f.read()
assert b"Status: RO" in starred_content
assert b"X-Status: F" in starred_content
# Verify other mailbox export does NOT include X-Status: F
response = s3_client.get_object(
Bucket=storage.bucket_name, Key=result_not_starred["result"]["s3_key"]
)
with gzip.open(BytesIO(response["Body"].read()), "rb") as f:
other_content = f.read()
assert b"Status: RO" in other_content
assert b"X-Status: F" not in other_content
@pytest.mark.django_db
def test_export_headers_prepended_before_received(
mailbox_fixture, admin_user, cleanup_exports

View File

@@ -246,13 +246,15 @@ def fixture_test_threads(test_mailboxes, wait_for_indexing):
thread6 = ThreadFactory(subject="Important Announcement")
threads.append(thread6)
ThreadAccessFactory(
mailbox=mailbox1, thread=thread6, role=enums.ThreadAccessRoleChoices.EDITOR
mailbox=mailbox1,
thread=thread6,
role=enums.ThreadAccessRoleChoices.EDITOR,
starred_at=timezone.now(),
)
message6 = MessageFactory(
thread=thread6,
subject="Important Announcement",
sender=contact4,
is_starred=True,
raw_mime=(
f"From: {contact4.email}\r\n"
f"To: {contact1.email}\r\n"
@@ -444,7 +446,7 @@ def fixture_test_threads(test_mailboxes, wait_for_indexing):
len(settings.OPENSEARCH_HOSTS) == 0,
reason="OpenSearch is not configured",
)
@pytest.mark.django_db
@pytest.mark.django_db(transaction=True)
class TestSearchModifiersE2E:
"""End-to-end tests for Gmail-style search modifiers."""
@@ -821,11 +823,18 @@ class TestSearchModifiersE2E:
assert str(test_threads["thread3"].id) in thread_ids
def test_search_e2e_modifiers_is_starred_search_modifier(
self, setup_search, api_client, test_url, test_threads
self, setup_search, api_client, test_url, test_threads, test_mailboxes
):
"""Test searching with the 'is:starred' modifier."""
"""Test searching with the 'is:starred' modifier.
is:starred requires mailbox_id since starred is per-mailbox via
ThreadAccess.starred_at. Only thread6 has starred_at set for mailbox1.
"""
mailbox1, _ = test_mailboxes
# Test English version
response = api_client.get(f"{test_url}?search=is:starred")
response = api_client.get(
f"{test_url}?search=is:starred&mailbox_id={mailbox1.id}"
)
# Verify response
assert response.status_code == 200
@@ -836,7 +845,9 @@ class TestSearchModifiersE2E:
assert str(test_threads["thread6"].id) in thread_ids
# Test French version
response = api_client.get(f"{test_url}?search=est:suivi")
response = api_client.get(
f"{test_url}?search=est:suivi&mailbox_id={mailbox1.id}"
)
# Verify the same results
assert response.status_code == 200

View File

@@ -232,6 +232,14 @@ def test_search_parse_query_is_starred_modifier_french():
assert result == {"text": "some text", "is_starred": True}
def test_search_parse_query_is_starred_modifier_dutch():
"""Test parsing 'is:gevolgd' modifier in Dutch."""
query = "is:gevolgd some text"
result = parse_search_query(query)
assert result == {"text": "some text", "is_starred": True}
def test_search_parse_query_is_read_modifier_english():
"""Test parsing 'is:read' modifier in English."""
query = "is:read some text"

View File

@@ -21,7 +21,7 @@ from core.services.search import (
reindex_all,
reindex_mailbox,
search_threads,
update_thread_unread_mailboxes,
update_thread_mailbox_flags,
)
from core.services.search.index import compute_unread_mailboxes
@@ -316,8 +316,8 @@ class TestComputeUnreadMailboxes:
@pytest.mark.django_db
def test_update_thread_unread_mailboxes(mock_es_client_index):
"""Test that update_thread_unread_mailboxes re-indexes the thread document."""
def test_update_thread_mailbox_flags(mock_es_client_index):
"""Test that update_thread_mailbox_flags re-indexes the thread document."""
thread = ThreadFactory()
mailbox = MailboxFactory()
MessageFactory(thread=thread)
@@ -328,10 +328,11 @@ def test_update_thread_unread_mailboxes(mock_es_client_index):
# Reset mock after setup (signals may have triggered calls)
mock_es_client_index.reset_mock()
success = update_thread_unread_mailboxes(thread)
success = update_thread_mailbox_flags(thread)
assert success
mock_es_client_index.index.assert_called_once()
call_args = mock_es_client_index.index.call_args[1]
assert call_args["id"] == str(thread.id)
assert str(mailbox.id) in call_args["body"]["unread_mailboxes"]
assert "starred_mailboxes" in call_args["body"]

View File

@@ -115,13 +115,14 @@ def test_search_threads_with_multiple_modifiers(mock_es_client):
break
assert subject_query_found, "Subject query was not found in the OpenSearch query"
# Check for starred filter
# Check for starred filter (now uses has_parent + starred_mailboxes)
starred_filter_found = False
for filter_item in call_args["body"]["query"]["bool"]["filter"]:
if "term" in filter_item:
if "is_starred" in filter_item["term"]:
starred_filter_found = True
assert filter_item["term"]["is_starred"] is True
if "has_parent" in filter_item:
hp = filter_item["has_parent"]
if hp.get("parent_type") == "thread" and "terms" in hp.get("query", {}):
if "starred_mailboxes" in hp["query"]["terms"]:
starred_filter_found = True
assert starred_filter_found, "Starred filter was not found in the OpenSearch query"
@@ -186,6 +187,142 @@ def test_search_threads_is_unread_without_mailbox_ids(mock_es_client):
)
def test_search_threads_filters_is_starred_true(mock_es_client):
"""Test that filters={'is_starred': True} uses has_parent on starred_mailboxes."""
search_threads("some text", mailbox_ids=["mbx-1"], filters={"is_starred": True})
call_args = mock_es_client.search.call_args[1]
filters = call_args["body"]["query"]["bool"]["filter"]
# Should use has_parent with starred_mailboxes
has_parent_found = False
for filter_item in filters:
if "has_parent" in filter_item:
hp = filter_item["has_parent"]
query = hp.get("query", {})
if "terms" in query and "starred_mailboxes" in query["terms"]:
has_parent_found = True
assert query["terms"]["starred_mailboxes"] == ["mbx-1"]
assert has_parent_found, "has_parent starred_mailboxes filter was not found"
# Should NOT emit a legacy {"term": {"is_starred": ...}} filter
for filter_item in filters:
if "term" in filter_item:
assert "is_starred" not in filter_item["term"], (
"Legacy is_starred term filter should not be emitted"
)
def test_search_threads_filters_is_starred_false(mock_es_client):
"""Test that filters={'is_starred': False} uses has_parent must_not on starred_mailboxes."""
search_threads("some text", mailbox_ids=["mbx-1"], filters={"is_starred": False})
call_args = mock_es_client.search.call_args[1]
filters = call_args["body"]["query"]["bool"]["filter"]
has_parent_found = False
for filter_item in filters:
if "has_parent" in filter_item:
hp = filter_item["has_parent"]
query = hp.get("query", {})
if "bool" in query and "must_not" in query["bool"]:
must_not = query["bool"]["must_not"]
if "terms" in must_not and "starred_mailboxes" in must_not["terms"]:
has_parent_found = True
assert must_not["terms"]["starred_mailboxes"] == ["mbx-1"]
assert has_parent_found, (
"has_parent must_not starred_mailboxes filter was not found"
)
def test_search_threads_filters_is_unread_true(mock_es_client):
"""Test that filters={'is_unread': True} uses has_parent on unread_mailboxes."""
search_threads("some text", mailbox_ids=["mbx-1"], filters={"is_unread": True})
call_args = mock_es_client.search.call_args[1]
filters = call_args["body"]["query"]["bool"]["filter"]
has_parent_found = False
for filter_item in filters:
if "has_parent" in filter_item:
hp = filter_item["has_parent"]
query = hp.get("query", {})
if "terms" in query and "unread_mailboxes" in query["terms"]:
has_parent_found = True
assert query["terms"]["unread_mailboxes"] == ["mbx-1"]
assert has_parent_found, "has_parent unread_mailboxes filter was not found"
for filter_item in filters:
if "term" in filter_item:
assert "is_unread" not in filter_item["term"], (
"Legacy is_unread term filter should not be emitted"
)
def test_search_threads_filters_is_unread_false(mock_es_client):
"""Test that filters={'is_unread': False} uses has_parent must_not on unread_mailboxes."""
search_threads("some text", mailbox_ids=["mbx-1"], filters={"is_unread": False})
call_args = mock_es_client.search.call_args[1]
filters = call_args["body"]["query"]["bool"]["filter"]
has_parent_found = False
for filter_item in filters:
if "has_parent" in filter_item:
hp = filter_item["has_parent"]
query = hp.get("query", {})
if "bool" in query and "must_not" in query["bool"]:
must_not = query["bool"]["must_not"]
if "terms" in must_not and "unread_mailboxes" in must_not["terms"]:
has_parent_found = True
assert must_not["terms"]["unread_mailboxes"] == ["mbx-1"]
assert has_parent_found, "has_parent must_not unread_mailboxes filter was not found"
def test_search_threads_filters_starred_without_mailbox_ids(mock_es_client):
"""Test that filters={'is_starred': True} without mailbox_ids does not add a filter."""
search_threads("some text", filters={"is_starred": True})
call_args = mock_es_client.search.call_args[1]
filters = call_args["body"]["query"]["bool"]["filter"]
for filter_item in filters:
if "has_parent" in filter_item:
hp = filter_item["has_parent"]
query = hp.get("query", {})
assert "starred_mailboxes" not in query.get("terms", {}), (
"starred_mailboxes filter should not be present without mailbox_ids"
)
if "term" in filter_item:
assert "is_starred" not in filter_item["term"], (
"Legacy is_starred term filter should not be emitted"
)
def test_search_threads_filters_other_fields_still_use_term(mock_es_client):
"""Test that non-mailbox-scoped filters still use the generic term filter."""
search_threads(
"some text",
mailbox_ids=["mbx-1"],
filters={"is_starred": True, "is_sender": True},
)
call_args = mock_es_client.search.call_args[1]
filters = call_args["body"]["query"]["bool"]["filter"]
sender_term_found = False
for filter_item in filters:
if "term" in filter_item and "is_sender" in filter_item["term"]:
sender_term_found = True
assert filter_item["term"]["is_sender"] is True
assert sender_term_found, "is_sender should still use the generic term filter"
def test_search_threads_with_exact_phrase(mock_es_client):
"""Test searching threads with exact phrases."""
# Call the function with the actual query containing an exact phrase