[Freeipa-devel] [PATCH] 647-651 [webui] Make utility section of navigation extensible

[Endi Sukma Dewata]

On 6/18/2014 6:11 AM, Petr Vobornik wrote:
>> 1. As discussed on IRC, the plugin is causing an error due to missing
>> extend.js. This needs to be fixed.
>
> Fixed

>> 4. I agree that the facet shouldn't define the hash. The hash should be
>> part of the plugin declaration.
>
> Ideally, facet should be router agnostic. We need a mechanism of mapping
> facet's state to hash and vice versa.
>
> Originally I did not want to do it now because it requires bigger
> refactoring of a router.
>
> While I was designing it, I ended up with a patch - attached.
>
> It's a replacement for original patch #649. Patch #651 and example
> plugin are updated accordingly.
>
> Basically I refactored router's `navigate_to_*`, `_create_*_hash` and
> `*_route_handler` methods into separate classes which are registered and
> mapped to facets. All in navigation.routing module.
>
> Btw, can you think of some better name(s)/placement for the module?

ACK. Some comments below, but I think they can be addressed later.

1. I'm not sure if we really need a HashCreator. Ideally the router 
should map a hash to a page. Links to another page can be hardcoded too 
(and substitute the parameters).

2. Ideally there should be no entity-specific navigation code. All 
routing should be handled in a generic way. This probably depends on the 
entity-facet separation that we discussed previously. So routes like this:

   /e/:entity/:facet/:pkeys/*args

should be replaced by individual routes for each entity:

   /user/:facet/:pkeys/*args
   /group/:facet/:pkeys/*args

Probably the entities should be written like plugins.

3. If we fix #1 and #2, is the "routing" module still necessary? The 
"routing" object probably should have been a "router", but are you 
concerned about conflicting with Dojo's router and the existing Router 
module?

4. The args/query in the navigation paths should be separated by a more 
standard delimiter "?" instead of "//". For example:

   /ipa/ui/#/e/user/search//filter=test

should be replaced with:

   /ipa/ui/#/e/user/search?filter=test

-- 
Endi S. Dewata