Skip to content

Conversation

@kmossco
Copy link
Contributor

@kmossco kmossco commented Jan 12, 2018

Changelog:

  • Added NetStandard 2.0 support
  • Renamed the compiled library name
  • Updated RestSharp, Newtonsoft.Json and Nunit
  • Updated the Nunit tests for the new syntax (thanks to @nicholi and his work)
  • Updated several models to have the new fields
  • Refactored the README file to increase legibility
  • Updated README file with the visitor's endpoint usage instructions

Bugfixes:

I have left this as multiple commits in case we need to revert quickly. This should make it easier.

Note:
I didn't move the library to async and removed the dependency on RestSharp because after exploring the work done by @michael-watson I wasn't confident that our current test coverage could protect us from any unintended consequences.

And once again, thank you so much for the time and effort you put into this library to both @nicholi and @michael-watson. If you could drop me an email at bruno+dotnet[at]intercom[dot]io I'd love to discuss with you how we can improve the visibility of the work you put into it. 👍

@choran
Copy link
Member

choran commented Jan 19, 2018

Wow, just general call out on the readme changes alone, think there is lots more helpful detail and examples here and even the presentation changes are great
Working my way through it now but just wanted to call that out already

Assert.Throws<ArgumentNullException>(() => companyClient.Update(null));
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need to add a test for something like plan here? just wondering since the way plan is a string and returns an object, it caused issues before. Maybe now is not the time? but it would be good to try and catch edge cases like that i.e. create company without plan and with plan tests to make sure they work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to revamp our testing and it's definitely something I wanna add in the roadmap. But for this PR decided not too change too many things, the specs were only changed to conform to the new NUnit specs.

Assert.Throws<ArgumentNullException>(() => contactsClient.Delete(new Contact()));
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re: my earlier comment on company tests ... I think you have the right approach alright, just make the minimum changes we need for this migration and then we can think about improving the unit tests i a more systematic way 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep that was my train of thought as well. Making this as minimal as possible, and then add to the roadmap to redo the tests. 👍

monthly_spend = company.monthly_spend,
custom_attributes = company.custom_attributes,
plan = company.plan != null ? company.plan.name : String.Empty
plan = company.plan != null ? company.plan.name : null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this change fix issue where we were over writing the company name with empty string when updating companies? Is that the benefit of having null here instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That line of code is already in the repo, I just had to make that change there to avoid rebasing. Basically because we ignore null in our JsonSerializer, if the company.plan.name isn't set, we won't send it in the payload. 👍

