Best way to submit a new feature

[indyteo]

Hi,
I was wondering what was the best way to submit a new feature to Fider.
I must say it's the first time ever I contribute to an Open Source project, and I would like to know what are the steps to do it.
Should I first create an issue where I explain the feature, then a pull request with the code, or only the PR with the explanations inside? Or something else?

Thanks in advance for your response :)

[goenning]

Hey @indyteo, thanks for joining us! Is your feature on this list https://feedback.fider.io/ ?

Either way, it’s better to start with an issue detailing what is the feature and a high level description of how you plan to add it. I am picky with what is included in the project, because I have to keep supporting it in the long run 🙂

So having an issue with as much description as possible will save time from both you and I.

[indyteo]

Hey 👋,
I saw you opened a PR introducing complete translations.
I suppose you're waiting it to be merged and then you'll want me to adapt my other PR to use localized messages instead, right?

On the other hand, I've been working on a rework of the webhook feature request (#893) by @acamilleri, but in a fully customizable manner. It complete the Webhook, Slack integration requests from https://feedback.fider.io.
Let me introduce my work 😄

Webhooks

This feature is available on the webhook branch.

Presentation

It allow us to create webhooks triggered by specific actions. Everything can be controlled on the admin panel, so it perfectly works with cloud-hosted solution.

A new tab "Manage Webhooks" has been added in the control panel, only visible by admins. The structure is similar to the "Manage Tags" page, with the four webhook types (i.e. trigger reasons) as sections.

Actions

On each webhook, three actions are available: Trigger, Edit and Delete.

Trigger

It trigger the webhook with dummy data to simulate a new post / new comment / ..., in order to check if the current configuration is valid. A disabled webhook can be triggered to test it. Failures during execution are marked as fail like in normal execution. In addition, a failure report will be available (more on that later).
A notification indicate whether the trigger was a success or a failure from the point of view of the application.

Edit / Add new

The edition / creation form include everything necessary to control the webhook, from the name to identify it in the control panel, to the HTTP headers sent with the request. URL and Content are subject to template formatting (more on that later).

Delete

This is pretty obvious but, this prompt for a confirmation whether we want to permanently delete the webhook or not.
It indicate us we also may want to disable it instead of completely remove it.

Other info

Failure reports

As I mentioned before, a webhook execution can fail, and a failure report with become available right after the manual trigger.
Those reports are not saved in the database, this is why we can't see previous reports, and must trigger it manually to see it.
When a failure append during a manual trigger, an hover info (a new little component based on tooltip and icons) is shown beside the webhook's name and clicking on it open a modal with the report data.

The failure report includes various information useful for debugging and understanding why the webhook failed.
For example, here in the following report, we can see the URL contains a space after the equal sign, and cause the server to reject the request with a 400 Bad Request.

Template formatting help

The power of this feature is that it rely on Go text/template package to format webhook URL and content.
Why URL? Because sometimes we want to pass something as a GET parameter instead, or even worse (and also a little crazy, but why not?), automatically adding a comment under a new post, through the API it would need to put the post_number as a path parameter of the URL.
Indeed, it's also possible for the content to be template formatted.

Because Go templates are powerful, they're also not so simple (even if creating a basic template is really easy!). So I decided to add two things (and I'm thinking about a third: creating a page about webhooks in the Fider docs):

• A preview section in the form, allowing us to parse templates with dummy data
• A modal with quick reminder of syntax, properties and functions

That's it for the user interface presentation 😃

Technical explanations

The three stacks are affected by this new feature.

Database changes

A new table, webhooks, has been created. It includes 9 columns: id, name, type, status, url, content, http_method, additional_http_headers and tenant_id.
HTTP headers are stored as JSONB in a single column.

Back-end changes

Types and structures

Webhooks are defined by an entity, containing the same fields as the database (except the tenant id).
HTTP headers are a simple key-value string-string map, parsed and serialized in JSON.
Other models such as WebhookTriggerResult or WebhookPreviewResult are entities used to communicate with the client.

Two enums WebhookType and WebhookStatus represent the type and status as int (stored in DB) marshaled as text (sent to client).
The types are the four trigger origins and the status is either "enabled", "disable", or "failed".

Commands, actions and queries covers every requests and operations.

API routes

A bunch of new routes located under the /_api/admin/webhook root have been created.

POST /_api/admin/webhook

Create a webhook with the given configuration (from the request body).

PUT /_api/admin/webhook/:id

Modify the webhook id with the request body data.

DELETE /_api/admin/webhook/:id

Delete the webhook id

GET /_api/admin/webhook/trigger/:id

Trigger the webhook id, returning the WebhookTriggerResult produced by execution (used by manual trigger button).

POST /_api/admin/webhook/preview

Send partial webhook configuration (type, URL and content) to render a preview of templates (used by form preview section).

GET /_api/admin/webhook/props/:type

Generate dummy props for the given webhook type (used to display template formatting help example properties).

Service and storage

The webhook service provide implementation for triggers by type and single trigger. It also includes a property management to generate webhook properties from entity data.
The tasks now include a bus dispatch of webhook trigger command with the appropriates properties.

The SQL connector has all CRUD operations and some more like "mark as failed" or "list active by type".

Other minor changes

The tpl package know include an implementation of the text/template Go package as well as the html/template to work with webhook templates and two new functions (format and quote) have been added.

Front-end changes

In the front-end, all actions corresponding to the added routes have been created, and all entity models also.
All webhooks are loaded once when the user access the manage webhook page, and then updated accordingly to his actions.

Concerning the automatic preview, to prevent a request to be executed every single character is typed, a "typing" state in kept in memory and refreshed every time a character change. After 2.5 seconds without any new character, the request is made and the preview result is written into the page. When typing again, preview is erased and will be replaced with the new updated one after.

Some little patches have been applied to multiple components:

• Props className and afterLabel on TextArea;
• New wrapper div c-modal-scroller in Modal to allow outsized modal to be scrolled;
• Props className and text on Loader (the text is inserted under the loader);
• Fixed rounded border with multiple buttons after Input;
• Prop target on Button for link buttons;
• New component HoverInfo with an information-circle Hero Icon and a data-tooltip to show informational text.

---

I know it's a lot of stuff, as always, but it make the feature complete 😁
Another thing, we were requested to add advanced search so it'll be the next thing I'll do, I think it'll be a great addition since it'll allow people to find posts matching specific conditions... 👀

[goenning]

Hey @indyteo, another great feature and write up. Thank you! This is exactly what I was thinking of a webhook feature, you actually implemented a lot more than I would have thought of 🙂

And I can see you’re off to something else again, so I might suggest creating an issue for each feature to avoid having this discussion growing more, it’ll make it easier later for anyone to search as well.

For your open PR: I haven’t looked at it yet, for PR with UI changes I often spend more time reviewing and checking it locally, which I hope to do it next week. I’m expecting some conflicts (hopefully minor) after the translation PR is merged, but at least it only affects the public pages as admin is not yet translated.

Please go ahead and create a PR for webhooks as well, I’m sure there are lot of people anscious about it 🙂

Q: I have never integrated with slack/discord/teams/zapier/etc, are they all just simple HTTP requests? They don’t need any kind of special processing and/or authentication? Is there any limitation on this solution that you can think of?!

I’m just thinking I’d there’s any case where we’d need a “special” webhook implementation to make it work with slack or any other service.

thanks again and have a great weekend 👍

[indyteo]

Yes you're right for the issue, or maybe directly in the PR next time? As it's more "How it was done" than "What was done" (and issues requesting for webhooks already exists #892).

Anyway, no problem for the email authentication PR, I understand it take time to test those things, and I must admit I didn't really worked on unit tests, and it might cause some trouble having some part of the project not covered.

The only user-end texts are the notice indicating that email authentication is currently disabled, and a few error messages when trying to avoid front-end's "disabled" states.

About the current integration of webhooks, yes, it basically is a simple HTTP request that trigger the code on the targeted application. Most of them uses a secret token, included in the URL, and this is why I decided to make the Manage Webhook control panel available only for admins and not collaborators. But sometime applications prefers not to include any key in the URL and requires authentication is headers. This is why I allowed total customization of them (and not only some specials like Content-Type).

Slack, Discord, Teams (and I think Zapier too but I didn't tried it) use the URL token strategy. Fider's API is based on the HTTP header Authorization. I never saw a webhook-based feature requiring special authentication involving TCP dialog for example.

The only improvement we could envisage about those "security"-oriented questions is to allow definition of secrets, basically variables, that would be saved when created and never shown back when accessing the panel. However, I don't know if such efforts would be really useful: only admins are allowed to see this area, and they could just change the URL to "steal" those secrets (I won't speak much more about this but it is possible really easily when you know some tools, and cannot be prevented without disabling completely the webhooks system, which would be counter-productive).

A special case that might need to get treated is the possibility of the infinite-loop. As I said, webhooks triggers HTTP requests. Fider's API provide the ability to interact using HTTP requests. Nothing prevent admins to create a webhooks that will create a post through the API, triggered every new post... 🙃

A workaround for this case is to include an additional HTTP header, after the user-provided ones, overwriting any previous value possibly associated by the user, like Webhook-Triggered-By: new_post and in the API, deny the post creation when this header is present. However, it would become impossible to prevent this with two webhooks. (First, trigger on new_post, create a comment, and second, trigger on new_comment, create a post, then we get another infinite loop). But I think we need to trust the admins on this.

Finally, I'm thinking about a little rework of the webhook type. Indeed, it's enough right now but if we add more trigger sources, such as change_tag, new_vote, delete_vote, edit_post, edit_comment, delete_comment, etc... it will become really messy. What I was thinking is to change the type to a scope, still an int, but more like a flag (0b0001 being new_post, 0b0010 for new_comment, etc...). In the UI, the sections will become Enabled, Disabled, and Failed, now based on the status, and the scope will be a bunch of check boxes so that admins will be able to enable a webhook for several trigger sources. Accordingly, this source will be available as a property during execution, to allow template customization based on event.

Please tell me if you're interested in this little rework right now, before the PR, or if you think it'll be good like this and this improvement could come later, in a second time.

Please also note that the advanced search feature may be delayed because I need to work on other projects and I'll be on vacation during the next month.

Thank you for your consideration for my work 😄

[goenning]

I think we can defer that for now, let's see how this one performs before doing that improvement 👍

The loop is an interesting, people might not realise that it's happening until it's too late. Can you think of any example where it'd be useful to have a web hook from Fider into Fider?

Yes you're right for the issue, or maybe directly in the PR next time? As it's more "How it was done" than "What was done" (and issues requesting for webhooks already exists #892).

I don't mind doing it directly in the PR. But like I mentioned initially, you might have worked on a feature that I'm not currently willing to merge. Advanced search sounds interesting, but I'd like to understand what are your thoughts on it. Is is a SQL-like query? Would you require an external service such as ElasticSearch? I think it's worth creating an issue and we can discuss it there.

Enjoy your vacation! 🏖️

[indyteo]

Concerning the Fider-to-Fider hook, several use-cases came to my mind, I don't know if someone would use them, but it would at least be possible:

1. Every time a new post is created, immediately create a comment on it (maybe it can be combined with another webhook creating an element somewhere associated with the post (a ticket on the internal website of the company?) an thus the comment under the post can link to it)
2. Automatically tag a newly created post with a tag (and thanks to templates we can even imagine tags "from team" and "from customers" being assigned dynamically)
3. Combined with the precedent, every time a comment is posted, it can remove an "not commented yet" tag (that was added at post's creation)
4. Every time a new comment is created on a post, automatically make the user vote (not a good idea to be honest, but it would be possible is someone want to do this, I suppose...)

But of course, a warning could be shown before saving if the webhook's URL target the current Fider tenant URL (what about a confirmation modal explaining it's dangerous and why it is so?). But it won't cover the case of another site... If someone want to do something bad, he will be able to do it. But I don't know if it's really a big concern because it's already the case 🤷
The way to "protect" users from a self mistake, even if they ignore the warning, would be to force them to manually include a HTTP header to "confirm" they want to do that. But as soon as they include it, the problem is the same (and they already need to include the Authorization header to authenticate and being able to post something on a Fider instance so their really cannot to it without knowing 😆).
The final solution is to prevent the elements created through the API to trigger any webhook. But if someone has a mobile App for example integrated with Fider, and create a lot of posts though the API, it would break the webhook feature for him. And a HTTP header from the webhook request to the API marking it as emitted by a webhook is not a solution because someone could use a proxy to filter it. No solution allow us to protect users from themselves, I think we need to trust them about this (with a warning because it's important to inform them however).