On 10/18/2011 10:52 AM, Petr Vobornik wrote:
> 3. Another goal is to replace entity names used in spec (see
> other_entity & nested_entity spec properties) with the actual entity
> objects. In this case it might be better to use the loops described in
> #2. This can be done separately.
Wouldn't it lead to the circular dependancy problem again? I think using
entity names and calling IPA.get_entity at the time when it is needed is
fine. But we should make some naming conversions of function params or
object properties to distinguish when we are working with just name or
entity itself.
It shouldn't. The circular dependency happens when we create an entity
that has a facet that depends on another entity, and that entity also
has a facet that depends on the first entity, but the first entity is
still being created. If we create all entities first separately, by the
time we create the facets all entities are already created.
Agreed on the parameter naming.
> 5. In entity.js:201, the use of entity.title for the breadcrumb tooltip
> might not be appropriate because usually the title is plural whereas the
> breadcrumb points to a single object. It would be better to use the
> entity.metadata.label_singular.
Fixed
The first and last elements in the breadcrumb actually don't have the
tooltip set so it will show a default tooltip which might not be
correct. This is an existing issue so we can fix this separately.
> 8. The facet_specs and dialog_specs lists can be replaced with
> ordered_map. It already has a method to find an element by its name.
> This can be done separately.
My intention was to be able specify entity facets and dialogs simply by
specifying their spec in entity spec (without calling builder function),
this is possible now. I didn't want to add initialization logic for
facet_specs (to fill the ordered_map) to entity in order to keep it as
simple as possible. It would be nice though. Maybe we could make an
utility object with methods for some simple initialization logic which
is used on many places - like checking if boolean is set or some more
complicated like initialization of ordered_map (needs key selector fn as
parameter).
I didn't mean to replace the spec with a class. I was referring to using
the ordered_map to replace the plain array (i.e. facet_specs and
dialog_specs) that contains the specs because the ordered_map already
has a method to lookup the elements by their name. So instead of:
entity.facet_specs.push(spec);
var facet = get_spec_by_name(entity.facet_specs, association_name);
it will be
entity.facet_specs.put(spec.name, spec);
var facet = entity.facet_specs.get(association_name);
This is not crucial, we can always improve it later.
ACK and pushed to master.
--
Endi S. Dewata
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
