Skip to content

List, edit, and delete a webhook trigger's custom URLs - #5111

Open
elias-ba wants to merge 4 commits into
mainfrom
fix/webhook-path-validation-ux
Open

List, edit, and delete a webhook trigger's custom URLs#5111
elias-ba wants to merge 4 commits into
mainfrom
fix/webhook-path-validation-ux

Conversation

@elias-ba

@elias-ba elias-ba commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Description

This PR changes how a webhook trigger's URLs are shown and edited, and fixes a bug where changing the custom path did not mark the workflow as unsaved.

Custom paths shipped in #5105. Demoing them showed three problems.

  1. One field made it look like naming a webhook replaced its default URL rather than adding a second one.
  2. A path the rules reject was quietly rewritten, so you got an endpoint you did not ask for.
  3. And a name already in use was only reported after saving, on a panel you had already left.

The panel now lists both URLs and lets you edit the custom one in place. A rejected path stays as you typed it and says what is wrong. A name in use is reported while you type.

Follow-up to #4952.

Validation steps

  1. Open a workflow with a webhook trigger. The panel lists the default URL, with an "Add custom URL" button under it.
  2. Add a path. Only the last segment is editable. Tab takes the suggested name, Enter saves.
  3. Type something invalid, then a name another webhook in the project already uses. Both are reported under the row, and what you typed stays put.
  4. Change the path and check that Save shows the unsaved dot. On main it does not.
  5. Save, then POST to both URLs and confirm both start a run.

Additional notes for the reviewer

AI Usage

Please disclose whether you've used AI anywhere in this PR (it's cool, we just want to know!):

  • I have used Claude Code
  • I have used another model
  • I have not used AI

You can read more details in our Responsible AI Policy

Pre-submission checklist

  • I have performed an AI review of my code (we recommend using /review with Claude Code)
  • I have implemented and tested all related authorization policies. (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

@github-project-automation github-project-automation Bot moved this to New Issues in Core Sep 2, 2026
@elias-ba elias-ba changed the title Show a webhook's URLs as a list, and fix the unsaved-changes check for custom paths List a webhook's URLs instead of showing one Sep 2, 2026
@elias-ba
elias-ba requested a review from doc-han September 2, 2026 22:54
@elias-ba elias-ba changed the title List a webhook's URLs instead of showing one List and edit a webhook's URLs Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

Security Review ✅

  • S0 (project scoping): New custom_path_taken?/3 at lib/lightning/workflows.ex:995 filters by project_id, and the check_custom_path handler at lib/lightning_web/channels/workflow_channel.ex:466-479 sources it from socket.assigns.project.id (verified at join), not from the payload.
  • S1 (authorization): N/A for a new gate — the channel join at lib/lightning_web/channels/workflow_channel.ex:47 already requires Permissions.can(:workflows, :access_read, ...) (or :create_workflow), and the new handler is a read-only advisory in line with validate_workflow_name.
  • S2 (audit trail): N/A, check_custom_path is a read-only existence query and adds no config writes.

@elias-ba elias-ba changed the title List and edit a webhook's URLs List, edit,and delete a webhook trigger's custom URLs Sep 2, 2026
@elias-ba elias-ba changed the title List, edit,and delete a webhook trigger's custom URLs List, edit, and delete a webhook trigger's custom URLs Sep 2, 2026
@elias-ba
elias-ba force-pushed the fix/webhook-path-validation-ux branch from a33c33f to 754d29e Compare September 2, 2026 22:56
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.7%. Comparing base (ac60463) to head (ec49ff0).

Additional details and impacted files
@@          Coverage Diff          @@
##            main   #5111   +/-   ##
=====================================
  Coverage   90.7%   90.7%           
=====================================
  Files        422     422           
  Lines      20151   20161   +10     
=====================================
+ Hits       18276   18291   +15     
+ Misses      1875    1870    -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The unsaved-changes check builds a comparable shape from each trigger, and the
webhook branch never copied custom_path. Renaming an endpoint left the Save
button quiet, so the edit was easy to lose by navigating away.
Uniqueness is only knowable on the server, and without asking you find out by
saving: the wizard closes, the save fails, and the reason lands on a panel you
have already left.

The answer is advisory. The partial unique index is still what guarantees it,
and a save can lose the race, so the error the server returns on save stays. The
query is served by the index that constraint already creates.

The answer is dropped the moment the path changes, and a request that fails
stops blocking rather than reporting the name as free. A stale error about the
previous path is cleared straight through the Y.Doc, since the debounced route
is slow enough for a save to land a fresh error for it to delete.
A webhook answers on its default URL and, once named, on a custom one as well.
A single field made that look like a choice between the two, and people asked
during the demo which URL was live. The panel now lists both, default first,
with add, edit, delete and copy on the row itself.

Only the path is editable. The origin and the project id stay as text beside the
field, so what you type is visibly part of a URL rather than a separate value
that turns into one. Enter saves and Tab takes the suggested name.

A path the server would reject is reported under the row and left as typed.
Rewriting it quietly is worse: you get an endpoint you did not ask for and no
reason why. Finish waits until the name has been checked, so a duplicate is
caught here rather than on the panel this closes to. A path the server refused
is shown but not offered for copying, because posting to it reaches whichever
workflow owns the name.
@elias-ba
elias-ba force-pushed the fix/webhook-path-validation-ux branch from e85e197 to 704eff5 Compare September 3, 2026 01:28
Auth is checked on the resolved trigger, so it covers both the default URL and
the custom one. Nothing tested it. Every auth test posted to the default URL,
and the one namespaced post in the suite was on a trigger with no auth method,
so a change that keyed auth methods off the first path segment would have gone
through green.

The panel now says so as well, since the section it sits under is a list of URLs
rather than one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: New Issues

Development

Successfully merging this pull request may close these issues.

1 participant