[MGNLPN-402] Trait field: voters adapters are not properly linked to their parent Created: 30/Sep/15 Updated: 24/Jan/18 Resolved: 01/Nov/17 |
|
| Status: | Closed |
| Project: | Magnolia Personalization |
| Component/s: | None |
| Affects Version/s: | 1.2.10 |
| Fix Version/s: | 1.2.12, 1.3.6, 1.4.7 |
| Type: | Bug | Priority: | Major |
| Reporter: | Sander K | Assignee: | Antonín Juran |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| 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: | Kromeriz 112, Kromeriz 113, Kromeriz 115, Kromeriz 116, Kromeriz 117, Kromeriz 118, Kromeriz 119, Kromeriz 120, Kromeriz 121 | ||||||||||||
| Story Points: | 21 | ||||||||||||
| Description |
|
I created new trait with definition like in attached picture. When I am selecting audiance for page variation and this is only trait I select and I leave checkbox un-checked then audiance info is not saved in Magnolia repository. Also attached configuration xml and module code. Module code needs some tweaking to run in your environment, but I hope this might help with recreating the issue. |
| Comments |
| Comment by Antti Hietala [ 24/Aug/17 ] |
|
This looks like a bug. An unselected checkbox should be stored as value false. Even if you set the defaultValue property on the field it is not stored as false. We propose to change the behavior of binary fields like checkbox so that an unselected choice is stored as false. This issue is not specific to personalization although the ticket was originally entered as such. The issue should be fixed in UI. |
| Comment by Milan Divilek [ 08/Sep/17 ] |
|
Reopen: Unfortunately fix doesn't work... Reproduce: |
| Comment by Mikaël Geljić [ 20/Sep/17 ] |
|
Hello there, I think this issue might have been misunderstood, can you enlighten me? In one of the last comments from Thomas on the support ticket, he gives reproduction steps on the demo... with the select field. Not checkbox. Judging from the commits, I doubt this one is gone. As such, it was *not* a problem that checkboxes don't store any property when they have no defaultValue (one can still configure defaultValue=false to have it; now the other way is not possible anymore). If I read correctly, the issue was that the "surrounding trait" is not saved—not that it's missing a property. |
| Comment by Mikaël Geljić [ 21/Sep/17 ] |
—That's a pretty wild assumption. We have the required and defaultValue params for that. The current fix (forcing a defaultValue) is a behavior change; and cannot be applied to other fields for the simple reason that JCR does not allow null properties. See section 10.4.2.4 of the JCR spec. At any rate, such changes would have to be a conscious decision from PD/Architects. Right now, the checkbox behavior is "altered" (all the more in a minor release), but the bug remains. |
| Comment by Roman Kovařík [ 25/Sep/17 ] |
|
Reopened: issue remains for select boxes (possibly other fields) in the trait picker. |
| Comment by Milan Divilek [ 13/Oct/17 ] |
|
Reopen: Problem is that creating page variant with no traits (only with segments) saves empty trait voter into jcr.. See screenshots |
| Comment by Roman Kovařík [ 13/Oct/17 ] |
|
Could we fix the test? The behaviour seems correct. |
| Comment by Milan Divilek [ 13/Oct/17 ] |
|
Problem is that this blocks whole variant.. Variant is never choose/render. It's not problem of test |
| Comment by Roman Kovařík [ 13/Oct/17 ] |
|
Reverted, info.magnolia.personalization.ui.TraitAggregatorTransformer#removeProperty seems redundant with the fix for the RuleSet. |
| Comment by Milan Divilek [ 24/Oct/17 ] |
|
Reopen: In segments app untouched trait is not stored for new segment. |