Summary

Isolate service connect UI APIs into their own object.

Review Request #15221 — Created Aug. 11, 2026 and submitted Aug. 28, 2026, 2:43 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-9.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

In prior changes I'd added some new classmethods to BaseHostingService
in order to implement the new Connected Services page. As this grows,
it's making the hosting service even more unwieldy. This change moves
those out into a helper object, which can be subclassed for
service-specific functionality.

Testing Done

Ran unit tests.

Commits

Summary	ID
Isolate service connect UI APIs into their own object. In prior changes I'd added some new classmethods to BaseHostingService in order to implement the new Connected Services page. As this grows, it's making the hosting service even more unwieldy. This change moves those out into a helper object, which can be subclassed for service-specific functionality. Testing Done: Ran unit tests.	sponzwvpxzsmzurmzzvowpwuumkrxwzv

Issues

Description	From	Last Updated
Can we make BaseHostingServiceConnectUI generic over the hosting service type?	maubin	Aug. 19, 2026, 8:49 a.m.
render_to_string() should already be a SafeString.	chipx86	Aug. 16, 2026, 1:34 p.m.
Can we make these keyword-only, so if we ever change any of this call sites will be stable?	chipx86	Aug. 16, 2026, 1:36 p.m.
I assume this is the case, but I want to check: This is just a priority list for the names, …	chipx86	Aug. 16, 2026, 1:36 p.m.
This is missing the full module path.	chipx86	Aug. 16, 2026, 1:37 p.m.
render_to_string() should be a SafeString already.	chipx86	Aug. 16, 2026, 1:37 p.m.
This is missing a Version Added.	chipx86	Aug. 16, 2026, 1:37 p.m.
'django.utils.safestring.mark_safe' imported but unused Column: 1 Error code: F401	reviewbot	Aug. 16, 2026, 1:41 p.m.
This is missing the full module path.	chipx86	Aug. 28, 2026, 11:17 a.m.

flake8 passed.

JSHint passed.

Change Summary:

Actually use the defined connect_ui_cls

Commits:

	Summary	ID
	Isolate service connect UI APIs into their own object. In prior changes I'd added some new classmethods to BaseHostingService in order to implement the new Connected Services page. As this grows, it's making the hosting service even more unwieldy. This change moves those out into a helper object, which can be subclassed for service-specific functionality. Testing Done: Ran unit tests.	sponzwvpxzsmzurmzzvowpwuumkrxwzv
	Isolate service connect UI APIs into their own object. In prior changes I'd added some new classmethods to BaseHostingService in order to implement the new Connected Services page. As this grows, it's making the hosting service even more unwieldy. This change moves those out into a helper object, which can be subclassed for service-specific functionality. Testing Done: Ran unit tests.	sponzwvpxzsmzurmzzvowpwuumkrxwzv

Diff:

Revision 2 (+544 -442)

Show changes

	reviewboard/admin/views.py
	reviewboard/hostingsvcs/base/connect_ui.py
	reviewboard/hostingsvcs/base/hosting_service.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

```
Looks good. A few very small things.
```
reviewboard/hostingsvcs/base/connect_ui.py (Diff revision 2)
The issue has been resolved. Show all issues
```
render_to_string() should already be a SafeString.
```

reviewboard/hostingsvcs/base/connect_ui.py (Diff revision 2)

The issue has been resolved. Show all issues

Can we make these keyword-only, so if we ever change any of this call sites will be stable?

reviewboard/hostingsvcs/base/connect_ui.py (Diff revision 2)

The issue has been dropped. Show all issues

I assume this is the case, but I want to check: This is just a priority list for the names, right? Since hosting services can set both of these to True.

david

Aug. 16, 2026, 1:40 p.m.

Yes. Something like github should be listed as "Source hosting" even if it also does issues.

reviewboard/hostingsvcs/base/connect_ui.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This is missing the full module path.
```
reviewboard/hostingsvcs/base/connect_ui.py (Diff revision 2)
The issue has been resolved. Show all issues
```
render_to_string() should be a SafeString already.
```
reviewboard/hostingsvcs/base/hosting_service.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This is missing a Version Added.
```

