Skip to content

Commit 03cd72f

Browse files
committed
🐛(labels) support identical label slugs across different mailboxes
- Replace single label lookup with multiple labels filter in ThreadViewSet - Allow filtering threads by label_slug across multiple mailboxes when user has access - Expect empty results instead of 403 errors for inaccessible labels - Add comprehensive tests for identical labels in different mailboxes scenarios This change enables users with access to multiple mailboxes to filter threads by label slug across all accessible mailboxes, while maintaining proper permission checks and supporting mailbox-specific filtering.
1 parent e32862e commit 03cd72f

File tree

3 files changed

+193
-12
lines changed

3 files changed

+193
-12
lines changed

src/backend/core/api/viewsets/thread.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,17 @@ def get_queryset(self):
6060

6161
if label_slug:
6262
# Filter threads by label slug, ensuring user has access to the label's mailbox
63-
try:
64-
label = models.Label.objects.get(
65-
slug=label_slug,
66-
mailbox__accesses__user=user,
67-
)
68-
queryset = queryset.filter(labels=label)
69-
except models.Label.DoesNotExist as e:
70-
raise drf.exceptions.PermissionDenied(
71-
"You do not have access to this label."
72-
) from e
63+
# Get labels accessible to the user, joining with mailbox access
64+
labels = models.Label.objects.filter(
65+
slug=label_slug,
66+
mailbox__accesses__user=user,
67+
)
68+
if mailbox_id:
69+
# Further filter by mailbox if specified
70+
labels = labels.filter(mailbox__id=mailbox_id)
71+
72+
# Filter threads that have any of these labels
73+
queryset = queryset.filter(labels__in=labels)
7374

7475
# Apply boolean filters
7576
# These filters operate on the Thread model's boolean fields

