From cd326bf21c09c80d4e4816713857dad8fc06d0e8 Mon Sep 17 00:00:00 2001 From: Git'Fellow <12234510+solracsf@users.noreply.github.com> Date: Fri, 29 May 2026 11:22:49 +0200 Subject: [PATCH] fix(export): type numeric answers as numbers in exports Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> --- lib/Service/SubmissionService.php | 27 +++++---- tests/Unit/Service/SubmissionServiceTest.php | 59 ++++++++++++++++++++ 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/lib/Service/SubmissionService.php b/lib/Service/SubmissionService.php index 5cff29713..8ab84c916 100644 --- a/lib/Service/SubmissionService.php +++ b/lib/Service/SubmissionService.php @@ -35,6 +35,7 @@ use OCP\IUserSession; use OCP\Mail\IMailer; +use PhpOffice\PhpSpreadsheet\Cell\DataType; use PhpOffice\PhpSpreadsheet\IOFactory; use PhpOffice\PhpSpreadsheet\Spreadsheet; use PhpOffice\PhpSpreadsheet\Writer\Csv; @@ -348,17 +349,23 @@ private function exportData(array $header, array $data, string $fileFormat, ?Fil $activeWorksheet->getStyle([$column, $row]) ->getAlignment() ->setWrapText(true); + } elseif (!is_string($value)) { + $activeWorksheet->setCellValue([$column, $row], $value); + } elseif ($fileFormat === 'csv') { + // CSV is escaped (not typed) to neutralise formula-triggering values when opened in a spreadsheet app. + $activeWorksheet->getCell([$column, $row]) + ->setValueExplicit($this->escapeCSV($value), DataType::TYPE_STRING); + } elseif (is_numeric($value) + && !in_array($value[0], ['=', '+', '-', '@'], true) + && (string)(+$value) === $value) { + // Store numeric-looking answers (e.g. Radio labels "1"-"5") as numbers so they can be + // aggregated. The round-trip check keeps values whose canonical numeric form would differ + // (leading-zero phone numbers, "1e5", oversized ids, ...) as text. + $activeWorksheet->setCellValue([$column, $row], +$value); } else { - // Explicitly set the type of the value to string for values that start with '=' to prevent it being interpreted as formulas - if (is_string($value)) { - $activeWorksheet->getCell([$column, $row]) - ->setValueExplicit($fileFormat === 'csv' - ? $this->escapeCSV($value) - : $value, - ); - } else { - $activeWorksheet->setCellValue([$column, $row], $value); - } + // Explicitly type as string so values that start with '=' are not interpreted as formulas. + $activeWorksheet->getCell([$column, $row]) + ->setValueExplicit($value, DataType::TYPE_STRING); } } } diff --git a/tests/Unit/Service/SubmissionServiceTest.php b/tests/Unit/Service/SubmissionServiceTest.php index 23330ee6e..6fe522b9e 100644 --- a/tests/Unit/Service/SubmissionServiceTest.php +++ b/tests/Unit/Service/SubmissionServiceTest.php @@ -32,6 +32,8 @@ use OCP\IUserManager; use OCP\IUserSession; use OCP\Mail\IMailer; +use PhpOffice\PhpSpreadsheet\Cell\DataType; +use PhpOffice\PhpSpreadsheet\IOFactory; use PHPUnit\Framework\MockObject\MockObject; use Psr\Log\LoggerInterface; @@ -565,6 +567,63 @@ public function testGetSubmissionsDataThrowsExceptionOnInvalidFormat() { $this->submissionService->getSubmissionsData($form, 'invalid'); } + public function dataExportNumericTyping() { + return [ + // Radio (and similar choice) answers are always strings, even when the option labels are + // pure numbers. For spreadsheet formats they must be written as numbers so they can be + // aggregated (e.g. averaged) in the spreadsheet application. + 'radio-numeric-label-xlsx' => ['3', 'xlsx', DataType::TYPE_NUMERIC, 3], + 'radio-numeric-label-ods' => ['5', 'ods', DataType::TYPE_NUMERIC, 5], + 'numeric-zero' => ['0', 'xlsx', DataType::TYPE_NUMERIC, 0], + 'numeric-decimal' => ['3.5', 'xlsx', DataType::TYPE_NUMERIC, 3.5], + // Values starting with a formula-trigger character keep the string typing so they are + // never interpreted as formulas. + 'formula' => ['=SUM(A1:A2)', 'xlsx', DataType::TYPE_STRING, '=SUM(A1:A2)'], + 'negative-number' => ['-5', 'xlsx', DataType::TYPE_STRING, '-5'], + // Strings whose canonical numeric form would differ must stay text to avoid data loss + // (leading-zero phone numbers, scientific notation, ...). + 'leading-zero' => ['0123456789', 'xlsx', DataType::TYPE_STRING, '0123456789'], + 'scientific' => ['1e5', 'xlsx', DataType::TYPE_STRING, '1e5'], + 'text' => ['Option A', 'xlsx', DataType::TYPE_STRING, 'Option A'], + ]; + } + + /** + * @dataProvider dataExportNumericTyping + * + * @param string $value the raw (string) answer as stored in the database + * @param string $fileFormat spreadsheet format to export to + * @param string $expectedType the expected PhpSpreadsheet cell data type + * @param mixed $expectedValue the expected cell value after a write/read round-trip + */ + public function testExportNumericTyping(string $value, string $fileFormat, string $expectedType, mixed $expectedValue) { + // Hand out real temporary files so the spreadsheet can actually be written and reloaded. + $this->tempManager->expects($this->any()) + ->method('getTemporaryFile') + ->willReturnCallback(fn () => tempnam(sys_get_temp_dir(), 'forms-export-')); + + $content = self::invokePrivate( + $this->submissionService, + 'exportData', + [['Question 1'], [[$value]], $fileFormat, null], + ); + + // Reload the produced file and inspect the actual data type of the answer cell. + $reloadPath = tempnam(sys_get_temp_dir(), 'forms-reload-'); + file_put_contents($reloadPath, $content); + $cell = IOFactory::load($reloadPath)->getSheet(0)->getCell([1, 2]); + + $this->assertSame($expectedType, $cell->getDataType()); + if ($expectedType === DataType::TYPE_NUMERIC) { + $this->assertEquals($expectedValue, $cell->getValue()); + } else { + // assertSame guards against a numeric-looking string being silently coerced. + $this->assertSame($expectedValue, $cell->getValue()); + } + + unlink($reloadPath); + } + /** * Setting up a very simple default CsvTest */