Diff comments:

> === 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-16 21:53:15 
> +0000
> @@ -130,6 +130,38 @@
>                  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.Within(
> +                    soupmatchers.Tag(
> +                        "store upload error messages", "ul",
> +                        attrs={"id": "store-upload-error-messages"}),
> +                    soupmatchers.Within(
> +                        soupmatchers.Tag(
> +                            "store upload error message", "li"),
> +                        soupmatchers.Tag(
> +                            "store upload error link", "a",
> +                            text="What does this mean?"))))))

Done.

> +
>      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/model/snapstoreclient.py'
> --- lib/lp/snappy/model/snapstoreclient.py    2017-06-14 10:25:23 +0000
> +++ lib/lp/snappy/model/snapstoreclient.py    2018-02-16 21:53:15 +0000
> @@ -316,7 +316,11 @@
>              elif "errors" in response_data:
>                  error_message = "\n".join(
>                      error["message"] for error in response_data["errors"])
> -                raise ScanFailedResponse(error_message)
> +                error_messages = [
> +                    {"message": error["message"], "link": error.get("link")}
> +                    for error in response_data["errors"]]

Good idea, done.

> +                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-16 21:53:15 
> +0000
> @@ -177,7 +177,15 @@
>          <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">What does this mean?</a>

Nice!

> +            </li>
> +          </ul>
>            <form action="" method="POST">
>              <input type="submit" name="field.actions.upload" value="Retry" />
>            </form>


-- 
https://code.launchpad.net/~maxiberta/launchpad/lp-1729580/+merge/337825
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to