launchpad-dev team mailing list archive

[Tim Penhey <tim.penhey@xxxxxxxxxxxxx>]

Hi Guys,

While trawling through the errors with my latest branch I'm pretty sure that I 
have found a bug with canonical_url.

Following a recent discussion about facets and layers, I decided to go and be 
more strict on the views for the code application by adding 
layer="lp.code.publisher.CodeLayer" to the ZCML directives for the pages.

This caused (quite) a number of tests to fail.

create_initialized_view needed to have the layer passed in.  That was fine.

However, I hit the following:

  canonical_url(branch) would be fine
  canonical_url(branch, view_name="+edit") would fail with one of two errors:
    - view not registered
    - or no request passed in

The view check uses the current browser request (if one isn't passed in).  
This is often the on the OverviewLayer or some other layer.  The views we want 
aren't registered on those, but on the "code" rootsite (or CodeLayer).  I went 
looking to see how it was done elsewhere, but it looks like *no-one* else 
specifies layers on the page registrations, only on default views.  So this 
kinda explains why we haven't hit this before.

I spent quite a bit of time talking this through with Michael Hudson this 
afternoon, and we came up with two main solutions:

 1) Drop the check for the registration of the view as it isn't checking the 
right thing anyway

 2) Change the implementation to use the Layer defined for the rootsite.  Since 
the getMultiAdapter check is just using the request to check on the layer that 
is being provided, we can fake this if we have a link from the rootsite to the 
layer for that rootsite. 
  - However right now we don't.  The suggestion was to change the meta-zcml 
directive for "publisher" to define the layer for the rootsite.  This could 
then be used to create the request and publisher factory using a closure and 
avoid a lot of the code duplication in the different app's publisher code.  We 
would still need the layers defined in each app, but the request class and 
factory method could be defined through the code that handles the "publisher" 
ZCML directive.  This approach is somewhat more complex than the "just remove 
the check" approach.

I wanted to get a second opinion and a sanity check.

Tim