Skip to content

Commit b05eb15

Browse files
AIlkivAIlkiv
committed
chore: split value and ID parsing into separate methods
Signed-off-by: ailkiv <a.ilkiv.ye@gmail.com>
1 parent eae88cd commit b05eb15

File tree

5 files changed

+209
-26
lines changed

5 files changed

+209
-26
lines changed

‎lib/Service/ColumnTypes/IColumnTypeBusiness.php‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ interface IColumnTypeBusiness {
2626
public function parseValue($value, ?Column $column): string;
2727

2828
/**
29-
* tests if the given string can be parsed to a value of the column type
29+
* tests if the given value can be parsed to a value of the column type
3030
*
3131
* @param mixed $value
3232
* @param Column|null $column
@@ -42,4 +42,22 @@ public function canBeParsed($value, ?Column $column): bool;
4242
* @param int|null $rowId
4343
*/
4444
public function validateValue(mixed $value, Column $column, string $userId, int $tableId, ?int $rowId): void;
45+
46+
/**
47+
* tests if the given string can be parsed to a value/id of the column type
48+
*
49+
* @param mixed $value
50+
* @param Column|null $column
51+
* @return bool
52+
*/
53+
public function canBeParsedDisplayValue($value, ?Column $column): bool;
54+
55+
/**
56+
* parses the given string to a value/id of the column type
57+
*
58+
* @param mixed $value
59+
* @param Column|null $column
60+
* @return string
61+
*/
62+
public function parseDisplayValue($value, ?Column $column): string;
4563
}

‎lib/Service/ColumnTypes/SelectionBusiness.php‎

Lines changed: 45 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,28 @@ public function parseValue($value, ?Column $column = null): string {
2323
}
2424

