launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/dds-missing-base-version-747202 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #747202 in Launchpad itself: "Don't present diff-generation link on +localpackagediffs if it can't be done"
  https://bugs.launchpad.net/launchpad/+bug/747202

For more details, see:
https://code.launchpad.net/~rvb/launchpad/dds-missing-base-version-747202/+merge/56311

This branch fixes the UI on the localpackagediffs page when dsd's base_version is None. It displays 'unknown' instead of an empty line for "Last common version:" and prevents the package diff slot to be displayed.

== Tests ==
./bin/test -cvv test_distroseriesdifference_views test_package_diff_no_base_version

== QA ==
- Fire up a local instance.
- Turn on the feature flag :
    'soyuz.derived-series-ui.enabled default 1 on'
- Update the dsd:
    update distroseriesdifference set base_version = NULL ;
- Access the page
    https://launchpad.dev/deribuntu/deriwarty/+localpackagediffs
- Open up the first row.
- Make sure:
    - "Last common version:" is Unknown.
    - the package diff request link is not there.
-- 
https://code.launchpad.net/~rvb/launchpad/dds-missing-base-version-747202/+merge/56311
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/dds-missing-base-version-747202 into lp:launchpad.

=== modified file 'lib/lp/registry/browser/distroseriesdifference.py'
--- lib/lp/registry/browser/distroseriesdifference.py	2011-04-01 14:16:37 +0000
+++ lib/lp/registry/browser/distroseriesdifference.py	2011-04-05 10:03:19 +0000
@@ -169,7 +169,7 @@
     def display_child_diff(self):
         """Only show the child diff if we need to."""
         return self.context.source_version != self.context.base_version
-  
+
     @property
     def show_package_diffs_request_link(self):
         """Return whether package diffs can be requested.
@@ -181,6 +181,7 @@
         request link.
         """
         return (check_permission('launchpad.Edit', self.context) and
+                self.context.base_version and
                 (not self.context.package_diff or
                  not self.context.parent_package_diff))
 

=== modified file 'lib/lp/registry/browser/tests/test_distroseriesdifference_views.py'
--- lib/lp/registry/browser/tests/test_distroseriesdifference_views.py	2011-03-31 16:21:15 +0000
+++ lib/lp/registry/browser/tests/test_distroseriesdifference_views.py	2011-04-05 10:03:19 +0000
@@ -9,7 +9,10 @@
 
 from BeautifulSoup import BeautifulSoup
 import soupmatchers
-from testtools.matchers import Not
+from testtools.matchers import (
+    MatchesAny,
+    Not,
+    )
 import transaction
 from zope.component import getUtility
 
@@ -416,3 +419,48 @@
                 ds_diff, '+listing-distroseries-extra')
             self.assertThat(view(), package_diff_request_matcher)
             self.assertTrue(view.show_package_diffs_request_link)
+
+    def test_package_diff_no_base_version(self):
+        # If diff's base_version is None packages diffs are not displayed
+        # and neither is the link to compute them.
+        versions={
+            'base': None, # No base version.
+            'derived': '0.1-1derived1',
+            'parent': '0.1-2'}
+        ds_diff = self.factory.makeDistroSeriesDifference(versions=versions)
+        package_diff_request_matcher = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Request link', 'a',
+                text=re.compile(
+                    '\s*Compute differences from last common version\s*')))
+
+        pending_package_diff_matcher = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Pending package diff', 'span',
+                attrs={'class': 'PENDING'}))
+
+        package_diff_header_matcher = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Package diffs header', 'dt',
+                text=re.compile(
+                    '\s*Differences from last common version:')))
+
+        unknown_base_version = soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                'Unknown base version', 'dd',
+                text=re.compile(
+                    '\s*Unknown')))
+
+        with celebrity_logged_in('admin'):
+            view = create_initialized_view(
+                ds_diff, '+listing-distroseries-extra')
+            html = view()
+            self.assertFalse(view.show_package_diffs_request_link)
+            self.assertThat(html, unknown_base_version)
+            self.assertThat(
+                html,
+                Not(
+                    MatchesAny(
+                        package_diff_request_matcher,
+                        pending_package_diff_matcher,
+                        package_diff_header_matcher)))

=== modified file 'lib/lp/registry/templates/distroseriesdifference-listing-extra.pt'
--- lib/lp/registry/templates/distroseriesdifference-listing-extra.pt	2011-03-31 20:40:32 +0000
+++ lib/lp/registry/templates/distroseriesdifference-listing-extra.pt	2011-04-05 10:03:19 +0000
@@ -48,10 +48,13 @@
         </span>
     </dt>
     <dt
-      tal:condition="not: view/show_package_diffs_request_link">
+      tal:condition="python: not(view.show_package_diffs_request_link) and context.base_version">
       Differences from last common version:
     </dt>
-    <dd>
+    <dd class="greyed-out" tal:condition="not: context/base_version">
+    Unknown
+    </dd>
+    <dd tal:condition="context/base_version">
       <ul class="package-diff-status">
         <tal:source-diff-option condition="view/display_child_diff">
           <li tal:condition="context/package_diff">