ActiveCampaign / postmark-dotnet

A .NET library for the Postmark API
http://developer.postmarkapp.com/
Other
50 stars 46 forks source link

Changing TrackLinks to allow Null #70

Closed DmitryEvmenov closed 4 years ago

DmitryEvmenov commented 5 years ago

Hey!

I'm going to grade to a newer version of the package to start using TrackLinks per message level which is currently not available on the version we use. When I was looking through their docs I noticed "This setting overrides the default setting for the server.". In our scenario we have PostMark servers with the TrackLinks option set to various option on each one on sever level, and our goal it to 1) Change the behavior to None for one specific message 2) Ensure we don't override all other settings on all other servers.

The current implementation makes me think that we the 3 options that we have we'll not have an ability to use Server level setting, we'll always be sending one of 3 options per message level explicitly. So I'd like to propose a change to allow sending null in that option to not override server setting. I made more digging into their FAQs and found that it seems to be a valid option according to what's said here: image

Please let me know how it sounds.

A couple of side notes: 1) I couldn't make the UnitTest run locally without the configuration init errors. I found some comments describing that this's intended that the configuration is not stored in the repo, so I assume this's OK. 2) I left the message default value to be set to None here and not changed it to null intentionally to ensure I don't introduce backward incompatible/breaking change. Though I think it might be good to change to to null with one of major release eventually.

hn3000 commented 4 years ago

The failed tests seem to have been caused by missing configuration settings:

Error Message:
 Assert.NotNull() Failure

Stack Trace:
   at Postmark.Tests.ClientBaseFixture.AssertSettingsAvailable() in /home/travis/build/wildbit/postmark-dotnet/src/Postmark.Tests/ClientBaseFixture.cs:line 20

Maybe the test could be re-run after the settings have been corrected?