On 12/13/2010 11:05 AM, Dmitri Pal wrote:
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?

There should be no hole. Just the oppoosite, the rule should be useless, as 'write' without attributes means nothing.

But I agree, I don't really like it either.


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?

I'm wondering if this is from the groups ACI that slipped in.  Rob?

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.

This was my understanding, and how I think it should be as well: mixing the two is probably a mistake. We'll need to fixz the undering lying enforcement before changing it at the web layer, though, or the UI could potentially receive a rule that it could not display completely.


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.

Description is in the Meta data, and gets returned with ipa permission_show, role_show, and privilege_show

2) There is a label it is the name of the task group the ACI is
associated with - it is missing


It is not in the metadata.

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.
They do "just expand" and contract, but we don't have any animation in there. The order stays the same, but the are under each one either shows or hides the controls.


6) The hierarchy is broken for permissions
What hierarchy?


7) When editing privileges there is unclear what the enroll button would
do.

These are auto-generated associations. We need to filter some of them out, but I need to make sure we get the right subset.

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.

Yeah, we need to clean up the terms used on multiple associations pages, especially this one.

In general though I like the style and where we are going with navigation.


_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to