Allow Multi URL fragments per namespace

Allow Multi URL fragments per namespace

Router can take combineFlags to its constructor or to parse/combine methods.
This allows configuration of how to process namespace collisions in the URL
(overwrite or merge). The router itself can dispatch multi URL fragments per
namespace as well now.

https://code.launchpad.net/~bcsaller/juju-gui/multins/+merge/164994

(do not edit description out of merge proposal)

[bcsaller, 2013-05-22 00:37:43]

Please take a look.

[jeff.pihach, 2013-05-22 14:41:14]

LGTM - Thanks, this looks awesome!

I was wondering if it would be too much to ask if you could add inline comments
into the ns routing stuff to explain what's going on? It is pretty difficult to
follow right now so you will likely end up supporting it until the end of time
;)

Using this current iteration would the developer be responsible for updating the
appropriate ns if there are multiples? Say they were going from:

:gui:/service/wordpress/:gui:/service/haproxy 
-to-
:gui:/service/wordpress/units/:gui:/service/haproxy

https://codereview.appspot.com/9621045/diff/1/app/assets/javascripts/ns-routi...
File app/assets/javascripts/ns-routing-app-extension.js (right):

https://codereview.appspot.com/9621045/diff/1/app/assets/javascripts/ns-routi...
app/assets/javascripts/ns-routing-app-extension.js:30: // Special case single
item arrays, this is very combine with combine off.
typo?

[gary.poster, 2013-05-22 15:56:06]

I'll say L-G-T-M now if you want me to, but I'd like to get an idea of your
reply to the question I posed in the test first

https://codereview.appspot.com/9621045/diff/1/app/assets/javascripts/ns-routi...
File app/assets/javascripts/ns-routing-app-extension.js (right):

https://codereview.appspot.com/9621045/diff/1/app/assets/javascripts/ns-routi...
app/assets/javascripts/ns-routing-app-extension.js:30: // Special case single
item arrays, this is very combine with combine off.
Not sure what you mean by "very combine".  Is this what you mean?

// Special case single item arrays.  This happens when a namespace can be
combined but there is only one value in the namespace.

https://codereview.appspot.com/9621045/diff/1/app/assets/javascripts/ns-routi...
app/assets/javascripts/ns-routing-app-extension.js:121: * @param {Object}
combineFlags undefined||true||{ns: true} control
What's the use case for "true"?

https://codereview.appspot.com/9621045/diff/1/test/test_routing.js
File test/test_routing.js (right):

https://codereview.appspot.com/9621045/diff/1/test/test_routing.js#newcode198
test/test_routing.js:198: // We can use the match to produce a valid url.
Could you add a test to show how you think we ought to work the addition and
removal of urls within a given namespace?  IOW, if the current url is
'/:gui:/services/mysql/:gui:/services/mediawiki/', what's the router.url syntax
to remove "/:gui:/services/mysql"?  What's the router.url syntax to add
"/:gui:/services/haproxy"?  Extra points if the spelling means you only have to
specify the change to the namespace, not the entire namespace. :-)

If you want to postpone that answer till later that's fine, but I'd like to at
least have a plan for it that we think will be workable.

[bcsaller, 2013-05-22 16:32:50]

Thanks for the reviews. I suspect we'll need to keep this thread open a little
longer.

https://codereview.appspot.com/9621045/diff/1/app/assets/javascripts/ns-routi...
File app/assets/javascripts/ns-routing-app-extension.js (right):

https://codereview.appspot.com/9621045/diff/1/app/assets/javascripts/ns-routi...
app/assets/javascripts/ns-routing-app-extension.js:30: // Special case single
item arrays, this is very combine with combine off.
On 2013/05/22 14:41:14, jeff.pihach wrote:
> typo?

Yes, 'every combine'

https://codereview.appspot.com/9621045/diff/1/app/assets/javascripts/ns-routi...
app/assets/javascripts/ns-routing-app-extension.js:121: * @param {Object}
combineFlags undefined||true||{ns: true} control
A developer can say merge everything I pass in for this next call with true
rather than qualify the namespace(s). Its may be a weak case.

https://codereview.appspot.com/9621045/diff/1/test/test_routing.js
File test/test_routing.js (right):

https://codereview.appspot.com/9621045/diff/1/test/test_routing.js#newcode198
test/test_routing.js:198: // We can use the match to produce a valid url.
On 2013/05/22 15:56:07, gary.poster wrote:
> Could you add a test to show how you think we ought to work the addition and
> removal of urls within a given namespace?  IOW, if the current url is
> '/:gui:/services/mysql/:gui:/services/mediawiki/', what's the router.url
syntax
> to remove "/:gui:/services/mysql"?  What's the router.url syntax to add
> "/:gui:/services/haproxy"?  Extra points if the spelling means you only have
to
> specify the change to the namespace, not the entire namespace. :-)
> 
> If you want to postpone that answer till later that's fine, but I'd like to at
> least have a plan for it that we think will be workable.

Yeah, I almost wrote about this in the MP. You rightly ask for the spelling of
"how" but knowing "when/why" are also important issues.
 
For example suppose the user selects three services, one with the panel open,
and hits a key for 'toggle panel'. We'd expect two to become active and one to
deactivate. To sync this with the URL we'd have to know to look at each panels
state and issue a change either adding or removing it from the URL.

While not difficult it means we've coupled application logic more closely into
the app than I think is intended. I also don't feel this example is that special
a case. 

I can easily add a spelling that remove fragments when found or clears an entire
namespace, such a spelling might be 
:ns:/-fragment or :ns:/-- understandable.

I'm happy to add this now but I don't love the direction this is going. I've
never really bought the use case that we need to share this state in the URL and
as the UI gets more desktop app like I buy it less.

[gary.poster, 2013-05-22 18:41:14]

LGTM given our discussion: namely, we may want to rip out the entire
multi-namespace code if we get approval of the URL plan from Alejandra.  That
won't be trivial, but will be a massive simplification.

[bcsaller, 2013-05-22 18:58:05]

*** Submitted:

Allow Multi URL fragments per namespace

Router can take combineFlags to its constructor or to parse/combine methods.
This allows configuration of how to process namespace collisions in the URL
(overwrite or merge). The router itself can dispatch multi URL fragments per
namespace as well now.

R=jeff.pihach, gary.poster
CC=
https://codereview.appspot.com/9621045