hypothesis / hypothesis/h

Admin group deletion fails with IntegrityError due to ORM cascade mismatch on memberships

Open
#10,106 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
3.2k
Forks
459
Avg merge
27d 1h
Merged PRs (30d)
1

Description

### Summary

Deleting a group via the admin panel (`/admin/groups/delete/{id}`) results in a 500 error and the group is not deleted. The issue appears to be in `GroupDeleteService.delete()` where `session.delete(group)` triggers SQLAlchemy to process the `memberships` relationship in a way that conflicts with the underlying schema.

### Deployment context

We are running a self‑hosted deployment of Hypothesis, with the backend running inside Docker containers (using the official `hypothesis/hypothesis` image). The admin panel is exposed behind an nginx reverse proxy, and the issue was observed when attempting to delete a group via the admin web interface. The environment uses PostgreSQL 15, and the database schema is the stock Hypothesis schema with no custom modifications.
### What happens

1. `GroupDeleteService.delete()` calls `self.request.db.delete(group)` (ORM‑level delete on the `Group` instance)
2. The `Group.memberships` relationship has no `lazy` parameter (defaults to `"select"`) and no `passive_deletes=True` or ORM‑level cascade (`cascade="all, delete-orphan"` is absent — contrast with `scopes` which has it)
3. SQLAlchemy, seeing `passive_deletes=False` (default), issues a SELECT to load the related `GroupMembership` rows, then attempts to handle them per the default relationship cascade (save‑update, merge — no delete)
4. Since `group_id` on `user_group` is `nullable=False`, the ORM tries `UPDATE user_group SET group_id=NULL`, which raises an `IntegrityError`
5. The transaction rolls back and the group remains

The database‑level foreign key on `user_group.group_id` is correctly defined with `ON DELETE CASCADE`, but SQLAlchemy never gets to rely on it because it tries to do its own bookkeeping first.

### Suggested fix

Replace the ORM‑level `session.delete(group)` with a bulk delete that bypasses relationship processing:

```python
from sqlalchemy import delete as sa_delete
from h.models import Group

# in GroupDeleteService.delete():
self.request.db.execute(
sa_delete(Group).where(Group.id == group.id)
)
```

This lets the database‑level `ON DELETE CASCADE` clean up `user_group` and `groupscope` rows without the ORM interfering. Annotations are soft‑deleted separately by the existing `_delete_annotations` call (annotations reference groups by string `groupid`, not by FK, so they are unaffected by the cascade).

Would this change be acceptable, or is there a reason the ORM‑level delete was preferred here? Happy to open a PR if the approach looks reasonable.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.