Skip to content
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

feat: add resource control for webhook form action #4333

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

Conversation

TrySound
Copy link
Member

Ref #4093 #3871

Here added new resource control for out properties. It is replaced action and method properties webhook form. Now users will be able to additionally provide headers. In another PR headers will let user to customize body format.

Also migrated "action" in existing webhook forms to resource prop type.

Screenshot 2024-10-22 at 16 07 38

Ref #4093 #3871

Here added new resource control for out properties.
It is replaced action and method properties webhook form.
Now users will be able to additionally provide headers.
In another PR headers will let user to customize body format.

Also migrated "action" in existing webhook forms to resource prop type.
@kof
Copy link
Member

kof commented Oct 23, 2024

  • Icon position is off:
image

@kof
Copy link
Member

kof commented Oct 23, 2024

Haven't tested, but looks great, lets list what this enables from use cases perspective, so that once @johnsicili is back we can record an edu video, maybe replacing the old form video.

Are all of these done? #4093

@kof
Copy link
Member

kof commented Oct 23, 2024

So far I see 2 use cases we need to demonstrate:

  1. As a user sometimes I need to use an API that only accepts json data and requires tokens in headers. One such example is signing up for newsletter in splunk https://docs.useplunk.com/api-reference/contacts/subscribe

  2. Using response json and rendering something in return as a success or error.

Am I correct to assume both use cases are supported now?

@TrySound
Copy link
Member Author

In this PR only headers are supported

@kof
Copy link
Member

kof commented Oct 24, 2024

Got it, so this unlocks a use case like useplunk.

@TrySound
Copy link
Member Author

Yes, though I'm planning a small launch with all those features requested so webhook forms would make more sense to users.

@kof
Copy link
Member

kof commented Oct 24, 2024

Btw you should try to post something to https://docs.useplunk.com/api-reference/contacts/subscribe to see if we are still missing something there

@sartipaz
Copy link

Hi, we are doing an ecommerce site (with a lot of handmade javascript) and we really need this feature, as I can see is already working and passes all the CI checks, I will mean a lot if we get those new configs for the headers.

Right now we are going to pass (in some hidden fields) the values we need in the headers thru zapier and then move the hidden fields to the corresponding header just to make the site work, but it is a huge security issue.

};

const areAllFormErrorsVisible = (form: null | HTMLFormElement) => {
if (form === null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like huge error if form is not available at the moment

if (
element.validity.valid === false &&
// rely on data-color=error convention in webstudio design system
element.getAttribute("data-color") !== "error"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like dirty hack

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is though it works and I didn't figure out another way.

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.

4 participants