We have moved to GitHub Issues
Created by Shannon Deminick 18 Jul 2017, 01:59:57 Updated by Shannon Deminick 01 Aug 2017, 07:53:10Tags: Unscheduled
Relates to: U4-239
Relates to: U4-8428
Relates to: U4-10075
Subtask of: U4-8632
Here are the current (simple) rules:
*A user can have zero, one or many start nodes, which can include the 'root' or an ID *A group can have zero or one start node which can be the 'root' or an ID *If a user's combined start nodes is empty or contains 'root', then they have access to root, otherwise they have multiple start nodes
This however means that if there are more restrictive start nodes set they really won't make any difference if the user or user group has less restrictive or root access start nodes so we have 2 options:
*Make a user's explicit start nodes overrule any of it's user group's start nodes OR *Calculate start nodes based on: **If the user or user group has root access but there are other non root start nodes selected, then we can decide that root is actually not allowed **We'd have to lookup all Paths for all nodes in the start node list and then de-duplicate based on path matches and only keep ones with the deepest level
First, if a user's combined start nodes is empty, then the user should have no start node. At all. Just to be consistent.
Next, trying to recap things:
Combined start nodes:
Start nodes are the only thing we can set on a user without creating a group.
Option A (current): user start nodes ''add'' start nodes, so they are added and de-duplicated with combined groups start nodes, keeping the upmost nodes
Option B: user start nodes ''define'' start nodes, so they entirely replace combined groups start nodes, then are de-duplicated, keeping the upmost nodes
Option C: user start nodes ''add or restrict'' start nodes, so they are added and de-duplicated with combined groups start nodes, keeping the deepest nodes
I am not a big fan of B, and think A is not enough. Would prefer C, which is similar to what you propose, but in a more general way: if a user start node is above or below a group start node, then it takes over. Else it's just added.
Now it means that getting the start nodes requires processing the paths which (a) has a perfs impact and (b) means we need paths ie IEntityService
It does implement option C in UserExtensions:
So if the tree is N1
Can be explained to users this way:
Would this make sense?
It all works quite well by my tests - but I hate having to carry the entity service around to get paths, or reference ApplicationContext.Current = ideas?
(not moving to review bc we cannot merge the code as-is due to ugly entity service usage)
I've updated the code since we shouldn't have a Model (IUser) calling back to services:
*The properties AllStartMediaIds and AllStartContentIds are new properties so we have the luxury of just removing them ... which I've now done.
*Now anytime these calculated start nodes need to be retrieved, the extension methods will be used
*I've added object level caching for the result of these calculations so they are not re-calculated over and over again
*I've renamed the methods since
GetAllContentStartNodes isn't really what this doing, these methods are calculating them.
*I've updated all of the unit tests that relied on the "AllStart..." properties
*I've added some TODO notes
Let me know what you think.
all good - and have done some of the TODOs (actually, did it yesterday I forgot to push) - now testing the caching part of it
happy w/caching - changing start nodes on a user currently logged in, and it immediately refreshes - good, merging
ah no actually it's my code, for you to merge ;)
Backwards Compatible: True
Due in version: 7.7.0
Sprint: Sprint 63
Story Points: 3