Task 13886 customizable fields metadata values handling#13908
Merged
raulbob merged 56 commits intoApr 21, 2026
Merged
Conversation
Implements the customizable fields (metadata and values): API Layer: - Added CustomizableFieldType enum with 11 supported field types - Added CustomizableFieldMetadataDto and CustomizableFieldCustomProperties for field configuration - Added CustomizableFieldValueDto with typed value accessors for all supported types - Added CustomizableFieldVisibilityRestrictions and CustomizableFieldVisibilityContext for disease-based field visibility control - Added CustomizableFieldMetadataFacade and CustomizableFieldValueFacade interfaces Backend Layer: - Added CustomizableFieldMetadata and CustomizableFieldValue JPA entities with JSON support - Added CustomizableFieldMetadataService and CustomizableFieldValueService with query methods - Added EJB facades for metadata and value management with full serialization support - Added database schema migration with proper indexing, history tables, and triggers - Added initial testing with CustomizableFieldFacadeEjbTest REST API: - Added CustomizableFieldMetadataResource for metadata CRUD and field operations - Added CustomizableFieldValueResource for value management UI Layer: - Extend AbstractEditForm to support preloaded metadata and values - Added CustomizableFieldsGroup component for grouping fields by UI group - Added CustomizableFieldInput base class with Binder integration for automatic value sync - Implement 11 concrete input components for all supported field types: TEXT, TEXTAREA, NUMBER, DECIMAL, DATE, DATE_TIME, COMBOBOX, CHECKBOX, YES_NO_UNKNOWN, CHECKBOX_LIST, RADIO_BUTTON_LIST - Added CustomizableFieldInputFactory for polymorphic component creation - Support field visibility restrictions, mandatory/readonly flags, and UI weighting
…ctions Updated app ci token
Migrate contactProximity from single enum to Set<ContactProximity> in the Android app to match the API/backend changes.
… null check in both updateContactCategory() and getContactCategoryForProximity() to handle malformed proximity data
…es in the Set<ContactProximity>
* Update sql schema with TestReport renamed columns * Fixed remarks in sql schema
Add check to avoid committing if no changes are detected.
- Refactored duplicated listeners for IGRA inputs into two listeners - Fixed error caused by locale conversions with IGRA numeric inputs
Implements the customizable fields (metadata and values): API Layer: - Added CustomizableFieldType enum with 11 supported field types - Added CustomizableFieldMetadataDto and CustomizableFieldCustomProperties for field configuration - Added CustomizableFieldValueDto with typed value accessors for all supported types - Added CustomizableFieldVisibilityRestrictions and CustomizableFieldVisibilityContext for disease-based field visibility control - Added CustomizableFieldMetadataFacade and CustomizableFieldValueFacade interfaces Backend Layer: - Added CustomizableFieldMetadata and CustomizableFieldValue JPA entities with JSON support - Added CustomizableFieldMetadataService and CustomizableFieldValueService with query methods - Added EJB facades for metadata and value management with full serialization support - Added database schema migration with proper indexing, history tables, and triggers - Added initial testing with CustomizableFieldFacadeEjbTest REST API: - Added CustomizableFieldMetadataResource for metadata CRUD and field operations - Added CustomizableFieldValueResource for value management UI Layer: - Extend AbstractEditForm to support preloaded metadata and values - Added CustomizableFieldsGroup component for grouping fields by UI group - Added CustomizableFieldInput base class with Binder integration for automatic value sync - Implement 11 concrete input components for all supported field types: TEXT, TEXTAREA, NUMBER, DECIMAL, DATE, DATE_TIME, COMBOBOX, CHECKBOX, YES_NO_UNKNOWN, CHECKBOX_LIST, RADIO_BUTTON_LIST - Added CustomizableFieldInputFactory for polymorphic component creation - Support field visibility restrictions, mandatory/readonly flags, and UI weighting
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 27 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive customizable fields feature enabling dynamic creation and management of custom metadata fields across case, epidemiological data, and exposure entities. The implementation includes API DTOs and enums, EJB facades and services, REST endpoints, database schema with partitioning, and a complete Vaadin UI layer with field-type-specific input components and metadata management views. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
…ta_values_handling
- UI Group is now an enum instead of a string tied to Context - Added handling for translation of captions and descriptions - Improved rendering of YesNoUnkown component
- fixed history triggers - added customizable fields tables to export
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
sormas-backend/src/main/java/de/symeda/sormas/backend/contact/ContactFacadeEjb.java (1)
622-666:⚠️ Potential issue | 🟠 MajorAdd restore of custom field values when restoring contacts and cases.
When contacts and cases are deleted, line 662 (ContactFacadeEjb) and line 2850 (CaseFacadeEjb) explicitly soft-delete customizable field values via
epiDataFacade.softDeleteCustomizableFieldValues()andcustomizableFieldValueService.softDeleteValuesForEntity(). However, the restore paths only callsuper.restore()without restoring these soft-deleted field values.Compare with existing patterns (SampleService, EventService, EventParticipantService) which explicitly restore their associated soft-deleted entities. Add equivalent restore calls for custom field values to prevent data loss during delete/restore cycles.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/java/de/symeda/sormas/backend/contact/ContactFacadeEjb.java` around lines 622 - 666, The restore(List<String> uuids) path in ContactFacadeEjb only calls super.restore() per-contact and never restores the soft-deleted customizable field values; update the loop that processes each contact (the restore(List<String> uuids) method and the per-uuid restore logic if needed) to, after a successful restore, call the appropriate restore method for custom field values (the counterpart to epiDataFacade.softDeleteCustomizableFieldValues and/or customizableFieldValueService.softDeleteValuesForEntity) for the contact’s epiData (e.g. epiDataFacade.restoreCustomizableFieldValues(contact.getEpiData()) or customizableFieldValueService.restoreValuesForEntity(contact.getUuid())), guarding for null epiData, and also ensure any case-related callbacks are preserved (e.g. call caseFacade.onCaseChanged when contact.getCaze() != null) as done in deleteContact.sormas-backend/src/main/java/de/symeda/sormas/backend/caze/CaseFacadeEjb.java (1)
2797-2801:⚠️ Potential issue | 🟠 MajorCaseFacadeEjb.restore() does not restore customizable field values that were soft-deleted during case deletion.
Lines 2849-2850 in the delete method soft-delete case and epi-data custom field values via
customizableFieldValueService.softDeleteValuesForEntity()andepiDataFacade.softDeleteCustomizableFieldValues(). However, the restore method (lines 2799-2801) only callssuper.restore(uuid), which restores the case entity itself but leaves the soft-deleted custom field values in a deleted state.Add matching restore calls for
CustomizableFieldValuerecords using arestoreValuesForEntity()method onCustomizableFieldValueService, or provide an equivalent mechanism to un-soft-delete custom field values when a case is restored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/java/de/symeda/sormas/backend/caze/CaseFacadeEjb.java` around lines 2797 - 2801, The restore method in CaseFacadeEjb currently only calls super.restore(uuid) and therefore does not un-soft-delete customizable field values; update CaseFacadeEjb.restore(String uuid) to, after calling super.restore(uuid), call the counterpart restore methods to re-enable soft-deleted custom fields—specifically invoke customizableFieldValueService.restoreValuesForEntity(uuid, EntityType.CASE) (or the actual signature used) and call the epi-data restore method matching epiDataFacade.softDeleteCustomizableFieldValues (e.g., epiDataFacade.restoreCustomizableFieldValues(uuid)) so that both case-level CustomizableFieldValue records and epi-data custom field values are restored when a case is restored.sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (1)
208-217:⚠️ Potential issue | 🟡 MinorGate the activity-as-case custom panel with the standard activity fields.
addActivityAsCaseFields()is case-only, butactivityAsCasePanelis added unconditionally. For contact epidata forms, activeEPIDATA_ACTIVITY_AS_CASEcustom fields can appear without the corresponding built-in section/context.Proposed fix
if (parentClass == CaseDataDto.class) { addActivityAsCaseFields(); - } - - activityAsCasePanel = new CustomizableFieldsGroup(CustomizableFieldGroup.EPIDATA_ACTIVITY_AS_CASE); - activityAsCasePanel.setVisibilityContext(new CustomizableFieldVisibilityContext().withDisease(disease)); - activityAsCasePanel.setFieldsMetadata(getCustomizableFieldsMetadata()); - activityAsCasePanel.setFieldsValues(getCustomizableFieldsValues()); - activityAsCasePanel.updateFieldsDisplay(); - getContent().addComponent(activityAsCasePanel, LOC_CUSTOMIZABLE_FIELDS_ACTIVITY_AS_CASE); + activityAsCasePanel = new CustomizableFieldsGroup(CustomizableFieldGroup.EPIDATA_ACTIVITY_AS_CASE); + activityAsCasePanel.setVisibilityContext(new CustomizableFieldVisibilityContext().withDisease(disease)); + activityAsCasePanel.setFieldsMetadata(getCustomizableFieldsMetadata()); + activityAsCasePanel.setFieldsValues(getCustomizableFieldsValues()); + activityAsCasePanel.updateFieldsDisplay(); + getContent().addComponent(activityAsCasePanel, LOC_CUSTOMIZABLE_FIELDS_ACTIVITY_AS_CASE); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java` around lines 208 - 217, The activity-as-case custom panel is created and added unconditionally; wrap the creation, configuration and getContent().addComponent(...) of activityAsCasePanel (the CustomizableFieldsGroup with CustomizableFieldGroup.EPIDATA_ACTIVITY_AS_CASE) in the same parentClass == CaseDataDto.class guard used for addActivityAsCaseFields() so the panel is only built for CaseDataDto forms; keep calls to setVisibilityContext(...withDisease(disease)), setFieldsMetadata(getCustomizableFieldsMetadata()), setFieldsValues(getCustomizableFieldsValues()), updateFieldsDisplay() inside that guard and only call getContent().addComponent(activityAsCasePanel, LOC_CUSTOMIZABLE_FIELDS_ACTIVITY_AS_CASE) when parentClass == CaseDataDto.class.
♻️ Duplicate comments (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputText.java (1)
87-94:⚠️ Potential issue | 🟡 Minor
null → ""substitution combined with the value-change listener silently rewrites the model.When the form opens a record whose stored value is
null,applyValueToWidget(null)sets the text field to"", which fires the value-change listener on line 75 — callingsetValue("")on the outer field. The model now holds""instead ofnull, and any dirty/equality check comparing the originalnullwith the new""will incorrectly report a change.Same class of issue as flagged on
CustomizableFieldInputCombobox: gate the listener one.isUserOriginated()(or suppress it during programmatic pushes) and, on the server side, consider normalizing empty string tonullbefore persisting so repeated load/save cycles don't churn the column.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputText.java` around lines 87 - 94, applyValueToWidget currently sets textField to "" for null which triggers the value-change listener and accidentally writes "" into the model; to fix, update the value-change listener (the listener that calls setValue on the outer CustomizableFieldInputText) to ignore programmatic events by checking e.isUserOriginated() (or use a boolean "suppressListeners" flag around programmatic textField.setValue calls in applyValueToWidget), and keep applyValueToWidget (textField/pendingValue) behavior but ensure programmatic setValue does not invoke the outer setValue; also consider server-side normalization of ""→null when persisting to avoid churn.
🟠 Major comments (22)
sormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldMetadataDto.java-67-70 (1)
67-70:⚠️ Potential issue | 🟠 MajorValidate that
uiGroupbelongs tocontextClass.The DTO can currently carry inconsistent values, e.g.
contextClass=CASEwith an EXPOSURE-onlyuiGroup. Unless the facade rejects this elsewhere, such metadata can be saved but never render in the intended UI group.Defensive DTO-level guard option
public void setContextClass(CustomizableFieldContext contextClass) { this.contextClass = contextClass; + validateUiGroupContext(); } @@ public void setUiGroup(CustomizableFieldGroup uiGroup) { this.uiGroup = uiGroup; + validateUiGroupContext(); } + + private void validateUiGroupContext() { + if (contextClass != null && uiGroup != null && uiGroup.getContext() != contextClass) { + throw new IllegalArgumentException("uiGroup must belong to contextClass"); + } + }Also applies to: 118-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldMetadataDto.java` around lines 67 - 70, CustomizableFieldMetadataDto currently allows mismatched combinations of contextClass and uiGroup (e.g., contextClass=CASE with an EXPOSURE-only uiGroup); add a DTO-level validation to enforce that uiGroup is valid for the chosen contextClass. Implement a validation method on CustomizableFieldMetadataDto (e.g., an `@AssertTrue` annotated boolean isUiGroupCompatible() or a custom ConstraintValidator) that checks uiGroup == null or uiGroup.isAllowedFor(contextClass) (or equivalent lookup), and return false with a clear message when incompatible; ensure the validator references the dto fields contextClass and uiGroup so the facade cannot accept inconsistent metadata.sormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldValueQueryContext.java-31-34 (1)
31-34:⚠️ Potential issue | 🟠 MajorImplement
createExpressioninstead of returningnull.Any criteria path that asks this context for a field expression will receive
null, which can turn filters/sorts into runtime failures. Map the supportedCustomizableFieldValuefields here, or throw an explicit unsupported-field exception if this context must not be used for dynamic expressions.Suggested direction
`@Override` protected Expression<?> createExpression(String name) { - return null; + switch (name) { + case CustomizableFieldValue.ENTITY_UUID: + return getRoot().get(CustomizableFieldValue.ENTITY_UUID); + case CustomizableFieldValue.CONTEXT_CLASS: + return getRoot().get(CustomizableFieldValue.CONTEXT_CLASS); + case CustomizableFieldValue.VALUE: + return getRoot().get(CustomizableFieldValue.VALUE); + case CustomizableFieldValue.CUSTOMIZABLE_FIELD_METADATA: + return getRoot().get(CustomizableFieldValue.CUSTOMIZABLE_FIELD_METADATA); + default: + throw new IllegalArgumentException("Unsupported customizable field value expression: " + name); + } }Adjust
getRoot()to the accessor exposed byQueryContextif it uses a different name.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldValueQueryContext.java` around lines 31 - 34, The createExpression method in CustomizableFieldValueQueryContext currently returns null causing runtime failures; implement it to map supported CustomizableFieldValue properties to JPA Criteria expressions or throw an explicit unsupported-field exception for unknown names: inspect the name parameter and return expressions for known fields (e.g., id, value, fieldId, fieldKey — whatever fields CustomizableFieldValue exposes) by using this.getRoot() (or the accessor provided by QueryContext if named differently) and root.get("<property>") to build the Expression, and for any unmapped name throw IllegalArgumentException or a custom UnsupportedFieldException so callers fail fast instead of receiving null.sormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldValueFacade.java-58-61 (1)
58-61:⚠️ Potential issue | 🟠 MajorAvoid exposing an unbounded
getAll()for customizable values.Custom field values can contain sensitive free-text data and may grow without bound. Prefer criteria/pagination and explicit authorization scoping, or keep this backend-internal if it is only needed for maintenance/export paths.
🛡️ Suggested API direction
- /** - * Get all field values - */ - List<CustomizableFieldValueDto> getAll(); + /** + * Query field values with explicit criteria and paging. + */ + List<CustomizableFieldValueDto> getAll(CustomizableFieldValueCriteria criteria, int first, int max);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldValueFacade.java` around lines 58 - 61, The public getAll() method on CustomizableFieldValueFacade exposes an unbounded collection of CustomizableFieldValueDto (potentially sensitive free-text) and should be replaced or restricted: remove or make getAll() non-public, and instead add a constrained API such as getByCriteria(...) or getPage(page, size, filterCriteria) that supports pagination, filtering and authorization scoping, or mark the interface/method as backend-internal/maintenance-only; update callers of CustomizableFieldValueFacade.getAll to use the new paginated/criteria method and ensure authorization checks are applied before returning CustomizableFieldValueDto instances.sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputDecimal.java-42-75 (1)
42-75:⚠️ Potential issue | 🟠 MajorTighten the decimal regex before persisting invalid tokens.
The current pattern allows
"-",".",",", and trailing separators like"12,"; those pass validation but are not valid decimal values for later parsing.🧮 Suggested regex fix
- /** Matches an optional leading minus, one or more digits, and an optional decimal part. */ - private static final String DECIMAL_PATTERN = "-?\\d*([.,]\\d*)?"; + /** Matches an optional leading minus, one or more digits, and an optional decimal part. */ + private static final String DECIMAL_PATTERN = "-?\\d+([.,]\\d+)?";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputDecimal.java` around lines 42 - 75, The DECIMAL_PATTERN currently allows lone minus/signs and trailing separators (e.g. "-", ".", "12,") so update the DECIMAL_PATTERN constant to a stricter regex that enforces at least one digit overall and, if a decimal separator is present, requires digits on at least one side (no standalone separators or trailing separators); adjust the existing validator in configureBinding to use that updated DECIMAL_PATTERN (reference: DECIMAL_PATTERN constant and configureBinding method) so only syntactically valid decimal strings pass validation before persisting.sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInput.java-219-236 (1)
219-236:⚠️ Potential issue | 🟠 MajorPropagate read-only state to inner widgets.
Line 235 sets read-only only on the CustomField wrapper. The inner widgets (TextField, CheckBox, etc.) created in
buildInputComponent()are never notified and can still accept user input. Their value-change listeners will callsetValue(...), potentially modifying data even when the wrapper is read-only.The timing compounds this:
applyMetadata()runs during construction (line 98) beforeinitContent()and the inner widget creation, so even if propagation were attempted at that point, the widget wouldn't exist yet.Override
setReadOnly()in the base class to propagate the state to the inner widget once it's created, and ensure each concrete input applies this state after building its widget.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInput.java` around lines 219 - 236, The wrapper's read-only flag is only set on CustomizableFieldInput in applyMetadata() and never propagated to the inner input component built in buildInputComponent(), so inner widgets can still accept input; override setReadOnly(boolean) in CustomizableFieldInput to store the flag and, if the inner component (e.g., the field returned from buildInputComponent() or a member like innerComponent) is non-null, call its setReadOnly(flag) so the state is propagated, and ensure each concrete subclass applies the read-only state immediately after creating its widget in initContent()/buildInputComponent() (or by calling the base setReadOnly propagation) so listeners and setValue(...) cannot modify data when the wrapper is read-only.sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInput.java-90-96 (1)
90-96:⚠️ Potential issue | 🟠 MajorMandatory validation for string/decimal fields is insufficient—override both
getEmptyValue()andisEmpty().Line 93's
asRequired()relies on Vaadin's required check, which in v8 usesgetEmptyValue()(defaulting tonullin CustomField). String subclasses emit""from TextField, but the outer CustomField's empty value remainsnull, soasRequired()incorrectly considers""non-empty and validation passes when it should fail. Similarly,FieldVisibleAndNotEmptyValidator(line 22) checksgetEmptyValue()directly, bypassing customisEmpty()logic.To properly validate mandatory custom text and decimal fields, override
getEmptyValue()in each String-typed subclass to return"", and also overrideisEmpty()in the base class to handle the generic case:✅ Suggested base fix
+import java.util.Collection; + `@Override` public T getValue() { return currentValue; } + + `@Override` + public T getEmptyValue() { + T value = getValue(); + if (value == null) { + return null; + } + if (value instanceof CharSequence) { + return (T) ""; + } + if (value instanceof Collection<?>) { + return (T) Collections.emptyList(); + } + return null; + } + + `@Override` + public boolean isEmpty() { + T value = getValue(); + if (value == null) { + return true; + } + if (value instanceof CharSequence) { + return StringUtils.isBlank((CharSequence) value); + } + if (value instanceof Collection<?>) { + return ((Collection<?>) value).isEmpty(); + } + return false; + }Also applies to: 143-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInput.java` around lines 90 - 96, The mandatory validation is failing because CustomizableFieldValueDto bindings rely on Vaadin's getEmptyValue() (default null) while inner components (e.g., TextField) return "" — update the CustomizableFieldInput class: override isEmpty() to consult the wrapped component's emptiness (use the inner field's isEmpty or value.equals(getEmptyValue()) logic) so generic checks work, and for all String-typed and decimal subclasses override getEmptyValue() to return "" (or an appropriate empty numeric representation) so asRequired() and FieldVisibleAndNotEmptyValidator use the correct empty value; apply the same changes where similar binding occurs (the other binding block referenced in the review).sormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldMetadataReferenceDto.java-20-26 (1)
20-26: 🛠️ Refactor suggestion | 🟠 MajorAdd a no-arg constructor to match established pattern.
The codebase strongly enforces no-arg constructors in ReferenceDto subclasses (33 of 44 existing classes have them). CustomizableFieldMetadataReferenceDto lacks one, breaking consistency and risking deserialization failures if this DTO is used with REST/serialization frameworks.
Proposed constructor addition
public class CustomizableFieldMetadataReferenceDto extends ReferenceDto { private static final long serialVersionUID = 1L; + public CustomizableFieldMetadataReferenceDto() { + } + public CustomizableFieldMetadataReferenceDto(String uuid) { super(uuid); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldMetadataReferenceDto.java` around lines 20 - 26, Add a public no-argument constructor to CustomizableFieldMetadataReferenceDto to match the established pattern of ReferenceDto subclasses and ensure proper deserialization; keep the existing public CustomizableFieldMetadataReferenceDto(String uuid) constructor and have the no-arg constructor call the superclass no-arg (or simply exist) so that frameworks can instantiate the DTO. Locate the class CustomizableFieldMetadataReferenceDto and add a public CustomizableFieldMetadataReferenceDto() {} alongside the existing constructor that calls super(uuid).sormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactController.java-1051-1063 (1)
1051-1063:⚠️ Potential issue | 🟠 MajorThree independent save calls are not atomic — partial failure will leave inconsistent state.
The commit path issues three separate service calls:
contactFacade.save(contactDto)(persists EpiData + exposures)customizableFieldValueFacade.saveEntityCustomFields(...)for the EpiData context- A per-exposure
saveEntityCustomFields(...)for each entry incollectExposureCustomizableFieldValues()If any call after (1) throws (network hiccup, validation, access-denied), the contact/exposure rows remain saved but custom-field values are lost or partially persisted, and the user sees
messageContactSaveddespite the failure. Consider one of:
- Wrap the whole commit in a single façade call that performs all persistence in one transaction (preferred).
- Or at minimum, catch exceptions around 2–3 and surface a clear error message instead of showing the success notification via
Notification.show(... messageContactSaved ...).Also worth noting:
epiDataForm.getValue()'s EpiData UUID is what's being used on line 1056 — this works because UUIDs are generated client-side, but it relies on that convention. Re-reading from the just-savedContactDto(contactFacade.save(...)returns the persisted DTO) would make the dependency explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactController.java` around lines 1051 - 1063, The current commit handler calls FacadeProvider.getContactFacade().save(contactDto) then separately calls getCustomizableFieldValueFacade().saveEntityCustomFields(...) for the EpiData and each exposure, which can leave state inconsistent on partial failure; change this by adding a single transactional façade method (e.g., ContactFacade.saveWithCustomFields or similar) that accepts the ContactDto, epiData customizable fields and the exposure-custom-fields map (use epiDataForm.getValue() and epiDataForm.collectExposureCustomizableFieldValues()) and performs all DB writes in one transaction, returning the persisted ContactDto; alternatively, if you cannot add a facade method now, wrap the customizable-field calls (the saveEntityCustomFields calls and the collectExposureCustomizableFieldValues iteration) in try/catch and on any exception avoid showing Notification.show(messageContactSaved) and instead surface an error notification with the exception message, and re-read the persisted EpiData UUID from the ContactDto returned by contactFacade.save(contactDto) rather than assuming epiDataForm.getValue() UUID.sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputCombobox.java-76-82 (1)
76-82:⚠️ Potential issue | 🟠 MajorProgrammatic widget setValue fires ValueChangeEvent and marks form dirty on initial load.
comboBox.setValue(pendingValue)and all programmatic pushes viaapplyValueToWidgetfire Vaadin's ValueChangeEvent on the inner widget. The listeneraddValueChangeListener(e -> setValue(e.getValue()))then calls the outer field'ssetValue(). Although Vaadin's equality check prevents recursivedoSetValue()calls (sincecurrentValueis already updated), the outersetValue()still fires a ValueChangeEvent to registered listeners. This triggersepiDataForm.addCustomizableFieldValueChangeListener(e -> editView.setDirty(true))inContactController, immediately marking the form dirty on load for any record with existing custom field values—causing unwanted "discard changes?" prompts.All 10 CustomizableFieldInput implementations share this pattern (Text, TextArea, Combobox, Number, Decimal, Date, DateTime, Checkbox, RadioButtonList, CheckboxList).
Use a
settingFromModelflag to suppress the listener during programmatic updates:Suggested fix for CustomizableFieldInputCombobox
+ private boolean settingFromModel = false; + `@Override` protected void applyValueToWidget(String value) { + settingFromModel = true; try { if (pendingValue != null) { comboBox.setValue(pendingValue); pendingValue = null; } comboBox.setValue(value); + } finally { + settingFromModel = false; + } } - comboBox.addValueChangeListener(e -> setValue(e.getValue())); + comboBox.addValueChangeListener(e -> { + if (!settingFromModel) { + setValue(e.getValue()); + } + });Apply the same fix to all 10 CustomizableFieldInput implementations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputCombobox.java` around lines 76 - 82, Programmatic calls like comboBox.setValue(pendingValue) trigger the inner ValueChangeListener (comboBox.addValueChangeListener(e -> setValue(e.getValue()))) which then fires outer field value change events and marks the form dirty; fix by adding a boolean flag (e.g., settingFromModel) on CustomizableFieldInputCombobox, set settingFromModel = true before any programmatic update (including applyValueToWidget and the pendingValue block), perform comboBox.setValue(...), then set settingFromModel = false afterwards, and modify the value change listener to return immediately when settingFromModel is true so only user-initiated changes call setValue; apply the same boolean-guard pattern to all 10 CustomizableFieldInput implementations (Text, TextArea, Combobox, Number, Decimal, Date, DateTime, Checkbox, RadioButtonList, CheckboxList).sormas-ui/src/main/java/de/symeda/sormas/ui/configuration/customizablefield/CustomizableFieldOptionsComponent.java-70-73 (1)
70-73:⚠️ Potential issue | 🟠 MajorAdd accessible labels/descriptions to the icon-only option buttons.
The add and delete controls currently have no text or tooltip/description, so assistive technologies may expose them only as icon glyphs. Please set localized descriptions or captions for both actions.
Also applies to: 174-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/configuration/customizablefield/CustomizableFieldOptionsComponent.java` around lines 70 - 73, The add and delete icon-only buttons in CustomizableFieldOptionsComponent lack accessible labels; update the ButtonHelper.createIconButtonWithCaption call that creates btnAdd (and the equivalent delete button creation around the code that adds each row) to provide a localized caption/description instead of null, or call setDescription/setCaption/setAriaLabel on the Button instance using the component's i18n/localization helper (e.g., use getString("customizableField.add") and getString("customizableField.delete") or the existing localization method) so assistive technologies receive meaningful text for the add (btnAdd) and delete action buttons.sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputNumber.java-42-75 (1)
42-75:⚠️ Potential issue | 🟠 MajorReject a lone minus sign in number validation.
Line 43 currently allows
"-"because\d*is optional. Since empty values are already handled separately on Line 74, require at least one digit in the regex.🐛 Proposed fix
- /** Matches an optional leading minus followed by one or more digits (or empty string). */ - private static final String INTEGER_PATTERN = "-?\\d*"; + /** Matches an optional leading minus followed by one or more digits. */ + private static final String INTEGER_PATTERN = "-?\\d+";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputNumber.java` around lines 42 - 75, The INTEGER_PATTERN currently allows a lone "-" because it uses "-?\\d*" (zero or more digits); update the pattern constant INTEGER_PATTERN to require at least one digit (use "-?\\d+") so configureBinding's validator (in method configureBinding) will reject a lone minus while still allowing negatives and relying on the existing empty/null check to permit blank values; adjust the constant only (INTEGER_PATTERN) so the validator behavior changes accordingly.sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputRadioButtonList.java-76-105 (1)
76-105:⚠️ Potential issue | 🟠 MajorGuard against stale radio values before applying them to the widget.
Lines 82 and 101 can throw
IllegalArgumentExceptionwhen callingsetValue()with a value not present inresolveOptions(). This occurs when administrators remove or rename options after values have already been saved to the database. Validate the value against the current options before applying it to theRadioButtonGroup.🛡️ Defensive direction
`@Override` protected Component buildInputComponent() { radioButtonGroup = new RadioButtonGroup<>(); - radioButtonGroup.setItems(resolveOptions()); + List<String> options = resolveOptions(); + radioButtonGroup.setItems(options); // Apply any value that arrived before the component was first rendered. if (pendingValue != null) { - radioButtonGroup.setValue(pendingValue); + applyAllowedValue(pendingValue, options); pendingValue = null; } @@ protected void applyValueToWidget(String value) { if (radioButtonGroup != null) { - radioButtonGroup.setValue(value); + applyAllowedValue(value, resolveOptions()); } else { pendingValue = value; } } + + private void applyAllowedValue(String value, List<String> options) { + radioButtonGroup.setValue(value == null || options.contains(value) ? value : null); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputRadioButtonList.java` around lines 76 - 105, The code currently calls radioButtonGroup.setValue(...) in buildInputComponent() and applyValueToWidget(String) without checking that the value exists in the current options (resolveOptions()), which can throw IllegalArgumentException for stale/removed options; before calling setValue() (both when applying pendingValue in buildInputComponent() and when applying value in applyValueToWidget()), check whether the value is null or is contained in the collection returned by resolveOptions() (or radioButtonGroup.getDataProvider()/getItems equivalent) and only call radioButtonGroup.setValue(value) when present; if the value is absent, clear the selection (e.g. leave null) and drop pendingValue to avoid throwing. Ensure you reference radioButtonGroup, pendingValue, resolveOptions(), buildInputComponent(), and applyValueToWidget() when making the changes.sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputCheckboxList.java-75-104 (1)
75-104:⚠️ Potential issue | 🟠 MajorFilter or otherwise handle stale checkbox selections before setting them.
When metadata options change, saved selections may include values no longer present in
resolveOptions(). Lines 81 and 100 apply those values directly to theCheckBoxGroup. In Vaadin 8.14.3, CheckBoxGroup silently fails to select values not in the item data provider—no exception is thrown, but the selection is ignored and the UI will not reflect the saved data.🛡️ Defensive direction
+import java.util.LinkedHashSet; import java.util.Collections; import java.util.List; import java.util.Set; @@ `@Override` protected Component buildInputComponent() { checkBoxGroup = new CheckBoxGroup<>(); - checkBoxGroup.setItems(resolveOptions()); + List<String> options = resolveOptions(); + checkBoxGroup.setItems(options); // Apply any value that arrived before the component was first rendered. if (pendingValue != null) { - checkBoxGroup.setValue(pendingValue); + checkBoxGroup.setValue(filterAllowedValues(pendingValue, options)); pendingValue = null; } @@ protected void applyValueToWidget(Set<String> value) { if (checkBoxGroup != null) { - checkBoxGroup.setValue(value != null ? value : Collections.emptySet()); + checkBoxGroup.setValue(filterAllowedValues(value, resolveOptions())); } else { pendingValue = value; } } + + private Set<String> filterAllowedValues(Set<String> value, List<String> options) { + if (value == null || value.isEmpty()) { + return Collections.emptySet(); + } + Set<String> allowedValues = new LinkedHashSet<>(value); + allowedValues.retainAll(options); + return allowedValues; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputCheckboxList.java` around lines 75 - 104, Saved/loaded selections may contain values not present in the current options from resolveOptions(), causing CheckBoxGroup (checkBoxGroup) to silently ignore them; before calling checkBoxGroup.setValue(...) in buildInputComponent() and applyValueToWidget(Set<String>), filter the incoming Set<String> (and pendingValue) against the current resolveOptions() result (or its item IDs) so only valid items are passed to checkBoxGroup.setValue; if filtering removes all entries, pass Collections.emptySet() and clear pendingValue accordingly to keep UI and state consistent.sormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldValueService.java-101-106 (1)
101-106: 🛠️ Refactor suggestion | 🟠 MajorUse
deletePermanent()instead of directem.remove()for consistency with service lifecycle.
deleteValuesForEntityperforms a hard deletion viaem.remove(value)directly, bypassing the inheriteddeletePermanent()method fromAbstractCoreAdoService. This skips lifecycle hooks and flush semantics thatsoftDeleteValuesForEntitycorrectly applies viadelete(value, deletionDetails). Either calldeletePermanent(value)for consistency, or add a comment explaining why direct removal is intentional here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldValueService.java` around lines 101 - 106, The method deleteValuesForEntity in CustomizableFieldValueService currently calls em.remove(value) directly; replace that direct removal with the service lifecycle call deletePermanent(value) for each CustomizableFieldValue to ensure lifecycle hooks and flush semantics are applied (mirroring how softDeleteValuesForEntity uses delete(value, deletionDetails)). Update the loop to call deletePermanent(value) on each value (or, if direct removal is truly required, add a short comment explaining why em.remove is intentional), ensuring you reference the existing deletePermanent(...) and delete(...) methods from AbstractCoreAdoService.sormas-backend/src/main/resources/sql/sormas_schema.sql-15655-15700 (1)
15655-15700:⚠️ Potential issue | 🟠 MajorEnforce metadata/value context consistency in the FK.
Line 15695 only references
customizablefieldmetadata(id), so the DB can accept acustomizablefieldvaluerow whosecontextClassdiffers from the referenced metadata’scontextClass. That can corrupt context-scoped custom field values.🛡️ Proposed constraint tightening
PRIMARY KEY (id), - UNIQUE(name, contextClass) + UNIQUE(name, contextClass), + UNIQUE(id, contextClass) ); @@ ALTER TABLE customizablefieldvalue ADD CONSTRAINT fk_customizablefieldvalue_metadata - FOREIGN KEY (customizablefieldmetadata_id) - REFERENCES customizablefieldmetadata(id) + FOREIGN KEY (customizablefieldmetadata_id, contextClass) + REFERENCES customizablefieldmetadata(id, contextClass) ON DELETE CASCADE;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/resources/sql/sormas_schema.sql` around lines 15655 - 15700, The current FK fk_customizablefieldvalue_metadata only references customizablefieldmetadata(id) allowing mismatched contextClass; replace it with a composite FK that enforces context equality by referencing both id and contextClass. Drop or ALTER the existing fk_customizablefieldvalue_metadata on table customizablefieldvalue and add: FOREIGN KEY (customizablefieldmetadata_id, contextClass) REFERENCES customizablefieldmetadata(id, contextClass) ON DELETE CASCADE; if customizablefieldmetadata(id, contextClass) is not a declared key, create a unique constraint or index on customizablefieldmetadata(id, contextClass) first so the composite FK can reference it.sormas-backend/src/main/resources/sql/sormas_schema.sql-15717-15720 (1)
15717-15720:⚠️ Potential issue | 🟠 MajorPass both id and contextClass columns to the delete_history_trigger to match the composite primary key.
customizablefieldvaluehas a composite primary key(id, contextClass)and is partitioned bycontextClass. Line 15720 passes only'id'todelete_history_trigger, which deletes all history rows matching that id regardless of context. This risks deleting history records for rows in other partitions that happen to share the same id value. The trigger must identify history rows using both key columns: pass'id'and'contextClass', or modify the function to accept multiple key columns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/resources/sql/sormas_schema.sql` around lines 15717 - 15720, customizablefieldvalue uses a composite primary key (id, contextClass) but the trigger call to delete_history_trigger currently passes only 'id', which risks removing history across partitions; update the trigger invocation for customizablefieldvalue to pass both key columns (e.g., 'id' and 'contextClass') to delete_history_trigger so it deletes history rows scoped by both id and contextClass (or alternately modify delete_history_trigger to accept and use multiple key columns); target the CREATE TRIGGER statement for delete_history_trigger and the delete_history_trigger function usage to include both 'id' and 'contextClass' and ensure customizablefieldvalue_history is filtered by both columns.sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseController.java-1017-1021 (1)
1017-1021:⚠️ Potential issue | 🟠 MajorPersist custom field values before refreshing the view.
saveCase(...)refreshes the UI before these callbacks save custom values. That can rebuild the form with stale custom field data immediately after a successful save.Suggested direction
- saveCase(cazeDto); - FacadeProvider.getCustomizableFieldValueFacade() - .saveEntityCustomFields(cazeDto.getEpiData().getUuid(), CustomizableFieldContext.EPIDATA, epiDataForm.collectCurrentFieldValues()); + // Prefer a save path that persists the case and custom fields before triggering SormasUI.refreshView(). + // For example, split saveCase into "persist" and "notify/refresh" phases, or move the refresh after afterSave.Also applies to: 1439-1445
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseController.java` around lines 1017 - 1021, The UI is being refreshed before custom fields are persisted, causing the form to rebuild with stale values; in the call to saveCaseWithFacilityChangedPrompt that currently passes a callback which invokes FacadeProvider.getCustomizableFieldValueFacade().saveEntityCustomFields(cazeDto.getUuid(), CustomizableFieldContext.CASE, updatedCaseFieldValues), move or wrap the saveEntityCustomFields call so it executes and completes before the UI refresh triggered by saveCaseWithFacilityChangedPrompt (e.g., invoke and await/save the custom field persistence first, then call the saveCaseWithFacilityChangedPrompt success path), and make the same change for the other saveCaseWithFacilityChangedPrompt invocation that also uses updatedCaseFieldValues.sormas-rest/src/main/java/de/symeda/sormas/rest/resources/CustomizableFieldValueResource.java-78-90 (1)
78-90:⚠️ Potential issue | 🟠 MajorReject invalid value payloads instead of silently dropping entries.
A null body causes an NPE, and unknown metadata UUIDs are ignored while still returning
200 OK. That can make API clients believe values were saved when they were discarded.Proposed fix
public Response saveEntityCustomFields( `@PathParam`("entityUuid") String entityUuid, `@QueryParam`("contextClass") CustomizableFieldContext contextClass, Map<String, CustomizableFieldValueDto> fieldValues) { + if (fieldValues == null) { + return Response.status(Response.Status.BAD_REQUEST).entity("fieldValues body is required").build(); + } Map<CustomizableFieldMetadataDto, CustomizableFieldValueDto> typed = new HashMap<>(); fieldValues.forEach((metadataUuid, value) -> { CustomizableFieldMetadataDto metadataDto = FacadeProvider.getCustomizableFieldMetadataFacade().getByUuid(metadataUuid); - if (metadataDto != null) { - typed.put(metadataDto, value); + if (metadataDto == null) { + throw new IllegalArgumentException("Unknown customizable field metadata UUID: " + metadataUuid); } + typed.put(metadataDto, value); }); getFacade().saveEntityCustomFields(entityUuid, contextClass, typed); return Response.ok().build(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-rest/src/main/java/de/symeda/sormas/rest/resources/CustomizableFieldValueResource.java` around lines 78 - 90, The saveEntityCustomFields handler currently NPEs on a null body and silently ignores unknown metadata UUIDs; update the method to validate the incoming Map<String,CustomizableFieldValueDto> fieldValues at the start (in saveEntityCustomFields), returning a 400 Bad Request if fieldValues is null, and further validate each metadataUuid by calling FacadeProvider.getCustomizableFieldMetadataFacade().getByUuid(metadataUuid) and collect any unknown UUIDs; if any unknown UUIDs are found, return a 400 Bad Request listing the invalid UUIDs instead of dropping them, otherwise proceed to build the typed Map<CustomizableFieldMetadataDto,CustomizableFieldValueDto> and call getFacade().saveEntityCustomFields(...).sormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadataFacadeEjb.java-134-140 (1)
134-140:⚠️ Potential issue | 🟠 MajorEnforce metadata name uniqueness on save, not only clone.
cloneFieldrejects duplicate names within a context, butsaveallows them. That makesgetByNameAndContextambiguous and can break admin/API behavior.Proposed fix
`@Override` public CustomizableFieldMetadataDto save(`@Valid` `@NotNull` CustomizableFieldMetadataDto dto) { + CustomizableFieldMetadata duplicate = service.getByNameAndContext(dto.getName(), dto.getContextClass()); + if (duplicate != null && !java.util.Objects.equals(duplicate.getUuid(), dto.getUuid())) { + throw new ValidationRuntimeException("Field name already exists in this context: " + dto.getName()); + } CustomizableFieldMetadata entity = service.getByUuid(dto.getUuid()); entity = fillOrBuildEntity(dto, entity, true); service.ensurePersisted(entity); return toDto(entity); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadataFacadeEjb.java` around lines 134 - 140, The save method in CustomizableFieldMetadataFacadeEjb currently allows duplicate metadata names in the same context while cloneField prevents them; update save(CustomizableFieldMetadataDto dto) to perform the same uniqueness check as cloneField by invoking getByNameAndContext (or equivalent service method) before persisting: if another entity with the same name and context exists and its UUID differs from dto.getUuid(), throw a validation/duplicate exception or return an appropriate error, then proceed to call fillOrBuildEntity(...) and service.ensurePersisted(...) only when the name is unique. Ensure the check references CustomizableFieldMetadataFacadeEjb.save, getByNameAndContext, fillOrBuildEntity, and service.ensurePersisted so reviewers can locate and verify the change.sormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadataFacadeEjb.java-142-159 (1)
142-159:⚠️ Potential issue | 🟠 MajorRestore associated values together with metadata.
deletesoft-deletes values for the metadata, butrestoreonly restores the metadata entity. Restored fields can come back with all historical values still deleted.Suggested direction
`@Override` public void restore(String uuid) { super.restore(uuid); + customizableFieldValueService.restoreValuesForMetadata(uuid); }If value restore is intentionally unsupported, block metadata restore or document that it is destructive.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadataFacadeEjb.java` around lines 142 - 159, The restore method in CustomizableFieldMetadataFacadeEjb only restores the metadata entity while delete calls customizableFieldValueService.softDeleteValuesForMetadata(uuid), so associated values remain deleted; update restore(String uuid) to also restore associated values by (1) loading the metadata via service.getByUuid(uuid) (or validating existence), (2) calling the corresponding restore method on customizableFieldValueService (e.g., customizableFieldValueService.restoreValuesForMetadata(uuid) or implementing one if missing) and (3) invoking super.restore(uuid) (or reversing the order if you need metadata present before values), or if value-restore is intentionally unsupported, change restore(String uuid) to block/throw or add clear documentation explaining the destructive behavior.sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/CustomizableFieldsGroup.java-295-304 (1)
295-304:⚠️ Potential issue | 🟠 MajorInclude cleared values so deletions are persisted.
This drops fields whose current value is
null. If a user clears an existing custom field, the save payload omits that field, so the previous backend value can survive.Proposed fix
public Map<CustomizableFieldMetadataDto, CustomizableFieldValueDto> getFieldsValues() { Map<CustomizableFieldMetadataDto, CustomizableFieldValueDto> result = new HashMap<>(); for (Map.Entry<CustomizableFieldMetadataDto, CustomizableFieldInput<?>> entry : fieldComponents.entrySet()) { CustomizableFieldValueDto value = entry.getValue().getFieldValue(); - if (value != null && value.getValue() != null) { + if (value != null) { result.put(entry.getKey(), value); } } return result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/CustomizableFieldsGroup.java` around lines 295 - 304, The getFieldsValues() method currently omits entries when getFieldValue() returns null, so cleared custom fields are not sent to the backend; modify getFieldsValues() to always put the metadata key into the result map (using fieldComponents.entrySet() and the value from CustomizableFieldInput.getFieldValue()), even if the returned CustomizableFieldValueDto is null or its getValue() is null, so deletions are persisted (i.e., remove the conditional that filters out null values and always result.put(entry.getKey(), value)).sormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadataService.java-194-215 (1)
194-215:⚠️ Potential issue | 🟠 MajorAlways exclude soft-deleted metadata, even without criteria.
When
criteria == null,buildCriteriaFilterreturnsnull, sogetIndexListandcountcan include deleted rows. Keep the deleted predicate outside optional filtering.Proposed fix
private Predicate buildCriteriaFilter(CustomizableFieldMetadataCriteria criteria, CriteriaBuilder cb, Root<CustomizableFieldMetadata> root) { - if (criteria == null) { - return null; - } - List<Predicate> predicates = new ArrayList<>(); + predicates.add(cb.isFalse(root.get(DeletableAdo.DELETED))); + + if (criteria == null) { + return cb.and(predicates.toArray(new Predicate[0])); + } if (criteria.getContextClass() != null) { predicates.add(cb.equal(root.get(CustomizableFieldMetadata.CONTEXT_CLASS), criteria.getContextClass())); } @@ - predicates.add(cb.isFalse(root.get(DeletableAdo.DELETED))); - if (!StringUtils.isBlank(criteria.getFreeTextFilter())) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadataService.java` around lines 194 - 215, buildCriteriaFilter currently returns null when criteria == null which allows soft-deleted rows into callers like getIndexList and count; change buildCriteriaFilter to always build a List<Predicate> (initialize at the top), add the deleted-exclusion predicate cb.isFalse(root.get(DeletableAdo.DELETED)) unconditionally, then only add other predicates when criteria fields are non-null, and finally return cb.and(predicates.toArray(new Predicate[0])) instead of returning null so callers always filter out deleted records.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 341e419f-7f50-4107-9f06-7ff8fe288a94
📒 Files selected for processing (86)
sormas-api/src/main/java/de/symeda/sormas/api/FacadeProvider.javasormas-api/src/main/java/de/symeda/sormas/api/common/DeletableEntityType.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldContext.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldCustomProperties.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldGroup.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldMetadataCriteria.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldMetadataDto.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldMetadataFacade.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldMetadataReferenceDto.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldType.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldValueCriteria.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldValueDto.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldValueFacade.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldValueReferenceDto.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldVisibilityContext.javasormas-api/src/main/java/de/symeda/sormas/api/customizablefield/CustomizableFieldVisibilityRestrictions.javasormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.javasormas-api/src/main/java/de/symeda/sormas/api/i18n/Strings.javasormas-api/src/main/java/de/symeda/sormas/api/importexport/DatabaseTable.javasormas-api/src/main/java/de/symeda/sormas/api/user/DefaultUserRole.javasormas-api/src/main/java/de/symeda/sormas/api/user/UserRight.javasormas-api/src/main/resources/captions.propertiessormas-api/src/main/resources/enum.propertiessormas-api/src/main/resources/strings.propertiessormas-backend/src/main/java/de/symeda/sormas/backend/caze/CaseFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/contact/ContactFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadata.javasormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadataFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadataJoins.javasormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadataQueryContext.javasormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldMetadataService.javasormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldValue.javasormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldValueFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldValueJoins.javasormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldValueQueryContext.javasormas-backend/src/main/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldValueService.javasormas-backend/src/main/java/de/symeda/sormas/backend/deletionconfiguration/CoreEntityDeletionService.javasormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiDataFacadeEjb.javasormas-backend/src/main/java/de/symeda/sormas/backend/importexport/DatabaseExportService.javasormas-backend/src/main/resources/META-INF/glassfish-ejb-jar.xmlsormas-backend/src/main/resources/META-INF/persistence.xmlsormas-backend/src/main/resources/sql/sormas_schema.sqlsormas-backend/src/test/java/de/symeda/sormas/backend/EntityMappingTest.javasormas-backend/src/test/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldFacadeEjbTest.javasormas-backend/src/test/java/de/symeda/sormas/backend/customizablefield/CustomizableFieldSchemaTest.javasormas-backend/src/test/resources/META-INF/persistence.xmlsormas-rest/src/main/java/de/symeda/sormas/rest/resources/CustomizableFieldMetadataResource.javasormas-rest/src/main/java/de/symeda/sormas/rest/resources/CustomizableFieldValueResource.javasormas-rest/src/main/webapp/WEB-INF/glassfish-web.xmlsormas-rest/src/main/webapp/WEB-INF/web.xmlsormas-ui/src/main/java/de/symeda/sormas/ui/ControllerProvider.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseController.javasormas-ui/src/main/java/de/symeda/sormas/ui/caze/CaseDataForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/configuration/AbstractConfigurationView.javasormas-ui/src/main/java/de/symeda/sormas/ui/configuration/customizablefield/CustomizableFieldEditForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/configuration/customizablefield/CustomizableFieldOptionsComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/configuration/customizablefield/CustomizableFieldTranslationsComponent.javasormas-ui/src/main/java/de/symeda/sormas/ui/configuration/customizablefield/CustomizableFieldsController.javasormas-ui/src/main/java/de/symeda/sormas/ui/configuration/customizablefield/CustomizableFieldsGrid.javasormas-ui/src/main/java/de/symeda/sormas/ui/configuration/customizablefield/CustomizableFieldsView.javasormas-ui/src/main/java/de/symeda/sormas/ui/contact/ContactController.javasormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/exposure/ExposureForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/exposure/ExposuresField.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/AbstractEditForm.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/CssStyles.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/CustomizableFieldsGroup.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInput.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputCheckbox.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputCheckboxList.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputCombobox.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputDate.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputDateTime.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputDecimal.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputFactory.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputNumber.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputRadioButtonList.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputText.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputTextArea.javasormas-ui/src/main/java/de/symeda/sormas/ui/utils/components/customizablefield/CustomizableFieldInputYesNoUnknown.javasormas-ui/src/main/webapp/VAADIN/themes/sormas/components/button.scsssormas-ui/src/main/webapp/VAADIN/themes/sormas/components/customizablefields.scsssormas-ui/src/main/webapp/VAADIN/themes/sormas/sormas.scsssormas-ui/src/main/webapp/WEB-INF/glassfish-web.xmlsormas-ui/src/main/webapp/WEB-INF/web.xmlsormas-ui/src/test/resources/META-INF/persistence.xml
… unit tests postgres v10
raulbob
deleted the
task-13886-customizable_fields_metadata_values_handling
branch
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
5 participants
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.
Fixes #13386
Summary by CodeRabbit
Release Notes
New Features
Access Control