[MGNLUI-2995] MultiField shouldn't rely on index/count of the Vaadin child components to define property ids Created: 12/Jun/14  Updated: 16/Jun/14  Resolved: 16/Jun/14

Status: Closed
Project: Magnolia UI
Component/s: forms
Affects Version/s: 5.3
Fix Version/s: 5.3

Type: Improvement Priority: Major
Reporter: Mikaël Geljić Assignee: Mikaël Geljić
Resolution: Fixed Votes: 0
Labels: multifield
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
dependency
is depended upon by MGNLPN-156 Enable setting AND/OR operand in trai... Closed
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Date of First Response:

 Description   

MultiField internally maintains its entries as a PropertysetItem, whose entry keys are integers (0-indexed), and whose values are the subfields' properties.

Currently, it calculates these integers based on count or index of its child components, which is rather fragile e.g. in case we want to have 'static' components in addition to the variable amount of subfields.

In that case, delete removes the wrong property due to mismatch between propertyIds and subfield index within the parent component.

We should carefully track what propertyId an entry is created with, and use it for deleting. In case we have to generate a new position manually, size of the itemPropertyIds collection is a better choice over componentCount.



 Comments   
Comment by Mikaël Geljić [ 13/Jun/14 ]

https://git.magnolia-cms.com/gitweb/?p=magnolia_ui.git;a=shortlog;h=refs/heads/MGNLUI-2995-mge
There are no existing unit tests for MultiField, however it is well covered by UI tests which passed like a charm.

Comment by Mikaël Geljić [ 16/Jun/14 ]

Relying on propertyId set during initialization was a no go (1st attempt, commit 0a0d31f), because after deleting one entry, itemIds are reorganized, see MultiField#removeValueProperty(int).

In order to make sure we always delete the expected property, we fetch it again dynamically during remove (2nd attempt, commit e91a489), see #findPropertyId(Item, Property).

See new branch https://git.magnolia-cms.com/gitweb/?p=magnolia_ui.git;a=shortlog;h=refs/heads/MGNLUI-2995-2-mge

Generated at Mon Feb 12 09:02:11 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.