Skip to content

Commit

Permalink
fix(Import): reliable detection of intented datetime type
Browse files Browse the repository at this point in the history
Signed-off-by: Arthur Schiwon <[email protected]>
  • Loading branch information
blizzz committed Nov 15, 2024
1 parent e8320cc commit 845ef0c
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 20 deletions.
4 changes: 2 additions & 2 deletions lib/Service/ColumnTypes/DatetimeTimeBusiness.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 : '');
}

/**
Expand All @@ -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');
}

}
57 changes: 49 additions & 8 deletions lib/Service/ImportService.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,18 @@ private function getPreviewData(Worksheet $worksheet): array {
$column = $this->columns[$colIndex];

if (($column && $column->getType() === 'datetime') || (is_array($columns[$colIndex]) && $columns[$colIndex]['type'] === 'datetime')) {
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('Y-m-d H:i');
$value = Date::excelToDateTimeObject($value)->format($format);
} catch (\TypeError) {
$value = (new \DateTimeImmutable($value))->format('Y-m-d H:i');
$value = (new \DateTimeImmutable($value))->format($format);
}
} elseif (($column && $column->getType() === 'number' && $column->getNumberSuffix() === '%')
|| (is_array($columns[$colIndex]) && $columns[$colIndex]['type'] === 'number' && $columns[$colIndex]['numberSuffix'] === '%')) {
Expand Down Expand Up @@ -372,18 +380,32 @@ private function createRow(Row $row): void {

$value = $cell->getValue();
$hasData = $hasData || !empty($value);
if ($column->getType() === 'datetime' && $column->getSubtype() === '') {

if ($column->getType() === 'datetime') {
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('Y-m-d H:i');
$value = Date::excelToDateTimeObject($value)->format($format);
} catch (\TypeError) {
$value = (new \DateTimeImmutable($value))->format('Y-m-d H:i');
$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') {
Expand Down Expand Up @@ -496,15 +518,34 @@ private function parseColumnDataType(Cell $cell): array {
];

try {
unset($dateValue);

Check failure on line 521 in lib/Service/ImportService.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

UndefinedVariable

lib/Service/ImportService.php:521:10: UndefinedVariable: Cannot find referenced variable $dateValue (see https://psalm.dev/024)

Check failure on line 521 in lib/Service/ImportService.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-stable28

UndefinedVariable

lib/Service/ImportService.php:521:10: UndefinedVariable: Cannot find referenced variable $dateValue (see https://psalm.dev/024)
if ($value === false) {
// introduced in PHP 8.3, but provided by symfony-polyfill in server
throw new \DateMalformedStringException('We do not accept `false` here');

Check failure on line 524 in lib/Service/ImportService.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-master

UndefinedClass

lib/Service/ImportService.php:524:15: UndefinedClass: Class, interface or enum named DateMalformedStringException does not exist (see https://psalm.dev/019)

Check failure on line 524 in lib/Service/ImportService.php

View workflow job for this annotation

GitHub Actions / static-psalm-analysis dev-stable28

UndefinedClass

lib/Service/ImportService.php:524:15: UndefinedClass: Class, interface or enum named DateMalformedStringException does not exist (see https://psalm.dev/019)
}
$dateValue = new \DateTimeImmutable($value);
} catch (\Exception $e) {
} catch (\Exception) {
}
if ((isset($dateValue) && $originDataType === DataType::TYPE_STRING)

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';
} else if (!$containsDate && $containsTime) {
$subType = 'time';
} else {
$subType = '';
}

$dataType = [
'type' => 'datetime',
'subtype' => $cell->getCalculateDateTimeType() === Cell::CALCULATE_DATE_TIME_ASIS ? 'date' : '',
'subtype' => $subType,
];
} elseif ($originDataType === DataType::TYPE_NUMERIC) {
if (str_contains($formattedValue, '%')) {
Expand Down
14 changes: 7 additions & 7 deletions tests/integration/features/APIv1.feature
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,10 @@ Feature: APIv1
And table "Import test" with emoji "πŸ‘¨πŸ»β€πŸ’»" exists for user "participant1" as "base1"
When user imports file "/<importfile>" into last created table
Then import results have the following data
| found_columns_count | 8 |
| created_columns_count | 8 |
| inserted_rows_count | 2 |
| errors_count | 0 |
| 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 |
Expand All @@ -207,9 +207,9 @@ Feature: APIv1
| date | datetime |
| truth | selection |
Then table contains at least following rows
| Col1 | Col2 | Col3 | num | emoji | special | date | truth |
| Val1 | Val2 | Val3 | 1 | πŸ’™ | Γ„ | 2024-02-24 | false |
| great | news | here | 99 | ⚠ | Γ– | 2016-06-01 | true |
| 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 |
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/resources/import-from-libreoffice.csv
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
Col1,Col2,Col3,num,emoji,special,date,truth
Val1,Val2,Val3,1,πŸ’™,Γ„,2024-02-24,false
great,news,here,99,⚠,Γ–,2016-06-01,true
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
Binary file modified tests/integration/resources/import-from-libreoffice.ods
Binary file not shown.
Binary file modified tests/integration/resources/import-from-libreoffice.xlsx
Binary file not shown.
Binary file modified tests/integration/resources/import-from-ms365.xlsx
Binary file not shown.

0 comments on commit 845ef0c

Please sign in to comment.