Skip to content

Bug 1429344 - Review requests in requests dropdown should link to MozReview or GitHub instead of Bugzilla details page#372

Merged
dylanwh merged 1 commit intomozilla-bteam:masterfrom
kyoshino:bug-1429344-external-requests
Mar 5, 2018
Merged

Bug 1429344 - Review requests in requests dropdown should link to MozReview or GitHub instead of Bugzilla details page#372
dylanwh merged 1 commit intomozilla-bteam:masterfrom
kyoshino:bug-1429344-external-requests

Conversation

@kyoshino
Copy link
Copy Markdown
Collaborator

@kyoshino kyoshino commented Jan 14, 2018

Fix Bug 1429344 - Review requests in requests dropdown should link to MozReview or GitHub instead of Bugzilla details page

Description

  • Retrieve attachments.mimetype from DB as attach_mimetype
  • Retrieve attachments.ispatch from DB as attach_ispatch instead of ispatch for consistency (this field is not used in other templates so the change should be safe)
  • Treat MozReview and Phabricator requests as patches
  • Support GitHub pull requests and Google Docs as well
  • Link directly to those external sites
  • For regular patches, link to the Splinter Review page if available

@kyoshino kyoshino self-assigned this Jan 14, 2018
Copy link
Copy Markdown
Contributor

@dylanwh dylanwh left a comment

Choose a reason for hiding this comment

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

requests.cgi is pretty nightmareish code. I can't even begin to understand why it is using positional rows and not using the type of $dbi->select that uses column names.

I'll write a new backend that is also performant.

@kyoshino
Copy link
Copy Markdown
Collaborator Author

kyoshino commented Feb 14, 2018

Ah, one thing I've noticed is that the splinter review URL page.cgi?id=splinter.html is customizable by admin. To avoid any potential breakage in the future or upstream, it has to be exposed as a string in the template like the extension list also added in this PR.

@kyoshino kyoshino added the WIP label Feb 14, 2018
…Review or GitHub instead of Bugzilla details page
@kyoshino kyoshino removed the WIP label Feb 28, 2018
@kyoshino
Copy link
Copy Markdown
Collaborator Author

Instead of an extension list, exposed the splinter_base param in the template so review links always work.

Comment thread request.cgi
requestees.login_name, flags.modification_date, attachments.ispatch
cclist_accessible, bugs.reporter, bugs.reporter_accessible,
bugs.assigned_to, attachments.ispatch');
bugs.assigned_to, attachments.mimetype');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

doesn't this need to also group by ispatch?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There's already ispatch 2 lines above. Looks like it's a duplicate, so just changing it to mimetype.

@dylanwh dylanwh merged commit cfb84c6 into mozilla-bteam:master Mar 5, 2018
@kyoshino kyoshino deleted the bug-1429344-external-requests branch March 5, 2018 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants