Hi, I agree with Salgado, I don't think removing that checks is the right solution.
And I don't really think there is a problem either. create_view and create_initialized_view have a layer parameter. Why don't you use that parametere? Cheers -- Francis J. Lacoste [email protected] On August 18, 2010, Guilherme Salgado wrote: > On Wed, 2010-08-18 at 23:24 +1200, Tim Penhey wrote: > > 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 > > I think that for the view tests where you create manually (or has a way > of specifying the request), we should make it easy to specify a request > that provides the layer on which the view is registered. That way the > above would work as the current browser request would provide the > correct layer. > > This won't be of help for testbrowser tests, though, but for those we're > using the zope publication, which should make sure the requests provide > the appropriate layer according to the vhost you used. > > > 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 > > I think that's the real problem -- we should either infer the layer from > the vhost used (in case of testbrowser tests) or make it easy for tests > to specify them. Would that fix the test failures you saw? > > > 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 > > As you saw, this is not widely used, but it's (IMO) a very nice > mechanism (that we should be using more!) to make sure we don't expose > 404 links to users, so I'd be against removing it. > > > 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. > > That'd work, but we'd be changing production code just to please tests > as the errors you're seeing don't happen in production. In production > the requests always implement the layer that is appropriate for the > vhost you're on. > > > - 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 > > > > _______________________________________________ > > Mailing list: https://launchpad.net/~launchpad-dev > > Post to : [email protected] > > Unsubscribe : https://launchpad.net/~launchpad-dev > > More help : https://help.launchpad.net/ListHelp
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-dev More help : https://help.launchpad.net/ListHelp
