Skip to content

Block integrations page when using GitHub App #595

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented May 19, 2025

Needs readthedocs/readthedocs.org#12193

This only blocks the add integration option, so users can still delete existing integrations manually.

Screenshot 2025-05-19 at 15-53-34 test-builds-stsewd - Integrations - Read the Docs

ref readthedocs/readthedocs.org#12130

@stsewd stsewd requested a review from a team as a code owner May 19, 2025 21:11
@stsewd stsewd requested a review from agjohnson May 19, 2025 21:11
@agjohnson
Copy link
Contributor

agjohnson commented May 19, 2025

Why block all integrations from being created? I get GitHub is in conflict, and Bitbucket/GitLab aren't useful for GHA projects, but generic API integration would still be a valid integration.

Feels like what we want here is instead:

  • The integration form should reject GitHub integrations when the project is using GHA with a helpful error.
  • Optional, our listing should emphasize previously connected GitHub integrations in an error state.

@agjohnson
Copy link
Contributor

Also, if there is a problem with the GHA integration where are we going to point users towards to debug? For example right now we can point them at this integration UI and tell them to look at the webhook request data for the integration. What will we do for GHA projects?

@stsewd
Copy link
Member Author

stsewd commented May 20, 2025

generic API integration would still be a valid integration.

What would be the use case for that? Triggering builds or changing the default branch should be managed by the GH app, we don't want two services doing the same/competing with each other. If users want to trigger builds automatically, they are better off using the API instead.

if there is a problem with the GHA integration where are we going to point users towards to debug?

In theory, users will have a problem only if their repo isn't connected to a remote repository (bc the user manually unlinked the project, or revoked access to that repo for the GH app).

@agjohnson
Copy link
Contributor

What would be the use case for that?

I don't think we need a use case to leave this UI as is. This is more about making our UX conditional, we should avoid that unless there is a strong reason to block integrations from creation.

Why block all integrations from being created?

This is my main question. I was more looking for a use case for why we need to disable integrations for GHA projects. Is allowing creation causing any problems for projects? It doesn't sound like we have a blocking reason to make this UI conditional. I'm looking to avoid this as much as possible, especially if there is something more helpful to show the user.

In theory, users will have a problem only if their repo isn't connected to a remote repository (bc the user manually unlinked the project, or revoked access to that repo for the GH app).

This is the sort of information we could be showing the user for a GHA integration in the listing. We don't need to have an edit view for GHA integration, but it feels more confusing to remove this concept for GHA integrations.

@humitos
Copy link
Member

humitos commented May 21, 2025

In general, I'm 👍🏼 on deprecating Generic API webhooks. These project should use the APIv3 as Santos mentioned.

However, I think this work is not related to that in particular and it's removing the Generic API webhooks for GHA projects as side effect when it probably shouldn't.

This side effect will make project with Generic API webhooks to be a in a weird state after they migrate to GHA since they won't be able to edit/modify/delete/create these integrations anymore.

@stsewd
Copy link
Member Author

stsewd commented May 21, 2025

This is more about making our UX conditional, we should avoid that unless there is a strong reason to block integrations from creation.

We don't want users creating incoming webhooks when there is no need for it, and also avoid having two competing webhooks, which could lead to duplicate builds.

This side effect will make project with Generic API webhooks to be a in a weird state after they migrate to GHA since they won't be able to edit/modify/delete/create these integrations anymore.

No, my suggestion is avoiding the creation of new integrations. Editing and removing existing integrations is still allowed.

@agjohnson
Copy link
Contributor

We don't want users creating incoming webhooks when there is no need for it, and also avoid having two competing webhooks, which could lead to duplicate builds.

But we already allow projects to create multiple integrations. I'm not seeing a true disadvantage to special casing GHA projects here.

I'd prefer this UI/UX is consistent across providers instead. If that means projects can create redundant integrations, it feels fine to leave that up to maintainers. I don't think this is a strong enough problem to warrant making this UI complex and only for GHA projects.

We can avoid redundant integrations by:

  • Showing the GHA integration in the listing so it's apparent that something is connecting the project to the repository.
  • Disallow GitHub integrations from being created if the project is a GHA project?

I still don't know about the UX around what we are doing for projects when the link between a GHA repository and our projects breaks. Users familiar with integrations might want to recreate something, and might even try recreating the GitHub integration. We'll have an opportunity to give the user a helpful message. Best case we can detect and show the state of the GHA "integration" in our listing though.

In general, I'm 👍🏼 on deprecating Generic API webhooks. These project should use the APIv3 as Santos mentioned.

Same, though my point wasn't about generic API integrations. For my point you can use any of the other integrations.

What I mean is that with any of these options we/users gain little from us dictating which integrations they can create. I say we allow creation so we aren't making a UI that is sometimes useful and sometimes useless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants