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.

Sign up for GitHub

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

Jump to bottom

Check for compatible apps from the appstore fails #12358

Merged
merged 4 commits into from
Nov 9, 2018
Merged

Check for compatible apps from the appstore fails #12358

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
90a35aa
Fix overlapping update button in notifications
MorrisJobke Nov 8, 2018
b04a44b
Use the version number when checking the appstore for compatible updates
nickvergessen Nov 8, 2018
03b140c
Improve style of incompatible apps list
MorrisJobke Nov 8, 2018
eff7a9c
Adjust unit tests
nickvergessen Nov 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/updatenotification/css/admin.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,7 @@
margin-top: 5px;
width: 300px;
}

#updatenotification .applist {
margin-bottom: 25px;
}
4 changes: 2 additions & 2 deletions apps/updatenotification/js/updatenotification.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion apps/updatenotification/js/updatenotification.js.map
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion apps/updatenotification/lib/Command/Check.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ protected function execute(InputInterface $input, OutputInterface $output) {
// Server
$r = $this->updateChecker->getUpdateState();
if (isset($r['updateAvailable']) && $r['updateAvailable']) {
$output->writeln($r['updateVersion'] . ' is available. Get more information on how to update at '. $r['updateLink'] . '.');
$output->writeln($r['updateVersionString'] . ' is available. Get more information on how to update at '. $r['updateLink'] . '.');
$updatesAvailableCount += 1;
}

Expand Down
3 changes: 2 additions & 1 deletion apps/updatenotification/lib/Settings/Admin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ public function getForm(): TemplateResponse {
'lastChecked' => $lastUpdateCheck,
'currentChannel' => $currentChannel,
'channels' => $channels,
'newVersionString' => empty($updateState['updateVersion']) ? '' : $updateState['updateVersion'],
'newVersion' => empty($updateState['updateVersion']) ? '' : $updateState['updateVersion'],
'newVersionString' => empty($updateState['updateVersionString']) ? '' : $updateState['updateVersionString'],
'downloadLink' => empty($updateState['downloadLink']) ? '' : $updateState['downloadLink'],
'changes' => $this->filterChanges($updateState['changes'] ?? []),
'updaterEnabled' => empty($updateState['updaterEnabled']) ? false : $updateState['updaterEnabled'],
Expand Down
5 changes: 3 additions & 2 deletions apps/updatenotification/lib/UpdateChecker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ public function getUpdateState(): array {

if (isset($data['version']) && $data['version'] !== '' && $data['version'] !== []) {
$result['updateAvailable'] = true;
$result['updateVersion'] = $data['versionstring'];
$result['updateVersion'] = $data['version'];
$result['updateVersionString'] = $data['versionstring'];
$result['updaterEnabled'] = $data['autoupdater'] === '1';
$result['versionIsEol'] = $data['eol'] === '1';
if (strpos($data['web'], 'https://') === 0) {
Expand Down Expand Up @@ -80,7 +81,7 @@ public function getUpdateState(): array {
public function populateJavaScriptVariables(array $data) {
$data['array']['oc_updateState'] = json_encode([
'updateAvailable' => true,
'updateVersion' => $this->getUpdateState()['updateVersion'],
'updateVersion' => $this->getUpdateState()['updateVersionString'],
'updateLink' => $this->getUpdateState()['updateLink'] ?? '',
]);
}
Expand Down
22 changes: 12 additions & 10 deletions apps/updatenotification/src/components/root.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,10 @@
</ul>
</template>

<a v-if="updaterEnabled" href="#" class="button" @click="clickUpdaterButton">{{ t('updatenotification', 'Open updater') }}</a>
<a v-if="downloadLink" :href="downloadLink" class="button" :class="{ hidden: !updaterEnabled }">{{ t('updatenotification', 'Download now') }}</a>
<p>
<a v-if="updaterEnabled" href="#" class="button" @click="clickUpdaterButton">{{ t('updatenotification', 'Open updater') }}</a>
<a v-if="downloadLink" :href="downloadLink" class="button" :class="{ hidden: !updaterEnabled }">{{ t('updatenotification', 'Download now') }}</a>
</p>
<div class="whatsNew" v-if="whatsNew">
<div class="toggleWhatsNew">
<span v-click-outside="hideMenu" @click="toggleMenu">{{ t('updatenotification', 'What\'s new?') }}</span>
Expand All @@ -55,8 +57,9 @@
</template>

<template v-if="!isDefaultUpdateServerURL">
<br />
<em>{{ t('updatenotification', 'A non-default update server is in use to be checked for updates:') }} <code>{{updateServerURL}}</code></em>
<p>
<em>{{ t('updatenotification', 'A non-default update server is in use to be checked for updates:') }} <code>{{updateServerURL}}</code></em>
</p>
</template>
</div>

Expand Down Expand Up @@ -153,7 +156,7 @@
}

$.ajax({
url: OC.linkToOCS('apps/updatenotification/api/v1/applist', 2) + this.newVersionString,
url: OC.linkToOCS('apps/updatenotification/api/v1/applist', 2) + this.newVersion,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error 1: We asked for updates on applist/Nextcloud 15.0.0 beta1 => 404

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error 3: Even if it gives back a 404 it should not say "Yes all fine", but rather "It can't be detected"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well that is exactly what it does now, after error2 was solved:
Could not connect to the appstore or the appstore returned no updates at all. Search manually for updates or make sure your server has access to the internet and can connect to the appstore.

type: 'GET',
beforeSend: function (request) {
request.setRequestHeader('Accept', 'application/json');
Expand Down Expand Up @@ -193,20 +196,18 @@
return t('updatenotification', 'Checking apps for compatible updates');
}

if (this.appstoreDisabled) {
if (this.appStoreDisabled) {
return t('updatenotification', 'Please make sure your config.php does not set <samp>appstoreenabled</samp> to false.');
}

if (this.appstoreFailed) {
if (this.appStoreFailed) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error 2: We checked an undefined argument because of the casing mismatch

return t('updatenotification', 'Could not connect to the appstore or the appstore returned no updates at all. Search manually for updates or make sure your server has access to the internet and can connect to the appstore.');
}

return this.missingAppUpdates.length === 0 ? t('updatenotification', '<strong>All</strong> apps have an update for this version available', this) : n('updatenotification',
'<strong>%n</strong> app has no update for this version available',
'<strong>%n</strong> apps have no update for this version available',
this.missingAppUpdates.length, {
version: this.newVersionString
});
this.missingAppUpdates.length);
},

productionInfoString: function() {
Expand Down Expand Up @@ -310,6 +311,7 @@
// Parse server data
var data = JSON.parse($('#updatenotification').attr('data-json'));

this.newVersion = data.newVersion;
this.newVersionString = data.newVersionString;
this.lastCheckedDate = data.lastChecked;
this.isUpdateChecked = data.isUpdateChecked;
Expand Down
4 changes: 3 additions & 1 deletion apps/updatenotification/tests/Settings/AdminTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public function testGetFormWithUpdate() {
->willReturn([
'updateAvailable' => true,
'updateVersion' => '8.1.2',
'updateVersionString' => 'Nextcloud 8.1.2',
'downloadLink' => 'https://downloads.nextcloud.org/server',
'changes' => [],
'updaterEnabled' => true,
Expand All @@ -129,7 +130,8 @@ public function testGetFormWithUpdate() {
'lastChecked' => 'LastCheckedReturnValue',
'currentChannel' => Util::getChannel(),
'channels' => $channels,
'newVersionString' => '8.1.2',
'newVersion' => '8.1.2',
'newVersionString' => 'Nextcloud 8.1.2',
'downloadLink' => 'https://downloads.nextcloud.org/server',
'changes' => [],
'updaterEnabled' => true,
Expand Down
14 changes: 8 additions & 6 deletions apps/updatenotification/tests/UpdateCheckerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ public function testGetUpdateStateWithUpdateAndInvalidLink() {
->expects($this->once())
->method('check')
->willReturn([
'version' => 123,
'versionstring' => 'Nextcloud 123',
'version' => '1.2.3',
'versionstring' => 'Nextcloud 1.2.3',
'web'=> 'javascript:alert(1)',
'url'=> 'javascript:alert(2)',
'changes' => 'javascript:alert(3)',
Expand All @@ -62,7 +62,8 @@ public function testGetUpdateStateWithUpdateAndInvalidLink() {

$expected = [
'updateAvailable' => true,
'updateVersion' => 'Nextcloud 123',
'updateVersion' => '1.2.3',
'updateVersionString' => 'Nextcloud 1.2.3',
'updaterEnabled' => false,
'versionIsEol' => true,
];
Expand Down Expand Up @@ -91,8 +92,8 @@ public function testGetUpdateStateWithUpdateAndValidLink() {
->expects($this->once())
->method('check')
->willReturn([
'version' => '123',
'versionstring' => 'Nextcloud 123',
'version' => '1.2.3',
'versionstring' => 'Nextcloud 1.2.3',
'web'=> 'https://docs.nextcloud.com/myUrl',
'url'=> 'https://downloads.nextcloud.org/server',
'changes' => 'https://updates.nextcloud.com/changelog_server/?version=123.0.0',
Expand All @@ -106,7 +107,8 @@ public function testGetUpdateStateWithUpdateAndValidLink() {

$expected = [
'updateAvailable' => true,
'updateVersion' => 'Nextcloud 123',
'updateVersion' => '1.2.3',
'updateVersionString' => 'Nextcloud 1.2.3',
'updaterEnabled' => true,
'versionIsEol' => false,
'updateLink' => 'https://docs.nextcloud.com/myUrl',
Expand Down