Skip to content

Channel Viewset Organization Updates #6130

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

Closed
ArthurMousatov wants to merge 2 commits into from
Closed

Channel Viewset Organization Updates #6130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4157171
6124: allow Channel viewset to handle organization property changes
ArthurMousatov Sep 9, 2026
5746393
Added change event to notify of contested invitation
ArthurMousatov Sep 9, 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
5 changes: 5 additions & 0 deletions contentcuration/contentcuration/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3903,6 +3903,11 @@ class Meta:
verbose_name_plural = "Invitations"

def accept(self):
if self.channel and self.organization:
self.channel.organization = self.organization
self.channel.save(update_fields=["organization"])
return

user = User.objects.filter(email__iexact=self.email).first()
if self.channel:
self._accept_channel_invitation(user)
Expand Down
257 changes: 257 additions & 0 deletions contentcuration/contentcuration/tests/viewsets/test_channel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from contentcuration import models as cc
from contentcuration.constants import channel_history
from contentcuration.constants import community_library_submission
from contentcuration.constants.organization_roles import ORGANIZATION_EDITOR
from contentcuration.models import AuditedSpecialPermissionsLicense
from contentcuration.models import Change
from contentcuration.models import Channel
Expand All @@ -34,6 +35,7 @@
from contentcuration.tests.viewsets.base import SyncTestMixin
from contentcuration.viewsets.channel import _unpublished_changes_query
from contentcuration.viewsets.sync.constants import CHANNEL
from contentcuration.viewsets.sync.constants import INVITATION
from contentcuration.viewsets.sync.utils import (
generate_added_to_community_library_event,
)
Expand Down Expand Up @@ -76,6 +78,24 @@ def test_create_channel(self):
except models.Channel.DoesNotExist:
self.fail("Channel was not created")

def test_create_channel_ignores_organization(self):
user = testdata.user()
organization = testdata.organization()
channel = self.channel_metadata
channel["organization"] = organization.id
self.client.force_authenticate(user=user)

response = self.sync_changes(
[
generate_create_event(
channel["id"], CHANNEL, channel, channel_id=channel["id"]
)
]
)

self.assertEqual(response.status_code, 200, response.content)
self.assertIsNone(models.Channel.objects.get(id=channel["id"]).organization_id)

def test_create_channels(self):
user = testdata.user()
self.client.force_authenticate(user=user)
Expand Down Expand Up @@ -121,6 +141,243 @@ def test_update_channel(self):
self.assertEqual(response.status_code, 200, response.content)
self.assertEqual(models.Channel.objects.get(id=channel.id).name, new_name)

def test_update_channel_organization_when_all_editors_have_access(self):
user = testdata.user()
organization = testdata.organization()
testdata.organization_role(user, organization, role=ORGANIZATION_EDITOR)
channel = models.Channel.objects.create(
actor_id=user.id, **self.channel_metadata
)
channel.editors.add(user)

self.client.force_authenticate(user=user)
response = self.sync_changes(
[
generate_update_event(
channel.id,
CHANNEL,
{"organization": organization.id},
channel_id=channel.id,
)
]
)

self.assertEqual(response.status_code, 200, response.content)
self.assertEqual(
models.Channel.objects.get(id=channel.id).organization_id,
organization.id,
)
self.assertFalse(
models.Invitation.objects.filter(
channel=channel, organization=organization
).exists()
)

def test_update_channel_organization_creates_contested_invitation(self):
user = testdata.user()
organization = testdata.organization()
channel = models.Channel.objects.create(
actor_id=user.id, **self.channel_metadata
)
channel.editors.add(user)

self.client.force_authenticate(user=user)
response = self.sync_changes(
[
generate_update_event(
channel.id,
CHANNEL,
{"organization": organization.id},
channel_id=channel.id,
)
]
)

self.assertEqual(response.status_code, 200, response.content)
channel.refresh_from_db()
self.assertIsNone(channel.organization_id)
invitation = models.Invitation.objects.get(
channel=channel, organization=organization
)
self.assertTrue(invitation)

change = models.Change.objects.filter(channel=channel, table=CHANNEL).latest(

@rtibblesbot rtibblesbot Sep 9, 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: Reading the persisted Change row through the real sync endpoint pins the notification end-to-end rather than asserting the serializer appended to self.changes.

"server_rev"
)
self.assertEqual(
change.kwargs["mods"]["organization_status"], "pending_invitation"
)
self.assertEqual(
change.kwargs["mods"]["requested_organization_id"], organization.id
)
self.assertEqual(change.kwargs["mods"]["invitation_id"], invitation.id)

