6009: trim comments, scope accept() to actual toggles, per rtibbles review

- InvitationSerializer.update(): read `accepted` only from validated_data
  (drop the `or instance.accepted` fallback) so accept() only fires on an
  actual incoming toggle, not on every later update to an
  already-accepted invitation, per rtibbles' review comment.
- Trim the LLM-verbose in-line comments this PR added in invitation.py
  and test_invitation.py down to 1-2 lines each, per rtibbles' request
  to keep them concise. Also softened the co-owner share_mode test
  comment, which asserted a maintainer confirmation that wasn't on
  record, to describe observed behavior instead.

Author: yazzylazy

contentcuration/contentcuration/tests/viewsets/test_invitation.py

@@ -147,11 +147,9 @@ def test_update_invitation_accept(self):
         self.assertTrue(models.Change.objects.filter(channel=self.channel).exists())
 
     def test_update_invitation_accept_admin_share_mode_grants_editor_access(self):
-        # A channel invitation's "co-owner" share_mode (admin) documents
-        # existing behavior rather than introducing a new access tier:
         # _accept_channel_invitation only special-cases VIEW_ACCESS, so
-        # anything else - including "admin" - grants the same editor access
-        # as "edit". This locks that in as intended, confirmed behavior.
+        # "admin" currently grants the same editor access as "edit" -
+        # documenting observed behavior, not asserting it's the intended design.
         invitation = models.Invitation.objects.create(
             share_mode=ADMIN_ACCESS, **self.invitation_db_metadata
         )
@@ -560,13 +558,8 @@ def test_invitation_with_channel_and_organization_is_rejected(self):
             pass
 
     def test_create_organization_invitation_without_user_id_is_rejected(self):
-        # Organization-scoped invitation changes get no special routing in
-        # handle_changes - like any other change with no channel_id, they
-        # only go through if the client tags user_id as its own id. Omitting
-        # it isn't org-specific behaviour, it's rejected the same way any
-        # other self-only change with a missing/mismatched user_id is, and
-        # the actor gets that feedback back in "disallowed" rather than a
-        # silent no-op.
+        # No org-specific routing in handle_changes - a missing user_id is
+        # rejected like any other self-only change, with feedback returned.
         invitation = self.invitation_metadata
         response = self.sync_changes(
             [
@@ -589,11 +582,8 @@ def test_create_organization_invitation_without_user_id_is_rejected(self):
             pass
 
     def test_organization_invitation_change_with_mismatched_user_id_is_rejected(self):
-        # A client-supplied user_id that doesn't match the actor must not be
-        # trusted, since that would let an org admin inject a change into an
-        # arbitrary user's sync feed. There's no org-specific routing to fall
-        # back to, so it's simply rejected like any other self-only change
-        # with a mismatched user_id.
+        # A user_id that doesn't match the actor is rejected, not routed
+        # elsewhere - it must not inject a change into another user's feed.
         unrelated_user = testdata.user("unrelated-target@inc.com")
         invitation = self.invitation_metadata
         response = self.sync_changes(
@@ -668,11 +658,8 @@ def test_revoke_organization_invitation_by_different_admin(self):
         self.assertTrue(invitation.revoked)
 
     def test_admin_cannot_force_accept_on_behalf_of_invitee(self):
-        # Org-admin edit rights on the queryset must not let an admin trigger
-        # instance.accept() on someone else's invitation by syncing
-        # {"accepted": true} themselves - accepted is correctly kept
-        # read-only for them (get_fields), so this must be a no-op, not a
-        # silent OrganizationRole grant.
+        # Org-admin edit rights must not let an admin trigger accept() on
+        # someone else's invitation - accepted stays read-only for them.
         invitation = models.Invitation.objects.create(
             id=uuid.uuid4().hex,
             organization=self.organization,
@@ -722,10 +709,8 @@ def test_delete_organization_invitation(self):
             pass
 
     def test_accept_organization_invitation_created_via_sync(self):
-        # Unlike the fixtures above (which set `invited` directly via the
-        # ORM), an invitation created through the sync API - the real
-        # creation path - never gets `invited` populated. The real invitee
-        # must still be able to accept it.
+        # Unlike the fixtures above, an invitation created via sync never
+        # gets `invited` populated - the real invitee must still accept it.
         invitation = self.invitation_metadata
         response = self.sync_changes(
             [

contentcuration/contentcuration/viewsets/invitation.py

@@ -74,13 +74,9 @@ def create(self, validated_data):
 
     def update(self, instance, validated_data):
         instance = super(InvitationSerializer, self).update(instance, validated_data)
-        # Read from validated_data, not initial_data (the raw client mods
-        # dict) - get_fields() correctly keeps accepted/revoked read-only for
-        # anyone but the invitee/sender, but initial_data bypasses that,
-        # letting e.g. an org admin force instance.accept() (and thus an
-        # active OrganizationRole) onto an invitation that isn't theirs to
-        # accept, even though the field itself stays correctly unwritten.
-        accepted = validated_data.get("accepted") or instance.accepted
+        # validated_data, not initial_data, respects get_fields' read-only
+        # flags; only trigger accept() on an actual incoming toggle.
+        accepted = validated_data.get("accepted")
         revoked = validated_data.get("revoked") or instance.revoked
 
         if accepted and not revoked:
@@ -105,10 +101,8 @@ def get_fields(self):
 
         # allow invitation state to be modified under the right conditions
         if request and request.user and self.instance:
-            # Match on email rather than the `invited` FK, since `invited` is
-            # only ever populated by the channel email-invite flow - it's
-            # never set for invitations created through the sync API, which
-            # would otherwise leave the real invitee unable to accept/decline.
+            # Match on email, not the `invited` FK - `invited` is only set by
+            # the channel email-invite flow, never for sync-created invitations.
             if (request.user.email or "").lower() == (
                 self.instance.email or ""
             ).lower():