[Freeipa-devel] [PATCH] 680-682 webui: validation reporting improvements

[Petr Vobornik]

On 30.6.2014 16:23, Endi Sukma Dewata wrote:
> On 6/27/2014 3:54 AM, Petr Vobornik wrote:
>> On 27.6.2014 09:48, Fraser Tweedale wrote:
>>> On Wed, Jun 25, 2014 at 06:58:52PM +0200, Petr Vobornik wrote:
>>>> Patch 618 fixes a bug.
>>>>
>>>> Patches 680 and 681 were implemented along with it. They address
>>>> pspacek's
>>>> usability rant :).
>>>>
>>>> [PATCH] 680 webui: show notification instead of modal dialog on
>>>> validation
>>>> error
>>>> [PATCH] 681 webui: fix required error notification in multivalued
>>>> widget
>>>> [PATCH] 682 webui: focus invalid widget on validation error
>>>> --
>>>> Petr Vobornik
>>>
>>> ACK on 680 and 682.
>>>
>>> On 681: diff makes sense; I'm not 100% sure my testing has covered
>>> cases that were previously failing.  ACK if you're confident,
>>> otherwise could you provide steps to verify?
>>
>> You need to find a required multivalued field. One is in "Identity/Realm
>> Domains". Delete all values and hit update. It's little bit related to
>> ticket: https://fedorahosted.org/freeipa/ticket/4057
>>
>> Also when verifying validators in multivalued field, it's good to check
>> if errors are provided only for "invalid" values, etc..  good test field
>> is in "DNS/DNS Zones/some zone/Settings/ there is "Allow query" field
>> which accepts network address, "any" or "none".
>
> ACK.

pushed to master:
* 93de5db39e5b2e5991c32a57958cedb0f8b41848 webui: show notification 
instead of modal dialog on validation error
* c693b28babf97d22c14d37e024d551b583c4327f webui: fix required error 
notification in multivalued widget
* 99c5f0511f697cc54a9de7994c3e6999c6fd119f webui: focus invalid widget 
on validation error


>
> This should be sufficient to close #4057. But just wondering, the Realm
> Domains page right now is implemented as a details page with a
> multi-valued widget. Would it make more sense to be a list page instead?
> The realmdomains-mod CLI is kind of unusual too with the
> --add/del-domain parameters. Why not use realmdomain-add/del commands?
> Are there other commands implemented in this fashion?
>

Wrt CLI: Depends how you look at it and if/how much should our CLI/API 
reflect data structure.

If you say, we have an object which contains/manages the realm domains 
used by IPA server, then it's implemented as in every other similar 
object with multivalued attribute. But it's also a singleton therefore 
the closest similar thing is a configuration. There could also be an 
argument that this functionality could have been added to config itself.

If you say that each realm domain is an object or if you don't want to 
reflect the data structure and just focus on API then it's not 
implemented the best way.

Wrt Web UI: Makes sense. The list UI would fit better (until the 
realmdomains object is extended by additional attr).
-- 
Petr Vobornik