Configurable + custom registration/profile fields (#255)#26
Open
fabiodalez-dev wants to merge 4 commits into
Open
Configurable + custom registration/profile fields (#255)#26fabiodalez-dev wants to merge 4 commits into
fabiodalez-dev wants to merge 4 commits into
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (6)
WalkthroughIl PR aggiunge la discovery dei campi di registrazione, il supporto ai campi personalizzati nella registrazione e nell’aggiornamento del profilo, nuovi dati personali e la visualizzazione delle informazioni tessera. ChangesCampi dinamici e accesso API
Flusso di registrazione
Modifica del profilo
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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. |
…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
…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 ( |
…) helper 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.
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 new server discovery endpoint
GET /api/v1/auth/registration-fields(Pinakes server PR #277) so signup and profile honour an instance's configurable built-in fields and admin-defined custom fields (#255).Models
RegistrationFieldsPayload,BuiltinFieldRule,CustomFieldDef,CustomFieldValue.RegisterRequestgainscustom_fields(Map<id,value>).UserProfileextended with fieldsGET /mealready returns (telefono, indirizzo, data_nascita, sesso, cod_fiscale, codice_tessera, card/last-access, locale, custom_fields).UpdateProfileRequestextended, all nullable → partialPATCH /me.PinakesApi:GET auth/registration-fields(no-auth).Screens
custom_fields.PATCH /me.Design consistency
All new inputs reuse the app's design system —
PinakesTextField,Switchfor checkbox, Material3DatePicker,Spacing/MaterialThemetokens. No rawOutlinedTextField. New strings added to all four locales (en/it/fr/de, 422 keys each).Build note
Not built in the environment that produced this (no Android SDK) — needs a Gradle build to confirm. Static review is clean;
kotlinx.serializationisignoreUnknownKeys/explicitNulls=false, so the richer server payloads never crash.Known minor follow-ups (non-blocking)
custom_field_invalid/missing_fieldsto field-level errors.Summary by CodeRabbit