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

Webhook improvements #22

Merged
merged 3 commits into from
Mar 27, 2024
Merged

Webhook improvements #22

merged 3 commits into from
Mar 27, 2024

Conversation

wcwynn
Copy link
Contributor

@wcwynn wcwynn commented Mar 22, 2024

Added ability to receive actual webhook details and fixed a bug with deletion

  • This commit adds some support to get the actual webhook url and other
    details. The webhook url is required as a parameter when deleting the
    webhook. When deleting the webhook, the configured webhook may be
    unknown to the application program. In such cases it is necessary to have
    a means to actually retrieve the url in order to provide the parameter required
    for a successful delete request. Similarly, when checking if the configured webhook
    is enabled, its necessary to actually examine the response to determine the flag
    value that was returned by the api.

wcwynn added 3 commits March 19, 2024 14:14
- This commit adds some support to get the actual webhook url and other
  details. The webhook url is required as a parameter when deleting the
  webhook. When deleting the webhook, the configured webhook may be
  unknown. In such cases it is necessary to have a means to actually
  retrieve the url in order to provide the parameter required for a
  succssful delete request.
- Changed from DEL to POST so that it works and matches the API spec
- Updated the test expectation to match the api spec
@nasa9084 nasa9084 self-requested a review March 27, 2024 11:52
Copy link
Owner

@nasa9084 nasa9084 left a comment

Choose a reason for hiding this comment

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

@wcwynn Thank you for your contribution!
Honestly, I personally don't use the webhook feature so I cannot confirm if the implementation is correct.
so let me merge this PR now and release it in a new version and then please make another PR if you found any issues when you use this feature 🙇

@nasa9084 nasa9084 merged commit 6849913 into nasa9084:main Mar 27, 2024
1 check passed
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.

2 participants