intercom / intercom-dotnet

Intercom API client library for .NET
https://developers.intercom.io/reference
Apache License 2.0
63 stars 53 forks source link

.NET SDK Create company, attributes missing #100

Closed FFRainman closed 6 years ago

FFRainman commented 6 years ago

With the current version of the .NET SDK (1.0.31) the following code:

Company company = new Company();
company.name = "company_name";
company.company_id = "company_id";
company.created_at = ConvertDateTimeToUnixTimestamp(DateTime.UtcNow);
company.industry = "industry_name";
companyClient.Create(company);

Doesn't correctly set the "created_at" and "industry" properties on the company.

Even when calling update to that company those properties doesn't get set.

kmossco commented 6 years ago

Looking at the current code I can see that a created_at attribute is missing, but we do have an industry attribute since this PR: https://github.com/intercom/intercom-dotnet/pull/94.

Not quite sure why it isn't being set, but looking into it. 👍

kmossco commented 6 years ago

I can see that the problem is that you are trying to update a value that is reserved for Intercom. If you check the company Model there are two lines that are relevant here:

https://github.com/intercom/intercom-dotnet/blob/7d2dff504425daa797ceb8bf2d91af2049fa25ba/src/Intercom/Data/Company.cs#L13 https://github.com/intercom/intercom-dotnet/blob/7d2dff504425daa797ceb8bf2d91af2049fa25ba/src/Intercom/Data/Company.cs#L14

These attributes are described as this in our docs:

image

So the created_at is read-only and reserved for Intercom. And the remote_created_at is the timestamp you want to edit to change that in the UI. This is the API expected behaviour, so not a problem of the SDK itself. 👍

FFRainman commented 6 years ago

created_at should probably be set as read only then in the Company model in the .NET SDK if it's something you cannot change.

It seems to be working now, at least with the created_at being set from Intercom but setting industry doesn't do anything, the value is still set as unknown in Intercom.

kmossco commented 6 years ago

Hey again @FFRainman can you send me the request_id of a call you made with the industry value and where it didn't go through, as well as a snippet of code you used for it? I can investigate this further. 👍

FFRainman commented 6 years ago

Here is a simple code snippet from .NET, not sure where to find the request_id of the call.

Company company = new Company();
company.name = "company_name";
company.company_id = "company_id";
company.remote_created_at = ConvertDateTimeToUnixTimestamp(DateTime.UtcNow);
company.industry = "industry_name";
companyClient.Create(company);
kmossco commented 6 years ago

The request_id should be in the headers that we return from the server. But with this code it should be enough, looking into it! 👍

kmossco commented 6 years ago

Hey again @FFRainman! I did another round of testing and this seems to be working in both the old version of the library and the PR #97. This is what I tested:

class MainClass
    {
        public static long UnixTimeNow()
        {
            var timeSpan = (DateTime.UtcNow - new DateTime(1970, 1, 1, 0, 0, 0));
            return (long)timeSpan.TotalSeconds;
        }

        public static void Main(string[] args)
        {
            string TOKEN = "tokenstuff="; 
            CompanyClient companyClient = new CompanyClient(new Authentication(TOKEN));

            Company company = new Company();
            company.name = "testing_company";
            company.company_id = "5ac104331d149fc717aa69db-qualification-company";
            company.remote_created_at = UnixTimeNow();
            company.industry = "testing stuff";
            companyClient.Create(company);

            companyClient.View(company);
        }
    }

This created the following company:

image

Listing the remote_created_at attribute returns:

"remote_created_at": "2018-04-01T16:10:48.000Z"

Is this still not working for you? If so it's possible that there is something in your setup that isn't quite right. Feel free to reopen this if that's the case. 👍