readthedocs / readthedocs.org

The source code that powers readthedocs.org
https://readthedocs.org/
MIT License
7.92k stars 3.58k forks source link

Integrations: deprecate old incoming webhook URLs #11037

Open stsewd opened 5 months ago

stsewd commented 5 months ago

What's the problem this feature will solve?

Currently, we have two URLs for incoming webhooks

https://github.com/readthedocs/readthedocs.org/blob/e5143773fc0826515f5bf508d4b659d8e9a4303f/readthedocs/api/v2/urls.py#L76-L105

One that only takes into consideration the project's slug, which means that only one integration of a type can exist per project. And the other one take into consideration the project and integration PK, so many integrations of the same type can co-exist.

Looks like we are currently linking to the more specific type of URL for all integrations, any project using the old URL is probably an old project.

Describe the solution you'd like

Just use one URL, ask users to migrate to the more specific URL type.

Maybe it also makes sense to allow only one incoming webhook, not sure when multiple incoming webhooks would be necessary, maybe for debugging only?

Additional context

Currently, if the user makes use of the more specific URL, and has more than one type of that integration, the integration will fail (since we use .get(), and it returns more than one result)

https://read-the-docs.sentry.io/issues/4634041415/?project=148442&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=24h&stream_index=3

humitos commented 5 months ago

I think these two points are key to decide before moving forward.

Just use one URL, ask users to migrate to the more specific URL type.

Is it possible to remove any "manual action from the user" from the equation? Can we migrate them automatically?

How many of these old integrations we have? How many of them are from "active projects"?

Maybe it also makes sense to allow only one incoming webhook, not sure when multiple incoming webhooks would be necessary, maybe for debugging only?

How many projects we have with multiple incoming webhooks? What are those?

I think that knowing how they are configured, we can infer why they need more than one integration. Maybe it's just unnecessary. In that case, I'd vote to remove this extra complexity from our code, but also simplify the UX.

humitos commented 5 months ago

Related to https://github.com/readthedocs/readthedocs.org/issues/10760

agjohnson commented 3 weeks ago

Needed here: