improve messageHandlerRegistry.ts #85
Conversation
| bot: TelegramBot, | ||
| message: TelegramBot.Message, | ||
| chatId: number, | ||
| parameterAfterCommand?: string |
There was a problem hiding this comment.
CommandParameter, I'd rename
| isMessageIsCommand(message.text, UserRegExps.Unsubscribe) || | ||
| isMatchingDashboardItem(message.text, UserMessages.Unsubscribe) | ||
| ) { | ||
| if (!parameterAfterCommand) { | ||
| return buildUnsubscribeInlineResponse(bot, message, chatId); | ||
| } | ||
|
|
||
| const [err, result] = await catchAsyncError<string>( |
There was a problem hiding this comment.
Looks like it just made us remove tons of manupulations with parameters
| @@ -134,7 +128,10 @@ export class MessageHandlerRegistry { | |||
| } | |||
| } | |||
|
|
|||
| export const withCommandArgument = ( | |||
| // This function is wrapper around the original User's query handler | |||
There was a problem hiding this comment.
I'd add comments the way they could be read as docs
https://www.valentinog.com/blog/jsdoc/
But that's absolutely optional
There was a problem hiding this comment.
we have TS, I do not think we'd ever use jsdoc, what do you think?
| if (!execResult) { | ||
| throw new Error('Please input any command from available'); | ||
| logger.log('info', getInfoMessage('Entered unsupported command')); |
There was a problem hiding this comment.
I'd change severity level here to be warning.
BUT
and it also seems like in current approach this error will be generated each time we call command wich has single handler serving ofr both cases /command and /command [parameter]
I'm still thinking of what is the best way to solve here 🤔
There was a problem hiding this comment.
@Proladge you're almost right. and it also seems like in current approach this error will be generated each time we call command wich has single handler serving ofr both cases /command and /command will appear at 220 line in this file.
There was a problem hiding this comment.
replaced info with warn
| } | ||
|
|
||
| /* tslint:disable:no-string-literal */ | ||
| if (execResult.groups['command'] && !execResult.groups['firstargument']) { | ||
| throw new Error(`please provide argument for \\${execResult.groups['command']}`); | ||
| logger.log('info', getInfoMessage(`No parameter for ${execResult.groups['command']}`)); |
There was a problem hiding this comment.
Same as above.
Probably we shouldn't even log it.
Since it's absolutely normal situatuin.
BUT who knews.
| isMessageIsCommand(message.text, UserRegExps.Subscribe) || | ||
| isMatchingDashboardItem(message.text, UserMessages.SubscriptionManager) | ||
| ) { | ||
| if (!parameterAfterCommand) { |
There was a problem hiding this comment.
much better now
@danbilokha 🙇
massive thank you, for improving our code base!!!
No description provided.