diff --git a/README.md b/README.md index b800287a..d6f6eb71 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/src/main/java/dev/thiagogonzaga/thrillhousebot/review/FindingFeedbackService.java b/src/main/java/dev/thiagogonzaga/thrillhousebot/review/FindingFeedbackService.java index 48ff28fd..d5518954 100644 --- a/src/main/java/dev/thiagogonzaga/thrillhousebot/review/FindingFeedbackService.java +++ b/src/main/java/dev/thiagogonzaga/thrillhousebot/review/FindingFeedbackService.java @@ -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); + return new FeedbackPreferenceSummary(storedKey, useful, notUseful, useful + notUseful); } /** Newest events for a repository (dashboard detail); empty when the key is blank. */ @@ -135,7 +146,7 @@ public List 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() diff --git a/src/test/java/dev/thiagogonzaga/thrillhousebot/dashboard/DashboardResourceTest.java b/src/test/java/dev/thiagogonzaga/thrillhousebot/dashboard/DashboardResourceTest.java index 4e4196a2..b35ed3a5 100644 --- a/src/test/java/dev/thiagogonzaga/thrillhousebot/dashboard/DashboardResourceTest.java +++ b/src/test/java/dev/thiagogonzaga/thrillhousebot/dashboard/DashboardResourceTest.java @@ -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( diff --git a/src/test/java/dev/thiagogonzaga/thrillhousebot/review/FindingFeedbackServiceTest.java b/src/test/java/dev/thiagogonzaga/thrillhousebot/review/FindingFeedbackServiceTest.java index 95927a80..ec8a283d 100644 --- a/src/test/java/dev/thiagogonzaga/thrillhousebot/review/FindingFeedbackServiceTest.java +++ b/src/test/java/dev/thiagogonzaga/thrillhousebot/review/FindingFeedbackServiceTest.java @@ -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(