Proposal: add a `handle` field to RoomPayload

hcfw007 commented 2 years ago

Updated by @huan

A Twitter handle is the username that appears at the end of your unique Twitter URL.

Related to:

Orignal

current RoomPayload interface: https://github.com/wechaty/puppet/blob/387db132491e819cfc6b23060917d0a99a8eb530/src/schemas/room.ts#L12-L21

my proposal:

export interface RoomPayload {
  id : string,

  topic        : string,
  avatar?      : string,
  memberIdList : string[],
  ownerId?     : string,
  adminIdList  : string[],
  external?    : boolean,
  chatId?      : string
}

reason: in wecom and wahtsapp (the puppets we currently working on), for each room there is an internal id that can be used as a key metadata or connect with other apps).

to take this to the next step, i'd also that wechaty could support some additional data for almost all payloads. Different IMs or even same IM but different puppet could provide different information for the same Contact, Room or Message. This kind of messages are usually just plain string and No need to create a specific field to do so.

e.g. current ContactPayload interface: https://github.com/wechaty/puppet/blob/387db132491e819cfc6b23060917d0a99a8eb530/src/schemas/contact.ts#L24-L45

my proposal:

export interface ContactPayload {
  id     : string,
  gender : ContactGender,
  type   : ContactType,
  // ... some other info
  additionalInfo?: object // could contain wexin and coworker. could also be string considering making grpc proto easier?.
}

reason: I put wexin and coworker into additionalInfo is these kinds of information all not universal. Coworker is only for wecom, and people might have many IM contact info instead of just weixin, we cannot put everything in it. Otherwise why don't we also add twittwer, facebook and linkedIn? Also this will allow puppet to pass some IM or puppet specific info to the user and make wechaty much more flexible.

With the consideration of downward compatibility, maybe we should only add this additionalInfo field and leave properties like weixin and coworkers there.

huan commented 2 years ago

I believe the id in room payload is what the chatId concept you mentioned?

Or could you please explain why a new chatId is necessary?

hcfw007 commented 2 years ago

Because a Room in wecom has two Ids, one is more like an internal id, when we send message via this room, we use internal id. The other one is more like an external id, when we tell other wecom contacts about this room, we use this external id. This id is also usable when calling some wecom official API.

huan commented 2 years ago

Could you please provide some examples of those Ids (the more the better) because I'm very interested in this design?

Thank you very much.

hcfw007 commented 2 years ago

Sure thing. example rooms:

externalId: "wr3OKqCQAAeyB3Wpnt82hK1A7YU3pYsA" internalId: "R:10696049441040201"
externalId: "wr3OKqCQAArDbMXXladDMe0XJfuFaO5w" internalId: "R:10696049576015462"
externalId: "wr3OKqCQAAG-dbqXHGFgyIQ09DrhFKuw" internalId: "R:10696049516028131"
externalId: "wr3OKqCQAAbYlfdm5GnOF_NXUhWmWE0Q" internalId: "R:10696049489037470"

example usages:

get customer room detail

huan commented 2 years ago

Could you please provide one example of each of the usage of externalId and internalId?

I hope we can be able to only keep one id as the primary id, and ignore the other one?

For example, it will be very easy for the internal implementation to convert one id to the other, because one primary id will be enough (and it should) for the whole Wechaty system.

hcfw007 commented 2 years ago

Happy to. Take the first example room: externalId: "wr3OKqCQAAeyB3Wpnt82hK1A7YU3pYsA" internalId: R:10696049441040201". If we want to send a message in this room, we call API.call(ApiType.SEND_TEXT, { conversation_id: R:10696049441040201, content: 'hello' }). If we want to sync the room detail by calling wecom official API, we post https://qyapi.weixin.qq.com/cgi-bin/externalcontact/groupchat/get?access_token=ACCESS_TOKEN with

{
  "chat_id":"wr3OKqCQAAeyB3Wpnt82hK1A7YU3pYsA",
  "need_name" : 1
}

huan commented 2 years ago

From this use case, it seems that using one primary id for both internal/external is possible: we can use an id mapper to map externalId to internalId by maintaining map storage internally.

Please let me know the reason if you think this does not work for your case.

hcfw007 commented 2 years ago

The problem is, there a room always have an internalId, we can get it from the wecom client. But externalId needs to be retrieved by a remote call, and it could fail. In this case the puppet can still work and we can retry sometime later. So the main ID must be the internalId. However the bot also needs to know the externalID when available because the bot need to call wecom official API.

hcfw007 commented 2 years ago

The internalId is unique among all wecom rooms, and externalId is only unique inside the enterprise. Actually the we, the bot, need both Ids to map rooms, especially when an company has multiple enterprise entities in wecom and wants to manage rooms in one terminal.

huan commented 2 years ago

Thanks for the explanation.

However it seems a little bit complicated, could you please draft a diagram to show the reason that we can not maintain one id for them?

I still think what you have describe should be encapsulate internally, and map one primary id externally

hcfw007 commented 2 years ago

Take Room2 as example. Reason bot need internalId: Account A-1-2 and Account A-2-1 belong to different wecom enterprise entities, so Room2's externalId (e.g. wr3OKqCQAAeyB3Wpnt82hK1A7YU3pYsA) is different to them. So the bot need to know the internalId (R: 10696049441040201) to know that Room2 in Account A-1-2 is exactly the one in Account A-2-1. Reason bot need externalId: Because tencent wecom API requries this externalId, so the bot needs it.

hcfw007 commented 2 years ago

Bottom line: we need both IDs. P.S. I really like the additionalInfo idea because it is very possible that each puppet has some IM-specific data. Do you mind give that a look.

huan commented 2 years ago

Thanks for the diagram, it's very clear.

I still think it is possible to use internal id as the primary key because obviously the puppet wxwork can map it to the external id, doesn't it?

hcfw007 commented 2 years ago

Yes, it is. But the bot side need to use both of them. We currently use DingDong to implement a getChatId method. If we maintain a map in puppet-wxwork, how can we get the other Id in bot?

huan commented 2 years ago

Cloud you please explain that why do you need to get the other Id in bot?

I think the best practice is to use one id in bot, and use it everywhere.

hcfw007 commented 2 years ago

As I explained before, tencent wecom API requries externalId, and we (as Juzi, not just bot) need internalId to decide the whether two rooms is one room or not (externalId could be different when calling from two different accounts). I understand the need for internalId is not in the bot level, but we do need a way to get it. If Room can provide a getChatId() method that's also fine with me.

huan commented 2 years ago

I understand the need for internalId is not in the bot leve

That's exactly my point, thank you very much for pointing it out.

So we confirmed that from the wechaty perspective, we can use the internal id as the primary key in the wechaty payload, and use it across the bots because they are the same across different accounts.

For your use case, I think what you need is a function with externalId = f(internalId, botId).

This can be implemented by the wechaty ding/dong protocol, or a external RESTful API build outside of the Wechaty system would be more suitable for this feature request.

hcfw007 commented 2 years ago

OK. Indeed add such a wxwork-specific id is not natural in this IM-irrelevant puppet. Can you consider my additionalInfo proposal? It would make puppet much more flexible and it is very normal for IMs to have some IM-specific info.

windmemory commented 2 years ago

I believe Wechaty is not only about bot itself, but also CONNECTING BOT WITH ALL OTHER SYSTEM, like OSSChat connecting Github and WeChat, FridayBot connecting every IMs supported by Wechaty. So we might need to think about something other than bot itself.

I do agree we only need one id as the unique identifier for every entities within Wechaty, and we should use it everywhere. However, sometimes we need another id to map the bot world with the other world. For example, we have weixin in ContactPayload originally to map the contact with the realwrold WeChat contact, since we can only see the weixin in WeChat app. And now we want to connect the Wechaty bot world with WeCom API world, so we would like to have another value to establish the connection, this value will never be the unique identifier in Wechaty, but this value is the key to connect.

So I would like to reach a consensus first that connecting Wechaty bot with other system is what Wechaty supported. This is very important becuase this is the essential goal that we are trying to achieve.

Let me know what do you think about this @huan

huan commented 2 years ago

The wexin name in the payload looks only related to WeChat, however, it can be generalized.

The weixin in the payload, is more like the username of GitHub, the handle(username) of Twitter, and others, which can be customized by the user and it's a unique name; on the other hand, the id, which is a more unique data string from the programming perspective.

connecting Wechaty bot with other systems is what Wechaty supported. This is very important because this is the essential goal that we are trying to achieve.

I definitely agree with the above statement. At the same time, I also want to design the API by following the Rule of Least Power, and KISS.

If we DO need a solution to map the room.id to another id in our use case, it would be better if we can list all the potential solutions and I'd like to continue discussing based on that list, and try to improve our system to meet the best suitable solution.

