[MGNLUI-3633] DelegatingMultiValueSubnodeTransformer behaves erratically Created: 22/Oct/15 Updated: 05/May/16 Resolved: 19/Nov/15 |
|
| Status: | Closed |
| Project: | Magnolia UI |
| Component/s: | forms |
| Affects Version/s: | 5.4.2 |
| Fix Version/s: | 5.4.4 |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Mikaël Geljić | Assignee: | Oanh Thai Hoang |
| Resolution: | Fixed | Votes: | 2 |
| Labels: | i18n, multifield, support, transformer | ||
| Remaining Estimate: | 0d | ||
| Time Spent: | 6d 2.5h | ||
| Original Estimate: | 4d | ||
| Attachments: |
|
||||||||||||||||||||
| 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: | |||||||||||||||||||||
| Sprint: | Basel 23 | ||||||||||||||||||||
| Story Points: | 8 | ||||||||||||||||||||
| Description |
|
The DelegatingMultiValueSubnodeTransformer doesn't work well with i18n, since 5.4.2. Given the following MultiValueFieldDefinition:
class: info.magnolia.ui.form.field.definition.MultiValueFieldDefinition
transformerClass: info.magnolia.ui.form.field.transformer.multi.DelegatingMultiValueSubnodeTransformer
field:
name: text
class: info.magnolia.ui.form.field.definition.TextFieldDefinition
i18n: true
When I add a new entry to the multi-field in English ("en") and I switch to German
There seem to be similar quirks when removing entries as well. Several notes:
|
| Comments |
| Comment by Aleksandr Pchelintcev [ 28/Oct/15 ] |
|
This should a problematic part: Node rootJcrItem = rootItem.getJcrItem(); try { if (rootJcrItem.hasNode(definition.getName())) { ... Since the new logic - the fields as well as transformers get re-created when locale is switched. Hence, the sub-node get also re-initialized. The snippet above proposes the following logic: Now we can end-up in a third state - the child item might have been created already, but hasn't been yet persisted => step 0: Checking that would anyway be a right/safer thing to do. That's just a thought from top of my head, but hope it works/helps. |
| Comment by Eric Hechinger [ 02/Nov/15 ] |
|
Hello mgeljic Given the following MultiValueFieldDefinition:
class: info.magnolia.ui.form.field.definition.MultiValueFieldDefinition
transformerClass: info.magnolia.ui.form.field.transformer.multi.DelegatingMultiValueSubnodeTransformer
*i18n: true*
field:
name: text
class: info.magnolia.ui.form.field.definition.TextFieldDefinition
i18n: true on the multifield level. generate another behavior. (correct in my opinion)
|
| Comment by Oanh Thai Hoang [ 02/Nov/15 ] |
|
After testing for this bug in versions: 5.4.1, 5.4.2 and 5.4.3-Snapshot (please refer to Testresultin3bundle5.pdf |
| Comment by Mikaël Geljić [ 02/Nov/15 ] |
|
Hi ehechinger, I agree your case works, but you're changing the "Given"
At least we don't make any assumption, customers choose what suits them best. |
| Comment by Oanh Thai Hoang [ 05/Nov/15 ] |
|
Thanks apchelintcev for your suggestion. Hi mgeljic, apchelintcev This is issue and I reproduced bug on version 5.4.2 The issue happens when user switching languages in form has MultiValueFieldDefinition field using DelegatingMultiValueSubnodeTransformer transformer. For my understanding: Nowadays (from 5.4.2), it does not work as before. When user switch to a new language, a new form for a new language will be created, new transformer, new field... The form has many section instances per language but still access the same component instance. We can imagine that 2 languages: 2 forms (each form is built one time) and different fields due to UI events. So for the case : adding new entry in "en" and switch to "de" -> the entry disappears. The reason is data for entry in "de" has been gone because we will create new transformer for de and data hasn't persisted to JCR. For my approach: 1. What we should do is checking data in transformer, I intend the checking is here: public DelegatingMultiValueSubnodeTransformer(Item relatedFormItem, ConfiguredFieldDefinition definition, Class<PropertysetItem> type, I18NAuthoringSupport i18NAuthoringSupport) { super(relatedFormItem, definition, type, i18NAuthoringSupport); // In case the child item has been create already but hasn't persisted yet if (subNode == null) { if (((JcrNodeAdapter) relatedFormItem).getChildren().containsKey(definition.getName())) { subNode = (JcrNodeAdapter) ((JcrNodeAdapter) relatedFormItem).getChildren().get(definition.getName()); } } } For my site, I don't think persist data to JCR before switching is good way, so continue to using that data for transformer that is acceptable. 2. Another problem come from transformer is how can we deal with storing a single set of entries. We should check the data exist before creating new data:
final JcrNodeAdapter child;
// Should be check the new Item Name existed because it can be created already from other language form.
if (getRootItem().getChild(newItemName) != null) {
child = (JcrNodeAdapter) getRootItem().getChild(newItemName);
} else {
child = new JcrNewNodeAdapter(getRootItem().getJcrItem(), childNodeType, newItemName);
}
There are 2 parts I found out that it make data can not store correctly. Here is my patch https://gist.github.com/oanhthai/32ab495aa6bf02dee6ae. Please feel free to review it. Cause the form presenter does not work as before. So keeping the same fields for i18n set to false on a MultiField and i18n to true on sub-fields is impossible but if we somehow make the FormBuilder can build again while switching language can help to build the same fields for each form of locale. For my point of view about user exp: i18n set to false on a MultiField and i18n to true on sub-fields 2. Make the Form Ui run the same as 5.4.1 if we provide the way to force the line has '*' can run if (currentFormSection != null) { FormSection newFormSection = Iterables.tryFind(newFormSections.values(), new FormSectionNameMatches(currentFormSection.getName())).orNull(); if (newFormSection == null) { *newFormSection = formBuilder.buildFormTab(tabDefinition, itemDatasource, null).getContainer();* *newFormSections.put(tabDefinition, newFormSection);* } ((SingleComponentContainer) currentFormSection.getParent()).setContent(newFormSection); } this code come from FormPresenterImpl class |
| Comment by Oanh Thai Hoang [ 05/Nov/15 ] |
|
Another patch related to Form presenter https://gist.github.com/oanhthai/d4bf5c3a8f16c45aa647. It will help the activeLocale always get correct locale at the first time (While checking source code, I found that the activeLocale always null at the first time). |
| Comment by Aleksandr Pchelintcev [ 13/Nov/15 ] |
|
oanh.thai I guess it is a good time to transform these comments and patches into a branch and a pull-request because it'll be easier to carry on with the review and discussion. Anyway from what I've read so far - it seems to mostly go in line with what I pointed out initially, so I guess you can go ahead. As for the form presenter issue - we should extract it into a separate ticket. |
| Comment by Jennylyn Sze [ 29/Apr/16 ] |
|
Is this issue really fixed in 5.4.4? I'm using 5.4.6 and still have this issue. Thanks! |
| Comment by Oanh Thai Hoang [ 05/May/16 ] |
|
HI jenny_87], I'm sorry for late reply. The real problem is that the multi field doesn't listen to data-source changes so ideally as you switch the language - the field should already look the same way (given it's not i18n-ized itself). So the actual issue doesn't fix yet. And it belongs to https://jira.magnolia-cms.com/browse/MGNLUI-3668. |