launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #28681
[Merge] ~cjwatson/launchpad:snap-build-redesign-store-status into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:snap-build-redesign-store-status into launchpad:master.
Commit message:
Redesign store status on SnapBuild:+index
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1979844 in Launchpad itself: "Add SHA-512 hash code to snap package build status page"
https://bugs.launchpad.net/launchpad/+bug/1979844
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/425711
This makes the store name and revision visible without needing to hover over the "Manage this package in the store" link, and also makes the status of store uploads a bit more structured.
A couple of screenshots:
* https://people.canonical.com/~cjwatson/launchpad/screenshots/1979844-upload-in-progress.png
* https://people.canonical.com/~cjwatson/launchpad/screenshots/1979844-upload-complete.png
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:snap-build-redesign-store-status into launchpad:master.
diff --git a/lib/lp/snappy/browser/tests/test_snapbuild.py b/lib/lp/snappy/browser/tests/test_snapbuild.py
index 2e1fb5f..98ba121 100644
--- a/lib/lp/snappy/browser/tests/test_snapbuild.py
+++ b/lib/lp/snappy/browser/tests/test_snapbuild.py
@@ -40,6 +40,7 @@ from lp.testing.layers import (
from lp.testing.pages import (
extract_text,
find_main_content,
+ find_tag_by_id,
find_tags_by_class,
)
from lp.testing.views import create_initialized_view
@@ -88,71 +89,88 @@ class TestSnapBuildView(TestCaseWithFactory):
"revision ID", "li", attrs={"id": "revision-id"},
text=re.compile(r"^\s*Revision: dummy\s*$"))))
- def test_store_upload_status_in_progress(self):
+ def test_no_store_status_if_not_fully_built(self):
+ build = self.factory.makeSnapBuild(status=BuildStatus.NEEDSBUILD)
+ getUtility(ISnapStoreUploadJobSource).create(build)
+ build_view = create_initialized_view(build, "+index")
+ self.assertIsNone(find_tag_by_id(build_view(), "store-status"))
+
+ def test_no_store_status_without_store_name(self):
build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
getUtility(ISnapStoreUploadJobSource).create(build)
build_view = create_initialized_view(build, "+index")
- self.assertThat(build_view(), soupmatchers.HTMLContains(
- soupmatchers.Tag(
- "store upload status", "li",
- attrs={"id": "store-upload-status"},
- text=re.compile(r"^\s*Store upload in progress\s*$"))))
+ self.assertIsNone(find_tag_by_id(build_view(), "store-status"))
+
+ def test_store_upload_status_in_progress(self):
+ build = self.factory.makeSnapBuild(
+ status=BuildStatus.FULLYBUILT, store_name="foo")
+ getUtility(ISnapStoreUploadJobSource).create(build)
+ build_view = create_initialized_view(build, "+index")
+ store_status = find_tag_by_id(build_view(), "store-status")
+ self.assertThat(store_status, soupmatchers.Tag(
+ "store upload status", "dd",
+ attrs={"id": "store-upload-status"},
+ text=re.compile(r"^\s*Store upload in progress\s*$")))
def test_store_upload_status_completed(self):
- build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+ build = self.factory.makeSnapBuild(
+ status=BuildStatus.FULLYBUILT, store_name="foo")
job = getUtility(ISnapStoreUploadJobSource).create(build)
naked_job = removeSecurityProxy(job)
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.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=re.compile(
- r"^\s*Manage this package in the store\s*$")))))
+ store_status = find_tag_by_id(build_view(), "store-status")
+ self.assertThat(store_status, soupmatchers.Within(
+ soupmatchers.Tag(
+ "store upload status", "dd",
+ attrs={"id": "store-upload-status"}),
+ soupmatchers.Tag(
+ "store link", "a", attrs={"href": job.store_url},
+ text=re.compile(
+ r"^\s*Manage this package in the store\s*$"))))
def test_store_upload_status_completed_no_url(self):
# A package that has been uploaded to the store may lack a URL if
# the upload was queued behind others for manual review.
- build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+ build = self.factory.makeSnapBuild(
+ status=BuildStatus.FULLYBUILT, store_name="foo")
job = getUtility(ISnapStoreUploadJobSource).create(build)
naked_job = removeSecurityProxy(job)
naked_job.job._status = JobStatus.COMPLETED
build_view = create_initialized_view(build, "+index")
- 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": config.snappy.store_url},
- text=re.compile(r"^\s*Uploaded to the store\s*$")))))
+ store_status = find_tag_by_id(build_view(), "store-status")
+ self.assertThat(store_status, soupmatchers.Within(
+ soupmatchers.Tag(
+ "store upload status", "dd",
+ attrs={"id": "store-upload-status"}),
+ soupmatchers.Tag(
+ "store link", "a", attrs={"href": config.snappy.store_url},
+ text=re.compile(r"^\s*Uploaded to the store\s*$"))))
def test_store_upload_status_failed(self):
- build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+ build = self.factory.makeSnapBuild(
+ status=BuildStatus.FULLYBUILT, store_name="foo")
job = getUtility(ISnapStoreUploadJobSource).create(build)
naked_job = removeSecurityProxy(job)
naked_job.job._status = JobStatus.FAILED
naked_job.error_message = "Scan failed."
build_view = create_initialized_view(build, "+index")
- self.assertThat(build_view(), soupmatchers.HTMLContains(
+ store_status = find_tag_by_id(build_view(), "store-status")
+ self.assertThat(store_status, soupmatchers.Tag(
+ "store upload status", "dd",
+ attrs={"id": "store-upload-status"}))
+ self.assertThat(store_status, soupmatchers.Within(
soupmatchers.Tag(
- "store upload status", "li",
- attrs={"id": "store-upload-status"}),
- soupmatchers.Within(
- soupmatchers.Tag(
- "store upload error messages", "ul",
- attrs={"id": "store-upload-error-messages"}),
- soupmatchers.Tag(
- "store upload error message", "li",
- text=re.compile(r"^\s*Scan failed.\s*$")))))
+ "store upload error messages", "ul",
+ attrs={"id": "store-upload-error-messages"}),
+ soupmatchers.Tag(
+ "store upload error message", "li",
+ text=re.compile(r"^\s*Scan failed.\s*$"))))
def test_store_upload_status_failed_with_extended_error_message(self):
- build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+ build = self.factory.makeSnapBuild(
+ status=BuildStatus.FULLYBUILT, store_name="foo")
job = getUtility(ISnapStoreUploadJobSource).create(build)
naked_job = removeSecurityProxy(job)
naked_job.job._status = JobStatus.FAILED
@@ -164,71 +182,62 @@ class TestSnapBuildView(TestCaseWithFactory):
{"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(
+ store_status = find_tag_by_id(build_view(), "store-status")
+ self.assertThat(store_status, Not(soupmatchers.Tag(
+ "store upload status", "dd",
+ attrs={"id": "store-upload-status"},
+ text=re.compile('.*This should not be shown.*'))))
+ error_messages = soupmatchers.Tag(
+ "store upload error messages", "ul",
+ attrs={"id": "store-upload-error-messages"})
+ self.assertThat(store_status, soupmatchers.Within(
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(
+ "store upload status", "dd",
+ attrs={"id": "store-upload-status"}),
+ error_messages))
+ self.assertThat(store_status, soupmatchers.Within(
+ error_messages,
+ soupmatchers.Tag(
+ "store upload error message", "li",
+ text=re.compile(".*The new version.*"))))
+ self.assertThat(store_status, soupmatchers.Within(
+ error_messages,
soupmatchers.Within(
soupmatchers.Tag(
- "store upload status", "li",
- attrs={"id": "store-upload-status"}),
+ "store upload error message", "li",
+ text=re.compile(r".*Scan failed\..*")),
soupmatchers.Tag(
- "store upload error messages", "ul",
- attrs={"id": "store-upload-error-messages"}))))
- self.assertThat(built_view, soupmatchers.HTMLContains(
+ "store upload error link", "a",
+ attrs={"href": "link1"}, text="(?)"))))
+ self.assertThat(store_status, soupmatchers.Within(
+ error_messages,
soupmatchers.Within(
soupmatchers.Tag(
- "store upload error messages", "ul",
- attrs={"id": "store-upload-error-messages"}),
- soupmatchers.Tag(
"store upload error message", "li",
- text=re.compile(".*The new version.*")))))
- self.assertThat(built_view, soupmatchers.HTMLContains(
- soupmatchers.Within(
+ text=re.compile(r".*Classic not allowed\..*")),
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(r".*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(r".*Classic not allowed\..*")),
- soupmatchers.Tag(
- "store upload error link", "a",
- attrs={"href": "link2"}, text="(?)")))))
+ "store upload error link", "a",
+ attrs={"href": "link2"}, text="(?)"))))
def test_store_upload_status_release_failed(self):
- build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+ build = self.factory.makeSnapBuild(
+ status=BuildStatus.FULLYBUILT, store_name="foo")
job = getUtility(ISnapStoreUploadJobSource).create(build)
naked_job = removeSecurityProxy(job)
naked_job.job._status = JobStatus.FAILED
naked_job.store_url = "http://sca.example/dev/click-apps/1/rev/1/";
naked_job.error_message = "Failed to publish"
build_view = create_initialized_view(build, "+index")
- self.assertThat(build_view(), soupmatchers.HTMLContains(
- soupmatchers.Within(
- soupmatchers.Tag(
- "store upload status", "li",
- attrs={"id": "store-upload-status"},
- text=re.compile(
- r"^\s*Releasing package to channels failed:\s+"
- r"Failed to publish\s*$")),
- soupmatchers.Tag(
- "store link", "a", attrs={"href": job.store_url}))))
+ store_status = find_tag_by_id(build_view(), "store-status")
+ self.assertThat(store_status, soupmatchers.Within(
+ soupmatchers.Tag(
+ "store upload status", "dd",
+ attrs={"id": "store-upload-status"},
+ text=re.compile(
+ r"^\s*Releasing package to channels failed:\s+"
+ r"Failed to publish\s*$")),
+ soupmatchers.Tag(
+ "store link", "a", attrs={"href": job.store_url})))
class TestSnapBuildOperations(BrowserTestCase):
diff --git a/lib/lp/snappy/templates/snapbuild-index.pt b/lib/lp/snappy/templates/snapbuild-index.pt
index ea087c4..f68e2c2 100644
--- a/lib/lp/snappy/templates/snapbuild-index.pt
+++ b/lib/lp/snappy/templates/snapbuild-index.pt
@@ -156,70 +156,93 @@
tal:attributes="href context/upload_log_url">uploadlog</a>
(<span tal:replace="file/content/filesize/fmt:bytes" />)
</li>
- <li id="store-upload-status"
- tal:define="job context/last_store_upload_job"
- tal:condition="job">
- <a tal:condition="context/store_upload_url"
- tal:attributes="href context/store_upload_url">
- Manage this package in the store
- </a>
- <tal:pending
- condition="context/store_upload_status/enumvalue:PENDING">
- <br tal:condition="context/store_upload_url" />
- Store upload in progress
- </tal:pending>
- <tal:failed-upload
- condition="context/store_upload_status/enumvalue:FAILEDTOUPLOAD">
- Store upload failed:
- <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|nothing"
- tal:attributes="href error/link"
- target="_blank"
- class="sprite maybe action-icon">(?)</a>
- </li>
- </ul>
- <form action="" method="POST">
- <input type="submit" name="field.actions.upload" value="Retry" />
- </form>
- </tal:failed-upload>
- <tal:failed-release
- condition="context/store_upload_status/enumvalue:FAILEDTORELEASE">
- <br />
- Releasing package to channels failed:
- <tal:error-message replace="context/store_upload_error_message" />
- <form action="" method="POST">
- <input type="submit" name="field.actions.upload" value="Retry" />
- </form>
- </tal:failed-release>
- <tal:uploaded
- condition="context/store_upload_status/enumvalue:UPLOADED">
- <tal:comment condition="nothing">
- If the package is in the uploaded state, we normally have a URL
- (rendered above), but we might not if the upload was queued for
- manual review behind other uploads of the same package. Linking
- to the front page of the store in that case isn't great, but
- it's the best we can reasonably do.
- </tal:comment>
- <a tal:condition="not: context/store_upload_url"
- tal:attributes="href modules/lp.services.config/config/snappy/store_url">
- Uploaded to the store
- </a>
- </tal:uploaded>
- </li>
- <li id="store-upload-status"
- tal:condition="python:
- context.status.title == 'Successfully built' and
- context.snap.can_upload_to_store and
- context.last_store_upload_job is None">
- <form action="" method="POST">
- <input type="submit" name="field.actions.upload"
- value="Upload this package to the store" />
- </form>
- </li>
</ul>
+ <div id="store-status"
+ tal:condition="python:
+ context.status.title == 'Successfully built' and
+ context.snap.store_name">
+ <h2>Store status</h2>
+ <div>
+ <dl>
+ <dt>Name:</dt>
+ <dd tal:content="context/snap/store_name" />
+ </dl>
+ <dl tal:condition="context/store_upload_revision">
+ <dt>Revision:</dt>
+ <dd tal:content="context/store_upload_revision" />
+ </dl>
+ <dl>
+ <dt>Upload:</dt>
+ <dd id="store-upload-status">
+ <tal:upload-job
+ define="job context/last_store_upload_job;
+ status context/store_upload_status;
+ url context/store_upload_url;
+ error_message context/store_upload_error_message;
+ error_messages context/store_upload_error_messages"
+ condition="job">
+ <a tal:condition="url" tal:attributes="href url">
+ Manage this package in the store
+ </a>
+ <tal:pending condition="status/enumvalue:PENDING">
+ <br tal:condition="url" />
+ Store upload in progress
+ </tal:pending>
+ <tal:failed-upload condition="status/enumvalue:FAILEDTOUPLOAD">
+ Store upload failed:
+ <ul id="store-upload-error-messages">
+ <li tal:repeat="error error_messages">
+ <span tal:replace="error/message"/>
+ <a tal:condition="error/link|nothing"
+ tal:attributes="href error/link"
+ target="_blank"
+ class="sprite maybe action-icon">(?)</a>
+ </li>
+ </ul>
+ <form action="" method="POST">
+ <input type="submit" name="field.actions.upload"
+ value="Retry" />
+ </form>
+ </tal:failed-upload>
+ <tal:failed-release condition="status/enumvalue:FAILEDTORELEASE">
+ <br />
+ Releasing package to channels failed:
+ <tal:error-message replace="error_message" />
+ <form action="" method="POST">
+ <input type="submit" name="field.actions.upload"
+ value="Retry" />
+ </form>
+ </tal:failed-release>
+ <tal:uploaded condition="status/enumvalue:UPLOADED">
+ <tal:comment condition="nothing">
+ If the package is in the uploaded state, we normally have
+ a URL (rendered above), but we might not if the upload was
+ queued for manual review behind other uploads of the same
+ package. Linking to the front page of the store in that
+ case isn't great, but it's the best we can reasonably do.
+ </tal:comment>
+ <a tal:condition="not: url"
+ tal:attributes="href modules/lp.services.config/config/snappy/store_url">
+ Uploaded to the store
+ </a>
+ </tal:uploaded>
+ </tal:upload-job>
+ <tal:no-upload-job
+ condition="python:
+ context.status.title == 'Successfully built' and
+ context.snap.can_upload_to_store and
+ context.last_store_upload_job is None">
+ <form action="" method="POST">
+ <input type="submit" name="field.actions.upload"
+ value="Upload this package to the store" />
+ </form>
+ </tal:no-upload-job>
+ </dd>
+ </dl>
+ </div>
+ </div>
+
<div
style="margin-top: 1.5em"
tal:define="link context/menu:context/rescore"
