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

Set webhook content type (#15952) #15953

Conversation

sgross-emlix
Copy link

@sgross-emlix sgross-emlix commented Dec 11, 2024

Description

Set correct content type for webhook's json payload.

Fixes #15952

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

See related issue's reproduction steps

Result is now : Your General Webhook Integration works!
And the gitlab pipeline is actually triggered

Checklist:

@sgross-emlix sgross-emlix requested a review from snipe as a code owner December 11, 2024 09:31
Copy link

welcome bot commented Dec 11, 2024

💖 Thanks for this pull request! 💖

We use semantic commit messages to streamline the release process and easily generate changelogs between versions. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix if it doesn't have one already.

Examples of commit messages with semantic prefixes:

  • Fixed #<issue number>: don't overwrite prevent_default if default wasn't prevented
  • Added #<issue number>: add checkout functionality to assets
  • Improved Asset Checkout: use new notification method for checkout

Things that will help get your PR across the finish line:

  • Document any user-facing changes you've made.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

We send json payload in HTTP `POST`. Communicate this explicitly towards
the server by setting content-type header appropriately.
Copy link

what-the-diff bot commented Dec 11, 2024

PR Summary

  • Improved HTTP requests handling
    By incorporating the use GuzzleHttp\Psr7\Request; line in our code, our practices for handling HTTP requests have been streamlined and made more efficient.

  • Updated method within 'testWebhook' function
    We've altered the post method inside our testWebhook function so that it now uses the send method together with a Request object. This update improves the overall performance of the function, giving us a faster and more reliable tool for our testing needs.

  • Corrected content-type header mistake
    In the functions googleWebhookTest and msTeamTestWebhook, we've corrected an error in the content-type header. It used 'applications/json' instead of 'application/json'. This correction ensures better compliance with the accepted standards and boosts the interoperability with other services adopting the same data format.

  • Enhanced Readability with the introduction of Headers Variable
    To ensure our code is easier to read and follow, we've introduced a headers variable in the testWebhook function. A step that makes our code cleaner and supports the speedy identification of any potential issues in the future.

Content type for json is `application/json` without the plural `s`
@sgross-emlix sgross-emlix force-pushed the sgross/15952/set-webhook-content-type branch from 7e9305c to a8f153b Compare December 11, 2024 09:33
@snipe snipe requested a review from Godmartinz December 11, 2024 11:24
@sgross-emlix
Copy link
Author

On second thought one might want to get rid of the Guzzle Client and use the Facades/Http class for everything. This might reduce code redundancy. But my php skill is too low and was merely enough for this hotfix in our instance.

@Godmartinz
Copy link
Collaborator

@sgross-emlix could you retarget this PR to develop? thanks

@snipe
Copy link
Owner

snipe commented Dec 11, 2024

I've fixed that typo on develop and master. Thanks for the heads up!

@snipe snipe closed this Dec 11, 2024
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.

Webhooks provide no or wrong content type
3 participants