Skip to content

Commit 24f7495

Browse files
OskarStarkOskarStark
committed
Fix label spacing in PR titles
When multiple labels are added to a PR, ensure they are concatenated without spaces between brackets
1 parent 035e724 commit 24f7495

File tree

2 files changed

+202
-1
lines changed

2 files changed

+202
-1
lines changed

‎src/Subscriber/AutoUpdateTitleWithLabelSubscriber.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,12 @@ public function onPullRequest(GitHubEvent $event): void
7676
$prTitle = (string) preg_replace('@^[\h\s]+@u', '', html_entity_decode($prTitle));
7777

7878
// Add back labels
79-
$prTitle = trim($prPrefix.' '.trim($prTitle));
79+
// Ensure we don't have an extra space if the title is empty after label removal
80+
if ('' === trim($prTitle)) {
81+
$prTitle = $prPrefix;
82+
} else {
83+
$prTitle = $prPrefix.' '.trim($prTitle);
84+
}
8085
if ($originalTitle === $prTitle) {
8186
return;
8287
}

‎tests/Subscriber/AutoUpdateTitleWithLabelSubscriberTest.php

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,4 +129,200 @@ public function testExtraBlankSpace()
129129
$this->assertSame(57753, $responseData['pull_request']);
130130
$this->assertSame('[ErrorHandler] restrict the maximum length of the X-Debug-Exception header', $responseData['new_title']);
131131
}
132+
133+
public function testMultipleLabelsWithoutSpaceBetweenBrackets()
134+
{
135+
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
136+
$this->pullRequestApi->method('show')->willReturn([
137+
'title' => 'Foo Bar',
138+
'labels' => [
139+
['name' => 'Platform', 'color' => 'dddddd'],
140+
['name' => 'Agent', 'color' => 'dddddd'],
141+
],
142+
]);
143+
144+
$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
145+
$responseData = $event->getResponseData();
146+
$this->assertCount(2, $responseData);
147+
$this->assertSame(1234, $responseData['pull_request']);
148+
$this->assertSame('[Agent][Platform] Foo Bar', $responseData['new_title']);
149+
}
150+
151+
public function testMultipleLabelsUpdateRemovesSpaceBetweenBrackets()
152+
{
153+
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
154+
$this->pullRequestApi->method('show')->willReturn([
155+
'title' => '[Platform] [Agent] Foo Bar', // Title already has labels with space
156+
'labels' => [
157+
['name' => 'Platform', 'color' => 'dddddd'],
158+
['name' => 'Agent', 'color' => 'dddddd'],
159+
],
160+
]);
161+
162+
$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
163+
$responseData = $event->getResponseData();
164+
$this->assertCount(2, $responseData);
165+
$this->assertSame(1234, $responseData['pull_request']);
166+
$this->assertSame('[Agent][Platform] Foo Bar', $responseData['new_title']);
167+
}
168+
169+
public function testAddingLabelToExistingLabeledTitle()
170+
{
171+
// Simulating when we already have [Platform] and are adding Agent label
172+
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
173+
$this->pullRequestApi->method('show')->willReturn([
174+
'title' => '[Platform] Foo Bar', // Title already has one label
175+
'labels' => [
176+
['name' => 'Platform', 'color' => 'dddddd'],
177+
['name' => 'Agent', 'color' => 'dddddd'], // New label added
178+
],
179+
]);
180+
181+
$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
182+
$responseData = $event->getResponseData();
183+
$this->assertCount(2, $responseData);
184+
$this->assertSame(1234, $responseData['pull_request']);
185+
$this->assertSame('[Agent][Platform] Foo Bar', $responseData['new_title']);
186+
}
187+
188+
public function testLabelsNotRecognizedAsValidLabels()
189+
{
190+
// This test simulates what happens when [Platform] and [Agent] are in the title
191+
// but they're NOT recognized as valid labels (wrong case or not in label list)
192+
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
193+
$this->pullRequestApi->method('show')->willReturn([
194+
'title' => '[Platform] [Agent] Foo Bar', // These are in title but not recognized
195+
'labels' => [
196+
['name' => 'Bug', 'color' => 'dddddd'], // Different valid label
197+
],
198+
]);
199+
200+
$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
201+
$responseData = $event->getResponseData();
202+
$this->assertCount(2, $responseData);
203+
$this->assertSame(1234, $responseData['pull_request']);
204+
// Since Platform and Agent are not recognized labels, they stay in the title with the space
205+
$this->assertSame('[Bug] [Platform] [Agent] Foo Bar', $responseData['new_title']);
206+
}
207+
208+
/**
209+
* This test reproduces the actual bug: when labels are attached to the PR
210+
* but are not in the repository's configured label list, they won't be
211+
* removed from the title, causing spacing issues.
212+
*/
213+
public function testBugWithLabelsNotInRepositoryLabelList()
214+
{
215+
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
216+
$this->pullRequestApi->method('show')->willReturn([
217+
'title' => 'Foo Bar',
218+
'labels' => [
219+
// These labels are attached to PR but not in the StaticLabelApi list
220+
['name' => 'Platform', 'color' => 'dddddd'],
221+
['name' => 'Agent', 'color' => 'dddddd'],
222+
],
223+
]);
224+
225+
$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
226+
$responseData = $event->getResponseData();
227+
228+
// The bug would cause this to be '[Platform] [Agent] Foo Bar'
229+
// because the labels are added with concatenation but not recognized
230+
// for removal from the title
231+
$this->assertCount(2, $responseData);
232+
$this->assertSame(1234, $responseData['pull_request']);
233+
234+
// This is what SHOULD happen (labels concatenated without spaces)
235+
$this->assertSame('[Agent][Platform] Foo Bar', $responseData['new_title']);
236+
}
237+
238+
/**
239+
* Test simulating incremental label additions as would happen with multiple webhooks
240+
*/
241+
public function testIncrementalLabelAdditions()
242+
{
243+
// Need to create a new mock for each test to avoid expectations interference
244+
$pullRequestApi1 = $this->getMockBuilder(NullPullRequestApi::class)
245+
->disableOriginalConstructor()
246+
->setMethods(['show'])
247+
->getMock();
248+
249+
$pullRequestApi1->expects($this->once())->method('show')->willReturn([
250+
'title' => 'Foo Bar',
251+
'labels' => [
252+
['name' => 'FrameworkBundle', 'color' => 'dddddd'],
253+
],
254+
]);
255+
256+
$store = $this->getMockBuilder(PersistingStoreInterface::class)->getMock();
257+
$store->method('exists')->willReturn(false);
258+
259+
$labelsApi = new StaticLabelApi();
260+
$subscriber1 = new AutoUpdateTitleWithLabelSubscriber(
261+
new LabelNameExtractor($labelsApi, new NullLogger()),
262+
$pullRequestApi1,
263+
new LockFactory($store)
264+
);
265+
266+
$dispatcher1 = new EventDispatcher();
267+
$dispatcher1->addSubscriber($subscriber1);
268+
269+
// First webhook: FrameworkBundle label added
270+
$event1 = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
271+
$dispatcher1->dispatch($event1, GitHubEvents::PULL_REQUEST);
272+
$responseData1 = $event1->getResponseData();
273+
$this->assertSame('[FrameworkBundle] Foo Bar', $responseData1['new_title']);
274+
275+
// Second webhook: Console label added (title now has [FrameworkBundle] in it)
276+
$pullRequestApi2 = $this->getMockBuilder(NullPullRequestApi::class)
277+
->disableOriginalConstructor()
278+
->setMethods(['show'])
279+
->getMock();
280+
281+
$pullRequestApi2->expects($this->once())->method('show')->willReturn([
282+
'title' => '[FrameworkBundle] Foo Bar', // Title from first update
283+
'labels' => [
284+
['name' => 'FrameworkBundle', 'color' => 'dddddd'],
285+
['name' => 'Console', 'color' => 'dddddd'],
286+
],
287+
]);
288+
289+
$subscriber2 = new AutoUpdateTitleWithLabelSubscriber(
290+
new LabelNameExtractor($labelsApi, new NullLogger()),
291+
$pullRequestApi2,
292+
new LockFactory($store)
293+
);
294+
295+
$dispatcher2 = new EventDispatcher();
296+
$dispatcher2->addSubscriber($subscriber2);
297+
298+
$event2 = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
299+
$dispatcher2->dispatch($event2, GitHubEvents::PULL_REQUEST);
300+
$responseData2 = $event2->getResponseData();
301+
// Should produce [Console][FrameworkBundle] without space
302+
$this->assertSame('[Console][FrameworkBundle] Foo Bar', $responseData2['new_title']);
303+
}
304+
305+
/**
306+
* Test that ensures no spaces are ever added between label brackets
307+
*/
308+
public function testNoSpacesBetweenLabelBrackets()
309+
{
310+
// Test with empty title after label removal
311+
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
312+
$this->pullRequestApi->method('show')->willReturn([
313+
'title' => '[Console] [FrameworkBundle]', // Only labels, no other text
314+
'labels' => [
315+
['name' => 'Console', 'color' => 'dddddd'],
316+
['name' => 'FrameworkBundle', 'color' => 'dddddd'],
317+
],
318+
]);
319+
320+
$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
321+
$responseData = $event->getResponseData();
322+
323+
// Should produce labels without spaces and no trailing space
324+
$this->assertCount(2, $responseData);
325+
$this->assertSame(1234, $responseData['pull_request']);
326+
$this->assertSame('[Console][FrameworkBundle]', $responseData['new_title']);
327+
}
132328
}

0 commit comments

Comments
 (0)