diff --git a/lib/Service/ColumnTypes/DatetimeDateBusiness.php b/lib/Service/ColumnTypes/DatetimeDateBusiness.php index 70b5b6922..5cbc8abaf 100644 --- a/lib/Service/ColumnTypes/DatetimeDateBusiness.php +++ b/lib/Service/ColumnTypes/DatetimeDateBusiness.php @@ -17,7 +17,7 @@ class DatetimeDateBusiness extends SuperBusiness implements IColumnTypeBusiness * @return string */ public function parseValue($value, ?Column $column = null): string { - return json_encode($this->isValidDate($value, 'Y-m-d') ? $value : ''); + return json_encode($this->isValidDate((string)$value, 'Y-m-d') ? (string)$value : ''); } /** @@ -26,7 +26,7 @@ public function parseValue($value, ?Column $column = null): string { * @return bool */ public function canBeParsed($value, ?Column $column = null): bool { - return $this->isValidDate($value, 'Y-m-d'); + return $this->isValidDate((string)$value, 'Y-m-d'); } } diff --git a/lib/Service/ColumnTypes/DatetimeTimeBusiness.php b/lib/Service/ColumnTypes/DatetimeTimeBusiness.php index 6d06cc39e..e0d41e4b5 100644 --- a/lib/Service/ColumnTypes/DatetimeTimeBusiness.php +++ b/lib/Service/ColumnTypes/DatetimeTimeBusiness.php @@ -17,7 +17,7 @@ class DatetimeTimeBusiness extends SuperBusiness implements IColumnTypeBusiness * @return string */ public function parseValue($value, ?Column $column = null): string { - return json_encode($this->isValidDate($value, 'H:i') ? $value : ''); + return json_encode($this->isValidDate((string)$value, 'H:i') ? $value : ''); } /** @@ -26,7 +26,7 @@ public function parseValue($value, ?Column $column = null): string { * @return bool */ public function canBeParsed($value, ?Column $column = null): bool { - return $this->isValidDate($value, 'H:i'); + return $this->isValidDate((string)$value, 'H:i'); } } diff --git a/lib/Service/ColumnTypes/SelectionCheckBusiness.php b/lib/Service/ColumnTypes/SelectionCheckBusiness.php index 71c1f6fb7..87f57ec76 100644 --- a/lib/Service/ColumnTypes/SelectionCheckBusiness.php +++ b/lib/Service/ColumnTypes/SelectionCheckBusiness.php @@ -10,8 +10,8 @@ use OCA\Tables\Db\Column; class SelectionCheckBusiness extends SuperBusiness implements IColumnTypeBusiness { - public const PATTERN_POSITIVE = ['yes', '1', true, 1, 'true']; - public const PATTERN_NEGATIVE = ['no', '0', false, 0, 'false']; + public const PATTERN_POSITIVE = ['yes', '1', true, 1, 'true', 'TRUE']; + public const PATTERN_NEGATIVE = ['no', '0', false, 0, 'false', 'FALSE']; /** * @param mixed $value diff --git a/lib/Service/ImportService.php b/lib/Service/ImportService.php index a30725019..09813aa35 100644 --- a/lib/Service/ImportService.php +++ b/lib/Service/ImportService.php @@ -136,7 +136,7 @@ private function getPreviewData(Worksheet $worksheet): array { $columns[] = [ 'title' => $title, 'type' => $this->rawColumnDataTypes[$colIndex]['type'], - 'subtype' => $this->rawColumnDataTypes[$colIndex]['subtype'], + 'subtype' => $this->rawColumnDataTypes[$colIndex]['subtype'] ?? null, 'numberDecimals' => $this->rawColumnDataTypes[$colIndex]['number_decimals'] ?? 0, 'numberPrefix' => $this->rawColumnDataTypes[$colIndex]['number_prefix'] ?? '', 'numberSuffix' => $this->rawColumnDataTypes[$colIndex]['number_suffix'] ?? '', @@ -154,13 +154,26 @@ private function getPreviewData(Worksheet $worksheet): array { $cellIterator = $row->getCellIterator(); $cellIterator->setIterateOnlyExistingCells(false); - foreach ($cellIterator as $cellIndex => $cell) { + foreach ($cellIterator as $cell) { $value = $cell->getValue(); - $colIndex = (int) $cellIndex; + // $cellIterator`s index is based on 1, not 0. + $colIndex = $cellIterator->getCurrentColumnIndex() - 1; $column = $this->columns[$colIndex]; if (($column && $column->getType() === 'datetime') || (is_array($columns[$colIndex]) && $columns[$colIndex]['type'] === 'datetime')) { - $value = Date::excelToDateTimeObject($value)->format('Y-m-d H:i'); + if (isset($columns[$colIndex]['subtype']) && $columns[$colIndex]['subtype'] === 'date') { + $format = 'Y-m-d'; + } elseif (isset($columns[$colIndex]['subtype']) && $columns[$colIndex]['subtype'] === 'time') { + $format = 'H:i'; + } else { + $format = 'Y-m-d H:i'; + } + + try { + $value = Date::excelToDateTimeObject($value)->format($format); + } catch (\TypeError) { + $value = (new \DateTimeImmutable($value))->format($format); + } } elseif (($column && $column->getType() === 'number' && $column->getNumberSuffix() === '%') || (is_array($columns[$colIndex]) && $columns[$colIndex]['type'] === 'number' && $columns[$colIndex]['numberSuffix'] === '%')) { $value = $value * 100; @@ -285,8 +298,14 @@ public function import(?int $tableId, ?int $viewId, string $path, bool $createMi * @throws PermissionError */ private function loop(Worksheet $worksheet): void { - $firstRow = $worksheet->getRowIterator()->current(); - $secondRow = $worksheet->getRowIterator()->seek(2)->current(); + $rowIterator = $worksheet->getRowIterator(); + $firstRow = $rowIterator->current(); + $rowIterator->next(); + if (!$rowIterator->valid()) { + return; + } + $secondRow = $rowIterator->current(); + unset($rowIterator); $this->getColumns($firstRow, $secondRow); if (empty(array_filter($this->columns))) { @@ -361,8 +380,32 @@ private function createRow(Row $row): void { $value = $cell->getValue(); $hasData = $hasData || !empty($value); + if ($column->getType() === 'datetime') { - $value = Date::excelToDateTimeObject($value)->format('Y-m-d H:i'); + if ($column->getType() === 'datetime' && $column->getSubtype() === 'date') { + $format = 'Y-m-d'; + } elseif ($column->getType() === 'datetime' && $column->getSubtype() === 'time') { + $format = 'H:i'; + } else { + $format = 'Y-m-d H:i'; + } + try { + $value = Date::excelToDateTimeObject($value)->format($format); + } catch (\TypeError) { + $value = (new \DateTimeImmutable($value))->format($format); + } + } elseif ($column->getType() === 'datetime' && $column->getSubtype() === 'date') { + try { + $value = Date::excelToDateTimeObject($value)->format('Y-m-d'); + } catch (\TypeError) { + $value = (new \DateTimeImmutable($value))->format('Y-m-d'); + } + } elseif ($column->getType() === 'datetime' && $column->getSubtype() === 'time') { + try { + $value = Date::excelToDateTimeObject($value)->format('H:i'); + } catch (\TypeError) { + $value = (new \DateTimeImmutable($value))->format('H:i'); + } } elseif ($column->getType() === 'number' && $column->getNumberSuffix() === '%') { $value = $value * 100; } elseif ($column->getType() === 'selection' && $column->getSubtype() === 'check') { @@ -414,6 +457,8 @@ private function getColumns(Row $firstRow, Row $secondRow): void { $index = 0; $countMatchingColumnsFromConfig = 0; $countCreatedColumnsFromConfig = 0; + $lastCellWasEmpty = false; + $hasGapInTitles = false; foreach ($cellIterator as $cell) { if ($cell && $cell->getValue() !== null && $cell->getValue() !== '') { $title = $cell->getValue(); @@ -437,14 +482,29 @@ private function getColumns(Row $firstRow, Row $secondRow): void { // Convert data type to our data type $dataTypes[] = $this->parseColumnDataType($secondRowCellIterator->current()); + if ($lastCellWasEmpty) { + $hasGapInTitles = true; + } + $lastCellWasEmpty = false; } else { $this->logger->debug('No cell given or cellValue is empty while loading columns for importing'); + if ($cell->getDataType() === 'null') { + // LibreOffice generated XLSX doc may have more empty columns in the first row. + // Continue without increasing error count, but leave a marker to detect gaps in titles. + $lastCellWasEmpty = true; + continue; + } $this->countErrors++; } $secondRowCellIterator->next(); $index++; } + if ($hasGapInTitles) { + $this->logger->info('Imported table is having a gap in column titles'); + $this->countErrors++; + } + $this->rawColumnTitles = $titles; $this->rawColumnDataTypes = $dataTypes; @@ -468,9 +528,33 @@ private function parseColumnDataType(Cell $cell): array { 'subtype' => 'line', ]; - if (Date::isDateTime($cell) || $originDataType === DataType::TYPE_ISO_DATE) { + try { + if ($value === false) { + throw new \Exception('We do not accept `false` here'); + } + $dateValue = new \DateTimeImmutable($value); + } catch (\Exception) { + } + + if (isset($dateValue) + || Date::isDateTime($cell) + || $originDataType === DataType::TYPE_ISO_DATE) { + // the formatted value stems from the office document and shows the original user intent + $dateAnalysis = date_parse($formattedValue); + $containsDate = $dateAnalysis['year'] !== false || $dateAnalysis['month'] !== false || $dateAnalysis['day'] !== false; + $containsTime = $dateAnalysis['hour'] !== false || $dateAnalysis['minute'] !== false || $dateAnalysis['second'] !== false; + + if ($containsDate && !$containsTime) { + $subType = 'date'; + } elseif (!$containsDate && $containsTime) { + $subType = 'time'; + } else { + $subType = ''; + } + $dataType = [ 'type' => 'datetime', + 'subtype' => $subType, ]; } elseif ($originDataType === DataType::TYPE_NUMERIC) { if (str_contains($formattedValue, '%')) { @@ -514,7 +598,10 @@ private function parseColumnDataType(Cell $cell): array { 'type' => 'number', ]; } - } elseif ($originDataType === DataType::TYPE_BOOL) { + } elseif ($originDataType === DataType::TYPE_BOOL + || ($originDataType === DataType::TYPE_FORMULA + && in_array($formattedValue, ['FALSE', 'TRUE'], true)) + ) { $dataType = [ 'type' => 'selection', 'subtype' => 'check', diff --git a/src/modules/modals/Import.vue b/src/modules/modals/Import.vue index e6b663630..ec71c91f0 100644 --- a/src/modules/modals/Import.vue +++ b/src/modules/modals/Import.vue @@ -41,7 +41,7 @@

{{ t('tables', 'Supported formats: xlsx, xls, csv, html, xml') }}
- {{ t('tables', 'First row of the file must contain column headings.') }} + {{ t('tables', 'First row of the file must contain column headings without gaps.') }}

diff --git a/tests/integration/features/APIv1.feature b/tests/integration/features/APIv1.feature index b70fc3c98..a1e2985df 100644 --- a/tests/integration/features/APIv1.feature +++ b/tests/integration/features/APIv1.feature @@ -187,31 +187,36 @@ Feature: APIv1 Then user deletes last created row Then user "participant1" deletes table with keyword "Rows check" - - @api1 - Scenario: Import csv table - Given file "/import.csv" exists for user "participant1" with following data - | Col1 | Col2 | Col3 | num | emoji | special | - | Val1 | Val2 | Val3 | 1 | 💙 | Ä | - | great | news | here | 99 | ⚠️ | Ö | - Given table "Import test" with emoji "👨🏻‍💻" exists for user "participant1" as "base1" - When user imports file "/import.csv" into last created table + @api1 @import + Scenario Outline: Import a document file + Given user "participant1" uploads file "" + And table "Import test" with emoji "👨🏻‍💻" exists for user "participant1" as "base1" + When user imports file "/" into last created table Then import results have the following data - | found_columns_count | 6 | - | created_columns_count | 6 | - | inserted_rows_count | 2 | - | errors_count | 0 | - Then table has at least following columns - | Col1 | - | Col2 | - | Col3 | - | num | - | emoji | - | special | + | found_columns_count | 10 | + | created_columns_count | 10 | + | inserted_rows_count | 2 | + | errors_count | 0 | + Then table has at least following typed columns + | Col1 | text | + | Col2 | text | + | Col3 | text | + | num | number | + | emoji | text | + | special | text | + | date | datetime | + | truth | selection | Then table contains at least following rows - | Col1 | Col2 | Col3 | num | emoji | special | - | Val1 | Val2 | Val3 | 1 | 💙 | Ä | - | great | news | here | 99 | ⚠️ | Ö | + | Date and Time | Col1 | Col2 | Col3 | num | emoji | special | date | truth | time | + | 2022-02-20 08:42 | Val1 | Val2 | Val3 | 1 | 💙 | Ä | 2024-02-24 | false | 18:48 | + | 2016-06-01 13:37 | great | news | here | 99 | ⚠ | Ö | 2016-06-01 | true | 01:23 | + + Examples: + | importfile | + | import-from-libreoffice.ods | + | import-from-libreoffice.xlsx | + | import-from-ms365.xlsx | + | import-from-libreoffice.csv | @api1 Scenario: Create, edit and delete views diff --git a/tests/integration/features/bootstrap/FeatureContext.php b/tests/integration/features/bootstrap/FeatureContext.php index e3b2f1128..0be0cec0e 100644 --- a/tests/integration/features/bootstrap/FeatureContext.php +++ b/tests/integration/features/bootstrap/FeatureContext.php @@ -13,6 +13,7 @@ use GuzzleHttp\Cookie\CookieJar; use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\GuzzleException; +use GuzzleHttp\Psr7\Utils; use PHPUnit\Framework\Assert; use PHPUnit\Framework\ExpectationFailedException; use Psr\Http\Message\ResponseInterface; @@ -64,6 +65,8 @@ class FeatureContext implements Context { private array $tableData = []; private array $viewData = []; + private $importColumnData = null; + // use CommandLineTrait; private CollectionManager $collectionManager; @@ -89,6 +92,7 @@ public function setUp() { * @AfterScenario */ public function cleanupUsers() { + $this->importColumnData = null; $this->collectionManager->cleanUp(); foreach ($this->createdUsers as $user) { $this->deleteUser($user); @@ -467,8 +471,21 @@ public function columnsForNodeV2(string $nodeType, string $nodeName, ?TableNode // (((((((((((((((((((((((((((( END API v2 ))))))))))))))))))))))))))))))))))) + /** + * @Given user :user uploads file :file + */ + public function uploadFile(string $user, string $file): void { + $this->setCurrentUser($user); + + $localFilePath = __DIR__ . '/../../resources/' . $file; + + $url = sprintf('%sremote.php/dav/files/%s/%s', $this->baseUrl, $user, $file); + $body = Utils::streamFor(fopen($localFilePath, 'rb')); + $this->sendRequestFullUrl('PUT', $url, $body); + Assert::assertEquals(201, $this->response->getStatusCode()); + } // IMPORT -------------------------- @@ -574,7 +591,7 @@ public function checkRowsExists(TableNode $table): void { $allValuesForColumn[] = $row[$indexForCol]; } foreach ($table->getColumn($key) as $item) { - Assert::assertTrue(in_array($item, $allValuesForColumn)); + Assert::assertTrue(in_array($item, $allValuesForColumn), sprintf('%s not in %s', $item, implode(', ', $allValuesForColumn))); } } } @@ -1190,6 +1207,36 @@ public function tableColumns(?TableNode $body = null): void { } } + /** + * @Then table has at least following typed columns + * + * @param TableNode|null $body + */ + public function tableTypedColumns(?TableNode $body = null): void { + $this->sendRequest( + 'GET', + '/apps/tables/api/1/tables/'.$this->tableId.'/columns' + ); + + $data = $this->getDataFromResponse($this->response); + Assert::assertEquals(200, $this->response->getStatusCode()); + + // check if no columns exists + if ($body === null) { + Assert::assertCount(0, $data); + return; + } + + $colByTitle = []; + foreach ($data as $d) { + $colByTitle[$d['title']] = $d['type']; + } + foreach ($body->getRows() as $columnData) { + Assert::assertArrayHasKey($columnData[0], $colByTitle); + Assert::assertSame($columnData[1], $colByTitle[$columnData[0]], sprintf('Column "%s" has unexpected type "%s"', $columnData[0], $colByTitle[$columnData[0]])); + } + } + /** * @Then user deletes last created column */ diff --git a/tests/integration/resources/import-from-libreoffice.csv b/tests/integration/resources/import-from-libreoffice.csv new file mode 100644 index 000000000..13d758cf7 --- /dev/null +++ b/tests/integration/resources/import-from-libreoffice.csv @@ -0,0 +1,3 @@ +Date and Time,Col1,Col2,Col3,num,emoji,special,date,truth,time +2022-02-20 08:42,Val1,Val2,Val3,1,💙,Ä,2024-02-24,false,18:48 +2016-06-01 13:37,great,news,here,99,⚠,Ö,2016-06-01,true,01:23 diff --git a/tests/integration/resources/import-from-libreoffice.csv.license b/tests/integration/resources/import-from-libreoffice.csv.license new file mode 100644 index 000000000..21db92f72 --- /dev/null +++ b/tests/integration/resources/import-from-libreoffice.csv.license @@ -0,0 +1,3 @@ +SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors +SPDX-License-Identifier: AGPL-3.0-or-later + diff --git a/tests/integration/resources/import-from-libreoffice.ods b/tests/integration/resources/import-from-libreoffice.ods new file mode 100644 index 000000000..617a94508 Binary files /dev/null and b/tests/integration/resources/import-from-libreoffice.ods differ diff --git a/tests/integration/resources/import-from-libreoffice.ods.license b/tests/integration/resources/import-from-libreoffice.ods.license new file mode 100644 index 000000000..21db92f72 --- /dev/null +++ b/tests/integration/resources/import-from-libreoffice.ods.license @@ -0,0 +1,3 @@ +SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors +SPDX-License-Identifier: AGPL-3.0-or-later + diff --git a/tests/integration/resources/import-from-libreoffice.xlsx b/tests/integration/resources/import-from-libreoffice.xlsx new file mode 100644 index 000000000..492d8706e Binary files /dev/null and b/tests/integration/resources/import-from-libreoffice.xlsx differ diff --git a/tests/integration/resources/import-from-libreoffice.xlsx.license b/tests/integration/resources/import-from-libreoffice.xlsx.license new file mode 100644 index 000000000..21db92f72 --- /dev/null +++ b/tests/integration/resources/import-from-libreoffice.xlsx.license @@ -0,0 +1,3 @@ +SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors +SPDX-License-Identifier: AGPL-3.0-or-later + diff --git a/tests/integration/resources/import-from-ms365.xlsx b/tests/integration/resources/import-from-ms365.xlsx new file mode 100644 index 000000000..7e9ad13c4 Binary files /dev/null and b/tests/integration/resources/import-from-ms365.xlsx differ diff --git a/tests/integration/resources/import-from-ms365.xlsx.license b/tests/integration/resources/import-from-ms365.xlsx.license new file mode 100644 index 000000000..21db92f72 --- /dev/null +++ b/tests/integration/resources/import-from-ms365.xlsx.license @@ -0,0 +1,3 @@ +SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors +SPDX-License-Identifier: AGPL-3.0-or-later +