Skip to content

Commit a86569c

Browse files
Bug 2011002 - Add feedback feature for Review Helper bot comments (#72)
1 parent b57d964 commit a86569c

File tree

11 files changed

+424
-43
lines changed

11 files changed

+424
-43
lines changed
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
3+
/**
4+
* This file is automatically generated. Use 'bin/celerity map' to rebuild it.
5+
*
6+
* @generated
7+
*/
8+
return array(
9+
'names' => array(
10+
'rsrc/css/reviewhelper/reviewhelper-feedback.css' => '0f9cbe2d',
11+
'rsrc/js/reviewhelper/behavior-reviewhelper-feedback.js' => '08402370',
12+
),
13+
'symbols' => array(
14+
'javelin-behavior-reviewhelper-feedback' => '08402370',
15+
'reviewhelper-feedback-css' => '0f9cbe2d',
16+
),
17+
'requires' => array(),
18+
'packages' => array(),
19+
);

moz-extensions/src/__phutil_library_map__.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
'BMOExternalAccountSearchConduitAPIMethod' => 'conduit/BMOExternalAccountSearchConduitAPIMethod.php',
1414
'BugStore' => 'email/adapter/BugStore.php',
1515
'BugzillaAccountSearchConduitAPIMethod' => 'conduit/BugzillaAccountSearchConduitAPIMethod.php',
16+
'CelerityMozExtensionsResources' => 'celerity/CelerityMozExtensionsResources.php',
1617
'CreatePolicyConduitAPIMethod' => 'conduit/CreatePolicyConduitAPIMethod.php',
1718
'DifferentialBugzillaBugIDCommitMessageField' => 'differential/customfield/DifferentialBugzillaBugIDCommitMessageField.php',
1819
'DifferentialBugzillaBugIDCustomFieldTestCase' => 'differential/customfield/__tests__/DifferentialBugzillaIdCustomFieldTestCase.php',
@@ -76,6 +77,8 @@
7677
'PhabricatorReviewHelperApplication' => 'reviewhelper/application/PhabricatorReviewHelperApplication.php',
7778
'PhabricatorReviewHelperConfigOptions' => 'reviewhelper/config/PhabricatorReviewHelperConfigOptions.php',
7879
'ReviewHelperEventListener' => 'reviewhelper/events/ReviewHelperEventListener.php',
80+
'ReviewHelperController' => 'reviewhelper/controller/ReviewHelperController.php',
81+
'ReviewHelperFeedbackController' => 'reviewhelper/controller/ReviewHelperFeedbackController.php',
7982
'ReviewHelperRequestController' => 'reviewhelper/controller/ReviewHelperRequestController.php',
8083
'ReviewHelperServiceException' => 'reviewhelper/exception/ReviewHelperServiceException.php',
8184
'PhabricatorReviewer' => 'email/adapter/PhabricatorReviewer.php',
@@ -125,6 +128,7 @@
125128
'xmap' => array(
126129
'BMOExternalAccountSearchConduitAPIMethod' => 'UserConduitAPIMethod',
127130
'BugzillaAccountSearchConduitAPIMethod' => 'UserConduitAPIMethod',
131+
'CelerityMozExtensionsResources' => 'CelerityResourcesOnDisk',
128132
'CreatePolicyConduitAPIMethod' => 'ConduitAPIMethod',
129133
'DifferentialBugzillaBugIDCommitMessageField' => 'DifferentialCommitMessageCustomField',
130134
'DifferentialBugzillaBugIDCustomFieldTestCase' => 'PhabricatorTestCase',
@@ -168,7 +172,9 @@
168172
'PhabricatorReviewHelperApplication' => 'PhabricatorApplication',
169173
'PhabricatorReviewHelperConfigOptions' => 'PhabricatorApplicationConfigOptions',
170174
'ReviewHelperEventListener' => 'PhabricatorEventListener',
171-
'ReviewHelperRequestController' => 'PhabricatorController',
175+
'ReviewHelperController' => 'PhabricatorController',
176+
'ReviewHelperFeedbackController' => 'ReviewHelperController',
177+
'ReviewHelperRequestController' => 'ReviewHelperController',
172178
'ReviewHelperServiceException' => 'Exception',
173179
'PhutilBMOAuthAdapter' => 'PhutilOAuthAuthAdapter',
174180
'PolicyQueryConduitAPIMethod' => 'ConduitAPIMethod',
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
// This Source Code Form is subject to the terms of the Mozilla Public
3+
// License, v. 2.0. If a copy of the MPL was not distributed with this
4+
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
5+
6+
/**
7+
* Celerity resource source for moz-extensions.
8+
* This allows moz-extensions to provide its own JavaScript and CSS resources.
9+
*/
10+
final class CelerityMozExtensionsResources extends CelerityResourcesOnDisk {
11+
12+
public function getName() {
13+
return 'moz-extensions';
14+
}
15+
16+
public function getPathToResources() {
17+
return $this->getMozExtensionsPath('webroot/');
18+
}
19+
20+
public function getPathToMap() {
21+
return $this->getMozExtensionsPath('resources/celerity/map.php');
22+
}
23+
24+
private function getMozExtensionsPath($to_file = '') {
25+
$root = dirname(dirname(dirname(__FILE__)));
26+
return $root . '/' . $to_file;
27+
}
28+
}

moz-extensions/src/reviewhelper/application/PhabricatorReviewHelperApplication.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ public function getRoutes() {
2929
return array(
3030
'/reviewhelper/' => array(
3131
'request/(?P<revisionID>[1-9]\d*)/' => 'ReviewHelperRequestController',
32+
'feedback/' => 'ReviewHelperFeedbackController',
3233
),
3334
);
3435
}

moz-extensions/src/reviewhelper/config/PhabricatorReviewHelperConfigOptions.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,22 @@ public function getOptions() {
4444
30
4545
)
4646
->setDescription(pht('Request timeout in seconds for the AI review service.')),
47+
$this->newOption(
48+
'reviewhelper.bot-username',
49+
'string',
50+
''
51+
)
52+
->setDescription(pht('Username of the bot account that posts AI review comments. ' .
53+
'Feedback buttons will appear on inline comments from this user.')),
4754
);
4855
}
4956

