Merge lp:~maxiberta/launchpad/lp-1729580 into lp:launchpad

[Maximiliano Bertacchini]

Expose extended error messages (with external link) for snap build jobs (LP: #1729580).

Expose extended error messages (with external link) for snap build jobs (LP: #1729580).

[Colin Watson (cjwatson)]

review: Approve

=== modified file 'lib/lp/snappy/browser/tests/test_snapbuild.py'
--- lib/lp/snappy/browser/tests/test_snapbuild.py	2017-10-20 13:35:42 +0000
+++ lib/lp/snappy/browser/tests/test_snapbuild.py	2018-02-19 17:53:06 +0000
@@ -14,7 +14,10 @@
 from pymacaroons import Macaroon
 import soupmatchers
 from storm.locals import Store
-from testtools.matchers import StartsWith
+from testtools.matchers import (
+    Not,
+    StartsWith,
+    )
 import transaction
 from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
@@ -130,6 +133,55 @@
                 text=re.compile(
                     r"^\s*Store upload failed:\s+Scan failed.\s*$"))))
 
+    def test_store_upload_status_failed_with_extended_error_message(self):
+        build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+        job = getUtility(ISnapStoreUploadJobSource).create(build)
+        naked_job = removeSecurityProxy(job)
+        naked_job.job._status = JobStatus.FAILED
+        naked_job.error_message = "This should not be shown."
+        naked_job.error_messages = [
+            {"message": "Scan failed.", "link": "link1"},
+            {"message": "Classic not allowed.", "link": "link2"}]
+        build_view = create_initialized_view(build, "+index")
+        built_view = build_view()
+        self.assertThat(built_view, Not(soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                "store upload status", "li",
+                attrs={"id": "store-upload-status"},
+                text=re.compile('.*This should not be shown.*')))))
+        self.assertThat(built_view, soupmatchers.HTMLContains(
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "store upload status", "li",
+                    attrs={"id": "store-upload-status"}),
+                soupmatchers.Tag(
+                    "store upload error messages", "ul",
+                    attrs={"id": "store-upload-error-messages"}))))
+        self.assertThat(built_view, soupmatchers.HTMLContains(
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "store upload error messages", "ul",
+                    attrs={"id": "store-upload-error-messages"}),
+                soupmatchers.Within(
+                    soupmatchers.Tag(
+                        "store upload error message", "li",
+                        text=re.compile(".*Scan failed\..*")),
+                    soupmatchers.Tag(
+                        "store upload error link", "a",
+                        attrs={"href": "link1"}, text="(?)")))))
+        self.assertThat(built_view, soupmatchers.HTMLContains(
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "store upload error messages", "ul",
+                    attrs={"id": "store-upload-error-messages"}),
+                soupmatchers.Within(
+                    soupmatchers.Tag(
+                        "store upload error message", "li",
+                        text=re.compile(".*Classic not allowed\..*")),
+                    soupmatchers.Tag(
+                        "store upload error link", "a",
+                        attrs={"href": "link2"}, text="(?)")))))
+
     def test_store_upload_status_release_failed(self):
         build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
         job = getUtility(ISnapStoreUploadJobSource).create(build)

=== modified file 'lib/lp/snappy/interfaces/snapbuild.py'
--- lib/lp/snappy/interfaces/snapbuild.py	2017-04-27 16:22:37 +0000
+++ lib/lp/snappy/interfaces/snapbuild.py	2018-02-19 17:53:06 +0000
@@ -39,7 +39,9 @@
     Bool,
     Choice,
     Datetime,
+    Dict,
     Int,
+    List,
     TextLine,
     )
 
@@ -213,6 +215,16 @@
             "this snap build to the store."),
         required=False, readonly=True))
 
