A few features request

[microknights]

I have done a few tests with the ContactApi and it worked like a charm :)

To succeed my goal i have a few features i would like to add, but i thought that i would clear it with you before pushing PR's.

1. A CreateOrUpdate without email as parameter. My contacts have not supplied email at creation point, that will come later on with an Update.

2. Add a HubSpotOptions, on Hubspot constructor, with a property saying IgnorePropertiesWithNullValues' as boolean. This means that CreateandUpdate` will only send those property values that is not null. This prevents sending empty values, and when updating prevent override a property with an empty value.

3. Add an extension class with .AddHubspot(<apikey>,<hubspotOptions>) to use DI through IServiceCollection. Hubspot, ContactApi and all other *Api will be added.

If needed we can create an Issue for those bullets that needs further discussion.

Cheers, Frank

[telmengedar]

Hi and sorry for not getting back to you
It seems i missed a notification here.

1. Currently the ContactApi just represents more or less some of the Methods exposed by HubSpot itself. A CreateOrUpdate without an e-mail would require a custom strategy on when to create or update. In the most simple case this would be something like: has contact an id? then update contact, in other cases create a new contact with the given properties. Anything more complex should at least get discussed i guess.

2. Perhaps the ignore null values behavior should even be the default behavior since most updates should only update a subset of properties. At least in my cases this seems to be the usual update behavior of a record.

3. agreed i think ...

Other than that i'm always open to extend this api as it currently only implements what i needed up until now.

[microknights]

Other than that i'm always open to extend this api as it currently only implements what i needed up until now.
Super, we are one the same mission. I'm currently on vacation, but will be more active when i am back.

Regarding 1)
I have create an separate CreateOrUpdate<T> without email as parameter, which the REST API fully support.

Regarding 2)
I have created:

            var client = new HubSpot("00000000-0000-0000-0000-000000000000", new HubSpotOptions
            {
                IgnorePropertiesWithNullValues = true
            });

We can set IgnorePropertiesWithNullValues with default as true.

Regarding 3)
The only thing here is whatever HubSpot is thread-safe, in regards of the registering as singleton/scoped/transient. And also if the Configuration should be in a seperated nuget?

Cheers, Frank

[telmengedar]

okay, so i think there are no issues with the first 2 cases

For the last point:
Over here we have the service running as a singleton since it basically just wraps calls to an HttpClient which in itself should be threadsafe if used correctly. On the contrary multiple persons report the HttpClient to leak connections (at least for a time) if instanciated multiple times.
So i guess a singleton should be the correct approach here.

For an extension using the IServiceCollection we need at least a reference to "Microsoft.Extensions.DependencyInjection.Abstractions" which would work in netstandard 2.0 but might not be super clean since it is only used in Net Core Apps (imho).

But even introducting a netcoreapp build is actually only an entry in the project file since the project is already multi-targeted at netstandard2.0 and net45. This new target would also be packed in the same nuget so if done right this should be possible without creating a separate nuget.

I tend to lean towards the solution of adding a netcoreapp build with the extension only compiled in there because it sounds more clean somehow.

Other than that ... have a nice vacation :D

[microknights]

Better late than never - WFH gives more time to do all the things, you usually don't get done ;)

I'll just refresh the changes before pressing the PR's button.

1. will have a PR later on.
2. is ready with a minor change, since IgnorePropertiesWithNullValues is placed under a Contact property.
3. will have a PR later on.
4. i have made a PR for removing netcoreapp as tagetframework, since there is no need for it. Microsoft.Extensions.DependencyInjection.Abstractions is a netstandard library
5. added a Test project, so the api can be tested. The AiKey is hidden using the UserSecret configuration.

Cheers, Frank

[microknights]

Hi Toni,

Thanks for the PR merges, i have two/three more in the pipeline (BlogPost and Engagements) :)

About your comment regarding >>
Other than that it deviates a bit from my code style, so a few details like member naming conventions or parameter order might change if i have to do something at it, but since it touches no exposed code it should be fine in that state.
<<
Can you point them out for me (or just change them), it is better that the signature is the same all over.

One thing thing i have noticed that we should get a common agreement on, is that you have [Name] attributes i.e. the HubSpotContact rather than using [JsonProperty/JsonIgnore]. This means that you have your own Converter instead of using JsonConverter (Newton) builtin conversion - is there a reason/thought about that?

My reasons for using JsonConvert, is that when we have an object graph in the json response we get the conversion for free - rather than making custom conversion for all our hubspot objects.
But maybe i am overlooked something?

[microknights]

hhmm my renaming on the (unit)test project didn't go well ;) i will clean that up with a PR, only one is needed.

[eikooc]

Hi, I'd also chime in that I created a PR for the engagements API. I work together with @microknights.

I added a PatchAsync HttpClient extension method since the version of dotnet we use doesn't have that method. It was needed to do a update request on the engagements API.

I also favor the JsonConvert approach, mostly because it reduces the amount of code having to be written in the project.