Bug 2061831 - Add additional information to bugzilla comments in REST API if comment edited before - #2697
Open
dklawren wants to merge 2 commits into
Open
Bug 2061831 - Add additional information to bugzilla comments in REST API if comment edited before#2697dklawren wants to merge 2 commits into
dklawren wants to merge 2 commits into
Conversation
… API if comment edited before
dklawren
requested review from
Xzzz and
cgsheeh
and
a balanced review from Copilot
August 7, 2026 20:00
There was a problem hiding this comment.
Pull request overview
Adds REST comment edit metadata with permission-aware visibility.
Changes:
- Preloads and serializes edit counts and timestamps.
- Documents the new fields.
- Adds REST integration tests for comment endpoints.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
Bugzilla/WebService/Bug.pm |
Adds metadata loading and serialization. |
docs/en/rst/api/core/v1/comment.rst |
Documents new response fields. |
qa/t/rest_bug_comment_edits.t |
Tests visibility and edit metadata. |
Suppressed comments (1)
Bugzilla/WebService/Bug.pm:1650
_bug_to_hashis invoked once per result inget,search, and duplicate queries, so preloading here issues an additional aggregation query for every returned bug whenever comments are requested. This defeats the stated request-level batching and can substantially increase query count for bulk results; preload comment metadata across the result set before serializing individual bugs.
$self->_preload_comment_edit_info($comments);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+123
to
+124
| # The bug-level route uses a separate code path, so check it too. | ||
| $t->get_ok($url . 'rest/bug/public_bug/comment' => api_headers($editbugs_key)) |
| my $comments | ||
| = $bug->comments({order => 'oldest_to_newest', after => $params->{new_since} | ||
| }); | ||
| $self->_preload_comment_edit_info($comments); |
Xzzz
approved these changes
Aug 10, 2026
Xzzz
left a comment
Xzzz
left a comment
Collaborator
There was a problem hiding this comment.
No correctness, security, or leak issues found. Visibility gating, hidden-revision handling, and null-handling all check out against the actual schema and framework internals.
Checks coverage hits all three visibility tiers across all three code paths (comment-id route, bug-comment route, Bug.get).
This looks good to me, I'd approve this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed (3 files, +116 lines)
No schema change, no checksetup.pl run needed. Your four decisions are all encoded: edit_count/last_change_time, gated on can_edit_comments, hidden revisions counted for admins only, wired directly into core.
★ Insight ─────────────────────────────────────
─────────────────────────────────────────────────