Skip to content

Commit e1c5830

Browse files
committed
Delete groups based on owners not creator
When deleting a user account we currently delete any groups that the user created and that don't contain any annotations by other users. This made sense when the creator was the only person who could edit the group (changing its name, description and type) or moderate annotations in the group, so groups with no creator were kind of broken. But we've now added the concept of group membership roles and in particular a group can have multiple "owners" who all have full permissions to edit and moderate the group. Even if the group's creator no longer exists, the group may still have other owners who have full control of the group. So instead of deleting groups that the user created, change `UserDeleteService` to delete groups that the user is the only owner of (and that don't contain any annotations by other users).
1 parent 795eafc commit e1c5830

File tree

1 file changed

+31
-14
lines changed

1 file changed

+31
-14
lines changed

h/services/user_delete.py

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -195,15 +195,15 @@ def delete_annotations(self, user):
195195

196196
def delete_groups(self, user):
197197
"""
198-
Delete groups created by `user` that have no annotations.
198+
Delete groups owned by `user` that have no annotations.
199199
200-
Delete groups that were created by `user` and that don't contain any
201-
non-deleted annotations.
200+
Delete groups that that `user` is the *only* owner of and that don't
201+
contain any non-deleted annotations.
202202
203203
If delete_annotations() (above) is called first then all of `user`'s
204204
own annotations will already have been deleted, so ultimately any
205-
groups created by `user` that don't contain any annotations by *other*
206-
users will get deleted.
205+
groups that `user` is the only owner of and that don't contain any
206+
annotations by *other* users will get deleted.
207207
208208
Known issue: if this method does not delete a group because it contains
209209
annotations by other users, and those annotations other users are later
@@ -215,22 +215,39 @@ def delete_groups(self, user):
215215
really large number of members this could take too long and cause a
216216
timeout.
217217
"""
218-
# pylint:disable=not-callable,use-implicit-booleaness-not-comparison-to-zero
219-
self.worker.delete(
220-
Group,
218+
# pylint:disable=not-callable
219+
# pylint:disable=use-implicit-booleaness-not-comparison-to-zero
220+
221+
# The IDs of all groups that have only one owner.
222+
groups_with_only_one_owner = (
223+
select(GroupMembership.group_id)
224+
.where(GroupMembership.roles.contains(["owner"]))
225+
.group_by(GroupMembership.group_id)
226+
.having(func.count(GroupMembership.group_id) == 1)
227+
)
228+
229+
# The IDs of all groups where `user` is the only owner.
230+
groups_where_user_is_only_owner = (
231+
select(GroupMembership.group_id)
232+
.where(GroupMembership.group_id.in_(groups_with_only_one_owner))
233+
.where(GroupMembership.user_id == user.id)
234+
)
235+
236+
# The IDs of all groups where `user` is the only owner *and* the group
237+
# doesn't contain any annotations by other users.
238+
groups_to_be_deleted = (
221239
select(Group.id)
222-
.where(Group.creator_id == user.id)
240+
.where(Group.id.in_(groups_where_user_is_only_owner))
223241
.outerjoin(
224242
Annotation,
225-
and_(
226-
Annotation.groupid == Group.pubid,
227-
Annotation.deleted.is_(False),
228-
),
243+
and_(Annotation.groupid == Group.pubid, Annotation.deleted.is_(False)),
229244
)
230245
.group_by(Group.id)
231-
.having(func.count(Annotation.id) == 0),
246+
.having(func.count(Annotation.id) == 0)
232247
)
233248

249+
self.worker.delete(Group, groups_to_be_deleted)
250+
234251
def delete_group_memberships(self, user):
235252
"""
236253
Delete `user`'s group memberships.

0 commit comments

Comments
 (0)