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

[Endi Sukma Dewata]

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.

-- 
Endi S. Dewata