public class AdminConversationReply : Reply
{
public string type
public override string type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my own dotnet stupidity but why do we need an override here now and not before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We needed this before, just didn't catch it. 🙈

public List<ConversationPart> conversation_parts { get; set; }
[JsonConverter(typeof(ListJsonConverter))]
public List<Tag> tags { get; set; }
public List<Customer> customers { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great to have all these fields, snooze and customers and waiting_since now included 👍

public long updated_at { get; set; }
public long notified_at { get; set; }
public object assigned_to { get; set; }
public string assigned_to { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a string now?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to have been changed when we added group conversations:

image

In here.

public class Visitor : Model
public class Visitor : User
{
public string user_id { get; set; }
Copy link
Member

@choran choran Jan 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am asking alot of stupid questions here so apologies but why are you changing the class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current Model class only has:

public virtual string id { set; get; }
public virtual string type { get; set; }

While we need more than that, as the visitors endpoint now returns more data. By inheriting from the User now we get access to all these here: Visitor Model. This avoids duplicating code.

@NPSF3000
Copy link

NPSF3000 commented Mar 2, 2018

Hi guys, any word on the status of this? We're moving our entire project to net core and this is one of the few blocking libraries left.

@mbrookson
Copy link

Hi, will this library only work with Xamarin iOS and Android once this migration to .NET Standard is resolved?

@kmossco
Copy link
Contributor Author

kmossco commented Mar 17, 2018

@NPSF3000 so sorry for the delay in getting this out of the door. Just in case you aren't following the issue #90: the reason behind this delay is that we unfortunately hit a snag creating the Nuget package in a way that it doesn't break plenty of apps that don't want to upgrade the library, and we are working to create a new release. We should have an update very soon though!

@kmossco
Copy link
Contributor Author

kmossco commented Mar 17, 2018

@mbrookson this will work with any project compatible with .NET Standard 2.0. We don't have any support for Xamarin bindings though, and don't really have it yet in our near future roadmap.

@michael-watson
Copy link

To add details, this library will work in any Xamarin project, you just won't have the UI controls that are currently provided in their native SDKs. The options are porting that code to C# (currently not open-source so no go there) or binding the iOS/Android libraries.

For anyone out there that is interested, I ran the iOS cocoapod through Objective Sharpie and got the output for the ios bindings. For Android, you would need the library as a jar or aar and then bind that. Typically Android libraries are much easier. The iOS portion you could just download the framework manually, build it and reference that in the Xamarin.iOS project. I have not tested any of the library bindings, so if something doesn't work, check they binding type reference for that method and fix whatever difference there is.

@kmossco If the source code for the iOS and Android portions of the messengers (iOS Controllers/Delegates and Android xaml/Activities/Adapters) were open sourced, it wouldn't take much to translate that directly to Xamarin C#. I have an upcoming project that would be great if I could use Intercom inside of the mobile application, but having to build the binding library process typically is a deal breaker for most developers. Feel free to PM me if you have interest in collaborating on the possibilities here.

iOS Binding Outputs

C# ApiDefinition

using System;
using Foundation;
//using Intercom;
using ObjCRuntime;

namespace intercom.binding.ios
{
    // @interface ICMCompany : NSObject
    [BaseType(typeof(NSObject))]
    interface ICMCompany
    {
        // @property (copy, nonatomic) NSString * _Nullable companyId;
        [NullAllowed, Export("companyId")]
        string CompanyId { get; set; }

        // @property (copy, nonatomic) NSString * _Nullable name;
        [NullAllowed, Export("name")]
        string Name { get; set; }

        // @property (nonatomic, strong) NSDate * _Nullable createdAt;
        [NullAllowed, Export("createdAt", ArgumentSemantic.Strong)]
        NSDate CreatedAt { get; set; }

        // @property (nonatomic, strong) NSNumber * _Nullable monthlySpend;
        [NullAllowed, Export("monthlySpend", ArgumentSemantic.Strong)]
        NSNumber MonthlySpend { get; set; }

        // @property (copy, nonatomic) NSString * _Nullable plan;
        [NullAllowed, Export("plan")]
        string Plan { get; set; }

        // @property (nonatomic, strong) NSDictionary<NSString *,id> * _Nullable customAttributes;
        [NullAllowed, Export("customAttributes", ArgumentSemantic.Strong)]
        NSDictionary<NSString, NSObject> CustomAttributes { get; set; }

        // +(NSString * _Nonnull)nullStringAttribute;
        [Static]
        [Export("nullStringAttribute")]
        //[Verify(MethodToProperty)]
        string NullStringAttribute { get; }

        // +(NSNumber * _Nonnull)nullNumberAttribute;
        [Static]
        [Export("nullNumberAttribute")]
        //[Verify(MethodToProperty)]
        NSNumber NullNumberAttribute { get; }

        // +(NSDate * _Nonnull)nullDateAttribute;
        [Static]
        [Export("nullDateAttribute")]
        //[Verify(MethodToProperty)]
        NSDate NullDateAttribute { get; }

        // -(NSDictionary<NSString *,id> * _Nonnull)attributes;
        [Export("attributes")]
        //[Verify(MethodToProperty)]
        NSDictionary<NSString, NSObject> Attributes { get; }
    }

    // @interface ICMUserAttributes : NSObject
    [BaseType(typeof(NSObject))]
    interface ICMUserAttributes
    {
        // @property (copy, nonatomic) NSString * _Nullable email;
        [NullAllowed, Export("email")]
        string Email { get; set; }

        // @property (copy, nonatomic) NSString * _Nullable userId;
        [NullAllowed, Export("userId")]
        string UserId { get; set; }

        // @property (copy, nonatomic) NSString * _Nullable name;
        [NullAllowed, Export("name")]
        string Name { get; set; }

        // @property (copy, nonatomic) NSString * _Nullable phone;
        [NullAllowed, Export("phone")]
        string Phone { get; set; }

        // @property (copy, nonatomic) NSString * _Nullable languageOverride;
        [NullAllowed, Export("languageOverride")]
        string LanguageOverride { get; set; }

        // @property (nonatomic, strong) NSDate * _Nullable signedUpAt;
        [NullAllowed, Export("signedUpAt", ArgumentSemantic.Strong)]
        NSDate SignedUpAt { get; set; }

        // @property (assign, nonatomic) BOOL unsubscribedFromEmails;
        [Export("unsubscribedFromEmails")]
        bool UnsubscribedFromEmails { get; set; }

        // @property (nonatomic, strong) NSArray<ICMCompany *> * _Nullable companies;
        [NullAllowed, Export("companies", ArgumentSemantic.Strong)]
        ICMCompany[] Companies { get; set; }

        // @property (nonatomic, strong) NSDictionary<NSString *,id> * _Nullable customAttributes;
        [NullAllowed, Export("customAttributes", ArgumentSemantic.Strong)]
        NSDictionary<NSString, NSObject> CustomAttributes { get; set; }

        // +(NSString * _Nonnull)nullStringAttribute;
        [Static]
        [Export("nullStringAttribute")]
        //[Verify(MethodToProperty)]
        string NullStringAttribute { get; }

        // +(NSNumber * _Nonnull)nullNumberAttribute;
        [Static]
        [Export("nullNumberAttribute")]
        //[Verify(MethodToProperty)]
        NSNumber NullNumberAttribute { get; }

        // +(NSDate * _Nonnull)nullDateAttribute;
        [Static]
        [Export("nullDateAttribute")]
        //[Verify(MethodToProperty)]
        NSDate NullDateAttribute { get; }

        // -(NSDictionary<NSString *,id> * _Nonnull)attributes;
        [Export("attributes")]
        //[Verify(MethodToProperty)]
        NSDictionary<NSString, NSObject> Attributes { get; }
    }

    // @interface Intercom : NSObject
    [BaseType(typeof(NSObject))]
    interface Intercom
    {
        // +(void)setApiKey:(NSString * _Nonnull)apiKey forAppId:(NSString * _Nonnull)appId;
        [Static]
        [Export("setApiKey:forAppId:")]
        void SetApiKey(string apiKey, string appId);

        // +(void)setUserHash:(NSString * _Nonnull)userHash;
        [Static]
        [Export("setUserHash:")]
        void SetUserHash(string userHash);

        // +(void)registerUnidentifiedUser;
        [Static]
        [Export("registerUnidentifiedUser")]
        void RegisterUnidentifiedUser();

        // +(void)registerUserWithUserId:(NSString * _Nonnull)userId email:(NSString * _Nonnull)email;
        [Static]
        [Export("registerUserWithUserId:email:")]
        void RegisterUserWithUserId(string userId, string email);

        // +(void)registerUserWithUserId:(NSString * _Nonnull)userId;
        [Static]
        [Export("registerUserWithUserId:")]
        void RegisterUserWithUserId(string userId);

        // +(void)registerUserWithEmail:(NSString * _Nonnull)email;
        [Static]
        [Export("registerUserWithEmail:")]
        void RegisterUserWithEmail(string email);

        // +(void)logout;
        [Static]
        [Export("logout")]
        void Logout();

        // +(void)reset __attribute__((deprecated("'+[Intercom reset]' is deprecated. 'Use +[Intercom logout]' instead.")));
        [Static]
        [Export("reset")]
        void Reset();

        // +(void)updateUser:(ICMUserAttributes * _Nonnull)userAttributes;
        [Static]
        [Export("updateUser:")]
        void UpdateUser(ICMUserAttributes userAttributes);

        // +(void)logEventWithName:(NSString * _Nonnull)name;
        [Static]
        [Export("logEventWithName:")]
        void LogEventWithName(string name);

        // +(void)logEventWithName:(NSString * _Nonnull)name metaData:(NSDictionary * _Nonnull)metaData;
        [Static]
        [Export("logEventWithName:metaData:")]
        void LogEventWithName(string name, NSDictionary metaData);

        // +(void)presentMessenger;
        [Static]
        [Export("presentMessenger")]
        void PresentMessenger();

        // +(void)presentMessageComposer;
        [Static]
        [Export("presentMessageComposer")]
        void PresentMessageComposer();

        // +(void)presentMessageComposerWithInitialMessage:(NSString * _Nonnull)message;
        [Static]
        [Export("presentMessageComposerWithInitialMessage:")]
        void PresentMessageComposerWithInitialMessage(string message);

        // +(void)presentConversationList;
        [Static]
        [Export("presentConversationList")]
        void PresentConversationList();

        // +(void)presentHelpCenter;
        [Static]
        [Export("presentHelpCenter")]
        void PresentHelpCenter();

        // +(void)setDeviceToken:(NSData * _Nonnull)deviceToken;
        [Static]
        [Export("setDeviceToken:")]
        void SetDeviceToken(NSData deviceToken);

        // +(BOOL)isIntercomPushNotification:(NSDictionary * _Nonnull)userInfo;
        [Static]
        [Export("isIntercomPushNotification:")]
        bool IsIntercomPushNotification(NSDictionary userInfo);

        // +(void)handleIntercomPushNotification:(NSDictionary * _Nonnull)userInfo;
        [Static]
        [Export("handleIntercomPushNotification:")]
        void HandleIntercomPushNotification(NSDictionary userInfo);

        // +(void)setBottomPadding:(CGFloat)bottomPadding;
        [Static]
        [Export("setBottomPadding:")]
        void SetBottomPadding(nfloat bottomPadding);

        // +(void)setInAppMessagesVisible:(BOOL)visible;
        [Static]
        [Export("setInAppMessagesVisible:")]
        void SetInAppMessagesVisible(bool visible);

        // +(void)setLauncherVisible:(BOOL)visible;
        [Static]
        [Export("setLauncherVisible:")]
        void SetLauncherVisible(bool visible);

        // +(void)hideMessenger;
        [Static]
        [Export("hideMessenger")]
        void HideMessenger();

        // +(NSUInteger)unreadConversationCount;
        [Static]
        [Export("unreadConversationCount")]
        //[Verify(MethodToProperty)]
        nuint UnreadConversationCount { get; }

        // +(void)enableLogging;
        [Static]
        [Export("enableLogging")]
        void EnableLogging();

        // +(void)setNeedsStatusBarAppearanceUpdate;
        [Static]
        [Export("setNeedsStatusBarAppearanceUpdate")]
        void SetNeedsStatusBarAppearanceUpdate();
    }

    [Static]
    //[Verify(ConstantsInterfaceAssociation)]
    partial interface Constants
    {
        // extern NSString *const _Nonnull IntercomUnreadConversationCountDidChangeNotification __attribute__((visibility("default")));
        [Field("IntercomUnreadConversationCountDidChangeNotification", "__Internal")]
        NSString IntercomUnreadConversationCountDidChangeNotification { get; }

        // extern NSString *const _Nonnull IntercomWindowWillShowNotification __attribute__((visibility("default")));
        [Field("IntercomWindowWillShowNotification", "__Internal")]
        NSString IntercomWindowWillShowNotification { get; }

        // extern NSString *const _Nonnull IntercomWindowDidShowNotification __attribute__((visibility("default")));
        [Field("IntercomWindowDidShowNotification", "__Internal")]
        NSString IntercomWindowDidShowNotification { get; }

        // extern NSString *const _Nonnull IntercomWindowWillHideNotification __attribute__((visibility("default")));
        [Field("IntercomWindowWillHideNotification", "__Internal")]
        NSString IntercomWindowWillHideNotification { get; }

        // extern NSString *const _Nonnull IntercomWindowDidHideNotification __attribute__((visibility("default")));
        [Field("IntercomWindowDidHideNotification", "__Internal")]
        NSString IntercomWindowDidHideNotification { get; }

        // extern NSString *const _Nonnull IntercomDidStartNewConversationNotification __attribute__((visibility("default")));
        [Field("IntercomDidStartNewConversationNotification", "__Internal")]
        NSString IntercomDidStartNewConversationNotification { get; }
    }

    // @interface Experimental (Intercom)
    [Category]
    [BaseType(typeof(Intercom))]
    interface Intercom_Experimental
    {
    }
}

StructsEnums

[Native]
public enum ICMPreviewPosition : uint
{
    BottomLeft = 0,
    BottomRight = 1,
    TopLeft = 2,
    TopRight = 3
}

@kmossco
Copy link
Contributor Author

kmossco commented Mar 19, 2018

@michael-watson that's awesome, thank you so much for sharing! We currently don't have any plans for open sourcing those libraries, but I'll reach out to the team anyway and present this message to see if there's anything we can do. In the mean time, we currently have de-scoped working on Xamarin bindings in this library so that we can focus on first bringing it to .NET Standard 2.0 and then ensure feature parity with our REST API features. When that's done, I think we can then look into it for sure. 😉

@kmossco kmossco force-pushed the bv/netstandard2.0 branch 4 times, most recently from 450b210 to f586ecf Compare March 27, 2018 18:06
@kmossco kmossco force-pushed the bv/netstandard2.0 branch from f586ecf to 3b7ab3f Compare March 27, 2018 18:25
last_seen_ip = user.last_seen_ip,
custom_attributes = user.custom_attributes,
last_seen_user_agent = user.user_agent_data,
new_session = user.new_session,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@choran to fix #105 added this here.

return result.Result;
}

public User IncrementUserSession(String id, List<String> companyIds)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we only allowed you to increment a user session passing an ID but that wouldn't increment any company sessions as we need a POST with both the user and company details. This allows that.

ClientResponse<User> result = null;
String body = JsonConvert.SerializeObject(new { id = user.id, new_session = true });
result = Post<User>(body);
result = Post<User>(Transform(user));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By passing a Transform we allow the user to have a new_session attribute and take that automatically as well as other details. Before, even though we were passing a User we were only adding the id and passing a new_session without companies. This also allows updating the session for all those companies in the user object.

@kmossco kmossco force-pushed the bv/netstandard2.0 branch from 1b9e4b5 to 571e209 Compare April 1, 2018 14:41
}

public User IncrementUserSession(User user)
public User IncrementUserSession(String id, List<String> companyIds)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows us to pass in a list of company_ids to increase the session as the same time as the user's. Fixed #105.

Copy link
Member

@choran choran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, after discussion and reviewing these changes we are releasing this as a major version update. So merging this into master now

@choran choran merged commit f6a2875 into master Apr 11, 2018
@kmossco kmossco deleted the bv/netstandard2.0 branch April 11, 2018 13:36
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.

6 participants