search

Speedygeek / ZendeskApi_v2

C# wrapper for the Zendesk API
Other
170 stars 200 forks source link

Add Custom Header #587

Closed jcrawfor74 closed 11 months ago

jcrawfor74 commented 11 months ago

We have a use case where we have a customisation on ZenDesk where a custom header is passed in and read and the site redirects to a client subsite based upon this header.

This means we need to customise the ZenDeskApi_V2 to support adding this custom header.

The following PR adds a new extended constructor that allows passing in the name of the custom header, and the value. If provided it will add the Custom Header to the Request. If not it works as it has previously.

It is an overloaded constructor so fully backwards compatible, as all existing constructors are retained.

CLAassistant commented 11 months ago

CLA assistant check
All committers have signed the CLA.

jcrawfor74 commented 11 months ago

Hi, I can see that this change has failed a unit test, but I have the inability to run these tests myself as they are not mocked unit tests but more like full integration tests that require connectivity to a ZenDesk environment to run.. In the test the first comment on the new ticket should say Help, second comment "New Comment". The tests failed as the data has flipped? I can see no area where my new custom header code would impact this call as these new custom header values would be NULL, and so there is no effective change to the code being executed. Given this is hitting a targeted environment, could this be a change to the test ZenDesk environment causing this failure?

jcrawfor74 commented 11 months ago

I think it would appear that I am correct, that you have a race condition in your tests causing the tests to randomly fail. My first commit failed 1 test on:

No code that I changed should have caused either of these tests to fail.

mozts2005 commented 11 months ago

I will have a look at the unit test.

But I think it would be better to add an additional headers dictionary to the constructor. This would allow others to change more than one header if need be.

jcrawfor74 commented 11 months ago

Ok, I will tweak accordingly Dictionary<string, string> and pass through all the constructors. Consider it done.