GitHub App: open beta #12217

stsewd · 2025-05-28T18:04:19Z

ericholscher

This looks like a pretty small change to ship it 👍

docs/user/reference/git-integration.rst

ericholscher · 2025-05-29T08:53:54Z

docs/user/reference/git-integration.rst

+- `Read the Docs Community <https://github.com/apps/read-the-docs-community/installations/new/>`__
+- `Read the Docs Business <https://github.com/apps/read-the-docs-business/installations/new/>`__
+
+Once you have installed the GitHub App, click on the :guilabel:`Projects` tab, and click on :guilabel:`Add project`,


This feels pretty clunky :/ I definitely know we're going to get support on this, and we should make sure we're including this information in the dashboard as well:

Don't see your project listed here? Make sure it's included in the <a>GitHub application</a>

We already have this message, wasn't sure where to put that other message related to the GH app...

Maybe having the two is fine?

readthedocs/ext-theme#606

It's not really clear to me what we're trying to solve here, or at least what the aim is on UX.

Is the user expected to have to update their GHA permissions every time they create a project from a new repository?

If so, and this is going to be consistently the case, then we should probably have some better UI for GitHub App users specifically. Basically a more prominent or mandatory block than the "Can't find the repository" block at the bottom, and something explicit and closer to what Eric mentioned above.

If this isn't a standard case, the button at the botton is okay but I'd replace the "Referesh your repositories" button instead of having two buttons for GHA users. Having two buttons is going to result in users clicking both repeatedly trying to figure out which one and which order helps.

The buttons solve different issues, one suggest the user to install the app into the repository they want to install, this action needs to be done from GH, the other button resyncs our DB with the repositories the use has access to. When the GH app users shouldn't need to refresh, but it could still be needed.

Is the user expected to have to update their GHA permissions every time they create a project from a new repository?

This depends on how the user grants access to repositories to the app, if they select individual projects, then it's needed, if they marked "select all", then there is no need, as GH will install the app on every repository when created.

This depends on how the user grants access to repositories to the app,

But not future repositories, correct? We might be somewhere in the middle then, it's going to be common enough that users will get tripped up here but not common enough to make this mandatory, visually.

The buttons solve different issues

I get they have different functions, but my concern is probably more that GHA users shouldn't need to refresh repositories. They definitely shouldn't have to click refresh after clicking to update our GHA permissions. But I thought refresh would be less common with GHA as we have repository create/update events through the webhook now. It's okay if we're not there yet though.

Can follow up more in readthedocs/ext-theme#606 I don't think the docs will change with that PR.

docs/user/reference/git-integration.rst

ericholscher · 2025-05-29T08:56:31Z

docs/user/reference/git-integration.rst

+- Go to the settings page of your GitHub repository,
+  click on :guilabel:`Webhooks`, and delete all the webhooks with URLs that start with:


Suggested change

- Go to the settings page of your GitHub repository,

click on :guilabel:`Webhooks`, and delete all the webhooks with URLs that start with:

- Click on :guilabel:`Webhooks`, and delete all the webhooks with URLs that start with:

The previous step was done in the RTD dashboard, this step needs to be done on GH, that's why I put "Go to the settings page of your GitHub repository".

docs/user/reference/git-integration.rst

readthedocs/settings/base.py

ericholscher · 2025-05-30T06:07:38Z

readthedocs/profiles/views.py

+        if count > 0:
+            messages.success(request, _(f"Successfully migrated {count} project(s)."))
+        else:
+            messages.info(request, _("No projects were migrated."))


This seems like an error? We should definitely communicate a bit more about next steps if this happens.

I understand this is because the user didn't click on "Migrate" button for any of the projects. If that's the case, it seems it's not an error.

This only happens if the user clicks on "migrate all", and there are no projects to migrate (because the ones listed have warnings).

humitos

Looks good to me as well.

docs/user/reference/git-integration.rst

humitos · 2025-06-02T09:50:48Z

docs/user/reference/git-integration.rst

+  - ``https://readthedocs.org/api/v2/webhook/<your-project-slug>`` or ``https://app.readthedocs.org/api/v2/webhook/<your-project-slug>`` for Read the Docs Community.
+  - ``https://readthedocs.com/api/v2/webhook/<your-project-slug>`` or ``https://app.readthedocs.com/api/v2/webhook/<your-project-slug>`` for Read the Docs Business.


Suggested change

- ``https://readthedocs.org/api/v2/webhook/<your-project-slug>`` or ``https://app.readthedocs.org/api/v2/webhook/<your-project-slug>`` for Read the Docs Community.

- ``https://readthedocs.com/api/v2/webhook/<your-project-slug>`` or ``https://app.readthedocs.com/api/v2/webhook/<your-project-slug>`` for Read the Docs Business.

- ``https://readthedocs.org/`` or ``https://app.readthedocs.org/`` for Read the Docs Community.

- ``https://readthedocs.com/`` or ``https://app.readthedocs.com/`` for Read the Docs Business.

I think we can probably trim this URL to make it easier to read.

In case a repository is connected to several RTD projects, the repository will have several webhooks (translations and subprojects are common to be linked to the same reposiotory)

humitos · 2025-06-02T09:54:16Z

readthedocs/oauth/notifications.py

