pull_request: improve warnings and blockers UI (bug 2024930) r=sheehan,shtrom

- add icons beside warnings/blockers to make them more prominent
- and check mark when there are no warnings/blockers
- add spinners when loading warnings/blockers
- other small html tweaks

Pull request: #1070

Author: zzzeid

src/lando/api/views.py

@@ -8,6 +8,7 @@
 from django.core.handlers.wsgi import WSGIRequest
 from django.http import HttpRequest, JsonResponse
 from django.utils.decorators import method_decorator
+from django.utils.html import escape
 from django.views import View
 from django.views.decorators.csrf import csrf_exempt
 
@@ -61,7 +62,10 @@ def _wrapper(self: View, request: WSGIRequest, *args, **kwargs) -> Callable:
 
 
 def generate_warnings_and_blockers(
-    target_repo: Repo, pull_request: PullRequest, request: HttpRequest
+    target_repo: Repo,
+    pull_request: PullRequest,
+    request: HttpRequest,
+    do_escape: bool = True,
 ) -> dict[str, list[str]]:
     """Run checks on a pull request and return blockers and warnings."""
     # PullRequestPatchHelper.diff doesn't include binary changes.
@@ -79,6 +83,12 @@ def generate_warnings_and_blockers(
     blockers += pr_checks.run(pr_blockers, pull_request)
     pr_warnings = [chk.name() for chk in ALL_PULL_REQUEST_WARNINGS]
     warnings = pr_checks.run(pr_warnings, pull_request)
+
+    if do_escape:
+        # Sanitize blockers and warnings as they may be rendered in a page.
+        warnings = [escape(warning) for warning in warnings]
+        blockers = [escape(blocker) for blocker in blockers]
+
     return {"warnings": warnings, "blockers": blockers}
 
 

src/lando/static_src/legacy/js/components/Stack.js

@@ -152,22 +152,23 @@ $.fn.stack = function() {
 
                             var has_blockers = blockers.length !== 0;
                             var has_warnings = warnings.length !== 0;
+                            var success_placeholder = `<li><span class="fa-li has-text-success"><i class="fa fa-check"></i></span>None found.</li>`;
 
                             if (!has_blockers) {
-                                $("#blockers").html("None found.");
+                                $("#blockers").html(success_placeholder);
                             } else {
                                 $("#blockers").html("");
                                 for (var blocker of blockers) {
-                                    $("#blockers").append(`<li>${blocker}</li>`);
+                                    $("#blockers").append(`<li><span class="fa-li has-text-danger"><i class="fa fa-ban"></i></span>${blocker}</li>`);
                                 }
                             }
 
                             if (!has_warnings) {
-                                $("#warnings").html("None found.");
+                                $("#warnings").html(success_placeholder);
                             } else {
                                 $("#warnings").html("");
                                 for (var warning of warnings) {
-                                    $("#warnings").append(`<li>${warning}</li>`);
+                                    $("#warnings").append(`<li><span class="fa-li has-text-warning"><i class="fa fa-warning"></i></span>${warning}</li>`);
                                 }
                             }
 

src/lando/ui/jinja2/stack/pull_request.html

@@ -15,52 +15,48 @@ <h1>
                     data-csrf-token="{{ csrf_token }}">Loading</button>
         </p>
     </p>
-    <p class="acknowledge-warnings-section">
-        <label class="checkbox">
-            <input type="checkbox" id="acknowledge-warnings" autocomplete="off" />
-            Acknowledge all warnings below to continue.
-        </label>
-    </p>
     <div class="StackPage-stack">
+        <p class="acknowledge-warnings-section">
+            <label class="checkbox">
+                <input type="checkbox" id="acknowledge-warnings" autocomplete="off" />
+                Acknowledge all warnings below to continue.
+            </label>
+        </p>
         <table>
             <tr>
                 <th>Warnings</th>
                 <td>
-                    <ul id="warnings">
+                    <ul id="warnings" class="fa-ul">
                         <li>
-                            Checking...
-                        </li />
+                            <span class="fa-li"><i class="fa fa-spinner fa-pulse"></i></span>Checking...
+                        </li>
                     </ul>
                 </td>
             </tr>
             <tr>
                 <th>Blockers</th>
                 <td>
-                    <ul id="blockers">
+                    <ul id="blockers" class="fa-ul">
                         <li>
-                            Checking...
-                        </li />
+                            <span class="fa-li"><i class="fa fa-spinner fa-pulse"></i></span>Checking...
+                        </li>
                     </ul>
                 </td>
             </tr>
             {% if is_try_compatible %}
                 <tr>
                     <th>Last try job</th>
                     <td>
-                        <ul>
-                            <li>
-                                <button class="button is-link is-small is-info post-try-job"
-                                        data-pull-number="{{ pull_request.number }}"
-                                        data-repo-name="{{ target_repo.name }}"
-                                        data-csrf-token="{{ csrf_token }}">Request a try push</button>
-                                {% if last_try_job %}
-                                    {{ last_try_job }}
-                                    {{ treeherder_link(last_try_job.landed_treeherder_revision, last_try_job.target_repo, "Treeherder") | safe }}
-                                {% else %}
-                                    No try jobs found.
-                                {% endif %}
-                            </li />
-                        </ul>
+                        <button class="button is-link is-small is-info post-try-job"
+                                data-pull-number="{{ pull_request.number }}"
+                                data-repo-name="{{ target_repo.name }}"
+                                data-csrf-token="{{ csrf_token }}">Request a try push</button>
+                        {% if last_try_job %}
+                            {{ last_try_job }}
+                            {{ treeherder_link(last_try_job.landed_treeherder_revision, last_try_job.target_repo, "Treeherder") | safe }}
+                        {% else %}
+                            No try jobs found.
+                        {% endif %}
                     </td>
                 </tr>
             {% endif %}