Launchpad itself

Code review comment for lp:~adiroiban/launchpad/bug-522188

  1. bug-522188
  2. Merge into devel
Revision history for this message
Adi Roiban (adiroiban) wrote :

Hi Graham,

I had to use another generic-edit template as some views were not compatible with the previous implementation of generic-edit.

Also, since the POTemplate admin page can rename the template name, I had to implement this dirty hack in order to avoid redirecting to a page that is no longer available.
I asked Curtis, and he agree that this is not a clean solution but he also said the code is correct and he is not aware of any other better solution.

Any suggestion is much appreciated.

Can you please review the changes?
Here is the new diff.

=== added file 'lib/lp/app/templates/generic-edit-next-url.pt'
--- lib/lp/app/templates/generic-edit-next-url.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/app/templates/generic-edit-next-url.pt 2010-02-22 22:44:11 +0000
@@ -0,0 +1,19 @@
+<html
+ xmlns="http://www.w3.org/1999/xhtml"
+ xmlns:tal="http://xml.zope.org/namespaces/tal"
+ xmlns:metal="http://xml.zope.org/namespaces/metal"
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n"
+ metal:use-macro="view/macro:page/main_only"
+ i18n:domain="launchpad">
+ <body>
+ <div metal:fill-slot="main">
+ <div metal:use-macro="context/@@launchpad_form/form">
+ <div metal:fill-slot="extra_info">
+ <input type="hidden" name="next_url"
+ tal:condition="view/next_url"
+ tal:attributes="value view/next_url" />
+ </div>
+ </div>
+ </div>
+ </body>
+</html>

=== modified file 'lib/lp/app/templates/generic-edit.pt'
--- lib/lp/app/templates/generic-edit.pt 2010-02-16 12:15:12 +0000
+++ lib/lp/app/templates/generic-edit.pt 2010-02-22 22:43:37 +0000
@@ -7,13 +7,7 @@
   i18n:domain="launchpad">
   <body>
     <div metal:fill-slot="main">
- <div metal:use-macro="context/@@launchpad_form/form">
- <div metal:fill-slot="extra_info">
- <input type="hidden" name="next_url"
- tal:condition="view/next_url"
- tal:attributes="value view/next_url" />
- </div>
- </div>
+ <div metal:use-macro="context/@@launchpad_form/form" />
     </div>
   </body>
 </html>

=== modified file 'lib/lp/translations/browser/configure.zcml'
--- lib/lp/translations/browser/configure.zcml 2010-02-10 13:04:10 +0000
+++ lib/lp/translations/browser/configure.zcml 2010-02-22 22:47:03 +0000
@@ -414,14 +414,14 @@
             for="lp.translations.interfaces.potemplate.IPOTemplate"
             class="lp.translations.browser.potemplate.POTemplateEditView"
             permission="launchpad.Edit"
- template="../../app/templates/generic-edit.pt"
+ template="../../app/templates/generic-edit-next-url.pt"
             layer="canonical.launchpad.layers.TranslationsLayer"/>
         <browser:page
             name="+admin"
             for="lp.translations.interfaces.potemplate.IPOTemplate"
             class="lp.translations.browser.potemplate.POTemplateAdminView"
             permission="launchpad.TranslationsAdmin"
- template="../../app/templates/generic-edit.pt"
+ template="../../app/templates/generic-edit-next-url.pt"
             layer="canonical.launchpad.layers.TranslationsLayer"/>
         <browser:page
             name="+export"

=== modified file 'lib/lp/translations/browser/potemplate.py'
--- lib/lp/translations/browser/potemplate.py 2010-02-16 12:15:12 +0000
+++ lib/lp/translations/browser/potemplate.py 2010-02-23 15:27:27 +0000
@@ -538,7 +538,10 @@

     @property
     def cancel_url(self):
- return self.next_url
+ if self.next_url == "DIRTY_HACK":
+ return canonical_url(self.context)
+ else:
+ return self.next_url

     @property
     def next_url(self):
@@ -548,6 +551,13 @@
         referrer = self.request.form.get('next_url')
         if referrer is None:
             referrer = self.request.getHeader('referer')
+ if (referrer is None
+ or canonical_url(self.context) in referrer):
+ # XXX: AdiRoiban 2010-02-32
+ # Since 'referer' can depend on the object name, and
+ # from this form we can rename the object I was forced
+ # to use this dirty hack.
+ return "DIRTY_HACK"

         if (referrer is not None
             and referrer.startswith(self.request.getApplicationURL())

« Back to merge proposal