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

【Proposal】New 'room-invite' event #1492

Closed
windmemory opened this issue Jul 19, 2018 · 19 comments
Closed

【Proposal】New 'room-invite' event #1492

windmemory opened this issue Jul 19, 2018 · 19 comments

Comments

@windmemory
Copy link
Member

Use case description

A friend send a room invitation to the user, the user needs to click on the card, open up a page, then click the button on the page to join the room.

Design proposal

We could add one room-invite event emitted when we receive the invitation from a user. It might looks like this:

public emit(event: 'room-invite', roomInvitation: RoomInvitation): boolean

And here is the definition of RoomInvitation:

export interface RoomInvitation {
  roomName: string,
  fromUser: string,
  timestamp: number,
  accept: () => Promise<boolean>
}

The reason that I wrapped several arguments into one object is that accepting the invitation is a complicate operation, I would like to keep this logic away from our developers, so provide a function to them so when they want to accept the invitation, just do invite.accept(), then they will be added into the room.

Request for comments, let me know if this works.

@windmemory
Copy link
Member Author

@zixia What do you think about this proposal? If you think this is good to go, I will try to submit a PR later.

@huan
Copy link
Member

huan commented Jul 19, 2018

Thanks for your RFC on the room-invite event, I think it is good to go.

Please submit PR and we can discuss the details based on that.

@lijiarui
Copy link
Member

I have another suggestion. I suggest adding a new Message type RoomInvitaion in MsgType

.on('message', function (message) {
  if (message.type() === bot.Message.Type.RoomInvitation) {
      message.accept()
  }
})

@zixia How do you think about this

@huan
Copy link
Member

huan commented Jul 19, 2018

@lijiarui Please discuss it with @windmemory, and put what you both are thinking right to your RFC.

@lijiarui
Copy link
Member

I listen to @windmemory

@windmemory
Copy link
Member Author

Okay, after a fierce discussion on this issue, we decide to use room-invite event. :)

Will submit PR later.

@huan
Copy link
Member

huan commented Jul 22, 2018

@windmemory Thank you very much for your workable prototype with the c9 runnable environment.

After I study your code, I had made some changes on both wechaty-puppet in your PR, and the wechaty code base so that it can support the room-invite event.

Please update your dependency to wechaty@0.19.101 and wechaty-puppet@0.9.3 or above; then you will be able to continue implementing the room-invite in wechaty-puppet-padchat.

Please feel free to let me know if you have any problems with the integrations when you continue developing.

@windmemory
Copy link
Member Author

@zixia Thanks for adding related changes in wechaty and wechaty-puppet.

I noticed that you declare a PuppetRoomInviteEvent in wechaty-puppet with following format:

export interface PuppetRoomInviteEvent {
  inviterId : string,
  roomId    : string,
}

I would like to point out that, when an invitation is sent along, there is no roomId in the payload, here is a sample payload:

{
  "msg": {
    "appmsg": {
      "appid": "",
      "sdkver": "0",
      "title": "邀请你加入群聊",
      "des": "\"高原ོ\"邀请你加入群聊Wechaty Developers' Home 2,进入可查看详情。",
      "action": "view",
      "type": "5",
      "showtype": "0",
      "soundtype": "0",
      "mediatagname": {},
      "messageext": {},
      "messageaction": {},
      "content": {},
      "contentattr": "0",
      "url": "http://support.weixin.qq.com/cgi-bin/mmsupport-bin/addchatroombyinvite?ticket=AVs3velIDxTkA0EOhKogxg%3D%3D",
      "lowurl": {},
      "dataurl": {},
      "lowdataurl": {},
      "appattach": {
        "totallen": "0",
        "attachid": {},
        "emoticonmd5": {},
        "fileext": {},
        "cdnthumbaeskey": {},
        "aeskey": {}
      },
      "extinfo": {},
      "sourceusername": {},
      "sourcedisplayname": {},
      "thumburl": "http://weixin.qq.com/cgi-bin/getheadimg?username=42e4fc96ae9472a2063bbabbe1cfbddbe3b59b105d1d59c217099cb8f3441f95",
      "md5": {},
      "statextstr": {}
    },
    "fromusername": "lylezhuifeng",
    "scene": "0",
    "appinfo": {
      "version": "1",
      "appname": {}
    },
    "commenturl": {}
  }
}