+        body=_(
+            textwrap.dedent(
+                """
+                Migrate your projects from the <a href="{% url "migrate_to_github_app" %}">migration page</a>.


Suggested change

Migrate your projects from the <a href="{% url "migrate_to_github_app" %}">migration page</a>.

Migrate your projects automatically using the <a href="{% url "migrate_to_github_app" %}">migration page</a>.

readthedocs/profiles/views.py

humitos · 2025-06-02T09:57:21Z

readthedocs/profiles/views.py

+        if count > 0:
+            messages.success(request, _(f"Successfully migrated {count} project(s)."))
+        else:
+            messages.info(request, _("No projects were migrated."))


I understand this is because the user didn't click on "Migrate" button for any of the projects. If that's the case, it seems it's not an error.

agjohnson

Seems good so far I think. I noted a number of instances of using "import" to describe project creation, I've been wanting to work towards no using that verb and just describing project creation and repository connection instead.

agjohnson · 2025-06-02T18:13:02Z

docs/user/reference/git-integration.rst

+- `Read the Docs Community <https://github.com/apps/read-the-docs-community/installations/new/>`__
+- `Read the Docs Business <https://github.com/apps/read-the-docs-business/installations/new/>`__
+
+Once you have installed the GitHub App, click on the :guilabel:`Projects` tab, and click on :guilabel:`Add project`,


It's not really clear to me what we're trying to solve here, or at least what the aim is on UX.

Is the user expected to have to update their GHA permissions every time they create a project from a new repository?

If so, and this is going to be consistently the case, then we should probably have some better UI for GitHub App users specifically. Basically a more prominent or mandatory block than the "Can't find the repository" block at the bottom, and something explicit and closer to what Eric mentioned above.

If this isn't a standard case, the button at the botton is okay but I'd replace the "Referesh your repositories" button instead of having two buttons for GHA users. Having two buttons is going to result in users clicking both repeatedly trying to figure out which one and which order helps.

docs/user/reference/git-integration.rst

Co-authored-by: Anthony <[email protected]>

GitHub App: open beta

2b05646

Ref readthedocs/meta#187

stsewd requested a review from a team as a code owner May 28, 2025 18:04

stsewd requested a review from humitos May 28, 2025 18:04

auto-assign bot assigned stsewd May 28, 2025

stsewd added 2 commits May 28, 2025 13:05

Linter

32d79cb

Fix tests and add docs

fbe6d2b

stsewd requested a review from a team as a code owner May 28, 2025 19:42

stsewd requested a review from ericholscher May 28, 2025 19:42

Fix label

4a6142d

stsewd marked this pull request as draft May 28, 2025 19:55

stsewd added 3 commits May 28, 2025 16:30

Docs

4bf8ecb

parenthesis

566b8de

Updates

5799157

stsewd marked this pull request as ready for review May 28, 2025 21:57

Create notification

e813e5c

stsewd mentioned this pull request May 28, 2025

GitHub App: add docs #12114

Open

ericholscher reviewed May 29, 2025

View reviewed changes

stsewd added 2 commits May 29, 2025 12:13

Updates from review

45d79df

Add session message on migration

cdfc7fa

stsewd mentioned this pull request May 29, 2025

GitHub App: link to app from import page readthedocs/ext-theme#606

Open

ericholscher reviewed May 30, 2025

View reviewed changes

humitos approved these changes Jun 2, 2025

View reviewed changes

stsewd added 3 commits June 2, 2025 11:32

Updates from review

48be9be

Mention the installation access token

4c9a394

Format

5dd4ae2

stsewd added this to 📍Roadmap Jun 2, 2025

stsewd moved this to Needs review in 📍Roadmap Jun 2, 2025

agjohnson reviewed Jun 2, 2025

View reviewed changes

stsewd and others added 2 commits June 2, 2025 16:29

Apply suggestions from code review

12f8b5c

Co-authored-by: Anthony <[email protected]>

Make it a definition list

2079bf9

agjohnson approved these changes Jun 3, 2025

View reviewed changes

		- Go to the settings page of your GitHub repository,
		click on :guilabel:`Webhooks`, and delete all the webhooks with URLs that start with:

	- Go to the settings page of your GitHub repository,
	click on :guilabel:`Webhooks`, and delete all the webhooks with URLs that start with:
	- Click on :guilabel:`Webhooks`, and delete all the webhooks with URLs that start with:

		- ``https://readthedocs.org/api/v2/webhook/<your-project-slug>`` or ``https://app.readthedocs.org/api/v2/webhook/<your-project-slug>`` for Read the Docs Community.
		- ``https://readthedocs.com/api/v2/webhook/<your-project-slug>`` or ``https://app.readthedocs.com/api/v2/webhook/<your-project-slug>`` for Read the Docs Business.

	Migrate your projects from the <a href="{% url "migrate_to_github_app" %}">migration page</a>.
	Migrate your projects automatically using the <a href="{% url "migrate_to_github_app" %}">migration page</a>.

Uh oh!

GitHub App: open beta #12217

Are you sure you want to change the base?

GitHub App: open beta #12217

Uh oh!

Conversation

stsewd commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericholscher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

humitos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agjohnson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stsewd commented May 28, 2025 •

edited

Loading