On 10/10/2011 10:13 AM, Petr Vobornik wrote:
https://fedorahosted.org/freeipa/ticket/1531

(3.0 Core Effort Iteration 01 September Y11 Release)

Implemented solution:
* all entities are created on application start
* dependant objects (facets and dialogs) are created at once on their
first use in entity.

Note(patch naming): patch 022 was second part of 021, but the file name
was wrong(021-1)

Some comments/issues:

1. One of the goals of this bug is to remove the temporary workaround in IPA.search_facet.create_content(). We should now be able to call the initialize_table_columns() during facet initialization.

2. Using lazy-loading to create entities, facets, and dialogs makes object creations a little bit unpredictable. This is probably fine for now, but if there's a problem the other option is to create all objects during application initialization. We can use a loop to create all entities first, then use another loop to create all dependent objects in each entity.

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.

4. In the original code, when creating a facet for indirect association it will try to find the corresponding direct facet and use it instead of creating a new one. In the new code, the indirect facet will always be created, but since there is no indirect facet group the facet will never appear. It would be better if we can avoid unnecessary creation of indirect facets.

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.

6. Invoking a method by concatenating the method name dynamically such as prepare_<facet type>_spec will work, but it's more error prone and will clutter up the namespace. It would be better to store the methods in a map like this:

  that.map.put('search', function(spec) {
    ...
  });

and use it like this:

  var method = that.map.get('search');
  method(spec);

This can be done separately.

7. The code in entity.js:474,998,1000 should have a deeper indentation because it's a continuation of the previous line.

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.

--
Endi S. Dewata

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to