Skip to content

Commit a8792b8

Browse files
GuySartorelliGuySartorelli
authored
Merge pull request #11579 from creative-commoners/pulls/6.0/remove-dbfield-casting
ENH Remove casting from DBField subclasses
2 parents 9b0a258 + a2c7bfb commit a8792b8

File tree

7 files changed

+351
-33
lines changed

7 files changed

+351
-33
lines changed

‎src/ORM/FieldType/DBBoolean.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public function saveInto(ModelData $dataObject): void
6969
if ($this->value instanceof DBField) {
7070
$this->value->saveInto($dataObject);
7171
} else {
72-
$dataObject->__set($fieldName, (bool) $this->value);
72+
$dataObject->__set($fieldName, $this->value);
7373
}
7474
} else {
7575
$class = static::class;
@@ -118,6 +118,7 @@ private function convertBooleanLikeValue(mixed $value): mixed
118118
case 'false':
119119
case 'f':
120120
case '0':
121+
case '':
121122
return false;
122123
case 'true':
123124
case 't':

‎src/ORM/FieldType/DBDecimal.php‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -90,24 +90,6 @@ public function setValue(mixed $value, null|array|ModelData $record = null, bool
9090
return $this;
9191
}
9292

93-
public function saveInto(ModelData $model): void
94-
{
95-
$fieldName = $this->name;
96-
97-
if ($fieldName) {
98-
if ($this->value instanceof DBField) {
99-
$this->value->saveInto($model);
100-
} else {
101-
$value = (float) preg_replace('/[^0-9.\-\+]/', '', $this->value ?? '');
102-
$model->__set($fieldName, $value);
103-
}
104-
} else {
105-
throw new \UnexpectedValueException(
106-
"DBField::saveInto() Called on a nameless '" . static::class . "' object"
107-
);
108-
}
109-
}
110-
11193
public function scaffoldFormField(?string $title = null, array $params = []): ?FormField
11294
{
11395
return NumericField::create($this->name, $title)

‎src/ORM/FieldType/DBMoney.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace SilverStripe\ORM\FieldType;
44

5+
use Stringable;
56
use NumberFormatter;
67
use SilverStripe\Forms\FormField;
78
use SilverStripe\Forms\MoneyField;
@@ -39,7 +40,7 @@ public function getFormatter(): NumberFormatter
3940
public function Nice(): string
4041
{
4142
if (!$this->exists()) {
42-
return null;
43+
return '';
4344
}
4445
$amount = $this->getAmount();
4546
$currency = $this->getCurrency();
@@ -59,10 +60,11 @@ public function Nice(): string
5960
*/
6061
public function getValue(): ?string
6162
{
62-
if (!$this->exists()) {
63+
$amount = $this->getAmount();
64+
// Ensure $amount is stringable
65+
if (!is_scalar($amount) && !($amount instanceof Stringable)) {
6366
return null;
6467
}
65-
$amount = $this->getAmount();
6668
$currency = $this->getCurrency();
6769
if (empty($currency)) {
6870
return $amount;
@@ -81,7 +83,7 @@ public function setCurrency(?string $currency, bool $markChanged = true): static
8183
return $this;
8284
}
8385

84-
public function getAmount(): ?float
86+
public function getAmount(): mixed
8587
{
8688
return $this->getField('Amount');
8789
}

‎src/ORM/FieldType/DBPercentage.php‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,4 @@ public function Nice(): string
5656
{
5757
return number_format($this->value * 100, $this->decimalSize - 2) . '%';
5858
}
59-
60-
public function saveInto(ModelData $model): void
61-
{
62-
parent::saveInto($model);
63-
64-
$fieldName = $this->name;
65-
if ($fieldName && $model->$fieldName > 1.0) {
66-
$model->__set($fieldName, 1.0);
67-
}
68-
}
6959
}

‎tests/php/ORM/DBBooleanTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ public static function provideSetValue(): array
5757
'value' => 'F',
5858
'expected' => false,
5959
],
60+
'blank-string' => [
61+
'value' => '',
62+
'expected' => false,
63+
],
6064
'true-string' => [
6165
'value' => 'true',
6266
'expected' => true,
@@ -65,6 +69,10 @@ public static function provideSetValue(): array
6569
'value' => 'false',
6670
'expected' => false,
6771
],
72+
'fish-string' => [
73+
'value' => 'fish',
74+
'expected' => 'fish',
75+
],
6876
'2-int' => [
6977
'value' => 2,
7078
'expected' => 2,

‎tests/php/ORM/DBMoneyTest.php‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use SilverStripe\ORM\DB;
88
use SilverStripe\Dev\SapphireTest;
99
use SilverStripe\i18n\i18n;
10+
use PHPUnit\Framework\Attributes\DataProvider;
1011

1112
class DBMoneyTest extends SapphireTest
1213
{
@@ -339,6 +340,84 @@ public function testHasAmount()
339340
$this->assertFalse($obj->MyMoney->hasAmount());
340341
}
341342

343+
public static function provideSetGetValue(): array
344+
{
345+
return [
346+
'int' => [
347+
'amount' => 3,
348+
'currency' => 'NZD',
349+
'expected' => '3 NZD',
350+
],
351+
'string-int' => [
352+
'amount' => '3',
353+
'currency' => 'NZD',
354+
'expected' => '3 NZD',
355+
],
356+
'negative-int' => [
357+
'amount' => -3,
358+
'currency' => 'NZD',
359+
'expected' => '-3 NZD',
360+
],
361+
'negative-string-int' => [
362+
'amount' => '-3',
363+
'currency' => 'NZD',
364+
'expected' => '-3 NZD',
365+
],
366+
'float' => [
367+
'amount' => 3.65,
368+
'currency' => 'NZD',
369+
'expected' => '3.65 NZD',
370+
],
371+
'string-float' => [
372+
'amount' => '3.65',
373+
'currency' => 'NZD',
374+
'expected' => '3.65 NZD',
375+
],
376+
'negative-float' => [
377+
'amount' => -3.65,
378+
'currency' => 'NZD',
379+
'expected' => '-3.65 NZD',
380+
],
381+
'negative-string-float' => [
382+
'amount' => '-3.65',
383+
'currency' => 'NZD',
384+
'expected' => '-3.65 NZD',
385+
],
386+
'no-curency' => [
387+
'amount' => '1.23',
388+
'currency' => null,
389+
'expected' => '1.23',
390+
],
391+
'non-numeric-no-currency' => [
392+
'amount' => 'fish',
393+
'currency' => null,
394+
'expected' => 'fish',
395+
],
396+
'string' => [
397+
'amount' => 'fish',
398+
'currency' => 'NZD',
399+
'expected' => 'fish NZD',
400+
],
401+
'array' => [
402+
'amount' => [],
403+
'currency' => 'NZD',
404+
'expected' => null,
405+
],
406+
'null' => [
407+
'amount' => null,
408+
'currency' => 'NZD',
409+
'expected' => null,
410+
],
411+
];
412+
}
413+
414+
#[DataProvider('provideSetGetValue')]
415+
public function testSetGetValue(mixed $amount, ?string $currency, mixed $expected): void
416+
{
417+
$field = new DBMoney('MyField');
418+
$field->setValue(['Amount' => $amount, 'Currency' => $currency]);
419+
$this->assertSame($expected, $field->getValue());
420+
}
342421

343422
/**
344423
* In some cases and locales, validation expects non-breaking spaces.
@@ -353,4 +432,103 @@ protected function clean($input)
353432
$nbsp = html_entity_decode(' ', 0, 'UTF-8');
354433
return str_replace(' ', $nbsp ?? '', trim($input ?? ''));
355434
}
435+
436+
public static function provideValidate(): array
437+
{
438+
return [
439+
'zero' => [
440+
'currency' => 'NZD',
441+
'amount' => 1,
442+
'expected' => true,
443+
],
444+
'no-cents' => [
445+
'currency' => 'NZD',
446+
'amount' => 1,
447+
'expected' => true,
448+
],
449+
'cents' => [
450+
'currency' => 'NZD',
451+
'amount' => 1.52,
452+
'expected' => true,
453+
],
454+
'negative-zero-point-five' => [
455+
'currency' => 'NZD',
456+
'amount' => -0.5,
457+
'expected' => true,
458+
],
459+
'string-zero' => [
460+
'currency' => 'NZD',
461+
'amount' => '0',
462+
'expected' => true,
463+
],
464+
'string-no-cents' => [
465+
'currency' => 'NZD',
466+
'amount' => '1',
467+
'expected' => true,
468+
],
469+
'string-cents' => [
470+
'currency' => 'NZD',
471+
'amount' => '1.5',
472+
'expected' => true,
473+
],
474+
'string-negative-zero-point-five' => [
475+
'currency' => 'NZD',
476+
'amount' => '-0.5',
477+
'expected' => true,
478+
],
479+
'string-fish' => [
480+
'currency' => 'NZD',
481+
'amount' => 'fish',
482+
'expected' => false,
483+
],
484+
'empty-string' => [
485+
'currency' => 'NZD',
486+
'amount' => '',
487+
'expected' => false,
488+
],
489+
'null' => [
490+
'currency' => 'NZD',
491+
'amount' => null,
492+
'expected' => true,
493+
],
494+
'true' => [
495+
'currency' => 'NZD',
496+
'amount' => true,
497+
'expected' => false,
498+
],
499+
'false' => [
500+
'currency' => 'NZD',
501+
'amount' => false,
502+
'expected' => false,
503+
],
504+
'currency-blank' => [
505+
'currency' => '',
506+
'amount' => 1,
507+
'expected' => true,
508+
],
509+
'currency-null' => [
510+
'currency' => null,
511+
'amount' => 1,
512+
'expected' => true,
513+
],
514+
'currency-true' => [
515+
'currency' => true,
516+
'amount' => 1,
517+
'expected' => false,
518+
],
519+
'currency-false' => [
520+
'currency' => false,
521+
'amount' => 1,
522+
'expected' => false,
523+
],
524+
];
525+
}
526+
527+
#[DataProvider('provideValidate')]
528+
public function testValidate(mixed $currency, mixed $amount, bool $expected): void
529+
{
530+
$money = new DBMoney();
531+
$money->setValue(['Currency' => $currency, 'Amount' => $amount]);
532+
$this->assertEquals($expected, $money->validate()->isValid());
533+
}
356534
}

0 commit comments

Comments
 (0)