[Freeipa-devel] [PATCH] 886-890 webui: API browser

[Tomas Babej]

On 07/03/2015 10:06 AM, Tomas Babej wrote:
> 
> 
> On 07/02/2015 04:55 PM, Martin Kosek wrote:
>> On 07/01/2015 04:51 PM, Petr Vobornik wrote:
>>> For those of you who don't want to try the patches:
>>> * https://pvoborni.fedorapeople.org/images/api-user-show.png
>>> * https://pvoborni.fedorapeople.org/images/api-user-add.png
>>>
>>> On 07/01/2015 09:35 AM, Martin Kosek wrote:
>>>> On 06/30/2015 06:35 PM, Petr Vobornik wrote:
>>>>> First part of API Browser - displaying the metadata in more consumable way.
>>>>>
>>>>> Second part, how to use it in different languages will be written as wiki pages
>>>>> first.
>>>>>
>>>>> The browser could be later enhanced with more infos and tooltips.
>>>>>
>>>>> Patch 886 extends backend to send more metadata.
>>>>> Patch 887,888,889 are webui fixes and prerequisites
>>>>> Patch 890 is the API browser
>>>>
>>>> Thanks, this is a very good start. I looked at a VM with the patches and have
>>>> couple usability suggestions:
>>>>
>>>> 1) It was hard for me to find where the API Browser is. But "IPA Server" looks
>>>> as a good tab where it should be though.
>>>
>>> could be moved to "Help" tab when it's introduced. For that we need at least
>>> one more link.
>>>
>>>>
>>>> 2) I have strong doubts about the "Objects" tab, this is only understandable to
>>>> users knowledgeable about FreeIPA framework internals. Common API user who just
>>>> want to consume the API and not know about the internals will not know what
>>>> this is.
>>>>
>>>> What I would do is make "API Browser" directly clickable so that it opens the
>>>> Commands tab. This is what most people will use. Other tabs may be stacked on
>>>> the left just like with Staged or Deleted users. For now, I would hide Objects
>>>> as I think it would cause more confusion. If we want to show it, there should
>>>> be some introduction what it is good for and maybe limitation of showed fields
>>>> to only those that has any value for the consumers.
>>>
>>> fixed, there is only "API Browser" and no submenu
>>>
>>>>
>>>> 3) In Commands tab, we will some more explanatory what the attributes of Param
>>>> needs and probably hide some. For example "exclude" is not needed for consumers.
>>>>
>>>
>>> Attributes as follows were kept: label, type, default, default_from, values,
>>> minlength, maxlength, pattern, minvalue, maxvalue, precision, cli_name,
>>> option_group
>>>
>>>> 4) Many attributes have "autofill": True. I wonder how usable it is without
>>>> knowing the actual default for the attribute. Can we show the default?
>>>
>>> default_from now contains list of attrs which are used for the default value,
>>> e.g.:
>>>   default value created from: givenname, sn
>>>
>>>
>>>>
>>>> 5) I would hide "Output Params" all together given we don't have them set up
>>>> correctly in FreeIPA framework and they may rather confuse people, with having
>>>> all the HBAC or SUDO with User objects.
>>>>
>>>
>>> Removed from metadata
>>>
>>>> I may think about it more, there were just my couple first thoughts. Others may
>>>> have different opinions here.
>>>>
>>>> Martin
>>>>
>>>
>>> Other changes:
>>> * cli options are shown with dashes as in CLI
>>> * required and multivalued were changed into tags next to option name. 'flags'
>>> which were shown as the tags are not displayed anymore
>>>
>>>
>>> updated patches attached.
>>
>> I like the new version, good job! ACK from my side.
>>
> 
> Patchset works fine in my testing.
> 
> Two (nitpick) questions:
> 
> 1.) Should we show objects that have defined no methods or params as
> 'pkinit'?
> 
> 2.) Relationships and attirbute members are displayed in rather raw form:
> 
> attribute_members:
> {"member":["user","group"],"memberindirect":["user","group"],"memberof":["group","netgroup","role","hbacrule","sudorule"],"memberofindirect":["group","netgroup","role","hbacrule","sudorule"]}
> 
> relationships: {"member":["Member","","no_"],"memberindirect":["Indirect
> Member",null,"no_indirect_"],"memberof":["Member
> Of","in_","not_in_"],"memberofindirect":["Indirect Member
> Of",null,"not_in_indirect_"]}
> 
> 
> Could we maybe special-case these (or introduce a way how to detect and
> display a dict of lists as formatted html)? Or are there arguments for
> the raw view of the API?
> 
> Tomas
> 

On a second thought, I don't think these issues should block this
patchset for now. We can improve/discuss that in 4.2.x.

Pushed to master: 2a976334c2160c91a61fb0c477777e7adbbd3150