Skip to content

Commit 71f1e0c

Browse files
committed
refactor(updatenotification): Migrate legacy code
1. Remove hook usage and just provide an initial state 2. Replace jQuery code with modern non-deprecated frontend code Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent 280f6df commit 71f1e0c

7 files changed

Lines changed: 86 additions & 45 deletions

File tree

apps/updatenotification/appinfo/info.xml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,11 @@
2424
<job>OCA\UpdateNotification\BackgroundJob\UpdateAvailableNotifications</job>
2525
</background-jobs>
2626

27-
<settings>
28-
<admin>OCA\UpdateNotification\Settings\Admin</admin>
29-
</settings>
30-
3127
<commands>
3228
<command>OCA\UpdateNotification\Command\Check</command>
3329
</commands>
30+
31+
<settings>
32+
<admin>OCA\UpdateNotification\Settings\Admin</admin>
33+
</settings>
3434
</info>

apps/updatenotification/js/legacy-notification.js

Lines changed: 0 additions & 15 deletions
This file was deleted.

apps/updatenotification/lib/AppInfo/Application.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ public function boot(IBootContext $context): void {
5050
IAppManager $appManager,
5151
IGroupManager $groupManager,
5252
ContainerInterface $container,
53-
LoggerInterface $logger): void {
53+
LoggerInterface $logger,
54+
): void {
5455
if ($config->getSystemValue('updatechecker', true) !== true) {
5556
// Updater check is disabled
5657
return;
@@ -72,8 +73,8 @@ public function boot(IBootContext $context): void {
7273
}
7374

7475
if ($updateChecker->getUpdateState() !== []) {
75-
Util::addScript('updatenotification', 'legacy-notification');
76-
\OC_Hook::connect('\OCP\Config', 'js', $updateChecker, 'populateJavaScriptVariables');
76+
Util::addScript('updatenotification', 'update-notification-legacy');
77+
$updateChecker->setInitialState();
7778
}
7879
}
7980
});

apps/updatenotification/lib/UpdateChecker.php

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,15 @@
1010

1111
use OC\Updater\ChangesCheck;
1212
use OC\Updater\VersionCheck;
13+
use OCP\AppFramework\Services\IInitialState;
1314

