Adam Young wrote:
Dmitri,
While I don't expect you to do the review of the patch, I would
appreciate at least a visual inspection of the completed UI. Since
there seems to be something wrong with the install/UI right now, I've
posed the lates on my Fedora People page. You should be able to see
it from here:
http://admiyo.fedorapeople.org/ipa/static/#navigation=2&role-entity=permission&ipaserver=0&permission-facet=details&permission-pkey=modifygroupmembership
I can see it.
Please let me know if you can or cannot see it. If you can, I'd like
to point out a couple things:
Selecting the object type radio button displays the attribute list in
a multi column form. Selecting one of the others hides it. Ben and I
decided to move it to the bottom, so that none of the other page
elements move. The values are from LDAP, and may be non-intuitive
for someone coming to IPA from outside of that realm. The set of
values displayed is from the sample data, which might need to be
updated. The live site on my machine has a smaller set. It made the
arbitrary decision to put 40 attributes per column, which is easy to
change.
This part is fine.
Biggest thing that was getting lost was the filter. So, we moved that
to the top, and added a checkbox for using/not using it. Since the
Filter can apply to any of the other types, v it was removed from the
radio button set. The radio button et needs a 'none' option, but that
requires that the filter checkbox be set. This will require a touch
more work.
The screen stopped making any sense to me. I do not understand the
structure now. Filter? What filter? Where it comes from? Why checkbox? I
am really confused by the current layout.
THe Add button off the list page uses the same code as the details.
The attributes table here pushes all of the buttons way down the page,
but I want to leave it that way until I confirm the the updates work.
Once Updates are fixed (Rob has a patch up for review already) We can
remove the attributes from this page. This means that creating a
permission that requires attribute values will happen in two stages:
add, with permissions and the object, then details page, where the
user will select the attributes. I can either leave the filter on, or
remove it from the add page as well. I think we need the rest of the
info that is on there in order to successfully get through the
permission-add rpc.
I have mixed feeling about this approach. What is the implication of
having half baked rule? Is it just meaningless until the attributes are
defined or it can potentially have a negative impact and open a security
hole?
I added another option to the radio button set: targetgroup.
I do not understand it.
This is showing up ion the ACIs that come up from permission-find. I
am currently populating the select with the values from doing a
group-find RPC. i've added a filter field, as we once again have the
possibility of having more than 100 groups, and needing to find a
group outside that set. CUrrently, the query re-exectes on click of
the radio button, but that is not ideal. Ben suggested that we could
resend the query with each keystroke, and re-populate the select box,
but that might generate a slew of network traffic, and slow down the
UI. I won't know unless I implement, and I've lower hanging fruit to
pick first.
Sorry this whole part just does not make sense to me. What is the target
group? Where it came from?
According to Rob, the set of permissions can be a mix of object level
(add and delete) and attribute (write) although this is odd. Thus,
the set of permissions is done via checkboxes. This does mean that
the user can select none, and will get an error back on submit.
I was hoping that we would be able to help to not mix things together.
If you specify add or delete you should not be able to drill down to the
attribute level forcing people to create explicit "write" riles
targeting attributes.
With Endi on PTO, Pavel is really the only one that can review this
code, and even he will have to take some time to learn the changes
that Endi and I have made since he was heads down in it.
In addition to the issues I explain above here is what I also noticed:
1) As we mentioned there is no "Description" in ACI. The description and
name is the same field for ACI.
2) There is a label it is the name of the task group the ACI is
associated with - it is missing
3) Rest of the screen does not make much sense at all but the attribute
part seems fine.
4) I do not like some of the levels on the left in the menu. It is all
mixed up.
5) The Privileges, Permissions and Role Groups are jumping and changing
places depending on your selection - this is wrong. They should just expand.
6) The hierarchy is broken for permissions
7) When editing privileges there is unclear what the enroll button would
do.
Options no include:
* Permissions Members
* Role Groups Members
* Membership in Permissions
Doe not make much sense. It is really confusing if the meaning of the
enroll button is going to change depending upon the selection made. But
currently it does not and it completely does not make sense.
There are more things like that related to the action panel.
In general though I like the style and where we are going with navigation.