def test_update_channel_organization_migration_creates_contested_invitation(self):
user = testdata.user()
current_organization = testdata.organization()
target_organization = testdata.organization("Target Organization")
testdata.organization_role(user, target_organization, role=ORGANIZATION_EDITOR)
channel = models.Channel.objects.create(
actor_id=user.id,
organization=current_organization,
**self.channel_metadata,
)
channel.editors.add(user)

self.client.force_authenticate(user=user)
response = self.sync_changes(
[
generate_update_event(
channel.id,
CHANNEL,
{"organization": target_organization.id},
channel_id=channel.id,
)
]
)

self.assertEqual(response.status_code, 200, response.content)
channel.refresh_from_db()
self.assertEqual(channel.organization_id, current_organization.id)
self.assertTrue(
models.Invitation.objects.filter(
channel=channel, organization=target_organization
).exists()
)

def test_update_channel_organization_recreates_live_invitation_after_revocation(
self,
):
user = testdata.user()
current_organization = testdata.organization()
target_organization = testdata.organization("Target Organization")
testdata.organization_role(user, target_organization, role=ORGANIZATION_EDITOR)
channel = models.Channel.objects.create(
actor_id=user.id,
organization=current_organization,
**self.channel_metadata,
)
channel.editors.add(user)

self.client.force_authenticate(user=user)
response = self.sync_changes(
[
generate_update_event(
channel.id,
CHANNEL,
{"organization": target_organization.id},
channel_id=channel.id,
)
]
)
self.assertEqual(response.status_code, 200, response.content)

invitation = models.Invitation.objects.get(
channel=channel, organization=target_organization
)
response = self.sync_changes(
[
generate_update_event(
invitation.id,
INVITATION,
{"revoked": True},
channel_id=channel.id,
user_id=user.id,
)
]
)
self.assertEqual(response.status_code, 200, response.content)
invitation.refresh_from_db()
self.assertTrue(invitation.revoked)

response = self.sync_changes(
[
generate_update_event(
channel.id,
CHANNEL,
{"organization": target_organization.id},
channel_id=channel.id,
)
]
)
self.assertEqual(response.status_code, 200, response.content)
self.assertEqual(
models.Invitation.objects.filter(
channel=channel,
organization=target_organization,
revoked=False,
declined=False,
accepted=False,
).count(),

@rtibblesbot rtibblesbot Sep 9, 2026 •
edited
Loading

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.

✅ Resolved — addressed in the current code.

praise: Driving revoke through the sync endpoint and asserting count() == 1 pins the revoked/declined/accepted scoping so a get_or_create regression fails loudly.

1,
)

def test_channel_cannot_move_into_deleted_organization(self):
user = testdata.user()
organization = testdata.organization()
deleted_organization = testdata.organization("Deleted Org")
testdata.organization_role(user, deleted_organization, role=ORGANIZATION_EDITOR)

@rtibblesbot rtibblesbot Sep 9, 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.

suggestion: With the role granted the test now reaches the guard, but line 332 still only asserts len(errors) == 1, which passes for any validation failure — a regression where the pk fails to resolve keeps it green. Assert the message, as test_non_admin_cannot_remove_channel_from_organization does: response.json()["errors"][0]["errors"]["organization"][0].

deleted_organization.deleted = True

@rtibblesbot rtibblesbot Sep 8, 2026 •
edited
Loading

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.

✅ Resolved — addressed in the current code.

blocking: Green without the soft delete — no OrganizationRole on deleted_organization, so filter_view_queryset excludes it on membership alone; deleted = False passes locally with the same payload. Add testdata.organization_role(user, deleted_organization).

deleted_organization.save(update_fields=["deleted"])
channel = models.Channel.objects.create(
actor_id=user.id,
organization=organization,
**self.channel_metadata,
)
channel.editors.add(user)

self.client.force_authenticate(user=user)
response = self.sync_changes(
[
generate_update_event(
channel.id,
CHANNEL,
{"organization": deleted_organization.id},
channel_id=channel.id,
)
]
)

self.assertEqual(response.status_code, 200, response.content)
self.assertEqual(len(response.json()["errors"]), 1, response.content)

