Skip to content

Commit

Permalink
fix(organization): assign invitations to members correctly TASK-1600 (#…
Browse files Browse the repository at this point in the history
…5541)

### 📣 Summary
Fixed an issue where invitations were not properly assigned to the
correct organization members and ensured that invitations reflect the
intended recipient and organization.


### 📖 Description
An issue caused invitations to be incorrectly assigned, leading to
inconsistencies in organization membership management. This fix ensures
that invitations are correctly linked to the intended recipients,
preventing access errors or misdirected invitations.
  • Loading branch information
noliveleger authored Feb 20, 2025
1 parent aa138a9 commit 9fb0cdb
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 27 deletions.
14 changes: 11 additions & 3 deletions kobo/apps/organizations/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ class Meta:
]

def get_url(self, obj):

request = self.context.get('request')
return reverse(
'organization-members-detail',
Expand All @@ -93,14 +94,21 @@ def get_invite(self, obj):
"""
Get the latest invite for the user if it exists
"""
invite = OrganizationInvitation.objects.filter(
invitee=obj.user
).order_by('-created').first()
try:
invites_per_member = self.context['invites_per_member']
except KeyError:
invite = OrganizationInvitation.objects.filter(
organization=obj.organization,
invitee=obj.user
).order_by('-created').first()
else:
invite = invites_per_member.get(obj.user_id)

if invite:
return OrgMembershipInviteSerializer(
invite, context=self.context
).data

return {}

def to_representation(self, instance):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ def setUp(self):
'invitees': ['bob', '[email protected]']
}

def _create_invite(self, user):
def _create_invite(self, invited_by):
"""
Helper method to create invitations
"""
self.client.force_login(user)
self.client.force_login(invited_by)
return self.client.post(self.list_url, data=self.invitation_data)

def _update_invite(self, user, guid, status):
Expand Down
48 changes: 43 additions & 5 deletions kobo/apps/organizations/tests/test_organization_members_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,22 @@ def setUp(self):
},
)

def _create_invite(self, user):
def _create_invite(self, invited_by: 'User', invitees=None):
"""
Helper method to create and accept invitations
"""
invitation_data = {
'invitees': ['registered_invitee', '[email protected]']
}
if not invitees:
invitation_data = {
'invitees': ['registered_invitee', '[email protected]']
}
else:
invitation_data = {'invitees': invitees}

list_url = reverse(
self._get_endpoint('organization-invites-list'),
kwargs={'organization_id': self.organization.id},
)
self.client.force_login(user)
self.client.force_login(invited_by)
self.client.post(list_url, data=invitation_data)

