-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
add: Message.forward() forward message #727
Conversation
You can directly forward files and multimedia, but limit the file size to 25Mb or less, otherwise it will fail because the server policy.
demo code: const QrcodeTerminal = require('qrcode-terminal')
const finis = require('finis')
/**
* Change `import { ... } from '../'`
* to `import { ... } from 'wechaty'`
* when you are runing with Docker or NPM instead of Git Source.
*/
import {
config,
Wechaty,
log,
MediaMessage,
MsgType,
} from './'
//const roomArr = { }
const welcome = `
| __ __ _ _
| \\ \\ / /__ ___| |__ __ _| |_ _ _
| \\ \\ /\\ / / _ \\/ __| '_ \\ / _\` | __| | | |
| \\ V V / __/ (__| | | | (_| | |_| |_| |
| \\_/\\_/ \\___|\\___|_| |_|\\__,_|\\__|\\__, |
| |___/
=============== Powered by Wechaty ===============
`
console.log(welcome)
const bot = Wechaty.instance({ profile: config.DEFAULT_PROFILE })
const forwards = <string[]>[]
bot
.on('logout' , user => log.info('Bot', `${user.name()} logouted`))
.on('login' , user => {
log.info('Bot', `${user.name()} logined`)
bot.say('Wechaty login')
})
.on('error' , e => {
log.info('Bot', 'error: %s', e)
bot.say('Wechaty error: ' e.message)
})
.on('scan', (url, code) => {
if (!/201|200/.test(String(code))) {
const loginUrl = url.replace(/\/qrcode\//, '/l/')
QrcodeTerminal.generate(loginUrl)
}
console.log(`${url}\n[${code}] Scan QR Code in above url to login: `)
})
.on('message', async m => {
try {
const room = m.room()
const forwardId = room ? m.obj.room as string : m.obj.from as string
const name = room ? room.topic() as string : m.from().name() as string
let doForward = false
console.log((room ? '[' room.topic() ']' : '')
'<' m.from().name() '>'
':' m.toStringDigest(),
)
if (!m.self()) {
if (/^(ding|ping|bing|code)$/i.test(m.toString())) {
doForward = true
m.say('dong')
log.info('Bot', 'REPLY: dong')
await m.say(new MediaMessage(__dirname '/../image/BotQrcode.png'))
await m.say('Scan now, because other Wechaty developers want to talk with you too!\n\n(secret code: wechaty)')
log.info('Bot', 'REPLY: Image')
} else if (/^(forward|f)$/i.test(m.toString())) {
if (forwards.indexOf(forwardId) < 0) {
forwards.push(forwardId)
await m.say(`你为${room ? '群[' name ']' : '自己'}订阅了信息转发,稍后接收到的信息将转发给${room ? '群' : '你'}`)
} else {
await m.say(`${room ? '群[' name ']' : '自己'} 已经订阅了信息转发`)
}
} else if (/^(unforward|unf)$/i.test(m.toString())) {
if (forwards.indexOf(forwardId) >= 0) {
forwards.splice(forwards.indexOf(forwardId), 1)
await m.say(`你为${room ? '群[' name ']' : '自己'}取消了信息转发,稍后接收到的信息将转发给${room ? '群' : '你'}`)
} else {
await m.say(`${room ? '群[' name ']' : '自己'} 已经取消了信息转发`)
}
} else if (/^(list)$/i.test(m.toString())) {
await m.say(`当前订阅列表数量为${forwards.length}\n列表如下:\n${forwards.join(',\n')}`)
} else if (/^(帮助|help|h|使用)$/i.test(m.toString())) {
await m.say(`你可以在任意聊天界面发送指令订阅/取消订阅信息同步\n'forward' 或 'f' 指令订阅,\n'unforward' 或 'unf' 指令取消订阅\n'list'查看订阅列表\n'help'查看帮助`)
} else {
doForward = true
// 有效消息,且当前聊天未参与同步转发
if (m.toString().length > 0 && forwards.indexOf(forwardId) < 0) {
await m.say(`你发的消息内容为: '${m.toString()}'`)
await m.say(`你可以在任意聊天界面发送指令订阅/取消订阅信息同步\n'forward' 或 'f' 指令订阅,\n'unforward' 或 'unf' 指令取消订阅\n'list'查看订阅列表\n'help'查看帮助`)
}
}
//过滤系统信息
if (m.type() === MsgType.SYSNOTICE || m.type() === MsgType.SYS) {
log.info('Bot', '系统信息,不转发')
doForward = false
}
// 过滤 桔小子 bot的 转发信息
if (/(消息来自)/i.test(m.toString())) {
doForward = false
log.info('Bot', '检测到疑似其他Bot信息,不转发')
}
}
if (doForward) {
forwards.forEach(id => {
if (id !== forwardId) { // 避免转发到当前会话
m.forward(id)
} else {
log.info('forwards.forEach()', `id=${id},forwardId=${forwardId}`)
}
});
} else {
log.info('Bot', 'doForward=false')
}
} catch (e) {
log.error('Bot', 'on(message) exception: %s', e)
}
})
bot.init()
.catch(e => {
log.error('Bot', 'init() fail: %s', e)
bot.quit()
process.exit(-1)
})
finis((code, signal) => {
const exitMsg = `Wechaty exit ${code} because of ${signal} `
console.log(exitMsg)
bot.say(exitMsg)
}) |
Thanks for @lijiarui ! parameter support Room/Contact/roomId/ContactId public forward(room: Room): Promise<any>
public forward(contact: Contact): Promise<any>
public forward(id: string): Promise<any>
public forward(sendTo: string|Room|Contact): Promise<any> {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @binsee, your forward
implementation is awesome!
This will help the wechaty user very much when we are dealing with many attachments.
I have some questions and suggestions for your PR, please read my review, and let me know if you have any question/suggestion.
Looking forward to merge your new PR, thanks!
@@ -42,7 42,9 @@ export interface MsgRawObj { | |||
MMActualSender: string, // getUserContact(message.MMActualSender,message.MMPeerUserName).isContact() | |||
MMPeerUserName: string, // message.MsgType == CONF.MSGTYPE_TEXT && message.MMPeerUserName == 'newsapp' | |||
ToUserName: string, | |||
FromUserName: string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MsgRawObj
is the object from the WebWxApp.
Does FromUserName
and Content
exist in the structure?
Just want to confirm that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When MsgType in [1,3,43,47,49],FromUserName and Content exist..
Other unconfirmed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then I think it's enough for us to believe that they always exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
@@ -132,6 134,16 @@ export interface MsgRawObj { | |||
* MsgType == CONF.MSGTYPE_VERIFYMSG | |||
*/ | |||
RecommendInfo?: RecommendInfo, | |||
|
|||
/** | |||
* Transpond Message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do those attributes all exist in the WebWxApp data structure?
Need to confirm them too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exist.
I will fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Let's make the MsgRawObj
reflect and only reflect the WebWxApp data struct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
src/message.ts
Outdated
*/ | ||
public forward(room: Room): Promise<any> | ||
public forward(contact: Contact): Promise<any> | ||
public forward(id: string): Promise<any> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Send to UserId
should not be supported.
The API of Message
class should be abstracted with the Room
and Contact
, but not the UserName
or other id
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
src/message.ts
Outdated
public forward(room: Room): Promise<any> | ||
public forward(contact: Contact): Promise<any> | ||
public forward(id: string): Promise<any> | ||
public forward(sendTo: string|Room|Contact): Promise<any> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should return Promise<boolean>
, according to the forward
function in wechaty-bro.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
src/message.ts
Outdated
public forward(contact: Contact): Promise<any> | ||
public forward(id: string): Promise<any> | ||
public forward(sendTo: string|Room|Contact): Promise<any> { | ||
const m = <MsgRawObj>this.rawObj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why <MsgRawObj>
is needed here?
Does not the this.rawObj
already that type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I remove it, the typescript check will be error.
src/message.ts(645,35): error TS2532: Object is possibly 'undefined'.
src/message.ts(645,61): error TS2532: Object is possibly 'undefined'.
src/message.ts(646,28): error TS2532: Object is possibly 'undefined'.
src/message.ts(648,43): error TS2532: Object is possibly 'undefined'.
src/message.ts(650,9): error TS2532: Object is possibly 'undefined'.
src/message.ts(651,30): error TS2532: Object is possibly 'undefined'.
src/message.ts(652,9): error TS2532: Object is possibly 'undefined'.
src/message.ts(653,25): error TS2532: Object is possibly 'undefined'.
src/message.ts(654,9): error TS2532: Object is possibly 'undefined'.
src/message.ts(655,50): error TS2532: Object is possibly 'undefined'.
src/message.ts(658,16): error TS2345: Argument of type 'MsgRawObj | undefined' is not assignable to parameter of type 'MsgRawObj'.
Type 'undefined' is not assignable to type 'MsgRawObj'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is because this.rawObj
might be undefined
.
So you need to check undefined
instead of force it to be MsgRawObj
.
Add the follow code at the begining of the function, then you will be able to use this.rawObj
because that will make sure it is not undefined
.
if (!this.rawObj) {
throw new Error('no rawObj!')
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
@@ -422,6 423,20 @@ export class Bridge { | |||
}) | |||
} | |||
|
|||
public forward(baseData: MsgRawObj, patchData: MsgRawObj): Promise<boolean> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I asked before: could we just use forward(rawObj: MsgRawObj)
at here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After thinking, I can only do this design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I understood after reading the code you provided. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
src/puppet-web/bridge.ts
Outdated
if (!patchData.MMActualContent && !patchData.MMSendContent) { | ||
throw new Error('cannot say nothing') | ||
} | ||
return this.proxyWechaty('forward', baseData, patchData) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also need to merge two args to one at here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
@@ -490,6 491,26 @@ export class PuppetWeb extends Puppet { | |||
return ret | |||
} | |||
|
|||
public async forward(baseData: MsgRawObj, patchData: MsgRawObj): Promise<boolean> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forwrd(rawObj: MsgRawObj)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
@@ -505,6 505,32 @@ | |||
return true | |||
} | |||
|
|||
function forward(baseData, patchData) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One parameter please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
@@ -54,6 55,7 @@ export abstract class Puppet extends EventEmitter implements Sayable { | |||
public abstract self(): Contact | |||
|
|||
public abstract send(message: Message | MediaMessage): Promise<boolean> | |||
public abstract forward(baseData: MsgRawObj, patchData: MsgRawObj): Promise<boolean> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
About
angular.module("Controllers").controller("transpondDialogController", ["$rootScope", "$scope", "$timeout", "$state", "$log", "$document", "chatFactory", "contactFactory", "appFactory", "chatroomFactory", "confFactory", "mmpop", "ngDialog", "utilFactory", "stateManageService", "accountFactory",
function(e, t, a, n, i, o, chatFactory, c, s, l, confFactory, f, u, m, g, accountFactory) {
function h(msg, userName) {
if (e.MsgType != confFactory.MSGTYPE_SYS) {
var newMsg = angular.copy(msg);
newMsg.ToUserName = userName;
newMsg.FromUserName = accountFactory.getUserName();
newMsg.isTranspond = !0;
newMsg.MsgIdBeforeTranspond = msg.MsgIdBeforeTranspond || msg.MsgId;
newMsg._h = void 0;
newMsg._offsetTop = void 0;
newMsg.MMSourceMsgId = msg.MsgId;
newMsg.Scene = 2;
newMsg = chatFactory.createMessage(newMsg); // call createMessage()
// The following parameters need to be overridden after calling createMessage()
newMsg.sendByLocal = !1;
newMsg.Content = m.htmlDecode(newMsg.Content.replace(/^@\w :<br\/>/, ""));
newMsg.MMActualSender = accountFactory.getUserName();
newMsg.MMSendContent && (newMsg.MMSendContent = newMsg.MMSendContent.replace(/^@\w :\s/, ""));
newMsg.MMDigest && (newMsg.MMDigest = newMsg.MMDigest.replace(/^@\w :/, ""));
newMsg.MMActualContent && (newMsg.MMActualContent = m.clearHtmlStr(newMsg.MMActualContent.replace(/^@\w :<br\/>/, "")));
// append and send message
chatFactory.appendMessage(newMsg);
chatFactory.sendMessage(newMsg)
}
}
createMessage: function(e) { // 创建消息体
switch (e.FromUserName || (e.FromUserName = accountFactory.getUserName()),
e.ToUserName || (e.ToUserName = this.getCurrentUserName()), //
e.ClientMsgId = e.LocalID = e.MsgId = (utilFactory.now() Math.random().toFixed(3)).replace(".", ""), //生成本地消息id
e.CreateTime = Math.round(utilFactory.now() / 1e3),
e.MMStatus = confFactory.MSG_SEND_STATUS_READY, //标记消息的状态为准备状态
e.sendByLocal = !0, // 是否是本地信息,如果需要转发文件,必须为 false
e.MsgType) { //判断消息类型
case confFactory.MSGTYPE_TEXT:
var t = [];
e.Content = e.Content.replace(/<input.*?un="(.*?)".*?value="(.*?)".*?>/g,
function(e, a, n) {
return t.push(a), n
}),
e.MMAtContacts = t.join(","),
e.MMSendContent = utilFactory.htmlDecode(utilFactory.clearHtmlStr(e.Content.replace(/<(?:img|IMG).*?text="(.*?)".*?>/g,
function(e, t) {
return t.replace(confFactory.MM_EMOTICON_WEB, "")
}).replace(/<(?:br|BR)\/?>/g, "\n"))).replace(/<(.*?)>/g, function(e) {
return emojiFactory.EmojiCodeMap[emojiFactory.QQFaceMap[e]] || e
}),
e.Content = e.Content.replace(/<(?!(img|IMG|br|BR))[^>]*>/g, "").replace(/\n/g, "<br>");
break;
case confFactory.MSGTYPE_APP:
if (e.AppMsgType == confFactory.APPMSGTYPE_URL) break;
e.AppMsgType = confFactory.APPMSGTYPE_ATTACH,
e.Content = "<msg><appmsg appid='wxeb7ec651dd0aefa9' sdkver=''><title>" e.FileName "</title><des></des><action></action><type>" confFactory.APPMSGTYPE_ATTACH "</type><content></content><url></url><lowurl></lowurl><appattach><totallen>" e.FileSize "</totallen><attachid>#MediaId#</attachid><fileext>" (e.MMFileExt || e.MMAppMsgFileExt) "</fileext></appattach><extinfo></extinfo></appmsg></msg>"
}
return e
},
function forward(baseData, patchData) {
var chatFactory = WechatyBro.glue.chatFactory
var confFactory = WechatyBro.glue.confFactory
if (!chatFactory || !confFactory) {
log('forward() chatFactory or confFactory not exist.')
return false
}
try {
var m = chatFactory.createMessage(baseData)
// Need to override the parametes after called createMessage()
m = Object.assign(m, patchData)
chatFactory.appendMessage(m)
chatFactory.sendMessage(m)
} catch (e) {
log('forward() exception: ' e.message)
return false
}
return true
} |
In addition, some notes are added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for update the PR and let me know the pattern of WebWxApp of forwarding messages.
This PR will LGTM after you follow my reviews and mark all of them to be resolved
.
src/message.ts
Outdated
*/ | ||
public forward(room: Room): Promise<boolean> | ||
public forward(contact: Contact): Promise<boolean> | ||
public forward(id: string): Promise<boolean> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Send to UserId
should not be supported.
The API of Message
class should be abstracted with the Room
and Contact
, but not the UserName
or other id
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
// The following parameters need to be overridden after calling createMessage() | ||
|
||
// If you want to forward the file, would like to skip the duplicate upload, sendByLocal must be false. | ||
// But need to pay attention to file.size> 25Mb, due to the server policy restrictions, need to re-upload |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we reuse the MediaId
instead of re-upload it?
Because some projects use the MediaId
directly, which means it might be valid across sessions and accounts.
See: #422 (comment)
I think the solution should be not to upload the same media file twice by caching the MediaId of the Uploaded File and reuse it in the future.
There also has a very interesting project that we could have a look: 微信表情轰炸器
It can send emoticon by MediaId directly, no need to upload anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The project 微信表情轰炸器 is worth having a look! :)
It only stores the MediaId, and reuse it for sending the emoticons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By using fiddler to intercept data, testing found files larger than 25mb in webwx can not use mediaId for multiplexing, the server will return processing failure.
I think this problem needs to be shelved and later resolved. Need to refactor the upload file flow.
"BaseResponse": {
"Ret": 1,
"ErrMsg": ""
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
src/message.ts
Outdated
newMsg.MMActualContent = UtilLib.stripHtml(m.MMActualContent.replace(/^@\w :<br\/>/, '')) | ||
|
||
return config.puppetInstance() | ||
.forward(m, newMsg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, let's keep the patch data design because it's the pattern from WebWxApp.
@@ -422,6 423,20 @@ export class Bridge { | |||
}) | |||
} | |||
|
|||
public forward(baseData: MsgRawObj, patchData: MsgRawObj): Promise<boolean> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I understood after reading the code you provided. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update.
I'll approve it after you follow my latest reviews.
Nice job!
src/message.ts
Outdated
public forward(contact: Contact): Promise<any> | ||
public forward(id: string): Promise<any> | ||
public forward(sendTo: string|Room|Contact): Promise<any> { | ||
const m = <MsgRawObj>this.rawObj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is because this.rawObj
might be undefined
.
So you need to check undefined
instead of force it to be MsgRawObj
.
Add the follow code at the begining of the function, then you will be able to use this.rawObj
because that will make sure it is not undefined
.
if (!this.rawObj) {
throw new Error('no rawObj!')
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the outdated reviews for the MsgRawObj
undefined
problem.
src/message.ts
Outdated
public forward(room: Room): Promise<boolean> | ||
public forward(contact: Contact): Promise<boolean> | ||
public forward(sendTo: Room|Contact): Promise<boolean> { | ||
const m = <MsgRawObj>this.rawObj |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is because this.rawObj
might be undefined
.
So you need to check undefined
instead of force it to be MsgRawObj
.
Add the follow code at the begining of the function, then you will be able to use this.rawObj
because that will make sure it is not undefined
.
if (!this.rawObj) {
throw new Error('no rawObj!')
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
fix: When forwarding a message within a group, the content is prefixed with @userid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your great job!
LGTM now.
I'll merge this PR after it getting total three approvements, and I'd like to tag a new version of v0.9
after we confirmed the code can work without any problem.
Looking forward to the new version, Cheers!
// The following parameters need to be overridden after calling createMessage() | ||
|
||
// If you want to forward the file, would like to skip the duplicate upload, sendByLocal must be false. | ||
// But need to pay attention to file.size> 25Mb, due to the server policy restrictions, need to re-upload |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The project 微信表情轰炸器 is worth having a look! :)
It only stores the MediaId, and reuse it for sending the emoticons.
``` m.forward(Room.load(id), { user: '$USER', room: 'Room[$ROOM]', custom: 'forward from $ROOM$USER' }) // source message: 'test msg' // from room: 'testRoom' // from user: 'Bot' // forward message: `forward from Room[testRoom]Bot:\ntest msg` ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the forward
methods should be moved from the class Message
to MediaMessage
.
src/message.ts
Outdated
*/ | ||
public forward(room: Room, option?: ForwardOption): Promise<boolean> | ||
public forward(contact: Contact, option?: ForwardOption): Promise<boolean> | ||
public forward(sendTo: Room|Contact, option?: ForwardOption): Promise<boolean> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the forward
methods should be moved from the class Message
to MediaMessage
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your opinion I agree, i will move it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think again.
Initially I put the message in the message, hoping to achieve through a single function of multi-group live.
In the multi-group live operation, text, voice, pictures are more types of messages.
And only use forward () this method, you can easily achieve a variety of message types of forwarding.
So, will forward () be retained in Message or maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comment about this at: #726 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
In room msg, the content prefix sender:, need to be removed, otherwise the forwarded sender will display the source message sender, causing self () to determine the error
src/message.ts
Outdated
newMsg.sendByLocal = false | ||
newMsg.MMActualSender = config.puppetInstance().userId || '' | ||
if (m.MMSendContent) | ||
newMsg.MMSendContent = m.MMSendContent.replace(/^@\w :\s/, '') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please always use {
and }
around the code block inside if
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved
@binsee Could you please mark all the reviews from me as Because I'm ok with all of them. After marked as Thanks! |
Thanks for the reply the "resolved" to my reviews. But I remember that there has a [resolved] button under each review, which you could click that button to make the review as |
@zixia Sorry,I can't find it. |
Changes Unknown when pulling 0a76682 on binsee:add-transpond-msg into ** on Chatie:master**. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me, Thanks!
@hczhcz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This Message.forward implementation is great!
Please run jsdoc (npm run doc
) so that the new API will take effect in the documentation.
@hczhcz Or I can submit a new development branch that invites more than three members to review. Please tell me which way to choose? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Please go ahead. It is not necessary to create another PR, just take care when merging. (Since the code has changed much when this PR is pending, there probably be a lot of things to do in the merging process...) Anyway, do not worry about that :D Thank you for your contribution!
Great! Thanks all for the time of reviewing the code, appreciate it! |
You can directly forward files and multimedia, but limit the file size to 25Mb or less, otherwise it will fail because the server policy.
Issues: #726