Skip to content

Commit 2ba59ca

Browse files
committed
bug #164 Fix space in auto title (Nyholm)
This PR was squashed before being merged into the master branch. Discussion ---------- Fix space in auto title This will fix #161. @wouterj, this was interesting. What happen is: 1. PR is posted it as `[RateLimiter][Security] ...` 2. Carson parses the title and adds label "RateLimiter", then "Security" 3. Github sends a "label was updated ping" about RateLimiter 4. Carson correctly title to `[RateLimiter] [Security] ...` because " Security was not marked as a label yet. 5. Github sends a "label was updated ping" about Security 6. Carson will update the title again, and correctly set it to `[RateLimiter][Security] ...` However, It is a race condition... if 3 and 5 is reversed, then you experience the wrong title. The easy fix is to update the two labels at the same time, which is what I do in this PR. Commits ------- 0eeb6cb Fix space in auto title
2 parents 0fba8c3 + 0eeb6cb commit 2ba59ca

File tree

2 files changed

+20
-16
lines changed

2 files changed

+20
-16
lines changed

src/Api/Label/GithubLabelApi.php

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -76,19 +76,7 @@ public function getIssueLabels($issueNumber, Repository $repository): array
7676

7777
public function addIssueLabel($issueNumber, string $label, Repository $repository)
7878
{
79-
$key = $this->getCacheKey($issueNumber, $repository);
80-
81-
if (isset($this->labelCache[$key][$label])) {
82-
return;
83-
}
84-
85-
$this->logger->debug('Adding label "{label}" for {repo}#{issue}', ['label' => $label, 'repo' => $repository->getFullName(), 'issue' => $issueNumber]);
86-
$this->labelsApi->add($repository->getVendor(), $repository->getName(), $issueNumber, $label);
87-
88-
// Update cache if already loaded
89-
if (isset($this->labelCache[$key])) {
90-
$this->labelCache[$key][$label] = true;
91-
}
79+
$this->addIssueLabels($issueNumber, [$label], $repository);
9280
}
9381

9482
public function removeIssueLabel($issueNumber, string $label, Repository $repository)
@@ -115,8 +103,24 @@ public function removeIssueLabel($issueNumber, string $label, Repository $reposi
115103

116104
public function addIssueLabels($issueNumber, array $labels, Repository $repository)
117105
{
106+
$key = $this->getCacheKey($issueNumber, $repository);
107+
$labelsToAdd = [];
108+
118109
foreach ($labels as $label) {
119-
$this->addIssueLabel($issueNumber, $label, $repository);
110+
if (!isset($this->labelCache[$key][$label])) {
111+
$labelsToAdd[] = $label;
112+
}
113+
}
114+
115+
if ([] !== $labelsToAdd) {
116+
$this->labelsApi->add($repository->getVendor(), $repository->getName(), $issueNumber, $labelsToAdd);
117+
}
118+
119+
// Update cache if already loaded
120+
foreach ($labels as $label) {
121+
if (isset($this->labelCache[$key])) {
122+
$this->labelCache[$key][$label] = true;
123+
}
120124
}
121125
}
122126

tests/Api/Label/GithubLabelApiTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public function testAddIssueLabel()
7878

7979
$this->backendApi->expects($this->once())
8080
->method('add')
81-
->with(self::USER_NAME, self::REPO_NAME, 1234, 'a');
81+
->with(self::USER_NAME, self::REPO_NAME, 1234, ['a']);
8282

8383
$this->api->addIssueLabel(1234, 'a', $this->repository);
8484
}
@@ -96,7 +96,7 @@ public function testAddIssueLabelUpdatesCache()
9696

9797
$this->backendApi->expects($this->once())
9898
->method('add')
99-
->with(self::USER_NAME, self::REPO_NAME, 1234, 'd');
99+
->with(self::USER_NAME, self::REPO_NAME, 1234, ['d']);
100100

101101
$this->assertSame(['a', 'b', 'c'], $this->api->getIssueLabels(1234, $this->repository));
102102

0 commit comments

Comments
 (0)