Skip to content
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

MM-54243 - Slash command for host change #668

Merged
merged 8 commits into from
Mar 29, 2024
Merged

Conversation

cpoile
Copy link
Member

@cpoile cpoile commented Mar 27, 2024

Summary

Ticket Link

changes needed after rebase

put behind enterprise for now (for community) -- will change on release

blank commit

hostlock if newHost is already the host

rearrange order

simplify

PR comment

PR comments

use TrimPrefix

implement host change slash command
@cpoile cpoile added the 2: Dev Review Requires review by a core committer label Mar 27, 2024
@cpoile cpoile requested a review from streamer45 March 27, 2024 21:32
server/enterprise/license.go Outdated Show resolved Hide resolved
server/state.go Show resolved Hide resolved
server/session.go Outdated Show resolved Hide resolved
@cpoile cpoile requested a review from streamer45 March 28, 2024 19:10
@codecov-commenter
Copy link

codecov-commenter commented Mar 28, 2024

Codecov Report

Attention: Patch coverage is 5.49451% with 86 lines in your changes are missing coverage. Please review.

Project coverage is 11.05%. Comparing base (98f05fb) to head (f6e326d).

Files Patch % Lines
server/host_controls.go 0.00% 39 Missing ⚠️
server/slash_command.go 0.00% 31 Missing ⚠️
server/state.go 35.71% 8 Missing and 1 partial ⚠️
server/websocket.go 0.00% 5 Missing ⚠️
server/session.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #668      +/-   ##
==========================================
- Coverage   11.16%   11.05%   -0.12%     
==========================================
  Files          26       27       +1     
  Lines        5606     5690      +84     
==========================================
+ Hits          626      629       +3     
- Misses       4923     5003      +80     
- Partials       57       58       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@streamer45
Copy link
Contributor

@cpoile One non-blocking thought that comes to mind. If an assigned host leaves, the call will be host-less until they either come back or a new host is assigned by an admin. I am wondering if this is always desired/expected. I wonder if it would be acceptable to promote someone else to host according to the usual rule and if the assigned one comes back, change again.

@streamer45 streamer45 added 3: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Mar 28, 2024
@cpoile
Copy link
Member Author

cpoile commented Mar 28, 2024

@streamer45 I'm trying something here, let me know what you think. The tricky part is (unless I'm missing something) we need to remember who the assigned host was, while allowing another person to be host.

@streamer45
Copy link
Contributor

@streamer45 I'm trying something here, let me know what you think. The tricky part is (unless I'm missing something) we need to remember who the assigned host was, while allowing another person to be host.

That sounds about right. Also, I was merely asking, I assume it makes sense to you as well? :p

@streamer45
Copy link
Contributor

One great thing (can be deferred) would be some e2e tests so that when I go and convert this to the new data store I know if I messed it all up :p

@cpoile
Copy link
Member Author

cpoile commented Mar 28, 2024

@streamer45 Definitely, https://mattermost.atlassian.net/browse/MM-57529

Also, just added something we were missing: hosts should be allowed to change hosts.

@cpoile
Copy link
Member Author

cpoile commented Mar 29, 2024

@streamer45 ok to merge?

