Add "attachment is patch" field to bug
Categories
(bugzilla.mozilla.org :: Search, enhancement)
Tracking
()
People
(Reporter: emceeaich, Assigned: dylan)
Details
Attachments
(2 files)
It's possible to filter bug searches on those bug with a patch (ex. https://mzl.la/2Fyjgzj) but not to indicate in a bug list if a bug has a patch.
Make attachment is patch available in search results in all formats.
|
||
Comment 1•5 years ago
|
||
We can do this, but…
- Emma’s search queries include “Attachment is patch: true” but it’s actually not working. For example, Bug 1518058 comes first but it doesn’t have any patch.
- As you know we have migrated to Phablicator, and some products including BMO are using GitHub. It means the number of traditional patches is decreasing. Should we count Phablicator and GitHub PRs as patches?
|
|
||
Updated•5 years ago
|
|
Reporter | |
Comment 2•5 years ago
|
||
Yes, Phab and GitHub PRs are the patches of interest.
Phab patches are handled through the plugin, if I recall right.
Should GitHub PRs follow a similar path instead of creating text attachment with the link to the PR?
|
|
||
Comment 3•5 years ago
|
||
Phab, GitHub and legacy MozReview requests are all handled in the BMO extension, and those are technically links written in a text attachment that can be distinguished with the text/x-phabricator-request, text/x-github-pull-request or text/x-review-board-request MIME type. On the modal bug page, these attachments are all styled in a yellow background colour just like a regular patch, except for Phab requests now hidden in favour of the separate section. I’ll make the search results consistent by treating all of them as patches.
|
||
Comment 4•5 years ago
|
||
I don't know if it's related, but here:
https://bugzilla.mozilla.org/buglist.cgi?bug_id=1539069&bug_id_type=anyexact&f1=attachments.mimetype&list_id=14640442&o1=anywordssubstr&resolution=---&classification=Client%20Software&classification=Developer%20Infrastructure&classification=Components&classification=Server%20Software&classification=Other&query_format=advanced&v1=text%2Fx-phabricator-request%2Ctext%2Fx-github-pull-request%2Ctext%2Fx-review-board-request
there is nothing returned (note that I put a bug_id)
And attachment shows:
https://bugzilla.mozilla.org/rest/bug/1539069/attachment
that text/x-phabricator-request is the mime type for the 2 attachments.
|
|
||
Comment 5•5 years ago
|
||
Calixte’s query contains resolution=--- but the bug is FIXED, so it’s unrelated. It’s rather Bug 1533610.
|
|
||
Comment 6•5 years ago
|
||
My bad...
|
|
||
Comment 7•5 years ago
|
||
(In reply to Kohei Yoshino [:kohei] (Bugzilla UX) (FxSiteCompat) from comment #1)
Emma’s search queries include “Attachment is patch: true” but it’s actually not working. For example, Bug 1518058 comes first but it doesn’t have any patch.
This problem is due to the query and basically the bad Custom Search UX. “Attachment is patch: true” doesn’t work but “Attachment is patch: 1” does work! Because… it’s… Perl. Activity Stream components have only 28 bugs with a regular patch, and Emma’s query returns no bugs if corrected.
|
Assignee | |
Comment 8•5 years ago
|
||
(In reply to Kohei Yoshino [:kohei] (Bugzilla UX) (FxSiteCompat) from comment #7)
(In reply to Kohei Yoshino [:kohei] (Bugzilla UX) (FxSiteCompat) from comment #1)
Emma’s search queries include “Attachment is patch: true” but it’s actually not working. For example, Bug 1518058 comes first but it doesn’t have any patch.
This problem is due to the query and basically the bad Custom Search UX. “Attachment is patch: true” doesn’t work but “Attachment is patch: 1” does work! Because… it’s… Perl. Activity Stream components have only 28 bugs with a regular patch, and Emma’s query returns no bugs if corrected.
This is also mysql's fault. It has no real boolean type, TRUE and FALSE are just 1 and 0 there
|
|
Assignee | |
Comment 9•5 years ago
|
||
The sql this is running is:
SELECT DISTINCT bug_id FROM attachments WHERE ispatch = 'true' AND isprivate = 0
|
|
Reporter | |
Comment 10•5 years ago
|
||
This looks like another thing that could use some syntactic sugar.
|
|
||
Comment 11•5 years ago
|
||
Yeah I’ll fix it. The internal code can convert true to 1 and false to 0.
|
|
Assignee | |
Comment 12•5 years ago
|
||
I'm on it, OPERATOR_FIELD_OVERRIDE
|
|
||
Comment 13•5 years ago
|
||
That one.
|
|
||
Comment 14•5 years ago
|
||
Er, SPECIAL_PARSING I guess.
|
|
Assignee | |
Comment 15•5 years ago
|
||
Doing this in a way that doesn't result in possible SQL injection is a bit tricky, so I'll take it.
|
|
||
Comment 16•5 years ago
|
||
Well that’s another bug…
|
|
||
Comment 17•5 years ago
|
||
|
|
||
Comment 18•5 years ago
|
||
Merged to master.
Description
•