launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/fix-snap-store-upload-status into lp:launchpad.

Commit message:
Fix rendering of SnapBuildView.store_upload_status.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/fix-snap-store-upload-status/+merge/300091

Fix rendering of SnapBuildView.store_upload_status.

I'd written structured(...) by mistake instead of structured(...).escapedtext, which you need in order to get HTML text.  The tests now check the generated HTML properly rather than doing an incorrect test on the view's attribute.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/fix-snap-store-upload-status into lp:launchpad.

=== modified file 'lib/lp/snappy/browser/snapbuild.py'
--- lib/lp/snappy/browser/snapbuild.py	2016-06-21 14:51:06 +0000
+++ lib/lp/snappy/browser/snapbuild.py	2016-07-14 15:20:42 +0000
@@ -94,9 +94,10 @@
         elif job.job.status == JobStatus.COMPLETED:
             return structured(
                 '<a href="%s">Manage this package in the store</a>',
-                job.store_url)
+                job.store_url).escapedtext
         else:
-            return structured("Store upload failed: %s", job.error_message)
+            return structured(
+                "Store upload failed: %s", job.error_message).escapedtext
 
 
 class SnapBuildCancelView(LaunchpadFormView):

=== modified file 'lib/lp/snappy/browser/tests/test_snapbuild.py'
--- lib/lp/snappy/browser/tests/test_snapbuild.py	2016-06-28 21:10:18 +0000
+++ lib/lp/snappy/browser/tests/test_snapbuild.py	2016-07-14 15:20:42 +0000
@@ -7,6 +7,7 @@
 
 from fixtures import FakeLogger
 from mechanize import LinkNotFoundError
+import soupmatchers
 from storm.locals import Store
 from testtools.matchers import StartsWith
 import transaction
@@ -77,8 +78,11 @@
         build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
         getUtility(ISnapStoreUploadJobSource).create(build)
         build_view = create_initialized_view(build, "+index")
-        self.assertEqual(
-            "Store upload in progress", build_view.store_upload_status)
+        self.assertThat(build_view(), soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                "store upload status", "li",
+                attrs={"id": "store-upload-status"},
+                text="Store upload in progress")))
 
     def test_store_upload_status_completed(self):
         build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
@@ -87,10 +91,14 @@
         naked_job.job._status = JobStatus.COMPLETED
         naked_job.store_url = "http://sca.example/dev/click-apps/1/rev/1/";
         build_view = create_initialized_view(build, "+index")
-        self.assertEqual(
-            '<a href="%s">Manage this package in the store</a>' % (
-                job.store_url),
-            build_view.store_upload_status.escapedtext)
+        self.assertThat(build_view(), soupmatchers.HTMLContains(
+            soupmatchers.Within(
+                soupmatchers.Tag(
+                    "store upload status", "li",
+                    attrs={"id": "store-upload-status"}),
+                soupmatchers.Tag(
+                    "store link", "a", attrs={"href": job.store_url},
+                    text="Manage this package in the store"))))
 
     def test_store_upload_status_failed(self):
         build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
@@ -99,9 +107,11 @@
         naked_job.job._status = JobStatus.FAILED
         naked_job.error_message = "Scan failed."
         build_view = create_initialized_view(build, "+index")
-        self.assertEqual(
-            "Store upload failed: Scan failed.",
-            build_view.store_upload_status.escapedtext)
+        self.assertThat(build_view(), soupmatchers.HTMLContains(
+            soupmatchers.Tag(
+                "store upload status", "li",
+                attrs={"id": "store-upload-status"},
+                text="Store upload failed: Scan failed.")))
 
 
 class TestSnapBuildOperations(BrowserTestCase):

=== modified file 'lib/lp/snappy/templates/snapbuild-index.pt'
--- lib/lp/snappy/templates/snapbuild-index.pt	2016-06-21 14:51:06 +0000
+++ lib/lp/snappy/templates/snapbuild-index.pt	2016-07-14 15:20:42 +0000
@@ -159,7 +159,8 @@
            tal:attributes="href context/upload_log_url">uploadlog</a>
         (<span tal:replace="file/content/filesize/fmt:bytes" />)
       </li>
-      <li tal:define="store_upload_status view/store_upload_status"
+      <li id="store-upload-status"
+          tal:define="store_upload_status view/store_upload_status"
           tal:condition="store_upload_status"
           tal:content="structure store_upload_status" />
     </ul>