fix(settings): Rework download/install/update/remove app handling and respect read-only app rootsfix/settings/read-only-apps-root

Signed-off-by: provokateurin <kate@provokateurin.de>

7 files changed, 77 insertions, 52 deletions

diff --git a/apps/settings/lib/Controller/AppSettingsController.php b/apps/settings/lib/Controller/AppSettingsController.php
index df563ac46b7..0281ca8b43b 100644
--- a/apps/settings/lib/Controller/AppSettingsController.php
+++ b/apps/settings/lib/Controller/AppSettingsController.php
@@ -347,7 +347,6 @@ class AppSettingsController extends Controller {
 				}
 			}
 			$appData['groups'] = $groups;
-			$appData['canUnInstall'] = !$appData['active'] && $appData['removable'];
 
 			// fix licence vs license
 			if (isset($appData['license']) && !isset($appData['licence'])) {
@@ -381,6 +380,14 @@ class AppSettingsController extends Controller {
 	 * @throws \Exception
 	 */
 	private function getAppsForCategory($requestedCategory = ''): array {
+		$anyAppsRootWritable = false;
+		foreach (\OC::$APPSROOTS as $appsRoot) {
+			if ($appsRoot['writable'] ?? false) {
+				$anyAppsRootWritable = true;
+				break;
+			}
+		}
+
 		$versionParser = new VersionParser();
 		$formattedApps = [];
 		$apps = $this->appFetcher->get();
@@ -411,11 +418,23 @@ class AppSettingsController extends Controller {
 			}
 			$phpVersion = $versionParser->getVersion($app['releases'][0]['rawPhpVersionSpec']);
 
+			$needsDownload = true;
+			$canUpdate = false;
+			$canUnInstall = false;
+
 			try {
-				$this->appManager->getAppPath($app['id']);
-				$existsLocally = true;
+				$appPath = $this->appManager->getAppPath($app['id']);
+				$needsDownload = false;
+
+				$appRootPath = dirname($appPath);
+				foreach (\OC::$APPSROOTS as $appsRoot) {
+					if ($appsRoot['path'] === $appRootPath) {
+						$appsRootWritable = $appsRoot['writable'] ?? false;
+						$canUpdate = $appsRootWritable;
+						$canUnInstall = $appsRootWritable;
+					}
+				}
 			} catch (AppPathNotFoundException) {
-				$existsLocally = false;
 			}
 
 			$phpDependencies = [];
@@ -482,9 +501,11 @@ class AppSettingsController extends Controller {
 				'score' => $app['ratingOverall'],
 				'ratingNumOverall' => $app['ratingNumOverall'],
 				'ratingNumThresholdReached' => $app['ratingNumOverall'] > 5,
-				'removable' => $existsLocally,
-				'active' => $this->appManager->isEnabledForUser($app['id']),
-				'needsDownload' => !$existsLocally,
+				'canDownload' => $anyAppsRootWritable,
+				'canUpdate' => $canUpdate,
+				'canUnInstall' => $canUnInstall && !$this->appManager->isShipped($app['id']),
+				'active' => $this->appManager->isEnabledForAnyone($app['id']),
+				'needsDownload' => $needsDownload,
 				'groups' => $groups,
 				'fromAppStore' => true,
 				'appstoreData' => $app,
diff --git a/apps/settings/src/app-types.ts b/apps/settings/src/app-types.ts
index 49f0d5a1709..fa181120420 100644
--- a/apps/settings/src/app-types.ts
+++ b/apps/settings/src/app-types.ts
@@ -44,9 +44,10 @@ export interface IAppstoreApp {
 	app_api: boolean
 	active: boolean
 	internal: boolean
-	removable: boolean
 	installed: boolean
+	canDownload: boolean
 	canInstall: boolean
+	canUpdate: boolean
 	canUnInstall: boolean
 	isCompatible: boolean
 	needsDownload: boolean
diff --git a/apps/settings/src/components/AppList.vue b/apps/settings/src/components/AppList.vue
index cfc778fe409..2016ad1e89a 100644
--- a/apps/settings/src/components/AppList.vue
+++ b/apps/settings/src/components/AppList.vue
@@ -17,6 +17,7 @@
 					<NcButton v-if="showUpdateAll"
 						id="app-list-update-all"
 						type="primary"
+						:disabled="!canUpdateAny"
 						@click="updateAll">
 						{{ n('settings', 'Update', 'Update all', counter) }}
 					</NcButton>
@@ -194,6 +195,9 @@ export default {
 		showUpdateAll() {
 			return this.hasPendingUpdate && this.useListView
 		},
+		canUpdateAny() {
+			return this.apps.filter(app => app.update && app.canUpdate).length > 0
+		},
 		apps() {
 			// Exclude ExApps from the list if AppAPI is disabled
 			const exApps = this.$store.getters.isAppApiEnabled ? this.appApiStore.getAllApps : []
@@ -324,7 +328,7 @@ export default {
 		updateAll() {
 			const limit = pLimit(1)
 			this.apps
-				.filter(app => app.update)
+				.filter(app => app.update && app.canUpdate)
 				.map((app) => limit(() => {
 					this.update(app.id)
 				}))
diff --git a/apps/settings/src/components/AppList/AppItem.vue b/apps/settings/src/components/AppList/AppItem.vue
index d0f39f3c74a..8b0b5a2a2a5 100644
--- a/apps/settings/src/components/AppList/AppItem.vue
+++ b/apps/settings/src/components/AppList/AppItem.vue
@@ -78,15 +78,15 @@
 			<div v-if="isLoading || isInitializing" class="icon icon-loading-small" />
 			<NcButton v-if="app.update"
 				type="primary"
-				:disabled="installing || isLoading || !defaultDeployDaemonAccessible || isManualInstall"
+				:disabled="installing || isLoading || !defaultDeployDaemonAccessible || isManualInstall || !app.canUpdate"
 				:title="updateButtonText"
 				@click.stop="update(app.id)">
 				{{ t('settings', 'Update to {update}', {update:app.update}) }}
 			</NcButton>
-			<NcButton v-if="app.canUnInstall"
+			<NcButton v-if="app.installed"
 				class="uninstall"
 				type="tertiary"
-				:disabled="installing || isLoading"
+				:disabled="installing || isLoading || !app.canUnInstall"
 				@click.stop="remove(app.id)">
 				{{ t('settings', 'Remove') }}
 			</NcButton>
@@ -95,22 +95,24 @@
 				@click.stop="disable(app.id)">
 				{{ disableButtonText }}
 			</NcButton>
-			<NcButton v-if="!app.active && (app.canInstall || app.isCompatible)"
-				:title="enableButtonTooltip"
-				:aria-label="enableButtonTooltip"
-				type="primary"
-				:disabled="!app.canInstall || installing || isLoading || !defaultDeployDaemonAccessible || isInitializing || isDeploying"
-				@click.stop="enable(app.id)">
-				{{ enableButtonText }}
-			</NcButton>
-			<NcButton v-else-if="!app.active"
-				:title="forceEnableButtonTooltip"
-				:aria-label="forceEnableButtonTooltip"
-				type="secondary"
-				:disabled="installing || isLoading || !defaultDeployDaemonAccessible"
-				@click.stop="forceEnable(app.id)">
-				{{ forceEnableButtonText }}
-			</NcButton>
+			<div v-if="!app.active && (!app.needsDownload || app.canDownload)">
+				<NcButton v-if="app.canInstall || app.isCompatible"
+					:title="enableButtonTooltip"
+					:aria-label="enableButtonTooltip"
+					type="primary"
+					:disabled="installing || isLoading || !defaultDeployDaemonAccessible || isInitializing || isDeploying"
+					@click.stop="enable(app.id)">
+					{{ enableButtonText }}
+				</NcButton>
+				<NcButton v-else
+					:title="forceEnableButtonTooltip"
+					:aria-label="forceEnableButtonTooltip"
+					type="secondary"
+					:disabled="installing || isLoading || !defaultDeployDaemonAccessible"
+					@click.stop="forceEnable(app.id)">
+					{{ forceEnableButtonText }}
+				</NcButton>
+			</div>
 		</component>
 	</component>
 </template>
diff --git a/apps/settings/src/components/AppStoreSidebar/AppDetailsTab.vue b/apps/settings/src/components/AppStoreSidebar/AppDetailsTab.vue
index 3aa42f1d15a..3b37d401219 100644
--- a/apps/settings/src/components/AppStoreSidebar/AppDetailsTab.vue
+++ b/apps/settings/src/components/AppStoreSidebar/AppDetailsTab.vue
@@ -47,13 +47,13 @@
 						class="update primary"
 						type="button"
 						:value="t('settings', 'Update to {version}', { version: app.update })"
-						:disabled="installing || isLoading || isManualInstall"
+						:disabled="installing || isLoading || isManualInstall || !app.canUpdate"
 						@click="update(app.id)">
-					<input v-if="app.canUnInstall"
+					<input v-if="app.installed"
 						class="uninstall"
 						type="button"
 						:value="t('settings', 'Remove')"
-						:disabled="installing || isLoading"
+						:disabled="installing || isLoading || !app.canUnInstall"
 						@click="remove(app.id, removeData)">
 					<input v-if="app.active"
 						class="enable"
@@ -61,22 +61,24 @@
 						:value="disableButtonText"
 						:disabled="installing || isLoading || isInitializing || isDeploying"
 						@click="disable(app.id)">
-					<input v-if="!app.active && (app.canInstall || app.isCompatible)"
-						:title="enableButtonTooltip"
-						:aria-label="enableButtonTooltip"
-						class="enable primary"
-						type="button"
-						:value="enableButtonText"
-						:disabled="!app.canInstall || installing || isLoading || !defaultDeployDaemonAccessible || isInitializing || isDeploying"
-						@click="enable(app.id)">
-					<input v-else-if="!app.active && !app.canInstall"
-						:title="forceEnableButtonTooltip"
-						:aria-label="forceEnableButtonTooltip"
-						class="enable force"
-						type="button"
-						:value="forceEnableButtonText"
-						:disabled="installing || isLoading"
-						@click="forceEnable(app.id)">
+					<div v-if="!app.active && (!app.needsDownload || app.canDownload)">
+						<input v-if="app.canInstall || app.isCompatible"
+							:title="enableButtonTooltip"
+							:aria-label="enableButtonTooltip"
+							class="enable primary"
+							type="button"
+							:value="enableButtonText"
+							:disabled="installing || isLoading || !defaultDeployDaemonAccessible || isInitializing || isDeploying"
+							@click="enable(app.id)">
+						<input v-else
+							:title="forceEnableButtonTooltip"
+							:aria-label="forceEnableButtonTooltip"
+							class="enable force"
+							type="button"
+							:value="forceEnableButtonText"
+							:disabled="installing || isLoading"
+							@click="forceEnable(app.id)">
+					</div>
 					<NcButton v-if="app?.app_api && (app.canInstall || app.isCompatible)"
 						:aria-label="t('settings', 'Advanced deploy options')"
 						type="secondary"
diff --git a/apps/settings/src/store/apps.js b/apps/settings/src/store/apps.js
index c58651a3cf5..eaeb11d8c98 100644
--- a/apps/settings/src/store/apps.js
+++ b/apps/settings/src/store/apps.js
@@ -87,9 +87,6 @@ const mutations = {
 		const app = state.apps.find(app => app.id === appId)
 		app.active = false
 		app.groups = []
-		if (app.removable) {
-			app.canUnInstall = true
-		}
 		if (app.id === 'app_api') {
 			state.appApiEnabled = false
 		}
diff --git a/lib/private/legacy/OC_App.php b/lib/private/legacy/OC_App.php
index 7fee946b776..e65ab09fa7e 100644
--- a/lib/private/legacy/OC_App.php
+++ b/lib/private/legacy/OC_App.php
@@ -510,10 +510,8 @@ class OC_App {
 				if ($appManager->isShipped($app)) {
 					$info['internal'] = true;
 					$info['level'] = self::officialApp;
-					$info['removable'] = false;
 				} else {
 					$info['internal'] = false;
-					$info['removable'] = true;
 				}
 
 				if (in_array($app, $supportedApps)) {