@data(
Expand Down Expand Up @@ -192,3 +196,37 @@ def test_post_request_is_not_allowed(self, user_role, expected_status):
data = {'role': 'admin'}
response = self.client.post(self.list_url, data)
self.assertEqual(response.status_code, expected_status)

def test_invitation_is_correctly_assigned_in_member_list(self):

bob_org = self.bob.organization
bob_org.mmo_override = True
bob_org.save(update_fields=['mmo_override'])

# Let someuser invite bob to join their org
self._create_invite(invited_by=self.someuser, invitees=['bob'])

# Look at bob's membership detail endpoint in bob's org,
# someuser's invite should not be there
self.client.force_login(self.bob)
bob_org_members_list_url = reverse(
self._get_endpoint('organization-members-list'),
kwargs={'organization_id': bob_org.id},
)
response = self.client.get(bob_org_members_list_url)
# The first member should be bob
assert response.data['results'][0]['user__username'] == 'bob'
assert response.data['results'][0]['invite'] == {}

# Look at bob's membership detail endpoint in someother's org,
# someuser's invite should **BE** there
self.client.force_login(self.someuser)
someuser_org_members_list_url = reverse(
self._get_endpoint('organization-members-list'),
kwargs={'organization_id': self.organization.id},
)
response = self.client.get(someuser_org_members_list_url)

# The last invite should be bob's one
assert response.data['results'][-1]['invite']['invitee'] == 'bob'
assert response.data['results'][-1]['invite']['status'] == 'pending'
86 changes: 69 additions & 17 deletions kobo/apps/organizations/views.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
from django.db import transaction
from django.db.models import Case, CharField, OuterRef, QuerySet, Value, When, Q
from django.db.models import (
Case,
CharField,
F,
OuterRef,
Q,
QuerySet,
Value,
When,
)
from django.db.models.expressions import Exists
from django.utils.http import http_date
from rest_framework import status, viewsets
Expand All @@ -20,7 +29,8 @@
from kpi.views.v2.asset import AssetViewSet
from .models import Organization, OrganizationOwner, OrganizationUser
from .models import (
OrganizationInvitation
OrganizationInvitation,
OrganizationInviteStatusChoices,
)
from .permissions import (
HasOrgRolePermission,
Expand Down Expand Up @@ -436,6 +446,23 @@ class OrganizationMemberViewSet(viewsets.ModelViewSet):
http_method_names = ['get', 'patch', 'delete']
lookup_field = 'user__username'

def paginate_queryset(self, queryset):
page = super().paginate_queryset(queryset)
members_user_ids = []
organization_id = self.kwargs['organization_id']

for obj in page:
if obj.model_type != '0_organization_user':
break
members_user_ids.append(obj.user_id)

self._invites_queryset = OrganizationInvitation.objects.filter( # noqa
status=OrganizationInviteStatusChoices.ACCEPTED,
invitee_id__in=members_user_ids,
organization_id=organization_id
).order_by('invitee_id', 'created')
return page

def get_queryset(self):
organization_id = self.kwargs['organization_id']

Expand All @@ -452,42 +479,67 @@ def get_queryset(self):
).values('pk')

# Annotate with the role based on organization ownership and admin status
queryset = OrganizationUser.objects.filter(
organization_id=organization_id
).select_related('user__extra_details').annotate(
role=Case(
When(Exists(owner_subquery), then=Value('owner')),
When(is_admin=True, then=Value('admin')),
default=Value('member'),
output_field=CharField()
),
has_mfa_enabled=Exists(mfa_subquery)
queryset = (
OrganizationUser.objects.filter(organization_id=organization_id)
.select_related('user__extra_details')
.annotate(
role=Case(
When(Exists(owner_subquery), then=Value('owner')),
When(is_admin=True, then=Value('admin')),
default=Value('member'),
output_field=CharField(),
),
has_mfa_enabled=Exists(mfa_subquery),
invite=Value(None, output_field=CharField()),
ordering_date=F('created'),
model_type=Value('0_organization_user', output_field=CharField()),
)
)

if self.action == 'list':
# Include invited users who are not yet part of this organization
invitation_queryset = OrganizationInvitation.objects.filter(
organization_id=organization_id,
status__in=['pending', 'resent']
status__in=[
OrganizationInviteStatusChoices.PENDING,
OrganizationInviteStatusChoices.RESENT,
],
)

# Get existing user IDs from the queryset
members_user_ids = queryset.values_list('user_id', flat=True)
invitees = invitation_queryset.filter(
Q(invitee_id__isnull=True) | ~Q(invitee_id__in=members_user_ids)
).annotate(
ordering_date=F('created'),
model_type=Value('1_organization_invitation', output_field=CharField()),
)
queryset = list(queryset) + list(invitees)
queryset = sorted(
queryset, key=lambda x: (x.model_type, x.ordering_date)
)

return queryset

def destroy(self, request, *args, **kwargs):
def get_serializer_context(self):
context = super().get_serializer_context()

if hasattr(self, '_invites_queryset'):
invites_per_member = {}
for invite in self._invites_queryset:
invites_per_member[invite.invitee_id] = invite
context['invites_per_member'] = invites_per_member

return context

def perform_destroy(self, instance):
"""
Revoke asset permissions before deleting the user from the organization
"""
member = self.get_object()
member = instance
with transaction.atomic():
revoke_org_asset_perms(member.organization, [member.user_id])
member.delete()
return Response(status=status.HTTP_204_NO_CONTENT)
super().perform_destroy(member)


class OrgMembershipInviteViewSet(viewsets.ModelViewSet):
Expand Down

0 comments on commit 9fb0cdb

Please sign in to comment.