Skip to content

Commit f52e4d9

Browse files
committed
fix(extra-fields): address PR #301 review (repeater config log, DRY)
- Warn in error log when repeater_config JSON is malformed (with field key) - Use msProductData::class instead of hardcoded string in ProductDataService - Drop redundant maxRows check in validateRows after normalizeRows enforces limit
1 parent a66b180 commit f52e4d9

2 files changed

Lines changed: 11 additions & 9 deletions

File tree

core/components/minishop3/src/Services/ExtraFields/RepeaterFieldService.php

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ class RepeaterFieldService
1111

1212
private modX $modx;
1313

14-
/** @var array<string, array<string, array>>|null */
14+
/** Request-scoped cache of repeater field configs keyed by model class. */
1515
private ?array $fieldsByClass = null;
1616

1717
public function __construct(modX $modx)
@@ -37,15 +37,22 @@ public function defaultConfig(): array
3737

3838
/**
3939
* @param mixed $json JSON string or array
40+
* @param string|null $fieldKey Extra field key for log context when config JSON is invalid
4041
*/
41-
public function parseConfig(mixed $json): array
42+
public function parseConfig(mixed $json, ?string $fieldKey = null): array
4243
{
4344
$config = $this->defaultConfig();
4445

4546
if (is_string($json) && $json !== '') {
4647
$decoded = json_decode($json, true);
4748
if (is_array($decoded)) {
4849
$config = array_merge($config, $decoded);
50+
} else {
51+
$context = $fieldKey !== null && $fieldKey !== '' ? " for field \"{$fieldKey}\"" : '';
52+
$this->modx->log(
53+
modX::LOG_LEVEL_WARN,
54+
'[ms3-repeater] malformed repeater_config JSON' . $context . ': ' . json_last_error_msg()
55+
);
4956
}
5057
} elseif (is_array($json)) {
5158
$config = array_merge($config, $json);
@@ -161,16 +168,11 @@ public function validateRows(array $rows, array $config): array
161168
{
162169
$errors = [];
163170
$minRows = (int)($config['minRows'] ?? 0);
164-
$maxRows = $config['maxRows'] ?? null;
165171

166172
if (count($rows) < $minRows) {
167173
$errors[] = "Minimum {$minRows} rows required";
168174
}
169175

170-
if ($maxRows !== null && count($rows) > (int)$maxRows) {
171-
$errors[] = "Maximum {$maxRows} rows allowed";
172-
}
173-
174176
foreach ($rows as $index => $row) {
175177
if (!is_array($row)) {
176178
$errors[] = "Row {$index} must be an object";
@@ -245,7 +247,7 @@ public function getRepeaterFieldsForClass(string $modelClass): array
245247
if ($key === '') {
246248
continue;
247249
}
248-
$map[$key] = $this->parseConfig($field->get('repeater_config'));
250+
$map[$key] = $this->parseConfig($field->get('repeater_config'), $key);
249251
}
250252

251253
$this->fieldsByClass[$modelClass] = $map;

core/components/minishop3/src/Services/Product/ProductDataService.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ protected function getProductRepeaterFields(): array
4848
{
4949
if ($this->productRepeaterFields === null) {
5050
$this->productRepeaterFields = $this->getRepeaterFieldService()->getRepeaterFieldsForClass(
51-
'MiniShop3\\Model\\msProductData'
51+
msProductData::class
5252
);
5353
}
5454

0 commit comments

Comments
 (0)