Skip to content

Commit 556fe9e

Browse files
bhlindemannbhlindemann
committed
feat(Settings): enhance validation and error handling for settings
- Added validation methods for default Swiper configuration and CSS variables in Settings.php. - Updated SettingsController to utilize new validation methods and handle errors appropriately. - Enhanced Twig templates to display validation errors for various settings fields, improving user feedback.
1 parent 36acbae commit 556fe9e

File tree

8 files changed

+322
-12
lines changed

8 files changed

+322
-12
lines changed

‎src/controllers/SettingsController.php‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ public function actionSave(): ?Response
139139
$this->requirePostRequest();
140140

141141
$plugin = SlideshowManager::getInstance();
142+
$section = $this->_validSettingsSection($this->request->getBodyParam('section', 'general'));
143+
$attributesToValidate = $this->_validationAttributesForSection($section);
142144

143145
// Load current settings from database
144146
$settings = Settings::loadFromDatabase();
@@ -174,9 +176,8 @@ public function actionSave(): ?Response
174176
$decoded = json_decode($rawStylesJson, true);
175177

176178
if (!is_array($decoded)) {
177-
$settings->addError('swiperCssVars', Craft::t('slideshow-manager', 'Style presets JSON is invalid.'));
179+
$settings->addError('swiperCssVars._styles', Craft::t('slideshow-manager', 'Style presets JSON is invalid.'));
178180
Craft::$app->getSession()->setError(Craft::t('slideshow-manager', 'Could not save settings.'));
179-
$section = $this->_validSettingsSection($this->request->getBodyParam('section', 'general'));
180181
$template = "slideshow-manager/settings/{$section}";
181182
return $this->renderTemplate($template, [
182183
'settings' => $settings,
@@ -186,9 +187,8 @@ public function actionSave(): ?Response
186187
// Ensure each preset is an object/array of CSS vars
187188
foreach ($decoded as $handle => $vars) {
188189
if (!is_string($handle) || $handle === '' || !is_array($vars)) {
189-
$settings->addError('swiperCssVars', Craft::t('slideshow-manager', 'Style presets must be an object keyed by style handle, with each value as an object.'));
190+
$settings->addError('swiperCssVars._styles', Craft::t('slideshow-manager', 'Style presets must be an object keyed by style handle, with each value as an object.'));
190191
Craft::$app->getSession()->setError(Craft::t('slideshow-manager', 'Could not save settings.'));
191-
$section = $this->_validSettingsSection($this->request->getBodyParam('section', 'general'));
192192
$template = "slideshow-manager/settings/{$section}";
193193
return $this->renderTemplate($template, [
194194
'settings' => $settings,
@@ -201,6 +201,12 @@ public function actionSave(): ?Response
201201
}
202202
}
203203

204+
// Skip validation for fields overridden by config.
205+
$attributesToValidate = array_values(array_filter(
206+
$attributesToValidate,
207+
static fn(string $attribute): bool => !$settings->isOverriddenByConfig($attribute),
208+
));
209+
204210
// Only update fields that were posted and are not overridden by config
205211
foreach ($settingsData as $key => $value) {
206212
if (!$settings->isOverriddenByConfig($key) && property_exists($settings, $key)) {
@@ -215,16 +221,12 @@ public function actionSave(): ?Response
215221
}
216222

217223
// Validate
218-
if (!$settings->validate()) {
224+
if (!$settings->validate($attributesToValidate)) {
219225
Craft::$app->getSession()->setError(Craft::t('slideshow-manager', 'Could not save settings.'));
220226

221227
// Log validation failure
222228
$this->logWarning('Settings validation failed', ['errors' => $settings->getErrors()]);
223229

224-
// Get the section to re-render the correct template with errors
225-
$section = $this->_validSettingsSection(
226-
$this->request->getBodyParam('section', 'general'),
227-
);
228230
$template = "slideshow-manager/settings/{$section}";
229231

230232
return $this->renderTemplate($template, [
@@ -233,7 +235,7 @@ public function actionSave(): ?Response
233235
}
234236

235237
// Save settings to database
236-
if ($settings->saveToDatabase()) {
238+
if ($settings->saveToDatabase($attributesToValidate)) {
237239
// Log successful save
238240
$this->logInfo('Settings saved successfully', [
239241
'userId' => Craft::$app->getUser()->getId(),
@@ -261,4 +263,20 @@ private function _validSettingsSection(string $section): string
261263

262264
return in_array($section, $allowed, true) ? $section : 'general';
263265
}
266+
267+
/**
268+
* Return top-level settings attributes validated for the active section.
269+
*
270+
* @param string $section
271+
* @return array
272+
*/
273+
private function _validationAttributesForSection(string $section): array
274+
{
275+
return match ($section) {
276+
'general' => ['pluginName', 'autoLoadSwiperCss', 'autoLoadSwiperJs', 'enableCache', 'cacheDuration', 'logLevel'],
277+
'basic', 'layout', 'controls', 'advanced' => ['defaultSwiperConfig'],
278+
'styles' => ['swiperCssVars'],
279+
default => [],
280+
};
281+
}
264282
}

‎src/models/Settings.php‎

Lines changed: 265 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,277 @@ protected function defineRules(): array
122122
return [
123123
[['pluginName'], 'string'],
124124
[['autoLoadSwiperCss', 'autoLoadSwiperJs', 'enableCache'], 'boolean'],
125-
[['cacheDuration'], 'integer', 'min' => 1],
125+
[['cacheDuration'], 'integer', 'min' => 1, 'max' => 2147483647],
126126
[['defaultSwiperConfig', 'swiperCssVars'], 'safe'],
127+
[['defaultSwiperConfig'], 'validateDefaultSwiperConfig'],
128+
[['swiperCssVars'], 'validateSwiperCssVars'],
127129
[['logLevel'], 'in', 'range' => ['debug', 'info', 'warning', 'error']],
128130
[['logLevel'], 'validateLogLevel'],
129131
];
130132
}
131133

134+
/**
135+
* Validate Swiper configuration values.
136+
*/
137+
public function validateDefaultSwiperConfig(string $attribute): void
138+
{
139+
$config = $this->$attribute;
140+
if (!is_array($config)) {
141+
$this->addError($attribute, Craft::t(static::pluginHandle(), 'Swiper configuration must be an object.'));
142+
return;
143+
}
144+
145+
$speed = $this->nestedValue($config, ['speed']);
146+
if ($speed !== null && (!$this->isNumeric($speed) || (float)$speed < 0 || (float)$speed > 100000)) {
147+
$this->addError('defaultSwiperConfig.speed', Craft::t(static::pluginHandle(), 'Speed must be between 0 and 100000 ms.'));
148+
}
149+
150+
$slidesPerView = $this->nestedValue($config, ['slidesPerView']);
151+
if ($slidesPerView !== null && (!$this->isNumeric($slidesPerView) || (float)$slidesPerView < 1 || (float)$slidesPerView > 20)) {
152+
$this->addError('defaultSwiperConfig.slidesPerView', Craft::t(static::pluginHandle(), 'Slides per view must be between 1 and 20.'));
153+
}
154+
155+
$spaceBetween = $this->nestedValue($config, ['spaceBetween']);
156+
if ($spaceBetween !== null && (!$this->isNumeric($spaceBetween) || (float)$spaceBetween < 0 || (float)$spaceBetween > 1000)) {
157+
$this->addError('defaultSwiperConfig.spaceBetween', Craft::t(static::pluginHandle(), 'Space between must be between 0 and 1000 pixels.'));
158+
}
159+
160+
$autoplayDelay = $this->nestedValue($config, ['autoplay', 'delay']);
161+
if ($autoplayDelay !== null && (!$this->isNumeric($autoplayDelay) || (float)$autoplayDelay < 100 || (float)$autoplayDelay > 600000)) {
162+
$this->addError('defaultSwiperConfig.autoplay.delay', Craft::t(static::pluginHandle(), 'Autoplay delay must be between 100 and 600000 ms.'));
163+
}
164+
165+
$customTemplate = $this->nestedValue($config, ['pagination', 'customTemplate']);
166+
if ($customTemplate !== null && is_string($customTemplate)) {
167+
if (preg_match_all('/\{([^}]+)\}/', $customTemplate, $matches) > 0) {
168+
foreach ($matches[1] as $token) {
169+
if (!in_array($token, ['current', 'total'], true)) {
170+
$this->addError(
171+
'defaultSwiperConfig.pagination.customTemplate',
172+
Craft::t(static::pluginHandle(), 'Unsupported token "{token}". Allowed tokens: {current}, {total}.', ['token' => $token])
173+
);
174+
break;
175+
}
176+
}
177+
}
178+
}
179+
180+
$loadPrevNext = $this->nestedValue($config, ['lazy', 'loadPrevNext']);
181+
if ($loadPrevNext !== null && (!$this->isNumeric($loadPrevNext) || (float)$loadPrevNext < 0 || (float)$loadPrevNext > 20)) {
182+
$this->addError('defaultSwiperConfig.lazy.loadPrevNext', Craft::t(static::pluginHandle(), 'Load prev/next must be between 0 and 20.'));
183+
}
184+
185+
$zoomMin = $this->nestedValue($config, ['zoom', 'minRatio']);
186+
$zoomMax = $this->nestedValue($config, ['zoom', 'maxRatio']);
187+
if ($zoomMin !== null && (!$this->isNumeric($zoomMin) || (float)$zoomMin < 1 || (float)$zoomMin > 20)) {
188+
$this->addError('defaultSwiperConfig.zoom.minRatio', Craft::t(static::pluginHandle(), 'Zoom min ratio must be between 1 and 20.'));
189+
}
190+
if ($zoomMax !== null && (!$this->isNumeric($zoomMax) || (float)$zoomMax < 1 || (float)$zoomMax > 20)) {
191+
$this->addError('defaultSwiperConfig.zoom.maxRatio', Craft::t(static::pluginHandle(), 'Zoom max ratio must be between 1 and 20.'));
192+
}
193+
if ($this->isNumeric($zoomMin) && $this->isNumeric($zoomMax) && (float)$zoomMax < (float)$zoomMin) {
194+
$this->addError('defaultSwiperConfig.zoom.maxRatio', Craft::t(static::pluginHandle(), 'Zoom max ratio must be greater than or equal to zoom min ratio.'));
195+
}
196+
197+
$breakpoints = $this->nestedValue($config, ['breakpoints']);
198+
if ($breakpoints !== null) {
199+
if (!is_array($breakpoints)) {
200+
$this->addError('defaultSwiperConfig.breakpoints', Craft::t(static::pluginHandle(), 'Breakpoints must be an array.'));
201+
} else {
202+
foreach ($breakpoints as $index => $breakpoint) {
203+
if (!is_array($breakpoint)) {
204+
$this->addError('defaultSwiperConfig.breakpoints', Craft::t(static::pluginHandle(), 'Breakpoint #{index} must be an object.', ['index' => (string)($index + 1)]));
205+
continue;
206+
}
207+
$width = $breakpoint['width'] ?? null;
208+
$bpSlidesPerView = $breakpoint['slidesPerView'] ?? null;
209+
$bpSpaceBetween = $breakpoint['spaceBetween'] ?? null;
210+
if (!$this->isNumeric($width) || (float)$width < 0) {
211+
$this->addError('defaultSwiperConfig.breakpoints', Craft::t(static::pluginHandle(), 'Breakpoint width must be a number greater than or equal to 0.'));
212+
break;
213+
}
214+
if (!$this->isNumeric($bpSlidesPerView) || (float)$bpSlidesPerView < 1 || (float)$bpSlidesPerView > 20) {
215+
$this->addError('defaultSwiperConfig.breakpoints', Craft::t(static::pluginHandle(), 'Breakpoint slides per view must be between 1 and 20.'));
216+
break;
217+
}
218+
if (!$this->isNumeric($bpSpaceBetween) || (float)$bpSpaceBetween < 0 || (float)$bpSpaceBetween > 1000) {
219+
$this->addError('defaultSwiperConfig.breakpoints', Craft::t(static::pluginHandle(), 'Breakpoint space between must be between 0 and 1000 pixels.'));
220+
break;
221+
}
222+
}
223+
}
224+
}
225+
}
226+
227+
/**
228+
* Validate CSS vars settings payload.
229+
*/
230+
public function validateSwiperCssVars(string $attribute): void
231+
{
232+
$cssVars = $this->$attribute;
233+
if (!is_array($cssVars)) {
234+
$this->addError($attribute, Craft::t(static::pluginHandle(), 'CSS vars configuration must be an object.'));
235+
return;
236+
}
237+
238+
$activeHandle = $cssVars['_active'] ?? '';
239+
if ($activeHandle !== '' && !preg_match('/^[a-z0-9_-]+$/i', (string)$activeHandle)) {
240+
$this->addError('swiperCssVars._active', Craft::t(static::pluginHandle(), 'Active style handle may only contain letters, numbers, hyphens, and underscores.'));
241+
}
242+
243+
$styles = $cssVars['_styles'] ?? null;
244+
if ($styles !== null && !is_array($styles)) {
245+
$this->addError('swiperCssVars._styles', Craft::t(static::pluginHandle(), 'Style presets must be an object keyed by style handle.'));
246+
}
247+
248+
$lengthFields = [
249+
'navigationSize',
250+
'navigationTopOffset',
251+
'navigationSidesOffset',
252+
'navigationPadding',
253+
'navigationRadius',
254+
'paginationBulletSize',
255+
'paginationBulletWidth',
256+
'paginationBulletHeight',
257+
'paginationBulletHorizontalGap',
258+
'paginationBulletVerticalGap',
259+
'paginationProgressbarSize',
260+
'paginationLeft',
261+
'paginationRight',
262+
'paginationTop',
263+
'paginationBottom',
264+
'scrollbarBorderRadius',
265+
'scrollbarTop',
266+
'scrollbarBottom',
267+
'scrollbarLeft',
268+
'scrollbarRight',
269+
'scrollbarSidesOffset',
270+
'scrollbarSize',
271+
];
272+
$colorFields = [
273+
'themeColor',
274+
'navigationColor',
275+
'navigationInactiveColor',
276+
'navigationBg',
277+
'navigationBgHover',
278+
'navigationBorderColor',
279+
'navigationBorderColorHover',
280+
'paginationColor',
281+
'paginationBulletInactiveColor',
282+
'paginationFractionColor',
283+
'paginationProgressbarBgColor',
284+
'scrollbarBgColor',
285+
'scrollbarDragBgColor',
286+
'thumbActiveColor',
287+
'slideBgColor',
288+
];
289+
$opacityFields = [
290+
'paginationBulletInactiveOpacity',
291+
'paginationBulletOpacity',
292+
];
293+
294+
foreach ($lengthFields as $field) {
295+
$value = trim((string)($cssVars[$field] ?? ''));
296+
if ($value === '') {
297+
continue;
298+
}
299+
if (!$this->isValidCssLengthOrKeyword($value, ['auto', 'inherit'])) {
300+
$this->addError(
301+
"swiperCssVars.{$field}",
302+
Craft::t(
303+
static::pluginHandle(),
304+
'Invalid value. Use a CSS length/percentage (for example: 44px, 0.5rem, 50%), `auto`, `inherit`, or `var(...)`/`calc(...)`.'
305+
)
306+
);
307+
}
308+
}
309+
310+
foreach ($colorFields as $field) {
311+
$value = trim((string)($cssVars[$field] ?? ''));
312+
if ($value === '') {
313+
continue;
314+
}
315+
if (!$this->isValidCssColor($value)) {
316+
$this->addError(
317+
"swiperCssVars.{$field}",
318+
Craft::t(
319+
static::pluginHandle(),
320+
'Invalid color. Use hex, rgb()/rgba(), hsl()/hsla(), `transparent`, `currentColor`, or `var(...)`.'
321+
)
322+
);
323+
}
324+
}
325+
326+
foreach ($opacityFields as $field) {
327+
$value = trim((string)($cssVars[$field] ?? ''));
328+
if ($value === '') {
329+
continue;
330+
}
331+
if (!$this->isNumeric($value) || (float)$value < 0 || (float)$value > 1) {
332+
$this->addError(
333+
"swiperCssVars.{$field}",
334+
Craft::t(static::pluginHandle(), 'Invalid opacity. Use a number between 0 and 1.')
335+
);
336+
}
337+
}
338+
}
339+
340+
/**
341+
* @param mixed $value
342+
*/
343+
private function isNumeric(mixed $value): bool
344+
{
345+
return is_int($value) || is_float($value) || (is_string($value) && is_numeric($value));
346+
}
347+
348+
/**
349+
* @param array<string,mixed> $source
350+
* @param array<int,string> $path
351+
* @return mixed
352+
*/
353+
private function nestedValue(array $source, array $path): mixed
354+
{
355+
$value = $source;
356+
foreach ($path as $segment) {
357+
if (!is_array($value) || !array_key_exists($segment, $value)) {
358+
return null;
359+
}
360+
$value = $value[$segment];
361+
}
362+
363+
return $value;
364+
}
365+
366+
private function isValidCssLengthOrKeyword(string $value, array $keywords = []): bool
367+
{
368+
if (in_array(strtolower($value), array_map('strtolower', $keywords), true)) {
369+
return true;
370+
}
371+
372+
if (preg_match('/^(var|calc|clamp|min|max)\(.+\)$/i', $value) === 1) {
373+
return true;
374+
}
375+
376+
return preg_match('/^-?(?:\d+|\d*\.\d+)(?:px|rem|em|%|vh|vw|vmin|vmax|ch|ex|pt|pc|cm|mm|in)$/i', $value) === 1;
377+
}
378+
379+
private function isValidCssColor(string $value): bool
380+
{
381+
if (preg_match('/^(transparent|currentColor|inherit)$/i', $value) === 1) {
382+
return true;
383+
}
384+
385+
if (preg_match('/^#(?:[0-9a-f]{3}|[0-9a-f]{4}|[0-9a-f]{6}|[0-9a-f]{8})$/i', $value) === 1) {
386+
return true;
387+
}
388+
389+
if (preg_match('/^(rgba?|hsla?)\(.+\)$/i', $value) === 1) {
390+
return true;
391+
}
392+
393+
return preg_match('/^var\(.+\)$/i', $value) === 1;
394+
}
395+
132396
/**
133397
* Validates the log level - debug requires devMode
134398
*/

‎src/templates/_layouts/settings.twig‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@
33
{% set title = title ?? 'Settings'|t('slideshow-manager') %}
44
{% set selectedSettingsItem = selectedSettingsItem ?? 'general' %}
55
{% set selectedSubnavItem = 'settings' %}
6+
{% if settings is defined and settings.hasErrors() %}
7+
{% set errorSummary = include('lindemannrock-base/_partials/error-summary', {
8+
errors: settings.getErrors(),
9+
translationCategory: 'slideshow-manager',
10+
}) %}
11+
{% endif %}
612

713
{% block sidebar %}
814
<nav id="settings-nav">

0 commit comments

Comments
 (0)