[Freeipa-devel] [PATCH] 074 Automember UI - default groups

Endi Sukma Dewata edewata at redhat.com
Tue Feb 7 06:51:20 UTC 2012


On 2/6/2012 9:56 AM, Petr Vobornik wrote:
> Updated patch attached.

ACK and pushed to master and ipa-2-2. See the comments below.

>> 1. When default user group is enabled, selecting the drop down list to
>> empty will generate an error: 'automemberdefaultgroup' is required. If
>> you click Cancel the UI will show empty value but the default user group
>> is actually still set on the server. I think when you select empty value
>> the UI should call automember-default-group-remove.
>>
>> 2. When default user group is disabled, clicking enable then disable
>> will generate an error: No default group set. Having a checked checkbox
>> with no selected value in the drop down list is actually equivalent to
>> having an unchecked checkbox. So I think the checkbox is redundant and
>> can be removed. We can disable the default user group by selecting an
>> empty value as in #1.
>
> #1 #2: Removed 'enable checkbox'. Seems less error prone. The question
> is: Is it sufficiently clear?

I don't think removing the Enabled checkbox makes it less clear than the 
original design. There's no concept of 'enabled' in the automember docs 
either. But whether this is intuitive enough is another question. See #3 
below.

There's one minor issue. Try reloading the page when you don't have a 
default group, then click the drop down list, the list is empty, you'd 
have to click the search icon to populate it.

>> 3. The position of the default user/host group interface seems to be
>> inconsistent with the rest of the UI. The behavior is also different,
>> when you change the value it updates the server immediately without
>> clicking any Update button. In my opinion it would be better to move it
>> into a separate facet. So when you open the User Group Rules you'll see
>> two tabs, one for the Rules (default), and the other for Settings. The
>> Settings will have a field with a drop down list for the default user
>> group. Any changes will have to be saved by clicking the Update button.
>
> I agree on the part that it is inconsistent with the rest of the UI and
> user is not properly informed that the changes are immediately executed.
>
> Nevertheless I wouldn't move it to other facet. It seems to me as too
> much clicking if you want to just check the default group.
>
> I can imagine this improvement: if a command is successfully executed it
> would show on the right of the combobox a message saying: "default group
> set" or "default group removed". The message can have style as the
> message used in adder dialog (after "add and add another"). The message
> would fade out after couple (5?) seconds.

It will work but still kind of inconsistent because there's no Update 
button to confirm your selection.

Another possibility, we can change the page to look like a details page. 
So it will have 2 sections: Default Group and Rules. The Default Group 
section will have one field: User Group with a drop down list. It also 
will have an Update button for this section only. The Rules section will 
have a search table with the Delete & Add buttons and the search field.

>> 5. The server uses the same automemberdefaultgroup attribute to store
>> the default group DN and the error message "No default group set". To
>> distinguish the result the UI (and possibly other applications using the
>> JSON RPC) will have to use a non-standard way such as checking whether
>> the attribute starts with a 'cn='. I think if there's no default group
>> the server should not return this attribute at all and return the
>> message in a separate error attribute.
>
> Yes it is inconsistent with the rest of the API. Agree on the empty
> part. Not having set default group is not an error so the message
> shouldn't be in error attribute. From UI perspective the message is not
> needed but I think CLI is using it.

I opened this bug: https://fedorahosted.org/freeipa/ticket/2346

-- 
Endi S. Dewata




More information about the Freeipa-devel mailing list