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

Check for compatible apps from the appstore fails #12358

Merged
merged 4 commits into from
Nov 9, 2018

Conversation

nickvergessen
Copy link
Member

based on #12354

MorrisJobke and others added 2 commits November 8, 2018 14:19
Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@@ -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.

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

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Make sense!! 🦅

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍

I added one CSS rule to give the app list a little more margin to the bottom (same as the <p> element have on that page).

Before and after:

bildschirmfoto 2018-11-08 um 15 27 48
bildschirmfoto 2018-11-08 um 15 28 22

@MorrisJobke MorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish backport-request and removed 3. to review Waiting for reviews labels Nov 8, 2018
@nickvergessen
Copy link
Member Author

👍 since I cant approve my own PR although it has commits from you

@MorrisJobke
Copy link
Member

@nickvergessen Failing unit tests 🙈

Signed-off-by: Joas Schilling <coding@schilljs.com>
@MorrisJobke MorrisJobke merged commit b7a59fb into master Nov 9, 2018
@MorrisJobke MorrisJobke deleted the bugfix/noid/wrong-update-check-version branch November 9, 2018 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish feature: activity and notification feature: install and update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants