launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24247
[Merge] ~pappacena/launchpad:pkg-upload-log-api into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:pkg-upload-log-api into launchpad:master.
Commit message:
API for Package Upload logs
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/377897
I'm adding some changes here the API part to fetch package upload logs, including pre-fetching related objects.
This branch should only be merged after https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/377717 , since it includes it's changes.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:pkg-upload-log-api into launchpad:master.
diff --git a/lib/lp/security.py b/lib/lp/security.py
index 21caa71..e349b54 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Security policies for using content objects."""
@@ -246,6 +246,7 @@ from lp.soyuz.interfaces.publishing import (
)
from lp.soyuz.interfaces.queue import (
IPackageUpload,
+ IPackageUploadLog,
IPackageUploadQueue,
)
from lp.soyuz.interfaces.sourcepackagerelease import ISourcePackageRelease
@@ -1918,6 +1919,15 @@ class ViewPackageUpload(AuthorizationBase):
methodcaller("checkUnauthenticated"), self.iter_adapters()))
+class ViewPackageUploadLog(DelegatedAuthorization):
+ """Anyone who can view a package upload can view its logs."""
+ permission = 'launchpad.View'
+ usedfor = IPackageUploadLog
+
+ def __init__(self, obj):
+ super(ViewPackageUploadLog, self).__init__(obj, obj.package_upload)
+
+
class EditPackageUpload(AdminByAdminsTeam):
permission = 'launchpad.Edit'
usedfor = IPackageUpload
diff --git a/lib/lp/soyuz/browser/configure.zcml b/lib/lp/soyuz/browser/configure.zcml
index 1ce6b53..4ab2e41 100644
--- a/lib/lp/soyuz/browser/configure.zcml
+++ b/lib/lp/soyuz/browser/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2020 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -661,6 +661,11 @@
path_expression="string:+upload/${id}"
attribute_to_parent="distroseries"
/>
+ <browser:url
+ for="lp.soyuz.interfaces.queue.IPackageUploadLog"
+ path_expression="string:+log/${id}"
+ attribute_to_parent="package_upload"
+ />
<browser:navigation
module="lp.soyuz.browser.queue"
classes="PackageUploadNavigation"
diff --git a/lib/lp/soyuz/browser/queue.py b/lib/lp/soyuz/browser/queue.py
index a476e1d..02d5f54 100644
--- a/lib/lp/soyuz/browser/queue.py
+++ b/lib/lp/soyuz/browser/queue.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Browser views for package queue."""
@@ -10,6 +10,7 @@ __all__ = [
'QueueItemsView',
]
+from collections import defaultdict
from operator import attrgetter
from lazr.delegates import delegate_to
@@ -207,7 +208,7 @@ class QueueItemsView(LaunchpadView):
jobs = load_related(Job, package_copy_jobs, ['job_id'])
person_ids.extend(map(attrgetter('requester_id'), jobs))
list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
- person_ids, need_validity=True, need_icon=True))
+ person_ids, need_validity=True))
def decoratedQueueBatch(self):
"""Return the current batch, converted to decorated objects.
@@ -222,10 +223,8 @@ class QueueItemsView(LaunchpadView):
return None
upload_ids = [upload.id for upload in uploads]
- puses = load_referencing(
- PackageUploadSource, uploads, ['packageuploadID'])
- pubs = load_referencing(
- PackageUploadBuild, uploads, ['packageuploadID'])
+ puses = sum([u.sources for u in uploads], [])
+ pubs = sum([u.builds for u in uploads], [])
source_sprs = load_related(
SourcePackageRelease, puses, ['sourcepackagereleaseID'])
@@ -491,8 +490,6 @@ class CompletePackageUpload:
# (i.e. no proxying of __set__).
pocket = None
date_created = None
- sources = None
- builds = None
customfiles = None
contains_source = None
contains_build = None
@@ -503,9 +500,7 @@ class CompletePackageUpload:
self.pocket = packageupload.pocket
self.date_created = packageupload.date_created
self.context = packageupload
- self.sources = list(packageupload.sources)
self.contains_source = len(self.sources) > 0
- self.builds = list(packageupload.builds)
self.contains_build = len(self.builds) > 0
self.customfiles = list(packageupload.customfiles)
diff --git a/lib/lp/soyuz/browser/tests/test_queue.py b/lib/lp/soyuz/browser/tests/test_queue.py
index c2d15f2..0e08739 100644
--- a/lib/lp/soyuz/browser/tests/test_queue.py
+++ b/lib/lp/soyuz/browser/tests/test_queue.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Unit tests for QueueItemsView."""
@@ -33,6 +33,7 @@ from lp.testing import (
login_person,
logout,
person_logged_in,
+ record_two_runs,
StormStatementRecorder,
TestCaseWithFactory,
)
@@ -342,10 +343,10 @@ class TestQueueItemsView(TestCaseWithFactory):
layer = LaunchpadFunctionalLayer
- def makeView(self, distroseries, user):
+ def makeView(self, distroseries, user, form=None):
"""Create a queue view."""
return create_initialized_view(
- distroseries, name='+queue', principal=user)
+ distroseries, name='+queue', principal=user, form=form)
def test_view_renders_source_upload(self):
login(ADMIN_EMAIL)
@@ -437,7 +438,34 @@ class TestQueueItemsView(TestCaseWithFactory):
with StormStatementRecorder() as recorder:
view = self.makeView(distroseries, queue_admin)
view()
- self.assertThat(recorder, HasQueryCount(Equals(56)))
+ self.assertThat(recorder, HasQueryCount(Equals(55)))
+
+ def test_package_upload_with_logs_query_count(self):
+ login(ADMIN_EMAIL)
+ uploads = []
+ distroseries = self.factory.makeDistroSeries()
+
+ for i in range(11):
+ uploads.append(self.factory.makeSourcePackageUpload(distroseries))
+ queue_admin = self.factory.makeArchiveAdmin(distroseries.main_archive)
+
+ def reject_some_package():
+ for upload in uploads:
+ if len(upload.logs) == 0:
+ person = self.factory.makePerson()
+ upload.rejectFromQueue(person)
+ break
+
+ def run_view():
+ with person_logged_in(queue_admin):
+ url = ("%s/+queue?queue_state=%s" % (
+ canonical_url(distroseries),
+ PackageUploadStatus.REJECTED.value))
+ self.getUserBrowser(url, queue_admin)
+
+ recorder1, recorder2 = record_two_runs(
+ run_view, reject_some_package, 1, 10)
+ self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
class TestCompletePackageUpload(TestCaseWithFactory):
diff --git a/lib/lp/soyuz/configure.zcml b/lib/lp/soyuz/configure.zcml
index 0e35ccb..9b50239 100644
--- a/lib/lp/soyuz/configure.zcml
+++ b/lib/lp/soyuz/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2020 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -151,6 +151,7 @@
displayarchs
displayversion
isPPA
+ logs
package_copy_job
package_copy_job_id
package_name
@@ -183,6 +184,12 @@
set_attributes="status distroseries pocket changesfile archive"/>
</class>
<class
+ class="lp.soyuz.model.queue.PackageUploadLog">
+ <require
+ permission="launchpad.View"
+ interface="lp.soyuz.interfaces.queue.IPackageUploadLog"/>
+ </class>
+ <class
class="lp.soyuz.model.queue.PackageUploadSource">
<allow
interface="lp.soyuz.interfaces.queue.IPackageUploadSource"/>
diff --git a/lib/lp/soyuz/interfaces/queue.py b/lib/lp/soyuz/interfaces/queue.py
index c57da73..a3e915e 100644
--- a/lib/lp/soyuz/interfaces/queue.py
+++ b/lib/lp/soyuz/interfaces/queue.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Queue interfaces."""
@@ -9,18 +9,19 @@ __all__ = [
'CustomUploadError',
'ICustomUploadHandler',
'IHasQueueItems',
- 'IPackageUploadQueue',
'IPackageUpload',
'IPackageUploadBuild',
- 'IPackageUploadSource',
'IPackageUploadCustom',
+ 'IPackageUploadLog',
+ 'IPackageUploadQueue',
'IPackageUploadSet',
+ 'IPackageUploadSource',
'NonBuildableSourceUploadError',
'QueueAdminUnauthorizedError',
'QueueBuildAcceptError',
'QueueInconsistentStateError',
'QueueSourceAcceptError',
- 'QueueStateWriteProtectedError',
+ 'QueueStateWriteProtectedError'
]
import httplib
@@ -34,9 +35,13 @@ from lazr.restful.declarations import (
exported,
operation_for_version,
operation_parameters,
+ operation_returns_collection_of,
REQUEST_USER,
)
-from lazr.restful.fields import Reference
+from lazr.restful.fields import (
+ CollectionField,
+ Reference,
+ )
from zope.interface import (
Attribute,
Interface,
@@ -53,6 +58,7 @@ from zope.schema import (
from zope.security.interfaces import Unauthorized
from lp import _
+from lp.registry.interfaces.person import IPerson
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.soyuz.enums import PackageUploadStatus
from lp.soyuz.interfaces.packagecopyjob import IPackageCopyJob
@@ -111,6 +117,42 @@ class IPackageUploadQueue(Interface):
"""
+class IPackageUploadLog(Interface):
+ """Entries of package upload status change"""
+
+ export_as_webservice_entry(publish_web_link=True, as_of="devel")
+
+ id = Int(title=_('ID'), required=True, readonly=True)
+
+ package_upload = Attribute(
+ _("The package upload that generated this log"))
+
+ date_created = exported(
+ Datetime(
+ title=_("When this action happened."), required=True,
+ readonly=True))
+
+ reviewer = exported(
+ Reference(
+ IPerson, title=_("Who did this action."),
+ required=True, readonly=True))
+
+ old_status = exported(
+ Choice(
+ vocabulary=PackageUploadStatus, description=_("Old status."),
+ required=True, readonly=True))
+
+ new_status = exported(
+ Choice(
+ vocabulary=PackageUploadStatus, description=_("New status."),
+ required=True, readonly=True))
+
+ comment = exported(
+ TextLine(
+ title=_("User's comment about this change."),
+ required=False, readonly=True))
+
+
class IPackageUpload(Interface):
"""A Queue item for the archive uploader."""
@@ -181,6 +223,13 @@ class IPackageUpload(Interface):
sources = Attribute("The queue sources associated with this queue item")
builds = Attribute("The queue builds associated with the queue item")
+ logs = exported(
+ CollectionField(
+ title=_("The package upload logs"),
+ value_type=Reference(schema=IPackageUploadLog),
+ readonly=True),
+ as_of="devel")
+
customfiles = Attribute("Custom upload files associated with this "
"queue item")
custom_file_urls = exported(
@@ -503,9 +552,11 @@ class IPackageUploadBuild(Interface):
readonly=False,
)
- build = Int(
- title=_("The related build"), required=True, readonly=False,
- )
+ build = Int(title=_("The related build"), required=True, readonly=False)
+
+ buildID = Int(
+ title=_("The Related build ID"), required=True,
+ readonly=True)
def binaries():
"""Returns the properties of the binaries in this build.
@@ -548,8 +599,11 @@ class IPackageUploadSource(Interface):
sourcepackagerelease = Int(
title=_("The related source package release"), required=True,
- readonly=False,
- )
+ readonly=False)
+
+ sourcepackagereleaseID = Int(
+ title=_("The related source package release ID"), required=True,
+ readonly=True)
def getSourceAncestryForDiffs():
"""Return a suitable ancestry publication for this context.
diff --git a/lib/lp/soyuz/interfaces/webservice.py b/lib/lp/soyuz/interfaces/webservice.py
index 3620e81..90ef0e1 100644
--- a/lib/lp/soyuz/interfaces/webservice.py
+++ b/lib/lp/soyuz/interfaces/webservice.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""All the interfaces that are exposed through the webservice.
@@ -35,6 +35,7 @@ __all__ = [
'ILiveFSBuild',
'ILiveFSSet',
'IPackageUpload',
+ 'IPackageUploadLog',
'IPackageset',
'IPackagesetSet',
'ISourcePackagePublishingHistory',
@@ -104,7 +105,10 @@ from lp.soyuz.interfaces.publishing import (
IBinaryPackagePublishingHistory,
ISourcePackagePublishingHistory,
)
-from lp.soyuz.interfaces.queue import IPackageUpload
+from lp.soyuz.interfaces.queue import (
+ IPackageUpload,
+ IPackageUploadLog,
+ )
_schema_circular_imports
diff --git a/lib/lp/soyuz/model/queue.py b/lib/lp/soyuz/model/queue.py
index 0fd1799..dc34a77 100644
--- a/lib/lp/soyuz/model/queue.py
+++ b/lib/lp/soyuz/model/queue.py
@@ -1,18 +1,22 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
__all__ = [
- 'PackageUploadQueue',
'PackageUpload',
'PackageUploadBuild',
- 'PackageUploadSource',
'PackageUploadCustom',
+ 'PackageUploadLog',
+ 'PackageUploadQueue',
'PackageUploadSet',
+ 'PackageUploadSource',
]
+from collections import defaultdict
from itertools import chain
+from operator import attrgetter
+import pytz
from sqlobject import (
ForeignKey,
SQLMultipleJoin,
@@ -29,6 +33,7 @@ from storm.locals import (
SQL,
Unicode,
)
+from storm.properties import DateTime
from storm.store import (
EmptyResultSet,
Store,
@@ -39,6 +44,7 @@ from zope.interface import implementer
from lp.app.errors import NotFoundError
from lp.archiveuploader.tagfiles import parse_tagfile_content
from lp.registry.interfaces.gpg import IGPGKeySet
+from lp.registry.interfaces.person import IPersonSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.model.sourcepackagename import SourcePackageName
from lp.services.auditor.client import AuditorClient
@@ -46,10 +52,16 @@ from lp.services.database.bulk import (
load_referencing,
load_related,
)
-from lp.services.database.constants import UTC_NOW
+from lp.services.database.constants import (
+ DEFAULT,
+ UTC_NOW,
+ )
from lp.services.database.datetimecol import UtcDateTimeCol
from lp.services.database.decoratedresultset import DecoratedResultSet
-from lp.services.database.enumcol import EnumCol
+from lp.services.database.enumcol import (
+ DBEnum,
+ EnumCol,
+ )
from lp.services.database.interfaces import (
IMasterStore,
IStore,
@@ -58,6 +70,7 @@ from lp.services.database.sqlbase import (
SQLBase,
sqlvalues,
)
+from lp.services.database.stormbase import StormBase
from lp.services.database.stormexpr import (
Array,
ArrayContains,
@@ -97,6 +110,7 @@ from lp.soyuz.interfaces.queue import (
IPackageUpload,
IPackageUploadBuild,
IPackageUploadCustom,
+ IPackageUploadLog,
IPackageUploadQueue,
IPackageUploadSet,
IPackageUploadSource,
@@ -205,6 +219,23 @@ class PackageUpload(SQLBase):
self.addSearchableVersions([self.package_copy_job.package_version])
@cachedproperty
+ def logs(self):
+ logs = Store.of(self).find(
+ PackageUploadLog,
+ PackageUploadLog.package_upload == self)
+ return list(logs.order_by(Desc('date_created')))
+
+ def _addLog(self, reviewer, new_status, comment=None):
+ del get_property_cache(self).logs # clean cache
+ return Store.of(self).add(PackageUploadLog(
+ package_upload=self,
+ reviewer=reviewer,
+ old_status=self.status,
+ new_status=new_status,
+ comment=comment
+ ))
+
+ @cachedproperty
def sources(self):
return list(self._sources)
@@ -571,6 +602,10 @@ class PackageUpload(SQLBase):
def acceptFromQueue(self, user=None):
"""See `IPackageUpload`."""
+ # XXX: Only tests are not passing user here. We should adjust the
+ # tests and always create the log entries after
+ if user is not None:
+ self._addLog(user, PackageUploadStatus.ACCEPTED, None)
if self.package_copy_job is None:
self._acceptNonSyncFromQueue()
else:
@@ -581,6 +616,7 @@ class PackageUpload(SQLBase):
def rejectFromQueue(self, user, comment=None):
"""See `IPackageUpload`."""
+ self._addLog(user, PackageUploadStatus.REJECTED, comment)
self.setRejected()
if self.package_copy_job is not None:
# Circular imports :(
@@ -1153,6 +1189,45 @@ def get_properties_for_binary(bpr):
}
+@implementer(IPackageUploadLog)
+class PackageUploadLog(StormBase):
+ """Tracking of status changes for a given package upload"""
+
+ __storm_table__ = "PackageUploadLog"
+
+ id = Int(primary=True)
+
+ package_upload_id = Int(name='package_upload')
+ package_upload = Reference(package_upload_id, PackageUpload.id)
+
+ date_created = DateTime(tzinfo=pytz.UTC, allow_none=False,
+ default=UTC_NOW)
+
+ reviewer_id = Int(name='reviewer', allow_none=False)
+ reviewer = Reference(reviewer_id, 'Person.id')
+
+ old_status = DBEnum(enum=PackageUploadStatus, allow_none=False)
+
+ new_status = DBEnum(enum=PackageUploadStatus, allow_none=False)
+
+ comment = Unicode(allow_none=True)
+
+ def __init__(self, package_upload, reviewer, old_status, new_status,
+ comment=None, date_created=DEFAULT):
+ self.package_upload = package_upload
+ self.reviewer = reviewer
+ self.old_status = old_status
+ self.new_status = new_status
+ self.comment = comment
+ self.date_created = date_created
+
+ def __repr__(self):
+ return (
+ "<{self.__class__.__name__} ~{self.reviewer.name} "
+ "changed {self.package_upload} to {self.new_status} "
+ "on {self.date_created}>").format(self=self)
+
+
@implementer(IPackageUploadBuild)
class PackageUploadBuild(SQLBase):
"""A Queue item's related builds."""
@@ -1528,8 +1603,10 @@ class PackageUploadSet:
PackageUploadBuild, rows, ["packageuploadID"])
pucs = load_referencing(
PackageUploadCustom, rows, ["packageuploadID"])
+ logs = load_referencing(
+ PackageUploadLog, rows, ["package_upload_id"])
- prefill_packageupload_caches(rows, puses, pubs, pucs)
+ prefill_packageupload_caches(rows, puses, pubs, pucs, logs)
return DecoratedResultSet(query, pre_iter_hook=preload_hook)
@@ -1550,18 +1627,32 @@ class PackageUploadSet:
PackageUpload.package_copy_job_id.is_in(pcj_ids))
-def prefill_packageupload_caches(uploads, puses, pubs, pucs):
+def prefill_packageupload_caches(uploads, puses, pubs, pucs, logs):
# Circular imports.
from lp.soyuz.model.archive import Archive
from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
from lp.soyuz.model.publishing import SourcePackagePublishingHistory
from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
+ logs_per_pu = defaultdict(list)
+ reviewer_ids = set()
+ for log in logs:
+ reviewer_ids.add(log.reviewer_id)
+ logs_per_pu[log.package_upload_id].append(log)
+
+ # preload reviwers of the logs
+ # Not using `need_icon` since reviwers are persons, and icons are only
+ # available for teams
+ list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
+ reviewer_ids, need_validity=True))
+
for pu in uploads:
cache = get_property_cache(pu)
cache.sources = []
cache.builds = []
cache.customfiles = []
+ cache.logs = sorted(
+ logs_per_pu[pu.id], key=attrgetter("date_created"), reverse=True)
for pus in puses:
get_property_cache(pus.packageupload).sources.append(pus)
diff --git a/lib/lp/soyuz/stories/soyuz/xx-queue-pages.txt b/lib/lp/soyuz/stories/soyuz/xx-queue-pages.txt
index 7417a7b..7db70f8 100644
--- a/lib/lp/soyuz/stories/soyuz/xx-queue-pages.txt
+++ b/lib/lp/soyuz/stories/soyuz/xx-queue-pages.txt
@@ -511,6 +511,7 @@ values:
>>> for row in filelist:
... print(extract_text(row))
pmount_1.0-1_all.deb (18 bytes) NEW 0.1-1 restricted admin extra
+ Accepted a moment ago by Sample Person
'netapplet' has gone straight to the 'done' queue because it's a single
source upload, and we can see its overridden values there:
@@ -546,6 +547,15 @@ Rejecting 'alsa-utils' source:
netapplet...ddtp... - Release 2006-...
netapplet...dist... - Release 2006-...
+ >>> upload_manager_browser.getControl(
+ ... name="queue_state", index=0).displayValue=['Rejected']
+ >>> upload_manager_browser.getControl("Update").click()
+ >>> logs = find_tags_by_class(
+ ... upload_manager_browser.contents, "log-content")
+ >>> for log in logs:
+ ... print(extract_text(log))
+ Rejected...a moment ago...by Sample Person...Foo
+
One rejection email is generated:
>>> run_package_upload_notification_jobs()
@@ -599,6 +609,21 @@ button will be disabled.
>>> upload_manager_browser.getControl(name="Reject").disabled
True
+Accepting alsa again, and check that the package upload log has more rows
+
+ >>> upload_manager_browser.getControl(name="QUEUE_ID").value = ['4']
+ >>> upload_manager_browser.getControl(name="Accept").click()
+ >>> upload_manager_browser.getControl(
+ ... name="queue_state", index=0).displayValue=['Accepted']
+ >>> upload_manager_browser.getControl("Update").click()
+ >>> pkg_content = first_tag_by_class(upload_manager_browser.contents,
+ ... "queue-4")
+ >>> logs = find_tags_by_class(str(pkg_content), "log-content")
+ >>> for log in logs:
+ ... print(extract_text(log))
+ Accepted...a moment ago...by Sample Person...
+ Rejected...a moment ago...by Sample Person...Foo
+
Clean up
========
diff --git a/lib/lp/soyuz/templates/distroseries-queue.pt b/lib/lp/soyuz/templates/distroseries-queue.pt
index 59b77d5..39629ce 100644
--- a/lib/lp/soyuz/templates/distroseries-queue.pt
+++ b/lib/lp/soyuz/templates/distroseries-queue.pt
@@ -149,6 +149,16 @@
</tbody>
<tbody tal:attributes="class string:${filelist_class}">
<metal:filelist use-macro="template/macros/package-filelist"/>
+ <tr class="log-content" tal:repeat="log packageupload/logs">
+ <td colspan="2" style="border: 0"></td>
+ <td colspan="8" style="border: 0">
+ <span tal:content="log/new_status"></span>
+ <span tal:attributes="title log/date_created/fmt:datetime"
+ tal:content="log/date_created/fmt:displaydate" />
+ by <span tal:content="structure log/reviewer/fmt:link" />
+ <p tal:condition="log/comment" tal:content="log/comment" />
+ </td>
+ </tr>
</tbody>
</tal:block>
</tal:batch>
diff --git a/lib/lp/soyuz/tests/test_packageupload.py b/lib/lp/soyuz/tests/test_packageupload.py
index cb4d038..297f01b 100644
--- a/lib/lp/soyuz/tests/test_packageupload.py
+++ b/lib/lp/soyuz/tests/test_packageupload.py
@@ -16,7 +16,10 @@ from lazr.restfulclient.errors import (
BadRequest,
Unauthorized,
)
-from testtools.matchers import Equals
+from testtools.matchers import (
+ Equals,
+ MatchesStructure,
+ )
import transaction
from zope.component import (
getUtility,
@@ -91,6 +94,27 @@ class PackageUploadTestCase(TestCaseWithFactory):
if os.path.exists(config.personalpackagearchive.root):
shutil.rmtree(config.personalpackagearchive.root)
+ def test_add_log_entry(self):
+ upload = self.factory.makePackageUpload(
+ status=PackageUploadStatus.UNAPPROVED)
+ upload = removeSecurityProxy(upload)
+ self.assertEqual(0, len(upload.logs))
+
+ person = self.factory.makePerson(name='lpusername')
+
+ upload._addLog(person, PackageUploadStatus.REJECTED, 'just because')
+
+ log = upload.logs[0]
+ self.assertThat(log, MatchesStructure.byEquality(
+ reviewer=person, old_status=PackageUploadStatus.UNAPPROVED,
+ new_status=PackageUploadStatus.REJECTED, comment='just because'))
+
+ expected_repr = (
+ "<PackageUploadLog ~lpusername "
+ "changed {self.package_upload} to Rejected "
+ "on {self.date_created}>").format(self=log)
+ self.assertEqual(str(expected_repr), repr(log))
+
def test_realiseUpload_for_overridden_component_archive(self):
# If the component of an upload is overridden to 'Partner' for
# example, then the new publishing record should be for the
@@ -358,8 +382,14 @@ class PackageUploadTestCase(TestCaseWithFactory):
# Accepting one of them works. (Since it's a single source upload,
# it goes straight to DONE.)
- upload_one.acceptFromQueue()
+ person = self.factory.makePerson()
+ upload_one.acceptFromQueue(person)
self.assertEqual("DONE", upload_one.status.name)
+
+ log = upload_one.logs[0]
+ self.assertThat(log, MatchesStructure.byEquality(
+ reviewer=person, old_status=PackageUploadStatus.UNAPPROVED,
+ new_status=PackageUploadStatus.ACCEPTED, comment=None))
transaction.commit()
# Trying to accept the second fails.
@@ -368,9 +398,15 @@ class PackageUploadTestCase(TestCaseWithFactory):
self.assertEqual("UNAPPROVED", upload_two.status.name)
# Rejecting the second upload works.
- upload_two.rejectFromQueue(self.factory.makePerson())
+ upload_two.rejectFromQueue(person, 'Because yes')
self.assertEqual("REJECTED", upload_two.status.name)
+ self.assertEqual(1, len(upload_two.logs))
+ log = upload_two.logs[0]
+ self.assertThat(log, MatchesStructure.byEquality(
+ reviewer=person, old_status=PackageUploadStatus.UNAPPROVED,
+ new_status=PackageUploadStatus.REJECTED, comment='Because yes'))
+
def test_rejectFromQueue_source_sends_email(self):
# Rejecting a source package sends an email to the uploader.
self.test_publisher.prepareBreezyAutotest()
@@ -1447,3 +1483,21 @@ class TestPackageUploadWebservice(TestCaseWithFactory):
person, component=self.universe),
5)
self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
+
+ def test_api_package_upload_log(self):
+ # API clients can see upload logs of a source uploads.
+ admin = self.makeQueueAdmin([self.universe])
+ upload, ws_upload = self.makeSourcePackageUpload(
+ admin, sourcepackagename="hello", component=self.universe)
+ with person_logged_in(admin):
+ upload.rejectFromQueue(admin, 'not a good change')
+ upload.acceptFromQueue(admin)
+
+ logs = removeSecurityProxy(upload).logs
+ ws_logs = ws_upload.logs
+ for log, ws_log in zip(logs, ws_logs):
+ self.assertEqual(log.comment, ws_log.comment)
+ self.assertEqual(log.date_created, ws_log.date_created)
+ self.assertEqual(log.new_status.title, ws_log.new_status)
+ self.assertEqual(log.old_status.title, ws_log.old_status)
+ self.assertEqual(log.reviewer.name, ws_log.reviewer.name)
\ No newline at end of file
