Skip to content

Commit

Permalink
fix(Import): DateTime import fixes
Browse files Browse the repository at this point in the history
- do not try to pass `null` to DateTimeImmutable as it would cause an
  Exception
- be stricter on small letter-only values that might be turned into a
  DateTimeImmutable object because they match with timezone abbreviations
- do not consider pure numbers a date

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
  • Loading branch information
blizzz committed Jan 16, 2025
1 parent cb9ea97 commit 0e60292
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 35 deletions.
77 changes: 42 additions & 35 deletions lib/Service/ImportService.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@
use Psr\Container\ContainerExceptionInterface;
use Psr\Container\NotFoundExceptionInterface;
use Psr\Log\LoggerInterface;
use function is_string;
use function mb_strlen;
use function preg_match;

class ImportService extends SuperService {

Expand Down Expand Up @@ -168,12 +171,7 @@ private function getPreviewData(Worksheet $worksheet): array {
} else {
$format = 'Y-m-d H:i';
}

try {
$value = Date::excelToDateTimeObject($value)->format($format);
} catch (\TypeError) {
$value = (new \DateTimeImmutable($value))->format($format);
}
$value = $this->parseAndFormatDateTimeString($value, $format);
} elseif (($column && $column->getType() === 'number' && $column->getNumberSuffix() === '%')
|| (is_array($columns[$colIndex]) && $columns[$colIndex]['type'] === 'number' && $columns[$colIndex]['numberSuffix'] === '%')) {
$value = $value * 100;
Expand Down Expand Up @@ -382,30 +380,14 @@ private function createRow(Row $row): void {
$hasData = $hasData || !empty($value);

if ($column->getType() === 'datetime') {
if ($column->getType() === 'datetime' && $column->getSubtype() === 'date') {
if ($column->getSubtype() === 'date') {
$format = 'Y-m-d';
} elseif ($column->getType() === 'datetime' && $column->getSubtype() === 'time') {
} elseif ($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');
}
$value = $this->parseAndFormatDateTimeString($value, $format);
} elseif ($column->getType() === 'number' && $column->getNumberSuffix() === '%') {
$value = $value * 100;
} elseif ($column->getType() === 'selection' && $column->getSubtype() === 'check') {
Expand Down Expand Up @@ -440,6 +422,37 @@ private function createRow(Row $row): void {

}

private function valueToDateTimeImmutable(mixed $value): ?\DateTimeImmutable {
if (
$value === false
|| $value === null
|| (is_string($value)
&& mb_strlen($value) < 3 // Let pass potential 3-letter month names
&& preg_match('/\d/', $value) !== 1) // or anything containing a digit
) {
return null;
}
try {
$dt = Date::excelToDateTimeObject($value);
return \DateTimeImmutable::createFromMutable($dt);
} catch (\TypeError) {
try {
return (new \DateTimeImmutable($value));
} catch (\Exception $e) {
$this->logger->debug('Could not parse string {value} as date time.', [
'exception' => $e,
'value' => $value,
]);
return null;
}
}
}

private function parseAndFormatDateTimeString(?string $value, string $format): string {
$dateTime = $this->valueToDateTimeImmutable($value);
return $dateTime?->format($format) ?: '';
}

/**
* @param Row $firstRow
* @param Row $secondRow
Expand Down Expand Up @@ -528,17 +541,11 @@ private function parseColumnDataType(Cell $cell): array {
'subtype' => 'line',
];

try {
if ($value === false) {
throw new \Exception('We do not accept `false` here');
}
$dateValue = new \DateTimeImmutable($value);
} catch (\Exception) {
}

if (isset($dateValue)
if (!is_numeric($formattedValue)
&& ($this->valueToDateTimeImmutable($value) instanceof \DateTimeImmutable
|| Date::isDateTime($cell)
|| $originDataType === DataType::TYPE_ISO_DATE) {
|| $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;
Expand Down
26 changes: 26 additions & 0 deletions tests/integration/features/APIv1.feature
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,32 @@ Feature: APIv1
| import-from-ms365.xlsx |
| import-from-libreoffice.csv |

@api1 @import @current
Scenario: Import a document with optional field
Given user "participant1" uploads file "import-from-libreoffice-optional-fields.csv"
And table "Import test" with emoji "πŸ‘¨πŸ»β€πŸ’»" exists for user "participant1" as "base1"
When user imports file "/import-from-libreoffice-optional-fields.csv" into last created table
Then import results have the following data
| found_columns_count | 9 |
| created_columns_count | 9 |
| inserted_rows_count | 2 |
| errors_count | 0 |
# At the moment, we only take the first row into account, when determining the cell format
# Hence, it is expected that all turn out to be text
Then table has at least following typed columns
| Case | text |
| Col1 | text |
| num | text |
| emoji | text |
| special | text |
| date | text |
| truth | text |
# the library handles "true" as boolean and so is converted into the text representation "1"
Then table contains at least following rows
| Case | Date and Time | Col1 | num | emoji | special | date | truth | time |
| A | | | | | | | | |
| B | 2016-06-01 13:37 | great | 99 | ⚠ | Γ– | 2016-06-01 | 1 | 01:23 |

@api1
Scenario: Create, edit and delete views
Given table "View test" with emoji "πŸ‘¨πŸ»β€πŸ’»" exists for user "participant1" as "view-test"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Case,Date and Time,Col1,num,emoji,special,date,truth,time
A,,,,,,,,
B,2016-06-01 13:37,great,99,⚠,Γ–,2016-06-01,true,01:23
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
SPDX-License-Identifier: AGPL-3.0-or-later

0 comments on commit 0e60292

Please sign in to comment.