So how should I deal with the roomId here? If I generate a random id, seems not acceptable, but I can not return null or undefined here, maybe an empty string?

@huan
Copy link
Member

huan commented Jul 23, 2018

About the room id, we need to get it before emit the event.

I don't know if the protocol could do that but it's worth to try first.

If it couldn't, then we will have to change the design.

@windmemory
Copy link
Member Author

Unfortunately, they are not the same for this case. I saw in friendship event, it is using fromusername as the contactId, that is completely reasonable for friendship event, because the one who send out the request is the one that we are going to be added.

However, for room-invite event, the fromusername is the person who send out the invitation, not the room. There is no roomId here. The only thing we have here is the roomName, which can be parsed from the des field. But I don't think we want to return roomName as the roomId to developer, that would cause confusion to them.

@windmemory
Copy link
Member Author

From the raw message that we received from protocol, there is no roomId information, only roomName is available. Maybe we could try to get the roomId with the roomName, but that might not work either, since roomName is not unique.

@windmemory
Copy link
Member Author

And actually I think we don't need the roomId before we emit the event, I saw in the interface of wechaty-puppet , event room-invite has a callback with roomInvitationId as argument. And the InvitationId could be the msgId, so we don't actually need the roomId before we emit the event.

@huan
Copy link
Member

huan commented Jul 23, 2018

Ok, if we do not have roomId with the invitation, it's ok. I'll remove it form payload later.

@windmemory
Copy link
Member Author

I would suggest to have a roomName in the payload, so our developers would be able to do something with the name, like filter the room name to decide whether they want their bot to join.

huan added a commit to wechaty/puppet that referenced this issue Jul 23, 2018
@huan
Copy link
Member

huan commented Jul 23, 2018

Now you got roomTopic as your request.

I had also added two more to the payload:

  1. roomMemberCount, total member number of the room, which should be able to extract from the invitation;
  2. roomMemberIdList, the Wechat will show whether you have friends in this room; if so, the invitation will show the names of your friends, which can be translated to the contact id by Contact.find(). If you have any problem of getting the contact ids, you can just set it to an empty array []
export interface RoomInvitationPayload {
  id               : string,
  inviterId        : string,
  roomTopic        : string,
  roomMemberCount  : number,
  roomMemberIdList : string[],   // Friends' Contact Id List of the Room
  timestamp        : number,     // Unix Timestamp, in seconds
}

@huan
Copy link
Member

huan commented Jul 23, 2018

I had refactored the code which get rid of the depending on the room id, and published them as wechaty-puppet@0.9.19 & wechaty@0.19.107.

Please upgrade and feel free to let me know if there have any problems with the implementation.

Hope we can use the room-invite soon!

@windmemory
Copy link
Member Author

@zixia Thanks for refactor the code in wechaty-puppet and wechaty. I think the feature will be available later today.

The way we accept the invitation in iPad protocol is slightly different from the way we are doing on our phone, so there is no roomMemberCount or roomMemberIdList in the payload. We are not opening the invitation like we do on phone, but use that url to get another url with the iPad protocol api. So currently I will return back empty value for roomMemberCount and roomMemberIdList.

@huan
Copy link
Member

huan commented Jul 24, 2018

Awesome, looking forward to using this new feature!

@windmemory
Copy link
Member Author

New version of wechaty-puppet-padchat has implemented the room-invite event.
Please use version 0.9.0 or above to use this feature.

Close this ticket. If anything wrong happened with the new event, please file issue here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants