launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/fix-packagediff-private-harder into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1025515 in Launchpad itself: "LP diffs are being linked at http://lplibrarian-private-download.internal:8000";
  https://bugs.launchpad.net/launchpad/+bug/1025515

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/fix-packagediff-private-harder/+merge/115352

== Summary ==

https://code.launchpad.net/~cjwatson/launchpad/fix-packagediff-private/+merge/114759 caused package diffs generated for not-yet-published SPRs to be inappropriately private.

== Proposed fix ==

Consider the privacy of the upload archive as well as that of published archives.

== LOC Rationale ==

+10.  Same rationale as https://code.launchpad.net/~cjwatson/launchpad/fix-packagediff-private/+merge/114759.

== Tests ==

bin/test -vvct test_packagediff

== Demo and Q/A ==

Upload a package to a frozen or stable series on dogfood.  Generate the pending package diff for it, then accept it.  Check that the package diff is public.
-- 
https://code.launchpad.net/~cjwatson/launchpad/fix-packagediff-private-harder/+merge/115352
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/fix-packagediff-private-harder into lp:launchpad.

=== modified file 'lib/lp/soyuz/model/packagediff.py'
--- lib/lp/soyuz/model/packagediff.py	2012-07-12 23:47:04 +0000
+++ lib/lp/soyuz/model/packagediff.py	2012-07-17 14:24:57 +0000
@@ -145,8 +145,9 @@
     @property
     def private(self):
         """See `IPackageDiff`."""
-        return all(
-            archive.private for archive in self.to_source.published_archives)
+        to_source = self.to_source
+        archives = [to_source.upload_archive] + to_source.published_archives
+        return all(archive.private for archive in archives)
 
     def _countDeletedLFAs(self):
         """How many files associated with either source package have been

=== modified file 'lib/lp/soyuz/tests/test_packagediff.py'
--- lib/lp/soyuz/tests/test_packagediff.py	2012-07-12 23:47:04 +0000
+++ lib/lp/soyuz/tests/test_packagediff.py	2012-07-17 14:24:57 +0000
@@ -119,3 +119,12 @@
         public_spr = self.factory.makeSourcePackageRelease(archive=ppa)
         public_diff = public_spr.requestDiffTo(p3a.owner, orig_spr)
         self.assertFalse(public_diff.private)
+
+    def test_packagediff_public_unpublished(self):
+        # If an SPR has been uploaded to a public archive but not yet
+        # published, diffs to it are public.
+        ppa = self.factory.makeArchive()
+        from_spr = self.factory.makeSourcePackageRelease(archive=ppa)
+        to_spr = self.factory.makeSourcePackageRelease(archive=ppa)
+        diff = from_spr.requestDiffTo(ppa.owner, to_spr)
+        self.assertFalse(diff.private)