src/backend/core/tests/api/test_thread_filter.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,8 @@ def test_filter_threads_by_invalid_label(
148148
response = api_client.get(
149149
url, {"label_slug": "00000000-0000-0000-0000-000000000000"}
150150
)
151-
assert response.status_code == status.HTTP_403_FORBIDDEN
151+
assert response.status_code == status.HTTP_200_OK
152+
assert response.data["count"] == 0
152153

153154
def test_filter_threads_by_label_no_access(self, api_client, url):
154155
"""Test filtering threads by a label the user doesn't have access to."""
@@ -161,7 +162,8 @@ def test_filter_threads_by_label_no_access(self, api_client, url):
161162

162163
# Try to filter by label in mailbox user doesn't have access to
163164
response = api_client.get(url, {"label_slug": str(label.slug)})
164-
assert response.status_code == status.HTTP_403_FORBIDDEN
165+
assert response.status_code == status.HTTP_200_OK
166+
assert response.data["count"] == 0
165167

166168
def test_filter_threads_by_label_combined_filters(
167169
self, api_client, url, setup_threads_with_labels

src/backend/core/tests/api/test_threads_list.py

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from core import enums
1010
from core.factories import (
1111
ContactFactory,
12+
LabelFactory,
1213
MailboxAccessFactory,
1314
MailboxFactory,
1415
MailDomainFactory,
@@ -438,6 +439,183 @@ def test_stats_specific_fields(self, api_client, url):
438439
assert response.data == {"has_draft": 1}
439440
assert "has_messages" not in response.data
440441

442+
def test_stats_with_identical_labels_different_mailboxes(self, api_client, url):
443+
"""Test that stats with identical label slugs in different mailboxes work correctly."""
444+
user = UserFactory()
445+
api_client.force_authenticate(user=user)
446+
447+
# Create two mailboxes with user access
448+
mailbox1 = MailboxFactory()
449+
mailbox2 = MailboxFactory()
450+
mailbox1.accesses.create(user=user, role=enums.MailboxRoleChoices.EDITOR)
451+
mailbox2.accesses.create(user=user, role=enums.MailboxRoleChoices.EDITOR)
452+
453+
# Create identical labels in both mailboxes
454+
label1_mbx1 = LabelFactory(name="Work", mailbox=mailbox1)
455+
label1_mbx2 = LabelFactory(name="Work", mailbox=mailbox2)
456+
457+
# Create threads in each mailbox with the respective labels
458+
thread1_mbx1 = ThreadFactory(has_unread=True, has_messages=True)
459+
ThreadAccessFactory(
460+
mailbox=mailbox1,
461+
thread=thread1_mbx1,
462+
role=enums.ThreadAccessRoleChoices.EDITOR,
463+
)
464+
thread1_mbx1.labels.add(label1_mbx1)
465+
466+
thread1_mbx2 = ThreadFactory(has_unread=True, has_messages=True)
467+
ThreadAccessFactory(
468+
mailbox=mailbox2,
469+
thread=thread1_mbx2,
470+
role=enums.ThreadAccessRoleChoices.EDITOR,
471+
)
472+
thread1_mbx2.labels.add(label1_mbx2)
473+
474+
# Test stats with label_slug filter - should return threads from both mailboxes
475+
response = api_client.get(
476+
url,
477+
{
478+
"label_slug": label1_mbx1.slug, # Same slug as label1_mbx2
479+
"stats_fields": "all,all_unread",
480+
},
481+
)
482+
483+
assert response.status_code == 200
484+
# Should count threads from both mailboxes since user has access to both
485+
assert response.data["all"] == 2
486+
assert response.data["all_unread"] == 2
487+
488+
# Test stats with label_slug and mailbox_id filter - should return only threads from that mailbox
489+
response = api_client.get(
490+
url,
491+
{
492+
"label_slug": label1_mbx1.slug,
493+
"mailbox_id": str(mailbox1.id),
494+
"stats_fields": "all,all_unread",
495+
},
496+
)
497+
498+
assert response.status_code == 200
499+
# Should count only threads from mailbox1
500+
assert response.data["all"] == 1
501+
assert response.data["all_unread"] == 1
502+
503+
def test_stats_with_identical_labels_hierarchical_different_mailboxes(
504+
self, api_client, url
505+
):
506+
"""Test that stats with identical hierarchical label slugs in different mailboxes work correctly."""
507+
user = UserFactory()
508+
api_client.force_authenticate(user=user)
509+
510+
# Create two mailboxes with user access
511+
mailbox1 = MailboxFactory()
512+
mailbox2 = MailboxFactory()
513+
mailbox1.accesses.create(user=user, role=enums.MailboxRoleChoices.EDITOR)
514+
mailbox2.accesses.create(user=user, role=enums.MailboxRoleChoices.EDITOR)
515+
516+
# Create identical hierarchical labels in both mailboxes
517+
# Mailbox1: Work/Projects
518+
label1_mbx1 = LabelFactory(name="Work", mailbox=mailbox1)
519+
child1_mbx1 = LabelFactory(name="Work/Projects", mailbox=mailbox1)
520+
521+
# Mailbox2: Work/Projects (same structure)
522+
label1_mbx2 = LabelFactory(name="Work", mailbox=mailbox2)
523+
child1_mbx2 = LabelFactory(name="Work/Projects", mailbox=mailbox2)
524+
525+
# Create threads with both parent and child labels
526+
thread1_mbx1 = ThreadFactory(has_unread=True, has_messages=True)
527+
ThreadAccessFactory(
528+
mailbox=mailbox1,
529+
thread=thread1_mbx1,
530+
role=enums.ThreadAccessRoleChoices.EDITOR,
531+
)
532+
thread1_mbx1.labels.add(label1_mbx1) # Add parent label
533+
thread1_mbx1.labels.add(child1_mbx1) # Add child label
534+
535+
thread1_mbx2 = ThreadFactory(has_unread=True, has_messages=True)
536+
ThreadAccessFactory(
537+
mailbox=mailbox2,
538+
thread=thread1_mbx2,
539+
role=enums.ThreadAccessRoleChoices.EDITOR,
540+
)
541+
thread1_mbx2.labels.add(label1_mbx2) # Add parent label
542+
thread1_mbx2.labels.add(child1_mbx2) # Add child label
543+
544+
# Test stats with child label_slug filter
545+
response = api_client.get(
546+
url,
547+
{
548+
"label_slug": child1_mbx1.slug, # Same slug as child1_mbx2
549+
"stats_fields": "all,all_unread",
550+
},
551+
)
552+
553+
assert response.status_code == 200
554+
# Should count threads from both mailboxes
555+
assert response.data["all"] == 2
556+
assert response.data["all_unread"] == 2
557+
558+
# Test stats with parent label_slug filter
559+
response = api_client.get(
560+
url,
561+
{
562+
"label_slug": label1_mbx1.slug, # Same slug as label1_mbx2
563+
"stats_fields": "all,all_unread",
564+
},
565+
)
566+
567+
assert response.status_code == 200
568+
# Should count threads from both mailboxes (parent labels)
569+
assert response.data["all"] == 2
570+
assert response.data["all_unread"] == 2
571+
572+
def test_stats_with_label_slug_no_access(self, api_client, url):
573+
"""Test that stats with label_slug filter respects user access permissions."""
574+
user = UserFactory()
575+
api_client.force_authenticate(user=user)
576+
577+
# Create mailbox with user access
578+
mailbox1 = MailboxFactory()
579+
mailbox1.accesses.create(user=user, role=enums.MailboxRoleChoices.EDITOR)
580+
581+
# Create mailbox without user access
582+
mailbox2 = MailboxFactory()
583+
584+
# Create identical labels in both mailboxes
585+
label1_mbx1 = LabelFactory(name="Work", mailbox=mailbox1)
586+
label1_mbx2 = LabelFactory(name="Work", mailbox=mailbox2)
587+
588+
# Create threads with these labels
589+
thread1_mbx1 = ThreadFactory(has_unread=True, has_messages=True)
590+
ThreadAccessFactory(
591+
mailbox=mailbox1,
592+
thread=thread1_mbx1,
593+
role=enums.ThreadAccessRoleChoices.EDITOR,
594+
)
595+
thread1_mbx1.labels.add(label1_mbx1)
596+
597+
thread1_mbx2 = ThreadFactory(has_unread=True, has_messages=True)
598+
ThreadAccessFactory(
599+
mailbox=mailbox2,
600+
thread=thread1_mbx2,
601+
role=enums.ThreadAccessRoleChoices.EDITOR,
602+
)
603+
thread1_mbx2.labels.add(label1_mbx2)
604+
605+
# Test stats with label_slug filter - should only return accessible threads
606+
response = api_client.get(
607+
url,
608+
{
609+
"label_slug": label1_mbx1.slug, # Same slug as label1_mbx2
610+
"stats_fields": "all,all_unread",
611+
},
612+
)
613+
614+
assert response.status_code == 200
615+
# Should count only threads from mailbox1 (user has access)
616+
assert response.data["all"] == 1
617+
assert response.data["all_unread"] == 1
618+
441619
def test_stats_no_matching_threads(self, api_client, url):
442620
"""Test retrieving stats when no threads match the filters."""
443621

0 commit comments

Comments
 (0)