Configurable + custom registration/profile fields (#255) - #26
Merged
Conversation
WalkthroughIl PR aggiunge la discovery dei campi di registrazione, il supporto ai campi personalizzati nella registrazione e nel profilo, nuovi dati personali e le informazioni della tessera. ChangesCampi dinamici e profilo
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RegisterScreen
participant RegisterViewModel
participant AuthRepository
participant PinakesApi
RegisterScreen->>RegisterViewModel: submit()
RegisterViewModel->>AuthRepository: register(customFields)
AuthRepository->>PinakesApi: POST registration request
PinakesApi-->>AuthRepository: registration result
AuthRepository-->>RegisterViewModel: ApiResult
RegisterViewModel-->>RegisterScreen: registration state
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
🔎 Adams Review — PR #26 (configurable + custom registration/profile fields)Adversarial pass (correctness / structural contract-adherence / UX). Correctness-clean — no serialization, decode, or contract-drift bugs. Three UX-hardening items surfaced, all non-blocking; they overlap the follow-ups already noted in the PR description.
✅ Correctness (L1) — cleanNo decode-breaking shapes, no ✅ Structural / contract adherence (L2) — cleanVerified exact against the server contract (PR #277):
ℹ️ UX hardening (L5) — 3 confirmed, non-blocking
Verdict: merge-ready pending the Gradle build. The three UX items are polish; #1 (schema loading gate) is the one most worth doing before this ships to users. Want me to add the loading gate + the fetch-failure notice? Lenses: L1 correctness, L2 structural, L5 UX. Findings verified against the actual Kotlin, not just flagged. |
fabiodalez-dev
added a commit
that referenced
this pull request
Jul 21, 2026
…equired validation 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.kt`:
- Around line 621-631: Update the DatePickerDialog in ProfileScreen so users can
clear an existing birth date by invoking onValueChange with an empty string,
while preserving the current save and cancel behavior for date selection and
dismissal.
In `@app/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kt`:
- Around line 38-39: Update ProfileViewModel’s builtinFields handling and the
validation paths around the profile save flow to distinguish an unloaded or
failed registration schema from a successfully loaded schema with required
fields. Track schema loading/error state; while the schema is unavailable, do
not treat empty telefono or indirizzo values as required, and preserve
required-field validation once the schema loads successfully. Apply the same
behavior to the related logic near the schema-loading and save-validation
sections.
- Around line 121-124: Estendi la validazione locale in saveEdit() per includere
nome obbligatorio e cognome secondo la configurazione esistente, impedendo
l’invio di valori vuoti al server. Aggiorna inoltre onEditNome() e
onEditCognome() per azzerare editErrorRes come già avviene negli altri handler
di modifica.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a4873a3d-e554-413f-82eb-cc9a597fe4b6
📒 Files selected for processing (11)
app/src/main/java/com/pinakes/app/data/model/Models.ktapp/src/main/java/com/pinakes/app/data/network/PinakesApi.ktapp/src/main/java/com/pinakes/app/data/repository/AuthRepository.ktapp/src/main/java/com/pinakes/app/data/repository/ProfileRepository.ktapp/src/main/java/com/pinakes/app/ui/screens/login/RegisterScreen.ktapp/src/main/java/com/pinakes/app/ui/screens/profile/ProfileScreen.ktapp/src/main/java/com/pinakes/app/ui/screens/profile/ProfileViewModel.kti18n/de.jsoni18n/en.jsoni18n/fr.jsoni18n/it.json
fabiodalez-dev
added a commit
that referenced
this pull request
Jul 21, 2026
…unit tests 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.
Owner
Author
|
Addressed all three in
Also extracted the register submit gate + required check into pure functions ( |
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<id,value>). 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.
…equired validation 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.
…unit tests 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.
fabiodalez-dev
force-pushed
the
feat/255-registration-fields
branch
from
August 5, 2026 13:08
0411753 to
fd0bc06
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Owner
Author
|
Review fixes completed in
Validation is green: Android CI passed unit tests, lint, debug assembly, and the minified release build; CodeRabbit completed successfully. Locally, the clean verification run reported 93 tests with no failures, errors, or skips. The CodeRabbit docstring-coverage notice was not changed: it is an advisory percentage, not a code defect or repository quality gate, and raising it to an arbitrary threshold here would add broad non-functional KDoc churn unrelated to the reviewed behavior. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consumes the configurable registration/profile field schema exposed by Pinakes and renders the matching Android form controls.
Models and networking
Screens
M,F,Altro, or empty.Design and localization
values,values-en,values-fr, andvalues-es; all four catalogs contain the same 429 keys.Review fixes
main.versionCode 10/versionName 1.4.0.Verification
./gradlew clean testDebugUnitTest assembleDebug lintDebugKnown follow-up (non-blocking)
Summary by CodeRabbit