Merge pull request #4331 from owncloud/improve_app-management

Improve app-management

7 files changed, 92 insertions, 66 deletions

diff --git a/core/css/styles.css b/core/css/styles.css
index 52a265d2031..1e7098d16a2 100644
--- a/core/css/styles.css
+++ b/core/css/styles.css
@@ -389,7 +389,7 @@ label.infield { cursor:text !important; top:1.05em; left:.85em; }
 
 
 /* Warnings and errors are the same */
-.warning, .update, .error {
+#body-login .warning, #body-login .update, #body-login .error {
 	display: block;
 	padding: 10px;
 	color: #dd3b3b;
@@ -401,6 +401,16 @@ label.infield { cursor:text !important; top:1.05em; left:.85em; }
 	border-radius: 5px;
 	cursor: default;
 }
+
+#body-user .warning, #body-settings .warning {
+        margin-top: 8px;
+        padding: 5px;
+        background: #fdd;
+        -webkit-border-radius: 3px;
+        -moz-border-radius: 3px;
+        border-radius: 3px;
+}
+
 .warning legend,
 .warning a,
 .error a {
diff --git a/lib/app.php b/lib/app.php
index f76b92cde1b..8f5dd1d685e 100644
--- a/lib/app.php
+++ b/lib/app.php
@@ -210,7 +210,8 @@ class OC_App{
 	/**
 	 * @brief enables an app
 	 * @param mixed $app app
-	 * @return bool
+	 * @throws \Exception
+	 * @return void
 	 *
 	 * This function set an app as enabled in appconfig.
 	 */
@@ -228,25 +229,25 @@ class OC_App{
 				}
 			}
 		}
+		$l = OC_L10N::get('core');
 		if($app!==false) {
 			// check if the app is compatible with this version of ownCloud
 			$info=OC_App::getAppInfo($app);
 			$version=OC_Util::getVersion();
 			if(!isset($info['require']) or !self::isAppVersionCompatible($version, $info['require'])) {
-				OC_Log::write('core',
-					'App "'.$info['name'].'" can\'t be installed because it is'
-					.' not compatible with this version of ownCloud',
-					OC_Log::ERROR);
-				return false;
+				throw new \Exception(
+					$l->t("App \"%s\" can't be installed because it is not compatible with this version of ownCloud.",
+						array($info['name'])
+					)
+				);
 			}else{
 				OC_Appconfig::setValue( $app, 'enabled', 'yes' );
 				if(isset($appdata['id'])) {
 					OC_Appconfig::setValue( $app, 'ocsid', $appdata['id'] );
 				}
-				return true;
 			}
 		}else{
-			return false;
+			throw new \Exception($l->t("No app name specified"));
 		}
 	}
 
