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?
>
>
>
>> 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.
>
>
One other thing that I noticed.
Can we have some kind of differentiation between the static part of the
label and dynamic part.
Right now the whole label:
role(s) enrolled in privilege useradmin
looks like one static text label. I suggest we have some kind of
stylistic difference between the static label and the inserted value so
that one can easily grasp the object currently viewed.
In this case it is "useradmin". I will leave it to Ben & Kyle to decide
how to make it distinct but it should definitely be.
And as Devid mentioned we should not do "(s)" - it is really ugly.
I hope that for this particular example the whole text will be different.
First we use "role groups" term (though I do not like it - I would
prefer just roles) secnderily the word "enrolled" is misleading here.
I see the label being:
Role groups that include privilege "useradmin"
or
Role groups that contain privilege "useradmin"
--
Thank you,
Dmitri Pal
Sr. Engineering Manager IPA project,
Red Hat Inc.
-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
