Summary

Guard deletion of the GitHub App record account.

Review Request #15194 — Created July 23, 2026 and submitted Aug. 31, 2026, 9:19 a.m.

Information

Owner

david

Repository

Review Board

Branch

release-9.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Installation accounts reference their hidden app-record account by
primary key stored in JSON data rather than a database foreign key, so
nothing at the database level stops the record from being deleted out
from under them. Deleting it would silently break every installation of
the app, which authenticates using the record's stored credentials.

The django admin allows implementing a get_deleted_objects() method in
order to allow us to prevent this.

Testing Done:
Ran unit tests.

Testing Done


Attempted to delete the app record and saw that it was protected.
Ran unit tests.

Commits

Summary	ID
Guard deletion of the GitHub App record account. Installation accounts reference their hidden app-record account by primary key stored in JSON data rather than a database foreign key, so nothing at the database level stops the record from being deleted out from under them. Deleting it would silently break every installation of the app, which authenticates using the record's stored credentials. The django admin allows implementing a `get_deleted_objects()` method in order to allow us to prevent this. Testing Done: Ran unit tests.	wmqqtrnulwzukrppwumnnmszmqprzqxp

Summary

Guard deletion of the GitHub App record account.

Installation accounts reference their hidden app-record account by primary key stored in JSON data rather than a database foreign key, so nothing at the database level stops the record from being deleted out from under them. Deleting it would silently break every installation of the app, which authenticates using the record's stored credentials. The django admin allows implementing a `get_deleted_objects()` method in order to allow us to prevent this. Testing Done: Ran unit tests.

wmqqtrnulwzukrppwumnnmszmqprzqxp

Issues

Description	From	Last Updated
Django-stubs types this as Sequence[... \| None], so we might need to handle this case.	chipx86	Aug. 28, 2026, 11:13 a.m.
This wraps weirdly. Can we just query for the objects above and then pass that in here?	chipx86	Aug. 28, 2026, 11:13 a.m.
Can we use the normal multi-line form here?	chipx86	Aug. 28, 2026, 11:14 a.m.
Can you put this in parens or indent to better indicate these aren't two things in the conditional?	chipx86	Aug. 28, 2026, 11:14 a.m.
We shouldn't have to type this.	chipx86	Aug. 28, 2026, 11:15 a.m.
Can we alphabetize these?	chipx86	Aug. 28, 2026, 11:15 a.m.
Can we alphabetize these?	chipx86	Aug. 28, 2026, 11:15 a.m.
Can we alphabetize these?	chipx86	Aug. 28, 2026, 11:15 a.m.
Can we alphabetize these?	chipx86	Aug. 28, 2026, 11:15 a.m.
Can we alphabetize these?	chipx86	Aug. 28, 2026, 11:15 a.m.
'collections.abc.Sequence' imported but unused Column: 5 Error code: F401	reviewbot	Aug. 28, 2026, 11:16 a.m.
'typing.ClassVar' imported but unused Column: 5 Error code: F401	reviewbot	Aug. 28, 2026, 11:16 a.m.

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

reviewboard/hostingsvcs/admin.py (Diff revision 1)

The issue has been dropped. Show all issues

Django-stubs types this as Sequence[... | None], so we might need to handle this case.

david

Aug. 28, 2026, 11:16 a.m.

No?

def get_deleted_objects(
    self, objs: Sequence[_ModelT] | QuerySet[_ModelT], request: HttpRequest
) -> tuple[list[str], dict[str, int], set[str], list[str]]: ...

reviewboard/hostingsvcs/admin.py (Diff revision 1)

The issue has been resolved. Show all issues

This wraps weirdly. Can we just query for the objects above and then pass that in here?

reviewboard/hostingsvcs/github/service.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we use the normal multi-line form here?
```

reviewboard/hostingsvcs/github/service.py (Diff revision 1)

The issue has been resolved. Show all issues

Can you put this in parens or indent to better indicate these aren't two things in the conditional?

reviewboard/hostingsvcs/tests/github/test_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We shouldn't have to type this.
```
reviewboard/hostingsvcs/tests/github/test_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we alphabetize these?
```
reviewboard/hostingsvcs/tests/github/test_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we alphabetize these?
```
reviewboard/hostingsvcs/tests/github/test_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we alphabetize these?
```
reviewboard/hostingsvcs/tests/github/test_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we alphabetize these?
```
reviewboard/hostingsvcs/tests/github/test_app.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we alphabetize these?
```

Commits:

	Summary	ID
	Guard deletion of the GitHub App record account. Installation accounts reference their hidden app-record account by primary key stored in JSON data rather than a database foreign key, so nothing at the database level stops the record from being deleted out from under them. Deleting it would silently break every installation of the app, which authenticates using the record's stored credentials. The django admin allows implementing a `get_deleted_objects()` method in order to allow us to prevent this. Testing Done: Ran unit tests.	wmqqtrnulwzukrppwumnnmszmqprzqxp
	Guard deletion of the GitHub App record account. Installation accounts reference their hidden app-record account by primary key stored in JSON data rather than a database foreign key, so nothing at the database level stops the record from being deleted out from under them. Deleting it would silently break every installation of the app, which authenticates using the record's stored credentials. The django admin allows implementing a `get_deleted_objects()` method in order to allow us to prevent this. Testing Done: Ran unit tests.	wmqqtrnulwzukrppwumnnmszmqprzqxp

Diff:

Revision 2 (+672 -2)

Show changes

	reviewboard/hostingsvcs/admin.py
	reviewboard/hostingsvcs/base/hosting_service.py
	reviewboard/hostingsvcs/github/service.py
	reviewboard/hostingsvcs/tests/github/test_app.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/hostingsvcs/tests/github/test_app.py (Diff revision 2)
The issue has been dropped. Show all issues
```
'collections.abc.Sequence' imported but unused

Column: 5
Error code: F401
```
reviewboard/hostingsvcs/tests/github/test_app.py (Diff revision 2)
The issue has been dropped. Show all issues
```
'typing.ClassVar' imported but unused

Column: 5
Error code: F401
```

Commits:

	Summary	ID
	Guard deletion of the GitHub App record account. Installation accounts reference their hidden app-record account by primary key stored in JSON data rather than a database foreign key, so nothing at the database level stops the record from being deleted out from under them. Deleting it would silently break every installation of the app, which authenticates using the record's stored credentials. The django admin allows implementing a `get_deleted_objects()` method in order to allow us to prevent this. Testing Done: Ran unit tests.	wmqqtrnulwzukrppwumnnmszmqprzqxp
	Guard deletion of the GitHub App record account. Installation accounts reference their hidden app-record account by primary key stored in JSON data rather than a database foreign key, so nothing at the database level stops the record from being deleted out from under them. Deleting it would silently break every installation of the app, which authenticates using the record's stored credentials. The django admin allows implementing a `get_deleted_objects()` method in order to allow us to prevent this. Testing Done: Ran unit tests.	wmqqtrnulwzukrppwumnnmszmqprzqxp

Diff:

Revision 3 (+668)

Show changes

	reviewboard/hostingsvcs/admin.py
	reviewboard/hostingsvcs/base/hosting_service.py
	reviewboard/hostingsvcs/github/service.py
	reviewboard/hostingsvcs/tests/github/test_app.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Status:: Completed
Change Summary:: Pushed to release-9.x (be176f5)