[MGNLUI-3402] CompositeFieldDefinition doesnt intialize fieldsName array sometimes Created: 21/Apr/15 Updated: 06/Aug/15 Resolved: 28/Apr/15 |
|
| Status: | Closed |
| Project: | Magnolia UI |
| Component/s: | forms |
| Affects Version/s: | 5.3.8, 5.4 |
| Fix Version/s: | 5.3.9 |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Christopher Zimmermann | Assignee: | Mikaël Geljić |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| 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)
|
||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||
| Date of First Response: | |||||||||
| Description |
|
If getFieldsName is called before setFields or addFields populates the fields collection, then the fieldsName array will never be populated based on the fields. This is because getFieldsName sets the fieldsName collection to an empty collection. - so that initFieldsName will not populate it the next time getFieldsName is called, even if setFields does add some fields. This problem occurs with YAML configuration - as it does call getFieldsName before setFields. |
| Comments |
| Comment by Christopher Zimmermann [ 21/Apr/15 ] |
|
updateFieldsName method replaces initFieldsName. It is called when addField, setFields and addFieldName is called so that the fieldsName collection is sure to contain an entry for all Node2Bean / Map2Bean calls and direct calls to addFieldName. |
| Comment by Christopher Zimmermann [ 21/Apr/15 ] |
|
I did discuss the approach with Eric beforehand. the fieldNames must include the names of the fields list - this is the bug that needs to be fixed. I'll ask Eric to take a look too. Except... I'm not aware of this method: getFieldsNameHasNoSideEffects(). |
| Comment by Christopher Zimmermann [ 22/Apr/15 ] |
|
| Comment by Mikaël Geljić [ 24/Apr/15 ] |
|
Guys,
Cheers, |
| Comment by Christopher Zimmermann [ 28/Apr/15 ] |
|
Made CompositeFieldDefinition a simple definition and added the fieldName logic to SwitchableFieldFactory. |
| Comment by Mikaël Geljić [ 28/Apr/15 ] |
|
Thanks, Reviewed
I also took the occasion to fix/improve CompositeFieldDefinition javadoc, and to reorder getter/setter methods for fields. |
| Comment by Mikaël Geljić [ 28/Apr/15 ] |
|
List of fieldNames is obtained via guava transform, which gives a view on the original list—while some transformers (e.g. MailSecurityTransformer) are modifying/clearing that list. |