Merge pull request #4273 from nextcloud/backport/4263/stable31

Author: provokateurin

lib/Controller/FolderController.php

@@ -39,13 +39,6 @@
 class FolderController extends OCSController {
 	private ?IUser $user;
 
-	protected const ALLOWED_ORDER_BY = [
-		'mount_point',
-		'quota',
-		'groups',
-		'acl',
-	];
-
 	public function __construct(
 		string $AppName,
 		IRequest $request,
@@ -102,7 +95,8 @@ private function formatFolder(array $folder): array {
 	 * @param bool $applicable Filter by applicable groups
 	 * @param non-negative-int $offset Number of items to skip.
 	 * @param ?positive-int $limit Number of items to return.
-	 * @param null|'mount_point'|'quota'|'groups'|'acl' $orderBy The key to order by
+	 * @param 'mount_point'|'quota'|'groups'|'acl' $orderBy The key to order by
+	 * @param 'asc'|'desc' $order Sort ascending or descending
 	 * @return DataResponse<Http::STATUS_OK, array<string, GroupFoldersFolder>, array{}>
 	 * @throws OCSNotFoundException Storage not found
 	 * @throws OCSBadRequestException Wrong limit used
@@ -111,58 +105,39 @@ private function formatFolder(array $folder): array {
 	 */
 	#[NoAdminRequired]
 	#[FrontpageRoute(verb: 'GET', url: '/folders')]
-	public function getFolders(bool $applicable = false, int $offset = 0, ?int $limit = null, ?string $orderBy = 'mount_point'): DataResponse {
+	public function getFolders(bool $applicable = false, int $offset = 0, ?int $limit = null, string $orderBy = 'mount_point', string $order = 'asc'): DataResponse {
+		/** @psalm-suppress DocblockTypeContradiction */
 		if ($limit !== null && $limit <= 0) {
 			throw new OCSBadRequestException('The limit must be greater than 0.');
 		}
 
+		/** @psalm-suppress DocblockTypeContradiction */
+		if (!in_array($orderBy, ['mount_point', 'quota', 'groups', 'acl'], true)) {
+			throw new OCSBadRequestException('The orderBy is not allowed.');
+		}
+
+		/** @psalm-suppress DocblockTypeContradiction */
+		if (!in_array($order, ['asc', 'desc'], true)) {
+			throw new OCSBadRequestException('The order is not allowed.');
+		}
+
 		$storageId = $this->getRootFolderStorageId();
 		if ($storageId === null) {
 			throw new OCSNotFoundException();
 		}
 
 		$folders = [];
-		foreach ($this->manager->getAllFoldersWithSize($storageId) as $id => $folder) {
+		$i = 0;
+		foreach ($this->manager->getAllFoldersWithSize($storageId, $offset, $limit, $orderBy, $order) as $id => $folder) {
 			// Make them string-indexed for OpenAPI JSON output
-			$folders[(string)$id] = $this->formatFolder($folder);
-		}
-
-		$orderBy = in_array($orderBy, self::ALLOWED_ORDER_BY, true)
-			? $orderBy
-			: 'mount_point';
-
-		// in case of equal orderBy value always fall back to the mount_point - same as on the frontend
-		/**
-		 * @var GroupFoldersFolder $a
-		 * @var GroupFoldersFolder $b
-		 */
-		uasort($folders, function (array $a, array $b) use ($orderBy) {
-			if ($orderBy === 'groups') {
-				if (($value = count($a['groups']) - count($b['groups'])) !== 0) {
-					return $value;
-				}
-			} else {
-				if (($value = $this->compareFolderNames((string)($a[$orderBy] ?? ''), (string)($b[$orderBy] ?? ''))) !== 0) {
-					return $value;
-				}
-			}
-
-			// fallback to mount_point
-			if (($value = $this->compareFolderNames($a['mount_point'] ?? '', $b['mount_point'])) !== 0) {
-				return $value;
-			}
-
-			// fallback to id
-			return $a['id'] - $b['id'];
-		});
+			// JavaScript doesn't preserve JSON object key orders, so we need to manually add this information.
+			$folders[(string)$id] = array_merge($this->formatFolder($folder), [
+				'sortIndex' => $offset + $i++,
+			]);
+		}
 
 		$isAdmin = $this->delegationService->isAdminNextcloud() || $this->delegationService->isDelegatedAdmin();
 		if ($isAdmin && !$applicable) {
-			// If only the default values are provided the pagination can be skipped.
-			if ($offset !== 0 || $limit !== null) {
-				$folders = array_slice($folders, $offset, $limit, true);
-			}
-
 			return new DataResponse($folders);
 		}
 
@@ -174,11 +149,6 @@ public function getFolders(bool $applicable = false, int $offset = 0, ?int $limi
 			$folders = array_filter(array_map($this->filterNonAdminFolder(...), $folders));
 		}
 
-		// If only the default values are provided the pagination can be skipped.
-		if ($offset !== 0 || $limit !== null) {
-			$folders = array_slice($folders, $offset, $limit, true);
-		}
-
 		return new DataResponse($folders);
 	}
 

lib/Folder/FolderManager.php

@@ -142,15 +142,30 @@ private function joinQueryWithFileCache(IQueryBuilder $query, int $rootStorageId
 	 * @return array<int, InternalFolderOut>
 	 * @throws Exception
 	 */
-	public function getAllFoldersWithSize(int $rootStorageId): array {
+	public function getAllFoldersWithSize(int $rootStorageId, int $offset = 0, ?int $limit = null, string $orderBy = 'mount_point', string $order = 'ASC'): array {
 		$applicableMap = $this->getAllApplicable();
 
 		$query = $this->connection->getQueryBuilder();
 
-		$query->select('folder_id', 'mount_point', 'quota', 'c.size', 'acl')
+		$query->select('f.folder_id', 'mount_point', 'quota', 'c.size', 'acl')
 			->from('group_folders', 'f');
 		$this->joinQueryWithFileCache($query, $rootStorageId);
 
+		$query->setFirstResult($offset);
+		$query->setMaxResults($limit);
+		if ($orderBy === 'groups') {
+			$query
+				->leftJoin('f', 'group_folders_groups', 'g', $query->expr()->eq('f.folder_id', 'g.folder_id'))
+				->groupBy('f.folder_id')
+				->orderBy($query->func()->count('g.applicable_id'), $order);
+		} else {
+			$query->orderBy($orderBy, $order);
+		}
+		// Fallback in case two rows are the same after ordering by the $orderBy
+		if ($orderBy !== 'mount_point') {
+			$query->addOrderBy('mount_point', 'ASC');
+		}
+
 		$rows = $query->executeQuery()->fetchAll();
 
 		$folderMappings = $this->getAllFolderMappings();

lib/ResponseDefinitions.php

@@ -54,6 +54,7 @@
  *     size: int,
  *     acl: bool,
  *     manage: list<GroupFoldersAclManage>,
+ *     sortIndex?: int,
  * }
  */
 class ResponseDefinitions {

openapi.json

@@ -182,6 +182,10 @@
                         "items": {
                             "$ref": "#/components/schemas/AclManage"
                         }
+                    },
+                    "sortIndex": {
+                        "type": "integer",
+                        "format": "int64"
                     }
                 }
             },
@@ -500,7 +504,6 @@
                         "description": "The key to order by",
                         "schema": {
                             "type": "string",
-                            "nullable": true,
                             "default": "mount_point",
                             "enum": [
                                 "mount_point",
@@ -510,6 +513,19 @@
                             ]
                         }
                     },
+                    {
+                        "name": "order",
+                        "in": "query",
+                        "description": "Sort ascending or descending",
+                        "schema": {
+                            "type": "string",
+                            "default": "asc",
+                            "enum": [
+                                "asc",
+                                "desc"
+                            ]
+                        }
+                    },
                     {
                         "name": "OCS-APIRequest",
                         "in": "header",

src/settings/Api.ts

@@ -15,12 +15,13 @@ export class Api {
 		return generateUrl(`apps/groupfolders/${endpoint}`)
 	}
 
-	async listFolders(offset = 0, limit?: number, orderBy?: string): Promise<Folder[]> {
+	async listFolders(offset = 0, limit?: number, orderBy?: string, order?: string): Promise<Folder[]> {
 		const response = await axios.get<OCSResponse<Folder[]>>(this.getUrl('folders'), {
 			params: {
 				offset,
 				limit,
 				orderBy,
+				order,
 			},
 		})
 		return Object.values(response.data.ocs.data)

src/settings/App.tsx

@@ -74,7 +74,7 @@ export class App extends Component<unknown, AppState> implements OC.Plugin<OC.Se
 
 	componentDidMount() {
 		// list first pageSize + 1 folders so we know if there are more pages
-		this.api.listFolders(0, pageSize + 1, this.state.sort).then((folders) => {
+		this.api.listFolders(0, pageSize + 1, this.state.sort, this.state.sortOrder === 1 ? 'asc' : 'desc').then((folders) => {
 			this.setState({ folders })
 		})
 		this.api.listGroups().then((groups) => {
@@ -188,11 +188,25 @@ export class App extends Component<unknown, AppState> implements OC.Plugin<OC.Se
 	}
 
 	onSortClick = (sort: SortKey) => {
+		let sortOrder = this.state.sortOrder
 		if (this.state.sort === sort) {
-			this.setState({ sortOrder: -this.state.sortOrder })
+			sortOrder = -sortOrder
 		} else {
-			this.setState({ sortOrder: 1, sort })
+			sortOrder = 1
 		}
+
+		this.setState({
+			sortOrder,
+			sort,
+		})
+
+		// Reset ordering and go back to the first page
+		this.api.listFolders(0, pageSize + 1, sort, sortOrder === 1 ? 'asc' : 'desc').then((folders) => {
+			this.setState({
+				folders,
+				currentPage: 0,
+			})
+		})
 	}
 
 	static supportACL(): boolean {
@@ -233,30 +247,7 @@ export class App extends Component<unknown, AppState> implements OC.Plugin<OC.Se
 				}
 				return folder.mount_point.toLowerCase().includes(this.state.filter.toLowerCase())
 			})
-			.sort((a, b) => {
-				switch (this.state.sort) {
-				case 'mount_point':
-					return a.mount_point.localeCompare(b.mount_point) * this.state.sortOrder
-				case 'quota':
-					if (a.quota < 0 && b.quota >= 0) {
-						return this.state.sortOrder
-					}
-					if (b.quota < 0 && a.quota >= 0) {
-						return -this.state.sortOrder
-					}
-					return (a.quota - b.quota) * this.state.sortOrder
-				case 'groups':
-					return (Object.keys(a.groups).length - Object.keys(b.groups).length) * this.state.sortOrder
-				case 'acl':
-					if (a.acl && !b.acl) {
-						return this.state.sortOrder
-					}
-					if (!a.acl && b.acl) {
-						return -this.state.sortOrder
-					}
-				}
-				return 0
-			})
+			.sort((a, b) => a.sortIndex! - b.sortIndex!)
 			.slice(this.state.currentPage * pageSize, this.state.currentPage * pageSize + pageSize)
 			.map(folder => {
 				const id = folder.id

src/types/openapi/openapi.ts

@@ -293,6 +293,8 @@ export type components = {
             size: number;
             acl: boolean;
             manage: components["schemas"]["AclManage"][];
+            /** Format: int64 */
+            sortIndex?: number;
         };
         Group: {
             gid: string;
@@ -416,6 +418,8 @@ export interface operations {
                 limit?: number | null;
                 /** @description The key to order by */
                 orderBy?: "mount_point" | "quota" | "groups" | "acl";
+                /** @description Sort ascending or descending */
+                order?: "asc" | "desc";
             };
             header: {
                 /** @description Required to be true for the API request to pass */