1415
class UpdateChecker {
15-
/** @var VersionCheck */
16-
private $updater;
17-
/** @var ChangesCheck */
18-
private $changesCheck;
1916

20-
/**
21-
* @param VersionCheck $updater
22-
*/
23-
public function __construct(VersionCheck $updater, ChangesCheck $changesCheck) {
24-
$this->updater = $updater;
25-
$this->changesCheck = $changesCheck;
17+
public function __construct(
18+
private VersionCheck $updater,
19+
private ChangesCheck $changesCheck,
20+
private IInitialState $initialState,
21+
) {
2622
}
2723

2824
/**
@@ -59,13 +55,17 @@ public function getUpdateState(): array {
5955
}
6056

6157
/**
62-
* @param array $data
58+
* Provide update information as initial state
6359
*/
64-
public function populateJavaScriptVariables(array $data) {
65-
$data['array']['oc_updateState'] = json_encode([
66-
'updateAvailable' => true,
67-
'updateVersion' => $this->getUpdateState()['updateVersionString'],
68-
'updateLink' => $this->getUpdateState()['updateLink'] ?? '',
60+
public function setInitialState(): void {
61+
$updateState = $this->getUpdateState();
62+
if (empty($updateState)) {
63+
return;
64+
}
65+
66+
$this->initialState->provideInitialState('updateState', [
67+
'updateVersion' => $updateState['updateVersionString'],
68+
'updateLink' => $updateState['updateLink'] ?? '',
6969
]);
7070
}
7171
}
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
3+
* SPDX-License-Identifier: AGPL-3.0-or-later
4+
*/
5+
6+
import { showInfo } from '@nextcloud/dialogs'
7+
import { loadState } from '@nextcloud/initial-state'
8+
import { t } from '@nextcloud/l10n'
9+
10+
interface IUpdateNotificationState {
11+
updateLink: string
12+
updateVersion: string
13+
}
14+
15+
/**
16+
* This only gets loaded if an update is available and the notifications app is not enabled for the user.
17+
*/
18+
window.addEventListener('DOMContentLoaded', function() {
19+
const { updateLink, updateVersion } = loadState<IUpdateNotificationState>('updatenotification', 'updateState')
20+
const text = t('core', '{version} is available. Get more information on how to update.', { version: updateVersion })
21+
22+
// On click open the update link in a new tab
23+
showInfo(text, { onClick: () => window.open(updateLink, '_blank') })
24+
})

apps/updatenotification/tests/UpdateCheckerTest.php

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,28 @@
1111
use OC\Updater\ChangesCheck;
1212
use OC\Updater\VersionCheck;
1313
use OCA\UpdateNotification\UpdateChecker;
14+
use OCP\AppFramework\Services\IInitialState;
15+
use PHPUnit\Framework\MockObject\MockObject;
1416
use Test\TestCase;
1517

1618
class UpdateCheckerTest extends TestCase {
17-
/** @var ChangesCheck|\PHPUnit\Framework\MockObject\MockObject */
18-
protected $changesChecker;
19-
/** @var VersionCheck|\PHPUnit\Framework\MockObject\MockObject */
20-
private $updater;
21-
/** @var UpdateChecker */
22-
private $updateChecker;
19+
20+
private ChangesCheck&MockObject $changesChecker;
21+
private VersionCheck&MockObject $updater;
22+
private IInitialState&MockObject $initialState;
23+
private UpdateChecker $updateChecker;
2324

2425
protected function setUp(): void {
2526
parent::setUp();
2627

2728
$this->updater = $this->createMock(VersionCheck::class);
2829
$this->changesChecker = $this->createMock(ChangesCheck::class);
29-
$this->updateChecker = new UpdateChecker($this->updater, $this->changesChecker);
30+
$this->initialState = $this->createMock(IInitialState::class);
31+
$this->updateChecker = new UpdateChecker(
32+
$this->updater,
33+
$this->changesChecker,
34+
$this->initialState,
35+
);
3036
}
3137

3238
public function testGetUpdateStateWithUpdateAndInvalidLink(): void {
@@ -110,4 +116,28 @@ public function testGetUpdateStateWithoutUpdate(): void {
110116
$expected = [];
111117
$this->assertSame($expected, $this->updateChecker->getUpdateState());
112118
}
119+
120+
public function testSetInitialState(): void {
121+
$this->updater
122+
->expects($this->once())
123+
->method('check')
124+
->willReturn([
125+
'version' => '1.2.3',
126+
'versionstring' => 'Nextcloud 1.2.3',
127+
'web' => 'https://docs.nextcloud.com/myUrl',
128+
'url' => 'https://downloads.nextcloud.org/server',
129+
'changes' => 'https://updates.nextcloud.com/changelog_server/?version=123.0.0',
130+
'autoupdater' => '1',
131+
'eol' => '0',
132+
]);
133+
134+
$this->initialState->expects(self::once())
135+
->method('provideInitialState')
136+
->with('updateState', [
137+
'updateVersion' => 'Nextcloud 1.2.3',
138+
'updateLink' => 'https://docs.nextcloud.com/myUrl',
139+
]);
140+
141+
$this->updateChecker->setInitialState();
142+
}
113143
}

webpack.modules.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ module.exports = {
107107
init: path.join(__dirname, 'apps/updatenotification/src', 'init.ts'),
108108
'view-changelog-page': path.join(__dirname, 'apps/updatenotification/src', 'view-changelog-page.ts'),
109109
updatenotification: path.join(__dirname, 'apps/updatenotification/src', 'updatenotification.js'),
110+
'update-notification-legacy': path.join(__dirname, 'apps/updatenotification/src', 'update-notification-legacy.ts'),
110111
},
111112
user_status: {
112113
menu: path.join(__dirname, 'apps/user_status/src', 'menu.js'),

0 commit comments

Comments
 (0)