hubspot-net / HubSpot.NET

C# .NET Wrapper around the common HubSpot APIs.
MIT License
112 stars 137 forks source link

Contact API Property dictionary values not updated when there is no existing dictionary key #37

Closed Psypher9 closed 5 years ago

Psypher9 commented 6 years ago

In the ContactHubSpotModel the Properties dictionary is missing values when the model itself has a value Example:

        public string FirstName {
            set {
                _FirstName = value;
                if (Properties.ContainsKey("firstname")) // no key available when Model's value set before sending DTO
                    Properties["firstname"].Value = value; // will be null when sent to HubSpot
            }
            get
            {
                if(string.IsNullOrWhiteSpace(_FirstName))
                { _FirstName = Properties.ContainsKey("firstname") ? Properties["firstname"].Value : string.Empty; }
                return _FirstName;
            }
        }

Repro Steps:

Just simply try to use the Create() example provided in the Examples assembly

microknights commented 5 years ago

I have played around with the v1-preview branch and ran into this issue.

Is there a reason why the backing field is used, rather than just use the Properties directly and have code like:

            get => Properties.TryGetValue("email", out var prop) ? prop.Value : string.Empty;
            set => Properties["email"] = new ContactProperty(value);
microknights commented 5 years ago

Since i have only played with v1-preview, i am unsure about the custom property behavior. But shouldn't a simple auto property be working?

        [DataMember(Name = "testproperty")]
        public string TestProperty { get; set; }

I can push some PR's to get it working, but i am not aware of how the full api is working - so i could easily be missing some points.

Psypher9 commented 5 years ago

This is a good point. That code is currently being updated to simply use the auto properties right now. It's something that I have been trying to finish up, but just haven't yet. This use of the backing fields was originally a way to try in order to get the properties dictionary to be fully populated.

I have since found a better way that doesn't require anyone's custom models to follow this same methodology. Thanks for catching this though. When are you hoping to have a fix for this?

microknights commented 5 years ago

well, i ran into other problems when doing Update. Couldn't figure out the problem, but Hotspot support said that only those fields that needs update should be transported over the wire. I've tested with the contacts example where it first creates and the updates - which leads to same error.

So the auto properties is only half the solution, there must also be a mechanism for only sending those properties that contains values - maybe a not null filter is enough. But how has this Update worked in the previous versions?

Psypher9 commented 5 years ago

This is a good point. That is actually the purpose of the CreateOrUpdateContactTransportModel. The abstract `PropertyTransport

` is where the magic of loading the Properties into HubSpot's required format happens. Currently, the preview requires passing the `ContactHubSpotModel` into a `COUCTM` constructor. I don't particularly like this methodology, and as of last night I was making a less awkward way of transferring the contact into the right format. Let me find an example.
Psypher9 commented 5 years ago

@microknights are you using this method here to update? Also, the Update specific method has been weird lately so I would recommend using the CreateOrUpdate method instead for the moment.

However, working on Update could be a good place to start.

I will say though that I have a few commits that should streamline this whole process. So perhaps wait until I have those changes merged in.