@cpoile cpoile merged commit c3ee42e into main Mar 29, 2024
6 checks passed
@cpoile cpoile deleted the MM-54243-host-change-control-3 branch March 29, 2024 00:18
streamer45 added a commit that referenced this pull request Apr 3, 2024
* Translations update from Mattermost Weblate (#661)

* Translated using Weblate (Chinese (Simplified))

Currently translated at 100.0% (185 of 185 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/zh_Hans/

* Translated using Weblate (Dutch)

Currently translated at 100.0% (185 of 185 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/nl/

---------

Co-authored-by: ThrRip <coding@thrrip.space>
Co-authored-by: Tom De Moor <tom@controlaltdieliet.be>

* [MM-57483] Add job config validation (#667)

* Add job config validation

* Update calls-offloader

* go mod tidy

* MM-56540 - Live captions 🎥  (#633)

* implement live captions server side and frontend in expanded_view

* linting; PR comments

* i18n

* clarify an unrelated component's naming: expanded_incoming_call_container

* live captions should not overlap rhs

* add EnableLiveCaptions and LiveCaptionsModelSize as console settings

* upgrade @calls/common for standalone

* add LiveCaptionsAudioLen as a new histogram metric; fix config passing

* remove debugging statement and go mod tidy

* update transcriber; fix defaults

* rename live captions metrics

* live captions metrics: buckets

* live captions metrics: tweak buckets

* add a metrics ws endpoint; move over struct defs from transcriber

* add a metrics for transcriber buffer full

* add a metrics for tick rate by transcriberID

* clean up gauges after transcription ended

* remove tick rate gauges -- not useful.

* remove tick rate gauges -- not useful.

* clean up naming

* only show CC button once recording has started

* move CC button

* tweak captions width; update transcriber dependency, default
numTranscribers -> 1

* new cc icon; 32px

* add TranscriberNumThreads and LiveCaptionsNumThreadsPerTranscriber

* update calls transcriber version

* revert update calls transcriber version

* update calls_transcriber_version to v0.2.2-dev

* update calls/common; LiveCaptionsData uses channel_id now

* send the channel_id with LiveCaptionsData for simplicity

* and simplify the receipt of LiveCaptionData

* add LiveCaptionsPktPayloadChBufFullCounter to metrics

* add live captions state; recState -> jobState

* combine job state messages under `call_job_state`

* moved type to jobStateClient

* add JobTypeCaptioning; use public.JobType as JobStateClient.Type

* better comment

* fix captions covering screensharing expand button

* add LiveCaptionsLanguage system console setting

* comment cleanup, useless code cleanup

* bump deps for transcriber and rtcd

* bumps for everyone except you rtcd

* [MM-57421] Live captions e2e tests (#665)

* MM-54243 - Slash command for host change (#668)

* host change control PR squashed

changes needed after rebase

put behind enterprise for now (for community) -- will change on release

blank commit

hostlock if newHost is already the host

rearrange order

simplify

PR comment

PR comments

use TrimPrefix

implement host change slash command

* removeUserSession doesn't need userID anymore

* host controls are professional licensed

* PR comments

* PR comments

* always have a host, but return it to the HostLockedID when they return

* host should be allowed to change host even if they aren't admin

* Translations update from Mattermost Weblate (#669)

* Translated using Weblate (Chinese (Simplified))

Currently translated at 100.0% (185 of 185 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/zh_Hans/

* Translated using Weblate (Dutch)

Currently translated at 100.0% (185 of 185 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/nl/

* Translated using Weblate (Japanese)

Currently translated at 100.0% (185 of 185 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/ja/

* Translated using Weblate (Dutch)

Currently translated at 100.0% (187 of 187 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/nl/

* Translated using Weblate (Swedish)

Currently translated at 100.0% (187 of 187 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/sv/

* Translated using Weblate (Japanese)

Currently translated at 100.0% (187 of 187 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/ja/

* Translated using Weblate (Chinese (Simplified))

Currently translated at 100.0% (187 of 187 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/zh_Hans/

---------

Co-authored-by: ThrRip <coding@thrrip.space>
Co-authored-by: Tom De Moor <tom@controlaltdieliet.be>
Co-authored-by: kaakaa <stooner.hoe@gmail.com>
Co-authored-by: MArtin Johnson <martinjohnson@bahnhof.se>

* [MM-57529] e2e tests for host change slash command (#671)

* host change e2e tests

* extend timeout

---------

Co-authored-by: Weblate (bot) <hosted@weblate.org>
Co-authored-by: ThrRip <coding@thrrip.space>
Co-authored-by: Tom De Moor <tom@controlaltdieliet.be>
Co-authored-by: Christopher Poile <cpoile@gmail.com>
Co-authored-by: kaakaa <stooner.hoe@gmail.com>
Co-authored-by: MArtin Johnson <martinjohnson@bahnhof.se>
streamer45 added a commit that referenced this pull request Apr 3, 2024
* Improve store consistency

* API router

* Fix JSON values binary conversion

* temp

* temp2

* temp3

* Return expected client state

* Remove channelState

* Fix tests

* Cleanup state

* calls_jobs store

* Jobs business logic

* Fix tests

* Fix linter

* Include jobs in returned client state

* More fixes and tests

* Link to Jira ticket

* GetActiveCallJobs

* Rename lockCall functions

* Make JSON columns not NULL

* [MM-42464] Merge in latest changes (#670)

* Translations update from Mattermost Weblate (#661)

* Translated using Weblate (Chinese (Simplified))

Currently translated at 100.0% (185 of 185 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/zh_Hans/

* Translated using Weblate (Dutch)

Currently translated at 100.0% (185 of 185 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/nl/

---------

Co-authored-by: ThrRip <coding@thrrip.space>
Co-authored-by: Tom De Moor <tom@controlaltdieliet.be>

* [MM-57483] Add job config validation (#667)

* Add job config validation

* Update calls-offloader

* go mod tidy

* MM-56540 - Live captions 🎥  (#633)

* implement live captions server side and frontend in expanded_view

* linting; PR comments

* i18n

* clarify an unrelated component's naming: expanded_incoming_call_container

* live captions should not overlap rhs

* add EnableLiveCaptions and LiveCaptionsModelSize as console settings

* upgrade @calls/common for standalone

* add LiveCaptionsAudioLen as a new histogram metric; fix config passing

* remove debugging statement and go mod tidy

* update transcriber; fix defaults

* rename live captions metrics

* live captions metrics: buckets

* live captions metrics: tweak buckets

* add a metrics ws endpoint; move over struct defs from transcriber

* add a metrics for transcriber buffer full

* add a metrics for tick rate by transcriberID

* clean up gauges after transcription ended

* remove tick rate gauges -- not useful.

* remove tick rate gauges -- not useful.

* clean up naming

* only show CC button once recording has started

* move CC button

* tweak captions width; update transcriber dependency, default
numTranscribers -> 1

* new cc icon; 32px

* add TranscriberNumThreads and LiveCaptionsNumThreadsPerTranscriber

* update calls transcriber version

* revert update calls transcriber version

* update calls_transcriber_version to v0.2.2-dev

* update calls/common; LiveCaptionsData uses channel_id now

* send the channel_id with LiveCaptionsData for simplicity

* and simplify the receipt of LiveCaptionData

* add LiveCaptionsPktPayloadChBufFullCounter to metrics

* add live captions state; recState -> jobState

* combine job state messages under `call_job_state`

* moved type to jobStateClient

* add JobTypeCaptioning; use public.JobType as JobStateClient.Type

* better comment

* fix captions covering screensharing expand button

* add LiveCaptionsLanguage system console setting

* comment cleanup, useless code cleanup

* bump deps for transcriber and rtcd

* bumps for everyone except you rtcd

* [MM-57421] Live captions e2e tests (#665)

* MM-54243 - Slash command for host change (#668)

* host change control PR squashed

changes needed after rebase

put behind enterprise for now (for community) -- will change on release

blank commit

hostlock if newHost is already the host

rearrange order

simplify

PR comment

PR comments

use TrimPrefix

implement host change slash command

* removeUserSession doesn't need userID anymore

* host controls are professional licensed

* PR comments

* PR comments

* always have a host, but return it to the HostLockedID when they return

* host should be allowed to change host even if they aren't admin

* Translations update from Mattermost Weblate (#669)

* Translated using Weblate (Chinese (Simplified))

Currently translated at 100.0% (185 of 185 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/zh_Hans/

* Translated using Weblate (Dutch)

Currently translated at 100.0% (185 of 185 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/nl/

* Translated using Weblate (Japanese)

Currently translated at 100.0% (185 of 185 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/ja/

* Translated using Weblate (Dutch)

Currently translated at 100.0% (187 of 187 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/nl/

* Translated using Weblate (Swedish)

Currently translated at 100.0% (187 of 187 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/sv/

* Translated using Weblate (Japanese)

Currently translated at 100.0% (187 of 187 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/ja/

* Translated using Weblate (Chinese (Simplified))

Currently translated at 100.0% (187 of 187 strings)

Translation: Calls/webapp
Translate-URL: https://translate.mattermost.com/projects/calls/webapp/zh_Hans/

---------

Co-authored-by: ThrRip <coding@thrrip.space>
Co-authored-by: Tom De Moor <tom@controlaltdieliet.be>
Co-authored-by: kaakaa <stooner.hoe@gmail.com>
Co-authored-by: MArtin Johnson <martinjohnson@bahnhof.se>

* [MM-57529] e2e tests for host change slash command (#671)

* host change e2e tests

* extend timeout

---------

Co-authored-by: Weblate (bot) <hosted@weblate.org>
Co-authored-by: ThrRip <coding@thrrip.space>
Co-authored-by: Tom De Moor <tom@controlaltdieliet.be>
Co-authored-by: Christopher Poile <cpoile@gmail.com>
Co-authored-by: kaakaa <stooner.hoe@gmail.com>
Co-authored-by: MArtin Johnson <martinjohnson@bahnhof.se>

---------

Co-authored-by: Weblate (bot) <hosted@weblate.org>
Co-authored-by: ThrRip <coding@thrrip.space>
Co-authored-by: Tom De Moor <tom@controlaltdieliet.be>
Co-authored-by: Christopher Poile <cpoile@gmail.com>
Co-authored-by: kaakaa <stooner.hoe@gmail.com>
Co-authored-by: MArtin Johnson <martinjohnson@bahnhof.se>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants