From fac4df51bdf62a2d8a0de6b015a5827f5e511ad1 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 16 Jul 2026 14:17:44 +0200 Subject: [PATCH 1/7] feat(#255): configurable registration fields + admin-defined custom fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Small communities can't force members to disclose personal data, so the registration form's hardcoded requirements become admin choices: - Per-field toggles (Settings → Email → Registrazione utenti) decide whether surname, phone and address are required at self-registration. Defaults keep the historical all-required behaviour; the toggles flow through ConfigStore (registration.require_cognome/telefono/indirizzo) into the public form (labels + required attributes), the registration controller, the profile and the admin user forms. An optional surname is stored as an empty string on purpose — the column stays NOT NULL because 70+ display paths CONCAT nome+cognome and a NULL there would blank the whole name; empty phone and address are stored as NULL (the columns were already nullable). - Custom fields: the admin defines label/type(text, textarea, email, url, number, checkbox)/required/active rows that render on the registration form and in the user profile, with server-side type validation and a 1000-char cap. Values are shown read-only on the admin user detail. Definitions live in registrazione_campi, per-user values in utenti_campi_valori (PK utente_id+campo_id, FK CASCADE both ways); account row and custom values insert atomically at registration. - Migration migrate_0.7.37.sql (CREATE TABLE IF NOT EXISTS only, idempotent) + the same tables in schema.sql for fresh installs; every reader degrades gracefully while the tables are not migrated yet. version.json → 0.7.37, What's New section added. - i18n: nine new strings in all four locales (key parity verified). - Tests: migration-0.7.37.unit.php (real migration against sandbox tables, idempotency, FK cascades, validation contract — 14 checks) and issue-255-registration.spec.js (real-browser flow: defaults required → admin relaxes + defines a Telegram field → registration without surname/phone/address stores the custom value → restore), wired into ci-e2e as a required step. Closes #255 (custom-field API exposure for notification integrations lands with the notifications work). --- .github/workflows/ci-e2e.yml | 6 + README.md | 15 ++ app/Controllers/ProfileController.php | 26 +- app/Controllers/RegistrationController.php | 69 ++++- app/Controllers/SettingsController.php | 103 ++++++++ app/Controllers/UsersController.php | 16 +- app/Routes/web.php | 5 +- app/Support/ConfigStore.php | 19 +- app/Support/RegistrationFields.php | 247 ++++++++++++++++++ app/Views/auth/register.php | 40 ++- app/Views/profile/index.php | 30 ++- app/Views/settings/index.php | 66 +++++ app/Views/utenti/modifica_utente.php | 14 + .../database/migrations/migrate_0.7.37.sql | 35 +++ installer/database/schema.sql | 25 ++ locale/de_DE.json | 11 +- locale/en_US.json | 11 +- locale/fr_FR.json | 11 +- locale/it_IT.json | 11 +- tests/issue-255-registration.spec.js | 139 ++++++++++ tests/migration-0.7.37.unit.php | 177 +++++++++++++ version.json | 2 +- 22 files changed, 1042 insertions(+), 36 deletions(-) create mode 100644 app/Support/RegistrationFields.php create mode 100644 installer/database/migrations/migrate_0.7.37.sql create mode 100644 tests/issue-255-registration.spec.js create mode 100644 tests/migration-0.7.37.unit.php diff --git a/.github/workflows/ci-e2e.yml b/.github/workflows/ci-e2e.yml index 7460a38a3..cdcd67d4e 100644 --- a/.github/workflows/ci-e2e.yml +++ b/.github/workflows/ci-e2e.yml @@ -251,6 +251,12 @@ jobs: --config=tests/playwright.config.js \ --workers=1 + - name: "[required] issue-255-registration.spec.js — configurable registration fields" + run: | + npx playwright test tests/issue-255-registration.spec.js \ + --config=tests/playwright.config.js \ + --workers=1 + # ── Orphan-spec backfill: tests that were authored against a fresh # install but never wired into the CI workflow. Adding them as # `continue-on-error: true` so we get the signal from a clean diff --git a/README.md b/README.md index 121abe001..29d1220bd 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,21 @@ Pinakes is a self-hosted, full-featured ILS for schools, municipalities, and pri --- +## What's New in v0.7.37 + +- **Configurable registration fields ([#255](https://github.com/fabiodalez-dev/Pinakes/issues/255)).** + Small communities can no longer be forced to collect personal data they don't + need: surname, phone and address each get an admin toggle (Settings → + Registration) deciding whether they are required at self-registration. + Defaults keep today's behaviour, so nothing changes until you opt out. +- **Custom registration fields (#255).** Administrators can define their own + fields (text, textarea, email, URL, number, checkbox — required or optional): + they appear on the registration form and in the user profile, and are shown + on the admin user detail. Ideal for community handles such as a Telegram + username. New migration `migrate_0.7.37.sql` adds the two supporting tables + (`registrazione_campi`, `utenti_campi_valori`); it is idempotent and runs + automatically on upgrade. + ## What's New in v0.7.35 - **Docker-aware in-app updater.** On the official Docker image the app files are diff --git a/app/Controllers/ProfileController.php b/app/Controllers/ProfileController.php index cb68dbc15..7e4325c63 100644 --- a/app/Controllers/ProfileController.php +++ b/app/Controllers/ProfileController.php @@ -30,6 +30,9 @@ public function show(Request $request, Response $response, mysqli $db, mixed $co $stmt->execute(); $user = $stmt->get_result()->fetch_assoc() ?: []; $stmt->close(); + // Custom registration fields (issue #255): definitions + this user's values. + $customFields = \App\Support\RegistrationFields::definitions($db); + $customFieldValues = \App\Support\RegistrationFields::valuesForUser($db, $uid); ob_start(); require __DIR__ . '/../Views/profile/index.php'; $content = ob_get_clean(); @@ -159,8 +162,18 @@ public function update(Request $request, Response $response, mysqli $db): Respon $locale = ($locale !== '' && isset($availableLocales[$locale])) ? $locale : null; } - // Validate required fields - if (empty($nome) || empty($cognome)) { + // Validate required fields. The surname follows the admin toggle + // (issue #255); when optional it is stored as '' — the column is NOT + // NULL by design so display paths that CONCAT nome+cognome stay whole. + if (empty($nome) || ($cognome === '' && \App\Support\RegistrationFields::isRequired('cognome'))) { + $profileUrl = RouteTranslator::route('profile'); + return $response->withHeader('Location', $profileUrl . '?error=required_fields')->withStatus(302); + } + + // Custom registration fields (issue #255): validate before writing. + $customDefinitions = \App\Support\RegistrationFields::definitions($db); + $customValidation = \App\Support\RegistrationFields::validate($customDefinitions, $data); + if ($customValidation['error'] !== null) { $profileUrl = RouteTranslator::route('profile'); return $response->withHeader('Location', $profileUrl . '?error=required_fields')->withStatus(302); } @@ -186,8 +199,15 @@ public function update(Request $request, Response $response, mysqli $db): Respon } if ($stmt->execute()) { + // Persist custom field values (issue #255) — best-effort: a failure + // here must not undo the successful profile update. + try { + \App\Support\RegistrationFields::saveValues($db, $uid, $customValidation['values']); + } catch (\Throwable $e) { + SecureLogger::error('ProfileController: custom field save failed', ['user_id' => $uid, 'error' => $e->getMessage()]); + } // Update session data - $_SESSION['user']['name'] = $nome . ' ' . $cognome; + $_SESSION['user']['name'] = trim($nome . ' ' . $cognome); // Apply locale change immediately (only when locale was in the form) if ($localeProvided) { if ($locale !== null) { diff --git a/app/Controllers/RegistrationController.php b/app/Controllers/RegistrationController.php index 1e5669fee..559369f31 100644 --- a/app/Controllers/RegistrationController.php +++ b/app/Controllers/RegistrationController.php @@ -15,9 +15,17 @@ class RegistrationController { - public function form(Request $request, Response $response): Response + public function form(Request $request, Response $response, mysqli $db): Response { Csrf::ensureToken(); + // Admin-configurable field requirements + custom fields (issue #255), + // consumed by the view for labels/required attributes and rendering. + $registrationRequired = [ + 'cognome' => \App\Support\RegistrationFields::isRequired('cognome'), + 'telefono' => \App\Support\RegistrationFields::isRequired('telefono'), + 'indirizzo' => \App\Support\RegistrationFields::isRequired('indirizzo'), + ]; + $customFields = \App\Support\RegistrationFields::definitions($db); ob_start(); require __DIR__ . '/../Views/auth/register.php'; $html = ob_get_clean(); @@ -64,8 +72,23 @@ public function register(Request $request, Response $response, mysqli $db): Resp return $response->withHeader('Location', RouteTranslator::route('register') . '?error=privacy_required')->withStatus(302); } - // Validate required fields - if ($nome === '' || $cognome === '' || $email === '' || $telefono === '' || $indirizzo === '' || $password === '' || $password !== $password2) { + // Validate required fields. Surname/phone/address are admin-configurable + // (issue #255, Settings → Registration); defaults keep them required so + // existing installs behave exactly as before until the admin opts out. + if ($nome === '' || $email === '' || $password === '' || $password !== $password2) { + return $response->withHeader('Location', RouteTranslator::route('register') . '?error=missing_fields')->withStatus(302); + } + if (($cognome === '' && \App\Support\RegistrationFields::isRequired('cognome')) + || ($telefono === '' && \App\Support\RegistrationFields::isRequired('telefono')) + || ($indirizzo === '' && \App\Support\RegistrationFields::isRequired('indirizzo')) + ) { + return $response->withHeader('Location', RouteTranslator::route('register') . '?error=missing_fields')->withStatus(302); + } + + // Admin-defined custom fields (issue #255): validate before any write. + $customDefinitions = \App\Support\RegistrationFields::definitions($db); + $customValidation = \App\Support\RegistrationFields::validate($customDefinitions, $data); + if ($customValidation['error'] !== null) { return $response->withHeader('Location', RouteTranslator::route('register') . '?error=missing_fields')->withStatus(302); } @@ -131,17 +154,18 @@ public function register(Request $request, Response $response, mysqli $db): Resp $data_accettazione_privacy = gmdate('Y-m-d H:i:s'); $privacy_policy_version = '1.0'; - // Build dynamic INSERT to handle NULL values properly for ENUM fields - $columns = 'nome, cognome, email, password, telefono, indirizzo, codice_tessera, stato, tipo_utente, email_verificata, token_verifica_email, data_token_verifica, data_scadenza_tessera, privacy_accettata, data_accettazione_privacy, privacy_policy_version'; - $placeholders = '?, ?, ?, ?, ?, ?, ?, ?, ?, 0, ?, ?, ?, 1, ?, ?'; - $types = 'ssssssssssssss'; + // Build dynamic INSERT to handle NULL values properly for ENUM fields. + // cognome stays in the base list: the column is NOT NULL by design (70+ + // display paths CONCAT nome+cognome and a NULL would blank the whole + // name), so an optional surname is stored as an empty string. + $columns = 'nome, cognome, email, password, codice_tessera, stato, tipo_utente, email_verificata, token_verifica_email, data_token_verifica, data_scadenza_tessera, privacy_accettata, data_accettazione_privacy, privacy_policy_version'; + $placeholders = '?, ?, ?, ?, ?, ?, ?, 0, ?, ?, ?, 1, ?, ?'; + $types = 'ssssssssssss'; $values = [ $nome, $cognome, $email, $hash, - $telefono, - $indirizzo, $codice_tessera, $stato, $ruolo, @@ -152,6 +176,21 @@ public function register(Request $request, Response $response, mysqli $db): Resp $privacy_policy_version ]; + // Nullable contact fields: store NULL (not '') when the admin made them + // optional and the user left them blank. + if ($telefono !== '') { + $columns .= ', telefono'; + $placeholders .= ', ?'; + $types .= 's'; + $values[] = $telefono; + } + if ($indirizzo !== '') { + $columns .= ', indirizzo'; + $placeholders .= ', ?'; + $types .= 's'; + $values[] = $indirizzo; + } + // Add optional fields only if they have values (to avoid ENUM truncation errors) if ($dataNascita !== null) { $columns .= ', data_nascita'; @@ -182,13 +221,23 @@ public function register(Request $request, Response $response, mysqli $db): Resp // throw as well, and must route to ?error=db, not bubble up as a 500. $stmt = null; try { + // Account row + custom field values land atomically: a failed custom + // value write must not leave a half-registered account behind. + $db->begin_transaction(); $stmt = $db->prepare(" INSERT INTO utenti ({$columns}) VALUES ({$placeholders}) "); $stmt->bind_param($types, ...$values); $stmt->execute(); $userId = (int) $stmt->insert_id; + \App\Support\RegistrationFields::saveValues($db, $userId, $customValidation['values']); + $db->commit(); } catch (\Throwable $e) { + try { + $db->rollback(); + } catch (\Throwable) { + // best-effort — never mask the original error below + } // Public form: collapse ANY unique-key duplicate (email / cod_fiscale / // codice_tessera) into one GENERIC message so an anonymous visitor can't tell // which field is taken (member enumeration). Only a genuine non-duplicate DB @@ -222,7 +271,7 @@ public function register(Request $request, Response $response, mysqli $db): Resp // Notify admins of new registration (email) $notificationService->notifyNewUserRegistration($userId); // Create in-app notification for admins - $notificationService->notifyNewUserInApp($userId, $nome . ' ' . $cognome, $email); + $notificationService->notifyNewUserInApp($userId, trim($nome . ' ' . $cognome), $email); } catch (\Throwable $e) { SecureLogger::error('[Registration] post-registration side-effect failed (account ' . $userId . ' was created): ' . $e->getMessage()); } diff --git a/app/Controllers/SettingsController.php b/app/Controllers/SettingsController.php index 08ccfe71a..256fae5a5 100644 --- a/app/Controllers/SettingsController.php +++ b/app/Controllers/SettingsController.php @@ -38,6 +38,10 @@ public function index(Request $request, Response $response, mysqli $db): Respons $loansSettings = $this->resolveLoansSettings($repository); $contactMessages = $this->loadContactMessages($db); $cookieBannerTexts = $this->resolveCookieBannerTexts($repository); + // Custom registration fields (issue #255) — edited from the email tab's + // "Registrazione utenti" block; include inactive ones so the admin can + // re-enable them. + $registrationCustomFields = \App\Support\RegistrationFields::definitions($db, false); $queryParams = $request->getQueryParams(); $activeTab = $queryParams['tab'] ?? 'general'; @@ -193,6 +197,16 @@ public function updateEmailSettings(Request $request, Response $response, mysqli $repository->set('registration', 'require_admin_approval', $requireApproval); ConfigStore::set('registration.require_admin_approval', (bool) $requireApproval); + // Built-in registration field requirements (issue #255) + foreach (\App\Support\RegistrationFields::BUILTIN_TOGGLES as $toggleKey) { + $flag = isset($data[$toggleKey]) ? '1' : '0'; + $repository->set('registration', $toggleKey, $flag); + ConfigStore::set('registration.' . $toggleKey, (bool) $flag); + } + + // Custom registration field definitions (issue #255) + $this->saveRegistrationCustomFields($db, $data); + ConfigStore::set('mail.driver', $driver); ConfigStore::set('mail.from_email', $fromEmail); ConfigStore::set('mail.from_name', $fromName); @@ -208,6 +222,90 @@ public function updateEmailSettings(Request $request, Response $response, mysqli return $this->redirect($response, '/admin/settings?tab=email'); } + /** + * Persist the admin-defined custom registration fields (issue #255). + * Existing rows arrive as custom_fields[][etichetta|tipo|obbligatorio|attivo|delete]; + * one optional new row arrives as new_custom_field_*. Deleting a definition + * cascades its per-user values (FK) — an explicit admin action. + * No-ops gracefully when the tables are not migrated yet. + * + * @param array $data + */ + private function saveRegistrationCustomFields(mysqli $db, array $data): void + { + try { + $probe = $db->prepare( + 'SELECT 1 FROM information_schema.tables + WHERE table_schema = DATABASE() AND table_name = ? LIMIT 1' + ); + if ($probe === false) { + return; + } + $tableName = 'registrazione_campi'; + $probe->bind_param('s', $tableName); + $probe->execute(); + $exists = $probe->get_result()->fetch_row() !== null; + $probe->close(); + if (!$exists) { + return; + } + + $rows = $data['custom_fields'] ?? []; + if (is_array($rows)) { + $update = $db->prepare( + 'UPDATE registrazione_campi SET etichetta = ?, tipo = ?, obbligatorio = ?, attivo = ? WHERE id = ?' + ); + $delete = $db->prepare('DELETE FROM registrazione_campi WHERE id = ?'); + if ($update === false || $delete === false) { + return; + } + foreach ($rows as $id => $row) { + $id = (int) $id; + if ($id <= 0 || !is_array($row)) { + continue; + } + if (!empty($row['delete'])) { + $delete->bind_param('i', $id); + $delete->execute(); + continue; + } + $label = trim(strip_tags((string) ($row['etichetta'] ?? ''))); + $tipo = (string) ($row['tipo'] ?? 'text'); + if ($label === '' || mb_strlen($label) > 100 + || !in_array($tipo, \App\Support\RegistrationFields::TYPES, true) + ) { + continue; + } + $required = !empty($row['obbligatorio']) ? 1 : 0; + $active = !empty($row['attivo']) ? 1 : 0; + $update->bind_param('ssiii', $label, $tipo, $required, $active, $id); + $update->execute(); + } + $update->close(); + $delete->close(); + } + + $newLabel = trim(strip_tags((string) ($data['new_custom_field_label'] ?? ''))); + $newType = (string) ($data['new_custom_field_type'] ?? 'text'); + if ($newLabel !== '' && mb_strlen($newLabel) <= 100 + && in_array($newType, \App\Support\RegistrationFields::TYPES, true) + ) { + $newRequired = !empty($data['new_custom_field_required']) ? 1 : 0; + $insert = $db->prepare( + 'INSERT INTO registrazione_campi (etichetta, tipo, obbligatorio, attivo, ordine) + SELECT ?, ?, ?, 1, COALESCE(MAX(ordine), 0) + 1 FROM registrazione_campi' + ); + if ($insert !== false) { + $insert->bind_param('ssi', $newLabel, $newType, $newRequired); + $insert->execute(); + $insert->close(); + } + } + } catch (\Throwable $e) { + \App\Support\SecureLogger::error('[Settings] custom registration fields save failed', ['error' => $e->getMessage()]); + } + } + public function updateEmailTemplate(Request $request, Response $response, mysqli $db, string $template): Response { $definition = SettingsMailTemplates::get($template); @@ -275,6 +373,11 @@ private function resolveEmailSettings(SettingsRepository $repository): array // Registration approval flag lives in its own config group but is edited // from this (email) form, so surface it here for the checkbox to pre-fill. $settings['require_admin_approval'] = (bool) ConfigStore::get('registration.require_admin_approval', true); + // Built-in field requirements (issue #255) — defaults keep the historical + // all-required behaviour. + foreach (\App\Support\RegistrationFields::BUILTIN_TOGGLES as $toggleKey) { + $settings[$toggleKey] = (bool) ConfigStore::get('registration.' . $toggleKey, true); + } $driver = $stored['driver_mode'] ?? $settings['type'] ?? $defaults['type']; $settings['type'] = $driver; diff --git a/app/Controllers/UsersController.php b/app/Controllers/UsersController.php index 28e089f31..3cd21db66 100644 --- a/app/Controllers/UsersController.php +++ b/app/Controllers/UsersController.php @@ -88,7 +88,9 @@ public function store(Request $request, Response $response, mysqli $db): Respons $role = $requestedRole; $isAdmin = $role === 'admin'; - if ($nome === '' || $cognome === '' || $email === '') { + if ($nome === '' || $email === '' + || ($cognome === '' && \App\Support\RegistrationFields::isRequired('cognome')) + ) { return $response->withHeader('Location', url('/admin/users/create?error=missing_fields'))->withStatus(302); } @@ -122,7 +124,7 @@ public function store(Request $request, Response $response, mysqli $db): Respons return $response->withHeader('Location', url('/admin/users/create?error=phone_too_long'))->withStatus(302); } - if (!$isAdmin && $telefono === '') { + if (!$isAdmin && $telefono === '' && \App\Support\RegistrationFields::isRequired('telefono')) { return $response->withHeader('Location', url('/admin/users/create?error=missing_fields'))->withStatus(302); } @@ -270,6 +272,10 @@ public function editForm(Request $request, Response $response, mysqli $db, int $ $utente = $result->fetch_assoc(); $stmt->close(); + // Custom registration field values (issue #255) — shown read-only so the + // admin can see community handles (e.g. Telegram) alongside the account. + $customFieldValues = \App\Support\RegistrationFields::labelledValuesForUser($db, $id); + ob_start(); require __DIR__ . '/../Views/utenti/modifica_utente.php'; $content = ob_get_clean(); @@ -323,11 +329,13 @@ public function update(Request $request, Response $response, mysqli $db, int $id $role = $requestedRole; $isAdmin = $role === 'admin'; - if ($nome === '' || $cognome === '' || $email === '') { + if ($nome === '' || $email === '' + || ($cognome === '' && \App\Support\RegistrationFields::isRequired('cognome')) + ) { return $response->withHeader('Location', url('/admin/users/edit/' . $id . '?error=missing_fields'))->withStatus(302); } - if (!$isAdmin && $telefono === '') { + if (!$isAdmin && $telefono === '' && \App\Support\RegistrationFields::isRequired('telefono')) { return $response->withHeader('Location', url('/admin/users/edit/' . $id . '?error=missing_fields'))->withStatus(302); } diff --git a/app/Routes/web.php b/app/Routes/web.php index b301e33c9..f97f92989 100644 --- a/app/Routes/web.php +++ b/app/Routes/web.php @@ -372,12 +372,13 @@ } // Public registration routes (all language variants) - $registerGetHandler = function ($request, $response) { + $registerGetHandler = function ($request, $response) use ($app) { if (!empty($_SESSION['user']['id'])) { return $response->withHeader('Location', RouteTranslator::route('user_dashboard'))->withStatus(302); } + $db = $app->getContainer()->get('db'); $controller = new RegistrationController(); - return $controller->form($request, $response); + return $controller->form($request, $response, $db); }; $registerPostHandler = function ($request, $response) use ($app) { if (!empty($_SESSION['user']['id'])) { diff --git a/app/Support/ConfigStore.php b/app/Support/ConfigStore.php index d8b856d9f..4b10b84fa 100644 --- a/app/Support/ConfigStore.php +++ b/app/Support/ConfigStore.php @@ -40,6 +40,11 @@ public static function all(): array ], 'registration' => [ 'require_admin_approval' => true, + // Per-field requirement toggles (issue #255): defaults preserve + // the historical all-required registration form. + 'require_cognome' => true, + 'require_telefono' => true, + 'require_indirizzo' => true, ], 'contacts' => [ 'page_title' => 'Contattaci', @@ -468,12 +473,18 @@ private static function loadDatabaseSettings(): array if (!empty($raw['registration'])) { self::$dbSettingsCache['registration'] = []; - if (isset($raw['registration']['require_admin_approval'])) { - $value = filter_var($raw['registration']['require_admin_approval'], FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); + // Boolean registration flags: approval + the per-field + // requirement toggles (issue #255). + $registrationFlags = ['require_admin_approval', 'require_cognome', 'require_telefono', 'require_indirizzo']; + foreach ($registrationFlags as $flagKey) { + if (!isset($raw['registration'][$flagKey])) { + continue; + } + $value = filter_var($raw['registration'][$flagKey], FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); if ($value === null) { - $value = in_array((string) $raw['registration']['require_admin_approval'], ['1', 'true', 'yes'], true); + $value = in_array((string) $raw['registration'][$flagKey], ['1', 'true', 'yes'], true); } - self::$dbSettingsCache['registration']['require_admin_approval'] = $value; + self::$dbSettingsCache['registration'][$flagKey] = $value; } } diff --git a/app/Support/RegistrationFields.php b/app/Support/RegistrationFields.php new file mode 100644 index 000000000..49ff44366 --- /dev/null +++ b/app/Support/RegistrationFields.php @@ -0,0 +1,247 @@ + settings key. */ + public const BUILTIN_TOGGLES = [ + 'cognome' => 'require_cognome', + 'telefono' => 'require_telefono', + 'indirizzo' => 'require_indirizzo', + ]; + + public const TYPES = ['text', 'textarea', 'email', 'url', 'number', 'checkbox']; + + private const MAX_VALUE_LENGTH = 1000; + + /** Whether a built-in field is required at registration (default: yes). */ + public static function isRequired(string $field): bool + { + $key = self::BUILTIN_TOGGLES[$field] ?? null; + if ($key === null) { + return true; + } + return (bool) ConfigStore::get('registration.' . $key, true); + } + + /** + * Custom field definitions, ordered. Empty when the table is absent. + * + * @return list + */ + public static function definitions(mysqli $db, bool $onlyActive = true): array + { + if (!self::tableExists($db, 'registrazione_campi')) { + return []; + } + $where = $onlyActive ? 'WHERE attivo = 1' : ''; + $res = $db->query( + "SELECT id, etichetta, tipo, obbligatorio, attivo, ordine + FROM registrazione_campi {$where} + ORDER BY ordine, id" + ); + $rows = []; + if ($res instanceof \mysqli_result) { + while ($row = $res->fetch_assoc()) { + $rows[] = [ + 'id' => (int) $row['id'], + 'etichetta' => (string) $row['etichetta'], + 'tipo' => (string) $row['tipo'], + 'obbligatorio' => (bool) $row['obbligatorio'], + 'attivo' => (bool) $row['attivo'], + 'ordine' => (int) $row['ordine'], + ]; + } + } + return $rows; + } + + /** + * Validate the submitted values for the given definitions. + * + * @param list $definitions + * @param array $post raw request body (custom_field[] keys) + * @return array{values: array, error: ?string} + * values maps field id => normalised value ('' allowed only for + * optional fields; checkboxes normalise to '1'/'') + */ + public static function validate(array $definitions, array $post): array + { + $submitted = $post['custom_field'] ?? []; + if (!is_array($submitted)) { + $submitted = []; + } + + $values = []; + foreach ($definitions as $def) { + $raw = trim((string) ($submitted[$def['id']] ?? '')); + + if ($def['tipo'] === 'checkbox') { + $values[$def['id']] = $raw !== '' ? '1' : ''; + if ($def['obbligatorio'] && $values[$def['id']] === '') { + return ['values' => [], 'error' => $def['etichetta']]; + } + continue; + } + + if ($raw === '') { + if ($def['obbligatorio']) { + return ['values' => [], 'error' => $def['etichetta']]; + } + $values[$def['id']] = ''; + continue; + } + + if (mb_strlen($raw) > self::MAX_VALUE_LENGTH) { + return ['values' => [], 'error' => $def['etichetta']]; + } + + $ok = match ($def['tipo']) { + 'email' => filter_var($raw, FILTER_VALIDATE_EMAIL) !== false, + 'url' => filter_var($raw, FILTER_VALIDATE_URL) !== false + && preg_match('#^https?://#i', $raw) === 1, + 'number' => is_numeric($raw), + default => true, // text / textarea: length-capped free text + }; + if (!$ok) { + return ['values' => [], 'error' => $def['etichetta']]; + } + $values[$def['id']] = $raw; + } + + return ['values' => $values, 'error' => null]; + } + + /** + * Persist the validated values for a user. Empty values delete the row so + * clearing a field in the profile really clears it. + * + * @param array $values field id => value + */ + public static function saveValues(mysqli $db, int $userId, array $values): void + { + if ($userId <= 0 || $values === [] || !self::tableExists($db, 'utenti_campi_valori')) { + return; + } + $upsert = $db->prepare( + 'INSERT INTO utenti_campi_valori (utente_id, campo_id, valore) + VALUES (?, ?, ?) + ON DUPLICATE KEY UPDATE valore = VALUES(valore)' + ); + $delete = $db->prepare( + 'DELETE FROM utenti_campi_valori WHERE utente_id = ? AND campo_id = ?' + ); + if ($upsert === false || $delete === false) { + throw new \RuntimeException('Unable to prepare custom field persistence'); + } + foreach ($values as $fieldId => $value) { + $fieldId = (int) $fieldId; + if ($value === '') { + $delete->bind_param('ii', $userId, $fieldId); + if (!$delete->execute()) { + throw new \RuntimeException('Unable to clear custom field value: ' . $delete->error); + } + continue; + } + $upsert->bind_param('iis', $userId, $fieldId, $value); + if (!$upsert->execute()) { + throw new \RuntimeException('Unable to save custom field value: ' . $upsert->error); + } + } + $upsert->close(); + $delete->close(); + } + + /** + * Values for one user keyed by field id. Empty when tables are missing. + * + * @return array + */ + public static function valuesForUser(mysqli $db, int $userId): array + { + if ($userId <= 0 || !self::tableExists($db, 'utenti_campi_valori')) { + return []; + } + $stmt = $db->prepare('SELECT campo_id, valore FROM utenti_campi_valori WHERE utente_id = ?'); + if ($stmt === false) { + return []; + } + $stmt->bind_param('i', $userId); + $stmt->execute(); + $res = $stmt->get_result(); + $values = []; + while ($row = $res->fetch_assoc()) { + $values[(int) $row['campo_id']] = (string) $row['valore']; + } + $stmt->close(); + return $values; + } + + /** + * Labelled values for one user (for display surfaces and API payloads). + * + * @return list + */ + public static function labelledValuesForUser(mysqli $db, int $userId): array + { + $values = self::valuesForUser($db, $userId); + if ($values === []) { + return []; + } + $out = []; + foreach (self::definitions($db, false) as $def) { + if (isset($values[$def['id']]) && $values[$def['id']] !== '') { + $out[] = [ + 'id' => $def['id'], + 'etichetta' => $def['etichetta'], + 'tipo' => $def['tipo'], + 'valore' => $values[$def['id']], + ]; + } + } + return $out; + } + + private static function tableExists(mysqli $db, string $table): bool + { + try { + $stmt = $db->prepare( + 'SELECT 1 FROM information_schema.tables + WHERE table_schema = DATABASE() AND table_name = ? LIMIT 1' + ); + if ($stmt === false) { + return false; + } + $stmt->bind_param('s', $table); + $stmt->execute(); + $exists = $stmt->get_result()->fetch_row() !== null; + $stmt->close(); + return $exists; + } catch (\Throwable) { + return false; + } + } +} diff --git a/app/Views/auth/register.php b/app/Views/auth/register.php index a3b9c5551..c6e9ec97f 100644 --- a/app/Views/auth/register.php +++ b/app/Views/auth/register.php @@ -122,13 +122,13 @@ class="w-full px-4 py-3 rounded-xl border border-gray-300 bg-white text-gray-900
aria-describedby="cognome-error" class="w-full px-4 py-3 rounded-xl border border-gray-300 bg-white text-gray-900 focus:ring-2 focus:ring-blue-500 focus:border-blue-500 transition-all duration-200" placeholder="" @@ -157,13 +157,13 @@ class="w-full px-4 py-3 rounded-xl border border-gray-300 bg-white text-gray-900
aria-describedby="telefono-error" class="w-full px-4 py-3 rounded-xl border border-gray-300 bg-white text-gray-900 focus:ring-2 focus:ring-blue-500 focus:border-blue-500 transition-all duration-200" placeholder="" @@ -174,12 +174,12 @@ class="w-full px-4 py-3 rounded-xl border border-gray-300 bg-white text-gray-900
+ + + + class="w-full px-4 py-3 rounded-xl border border-gray-300 bg-white text-gray-900 focus:ring-2 focus:ring-blue-500 focus:border-blue-500 transition-all duration-200" /> + + +
+ +
- - + + aria-describedby="cognome-error" value="">
@@ -446,6 +447,31 @@ value="">
+ + +
+ + + + + + + + + + value=""> + + +
+ + 1): diff --git a/app/Views/settings/index.php b/app/Views/settings/index.php index f9e7b1702..80f4f910e 100644 --- a/app/Views/settings/index.php +++ b/app/Views/settings/index.php @@ -318,6 +318,72 @@ class="block w-full rounded-lg border-gray-300 focus:border-gray-500 focus:ring- + +
+

