Bug 2059616: enable readable-status unconditionally - #180
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes BugModal’s “readable status” payload (used to generate the Tracking subtitle summary) available across all products by removing the previous Mozilla/product gating and simplifying the template-side Tracking subtitle fallback.
Changes:
- Removed the Firefox/Mozilla-specific Tracking subtitle construction logic from the BugModal edit template.
- Removed product allowlisting and now always attempts to construct
readable_bug_status_jsoninBugModal/Extension.pm. - Adjusted the Tracking module subtitle fallback when
readable_bug_status_jsonis unavailable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| extensions/BugModal/template/en/default/bug_modal/edit.html.tmpl | Removes Firefox-specific Tracking subtitle building; uses readable-status span when available and a simplified fallback subtitle otherwise. |
| extensions/BugModal/Extension.pm | Removes product gating and unconditionally builds readable-status JSON, including optional tracking-derived fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [% sub = [{ unfiltered = readable_bug_status_span }] %] | ||
| [% ELSE %] | ||
| [% sub = [firefox_flags_subtitle] %] | ||
| [% sub = ["Not tracked"] %] |
| Bugzilla::Extension::TrackingFlags->template_before_process({ | ||
| file => 'bug/edit.html.tmpl', vars => $vars, | ||
| }); | ||
|
|
||
| if (any { $bug->product eq $_ } READABLE_BUG_STATUS_PRODUCTS) { | ||
| my @flags = map { {name => $_->name, status => $_->status} } @{$bug->flags}; | ||
| $vars->{readable_bug_status_json} = encode_json({ | ||
| dupe_of => $bug->dup_id, | ||
| id => $bug->id, | ||
| keywords => [map { $_->name } @{$bug->keyword_objects}], | ||
| priority => $bug->priority, | ||
| resolution => $bug->resolution, | ||
| status => $bug->bug_status, | ||
| flags => \@flags, | ||
| target_milestone => $bug->target_milestone, | ||
| map { $_->name => $_->bug_flag($bug->id)->value } @{$vars->{tracking_flags}}, | ||
| }); | ||
|
|
||
| # HTML4 attributes cannot be longer than this, so just skip it in this case. | ||
| if (length($vars->{readable_bug_status_json}) > 65536) { | ||
| delete $vars->{readable_bug_status_json}; | ||
| } | ||
| # bugzilla-readable-status |
There was a problem hiding this comment.
has_extension() looks useful. This appears to be added by PR #66 so it needs to land first.
| status => $bug->bug_status, | ||
| flags => \@flags, | ||
| target_milestone => $bug->target_milestone, | ||
| Bugzilla->has_extension('TrackingFlags') |
There was a problem hiding this comment.
CoPilot's review comment about has_extension cited the wrong block of code. It should be this one.
|
Was mentioned in one of the review comments, but this has a hard dependency on PR #66 and it needs to land first before any further review is done on this PR. |
Details
This was originally PR #74 but that got backed out. This is resurrecting the patch from that PR.
Additional info
PR Review Context for Copilot
Goal
Enable readable status output in BugModal for all products (not just Mozilla-specific ones), so the Tracking section subtitle provides a useful plain-English summary everywhere.
Background and History
A very similar change landed on harmony before and was backed out just before the 5.9.1 release because it broke the tree.
Reported historical breakage cause:
Some dependencies may have changed since then, but this patch should still be reviewed with that prior failure mode in mind.
Important Runtime Assumptions
TrackingFlags ships with harmony, but admins may disable it.
This patch should therefore work in both scenarios:
Intended Behavior Change
Remove Mozilla-only gating around readable status generation and make readable status available generally in BugModal.
The Tracking subtitle should still have a sensible fallback when readable status data cannot be generated.
What Reviewers Should Check Closely
Reviewer Intent
Please review this patch as an always-on readable-status change with emphasis on: