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
GroupDeleteService.delete() calls self.request.db.delete(group) (ORM‑level delete on the Group instance)
- 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)
- 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)
- Since
group_id on user_group is nullable=False, the ORM tries UPDATE user_group SET group_id=NULL, which raises an IntegrityError
- 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:
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.
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 inGroupDeleteService.delete()wheresession.delete(group)triggers SQLAlchemy to process themembershipsrelationship 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/hypothesisimage). 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
GroupDeleteService.delete()callsself.request.db.delete(group)(ORM‑level delete on theGroupinstance)Group.membershipsrelationship has nolazyparameter (defaults to"select") and nopassive_deletes=Trueor ORM‑level cascade (cascade="all, delete-orphan"is absent — contrast withscopeswhich has it)passive_deletes=False(default), issues a SELECT to load the relatedGroupMembershiprows, then attempts to handle them per the default relationship cascade (save‑update, merge — no delete)group_idonuser_groupisnullable=False, the ORM triesUPDATE user_group SET group_id=NULL, which raises anIntegrityErrorThe database‑level foreign key on
user_group.group_idis correctly defined withON 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:This lets the database‑level
ON DELETE CASCADEclean upuser_groupandgroupscoperows without the ORM interfering. Annotations are soft‑deleted separately by the existing_delete_annotationscall (annotations reference groups by stringgroupid, 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.