Skip to content

6009: add organizations to invitation querysets and sync #6039

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
yasinelmi wants to merge 8 commits into
base: unstable
Choose a base branch
from
Draft

6009: add organizations to invitation querysets and sync #6039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cf2a01e
6009: add organizations to invitation querysets and sync
yazzylazy Jul 12, 2026
2045c38
fix: add organization_id support to sync event builders, fix test fix…
yazzylazy Jul 13, 2026
475ba8e
6009: enforce channel/organization mutual exclusivity on Invitation
yazzylazy Jul 13, 2026
adf1830
6009: add full organization_id broadcast parity to Change/sync
yazzylazy Jul 19, 2026
bd756fe
Merge branch 'unstable' of https://github.com/learningequality/studio…
yazzylazy Jul 19, 2026
683d1ca
fix: resolve migration leaf conflict after merging unstable
yazzylazy Jul 19, 2026
22b1f51
6009: revert Change/sync organization_id broadcast, per rtibbles review
yazzylazy Jul 20, 2026
eb4a8b6
6009: fix org invitation review findings (rtibblesbot)
yazzylazy Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 51 additions & 1 deletion contentcuration/contentcuration/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1895,6 +1895,40 @@ class Meta:
def __str__(self):
return self.name

@classmethod
def filter_edit_queryset(cls, queryset, user):

@rtibblesbot rtibblesbot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nitpick: Organization.deleted is a soft-delete flag (line 1880) and CustomManager doesn't filter it, so neither of these classmethods excludes deleted orgs — and both call sites pass an unfiltered base queryset (Organization.objects.all() in the serializer field, Organization.objects.filter(id__in=...) in SyncView). An admin of a soft-deleted org can still create invitations into it, and accepting one creates a live OrganizationRole. Channel defers deleted filtering to its viewset, so either layer is consistent with the repo — but with no Organization viewset yet, these methods are currently the only gate.

if user.is_anonymous:
return queryset.none()

if user.is_admin:
return queryset

return queryset.filter(

@rtibblesbot rtibblesbot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

praise: Checking status=ORGANIZATION_ROLE_STATUS_ACTIVE alongside the role — and keeping all three conditions in one filter() so they match a single user_roles row rather than across rows — is the easy thing to get wrong here. test_filter_edit_queryset__pending_admin_cannot_edit locks it in.

user_roles__user=user,
user_roles__role=ORGANIZATION_ADMIN,
user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE,
deleted=False,
).distinct()

@classmethod
def filter_view_queryset(cls, queryset, user):
if user.is_anonymous:
return queryset.none()

if user.is_admin:
return queryset

return queryset.filter(
user_roles__user=user,
user_roles__role__in=[
ORGANIZATION_ADMIN,
ORGANIZATION_EDITOR,
ORGANIZATION_VIEWER,
],
user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE,
deleted=False,
).distinct()