5057
protected function didValidateOption(PhabricatorConfigOption $option, $value) {
5158
$key = $option->getKey();
5259
if ($key === 'reviewhelper.url') {
5360
$this->validateServiceURL($value);
61+
} elseif ($key === 'reviewhelper.bot-username') {
62+
$this->validateBotUsername($value);
5463
}
5564
}
5665

@@ -83,4 +92,24 @@ private function validateServiceURL($value) {
8392
);
8493
}
8594
}
95+
96+
private function validateBotUsername($value) {
97+
if ($value === '' || $value === null) {
98+
return;
99+
}
100+
101+
$user = id(new PhabricatorUser())->loadOneWhere(
102+
'userName = %s',
103+
$value
104+
);
105+
106+
if (!$user) {
107+
throw new PhabricatorConfigValidationException(
108+
pht(
109+
'The bot username "%s" does not match any existing Phabricator user.',
110+
$value
111+
)
112+
);
113+
}
114+
}
86115
}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
<?php
2+
// This Source Code Form is subject to the terms of the Mozilla Public
3+
// License, v. 2.0. If a copy of the MPL was not distributed with this
4+
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
5+
6+
abstract class ReviewHelperController extends PhabricatorController {
7+
8+
/**
9+
* Make a request to the Review Helper service.
10+
*
11+
* @param string $endpoint The endpoint path (e.g., '/request' or '/feedback')
12+
* @param array $payload The request payload
13+
* @return array The parsed JSON response
14+
* @throws ReviewHelperServiceException
15+
*/
16+
protected function makeServiceRequest($endpoint, array $payload) {
17+
$service_url = PhabricatorEnv::getEnvConfig('reviewhelper.url');
18+
$auth_key = PhabricatorEnv::getEnvConfig('reviewhelper.auth-key');
19+
$timeout = PhabricatorEnv::getEnvConfig('reviewhelper.timeout');
20+
21+
if (!$service_url) {
22+
throw new ReviewHelperServiceException(
23+
pht('Review Helper service is not configured.')
24+
);
25+
}
26+
27+
$url = rtrim($service_url, '/') . $endpoint;
28+
29+
$future = id(new HTTPSFuture($url))
30+
->setMethod('POST')
31+
->addHeader('Content-Type', 'application/json')
32+
->addHeader('Authorization', 'Bearer ' . $auth_key)
33+
->addHeader('User-Agent', 'Phabricator')
34+
->addHeader('Origin', rtrim(PhabricatorEnv::getAnyBaseURI(), '/'))
35+
->setTimeout($timeout)
36+
->setData(phutil_json_encode($payload));
37+
38+
try {
39+
list($status, $body) = $future->resolve();
40+
} catch (HTTPFutureResponseStatus $ex) {
41+
throw new ReviewHelperServiceException(
42+
pht(
43+
'Review Helper encountered a connection error (%s).',
44+
$ex->getStatusCode()
45+
)
46+
);
47+
}
48+
49+
if ($status->isTimeout()) {
50+
throw new ReviewHelperServiceException(
51+
pht('Review Helper request timed out. Please try again later.')
52+
);
53+
}
54+
55+
// On 422 errors, Review Helper will return user friendly error messages
56+
if ($status->getStatusCode() == 422) {
57+
try {
58+
$data = phutil_json_decode($body);
59+
if (is_array($data) && array_key_exists('error_message', $data)) {
60+
throw new ReviewHelperServiceException($data['error_message']);
61+
}
62+
} catch (PhutilJSONParserException $ex) {
63+
// Fall through to generic error below
64+
}
65+
}
66+
67+
if ($status->isError()) {
68+
throw new ReviewHelperServiceException(
69+
pht(
70+
'Review Helper returned an HTTP error (%s).',
71+
$status->getStatusCode()
72+
)
73+
);
74+
}
75+
76+
try {
77+
$data = phutil_json_decode($body);
78+
} catch (PhutilJSONParserException $ex) {
79+
throw new ReviewHelperServiceException(
80+
pht('Review Helper returned malformed JSON.')
81+
);
82+
}
83+
84+
if (!is_array($data)) {
85+
throw new ReviewHelperServiceException(
86+
pht('Review Helper returned an unexpected response.')
87+
);
88+
}
89+
90+
return $data;
91+
}
92+
}
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
// This Source Code Form is subject to the terms of the Mozilla Public
3+
// License, v. 2.0. If a copy of the MPL was not distributed with this
4+
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
5+
6+
final class ReviewHelperFeedbackController extends ReviewHelperController {
7+
8+
public function handleRequest(AphrontRequest $request) {
9+
$viewer = $request->getViewer();
10+
11+
if (!$request->isAjax() || !$request->hasCSRF()) {
12+
return new Aphront400Response();
13+
}
14+
15+
$comment_id = $request->getStr('commentID');
16+
$feedback_type = $request->getStr('feedbackType');
17+
18+
if (!in_array($feedback_type, array('up', 'down'))) {
19+
return id(new AphrontAjaxResponse())
20+
->setError(pht('Invalid feedback type.'));
21+
}
22+
23+
if (!$comment_id) {
24+
return id(new AphrontAjaxResponse())
25+
->setError(pht('Missing comment ID.'));
26+
}
27+
28+
try {
29+
$data = $this->submitFeedback($viewer, $comment_id, $feedback_type);
30+
} catch (ReviewHelperServiceException $ex) {
31+
MozLogger::log(
32+
'Review Helper feedback failed',
33+
'reviewhelper.feedback.error',
34+
array('Fields' => array(
35+
'comment_id' => $comment_id,
36+
'feedback_type' => $feedback_type,
37+
'user_phid' => $viewer->getPHID(),
38+
'error' => $ex->getMessage(),
39+
))
40+
);
41+
return id(new AphrontAjaxResponse())
42+
->setError($ex->getMessage());
43+
}
44+
45+
return id(new AphrontAjaxResponse())
46+
->setContent($data);
47+
}
48+
49+
private function submitFeedback(
50+
PhabricatorUser $viewer,
51+
$comment_id,
52+
$feedback_type
53+
) {
54+
$payload = array(
55+
'comment_id' => $comment_id,
56+
'feedback_type' => $feedback_type,
57+
'user_id' => $viewer->getID(),
58+
'user_name' => $viewer->getUsername(),
59+
);
60+
61+
return $this->makeServiceRequest('/feedback', $payload);
62+
}
63+
}