@rtibblesbot rtibblesbot Sep 9, 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.

suggestion: len(errors) == 1 passes whatever the error is — an unrelated 500 surfacing as a sync error would keep this green. test_non_admin_cannot_remove_channel_from_organization (line 362) now asserts the message; mirror that here with "Cannot assign a channel to a deleted organization."

self.assertEqual(
models.Channel.objects.get(id=channel.id).organization_id,
organization.id,
)

def test_non_admin_cannot_remove_channel_from_organization(self):
user = testdata.user()
organization = testdata.organization()
channel = models.Channel.objects.create(
actor_id=user.id,
organization=organization,
**self.channel_metadata,
)
channel.editors.add(user)

self.client.force_authenticate(user=user)
response = self.sync_changes(
[
generate_update_event(
channel.id,
CHANNEL,
{"organization": None},
channel_id=channel.id,
)
]
)

self.assertEqual(response.status_code, 200, response.content)
self.assertEqual(len(response.json()["errors"]), 1, response.content)

@rtibblesbot rtibblesbot Sep 8, 2026 •
edited
Loading

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.

✅ Resolved — addressed in the current code.

suggestion: len(errors) == 1 passes regardless of what the error is, which is why the Internal server error behaviour above is invisible in CI — assert the payload.

The positive half is also missing: no test that an org admin (or a Studio admin) can remove the channel, so the filter_edit_queryset(...).exists() branch at channel.py:357 is unexercised and a regression denying everyone would pass.

self.assertEqual(
response.json()["errors"][0]["errors"]["organization"][0],
"Only organization admins can remove a channel from an organization.",
)
self.assertEqual(
models.Channel.objects.get(id=channel.id).organization_id,
organization.id,
)

@rtibblesbot rtibblesbot Sep 9, 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.

suggestion: The AC "only organization admins can remove" is tested only negatively — nothing covers an organization admin successfully setting organization to None, so a regression that blocks admins too would pass. Add the mirror test with testdata.organization_role(user, organization, role=ORGANIZATION_ADMIN) asserting organization_id is None and no errors.


def test_update_channel_thumbnail_encoding(self):
user = testdata.user()
channel = models.Channel.objects.create(
Expand Down
51 changes: 51 additions & 0 deletions contentcuration/contentcuration/tests/viewsets/test_invitation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,32 @@ def test_revoke_organization_invitation_by_admin(self):
)
],
)
self.assertEqual(response.status_code, 200, response.content)
invitation.refresh_from_db()
self.assertTrue(invitation.revoked, response.content)

def test_update_contested_invitation_by_admin(self):
channel = models.Channel.objects.create(
actor_id=self.org_admin.id, organization=self.organization
)
invitation = models.Invitation.objects.create(
id=uuid.uuid4().hex,
channel=channel,
organization=self.organization,
sender=self.org_admin,
)
response = self.sync_changes(
[
generate_update_event(
invitation.id,
INVITATION,
{"revoked": True},
channel_id=channel.id,
user_id=self.org_admin.id,
)
]
)

self.assertEqual(response.status_code, 200, response.content)
invitation.refresh_from_db()
self.assertTrue(invitation.revoked)
Expand Down Expand Up @@ -775,6 +801,31 @@ def test_accept_invitation_by_admin_succeeds(self):
invitation.refresh_from_db()
self.assertTrue(invitation.accepted)

def test_accept_contested_channel_organization_invitation_by_admin_migrates_channel(
self,
):
current_organization = testdata.organization()
target_organization = testdata.organization("Target Organization")
invitation = models.Invitation.objects.create(
channel=self.channel,
organization=target_organization,
sender=self.user,
)
self.channel.organization = current_organization
self.channel.save(update_fields=["organization"])
admin_user = self._make_admin()

self.client.force_authenticate(user=admin_user)
response = self.client.post(
reverse("invitation-accept", kwargs={"pk": invitation.id})
)

self.assertEqual(response.status_code, 200, response.content)
invitation.refresh_from_db()
self.channel.refresh_from_db()
self.assertTrue(invitation.accepted)
self.assertEqual(self.channel.organization_id, target_organization.id)

def test_decline_invitation_by_admin_succeeds(self):
invitation = models.Invitation.objects.create(**self.invitation_db_metadata)
admin_user = self._make_admin()
Expand Down
Loading
Loading