[Freeipa-devel] [PATCH] 749-754 webui: new ID views section

[Endi Sukma Dewata]

On 9/24/2014 9:43 AM, Petr Vobornik wrote:
> On 24.9.2014 16:30, Endi Sukma Dewata wrote:
>> On 9/19/2014 7:29 AM, Petr Vobornik wrote:
>>> Hello,
>>>
>>> attached patches implements Web UI part of ID Views. Backend is
>>> currently on review as well - thread "[PATCHES 247-259] ID views -
>>> management part".
>>>
>>> https://fedorahosted.org/freeipa/ticket/4535
>>>
>>> I expect that backed can change and that the UI might influence it as
>>> well. Therefore no UI integration tests nor static data files are
>>> included in this patchset. They will follow when backend is stable.
>>>
>
> snip
>
>>
>>> == [PATCH] 754 webui: new ID views section ==
>>
>> I'll test this separately. Does your patch #754-1 work with Tomas'
>> latest patches (#247-2 - 270)?
>>
>
> It was tested with tomas' git branch which matched
> http://www.redhat.com/archives/freeipa-devel/2014-September/msg00336.html
>

OK, some comments/questions:

1. For consistency, the "ID view" should be capitalized into "ID View" 
in the navigation tab, page title, and dialog title. See "ID Ranges" as 
an example.

2. The tab titles in the ID view details page are quite long, and the 
"User ID overrides" and "Group ID overrides" labels aren't quite 
appropriate because the ID view can override other attributes too. How 
about using facet groups like in User Groups? For example:
  - <ID view> applies to:
    - Hosts
  - <ID view> overrides:
    - Users
    - Groups
  - Settings

3. Since the tab already says "Applied to hosts", the current button 
labels is kind of redundant. How about renaming and reorder the buttons 
like this?
  - Refresh
  - Remove
  - Add
  - Add hosts in host group
  - Remove hosts in host group

4. If I understand correctly the description field for the User ID 
Overrides and Group ID Overrides should be optional too because it's 
also used to optionally override the description attribute of the 
original entry.

5. Not sure if this is a problem. The search field in User/Group ID 
Overrides can be used to find the overriding attributes, but not the 
"User/Group to override".

6. Can multiple ID views be applied to the same host? Does the order 
matter? If so, how would the UI manage the order?

7. Related to #6, there probably should be a tab in the Host details 
page showing which ID views apply to it.

8. If we implement #7, are the "Un-apply from hosts/host groups" buttons 
in the ID views search page still necessary? Or can it be moved into 
that page (i.e. unapplying one host at a time)?

9. This probably requires server support. In the "Apply to hosts" 
association dialog, if a host is already added it will still appear in 
the dialog box. As a comparison, a User that has been added into a User 
Group will not appear in the association dialog anymore.

10. The use of association dialog for "Apply to host groups" and 
"Un-apply from host groups" is a bit unusual because it's used to select 
host groups, and once selected the host groups are not added to/removed 
from the main list because the main list contains hosts, not host 
groups. Would it be very common to select multiple host groups at once 
to apply ID view? If not, it might make more sense to just use a plain 
dialog to select one host group at a time. The dialog could probably 
show information about the host groups (i.e. host members) before 
applying/unapplying the ID view.

I think the only major issue is #4. Once it's addressed, ACK.
The others are minor issues, questions, and suggestions.

-- 
Endi S. Dewata