fix and improve detection and import of ods, xlsx and csv documents #1446
Conversation
blizzz
force-pushed
the
fix/1440/import-types
branch
from
d599204
to
a67ba6e
Compare
blizzz
changed the title
blizzz
changed the title
blizzz
force-pushed
the
fix/1440/import-types
branch
3 times, most recently
from
b154e9e
to
e8320cc
Compare
blizzz
added
3. to review
blizzz
force-pushed
the
fix/1440/import-types
branch
from
845ef0c
to
095314a
Compare
blizzz
changed the title
blizzz
force-pushed
the
fix/1440/import-types
branch
from
095314a
to
3beaa58
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
improved handling of LibreOffice-generated XLSX documents Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
… and get date value with fault tolerance Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
blizzz
force-pushed
the
fix/1440/import-types
branch
from
3beaa58
to
044d62d
Compare
|
/backport to stable0.8 |
lib/Service/ImportService.php
Outdated
| if ($cell->getDataType() === 'null') { | ||
| // LibreOffice generated XLSX doc may have more empty columns in the first row. | ||
| // Continue without increasing error count. | ||
| // Question: What about tables where a column does not have a heading? |
There was a problem hiding this comment.
Is this question left as a TODO? or addressed below?
There was a problem hiding this comment.
Ha. Good finding. Forgot about this one. Let me check me one thing, to ensure that this does not change from the previous behavior. Might indeed need a touch.
There is also a documentation component in that headers were expected on all columns.
There was a problem hiding this comment.
Okay, import works as before. But there is one difference:
Previously it was one.
It was testing with a three-column table where the middle-one was not having a caption. But the values were set (always strings for simplicity). Before the change and now, only the columns with a caption were being added, the middle one ignored.
On import, however, the first two columns were added and the third column ignored.
To visualize, the original
| Caption 1 | Caption 3 | |
|---|---|---|
| Aaa | A | Zzz |
| Bbb | B | Yyy |
| Ccc | C | Xxx |
turns (elements taken over in italic) into
| Caption 1 | Caption 3 |
|---|---|
| Aaa | A |
| Bbb | B |
| Ccc | C |
Our columns require a caption. Something to document, but I look into increasing the error again, if it is not a trailing column.
There was a problem hiding this comment.
There was a problem hiding this comment.
We could also add that as a note to the import dialog, most users will probably never get to the wiki
There was a problem hiding this comment.
We could also add that as a note to the import dialog, most users will probably never get to the wiki
👍 Added a brief supplement in 36179dc
…ders Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
backportbot
bot
removed
the
backport-request
addresses #1440
This PR fixes a few corner cases with importing from different document types, especially about recognization of date formats (but not only). For instance, it is not possible again to re-import and exported CSV.
Integration tests are extended with column formats and do not only test csv import anymore, but also ODS, XLSX (from 365) and XLSX (from LibreOffice) – they have some subtle differences and failed differently along the way.
Historic notes
With the first commit, the added test fails as expected.
The second commit brings in two tests for a similar table in xlsx files (by SharePoint/365 and by LibreOffice). The first tests succeeds with a little datetime adjustment – containing the time, although only the date was entered as value. Maybe it is possible to read out the format and adjust automatically. The second test against the LibreOffice-created file fails with import errors. This needs further investigation.
P.S.: Changed back to date-only type.
The third commit adds the treatment of date-only datetime types imported from native xlsx.
The eights commit also fixes the rendering of the value in the preview dialog. For instance in this screenshot, the Date used to be shown as Excel timestamp.