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

WebHooks Module #2323

Closed
wants to merge 2 commits into from
Closed

Conversation

jrestall
Copy link
Contributor

@jrestall jrestall commented Sep 4, 2018

Hi,

I've created a WebHooks module, are you interested in this being part of the core project? It works really well alongside the GraphQL/decoupled CMS functionality.

It allows users to trigger functionality on external sites (e.g. Netlify, Discord, Twilio, MailGun etc) when events occur within the CMS. I've tried to make the events extensible so that we could add more in the future.

Here's some screenshots to give you an idea of the functionality:

webhook1

screencapture-localhost-5000-orchardcore-webhooks-webhook-edit-a3dea2b4edb1448e911dae412104b842-2018-09-04-18_32_13

webhook2

Appreciate any feedback on the code/functionality.

@giannik
Copy link
Contributor

giannik commented Sep 4, 2018

Cool stuff!!

@Jetski5822
Copy link
Member

Jetski5822 commented Sep 4, 2018 via email

@agriffard
Copy link
Member

@jrestall Do you want to make a demo during the Orchard weekly meeting? Today at 12:00 PST .

@jrestall
Copy link
Contributor Author

jrestall commented Sep 4, 2018

Thanks @giannik and @Jetski5822!
I appreciate the offer @agriffard, I'd be happy to demo the module at the meeting, 7am NZT will work well for me before work.

I've fixed the failing Linux unit test.

There's one item I'd like to fix if anyone can help. I can't reference HttpClientHandler.DangerousAcceptAnyServerCertificateValidator from my module. Is it because it's not in the netstandard2.0 spec or some other reason?

// TODO: Why won't DangerousAcceptAnyServerCertificateValidator resolve - Need for Linux/MACOS support.  
//ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator

@sebastienros
Copy link
Member

That really nice, and I would have thought someone would have said it before me, but why not make them Workflow activities instead? We could trigger them in many more ways than just content events, and it could also integrate with other external webhooks. You wouldn't lose anything, and just have less code to maintain IMO.

@giannik
Copy link
Contributor

giannik commented Sep 4, 2018 via email

@jrestall
Copy link
Contributor Author

jrestall commented Sep 5, 2018

Hi @sebastienros,

Thanks for the feedback here and on the call, I agree integrating webhooks with workflow would be very powerful and help reduce code duplication, especially around defining, selecting and triggering events.

I considered it originally but wanted to prioritize the user experience of the module by having a really simple and clean UI for adding webhooks. I envision a headless CMS recipe for Orchard in the future and want a core feature such as webhooks to be as easy as possible to configure, so no creating workflows or connecting activities for the common use case.

I agree whilst the UX is good, it would be really nice to have workflows drive the triggering and activities. I only want the best to go into Orchard as a core module so perhaps we put this on NuGet for others to use until enhancements such as below are made?

  • Create a 'Send Webhook' Workflow Task. Inherit the 'HTTP Request' Task?
  • Modify Create Webhook page to reuse the 'Send Webhook' task and reuse the events selection dialog from the workflow module. Triggers section changes to list the start activities in the workflow. Have it create the workflow automatically.
  • Change Webhook Index page to list webhook workflow types
  • Change Webhook Edit page to have an advanced link to the full workflow editor.

Unfortunately I can't commit to working on these enhancements right now. My focus is switching to GraphQL parts/fields support as it's needed for my headless work and the webhook module meets my immediate needs of publishing a static site to Netlify on content changes. If I get time I can look at implementing the enhancements or others can contribute.

@sebastienros
Copy link
Member

Please make it a module in the gallery in the meantime. And please give us a demo of how you are building the site with Netlify when you are done. We need a blog post showing this off.

It doesn't look like the editor is actually much different than the default HTTP Request workflow task, or is it? What is missing in the task compared to your screen?

My idea is that we could pretty easily create a scaffolded workflow type from such a screen, but we maybe miss some metadata and services in the workflow to filter them out so they are not directly visible in the workflow screens (or under a specific mode with permissions).

The same way I like the content event matrix, which itself could too be a custom task.

Someone should definitely reuse your UI and improve on the current tasks as a first step. I really like the attention to details, and the effort you put in the user experience.

@jrestall
Copy link
Contributor Author

jrestall commented Sep 6, 2018

Thanks @sebastienros , I'll definitely demo the ReactJS/Headless Orchard/GraphQL/GatsbyJS/Netlify setup if it all comes together.

Yea the editor is basically the same expect for the liquid variables available within it (Webhook, EventName, ContentItem). There's also the help text above fields and hint text under each field that is specific to helping with the Webhooks inputs and examples. It would be nice to support that without duplicating everything else HTTP Request task does.

Closing this pull request now, thanks everyone for your inputs.

@Skrypt
Copy link
Contributor

Skrypt commented Dec 9, 2021

Moved to jrestall/modules/webhooks

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.

6 participants