2012875 - jinja: job view uses `phabricator_revision_url` for revisions not linked to phabricator

Status: ASSIGNED

(Conduit :: Lando, defect)

Description

[Olivier Mehani [:shtrom]]

https://github.com/mozilla-conduit/lando/blob/c47447edb21bcbd3809d2d03035fdacd6d0d1d70/src/lando/ui/jinja2/partials/job.html#L31-L40

Comment 1

[Olivier Mehani [:shtrom]]

Attachment: 2026-01-28T14:23:15,631601026+11:00.png

Note: the admin page shows them better

Comment 2

[Olivier Mehani [:shtrom]]

We should probably bite the bullet, and have a generic filter that stringifies / linkifies any revision, and makes the correct choice based on what the revision metadata contains, rather than pushing this logic to the templates.

Comment 3

[Olivier Mehani [:shtrom]]

Attachment: image.png

This is a problem for Try pushes as well, which makes up links to invalid Phabricator links. In this case, it would be better to have either no link, or links to the commits (when they exist?)

Comment 4

[Olivier Mehani [:shtrom]]

This is also a problem for PRs. We could redirect them to the pull-request page, but we'll need to add a pull_repo to the Revision model, to allow full information when resolving the pull_number

Comment 6

[BMO Github Automation]

Attachment: [mozilla-conduit/lando] revision: differentiate revision strings depending on origin (bug 2012875) (#1340)

Comment 7

[Olivier Mehani [:shtrom]]

This is partially blocked by bug 2012875 because the error-breakdown Jinja template heavily assumes that a (Phab) revision_id will be provided https://github.com/mozilla-conduit/lando/blob/cc19927647809a11edf1af643b74dcca8a8b47be/src/lando/ui/jinja2/partials/error-breakdown.html#L14-L17, which is not always the case (e.g. Try or PRs).

This is the last user of phabricator_revision_url