[Freeipa-devel] Web UI refactoring effort ready for review

[Martin Kosek]

On 05/03/2013 07:35 PM, Endi Sukma Dewata wrote:
> Hi,
> 
> Sorry for the delay, I have some questions & comments.
> 
> Registry:
> 
> In the simpleuser.js the new 'user' entity is registered first then the old
> 'user' entity is removed, which could be confusing because they are both
> identified using 'user'. Should register() automatically remove the old object?
> Ideally a class should have complete methods to manage the objects it stores
> (e.g. unregister(), remove()).
> 
> How is reg.entity created? Are there others beside 'entity'?
> 
> How is Registries_registry in reg.js used? It doesn't seem to be used anywhere
> else.
> 
> Plugins:
> 
> In plugins.py the list of plugins is generated using os.listdir(). Then each
> plugin also has a list of dependencies which I suppose can include other
> plugins. Then when registering the plugin task, it will have a priority as well.
> 
> So there seem to be several factors that determine the execution order of the
> plugins. There should be a document explaining how this will work, so plugin
> writers can be sure that the code will be executed at the right time.
> 
> In general I'd avoid using task priority because it doesn't guarantee the
> correct execution order unless the priorities of all tasks are well coordinated
> (which might be challenging if there are multiple plugins owned by different
> people).
> 
> Could you add more examples of simple plugins for various scenarios including
> custom entity, custom facet, custom field, custom menu? They can be included in
> the RPM for reference.
> 
> Writing a plugin seems to still require programming skills, reliance on good
> docs, and probably even some source code familiarity. What do you think about
> simplifying this a little further? So we'll have 2 ways to define a plugin: one
> is programatically using the current framework already implemented (e.g.
> simpleuser.js), and the other is completely declaratively using a plain json
> data (e.g. simpleuser.json). The declarative plugin will obviously be more
> limited, but much simpler to use.
> 
> Builder:
> 
>> b) Second big issue was build of objects. Entities and facets have
>> complex build logic. It can be simplified into three steps:
>>      1) modifications of spec
>>      2) creation of object and class inheritance
>>      3) init logic
> 
> Yes, creating an object has become very complicated now with the builders,
> factories, constructors, preops, postops, inits, overrides, diff, etc. I think
> the problem is that we're trying to create/modify the spec before creating the
> object and we need a whole set of mechanisms to do that. Maybe we can simplify
> it into two basic steps:
> 
> 1. Create an empty/simple object.
> 2. Initialize the object.
> 
> The initialization process could be split further into smaller operations such as:
> 
> * Load the spec and modify it if necessary
> * Creating dependent objects and initializing them
> * Other initialization steps
> 
> The builder, factory, preops, and postops can be included as part of the
> initialization step. They can be normal class methods rather than loosely
> defined functions and can be overridden by subclasses. There's probably a lot
> more details that need to be discussed.
> 
>> 1. Move ./_base/metadata_provider to ./metadata?
>> Might simplify stuff.
> 
> This seems to be IPA-specific, so yes.
> 
>> 2. Move actions/buttons spec from factories to pre_ops associated with
>> the factories.
>>
>> Example of stuff to be moved (search.js):
>>       spec.actions = spec.actions || [];
>>       spec.actions.unshift(
>>           'refresh',
>>           'batch_remove',
>>           'add');
>>
>> It may simplify/allow removal of those items in spec or pre_ops of child
>> factories. Currently there is no way how to intercept them.
> 
> Sure, I don't see any problem with that.
> 
> In general there is no major issue that would warrant a NACK. As long as the
> API is well documented for plugin writers it should be sufficient.
> 

Thanks for review Endi! Since we do not have a NACK, lets do small changes we
can do now for 3.2 GA and create tickets for the rest (can be done in 3.2
stabilization phase).

Martin