SkygearIO / chat

Skygear Plugin - Chat SDK
Apache License 2.0
17 stars 18 forks source link

Admins params doesn't work in createConversation api #192

Closed carmenlau closed 6 years ago

carmenlau commented 6 years ago

Current situation

plugin

Accepts admin_ids but expects a list of admin object

iOS SDK

Accepts list of admins id, but have not passed it to lambda

 (void)createConversationWithParticipantIDs:(NSArray<NSString *> *)participantIDs
                                       title:(NSString *)title
                                    metadata:(NSDictionary<NSString *, id> *)metadata
                                    adminIDs:(NSArray<NSString *> *)adminIDs
                      distinctByParticipants:(BOOL)distinctByParticipants
                                  completion:(SKYChatConversationCompletion)completion

Android SDK

Accepts free options Map, in guides teach developer add list of string users ids to options admin_ids

JS SDK

Accepts free options object in object, in guides teach developer add list of users object to options admins

Changes needed

plugin

admin_ids should be camelcase like distinctByParticipants. adminIDs should accept list of ids not list of user objects. Both id format <uuid> or user/<uuid> should be allow

iOS SDK

Fix to pass adminIDs to lambda correctly

Android SDK

Update admin_ids to adminIDs

JS SDK

Update doc and guides to accept list of ids for both participants and admins (to consistent with ios and android). SDK will accept both list of objects or ids, it will convert objects to ids before sending to server. Options key become admins

chpapa commented 6 years ago

@carmenlau to avoid confusion, shall our sdk just accept user ID in static type language, and user ID or user object in JS?

I think currently we don't have a standard of accept object or ID, so it is kind of confusing across different APIs.

chpapa commented 6 years ago

Or shall we accept both object and ID in all SDK?

carmenlau commented 6 years ago

@carmenlau to avoid confusion, shall our sdk just accept user ID in static type language, and user ID or user object in JS?

Yes, agree that ids for static type and ids + objects in js can help avoid confusion. In my change suggestion, ios and android will only accept adminIDs as list of id. For JS, SDK will accept both list of user ids or objects. But keeping 2 keys admins and adminIDs in the options will cause another kinds of confusion? So maybe we should only use adminIDs or admins? and it will accept both objects and ids.