freelawproject / courtlistener

A fully-searchable and accessible archive of court data including growing repositories of opinions, oral arguments, judges, judicial financial records, and federal filings.
https://www.courtlistener.com
Other
550 stars 151 forks source link

4657 Allow users to select the preferred webhook version #4689

Open albertisfu opened 1 week ago

albertisfu commented 1 week ago

As requested in #4657, this PR introduces changes in the webhooks UI, allowing users to select which version of the webhook to use when creating a webhook endpoint.

4657 we planed to do:

Modify the Event Type drop down to list Seach Alert v1, Search Alert v2, etc

However, while working on this, I realized this approach could complicate code maintainability. Specifically, it would require adding new Event Type options each time a new webhook version is introduced, and adjusting code to handle multiple versions of the same event type when sending webhooks.

Instead, I propose we continue using the same event types and allow users to select the version independently. Screenshot 2024-11-13 at 6 53 53 p m

With this change, users can create only one webhook per event_type and version combination. Since we currently have only two versions, users can create up to two webhook endpoints per event type.

The UI will display available versions based on the selected event type. If a user already has a webhook endpoint for a specific event type and version, only the remaining version will be shown for that event type: Screenshot 2024-11-13 at 6 54 28 p m

Once created, the event type and version cannot be changed in the Edit view: Screenshot 2024-11-13 at 6 55 35 p m

A new "Version" column has been added to the webhook endpoints list: Screenshot 2024-11-13 at 6 55 19 p m

If a user has already created one endpoint for each event_type and version combination, an alert will appear when they try to create a new webhook:

Screenshot 2024-11-13 at 6 56 02 p m

For now, these changes are only in the UI. In #4653 and #4682, we will use the version to apply the correct serializer and version in the payload for each webhook event type. So my question is if when this is ready it's ok to merge or if we should put it behind a waffle to hide it temporarily, or wait until the other PRs are ready and merge them into this one to release everything as a unit?

mlissner commented 1 week ago

or wait until the other PRs are ready and merge them into this one to release everything as a unit?

Seems easiest, right?

mlissner commented 1 week ago

Process notes: I took this off your personal backlog, Alberto, since we're hoping not to use those anymore, and assigned it to you instead, since you're the one in charge of making sure it gets done. I added Eduardo as a reviewer.

Thanks all!

semgrep-app[bot] commented 3 days ago

Semgrep found 8 direct-use-of-jinja2 findings:

Detected direct use of jinja2. If not done properly, this may bypass HTML escaping which opens up the application to cross-site scripting (XSS) vulnerabilities. Prefer using the Flask method 'render_template()' and templates with a '.html' extension in order to prevent XSS.

Ignore this finding from direct-use-of-jinja2.

albertisfu commented 2 days ago

Thanks @ERosendo I've applied both of your suggestions. We'll merge #4705 into this one when it's ready, and then merge this one into main to deploy it to production.