2525
$intValue = (int)$value;
26-
if ((string)$intValue === (string)$value) {
27-
// if it seems to be an option ID
28-
foreach ($column->getSelectionOptionsArray() as $option) {
29-
if ($option['id'] === $intValue && $option['label'] !== $value) {
30-
return json_encode($option['id']);
31-
}
26+
if (!is_numeric($value) || $intValue != $value) {
27+
return '';
28+
}
29+
30+
foreach ($column->getSelectionOptionsArray() as $option) {
31+
if ($option['id'] === $intValue) {
32+
return json_encode($option['id']);
3233
}
33-
} else {
34-
foreach ($column->getSelectionOptionsArray() as $option) {
35-
if ($option['label'] === $value) {
36-
return json_encode($option['id']);
37-
}
34+
}
35+
36+
return '';
37+
}
38+
39+
public function parseDisplayValue($value, ?Column $column = null): string {
40+
if (!$column) {
41+
$this->logger->warning('No column given, but expected on ' . __FUNCTION__ . ' within ' . __CLASS__, ['exception' => new \Exception()]);
42+
return '';
43+
}
44+
45+
foreach ($column->getSelectionOptionsArray() as $option) {
46+
if ($option['label'] === $value) {
47+
return json_encode($option['id']);
3848
}
3949
}
4050

@@ -56,22 +66,34 @@ public function canBeParsed($value, ?Column $column = null): bool {
5666
}
5767

5868
$intValue = (int)$value;
59-
if ((string)$intValue === (string)$value) {
60-
// if it seems to be an option ID
61-
foreach ($column->getSelectionOptionsArray() as $option) {
62-
if ($option['id'] === $intValue && $option['label'] !== $value) {
63-
return true;
64-
}
65-
}
66-
} else {
67-
foreach ($column->getSelectionOptionsArray() as $option) {
68-
if ($option['label'] === $value) {
69-
return true;
70-
}
69+
if (!is_numeric($value) || $intValue != $value) {
70+
return false;
71+
}
72+
73+
foreach ($column->getSelectionOptionsArray() as $option) {
74+
if ($option['id'] === $intValue) {
75+
return true;
7176
}
7277
}
7378

7479
return false;
7580
}
7681

82+
public function canBeParsedDisplayValue($value, ?Column $column = null): bool {
83+
if (!$column) {
84+
$this->logger->warning('No column given, but expected on ' . __FUNCTION__ . ' within ' . __CLASS__, ['exception' => new \Exception()]);
85+
return false;
86+
}
87+
if ($value === null) {
88+
return true;
89+
}
90+
91+
foreach ($column->getSelectionOptionsArray() as $option) {
92+
if ($option['label'] === $value) {
93+
return true;
94+
}
95+
}
96+
97+
return false;
98+
}
7799
}

‎lib/Service/ColumnTypes/SuperBusiness.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ public function parseValue($value, ?Column $column = null): string {
2828
return json_encode($value);
2929
}
3030

31+
public function parseDisplayValue($value, ?Column $column = null): string {
32+
return $this->parseValue($value, $column);
33+
}
34+
3135
/**
3236
* @param mixed $value
3337
* @param Column|null $column
@@ -41,6 +45,10 @@ public function validateValue(mixed $value, Column $column, string $userId, int
4145
// override this method in the child class when needed
4246
}
4347

48+
public function canBeParsedDisplayValue($value, ?Column $column = null): bool {
49+
return $this->canBeParsed($value, $column);
50+
}
51+
4452
protected function isValidDate(string $dateString, string $format): bool {
4553
try {
4654
$dateTime = new DateTime($dateString);

‎lib/Service/ImportService.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,12 +367,12 @@ private function parseValueByColumnType(string $value, Column $column): string {
367367
$businessClassName .= ucfirst($column->getType()) . ucfirst($column->getSubtype()) . 'Business';
368368
/** @var IColumnTypeBusiness $columnBusiness */
369369
$columnBusiness = Server::get($businessClassName);
370-
if (!$columnBusiness->canBeParsed($value, $column)) {
370+
if (!$columnBusiness->canBeParsedDisplayValue($value, $column)) {
371371
$this->logger->warning('Value ' . $value . ' could not be parsed for column ' . $column->getTitle());
372372
$this->countParsingErrors++;
373373
return '';
374374
}
375-
return $columnBusiness->parseValue($value, $column);
375+
return $columnBusiness->parseDisplayValue($value, $column);
376376
} catch (NotFoundExceptionInterface|ContainerExceptionInterface $e) {
377377
$this->logger->debug('Column type business class not found', ['exception' => $e]);
378378
}

‎tests/unit/Service/ColumnTypes/SelectionBusinessTest.php‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
8+
namespace OCA\Tables\Service\ColumnTypes;
9+
10+
use OCA\Tables\Db\Column;
11+
use PHPUnit\Framework\TestCase;
12+
use Psr\Log\LoggerInterface;
13+
14+
class SelectionBusinessTest extends TestCase {
15+
16+
private SelectionBusiness $selectionBusiness;
17+
private LoggerInterface $logger;
18+
private Column $column;
19+
20+
public function setUp(): void {
21+
$this->logger = $this->createMock(LoggerInterface::class);
22+
$this->selectionBusiness = new SelectionBusiness($this->logger);
23+
24+
$this->column = $this->createMock(Column::class);
25+
$this->column->method('getSelectionOptionsArray')
26+
->willReturn([
27+
['id' => 1, 'label' => 'Option 1'],
28+
['id' => 2, 'label' => 'Option 2'],
29+
['id' => 3, 'label' => 'Option 3'],
30+
['id' => 4, 'label' => '1'],
31+
]);
32+
}
33+
34+
public function parseValueProvider(): array {
35+
return [
36+
'valid integer value' => [2, '2'],
37+
'valid string value' => ['2', '2'],
38+
'valid string value for numeric option' => ['4', '4'],
39+
'invalid value' => [5, ''],
40+
'null value' => [null, ''],
41+
'empty string' => ['', ''],
42+
'float value' => [1.5, ''],
43+
'boolean value' => [true, ''],
44+
'array value' => [[1], ''],
45+
];
46+
}
47+
48+
/**
49+
* @dataProvider parseValueProvider
50+
*/
51+
public function testParseValue($value, string $expected): void {
52+
$result = $this->selectionBusiness->parseValue($value, $this->column);
53+
$this->assertEquals($expected, $result);
54+
}
55+
56+
public function parseDisplayValueProvider(): array {
57+
return [
58+
'valid label' => ['Option 2', '2'],
59+
'invalid label' => ['Invalid Option', ''],
60+
'valid label for numeric option' => ['1', '4'],
61+
'null value' => [null, ''],
62+
'empty string' => ['', ''],
63+
'boolean value' => [true, ''],
64+
'array value' => [[1], ''],
65+
];
66+
}
67+
68+
/**
69+
* @dataProvider parseDisplayValueProvider
70+
*/
71+
public function testParseDisplayValue($value, string $expected): void {
72+
$result = $this->selectionBusiness->parseDisplayValue($value, $this->column);
73+
$this->assertEquals($expected, $result);
74+
}
75+
76+
public function canBeParsedProvider(): array {
77+
return [
78+
'valid integer 1' => [1, true],
79+
'valid string 4' => ['4', true],
80+
'invalid integer' => [5, false],
81+
'invalid integer 0' => [0, false],
82+
'null value' => [null, true],
83+
'empty string' => ['', false],
84+
'float value' => [1.5, false],
85+
'boolean value' => [true, false],
86+
'array value' => [[1], false],
87+
];
88+
}
89+
90+
/**
91+
* @dataProvider canBeParsedProvider
92+
*/
93+
public function testCanBeParsed($value, bool $expected): void {
94+
$result = $this->selectionBusiness->canBeParsed($value, $this->column);
95+
$this->assertEquals($expected, $result);
96+
}
97+
98+
public function canBeParsedDisplayValueProvider(): array {
99+
return [
100+
'valid label' => ['Option 2', true],
101+
'invalid label' => ['Invalid Option', false],
102+
'valid label for numeric option' => ['1', true],
103+
'null value' => [null, true],
104+
'empty string' => ['', false],
105+
'boolean value' => [true, false],
106+
'array value' => [[1], false],
107+
];
108+
}
109+
110+
/**
111+
* @dataProvider canBeParsedDisplayValueProvider
112+
*/
113+
public function testCanBeParsedDisplayValue($value, bool $expected): void {
114+
$result = $this->selectionBusiness->canBeParsedDisplayValue($value, $this->column);
115+
$this->assertEquals($expected, $result);
116+
}
117+
118+
public function withoutColumnProvider(): array {
119+
return [
120+
'parseValue' => ['parseValue', 1, ''],
121+
'parseDisplayValue' => ['parseDisplayValue', 'Option 1', ''],
122+
'canBeParsed' => ['canBeParsed', 1, false],
123+
'canBeParsedDisplayValue' => ['canBeParsedDisplayValue', 'Option 1', false],
124+
];
125+
}
126+
127+
/**
128+
* @dataProvider withoutColumnProvider
129+
*/
130+
public function testMethodsWithoutColumn(string $method, $value, $expected): void {
131+
$result = $this->selectionBusiness->$method($value, null);
132+
$this->assertEquals($expected, $result);
133+
}
134+
135+
}

0 commit comments

Comments
 (0)