Skip to content

Commit dd1f06f

Browse files
ManukMinasyanManukMinasyan
committed
fix: review fixes for currency field settings and data consistency
- Use CurrencyFieldSettingsData with camelCase props and MapName mapper consistent with other data classes - Hydrate settings via data class instead of raw array access - Remove dead grouping setting (collected but never consumed) - Reduce display_type to symbol/code (the two actually implemented) - Remove obsolete HRK currency (replaced by EUR in 2023) - Add 3 decimal places option for BHD/KWD/OMR currencies - Stop hardcoding 2 decimal places in import/export transformers
1 parent 3021646 commit dd1f06f

File tree

4 files changed

+54
-41
lines changed

4 files changed

+54
-41
lines changed

‎src/Data/Settings/CurrencyFieldSettingsData.php‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,25 @@
44

55
namespace Relaticle\CustomFields\Data\Settings;
66

7+
use Spatie\LaravelData\Attributes\MapName;
78
use Spatie\LaravelData\Data;
9+
use Spatie\LaravelData\Mappers\SnakeCaseMapper;
810

11+
#[MapName(SnakeCaseMapper::class)]
912
class CurrencyFieldSettingsData extends Data
1013
{
1114
public function __construct(
12-
public string $currency_code = 'USD',
13-
public string $display_type = 'symbol',
14-
public int $decimal_places = 2,
15-
public string $grouping = 'default',
15+
public string $currencyCode = 'USD',
16+
public string $displayType = 'symbol',
17+
public int $decimalPlaces = 2,
1618
) {}
19+
20+
public static function fromAdditional(array $additional): self
21+
{
22+
return new self(
23+
currencyCode: $additional['currency_code'] ?? 'USD',
24+
displayType: $additional['display_type'] ?? 'symbol',
25+
decimalPlaces: (int) ($additional['decimal_places'] ?? 2),
26+
);
27+
}
1728
}

‎src/FieldTypeSystem/Definitions/CurrencyFieldType.php‎

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,14 @@ public function configure(): FieldSchema
4848
$state = preg_replace('/[^0-9.-]/', '', $state);
4949
}
5050

51-
return round(floatval($state), 2);
51+
return (float) $state;
5252
})
5353
->exportTransformer(function (mixed $value): ?string {
5454
if ($value === null) {
5555
return null;
5656
}
5757

58-
return number_format((float) $value, 2, '.', '');
58+
return rtrim(rtrim(number_format((float) $value, 10, '.', ''), '0'), '.');
5959
});
6060
}
6161

@@ -87,9 +87,7 @@ private function settingsSchema(): array
8787
->label('Display')
8888
->options([
8989
'symbol' => 'Symbol ($1,200.50)',
90-
'narrow_symbol' => 'Narrow Symbol ($1,200.50)',
9190
'code' => 'Code (USD 1,200.50)',
92-
'name' => 'Name (1,200.50 US dollars)',
9391
])
9492
->afterStateHydrated(function (Select $component, mixed $state): void {
9593
if (blank($state)) {
@@ -102,36 +100,25 @@ private function settingsSchema(): array
102100
->label('Decimal Places')
103101
->options(function (Get $get): array {
104102
$code = $get('settings.additional.currency_code') ?? 'USD';
105-
$formatter = new NumberFormatter('en_US', NumberFormatter::CURRENCY);
106-
$sample = $formatter->formatCurrency(1200.50, $code) ?: '1,200.50';
107103

108-
$formatterNoDecimals = new NumberFormatter('en_US', NumberFormatter::CURRENCY);
109-
$formatterNoDecimals->setAttribute(NumberFormatter::FRACTION_DIGITS, 0);
104+
$options = [];
110105

111-
$sampleNoDecimals = $formatterNoDecimals->formatCurrency(1200, $code) ?: '1,200';
106+
foreach ([0, 2, 3] as $digits) {
107+
$formatter = new NumberFormatter('en_US', NumberFormatter::CURRENCY);
108+
$formatter->setAttribute(NumberFormatter::FRACTION_DIGITS, $digits);
109+
$sample = $formatter->formatCurrency(1200.50, $code) ?: number_format(1200.50, $digits);
112110

113-
return [
114-
'2' => sprintf('2 decimals (%s)', $sample),
115-
'0' => sprintf('No decimals (%s)', $sampleNoDecimals),
116-
];
111+
$label = $digits === 0 ? 'No decimals' : "{$digits} decimals";
112+
$options[(string) $digits] = "{$label} ({$sample})";
113+
}
114+
115+
return $options;
117116
})
118117
->afterStateHydrated(function (Select $component, mixed $state): void {
119118
$component->state($state === null ? '2' : (string) $state);
120119
})
121120
->required(),
122121

123-
Select::make('settings.additional.grouping')
124-
->label('Grouping')
125-
->options([
126-
'default' => 'Default (1,200.50)',
127-
'none' => 'None (1200.50)',
128-
])
129-
->afterStateHydrated(function (Select $component, mixed $state): void {
130-
if (blank($state)) {
131-
$component->state('default');
132-
}
133-
})
134-
->required(),
135122
]),
136123
];
137124
}
@@ -195,7 +182,6 @@ private function getCurrencyOptions(): array
195182
'VND' => 'Vietnamese Dong',
196183
'BGN' => 'Bulgarian Lev',
197184
'RON' => 'Romanian Leu',
198-
'HRK' => 'Croatian Kuna',
199185
'ISK' => 'Icelandic Krona',
200186
'UAH' => 'Ukrainian Hryvnia',
201187
'PKR' => 'Pakistani Rupee',