moz-extensions/src/reviewhelper/controller/ReviewHelperRequestController.php

Lines changed: 4 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// License, v. 2.0. If a copy of the MPL was not distributed with this
44
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
55

6-
final class ReviewHelperRequestController extends PhabricatorController {
6+
final class ReviewHelperRequestController extends ReviewHelperController {
77

88
public function handleRequest(AphrontRequest $request) {
99
$viewer = $request->getViewer();
@@ -65,55 +65,17 @@ private function requestReview(
6565
PhabricatorUser $viewer,
6666
DifferentialRevision $revision
6767
) {
68-
$service_url = PhabricatorEnv::getEnvConfig('reviewhelper.url');
69-
$auth_key = PhabricatorEnv::getEnvConfig('reviewhelper.auth-key');
70-
$timeout = PhabricatorEnv::getEnvConfig('reviewhelper.timeout');
71-
7268
$payload = array(
7369
'revision_id' => $revision->getID(),
7470
'diff_id' => max($revision->getDiffIDs()),
7571
'user_id' => $viewer->getID(),
7672
'user_name' => $viewer->getUsername(),
7773
);
7874

79-
$future = id(new HTTPSFuture($service_url))
80-
->setMethod('POST')
81-
->addHeader('Content-Type', 'application/json')
82-
->addHeader('Authorization', 'Bearer ' . $auth_key)
83-
->addHeader('User-Agent', 'Phabricator')
84-
->addHeader('Origin', rtrim(PhabricatorEnv::getAnyBaseURI(), '/'))
85-
->setTimeout($timeout)
86-
->setData(phutil_json_encode($payload));
87-
88-
try {
89-
list($status, $body) = $future->resolve();
90-
} catch (HTTPFutureResponseStatus $ex) {
91-
throw new ReviewHelperServiceException(
92-
pht('The AI review service encountered a connection or unexpected response error (%s).', $ex->getStatusCode())
93-
);
94-
}
95-
96-
if ($status->isTimeout()) {
97-
throw new ReviewHelperServiceException(
98-
pht('The AI review service request timed out. Please try again later.')
99-
);
100-
}
101-
102-
if ($status->isError()) {
103-
throw new ReviewHelperServiceException(
104-
pht('The AI review service returned an HTTP error response (%s).', $status->getStatusCode())
105-
);
106-
}
107-
108-
try {
109-
$data = phutil_json_decode($body);
110-
} catch (PhutilJSONParserException $ex) {
111-
throw new ReviewHelperServiceException(
112-
pht('The AI review service returned malformed JSON.')
113-
);
114-
}
75+
$data = $this->makeServiceRequest('/request', $payload);
11576

116-
if (!is_array($data) || !array_key_exists('message', $data)) {
77+
// Additional validation specific to this endpoint
78+
if (!array_key_exists('message', $data)) {
11779
throw new ReviewHelperServiceException(
11880
pht('The AI review service returned an unexpected or malformed response.')
11981
);

moz-extensions/src/reviewhelper/events/ReviewHelperEventListener.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,5 +62,21 @@ private function handleActionEvent($event) {
6262
array_unshift($actions, $action);
6363

6464
$event->setValue('actions', $actions);
65+
66+
// Initialize feedback feature only if bot username is configured
67+
$bot_username = PhabricatorEnv::getEnvConfig('reviewhelper.bot-username');
68+
if (!$bot_username) {
69+
return;
70+
}
71+
72+
require_celerity_resource('reviewhelper-feedback-css', 'moz-extensions');
73+
Javelin::initBehavior(
74+
'reviewhelper-feedback',
75+
array(
76+
'botUsername' => $bot_username,
77+
'feedbackURI' => '/reviewhelper/feedback/',
78+
),
79+
'moz-extensions'
80+
);
6581
}
6682
}

0 commit comments

Comments
 (0)