diff --git a/lib/installer.php b/lib/installer.php
index c29f9ec8982..b9684eaeea0 100644
--- a/lib/installer.php
+++ b/lib/installer.php
@@ -27,6 +27,7 @@ class OC_Installer{
 	/**
 	 * @brief Installs an app
 	 * @param $data array with all information
+	 * @throws \Exception
 	 * @returns integer
 	 *
 	 * This function installs an app. All information needed are passed in the
@@ -56,23 +57,22 @@ class OC_Installer{
 	 * needed to get the app working.
 	 */
 	public static function installApp( $data = array()) {
+		$l = \OC_L10N::get('lib');
+
 		if(!isset($data['source'])) {
-			OC_Log::write('core', 'No source specified when installing app', OC_Log::ERROR);
-			return false;
+			throw new \Exception($l->t("No source specified when installing app"));
 		}
 
 		//download the file if necesary
 		if($data['source']=='http') {
 			$path=OC_Helper::tmpFile();
 			if(!isset($data['href'])) {
-				OC_Log::write('core', 'No href specified when installing app from http', OC_Log::ERROR);
-				return false;
+				throw new \Exception($l->t("No href specified when installing app from http"));
 			}
 			copy($data['href'], $path);
 		}else{
 			if(!isset($data['path'])) {
-				OC_Log::write('core', 'No path specified when installing app from local file', OC_Log::ERROR);
-				return false;
+				throw new \Exception($l->t("No path specified when installing app from local file"));
 			}
 			$path=$data['path'];
 		}
@@ -86,8 +86,7 @@ class OC_Installer{
 			rename($path, $path.'.tgz');
 			$path.='.tgz';
 		}else{
-			OC_Log::write('core', 'Archives of type '.$mime.' are not supported', OC_Log::ERROR);
-			return false;
+			throw new \Exception($l->t("Archives of type %s are not supported", array($mime)));
 		}
 
 		//extract the archive in a temporary folder
@@ -97,12 +96,11 @@ class OC_Installer{
 		if($archive=OC_Archive::open($path)) {
 			$archive->extract($extractDir);
 		} else {
-			OC_Log::write('core', 'Failed to open archive when installing app', OC_Log::ERROR);
 			OC_Helper::rmdirr($extractDir);
 			if($data['source']=='http') {
 				unlink($path);
 			}
-			return false;
+			throw new \Exception($l->t("Failed to open archive when installing app"));
 		}
 
 		//load the info.xml file of the app
@@ -118,62 +116,48 @@ class OC_Installer{
 			}
 		}
 		if(!is_file($extractDir.'/appinfo/info.xml')) {
-			OC_Log::write('core', 'App does not provide an info.xml file', OC_Log::ERROR);
 			OC_Helper::rmdirr($extractDir);
 			if($data['source']=='http') {
 				unlink($path);
 			}
-			return false;
+			throw new \Exception($l->t("App does not provide an info.xml file"));
 		}
 		$info=OC_App::getAppInfo($extractDir.'/appinfo/info.xml', true);
 		// check the code for not allowed calls
 		if(!OC_Installer::checkCode($info['id'], $extractDir)) {
-			OC_Log::write('core', 'App can\'t be installed because of not allowed code in the App', OC_Log::ERROR);
 			OC_Helper::rmdirr($extractDir);
-			return false;
+			throw new \Exception($l->t("App can't be installed because of not allowed code in the App"));
 		}
 
 		// check if the app is compatible with this version of ownCloud
 		if(
-            !isset($info['require'])
-            or !OC_App::isAppVersionCompatible(OC_Util::getVersion(), $info['require'])
-        ) {
-			OC_Log::write('core',
-				'App can\'t be installed because it is not compatible with this version of ownCloud',
-				OC_Log::ERROR);
+			!isset($info['require'])
+				or !OC_App::isAppVersionCompatible(OC_Util::getVersion(), $info['require'])
+		) {
 			OC_Helper::rmdirr($extractDir);
-			return false;
+			throw new \Exception($l->t("App can't be installed because it is not compatible with this version of ownCloud"));
 		}
 
 		// check if shipped tag is set which is only allowed for apps that are shipped with ownCloud
 		if(isset($info['shipped']) and ($info['shipped']=='true')) {
-			OC_Log::write('core',
-				'App can\'t be installed because it contains the <shipped>true</shippe>'
-				.' tag which is not allowed for non shipped apps',
-				OC_Log::ERROR);
 			OC_Helper::rmdirr($extractDir);
-			return false;
+			throw new \Exception($l->t("App can't be installed because it contains the <shipped>true</shipped> tag which is not allowed for non shipped apps"));
 		}
 
 		// check if the ocs version is the same as the version in info.xml/version
 		if(!isset($info['version']) or ($info['version']<>$data['appdata']['version'])) {
-			OC_Log::write('core',
-				'App can\'t be installed because the version in info.xml/version is not the same'
-				.' as the version reported from the app store',
-				OC_Log::ERROR);
 			OC_Helper::rmdirr($extractDir);
-			return false;
+			throw new \Exception($l->t("App can't be installed because the version in info.xml/version is not the same as the version reported from the app store"));
 		}
 
 		$basedir=OC_App::getInstallPath().'/'.$info['id'];
 		//check if the destination directory already exists
 		if(is_dir($basedir)) {
-			OC_Log::write('core', 'App directory already exists', OC_Log::WARN);
 			OC_Helper::rmdirr($extractDir);
 			if($data['source']=='http') {
 				unlink($path);
 			}
-			return false;
+			throw new \Exception($l->t("App directory already exists"));
 		}
 
 		if(isset($data['pretent']) and $data['pretent']==true) {
@@ -182,12 +166,11 @@ class OC_Installer{
 
 		//copy the app to the correct place
 		if(@!mkdir($basedir)) {
-			OC_Log::write('core', 'Can\'t create app folder. Please fix permissions. ('.$basedir.')', OC_Log::ERROR);
 			OC_Helper::rmdirr($extractDir);
 			if($data['source']=='http') {
 				unlink($path);
 			}
-			return false;
+			throw new \Exception($l->t("Can't create app folder. Please fix permissions. %s", array($basedir)));
 		}
 		OC_Helper::copyr($extractDir, $basedir);
 
diff --git a/settings/ajax/enableapp.php b/settings/ajax/enableapp.php
index ab84aee5166..735794360b3 100644
--- a/settings/ajax/enableapp.php
+++ b/settings/ajax/enableapp.php
@@ -3,10 +3,10 @@
 OC_JSON::checkAdminUser();
 OCP\JSON::callCheck();
 
-$appid = OC_App::enable(OC_App::cleanAppId($_POST['appid']));
-if($appid !== false) {
-	OC_JSON::success(array('data' => array('appid' => $appid)));
-} else {
-	$l = OC_L10N::get('settings');	
-	OC_JSON::error(array("data" => array( "message" => $l->t("Could not enable app. ") )));
+try {
+	OC_App::enable(OC_App::cleanAppId($_POST['appid']));
+	OC_JSON::success();
+} catch (Exception $e) {
+	OC_Log::write('core', $e->getMessage(), OC_Log::ERROR);
+	OC_JSON::error(array("data" => array("message" => $e->getMessage()) ));
 }
diff --git a/settings/css/settings.css b/settings/css/settings.css
index 20df5d86d65..d5ffe448482 100644
--- a/settings/css/settings.css
+++ b/settings/css/settings.css
@@ -61,7 +61,7 @@ select.quota.active { background: #fff; }
 .ie8 table.hascontrols tbody tr{border-collapse:collapse;border: 1px solid #ddd !important;}
 
 /* APPS */
-.appinfo { margin: 1em; }
+.appinfo { margin: 1em 40px; }
 h3 { font-size: 1.4em; font-weight: bold; }
 ul.applist a {
 	height: 2.2em;
@@ -72,6 +72,12 @@ ul.applist .app-external {
 }
 li { color:#888; }
 li.active { color:#000; }
+#leftcontent .appwarning {
+	background: #fcc;
+}
+#leftcontent .appwarning:hover {
+	background: #fbb;
+}
 small.externalapp { color:#FFF; background-color:#BBB; font-weight:bold; font-size: 0.6em; margin: 0; padding: 0.1em 0.2em; border-radius: 4px;}
 small.externalapp.list { float: right; }
 small.recommendedapp { color:#FFF; background-color:#888; font-weight:bold; font-size: 0.6em; margin: 0; padding: 0.1em 0.2em; border-radius: 4px;}
diff --git a/settings/js/apps.js b/settings/js/apps.js
index cf8cd91bd40..d9817aff6b6 100644
--- a/settings/js/apps.js
+++ b/settings/js/apps.js
@@ -27,7 +27,7 @@ OC.Settings.Apps = OC.Settings.Apps || {
 		}
 		page.find('small.externalapp').attr('style', 'visibility:visible');
 		page.find('span.author').text(app.author);
-		page.find('span.licence').text(app.licence);
+		page.find('span.licence').text(app.license);
 
 		if (app.update !== false) {
 			page.find('input.update').show();
@@ -50,44 +50,64 @@ OC.Settings.Apps = OC.Settings.Apps || {
 			page.find('p.appslink').hide();
 			page.find('span.score').hide();
 		}
+		if (typeof($('#leftcontent li[data-id="'+app.id+'"]').data('errormsg')) !== "undefined") {
+			page.find(".warning").show();
+			page.find(".warning").text($('#leftcontent li[data-id="'+app.id+'"]').data('errormsg'));
+		} else {
+			page.find(".warning").hide();
+		}
 	},
 	enableApp:function(appid, active, element) {
 		console.log('enableApp:', appid, active, element);
 		var appitem=$('#leftcontent li[data-id="'+appid+'"]');
-		var appData = appitem.data('app');
-		appData.active = !active;
-		appitem.data('app', appData);
 		element.val(t('settings','Please wait....'));
 		if(active) {
 			$.post(OC.filePath('settings','ajax','disableapp.php'),{appid:appid},function(result) {
 				if(!result || result.status !== 'success') {
-					OC.dialogs.alert('Error while disabling app', t('core', 'Error'));
+					if (result.data && result.data.message) {
+						OC.Settings.Apps.showErrorMessage(result.data.message);
+						appitem.data('errormsg', result.data.message);
+					} else {
+						OC.Settings.Apps.showErrorMessage(t('settings', 'Error while disabling app'));
+						appitem.data('errormsg', t('settings', 'Error while disabling app'));
+					}
+					element.val(t('settings','Disable'));
+					appitem.addClass('appwarning');
 				}
 				else {
-					element.data('active',false);
+					appitem.data('active',false);
 					OC.Settings.Apps.removeNavigation(appid);
+					appitem.removeClass('active');
 					element.val(t('settings','Enable'));
 				}
 			},'json');
-			$('#leftcontent li[data-id="'+appid+'"]').removeClass('active');
 		} else {
 			$.post(OC.filePath('settings','ajax','enableapp.php'),{appid:appid},function(result) {
 				if(!result || result.status !== 'success') {
-					OC.dialogs.alert('Error while enabling app', t('core', 'Error'));
-				}
-				else {
+					if (result.data && result.data.message) {
+						OC.Settings.Apps.showErrorMessage(result.data.message);
+						appitem.data('errormsg', result.data.message);
+					} else {
+						OC.Settings.Apps.showErrorMessage(t('settings', 'Error while enabling app'));
+						appitem.data('errormsg', t('settings', 'Error while disabling app'));
+					}
+					element.val(t('settings','Enable'));
+					appitem.addClass('appwarning');
+				} else {
 					OC.Settings.Apps.addNavigation(appid);
-					element.data('active',true);
+					appitem.data('active',true);
+					appitem.addClass('active');
 					element.val(t('settings','Disable'));
 				}
 			},'json')
 			.fail(function() { 
-				OC.dialogs.alert('Error while enabling app', t('core', 'Error'));
-				element.data('active',false);
+				OC.Settings.Apps.showErrorMessage(t('settings', 'Error while enabling app'));
+				appitem.data('errormsg', t('settings', 'Error while enabling app'));
+				appitem.data('active',false);
+				appitem.addClass('appwarning');
 				OC.Settings.Apps.removeNavigation(appid);
 				element.val(t('settings','Enable'));
 			});
-			$('#leftcontent li[data-id="'+appid+'"]').addClass('active');
 		}
 	},
 	updateApp:function(appid, element) {
@@ -95,7 +115,8 @@ OC.Settings.Apps = OC.Settings.Apps || {
 		element.val(t('settings','Updating....'));
 		$.post(OC.filePath('settings','ajax','updateapp.php'),{appid:appid},function(result) {
 			if(!result || result.status !== 'success') {
-				OC.dialogs.alert(t('settings','Error while updating app'),t('settings','Error'));
+				OC.Settings.Apps.showErrorMessage(t('settings','Error while updating app'),t('settings','Error'));
+				element.val(t('settings','Update'));
 			}
 			else {
 				element.val(t('settings','Updated'));
@@ -167,6 +188,10 @@ OC.Settings.Apps = OC.Settings.Apps || {
 				}
 			}
 		});
+	},
+	showErrorMessage: function(message) {
+		$('.appinfo .warning').show();
+		$('.appinfo .warning').text(message);
 	}
 };
 
diff --git a/settings/templates/apps.php b/settings/templates/apps.php
index d60fd82f917..0b76f775fea 100644
--- a/settings/templates/apps.php
+++ b/settings/templates/apps.php
@@ -41,5 +41,6 @@
 		print_unescaped($l->t('<span class="licence"></span>-licensed by <span class="author"></span>'));?></p>
 	<input class="enable hidden" type="submit" />
 	<input class="update hidden" type="submit" value="<?php p($l->t('Update')); ?>" />
+	<div class="warning hidden"></div>
 	</div>
 </div>