Summary

Improve GitHub repo owner/name storage.

Review Request #15342 — Created Sept. 24, 2026 and updated Sept. 25, 2026, 9:04 a.m.

Information

Owner

david

Repository

Review Board

Branch

release-9.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

The way that GitHub repos were storing their owner name was quite

awkward. Each individual form (private user, public user, private org,

public org) had its own prefix, and when we'd need to conditionally look

up keys based on this.
This change adds a new storage mechanism and accessor for the GitHub

repository owner and name. These are stored in a new canonical form by

the GitHub repository form(s). If the repo does not have the new

canonical form, it will fall back to the ugly lookups previously used.

The fallback (and check_repository(), which resolves the owner and

name inline from submitted form data before anything is saved) respects

plan_field_prefix, keeping subclasses like GitHub Enterprise working.
As part of this, get_repository_display_path() no longer

reverse-engineers the owner and name from the clone URL, since the same

information is available in extra_data. If the stored data is unusable,

it falls back to the raw path rather than parsing it.
The _get_repository_owner_raw() and _get_repository_name_raw()

methods are removed. Their only remaining caller was

check_repository(), which now resolves the owner and name itself.

Testing Done

Ran unit tests.

Commits

Summary	ID
Improve GitHub repo owner/name storage. The way that GitHub repos were storing their owner name was quite awkward. Each individual form (private user, public user, private org, public org) had its own prefix, and when we'd need to conditionally look up keys based on this. This change adds a new storage mechanism and accessor for the GitHub repository owner and name. These are stored in a new canonical form by the GitHub repository form(s). If the repo does not have the new canonical form, it will fall back to the ugly lookups previously used. The fallback (and `check_repository()`, which resolves the owner and name inline from submitted form data before anything is saved) respects `plan_field_prefix`, keeping subclasses like GitHub Enterprise working. As part of this, `get_repository_display_path()` no longer reverse-engineers the owner and name from the clone URL, since the same information is available in extra_data. If the stored data is unusable, it falls back to the raw path rather than parsing it. The `_get_repository_owner_raw()` and `_get_repository_name_raw()` methods are removed. Their only remaining caller was `check_repository()`, which now resolves the owner and name itself. Testing Done: Ran unit tests.	ulsqxkwnsuosurznqrplyxxyozyrmvwm

Summary

Improve GitHub repo owner/name storage.

The way that GitHub repos were storing their owner name was quite awkward. Each individual form (private user, public user, private org, public org) had its own prefix, and when we'd need to conditionally look up keys based on this. This change adds a new storage mechanism and accessor for the GitHub repository owner and name. These are stored in a new canonical form by the GitHub repository form(s). If the repo does not have the new canonical form, it will fall back to the ugly lookups previously used. The fallback (and `check_repository()`, which resolves the owner and name inline from submitted form data before anything is saved) respects `plan_field_prefix`, keeping subclasses like GitHub Enterprise working. As part of this, `get_repository_display_path()` no longer reverse-engineers the owner and name from the clone URL, since the same information is available in extra_data. If the stored data is unusable, it falls back to the raw path rather than parsing it. The `_get_repository_owner_raw()` and `_get_repository_name_raw()` methods are removed. Their only remaining caller was `check_repository()`, which now resolves the owner and name itself. Testing Done: Ran unit tests.

ulsqxkwnsuosurznqrplyxxyozyrmvwm

Issues

Description	From	Last Updated
Should we call .lower() on the owner and name when we save it so that its always in the casing …	maubin	Sept. 25, 2026, 8:11 a.m.
This calls GitHub directly, so it won't work for GitHub Enterprise. We should use self._hosting_service_cls here instead.	maubin	Sept. 25, 2026, 8:03 a.m.
We should document that this can now raise KeyError and InvalidPlanError.	maubin	Sept. 25, 2026, 8:03 a.m.

flake8 passed.

JSHint passed.

The issue has been dropped. Show all issues

Should we call .lower() on the owner and name when we save it so that its always in the casing that GitHub uses in its API? Instead of always having to remember to call .lower() when comparing them.

david

Sept. 25, 2026, 8:14 a.m.

I don't think that buys anything. GitHub's API doesn't force lowercase, it's case insensitive. Because the API just returns what the owner chose (microsoft/TypeScript, reviewboard/ReviewBot), we still need to do lower() in places like get_accessible_repositories(). Storing it as lowercase would give us one lower() call instead of two, but also impacts the way we show things in the UI.

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

The issue has been resolved. Show all issues

This calls GitHub directly, so it won't work for GitHub Enterprise. We should use self._hosting_service_cls here instead.

reviewboard/hostingsvcs/github/service.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We should document that this can now raise KeyError and InvalidPlanError.
```

Commits:

	Summary	ID
	Improve GitHub repo owner/name storage. The way that GitHub repos were storing their owner name was quite awkward. Each individual form (private user, public user, private org, public org) had its own prefix, and when we'd need to conditionally look up keys based on this. This change adds a new storage mechanism and accessor for the GitHub repository owner and name. These are stored in a new canonical form by the GitHub repository form(s). If the repo does not have the new canonical form, it will fall back to the ugly lookups previously used. The fallback (and `check_repository()`, which resolves the owner and name inline from submitted form data before anything is saved) respects `plan_field_prefix`, keeping subclasses like GitHub Enterprise working. As part of this, `get_repository_display_path()` no longer reverse-engineers the owner and name from the clone URL, since the same information is available in extra_data. If the stored data is unusable, it falls back to the raw path rather than parsing it. The `_get_repository_owner_raw()` and `_get_repository_name_raw()` methods are removed. Their only remaining caller was `check_repository()`, which now resolves the owner and name itself. Testing Done: Ran unit tests.	ulsqxkwnsuosurznqrplyxxyozyrmvwm
	Improve GitHub repo owner/name storage. The way that GitHub repos were storing their owner name was quite awkward. Each individual form (private user, public user, private org, public org) had its own prefix, and when we'd need to conditionally look up keys based on this. This change adds a new storage mechanism and accessor for the GitHub repository owner and name. These are stored in a new canonical form by the GitHub repository form(s). If the repo does not have the new canonical form, it will fall back to the ugly lookups previously used. The fallback (and `check_repository()`, which resolves the owner and name inline from submitted form data before anything is saved) respects `plan_field_prefix`, keeping subclasses like GitHub Enterprise working. As part of this, `get_repository_display_path()` no longer reverse-engineers the owner and name from the clone URL, since the same information is available in extra_data. If the stored data is unusable, it falls back to the raw path rather than parsing it. The `_get_repository_owner_raw()` and `_get_repository_name_raw()` methods are removed. Their only remaining caller was `check_repository()`, which now resolves the owner and name itself. Testing Done: Ran unit tests.	ulsqxkwnsuosurznqrplyxxyozyrmvwm

Diff:

Revision 2 (+806 -298)

Show changes

	reviewboard/admin/tests/test_connected_service_repositories_view.py
	reviewboard/hostingsvcs/github/forms.py
	reviewboard/hostingsvcs/github/service.py
	reviewboard/hostingsvcs/github/views.py
	reviewboard/hostingsvcs/tests/github/test_forms.py
	reviewboard/hostingsvcs/tests/github/test_github.py
	reviewboard/scmtools/tests/test_repository_form.py
	reviewboard/webapi/tests/test_repository.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```