[MM-59996] Make stats and client logs more easily accessible through slash commands #840
Conversation
streamer45
added
2: Dev Review
webapp/src/client.ts
Outdated
| const storage = window.desktop ? localStorage : sessionStorage; | ||
| storage.setItem(STORAGE_CALLS_CLIENT_STATS_KEY, JSON.stringify(stats)); |
There was a problem hiding this comment.
I didn't want to overcomplicate it by adding listeners, so the stats are saved when the client disconnects (user leaves call) and can be retrieved after that.
This slightly differs from Web browser behavior, where stats can be fetched at any time during or after a call, but the simple fix is enough to cover the customer's use case (troubleshooting).
| @@ -245,6 +264,17 @@ func (p *Plugin) ExecuteCommand(_ *plugin.Context, args *model.CommandArgs) (*mo | |||
| return resp, nil | |||
| } | |||
|
|
|||
| if subCmd == logsCommandTrigger { | |||
There was a problem hiding this comment.
This if subCmd == subCommandCXyz { ... } format in this function is getting kind of long.
Either in this PR or another can we shorten it?
Maybe something like the following (or a switch):
if subCmd == subCommandXyz {
return subCommandXyzHelper()
}
There was a problem hiding this comment.
On the other hand, having a long function that reads simply top-to-bottom can be easier and quicker to read and understand, compared to having to jump back and forth to/from functions. So there's an argument for keeping it simple, too. :)
There was a problem hiding this comment.
@cpoile single responsibility principle + easier to write tests for / understand the code coverage, especially if you use an IDE tool that shows highlights for code coverage.
I'm fine with leaving it as is if nobody else thinks it's an issue, but it's definitely a code smell.
There was a problem hiding this comment.
Another option is I can make this change if you all want in another ticket.
There was a problem hiding this comment.
@enzowritescode I added a wrapping utility to clean it up a little. Let me know if that works.
| @@ -245,6 +264,17 @@ func (p *Plugin) ExecuteCommand(_ *plugin.Context, args *model.CommandArgs) (*mo | |||
| return resp, nil | |||
| } | |||
|
|
|||
| if subCmd == logsCommandTrigger { | |||
There was a problem hiding this comment.
On the other hand, having a long function that reads simply top-to-bottom can be easier and quicker to read and understand, compared to having to jump back and forth to/from functions. So there's an argument for keeping it simple, too. :)
| @@ -99,8 +104,8 @@ export default class CallsClient extends EventEmitter { | |||
| }; | |||
| } | |||
|
|
|||
| const defaultInputID = window.localStorage.getItem('calls_default_audio_input'); | |||
| const defaultOutputID = window.localStorage.getItem('calls_default_audio_output'); | |||
| const defaultInputID = window.localStorage.getItem(STORAGE_CALLS_DEFAULT_AUDIO_INPUT_KEY); | |||
| import {pluginId} from './manifest'; | ||
|
|
||
| let clientLogs = ''; |
This reverts commit 7710471.
streamer45
added
3: Reviews Complete
* [MM-59996] Make stats and client logs more easily accessible through slash commands (#840) * Make stats and client logs more easily accessible through slash commands * Util function * Command response wrapper * Fix e2e * Revert "Fix e2e" This reverts commit 7710471. * Use Playbooks v2 * Fix e2e tests pipeline (#859) * update e2e test pipeline
Summary
PR fixes an issue with the
/call statscommand not working on the Desktop app. This bug has existed since we introduced the global widget, which is a different window and requires special handling in order to communicate any data back to the center channel (main app).Here, we are using a trick that doesn't require touching the Desktop. We use the
localStorage, which is shared between all the windows. This is a temporary solution while we work on a better path forward (MM-60028).While we are here, I also wanted to address another significant limitation concerning Call logs. On Desktop, these would disappear along with the widget window as soon as the call dropped, making it almost impossible to debug network issues.
To address this, PR implements a convenient storage-backed
/call logscommand that returns the client logs for the last call session without requiring users to go through multiple hops (i.e., the console) in order to access them (see screenshot).Screenshot
Ticket Link
https://mattermost.atlassian.net/browse/MM-59996
https://mattermost.atlassian.net/browse/MM-59995