On 10/18/2011 02:25 PM, Endi Sukma Dewata wrote:
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.
Nice work. I am not 100% satisfied with the solution, but that is due
to limitations in the Javascript language. I think this is the best we
can get with the tools we have today.
A more perfect solution would allow us to use a lazy load proxy, and to
hide from an end component that the object is a proxy. the getters code
we had a while back sort of gave us that, but as we found, browser specific.
Ideally, when we set a Facet on an entity, that would be a lazy load
proxy, as would be the case where we explicitly link one entity up with
another.
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