‎src/Models/CustomField.php‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Relaticle\CustomFields\CustomFields;
1818
use Relaticle\CustomFields\Data\CustomFieldSettingsData;
1919
use Relaticle\CustomFields\Data\FieldTypeData;
20+
use Relaticle\CustomFields\Data\Settings\CurrencyFieldSettingsData;
2021
use Relaticle\CustomFields\Database\Factories\CustomFieldFactory;
2122
use Relaticle\CustomFields\Enums\CustomFieldsFeature;
2223
use Relaticle\CustomFields\Enums\CustomFieldWidth;
@@ -207,21 +208,35 @@ public function getFieldName(): string
207208
return 'custom_fields.'.$this->code;
208209
}
209210

211+
public function getCurrencySettings(): CurrencyFieldSettingsData
212+
{
213+
$additional = $this->settings->additional ?? [];
214+
215+
// Legacy fallback: decimal_places may live in validation_rules
216+
if (! isset($additional['decimal_places']) && $this->validation_rules?->has('decimal_places')) { // @phpstan-ignore nullsafe.neverNull
217+
$additional['decimal_places'] = $this->validation_rules->get('decimal_places');
218+
}
219+
220+
// Apply config default for currency_code
221+
if (! isset($additional['currency_code'])) {
222+
$additional['currency_code'] = config('custom-fields.currency.default_code', 'USD');
223+
}
224+
225+
return CurrencyFieldSettingsData::fromAdditional($additional);
226+
}
227+
210228
public function getDecimalPlaces(int $default = 2): int
211229
{
212-
return (int) ($this->settings->additional['decimal_places']
213-
?? $this->validation_rules?->get('decimal_places') // @phpstan-ignore nullsafe.neverNull
214-
?? $default);
230+
return $this->getCurrencySettings()->decimalPlaces;
215231
}
216232

217233
public function getCurrencyCode(): string
218234
{
219-
return $this->settings->additional['currency_code']
220-
?? config('custom-fields.currency.default_code', 'USD');
235+
return $this->getCurrencySettings()->currencyCode;
221236
}
222237

223238
public function getCurrencyDisplayType(): string
224239
{
225-
return $this->settings->additional['display_type'] ?? 'symbol';
240+
return $this->getCurrencySettings()->displayType;
226241
}
227242
}

‎tests/Feature/Filament/Components/CurrencyFieldComponentsTest.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,14 @@
150150
});
151151

152152
describe('CurrencyFieldType Export Transformer', function (): void {
153-
it('formats values with 2 decimal places for CSV export', function (): void {
153+
it('exports values preserving meaningful precision', function (): void {
154154
$transformer = (new CurrencyFieldType)->configure()->getExportTransformer();
155155

156-
expect($transformer(1234.5))->toBe('1234.50')
157-
->and($transformer(0))->toBe('0.00')
158-
->and($transformer(99.999))->toBe('100.00')
159-
->and($transformer(42))->toBe('42.00');
156+
expect($transformer(1234.5))->toBe('1234.5')
157+
->and($transformer(0))->toBe('0')
158+
->and($transformer(99.999))->toBe('99.999')
159+
->and($transformer(42))->toBe('42')
160+
->and($transformer(1234.50))->toBe('1234.5');
160161
});
161162

162163
it('returns null for null values in export', function (): void {

0 commit comments

Comments
 (0)