launchpad-reviewers team mailing list archive

[Aaron Bentley <aaron@xxxxxxxxxxxxx>]

Aaron Bentley has proposed merging lp:~abentley/launchpad/sharing-language-bugs into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #758845 in Launchpad itself: "AJAX overlay says "Select productseries" in the translation sharing details"
  https://bugs.launchpad.net/launchpad/+bug/758845
  Bug #758852 in Launchpad itself: "Unhelpful title for "enable translations on an upstream project" dialog"
  https://bugs.launchpad.net/launchpad/+bug/758852
  Bug #758857 in Launchpad itself: "Linked branch in translations sharing details page shows branch URL without lp:// prefix"
  https://bugs.launchpad.net/launchpad/+bug/758857

For more details, see:
https://code.launchpad.net/~abentley/launchpad/sharing-language-bugs/+merge/57715

= Summary =
Fix bugs:
#758845: AJAX overlay says "Select productseries" in the translation sharing details Remove
#758852: Unhelpful title for "enable translations on an upstream project" dialog Remove
#758857: Linked branch in translations sharing details page shows branch URL without lp:// prefix Remove

== Proposed fix ==
Change AJAX overlay to "Select project series".  Change title of "usage" dialog to "Configure translations", the same as the title of the page it comes from.  Add lp: prefix to branch names.

== Pre-implementation notes ==
Discussed 758857 with jml

== Implementation details ==
None

== Tests ==
bin/test -t test_branch_link_text sharing_details
firefox lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.html

== Demo and Q/A ==
Go to the +sharing-details and verify the text is as expected.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js
  lib/lp/translations/browser/tests/test_sharing_details.py
  lib/lp/translations/javascript/sourcepackage_sharing_details.js
  lib/lp/translations/browser/sourcepackage.py
-- 
https://code.launchpad.net/~abentley/launchpad/sharing-language-bugs/+merge/57715
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~abentley/launchpad/sharing-language-bugs into lp:launchpad.

=== modified file 'lib/lp/translations/browser/sourcepackage.py'
--- lib/lp/translations/browser/sourcepackage.py	2011-04-11 17:16:28 +0000
+++ lib/lp/translations/browser/sourcepackage.py	2011-04-14 15:52:31 +0000
@@ -151,7 +151,7 @@
         if self.has_upstream_branch:
             # Normally should use BranchFormatterAPI(branch).link(None), but
             # on this page, that information is redundant.
-            title = self.upstream_branch.unique_name
+            title = 'lp:' + self.upstream_branch.unique_name
             url = canonical_url(self.upstream_branch)
         else:
             title = ''

=== modified file 'lib/lp/translations/browser/tests/test_sharing_details.py'
--- lib/lp/translations/browser/tests/test_sharing_details.py	2011-04-11 21:52:14 +0000
+++ lib/lp/translations/browser/tests/test_sharing_details.py	2011-04-14 15:52:31 +0000
@@ -164,6 +164,11 @@
         self.configureSharing(set_upstream_branch=True)
         self.assertTrue(self.view.has_upstream_branch)
 
+    def test_branch_link_text(self):
+        self.configureSharing(set_upstream_branch=True)
+        expected_text = '>lp:%s</a>' % self.view.upstream_branch.unique_name
+        self.assertIn(expected_text, self.view.branch_link.escapedtext)
+
     def test_is_upstream_translations_enabled__no_packaging_link(self):
         # If the source package is not linked to an upstream series,
         # is_upstream_translations_enabled returns False.

=== modified file 'lib/lp/translations/javascript/sourcepackage_sharing_details.js'
--- lib/lp/translations/javascript/sourcepackage_sharing_details.js	2011-04-07 15:44:38 +0000
+++ lib/lp/translations/javascript/sourcepackage_sharing_details.js	2011-04-14 15:52:31 +0000
@@ -321,7 +321,7 @@
         var check = this.get('tsconfig').get('branch');
         if (Y.Lang.isValue(branch)){
             check.set_link(
-                branch.get('unique_name'), branch.get('web_link'));
+                'lp:' + branch.get('unique_name'), branch.get('web_link'));
         }
         else {
             check.clear_link();
@@ -490,7 +490,7 @@
     add_activator(picker, '#branch-complete-picker a');
     var productseries_picker_config = {
         picker_activator: '#packaging-complete-picker a',
-        header : 'Select productseries',
+        header : 'Select project series',
         step_title: 'Search',
         save: Y.bind('select_productseries', sharing_controller),
         context: cache.product
@@ -534,7 +534,7 @@
     var autoimport = sharing_controller.get('tsconfig').get('autoimport');
     sharing_controller.set_check_picker(autoimport, import_overlay);
     var usage_overlay = new Y.lazr.FormOverlay({
-        headerContent: '<h2>Usage settings<h2>',
+        headerContent: '<h2>Configure translations<h2>',
         centered: true,
         visible: false
     });

=== modified file 'lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js'
--- lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js	2011-04-07 15:39:20 +0000
+++ lib/lp/translations/javascript/tests/test_sourcepackage_sharing_details.js	2011-04-14 15:52:31 +0000
@@ -37,6 +37,18 @@
             product_series.set_link('ps', 'http://');
             Y.Assert.isTrue(sharing_config.get('branch').get('enabled'));
         },
+        test_set_branch: function() {
+            var lp_client = new Y.lp.client.Launchpad();
+            var branch = lp_client.wrap_resource('http://example.com', {
+                unique_name: 'unique',
+                web_link: 'http://example.com',
+                resource_type_link: 'http://foo_type'
+            });
+            var sharing_controller = new TranslationSharingController();
+            sharing_controller.set_branch(branch);
+            var check = sharing_controller.get('tsconfig').get('branch');
+            Y.Assert.areEqual('lp:unique', check.get('text'));
+        },
         test_autoimport: function() {
             var sharing_config = new TranslationSharingConfig();
             Y.Assert.isFalse(sharing_config.get('autoimport').get('enabled'));
@@ -125,7 +137,7 @@
             Y.Assert.areEqual(
                 tsconfig.get('product_series').get('url'), 'http://web1');
             Y.Assert.areEqual(
-                tsconfig.get('branch').get('text'), 'title2');
+                tsconfig.get('branch').get('text'), 'lp:title2');
             Y.Assert.isTrue(tsconfig.get('autoimport').get('complete'));
             Y.Assert.isTrue(
                 tsconfig.get('translations_usage').get('complete'));