Skip to content

Bug 2081207 - Add a Crash Data URL field for the graph and table - #2781

Open
calixteman wants to merge 2 commits into
mozilla:masterfrom
calixteman:crash-data-url
Open

calixteman wants to merge 2 commits into
mozilla:masterfrom
calixteman:crash-data-url

Conversation

@calixteman

Copy link
Copy Markdown
Contributor

Use cf_crash_data_url for Socorro filters in both widgets, replacing bug_file_loc as the table's filter source. Require the Socorro search URL prefix and a nonempty crash signature. Use MEDIUMTEXT for long URLs.

Use cf_crash_data_url for Socorro filters in both widgets, replacing
bug_file_loc as the table's filter source. Require the Socorro search
URL prefix and a nonempty crash signature. Use MEDIUMTEXT for long URLs.
Copilot AI balanced review requested due to automatic review settings October 14, 2026 07:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The database, validation, rendering, and external Socorro query changes warrant final human integration review.

0 open findings

What changed in this PR

Adds a validated Crash Data URL field and uses its Socorro filters in crash graph and table widgets.

Changes:

  • Creates and validates the MEDIUMTEXT custom field.
  • Renders the field alongside crash signatures.
  • Forwards applicable URL filters to both crash widgets and adds regression tests.
File Description
qa/​t/​rest_bug_crash_data_url.t Tests validation, persistence, rendering, and graph filtering.
extensions/​BugModal/​template/​en/​default/​bug_modal/​edit.html.tmpl Defers URL rendering to the crash module.
extensions/​BugModal/​template/​en/​default/​bug_modal/​create.html.tmpl Defers URL rendering on bug creation.
extensions/​BMO/​web/​socorro-lens/​main.js Applies additional Socorro filters to graph queries.
extensions/​BMO/​web/​js/​firefox-crash-table.js Sources table filters from the new field.
extensions/​BMO/​template/​en/​default/​hook/​global/​user-error-errors.html.tmpl Adds validation errors.
extensions/​BMO/​template/​en/​default/​hook/​bug_modal/​edit-custom_field-cf_crash_signature.html.tmpl Renders the URL and filtered graph.
extensions/​BMO/​lib/​Data.pm Matches URL-field visibility to crash signatures.
extensions/​BMO/​Extension.pm Creates and validates the field.
Bugzilla/​Template.pm Passes the URL into Socorro Lens.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread extensions/BMO/Extension.pm Outdated

# Allow unrelated edits even if an existing URL lacks a signature.
# The accessor only exists once checksetup.pl has created the field.
if ($new_bug->can('cf_crash_data_url')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check runs during update() so a mass edit can partly save before it throws, could it move to object_end_of_set_all so every bug is checked before any write

if (!sayNo.has(p[0])) {
extraSocorroArgs.push(p[0] + "=" + encodeURIComponent(p[1]));
}
const crashDataUrl = document.querySelector("#field-value-cf_crash_data_url [data-crash-data-url]")?.dataset.crashDataUrl;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

existing bugs with a crash-stats search url in bug_file_loc lose their table filters, could we migrate those values or fall back to bug_file_loc when cf_crash_data_url is empty

const extraSocorroArgs = []
const baseUrl = "https://crash-stats.mozilla.org/search/";
const baseUrl = "https://crash-stats.mozilla.org/search/?";
const sayNo = new Set(["_columns", "_facets", "_facets_size", "_sort", "_results_number", "date", "channel", "product", "version", "build_id"]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

table keeps the url's signature and some _ params while the graph drops them, worth using the same ignore list in both

…ash_data_url

The new code is deployed before checksetup.pl runs, and until then
Bugzilla::Bug has no cf_crash_data_url accessor, so the signature check
in object_start_of_update made every bug update die.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants