ui: show link back to lando PR view from jobs and error breakdowns (bug 2002161)

 * ui: show link back to lando PR view from jobs
 * ui: show correct link back to lando PR view from error breakdowns (bug 2002161)
 * jinja: add pull_request_link jinja function (bug 2002161)

Pull request: #769

Author: shtrom

src/lando/jinja.py

@@ -1,7 +1,6 @@
 import logging
 import re
 import urllib.parse
-from typing import Optional
 
 from compressor.contrib.jinja2ext import CompressorExtension
 from django.conf import settings
@@ -13,6 +12,7 @@
 from markupsafe import Markup
 
 from lando.main.models import JobStatus, LandingJob, Repo, UpliftJob
+from lando.main.models.revision import Revision
 from lando.main.scm import SCM_TYPE_GIT
 from lando.treestatus.models import (
     ReasonCategory,
@@ -255,13 +255,13 @@ def linkify_bug_numbers(text: str) -> str:
     return re.sub(search, replace, str(text), flags=re.IGNORECASE)
 
 
-def linkify_revision_urls(text: str) -> str:
+def linkify_phabricator_revision_urls(text: str) -> str:
     search = r"(?=\b)(" + re.escape(settings.PHABRICATOR_URL) + r"/D\d+)(?=\b)"
     replace = r'<a href="\g<1>">\g<1></a>'
     return re.sub(search, replace, str(text), flags=re.IGNORECASE)
 
 
-def linkify_revision_ids(text: str) -> str:
+def linkify_phabricator_revision_ids(text: str) -> str:
     """Linkify revision IDs to proper Phabricator URLs."""
     search = r"\b(D\d+)\b"
     replace = (
@@ -317,7 +317,11 @@ def bug_url(text: str) -> str:
     )
 
 
-def revision_url(revision_id: int | str, diff_id: Optional[str] = None) -> str:
+def pull_request_url(repo: Repo, revision: Revision) -> str:
+    return f"{repo.normalized_url}/pull/{revision.pull_number}"
+
+
+def phabricator_revision_url(revision_id: int | str, diff_id: str | None = None) -> str:
     if isinstance(revision_id, int):
         path = f"D{revision_id}"
     elif isinstance(revision_id, str) and not revision_id.startswith("D"):
@@ -438,6 +442,7 @@ def environment(**options):  # noqa: ANN201
             "config": settings,
             "get_messages": messages.get_messages,
             "graph_height": graph_height,
+            "pull_request_url": pull_request_url,
             "treeherder_link": treeherder_link,
             "new_settings_form": UserSettingsForm,
             "static_url": settings.STATIC_URL,
@@ -456,8 +461,8 @@ def environment(**options):  # noqa: ANN201
             "graph_x_pos": graph_x_pos,
             "linkify_bug_numbers": linkify_bug_numbers,
             "linkify_faq": linkify_faq,
-            "linkify_revision_ids": linkify_revision_ids,
-            "linkify_revision_urls": linkify_revision_urls,
+            "linkify_phabricator_revision_ids": linkify_phabricator_revision_ids,
+            "linkify_phabricator_revision_urls": linkify_phabricator_revision_urls,
             "linkify_sec_bug_docs": linkify_sec_bug_docs,
             "linkify_transplant_details": linkify_transplant_details,
             "message_type_to_notification_class": message_type_to_notification_class,
@@ -467,7 +472,7 @@ def environment(**options):  # noqa: ANN201
             "reviewer_to_action_text": reviewer_to_action_text,
             "reviewer_to_status_badge_class": reviewer_to_status_badge_class,
             "revision_status_to_badge_class": revision_status_to_badge_class,
-            "revision_url": revision_url,
+            "phabricator_revision_url": phabricator_revision_url,
             "static": static,
             "tostatusbadgeclass": tostatusbadgeclass,
             "tostatusbadgename": tostatusbadgename,

src/lando/ui/jinja2/partials/error-breakdown.html

@@ -7,9 +7,15 @@
     {% set tree = job.target_repo.short_name|default("Repo " + job.target_repo.name) %}
     {% set rejects_paths = job.error_breakdown.rejects_paths %}
     <p>
-        While applying <a href="{{ job.error_breakdown.revision_id|revision_url() }}">revision D{{
-        job.error_breakdown.revision_id }}</a> to <code>{{ tree }}</code>, the following files had
-        conflicts:
+        {% if job.is_pull_request_job %}
+            {% set top_revision = job.revisions[0] %}
+            While applying <a href="{{ pull_request_url(job.target_repo, top_revision) }}">pull request #{{ top_revision.pull_number }}</a> to
+            <code>{{ tree }}</code>, the following files had conflicts:
+        {% else %}
+            While applying <a href="{{ job.error_breakdown.revision_id|phabricator_revision_url() }}">revision D{{
+            job.error_breakdown.revision_id }}</a> to <code>{{ tree }}</code>, the following files had
+            conflicts:
+        {% endif %}
     </p>
     <p>
         (Hint: try rebasing your changes on the latest commits from <code>{{ tree }}</code> and re-submitting.)

src/lando/ui/jinja2/partials/job.html

@@ -13,6 +13,12 @@
                 <strong>Landing Job:</strong>
                 <a href="{{ url('jobs-page', args=[job.id]) }}">{{ job.id }}</a>
             </p>
+        {% elif has_revisions and job.is_pull_request_job %}
+            {% set top_revision = job.revisions[0] %}
+            <p>
+                <strong>Pull request:</strong>
+                <a href="{{ url('pull-request', args=[job.target_repo.name, top_revision.pull_number]) }}">{{ job.target_repo.name }}#{{ top_revision.pull_number }}</a>
+            </p>
         {% elif has_revisions %}
             {% set top_revision = job.revisions[0] %}
             {% if top_revision.revision_id %}
@@ -27,7 +33,7 @@
                 <strong>Revisions:</strong>
                 {% for i in job.serialized_landing_path -%}
                     {{- "" if loop.first else " ← " -}}
-                    <a href="{{ i['revision_id']|revision_url(diff_id=i['diff_id']) }}">
+                    <a href="{{ i['revision_id']|phabricator_revision_url(diff_id=i['diff_id']) }}">
                         {{ i['revision_id'] }}
                         {% if i['diff_id'] %}diff {{ i['diff_id'] }}{% endif %}
                     </a>

src/lando/ui/jinja2/stack/partials/landing-preview.html

@@ -49,7 +49,7 @@ <h3 class="StackPage-landingPreview-sectionLabel">Commits:</h3>
                 </div>
                 <div class="StackPage-landingPreview-displayMessagePanel">
                     <pre class="StackPage-landingPreview-commitMessage">{{
-        revision['commit_message']|escape_html|linkify_bug_numbers|linkify_revision_urls|safe
+        revision['commit_message']|escape_html|linkify_bug_numbers|linkify_phabricator_revision_urls|safe
       }}</pre>
                     <div class="StackPage-landingPreview-seeMore"></div>
                 </div>
@@ -87,8 +87,8 @@ <h3 class="StackPage-landingPreview-sectionLabel">Warnings:</h3>
                                 <li class="StackPage-landingPreview-warning">
                                     <label>
                                         <input type="checkbox" name="warnings[]" value="1" />
-                                        {{ dw.message|escape_html|linkify_bug_numbers|linkify_revision_urls|linkify_faq|linkify_sec_bug_docs|safe }}
-                                        [{{ w.revision_id | linkify_revision_ids | safe }}]
+                                        {{ dw.message|escape_html|linkify_bug_numbers|linkify_phabricator_revision_urls|linkify_faq|linkify_sec_bug_docs|safe }}
+                                        [{{ w.revision_id | linkify_phabricator_revision_ids | safe }}]
                                     </label>
                                 </li>
                             {% endfor %}
@@ -97,12 +97,12 @@ <h3 class="StackPage-landingPreview-sectionLabel">Warnings:</h3>
                         <li class="StackPage-landingPreview-warning">
                             <label>
                                 <input type="checkbox" name="warnings[]" value="1" />
-                                {{ warning['display']|escape_html|linkify_bug_numbers|linkify_revision_urls|linkify_faq|linkify_sec_bug_docs|safe }}
+                                {{ warning['display']|escape_html|linkify_bug_numbers|linkify_phabricator_revision_urls|linkify_faq|linkify_sec_bug_docs|safe }}
                                 [
                                 {% for instance in warning['instances'] %}
                                     {{
                                     ", " if not loop.first else ""
-                                    }}{{ instance['revision_id'] | linkify_revision_ids | safe }}
+                                    }}{{ instance['revision_id'] | linkify_phabricator_revision_ids | safe }}
                                 {% endfor %}
                                 ]
                             </label>

src/lando/ui/pull_requests.py

@@ -53,9 +53,7 @@ def get(
             pull_request = client.build_pull_request(number)
         except HTTPError as e:
             if e.response.status_code == 404:
-                raise Http404(
-                    f"Pull request {repo_name}#{number} doesn't exist."
-                ) from e
+                raise Http404(f"Pull request {repo_name}#{number} doesn't exist") from e
             raise e
 
         landing_jobs = get_jobs_for_pull(target_repo, number)

src/lando/ui/tests/test_template_helpers.py

@@ -8,13 +8,13 @@
     build_manual_uplift_instructions,
     linkify_bug_numbers,
     linkify_faq,
-    linkify_revision_ids,
-    linkify_revision_urls,
+    linkify_phabricator_revision_ids,
+    linkify_phabricator_revision_urls,
     linkify_sec_bug_docs,
     linkify_transplant_details,
+    phabricator_revision_url,
     repo_branch_url,
     repo_path,
-    revision_url,
 )
 from lando.main.models import SCM_TYPE_GIT, SCM_TYPE_HG, JobStatus, LandingJob, Repo
 from lando.main.models.revision import Revision
@@ -122,8 +122,8 @@ def test_linkify_bug_numbers(input_text, output_text):
         ),
     ],
 )
-def test_linkify_revision_urls(input_text, output_text):
-    assert output_text == linkify_revision_urls(input_text)
+def test_linkify_phabricator_revision_urls(input_text, output_text):
+    assert output_text == linkify_phabricator_revision_urls(input_text)
 
 
 @pytest.mark.parametrize(
@@ -150,8 +150,8 @@ def test_linkify_revision_urls(input_text, output_text):
         ),
     ],
 )