+

+
+ + + +
+
+ +
+

+

+ __('Testo'), 'textarea' => __('Testo lungo'), 'email' => __('Email'), 'url' => __('URL'), 'number' => __('Numero'), 'checkbox' => __('Casella di spunta')]; ?> + +
+ +
+ + + + + +
+ +
+ +
+ " + class="flex-1 min-w-[10rem] px-3 py-2 rounded-lg border border-gray-300 bg-white text-sm text-gray-900"> + + +
+

+
diff --git a/app/Views/utenti/modifica_utente.php b/app/Views/utenti/modifica_utente.php index 69c12351f..451c84bc1 100644 --- a/app/Views/utenti/modifica_utente.php +++ b/app/Views/utenti/modifica_utente.php @@ -107,6 +107,20 @@ " style="text-transform: uppercase;">

+ +
+

+
+ +
+
+
+
+ +
+

+
+
diff --git a/installer/database/migrations/migrate_0.7.37.sql b/installer/database/migrations/migrate_0.7.37.sql new file mode 100644 index 000000000..3af75acf6 --- /dev/null +++ b/installer/database/migrations/migrate_0.7.37.sql @@ -0,0 +1,35 @@ +-- Migration 0.7.37 — configurable registration fields (issue #255) +-- +-- Adds the admin-defined custom registration fields: definitions in +-- `registrazione_campi`, per-user values in `utenti_campi_valori`. +-- The built-in field toggles (require_cognome / require_telefono / +-- require_indirizzo) need no schema: they live in system_settings with +-- code-side defaults that preserve the historical behaviour. +-- +-- The surname stays a NOT NULL column on purpose: an optional surname is +-- stored as an empty string, because 70+ display paths concatenate +-- nome+cognome with CONCAT() and a NULL there would blank the whole name. +-- +-- Idempotent (project rule 6): CREATE TABLE IF NOT EXISTS only; safe to +-- re-run on any install. + +CREATE TABLE IF NOT EXISTS `registrazione_campi` ( + `id` int NOT NULL AUTO_INCREMENT, + `etichetta` varchar(100) COLLATE utf8mb4_unicode_ci NOT NULL, + `tipo` enum('text','textarea','email','url','number','checkbox') COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'text', + `obbligatorio` tinyint(1) NOT NULL DEFAULT '0', + `attivo` tinyint(1) NOT NULL DEFAULT '1', + `ordine` int NOT NULL DEFAULT '0', + `created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; + +CREATE TABLE IF NOT EXISTS `utenti_campi_valori` ( + `utente_id` int NOT NULL, + `campo_id` int NOT NULL, + `valore` text COLLATE utf8mb4_unicode_ci, + PRIMARY KEY (`utente_id`, `campo_id`), + KEY `idx_ucv_campo` (`campo_id`), + CONSTRAINT `utenti_campi_valori_utente_fk` FOREIGN KEY (`utente_id`) REFERENCES `utenti` (`id`) ON DELETE CASCADE, + CONSTRAINT `utenti_campi_valori_campo_fk` FOREIGN KEY (`campo_id`) REFERENCES `registrazione_campi` (`id`) ON DELETE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; diff --git a/installer/database/schema.sql b/installer/database/schema.sql index c2b07b3f5..a72d0007d 100755 --- a/installer/database/schema.sql +++ b/installer/database/schema.sql @@ -1010,6 +1010,31 @@ CREATE TABLE `utenti` ( /*!40101 SET character_set_client = @saved_cs_client */; /*!40101 SET @saved_cs_client = @@character_set_client */; /*!50503 SET character_set_client = utf8mb4 */; +CREATE TABLE `registrazione_campi` ( + `id` int NOT NULL AUTO_INCREMENT, + `etichetta` varchar(100) COLLATE utf8mb4_unicode_ci NOT NULL, + `tipo` enum('text','textarea','email','url','number','checkbox') COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT 'text', + `obbligatorio` tinyint(1) NOT NULL DEFAULT '0', + `attivo` tinyint(1) NOT NULL DEFAULT '1', + `ordine` int NOT NULL DEFAULT '0', + `created_at` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (`id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; +/*!40101 SET character_set_client = @saved_cs_client */; +/*!40101 SET @saved_cs_client = @@character_set_client */; +/*!50503 SET character_set_client = utf8mb4 */; +CREATE TABLE `utenti_campi_valori` ( + `utente_id` int NOT NULL, + `campo_id` int NOT NULL, + `valore` text COLLATE utf8mb4_unicode_ci, + PRIMARY KEY (`utente_id`, `campo_id`), + KEY `idx_ucv_campo` (`campo_id`), + CONSTRAINT `utenti_campi_valori_utente_fk` FOREIGN KEY (`utente_id`) REFERENCES `utenti` (`id`) ON DELETE CASCADE, + CONSTRAINT `utenti_campi_valori_campo_fk` FOREIGN KEY (`campo_id`) REFERENCES `registrazione_campi` (`id`) ON DELETE CASCADE +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; +/*!40101 SET character_set_client = @saved_cs_client */; +/*!40101 SET @saved_cs_client = @@character_set_client */; +/*!50503 SET character_set_client = utf8mb4 */; CREATE TABLE `wishlist` ( `id` int NOT NULL AUTO_INCREMENT, `utente_id` int NOT NULL, diff --git a/locale/de_DE.json b/locale/de_DE.json index b81efb85c..b6d55ec6e 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -6525,5 +6525,14 @@ "Spaziatura": "Abstand", "Margine interno (mm)": "Innenabstand (mm)", "Il contenuto si adatta automaticamente alle dimensioni dell'etichetta. Aggiungi un margine interno extra per rifinire il layout.": "Der Inhalt passt sich automatisch an die Etikettengröße an. Fügen Sie zusätzlichen Innenabstand hinzu, um das Layout zu optimieren.", - "Sembra che tu stia eseguendo l'immagine Docker: i file dell'applicazione appartengono all'immagine e non sono modificabili da questo pulsante. Aggiorna spostando il container alla nuova immagine con «docker compose pull && docker compose up -d» (i tuoi dati nel database e nei volumi storage/uploads restano al sicuro). Il pulsante di aggiornamento è pensato per installazioni classiche/hosting condiviso dove l'utente del web server è proprietario dei file.": "Es sieht so aus, als würden Sie das Docker-Image verwenden: Die Anwendungsdateien gehören zum Image und können über diese Schaltfläche nicht geändert werden. Aktualisieren Sie, indem Sie den Container auf das neue Image umstellen: „docker compose pull && docker compose up -d“ (Ihre Daten in der Datenbank und in den storage/uploads-Volumes bleiben sicher). Die Update-Schaltfläche ist für klassische Installationen/Shared Hosting gedacht, bei denen der Webserver-Benutzer Eigentümer der Dateien ist." + "Sembra che tu stia eseguendo l'immagine Docker: i file dell'applicazione appartengono all'immagine e non sono modificabili da questo pulsante. Aggiorna spostando il container alla nuova immagine con «docker compose pull && docker compose up -d» (i tuoi dati nel database e nei volumi storage/uploads restano al sicuro). Il pulsante di aggiornamento è pensato per installazioni classiche/hosting condiviso dove l'utente del web server è proprietario dei file.": "Es sieht so aus, als würden Sie das Docker-Image verwenden: Die Anwendungsdateien gehören zum Image und können über diese Schaltfläche nicht geändert werden. Aktualisieren Sie, indem Sie den Container auf das neue Image umstellen: „docker compose pull && docker compose up -d“ (Ihre Daten in der Datenbank und in den storage/uploads-Volumes bleiben sicher). Die Update-Schaltfläche ist für klassische Installationen/Shared Hosting gedacht, bei denen der Webserver-Benutzer Eigentümer der Dateien ist.", + "Numero": "Zahl", + "Casella di spunta": "Kontrollkästchen", + "Campi personalizzati": "Benutzerdefinierte Felder", + "Campi obbligatori alla registrazione": "Pflichtfelder bei der Registrierung", + "Disattiva i campi che la tua comunità non vuole raccogliere: diventeranno facoltativi nel modulo di registrazione.": "Deaktiviere die Felder, die deine Gemeinschaft nicht erheben möchte: sie werden im Registrierungsformular optional.", + "Campi aggiuntivi definiti da te (es. username Telegram): compaiono nel modulo di registrazione e nel profilo utente.": "Von dir definierte Zusatzfelder (z. B. Telegram-Benutzername): sie erscheinen im Registrierungsformular und im Benutzerprofil.", + "Etichetta nuovo campo": "Bezeichnung des neuen Felds", + "Le modifiche ai campi vengono salvate insieme alle impostazioni email.": "Feldänderungen werden zusammen mit den E-Mail-Einstellungen gespeichert.", + "L'utente li gestisce dal proprio profilo.": "Der Benutzer verwaltet sie über sein Profil." } diff --git a/locale/en_US.json b/locale/en_US.json index a5cbc8072..9e73ddead 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -6525,5 +6525,14 @@ "Spaziatura": "Spacing", "Margine interno (mm)": "Inner padding (mm)", "Il contenuto si adatta automaticamente alle dimensioni dell'etichetta. Aggiungi un margine interno extra per rifinire il layout.": "Content scales automatically to the label size. Add extra inner padding to fine-tune the layout.", - "Sembra che tu stia eseguendo l'immagine Docker: i file dell'applicazione appartengono all'immagine e non sono modificabili da questo pulsante. Aggiorna spostando il container alla nuova immagine con «docker compose pull && docker compose up -d» (i tuoi dati nel database e nei volumi storage/uploads restano al sicuro). Il pulsante di aggiornamento è pensato per installazioni classiche/hosting condiviso dove l'utente del web server è proprietario dei file.": "It looks like you're running the Docker image: the application files belong to the image and can't be modified from this button. Update by moving the container to the new image with “docker compose pull && docker compose up -d” (your data in the database and the storage/uploads volumes stays safe). The update button is meant for classic/shared-hosting installs where the web-server user owns the files." + "Sembra che tu stia eseguendo l'immagine Docker: i file dell'applicazione appartengono all'immagine e non sono modificabili da questo pulsante. Aggiorna spostando il container alla nuova immagine con «docker compose pull && docker compose up -d» (i tuoi dati nel database e nei volumi storage/uploads restano al sicuro). Il pulsante di aggiornamento è pensato per installazioni classiche/hosting condiviso dove l'utente del web server è proprietario dei file.": "It looks like you're running the Docker image: the application files belong to the image and can't be modified from this button. Update by moving the container to the new image with “docker compose pull && docker compose up -d” (your data in the database and the storage/uploads volumes stays safe). The update button is meant for classic/shared-hosting installs where the web-server user owns the files.", + "Numero": "Number", + "Casella di spunta": "Checkbox", + "Campi personalizzati": "Custom fields", + "Campi obbligatori alla registrazione": "Required fields at registration", + "Disattiva i campi che la tua comunità non vuole raccogliere: diventeranno facoltativi nel modulo di registrazione.": "Untick the fields your community doesn't want to collect: they become optional on the registration form.", + "Campi aggiuntivi definiti da te (es. username Telegram): compaiono nel modulo di registrazione e nel profilo utente.": "Additional fields you define (e.g. a Telegram username): they appear on the registration form and in the user profile.", + "Etichetta nuovo campo": "New field label", + "Le modifiche ai campi vengono salvate insieme alle impostazioni email.": "Field changes are saved together with the email settings.", + "L'utente li gestisce dal proprio profilo.": "The user manages them from their profile." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 028f4f2fa..3d23eb4c9 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -6525,5 +6525,14 @@ "Spaziatura": "Espacement", "Margine interno (mm)": "Marge intérieure (mm)", "Il contenuto si adatta automaticamente alle dimensioni dell'etichetta. Aggiungi un margine interno extra per rifinire il layout.": "Le contenu s'adapte automatiquement à la taille de l'étiquette. Ajoutez une marge intérieure supplémentaire pour affiner la mise en page.", - "Sembra che tu stia eseguendo l'immagine Docker: i file dell'applicazione appartengono all'immagine e non sono modificabili da questo pulsante. Aggiorna spostando il container alla nuova immagine con «docker compose pull && docker compose up -d» (i tuoi dati nel database e nei volumi storage/uploads restano al sicuro). Il pulsante di aggiornamento è pensato per installazioni classiche/hosting condiviso dove l'utente del web server è proprietario dei file.": "Il semble que vous utilisiez l'image Docker : les fichiers de l'application appartiennent à l'image et ne peuvent pas être modifiés depuis ce bouton. Mettez à jour en déplaçant le conteneur vers la nouvelle image avec « docker compose pull && docker compose up -d » (vos données dans la base et dans les volumes storage/uploads restent en sécurité). Le bouton de mise à jour est prévu pour les installations classiques/hébergement mutualisé où l'utilisateur du serveur web est propriétaire des fichiers." + "Sembra che tu stia eseguendo l'immagine Docker: i file dell'applicazione appartengono all'immagine e non sono modificabili da questo pulsante. Aggiorna spostando il container alla nuova immagine con «docker compose pull && docker compose up -d» (i tuoi dati nel database e nei volumi storage/uploads restano al sicuro). Il pulsante di aggiornamento è pensato per installazioni classiche/hosting condiviso dove l'utente del web server è proprietario dei file.": "Il semble que vous utilisiez l'image Docker : les fichiers de l'application appartiennent à l'image et ne peuvent pas être modifiés depuis ce bouton. Mettez à jour en déplaçant le conteneur vers la nouvelle image avec « docker compose pull && docker compose up -d » (vos données dans la base et dans les volumes storage/uploads restent en sécurité). Le bouton de mise à jour est prévu pour les installations classiques/hébergement mutualisé où l'utilisateur du serveur web est propriétaire des fichiers.", + "Numero": "Nombre", + "Casella di spunta": "Case à cocher", + "Campi personalizzati": "Champs personnalisés", + "Campi obbligatori alla registrazione": "Champs obligatoires à l'inscription", + "Disattiva i campi che la tua comunità non vuole raccogliere: diventeranno facoltativi nel modulo di registrazione.": "Décochez les champs que votre communauté ne souhaite pas collecter : ils deviendront facultatifs dans le formulaire d'inscription.", + "Campi aggiuntivi definiti da te (es. username Telegram): compaiono nel modulo di registrazione e nel profilo utente.": "Champs supplémentaires définis par vous (p. ex. nom d'utilisateur Telegram) : ils apparaissent dans le formulaire d'inscription et dans le profil utilisateur.", + "Etichetta nuovo campo": "Libellé du nouveau champ", + "Le modifiche ai campi vengono salvate insieme alle impostazioni email.": "Les modifications des champs sont enregistrées avec les paramètres e-mail.", + "L'utente li gestisce dal proprio profilo.": "L'utilisateur les gère depuis son profil." } diff --git a/locale/it_IT.json b/locale/it_IT.json index 0af146675..e8f173dac 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -6525,5 +6525,14 @@ "Spaziatura": "Spaziatura", "Margine interno (mm)": "Margine interno (mm)", "Il contenuto si adatta automaticamente alle dimensioni dell'etichetta. Aggiungi un margine interno extra per rifinire il layout.": "Il contenuto si adatta automaticamente alle dimensioni dell'etichetta. Aggiungi un margine interno extra per rifinire il layout.", - "Sembra che tu stia eseguendo l'immagine Docker: i file dell'applicazione appartengono all'immagine e non sono modificabili da questo pulsante. Aggiorna spostando il container alla nuova immagine con «docker compose pull && docker compose up -d» (i tuoi dati nel database e nei volumi storage/uploads restano al sicuro). Il pulsante di aggiornamento è pensato per installazioni classiche/hosting condiviso dove l'utente del web server è proprietario dei file.": "Sembra che tu stia eseguendo l'immagine Docker: i file dell'applicazione appartengono all'immagine e non sono modificabili da questo pulsante. Aggiorna spostando il container alla nuova immagine con «docker compose pull && docker compose up -d» (i tuoi dati nel database e nei volumi storage/uploads restano al sicuro). Il pulsante di aggiornamento è pensato per installazioni classiche/hosting condiviso dove l'utente del web server è proprietario dei file." + "Sembra che tu stia eseguendo l'immagine Docker: i file dell'applicazione appartengono all'immagine e non sono modificabili da questo pulsante. Aggiorna spostando il container alla nuova immagine con «docker compose pull && docker compose up -d» (i tuoi dati nel database e nei volumi storage/uploads restano al sicuro). Il pulsante di aggiornamento è pensato per installazioni classiche/hosting condiviso dove l'utente del web server è proprietario dei file.": "Sembra che tu stia eseguendo l'immagine Docker: i file dell'applicazione appartengono all'immagine e non sono modificabili da questo pulsante. Aggiorna spostando il container alla nuova immagine con «docker compose pull && docker compose up -d» (i tuoi dati nel database e nei volumi storage/uploads restano al sicuro). Il pulsante di aggiornamento è pensato per installazioni classiche/hosting condiviso dove l'utente del web server è proprietario dei file.", + "Numero": "Numero", + "Casella di spunta": "Casella di spunta", + "Campi personalizzati": "Campi personalizzati", + "Campi obbligatori alla registrazione": "Campi obbligatori alla registrazione", + "Disattiva i campi che la tua comunità non vuole raccogliere: diventeranno facoltativi nel modulo di registrazione.": "Disattiva i campi che la tua comunità non vuole raccogliere: diventeranno facoltativi nel modulo di registrazione.", + "Campi aggiuntivi definiti da te (es. username Telegram): compaiono nel modulo di registrazione e nel profilo utente.": "Campi aggiuntivi definiti da te (es. username Telegram): compaiono nel modulo di registrazione e nel profilo utente.", + "Etichetta nuovo campo": "Etichetta nuovo campo", + "Le modifiche ai campi vengono salvate insieme alle impostazioni email.": "Le modifiche ai campi vengono salvate insieme alle impostazioni email.", + "L'utente li gestisce dal proprio profilo.": "L'utente li gestisce dal proprio profilo." } diff --git a/tests/issue-255-registration.spec.js b/tests/issue-255-registration.spec.js new file mode 100644 index 000000000..28bfe6bb7 --- /dev/null +++ b/tests/issue-255-registration.spec.js @@ -0,0 +1,139 @@ +// @ts-check +/** + * Issue #255 — configurable registration fields. + * + * End-to-end through the real UI: + * 1. Defaults: surname/phone/address are required on the public form. + * 2. Admin unticks the three requirement toggles and defines a custom + * "ZZ Telegram 255" field from Settings → Email (Registrazione utenti). + * 3. The public form drops the required attributes, shows the custom field, + * and a registration WITHOUT surname/phone/address succeeds; the custom + * value is stored in utenti_campi_valori and surname lands as ''. + * 4. Cleanup restores the toggles, deletes the custom field (cascade wipes + * the value) and removes the test user. + */ + +const { test, expect } = require('@playwright/test'); +const { execFileSync } = require('child_process'); + +const BASE = process.env.E2E_BASE_URL || 'http://localhost:8081'; +const ADMIN_EMAIL = process.env.E2E_ADMIN_EMAIL || ''; +const ADMIN_PASS = process.env.E2E_ADMIN_PASS || ''; +const DB_USER = process.env.E2E_DB_USER || ''; +const DB_PASS = process.env.E2E_DB_PASS || ''; +const DB_SOCKET = process.env.E2E_DB_SOCKET || ''; +const DB_NAME = process.env.E2E_DB_NAME || ''; + +test.skip(!ADMIN_EMAIL || !ADMIN_PASS || !DB_USER || !DB_NAME, + 'E2E credentials not configured (set E2E_ADMIN_EMAIL, E2E_ADMIN_PASS, E2E_DB_*)'); + +function dbQuery(sql) { + const args = ['-u', DB_USER]; + if (DB_SOCKET) args.push('--socket=' + DB_SOCKET); + args.push(DB_NAME, '-N', '-B', '-e', sql); + return execFileSync('mysql', args, { + encoding: 'utf-8', timeout: 10000, + env: { ...process.env, MYSQL_PWD: DB_PASS }, + }).trim(); +} + +const TOKEN = Math.random().toString(16).slice(2, 8); +const FIELD_LABEL = `ZZ Telegram 255 ${TOKEN}`; +const USER_EMAIL = `zz-255-${TOKEN}@example.test`; + +test.describe.serial('Issue #255 — configurable registration fields', () => { + /** @type {import('@playwright/test').Page} */ + let admin; + + test.beforeAll(async ({ browser }) => { + const ctx = await browser.newContext(); + admin = await ctx.newPage(); + await admin.goto(`${BASE}/login`); + await admin.fill('input[name="email"]', ADMIN_EMAIL); + await admin.fill('input[name="password"]', ADMIN_PASS); + await Promise.all([ + admin.waitForURL(/\/admin\//, { timeout: 15000 }), + admin.click('button[type="submit"]'), + ]); + }); + + test.afterAll(async () => { + // Restore defaults + remove test data regardless of earlier failures. + dbQuery("UPDATE system_settings SET setting_value='1' WHERE category='registration' AND setting_key IN ('require_cognome','require_telefono','require_indirizzo')"); + dbQuery(`DELETE FROM registrazione_campi WHERE etichetta='${FIELD_LABEL.replace(/'/g, "''")}'`); + dbQuery(`DELETE FROM utenti WHERE email='${USER_EMAIL}'`); + await admin?.context()?.close(); + }); + + test('1. Defaults: surname/phone/address required on the public form', async ({ page }) => { + await page.goto(`${BASE}/registrati`); + for (const field of ['cognome', 'telefono', 'indirizzo']) { + await expect(page.locator(`[name="${field}"]`)).toHaveAttribute('required', /.*/); + } + }); + + test('2. Admin relaxes the toggles and adds a custom field', async () => { + await admin.goto(`${BASE}/admin/settings?tab=email`); + for (const toggle of ['require_cognome', 'require_telefono', 'require_indirizzo']) { + await admin.locator(`input[name="${toggle}"]`).uncheck(); + } + await admin.fill('input[name="new_custom_field_label"]', FIELD_LABEL); + await admin.selectOption('select[name="new_custom_field_type"]', 'text'); + await admin.click('form[action*="/admin/settings/email"] button[type="submit"]'); + await admin.waitForLoadState('domcontentloaded'); + + // Persisted: toggles off + field row present after reload. + await admin.goto(`${BASE}/admin/settings?tab=email`); + for (const toggle of ['require_cognome', 'require_telefono', 'require_indirizzo']) { + await expect(admin.locator(`input[name="${toggle}"]`)).not.toBeChecked(); + } + await expect(admin.locator(`input[value="${FIELD_LABEL}"]`)).toBeVisible(); + const fieldId = dbQuery(`SELECT id FROM registrazione_campi WHERE etichetta='${FIELD_LABEL.replace(/'/g, "''")}'`); + expect(Number(fieldId)).toBeGreaterThan(0); + }); + + test('3. Public registration without surname/phone/address succeeds with the custom value', async ({ page }) => { + await page.goto(`${BASE}/registrati`); + for (const field of ['cognome', 'telefono', 'indirizzo']) { + await expect(page.locator(`[name="${field}"]`)).not.toHaveAttribute('required', /.*/); + } + const fieldId = dbQuery(`SELECT id FROM registrazione_campi WHERE etichetta='${FIELD_LABEL.replace(/'/g, "''")}'`); + const customInput = page.locator(`[name="custom_field[${fieldId}]"]`); + await expect(customInput).toBeVisible(); + + await page.fill('input[name="nome"]', 'SoloNome255'); + await page.fill('input[name="email"]', USER_EMAIL); + await page.fill('input[name="password"]', 'Password255!ok'); + await page.fill('input[name="password_confirm"]', 'Password255!ok'); + await customInput.fill('@solonome'); + await page.check('input[name="privacy_acceptance"]'); + await Promise.all([ + page.waitForURL(/register|registrati|success|successo/i, { timeout: 20000 }), + page.click('button[type="submit"]'), + ]); + + const row = dbQuery(`SELECT nome, cognome, IFNULL(telefono,''), IFNULL(indirizzo,'') FROM utenti WHERE email='${USER_EMAIL}'`); + expect(row).toContain('SoloNome255'); + const [, cognome, telefono, indirizzo] = row.split('\t'); + expect(cognome).toBe(''); + expect(telefono).toBe(''); + expect(indirizzo).toBe(''); + + const stored = dbQuery(`SELECT v.valore FROM utenti_campi_valori v JOIN utenti u ON u.id=v.utente_id WHERE u.email='${USER_EMAIL}'`); + expect(stored).toBe('@solonome'); + }); + + test('4. Restoring the toggles makes the fields required again', async ({ page }) => { + await admin.goto(`${BASE}/admin/settings?tab=email`); + for (const toggle of ['require_cognome', 'require_telefono', 'require_indirizzo']) { + await admin.locator(`input[name="${toggle}"]`).check(); + } + await admin.click('form[action*="/admin/settings/email"] button[type="submit"]'); + await admin.waitForLoadState('domcontentloaded'); + + await page.goto(`${BASE}/registrati`); + for (const field of ['cognome', 'telefono', 'indirizzo']) { + await expect(page.locator(`[name="${field}"]`)).toHaveAttribute('required', /.*/); + } + }); +}); diff --git a/tests/migration-0.7.37.unit.php b/tests/migration-0.7.37.unit.php new file mode 100644 index 000000000..cf0e9006a --- /dev/null +++ b/tests/migration-0.7.37.unit.php @@ -0,0 +1,177 @@ + 1, 'etichetta' => 'Telegram', 'tipo' => 'text', 'obbligatorio' => true], + ['id' => 2, 'etichetta' => 'Sito', 'tipo' => 'url', 'obbligatorio' => false], + ['id' => 3, 'etichetta' => 'Newsletter', 'tipo' => 'checkbox', 'obbligatorio' => false], + ['id' => 4, 'etichetta' => 'Anno', 'tipo' => 'number', 'obbligatorio' => false], + ['id' => 5, 'etichetta' => 'Contatto', 'tipo' => 'email', 'obbligatorio' => false], +]; + +$r = RegistrationFields::validate($defs, ['custom_field' => [1 => '@mario', 2 => '', 4 => '', 5 => '']]); +$check($r['error'] === null && $r['values'][1] === '@mario' && $r['values'][2] === '', 'required text present + optional empties accepted'); + +$r = RegistrationFields::validate($defs, ['custom_field' => [2 => 'https://x.y']]); +$check($r['error'] !== null, 'missing required field rejected'); + +$r = RegistrationFields::validate($defs, ['custom_field' => [1 => 'x', 2 => 'javascript:alert(1)']]); +$check($r['error'] !== null, 'non-http(s) URL rejected'); + +$r = RegistrationFields::validate($defs, ['custom_field' => [1 => 'x', 4 => 'abc']]); +$check($r['error'] !== null, 'non-numeric number rejected'); + +$r = RegistrationFields::validate($defs, ['custom_field' => [1 => 'x', 5 => 'not-an-email']]); +$check($r['error'] !== null, 'invalid email rejected'); + +$r = RegistrationFields::validate($defs, ['custom_field' => [1 => 'x', 3 => 'on']]); +$check($r['error'] === null && $r['values'][3] === '1', 'checkbox normalises to 1'); + +$r = RegistrationFields::validate($defs, ['custom_field' => [1 => str_repeat('a', 1001)]]); +$check($r['error'] !== null, 'over-long value rejected'); + +// ── DB sections ────────────────────────────────────────────────────────────── +echo "A. Real migration against sandbox tables\n"; + +$env = []; +foreach (preg_split('/\r?\n/', (string) @file_get_contents($root . '/.env')) as $line) { + if (!str_contains($line, '=') || str_starts_with(trim($line), '#')) { + continue; + } + [$key, $value] = explode('=', $line, 2); + $env[trim($key)] = trim(trim($value), "\"'"); +} +try { + $socket = getenv('E2E_DB_SOCKET') ?: ($env['DB_SOCKET'] ?? ''); + $db = is_string($socket) && $socket !== '' && file_exists($socket) + ? new mysqli(null, $env['DB_USER'] ?? '', $env['DB_PASS'] ?? ($env['DB_PASSWORD'] ?? ''), $env['DB_NAME'] ?? '', 0, $socket) + : new mysqli($env['DB_HOST'] ?? '127.0.0.1', $env['DB_USER'] ?? '', $env['DB_PASS'] ?? ($env['DB_PASSWORD'] ?? ''), $env['DB_NAME'] ?? '', (int) ($env['DB_PORT'] ?? 3306)); + $db->set_charset('utf8mb4'); +} catch (\Throwable $e) { + echo "SKIP: no database reachable — DB sections skipped: {$e->getMessage()}\n"; + echo $fail === 0 ? "\nALL {$pass} PASS (DB sections skipped)\n" : "\n{$pass} PASS, {$fail} FAIL\n"; + exit($fail === 0 ? 0 : 1); +} + +$migration = (string) file_get_contents($root . '/installer/database/migrations/migrate_0.7.37.sql'); + +// Retarget the REAL migration at sandbox table names (project pattern: same +// DDL, only the names rewritten) so the test never collides with live tables. +$sandbox = static fn (string $sql): string => str_replace( + ['`registrazione_campi`', '`utenti_campi_valori`', '`utenti_campi_valori_utente_fk`', '`utenti_campi_valori_campo_fk`', '`idx_ucv_campo`'], + ['`zz_mig_registrazione_campi`', '`zz_mig_utenti_campi_valori`', '`zz_mig_ucv_utente_fk`', '`zz_mig_ucv_campo_fk`', '`zz_mig_idx_ucv_campo`'], + $sql +); + +$runMigration = static function () use ($db, $migration, $sandbox): void { + foreach (array_filter(array_map('trim', preg_split('/;\s*\n/', $sandbox(preg_replace('/^--.*$/m', '', $migration) ?? $migration)))) as $statement) { + if ($statement !== '') { + $db->query($statement); + } + } +}; + +$cleanup = static function () use ($db): void { + $db->query('DROP TABLE IF EXISTS zz_mig_utenti_campi_valori'); + $db->query('DROP TABLE IF EXISTS zz_mig_registrazione_campi'); + $db->query("DELETE FROM utenti WHERE email LIKE 'zz-mig-237-%@example.test'"); +}; + +try { + $cleanup(); + $runMigration(); + + $tables = static function (string $name) use ($db): bool { + $stmt = $db->prepare('SELECT 1 FROM information_schema.tables WHERE table_schema = DATABASE() AND table_name = ? LIMIT 1'); + $stmt->bind_param('s', $name); + $stmt->execute(); + $exists = $stmt->get_result()->fetch_row() !== null; + $stmt->close(); + return $exists; + }; + $check($tables('zz_mig_registrazione_campi') && $tables('zz_mig_utenti_campi_valori'), 'migration creates both tables'); + + $pk = $db->query( + "SELECT GROUP_CONCAT(column_name ORDER BY ordinal_position) FROM information_schema.key_column_usage + WHERE table_schema = DATABASE() AND table_name = 'zz_mig_utenti_campi_valori' AND constraint_name = 'PRIMARY'" + )->fetch_row()[0] ?? ''; + $check($pk === 'utente_id,campo_id', 'values PK is (utente_id, campo_id)'); + + $fkCount = (int) ($db->query( + "SELECT COUNT(*) FROM information_schema.table_constraints + WHERE table_schema = DATABASE() AND table_name = 'zz_mig_utenti_campi_valori' AND constraint_type = 'FOREIGN KEY'" + )->fetch_row()[0] ?? 0); + $check($fkCount === 2, 'values table carries both FKs'); + + // Idempotency: a second run must not error and must not duplicate anything. + $runMigration(); + $check(true, 'second migration run is a no-op (no error)'); + + echo "B. FK cascade semantics\n"; + $db->query("INSERT INTO utenti (nome, cognome, email, password, codice_tessera) VALUES ('ZZ', '', 'zz-mig-237-1@example.test', 'x', CONCAT('TZZ', FLOOR(RAND()*1000000)))"); + $uid = (int) $db->insert_id; + $db->query("INSERT INTO zz_mig_registrazione_campi (etichetta, tipo) VALUES ('Telegram', 'text')"); + $fid = (int) $db->insert_id; + $db->query("INSERT INTO zz_mig_utenti_campi_valori (utente_id, campo_id, valore) VALUES ({$uid}, {$fid}, '@zz')"); + + $db->query("DELETE FROM zz_mig_registrazione_campi WHERE id = {$fid}"); + $left = (int) ($db->query("SELECT COUNT(*) FROM zz_mig_utenti_campi_valori WHERE campo_id = {$fid}")->fetch_row()[0] ?? -1); + $check($left === 0, 'deleting a field definition cascades its values'); + + $db->query("INSERT INTO zz_mig_registrazione_campi (etichetta, tipo) VALUES ('Sito', 'url')"); + $fid2 = (int) $db->insert_id; + $db->query("INSERT INTO zz_mig_utenti_campi_valori (utente_id, campo_id, valore) VALUES ({$uid}, {$fid2}, 'https://x.y')"); + $db->query("DELETE FROM utenti WHERE id = {$uid}"); + $left = (int) ($db->query("SELECT COUNT(*) FROM zz_mig_utenti_campi_valori WHERE utente_id = {$uid}")->fetch_row()[0] ?? -1); + $check($left === 0, 'deleting a user cascades their values'); + + // Optional-surname contract: the utenti schema accepts '' for cognome + // (kept NOT NULL by design; see migration header comment). + $db->query("INSERT INTO utenti (nome, cognome, email, password, codice_tessera) VALUES ('SoloNome', '', 'zz-mig-237-2@example.test', 'x', CONCAT('TZY', FLOOR(RAND()*1000000)))"); + $uid2 = (int) $db->insert_id; + $stored = $db->query("SELECT cognome FROM utenti WHERE id = {$uid2}")->fetch_row()[0] ?? null; + $check($stored === '', "utenti accepts an empty surname (stored as '')"); +} finally { + $cleanup(); + $db->close(); +} + +echo $fail === 0 ? "\nALL {$pass} PASS\n" : "\n{$pass} PASS, {$fail} FAIL\n"; +exit($fail === 0 ? 0 : 1); diff --git a/version.json b/version.json index 5b87231a9..89d8f318f 100644 --- a/version.json +++ b/version.json @@ -1,5 +1,5 @@ { "name": "Pinakes", - "version": "0.7.35", + "version": "0.7.37", "description": "Library Management System - Sistema di Gestione Bibliotecaria" } From 15aecd0051ce34feb1fc6fc33a79ccfe1a555309 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 16 Jul 2026 15:55:46 +0200 Subject: [PATCH 2/7] fix(#255): dedicated Registration settings tab + hardened validation + 25-check E2E MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the review round and move the registration settings where they belong: - New Settings → Registrazione tab with its own form and POST endpoint (/admin/settings/registration): admin-approval flag, the three built-in requirement toggles and the custom-field editor move out of the email form. Email settings save is registration-free again. - Validation hardening: RegistrationFields::validate() rejects non-scalar payloads (custom_field[id][]=x would cast to "Array", pass required checks and tick checkboxes); the requirement toggles now also govern phone and address in the profile (view + server) and address in the admin create/edit forms, so a member can't blank a field the library requires. - Atomic persistence: the custom-field definitions save runs in a transaction with per-statement checks and a visible error flash on rollback; the profile UPDATE and the user's custom values commit or roll back together. - View escaping: htmlspecialchars(..., ENT_QUOTES, 'UTF-8') replaces HtmlHelper::e() in the new view code (project rule). - migration-0.7.37.unit.php now FAILS when no database is reachable — a silently-skipped migration test is a false green in CI (both workflows provision MySQL). The redundant extra ci-e2e step was skipped on purpose: ci-quality already runs every tests/*.unit.php against MySQL. - issue-255-registration.spec.js grows from 4 to 25 real-browser checks: server-side enforcement of each built-in requirement via novalidate submits, all six custom types created and rendered with the right controls, the full 2^3 toggle matrix, a registration storing every type, invalid values (bad email, javascript: URL, non-numeric number, missing required, >1000 chars) rejected by the server with crafted POSTs, label tags neutralised on save, script-carrying values rendered escaped on the admin detail (never executed), and inactive fields disappearing from the form. Verified: PHPStan L5 = 0, unit 38/38, issue-255 E2E 25/25, full-test.spec.js 136 passed (one external scraping-provider 504 flake reran green). --- app/Controllers/ProfileController.php | 39 ++- app/Controllers/SettingsController.php | 156 +++++++---- app/Controllers/UsersController.php | 6 + app/Routes/web.php | 6 + app/Support/RegistrationFields.php | 9 +- app/Views/profile/index.php | 14 +- app/Views/settings/index.php | 19 +- app/Views/utenti/modifica_utente.php | 4 +- locale/de_DE.json | 5 +- locale/en_US.json | 5 +- locale/fr_FR.json | 5 +- locale/it_IT.json | 5 +- tests/issue-255-registration.spec.js | 357 ++++++++++++++++++++----- tests/migration-0.7.37.unit.php | 7 +- 14 files changed, 486 insertions(+), 151 deletions(-) diff --git a/app/Controllers/ProfileController.php b/app/Controllers/ProfileController.php index 7e4325c63..957adb1ec 100644 --- a/app/Controllers/ProfileController.php +++ b/app/Controllers/ProfileController.php @@ -162,10 +162,15 @@ public function update(Request $request, Response $response, mysqli $db): Respon $locale = ($locale !== '' && isset($availableLocales[$locale])) ? $locale : null; } - // Validate required fields. The surname follows the admin toggle - // (issue #255); when optional it is stored as '' — the column is NOT + // Validate required fields. Surname/phone/address follow the admin + // toggles (issue #255) so a member can't blank a field the library + // requires. An optional surname is stored as '' — the column is NOT // NULL by design so display paths that CONCAT nome+cognome stay whole. - if (empty($nome) || ($cognome === '' && \App\Support\RegistrationFields::isRequired('cognome'))) { + if (empty($nome) + || ($cognome === '' && \App\Support\RegistrationFields::isRequired('cognome')) + || ($telefono === null && \App\Support\RegistrationFields::isRequired('telefono')) + || ($indirizzo === null && \App\Support\RegistrationFields::isRequired('indirizzo')) + ) { $profileUrl = RouteTranslator::route('profile'); return $response->withHeader('Location', $profileUrl . '?error=required_fields')->withStatus(302); } @@ -198,14 +203,30 @@ public function update(Request $request, Response $response, mysqli $db): Respon $stmt->bind_param('sssssssi', $nome, $cognome, $telefono, $data_nascita, $cod_fiscale, $sesso, $indirizzo, $uid); } - if ($stmt->execute()) { - // Persist custom field values (issue #255) — best-effort: a failure - // here must not undo the successful profile update. - try { + // Profile row + custom field values land atomically: a failed custom + // value write rolls back the profile update too, so the user never + // sees "saved" with half the form persisted. + $updated = false; + try { + $db->begin_transaction(); + $updated = $stmt->execute(); + if ($updated) { \App\Support\RegistrationFields::saveValues($db, $uid, $customValidation['values']); - } catch (\Throwable $e) { - SecureLogger::error('ProfileController: custom field save failed', ['user_id' => $uid, 'error' => $e->getMessage()]); + $db->commit(); + } else { + $db->rollback(); + } + } catch (\Throwable $e) { + try { + $db->rollback(); + } catch (\Throwable) { + // best-effort — never mask the original error } + SecureLogger::error('ProfileController: profile/custom-field save failed', ['user_id' => $uid, 'error' => $e->getMessage()]); + $updated = false; + } + + if ($updated) { // Update session data $_SESSION['user']['name'] = trim($nome . ' ' . $cognome); // Apply locale change immediately (only when locale was in the form) diff --git a/app/Controllers/SettingsController.php b/app/Controllers/SettingsController.php index 256fae5a5..f619fc9bf 100644 --- a/app/Controllers/SettingsController.php +++ b/app/Controllers/SettingsController.php @@ -192,20 +192,8 @@ public function updateEmailSettings(Request $request, Response $response, mysqli } $repository->set('email', 'smtp_security', $encryption); - // Handle registration setting (require_admin_approval is in the same form) - $requireApproval = isset($data['require_admin_approval']) ? '1' : '0'; - $repository->set('registration', 'require_admin_approval', $requireApproval); - ConfigStore::set('registration.require_admin_approval', (bool) $requireApproval); - - // Built-in registration field requirements (issue #255) - foreach (\App\Support\RegistrationFields::BUILTIN_TOGGLES as $toggleKey) { - $flag = isset($data[$toggleKey]) ? '1' : '0'; - $repository->set('registration', $toggleKey, $flag); - ConfigStore::set('registration.' . $toggleKey, (bool) $flag); - } - - // Custom registration field definitions (issue #255) - $this->saveRegistrationCustomFields($db, $data); + // Registration settings moved to their own tab/form (issue #255): + // see updateRegistrationSettings(). ConfigStore::set('mail.driver', $driver); ConfigStore::set('mail.from_email', $fromEmail); @@ -222,6 +210,37 @@ public function updateEmailSettings(Request $request, Response $response, mysqli return $this->redirect($response, '/admin/settings?tab=email'); } + /** + * Save the Registration tab (issue #255): admin-approval flag, the + * built-in field requirement toggles and the custom field definitions. + */ + public function updateRegistrationSettings(Request $request, Response $response, mysqli $db): Response + { + $data = (array) $request->getParsedBody(); + // CSRF validated by CsrfMiddleware + + $repository = new SettingsRepository($db); + $repository->ensureTables(); + + $requireApproval = isset($data['require_admin_approval']) ? '1' : '0'; + $repository->set('registration', 'require_admin_approval', $requireApproval); + ConfigStore::set('registration.require_admin_approval', (bool) $requireApproval); + + foreach (\App\Support\RegistrationFields::BUILTIN_TOGGLES as $toggleKey) { + $flag = isset($data[$toggleKey]) ? '1' : '0'; + $repository->set('registration', $toggleKey, $flag); + ConfigStore::set('registration.' . $toggleKey, (bool) $flag); + } + + $this->saveRegistrationCustomFields($db, $data); + + // saveRegistrationCustomFields surfaces its own error flash on failure. + if (empty($_SESSION['error_message'])) { + $_SESSION['success_message'] = __('Impostazioni di registrazione aggiornate correttamente.'); + } + return $this->redirect($response, '/admin/settings?tab=registration'); + } + /** * Persist the admin-defined custom registration fields (issue #255). * Existing rows arrive as custom_fields[][etichetta|tipo|obbligatorio|attivo|delete]; @@ -250,59 +269,82 @@ private function saveRegistrationCustomFields(mysqli $db, array $data): void return; } - $rows = $data['custom_fields'] ?? []; - if (is_array($rows)) { - $update = $db->prepare( - 'UPDATE registrazione_campi SET etichetta = ?, tipo = ?, obbligatorio = ?, attivo = ? WHERE id = ?' - ); - $delete = $db->prepare('DELETE FROM registrazione_campi WHERE id = ?'); - if ($update === false || $delete === false) { - return; - } - foreach ($rows as $id => $row) { - $id = (int) $id; - if ($id <= 0 || !is_array($row)) { - continue; - } - if (!empty($row['delete'])) { - $delete->bind_param('i', $id); - $delete->execute(); - continue; + // All row mutations land atomically: a mid-batch failure must not + // leave half the definitions updated while the page reports success. + $db->begin_transaction(); + try { + $rows = $data['custom_fields'] ?? []; + if (is_array($rows)) { + $update = $db->prepare( + 'UPDATE registrazione_campi SET etichetta = ?, tipo = ?, obbligatorio = ?, attivo = ? WHERE id = ?' + ); + $delete = $db->prepare('DELETE FROM registrazione_campi WHERE id = ?'); + if ($update === false || $delete === false) { + throw new \RuntimeException('Unable to prepare custom field statements'); } - $label = trim(strip_tags((string) ($row['etichetta'] ?? ''))); - $tipo = (string) ($row['tipo'] ?? 'text'); - if ($label === '' || mb_strlen($label) > 100 - || !in_array($tipo, \App\Support\RegistrationFields::TYPES, true) - ) { - continue; + foreach ($rows as $id => $row) { + $id = (int) $id; + if ($id <= 0 || !is_array($row)) { + continue; + } + if (!empty($row['delete'])) { + $delete->bind_param('i', $id); + if (!$delete->execute()) { + throw new \RuntimeException('Custom field delete failed: ' . $delete->error); + } + continue; + } + $label = trim(strip_tags((string) ($row['etichetta'] ?? ''))); + $tipo = (string) ($row['tipo'] ?? 'text'); + if ($label === '' || mb_strlen($label) > 100 + || !in_array($tipo, \App\Support\RegistrationFields::TYPES, true) + ) { + continue; + } + $required = !empty($row['obbligatorio']) ? 1 : 0; + $active = !empty($row['attivo']) ? 1 : 0; + $update->bind_param('ssiii', $label, $tipo, $required, $active, $id); + if (!$update->execute()) { + throw new \RuntimeException('Custom field update failed: ' . $update->error); + } } - $required = !empty($row['obbligatorio']) ? 1 : 0; - $active = !empty($row['attivo']) ? 1 : 0; - $update->bind_param('ssiii', $label, $tipo, $required, $active, $id); - $update->execute(); + $update->close(); + $delete->close(); } - $update->close(); - $delete->close(); - } - $newLabel = trim(strip_tags((string) ($data['new_custom_field_label'] ?? ''))); - $newType = (string) ($data['new_custom_field_type'] ?? 'text'); - if ($newLabel !== '' && mb_strlen($newLabel) <= 100 - && in_array($newType, \App\Support\RegistrationFields::TYPES, true) - ) { - $newRequired = !empty($data['new_custom_field_required']) ? 1 : 0; - $insert = $db->prepare( - 'INSERT INTO registrazione_campi (etichetta, tipo, obbligatorio, attivo, ordine) - SELECT ?, ?, ?, 1, COALESCE(MAX(ordine), 0) + 1 FROM registrazione_campi' - ); - if ($insert !== false) { + $newLabel = trim(strip_tags((string) ($data['new_custom_field_label'] ?? ''))); + $newType = (string) ($data['new_custom_field_type'] ?? 'text'); + if ($newLabel !== '' && mb_strlen($newLabel) <= 100 + && in_array($newType, \App\Support\RegistrationFields::TYPES, true) + ) { + $newRequired = !empty($data['new_custom_field_required']) ? 1 : 0; + $insert = $db->prepare( + 'INSERT INTO registrazione_campi (etichetta, tipo, obbligatorio, attivo, ordine) + SELECT ?, ?, ?, 1, COALESCE(MAX(ordine), 0) + 1 FROM registrazione_campi' + ); + if ($insert === false) { + throw new \RuntimeException('Unable to prepare custom field insert'); + } $insert->bind_param('ssi', $newLabel, $newType, $newRequired); - $insert->execute(); + if (!$insert->execute()) { + throw new \RuntimeException('Custom field insert failed: ' . $insert->error); + } $insert->close(); } + $db->commit(); + } catch (\Throwable $inner) { + try { + $db->rollback(); + } catch (\Throwable) { + // best-effort — surface the original error below + } + throw $inner; } } catch (\Throwable $e) { \App\Support\SecureLogger::error('[Settings] custom registration fields save failed', ['error' => $e->getMessage()]); + // Surface the failure: the settings page must not claim success + // when the field definitions were rolled back. + $_SESSION['error_message'] = __('Salvataggio dei campi personalizzati non riuscito. Riprova.'); } } diff --git a/app/Controllers/UsersController.php b/app/Controllers/UsersController.php index 3cd21db66..cd9def021 100644 --- a/app/Controllers/UsersController.php +++ b/app/Controllers/UsersController.php @@ -130,6 +130,9 @@ public function store(Request $request, Response $response, mysqli $db): Respons $indirizzo = trim(strip_tags((string) ($data['indirizzo'] ?? ''))); $indirizzo = $indirizzo !== '' ? $indirizzo : null; + if (!$isAdmin && $indirizzo === null && \App\Support\RegistrationFields::isRequired('indirizzo')) { + return $response->withHeader('Location', url('/admin/users/create?error=missing_fields'))->withStatus(302); + } $cod_fiscale = strtoupper(trim((string) ($data['cod_fiscale'] ?? ''))); $cod_fiscale = $cod_fiscale !== '' ? $cod_fiscale : null; @@ -358,6 +361,9 @@ public function update(Request $request, Response $response, mysqli $db, int $id $indirizzo = trim(strip_tags((string) ($data['indirizzo'] ?? ''))); $indirizzo = $indirizzo !== '' ? $indirizzo : null; + if (!$isAdmin && $indirizzo === null && \App\Support\RegistrationFields::isRequired('indirizzo')) { + return $response->withHeader('Location', url('/admin/users/edit/' . $id . '?error=missing_fields'))->withStatus(302); + } $cod_fiscale = strtoupper(trim((string) ($data['cod_fiscale'] ?? ''))); $cod_fiscale = $cod_fiscale !== '' ? $cod_fiscale : null; diff --git a/app/Routes/web.php b/app/Routes/web.php index f97f92989..479e13696 100644 --- a/app/Routes/web.php +++ b/app/Routes/web.php @@ -572,6 +572,12 @@ return $controller->updateEmailSettings($request, $response, $db); })->add(new CsrfMiddleware())->add(new AdminAuthMiddleware()); + $app->post('/admin/settings/registration', function ($request, $response) use ($app) { + $db = $app->getContainer()->get('db'); + $controller = new SettingsController(); + return $controller->updateRegistrationSettings($request, $response, $db); + })->add(new CsrfMiddleware())->add(new AdminAuthMiddleware()); + $app->post('/admin/settings/contacts', function ($request, $response) use ($app) { $db = $app->getContainer()->get('db'); $controller = new SettingsController(); diff --git a/app/Support/RegistrationFields.php b/app/Support/RegistrationFields.php index 49ff44366..7cb5552ee 100644 --- a/app/Support/RegistrationFields.php +++ b/app/Support/RegistrationFields.php @@ -97,7 +97,14 @@ public static function validate(array $definitions, array $post): array $values = []; foreach ($definitions as $def) { - $raw = trim((string) ($submitted[$def['id']] ?? '')); + // Reject non-scalar payloads (custom_field[id][]=x) BEFORE casting: + // (string) on an array would coerce to "Array", passing required + // checks and persisting garbage / ticking checkboxes. + $submittedValue = $submitted[$def['id']] ?? null; + if ($submittedValue !== null && !is_scalar($submittedValue)) { + return ['values' => [], 'error' => $def['etichetta']]; + } + $raw = trim((string) ($submittedValue ?? '')); if ($def['tipo'] === 'checkbox') { $values[$def['id']] = $raw !== '' ? '1' : ''; diff --git a/app/Views/profile/index.php b/app/Views/profile/index.php index 9557a60ce..48f4346d7 100644 --- a/app/Views/profile/index.php +++ b/app/Views/profile/index.php @@ -413,8 +413,9 @@
- - + + value="">
@@ -442,8 +443,9 @@
- - + + value="">
@@ -462,11 +464,11 @@ - + - value=""> + value="">
diff --git a/app/Views/settings/index.php b/app/Views/settings/index.php index 80f4f910e..a9810c4c4 100644 --- a/app/Views/settings/index.php +++ b/app/Views/settings/index.php @@ -29,6 +29,10 @@ +
+ +
+ +
+ + + +
+
+

diff --git a/app/Views/utenti/crea_utente.php b/app/Views/utenti/crea_utente.php index 503f6dcb2..a29351910 100644 --- a/app/Views/utenti/crea_utente.php +++ b/app/Views/utenti/crea_utente.php @@ -72,7 +72,7 @@
- "> + class="mt-1 block w-full rounded-md border-gray-300 focus:border-gray-900 focus:ring-gray-900" placeholder="">
diff --git a/app/Views/utenti/modifica_utente.php b/app/Views/utenti/modifica_utente.php index 4d66fbced..63651501b 100644 --- a/app/Views/utenti/modifica_utente.php +++ b/app/Views/utenti/modifica_utente.php @@ -80,7 +80,7 @@
- + class="mt-1 block w-full rounded-md border-gray-300 focus:border-gray-900 focus:ring-gray-900">
diff --git a/locale/de_DE.json b/locale/de_DE.json index 84b209c53..1da9e9cfe 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -6533,9 +6533,9 @@ "Disattiva i campi che la tua comunità non vuole raccogliere: diventeranno facoltativi nel modulo di registrazione.": "Deaktiviere die Felder, die deine Gemeinschaft nicht erheben möchte: sie werden im Registrierungsformular optional.", "Campi aggiuntivi definiti da te (es. username Telegram): compaiono nel modulo di registrazione e nel profilo utente.": "Von dir definierte Zusatzfelder (z. B. Telegram-Benutzername): sie erscheinen im Registrierungsformular und im Benutzerprofil.", "Etichetta nuovo campo": "Bezeichnung des neuen Felds", - "Le modifiche ai campi vengono salvate insieme alle impostazioni email.": "Feldänderungen werden zusammen mit den E-Mail-Einstellungen gespeichert.", "L'utente li gestisce dal proprio profilo.": "Der Benutzer verwaltet sie über sein Profil.", "Salva impostazioni registrazione": "Registrierungseinstellungen speichern", "Impostazioni di registrazione aggiornate correttamente.": "Registrierungseinstellungen erfolgreich aktualisiert.", - "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Speichern der benutzerdefinierten Felder fehlgeschlagen. Bitte erneut versuchen." + "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Speichern der benutzerdefinierten Felder fehlgeschlagen. Bitte erneut versuchen.", + "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Feldänderungen werden zusammen mit den Registrierungseinstellungen gespeichert." } diff --git a/locale/en_US.json b/locale/en_US.json index 6332a068f..2a8f16e9b 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -6533,9 +6533,9 @@ "Disattiva i campi che la tua comunità non vuole raccogliere: diventeranno facoltativi nel modulo di registrazione.": "Untick the fields your community doesn't want to collect: they become optional on the registration form.", "Campi aggiuntivi definiti da te (es. username Telegram): compaiono nel modulo di registrazione e nel profilo utente.": "Additional fields you define (e.g. a Telegram username): they appear on the registration form and in the user profile.", "Etichetta nuovo campo": "New field label", - "Le modifiche ai campi vengono salvate insieme alle impostazioni email.": "Field changes are saved together with the email settings.", "L'utente li gestisce dal proprio profilo.": "The user manages them from their profile.", "Salva impostazioni registrazione": "Save registration settings", "Impostazioni di registrazione aggiornate correttamente.": "Registration settings updated successfully.", - "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Saving the custom fields failed. Please try again." + "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Saving the custom fields failed. Please try again.", + "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Field changes are saved together with the registration settings." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 216fa7ec3..e3e839ea3 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -6533,9 +6533,9 @@ "Disattiva i campi che la tua comunità non vuole raccogliere: diventeranno facoltativi nel modulo di registrazione.": "Décochez les champs que votre communauté ne souhaite pas collecter : ils deviendront facultatifs dans le formulaire d'inscription.", "Campi aggiuntivi definiti da te (es. username Telegram): compaiono nel modulo di registrazione e nel profilo utente.": "Champs supplémentaires définis par vous (p. ex. nom d'utilisateur Telegram) : ils apparaissent dans le formulaire d'inscription et dans le profil utilisateur.", "Etichetta nuovo campo": "Libellé du nouveau champ", - "Le modifiche ai campi vengono salvate insieme alle impostazioni email.": "Les modifications des champs sont enregistrées avec les paramètres e-mail.", "L'utente li gestisce dal proprio profilo.": "L'utilisateur les gère depuis son profil.", "Salva impostazioni registrazione": "Enregistrer les paramètres d'inscription", "Impostazioni di registrazione aggiornate correttamente.": "Paramètres d'inscription mis à jour avec succès.", - "Salvataggio dei campi personalizzati non riuscito. Riprova.": "L'enregistrement des champs personnalisés a échoué. Veuillez réessayer." + "Salvataggio dei campi personalizzati non riuscito. Riprova.": "L'enregistrement des champs personnalisés a échoué. Veuillez réessayer.", + "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Les modifications des champs sont enregistrées avec les paramètres d'inscription." } diff --git a/locale/it_IT.json b/locale/it_IT.json index 95669546b..781da155f 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -6533,9 +6533,9 @@ "Disattiva i campi che la tua comunità non vuole raccogliere: diventeranno facoltativi nel modulo di registrazione.": "Disattiva i campi che la tua comunità non vuole raccogliere: diventeranno facoltativi nel modulo di registrazione.", "Campi aggiuntivi definiti da te (es. username Telegram): compaiono nel modulo di registrazione e nel profilo utente.": "Campi aggiuntivi definiti da te (es. username Telegram): compaiono nel modulo di registrazione e nel profilo utente.", "Etichetta nuovo campo": "Etichetta nuovo campo", - "Le modifiche ai campi vengono salvate insieme alle impostazioni email.": "Le modifiche ai campi vengono salvate insieme alle impostazioni email.", "L'utente li gestisce dal proprio profilo.": "L'utente li gestisce dal proprio profilo.", "Salva impostazioni registrazione": "Salva impostazioni registrazione", "Impostazioni di registrazione aggiornate correttamente.": "Impostazioni di registrazione aggiornate correttamente.", - "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Salvataggio dei campi personalizzati non riuscito. Riprova." + "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Salvataggio dei campi personalizzati non riuscito. Riprova.", + "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione." } diff --git a/tests/issue-255-registration.spec.js b/tests/issue-255-registration.spec.js index c5caf99f0..1146b7fb9 100644 --- a/tests/issue-255-registration.spec.js +++ b/tests/issue-255-registration.spec.js @@ -105,11 +105,16 @@ test.describe.serial('Issue #255 — configurable registration fields (25 checks admin.waitForURL(/\/admin\//, { timeout: 15000 }), admin.click('button[type="submit"]'), ]); - setToggles(true, true, true); + // Tests 1-4 must exercise the application's ACTUAL defaults (no toggle + // rows in system_settings → code-side default = required), not a value + // this suite planted. Deleting also makes the run deterministic on a + // DB dirtied by earlier manual testing. + dbQuery("DELETE FROM system_settings WHERE category='registration' AND setting_key IN ('require_cognome','require_telefono','require_indirizzo')"); }); test.afterAll(async () => { - setToggles(true, true, true); + // Restore the pristine default state (absent rows = code default). + dbQuery("DELETE FROM system_settings WHERE category='registration' AND setting_key IN ('require_cognome','require_telefono','require_indirizzo')"); dbQuery(`DELETE FROM registrazione_campi WHERE etichetta LIKE 'ZZ255 %${TOKEN}%' OR etichetta LIKE '%${TOKEN}%'`); dbQuery(`DELETE FROM utenti WHERE email LIKE 'zz-255-%${TOKEN}@example.test'`); await admin?.context()?.close(); diff --git a/tests/schema-integrity.spec.js b/tests/schema-integrity.spec.js index 4e4d9c71f..c7c09fda0 100644 --- a/tests/schema-integrity.spec.js +++ b/tests/schema-integrity.spec.js @@ -7,7 +7,7 @@ * correctly, whether via fresh install (schema.sql) or upgrade (migrate_*.sql). * * IMPORTANT — two-tier table model: - * CORE_TABLES (57): always created by the base installer via schema.sql. + * CORE_TABLES (59): always created by the base installer via schema.sql. * Archives tables (5): created only when the Archives plugin is activated * (archival_units, authority_records, archival_unit_authority, * autori_authority_link, archival_unit_files). Tests 10–12 skip @@ -96,7 +96,7 @@ function parseCoreTablesFromSchema() { // ── Expected state ──────────────────────────────────────────────────────────── /** - * 57 tables always created by the base installer via schema.sql. + * 59 tables always created by the base installer via schema.sql. * Present after fresh install AND after upgrade, regardless of which * optional plugins the user has activated. * @@ -112,9 +112,9 @@ const CORE_TABLES = [ 'ncip_partners', 'ncip_transactions', 'oai_deleted_records', 'oai_resumption_tokens', 'plugin_data', 'plugin_hooks', 'plugin_logs', 'plugin_settings', 'plugins', 'posizioni', 'preferenze_notifica_utenti', - 'prenotazioni', 'prestiti', 'recensioni', 'scaffali', 'sedi', 'staff', + 'prenotazioni', 'prestiti', 'recensioni', 'registrazione_campi', 'scaffali', 'sedi', 'staff', 'system_settings', 'tag', 'themes', 'update_logs', 'user_sessions', - 'utenti', 'volumi', 'wishlist', 'z39_access_logs', 'z39_rate_limits', + 'utenti', 'utenti_campi_valori', 'volumi', 'wishlist', 'z39_access_logs', 'z39_rate_limits', ]; /** All 16 bundled plugins that must be registered in `plugins` table */ From 532c4bc1f58cf7da75eb49566d1135c516619084 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 16 Jul 2026 16:45:47 +0200 Subject: [PATCH 4/7] fix(#255): admin-form client validation follows the toggles + review leftovers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - crea/modifica utente: the JS that forced the phone required for every non-admin now honours require_telefono, and the address gains the same toggle-driven required the server enforces; the surname asterisk follows its toggle like the input's required attribute already did. - SettingsController: the non-scalar payload guard also covers the new_custom_field_* inputs (an array would cast to "Array" and create a garbage definition). - E2E teardown deletes utenti_campi_valori rows before their parents: FK-safe order per project test rules — teardown must not depend on the cascades the suite itself verifies. Already addressed in the previous commit (review overlap): registration-tab note wording, required_fields in the profile error map, CSRF hidden escaping, stale it_IT string, default-purity of tests 1-4, hardcoded CORE_TABLES. Skipped with evidence: the ConfigStore boolean finding — (bool)'0' is FALSE in PHP ('0' is falsy), so unchecked toggles already reach ConfigStore as false. Verified: PHPStan L5 = 0, issue-255 E2E 25/25. --- app/Controllers/SettingsController.php | 7 +++++++ app/Views/utenti/crea_utente.php | 12 ++++++++++-- app/Views/utenti/modifica_utente.php | 12 ++++++++++-- tests/issue-255-registration.spec.js | 5 +++++ 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/app/Controllers/SettingsController.php b/app/Controllers/SettingsController.php index 062e0411e..9650ee148 100644 --- a/app/Controllers/SettingsController.php +++ b/app/Controllers/SettingsController.php @@ -320,6 +320,13 @@ private function saveRegistrationCustomFields(mysqli $db, array $data): void $delete->close(); } + // Same non-scalar guard for the add-new inputs: an array here + // would cast to "Array" and create a garbage definition. + foreach (['new_custom_field_label', 'new_custom_field_type', 'new_custom_field_required'] as $newKey) { + if (isset($data[$newKey]) && !is_scalar($data[$newKey])) { + throw new \RuntimeException('Non-scalar custom field payload rejected'); + } + } $newLabel = trim(strip_tags((string) ($data['new_custom_field_label'] ?? ''))); $newType = (string) ($data['new_custom_field_type'] ?? 'text'); if ($newLabel !== '' && mb_strlen($newLabel) <= 100 diff --git a/app/Views/utenti/crea_utente.php b/app/Views/utenti/crea_utente.php index a29351910..5f80c9a2e 100644 --- a/app/Views/utenti/crea_utente.php +++ b/app/Views/utenti/crea_utente.php @@ -71,7 +71,7 @@ ">
- + class="mt-1 block w-full rounded-md border-gray-300 focus:border-gray-900 focus:ring-gray-900" placeholder="">
@@ -152,6 +152,11 @@ (function() { const roleField = document.getElementById('tipo_utente'); const phoneField = document.getElementById('telefono'); + const addressField = document.getElementById('indirizzo'); + // Requirement toggles (issue #255): the client mirrors the server checks in + // UsersController, which apply only to non-admin users. + const REQUIRE_TELEFONO = ; + const REQUIRE_INDIRIZZO = ; const tesseraSection = document.querySelector('[data-admin-tessera]'); const adminBlocks = document.querySelectorAll('[data-admin-hide]'); const roleHint = document.getElementById('role-hint'); @@ -186,7 +191,10 @@ function applyRoleState() { const isAdmin = roleField.value === 'admin'; if (phoneField) { - phoneField.required = !isAdmin; + phoneField.required = !isAdmin && REQUIRE_TELEFONO; + if (addressField) { + addressField.required = !isAdmin && REQUIRE_INDIRIZZO; + } phoneField.placeholder = isAdmin ? __('Opzionale per amministratori') : __('+39 123 456 7890'); } diff --git a/app/Views/utenti/modifica_utente.php b/app/Views/utenti/modifica_utente.php index 63651501b..149f1bd65 100644 --- a/app/Views/utenti/modifica_utente.php +++ b/app/Views/utenti/modifica_utente.php @@ -79,7 +79,7 @@
- + class="mt-1 block w-full rounded-md border-gray-300 focus:border-gray-900 focus:ring-gray-900">
@@ -173,6 +173,11 @@ (function() { const roleField = document.getElementById('tipo_utente'); const phoneField = document.getElementById('telefono'); + const addressField = document.getElementById('indirizzo'); + // Requirement toggles (issue #255): the client mirrors the server checks in + // UsersController, which apply only to non-admin users. + const REQUIRE_TELEFONO = ; + const REQUIRE_INDIRIZZO = ; const tesseraSection = document.querySelector('[data-admin-tessera]'); const adminBlocks = document.querySelectorAll('[data-admin-hide]'); const roleHint = document.getElementById('role-hint'); @@ -195,7 +200,10 @@ function applyRoleState() { const isAdmin = roleField.value === 'admin'; if (phoneField) { - phoneField.required = !isAdmin; + phoneField.required = !isAdmin && REQUIRE_TELEFONO; + if (addressField) { + addressField.required = !isAdmin && REQUIRE_INDIRIZZO; + } phoneField.placeholder = isAdmin ? __('Opzionale per amministratori') : '+39 123 456 7890'; } diff --git a/tests/issue-255-registration.spec.js b/tests/issue-255-registration.spec.js index 1146b7fb9..9e1ce6542 100644 --- a/tests/issue-255-registration.spec.js +++ b/tests/issue-255-registration.spec.js @@ -115,6 +115,11 @@ test.describe.serial('Issue #255 — configurable registration fields (25 checks test.afterAll(async () => { // Restore the pristine default state (absent rows = code default). dbQuery("DELETE FROM system_settings WHERE category='registration' AND setting_key IN ('require_cognome','require_telefono','require_indirizzo')"); + // FK-safe order (children first, then parents) — the cascades exist and + // are asserted by the migration test, but teardown must not depend on + // the very behaviour the suite verifies. + dbQuery(`DELETE v FROM utenti_campi_valori v JOIN registrazione_campi c ON c.id = v.campo_id WHERE c.etichetta LIKE '%${TOKEN}%'`); + dbQuery(`DELETE v FROM utenti_campi_valori v JOIN utenti u ON u.id = v.utente_id WHERE u.email LIKE 'zz-255-%${TOKEN}@example.test'`); dbQuery(`DELETE FROM registrazione_campi WHERE etichetta LIKE 'ZZ255 %${TOKEN}%' OR etichetta LIKE '%${TOKEN}%'`); dbQuery(`DELETE FROM utenti WHERE email LIKE 'zz-255-%${TOKEN}@example.test'`); await admin?.context()?.close(); From 4b660568c359e974de608675b8117eae2f9d95ee Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 16 Jul 2026 18:37:09 +0200 Subject: [PATCH 5/7] fix(#255): address self-review findings (optional-surname blast radius, custom-field UX) Self-review of this branch surfaced three confirmed issues plus a few lower-severity ones; all fixed here. Confirmed: - Optional surname left a trailing space. cognome='' (the new optional path) made 6 unchanged display sites render "Mario " via naive nome.' '.cognome. Added a reusable full_name() helper that collapses the gap and applied it at all six (utenti/index, admin/stats, settings/messages-tab, PrestitiController, two UsersController audit fields). - A custom field made required AFTER signup walled existing users out of unrelated profile edits. RegistrationFields::validate() gained an $enforceRequired flag; the profile passes false so required is enforced at registration (the field's contract) but a blank pre-existing value no longer blocks a profile save. Format checks still apply to anything typed. - Format-invalid custom values collapsed into the generic "fill required fields" banner. validate() now returns error_reason (missing|format) and both registration and profile map a format failure to a distinct custom_field_invalid code + message, so a user who DID fill the field isn't told to fill it. Lower-severity: - Un-nested the admin-form JS so addressField.required no longer depends on the phone field's DOM presence. - updateRegistrationSettings now persists the custom-field batch first and bails before writing the toggles on failure, so a field-batch error never leaves toggles applied under a "save failed" flash. saveRegistrationCustomFields returns bool instead of signalling through the session. - Deleting a custom field (cascades to every user's stored value) now asks for confirmation at submit. - Fixed a stale comment in resolveEmailSettings that still pointed at the email form. New i18n strings in all four locales (parity verified). Tests: migration unit grows to 21 checks (error_reason, enforceRequired=false, full_name); issue-255 E2E grows to 27 (distinct error code, no-trailing-space). PHPStan L5 = 0, unit 38/38, full-test.spec.js 136 passed. --- app/Controllers/PrestitiController.php | 2 +- app/Controllers/ProfileController.php | 10 ++++-- app/Controllers/RegistrationController.php | 5 ++- app/Controllers/SettingsController.php | 33 ++++++++++-------- app/Controllers/UsersController.php | 4 +-- app/Support/RegistrationFields.php | 37 +++++++++++++------- app/Views/admin/stats.php | 2 +- app/Views/auth/register.php | 2 ++ app/Views/profile/index.php | 1 + app/Views/settings/index.php | 15 +++++++++ app/Views/settings/messages-tab.php | 2 +- app/Views/utenti/crea_utente.php | 6 ++-- app/Views/utenti/index.php | 2 +- app/Views/utenti/modifica_utente.php | 6 ++-- app/helpers.php | 18 ++++++++++ locale/de_DE.json | 4 ++- locale/en_US.json | 4 ++- locale/fr_FR.json | 4 ++- locale/it_IT.json | 4 ++- tests/issue-255-registration.spec.js | 39 ++++++++++++++++++++++ tests/migration-0.7.37.unit.php | 20 +++++++++++ 21 files changed, 175 insertions(+), 45 deletions(-) diff --git a/app/Controllers/PrestitiController.php b/app/Controllers/PrestitiController.php index dc6a4ae74..7676c3aa4 100644 --- a/app/Controllers/PrestitiController.php +++ b/app/Controllers/PrestitiController.php @@ -89,7 +89,7 @@ public function createForm(Request $request, Response $response, mysqli $db): Re $stmt->close(); } if ($presetUser) { - $presetUserName = $presetUser['nome'] . ' ' . $presetUser['cognome'] . ' (' . $presetUser['codice_tessera'] . ')'; + $presetUserName = full_name($presetUser['nome'] ?? '', $presetUser['cognome'] ?? '') . ' (' . $presetUser['codice_tessera'] . ')'; $presetUserLocked = true; } else { $presetUserId = 0; diff --git a/app/Controllers/ProfileController.php b/app/Controllers/ProfileController.php index 957adb1ec..336936cb2 100644 --- a/app/Controllers/ProfileController.php +++ b/app/Controllers/ProfileController.php @@ -176,11 +176,17 @@ public function update(Request $request, Response $response, mysqli $db): Respon } // Custom registration fields (issue #255): validate before writing. + // enforceRequired=false — a field marked obbligatorio AFTER this user + // signed up must not wall them out of unrelated profile edits (they + // never saw it). Format checks still apply to anything they typed. $customDefinitions = \App\Support\RegistrationFields::definitions($db); - $customValidation = \App\Support\RegistrationFields::validate($customDefinitions, $data); + $customValidation = \App\Support\RegistrationFields::validate($customDefinitions, $data, false); if ($customValidation['error'] !== null) { + // Name the actual problem instead of the generic "required fields" + // banner (which is wrong when the user did fill the field). + $customError = $customValidation['error_reason'] === 'format' ? 'custom_field_invalid' : 'required_fields'; $profileUrl = RouteTranslator::route('profile'); - return $response->withHeader('Location', $profileUrl . '?error=required_fields')->withStatus(302); + return $response->withHeader('Location', $profileUrl . '?error=' . $customError)->withStatus(302); } // Update user — only include locale in SQL when the field was posted diff --git a/app/Controllers/RegistrationController.php b/app/Controllers/RegistrationController.php index 559369f31..6750152cd 100644 --- a/app/Controllers/RegistrationController.php +++ b/app/Controllers/RegistrationController.php @@ -86,10 +86,13 @@ public function register(Request $request, Response $response, mysqli $db): Resp } // Admin-defined custom fields (issue #255): validate before any write. + // Required IS enforced at signup (the field's contract); a format error + // gets a distinct code so the user isn't told to "fill" a field they did. $customDefinitions = \App\Support\RegistrationFields::definitions($db); $customValidation = \App\Support\RegistrationFields::validate($customDefinitions, $data); if ($customValidation['error'] !== null) { - return $response->withHeader('Location', RouteTranslator::route('register') . '?error=missing_fields')->withStatus(302); + $customError = $customValidation['error_reason'] === 'format' ? 'custom_field_invalid' : 'missing_fields'; + return $response->withHeader('Location', RouteTranslator::route('register') . '?error=' . $customError)->withStatus(302); } // Validate input lengths diff --git a/app/Controllers/SettingsController.php b/app/Controllers/SettingsController.php index 9650ee148..adecd24fb 100644 --- a/app/Controllers/SettingsController.php +++ b/app/Controllers/SettingsController.php @@ -222,6 +222,14 @@ public function updateRegistrationSettings(Request $request, Response $response, $repository = new SettingsRepository($db); $repository->ensureTables(); + // Persist the custom-field definitions FIRST (own transaction). On + // failure, bail BEFORE writing the toggles — so a field-batch failure + // never leaves the toggles applied under a "save failed" flash. + if (!$this->saveRegistrationCustomFields($db, $data)) { + $_SESSION['error_message'] = __('Salvataggio dei campi personalizzati non riuscito. Riprova.'); + return $this->redirect($response, '/admin/settings?tab=registration'); + } + $requireApproval = isset($data['require_admin_approval']) ? '1' : '0'; $repository->set('registration', 'require_admin_approval', $requireApproval); ConfigStore::set('registration.require_admin_approval', (bool) $requireApproval); @@ -232,12 +240,7 @@ public function updateRegistrationSettings(Request $request, Response $response, ConfigStore::set('registration.' . $toggleKey, (bool) $flag); } - $this->saveRegistrationCustomFields($db, $data); - - // saveRegistrationCustomFields surfaces its own error flash on failure. - if (empty($_SESSION['error_message'])) { - $_SESSION['success_message'] = __('Impostazioni di registrazione aggiornate correttamente.'); - } + $_SESSION['success_message'] = __('Impostazioni di registrazione aggiornate correttamente.'); return $this->redirect($response, '/admin/settings?tab=registration'); } @@ -249,8 +252,10 @@ public function updateRegistrationSettings(Request $request, Response $response, * No-ops gracefully when the tables are not migrated yet. * * @param array $data + * @return bool true when the batch committed (or there was nothing to do); + * false when it rolled back — the caller decides the flash. */ - private function saveRegistrationCustomFields(mysqli $db, array $data): void + private function saveRegistrationCustomFields(mysqli $db, array $data): bool { try { $probe = $db->prepare( @@ -258,7 +263,7 @@ private function saveRegistrationCustomFields(mysqli $db, array $data): void WHERE table_schema = DATABASE() AND table_name = ? LIMIT 1' ); if ($probe === false) { - return; + return true; } $tableName = 'registrazione_campi'; $probe->bind_param('s', $tableName); @@ -266,7 +271,7 @@ private function saveRegistrationCustomFields(mysqli $db, array $data): void $exists = $probe->get_result()->fetch_row() !== null; $probe->close(); if (!$exists) { - return; + return true; } // All row mutations land atomically: a mid-batch failure must not @@ -357,10 +362,9 @@ private function saveRegistrationCustomFields(mysqli $db, array $data): void } } catch (\Throwable $e) { \App\Support\SecureLogger::error('[Settings] custom registration fields save failed', ['error' => $e->getMessage()]); - // Surface the failure: the settings page must not claim success - // when the field definitions were rolled back. - $_SESSION['error_message'] = __('Salvataggio dei campi personalizzati non riuscito. Riprova.'); + return false; } + return true; } public function updateEmailTemplate(Request $request, Response $response, mysqli $db, string $template): Response @@ -427,8 +431,9 @@ private function resolveEmailSettings(SettingsRepository $repository): array ]; $settings = array_merge($defaults, $stored); - // Registration approval flag lives in its own config group but is edited - // from this (email) form, so surface it here for the checkbox to pre-fill. + // Registration approval flag lives in its own config group and is edited + // from the Registration tab (issue #255); surface it here only because + // resolveEmailSettings feeds several tabs' pre-fill. $settings['require_admin_approval'] = (bool) ConfigStore::get('registration.require_admin_approval', true); // Built-in field requirements (issue #255) — defaults keep the historical // all-required behaviour. diff --git a/app/Controllers/UsersController.php b/app/Controllers/UsersController.php index cd9def021..2441c353f 100644 --- a/app/Controllers/UsersController.php +++ b/app/Controllers/UsersController.php @@ -596,7 +596,7 @@ public function delete(Request $request, Response $response, mysqli $db, int $id if ($userToDelete) { \App\Support\SecureLogger::info('User marked as deleted (has loan history)', [ 'user_id' => $id, - 'user_name' => $userToDelete['nome'] . ' ' . $userToDelete['cognome'], + 'user_name' => full_name($userToDelete['nome'] ?? '', $userToDelete['cognome'] ?? ''), 'user_email' => $userToDelete['email'], 'loan_count' => $loanResult['count'], 'deleted_by' => $_SESSION['user']['id'] ?? 'unknown' @@ -627,7 +627,7 @@ public function delete(Request $request, Response $response, mysqli $db, int $id if ($userToDelete) { \App\Support\SecureLogger::info('User deleted', [ 'deleted_user_id' => $id, - 'deleted_user_name' => $userToDelete['nome'] . ' ' . $userToDelete['cognome'], + 'deleted_user_name' => full_name($userToDelete['nome'] ?? '', $userToDelete['cognome'] ?? ''), 'deleted_user_email' => $userToDelete['email'], 'deleted_user_role' => $userToDelete['tipo_utente'], 'deleted_by' => $_SESSION['user']['id'] ?? 'unknown', diff --git a/app/Support/RegistrationFields.php b/app/Support/RegistrationFields.php index 7cb5552ee..971146c2e 100644 --- a/app/Support/RegistrationFields.php +++ b/app/Support/RegistrationFields.php @@ -84,46 +84,59 @@ public static function definitions(mysqli $db, bool $onlyActive = true): array * * @param list $definitions * @param array $post raw request body (custom_field[] keys) - * @return array{values: array, error: ?string} - * values maps field id => normalised value ('' allowed only for - * optional fields; checkboxes normalise to '1'/'') + * @param bool $enforceRequired when false (profile self-edit), a blank + * required field is accepted rather than rejected — a custom field + * marked obbligatorio AFTER signup must not wall an existing user + * (who never saw it) out of unrelated profile edits. Format checks + * still apply to any value the user actually typed. Registration + * passes true so the signup contract stays enforced. + * @return array{values: array, error: ?string, error_reason: ?string} + * values maps field id => normalised value ('' allowed for optional + * — or, when $enforceRequired is false, any — fields; checkboxes + * normalise to '1'/''). error is the offending field's label (null + * when valid); error_reason is 'missing' or 'format' so callers can + * show a message that names the actual problem. */ - public static function validate(array $definitions, array $post): array + public static function validate(array $definitions, array $post, bool $enforceRequired = true): array { $submitted = $post['custom_field'] ?? []; if (!is_array($submitted)) { $submitted = []; } + $fail = static fn (string $label, string $reason): array => + ['values' => [], 'error' => $label, 'error_reason' => $reason]; + $values = []; foreach ($definitions as $def) { + $required = $def['obbligatorio'] && $enforceRequired; // Reject non-scalar payloads (custom_field[id][]=x) BEFORE casting: // (string) on an array would coerce to "Array", passing required // checks and persisting garbage / ticking checkboxes. $submittedValue = $submitted[$def['id']] ?? null; if ($submittedValue !== null && !is_scalar($submittedValue)) { - return ['values' => [], 'error' => $def['etichetta']]; + return $fail($def['etichetta'], 'format'); } $raw = trim((string) ($submittedValue ?? '')); if ($def['tipo'] === 'checkbox') { $values[$def['id']] = $raw !== '' ? '1' : ''; - if ($def['obbligatorio'] && $values[$def['id']] === '') { - return ['values' => [], 'error' => $def['etichetta']]; + if ($required && $values[$def['id']] === '') { + return $fail($def['etichetta'], 'missing'); } continue; } if ($raw === '') { - if ($def['obbligatorio']) { - return ['values' => [], 'error' => $def['etichetta']]; + if ($required) { + return $fail($def['etichetta'], 'missing'); } $values[$def['id']] = ''; continue; } if (mb_strlen($raw) > self::MAX_VALUE_LENGTH) { - return ['values' => [], 'error' => $def['etichetta']]; + return $fail($def['etichetta'], 'format'); } $ok = match ($def['tipo']) { @@ -134,12 +147,12 @@ public static function validate(array $definitions, array $post): array default => true, // text / textarea: length-capped free text }; if (!$ok) { - return ['values' => [], 'error' => $def['etichetta']]; + return $fail($def['etichetta'], 'format'); } $values[$def['id']] = $raw; } - return ['values' => $values, 'error' => null]; + return ['values' => $values, 'error' => null, 'error_reason' => null]; } /** diff --git a/app/Views/admin/stats.php b/app/Views/admin/stats.php index 4a9ff3b39..4d233b34b 100644 --- a/app/Views/admin/stats.php +++ b/app/Views/admin/stats.php @@ -248,7 +248,7 @@ class="w-10 h-14 object-cover rounded shadow-sm" diff --git a/app/Views/auth/register.php b/app/Views/auth/register.php index c6e9ec97f..fd9ce9643 100644 --- a/app/Views/auth/register.php +++ b/app/Views/auth/register.php @@ -65,6 +65,8 @@ class="h-20 w-auto object-contain"> + + diff --git a/app/Views/profile/index.php b/app/Views/profile/index.php index b3ecf9c1c..75b933031 100644 --- a/app/Views/profile/index.php +++ b/app/Views/profile/index.php @@ -322,6 +322,7 @@ 'csrf' => __('Errore di sicurezza. Riprova.'), 'required' => __('Nome e cognome sono obbligatori.'), 'required_fields' => __('Compila tutti i campi obbligatori.'), + 'custom_field_invalid' => __('Controlla i campi personalizzati: un valore inserito non è valido.'), 'password_mismatch' => __('Le password non coincidono.'), 'password_too_short' => __('La password deve essere lunga almeno 8 caratteri.'), 'password_too_long' => __('La password non può superare i 72 caratteri.'), diff --git a/app/Views/settings/index.php b/app/Views/settings/index.php index 983180768..8770da8c4 100644 --- a/app/Views/settings/index.php +++ b/app/Views/settings/index.php @@ -410,6 +410,21 @@ class="flex-1 min-w-[10rem] px-3 py-2 rounded-lg border border-gray-300 bg-white +
diff --git a/app/Views/settings/messages-tab.php b/app/Views/settings/messages-tab.php index 69445fe4f..d6a9cdeb8 100644 --- a/app/Views/settings/messages-tab.php +++ b/app/Views/settings/messages-tab.php @@ -52,7 +52,7 @@
- + diff --git a/app/Views/utenti/crea_utente.php b/app/Views/utenti/crea_utente.php index 5f80c9a2e..40645ed44 100644 --- a/app/Views/utenti/crea_utente.php +++ b/app/Views/utenti/crea_utente.php @@ -192,11 +192,11 @@ function applyRoleState() { const isAdmin = roleField.value === 'admin'; if (phoneField) { phoneField.required = !isAdmin && REQUIRE_TELEFONO; - if (addressField) { - addressField.required = !isAdmin && REQUIRE_INDIRIZZO; - } phoneField.placeholder = isAdmin ? __('Opzionale per amministratori') : __('+39 123 456 7890'); } + if (addressField) { + addressField.required = !isAdmin && REQUIRE_INDIRIZZO; + } adminBlocks.forEach((section) => { section.style.display = isAdmin ? 'none' : (section.dataset.originalDisplay || ''); diff --git a/app/Views/utenti/index.php b/app/Views/utenti/index.php index 9010a5f2d..4812590d8 100644 --- a/app/Views/utenti/index.php +++ b/app/Views/utenti/index.php @@ -80,7 +80,7 @@

- +

diff --git a/app/Views/utenti/modifica_utente.php b/app/Views/utenti/modifica_utente.php index 149f1bd65..40503cbb5 100644 --- a/app/Views/utenti/modifica_utente.php +++ b/app/Views/utenti/modifica_utente.php @@ -201,11 +201,11 @@ function applyRoleState() { const isAdmin = roleField.value === 'admin'; if (phoneField) { phoneField.required = !isAdmin && REQUIRE_TELEFONO; - if (addressField) { - addressField.required = !isAdmin && REQUIRE_INDIRIZZO; - } phoneField.placeholder = isAdmin ? __('Opzionale per amministratori') : '+39 123 456 7890'; } + if (addressField) { + addressField.required = !isAdmin && REQUIRE_INDIRIZZO; + } adminBlocks.forEach((section) => { section.style.display = isAdmin ? 'none' : (section.dataset.originalDisplay || ''); diff --git a/app/helpers.php b/app/helpers.php index 47fc38b0d..bdf945384 100644 --- a/app/helpers.php +++ b/app/helpers.php @@ -326,6 +326,24 @@ function translate_loan_status(string $status): string } } +if (!function_exists('full_name')) { + /** + * Join a member's given name and surname into a display name. + * + * Since issue #255 the surname can be an empty string (admin made it + * optional at registration). A naive "$nome . ' ' . $cognome" then leaves + * a trailing space; this collapses to just the given name in that case. + * + * @param string|null $nome + * @param string|null $cognome + * @return string + */ + function full_name(?string $nome, ?string $cognome): string + { + return trim(trim((string) $nome) . ' ' . trim((string) $cognome)); + } +} + if (!function_exists('translate_book_status')) { /** * Translate a book availability status code to its localized label. diff --git a/locale/de_DE.json b/locale/de_DE.json index 1da9e9cfe..9fabcf308 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -6537,5 +6537,7 @@ "Salva impostazioni registrazione": "Registrierungseinstellungen speichern", "Impostazioni di registrazione aggiornate correttamente.": "Registrierungseinstellungen erfolgreich aktualisiert.", "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Speichern der benutzerdefinierten Felder fehlgeschlagen. Bitte erneut versuchen.", - "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Feldänderungen werden zusammen mit den Registrierungseinstellungen gespeichert." + "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Feldänderungen werden zusammen mit den Registrierungseinstellungen gespeichert.", + "Controlla i campi personalizzati: un valore inserito non è valido.": "Überprüfe die benutzerdefinierten Felder: einer der eingegebenen Werte ist ungültig.", + "Eliminare i campi personalizzati selezionati? I valori salvati dagli utenti per questi campi verranno rimossi definitivamente.": "Ausgewählte benutzerdefinierte Felder löschen? Die von Benutzern gespeicherten Werte für diese Felder werden dauerhaft entfernt." } diff --git a/locale/en_US.json b/locale/en_US.json index 2a8f16e9b..91c1a03ff 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -6537,5 +6537,7 @@ "Salva impostazioni registrazione": "Save registration settings", "Impostazioni di registrazione aggiornate correttamente.": "Registration settings updated successfully.", "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Saving the custom fields failed. Please try again.", - "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Field changes are saved together with the registration settings." + "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Field changes are saved together with the registration settings.", + "Controlla i campi personalizzati: un valore inserito non è valido.": "Check the custom fields: one of the values entered is not valid.", + "Eliminare i campi personalizzati selezionati? I valori salvati dagli utenti per questi campi verranno rimossi definitivamente.": "Delete the selected custom fields? The values users saved for these fields will be permanently removed." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index e3e839ea3..8a8b9e227 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -6537,5 +6537,7 @@ "Salva impostazioni registrazione": "Enregistrer les paramètres d'inscription", "Impostazioni di registrazione aggiornate correttamente.": "Paramètres d'inscription mis à jour avec succès.", "Salvataggio dei campi personalizzati non riuscito. Riprova.": "L'enregistrement des champs personnalisés a échoué. Veuillez réessayer.", - "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Les modifications des champs sont enregistrées avec les paramètres d'inscription." + "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Les modifications des champs sont enregistrées avec les paramètres d'inscription.", + "Controlla i campi personalizzati: un valore inserito non è valido.": "Vérifiez les champs personnalisés : une des valeurs saisies n'est pas valide.", + "Eliminare i campi personalizzati selezionati? I valori salvati dagli utenti per questi campi verranno rimossi definitivamente.": "Supprimer les champs personnalisés sélectionnés ? Les valeurs enregistrées par les utilisateurs pour ces champs seront définitivement supprimées." } diff --git a/locale/it_IT.json b/locale/it_IT.json index 781da155f..ff9c54636 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -6537,5 +6537,7 @@ "Salva impostazioni registrazione": "Salva impostazioni registrazione", "Impostazioni di registrazione aggiornate correttamente.": "Impostazioni di registrazione aggiornate correttamente.", "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Salvataggio dei campi personalizzati non riuscito. Riprova.", - "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione." + "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.", + "Controlla i campi personalizzati: un valore inserito non è valido.": "Controlla i campi personalizzati: un valore inserito non è valido.", + "Eliminare i campi personalizzati selezionati? I valori salvati dagli utenti per questi campi verranno rimossi definitivamente.": "Eliminare i campi personalizzati selezionati? I valori salvati dagli utenti per questi campi verranno rimossi definitivamente." } diff --git a/tests/issue-255-registration.spec.js b/tests/issue-255-registration.spec.js index 9e1ce6542..e02427bdd 100644 --- a/tests/issue-255-registration.spec.js +++ b/tests/issue-255-registration.spec.js @@ -367,4 +367,43 @@ test.describe.serial('Issue #255 — configurable registration fields (25 checks dbQuery(`UPDATE registrazione_campi SET attivo=1 WHERE id=${id}`); } }); + + // ── review-round fixes (adamsreview) ─────────────────────────────────────── + + test('26. a format-invalid custom value redirects to the distinct error code (not missing_fields)', async ({ page }) => { + setToggles(false, false, false); + const email = `zz-255-fmt-${TOKEN}@example.test`; + const id = fieldId(LBL('email')); + await fillRegistration(page, { + fields: { nome: 'Fmt255', email, password: 'Password255!ok', password_confirm: 'Password255!ok' }, + }); + // Force a bad email value into the (email-typed) custom control via a text + // control so the browser doesn't drop it, then submit past client validation. + await page.evaluate((fieldName) => { + const el = document.querySelector(`[name="${fieldName}"]`); + if (el instanceof HTMLInputElement) { el.type = 'text'; el.value = 'not-an-email'; } + }, `custom_field[${id}]`); + await submitNoValidate(page); + expect(page.url()).toContain('error=custom_field_invalid'); + expect(userCount(email)).toBe(0); + }); + + test('27. a surname-less member renders with no trailing space in the admin user list', async ({ page }) => { + setToggles(false, false, false); + const email = `zz-255-noln-${TOKEN}@example.test`; + await fillRegistration(page, { + fields: { nome: 'SoloNome27', email, password: 'Password255!ok', password_confirm: 'Password255!ok' }, + }); + await submitNoValidate(page); + expect(userCount(email)).toBe(1); + // Stored surname is '' (optional); the admin list must not show "SoloNome27 ". + await admin.goto(`${BASE}/admin/users`); + const cell = admin.locator('td', { hasText: 'SoloNome27' }).first(); + await expect(cell).toBeVisible(); + const text = (await cell.innerText()).trim(); + // innerText of the cell may include the email on the next line; assert the + // name line itself has no trailing space before a newline or end. + expect(text).not.toMatch(/SoloNome27 (?:\n|$)/); + expect(text).toContain('SoloNome27'); + }); }); diff --git a/tests/migration-0.7.37.unit.php b/tests/migration-0.7.37.unit.php index 7d19a66ae..98e44328e 100644 --- a/tests/migration-0.7.37.unit.php +++ b/tests/migration-0.7.37.unit.php @@ -67,6 +67,26 @@ $r = RegistrationFields::validate($defs, ['custom_field' => [1 => str_repeat('a', 1001)]]); $check($r['error'] !== null, 'over-long value rejected'); +// error_reason distinguishes 'missing' from 'format' (issue #255 review fix #3) +$r = RegistrationFields::validate($defs, ['custom_field' => [2 => 'https://x.y']]); +$check($r['error_reason'] === 'missing', 'missing required reports reason=missing'); +$r = RegistrationFields::validate($defs, ['custom_field' => [1 => 'x', 5 => 'not-an-email']]); +$check($r['error_reason'] === 'format', 'invalid value reports reason=format'); + +// enforceRequired=false (profile self-edit): a blank required field is +// tolerated so a field made required AFTER signup can't wall an existing user +// out of unrelated edits — but format checks still apply (review fix #2). +$r = RegistrationFields::validate($defs, ['custom_field' => [1 => '', 2 => '']], false); +$check($r['error'] === null, 'enforceRequired=false tolerates a blank required field'); +$r = RegistrationFields::validate($defs, ['custom_field' => [1 => 'x', 5 => 'bad-email']], false); +$check($r['error_reason'] === 'format', 'enforceRequired=false still rejects a bad format'); + +// full_name() collapses the trailing space for a surname-less member (fix #1) +require_once dirname(__DIR__) . '/app/helpers.php'; +$check(full_name('Mario', '') === 'Mario', "full_name('Mario','') === 'Mario' (no trailing space)"); +$check(full_name('Mario', 'Rossi') === 'Mario Rossi', "full_name joins name + surname"); +$check(full_name(null, null) === '', 'full_name(null,null) === ""'); + // ── DB sections ────────────────────────────────────────────────────────────── echo "A. Real migration against sandbox tables\n"; From e6f7fcc7b75db5479ba5ed49adc63a713340a5ca Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 16 Jul 2026 19:30:44 +0200 Subject: [PATCH 6/7] fix(#255): CI DB connection in issue-255 spec + htmlspecialchars in touched views MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - issue-255-registration.spec.js dbQuery now prefers TCP host/port (CI's MySQL listens on 127.0.0.1, not the default socket) and falls back to the socket for local dev — mirrors schema-integrity.spec.js. The required CI step was red with "Can't connect through socket /var/run/mysqld/mysqld.sock". - The three display sites I touched for the full_name() fix now escape with htmlspecialchars(..., ENT_QUOTES, 'UTF-8') instead of HtmlHelper::e(), per the view-escaping rule. --- app/Views/admin/stats.php | 2 +- app/Views/settings/messages-tab.php | 2 +- app/Views/utenti/index.php | 2 +- tests/issue-255-registration.spec.js | 16 +++++++++++++--- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/app/Views/admin/stats.php b/app/Views/admin/stats.php index 4d233b34b..b4c02cd81 100644 --- a/app/Views/admin/stats.php +++ b/app/Views/admin/stats.php @@ -248,7 +248,7 @@ class="w-10 h-14 object-cover rounded shadow-sm"

diff --git a/app/Views/settings/messages-tab.php b/app/Views/settings/messages-tab.php index d6a9cdeb8..b8e8a62ca 100644 --- a/app/Views/settings/messages-tab.php +++ b/app/Views/settings/messages-tab.php @@ -52,7 +52,7 @@
- + diff --git a/app/Views/utenti/index.php b/app/Views/utenti/index.php index 4812590d8..337deefc2 100644 --- a/app/Views/utenti/index.php +++ b/app/Views/utenti/index.php @@ -80,7 +80,7 @@

- +

diff --git a/tests/issue-255-registration.spec.js b/tests/issue-255-registration.spec.js index e02427bdd..6bcb86138 100644 --- a/tests/issue-255-registration.spec.js +++ b/tests/issue-255-registration.spec.js @@ -29,15 +29,25 @@ const ADMIN_PASS = process.env.E2E_ADMIN_PASS || ''; const DB_USER = process.env.E2E_DB_USER || ''; const DB_PASS = process.env.E2E_DB_PASS || ''; const DB_SOCKET = process.env.E2E_DB_SOCKET || ''; +const DB_HOST = process.env.E2E_DB_HOST || ''; +const DB_PORT = process.env.E2E_DB_PORT || ''; const DB_NAME = process.env.E2E_DB_NAME || ''; test.skip(!ADMIN_EMAIL || !ADMIN_PASS || !DB_USER || !DB_NAME, 'E2E credentials not configured (set E2E_ADMIN_EMAIL, E2E_ADMIN_PASS, E2E_DB_*)'); function dbQuery(sql) { - const args = ['-u', DB_USER]; - if (DB_SOCKET) args.push('--socket=' + DB_SOCKET); - args.push(DB_NAME, '-N', '-B', '-e', sql); + // Prefer TCP host/port (CI's MySQL service listens on 127.0.0.1, not the + // default socket); fall back to the socket (local dev). Mirrors the working + // connection logic in schema-integrity.spec.js. + const args = []; + if (DB_HOST) { + args.push('-h', DB_HOST); + if (DB_PORT) args.push('-P', DB_PORT); + } else if (DB_SOCKET) { + args.push('-S', DB_SOCKET); + } + args.push('-u', DB_USER, DB_NAME, '-N', '-B', '-e', sql); return execFileSync('mysql', args, { encoding: 'utf-8', timeout: 10000, env: { ...process.env, MYSQL_PWD: DB_PASS }, From 916a11c0133254af53c0ecca8870e8848cced3b2 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 16 Jul 2026 20:56:44 +0200 Subject: [PATCH 7/7] feat(#255): expose configurable registration fields through the Mobile API + hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends the feature to the companion-app surface and hardens the admin editor. Mobile API (mobile-api 1.2.2 → 1.3.0, requires_app 0.7.37): - GET /api/v1/health advertises the built-in requirements (require_cognome/ telefono/indirizzo) and the active custom-field definitions, so the app can render the right registration form. - POST /api/v1/auth/register reaches web parity: built-in requirements follow the toggles, telefono/indirizzo are omitted (NULL) when optional and blank, custom fields are validated and saved in the same transaction as the account (1062 → email_exists), notifications run AFTER commit so a mail failure can't 500 a created account, and requires_approval reflects the real setting. - GET/PATCH /api/v1/me expose and update custom_fields with PATCH semantics (omitted ids preserved, an explicit empty value clears one field); also surface email_verificata + stato. OpenAPI schemas updated (incl. the tipo_utente enum fix: standard/premium/staff/admin). Web hardening: - Changing a custom field's TYPE is refused when it already has stored values (RegistrationFields::hasStoredValues + a DomainException whose message reaches the admin), preventing silent data corruption (e.g. text→number). - Profile custom-field validation runs only when the section was actually submitted (custom_fields_present marker), so a partial profile POST never wipes or spuriously requires custom values. - User detail page shows the stored custom values read-only. New RegistrationFields helpers reused across both surfaces: apiDefinitions(), editableFieldsForUser(), hasStoredValues(). Tests: new tests/registration-fields-255.unit.php (26 DB-backed checks over the API helpers + the validate() enforceRequired matrix + save/read round-trip); issue-255 E2E grows to 29 (no-block existing user, destructive type-change guard); mobile-api E2E adds register-with-custom-fields + PATCH /me cases. i18n parity across all four locales. PHPStan L5 = 0, unit 39/39, issue-255 29/29, mobile-api 76/76 + idempotency 41/41. --- README.md | 4 +- app/Controllers/ProfileController.php | 26 ++- app/Controllers/SettingsController.php | 19 ++- app/Controllers/UsersController.php | 2 + app/Support/RegistrationFields.php | 61 +++++++ app/Views/profile/index.php | 13 +- app/Views/utenti/dettagli_utente.php | 8 + locale/de_DE.json | 1 + locale/en_US.json | 1 + locale/fr_FR.json | 3 +- locale/it_IT.json | 1 + .../plugins/mobile-api/MobileApiPlugin.php | 2 +- storage/plugins/mobile-api/plugin.json | 4 +- .../src/Controllers/ActionsController.php | 47 +++++- .../src/Controllers/AuthController.php | 107 +++++++++--- .../src/Controllers/HealthController.php | 12 ++ .../src/Controllers/OpenApiController.php | 86 +++++++++- tests/issue-255-registration.spec.js | 34 +++- tests/mobile-api-idempotency.spec.js | 9 +- tests/mobile-api.spec.js | 133 ++++++++++++++- tests/registration-fields-255.unit.php | 159 ++++++++++++++++++ 21 files changed, 667 insertions(+), 65 deletions(-) create mode 100644 tests/registration-fields-255.unit.php diff --git a/README.md b/README.md index 29d1220bd..d4f4dfd69 100644 --- a/README.md +++ b/README.md @@ -48,7 +48,9 @@ Pinakes is a self-hosted, full-featured ILS for schools, municipalities, and pri fields (text, textarea, email, URL, number, checkbox — required or optional): they appear on the registration form and in the user profile, and are shown on the admin user detail. Ideal for community handles such as a Telegram - username. New migration `migrate_0.7.37.sql` adds the two supporting tables + username. The bundled Mobile API exposes the requirements and definitions in + `/api/v1/health`, accepts the values during app registration/profile editing, + and includes them in `/api/v1/me`. New migration `migrate_0.7.37.sql` adds the two supporting tables (`registrazione_campi`, `utenti_campi_valori`); it is idempotent and runs automatically on upgrade. diff --git a/app/Controllers/ProfileController.php b/app/Controllers/ProfileController.php index 336936cb2..177853c9c 100644 --- a/app/Controllers/ProfileController.php +++ b/app/Controllers/ProfileController.php @@ -179,14 +179,20 @@ public function update(Request $request, Response $response, mysqli $db): Respon // enforceRequired=false — a field marked obbligatorio AFTER this user // signed up must not wall them out of unrelated profile edits (they // never saw it). Format checks still apply to anything they typed. - $customDefinitions = \App\Support\RegistrationFields::definitions($db); - $customValidation = \App\Support\RegistrationFields::validate($customDefinitions, $data, false); - if ($customValidation['error'] !== null) { - // Name the actual problem instead of the generic "required fields" - // banner (which is wrong when the user did fill the field). - $customError = $customValidation['error_reason'] === 'format' ? 'custom_field_invalid' : 'required_fields'; - $profileUrl = RouteTranslator::route('profile'); - return $response->withHeader('Location', $profileUrl . '?error=' . $customError)->withStatus(302); + $customValues = null; + $customFieldsSubmitted = array_key_exists('custom_fields_present', $data) + || array_key_exists('custom_field', $data); + if ($customFieldsSubmitted) { + $customDefinitions = \App\Support\RegistrationFields::definitions($db); + $customValidation = \App\Support\RegistrationFields::validate($customDefinitions, $data, false); + if ($customValidation['error'] !== null) { + // Name the actual problem instead of the generic "required fields" + // banner (which is wrong when the user did fill the field). + $customError = $customValidation['error_reason'] === 'format' ? 'custom_field_invalid' : 'required_fields'; + $profileUrl = RouteTranslator::route('profile'); + return $response->withHeader('Location', $profileUrl . '?error=' . $customError)->withStatus(302); + } + $customValues = $customValidation['values']; } // Update user — only include locale in SQL when the field was posted @@ -217,7 +223,9 @@ public function update(Request $request, Response $response, mysqli $db): Respon $db->begin_transaction(); $updated = $stmt->execute(); if ($updated) { - \App\Support\RegistrationFields::saveValues($db, $uid, $customValidation['values']); + if ($customValues !== null) { + \App\Support\RegistrationFields::saveValues($db, $uid, $customValues); + } $db->commit(); } else { $db->rollback(); diff --git a/app/Controllers/SettingsController.php b/app/Controllers/SettingsController.php index adecd24fb..85e45b770 100644 --- a/app/Controllers/SettingsController.php +++ b/app/Controllers/SettingsController.php @@ -226,7 +226,9 @@ public function updateRegistrationSettings(Request $request, Response $response, // failure, bail BEFORE writing the toggles — so a field-batch failure // never leaves the toggles applied under a "save failed" flash. if (!$this->saveRegistrationCustomFields($db, $data)) { - $_SESSION['error_message'] = __('Salvataggio dei campi personalizzati non riuscito. Riprova.'); + if (empty($_SESSION['error_message'])) { + $_SESSION['error_message'] = __('Salvataggio dei campi personalizzati non riuscito. Riprova.'); + } return $this->redirect($response, '/admin/settings?tab=registration'); } @@ -280,6 +282,10 @@ private function saveRegistrationCustomFields(mysqli $db, array $data): bool try { $rows = $data['custom_fields'] ?? []; if (is_array($rows)) { + $currentTypes = []; + foreach (\App\Support\RegistrationFields::definitions($db, false) as $definition) { + $currentTypes[$definition['id']] = $definition['tipo']; + } $update = $db->prepare( 'UPDATE registrazione_campi SET etichetta = ?, tipo = ?, obbligatorio = ?, attivo = ? WHERE id = ?' ); @@ -314,6 +320,14 @@ private function saveRegistrationCustomFields(mysqli $db, array $data): bool ) { continue; } + $currentType = $currentTypes[$id] ?? null; + if ($currentType !== null && $currentType !== $tipo + && \App\Support\RegistrationFields::hasStoredValues($db, $id) + ) { + throw new \DomainException( + __('Non puoi cambiare il tipo di un campo che contiene già valori. Crea un nuovo campo o elimina prima i dati esistenti.') + ); + } $required = !empty($row['obbligatorio']) ? 1 : 0; $active = !empty($row['attivo']) ? 1 : 0; $update->bind_param('ssiii', $label, $tipo, $required, $active, $id); @@ -362,6 +376,9 @@ private function saveRegistrationCustomFields(mysqli $db, array $data): bool } } catch (\Throwable $e) { \App\Support\SecureLogger::error('[Settings] custom registration fields save failed', ['error' => $e->getMessage()]); + if ($e instanceof \DomainException) { + $_SESSION['error_message'] = $e->getMessage(); + } return false; } return true; diff --git a/app/Controllers/UsersController.php b/app/Controllers/UsersController.php index 2441c353f..0390a9058 100644 --- a/app/Controllers/UsersController.php +++ b/app/Controllers/UsersController.php @@ -654,6 +654,8 @@ public function details(Request $request, Response $response, mysqli $db, int $i $utente = $result->fetch_assoc(); $stmt->close(); + $customFieldValues = \App\Support\RegistrationFields::labelledValuesForUser($db, $id); + // Fetch user's loan history $loanStmt = $db->prepare(" SELECT p.id, p.data_prestito, p.data_scadenza, p.data_restituzione, p.stato, l.titolo as libro_titolo diff --git a/app/Support/RegistrationFields.php b/app/Support/RegistrationFields.php index 971146c2e..d164c3675 100644 --- a/app/Support/RegistrationFields.php +++ b/app/Support/RegistrationFields.php @@ -245,6 +245,67 @@ public static function labelledValuesForUser(mysqli $db, int $userId): array return $out; } + /** + * Active definitions exposed to public clients (registration discovery). + * + * @return list + */ + public static function apiDefinitions(mysqli $db): array + { + $out = []; + foreach (self::definitions($db) as $def) { + $out[] = [ + 'id' => $def['id'], + 'label' => $def['etichetta'], + 'type' => $def['tipo'], + 'required' => $def['obbligatorio'], + ]; + } + return $out; + } + + /** + * Active editable fields plus this user's current values (mobile/profile API). + * + * @return list + */ + public static function editableFieldsForUser(mysqli $db, int $userId): array + { + $values = self::valuesForUser($db, $userId); + $out = []; + foreach (self::definitions($db) as $def) { + $out[] = [ + 'id' => $def['id'], + 'label' => $def['etichetta'], + 'type' => $def['tipo'], + 'required' => $def['obbligatorio'], + 'value' => $values[$def['id']] ?? '', + ]; + } + return $out; + } + + /** + * Whether changing/deleting a definition would affect stored user data. + */ + public static function hasStoredValues(mysqli $db, int $fieldId): bool + { + if ($fieldId <= 0 || !self::tableExists($db, 'utenti_campi_valori')) { + return false; + } + $stmt = $db->prepare( + 'SELECT 1 FROM utenti_campi_valori WHERE campo_id = ? LIMIT 1' + ); + if ($stmt === false) { + throw new \RuntimeException('Unable to inspect stored custom field values'); + } + $stmt->bind_param('i', $fieldId); + $stmt->execute(); + $exists = $stmt->get_result()->fetch_row() !== null; + $stmt->close(); + return $exists; + } + private static function tableExists(mysqli $db, string $table): bool { try { diff --git a/app/Views/profile/index.php b/app/Views/profile/index.php index 75b933031..e99dd7b58 100644 --- a/app/Views/profile/index.php +++ b/app/Views/profile/index.php @@ -451,6 +451,9 @@ value="">

+ + + - + - + - + diff --git a/app/Views/utenti/dettagli_utente.php b/app/Views/utenti/dettagli_utente.php index 3970e86d5..b6e061da4 100644 --- a/app/Views/utenti/dettagli_utente.php +++ b/app/Views/utenti/dettagli_utente.php @@ -176,6 +176,14 @@ function getLoanStatusBadge($status) {
+ +
+
+
+ +
+
+ diff --git a/locale/de_DE.json b/locale/de_DE.json index 9fabcf308..0d826c08a 100644 --- a/locale/de_DE.json +++ b/locale/de_DE.json @@ -6538,6 +6538,7 @@ "Impostazioni di registrazione aggiornate correttamente.": "Registrierungseinstellungen erfolgreich aktualisiert.", "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Speichern der benutzerdefinierten Felder fehlgeschlagen. Bitte erneut versuchen.", "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Feldänderungen werden zusammen mit den Registrierungseinstellungen gespeichert.", + "Non puoi cambiare il tipo di un campo che contiene già valori. Crea un nuovo campo o elimina prima i dati esistenti.": "Der Typ eines Feldes mit bereits gespeicherten Werten kann nicht geändert werden. Erstelle ein neues Feld oder lösche zuerst die vorhandenen Daten.", "Controlla i campi personalizzati: un valore inserito non è valido.": "Überprüfe die benutzerdefinierten Felder: einer der eingegebenen Werte ist ungültig.", "Eliminare i campi personalizzati selezionati? I valori salvati dagli utenti per questi campi verranno rimossi definitivamente.": "Ausgewählte benutzerdefinierte Felder löschen? Die von Benutzern gespeicherten Werte für diese Felder werden dauerhaft entfernt." } diff --git a/locale/en_US.json b/locale/en_US.json index 91c1a03ff..1179b4f4b 100644 --- a/locale/en_US.json +++ b/locale/en_US.json @@ -6538,6 +6538,7 @@ "Impostazioni di registrazione aggiornate correttamente.": "Registration settings updated successfully.", "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Saving the custom fields failed. Please try again.", "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Field changes are saved together with the registration settings.", + "Non puoi cambiare il tipo di un campo che contiene già valori. Crea un nuovo campo o elimina prima i dati esistenti.": "You cannot change the type of a field that already contains values. Create a new field or delete the existing data first.", "Controlla i campi personalizzati: un valore inserito non è valido.": "Check the custom fields: one of the values entered is not valid.", "Eliminare i campi personalizzati selezionati? I valori salvati dagli utenti per questi campi verranno rimossi definitivamente.": "Delete the selected custom fields? The values users saved for these fields will be permanently removed." } diff --git a/locale/fr_FR.json b/locale/fr_FR.json index 8a8b9e227..1641cc4c2 100644 --- a/locale/fr_FR.json +++ b/locale/fr_FR.json @@ -6537,7 +6537,8 @@ "Salva impostazioni registrazione": "Enregistrer les paramètres d'inscription", "Impostazioni di registrazione aggiornate correttamente.": "Paramètres d'inscription mis à jour avec succès.", "Salvataggio dei campi personalizzati non riuscito. Riprova.": "L'enregistrement des champs personnalisés a échoué. Veuillez réessayer.", - "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Les modifications des champs sont enregistrées avec les paramètres d'inscription.", + "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Les modifications des champs sont enregistrées avec les paramètres d’inscription.", + "Non puoi cambiare il tipo di un campo che contiene già valori. Crea un nuovo campo o elimina prima i dati esistenti.": "Vous ne pouvez pas modifier le type d’un champ qui contient déjà des valeurs. Créez un nouveau champ ou supprimez d’abord les données existantes.", "Controlla i campi personalizzati: un valore inserito non è valido.": "Vérifiez les champs personnalisés : une des valeurs saisies n'est pas valide.", "Eliminare i campi personalizzati selezionati? I valori salvati dagli utenti per questi campi verranno rimossi definitivamente.": "Supprimer les champs personnalisés sélectionnés ? Les valeurs enregistrées par les utilisateurs pour ces champs seront définitivement supprimées." } diff --git a/locale/it_IT.json b/locale/it_IT.json index ff9c54636..1025af92b 100644 --- a/locale/it_IT.json +++ b/locale/it_IT.json @@ -6538,6 +6538,7 @@ "Impostazioni di registrazione aggiornate correttamente.": "Impostazioni di registrazione aggiornate correttamente.", "Salvataggio dei campi personalizzati non riuscito. Riprova.": "Salvataggio dei campi personalizzati non riuscito. Riprova.", "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.": "Le modifiche ai campi vengono salvate insieme alle impostazioni di registrazione.", + "Non puoi cambiare il tipo di un campo che contiene già valori. Crea un nuovo campo o elimina prima i dati esistenti.": "Non puoi cambiare il tipo di un campo che contiene già valori. Crea un nuovo campo o elimina prima i dati esistenti.", "Controlla i campi personalizzati: un valore inserito non è valido.": "Controlla i campi personalizzati: un valore inserito non è valido.", "Eliminare i campi personalizzati selezionati? I valori salvati dagli utenti per questi campi verranno rimossi definitivamente.": "Eliminare i campi personalizzati selezionati? I valori salvati dagli utenti per questi campi verranno rimossi definitivamente." } diff --git a/storage/plugins/mobile-api/MobileApiPlugin.php b/storage/plugins/mobile-api/MobileApiPlugin.php index 6d7d267ec..588faf89e 100644 --- a/storage/plugins/mobile-api/MobileApiPlugin.php +++ b/storage/plugins/mobile-api/MobileApiPlugin.php @@ -672,7 +672,7 @@ public function healthAction( SecureLogger::warning('[MobileApi] VAPID key access in health failed: ' . $e->getMessage()); } - return (new HealthController())->index( + return (new HealthController($this->db))->index( $request, $response, $this->isAppAccessEnabled(), diff --git a/storage/plugins/mobile-api/plugin.json b/storage/plugins/mobile-api/plugin.json index 23596dfe6..bbe31f5e8 100644 --- a/storage/plugins/mobile-api/plugin.json +++ b/storage/plugins/mobile-api/plugin.json @@ -2,12 +2,12 @@ "name": "mobile-api", "display_name": "Mobile API", "description": "API REST/JSON versionata (/api/v1) per l'app companion mobile di Pinakes: discovery, autenticazione a token per dispositivo, ricerca catalogo, prestiti/prenotazioni, wishlist, profilo, messaggi e notifiche push. Disattivata per default finché non viene abilitata.", - "version": "1.2.2", + "version": "1.3.0", "author": "Fabiodalez", "author_url": "", "plugin_url": "", "requires_php": "8.1", - "requires_app": "0.7.3", + "requires_app": "0.7.37", "max_app_version": "0.7.99", "path": "mobile-api", "main_file": "wrapper.php", diff --git a/storage/plugins/mobile-api/src/Controllers/ActionsController.php b/storage/plugins/mobile-api/src/Controllers/ActionsController.php index 0793347c2..ee2610f97 100644 --- a/storage/plugins/mobile-api/src/Controllers/ActionsController.php +++ b/storage/plugins/mobile-api/src/Controllers/ActionsController.php @@ -465,7 +465,8 @@ public function getProfile(Request $request, ResponseInterface $response): Respo try { $stmt = $this->db->prepare( 'SELECT id, nome, cognome, email, telefono, indirizzo, codice_tessera, tipo_utente, - data_nascita, sesso, cod_fiscale, data_scadenza_tessera, data_ultimo_accesso, locale + data_nascita, sesso, cod_fiscale, data_scadenza_tessera, data_ultimo_accesso, locale, + email_verificata, stato FROM utenti WHERE id = ? LIMIT 1' ); if ($stmt === false) { @@ -496,6 +497,9 @@ public function getProfile(Request $request, ResponseInterface $response): Respo 'card_expires_at' => $this->nullableString($row['data_scadenza_tessera'] ?? null), 'last_access_at' => $this->nullableString($row['data_ultimo_accesso'] ?? null), 'locale' => $this->nullableString($row['locale'] ?? null), + 'email_verificata' => ((int) ($row['email_verificata'] ?? 0)) === 1, + 'stato' => (string) ($row['stato'] ?? ''), + 'custom_fields' => \App\Support\RegistrationFields::editableFieldsForUser($this->db, $userId), ]; return ResponseEnvelope::success($response, $data, [], 200); @@ -543,8 +547,35 @@ public function updateProfile(Request $request, ResponseInterface $response): Re $forward['locale'] = (string) $body['locale']; } - if (trim((string) $forward['nome']) === '' || trim((string) $forward['cognome']) === '') { - return ResponseEnvelope::error($response, 'required_fields', __('Nome e cognome sono obbligatori.'), 422); + if (array_key_exists('custom_fields', $body)) { + if (!is_array($body['custom_fields'])) { + return ResponseEnvelope::error( + $response, + 'custom_field_invalid', + __('Controlla i campi personalizzati: un valore inserito non è valido.'), + 422 + ); + } + // PATCH semantics apply within the custom-field map too: values for + // omitted ids are preserved, while an explicitly-sent empty string + // clears that one field. + $customForward = \App\Support\RegistrationFields::valuesForUser($this->db, $userId); + foreach ($body['custom_fields'] as $fieldId => $value) { + $customForward[(int) $fieldId] = $value; + } + $forward['custom_field'] = $customForward; + $forward['custom_fields_present'] = '1'; + } + + if (trim((string) $forward['nome']) === '' + || (trim((string) $forward['cognome']) === '' + && \App\Support\RegistrationFields::isRequired('cognome')) + || (trim((string) $forward['telefono']) === '' + && \App\Support\RegistrationFields::isRequired('telefono')) + || (trim((string) $forward['indirizzo']) === '' + && \App\Support\RegistrationFields::isRequired('indirizzo')) + ) { + return ResponseEnvelope::error($response, 'required_fields', __('Compila tutti i campi obbligatori.'), 422); } try { @@ -555,7 +586,15 @@ public function updateProfile(Request $request, ResponseInterface $response): Re if (isset($outcome['error'])) { $code = (string) $outcome['error']; if ($code === 'required_fields') { - return ResponseEnvelope::error($response, 'required_fields', __('Nome e cognome sono obbligatori.'), 422); + return ResponseEnvelope::error($response, 'required_fields', __('Compila tutti i campi obbligatori.'), 422); + } + if ($code === 'custom_field_invalid') { + return ResponseEnvelope::error( + $response, + 'custom_field_invalid', + __('Controlla i campi personalizzati: un valore inserito non è valido.'), + 422 + ); } return ResponseEnvelope::error($response, 'update_failed', __('Errore durante l\'aggiornamento del profilo.'), 500); } diff --git a/storage/plugins/mobile-api/src/Controllers/AuthController.php b/storage/plugins/mobile-api/src/Controllers/AuthController.php index d354c8236..cc6b04960 100644 --- a/storage/plugins/mobile-api/src/Controllers/AuthController.php +++ b/storage/plugins/mobile-api/src/Controllers/AuthController.php @@ -13,6 +13,7 @@ use App\Support\Mailer; use App\Support\NotificationService; use App\Support\RateLimiter; +use App\Support\RegistrationFields; use App\Support\RouteTranslator; use App\Support\SecureLogger; use mysqli; @@ -160,14 +161,43 @@ public function register(Request $request, ResponseInterface $response): Respons $password = (string) ($body['password'] ?? ''); $password2 = (string) ($body['password_confirm'] ?? ''); $privacy = !empty($body['privacy_acceptance']); + $customFields = $body['custom_fields'] ?? []; + if (!is_array($customFields)) { + return ResponseEnvelope::error( + $response, + 'custom_field_invalid', + __('Controlla i campi personalizzati: un valore inserito non è valido.'), + 422 + ); + } // Same validation rules as the web RegistrationController. if (!$privacy) { return ResponseEnvelope::error($response, 'privacy_required', __('Devi accettare la privacy policy.'), 422); } - if ($nome === '' || $cognome === '' || $email === '' || $telefono === '' || $indirizzo === '' || $password === '' || $password !== $password2) { + if ($nome === '' || $email === '' || $password === '' || $password !== $password2) { + return ResponseEnvelope::error($response, 'missing_fields', __('Compila tutti i campi obbligatori.'), 422); + } + if (($cognome === '' && RegistrationFields::isRequired('cognome')) + || ($telefono === '' && RegistrationFields::isRequired('telefono')) + || ($indirizzo === '' && RegistrationFields::isRequired('indirizzo')) + ) { return ResponseEnvelope::error($response, 'missing_fields', __('Compila tutti i campi obbligatori.'), 422); } + $customDefinitions = RegistrationFields::definitions($this->db); + $customValidation = RegistrationFields::validate( + $customDefinitions, + ['custom_field' => $customFields] + ); + if ($customValidation['error'] !== null) { + $code = $customValidation['error_reason'] === 'format' + ? 'custom_field_invalid' + : 'missing_fields'; + $message = $code === 'custom_field_invalid' + ? __('Controlla i campi personalizzati: un valore inserito non è valido.') + : __('Compila tutti i campi obbligatori.'); + return ResponseEnvelope::error($response, $code, $message, 422); + } if (!filter_var($email, FILTER_VALIDATE_EMAIL) || strlen($email) > 255) { return ResponseEnvelope::error($response, 'invalid_email', __('Indirizzo email non valido.'), 422); } @@ -207,36 +237,71 @@ public function register(Request $request, ResponseInterface $response): Respons $stato = 'sospeso'; // requires admin approval (same as web) $ruolo = 'standard'; - $columns = 'nome, cognome, email, password, telefono, indirizzo, codice_tessera, stato, tipo_utente, email_verificata, token_verifica_email, data_token_verifica, data_scadenza_tessera, privacy_accettata, data_accettazione_privacy, privacy_policy_version'; - $placeholders = '?, ?, ?, ?, ?, ?, ?, ?, ?, 0, ?, ?, ?, 1, ?, ?'; - $types = 'ssssssssssssss'; + $columns = 'nome, cognome, email, password, codice_tessera, stato, tipo_utente, email_verificata, token_verifica_email, data_token_verifica, data_scadenza_tessera, privacy_accettata, data_accettazione_privacy, privacy_policy_version'; + $placeholders = '?, ?, ?, ?, ?, ?, ?, 0, ?, ?, ?, 1, ?, ?'; + $types = 'ssssssssssss'; $values = [ - $nome, $cognome, $email, $hash, $telefono, $indirizzo, $codiceTessera, - $stato, $ruolo, $token, $scadenzaToken, $scadenzaTessera, $accettazione, $privacyVersion, + $nome, $cognome, $email, $hash, $codiceTessera, $stato, $ruolo, + $token, $scadenzaToken, $scadenzaTessera, $accettazione, $privacyVersion, ]; + if ($telefono !== '') { + $columns .= ', telefono'; + $placeholders .= ', ?'; + $types .= 's'; + $values[] = $telefono; + } + if ($indirizzo !== '') { + $columns .= ', indirizzo'; + $placeholders .= ', ?'; + $types .= 's'; + $values[] = $indirizzo; + } - $stmt = $this->db->prepare("INSERT INTO utenti ({$columns}) VALUES ({$placeholders})"); - if ($stmt === false) { - return ResponseEnvelope::error($response, 'internal_error', __('Errore del server.'), 500); + $stmt = null; + try { + $this->db->begin_transaction(); + $stmt = $this->db->prepare("INSERT INTO utenti ({$columns}) VALUES ({$placeholders})"); + if ($stmt === false) { + throw new \RuntimeException('Unable to prepare registration insert'); + } + $stmt->bind_param($types, ...$values); + $stmt->execute(); + $userId = (int) $stmt->insert_id; + RegistrationFields::saveValues($this->db, $userId, $customValidation['values']); + $this->db->commit(); + } catch (\Throwable $e) { + try { + $this->db->rollback(); + } catch (\Throwable) { + // best-effort — preserve the original failure + } + if ($e instanceof \mysqli_sql_exception && $e->getCode() === 1062) { + return ResponseEnvelope::error($response, 'email_exists', __('Esiste già un account con questa email.'), 409); + } + throw $e; + } finally { + if ($stmt instanceof \mysqli_stmt) { + $stmt->close(); + } } - $stmt->bind_param($types, ...$values); - if (!$stmt->execute()) { - $stmt->close(); - return ResponseEnvelope::error($response, 'internal_error', __('Errore durante la registrazione.'), 500); + + // Registration already committed: notification failures must not + // turn a successfully-created account into an API 500. + try { + $notifications = new NotificationService($this->db); + $notifications->sendUserRegistrationPending($userId); + $notifications->notifyNewUserRegistration($userId); + $notifications->notifyNewUserInApp($userId, trim($nome . ' ' . $cognome), $email); + } catch (\Throwable $e) { + SecureLogger::error('[MobileApi] registration side-effect failed for user ' . $userId . ': ' . $e->getMessage()); } - $userId = (int) $stmt->insert_id; - $stmt->close(); - // Reuse the web email-verification flow verbatim. - $notifications = new NotificationService($this->db); - $notifications->sendUserRegistrationPending($userId); - $notifications->notifyNewUserRegistration($userId); - $notifications->notifyNewUserInApp($userId, $nome . ' ' . $cognome, $email); + $requiresApproval = (bool) ConfigStore::get('registration.require_admin_approval', true); $data = [ 'user_id' => $userId, 'email_verification' => true, - 'requires_approval' => true, + 'requires_approval' => $requiresApproval, ]; return ResponseEnvelope::success( diff --git a/storage/plugins/mobile-api/src/Controllers/HealthController.php b/storage/plugins/mobile-api/src/Controllers/HealthController.php index 55939d08d..f2a9562b8 100644 --- a/storage/plugins/mobile-api/src/Controllers/HealthController.php +++ b/storage/plugins/mobile-api/src/Controllers/HealthController.php @@ -7,7 +7,9 @@ use App\Plugins\MobileApi\Support\ProxyTrust; use App\Plugins\MobileApi\Support\ResponseEnvelope; use App\Support\ConfigStore; +use App\Support\RegistrationFields; use App\Support\SecureLogger; +use mysqli; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; @@ -27,6 +29,10 @@ final class HealthController /** Mobile API contract version advertised to clients for forward-compat. */ public const API_VERSION = 'v1'; + public function __construct(private mysqli $db) + { + } + public function index( ServerRequestInterface $request, ResponseInterface $response, @@ -84,6 +90,12 @@ public function index( 'catalogue_mode' => $catalogueMode, 'app_access_enabled' => $appAccessEnabled, 'registration_enabled' => $registrationEnabled, + 'registration' => [ + 'require_cognome' => RegistrationFields::isRequired('cognome'), + 'require_telefono' => RegistrationFields::isRequired('telefono'), + 'require_indirizzo' => RegistrationFields::isRequired('indirizzo'), + 'custom_fields' => RegistrationFields::apiDefinitions($this->db), + ], 'private_mode' => $privateMode, // VAPID public key (applicationServerKey) for Web Push / UnifiedPush // subscription on the device. Empty if push isn't set up. diff --git a/storage/plugins/mobile-api/src/Controllers/OpenApiController.php b/storage/plugins/mobile-api/src/Controllers/OpenApiController.php index d633e3ad8..92adfa5ab 100644 --- a/storage/plugins/mobile-api/src/Controllers/OpenApiController.php +++ b/storage/plugins/mobile-api/src/Controllers/OpenApiController.php @@ -125,6 +125,9 @@ private function build(string $baseUrl, string $version): array 'LoginRequest' => $this->loginRequestSchema(), 'LoginResponse' => $this->loginResponseSchema(), 'RegisterRequest' => $this->registerRequestSchema(), + 'RegistrationConfig'=> $this->registrationConfigSchema(), + 'CustomFieldDefinition' => $this->customFieldDefinitionSchema(), + 'CustomProfileField'=> $this->customProfileFieldSchema(), 'ForgotRequest' => $this->forgotRequestSchema(), 'DeviceItem' => $this->deviceItemSchema(), @@ -939,16 +942,63 @@ private function registerRequestSchema(): array { return [ 'type' => 'object', - 'required' => ['nome', 'cognome', 'email', 'telefono', 'indirizzo', 'password', 'password_confirm', 'privacy_acceptance'], + 'required' => ['nome', 'email', 'password', 'password_confirm', 'privacy_acceptance'], 'properties' => [ 'nome' => ['type' => 'string', 'maxLength' => 100], - 'cognome' => ['type' => 'string', 'maxLength' => 100], + 'cognome' => ['type' => 'string', 'maxLength' => 100, 'description' => 'Required only when health.registration.require_cognome is true.'], 'email' => ['type' => 'string', 'format' => 'email', 'maxLength' => 255], - 'telefono' => ['type' => 'string'], - 'indirizzo' => ['type' => 'string'], + 'telefono' => ['type' => 'string', 'description' => 'Required only when health.registration.require_telefono is true.'], + 'indirizzo' => ['type' => 'string', 'description' => 'Required only when health.registration.require_indirizzo is true.'], 'password' => ['type' => 'string', 'minLength' => 8, 'maxLength' => 72], 'password_confirm' => ['type' => 'string', 'minLength' => 8, 'maxLength' => 72], 'privacy_acceptance' => ['type' => 'boolean'], + 'custom_fields' => [ + 'type' => 'object', + 'description' => 'Values keyed by the numeric field id advertised by health.registration.custom_fields.', + 'additionalProperties' => ['type' => 'string', 'maxLength' => 1000], + ], + ], + ]; + } + + /** @return array */ + private function customFieldDefinitionSchema(): array + { + return [ + 'type' => 'object', + 'required' => ['id', 'label', 'type', 'required'], + 'properties' => [ + 'id' => ['type' => 'integer'], + 'label' => ['type' => 'string'], + 'type' => ['type' => 'string', 'enum' => ['text', 'textarea', 'email', 'url', 'number', 'checkbox']], + 'required' => ['type' => 'boolean'], + ], + ]; + } + + /** @return array */ + private function customProfileFieldSchema(): array + { + $schema = $this->customFieldDefinitionSchema(); + $schema['required'][] = 'value'; + $schema['properties']['value'] = ['type' => 'string', 'maxLength' => 1000]; + return $schema; + } + + /** @return array */ + private function registrationConfigSchema(): array + { + return [ + 'type' => 'object', + 'required' => ['require_cognome', 'require_telefono', 'require_indirizzo', 'custom_fields'], + 'properties' => [ + 'require_cognome' => ['type' => 'boolean'], + 'require_telefono' => ['type' => 'boolean'], + 'require_indirizzo' => ['type' => 'boolean'], + 'custom_fields' => [ + 'type' => 'array', + 'items' => ['$ref' => '#/components/schemas/CustomFieldDefinition'], + ], ], ]; } @@ -1190,10 +1240,20 @@ private function userProfileSchema(): array 'nome' => ['type' => 'string'], 'cognome' => ['type' => 'string'], 'email' => ['type' => 'string', 'format' => 'email'], - 'tipo_utente' => ['type' => 'string', 'enum' => ['utente', 'staff', 'admin']], + 'tipo_utente' => ['type' => 'string', 'enum' => ['standard', 'premium', 'staff', 'admin']], 'email_verificata' => ['type' => 'boolean'], 'stato' => ['type' => 'string'], 'avatar_url' => ['type' => 'string', 'format' => 'uri', 'nullable' => true], + 'telefono' => ['type' => 'string', 'nullable' => true], + 'indirizzo' => ['type' => 'string', 'nullable' => true], + 'data_nascita' => ['type' => 'string', 'format' => 'date', 'nullable' => true], + 'cod_fiscale' => ['type' => 'string', 'nullable' => true], + 'sesso' => ['type' => 'string', 'nullable' => true], + 'locale' => ['type' => 'string', 'nullable' => true], + 'custom_fields' => [ + 'type' => 'array', + 'items' => ['$ref' => '#/components/schemas/CustomProfileField'], + ], ], ]; } @@ -1204,8 +1264,19 @@ private function updateProfileRequestSchema(): array return [ 'type' => 'object', 'properties' => [ - 'nome' => ['type' => 'string', 'maxLength' => 100], - 'cognome' => ['type' => 'string', 'maxLength' => 100], + 'nome' => ['type' => 'string', 'maxLength' => 100], + 'cognome' => ['type' => 'string', 'maxLength' => 100], + 'telefono' => ['type' => 'string'], + 'indirizzo' => ['type' => 'string'], + 'data_nascita' => ['type' => 'string', 'format' => 'date', 'nullable' => true], + 'cod_fiscale' => ['type' => 'string', 'nullable' => true], + 'sesso' => ['type' => 'string', 'nullable' => true], + 'locale' => ['type' => 'string', 'nullable' => true], + 'custom_fields' => [ + 'type' => 'object', + 'description' => 'When omitted, all custom values are preserved. Within the map, omitted ids are preserved and an explicitly empty value clears that field.', + 'additionalProperties' => ['type' => 'string', 'maxLength' => 1000], + ], ], ]; } @@ -1327,6 +1398,7 @@ private function healthPayloadSchema(): array ], 'app_access_enabled' => ['type' => 'boolean'], 'registration_enabled' => ['type' => 'boolean'], + 'registration' => ['$ref' => '#/components/schemas/RegistrationConfig'], 'private_mode' => ['type' => 'boolean'], 'vapid_public_key' => [ 'type' => 'string', diff --git a/tests/issue-255-registration.spec.js b/tests/issue-255-registration.spec.js index 6bcb86138..99caa4d87 100644 --- a/tests/issue-255-registration.spec.js +++ b/tests/issue-255-registration.spec.js @@ -1,6 +1,6 @@ // @ts-check /** - * Issue #255 — configurable registration fields. 25-check E2E suite. + * Issue #255 — configurable registration fields. 29-check E2E suite. * * Coverage (all through the real browser): * 1-4 defaults + SERVER-side enforcement of each built-in requirement @@ -18,6 +18,7 @@ * 23-24 sanitization: a tag-carrying label is neutralised on save; a * script-carrying VALUE is escaped when rendered back (admin detail). * 25 an inactive (attivo=0) field disappears from the public form. + * 26-29 profile/admin-detail regressions and destructive type-change guard. */ const { test, expect } = require('@playwright/test'); @@ -101,7 +102,7 @@ function userCount(email) { return Number(dbQuery(`SELECT COUNT(*) FROM utenti WHERE email='${email}'`)); } -test.describe.serial('Issue #255 — configurable registration fields (25 checks)', () => { +test.describe.serial('Issue #255 — configurable registration fields (29 checks)', () => { /** @type {import('@playwright/test').Page} */ let admin; @@ -356,7 +357,7 @@ test.describe.serial('Issue #255 — configurable registration fields (25 checks expect(userCount(email)).toBe(1); const uid = dbQuery(`SELECT id FROM utenti WHERE email='${email}'`); - await admin.goto(`${BASE}/admin/users/edit/${uid}`); + await admin.goto(`${BASE}/admin/users/details/${uid}`); // The literal payload must be visible as TEXT (escaped), never executed. await expect(admin.locator('dd', { hasText: 'window.__pwned' })).toBeVisible(); const pwned = await admin.evaluate(() => /** @type {any} */ (window).__pwned); @@ -416,4 +417,31 @@ test.describe.serial('Issue #255 — configurable registration fields (25 checks expect(text).not.toMatch(/SoloNome27 (?:\n|$)/); expect(text).toContain('SoloNome27'); }); + + test('28. a newly-required custom field does not block existing users in the profile form', async () => { + const id = fieldId(LBL('text')); + dbQuery(`UPDATE registrazione_campi SET obbligatorio=1 WHERE id=${id}`); + try { + await admin.goto(`${BASE}/profilo`); + const control = admin.locator(`[name="custom_field[${id}]"]`); + await expect(control).toBeVisible(); + await expect(control).not.toHaveAttribute('required', /.*/); + await expect(admin.locator('input[name="custom_fields_present"]')).toHaveValue('1'); + } finally { + dbQuery(`UPDATE registrazione_campi SET obbligatorio=0 WHERE id=${id}`); + } + }); + + test('29. a field type with stored values cannot be changed destructively', async () => { + const id = fieldId(LBL('text')); + expect(Number(dbQuery(`SELECT COUNT(*) FROM utenti_campi_valori WHERE campo_id=${id}`))).toBeGreaterThan(0); + await admin.goto(`${BASE}/admin/settings?tab=registration`); + await admin.selectOption(`select[name="custom_fields[${id}][tipo]"]`, 'checkbox'); + await Promise.all([ + admin.waitForLoadState('domcontentloaded'), + admin.click('form[action*="/admin/settings/registration"] button[type="submit"]'), + ]); + expect(dbQuery(`SELECT tipo FROM registrazione_campi WHERE id=${id}`)).toBe('text'); + await expect(admin.locator('body')).toContainText(/non puoi cambiare il tipo|cannot change the type|ne pouvez pas modifier le type|Typ eines Feldes/i); + }); }); diff --git a/tests/mobile-api-idempotency.spec.js b/tests/mobile-api-idempotency.spec.js index 01560040c..314736def 100644 --- a/tests/mobile-api-idempotency.spec.js +++ b/tests/mobile-api-idempotency.spec.js @@ -294,14 +294,17 @@ test.describe('Mobile API — two calls per endpoint (idempotency + ETag/304)', // Test borrower with a known password hash for USER_PASS. let uid = parseInt(dbScalar(`SELECT id FROM utenti WHERE email='${USER_EMAIL}' LIMIT 1`) || '0', 10); if (uid === 0) { - dbExec(`INSERT INTO utenti (nome, cognome, email, password, tipo_utente, stato, email_verificata, codice_tessera, created_at) - VALUES ('Idem','Test','${USER_EMAIL}','x','standard','attivo',1,'${USER_CARD}',NOW())`); + dbExec(`INSERT INTO utenti (nome, cognome, email, password, telefono, indirizzo, tipo_utente, stato, email_verificata, codice_tessera, created_at) + VALUES ('Idem','Test','${USER_EMAIL}','x','000','E2E address','standard','attivo',1,'${USER_CARD}',NOW())`); uid = parseInt(dbScalar(`SELECT id FROM utenti WHERE email='${USER_EMAIL}' LIMIT 1`) || '0', 10); ctx.createdUser = true; } // Set the password to USER_PASS using PHP password_hash so /auth/login matches. const realHash = execFileSync('php', ['-r', `echo password_hash(${JSON.stringify(USER_PASS)}, PASSWORD_DEFAULT);`], { encoding: 'utf-8' }).trim(); - dbExec(`UPDATE utenti SET password=${sqlStr(realHash)}, stato='attivo', email_verificata=1 WHERE id=${uid}`); + dbExec(`UPDATE utenti + SET password=${sqlStr(realHash)}, telefono='000', indirizzo='E2E address', + stato='attivo', email_verificata=1 + WHERE id=${uid}`); ctx.userId = uid; // Fixtures: a non-deleted book. diff --git a/tests/mobile-api.spec.js b/tests/mobile-api.spec.js index 5e207ed7f..986b6e7c9 100644 --- a/tests/mobile-api.spec.js +++ b/tests/mobile-api.spec.js @@ -258,6 +258,8 @@ const TEST_USER_A_EMAIL = 'mobile_api_test_a@pinakes.test'; const TEST_USER_A_PASS = 'TestA1secure!'; const TEST_USER_B_EMAIL = 'mobile_api_test_b@pinakes.test'; const TEST_USER_B_PASS = 'TestB1secure!'; +const REGISTRATION_EMAIL = 'mobile_api_registration_fields@pinakes.test'; +const CUSTOM_FIELD_LABEL = 'Mobile API Telegram handle'; /** * Insert a verified + active test user directly in the DB. @@ -303,6 +305,10 @@ test.describe.serial('Mobile API plugin — E2E suite', () => { let reservationId = 0; /** @type {number} */ let wishlistBookId = 0; + /** @type {number} */ + let customFieldId = 0; + /** @type {Record} */ + const originalRegistrationSettings = {}; /** Track rows created during tests for cleanup. */ /** @type {number[]} */ const mobileTokenIds = []; @@ -319,6 +325,36 @@ test.describe.serial('Mobile API plugin — E2E suite', () => { throw new Error('mobile-api: could not ensure test users'); } + for (const key of ['require_cognome', 'require_telefono', 'require_indirizzo']) { + originalRegistrationSettings[key] = dbQuery( + `SELECT COALESCE((SELECT setting_value FROM system_settings WHERE category='registration' AND setting_key='${key}' LIMIT 1), '__MISSING__')` + ); + dbExec( + `INSERT INTO system_settings (category, setting_key, setting_value) + VALUES ('registration', '${key}', '0') + ON DUPLICATE KEY UPDATE setting_value='0'` + ); + } + dbExec( + `INSERT INTO registrazione_campi (etichetta, tipo, obbligatorio, attivo, ordine) + SELECT '${CUSTOM_FIELD_LABEL}', 'text', 1, 1, ordering.next_order + FROM (SELECT COALESCE(MAX(ordine), 0) + 1 AS next_order FROM registrazione_campi) ordering + WHERE NOT EXISTS ( + SELECT 1 FROM registrazione_campi WHERE etichetta='${CUSTOM_FIELD_LABEL}' + )` + ); + customFieldId = parseInt(dbQuery( + `SELECT id FROM registrazione_campi WHERE etichetta='${CUSTOM_FIELD_LABEL}' LIMIT 1` + ), 10) || 0; + if (customFieldId === 0) { + throw new Error('mobile-api: could not create registration custom field'); + } + dbExec( + `INSERT INTO utenti_campi_valori (utente_id, campo_id, valore) + VALUES (${userIdA}, ${customFieldId}, '@before-patch') + ON DUPLICATE KEY UPDATE valore='@before-patch'` + ); + // Find a book with available copies for reservation tests. const bookRow = dbQuery( 'SELECT id FROM libri WHERE deleted_at IS NULL AND copie_disponibili > 0 ORDER BY id LIMIT 1' @@ -342,10 +378,21 @@ test.describe.serial('Mobile API plugin — E2E suite', () => { dbExec(`DELETE FROM mobile_availability_watchers WHERE user_id IN (${userIdA},${userIdB})`); dbExec(`DELETE FROM mobile_push_log WHERE user_id IN (${userIdA},${userIdB})`); dbExec(`DELETE FROM mobile_app_tokens WHERE user_id IN (${userIdA},${userIdB})`); + dbExec(`DELETE FROM utenti_campi_valori WHERE campo_id = ${customFieldId} OR utente_id IN (${userIdA},${userIdB})`); dbExec(`DELETE FROM wishlist WHERE utente_id IN (${userIdA},${userIdB})`); dbExec(`DELETE FROM prenotazioni WHERE utente_id IN (${userIdA},${userIdB})`); dbExec(`DELETE FROM prestiti WHERE utente_id IN (${userIdA},${userIdB})`); - dbExec(`DELETE FROM utenti WHERE email IN ('${TEST_USER_A_EMAIL}', '${TEST_USER_B_EMAIL}')`); + dbExec(`DELETE FROM registrazione_campi WHERE id = ${customFieldId}`); + dbExec(`DELETE FROM utenti WHERE email IN ('${TEST_USER_A_EMAIL}', '${TEST_USER_B_EMAIL}', '${REGISTRATION_EMAIL}')`); + for (const [key, value] of Object.entries(originalRegistrationSettings)) { + dbExec(`DELETE FROM system_settings WHERE category='registration' AND setting_key='${key}'`); + if (value !== '__MISSING__') { + dbExec( + `INSERT INTO system_settings (category, setting_key, setting_value) + VALUES ('registration', '${key}', '${value.replace(/'/g, "''")}')` + ); + } + } } catch (_) { /* best-effort */ } }); @@ -393,6 +440,16 @@ test.describe.serial('Mobile API plugin — E2E suite', () => { expect(d.api_version).toBe('v1'); // After enablement in beforeAll the gate must be true. expect(d.app_access_enabled).toBe(true); + expect(d.registration).toMatchObject({ + require_cognome: false, + require_telefono: false, + require_indirizzo: false, + }); + expect(d.registration.custom_fields).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: customFieldId, required: true, type: 'text' }), + ]) + ); }); test('8. /health features object contains expected keys', async ({ request }) => { @@ -426,6 +483,31 @@ test.describe.serial('Mobile API plugin — E2E suite', () => { expect(d.features.push).toBe(true); // push available once keyed }); + test('9c. POST /auth/register follows optional built-ins and stores custom fields', async ({ request }) => { + dbExec(`DELETE FROM utenti WHERE email='${REGISTRATION_EMAIL}'`); + const res = await apiPost(request, '/auth/register', { + nome: 'MobileRegistration', + email: REGISTRATION_EMAIL, + password: 'MobilePass1!', + password_confirm: 'MobilePass1!', + privacy_acceptance: true, + custom_fields: { [customFieldId]: '@registered-from-app' }, + }); + const body = await envelope(res, 201); + expect(body.error).toBeNull(); + const stored = dbQuery( + `SELECT CONCAT(cognome, '|', COALESCE(telefono, 'NULL'), '|', COALESCE(indirizzo, 'NULL')) + FROM utenti WHERE email='${REGISTRATION_EMAIL}' LIMIT 1` + ); + expect(stored).toBe('|NULL|NULL'); + const storedCustom = dbQuery( + `SELECT v.valore FROM utenti_campi_valori v + JOIN utenti u ON u.id=v.utente_id + WHERE u.email='${REGISTRATION_EMAIL}' AND v.campo_id=${customFieldId} LIMIT 1` + ); + expect(storedCustom).toBe('@registered-from-app'); + }); + // ══ 3. Auth — login ════════════════════════════════════════════════════════ test('10. POST /auth/login success → token returned + user payload', async ({ request }) => { @@ -849,17 +931,41 @@ test.describe.serial('Mobile API plugin — E2E suite', () => { expect(d).toHaveProperty('cognome'); expect(d).toHaveProperty('email'); expect(d).toHaveProperty('tipo_utente'); + expect(d.custom_fields).toEqual( + expect.arrayContaining([ + expect.objectContaining({ id: customFieldId, value: '@before-patch' }), + ]) + ); }); test('48. PATCH /me → update nome', async ({ request }) => { test.skip(!tokenA, 'tokenA not set'); const res = await apiPatch(request, '/me', { nome: 'TestAggiornato' }, tokenA); - // 200 on success, 422/500 on validation error — just check it doesn't explode. - expect([200, 422, 500]).toContain(res.status()); - if (res.status() === 200) { - const body = await res.json(); - expect(body.data.nome).toBe('TestAggiornato'); - } + const body = await envelope(res, 200); + expect(body.data.nome).toBe('TestAggiornato'); + expect(dbQuery( + `SELECT valore FROM utenti_campi_valori WHERE utente_id=${userIdA} AND campo_id=${customFieldId}` + )).toBe('@before-patch'); + }); + + test('48b. PATCH /me updates custom fields only when explicitly supplied', async ({ request }) => { + test.skip(!tokenA, 'tokenA not set'); + const res = await apiPatch(request, '/me', { + custom_fields: { [customFieldId]: '@after-patch' }, + }, tokenA); + const body = await envelope(res, 200); + const field = body.data.custom_fields.find((item) => item.id === customFieldId); + expect(field.value).toBe('@after-patch'); + expect(dbQuery( + `SELECT valore FROM utenti_campi_valori WHERE utente_id=${userIdA} AND campo_id=${customFieldId}` + )).toBe('@after-patch'); + }); + + test('48c. PATCH /me accepts an empty surname when the instance makes it optional', async ({ request }) => { + test.skip(!tokenA, 'tokenA not set'); + const res = await apiPatch(request, '/me', { cognome: '' }, tokenA); + const body = await envelope(res, 200); + expect(body.data.cognome).toBe(''); }); test('49. PATCH /me with empty nome → 422', async ({ request }) => { @@ -1200,6 +1306,19 @@ test.describe.serial('Mobile API plugin — E2E suite', () => { if (res.status() === 200) { const ct = res.headers()['content-type'] || ''; expect(ct).toContain('json'); + const document = await res.json(); + const schemas = document.components.schemas; + expect(schemas.RegisterRequest.required).toEqual([ + 'nome', 'email', 'password', 'password_confirm', 'privacy_acceptance', + ]); + expect(schemas.RegistrationConfig.properties.custom_fields.items.$ref) + .toBe('#/components/schemas/CustomFieldDefinition'); + expect(schemas.UserProfile.properties.custom_fields.items.$ref) + .toBe('#/components/schemas/CustomProfileField'); + expect(schemas.UserProfile.properties.tipo_utente.enum) + .toEqual(['standard', 'premium', 'staff', 'admin']); + expect(schemas.HealthPayload.properties.registration.$ref) + .toBe('#/components/schemas/RegistrationConfig'); } }); diff --git a/tests/registration-fields-255.unit.php b/tests/registration-fields-255.unit.php new file mode 100644 index 000000000..89b4e707a --- /dev/null +++ b/tests/registration-fields-255.unit.php @@ -0,0 +1,159 @@ +set_charset('utf8mb4'); +} catch (\Throwable $e) { + fwrite(STDERR, "FAIL: database unreachable — this suite is DB-backed: {$e->getMessage()}\n"); + exit(1); +} + +$TOKEN = 'zzrf255' . substr((string) getmypid(), -4); +$cleanup = static function () use ($db, $TOKEN): void { + $db->query("DELETE v FROM utenti_campi_valori v JOIN registrazione_campi c ON c.id = v.campo_id WHERE c.etichetta LIKE '{$TOKEN}%'"); + $db->query("DELETE FROM registrazione_campi WHERE etichetta LIKE '{$TOKEN}%'"); + $db->query("DELETE FROM utenti WHERE email LIKE '{$TOKEN}-%@example.test'"); +}; + +try { + $cleanup(); + + // Seed: one active text field, one active required checkbox, one INACTIVE field. + $db->query("INSERT INTO registrazione_campi (etichetta, tipo, obbligatorio, attivo, ordine) VALUES ('{$TOKEN} Telegram', 'text', 0, 1, 2)"); + $idText = (int) $db->insert_id; + $db->query("INSERT INTO registrazione_campi (etichetta, tipo, obbligatorio, attivo, ordine) VALUES ('{$TOKEN} News', 'checkbox', 1, 1, 1)"); + $idCheck = (int) $db->insert_id; + $db->query("INSERT INTO registrazione_campi (etichetta, tipo, obbligatorio, attivo, ordine) VALUES ('{$TOKEN} Dead', 'url', 0, 0, 3)"); + $idDead = (int) $db->insert_id; + + $db->query("INSERT INTO utenti (nome, cognome, email, password, codice_tessera) VALUES ('Rf', '', '{$TOKEN}-1@example.test', 'x', CONCAT('TRF', FLOOR(RAND()*1000000)))"); + $uid = (int) $db->insert_id; + + // ── apiDefinitions() ───────────────────────────────────────────────────── + echo "A. apiDefinitions()\n"; + $api = RegistrationFields::apiDefinitions($db); + // Filter to our token so a shared DB with other fields doesn't skew asserts. + $mine = array_values(array_filter($api, static fn ($d) => str_contains((string) $d['label'], $TOKEN))); + $check(count($mine) === 2, 'apiDefinitions returns only ACTIVE fields (2 of 3, dead excluded)'); + $check(array_keys($mine[0]) === ['id', 'label', 'type', 'required'], 'apiDefinitions shape is {id,label,type,required} (no italian keys)'); + $check($mine[0]['label'] === "{$TOKEN} News" && $mine[1]['label'] === "{$TOKEN} Telegram", 'apiDefinitions honours ordine (News before Telegram)'); + $check($mine[0]['required'] === true && $mine[1]['required'] === false, 'apiDefinitions maps obbligatorio → required bool'); + $check(!array_filter($api, static fn ($d) => ($d['label'] ?? '') === "{$TOKEN} Dead"), 'apiDefinitions excludes the inactive field'); + + // ── hasStoredValues() ──────────────────────────────────────────────────── + echo "B. hasStoredValues()\n"; + $check(RegistrationFields::hasStoredValues($db, $idText) === false, 'hasStoredValues false before any value stored'); + $check(RegistrationFields::hasStoredValues($db, 0) === false, 'hasStoredValues false for id <= 0'); + $check(RegistrationFields::hasStoredValues($db, 999999999) === false, 'hasStoredValues false for unknown id'); + + // ── validate() enforceRequired matrix ──────────────────────────────────── + echo "C. validate() enforceRequired matrix\n"; + $defs = RegistrationFields::definitions($db); + $defsMine = array_values(array_filter($defs, static fn ($d) => str_contains((string) $d['etichetta'], $TOKEN) && $d['attivo'])); + + // required checkbox unchecked → missing when enforced, ok when not. + $r = RegistrationFields::validate($defsMine, ['custom_field' => [$idText => 'x']], true); + $check($r['error'] !== null && $r['error_reason'] === 'missing', 'enforceRequired=true: unchecked required checkbox → missing'); + $r = RegistrationFields::validate($defsMine, ['custom_field' => [$idText => 'x']], false); + $check($r['error'] === null, 'enforceRequired=false: unchecked required checkbox tolerated'); + + // checkbox normalisation + $r = RegistrationFields::validate($defsMine, ['custom_field' => [$idCheck => 'on', $idText => 'x']], true); + $check($r['error'] === null && $r['values'][$idCheck] === '1', 'checkbox normalises truthy → "1"'); + $r = RegistrationFields::validate($defsMine, ['custom_field' => [$idCheck => '', $idText => 'x']], false); + $check(($r['values'][$idCheck] ?? null) === '', 'checkbox normalises blank → ""'); + + // non-scalar payload rejected as format + $r = RegistrationFields::validate($defsMine, ['custom_field' => [$idText => ['x'], $idCheck => 'on']], false); + $check($r['error'] !== null && $r['error_reason'] === 'format', 'non-scalar custom value rejected as format'); + + // over-long value + $r = RegistrationFields::validate($defsMine, ['custom_field' => [$idText => str_repeat('a', 1001), $idCheck => 'on']], false); + $check($r['error'] !== null && $r['error_reason'] === 'format', 'over-long value rejected as format'); + + // text field accepts a normal value; values map returned + $r = RegistrationFields::validate($defsMine, ['custom_field' => [$idText => '@handle', $idCheck => 'on']], true); + $check($r['error'] === null && $r['values'][$idText] === '@handle', 'valid text value passes and is returned'); + + // ── saveValues() / valuesForUser() round-trip ──────────────────────────── + echo "D. saveValues() / valuesForUser() round-trip\n"; + RegistrationFields::saveValues($db, $uid, [$idText => '@handle', $idCheck => '1']); + $vals = RegistrationFields::valuesForUser($db, $uid); + $check(($vals[$idText] ?? null) === '@handle' && ($vals[$idCheck] ?? null) === '1', 'saveValues persists, valuesForUser reads back'); + $check(RegistrationFields::hasStoredValues($db, $idText) === true, 'hasStoredValues true after a value is stored'); + + // empty string clears the row + RegistrationFields::saveValues($db, $uid, [$idText => '']); + $vals = RegistrationFields::valuesForUser($db, $uid); + $check(!array_key_exists($idText, $vals), 'saveValues("") deletes the row (field cleared)'); + $check(($vals[$idCheck] ?? null) === '1', 'clearing one field leaves the others intact'); + + // per-user isolation + $db->query("INSERT INTO utenti (nome, cognome, email, password, codice_tessera) VALUES ('Rf2', '', '{$TOKEN}-2@example.test', 'x', CONCAT('TRG', FLOOR(RAND()*1000000)))"); + $uid2 = (int) $db->insert_id; + $check(RegistrationFields::valuesForUser($db, $uid2) === [], 'valuesForUser is per-user (other user has none)'); + $check(RegistrationFields::valuesForUser($db, 0) === [], 'valuesForUser empty for id <= 0'); + + // ── editableFieldsForUser() ────────────────────────────────────────────── + echo "E. editableFieldsForUser()\n"; + $edit = RegistrationFields::editableFieldsForUser($db, $uid); + $editMine = array_values(array_filter($edit, static fn ($d) => str_contains((string) $d['label'], $TOKEN))); + $check(count($editMine) === 2, 'editableFieldsForUser returns the active fields'); + $check(array_key_exists('value', $editMine[0]), 'editableFieldsForUser includes a value key'); + $byLabel = []; + foreach ($editMine as $e) { $byLabel[$e['label']] = $e; } + $check($byLabel["{$TOKEN} News"]['value'] === '1', "editableFieldsForUser carries this user's stored value"); + $check($byLabel["{$TOKEN} Telegram"]['value'] === '', 'editableFieldsForUser returns "" for an unset field'); + + // ── labelledValuesForUser() only surfaces non-empty ────────────────────── + echo "F. labelledValuesForUser()\n"; + $labelled = RegistrationFields::labelledValuesForUser($db, $uid); + $labelledMine = array_values(array_filter($labelled, static fn ($d) => str_contains((string) $d['etichetta'], $TOKEN))); + $check(count($labelledMine) === 1 && $labelledMine[0]['etichetta'] === "{$TOKEN} News", 'labelledValuesForUser surfaces only fields with a stored value'); +} finally { + $cleanup(); + $db->close(); +} + +echo "\n" . ($fail === 0 ? "ALL {$pass} PASS\n" : "{$pass} PASS, {$fail} FAIL\n"); +exit($fail === 0 ? 0 : 1);