[MAGNOLIA-6606] Variants Not Marked as Modified when Audience Selected Created: 03/Feb/16 Updated: 23/May/16 Resolved: 28/Mar/16 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | None |
| Affects Version/s: | 5.4.3 |
| Fix Version/s: | 5.4.6 |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Richard Gange | Assignee: | Oanh Thai Hoang |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | support | ||
| Remaining Estimate: | 0d | ||
| Time Spent: | 4d 7h | ||
| Original Estimate: | 5d | ||
| 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: | Saigon 37 | ||||||||||||||||
| Story Points: | 8 | ||||||||||||||||
| Description |
| Comments |
| Comment by Oanh Thai Hoang [ 25/Feb/16 ] |
|
Root cause: Solution: There are 3 ways to fix it: modify function shouldIgnoreUpdate in Core module or fix wrong property name in Personalization module 1/ Modify the function shouldIgnoreUpdate to support ignore mgn:variantTitle and mgnl:assignedSegments... It's a easy and simple solution but have one weakness: if user change the name of property, it couldn't work as expectation
+ Define the constants
/**
* List contains special property Names that we should force to update last update time whenever they change.
*/
private final static List<String> specialPropertyNames = Arrays.asList(
NodeTypes.MGNL_PREFIX + "variantTitle",
NodeTypes.MGNL_PREFIX + "variationOf",
NodeTypes.MGNL_PREFIX + "assignedSegments",
NodeTypes.Renderable.TEMPLATE
);
/**
+ And change the function to unignore list of special property names
protected boolean shouldIgnoreUpdate(final String propertyName) {
// our update doesn't count as update (activation, versioning, last update, etc) - except for changing the template
return propertyName.startsWith(NodeTypes.JCR_PREFIX) || (propertyName.startsWith(NodeTypes.MGNL_PREFIX) && !specialPropertyNames.contains(propertyName));
}
2/ Create SaveChooseAudienceDialogAction to update mgnl:lastModified property
+ Create SaveChooseAudienceDialogAction extends from SaveDialogAction. This class is used to update the lastModified property: NodeTypes.LastModified.update(node); + Change bootstrap file: magnolia-personalization-integration/src/main/resources/mgnl-bootstrap/personalization-integration/config.modules.personalization-integration.dialogs.chooseAudience.xml to update new save action class 3/ Fix wrong property name in Personalization module. It is a complex solution
|
| Comment by Oanh Thai Hoang [ 25/Feb/16 ] |
|
Hi, mgeljic]. Could you please have a look and approve for one of my solution above. Thanks in advance. |
| Comment by Mikaël Geljić [ 29/Feb/16 ] |
|
Right, this is an interesting and tricky one I just sat with pmundt to consider those options, here's what we can say for now:
I try to clarify feasibility of such option 4. If it's a dead-end, we may go for option 2. |
| Comment by Mikaël Geljić [ 17/Mar/16 ] |
|
Well, ok, option 4 is not realistic in a timely manner (we've got better fish to fry atm), but I started to think I could live with option 1 as well:
pmundt any opinion? Basically you can make the final call now |
| Comment by Philip Mundt [ 18/Mar/16 ] |
|
mgeljic I agree with you. I could also live with option 1. |
| Comment by Mikaël Geljić [ 18/Mar/16 ] |
|
And got the OK here, provided we don't make public constants out of it (specialPropertyNames is private so we're good on that end). Finally, just for the mention: option 5 // our update doesn't count as update (activation, versioning, last update, etc)
I doubt that set is gonna be any bigger. |
| Comment by Oanh Thai Hoang [ 21/Mar/16 ] |
|
Reopen due to changing approach. We should pick up option change in main only |