-def test_linkify_revision_ids(input_text, output_text):
-    assert output_text == linkify_revision_ids(input_text)
+def test_linkify_phabricator_revision_ids(input_text, output_text):
+    assert output_text == linkify_phabricator_revision_ids(input_text)
 
 
 @pytest.mark.django_db
@@ -280,29 +280,29 @@ def test_repo_branch_url(repo, path):
 def test_revision_url__integer():
     revision_id = 1234
     expected_result = "http://phabricator.test/D1234"
-    actual_result = revision_url(revision_id)
+    actual_result = phabricator_revision_url(revision_id)
     assert expected_result == actual_result
 
 
 def test_revision_url__prepended_string():
     revision_id = "D1234"
     expected_result = "http://phabricator.test/D1234"
-    actual_result = revision_url(revision_id)
+    actual_result = phabricator_revision_url(revision_id)
     assert expected_result == actual_result
 
 
 def test_revision_url__string():
     revision_id = "1234"
     expected_result = "http://phabricator.test/D1234"
-    actual_result = revision_url(revision_id)
+    actual_result = phabricator_revision_url(revision_id)
     assert expected_result == actual_result
 
 
 def test_revision_url__general_case_with_diff():
     revision_id = 123
     diff_id = 456
     expected_result = "http://phabricator.test/D123?id=456"
-    actual_result = revision_url(revision_id, diff_id)
+    actual_result = phabricator_revision_url(revision_id, diff_id)
     assert expected_result == actual_result