windmemory commented 2 years ago

The weixin in the payload, is more like the username of GitHub, the handle(username) of Twitter, and others, which can be customized by the user and it's a unique name; on the other hand, the id, which is a more unique data string from the programming perspective.

I agree that this should be the way that we use weixin in Wechaty. And we might change the name to a more accurate one in the future for this value. And I think we should try to come up with another weixin for the Room entity in Wechaty.

So the first solution might be a single value for an extra field in RoomPayload .

Solution 1: Room Code

Let's add another field with name like roomCode. We can use this as an additional identifier that could be displayed in GUI, or some other ids that can be used to connect other services Pros:

easy to understand
easy to implement Cons:
only resolve the id problem

Solution 2: additional info

This is a more generalized solution, that we could make up an object that contains values. The schema might be different from different IMs. But this solution gives the most flexibility to different future puppets. Pros:

most flexible solution Cons:
not strongly typed

Solution 3: another method to do the conversion

We could add a method like the function metioned above to do the id conversion, like externalId = f(internalId, botId). Pros:

no extra field added Cons:
hard to understand
require extra action to get the id, which will increase the complexity of the code

huan commented 2 years ago

Thanks for the analyzing and the three solutions with pro/con s

For the weixin I think a better name for it should be handle because it's what Twitter is using, and it will not be confusing with the others like xxx_name.

How do you think about this name? I think if it's a good name maybe we can generalize it for the room payload ad well.

hcfw007 commented 2 years ago

I like solution 2, and we can force this additional info to be string, because it should not be used to transfer binary data, and it can be very flexible with JSON data. And since it's 'additional', semantically it indicates that it's not critical for wechaty core functions, so that bot makers should handle it with additional code. And it is quite normal for every IM to have some unique features that cannot be categorised into any existing fields.

However I do understand that avoiding adding fields which are not closely related to chatbot. So I'm OK with this handle solution too.

windmemory commented 2 years ago

Thanks for the analyzing and the three solutions with pro/con s

For the weixin I think a better name for it should be handle because it's what Twitter is using, and it will not be confusing with the others like xxx_name.

How do you think about this name? I think if it's a good name maybe we can generalize it for the room payload ad well.

To be honest, I don't like the name handle, because this reminds me the windows system API, where everything in the UI has its handle. But I do like the concept of handle.

Since currently I don't have a better name for this, I will vote for handle for now.

huan commented 2 years ago

@windmemory great to know that you like the concept of the handle! I like it too because it seems is an missing part of the Wechaty API. And we can forget the win32 API for now and look forward: Twitter use it everywhere in products, docs, and blogs.

@hcfw007 thanks for sharing your thoughts and votes. However, I think the additionalInfo will not be a option because it lacks the semantic meaning of the data.

So I think we can go ahead with the handle design.

@windmemory please feel free to draft an PR to add this new feature as your need, or you can wait me to working on it after my finishing the CQRS module.

hcfw007 commented 2 years ago

Alright. Should we also add handle for Contact class?

huan commented 2 years ago

Yes, I think we should have two more APIs:

contact.handle()
room.handle()

and we should rename the weixin in contactPayload to handle, and add a new handle to the room payload to support the new API.

hcfw007 commented 2 years ago

Indeed, there are quite some fields with Wechat mark. However if we wish to deprecate them all that would be a huge impact.

huan commented 2 years ago

Yes, so we should not remove them before v2.0.

The weixin and the handle both will be set in the following codes.

hcfw007 commented 2 years ago

I have opened a PR(https://github.com/wechaty/grpc/pull/173) for this about it in wechaty-grpc with only features you add in https://github.com/wechaty/wechaty/pull/2381.

I'll add search options for room and contact if you agree with me on https://github.com/wechaty/puppet/pull/188#issuecomment-1066750392

hcfw007 commented 2 years ago

I think room handle payload setter and getter should also be added to puppet-service besides your PR (https://github.com/wechaty/puppet-service/pull/198)

hcfw007 commented 2 years ago

I think room handle payload setter and getter should also be added to puppet-service besides your PR (wechaty/puppet-service#198)

https://github.com/wechaty/puppet-service/pull/205

huan commented 2 years ago

Please help the community to close this issue if it has been all done, thanks!

hcfw007 commented 2 years ago

Please help the community to close this issue if it has been all done, thanks!

I have just tested it and got the room handle successfully. Issue closed, cheers!

wechaty / puppet