Generic Webhooks v0 #337
Generic Webhooks v0 #337
Conversation
🦋 Changeset detectedLatest commit: b07e123 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
colehpage
force-pushed
the
generic_webhooks_v0
branch
from
2de42d8
to
7cfc9c7
Compare
packages/api/migrations/mongo/20240305195342-add_generic_webhook_fields.ts
Outdated
Show resolved
Hide resolved
|
import linting....fixed.... |
| // Converts this map to a native JavaScript Map for JSON.stringify(). Set the flattenMaps option to convert this map to a POJO instead. | ||
| // doc.myMap.toJSON() instanceof Map; // true | ||
| // doc.myMap.toJSON({ flattenMaps: true }) instanceof Map; // false | ||
| toJSON: (options?: { |
There was a problem hiding this comment.
so we need to set flattenMaps to true to convert the document to JSON ? not very intuitive
There was a problem hiding this comment.
Naw, need to set flattenMaps to true only if the input has nested Maps - in these cases it's probably not needed at all since inputting nested maps would rarely happen. the regular call to toJSON() it still preps it for use with json.stringify etc.
There was a problem hiding this comment.
I didn't need them for queryParams/headers I should have left those out, nobody is inputting raw maps their either, that was my mistake - these use cases don't mind if maps are flattened or not (and they shouldn't be nested anyway) we just care that they are available for JSON parsing/strings - just a call to toJSON should almost always be sufficient unless you need to work with nested js objects instead of maps for some reason or another
There was a problem hiding this comment.
we really just want to make sure the object is valid for JSON.stringify() and the raw method does that
@wrn14897 basically if the input has nested maps json.stringify may return an empty object for the Map instances but I don't believe that should be an issue with how these request pieces are inputted and stored because they really shouldn't have them to begin with - it's "safer" with broader compatibility to flatten, but only needed in cases where the mongoose map could be stored with nested maps, which in this case it really should not since it's JSON in and JSON out (and that's validated on user input to begin with)
There was a problem hiding this comment.
I see. Thanks for the detailed explanation. This makes sense.
Generic Webhook Option for Alerts
DETAILS:
This PR enables the creation and use of generic webhooks alongside the existing slack webhooks. This allows users to configure arbitrary webhook consumers/destinations with custom payloads.
For now the lack of signage/security features means more complex webhooks that perform actions on alert will most likely be gated off due to their internal requirements, but this should unlock a variety of message-focused consumers alongside the existing slack implementation. Query parameter usage was built into the migration and logic, and can be enabled in a later version when security options make those more complex use cases (like caching) worthwhile. For the time being many consumers allow/mirror QP functionality in the body of the request, and otherwise building into the url manually achieves the same purpose.
This implementation assumes and is limited to POST requests only, which is the ideal sender behavior and has exceptionally large coverage, but optionality for GETs and PUTs can be added in later versions if they are desired.
Message templating is still quite limited while the more robust templating system is in development, and users should refer to their specific consumer documentation for implementation.
As a minor addition, with the added complexity beyond just single slack webhooks, optional descriptions were also added to the webhook model and displayed on the settings page.
V1+ NEXT STEPS:
VISUALS:
TEAM SETTINGS UPDATE:
GENERIC WEBHOOK CREATION:
ALERT CREATION UPDATE:
