Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -471,14 +471,14 @@ summary (`GET /api/dashboard/summary`, field `skippedReviewsByReason`). Check, i
1. **Is the App installed on the repository?** No webhook delivery at all means the
GitHub App isn't installed (or the webhook URL/secret is wrong). Check the App's
**Advanced β†’ Recent Deliveries** page on GitHub.
2. **Is the PR a draft?** (`reason=DRAFT`) β€” with `WEBHOOK_TRIGGERS_SKIP_DRAFTS=true`,
2. **Is the PR a draft?** (`reason=DRAFT`) β€” with `WEBHOOK_SKIP_DRAFTS=true`,
drafts are skipped until marked ready for review.
3. **Is the PR paused?** (`reason=PAUSED`) β€” someone commented `/pause`; comment
`/resume` to re-enable reviews.
4. **Label gates** (`reason=MISSING_REQUIRED_LABEL` / `EXCLUDED_LABEL`) β€” check
`WEBHOOK_TRIGGERS_REQUIRED_LABELS` and `WEBHOOK_TRIGGERS_EXCLUDED_LABELS`.
`WEBHOOK_REQUIRED_LABELS` and `WEBHOOK_EXCLUDED_LABELS`.
5. **Base branch filters** (`reason=BASE_BRANCH_NOT_ALLOWED` / `IGNORED_BASE_BRANCH`) β€”
check `WEBHOOK_TRIGGERS_BASE_BRANCHES` and `WEBHOOK_TRIGGERS_IGNORED_BASE_BRANCHES`.
check `WEBHOOK_BASE_BRANCHES` and `WEBHOOK_IGNORED_BASE_BRANCHES`.
6. **Rate window** (`reason=RATE_LIMITED`) β€” an automatic review already completed
within `AUTO_REVIEW_MIN_INTERVAL`; a manual `/review` bypasses the window.
7. **Redelivery** (`reason=DUPLICATE_DELIVERY`) β€” GitHub redelivered a webhook the bot
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,22 @@ public FeedbackPreferenceSummary summarize(String repositoryKey) {
return new FeedbackPreferenceSummary("", 0, 0, 0);
}
var key = repositoryKey.strip();
// Match stored webhook casing case-insensitively so dashboard filters like Owner/Repo work.
long useful =
repository.count("repository = ?1 and signal = ?2", key, FindingFeedback.SIGNAL_USEFUL);
repository.count(
"lower(repository) = lower(?1) and signal = ?2", key, FindingFeedback.SIGNAL_USEFUL);
long notUseful =
repository.count("repository = ?1 and signal = ?2", key, FindingFeedback.SIGNAL_NOT_USEFUL);
return new FeedbackPreferenceSummary(key, useful, notUseful, useful + notUseful);
repository.count(
"lower(repository) = lower(?1) and signal = ?2",
key,
FindingFeedback.SIGNAL_NOT_USEFUL);
var storedKey =
repository
.find("lower(repository) = lower(?1) order by repository", key)
.firstResultOptional()
.map(f -> f.repository)
.orElse(key);
Comment thread
devops-thiago marked this conversation as resolved.
return new FeedbackPreferenceSummary(storedKey, useful, notUseful, useful + notUseful);
}

/** Newest events for a repository (dashboard detail); empty when the key is blank. */
Expand All @@ -135,7 +146,7 @@ public List<FeedbackEvent> listRecent(String repositoryKey, int limit) {
return List.of();
}
return repository
.find("repository = ?1 order by createdAt desc", repositoryKey.strip())
.find("lower(repository) = lower(?1) order by createdAt desc", repositoryKey.strip())
.page(0, Math.min(limit, 100))
.list()
.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,33 @@ void shouldRejectFeedbackForAnInaccessibleRepository() {
.body("error", equalTo("Repository access denied"));
}

@Test
void shouldReturnFeedbackWhenRepositoryQueryCasingDiffers() {
findingFeedbackService.recordFeedback(
new FindingFeedbackService.FeedbackInput(
"Owner/Repo",
1,
10L,
1,
FindingFeedback.SIGNAL_USEFUL,
FindingFeedback.SOURCE_REACTION,
"octocat",
301L));
when(sessionValidator.hasRepositoryAccess(VALID_TOKEN, "owner/repo")).thenReturn(true);

given()
.cookie(COOKIE_NAME, VALID_TOKEN)
.queryParam("repository", "owner/repo")
.when()
.get("/feedback")
.then()
.statusCode(200)
.body("repositories[0].repository", equalTo("Owner/Repo"))
.body("repositories[0].usefulCount", equalTo(1))
.body("repositories[0].totalEvents", equalTo(1))
.body("recent", hasSize(1));
}

@Test
void shouldFilterInaccessibleRepositoriesFromFeedbackAggregates() {
findingFeedbackService.recordFeedback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,30 @@ void summarizeAndListRecentHandleBlankKeys() {
assertTrue(service.listRecent("owner/repo", -1).isEmpty());
}

@Test
void summarizeAndListRecentMatchRepositoryCaseInsensitively() {
assertTrue(
service.recordFeedback(
new FindingFeedbackService.FeedbackInput(
"Owner/Repo",
1,
10L,
1,
FindingFeedback.SIGNAL_USEFUL,
FindingFeedback.SOURCE_REACTION,
"octocat",
301L)));

var summary = service.summarize("owner/repo");
assertEquals("Owner/Repo", summary.repository());
assertEquals(1, summary.usefulCount());
assertEquals(1, summary.totalEvents());

var recent = service.listRecent("OWNER/REPO", 10);
assertEquals(1, recent.size());
assertEquals("Owner/Repo", recent.get(0).repository());
}

@Test
void recordAcceptsNullReactionIdAndRejectsNullReactor() {
assertTrue(
Expand Down
Loading