-
Notifications
You must be signed in to change notification settings - Fork 30
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
Maintain get list of all ContactLists, create new ContactList and delete ContactList #67
Conversation
Added list of contact lists request
Added list of contact lists request
Add DataMember attributes, use PascalCase for names and replace generic names with more specific ones to avoid confusion - these are recomendations of @nover
Syntax errors corrected
Change DateTime properties to string because deserialization of .NET type DateTime in WCF requires some more work e.g. the use of attributes [IgnoreDataMember], [OnSerializing] and [OnDeserialized] along with a private property - see https://stackoverflow.com/questions/18511147/change-default-date-serialization-in-wcf. String datestamps can be easily translated to DateTime explicitly by the caller as necessary. Adding DataMember attributes revealed the problem of deserialization of the DateTime properties (but not before runtime) - it caused exceptions where as without the DataMember attribute, the deserialization failure was not notified.
…ged each of their names to inclide the postfix "TimeStamp" to imply to the user that unix epoch is the value.
…by permitting data conversion to be performed optionally. This makes it easier to deal with HubSpot request entities that use standard object serialization (e.g. CreateContactListRequestHubSpotEntity). Note that most request entities use HubSpot's novel "property-value" serialization.
…ute to ensure that custom DataMember names are respected.
Fixed the problem with unpopulated properties of response. It was caused by DataMember custom names not being respected because of missing DataContract attributes on the class. |
Delete ContactList has been tested with my HubSpot account in my private test environment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall I think it looks good. A few comments here and there
…ListOfContactsClient.cs implemented.
I have tested the latest commits with our production client system and they have caused no new problems @nover. I am able to export new HubSpot contacts to contact lists. |
@nover |
@davymirfin I think last I checked some of the suggestions from my review had not been applied. If that is the case, then I think we are about ready to merge this. |
@davymirfin can you resolve the conflicts please, so we can get this merged in? :) |
@nover I have resolved the conflicts. Sorry it took me so long. I was a little unsure what was required. |
This new async method uses a request entity that must be serialized via standard object serialization. This is unusual because most of the currently implemented requests use HubSpot's custom "property-value" serialization syntax.
I have changed Core\Requests slightly in an attempt to try to make it easier to switch between serialization approaches.
I have tested the following in my development environment with my own HubSpot account:
public async Task CreateAsync(CreateContactListRequestHubSpotEntity payload) where T : IHubSpotEntity, new()
I was able to create new ContactLists but I have not attempted to set filters.
There is a problem with the response (CreateContactListResponseHubSpotEntity) deserialization. Some fields are not returned (e.g. CreatedAtTimeStamp and UpdatedAtTimeStamp) but there are enough fields that are correctly populated to make this version useful e.g. the timestamp MetaData.lastProcessingStateChangeAt is correctly populated.
Another job that is still to be done is the commented out code in CreateContactListRequestHubSpotEntity that must be removed.