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 service name/id field to webhook alerts #3199

Merged
merged 5 commits into from
Jul 26, 2023

Conversation

allending313
Copy link
Contributor

@allending313 allending313 commented Jul 26, 2023

  • Identified the issue which this PR solves.
  • Read the CONTRIBUTING document.
  • Code builds clean without any errors or warnings.
  • Added appropriate tests for any new functionality.
  • All new and existing tests passed.
  • Added comments in the code, where necessary.
  • Ran make check to catch common errors. Fixed any that came up.

Description:
Webhook alert bundle notifications included the service id and name which were not present in the alert type. This feat adds the service name and id field to the webhook alert notification post data.

Which issue(s) this PR fixes:
#3009

Before:
127.0.0.1 - - [26/Jul/2023 09:21:03] "POST /webhook HTTP/1.1" 200 -
Data received from Webhook is:  {'AppName': 'GoAlert', 'Type': 'Alert', 'AlertID': 79679, 'Summary': 'test3', 'Details': ''}

After:
127.0.0.1 - - [26/Jul/2023 09:39:48] "POST /webhook HTTP/1.1" 200 -
Data received from Webhook is:  {'AppName': 'GoAlert', 'Type': 'Alert', 'AlertID': 79679, 'Summary': 'test3', 'Details': '', 'ServiceID': 'df9cfe94-7a76-4757-94b5-bf6013af831e', 'ServiceName': 'test_webhook'}

Copy link
Contributor

@andrewbenington andrewbenington left a comment

Choose a reason for hiding this comment

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

I made the changes I'm suggesting and the functionality is still there, I don't think the Notification store needs to be changed at all

Copy link
Contributor

@andrewbenington andrewbenington left a comment

Choose a reason for hiding this comment

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

lgtm!

@allending313 allending313 linked an issue Jul 26, 2023 that may be closed by this pull request
@allending313 allending313 merged commit 7e540ea into master Jul 26, 2023
@allending313 allending313 deleted the feat/webhook-service-name branch July 26, 2023 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Service "Name" field to user notification webhook data.
3 participants