Bug 2065171 - Migrate BugUserLastVisit REST resource to native Mojo API by Xzzz · Pull Request #2743 · mozilla/bmo

Xzzz · 2026-09-14T14:25:06Z

Summary

Ports Bugzilla::WebService::BugUserLastVisit's get/update methods into a native Bugzilla::API::V1::BugUserLastVisit Mojo controller, mirroring the pattern already used for Classification/Component/Teams/Reminders/Configuration/Bugzilla (system info).

This is a child bug of 2057358, see there for details.

Changes

Add Bugzilla/API/V1/BugUserLastVisit.pm: GET/POST /rest/bug_user_last_visit and /rest/bug_user_last_visit/<id> (login required), same JSON response shape as the legacy endpoints
Delete Bugzilla/WebService/BugUserLastVisit.pm and Bugzilla/WebService/Server/REST/Resources/BugUserLastVisit.pm
Remove the BugUserLastVisit entry from WS_DISPATCH in Bugzilla/WebService/Constants.pm and drop the corresponding use line in Bugzilla/WebService/Server/REST.pm

Breaking change: removing the WS_DISPATCH entry also removes BugUserLastVisit.get/update from JSON-RPC and XML-RPC, not just the legacy REST dispatcher, since all three share that table. Native Mojo routes only serve REST. This matches the same tradeoff already made in the Classification and Bugzilla (system-info) migrations earlier in this series.

Test plan

GET /rest/bug_user_last_visit (anonymous => login_required, authenticated => list of last-visited bugs)
GET /rest/bug_user_last_visit/<id>
GET /rest/bug_user_last_visit?ids=<id>&ids=<id>
POST /rest/bug_user_last_visit/<id>
POST /rest/bug_user_last_visit with {"ids":[...]} body
OPTIONS on both routes returns Allow: GET, POST
Confirm response shape (bare JSON array, last_visit_ts with trailing Z) matches the legacy endpoint

References

Bugzilla link: 2065171
Part of 2057358

dklawren · 2026-09-14T17:04:16Z

+    return [$id];
+  }
+
+  if ($self->req->method eq 'POST') {


ids on POST is now only accepted from a JSON body.

The legacy layer took it from the query string too — Bugzilla::WebService::Server::REST::_retrieve_json_params deliberately merged query-string params for non-GET requests ("Allow parameters in the query string if request was non-GET"), and CGI parsed urlencoded bodies. So both of these used to work and now fail:

POST /rest/bug_user_last_visit?ids=123 → param_required (empty body decodes to {})

POST /rest/bug_user_last_visit with Content-Type: application/x-www-form-urlencoded and body ids=123&ids=456 → decode_json throws → rest_malformed_json

Suggest falling back to $self->every_param('ids') when the body is absent or isn't JSON.

Fixed in "Bug 2065171 - Merge query-string and JSON body params for ids/include_fields"

dklawren · 2026-09-14T17:04:16Z

+    try { $params = decode_json($self->req->body || '{}'); }
+    catch { $error = 'rest_malformed_json'; };
+    return (undef, $error) if $error;
+    my $ids = $params->{ids} // [];


Input type validation was dropped along with validate(@_, 'ids').

ref $ids ? $ids : [$ids] passes any reference straight through, so {"ids":{"a":1}} returns a hashref and the @$ids check in update dies with "Not an ARRAY reference" → HTTP 500 / unknown-fatal. Legacy returned a clean invalid_params user error.

Suggest ref $ids eq 'ARRAY' with a user error otherwise.

Fixed following your suggestion: ref $ids eq 'ARRAY' ? $ids : [$ids]; and added user_error "ids must be an array" if $ids ne 'ARRAY'.
Committed in "Bug 2065171 - Reject non-array ids with invalid_params"

dklawren · 2026-09-14T17:04:16Z

    'User'             => 'Bugzilla::WebService::User',
    'Product'          => 'Bugzilla::WebService::Product',
    'Group'            => 'Bugzilla::WebService::Group',
-    'BugUserLastVisit' => 'Bugzilla::WebService::BugUserLastVisit',


Dropping this entry removes the methods from JSON-RPC and XML-RPC as well, not just from the legacy REST layer — anything calling jsonrpc.cgi with method=BugUserLastVisit.get/update will start getting an unknown-method error.

This matches the pattern of the earlier migrations in the series, so probably intentional, but it's an undocumented breaking change that seems worth calling out in the PR description / API docs.

Correct, and yes intentional: same tradeoff as the Classification/Bugzilla(system-info) migrations before this one => dropping the WS_DISPATCH entry removes BugUserLastVisit.get/update from JSON-RPC and XML-RPC as well as from the legacy REST dispatcher, since all three share that table. Native Mojo routes only serve REST.

I hadn't called this out explicitly in the description => added a note to it now
Happy to raise that on 2057358 too, as it'd apply to every resource in the series, not just this one.

And yes, I totally agree, API documentation needs an update too, and even a big one ;)

…_fields

…er body

Xzzz · 2026-09-14T20:08:21Z

Reading API doc, I realized that docs/en/rst/api/core/v1/general.rst and the legacy REST dispatcher (Bugzilla::WebService::Server::REST::_retrieve_json_params) both establish that query-string params override the body for non-GET requests.
My _request_params fix does the opposite: it merges {%$params,%$body_params}, so JSON body wins on a key collision.

=> Fixed in "Bug 2065171 - Fix ids/include_fields precedence: query string wins over body"

`_request_params->{ids} // []` made a missing ids param filter to nothing instead of returning every visited bug, since an empty arrayref is truthy. Legacy left $ids undef when the param is absent, skipping filter entirely. Return undef in that case matches legacy behavior. An empty array still filters to nothing.

dklawren · 2026-09-15T21:10:45Z

+  if (my $id = $self->param('id')) {
+    return [$id];
+  }
+
+  my $ids = $self->_request_params->{ids};
+  return undef unless defined $ids;
+  return (undef, 'invalid_params', {type_error => 'ids must be an array'})
+    if ref $ids && ref $ids ne 'ARRAY';
+  return ref $ids eq 'ARRAY' ? $ids : [$ids];


two precedence differences from the legacy layer here

path id short-circuits ids, but legacy merged the request body over path params (_retrieve_json_params applies %$extra_params last), so POST /rest/bug_user_last_visit/123 with body {"ids":[456]} used to update 456 and now updates 123. query-string precedence is unchanged, just the body

if (my $id = $self->param('id')) is a truthiness test, so /bug_user_last_visit/0 falls through to the no-ids branch and get returns the whole last-visited list instead of an empty one. $self->param also falls back to request params, so ?id=5 is now treated as a filter where legacy only looked at ids

checking defined $self->stash('id') instead would fix both halves of 2

Fixed:

ids precedence now matches legacy:
-> for POST, a body/query-string ids overrides the path id (falls back to the path id only when the request has no ids at all)
-> for GET, no change (the path id still wins)

Switched from $self->param('id') to defined $self->stash('id'), which fixes both id=0 (was falsy, fell through to "no ids") and the stray ?id=5 query param being misread as a path id.

dklawren · 2026-09-15T21:10:46Z

+  return ref $ids eq 'ARRAY' ? $ids : [$ids];
+}
+
+sub _request_params {


this param-merging layer is hand-rolled and has no automated coverage

the other native migrations in this series each have a qa test (rest_classification.t, rest_components.t, rest_reminders.t) and 65642f5 updated rest_bugzilla.t, so qa/t/rest_bug_user_last_visit.t would fit the convention

worth covering the cases already in the PR test plan plus the query-string-vs-json-body precedence, since three frontend call sites depend on this endpoint (bug_modal.js, MyDashboard/query.js, show-header.html.tmpl) and they post a json body with no Content-Type, which is exactly the path decode_json here handles

Added qa/t/rest_bug_user_last_visit.t, matching the rest_classification.t, rest_components.t, and rest_reminders.t convention.
It covers the PR test plan cases plus the query-string-vs-json-body/path precedence (including the no-Content-Type case the three frontend callers rely on).

=> Fixed in "Bug 2065171 - Add qa/t/rest_bug_user_last_visit.t"

_request_params duplicated the query-string/JSON-body merge logic. Now call a single shared Bugzilla::WebService::Util::merge_request_params helper, so it's a one-place change to drop later if query-string-on-POST support is ever removed.

Xzzz · 2026-09-17T12:56:53Z

Pushed a follow-up commit: _request_params now calls a shared Bugzilla::WebService::Util::merge_request_params helper instead of doing the query-string/JSON-body merge inline. Extracted a single reusable helper rather than keeping two copies.
The include_fields/exclude_fields split stays local here since it's specific to this resource.
=> No behavior change

_ids_from_request short-circuited to the path id whenever present, never consulting the merged query-string/body params. Legacy's _retrieve_json_params merges non-GET request-body/query params in *after* the path-derived params, so those win for POST. For GET, the path id still wins (legacy's override step only ran for non-GET requests), so that precedence is unchanged. Also switch from $self->param('id') (a truthiness check that also falls back to a same-named query param) to $self->stash('id') (defined check, route-placeholder only). This fixes two more bugs: - /bug_user_last_visit/0 was falling through to the no-ids branch since "0" is falsy - a stray ?id=5 query parameter (distinct from ids) was being treated as if it were a path id

Covers: anonymous access requiring login, OPTIONS, POST via path id, POST via a JSON ids body, POST with a JSON body posted with no Content-Type header, GET via path id vs query-string ids precedence, GET via query-string ids, and GET with no ids returning every visited bug

Bug 2065171 - Migrate BugUserLastVisit REST resource to native Mojo API

1bbf944

dklawren requested changes Sep 14, 2026

View reviewed changes

Xzzz added 4 commits September 14, 2026 19:49

Bug 2065171 - Merge query-string and JSON body params for ids/include…

dc33218

…_fields

Bug 2065171 - Reject non-array ids with invalid_params

f2621a2

Bug 2065171 - Use resolved bug id, restrict :id route to digits

aeaff1f

Bug 2065171 - Fix ids/include_fields precedence: query string wins ov…

89aabbf

…er body

dklawren requested changes Sep 14, 2026

View reviewed changes

Comment thread Bugzilla/API/V1/BugUserLastVisit.pm Outdated

dklawren requested changes Sep 16, 2026

View reviewed changes

Xzzz mentioned this pull request Sep 17, 2026

Bug 2065173 - Migrate Group REST resource to native Mojo API #2745

Open

Xzzz added 2 commits September 17, 2026 16:02

Xzzz requested a review from dklawren September 17, 2026 16:03

Conversation

Xzzz commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test plan

References

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xzzz Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xzzz commented Sep 14, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xzzz commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Xzzz commented Sep 14, 2026 •

edited

Loading

Xzzz Sep 14, 2026 •

edited

Loading