Closed
Bug 994896
Opened 11 years ago
Closed 6 years ago
Add the ability to get comments, attachments, and history using Bug.get
Categories
(bugzilla.mozilla.org :: API, enhancement, P2)
Tracking
()
RESOLVED
FIXED
People
(Reporter: dkl, Assigned: kohei)
References
Details
Attachments
(1 file, 3 obsolete files)
Once bug 540818 is done we will have the ability to add fields to the WebService calls that are not always returned by default. This will allow people to get comments and attachments using Bug.get if they explicitly ask for them. This bug is about adding that ability and i will post a patch for review soon.
dkl
Reporter | ||
Updated•11 years ago
|
Assignee: webservice → dkl
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86_64 → All
Target Milestone: --- → Bugzilla 5.0
Assignee | ||
Updated•11 years ago
|
Severity: normal → enhancement
Reporter | ||
Comment 1•11 years ago
|
||
Adding this patch here for now. Once 540818 is committed, I may have to update this patch a little to allow it to apply cleanly.
dkl
Attachment #8405010 -
Flags: review?
Assignee | ||
Comment 2•11 years ago
|
||
It would be nice if we can get the history also.
Reporter | ||
Updated•11 years ago
|
Summary: Add the ability to get comments and attachments using Bug.get → Add the ability to get comments, attachments, and history using Bug.get
Reporter | ||
Comment 3•11 years ago
|
||
Attachment #8405010 -
Attachment is obsolete: true
Attachment #8405010 -
Flags: review?
Attachment #8405473 -
Flags: review?(gerv)
Comment 4•11 years ago
|
||
Comment on attachment 8405473 [details] [diff] [review]
994896_1.patch
Review of attachment 8405473 [details] [diff] [review]:
-----------------------------------------------------------------
::: Bugzilla/WebService/Bug.pm
@@ +1481,5 @@
> + changes => []
> + };
> +
> + my %api_name = reverse %{ Bugzilla::Bug::FIELD_MAP() };
> + $api_name{'bug_group'} = 'groups';
Can we have a comment to explain this line? Why is this not in FIELD_MAP() itself?
@@ +1492,5 @@
> + }
> + $change->{removed} = $self->type('string', $change->{removed});
> + $change->{added} = $self->type('string', $change->{added});
> + $change->{field_name} = $self->type('string', $api_field);
> + delete $change->{fieldname};
It looked to me here like you set a value then immediately delete it, but then I noticed the lack of an _. This is probably worth a comment, though. Or do the delete further up, where you extract the value. That would make it clearer.
@@ +2748,5 @@
> +=over
> +
> +=item C<id>
> +
> +C<int> The id of the flag.
By this, you actually mean the id of this particular instance of this flag? I.e. a unique-across-Bugzilla ID for this particular flag set this particular time on this particular bug?
@@ +2756,5 @@
> +C<string> The name of the flag.
> +
> +=item C<type_id>
> +
> +C<int> The type id of the flag.
Is it worth noting how one can use the API to convert this into a useful flag name?
@@ +2806,5 @@
> +
> +=item count
> +
> +C<int> The number of the comment local to the bug. The Description is 0,
> +comments start with 1.
Add a note that the numbers may not be consecutive if you are unable to see some comments? (I hope that's true, otherwise the comment numbers will be wrong!)
@@ +2825,5 @@
> +
> +C<dateTime> This is exactly same as the C<time> key. Use this field instead of
> +C<time> for consistency with other methods including L</get> and L</attachments>.
> +For compatibility, C<time> is still usable. However, please note that C<time>
> +may be deprecated and removed in a future release.
We should set specific deprecation version numbers rather than this vagueness, because otherwise we will either not remove them for fear of upsetting someone, or we will remove them and someone will be upset with us.
If API breaking changes are only on major releases, then you should mention Bugzilla 6.0 here.
Comment 5•11 years ago
|
||
There's an inconsistency in your treatment of attach_id. On comments, if there's no attach_id, the field is null. On history entries, if there's no attach_id, the field is not present. I suggest you adopt the same strategy for both; I'd recommend the second one.
+For compatibility, C<time> is still usable. However, please note that C<time>
+may be deprecated and removed in a future release.
Can we get rid of it immediately on the REST API (just so we don't introduce already-deprecated things) or is that complicated?
Also, we should specifically document that you can get attachment metadata without the data (which can be big) using:
http://bugzilla/rest/bug/1?include_fields=attachments&exclude_fields=attachments.data
This is commonly-requested thing, so I think it makes sense to say something specifically.
Gerv
Updated•11 years ago
|
Attachment #8405473 -
Flags: review?(gerv) → review-
Updated•10 years ago
|
Target Milestone: Bugzilla 5.0 → ---
Reporter | ||
Comment 8•9 years ago
|
||
Attachment #8405473 -
Attachment is obsolete: true
Attachment #8776056 -
Flags: review?(dylan)
Comment 9•9 years ago
|
||
I suppose to review this properly, we need to green-up bugzilla-master: https://treeherder.mozilla.org/#/jobs?repo=bugzilla-master
Comment 10•8 years ago
|
||
I actually got this mostly reviewed today. I know this will make kohei happy :-)
Assignee | ||
Comment 11•8 years ago
|
||
\o/
Comment 12•8 years ago
|
||
Awesome.
What release is this expected to land in?
Comment 13•8 years ago
|
||
(In reply to Leif Gruenwoldt from comment #12)
> Awesome.
>
> What release is this expected to land in?
In 6.0
Target Milestone: --- → Bugzilla 6.0
Updated•8 years ago
|
Attachment #8776056 -
Flags: review?(dylan) → review?(dylan)
Assignee | ||
Comment 14•6 years ago
|
||
Is the patch ready? If so, I want to see a PR for BMO so it can be included later in Bugzilla 6.0 :) If :dkl doesn’t have time now, I can take over.
Component: WebService → API
Flags: needinfo?(dkl)
Priority: -- → P1
Product: Bugzilla → bugzilla.mozilla.org
QA Contact: default-qa
Target Milestone: Bugzilla 6.0 → ---
Version: 4.5.2 → Production
Assignee | ||
Updated•6 years ago
|
Priority: P1 → P2
Assignee | ||
Comment 15•6 years ago
|
||
Looks like the patch has to be updated anyway for the current, unversioned BMO code. Taking.
Assignee: dkl → kohei.yoshino
Flags: needinfo?(dkl)
Assignee | ||
Comment 16•6 years ago
|
||
Attachment #8776056 -
Attachment is obsolete: true
Attachment #8776056 -
Flags: review?(dylan)
Assignee | ||
Comment 17•6 years ago
|
||
Merged to master.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•