wechaty / grpc

gRPC Service & Protocol Buffers for Wechaty Puppet
https://wechaty.github.io/grpc/
Apache License 2.0
26 stars 22 forks source link

Feat/conversation read wechaty #188

Closed hcfw007 closed 1 year ago

hcfw007 commented 1 year ago

支持会话已读功能 https://github.com/wechaty/grpc/issues/189

huan commented 1 year ago

https://github.com/wechaty/puppet/blob/c70a9b47a49170e94da7c06c4ead2348dc6dd88f/src/mixins/message-mixin.ts#L52-L57

The above code is two functions actually (overloaded):

    conversationReadMark (conversationId: string) : Promise<boolean>
    conversationReadMark (conversationId: string, hasRead: boolean) : Promise<void>

The first is the getter, and the second is the setter.

I forget how we deal with the similar case before, can you help us to link 1-2 examples before?

hcfw007 commented 1 year ago

https://github.com/wechaty/puppet/blob/c70a9b47a49170e94da7c06c4ead2348dc6dd88f/src/mixins/message-mixin.ts#L52-L57

The above code is two functions actually (overloaded):

    conversationReadMark (conversationId: string) : Promise<boolean>
    conversationReadMark (conversationId: string, hasRead: boolean) : Promise<void>

The first is the getter, and the second is the setter.

I forget how we deal with the similar case before, can you help us to link 1-2 examples before?

This will work because all fields in proto3 are optional.

Example: https://github.com/wechaty/grpc/blob/8d71b1750f32c9e78574f4264d8d79ada8a4a062/proto/wechaty/puppet/contact.proto#L78-L88

However it is not necessary for most Conatc and Room methods to be both setter and getter because we usually get them from contact / room payload. (This ia a different problem)

huan commented 1 year ago

Great news! We can remote lots of DEPRECATED definitions in our gRPC protocol because comments said DEPRECATED, will be removed after Dec 31, 2022!

hcfw007 commented 1 year ago

@huan this PR is ready to be reviewed

hcfw007 commented 1 year ago

I have dealed with this build error in the forked version before. I'll open another PR to fix it. (That as I recalled is due to some version issue)

huan commented 1 year ago

Thanks! Then let's merge the fix CI one before merging this one.