Commits:

	Summary	ID
	Isolate service connect UI APIs into their own object. In prior changes I'd added some new classmethods to BaseHostingService in order to implement the new Connected Services page. As this grows, it's making the hosting service even more unwieldy. This change moves those out into a helper object, which can be subclassed for service-specific functionality. Testing Done: Ran unit tests.	sponzwvpxzsmzurmzzvowpwuumkrxwzv
	Isolate service connect UI APIs into their own object. In prior changes I'd added some new classmethods to BaseHostingService in order to implement the new Connected Services page. As this grows, it's making the hosting service even more unwieldy. This change moves those out into a helper object, which can be subclassed for service-specific functionality. Testing Done: Ran unit tests.	sponzwvpxzsmzurmzzvowpwuumkrxwzv

Diff:

Revision 3 (+566 -444)

Show changes

	reviewboard/admin/views.py
	reviewboard/hostingsvcs/base/connect_ui.py
	reviewboard/hostingsvcs/base/hosting_service.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/hostingsvcs/base/connect_ui.py (Diff revision 3)
The issue has been dropped. Show all issues
```
'django.utils.safestring.mark_safe' imported but unused

Column: 1
Error code: F401
```

Commits:

	Summary	ID
	Isolate service connect UI APIs into their own object. In prior changes I'd added some new classmethods to BaseHostingService in order to implement the new Connected Services page. As this grows, it's making the hosting service even more unwieldy. This change moves those out into a helper object, which can be subclassed for service-specific functionality. Testing Done: Ran unit tests.	sponzwvpxzsmzurmzzvowpwuumkrxwzv
	Isolate service connect UI APIs into their own object. In prior changes I'd added some new classmethods to BaseHostingService in order to implement the new Connected Services page. As this grows, it's making the hosting service even more unwieldy. This change moves those out into a helper object, which can be subclassed for service-specific functionality. Testing Done: Ran unit tests.	sponzwvpxzsmzurmzzvowpwuumkrxwzv

Diff:

Revision 4 (+564 -444)

Show changes

	reviewboard/admin/views.py
	reviewboard/hostingsvcs/base/connect_ui.py
	reviewboard/hostingsvcs/base/hosting_service.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

The issue has been dropped. Show all issues

Can we make BaseHostingServiceConnectUI generic over the hosting service type?

david

Aug. 19, 2026, 8:47 a.m.

While I can do that in the definition, I think it breaks for subclasses because it makes the two classes depend on each other at definition time. Neither of these works:

class GitHubConnectUI(BaseGitHubConnectUI[GitHub]):
    ...

class GitHub(BaseHostingService...):
    connect_ui_cls = GitHubConnectUI




class GitHub(BaseHostingService...):
    connect_ui_cls = GitHubConnectUI

class GitHubConnectUI(BaseGitHubConnectUI[GitHub]):
    ...

maubin

Aug. 19, 2026, 8:49 a.m.
```
Ah I see.
```

Ship it!

```
Ship It!
```

Change Summary:

Add new file to coderef

Commits:

	Summary	ID
	Isolate service connect UI APIs into their own object. In prior changes I'd added some new classmethods to BaseHostingService in order to implement the new Connected Services page. As this grows, it's making the hosting service even more unwieldy. This change moves those out into a helper object, which can be subclassed for service-specific functionality. Testing Done: Ran unit tests.	sponzwvpxzsmzurmzzvowpwuumkrxwzv
	Isolate service connect UI APIs into their own object. In prior changes I'd added some new classmethods to BaseHostingService in order to implement the new Connected Services page. As this grows, it's making the hosting service even more unwieldy. This change moves those out into a helper object, which can be subclassed for service-specific functionality. Testing Done: Ran unit tests.	sponzwvpxzsmzurmzzvowpwuumkrxwzv

Diff:

Revision 5 (+566 -444)

Show changes

	docs/manual/extending/coderef/index.rst
	reviewboard/admin/views.py
	reviewboard/hostingsvcs/base/connect_ui.py
	reviewboard/hostingsvcs/base/hosting_service.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

reviewboard/hostingsvcs/base/connect_ui.py (Diff revision 5)
The issue has been resolved. Show all issues
```
This is missing the full module path.
```

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