launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04355
lp:~allenap/launchpad/localpackagediffs-show-changed-by-bug-798865 into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/localpackagediffs-show-changed-by-bug-798865 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #798865 in Launchpad itself: "+localpackagediffs page should show who made the change rather than sponsor"
https://bugs.launchpad.net/launchpad/+bug/798865
For more details, see:
https://code.launchpad.net/~allenap/launchpad/localpackagediffs-show-changed-by-bug-798865/+merge/69072
This changes the "Last changed by" column to "Last changed" because it shows a date too (it already did that before this branch). It also shows the in that column the person who made the package change instead of the person who uploaded it. In many cases these people are different. It does, however, show the uploader in smaller text.
Screenshot:
http://people.canonical.com/~gavin/ui/localpackagediffs-show-changed-by-bug-798865/last-changed.png
--
https://code.launchpad.net/~allenap/launchpad/localpackagediffs-show-changed-by-bug-798865/+merge/69072
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/localpackagediffs-show-changed-by-bug-798865 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css 2011-07-04 13:24:48 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css 2011-07-25 11:10:42 +0000
@@ -306,6 +306,9 @@
min-width: 1em;
outline: 1px #666;
}
+.nowrap {
+ white-space: nowrap;
+ }
/* =========================
Universal presentation
=== modified file 'lib/lp/registry/browser/tests/test_distroseries.py'
--- lib/lp/registry/browser/tests/test_distroseries.py 2011-07-21 09:27:45 +0000
+++ lib/lp/registry/browser/tests/test_distroseries.py 2011-07-25 11:10:42 +0000
@@ -91,6 +91,7 @@
login,
login_celebrity,
login_person,
+ normalize_whitespace,
person_logged_in,
StormStatementRecorder,
TestCaseWithFactory,
@@ -857,15 +858,15 @@
return (derived_series, parent_series)
-class TestDistroSeriesLocalDifferences(
+class TestDistroSeriesLocalDifferences_WRONG_LAYER(
DistroSeriesDifferenceMixin, TestCaseWithFactory):
"""Test the distroseries +localpackagediffs page."""
layer = DatabaseFunctionalLayer
def setUp(self):
- super(TestDistroSeriesLocalDifferences,
- self).setUp('foo.bar@xxxxxxxxxxxxx')
+ sup = super(TestDistroSeriesLocalDifferences_WRONG_LAYER, self)
+ sup.setUp('foo.bar@xxxxxxxxxxxxx')
set_derived_series_ui_feature_flag(self)
self.simple_user = self.factory.makePerson()
@@ -1076,8 +1077,8 @@
self._assertQueryCount(derived_series)
-class TestDistroSeriesLocalDifferencesZopeless(TestCaseWithFactory,
- DistroSeriesDifferenceMixin):
+class TestDistroSeriesLocalDifferences(TestCaseWithFactory,
+ DistroSeriesDifferenceMixin):
"""Test the distroseries +localpackagediffs view."""
layer = LaunchpadFunctionalLayer
@@ -1349,6 +1350,38 @@
self.assertEqual(versions['derived'], derived_span[0].string.strip())
self.assertEqual(versions['parent'], parent_span[0].string.strip())
+ def test_diff_row_shows_spr_creator(self):
+ # The SPR creator (i.e. who make the package change, rather than the
+ # uploader) is shown on each difference row.
+ set_derived_series_ui_feature_flag(self)
+ dsd = self.makePackageUpgrade()
+ view = self.makeView(dsd.derived_series)
+ root = html.fromstring(view())
+ [creator_cell] = root.cssselect(
+ "table.listing tbody td.last-changed")
+ self.assertEqual(
+ "a moment ago by %s" % (
+ dsd.source_package_release.creator.displayname,),
+ normalize_whitespace(creator_cell.text_content()))
+
+ def test_diff_row_shows_spr_creator_and_uploader_if_different(self):
+ # When the SPR creator and uploader are different both are named on
+ # each difference row.
+ set_derived_series_ui_feature_flag(self)
+ dsd = self.makePackageUpgrade()
+ uploader = self.factory.makePerson()
+ removeSecurityProxy(dsd.source_package_release).dscsigningkey = (
+ self.factory.makeGPGKey(uploader))
+ view = self.makeView(dsd.derived_series)
+ root = html.fromstring(view())
+ [creator_cell] = root.cssselect(
+ "table.listing tbody td.last-changed")
+ self.assertEqual(
+ "a moment ago by %s (uploaded by %s)" % (
+ dsd.source_package_release.creator.displayname,
+ dsd.source_package_release.dscsigningkey.owner.displayname),
+ normalize_whitespace(creator_cell.text_content()))
+
def test_getUpgrades_shows_updates_in_parent(self):
# The view's getUpgrades methods lists packages that can be
# trivially upgraded: changed in the parent, not changed in the
@@ -1477,12 +1510,6 @@
recorder2,
HasQueryCount(Equals(recorder1.count)))
-
-class TestDistroSeriesLocalDifferencesFunctional(TestCaseWithFactory,
- DistroSeriesDifferenceMixin):
-
- layer = LaunchpadFunctionalLayer
-
def makeDSDJob(self, dsd):
"""Create a `DistroSeriesDifferenceJob` to update `dsd`."""
job_source = getUtility(IDistroSeriesDifferenceJobSource)
@@ -2213,7 +2240,7 @@
DistroSeriesDifferenceMixin):
"""Test the distroseries +missingpackages page."""
- layer = DatabaseFunctionalLayer
+ layer = LaunchpadFunctionalLayer
def setUp(self):
super(DistroSeriesMissingPackagesPageTestCase,
@@ -2244,6 +2271,46 @@
html_content, packageset_text, 'parent-packagesets',
'Parent packagesets')
+ def test_diff_row_shows_spr_creator(self):
+ # The parent SPR creator (i.e. who make the package change, rather
+ # than the uploader) is shown on each difference row.
+ missing_type = DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES
+ dsd = self.factory.makeDistroSeriesDifference(
+ difference_type=missing_type)
+ with person_logged_in(self.simple_user):
+ view = create_initialized_view(
+ dsd.derived_series, '+missingpackages',
+ principal=self.simple_user)
+ root = html.fromstring(view())
+ [creator_cell] = root.cssselect(
+ "table.listing tbody td.last-changed")
+ self.assertEqual(
+ dsd.parent_source_package_release.creator.displayname,
+ creator_cell.find(".//a").text)
+
+ def test_diff_row_shows_spr_creator_and_uploader_if_different(self):
+ # When the SPR creator and uploader are different both are named on
+ # each difference row.
+ missing_type = DistroSeriesDifferenceType.MISSING_FROM_DERIVED_SERIES
+ dsd = self.factory.makeDistroSeriesDifference(
+ difference_type=missing_type)
+ uploader = self.factory.makePerson()
+ naked_spr = removeSecurityProxy(dsd.parent_source_package_release)
+ naked_spr.dscsigningkey = self.factory.makeGPGKey(uploader)
+ with person_logged_in(self.simple_user):
+ view = create_initialized_view(
+ dsd.derived_series, '+missingpackages',
+ principal=self.simple_user)
+ root = html.fromstring(view())
+ [creator_cell] = root.cssselect(
+ "table.listing tbody td.last-changed")
+ parent_spr = dsd.parent_source_package_release
+ self.assertEqual(
+ "a moment ago by %s (uploaded by %s)" % (
+ parent_spr.creator.displayname,
+ parent_spr.dscsigningkey.owner.displayname),
+ normalize_whitespace(creator_cell.text_content()))
+
class DistroSerieUniquePackageDiffsTestCase(TestCaseWithFactory,
DistroSeriesDifferenceMixin):
=== modified file 'lib/lp/registry/model/distroseriesdifference.py'
--- lib/lp/registry/model/distroseriesdifference.py 2011-07-08 09:06:24 +0000
+++ lib/lp/registry/model/distroseriesdifference.py 2011-07-25 11:10:42 +0000
@@ -481,11 +481,13 @@
gpgkeys = bulk.load_related(GPGKey, sprs, ("dscsigningkeyID",))
# Load DistroSeriesDifferenceComment owners,
- # SourcePackageRecipeBuild requesters and GPGKey owners.
+ # SourcePackageRecipeBuild requesters and GPGKey owners, and
+ # SourcePackageRelease creators.
person_ids = set().union(
(dsdc.message.ownerID for dsdc in latest_comments),
(sprb.requester_id for sprb in sprbs),
- (gpgkey.ownerID for gpgkey in gpgkeys))
+ (gpgkey.ownerID for gpgkey in gpgkeys),
+ (spr.creatorID for spr in sprs))
uploaders = getUtility(IPersonSet).getPrecachedPersonsFromIDs(
person_ids, need_validity=True)
list(uploaders)
=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt 2011-07-20 16:59:53 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt 2011-07-25 11:10:42 +0000
@@ -69,7 +69,7 @@
class="package-sets">
Package-sets
</th>
- <th>Last uploaded</th>
+ <th>Last changed</th>
<th>Latest comment</th>
</tr>
</thead>
@@ -145,26 +145,42 @@
<tal:replace replace="difference/@@/packagesets_names" />
</td>
- <td>
+ <td class="last-changed">
<tal:parent condition="not: view/show_derived_version">
<tal:published condition="parent_source_pub">
<span tal:attributes="title parent_source_pub/datepublished/fmt:datetime"
tal:content="parent_source_pub/datepublished/fmt:approximatedate">2005-09-16</span>
- <tal:signer condition="parent_source_pub/sourcepackagerelease/uploader">
- by <a tal:replace="structure parent_source_pub/sourcepackagerelease/uploader/fmt:link">Steph Smith</a>
- </tal:signer>
+ <tal:creator define="spr parent_source_pub/sourcepackagerelease">
+ <span class="nowrap">by <a tal:replace="structure spr/creator/fmt:link" /></span>
+ </tal:creator>
+ <tal:uploader
+ define="spr parent_source_pub/sourcepackagerelease"
+ condition="python: spr.uploader not in (None, spr.creator)">
+ <br />
+ <span class="discreet nowrap">
+ (uploaded by <a tal:replace="structure spr/uploader/fmt:link" />)
+ </span>
+ </tal:uploader>
</tal:published>
<tal:not-published condition="not:parent_source_pub">
<span tal:content="difference/parent_source_version" />
</tal:not-published>
</tal:parent>
<tal:derived condition="view/show_derived_version">
- <tal:published condition="parent_source_pub">
+ <tal:published condition="source_pub">
<span tal:attributes="title source_pub/datepublished/fmt:datetime"
tal:content="source_pub/datepublished/fmt:approximatedate">2005-09-16</span>
- <tal:signer condition="source_pub/sourcepackagerelease/uploader">
- by <a tal:replace="structure source_pub/sourcepackagerelease/uploader/fmt:link">Steph Smith</a>
- </tal:signer>
+ <tal:creator define="spr source_pub/sourcepackagerelease">
+ <span class="nowrap">by <a tal:replace="structure spr/creator/fmt:link" /></span>
+ </tal:creator>
+ <tal:uploader
+ define="spr source_pub/sourcepackagerelease"
+ condition="python: spr.uploader not in (None, spr.creator)">
+ <br />
+ <span class="discreet nowrap">
+ (uploaded by <a tal:replace="structure spr/uploader/fmt:link" />)
+ </span>
+ </tal:uploader>
</tal:published>
<tal:not-published condition="not:source_pub">
<span tal:content="difference/source_version" />
=== modified file 'lib/lp/soyuz/model/sourcepackagerelease.py'
--- lib/lp/soyuz/model/sourcepackagerelease.py 2011-06-16 11:31:24 +0000
+++ lib/lp/soyuz/model/sourcepackagerelease.py 2011-07-25 11:10:42 +0000
@@ -47,7 +47,6 @@
LibraryFileContent,
)
from canonical.launchpad.helpers import shortlist
-from lp.app.errors import NotFoundError
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.archiveuploader.utils import determine_source_file_type
from lp.buildmaster.enums import BuildStatus
@@ -61,10 +60,7 @@
PackageDiffStatus,
PackagePublishingStatus,
)
-from lp.soyuz.interfaces.archive import (
- IArchiveSet,
- MAIN_ARCHIVE_PURPOSES,
- )
+from lp.soyuz.interfaces.archive import MAIN_ARCHIVE_PURPOSES
from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
from lp.soyuz.interfaces.packagediff import PackageDiffAlreadyRequested
from lp.soyuz.interfaces.sourcepackagerelease import ISourcePackageRelease
@@ -574,7 +570,7 @@
queue = getUtility(ITranslationImportQueue)
- only_templates=self.sourcepackage.has_sharing_translation_templates
+ only_templates = self.sourcepackage.has_sharing_translation_templates
queue.addOrUpdateEntriesFromTarball(
tarball, by_maintainer, importer,
sourcepackagename=self.sourcepackagename,
Follow ups
