Skip to content

Commit cd1d4f0

Browse files
author
ace-n
committed
Address more comments (clarify content type, add test labels)
1 parent 389d394 commit cd1d4f0

File tree

5 files changed

+71
-64
lines changed

5 files changed

+71
-64
lines changed

‎functions/slack_slash_command/index.php‎

Lines changed: 46 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,26 @@
2020
use Psr\Http\Message\ResponseInterface;
2121
use GuzzleHttp\Psr7\Response;
2222

23-
require_once './vendor/autoload.php';
23+
use Google_Service_Kgsearch;
24+
use Google_Service_Kgsearch_SearchResponse;
25+
2426
// [END functions_slack_setup]
2527

2628
// [START functions_verify_webhook]
2729
/**
28-
* Verify that the webhook request came from Slack.
29-
*/
30+
* Verify that the webhook request came from Slack.
31+
*/
3032
function isValidSlackWebhook(ServerRequestInterface $request): bool
3133
{
3234
/**
33-
* PHP Functions Framework does not support "global"/instance-scoped
34-
* constants, so we fetch these values within PHP functtions themselves.
35-
*/
36-
$SLACK_SECRET = getenv("SLACK_SECRET");
35+
* PHP Functions Framework does not support "global"/instance-scoped
36+
* constants, so we fetch these values within PHP functtions themselves.
37+
*/
38+
$SLACK_SECRET = getenv('SLACK_SECRET');
3739

3840
// Check for headers
39-
$timestamp = $request->getHeader("X-Slack-Request-Timestamp");
40-
$signature = $request->getHeader("X-Slack-Signature");
41+
$timestamp = $request->getHeader('X-Slack-Request-Timestamp');
42+
$signature = $request->getHeader('X-Slack-Signature');
4143
if (!$timestamp || !$signature) {
4244
return false;
4345
} else {
@@ -55,63 +57,63 @@ function isValidSlackWebhook(ServerRequestInterface $request): bool
5557

5658
// [START functions_slack_format]
5759
/**
58-
* Format the Knowledge Graph API response into a richly formatted Slack message.
59-
*/
60+
* Format the Knowledge Graph API response into a richly formatted Slack message.
61+
*/
6062
function formatSlackMessage($kgResponse, $query): string
6163
{
6264
$responseJson = [
63-
"response_type" => "in_channel",
64-
"text" => "Query: " . $query
65+
'response_type' => 'in_channel',
66+
'text' => 'Query: ' . $query
6567
];
6668

67-
$entityList = $kgResponse["itemListElement"];
69+
$entityList = $kgResponse['itemListElement'];
6870

6971
// Extract the first entity from the result list, if any
7072
if (empty($entityList)) {
71-
$attachmentJson = ["text" => "No results match your query..."];
72-
$responseJson["attachments"] = $attachmentJson;
73+
$attachmentJson = ['text' => 'No results match your query...'];
74+
$responseJson['attachments'] = $attachmentJson;
7375

7476
return json_encode($responseJson);
7577
}
7678

77-
$entity = $entityList[0]["result"];
79+
$entity = $entityList[0]['result'];
7880

7981
// Construct Knowledge Graph response attachment
80-
$title = $entity["name"];
81-
if (isset($entity["description"])) {
82-
$title = $title . " " . $entity["description"];
82+
$title = $entity['name'];
83+
if (isset($entity['description'])) {
84+
$title = $title . ' ' . $entity['description'];
8385
}
84-
$attachmentJson = ["title" => $title];
86+
$attachmentJson = ['title' => $title];
8587

86-
if (isset($entity["detailedDescription"])) {
87-
$detailedDescJson = $entity["detailedDescription"];
88+
if (isset($entity['detailedDescription'])) {
89+
$detailedDescJson = $entity['detailedDescription'];
8890
$attachmentJson = array_merge([
89-
"title_link" => $detailedDescJson[ "url"],
90-
"text" => $detailedDescJson["articleBody"],
91+
'title_link' => $detailedDescJson[ 'url'],
92+
'text' => $detailedDescJson['articleBody'],
9193
], $attachmentJson);
9294
}
9395

94-
if ($entity["image"]) {
95-
$imageJson = $entity["image"];
96-
$attachmentJson["image_url"] = $imageJson["contentUrl"];
96+
if ($entity['image']) {
97+
$imageJson = $entity['image'];
98+
$attachmentJson['image_url'] = $imageJson['contentUrl'];
9799
}
98100

99-
$responseJson["attachments"] = array($attachmentJson);
101+
$responseJson['attachments'] = array($attachmentJson);
100102

101103
return json_encode($responseJson);
102104
}
103105
// [END functions_slack_format]
104106

105107
// [START functions_slack_request]
106108
/**
107-
* Send the user's search query to the Knowledge Graph API.
108-
*/
109+
* Send the user's search query to the Knowledge Graph API.
110+
*/
109111
function searchKnowledgeGraph($query): Google_Service_Kgsearch_SearchResponse
110112
{
111113
/**
112-
* PHP Functions Framework does not support "global"/instance-scoped
113-
* constants, so we fetch these values within PHP functions themselves.
114-
*/
114+
* PHP Functions Framework does not support "global"/instance-scoped
115+
* constants, so we fetch these values within PHP functions themselves.
116+
*/
115117
$API_KEY = getenv("KG_API_KEY");
116118

117119
$apiClient = new Google\Client();
@@ -129,31 +131,32 @@ function searchKnowledgeGraph($query): Google_Service_Kgsearch_SearchResponse
129131

130132
// [START functions_slack_search]
131133
/**
132-
* Receive a Slash Command request from Slack.
133-
*/
134+
* Receive a Slash Command request from Slack.
135+
*/
134136
function receiveRequest(ServerRequestInterface $request): ResponseInterface
135137
{
136138
// Validate request
137139
if ($request->getMethod() !== 'POST') {
138140
// [] = empty headers
139-
return new Response(405, [], '');
141+
return new Response(405);
140142
}
141143

142-
// Slack sends requests as URL-encoded strings
144+
// Parse incoming URL-encoded requests from Slack
145+
// (Slack requests use the "application/x-www-form-urlencoded" format)
143146
$bodyStr = $request->getBody();
144147
parse_str($bodyStr, $bodyParams);
145148

146-
if (!isset($bodyParams["text"])) {
149+
if (!isset($bodyParams['text'])) {
147150
// [] = empty headers
148-
return new Response(400, [], '');
151+
return new Response(400);
149152
}
150153

151154
if (!isValidSlackWebhook($request, $bodyStr)) {
152155
// [] = empty headers
153-
return new Response(403, [], '');
156+
return new Response(403);
154157
}
155158

156-
$query = $bodyParams["text"];
159+
$query = $bodyParams['text'];
157160

158161
// Call knowledge graph API
159162
$kgResponse = searchKnowledgeGraph($query);
@@ -164,7 +167,7 @@ function receiveRequest(ServerRequestInterface $request): ResponseInterface
164167

165168
return new Response(
166169
200,
167-
["Content-Type" => "application/json"],
170+
['Content-Type' => 'application/json'],
168171
$formatted_message
169172
);
170173
}

‎functions/slack_slash_command/test/DeployTest.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ class DeployTest extends TestCase
4343
* @dataProvider cases
4444
*/
4545
public function testFunction(
46+
$label,
4647
$body,
4748
$method,
4849
$expected,
@@ -54,11 +55,15 @@ public function testFunction(
5455
'',
5556
['headers' => $headers, 'body' => $body]
5657
);
57-
$this->assertEquals($statusCode, $response->getStatusCode());
58+
$this->assertEquals(
59+
$statusCode,
60+
$response->getStatusCode(),
61+
$label . ': status code'
62+
);
5863

5964
if ($expected !== null) {
6065
$output = (string) $response->getBody();
61-
$this->assertContains($expected, $output);
66+
$this->assertContains($expected, $output, $label . ': contains');
6267
}
6368
}
6469

‎functions/slack_slash_command/test/SystemTest.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class SystemTest extends TestCase
3838
* @dataProvider cases
3939
*/
4040
public function testFunction(
41+
$label,
4142
$body,
4243
$method,
4344
$expected,
@@ -49,7 +50,8 @@ public function testFunction(
4950
'/',
5051
['headers' => $headers, 'body' => $body]
5152
);
52-
$this->assertEquals($statusCode, $response->getStatusCode());
53+
$this->assertEquals(
54+
$statusCode, $response->getStatusCode(), $label . ": status code");
5355

5456
if ($expected !== null) {
5557
$output = (string) $response->getBody();

‎functions/slack_slash_command/test/TestCasesTrait.php‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -40,38 +40,33 @@ trait TestCasesTrait
4040
{
4141
public static function cases(): array
4242
{
43-
$SLACK_SIGNATURE = getenv('SLACK_TEST_SIGNATURE');
44-
4543
return [
46-
// Only allows POST
4744
[
45+
'label' => 'Only allows POST',
4846
'body' => '',
4947
'method' => 'GET',
5048
'expected' => null,
5149
'statusCode' => '405',
52-
'headers' => _valid_headers(''),
50+
'headers' => _valid_headers('')
5351
],
54-
55-
// Requires valid auth headers
5652
[
53+
'label' => 'Requires valid auth headers',
5754
'body' => 'text=foo',
5855
'method' => 'POST',
5956
'expected' => null,
6057
'statusCode' => '403',
6158
'headers' => [],
6259
],
63-
64-
// Doesn't allow blank body
6560
[
61+
'label' => 'Doesn\'t allow blank body',
6662
'body' => '',
6763
'method' => 'POST',
6864
'expected' => null,
6965
'statusCode' => '400',
7066
'headers' => _valid_headers(''),
7167
],
72-
73-
// Prohibits invalid signature
7468
[
69+
'label' => 'Prohibits invalid signature',
7570
'body' => 'text=foo',
7671
'method' => 'POST',
7772
'expected' => null,
@@ -82,27 +77,24 @@ public static function cases(): array
8277
'bad_signature'
8378
],
8479
],
85-
86-
// Handles no-result query
8780
[
81+
'label' => 'Handles no-result query',
8882
'body' => 'text=asdfjkl13579',
8983
'method' => 'POST',
9084
'expected' => 'No results match your query',
9185
'statusCode' => '200',
9286
'headers' => _valid_headers('text=asdfjkl13579'),
9387
],
94-
95-
// Handles query with results
9688
[
89+
'label' => 'Handles query with results',
9790
'body' => 'text=lion',
9891
'method' => 'POST',
9992
'expected' => 'https:\/\/en.wikipedia.org\/wiki\/Lion',
10093
'statusCode' => '200',
10194
'headers' => _valid_headers('text=lion'),
10295
],
103-
104-
// Ignores extra URL parameters
10596
[
97+
'label' => 'Ignores extra URL parameters',
10698
'body' => 'unused=foo&text=lion',
10799
'method' => 'POST',
108100
'expected' => 'https:\/\/en.wikipedia.org\/wiki\/Lion',

‎functions/slack_slash_command/test/UnitTest.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public static function setUpBeforeClass(): void
4141
* @dataProvider cases
4242
*/
4343
public function testFunction(
44+
$label,
4445
$body,
4546
$method,
4647
$expected,
@@ -50,11 +51,15 @@ public function testFunction(
5051
$request = new ServerRequest($method, '/', $headers, $body);
5152
$response = $this->runFunction(self::$name, [$request]);
5253

53-
$this->assertEquals($statusCode, $response->getStatusCode());
54+
$this->assertEquals(
55+
$statusCode,
56+
$response->getStatusCode(),
57+
$label . ": status code"
58+
);
5459

5560
if ($expected !== null) {
5661
$output = (string) $response->getBody();
57-
$this->assertContains($expected, $output);
62+
$this->assertContains($expected, $output, $label . ": contains");
5863
}
5964
}
6065

0 commit comments

Comments
 (0)