diff --git a/lib/Db/Row2Mapper.php b/lib/Db/Row2Mapper.php index 2cabb62a3..fb83ac54c 100644 --- a/lib/Db/Row2Mapper.php +++ b/lib/Db/Row2Mapper.php @@ -175,9 +175,7 @@ public function findAll(array $tableColumns, array $columns, int $tableId, ?int $wantedRowIdsArray = $this->getWantedRowIds($userId, $tableId, $filter, $limit, $offset); - // TODO add sorting - - return $this->getRows($wantedRowIdsArray, $columnIdsArray); + return $this->getRows($wantedRowIdsArray, $columnIdsArray, $sort ?? []); } /** @@ -186,7 +184,7 @@ public function findAll(array $tableColumns, array $columns, int $tableId, ?int * @return Row2[] * @throws InternalError */ - private function getRows(array $rowIds, array $columnIds): array { + private function getRows(array $rowIds, array $columnIds, array $sort = []): array { $qb = $this->db->getQueryBuilder(); $qbSqlForColumnTypes = null; @@ -224,14 +222,16 @@ private function getRows(array $rowIds, array $columnIds): array { ->innerJoin('t1', 'tables_row_sleeves', 'rs', 'rs.id = t1.row_id'); try { - $result = $this->db->executeQuery($qb->getSQL(), $qb->getParameters(), $qb->getParameterTypes()); + $result = $qb->executeQuery(); } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), ); } try { - $sleeves = $this->rowSleeveMapper->findMultiple($rowIds); + $sleeves = $this->rowSleeveMapper->findMultiple($rowIds, function (IQueryBuilder $qb, string $sleevesAlias) use ($sort) { + $this->addSortQueryForMultipleSleeveFinder($qb, $sleevesAlias, $sort); + }); } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); @@ -263,6 +263,59 @@ private function addFilterToQuery(IQueryBuilder &$qb, array $filters, string $us } } + /** + * This method is passed to RowSleeveMapper::findMultiple() when the rows need sorting. The RowSleeveMapper does not have + * knowledge about the column information, as they reside in this class, and the mapper is called from here. + * + * @throws InternalError + */ + private function addSortQueryForMultipleSleeveFinder(IQueryBuilder $qb, string $sleevesAlias, array $sort): void { + $i = 1; + foreach (array_reverse($sort) as $sortData) { + if (!isset($this->columns[$sortData['columnId']]) && !isset($this->allColumns[$sortData['columnId']]) && $sortData['columnId'] > 0) { + throw new InternalError('No column found to build filter with for id ' . $sortData['columnId']); + } + + // if is normal column + if ($sortData['columnId'] >= 0) { + $column = $this->columns[$sortData['columnId']] ?? $this->allColumns[$sortData['columnId']]; + $valueTable = 'tables_row_cells_' . $column->getType(); + $alias = 'sort' . $i; + $qb->leftJoin($sleevesAlias, $valueTable, $alias, + $qb->expr()->andX( + $qb->expr()->eq($sleevesAlias . '.id', $alias . '.row_id'), + $qb->expr()->eq($alias . '.column_id', $qb->createNamedParameter($sortData['columnId'])) + ) + ); + $qb->orderBy($alias . '.value', $sortData['mode']); + } elseif (Column::isValidMetaTypeId($sortData['columnId'])) { + $fieldName = match ($sortData['columnId']) { + Column::TYPE_META_ID => 'id', + Column::TYPE_META_CREATED_BY => 'created_by', + Column::TYPE_META_CREATED_AT => 'created_at', + Column::TYPE_META_UPDATED_BY => 'updated_by', + Column::TYPE_META_UPDATED_AT => 'updated_at', + default => null, + }; + + if ($fieldName === null) { + // Can happen, when– + // … a new meta column was introduced, but not considered here + // … a meta column was removed and existing sort rules are not being adapted + // those case are being ignored, but would require developer attention + $this->logger->error('No meta column (ID: {columnId}) found for sorting id', [ + 'columnId' => $sortData['columnId'], + ]); + continue; + } + + $qb->orderBy($sleevesAlias . '.' . $fieldName, $sortData['mode']); + } + $i++; + } + } + + private function replacePlaceholderValues(array &$filters, string $userId): void { foreach ($filters as &$filterGroup) { foreach ($filterGroup as &$filter) { diff --git a/lib/Db/RowSleeveMapper.php b/lib/Db/RowSleeveMapper.php index a61f7531f..e590baf8d 100644 --- a/lib/Db/RowSleeveMapper.php +++ b/lib/Db/RowSleeveMapper.php @@ -40,14 +40,26 @@ public function find(int $id): RowSleeve { /** * @param int[] $ids + * @param ?callable(IQueryBuilder, string): void $sorterCallback * @return RowSleeve[] * @throws Exception */ - public function findMultiple(array $ids): array { + public function findMultiple(array $ids, ?callable $sorterCallback = null): array { + $sleeveAlias = 'sleeves'; $qb = $this->db->getQueryBuilder(); - $qb->select('*') - ->from($this->table) - ->where($qb->expr()->in('id', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY))); + $qb->select( + $sleeveAlias . '.id', + $sleeveAlias . '.table_id', + $sleeveAlias . '.created_by', + $sleeveAlias . '.created_at', + $sleeveAlias . '.last_edit_by', + $sleeveAlias . '.last_edit_at', + ) + ->from($this->table, $sleeveAlias) + ->where($qb->expr()->in($sleeveAlias . '.id', $qb->createNamedParameter($ids, IQueryBuilder::PARAM_INT_ARRAY))); + if ($sorterCallback !== null) { + $sorterCallback($qb, $sleeveAlias); + } return $this->findEntities($qb); } diff --git a/tests/integration/features/APIv2.feature b/tests/integration/features/APIv2.feature index 3300a7a9d..f20f7d3ab 100644 --- a/tests/integration/features/APIv2.feature +++ b/tests/integration/features/APIv2.feature @@ -1328,3 +1328,115 @@ Feature: APIv2 Then the reported status is "404" And the user "participant1-v2" fetches table "t1", it has exactly these rows "r-not-updatable,r-not-deletable" And the column "statement" of row "r-not-updatable" has the value "testing not updated" + + @views + Scenario: have a view with a sort order + Given as user "participant1-v2" + And table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And column "weight" exists with following properties + | type | number | + | mandatory | 1 | + | description | Importance of statement | + And column "code" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | Shorthand of Statement | + And user "participant1-v2" create view "Important Statements" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" sets columns "statement,weight,code" to view "v1" + And following sort order is applied to view "v1": + | weight | DESC | + And using "view" "v1" + And user "participant1-v2" creates row "r1" with following values: + | statement | Be yourself; everyone else is already taken. | + | weight | 75 | + | code | wil01 | + And user "participant1-v2" creates row "r2" with following values: + | statement | A room without books is like a body without a soul. | + | weight | 67 | + | code | cic01 | + And user "participant1-v2" creates row "r3" with following values: + | statement | To live is the rarest thing in the world. Most people exist, that is all. | + | weight | 92 | + | code | wil02 | + Then "view" "v1" has exactly these rows "r3,r1,r2" in exactly this order + + @views + Scenario: have a view with a sort order from a meta column + Given as user "participant1-v2" + And table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And column "weight" exists with following properties + | type | number | + | mandatory | 1 | + | description | Importance of statement | + And column "code" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | Shorthand of Statement | + And user "participant1-v2" create view "Important Statements" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" sets columns "statement,weight,code" to view "v1" + And following sort order is applied to view "v1": + | meta-id | DESC | + And using "view" "v1" + And user "participant1-v2" creates row "r1" with following values: + | statement | Be yourself; everyone else is already taken. | + | weight | 75 | + | code | wil01 | + And user "participant1-v2" creates row "r2" with following values: + | statement | A room without books is like a body without a soul. | + | weight | 67 | + | code | cic01 | + And user "participant1-v2" creates row "r3" with following values: + | statement | To live is the rarest thing in the world. Most people exist, that is all. | + | weight | 92 | + | code | wil02 | + Then "view" "v1" has exactly these rows "r3,r2,r1" in exactly this order + + @views + Scenario: have a view with a multiple sort orders + Given as user "participant1-v2" + And table "Table 1 via api v2" with emoji "👋" exists for user "participant1-v2" as "t1" via v2 + And column "statement" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | State your business | + And column "weight" exists with following properties + | type | number | + | mandatory | 1 | + | description | Importance of statement | + And column "code" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | Shorthand of Statement | + And user "participant1-v2" create view "Important Statements" with emoji "⚡️" for "t1" as "v1" + And user "participant1-v2" sets columns "statement,weight,code" to view "v1" + And following sort order is applied to view "v1": + | weight | DESC | + | code | ASC | + And using "view" "v1" + And user "participant1-v2" creates row "r1" with following values: + | statement | Be yourself; everyone else is already taken. | + | weight | 92 | + | code | wil01 | + And user "participant1-v2" creates row "r2" with following values: + | statement | A room without books is like a body without a soul. | + | weight | 67 | + | code | cic01 | + And user "participant1-v2" creates row "r3" with following values: + | statement | To live is the rarest thing in the world. Most people exist, that is all. | + | weight | 92 | + | code | wil02 | + Then "view" "v1" has exactly these rows "r1,r3,r2" in exactly this order diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index 2d31bb536..e79a3f513 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -9,6 +9,8 @@ use Behat\Behat\Context\Context; use Behat\Gherkin\Node\TableNode; +use Behat\Step\Given; +use Behat\Step\When; use GuzzleHttp\Client; use GuzzleHttp\Cookie\CookieJar; use GuzzleHttp\Exception\ClientException; @@ -48,7 +50,6 @@ class FeatureContext implements Context { private ?int $tableId = null; private ?int $columnId = null; private ?int $rowId = null; - private ?array $tableColumns = []; private ?array $importResult = null; private ?array $activeNode = null; @@ -63,7 +64,6 @@ class FeatureContext implements Context { // Store data from last request to perform assertions, id is used as a key private array $tableData = []; - private array $viewData = []; private $importColumnData = null; @@ -102,7 +102,6 @@ public function cleanupUsers() { } } - /** * @Given table :table with emoji :emoji exists for user :user as :tableName via v2 * @@ -821,6 +820,38 @@ public function updateTable(string $user, string $title, ?string $emoji, string Assert::assertEquals($tableToVerify['ownership'], $user); } + protected function sendUpdateViewRequest(string $viewName, array $data): void { + $viewId = $this->collectionManager->getByAlias('view', $viewName)['id']; + $this->sendRequest( + 'PUT', + sprintf('/apps/tables/api/1/views/%d', $viewId), + [ 'data' => $data ] + ); + } + + #[When('following sort order is applied to view :viewName:')] + public function applySortToView(string $viewName, TableNode $sortOrder): void { + $sortData = []; + foreach ($sortOrder->getRows() as $row) { + + $columnId = match ($row[0]) { + 'meta-id' => -1, + 'meta-created-by' => -2, + 'meta-updated-by' => -3, + 'meta-created-at' => -4, + 'meta-updated-at' => -5, + default => $this->collectionManager->getByAlias('column', $row[0])['id'], + }; + + $sortData[] = [ + 'columnId' => $columnId, + 'mode' => $row[1] + ]; + + } + $this->sendUpdateViewRequest($viewName, ['sort' => json_encode($sortData)]); + } + /** * @When user :user update view :viewName with title :title and emoji :emoji * @@ -837,12 +868,7 @@ public function updateView(string $user, string $viewName, string $title, ?strin $data['emoji'] = $emoji; } - $this->sendRequest( - 'PUT', - '/apps/tables/api/1/views/'.$this->viewIds[$viewName], - [ 'data' => $data ] - ); - + $this->sendUpdateViewRequest($viewName, $data); $updatedItem = $this->getDataFromResponse($this->response); Assert::assertEquals(200, $this->response->getStatusCode()); @@ -875,13 +901,7 @@ public function applyColumnsToView(string $user, string $columnList, string $vie return $col['id']; }, $columns); - $view = $this->collectionManager->getByAlias('view', $viewAlias); - - $this->sendRequest( - 'PUT', - '/apps/tables/api/1/views/' . $view['id'], - [ 'data' => ['columns' => json_encode($columns)] ] - ); + $this->sendUpdateViewRequest($viewAlias, ['columns' => json_encode($columns)]); } /** @@ -953,7 +973,6 @@ public function deleteTable(string $user, string $keyword): void { $this->tableId = null; $this->columnId = null; - $this->tableColumns = []; } /** @@ -1163,7 +1182,6 @@ public function createColumn(string $title, ?TableNode $properties = null): void $newColumn = $this->getDataFromResponse($this->response); $this->columnId = $newColumn['id']; - $this->tableColumns[$newColumn['title']] = $newColumn['id']; Assert::assertEquals(200, $this->response->getStatusCode()); Assert::assertEquals($newColumn['title'], $title); @@ -1320,7 +1338,7 @@ public function updateColumn(?TableNode $properties = null): void { public function createRow(?TableNode $properties = null): void { $props = []; foreach ($properties->getRows() as $row) { - $columnId = $this->tableColumns[$row[0]]; + $columnId = $this->collectionManager->getByAlias('column', $row[0])['id']; $props[$columnId] = $row[1]; } @@ -1371,7 +1389,7 @@ public function userTriesToCreateRowUsingV2OnNodeXWithFollowingValues(string $us $props = []; foreach ($properties->getRows() as $row) { - $columnId = $this->tableColumns[$row[0]]; + $columnId = $this->collectionManager->getByAlias('column', $row[0])['id']; $props[$columnId] = $row[1]; } @@ -1392,7 +1410,7 @@ public function userTriesToCreateRowUsingV2WithFollowingValues(string $user, Tab $props = []; foreach ($properties->getRows() as $row) { - $columnId = $this->tableColumns[$row[0]]; + $columnId = $this->collectionManager->getByAlias('column', $row[0])['id']; $props[$columnId] = $row[1]; } @@ -1411,7 +1429,7 @@ public function userTriesToCreateRowUsingV2WithFollowingValues(string $user, Tab public function createRowV2(TableNode $properties): void { $props = []; foreach ($properties->getRows() as $row) { - $columnId = $this->tableColumns[$row[0]]; + $columnId = $this->collectionManager->getByAlias('column', $row[0])['id']; $props[$columnId] = $row[1]; } @@ -1449,7 +1467,7 @@ public function createRowV2(TableNode $properties): void { public function createRowLegacy(?TableNode $properties = null): void { $props = []; foreach ($properties->getRows() as $row) { - $columnId = $this->tableColumns[$row[0]]; + $columnId = $this->collectionManager->getByAlias('column', $row[0])['id']; $props[$columnId] = $row[1]; } @@ -1507,7 +1525,7 @@ public function deleteRow(): void { public function updateRow(?TableNode $properties = null): void { $props = []; foreach ($properties->getRows() as $row) { - $columnId = $this->tableColumns[$row[0]]; + $columnId = $this->collectionManager->getByAlias('column', $row[0])['id']; $props[$columnId] = $row[1]; } @@ -1554,7 +1572,7 @@ public function updateRow(?TableNode $properties = null): void { public function updateRowLegacy(?TableNode $properties = null): void { $props = []; foreach ($properties->getRows() as $row) { - $columnId = $this->tableColumns[$row[0]]; + $columnId = $this->collectionManager->getByAlias('column', $row[0])['id']; $props[$columnId] = $row[1]; } @@ -1587,11 +1605,8 @@ public function updateRowLegacy(?TableNode $properties = null): void { * User management */ - /** - * @Given /^as user "([^"]*)"$/ - * @param string $user - */ - public function setCurrentUser($user) { + #[Given('as user :user')] + public function setCurrentUser(?string $user): void { $this->currentUser = $user; } @@ -2466,7 +2481,7 @@ public function theInsertedRowHasTheFollowingValues(TableNode $columnValues) { $expected = []; foreach ($columnValues->getRows() as $row) { - $columnId = $this->tableColumns[$row[0]]; + $columnId = $this->collectionManager->getByAlias('column', $row[0])['id']; $expected[$columnId] = $row[1]; } @@ -2510,7 +2525,7 @@ public function userCreatesRowWithFollowingValues(string $user, string $rowAlias $props = []; foreach ($properties->getRows() as $row) { - $columnId = $this->tableColumns[$row[0]]; + $columnId = $this->collectionManager->getByAlias('column', $row[0])['id']; $props[$columnId] = $row[1]; } @@ -2538,7 +2553,7 @@ public function userUpdatesRowWithFollowingValues(string $user, string $rowAlias $props = []; foreach ($properties->getRows() as $row) { - $columnId = $this->tableColumns[$row[0]]; + $columnId = $this->collectionManager->getByAlias('column', $row[0])['id']; $props[$columnId] = $row[1]; } @@ -2616,6 +2631,38 @@ public function nodeHasExactlyThoseRows(string $user, string $nodeType, string $ Assert::assertSame($expectedIds, $actualRowIds); } + /** + * @Then :nodeType :nodeAlias has exactly these rows :rowAliasList in exactly this order + */ + public function nodeHasExactlyThoseSortedRows(string $nodeType, string $nodeAlias, string $rowAliasList): void { + $nodeItem = $this->collectionManager->getByAlias($nodeType, $nodeAlias); + + $this->sendRequest( + 'GET', + sprintf('/apps/tables/api/1/%ss/%s/rows', $nodeType, $nodeItem['id']), + ); + + $allRows = $this->getDataFromResponse($this->response); + Assert::assertEquals(200, $this->response->getStatusCode()); + + $rowAliases = explode(',', $rowAliasList); + $expectedIds = array_map(function ($rowAlias): int { + $row = $this->collectionManager->getByAlias('row', $rowAlias); + return $row['id']; + }, $rowAliases); + + $actualRowIds = array_map(function (array $actualRow): int { + if ($this->collectionManager->getById('row', (int)$actualRow['id'])) { + $this->collectionManager->update($actualRow, 'row', (int)$actualRow['id']); + } else { + $this->collectionManager->register($actualRow, 'row', (int)$actualRow['id']); + } + return (int)$actualRow['id']; + }, $allRows); + + Assert::assertSame($expectedIds, $actualRowIds); + } + /** * @When the column :columnName of row :rowAlias has the value :value */