launchpad-reviewers team mailing list archive

[Abel Deuring <abel.deuring@xxxxxxxxxxxxx>]

Abel Deuring has proposed merging lp:~adeuring/launchpad/translation-branch-sync-return-to-referrer into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~adeuring/launchpad/translation-branch-sync-return-to-referrer/+merge/53597

This branch lets the class ProductSeriesTranslationsSettingsView inherit from ReturnToReferrerMixin. This will make the changes from lp:~launchpad/launchpad/translation-sharing-status look more consistent and convenient for users: The latter branch adds a page "translation sharing status" for source packages, where users can see if a package shares translations with an upstream project. The page contains links to several forms where users can make configuration changes required for translation sharing. All these forms except for the one changed here already leads the user back to the "translation sharing status" page, so this branch just fixes an inconsistency.

The change itself is quite simple: Use another mixin class for ProductSeriesTranslationsSettingsView. Since cancel_url is set by ReturnToReferrerMixin, ProductSeriesTranslationsSettingsView should not change it, so I deleted its __init__() method.

This change requries an obviuos change to an existing page test, which also tests that the browser class indeed does what it is intended to do.
-- 
https://code.launchpad.net/~adeuring/launchpad/translation-branch-sync-return-to-referrer/+merge/53597
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/translation-branch-sync-return-to-referrer into lp:launchpad.

=== modified file 'lib/lp/translations/browser/productseries.py'
--- lib/lp/translations/browser/productseries.py	2011-03-11 08:33:16 +0000
+++ lib/lp/translations/browser/productseries.py	2011-03-16 10:23:26 +0000
@@ -42,6 +42,7 @@
     LaunchpadFormView,
     )
 from lp.app.enums import service_uses_launchpad
+from lp.app.browser.launchpadform import ReturnToReferrerMixin
 from lp.app.widgets.itemswidgets import LaunchpadRadioWidgetWithDescription
 from lp.code.interfaces.branchjob import IRosettaUploadJobSource
 from lp.registry.interfaces.productseries import IProductSeries
@@ -484,8 +485,10 @@
         self.hint = None
 
 
-class ProductSeriesTranslationsSettingsView(LaunchpadEditFormView,
-                                            ProductSeriesTranslationsMixin):
+class ProductSeriesTranslationsSettingsView(ReturnToReferrerMixin,
+                                            LaunchpadEditFormView,
+                                            ProductSeriesTranslationsMixin,
+                                            ):
     """Edit settings for translations import and export."""
 
     schema = IProductSeries
@@ -497,11 +500,6 @@
     settings_widget = custom_widget('translations_autoimport_mode',
                   SettingsRadioWidget)
 
-    def __init__(self, context, request):
-        super(ProductSeriesTranslationsSettingsView, self).__init__(
-            context, request)
-        self.cancel_url = canonical_url(self.context, rootsite='translations')
-
     @action(u"Save settings", name="save_settings")
     def change_settings_action(self, action, data):
         """Change the translation settings."""

=== modified file 'lib/lp/translations/stories/productseries/xx-productseries-translations-settings.txt'
--- lib/lp/translations/stories/productseries/xx-productseries-translations-settings.txt	2009-12-10 10:46:05 +0000
+++ lib/lp/translations/stories/productseries/xx-productseries-translations-settings.txt	2011-03-16 10:23:26 +0000
@@ -1,11 +1,13 @@
-= Changing the Bazaar import settings for a product series =
+Changing the Bazaar import settings for a product series
+========================================================
 
 Product maintainers can request a one-time import of translation files
 from the Bazaar branch that is officially linked to the release series
 of a product. This function complements the general import settings
 found on the "Settings" page.
 
-== Getting there ==
+Getting there
+-------------
 
 The maintainer of a product sees a menu option called "Settings" that
 leads to the settings page.
@@ -18,7 +20,8 @@
     >>> print browser.url
     http://translations.l...d.dev/evolution/trunk/+translations-settings
 
-== The branch display ==
+The branch display
+------------------
 
 An official Bazaar branch is linked to this product settings. It is
 displayed on the page.
@@ -47,7 +50,8 @@
     >>> print browser.url
     http://launchpad.dev/evolution/trunk/+linkbranch
 
-== Pointer to one-time import ==
+Pointer to one-time import
+--------------------------
 
 The user is also reminded that a one-time import of translation files
 can be requested and a link to that page is provided.
@@ -63,14 +67,14 @@
     >>> print browser.url
     http://translations.l...d.dev/evolution/trunk/+request-bzr-import
 
-== Changing the setting ==
+Changing the setting
+--------------------
 
 The setting is changed by selecting the desired mode with the radio
 buttons in the form.
 
-    >>> browser.open(
-    ...     'http://translations.launchpad.dev/evolution/trunk/'
-    ...     '+translations-settings')
+    >>> browser.open('http://translations.launchpad.dev/evolution/trunk/')
+    >>> browser.getLink('Set up branch synchronization').click()
     >>> print_radio_button_field(browser.contents,
     ...                          'translations_autoimport_mode')
     (*) None
@@ -80,8 +84,18 @@
     ...     name='field.translations_autoimport_mode').value = (
     ...         ['IMPORT_TEMPLATES'])
     >>> browser.getControl('Save settings').click()
+
+The user is automatically redirected to the page he came from.
+
+    >>> print browser.url
+    http://translations.launchpad.dev/evolution/trunk/
     >>> print "\n".join(get_feedback_messages(browser.contents))
     The settings have been updated.
+
+If he looks at the synchonization settings page again, he sees that
+the changes have been saved.
+
+    >>> browser.getLink('Set up branch synchronization').click()
     >>> print_radio_button_field(browser.contents,
     ...                          'translations_autoimport_mode')
     ( ) None