From 72371ded91b62b54353449ea2330db75e1336fbb Mon Sep 17 00:00:00 2001 From: Kostiantyn Miakshyn Date: Tue, 27 May 2025 16:46:22 +0200 Subject: [PATCH] Enhancement: Update rows via import Signed-off-by: Kostiantyn Miakshyn --- cypress/e2e/tables-import.cy.js | 34 ++++++++ lib/Controller/Api1Controller.php | 2 +- lib/Controller/RowController.php | 9 +-- lib/Db/Column.php | 2 + lib/ResponseDefinitions.php | 1 + lib/Service/ColumnService.php | 2 +- lib/Service/ImportService.php | 81 +++++++++++++++---- lib/Service/RowService.php | 11 ++- openapi.json | 5 ++ src/modules/modals/ImportPreview.vue | 10 ++- src/modules/modals/ImportResults.vue | 7 ++ .../ncTable/mixins/exportTableMixin.js | 2 +- src/types/openapi/openapi.ts | 2 + tests/integration/features/APIv1.feature | 32 ++++++++ .../features/bootstrap/FeatureContext.php | 38 +++++---- 15 files changed, 192 insertions(+), 46 deletions(-) diff --git a/cypress/e2e/tables-import.cy.js b/cypress/e2e/tables-import.cy.js index 4a45cff6be..342872c733 100644 --- a/cypress/e2e/tables-import.cy.js +++ b/cypress/e2e/tables-import.cy.js @@ -61,6 +61,40 @@ describe('Import csv', () => { cy.get('[data-cy="importResultRowErrors"]').should('contain.text', '0') }) + it('Import csv from device with updating of existent files', () => { + cy.intercept({ method: 'GET', url: '**/apps/tables/row/table/*' }).as('rowsReq') + + cy.loadTable('Welcome to Nextcloud Tables!') + + cy.wait('@rowsReq').then(({ response }) => { + const firstRow = response.body[0] + const csv = [ + ['id', 'What', 'How to do'], + [firstRow.id, 'What (Updated)', 'How to do (Updated)'], + ] + + cy.writeFile('cypress/fixtures/test-import-update.csv', csv.map(row => row.join(',')).join('\n')) + }) + + cy.clickOnTableThreeDotMenu('Import') + cy.get('.modal__content button').contains('Upload from device').click() + cy.get('input[type="file"]').selectFile('cypress/fixtures/test-import-update.csv', { force: true }) + + cy.get('.modal__content button').contains('Preview').click() + cy.get('.file_import__preview tbody tr', { timeout: 20000 }).should('have.length', 3) + + cy.intercept({ method: 'POST', url: '**/apps/tables/importupload/table/*'}).as('importUploadReq') + cy.get('.modal__content button').contains('Import').click() + cy.wait('@importUploadReq') + cy.get('[data-cy="importResultColumnsFound"]', { timeout: 20000 }).should('contain.text', '2') + cy.get('[data-cy="importResultColumnsMatch"]').should('contain.text', '3') + cy.get('[data-cy="importResultColumnsCreated"]').should('contain.text', '0') + cy.get('[data-cy="importResultRowsInserted"]').should('contain.text', '0') + cy.get('[data-cy="importResultRowsUpdated"]').should('contain.text', '1') + cy.get('[data-cy="importResultParsingErrors"]').should('contain.text', '0') + cy.get('[data-cy="importResultRowErrors"]').should('contain.text', '0') + }) + }) describe('Import csv from Files file action', () => { diff --git a/lib/Controller/Api1Controller.php b/lib/Controller/Api1Controller.php index 559f5598b0..9e39b9622a 100644 --- a/lib/Controller/Api1Controller.php +++ b/lib/Controller/Api1Controller.php @@ -1348,7 +1348,7 @@ public function updateRow(int $rowId, ?int $viewId, $data): DataResponse { } try { - return new DataResponse($this->rowService->updateSet($rowId, $viewId, $dataNew, $this->userId)->jsonSerialize()); + return new DataResponse($this->rowService->updateSet($rowId, $viewId, $dataNew, $this->userId, null)->jsonSerialize()); } catch (BadRequestError $e) { $this->logger->warning('An bad request was encountered: ' . $e->getMessage(), ['exception' => $e]); return new DataResponse(['message' => $e->translatedMessage ?: $e->getMessage()], Http::STATUS_BAD_REQUEST); diff --git a/lib/Controller/RowController.php b/lib/Controller/RowController.php index fa8f5dceb2..144089f7e4 100644 --- a/lib/Controller/RowController.php +++ b/lib/Controller/RowController.php @@ -64,7 +64,7 @@ public function update( $columnId, $data ) { - return $this->service->updateSet($id, $viewId, ['columnId' => $columnId, 'value' => $data], $this->userId); + return $this->service->updateSet($id, $viewId, ['columnId' => $columnId, 'value' => $data], $this->userId, null); }); } @@ -73,18 +73,13 @@ public function updateSet( int $id, ?int $viewId, array $data, - ): DataResponse { return $this->handleError(function () use ( $id, $viewId, $data ) { - return $this->service->updateSet( - $id, - $viewId, - $data, - $this->userId); + return $this->service->updateSet($id, $viewId, $data, $this->userId, null); }); } diff --git a/lib/Db/Column.php b/lib/Db/Column.php index 2eaa96135c..60f7bf8491 100644 --- a/lib/Db/Column.php +++ b/lib/Db/Column.php @@ -108,6 +108,8 @@ class Column extends EntitySuper implements JsonSerializable { public const SUBTYPE_TEXT_LINE = 'line'; + public const META_ID_TITLE = 'id'; + protected ?string $title = null; protected ?int $tableId = null; protected ?string $createdBy = null; diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 8b2ce56928..f12df4dc39 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -147,6 +147,7 @@ * matching_columns_count: int, * created_columns_count: int, * inserted_rows_count: int, + * updated_rows_count: int, * errors_parsing_count: int, * errors_count: int, * } diff --git a/lib/Service/ColumnService.php b/lib/Service/ColumnService.php index ed4707c7d7..54e8f55ca9 100644 --- a/lib/Service/ColumnService.php +++ b/lib/Service/ColumnService.php @@ -473,7 +473,7 @@ public function findOrCreateColumnsByTitleForTableAsArray(?int $tableId, ?int $v $result[$i] = ''; } // if column was not found - if ($result[$i] === '' && $createUnknownColumns) { + if ($result[$i] === '' && $createUnknownColumns && $dataTypes[$i]['type'] !== Column::TYPE_META_ID) { $description = $this->l->t('This column was automatically created by the import service.'); $result[$i] = $this->create( $userId, diff --git a/lib/Service/ImportService.php b/lib/Service/ImportService.php index 6d2d4f93db..24af27a12e 100644 --- a/lib/Service/ImportService.php +++ b/lib/Service/ImportService.php @@ -34,7 +34,6 @@ use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; use Psr\Log\LoggerInterface; -use Throwable; use TypeError; use function file_exists; use function is_string; @@ -55,9 +54,11 @@ class ImportService extends SuperService { private ?int $viewId = null; private array $columns = []; private bool $createUnknownColumns = true; + private ?int $idColumnIndex = null; private int $countMatchingColumns = 0; private int $countCreatedColumns = 0; private int $countInsertedRows = 0; + private int $countUpdatedRows = 0; private int $countErrors = 0; private int $countParsingErrors = 0; @@ -152,7 +153,7 @@ private function getPreviewData(Worksheet $worksheet): array { $column = $this->columns[$colIndex]; $columns[] = $column; } else { - $columns[] = [ + $column = [ 'title' => $title, 'type' => $this->rawColumnDataTypes[$colIndex]['type'], 'subtype' => $this->rawColumnDataTypes[$colIndex]['subtype'] ?? null, @@ -160,6 +161,11 @@ private function getPreviewData(Worksheet $worksheet): array { 'numberPrefix' => $this->rawColumnDataTypes[$colIndex]['number_prefix'] ?? '', 'numberSuffix' => $this->rawColumnDataTypes[$colIndex]['number_suffix'] ?? '', ]; + if (mb_strtolower($title) === Column::META_ID_TITLE) { + $column['id'] = Column::TYPE_META_ID; + } + + $columns[] = $column; } } @@ -179,7 +185,7 @@ private function getPreviewData(Worksheet $worksheet): array { $colIndex = $cellIterator->getCurrentColumnIndex() - 1; $column = $this->columns[$colIndex]; - if (!array_key_exists($colIndex, $this->columns)) { + if (!array_key_exists($colIndex, $columns)) { continue; } @@ -324,6 +330,7 @@ public function import(?int $tableId, ?int $viewId, string $path, bool $createMi 'matching_columns_count' => $this->countMatchingColumns, 'created_columns_count' => $this->countCreatedColumns, 'inserted_rows_count' => $this->countInsertedRows, + 'updated_rows_count' => $this->countUpdatedRows, 'errors_parsing_count' => $this->countParsingErrors, 'errors_count' => $this->countErrors, ]; @@ -354,7 +361,7 @@ private function loop(Worksheet $worksheet): void { foreach ($worksheet->getRowIterator(2) as $row) { // parse row data - $this->createRow($row); + $this->upsertRow($row); } } @@ -387,7 +394,7 @@ private function parseValueByColumnType(string $value, Column $column): string { * @throws MultipleObjectsReturnedException * @throws NotFoundError */ - private function createRow(Row $row): void { + private function upsertRow(Row $row): void { $cellIterator = $row->getCellIterator(); $cellIterator->setIterateOnlyExistingCells(false); @@ -395,17 +402,38 @@ private function createRow(Row $row): void { $i = -1; $data = []; $hasData = false; + $id = null; foreach ($cellIterator as $cell) { $i++; + if ($this->idColumnIndex !== null && $i === $this->idColumnIndex) { + // if this is the ID column, we need to get the ID from the cell + if ($cell && $cell->getValue() !== null) { + $id = $cell->getValue(); + } + if ($id !== null && !is_numeric($id)) { + $this->logger->warning('ID column value is not numeric: ' . $id); + $this->countErrors++; + return; + } + $id = (int)$id; + continue; + } + + $columnKey = $i; + if ($this->columnsConfig && $this->idColumnIndex !== null && $i > $this->idColumnIndex) { + // if we have an ID column, we need to adjust the index + $columnKey = $i - 1; + } + // only add the dataset if column is known - if (!isset($this->columns[$i]) || $this->columns[$i] === '') { + if (!isset($this->columns[$columnKey]) || $this->columns[$columnKey] === '') { $this->logger->debug('Column unknown while fetching rows data for importing.'); continue; } /** @var Column $column */ - $column = $this->columns[$i]; + $column = $this->columns[$columnKey]; // if cell is empty if (!$cell || $cell->getValue() === null) { @@ -446,26 +474,31 @@ private function createRow(Row $row): void { ]; } - if ($hasData) { + if (!$hasData) { + $this->logger->debug('Skipped empty row ' . $row->getRowIndex() . ' during import'); + return; + } + + if ($id) { + $this->rowService->updateSet($id, $this->viewId, $data, $this->userId, $this->tableId); + $this->countUpdatedRows++; + } else { $this->rowService->create($this->tableId, $this->viewId, $data); $this->countInsertedRows++; - } else { - $this->logger->debug('Skipped empty row ' . $row->getRowIndex() . ' during import'); } } catch (PermissionError $e) { - $this->logger->error('Could not create row while importing, no permission.', ['exception' => $e]); + $this->logger->error('Could not create/update row while importing, no permission.', ['exception' => $e]); $this->countErrors++; } catch (BadRequestError|InternalError $e) { - $this->logger->error('Error while creating new row for import.', ['exception' => $e]); + $this->logger->error('Error while creating/updating new row for import.', ['exception' => $e]); $this->countErrors++; } catch (NotFoundError $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); - throw new NotFoundError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); - } catch (Throwable $e) { + throw new NotFoundError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage(), 0, $e); + } catch (\Throwable $e) { $this->countErrors++; - $this->logger->error('Error while creating new row for import.', ['exception' => $e]); + $this->logger->error('Error while creating/updating new row for import.', ['exception' => $e]); } - } private function valueToDateTimeImmutable(mixed $value): ?DateTimeImmutable { @@ -522,9 +555,25 @@ private function getColumns(Row $firstRow, Row $secondRow): void { if ($cell && $cell->getValue() !== null && $cell->getValue() !== '') { $title = $cell->getValue(); + if (!$this->columnsConfig && mb_strtolower($title) === Column::META_ID_TITLE) { + $this->idColumnIndex = $index; + $titles[] = $title; + $dataTypes[] = $this->parseColumnDataType($secondRowCellIterator->current()); + $secondRowCellIterator->next(); + $index++; + continue; + } if (isset($this->columnsConfig[$index]) && $this->columnsConfig[$index]['action'] === 'exist' && $this->columnsConfig[$index]['existColumn']) { $title = $this->columnsConfig[$index]['existColumn']['label']; $countMatchingColumnsFromConfig++; + + // no need to create the ID (Meta) column as it used for update + if ($this->columnsConfig[$index]['existColumn']['id'] === Column::TYPE_META_ID) { + $this->idColumnIndex = $index; + $secondRowCellIterator->next(); + $index++; + continue; + } } if (isset($this->columnsConfig[$index]) && $this->columnsConfig[$index]['action'] === 'new' && $this->createUnknownColumns) { $column = $this->columnService->create( diff --git a/lib/Service/RowService.php b/lib/Service/RowService.php index 885cf03893..01ca00e491 100644 --- a/lib/Service/RowService.php +++ b/lib/Service/RowService.php @@ -410,6 +410,7 @@ public function updateSet( ?int $viewId, array $data, string $userId, + ?int $tableId, ): Row2 { try { $item = $this->getRowById($id); @@ -459,8 +460,14 @@ public function updateSet( throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); } } else { - // if no view id is set, we assume a table and take the tableId from the row - $tableId = $item->getTableId(); + if ($tableId === null) { + $tableId = $item->getTableId(); + } + if ($tableId !== $item->getTableId()) { + $e = new \Exception('Row does not belong to table with id ' . $tableId); + $this->logger->error($e->getMessage(), ['exception' => $e]); + throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': ' . $e->getMessage()); + } // security if (!$this->permissionsService->canReadRowsByElementId($item->getTableId(), 'table', $userId)) { diff --git a/openapi.json b/openapi.json index e280c51cb2..800e873d73 100644 --- a/openapi.json +++ b/openapi.json @@ -310,6 +310,7 @@ "matching_columns_count", "created_columns_count", "inserted_rows_count", + "updated_rows_count", "errors_parsing_count", "errors_count" ], @@ -330,6 +331,10 @@ "type": "integer", "format": "int64" }, + "updated_rows_count": { + "type": "integer", + "format": "int64" + }, "errors_parsing_count": { "type": "integer", "format": "int64" diff --git a/src/modules/modals/ImportPreview.vue b/src/modules/modals/ImportPreview.vue index c670a2317f..793f3c6f33 100644 --- a/src/modules/modals/ImportPreview.vue +++ b/src/modules/modals/ImportPreview.vue @@ -70,6 +70,7 @@ import { ColumnTypes } from '../../shared/components/ncTable/mixins/columnHandle import { emit } from '@nextcloud/event-bus' import { useTablesStore } from '../../store/store.js' import { useDataStore } from '../../store/data.js' +import { TYPE_META_ID } from '../../shared/constants.js' export default { name: 'ImportPreview', @@ -116,10 +117,17 @@ export default { return [] } - return this.existingColumns.map(column => ({ + const columns = this.existingColumns.map(column => ({ id: column.id, label: column.title, })) + + columns.unshift({ + id: TYPE_META_ID, + label: t('tables', 'ID (Meta)'), + }) + + return columns }, }, diff --git a/src/modules/modals/ImportResults.vue b/src/modules/modals/ImportResults.vue index 913b6dc16e..94092d5b77 100644 --- a/src/modules/modals/ImportResults.vue +++ b/src/modules/modals/ImportResults.vue @@ -37,6 +37,13 @@ + + {{ t('tables', 'Updated rows') }} + + {{ results.updated_rows_count }} + + + {{ t('tables', 'Value parsing errors') }} diff --git a/src/shared/components/ncTable/mixins/exportTableMixin.js b/src/shared/components/ncTable/mixins/exportTableMixin.js index e5e8efa264..94fd5ac8da 100644 --- a/src/shared/components/ncTable/mixins/exportTableMixin.js +++ b/src/shared/components/ncTable/mixins/exportTableMixin.js @@ -21,7 +21,7 @@ export default { const data = [] rows.forEach(row => { - const rowData = {} + const rowData = { ID: row.id } columns.forEach(column => { // if a normal column if (column.id >= 0) { diff --git a/src/types/openapi/openapi.ts b/src/types/openapi/openapi.ts index bf7f1038ae..5da9d4f630 100644 --- a/src/types/openapi/openapi.ts +++ b/src/types/openapi/openapi.ts @@ -937,6 +937,8 @@ export type components = { /** Format: int64 */ readonly inserted_rows_count: number; /** Format: int64 */ + readonly updated_rows_count: number; + /** Format: int64 */ readonly errors_parsing_count: number; /** Format: int64 */ readonly errors_count: number; diff --git a/tests/integration/features/APIv1.feature b/tests/integration/features/APIv1.feature index 61a4234673..0c37c8beb6 100644 --- a/tests/integration/features/APIv1.feature +++ b/tests/integration/features/APIv1.feature @@ -249,6 +249,38 @@ Feature: APIv1 | import-from-ms365.xlsx | | import-from-libreoffice.csv | + @api1 @import @rows + Scenario: Import a document file that updates existing rows + Given table "Import check" with emoji "👨🏻‍💻" exists for user "participant1" as "base1" + Then column "one" exists with following properties + | type | text | + | subtype | line | + | mandatory | 1 | + | description | This is a description! | + Then column "two" exists with following properties + | type | number | + | mandatory | 1 | + | description | This is a description! | + Then row exists with following values + | one | AHA | + | two | 88 | + Given file "update-rows.csv" exists for user "participant1" with the following data + | ID | one | two | + | {rowId} | AHA updated | 99 | + | | new row | 100 | + When user imports file "update-rows.csv" into last created table + Then import results have the following data + | found_columns_count | 3 | + | matching_columns_count | 2 | + | created_columns_count | 1 | + | inserted_rows_count | 1 | + | updated_rows_count | 1 | + | errors_count | 0 | + Then table contains at least following rows + | one | two | + | AHA updated | 99 | + | new row | 100 | + @api1 @import Scenario: Import a document with optional field Given user "participant1" uploads file "import-from-libreoffice-optional-fields.csv" diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index f4457d3a35..ac1f707905 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -489,35 +489,39 @@ public function uploadFile(string $user, string $file): void { // IMPORT -------------------------- /** - * @Given file :file exists for user :user with following data + * @Given file :file exists for user :user with the following data * * @param string $user * @param string $file - * @param TableNode|null $table + * @param TableNode $table */ - public function createCsvFile(string $user, string $file, ?TableNode $table = null): void { + public function createCsvFile(string $user, string $file, TableNode $table): void { $this->setCurrentUser($user); - $url = $this->baseUrl . 'remote.php/dav/files/' . $user . $file; - $body = $this->tableNodeToCsv($table); - $headers = ['Content-Type' => 'text/csv']; - $this->sendRequestFullUrl('PUT', $url, $body, $headers, []); + $url = sprintf('%sremote.php/dav/files/%s/%s', $this->baseUrl, $user, $file); + $body = Utils::streamFor($this->tableNodeToCsv($table)); + + $this->sendRequestFullUrl('PUT', $url, $body); Assert::assertEquals(201, $this->response->getStatusCode()); } - private function tableNodeToCsv(TableNode $node): string { - $out = ''; + /** + * @param TableNode $node + * @return false|resource + */ + private function tableNodeToCsv(TableNode $node) { + $resource = fopen('php://temp', 'rb+'); foreach ($node->getRows() as $row) { - foreach ($row as $value) { - if ($out !== '' && substr($out, -1) !== "\n") { - $out .= ','; - } - $out .= trim($value); - } - $out .= "\n"; + $fields = array_map(function ($cell) { + return str_replace('{rowId}', $this->rowId, $cell); + }, $row); + fputcsv($resource, $fields); } - return $out; + + rewind($resource); + + return $resource; } /**