Skip to content

Commit

Permalink
fix: Make the High-performance backend warnings more actionable
Browse files Browse the repository at this point in the history
Signed-off-by: Joas Schilling <[email protected]>
Signed-off-by: Maksim Sukharev <[email protected]>
  • Loading branch information
nickvergessen committed Jan 22, 2025
1 parent fabd117 commit 7e07e7b
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 19 deletions.
3 changes: 3 additions & 0 deletions lib/Settings/Admin/AdminSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use OCP\IUserSession;
use OCP\L10N\IFactory;
use OCP\Settings\ISettings;
use OCP\Support\Subscription\IRegistry;
use OCP\Util;

class AdminSettings implements ISettings {
Expand All @@ -38,6 +39,7 @@ public function __construct(
private ICacheFactory $memcacheFactory,
private IGroupManager $groupManager,
private MatterbridgeManager $bridgeManager,
private IRegistry $subscription,
IUserSession $userSession,
private IL10N $l10n,
private IFactory $l10nFactory,
Expand Down Expand Up @@ -126,6 +128,7 @@ protected function initTurnServers(): void {
}

protected function initSignalingServers(): void {
$this->initialState->provideInitialState('has_valid_subscription', $this->subscription->delegateHasValidSubscription());
$this->initialState->provideInitialState('has_cache_configured', $this->memcacheFactory->isAvailable());
$this->initialState->provideInitialState('signaling_mode', $this->talkConfig->getSignalingMode(false));
$this->initialState->provideInitialState('signaling_servers', [
Expand Down
11 changes: 9 additions & 2 deletions lib/SetupCheck/HighPerformanceBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use OCP\IURLGenerator;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
use OCP\Support\Subscription\IRegistry;

class HighPerformanceBackend implements ISetupCheck {
public function __construct(
Expand All @@ -24,6 +25,7 @@ public function __construct(
readonly protected IURLGenerator $urlGenerator,
readonly protected IL10N $l,
readonly protected Manager $signalManager,
readonly protected IRegistry $subscription,
) {
}

Expand All @@ -39,11 +41,16 @@ public function run(): SetupResult {
if ($this->talkConfig->getSignalingMode() === Config::SIGNALING_INTERNAL) {
$setupResult = SetupResult::error(...);
if ($this->talkConfig->getHideSignalingWarning()) {
$setupResult = SetupResult::warning(...);
$setupResult = SetupResult::info(...);
}
$documentation = 'https://nextcloud-talk.readthedocs.io/en/latest/quick-install/';
if ($this->subscription->delegateHasValidSubscription()) {
$documentation = 'https://portal.nextcloud.com/article/Nextcloud-Talk/High-Performance-Backend/Installation-of-Nextcloud-Talk-High-Performance-Backend';
}

return $setupResult(
$this->l->t('No High-performance backend configured - Running Nextcloud Talk without the High-performance backend only scales for very small calls (max. 2-3 participants). Please set up the High-performance backend to ensure calls with multiple participants work seamlessly.'),
'https://portal.nextcloud.com/article/Nextcloud-Talk/High-Performance-Backend/Installation-of-Nextcloud-Talk-High-Performance-Backend',
$documentation,
);
}

Expand Down
12 changes: 8 additions & 4 deletions lib/SetupCheck/RecordingBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,20 @@ public function getCategory(): string {
}

public function getName(): string {
return $this->l->t('Recording backend');
$name = $this->l->t('Recording backend');
if ($this->talkConfig->getSignalingMode() === Config::SIGNALING_INTERNAL) {
return '[skip] ' . $name;
}
return $name;
}

public function run(): SetupResult {
if ($this->talkConfig->getSignalingMode() !== Config::SIGNALING_INTERNAL) {
return SetupResult::success();
if ($this->talkConfig->getSignalingMode() === Config::SIGNALING_INTERNAL) {
return SetupResult::success($this->l->t('Using the recording backend requires a High-performance backend.'));
}
if (empty($this->talkConfig->getRecordingServers())) {
return SetupResult::info($this->l->t('No recording backend configured'));
}
return SetupResult::error($this->l->t('Using the recording backend requires a High-performance backend.'));
return SetupResult::success();
}
}
12 changes: 8 additions & 4 deletions lib/SetupCheck/SIPConfiguration.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,20 @@ public function getCategory(): string {
}

public function getName(): string {
return $this->l->t('SIP dial-in');
$name = $this->l->t('SIP configuration');
if ($this->talkConfig->getSignalingMode() === Config::SIGNALING_INTERNAL) {
return '[skip] ' . $name;
}
return $name;
}

public function run(): SetupResult {
if ($this->talkConfig->getSignalingMode() !== Config::SIGNALING_INTERNAL) {
return SetupResult::success();
if ($this->talkConfig->getSignalingMode() === Config::SIGNALING_INTERNAL) {
return SetupResult::success($this->l->t('Using the SIP functionality requires a High-performance backend.'));
}
if ($this->talkConfig->getSIPSharedSecret() === '' && $this->talkConfig->getDialInInfo() === '') {
return SetupResult::info($this->l->t('No SIP backend configured'));
}
return SetupResult::error($this->l->t('Using the SIP functionality requires a High-performance backend.'));
return SetupResult::success();
}
}
33 changes: 25 additions & 8 deletions src/components/AdminSettings/SignalingServers.vue
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,24 @@
<template>
<section id="signaling_server" class="signaling-servers section">
<NcNoteCard v-if="!serversProxy.length"
type="error"
:heading="t('spreed', 'Nextcloud Talk setup not complete')"
:text="t('spreed', 'Install the High-performance backend to ensure calls with multiple participants work seamlessly.')" />
type="warning"
:heading="t('spreed', 'Nextcloud Talk setup not complete')">
{{ t('spreed', 'Please note that in calls with more than 2 participants without the High-performance backend, participants will most likely experience connectivity issues and cause high load on participating devices.') }}
{{ t('spreed', 'Install the High-performance backend to ensure calls with multiple participants work seamlessly.') }}

<NcButton v-if="props.hasValidSubscription"
type="primary"
class="additional-top-margin"
href="https://portal.nextcloud.com/article/Nextcloud-Talk/High-Performance-Backend/Installation-of-Nextcloud-Talk-High-Performance-Backend">
{{ t('spreed', 'Nextcloud portal') }} ↗
</NcButton>
<NcButton v-else
type="primary"
class="additional-top-margin"
href="https://nextcloud-talk.readthedocs.io/en/latest/quick-install/">
{{ t('spreed', 'Quick installation guide') }} ↗
</NcButton>
</NcNoteCard>

<h2>
{{ t('spreed', 'High-performance backend') }}
Expand Down Expand Up @@ -57,10 +72,9 @@
@update:model-value="debounceUpdateServers" />

<template v-if="!serversProxy.length">
<NcNoteCard type="warning"
class="additional-top-margin"
:text="t('spreed', 'Please note that in calls with more than 2 participants without the High-performance backend, participants will most likely experience connectivity issues and cause high load on participating devices.')" />
<NcCheckboxRadioSwitch v-model="hideWarningProxy"
type="switch"
class="additional-top-margin"
:disabled="loading"
@update:model-value="updateHideWarning">
{{ t('spreed', 'Don\'t warn about connectivity issues in calls with more than 2 participants') }}
Expand Down Expand Up @@ -96,6 +110,7 @@ const props = defineProps<{
hideWarning: InitialState['spreed']['signaling_servers']['hideWarning'],
secret: InitialState['spreed']['signaling_servers']['secret'],
servers: InitialState['spreed']['signaling_servers']['servers'],
hasValidSubscription: InitialState['spreed']['has_valid_subscription'],
}>()

const emit = defineEmits<{
Expand Down Expand Up @@ -161,7 +176,9 @@ function updateHideWarning(value: boolean) {
loading.value = true
OCP.AppConfig.setValue('spreed', 'hide_signaling_warning', value ? 'yes' : 'no', {
success: () => {
showSuccess(t('spreed', 'Missing High-performance backend warning hidden'))
if (value) {
showSuccess(t('spreed', 'Missing High-performance backend warning hidden'))
}
loading.value = false
},
})
Expand Down Expand Up @@ -193,6 +210,6 @@ function updateServers() {
}

.additional-top-margin {
margin-top: 35px !important;
margin-top: 1em !important;
}
</style>
3 changes: 3 additions & 0 deletions src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ export type getCapabilitiesResponse = ApiResponse<operations['room-get-capabilit
// Initial state
export type InitialState = {
spreed: {
'has_cache_configured': boolean,
'has_valid_subscription': boolean,
'signaling_mode': string,
'signaling_servers': {
hideWarning: boolean,
secret: string,
Expand Down
5 changes: 4 additions & 1 deletion src/views/AdminSettings.vue
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import WebServerSetupChecks from '../components/AdminSettings/WebServerSetupChec
import { hasTalkFeature } from '../services/CapabilitiesManager.ts'
import type { InitialState } from '../types/index.ts'

const hasValidSubscription = loadState<InitialState['spreed']['has_valid_subscription']>('spreed', 'has_valid_subscription')

const supportFederation = hasTalkFeature('local', 'federation-v1')

const signalingServers = ref<InitialState['spreed']['signaling_servers']>(loadState('spreed', 'signaling_servers', {
Expand All @@ -38,7 +40,8 @@ const hasSignalingServers = computed(() => signalingServers.value.servers.length
<div>
<SignalingServers :servers.sync="signalingServers.servers"
:secret.sync="signalingServers.secret"
:hide-warning.sync="signalingServers.hideWarning" />
:hide-warning.sync="signalingServers.hideWarning"
:has-valid-subscription="hasValidSubscription" />
<HostedSignalingServer :has-signaling-servers="hasSignalingServers" />
<GeneralSettings :has-signaling-servers="hasSignalingServers" />
<AllowedGroups />
Expand Down
5 changes: 5 additions & 0 deletions tests/php/Settings/Admin/AdminSettingsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
use OCP\IL10N;
use OCP\IUserSession;
use OCP\L10N\IFactory;
use OCP\Support\Subscription\IRegistry;
use PHPUnit\Framework\Assert;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
Expand All @@ -31,6 +32,7 @@ class AdminSettingsTest extends TestCase {
protected ICacheFactory&MockObject $cacheFactory;
protected IGroupManager&MockObject $groupManager;
protected MatterbridgeManager&MockObject $matterbridgeManager;
protected IRegistry&MockObject $subscription;
protected IUserSession&MockObject $userSession;
protected IL10N&MockObject $l10n;
protected IFactory&MockObject $l10nFactory;
Expand All @@ -46,6 +48,7 @@ public function setUp(): void {
$this->cacheFactory = $this->createMock(ICacheFactory::class);
$this->groupManager = $this->createMock(IGroupManager::class);
$this->matterbridgeManager = $this->createMock(MatterbridgeManager::class);
$this->subscription = $this->createMock(IRegistry::class);
$this->userSession = $this->createMock(IUserSession::class);
$this->l10n = $this->createMock(IL10N::class);
$this->l10nFactory = $this->createMock(IFactory::class);
Expand All @@ -67,6 +70,7 @@ protected function getAdminSettings(array $methods = []): AdminSettings {
$this->cacheFactory,
$this->groupManager,
$this->matterbridgeManager,
$this->subscription,
$this->userSession,
$this->l10n,
$this->l10nFactory
Expand All @@ -82,6 +86,7 @@ protected function getAdminSettings(array $methods = []): AdminSettings {
$this->cacheFactory,
$this->groupManager,
$this->matterbridgeManager,
$this->subscription,
$this->userSession,
$this->l10n,
$this->l10nFactory,
Expand Down

0 comments on commit 7e07e7b

Please sign in to comment.