[MGNLPN-354] Visitor trait not working properly Created: 21/Dec/16 Updated: 02/May/17 Resolved: 17/Mar/17 |
|
| Status: | Closed |
| Project: | Magnolia Personalization |
| Component/s: | None |
| Affects Version/s: | 1.3.1, 1.4 |
| Fix Version/s: | 1.2.10, 1.3.3, 1.4.3 |
| Type: | Bug | Priority: | Neutral |
| Reporter: | Ondrej Chytil | Assignee: | Antonín Juran |
| Resolution: | Fixed | Votes: | 1 |
| Labels: | support | ||
| 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
|
||||||||||||||||||||||||
| Release notes required: |
Yes
|
||||||||||||||||||||||||
| Documentation update required: |
Yes
|
||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||
| Sprint: | Kromeriz 87 | ||||||||||||||||||||||||
| Story Points: | 21 | ||||||||||||||||||||||||
| Description |
|
VisitorDetectorFilter sets the Visitor trait object as returning for the logged user and for anonymous user this value is set according to the cookie value. So basically in the end every request is returning visitor. In VisitorVoter then the cascade conditions don't allow to check only of user is logged in making this option unusable at all. Ideally only the selected values in the trait would be processed. Meaning that if the editor marks the checkbox for the logged user, only the logged user condition will be processed. Suggestions:
|
| Comments |
| Comment by Antonín Juran [ 15/Feb/17 ] | ||||||||
|
For RN: | ||||||||
| Comment by Roman Kovařík [ 17/Feb/17 ] | ||||||||
| Comment by Antti Hietala [ 22/Feb/17 ] | ||||||||
|
I would favor ease-of-use and fewer values in the visitor trait UI. While the proposed UI allows the editor to choose any combination of values, in practice only New, Returning and LoggedIn have meaningful use cases. I would propose that we simplify the ruleField and valueField to meet Ondrej's original request, see screenshot. Why I think these 3 values are enough:
Technically possible values but very rare. I would leave these out:
| ||||||||
| Comment by Roman Kovařík [ 22/Feb/17 ] | ||||||||
|
This wouldn't be compatible with the current impl and it wouldn't be possible to migrate without losing the current behaviour. | ||||||||
| Comment by Antti Hietala [ 22/Feb/17 ] | ||||||||
|
Splitting the behavior between existing and new installations is unfortunately not an option. It's not something we typically would do. We'd have to at some point merge the behaviors or maintain two branches forever. Proposal: Let's merge multiselect choices to single-select like this:
Example: Registered to Returning Before the upgrade, an editor targets content to Registered visitors. She checks the Registered checkbox and Magnolia stores her choice as registered=true in the voter. In the upgrade we migrate the stored property to value=returning (This is how SimpleTraitValueTransformer used with option group field would store the value. Just an example, possibility.) After the upgrade the variant still behaves as expected since a registered user is by definition a returning user. Personally unidentifiable content such as "Welcome back" is shown to the visitor and that's OK. Exclusive members-only content is not served because it should only be served to logged-in visitors. When the editor opens the Choose audience dialog she sees that her previous choice has been mapped to a new choice. The Returning radio button is now selected. | ||||||||
| Comment by Antonín Juran [ 23/Feb/17 ] | ||||||||
|
ahietala We have cookie form processor and registered user detector. This functionality will be lost after transformation of registered user to returning user. | ||||||||
| Comment by Antti Hietala [ 27/Feb/17 ] | ||||||||
|
ajuran, I would prefer to keep (not deprecate) the cookie form processor and registered user detector. If a client wants to add the Registered option (radio button) in the UI himself, the underlying functionality should be ready to process the option. | ||||||||
| Comment by Milan Divilek [ 08/Mar/17 ] | ||||||||
|
This should not depend on variants order, but exact priority should be defined. |