From 681875e73d1d0a236b366f9a036cf1e0dafe1262 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 21 Jul 2026 16:33:00 +0200 Subject: [PATCH 1/4] feat(#255): configurable + custom registration/profile fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consume the new server discovery endpoint GET /api/v1/auth/registration-fields so signup and profile honour an instance's configurable built-in fields and admin-defined custom fields. Models: RegistrationFieldsPayload / BuiltinFieldRule / CustomFieldDef / CustomFieldValue. RegisterRequest gains custom_fields (Map). UserProfile extended with the fields GET /me already returns (telefono, indirizzo, data_nascita, sesso, cod_fiscale, codice_tessera, card/last-access, locale, custom_fields). UpdateProfileRequest extended (all nullable → partial PATCH). PinakesApi: GET auth/registration-fields (no-auth). Register: fetch the schema on entry; built-in required-ness (cognome/telefono/ indirizzo) driven by the schema (default required when absent, matching the server); custom fields rendered dynamically by type and sent as custom_fields. Profile: view + edit telefono/indirizzo/data_nascita (date picker)/cod_fiscale/ sesso and the custom fields via partial PATCH /me. All new inputs reuse the app design system — PinakesTextField, Switch for checkbox, Material3 DatePicker, Spacing/MaterialTheme tokens (no raw OutlinedTextField). New strings added to all four locales (en/it/fr/de). Not built here (no Android SDK in that environment) — needs a Gradle build to confirm. Static review clean; kotlinx JSON is ignoreUnknownKeys/explicitNulls= false so the richer server payloads never crash older behaviour. --- .../java/com/pinakes/app/data/model/Models.kt | 66 ++++++++ .../pinakes/app/data/network/PinakesApi.kt | 6 + .../app/data/repository/AuthRepository.kt | 12 ++ .../app/data/repository/ProfileRepository.kt | 8 +- .../app/ui/screens/login/RegisterScreen.kt | 129 ++++++++++++++- .../app/ui/screens/profile/ProfileScreen.kt | 156 +++++++++++++++++- .../ui/screens/profile/ProfileViewModel.kt | 71 +++++++- i18n/de.json | 6 + i18n/en.json | 6 + i18n/fr.json | 6 + i18n/it.json | 6 + 11 files changed, 458 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/com/pinakes/app/data/model/Models.kt b/app/src/main/java/com/pinakes/app/data/model/Models.kt index bad397f..6a60ed5 100644 --- a/app/src/main/java/com/pinakes/app/data/model/Models.kt +++ b/app/src/main/java/com/pinakes/app/data/model/Models.kt @@ -76,11 +76,55 @@ data class RegisterRequest( val nome: String, val cognome: String, val email: String, + // telefono/indirizzo stay non-null but the screen may send "" when the instance + // marks them optional (their required-ness is driven by the discovery schema). val telefono: String, val indirizzo: String, val password: String, // min 8, max 72 @SerialName("password_confirm") val passwordConfirm: String, @SerialName("privacy_acceptance") val privacyAcceptance: Boolean, + // Instance-defined custom registration fields, keyed by the field id (as a string) → + // scalar value. checkbox → "1"/"". Omitted when the instance defines none (explicitNulls + // = false drops the null). See [RegistrationFieldsPayload.customFields]. + @SerialName("custom_fields") val customFields: Map? = null, +) + +// ---------- Registration discovery ---------- +// Mirrors GET /auth/registration-fields (public, no token). Tells the client which built-in +// fields the instance requires and what extra custom fields to render on the sign-up form. +@Serializable +data class RegistrationFieldsPayload( + @SerialName("registration_enabled") val registrationEnabled: Boolean = false, + // Config-driven built-ins only: keys are "cognome" | "telefono" | "indirizzo". + @SerialName("builtin_fields") val builtinFields: Map = emptyMap(), + @SerialName("custom_fields") val customFields: List = emptyList(), +) + +@Serializable +data class BuiltinFieldRule( + val required: Boolean = false, + val configurable: Boolean = false, +) + +// One active custom field definition (matches RegistrationFields::apiDefinitions()). `options` +// is tolerated for forward-compat only — the current server emits neither it nor a select type. +@Serializable +data class CustomFieldDef( + val id: Int = 0, + val label: String = "", + val type: String = "text", // text | textarea | email | url | number | checkbox + val required: Boolean = false, + val options: List = emptyList(), +) + +// A custom field with the current user's value (matches editableFieldsForUser() in GET /me). +@Serializable +data class CustomFieldValue( + val id: Int = 0, + val label: String = "", + val type: String = "text", // text | textarea | email | url | number | checkbox + val required: Boolean = false, + val value: String = "", ) @Serializable @@ -97,6 +141,19 @@ data class UserProfile( @SerialName("email_verificata") val emailVerificata: Boolean = false, val stato: String? = null, @SerialName("avatar_url") val avatarUrl: String? = null, + // Editable profile fields GET /me returns (previously dropped by the model). + val telefono: String? = null, + val indirizzo: String? = null, + @SerialName("data_nascita") val dataNascita: String? = null, // "yyyy-MM-dd" + val sesso: String? = null, + @SerialName("cod_fiscale") val codFiscale: String? = null, + // Read-only membership fields. + @SerialName("codice_tessera") val codiceTessera: String? = null, + @SerialName("card_expires_at") val cardExpiresAt: String? = null, + @SerialName("last_access_at") val lastAccessAt: String? = null, + val locale: String? = null, + // Instance-defined custom fields with the user's current values. + @SerialName("custom_fields") val customFields: List = emptyList(), ) { val fullName: String get() = listOf(nome, cognome).filter { it.isNotBlank() }.joinToString(" ") } @@ -105,6 +162,15 @@ data class UserProfile( data class UpdateProfileRequest( val nome: String? = null, val cognome: String? = null, + val telefono: String? = null, + val indirizzo: String? = null, + @SerialName("data_nascita") val dataNascita: String? = null, // "yyyy-MM-dd" + @SerialName("cod_fiscale") val codFiscale: String? = null, + val sesso: String? = null, + val locale: String? = null, + // Only sent ids change; "" clears a field. Null → omitted (explicitNulls = false) so PATCH + // stays partial. + @SerialName("custom_fields") val customFields: Map? = null, ) @Serializable diff --git a/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt b/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt index dbcf9da..d3a035e 100644 --- a/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt +++ b/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt @@ -22,6 +22,7 @@ import com.pinakes.app.data.model.ReviewRequest import com.pinakes.app.data.model.PushPrefs import com.pinakes.app.data.model.PushSubscribeRequest import com.pinakes.app.data.model.RegisterRequest +import com.pinakes.app.data.model.RegistrationFieldsPayload import com.pinakes.app.data.model.ReservationItem import com.pinakes.app.data.model.ReservationRequest import com.pinakes.app.data.model.UpdateProfileRequest @@ -62,6 +63,11 @@ interface PinakesApi { @Headers(NO_AUTH) suspend fun register(@Body body: RegisterRequest): Envelope + /** Public discovery of the sign-up form: which built-ins are required + custom fields. */ + @GET("auth/registration-fields") + @Headers(NO_AUTH) + suspend fun registrationFields(): Envelope + @POST("auth/forgot-password") @Headers(NO_AUTH) suspend fun forgotPassword(@Body body: ForgotRequest): Envelope diff --git a/app/src/main/java/com/pinakes/app/data/repository/AuthRepository.kt b/app/src/main/java/com/pinakes/app/data/repository/AuthRepository.kt index eb0cbae..15e007d 100644 --- a/app/src/main/java/com/pinakes/app/data/repository/AuthRepository.kt +++ b/app/src/main/java/com/pinakes/app/data/repository/AuthRepository.kt @@ -5,6 +5,7 @@ import com.pinakes.app.data.model.ForgotRequest import com.pinakes.app.data.model.HealthPayload import com.pinakes.app.data.model.LoginRequest import com.pinakes.app.data.model.RegisterRequest +import com.pinakes.app.data.model.RegistrationFieldsPayload import com.pinakes.app.data.network.ApiResult import com.pinakes.app.data.network.NetworkModule import com.pinakes.app.data.network.apiCall @@ -121,6 +122,15 @@ class AuthRepository( } } + /** + * Public discovery of the sign-up form schema (built-in required flags + custom fields) for + * the committed instance. No token required. + */ + suspend fun registrationFields(): ApiResult { + val api = network.api() + return apiCall { api.registrationFields() } + } + suspend fun register( nome: String, cognome: String, @@ -130,6 +140,7 @@ class AuthRepository( password: String, passwordConfirm: String, privacyAccepted: Boolean, + customFields: Map? = null, ): ApiResult { val api = network.api() return apiCall { @@ -143,6 +154,7 @@ class AuthRepository( password = password, passwordConfirm = passwordConfirm, privacyAcceptance = privacyAccepted, + customFields = customFields?.takeIf { it.isNotEmpty() }, ) ) } diff --git a/app/src/main/java/com/pinakes/app/data/repository/ProfileRepository.kt b/app/src/main/java/com/pinakes/app/data/repository/ProfileRepository.kt index 73c452d..f955f73 100644 --- a/app/src/main/java/com/pinakes/app/data/repository/ProfileRepository.kt +++ b/app/src/main/java/com/pinakes/app/data/repository/ProfileRepository.kt @@ -21,11 +21,15 @@ class ProfileRepository(private val network: NetworkModule) { return apiCall { api.me() } } - suspend fun updateProfile(nome: String?, cognome: String?): ApiResult { + /** + * PATCH /me with only the changed fields. Null fields are omitted (partial PATCH via + * explicitNulls = false); a custom field mapped to "" clears that one field. + */ + suspend fun updateProfile(request: UpdateProfileRequest): ApiResult { val api = network.api() // PATCH /me returns null/UserProfile. Model it as Unit so apiCall never tries to cast a // null body to UserProfile, then re-fetch the canonical profile on success. - return when (val res = apiCall { api.updateMe(UpdateProfileRequest(nome, cognome)).asUnit() }) { + return when (val res = apiCall { api.updateMe(request).asUnit() }) { is ApiResult.Success -> me() is ApiResult.Failure -> res } diff --git a/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt index 6571383..af37ad6 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt @@ -8,6 +8,7 @@ import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.widthIn import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.text.KeyboardActions @@ -16,6 +17,7 @@ import androidx.compose.foundation.verticalScroll import androidx.compose.material3.Checkbox import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Surface +import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue @@ -31,6 +33,8 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.lifecycle.viewModelScope import com.pinakes.app.R +import com.pinakes.app.data.model.BuiltinFieldRule +import com.pinakes.app.data.model.CustomFieldDef import com.pinakes.app.data.network.ApiResult import com.pinakes.app.data.repository.AuthRepository import com.pinakes.app.ui.common.AppViewModel @@ -60,13 +64,39 @@ data class RegisterUiState( val sent: Boolean = false, val error: String? = null, val errorRes: Int? = null, -) + // Discovery schema (GET /auth/registration-fields). Empty until fetched. + val builtinFields: Map = emptyMap(), + val customFields: List = emptyList(), + // Custom-field values keyed by CustomFieldDef.id. checkbox → "1"/"". + val customValues: Map = emptyMap(), +) { + /** A config-driven built-in is required unless the instance explicitly opts it out. */ + fun builtinRequired(key: String): Boolean = builtinFields[key]?.required ?: true +} @HiltViewModel class RegisterViewModel @Inject constructor(private val auth: AuthRepository) : ViewModel() { private val _state = MutableStateFlow(RegisterUiState()) val state: StateFlow = _state.asStateFlow() + init { loadSchema() } + + /** + * Fetch the sign-up form schema so required-asterisks and the dynamic custom-field section + * reflect this instance. On failure the built-in defaults (all required) still let the user + * register — the server remains the authority. + */ + fun loadSchema() { + viewModelScope.launch { + when (val res = auth.registrationFields()) { + is ApiResult.Success -> _state.update { + it.copy(builtinFields = res.data.builtinFields, customFields = res.data.customFields) + } + is ApiResult.Failure -> { /* keep built-in defaults; never block registration */ } + } + } + } + fun onNomeChange(value: String) = update { it.copy(nome = value) } fun onCognomeChange(value: String) = update { it.copy(cognome = value) } fun onEmailChange(value: String) = update { it.copy(email = value) } @@ -75,6 +105,7 @@ class RegisterViewModel @Inject constructor(private val auth: AuthRepository) : fun onPasswordChange(value: String) = update { it.copy(password = value) } fun onPasswordConfirmChange(value: String) = update { it.copy(passwordConfirm = value) } fun onPrivacyChange(value: Boolean) = update { it.copy(privacyAccepted = value) } + fun onCustomFieldChange(id: Int, value: String) = update { it.copy(customValues = it.customValues + (id to value)) } private fun update(block: (RegisterUiState) -> RegisterUiState) { _state.update { block(it).copy(error = null, errorRes = null) } @@ -85,9 +116,24 @@ class RegisterViewModel @Inject constructor(private val auth: AuthRepository) : // done — account creation is not idempotent. if (_state.value.loading || _state.value.sent) return val s = _state.value + // Build the custom_fields payload: text-like → trimmed value, checkbox → "1"/"". + val customPayload: Map = s.customFields.associate { def -> + val raw = s.customValues[def.id].orEmpty() + val value = if (def.type == "checkbox") (if (raw == "1") "1" else "") else raw.trim() + def.id.toString() to value + } + // A required custom field must be filled (checkbox required → must be checked). + val missingCustom = s.customFields.any { def -> + def.required && (customPayload[def.id.toString()].orEmpty().isEmpty()) + } val validation = when { - s.nome.isBlank() || s.cognome.isBlank() || s.email.isBlank() || - s.telefono.isBlank() || s.indirizzo.isBlank() -> R.string.register_error_required + // nome/email/password are always required; cognome/telefono/indirizzo only when the + // instance requires them. + s.nome.isBlank() || s.email.isBlank() || + (s.builtinRequired("cognome") && s.cognome.isBlank()) || + (s.builtinRequired("telefono") && s.telefono.isBlank()) || + (s.builtinRequired("indirizzo") && s.indirizzo.isBlank()) || + missingCustom -> R.string.register_error_required // Mirror the backend (AuthController) rules so the user gets a clear // client-side error instead of a server 422: 8-72 chars, plus at least // one uppercase, one lowercase and one digit. @@ -114,6 +160,7 @@ class RegisterViewModel @Inject constructor(private val auth: AuthRepository) : password = s.password, passwordConfirm = s.passwordConfirm, privacyAccepted = s.privacyAccepted, + customFields = customPayload, )) { is ApiResult.Success -> _state.update { it.copy(loading = false, sent = true) } is ApiResult.Failure -> _state.update { @@ -194,7 +241,7 @@ fun RegisterScreen(onBackToLogin: () -> Unit) { PinakesTextField( value = state.nome, onValueChange = vm::onNomeChange, - label = stringResource(R.string.profile_first_name), + label = requiredLabel(stringResource(R.string.profile_first_name), true), modifier = form, keyboardOptions = KeyboardOptions(imeAction = ImeAction.Next), ) @@ -202,7 +249,7 @@ fun RegisterScreen(onBackToLogin: () -> Unit) { PinakesTextField( value = state.cognome, onValueChange = vm::onCognomeChange, - label = stringResource(R.string.profile_last_name), + label = requiredLabel(stringResource(R.string.profile_last_name), state.builtinRequired("cognome")), modifier = form, keyboardOptions = KeyboardOptions(imeAction = ImeAction.Next), ) @@ -210,7 +257,7 @@ fun RegisterScreen(onBackToLogin: () -> Unit) { PinakesTextField( value = state.email, onValueChange = vm::onEmailChange, - label = stringResource(R.string.login_email_label), + label = requiredLabel(stringResource(R.string.login_email_label), true), modifier = form, keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Email, imeAction = ImeAction.Next), ) @@ -218,7 +265,7 @@ fun RegisterScreen(onBackToLogin: () -> Unit) { PinakesTextField( value = state.telefono, onValueChange = vm::onTelefonoChange, - label = stringResource(R.string.register_phone), + label = requiredLabel(stringResource(R.string.register_phone), state.builtinRequired("telefono")), modifier = form, keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Phone, imeAction = ImeAction.Next), ) @@ -226,10 +273,20 @@ fun RegisterScreen(onBackToLogin: () -> Unit) { PinakesTextField( value = state.indirizzo, onValueChange = vm::onIndirizzoChange, - label = stringResource(R.string.register_address), + label = requiredLabel(stringResource(R.string.register_address), state.builtinRequired("indirizzo")), modifier = form, keyboardOptions = KeyboardOptions(imeAction = ImeAction.Next), ) + // Instance-defined custom fields, rendered by type. + state.customFields.forEach { def -> + Spacer(Modifier.height(Spacing.md)) + CustomFieldInput( + def = def, + value = state.customValues[def.id].orEmpty(), + onValueChange = { vm.onCustomFieldChange(def.id, it) }, + modifier = form, + ) + } Spacer(Modifier.height(Spacing.md)) PasswordField( value = state.password, @@ -280,3 +337,59 @@ fun RegisterScreen(onBackToLogin: () -> Unit) { } } } + +/** Appends a " *" marker to a field label when the instance requires it. */ +private fun requiredLabel(label: String, required: Boolean): String = if (required) "$label *" else label + +/** + * Renders one instance-defined custom field by its [CustomFieldDef.type]: + * text/email/url/number → single-line input with matching keyboard; textarea → multiline; + * checkbox → a labelled [Switch] whose value is carried as "1"/"". + */ +@Composable +fun CustomFieldInput( + def: CustomFieldDef, + value: String, + onValueChange: (String) -> Unit, + modifier: Modifier = Modifier, +) { + val label = requiredLabel(def.label, def.required) + when (def.type) { + "checkbox" -> { + Row(modifier = modifier, verticalAlignment = Alignment.CenterVertically) { + Switch(checked = value == "1", onCheckedChange = { onValueChange(if (it) "1" else "") }) + Spacer(Modifier.width(Spacing.md)) + Text( + text = label, + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + "textarea" -> { + PinakesTextField( + value = value, + onValueChange = onValueChange, + label = label, + modifier = modifier, + singleLine = false, + keyboardOptions = KeyboardOptions(imeAction = ImeAction.Default), + ) + } + else -> { + val keyboardType = when (def.type) { + "email" -> KeyboardType.Email + "url" -> KeyboardType.Uri + "number" -> KeyboardType.Number + else -> KeyboardType.Text + } + PinakesTextField( + value = value, + onValueChange = onValueChange, + label = label, + modifier = modifier, + keyboardOptions = KeyboardOptions(keyboardType = keyboardType, imeAction = ImeAction.Next), + ) + } + } +} diff --git a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt index acd95bb..60f57f4 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt @@ -15,10 +15,12 @@ import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.CircleShape +import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.outlined.Logout import androidx.compose.material.icons.outlined.BrightnessMedium +import androidx.compose.material.icons.outlined.CalendarMonth import androidx.compose.material.icons.outlined.ChatBubbleOutline import androidx.compose.material.icons.outlined.DevicesOther import androidx.compose.material.icons.outlined.Edit @@ -30,9 +32,13 @@ import androidx.compose.material.icons.outlined.Notifications import androidx.compose.material.icons.outlined.PhoneAndroid import androidx.compose.material.icons.outlined.RateReview import androidx.compose.material3.AlertDialog +import androidx.compose.material3.DatePicker +import androidx.compose.material3.DatePickerDialog +import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.RadioButton +import androidx.compose.material3.rememberDatePickerState import androidx.compose.material3.Scaffold import androidx.compose.material3.SnackbarHost import androidx.compose.material3.SnackbarHostState @@ -49,15 +55,23 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.input.ImeAction +import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp +import java.time.Instant +import java.time.LocalDate +import java.time.ZoneOffset import androidx.core.os.LocaleListCompat import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.pinakes.app.R +import com.pinakes.app.data.model.CustomFieldDef +import com.pinakes.app.data.model.CustomFieldValue import com.pinakes.app.data.model.DeviceItem import com.pinakes.app.data.model.UserProfile import com.pinakes.app.ui.common.AppViewModel +import com.pinakes.app.ui.screens.login.CustomFieldInput import com.pinakes.app.ui.common.DateFormat import com.pinakes.app.ui.common.UiState import com.pinakes.app.ui.common.resolvedMessage @@ -116,12 +130,28 @@ fun ProfileScreen( } if (state.editing) { + val editingProfile = (state.profile as? UiState.Success)?.data EditProfileDialog( nome = state.editNome, cognome = state.editCognome, + telefono = state.editTelefono, + indirizzo = state.editIndirizzo, + dataNascita = state.editDataNascita, + codFiscale = state.editCodFiscale, + sesso = state.editSesso, + telefonoRequired = vm.builtinRequired("telefono"), + indirizzoRequired = vm.builtinRequired("indirizzo"), + customFields = editingProfile?.customFields.orEmpty(), + customValues = state.editCustomValues, saving = state.savingProfile, onNome = vm::onEditNome, onCognome = vm::onEditCognome, + onTelefono = vm::onEditTelefono, + onIndirizzo = vm::onEditIndirizzo, + onDataNascita = vm::onEditDataNascita, + onCodFiscale = vm::onEditCodFiscale, + onSesso = vm::onEditSesso, + onCustomField = vm::onEditCustomField, onSave = vm::saveEdit, onDismiss = vm::cancelEdit, ) @@ -202,6 +232,23 @@ private fun ProfileContent( ) } + // Read-only membership info (only shown when the server provides it). + val cardNumber = profile.codiceTessera?.takeIf { it.isNotBlank() } + val cardExpiry = profile.cardExpiresAt?.takeIf { it.isNotBlank() } + if (cardNumber != null || cardExpiry != null) { + Spacer(Modifier.height(Spacing.xl)) + Surface( + shape = MaterialTheme.shapes.medium, + color = MaterialTheme.colorScheme.surfaceContainerLow, + modifier = Modifier.fillMaxWidth(), + ) { + Column(Modifier.padding(Spacing.lg), verticalArrangement = Arrangement.spacedBy(Spacing.xs)) { + cardNumber?.let { ReadOnlyRow(stringResource(R.string.profile_card_number), it) } + cardExpiry?.let { ReadOnlyRow(stringResource(R.string.profile_card_expires), DateFormat.date(it)) } + } + } + } + Spacer(Modifier.height(Spacing.xl)) // Actions @@ -400,6 +447,14 @@ private fun ThemeSection() { } } +@Composable +private fun ReadOnlyRow(label: String, value: String) { + Row(Modifier.fillMaxWidth(), verticalAlignment = Alignment.CenterVertically) { + Text(label, style = MaterialTheme.typography.labelMedium, color = MaterialTheme.colorScheme.onSurfaceVariant, modifier = Modifier.weight(1f)) + Text(value, style = MaterialTheme.typography.bodyMedium, color = MaterialTheme.colorScheme.onSurface) + } +} + @Composable private fun ActionRow(icon: ImageVector, label: String, onClick: () -> Unit) { Surface( @@ -450,9 +505,24 @@ private fun DeviceRow(device: DeviceItem, onRevoke: () -> Unit) { private fun EditProfileDialog( nome: String, cognome: String, + telefono: String, + indirizzo: String, + dataNascita: String, + codFiscale: String, + sesso: String, + telefonoRequired: Boolean, + indirizzoRequired: Boolean, + customFields: List, + customValues: Map, saving: Boolean, onNome: (String) -> Unit, onCognome: (String) -> Unit, + onTelefono: (String) -> Unit, + onIndirizzo: (String) -> Unit, + onDataNascita: (String) -> Unit, + onCodFiscale: (String) -> Unit, + onSesso: (String) -> Unit, + onCustomField: (Int, String) -> Unit, onSave: () -> Unit, onDismiss: () -> Unit, ) { @@ -461,9 +531,41 @@ private fun EditProfileDialog( shape = MaterialTheme.shapes.large, title = { Text(stringResource(R.string.profile_edit_title), style = MaterialTheme.typography.titleMedium) }, text = { - Column(verticalArrangement = Arrangement.spacedBy(Spacing.md)) { + Column( + Modifier.verticalScroll(rememberScrollState()), + verticalArrangement = Arrangement.spacedBy(Spacing.md), + ) { PinakesTextField(value = nome, onValueChange = onNome, label = stringResource(R.string.profile_first_name), modifier = Modifier.fillMaxWidth()) PinakesTextField(value = cognome, onValueChange = onCognome, label = stringResource(R.string.profile_last_name), modifier = Modifier.fillMaxWidth()) + PinakesTextField( + value = telefono, + onValueChange = onTelefono, + label = requiredLabel(stringResource(R.string.register_phone), telefonoRequired), + modifier = Modifier.fillMaxWidth(), + keyboardOptions = KeyboardOptions(keyboardType = KeyboardType.Phone, imeAction = ImeAction.Next), + ) + PinakesTextField( + value = indirizzo, + onValueChange = onIndirizzo, + label = requiredLabel(stringResource(R.string.register_address), indirizzoRequired), + modifier = Modifier.fillMaxWidth(), + ) + DateField( + value = dataNascita, + label = stringResource(R.string.profile_birth_date), + onValueChange = onDataNascita, + ) + PinakesTextField(value = codFiscale, onValueChange = onCodFiscale, label = stringResource(R.string.profile_tax_code), modifier = Modifier.fillMaxWidth()) + PinakesTextField(value = sesso, onValueChange = onSesso, label = stringResource(R.string.profile_gender), modifier = Modifier.fillMaxWidth()) + // Instance-defined custom fields, pre-filled from the user's current values. + customFields.forEach { f -> + CustomFieldInput( + def = CustomFieldDef(id = f.id, label = f.label, type = f.type, required = f.required), + value = customValues[f.id].orEmpty(), + onValueChange = { onCustomField(f.id, it) }, + modifier = Modifier.fillMaxWidth(), + ) + } } }, confirmButton = { PrimaryButton(label = stringResource(R.string.action_save), onClick = onSave, loading = saving) }, @@ -471,6 +573,58 @@ private fun EditProfileDialog( ) } +/** Appends a " *" marker to a field label when the instance requires it. */ +private fun requiredLabel(label: String, required: Boolean): String = if (required) "$label *" else label + +/** + * A read/tap field showing a yyyy-MM-dd date; tapping opens a Material3 date picker (any date, + * past-friendly for a birth date). Empty value shows nothing selected. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +private fun DateField(value: String, label: String, onValueChange: (String) -> Unit) { + var pickerOpen by remember { mutableStateOf(false) } + Surface( + onClick = { pickerOpen = true }, + shape = MaterialTheme.shapes.medium, + color = MaterialTheme.colorScheme.surfaceContainerLow, + modifier = Modifier.fillMaxWidth(), + ) { + Row(Modifier.padding(Spacing.lg), verticalAlignment = Alignment.CenterVertically) { + Column(Modifier.weight(1f)) { + Text(label, style = MaterialTheme.typography.labelMedium, color = MaterialTheme.colorScheme.onSurfaceVariant) + Text( + value.ifBlank { stringResource(R.string.profile_date_none) }, + style = MaterialTheme.typography.bodyLarge, + color = MaterialTheme.colorScheme.onSurface, + ) + } + Icon(Icons.Outlined.CalendarMonth, contentDescription = null, tint = MaterialTheme.colorScheme.onSurfaceVariant, modifier = Modifier.size(20.dp)) + } + } + + if (pickerOpen) { + val initialMillis = value.takeIf { it.isNotBlank() }?.let { + runCatching { LocalDate.parse(it).atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli() }.getOrNull() + } + val pickerState = rememberDatePickerState(initialSelectedDateMillis = initialMillis) + DatePickerDialog( + onDismissRequest = { pickerOpen = false }, + confirmButton = { + TextButton(onClick = { + pickerState.selectedDateMillis?.let { + onValueChange(Instant.ofEpochMilli(it).atZone(ZoneOffset.UTC).toLocalDate().toString()) + } + pickerOpen = false + }) { Text(stringResource(R.string.action_save)) } + }, + dismissButton = { TextButton(onClick = { pickerOpen = false }) { Text(stringResource(R.string.action_cancel)) } }, + ) { + DatePicker(state = pickerState, showModeToggle = false) + } + } +} + @Composable private fun ChangePasswordDialog( current: String, diff --git a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt index 4c145e1..1cb3fb9 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt @@ -2,7 +2,9 @@ package com.pinakes.app.ui.screens.profile import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import com.pinakes.app.data.model.BuiltinFieldRule import com.pinakes.app.data.model.DeviceItem +import com.pinakes.app.data.model.UpdateProfileRequest import com.pinakes.app.data.model.UserProfile import com.pinakes.app.data.network.ApiResult import com.pinakes.app.data.network.ErrorCodes @@ -26,6 +28,15 @@ data class ProfileUiState( val editing: Boolean = false, val editNome: String = "", val editCognome: String = "", + val editTelefono: String = "", + val editIndirizzo: String = "", + val editDataNascita: String = "", + val editCodFiscale: String = "", + val editSesso: String = "", + // Custom-field edit values keyed by field id (checkbox → "1"/""). + val editCustomValues: Map = emptyMap(), + // Required-ness for telefono/indirizzo comes from the registration schema. + val builtinFields: Map = emptyMap(), val savingProfile: Boolean = false, // Password dialog val changingPassword: Boolean = false, @@ -49,7 +60,19 @@ class ProfileViewModel @Inject constructor( private val _state = MutableStateFlow(ProfileUiState()) val state: StateFlow = _state.asStateFlow() - init { load(); loadDevices() } + init { load(); loadDevices(); loadSchema() } + + /** Fetch the registration schema so telefono/indirizzo required-ness matches the instance. */ + fun loadSchema() { + viewModelScope.launch { + when (val res = auth.registrationFields()) { + is ApiResult.Success -> _state.update { it.copy(builtinFields = res.data.builtinFields) } + is ApiResult.Failure -> { /* keep defaults */ } + } + } + } + + fun builtinRequired(key: String): Boolean = _state.value.builtinFields[key]?.required ?: true fun load() { _state.update { it.copy(profile = UiState.Loading) } @@ -76,17 +99,59 @@ class ProfileViewModel @Inject constructor( // ---- Edit profile ---- fun startEdit() { val p = (_state.value.profile as? UiState.Success)?.data ?: return - _state.update { it.copy(editing = true, editNome = p.nome, editCognome = p.cognome) } + _state.update { + it.copy( + editing = true, + editNome = p.nome, + editCognome = p.cognome, + editTelefono = p.telefono.orEmpty(), + editIndirizzo = p.indirizzo.orEmpty(), + editDataNascita = p.dataNascita.orEmpty(), + editCodFiscale = p.codFiscale.orEmpty(), + editSesso = p.sesso.orEmpty(), + editCustomValues = p.customFields.associate { f -> f.id to f.value }, + ) + } } fun cancelEdit() = _state.update { it.copy(editing = false) } fun onEditNome(v: String) = _state.update { it.copy(editNome = v) } fun onEditCognome(v: String) = _state.update { it.copy(editCognome = v) } + fun onEditTelefono(v: String) = _state.update { it.copy(editTelefono = v) } + fun onEditIndirizzo(v: String) = _state.update { it.copy(editIndirizzo = v) } + fun onEditDataNascita(v: String) = _state.update { it.copy(editDataNascita = v) } + fun onEditCodFiscale(v: String) = _state.update { it.copy(editCodFiscale = v) } + fun onEditSesso(v: String) = _state.update { it.copy(editSesso = v) } + fun onEditCustomField(id: Int, v: String) = _state.update { it.copy(editCustomValues = it.editCustomValues + (id to v)) } fun saveEdit() { val s = _state.value + val original = (s.profile as? UiState.Success)?.data + // Build a partial PATCH: only include a built-in when it actually changed (null → omitted). + fun changed(new: String, old: String?): String? = new.trim().takeIf { it != old.orEmpty() } + // Custom fields: only the ids whose value differs from the loaded one. + val customPatch: Map = original?.customFields + ?.mapNotNull { def -> + val edited = s.editCustomValues[def.id].orEmpty() + val normalized = if (def.type == "checkbox") (if (edited == "1") "1" else "") else edited.trim() + if (normalized != def.value) def.id.toString() to normalized else null + } + ?.toMap() + .orEmpty() + + val request = UpdateProfileRequest( + nome = changed(s.editNome, original?.nome), + cognome = changed(s.editCognome, original?.cognome), + telefono = changed(s.editTelefono, original?.telefono), + indirizzo = changed(s.editIndirizzo, original?.indirizzo), + dataNascita = changed(s.editDataNascita, original?.dataNascita), + codFiscale = changed(s.editCodFiscale, original?.codFiscale), + sesso = changed(s.editSesso, original?.sesso), + customFields = customPatch.takeIf { it.isNotEmpty() }, + ) + _state.update { it.copy(savingProfile = true) } viewModelScope.launch { - when (val res = profile.updateProfile(s.editNome.trim(), s.editCognome.trim())) { + when (val res = profile.updateProfile(request)) { is ApiResult.Success -> _state.update { it.copy(profile = UiState.Success(res.data), savingProfile = false, editing = false, snackbar = null, snackbarRes = R.string.profile_updated) } diff --git a/i18n/de.json b/i18n/de.json index ce3e8dd..cef6daf 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -193,6 +193,12 @@ "profile_edit_title": "Profil bearbeiten", "profile_first_name": "Vorname", "profile_last_name": "Nachname", + "profile_birth_date": "Geburtsdatum", + "profile_tax_code": "Steuernummer", + "profile_gender": "Geschlecht", + "profile_date_none": "Nicht festgelegt", + "profile_card_number": "Ausweisnummer", + "profile_card_expires": "Ausweis läuft ab", "profile_change_password_title": "Passwort ändern", "profile_current_password": "Aktuelles Passwort", "profile_new_password": "Neues Passwort", diff --git a/i18n/en.json b/i18n/en.json index 8e67fc0..1f53115 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -193,6 +193,12 @@ "profile_edit_title": "Edit profile", "profile_first_name": "First name", "profile_last_name": "Last name", + "profile_birth_date": "Date of birth", + "profile_tax_code": "Tax code", + "profile_gender": "Gender", + "profile_date_none": "Not set", + "profile_card_number": "Card number", + "profile_card_expires": "Card expires", "profile_change_password_title": "Change password", "profile_current_password": "Current password", "profile_new_password": "New password", diff --git a/i18n/fr.json b/i18n/fr.json index d4ff1dd..5f5c2cf 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -193,6 +193,12 @@ "profile_edit_title": "Modifier le profil", "profile_first_name": "Prénom", "profile_last_name": "Nom", + "profile_birth_date": "Date de naissance", + "profile_tax_code": "Code fiscal", + "profile_gender": "Sexe", + "profile_date_none": "Non renseignée", + "profile_card_number": "Numéro de carte", + "profile_card_expires": "Expiration de la carte", "profile_change_password_title": "Changer le mot de passe", "profile_current_password": "Mot de passe actuel", "profile_new_password": "Nouveau mot de passe", diff --git a/i18n/it.json b/i18n/it.json index 2d00d6a..2103d57 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -193,6 +193,12 @@ "profile_edit_title": "Modifica profilo", "profile_first_name": "Nome", "profile_last_name": "Cognome", + "profile_birth_date": "Data di nascita", + "profile_tax_code": "Codice fiscale", + "profile_gender": "Sesso", + "profile_date_none": "Non impostata", + "profile_card_number": "Numero tessera", + "profile_card_expires": "Scadenza tessera", "profile_change_password_title": "Cambia password", "profile_current_password": "Password attuale", "profile_new_password": "Nuova password", From 88cc03d1721102ed97436fd5492ef545c330c8b5 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 21 Jul 2026 17:52:21 +0200 Subject: [PATCH 2/4] fix(#26 review): schema loading gate, fetch-failure notice, profile required validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adams-review UX follow-ups on the configurable/custom fields work: 1. Registration schema loading gate. loadSchema() ran async in init with no flag gating submit, so a fast user could submit before the custom-field section appeared → the required-custom-field check passed vacuously → opaque server 422 on a field never shown. Added schemaLoading (starts true); submit() returns early while it's set, and the submit button shows a spinner. 2. Silent schema-fetch failure. On failure the code kept built-in defaults with no signal. Added schemaFailed → an inline notice + Retry (loadSchema) so the user knows extra fields may be missing. A failed fetch clears schemaLoading, so a schema-down instance still lets the user register with the defaults (preserves the "never block registration" intent). 3. Profile-edit required validation. The edit dialog showed asterisks but Save always fired, relying on a server 422. saveEdit() now validates required telefono/indirizzo (per the instance schema) + required custom fields client-side and shows an inline error (editErrorRes), cleared as the user fixes the field — mirroring the register flow. New string register_schema_failed added to all four locales (en/it/fr/de); reuses existing action_retry + register_error_required. Design-system-consistent (PinakesTextField/PinakesTextButton/Surface/Spacing tokens). Static review only — needs a Gradle build to confirm compilation. --- .../app/ui/screens/login/RegisterScreen.kt | 57 +++++++++++++++++-- .../app/ui/screens/profile/ProfileScreen.kt | 10 ++++ .../ui/screens/profile/ProfileViewModel.kt | 28 ++++++++- i18n/de.json | 3 +- i18n/en.json | 3 +- i18n/fr.json | 3 +- i18n/it.json | 3 +- 7 files changed, 95 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt index af37ad6..0dade99 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt @@ -69,6 +69,14 @@ data class RegisterUiState( val customFields: List = emptyList(), // Custom-field values keyed by CustomFieldDef.id. checkbox → "1"/"". val customValues: Map = emptyMap(), + // True while the initial schema fetch is in flight. Gates submit so a fast + // user can't submit before the custom-field section has appeared (which + // would make the required-custom-field check pass vacuously). Starts true. + val schemaLoading: Boolean = true, + // True when the schema fetch failed. Registration still works with the + // built-in defaults (server stays authoritative), but the UI surfaces a + // retry so the user knows extra fields may be missing. + val schemaFailed: Boolean = false, ) { /** A config-driven built-in is required unless the instance explicitly opts it out. */ fun builtinRequired(key: String): Boolean = builtinFields[key]?.required ?: true @@ -87,12 +95,24 @@ class RegisterViewModel @Inject constructor(private val auth: AuthRepository) : * register — the server remains the authority. */ fun loadSchema() { + _state.update { it.copy(schemaLoading = true, schemaFailed = false) } viewModelScope.launch { when (val res = auth.registrationFields()) { is ApiResult.Success -> _state.update { - it.copy(builtinFields = res.data.builtinFields, customFields = res.data.customFields) + it.copy( + builtinFields = res.data.builtinFields, + customFields = res.data.customFields, + schemaLoading = false, + schemaFailed = false, + ) + } + // Keep built-in defaults so registration is never blocked, but + // flag the failure so the UI can offer a retry — otherwise + // required custom fields would silently never appear and the + // user would hit an opaque server 422. + is ApiResult.Failure -> _state.update { + it.copy(schemaLoading = false, schemaFailed = true) } - is ApiResult.Failure -> { /* keep built-in defaults; never block registration */ } } } } @@ -113,8 +133,12 @@ class RegisterViewModel @Inject constructor(private val auth: AuthRepository) : fun submit() { // Guard against duplicate submits while a request is in flight or already - // done — account creation is not idempotent. - if (_state.value.loading || _state.value.sent) return + // done — account creation is not idempotent. Also block while the schema + // is still loading: submitting before the custom-field section appears + // would make the required-custom-field check below pass vacuously and + // land an opaque server 422. (A failed fetch clears schemaLoading, so a + // schema-down instance still lets the user register with the defaults.) + if (_state.value.loading || _state.value.sent || _state.value.schemaLoading) return val s = _state.value // Build the custom_fields payload: text-like → trimmed value, checkbox → "1"/"". val customPayload: Map = s.customFields.associate { def -> @@ -277,6 +301,26 @@ fun RegisterScreen(onBackToLogin: () -> Unit) { modifier = form, keyboardOptions = KeyboardOptions(imeAction = ImeAction.Next), ) + // Schema fetch failed: registration still works with the built-in + // defaults, but tell the user extra fields may be missing and let + // them retry rather than silently omitting required custom fields. + if (state.schemaFailed) { + Spacer(Modifier.height(Spacing.md)) + Surface(shape = MaterialTheme.shapes.small, color = MaterialTheme.colorScheme.surfaceContainerLow, modifier = form) { + Row( + modifier = Modifier.padding(Spacing.md), + verticalAlignment = Alignment.CenterVertically, + ) { + Text( + text = stringResource(R.string.register_schema_failed), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.weight(1f), + ) + PinakesTextButton(label = stringResource(R.string.action_retry), onClick = vm::loadSchema) + } + } + } // Instance-defined custom fields, rendered by type. state.customFields.forEach { def -> Spacer(Modifier.height(Spacing.md)) @@ -330,7 +374,10 @@ fun RegisterScreen(onBackToLogin: () -> Unit) { label = stringResource(R.string.register_action), onClick = vm::submit, modifier = form, - loading = state.loading, + // Spinner + disabled while the account request is in flight OR + // the schema is still loading — the submit() guard also blocks + // the latter, this just reflects it visually. + loading = state.loading || state.schemaLoading, ) Spacer(Modifier.height(Spacing.sm)) PinakesTextButton(label = stringResource(R.string.auth_back_to_login), onClick = onBackToLogin) diff --git a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt index 60f57f4..e32ac54 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt @@ -144,6 +144,7 @@ fun ProfileScreen( customFields = editingProfile?.customFields.orEmpty(), customValues = state.editCustomValues, saving = state.savingProfile, + errorRes = state.editErrorRes, onNome = vm::onEditNome, onCognome = vm::onEditCognome, onTelefono = vm::onEditTelefono, @@ -515,6 +516,7 @@ private fun EditProfileDialog( customFields: List, customValues: Map, saving: Boolean, + errorRes: Int?, onNome: (String) -> Unit, onCognome: (String) -> Unit, onTelefono: (String) -> Unit, @@ -566,6 +568,14 @@ private fun EditProfileDialog( modifier = Modifier.fillMaxWidth(), ) } + if (errorRes != null) { + Text( + text = stringResource(errorRes), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + modifier = Modifier.fillMaxWidth(), + ) + } } }, confirmButton = { PrimaryButton(label = stringResource(R.string.action_save), onClick = onSave, loading = saving) }, diff --git a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt index 1cb3fb9..e33f4fd 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt @@ -37,6 +37,9 @@ data class ProfileUiState( val editCustomValues: Map = emptyMap(), // Required-ness for telefono/indirizzo comes from the registration schema. val builtinFields: Map = emptyMap(), + // Inline validation error shown in the edit dialog (e.g. a required field + // left blank) — surfaced client-side instead of relying on a server 422. + val editErrorRes: Int? = null, val savingProfile: Boolean = false, // Password dialog val changingPassword: Boolean = false, @@ -102,6 +105,7 @@ class ProfileViewModel @Inject constructor( _state.update { it.copy( editing = true, + editErrorRes = null, editNome = p.nome, editCognome = p.cognome, editTelefono = p.telefono.orEmpty(), @@ -116,16 +120,34 @@ class ProfileViewModel @Inject constructor( fun cancelEdit() = _state.update { it.copy(editing = false) } fun onEditNome(v: String) = _state.update { it.copy(editNome = v) } fun onEditCognome(v: String) = _state.update { it.copy(editCognome = v) } - fun onEditTelefono(v: String) = _state.update { it.copy(editTelefono = v) } - fun onEditIndirizzo(v: String) = _state.update { it.copy(editIndirizzo = v) } + fun onEditTelefono(v: String) = _state.update { it.copy(editTelefono = v, editErrorRes = null) } + fun onEditIndirizzo(v: String) = _state.update { it.copy(editIndirizzo = v, editErrorRes = null) } fun onEditDataNascita(v: String) = _state.update { it.copy(editDataNascita = v) } fun onEditCodFiscale(v: String) = _state.update { it.copy(editCodFiscale = v) } fun onEditSesso(v: String) = _state.update { it.copy(editSesso = v) } - fun onEditCustomField(id: Int, v: String) = _state.update { it.copy(editCustomValues = it.editCustomValues + (id to v)) } + fun onEditCustomField(id: Int, v: String) = _state.update { it.copy(editCustomValues = it.editCustomValues + (id to v), editErrorRes = null) } fun saveEdit() { val s = _state.value val original = (s.profile as? UiState.Success)?.data + + // Client-side required-field validation, mirroring the register flow, so + // clearing a required field gives an immediate, clear message instead of + // an opaque server 422 round-trip. telefono/indirizzo are required only + // when the instance requires them; required custom fields must be filled. + val missingBuiltin = + (builtinRequired("telefono") && s.editTelefono.isBlank()) || + (builtinRequired("indirizzo") && s.editIndirizzo.isBlank()) + val missingCustom = original?.customFields?.any { def -> + val v = s.editCustomValues[def.id].orEmpty() + val normalized = if (def.type == "checkbox") (if (v == "1") "1" else "") else v.trim() + def.required && normalized.isEmpty() + } ?: false + if (missingBuiltin || missingCustom) { + _state.update { it.copy(editErrorRes = R.string.register_error_required) } + return + } + // Build a partial PATCH: only include a built-in when it actually changed (null → omitted). fun changed(new: String, old: String?): String? = new.trim().takeIf { it != old.orEmpty() } // Custom fields: only the ids whose value differs from the loaded one. diff --git a/i18n/de.json b/i18n/de.json index cef6daf..b2d8f27 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -420,5 +420,6 @@ "book_club_privacy_private": "Privat", "book_club_privacy_invite": "Nur mit Einladung", "book_club_privacy_hidden": "Verborgen", - "book_club_error_dashboard": "Dein Lesebereich konnte nicht geladen werden. Zum Aktualisieren ziehen." + "book_club_error_dashboard": "Dein Lesebereich konnte nicht geladen werden. Zum Aktualisieren ziehen.", + "register_schema_failed": "Zusätzliche Registrierungsfelder konnten nicht geladen werden — du kannst dich trotzdem registrieren oder es erneut versuchen." } diff --git a/i18n/en.json b/i18n/en.json index 1f53115..3212d8e 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -420,5 +420,6 @@ "book_club_privacy_private": "Private", "book_club_privacy_invite": "Invite only", "book_club_privacy_hidden": "Hidden", - "book_club_error_dashboard": "Couldn't load your reading section. Pull to refresh." + "book_club_error_dashboard": "Couldn't load your reading section. Pull to refresh.", + "register_schema_failed": "Couldn't load the extra registration fields — you can still sign up, or retry." } diff --git a/i18n/fr.json b/i18n/fr.json index 5f5c2cf..d7ef053 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -420,5 +420,6 @@ "book_club_privacy_private": "Privé", "book_club_privacy_invite": "Sur invitation", "book_club_privacy_hidden": "Masqué", - "book_club_error_dashboard": "Impossible de charger votre section de lecture. Tirez pour actualiser." + "book_club_error_dashboard": "Impossible de charger votre section de lecture. Tirez pour actualiser.", + "register_schema_failed": "Impossible de charger les champs d'inscription supplémentaires — vous pouvez quand même vous inscrire, ou réessayer." } diff --git a/i18n/it.json b/i18n/it.json index 2103d57..e77d759 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -420,5 +420,6 @@ "book_club_privacy_private": "Privato", "book_club_privacy_invite": "Solo su invito", "book_club_privacy_hidden": "Nascosto", - "book_club_error_dashboard": "Impossibile caricare la sezione delle tue letture. Trascina per aggiornare." + "book_club_error_dashboard": "Impossibile caricare la sezione delle tue letture. Trascina per aggiornare.", + "register_schema_failed": "Impossibile caricare i campi di registrazione aggiuntivi — puoi comunque registrarti, oppure riprovare." } From 2d0af8dd1d14a4edd75d007428a440a2f4da913b Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 21 Jul 2026 18:19:36 +0200 Subject: [PATCH 3/4] fix(#26 review): CodeRabbit findings + extract testable validation + unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review of the #255 mobile work: 1. (major, a regression from the earlier review-fix) Profile edit could not be saved when the registration schema was unloaded/failed: builtinRequired() defaulted to true on an empty map, so an optional-but-empty telefono/indirizzo blocked saving. Profile builtinRequired() now defaults to FALSE when the key is absent — the user already has an account, so an unknown schema must not invent a required field; the server stays authoritative. Registration keeps the conservative default-true (unchanged). 2. (minor) saveEdit() now also validates nome (always required) + cognome (when the instance requires it) client-side; onEditNome/onEditCognome clear editErrorRes like the other edit handlers. 3. (minor) The birth-date DatePickerDialog gained a Clear action so an existing date can be removed (PATCH /me accepts the empty value) — previously only confirm/cancel existed. New string action_clear across all four locales. Also: extracted the register submit gate + required-field check into pure functions on RegisterUiState (canSubmit(), hasBlankRequiredField()) so submit() calls them (DRY) and they are unit-testable. Added RegisterUiStateTest (15 JUnit tests) covering builtinRequired defaults, the submit gate (schemaLoading/loading/sent block; ready allows), and required validation (core fields, config-required built-ins, required custom fields incl. checkbox). Pure JUnit, matches the existing test style; still needs a Gradle build to run in CI. --- .../app/ui/screens/login/RegisterScreen.kt | 41 +++++-- .../app/ui/screens/profile/ProfileScreen.kt | 14 ++- .../ui/screens/profile/ProfileViewModel.kt | 18 ++- .../com/pinakes/app/RegisterUiStateTest.kt | 116 ++++++++++++++++++ i18n/de.json | 3 +- i18n/en.json | 3 +- i18n/fr.json | 3 +- i18n/it.json | 3 +- 8 files changed, 180 insertions(+), 21 deletions(-) create mode 100644 app/src/test/java/com/pinakes/app/RegisterUiStateTest.kt diff --git a/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt index 0dade99..276f559 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.kt @@ -80,6 +80,31 @@ data class RegisterUiState( ) { /** A config-driven built-in is required unless the instance explicitly opts it out. */ fun builtinRequired(key: String): Boolean = builtinFields[key]?.required ?: true + + /** + * Submit must not proceed while the account request is in flight, already + * sent, or the schema is still loading (submitting before the custom-field + * section appears would make [hasBlankRequiredField] pass vacuously). + */ + fun canSubmit(): Boolean = !loading && !sent && !schemaLoading + + /** + * True when a required field is blank: nome/email (always required), a + * config-required built-in (cognome/telefono/indirizzo), or a required + * custom field. Password is validated separately (length/strength), so it + * is intentionally not part of this check. Pure — unit-tested. + */ + fun hasBlankRequiredField(): Boolean { + if (nome.isBlank() || email.isBlank()) return true + if (builtinRequired("cognome") && cognome.isBlank()) return true + if (builtinRequired("telefono") && telefono.isBlank()) return true + if (builtinRequired("indirizzo") && indirizzo.isBlank()) return true + return customFields.any { def -> + val raw = customValues[def.id].orEmpty() + val value = if (def.type == "checkbox") (if (raw == "1") "1" else "") else raw.trim() + def.required && value.isEmpty() + } + } } @HiltViewModel @@ -138,7 +163,7 @@ class RegisterViewModel @Inject constructor(private val auth: AuthRepository) : // would make the required-custom-field check below pass vacuously and // land an opaque server 422. (A failed fetch clears schemaLoading, so a // schema-down instance still lets the user register with the defaults.) - if (_state.value.loading || _state.value.sent || _state.value.schemaLoading) return + if (!_state.value.canSubmit()) return val s = _state.value // Build the custom_fields payload: text-like → trimmed value, checkbox → "1"/"". val customPayload: Map = s.customFields.associate { def -> @@ -146,18 +171,10 @@ class RegisterViewModel @Inject constructor(private val auth: AuthRepository) : val value = if (def.type == "checkbox") (if (raw == "1") "1" else "") else raw.trim() def.id.toString() to value } - // A required custom field must be filled (checkbox required → must be checked). - val missingCustom = s.customFields.any { def -> - def.required && (customPayload[def.id.toString()].orEmpty().isEmpty()) - } val validation = when { - // nome/email/password are always required; cognome/telefono/indirizzo only when the - // instance requires them. - s.nome.isBlank() || s.email.isBlank() || - (s.builtinRequired("cognome") && s.cognome.isBlank()) || - (s.builtinRequired("telefono") && s.telefono.isBlank()) || - (s.builtinRequired("indirizzo") && s.indirizzo.isBlank()) || - missingCustom -> R.string.register_error_required + // Required-field check (core + config-required built-ins + required + // custom fields) lives on the state so it's unit-testable. + s.hasBlankRequiredField() -> R.string.register_error_required // Mirror the backend (AuthController) rules so the user gets a clear // client-side error instead of a server 422: 8-72 chars, plus at least // one uppercase, one lowercase and one digit. diff --git a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt index e32ac54..3a2a917 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt @@ -628,7 +628,19 @@ private fun DateField(value: String, label: String, onValueChange: (String) -> U pickerOpen = false }) { Text(stringResource(R.string.action_save)) } }, - dismissButton = { TextButton(onClick = { pickerOpen = false }) { Text(stringResource(R.string.action_cancel)) } }, + dismissButton = { + Row { + // Clear an existing birth date (PATCH /me accepts an empty + // value → the field is cleared server-side). Only offered + // when there is a date to remove. + if (value.isNotBlank()) { + TextButton(onClick = { onValueChange(""); pickerOpen = false }) { + Text(stringResource(R.string.action_clear)) + } + } + TextButton(onClick = { pickerOpen = false }) { Text(stringResource(R.string.action_cancel)) } + } + }, ) { DatePicker(state = pickerState, showModeToggle = false) } diff --git a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt index e33f4fd..4581048 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt @@ -75,7 +75,15 @@ class ProfileViewModel @Inject constructor( } } - fun builtinRequired(key: String): Boolean = _state.value.builtinFields[key]?.required ?: true + /** + * Profile-side required-ness. Unlike registration (which defaults to + * required-when-absent, the conservative signup choice), profile editing + * defaults to NOT required when the registration schema hasn't loaded (or + * failed): the user already has an account, so an unloaded schema must not + * make an optional telefono/indirizzo suddenly block saving — the server + * stays authoritative. Once the schema loads, the real flags apply. + */ + fun builtinRequired(key: String): Boolean = _state.value.builtinFields[key]?.required ?: false fun load() { _state.update { it.copy(profile = UiState.Loading) } @@ -118,8 +126,8 @@ class ProfileViewModel @Inject constructor( } } fun cancelEdit() = _state.update { it.copy(editing = false) } - fun onEditNome(v: String) = _state.update { it.copy(editNome = v) } - fun onEditCognome(v: String) = _state.update { it.copy(editCognome = v) } + fun onEditNome(v: String) = _state.update { it.copy(editNome = v, editErrorRes = null) } + fun onEditCognome(v: String) = _state.update { it.copy(editCognome = v, editErrorRes = null) } fun onEditTelefono(v: String) = _state.update { it.copy(editTelefono = v, editErrorRes = null) } fun onEditIndirizzo(v: String) = _state.update { it.copy(editIndirizzo = v, editErrorRes = null) } fun onEditDataNascita(v: String) = _state.update { it.copy(editDataNascita = v) } @@ -136,7 +144,9 @@ class ProfileViewModel @Inject constructor( // an opaque server 422 round-trip. telefono/indirizzo are required only // when the instance requires them; required custom fields must be filled. val missingBuiltin = - (builtinRequired("telefono") && s.editTelefono.isBlank()) || + s.editNome.isBlank() || + (builtinRequired("cognome") && s.editCognome.isBlank()) || + (builtinRequired("telefono") && s.editTelefono.isBlank()) || (builtinRequired("indirizzo") && s.editIndirizzo.isBlank()) val missingCustom = original?.customFields?.any { def -> val v = s.editCustomValues[def.id].orEmpty() diff --git a/app/src/test/java/com/pinakes/app/RegisterUiStateTest.kt b/app/src/test/java/com/pinakes/app/RegisterUiStateTest.kt new file mode 100644 index 0000000..dd18cce --- /dev/null +++ b/app/src/test/java/com/pinakes/app/RegisterUiStateTest.kt @@ -0,0 +1,116 @@ +package com.pinakes.app + +import com.pinakes.app.data.model.BuiltinFieldRule +import com.pinakes.app.data.model.CustomFieldDef +import com.pinakes.app.ui.screens.login.RegisterUiState +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Pure-logic coverage for the registration form's submit gate + required-field + * validation (issue #255 / the #26 review hardening). These are the exact + * functions RegisterViewModel.submit() calls, so they guard the real behaviour + * without needing the async ViewModel / repository. + */ +class RegisterUiStateTest { + + private fun ready() = RegisterUiState( + nome = "Mario", email = "m@x.it", password = "Abcdef12", + passwordConfirm = "Abcdef12", privacyAccepted = true, + schemaLoading = false, + ) + + // ── builtinRequired: default required-when-absent (conservative signup) ────── + @Test fun builtinRequiredDefaultsTrueWhenAbsent() { + assertTrue(RegisterUiState().builtinRequired("cognome")) + } + + @Test fun builtinRequiredHonoursFalse() { + val s = RegisterUiState(builtinFields = mapOf("telefono" to BuiltinFieldRule(required = false, configurable = true))) + assertFalse(s.builtinRequired("telefono")) + } + + @Test fun builtinRequiredHonoursTrue() { + val s = RegisterUiState(builtinFields = mapOf("indirizzo" to BuiltinFieldRule(required = true, configurable = true))) + assertTrue(s.builtinRequired("indirizzo")) + } + + // ── canSubmit: the submit gate ────────────────────────────────────────────── + @Test fun canSubmitFalseWhileSchemaLoading() { + assertFalse(ready().copy(schemaLoading = true).canSubmit()) + } + + @Test fun canSubmitFalseWhileLoading() { + assertFalse(ready().copy(loading = true).canSubmit()) + } + + @Test fun canSubmitFalseWhenAlreadySent() { + assertFalse(ready().copy(sent = true).canSubmit()) + } + + @Test fun canSubmitTrueWhenReady() { + assertTrue(ready().canSubmit()) + } + + // ── hasBlankRequiredField: core + config-required built-ins + custom ───────── + @Test fun blankNomeIsMissing() { + assertTrue(ready().copy(nome = "").hasBlankRequiredField()) + } + + @Test fun blankEmailIsMissing() { + assertTrue(ready().copy(email = "").hasBlankRequiredField()) + } + + @Test fun requiredCognomeBlankIsMissing() { + val s = ready().copy( + cognome = "", + builtinFields = mapOf("cognome" to BuiltinFieldRule(required = true, configurable = true)), + ) + assertTrue(s.hasBlankRequiredField()) + } + + @Test fun optionalTelefonoBlankIsNotMissing() { + val s = ready().copy( + telefono = "", + builtinFields = mapOf("telefono" to BuiltinFieldRule(required = false, configurable = true)), + ) + assertFalse(s.hasBlankRequiredField()) + } + + @Test fun requiredCustomFieldEmptyIsMissing() { + val s = ready().copy( + customFields = listOf(CustomFieldDef(id = 7, label = "Matricola", type = "number", required = true)), + customValues = emptyMap(), + ) + assertTrue(s.hasBlankRequiredField()) + } + + @Test fun requiredCheckboxUncheckedIsMissing() { + val s = ready().copy( + customFields = listOf(CustomFieldDef(id = 9, label = "Consenso", type = "checkbox", required = true)), + customValues = mapOf(9 to ""), + ) + assertTrue(s.hasBlankRequiredField()) + } + + @Test fun requiredCheckboxCheckedIsFilled() { + val s = ready().copy( + customFields = listOf(CustomFieldDef(id = 9, label = "Consenso", type = "checkbox", required = true)), + customValues = mapOf(9 to "1"), + ) + assertFalse(s.hasBlankRequiredField()) + } + + @Test fun optionalCustomFieldEmptyIsNotMissing() { + val s = ready().copy( + customFields = listOf(CustomFieldDef(id = 3, label = "Telegram", type = "text", required = false)), + customValues = emptyMap(), + ) + assertFalse(s.hasBlankRequiredField()) + } + + @Test fun allRequiredFilledIsNotMissing() { + assertFalse(ready().hasBlankRequiredField()) + } +} diff --git a/i18n/de.json b/i18n/de.json index b2d8f27..5fbb3e8 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -421,5 +421,6 @@ "book_club_privacy_invite": "Nur mit Einladung", "book_club_privacy_hidden": "Verborgen", "book_club_error_dashboard": "Dein Lesebereich konnte nicht geladen werden. Zum Aktualisieren ziehen.", - "register_schema_failed": "Zusätzliche Registrierungsfelder konnten nicht geladen werden — du kannst dich trotzdem registrieren oder es erneut versuchen." + "register_schema_failed": "Zusätzliche Registrierungsfelder konnten nicht geladen werden — du kannst dich trotzdem registrieren oder es erneut versuchen.", + "action_clear": "Löschen" } diff --git a/i18n/en.json b/i18n/en.json index 3212d8e..b38b153 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -421,5 +421,6 @@ "book_club_privacy_invite": "Invite only", "book_club_privacy_hidden": "Hidden", "book_club_error_dashboard": "Couldn't load your reading section. Pull to refresh.", - "register_schema_failed": "Couldn't load the extra registration fields — you can still sign up, or retry." + "register_schema_failed": "Couldn't load the extra registration fields — you can still sign up, or retry.", + "action_clear": "Clear" } diff --git a/i18n/fr.json b/i18n/fr.json index d7ef053..89010de 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -421,5 +421,6 @@ "book_club_privacy_invite": "Sur invitation", "book_club_privacy_hidden": "Masqué", "book_club_error_dashboard": "Impossible de charger votre section de lecture. Tirez pour actualiser.", - "register_schema_failed": "Impossible de charger les champs d'inscription supplémentaires — vous pouvez quand même vous inscrire, ou réessayer." + "register_schema_failed": "Impossible de charger les champs d'inscription supplémentaires — vous pouvez quand même vous inscrire, ou réessayer.", + "action_clear": "Effacer" } diff --git a/i18n/it.json b/i18n/it.json index e77d759..48edcc8 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -421,5 +421,6 @@ "book_club_privacy_invite": "Solo su invito", "book_club_privacy_hidden": "Nascosto", "book_club_error_dashboard": "Impossibile caricare la sezione delle tue letture. Trascina per aggiornare.", - "register_schema_failed": "Impossibile caricare i campi di registrazione aggiuntivi — puoi comunque registrarti, oppure riprovare." + "register_schema_failed": "Impossibile caricare i campi di registrazione aggiuntivi — puoi comunque registrarti, oppure riprovare.", + "action_clear": "Cancella" } From 04117533a2d96b4a3d362adfdf7739c679cd4371 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 21 Jul 2026 20:09:33 +0200 Subject: [PATCH 4/4] build: bump to 1.4.0 (versionCode 8) + fix RegisterUiStateTest ready() helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Version bump for the #255 configurable/custom registration + profile fields (aligns with the mobile-api server plugin 1.4.0). Also fixes the RegisterUiStateTest ready() helper: with an empty builtinFields map the config-driven built-ins (cognome/telefono/indirizzo) default to required, so the "ready to submit" fixture must fill them — otherwise the all-fields-filled / optional-field assertions saw them as blank-and-required. The production logic was correct (the Gradle build compiled cleanly); only the test fixture was wrong. Full unit suite now green (RegisterUiStateTest + all existing tests, 0 failures) and assembleDebug produces an installable APK. --- app/build.gradle.kts | 4 ++-- app/src/test/java/com/pinakes/app/RegisterUiStateTest.kt | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 41a7cf1..7cf2994 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -165,8 +165,8 @@ android { applicationId = "com.pinakes.app" minSdk = 26 targetSdk = 35 - versionCode = 7 - versionName = "1.3.1" + versionCode = 8 + versionName = "1.4.0" vectorDrawables { useSupportLibrary = true diff --git a/app/src/test/java/com/pinakes/app/RegisterUiStateTest.kt b/app/src/test/java/com/pinakes/app/RegisterUiStateTest.kt index dd18cce..d8c2122 100644 --- a/app/src/test/java/com/pinakes/app/RegisterUiStateTest.kt +++ b/app/src/test/java/com/pinakes/app/RegisterUiStateTest.kt @@ -15,9 +15,14 @@ import org.junit.Test */ class RegisterUiStateTest { + // A form ready to submit: every field the default (empty) schema treats as + // required is filled, so each test can blank exactly the one field it probes. + // (With an empty builtinFields map, cognome/telefono/indirizzo default to + // required, so they must be non-blank here.) private fun ready() = RegisterUiState( - nome = "Mario", email = "m@x.it", password = "Abcdef12", - passwordConfirm = "Abcdef12", privacyAccepted = true, + nome = "Mario", email = "m@x.it", + cognome = "Rossi", telefono = "3331234567", indirizzo = "Via Roma 1", + password = "Abcdef12", passwordConfirm = "Abcdef12", privacyAccepted = true, schemaLoading = false, )