Skip to content
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

Merged
merged 18 commits into from
May 5, 2021

Conversation

davymirfin
Copy link
Contributor

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.

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
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.
@davymirfin
Copy link
Contributor Author

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.

@davymirfin
Copy link
Contributor Author

Delete ContactList has been tested with my HubSpot account in my private test environment.

@davymirfin davymirfin changed the title Added method to create new ContactList Added 2 new methods Create new ContactList and Delete ContactList Jan 22, 2021
@davymirfin davymirfin changed the title Added 2 new methods Create new ContactList and Delete ContactList Maintain get list of all ContactLists, create new ContactList and delete ContactList Jan 24, 2021
Copy link
Contributor

@nover nover left a 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

src/Core/Requests/RequestDataConverter.cs Show resolved Hide resolved
src/Core/Requests/RequestSerializer.cs Show resolved Hide resolved
src/ListOfContacts/HubSpotListOfContactsClient.cs Outdated Show resolved Hide resolved
src/hubspot-client.csproj Outdated Show resolved Hide resolved
@davymirfin
Copy link
Contributor Author

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.

@davymirfin
Copy link
Contributor Author

@nover
Is any further action needed from me re create/delete ContactList (other than appropriate tests)?

@nover
Copy link
Contributor

nover commented Feb 23, 2021

@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.

@nover
Copy link
Contributor

nover commented Mar 12, 2021

@davymirfin can you resolve the conflicts please, so we can get this merged in? :)

@davymirfin
Copy link
Contributor Author

davymirfin commented Apr 7, 2021

@nover I have resolved the conflicts. Sorry it took me so long. I was a little unsure what was required.

@nover nover merged commit c33954b into skarpdev:master May 5, 2021
@nover nover mentioned this pull request May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants