Skip to content

Commit 38f4d1d

Browse files
author
kobo-bot[bot]
committed
Merge branch 'release/2.025.47' into release/2.026.03
2 parents b7a532f + 2fe5589 commit 38f4d1d

File tree

6 files changed

+83
-14
lines changed

6 files changed

+83
-14
lines changed

kobo/apps/long_running_migrations/jobs/0015_fix_duplicate_organizations.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def run():
8484
for org_user in org_user_rows:
8585
user_to_orgs[org_user.user_id].append(org_user.organization)
8686

87-
for uid, orgs in user_to_orgs.items():
87+
for user_id, orgs in user_to_orgs.items():
8888
mmos = [org for org in orgs if is_effective_mmo(org)]
8989
if mmos:
9090
keep = get_newest_org(mmos)
@@ -98,14 +98,16 @@ def run():
9898
to_remove = [org for org in orgs if org.id != keep.id]
9999

100100
# Transfer asset if the user is not the owner of the kept org
101-
if keep.owner_user_object.pk != uid:
102-
transfer_member_data_ownership_to_org(uid)
101+
if keep.owner_user_object.pk != user_id:
102+
transfer_member_data_ownership_to_org(user_id)
103103

104104
for org in to_remove:
105105
with transaction.atomic():
106-
revoke_org_asset_perms(org, [uid])
106+
if org.owner_user_object.pk != user_id:
107+
revoke_org_asset_perms(org, [user_id])
108+
107109
OrganizationUser.objects.filter(
108-
organization_id=org.id, user_id=uid
110+
organization_id=org.id, user_id=user_id
109111
).delete()
110112

111113
# If no members remain, delete the organization
@@ -114,7 +116,9 @@ def run():
114116
organization_id=org.id
115117
).delete()
116118
org.delete()
117-
logging.info(f'Deleted organization {org.id} for user {uid}')
119+
logging.info(
120+
f'Deleted organization {org.id} for user {user_id}'
121+
)
118122

119123
# Ensure MMO status is consistent for multi-member orgs
120124
if keep.mmo_override is False and keep.organization_users.count() > 1:

kobo/apps/long_running_migrations/tasks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def async_execute(migration_id: int):
2525
cache.delete(lock_key)
2626

2727

28-
@celery_app.task(queue='kpi_low_priority_queue')
28+
@celery_app.task(queue='kpi_long_running_tasks_queue')
2929
def execute_long_running_migrations():
3030
# Adding an offset to account for potential delays in task execution and
3131
# clock drift between the Celery workers and the database, ensuring tasks

kobo/apps/organizations/tests/test_fix_duplicate_organizations.py

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
OrganizationOwner,
1010
OrganizationUser,
1111
)
12-
from kpi.models import Asset
12+
from kpi.models import Asset, ObjectPermission
1313

1414
job = import_module('kobo.apps.long_running_migrations.jobs.0015_fix_duplicate_organizations') # noqa
1515

@@ -149,6 +149,57 @@ def test_asset_transfer_to_kept_organization(self):
149149
self.assertEqual(asset2.owner, self.anotheruser)
150150
self.assertEqual(asset3.owner, self.anotheruser)
151151

152+
def test_owner_does_not_lose_permissions_on_removal(self):
153+
"""
154+
Verify that when a duplicate organization is removed, if the user being
155+
processed is the owner of that org, they DO NOT lose their own permissions
156+
"""
157+
# Give someuser a second organization where they are also the owner
158+
self._create_organization_for_user(user=self.someuser)
159+
160+
# Create an asset owned by someuser and assign them a permission
161+
asset = Asset.objects.create(name='Owner Project', owner=self.someuser)
162+
163+
# Get all permissions for the asset
164+
initial_perms = ObjectPermission.objects.filter(
165+
user=self.someuser,
166+
asset=asset,
167+
deny=False,
168+
).values('permission_id', 'permission__codename')
169+
170+
expected_permissions = [
171+
'add_submissions',
172+
'change_asset',
173+
'change_submissions',
174+
'delete_submissions',
175+
'manage_asset',
176+
'validate_submissions',
177+
'view_asset',
178+
'view_submissions',
179+
]
180+
initial_codenames = sorted([
181+
obj_perm['permission__codename'] for obj_perm in initial_perms
182+
])
183+
assert expected_permissions == initial_codenames
184+
185+
job.run()
186+
187+
# Verify that only one organization remains for someuser
188+
self.assertEqual(
189+
OrganizationUser.objects.filter(user=self.someuser).count(), 1
190+
)
191+
192+
# Verify that someuser still has all their initial permissions on the asset
193+
final_perms = set(ObjectPermission.objects.filter(
194+
user=self.someuser,
195+
asset=asset,
196+
deny=False,
197+
).values_list('permission_id', flat=True))
198+
199+
assert set([
200+
obj_perm['permission_id'] for obj_perm in initial_perms
201+
]) == final_perms
202+
152203
def _create_organization_for_user(self, user, mmo_override=False):
153204
org = Organization.objects.create(name='Org', mmo_override=mmo_override)
154205
org_user = OrganizationUser.objects.create(organization=org, user=user)

kobo/apps/organizations/utils.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,23 @@ def revoke_org_asset_perms(organization: Organization, user_ids: list[int]):
4545
the organization.
4646
"""
4747
Asset = apps.get_model('kpi', 'Asset') # noqa
48+
owner_id = organization.owner_user_object.pk
49+
50+
# Filter out the owner from user_ids to prevent them from
51+
# revoking their own access
52+
safe_user_ids = [user_id for user_id in user_ids if user_id != owner_id]
53+
54+
if not safe_user_ids:
55+
return
56+
4857
subquery = Asset.objects.values_list('pk', flat=True).filter(
49-
owner=organization.owner_user_object
58+
owner_id=owner_id
5059
)
51-
ObjectPermission.objects.filter(
52-
asset_id__in=subquery, user_id__in=user_ids
53-
).delete()
60+
perms_to_delete = ObjectPermission.objects.filter(
61+
asset_id__in=subquery, user_id__in=safe_user_ids
62+
)
63+
64+
batch_size = settings.DEFAULT_BATCH_SIZE
65+
while perms_to_delete.exists():
66+
pks_to_delete = list(perms_to_delete.values_list('pk', flat=True)[:batch_size])
67+
ObjectPermission.objects.filter(pk__in=pks_to_delete).delete()

kobo/settings/base.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2181,6 +2181,7 @@ def dj_stripe_request_callback_method():
21812181
ORG_INVITATION_RESENT_RESET_AFTER = 15 * 60 # in seconds
21822182

21832183
# Batch sizes
2184+
DEFAULT_BATCH_SIZE = 1000
21842185
LOG_DELETION_BATCH_SIZE = 1000
21852186
USER_ASSET_ORG_TRANSFER_BATCH_SIZE = 1000
21862187
SUBMISSION_DELETION_BATCH_SIZE = 1000

kpi/utils/mongo_helper.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ class MongoHelper:
7373
# Match KoBoCAT's variables of ParsedInstance class
7474
USERFORM_ID = '_userform_id'
7575
SUBMISSION_UUID = '_uuid'
76-
DEFAULT_BATCHSIZE = 1000
7776
COLLECTION = 'instances'
7877

7978
@classmethod
@@ -158,7 +157,7 @@ def get_instances(
158157
cursor.sort(sort_key, sort_dir)
159158

160159
# set batch size
161-
cursor.batch_size = cls.DEFAULT_BATCHSIZE
160+
cursor.batch_size = settings.DEFAULT_BATCH_SIZE
162161

163162
return cursor, total_count
164163

0 commit comments

Comments
 (0)