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

Display app names in update page for app updates #17434

Merged
merged 2 commits into from
Aug 20, 2015
Merged

Display app names in update page for app updates #17434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b919ae9
Display app names in update page for app updates
Jul 7, 2015
a2674b2
Additions to update page
Aug 20, 2015
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
7 changes: 7 additions & 0 deletions core/css/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,13 @@ label.infield {
color: #ccc;
}

#body-login .update .appList {
list-style: disc;
text-align: left;
margin-left: 25px;
margin-right: 25px;
}

#body-login .v-align {
width: inherit;
}
Expand Down
20 changes: 17 additions & 3 deletions core/templates/update.admin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,26 @@
<div class="update" data-productname="<?php p($_['productName']) ?>" data-version="<?php p($_['version']) ?>">
<div class="updateOverview">
<?php if ($_['isAppsOnlyUpgrade']) { ?>
Copy link
Member

Choose a reason for hiding this comment

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

Do app upgrades not occur at the same time as core upgrades? If they do, then we should surely display 'The following apps will be updated' alongside 'ownCloud will be updated'?

Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell (without doing an actual upgrade), we will see:

ownCloud will be updated to version x

External file storage (files_external)
My awesome app (xenopathic_is_awesome)

IMHO we still need to display 'The following apps will be updated' under 'ownCloud will be updated'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I also thought about this.
I'll try and add them to both pages.

<h2 class="title bold"><?php p($l->t('Apps update required.')); ?></h2>
Copy link
Member

Choose a reason for hiding this comment

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

This should say »App update required« (App without s, no dot at the end.)

Copy link
Contributor

Choose a reason for hiding this comment

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

#18452

Copy link
Member

Choose a reason for hiding this comment

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

Pull request at #18453

<?php } else { ?>
<h2 class="title bold"><?php p($l->t('%s will be updated to version %s.',
array($_['productName'], $_['version']))); ?></h2>
<?php if (!empty($_['appList'])) { ?>
<?php } ?>
<?php if (!empty($_['appsToUpgrade'])) { ?>
<div class="infogroup">
<span class="bold"><?php p($l->t('These apps will be updated:')); ?></span>
<ul class="content appList">
<?php foreach ($_['appsToUpgrade'] as $appInfo) { ?>
<li><?php p($appInfo['name']) ?> (<?php p($appInfo['id']) ?>)</li>
<?php } ?>
</ul>
</div>
<?php } ?>
<?php if (!empty($_['incompatibleAppsList'])) { ?>
<div class="infogroup">
<span class="bold"><?php p($l->t('The following apps will be disabled:')) ?></span>
<span class="bold"><?php p($l->t('These incompatible apps will be disabled:')) ?></span>
<ul class="content appList">
<?php foreach ($_['appList'] as $appInfo) { ?>
<?php foreach ($_['incompatibleAppsList'] as $appInfo) { ?>
<li><?php p($appInfo['name']) ?> (<?php p($appInfo['id']) ?>)</li>
<?php } ?>
</ul>
Expand Down
57 changes: 36 additions & 21 deletions lib/base.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,27 +346,7 @@ public static function checkUpgrade($showTemplate = true) {
if (\OCP\Util::needUpgrade()) {
$systemConfig = \OC::$server->getSystemConfig();
if ($showTemplate && !$systemConfig->getValue('maintenance', false)) {
$version = OC_Util::getVersion();
$oldTheme = $systemConfig->getValue('theme');
$systemConfig->setValue('theme', '');
OC_Util::addScript('config'); // needed for web root
OC_Util::addScript('update');
$tmpl = new OC_Template('', 'update.admin', 'guest');
$tmpl->assign('version', OC_Util::getVersionString());

// get third party apps
$apps = OC_App::getEnabledApps();
$incompatibleApps = array();
foreach ($apps as $appId) {
$info = OC_App::getAppInfo($appId);
if(!OC_App::isAppCompatible($version, $info)) {
$incompatibleApps[] = $info;
}
}
$tmpl->assign('appList', $incompatibleApps);
$tmpl->assign('productName', 'ownCloud'); // for now
$tmpl->assign('oldTheme', $oldTheme);
$tmpl->printPage();
self::printUpgradePage();
exit();
} else {
return true;
Expand All @@ -375,6 +355,41 @@ public static function checkUpgrade($showTemplate = true) {
return false;
}

/**
* Prints the upgrade page
*/
private static function printUpgradePage() {
$systemConfig = \OC::$server->getSystemConfig();
$oldTheme = $systemConfig->getValue('theme');
$systemConfig->setValue('theme', '');
Copy link
Member

Choose a reason for hiding this comment

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

This is a thing?! 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly... this code was just moved here, I didn't write it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Please leave it. It caused a lot of trouble in the past.

\OCP\Util::addScript('config'); // needed for web root
\OCP\Util::addScript('update');

// check whether this is a core update or apps update
$installedVersion = $systemConfig->getValue('version', '0.0.0');
$currentVersion = implode('.', OC_Util::getVersion());

$appManager = \OC::$server->getAppManager();

$tmpl = new OC_Template('', 'update.admin', 'guest');
$tmpl->assign('version', OC_Util::getVersionString());

// if not a core upgrade, then it's apps upgrade
if (version_compare($currentVersion, $installedVersion, '=')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the part I added

$tmpl->assign('isAppsOnlyUpgrade', true);
} else {
$tmpl->assign('isAppsOnlyUpgrade', false);
}

// get third party apps
$ocVersion = OC_Util::getVersion();
$tmpl->assign('appsToUpgrade', $appManager->getAppsNeedingUpgrade($ocVersion));
$tmpl->assign('incompatibleAppsList', $appManager->getIncompatibleApps($ocVersion));
$tmpl->assign('productName', 'ownCloud'); // for now
$tmpl->assign('oldTheme', $oldTheme);
$tmpl->printPage();
}

public static function initTemplateEngine() {
// Add the stuff we need always
// following logic will import all vendor libraries that are
Expand Down
69 changes: 69 additions & 0 deletions lib/private/app/appmanager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,4 +209,73 @@ public function clearAppsCache() {
$settingsMemCache = $this->memCacheFactory->create('settings');
$settingsMemCache->clear('listApps');
}

/**
* Returns a list of apps that need upgrade
*
* @param array $version ownCloud version as array of version components
* @return array list of app info from apps that need an upgrade
*
* @internal
*/
public function getAppsNeedingUpgrade($ocVersion) {
$appsToUpgrade = [];
$apps = $this->getInstalledApps();
foreach ($apps as $appId) {
$appInfo = $this->getAppInfo($appId);
$appDbVersion = $this->appConfig->getValue($appId, 'installed_version');
if ($appDbVersion
&& isset($appInfo['version'])
&& version_compare($appInfo['version'], $appDbVersion, '>')
&& \OC_App::isAppCompatible($ocVersion, $appInfo)
) {
$appsToUpgrade[] = $appInfo;
}
}

return $appsToUpgrade;
}

/**
* Returns the app information from "appinfo/info.xml".
*
* If no version was present in "appinfo/info.xml", reads it
* from the external "appinfo/version" file instead.
*
* @param string $appId app id
*
* @return array app iinfo
*
* @internal
*/
public function getAppInfo($appId) {
$appInfo = \OC_App::getAppInfo($appId);
if (!isset($appInfo['version'])) {
// read version from separate file
$appInfo['version'] = \OC_App::getAppVersion($appId);
}
return $appInfo;
}

/**
* Returns a list of apps incompatible with the given version
*
* @param array $version ownCloud version as array of version components
*
* @return array list of app info from incompatible apps
*
* @internal
*/
public function getIncompatibleApps($version) {
$apps = $this->getInstalledApps();
$incompatibleApps = array();
foreach ($apps as $appId) {
$info = $this->getAppInfo($appId);
if (!\OC_App::isAppCompatible($version, $info)) {
$incompatibleApps[] = $info;
}
}
return $incompatibleApps;
}

}
70 changes: 70 additions & 0 deletions tests/lib/app/manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,4 +204,74 @@ public function testGetAppsForUser() {
$this->appConfig->setValue('test4', 'enabled', '["asd"]');
$this->assertEquals(['test1', 'test3'], $this->manager->getEnabledAppsForUser($user));
}

public function testGetAppsNeedingUpgrade() {
$this->manager = $this->getMockBuilder('\OC\App\AppManager')
->setConstructorArgs([$this->userSession, $this->appConfig, $this->groupManager, $this->cacheFactory])
->setMethods(['getAppInfo'])
->getMock();

$appInfos = [
'test1' => ['id' => 'test1', 'version' => '1.0.1', 'requiremax' => '9.0.0'],
'test2' => ['id' => 'test2', 'version' => '1.0.0', 'requiremin' => '8.2.0'],
'test3' => ['id' => 'test3', 'version' => '1.2.4', 'requiremin' => '9.0.0'],
'test4' => ['id' => 'test4', 'version' => '3.0.0', 'requiremin' => '8.1.0'],
'testnoversion' => ['id' => 'testnoversion', 'requiremin' => '8.2.0'],
];

$this->manager->expects($this->any())
->method('getAppInfo')
->will($this->returnCallback(
function($appId) use ($appInfos) {
return $appInfos[$appId];
}
));

$this->appConfig->setValue('test1', 'enabled', 'yes');
$this->appConfig->setValue('test1', 'installed_version', '1.0.0');
$this->appConfig->setValue('test2', 'enabled', 'yes');
$this->appConfig->setValue('test2', 'installed_version', '1.0.0');
$this->appConfig->setValue('test3', 'enabled', 'yes');
$this->appConfig->setValue('test3', 'installed_version', '1.0.0');
$this->appConfig->setValue('test4', 'enabled', 'yes');
$this->appConfig->setValue('test4', 'installed_version', '2.4.0');

$apps = $this->manager->getAppsNeedingUpgrade('8.2.0');

$this->assertCount(2, $apps);
$this->assertEquals('test1', $apps[0]['id']);
$this->assertEquals('test4', $apps[1]['id']);
}

public function testGetIncompatibleApps() {
$this->manager = $this->getMockBuilder('\OC\App\AppManager')
->setConstructorArgs([$this->userSession, $this->appConfig, $this->groupManager, $this->cacheFactory])
->setMethods(['getAppInfo'])
->getMock();

$appInfos = [
'test1' => ['id' => 'test1', 'version' => '1.0.1', 'requiremax' => '8.0.0'],
'test2' => ['id' => 'test2', 'version' => '1.0.0', 'requiremin' => '8.2.0'],
'test3' => ['id' => 'test3', 'version' => '1.2.4', 'requiremin' => '9.0.0'],
'testnoversion' => ['id' => 'testnoversion', 'requiremin' => '8.2.0'],
];

$this->manager->expects($this->any())
->method('getAppInfo')
->will($this->returnCallback(
function($appId) use ($appInfos) {
return $appInfos[$appId];
}
));

$this->appConfig->setValue('test1', 'enabled', 'yes');
$this->appConfig->setValue('test2', 'enabled', 'yes');
$this->appConfig->setValue('test3', 'enabled', 'yes');

$apps = $this->manager->getIncompatibleApps('8.2.0');

$this->assertCount(2, $apps);
$this->assertEquals('test1', $apps[0]['id']);
$this->assertEquals('test3', $apps[1]['id']);
}
}