intercom / intercom-dotnet

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

Company Name Not Being Set When Using UsersClient.Update #47

Closed alcjamgit closed 7 years ago

alcjamgit commented 7 years ago

Hi,

When adding new company for the user, this is what i do:

usersClient.Update(new User() { email = "example@example.com", companies = new List() { new Company() { company_id = "1234", name = "company_name" } } });

This posts okay, but the name in intercom is not "company_name" but the id (guid). I'm using version 1.0.20.

jasenf commented 7 years ago

confirm same problem.

patoncrispy commented 7 years ago

I'm also having this problem.

markalanevans commented 7 years ago

@AHBroyles It appears that the you guys broke the company updating inside the user object when you updated this package.

https://github.com/intercom/intercom-dotnet/blob/master/src/Intercom/Clients/UsersClient.cs#L412

This essentially only updates company name and company_id and breaks the ability to create/update new companies, or even use the "remove" key and delete companies. which is all defined in your API.

markalanevans commented 7 years ago

@khalilovcmd Are you sure you wanted to do this: https://github.com/intercom/intercom-dotnet/commit/415d795a4b7e90b98b9b501ba6c63ebe08ce5cd6 and the subsequent updates

It breaks the functionality of syncing all the company data, and prevents the possibility of deleting a company using the remove flag, basically only updating company_id and name.

markalanevans commented 7 years ago

@patoncrispy , @jasenf version 1.0.7 (nuget) works, you just can't sync user.phone until they fix this

khalilovcmd commented 7 years ago

@alcjamgit @markalanevans Sorry for the late response, and thank you for reporting this issue and tagging me. I have prepared a PR that will help resolve this.

khalilovcmd commented 7 years ago

You should be able to install the newest Nuget Package.

markalanevans commented 7 years ago

@khalilovcmd Why are you doing any filtering on companies at all?

 if (user.companies != null && user.companies.Any())
 -                companies = user.companies.Select (c => new Company () { id = c.id, company_id = c.company_id }).ToList ();        +            {
 +                companies = user.companies.Select(c => new Company()
 +                {
 +                    remote_created_at = c.remote_created_at,
 +                    company_id = c.company_id,
 +                    name = c.name,
 +                    monthly_spend = c.monthly_spend,
 +                    custom_attributes = c.custom_attributes,
 +                    plan = c.plan
 +                }).ToList();
 +            }

Why override and remap the values? What are you trying to prevent?

khalilovcmd commented 7 years ago

@markalanevans Sorry for my rusty .NET knowledge.

My intent here is an equivalent of a List<>.Map. Therefore, I used the select method to only extract the required parameters in order to avoid getting an API Error for including parameters that are not needed.

Would you like to do a refactor over here?

markalanevans commented 7 years ago

@khalilovcmd What you did will definitely work, I was just trying to understand why sending the Customer object itself was an issue, I didn't know there were attributes on it that you should not be sending.

Thank you ver much!

khalilovcmd commented 7 years ago

Sure 👍 Glad it is clearer now.