+    store_upload_error_messages = exported(List(
+        title=_("Store upload error messages"),
+        description=_(
+            "A list of dict(message, link) where message is an error "
+            "description and link, if any, is an external link to extra "
+            "details, from the last attempt to upload this snap build "
+            "to the store."),
+        value_type=Dict(key_type=TextLine()),
+        required=False, readonly=True))
+
     def getFiles():
         """Retrieve the build's `ISnapFile` records.
 

=== modified file 'lib/lp/snappy/interfaces/snapstoreclient.py'
--- lib/lp/snappy/interfaces/snapstoreclient.py	2017-06-14 10:25:23 +0000
+++ lib/lp/snappy/interfaces/snapstoreclient.py	2018-02-19 17:53:06 +0000
@@ -29,10 +29,12 @@
 
 class SnapStoreError(Exception):
 
-    def __init__(self, message="", detail=None, can_retry=False):
+    def __init__(
+            self, message="", detail=None, messages=None, can_retry=False):
         super(SnapStoreError, self).__init__(message)
         self.message = message
         self.detail = detail
+        self.messages = messages
         self.can_retry = can_retry
 
 

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2018-01-23 10:59:44 +0000
+++ lib/lp/snappy/model/snapbuild.py	2018-02-19 17:53:06 +0000
@@ -456,6 +456,11 @@
         job = self.last_store_upload_job
         return job and job.error_message
 
+    @property
+    def store_upload_error_messages(self):
+        job = self.last_store_upload_job
+        return job and job.error_messages or []
+
     def scheduleStoreUpload(self):
         """See `ISnapBuild`."""
         if not self.snap.can_upload_to_store:

=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py	2017-06-29 16:58:01 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2018-02-19 17:53:06 +0000
@@ -232,6 +232,16 @@
         self.metadata["error_detail"] = detail
 
     @property
+    def error_messages(self):
+        """See `ISnapStoreUploadJob`."""
+        return self.metadata.get("error_messages")
+
+    @error_messages.setter
+    def error_messages(self, messages):
+        """See `ISnapStoreUploadJob`."""
+        self.metadata["error_messages"] = messages
+
+    @property
     def store_url(self):
         """See `ISnapStoreUploadJob`."""
         return self.metadata.get("store_url")
@@ -326,6 +336,7 @@
                     self.attempt_count <= self.max_retries):
                 raise RetryableSnapStoreError(e.message, detail=e.detail)
             self.error_message = str(e)
+            self.error_messages = getattr(e, "messages", None)
             self.error_detail = getattr(e, "detail", None)
             if isinstance(e, UnauthorizedUploadResponse):
                 mailer = SnapBuildMailer.forUnauthorizedUpload(self.snapbuild)

=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
--- lib/lp/snappy/model/snapstoreclient.py	2017-06-14 10:25:23 +0000
+++ lib/lp/snappy/model/snapstoreclient.py	2018-02-19 17:53:06 +0000
@@ -316,7 +316,14 @@
             elif "errors" in response_data:
                 error_message = "\n".join(
                     error["message"] for error in response_data["errors"])
-                raise ScanFailedResponse(error_message)
+                error_messages = []
+                for error in response_data["errors"]:
+                    error_detail = {"message": error["message"]}
+                    if "link" in error:
+                        error_detail["link"] = error["link"]
+                    error_messages.append(error_detail)
+                raise ScanFailedResponse(
+                    error_message, messages=error_messages)
             elif not response_data["can_release"]:
                 return response_data["url"], None
             else:

=== modified file 'lib/lp/snappy/templates/snapbuild-index.pt'
--- lib/lp/snappy/templates/snapbuild-index.pt	2017-04-03 12:35:03 +0000
+++ lib/lp/snappy/templates/snapbuild-index.pt	2018-02-19 17:53:06 +0000
@@ -177,7 +177,18 @@
         <tal:failed-upload
             condition="context/store_upload_status/enumvalue:FAILEDTOUPLOAD">
           Store upload failed:
-          <tal:error-message replace="context/store_upload_error_message" />
+          <tal:error-message
+            condition="not: context/store_upload_error_messages"
+            replace="context/store_upload_error_message" />
+          <ul id="store-upload-error-messages">
+            <li tal:repeat="error context/store_upload_error_messages">
+              <span tal:replace="error/message"/>
+              <a tal:condition="error/link"
+                  tal:attributes="href error/link"
+                  target="help"
+                  class="sprite maybe action-icon">(?)</a>
+            </li>
+          </ul>
           <form action="" method="POST">
             <input type="submit" name="field.actions.upload" value="Retry" />
           </form>

=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py	2017-06-29 18:06:03 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2018-02-19 17:53:06 +0000
@@ -31,9 +31,9 @@
     )
 from lp.snappy.interfaces.snapstoreclient import (
     BadRefreshResponse,
-    BadScanStatusResponse,
     ISnapStoreClient,
     ReleaseFailedResponse,
+    ScanFailedResponse,
     UnauthorizedUploadResponse,
     UploadFailedResponse,
     UploadNotScannedYetResponse,
@@ -433,7 +433,11 @@
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
         client.upload.result = self.status_url
-        client.checkStatus.failure = BadScanStatusResponse("Scan failed.")
+        client.checkStatus.failure = ScanFailedResponse(
+            "Scan failed.\nConfinement not allowed.",
+            messages=[
+                {"message": "Scan failed.", "link": "link1"},
+                {"message": "Confinement not allowed.", "link": "link2"}])
         self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
         with dbuser(config.ISnapStoreUploadJobSource.dbuser):
             JobRunner([job]).runAll()
@@ -443,7 +447,12 @@
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertIsNone(job.store_url)
         self.assertIsNone(job.store_revision)
-        self.assertEqual("Scan failed.", job.error_message)
+        self.assertEqual(
+            "Scan failed.\nConfinement not allowed.", job.error_message)
+        self.assertEqual([
+            {"message": "Scan failed.", "link": "link1"},
+            {"message": "Confinement not allowed.", "link": "link2"}],
+            job.error_messages)
         [notification] = pop_notifications()
         self.assertEqual(
             config.canonical.noreply_from_address, notification["From"])

=== modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
--- lib/lp/snappy/tests/test_snapstoreclient.py	2017-06-14 10:25:23 +0000
+++ lib/lp/snappy/tests/test_snapstoreclient.py	2018-02-19 17:53:06 +0000
@@ -581,6 +581,7 @@
                     "errors": [
                         {"code": None,
                          "message": "You cannot use that reserved namespace.",
+                         "link": "http://example.com"
                          }],
                     }}