class OrganizationRole(models.Model):
"""
Expand Down Expand Up @@ -3807,7 +3841,14 @@ def filter_edit_queryset(cls, queryset, user):
return queryset

return queryset.filter(

@rtibblesbot rtibblesbot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

blocking: This grants an active ORGANIZATION_ADMIN edit access to the org's invitations, but InvitationSerializer.get_fields is unchanged and still gates writability on channel-era relationships (if self.instance.sender == request.user: fields["revoked"].read_only = False). An org admin who isn't the sender therefore cannot revoke: revoked stays read-only, DRF drops it from validated_data, BulkListSerializer.update skips the object because obj.keys() is empty (viewsets/base.py:283), and the request returns 200 with nothing changed and no error.

Multi-admin orgs are the point of org-level roles, so "only the original inviter can revoke" is unlikely to be intended. test_revoke_organization_invitation_by_admin makes self.org_admin the sender, so it exercises the pre-existing sender rule; a test where admin A revokes an invitation sent by admin B would fail today. Extending get_fields to unlock revoked for org admins (mirroring the sender case) would close it.

Same shape for accept/decline over sync: gated on self.instance.invited == request.user, and invited is never set for API-created invitations. Latent, since the REST actions compare emails via _ensure_invitee, but worth noting.

Q(email__iexact=user.email) | Q(sender=user) | Q(channel__editors=user)
Q(email__iexact=user.email)
| Q(sender=user)
| Q(channel__editors=user)
| Q(
organization__user_roles__user=user,
organization__user_roles__role=ORGANIZATION_ADMIN,
organization__user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE,
)
).distinct()

@classmethod
Expand All @@ -3822,6 +3863,15 @@ def filter_view_queryset(cls, queryset, user):
| Q(sender=user)
| Q(channel__editors=user)
| Q(channel__viewers=user)
| Q(
organization__user_roles__user=user,
organization__user_roles__role__in=[
ORGANIZATION_ADMIN,
ORGANIZATION_EDITOR,
ORGANIZATION_VIEWER,
],
organization__user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE,
)
).distinct()


Expand Down
170 changes: 170 additions & 0 deletions contentcuration/contentcuration/tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,15 @@
from contentcuration.constants import channel_history
from contentcuration.constants import community_library_submission
from contentcuration.constants import user_history
from contentcuration.constants.organization_roles import ORGANIZATION_ADMIN
from contentcuration.constants.organization_roles import ORGANIZATION_EDITOR
from contentcuration.constants.organization_roles import (
ORGANIZATION_ROLE_STATUS_ACTIVE,
)
from contentcuration.constants.organization_roles import (
ORGANIZATION_ROLE_STATUS_PENDING,
)
from contentcuration.constants.organization_roles import ORGANIZATION_VIEWER
from contentcuration.models import AssessmentItem
from contentcuration.models import AuditedSpecialPermissionsLicense
from contentcuration.models import Change
Expand All @@ -34,6 +43,8 @@
from contentcuration.models import Language
from contentcuration.models import License
from contentcuration.models import object_storage_name
from contentcuration.models import Organization
from contentcuration.models import OrganizationRole
from contentcuration.models import RecommendationsEvent
from contentcuration.models import RecommendationsInteractionEvent
from contentcuration.models import User
Expand Down Expand Up @@ -309,6 +320,165 @@ def create_change(server_rev, applied):
self.assertEqual(channel.get_server_rev(), 2)


class OrganizationTestCase(PermissionQuerysetTestCase):
@property
def base_queryset(self):
return Organization.objects.all()

def test_filter_edit_queryset__admin_role(self):
organization = testdata.organization()
user = testdata.user()

queryset = Organization.filter_edit_queryset(self.base_queryset, user=user)
self.assertQuerysetDoesNotContain(queryset, pk=organization.id)

OrganizationRole.objects.create(
user=user,
organization=organization,
role=ORGANIZATION_ADMIN,
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
)
queryset = Organization.filter_edit_queryset(self.base_queryset, user=user)
self.assertQuerysetContains(queryset, pk=organization.id)

def test_filter_edit_queryset__editor_role_cannot_edit(self):
organization = testdata.organization()
user = testdata.user()
OrganizationRole.objects.create(
user=user,
organization=organization,
role=ORGANIZATION_EDITOR,
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
)

queryset = Organization.filter_edit_queryset(self.base_queryset, user=user)
self.assertQuerysetDoesNotContain(queryset, pk=organization.id)

def test_filter_edit_queryset__pending_admin_cannot_edit(self):
organization = testdata.organization()
user = testdata.user()
OrganizationRole.objects.create(
user=user,
organization=organization,
role=ORGANIZATION_ADMIN,
status=ORGANIZATION_ROLE_STATUS_PENDING,
)

queryset = Organization.filter_edit_queryset(self.base_queryset, user=user)
self.assertQuerysetDoesNotContain(queryset, pk=organization.id)

def test_filter_edit_queryset__anonymous(self):
organization = testdata.organization()

queryset = Organization.filter_edit_queryset(
self.base_queryset, user=self.anonymous_user
)
self.assertQuerysetDoesNotContain(queryset, pk=organization.id)

def test_filter_view_queryset__viewer_role(self):
organization = testdata.organization()
user = testdata.user()

queryset = Organization.filter_view_queryset(self.base_queryset, user=user)
self.assertQuerysetDoesNotContain(queryset, pk=organization.id)

OrganizationRole.objects.create(
user=user,
organization=organization,
role=ORGANIZATION_VIEWER,
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
)
queryset = Organization.filter_view_queryset(self.base_queryset, user=user)
self.assertQuerysetContains(queryset, pk=organization.id)

def test_filter_view_queryset__anonymous(self):
organization = testdata.organization()

queryset = Organization.filter_view_queryset(
self.base_queryset, user=self.anonymous_user
)
self.assertQuerysetDoesNotContain(queryset, pk=organization.id)


class InvitationOrganizationTestCase(PermissionQuerysetTestCase):
@property
def base_queryset(self):
return Invitation.objects.all()

def _make_org_invitation(self):
organization = testdata.organization()
invitee = testdata.user(email="org-invitee@le.com")
invitation = Invitation.objects.create(
email=invitee.email, organization=organization
)
return organization, invitation

def test_filter_edit_queryset__organization_admin(self):
organization, invitation = self._make_org_invitation()
user = testdata.user()

queryset = Invitation.filter_edit_queryset(self.base_queryset, user=user)
self.assertQuerysetDoesNotContain(queryset, pk=invitation.id)

OrganizationRole.objects.create(
user=user,
organization=organization,
role=ORGANIZATION_ADMIN,
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
)
queryset = Invitation.filter_edit_queryset(self.base_queryset, user=user)
self.assertQuerysetContains(queryset, pk=invitation.id)

def test_filter_edit_queryset__organization_editor_cannot_edit(self):
organization, invitation = self._make_org_invitation()
user = testdata.user()
OrganizationRole.objects.create(
user=user,
organization=organization,
role=ORGANIZATION_EDITOR,
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
)

queryset = Invitation.filter_edit_queryset(self.base_queryset, user=user)
self.assertQuerysetDoesNotContain(queryset, pk=invitation.id)

def test_filter_view_queryset__organization_editor(self):
organization, invitation = self._make_org_invitation()
user = testdata.user()

queryset = Invitation.filter_view_queryset(self.base_queryset, user=user)
self.assertQuerysetDoesNotContain(queryset, pk=invitation.id)

OrganizationRole.objects.create(
user=user,
organization=organization,
role=ORGANIZATION_EDITOR,
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
)
queryset = Invitation.filter_view_queryset(self.base_queryset, user=user)
self.assertQuerysetContains(queryset, pk=invitation.id)

def test_filter_view_queryset__organization_viewer(self):
organization, invitation = self._make_org_invitation()
user = testdata.user()
OrganizationRole.objects.create(
user=user,
organization=organization,
role=ORGANIZATION_VIEWER,
status=ORGANIZATION_ROLE_STATUS_ACTIVE,
)

queryset = Invitation.filter_view_queryset(self.base_queryset, user=user)
self.assertQuerysetContains(queryset, pk=invitation.id)

def test_filter_view_queryset__unrelated_user(self):
organization, invitation = self._make_org_invitation()
user = testdata.user()

queryset = Invitation.filter_view_queryset(self.base_queryset, user=user)
self.assertQuerysetDoesNotContain(queryset, pk=invitation.id)


class ContentNodeTestCase(PermissionQuerysetTestCase):
@property
def base_queryset(self):
Expand Down
16 changes: 16 additions & 0 deletions contentcuration/contentcuration/tests/testdata.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
from contentcuration.constants import (
community_library_submission as community_library_submission_constants,
)
from contentcuration.constants.organization_roles import ORGANIZATION_ADMIN
from contentcuration.constants.organization_roles import (
ORGANIZATION_ROLE_STATUS_ACTIVE,
)
from contentcuration.tests.utils import mixer


Expand Down Expand Up @@ -253,6 +257,18 @@ def channel(name="testchannel"):
return channel


def organization(name="Test Organization"):
return cc.Organization.objects.create(name=name)


def organization_role(
user, organization, role=ORGANIZATION_ADMIN, status=ORGANIZATION_ROLE_STATUS_ACTIVE
):
return cc.OrganizationRole.objects.create(
user=user, organization=organization, role=role, status=status
)


def random_string(chars=10):
"""
Generate a random string
Expand Down
Loading
Loading