Skip to content

fix: resolve three open bugs (#278, #280, #281)#285

Merged
bd73-com merged 4 commits intomainfrom
claude/fix-github-issues-PEdGa
Mar 26, 2026
Merged

fix: resolve three open bugs (#278, #280, #281)#285
bd73-com merged 4 commits intomainfrom
claude/fix-github-issues-PEdGa

Conversation

@bd73-com
Copy link
Copy Markdown
Owner

@bd73-com bd73-com commented Mar 26, 2026

Summary

Fixes three open GitHub issues: fragile migration test assertion (#281), missing lazy-retry guard on campaign admin routes (#280), and missing Zod schema validation on the batch-delete error logs endpoint (#278). All changes are backend-only with no user-facing impact.

Changes

#281 — Fragile migration test assertion

  • Replaced toHaveBeenCalledTimes(25) with toBeGreaterThan(0) in server/routes.migration.test.ts
  • The 13 per-DDL string assertions already validate each ensure function, making the exact count redundant and brittle

#280 — Missing lazy-retry guard for campaign admin routes

  • Added requireCampaignConfigsReady function mirroring the existing requireConditionsReady pattern
  • Applied the guard to all three /api/admin/automated-campaigns routes (GET, PATCH, POST trigger)
  • Changed campaignConfigsReady from const to let to allow lazy mutation on retry

#278 — Missing Zod validation on batch-delete error logs

  • Added batchDeleteSchema with Zod validation: typed ids, filters with enum validation, excludeIds, .strict() on both outer and nested objects
  • XOR constraint via .refine() ensures exactly one of ids or filters is provided
  • Added .refine() rejecting excludeIds when ids is present (was silently ignored)
  • Moved empty-filters check into Zod .refine() for consistent error format
  • Capped ids and excludeIds arrays at 500 to prevent oversized SQL IN clauses
  • Removed redundant manual validation that Zod now handles

Tests

  • Updated 8 existing tests to match new Zod error response format
  • Added 3 new tests: strict schema rejection (top-level + nested), excludeIds+ids rejection

How to test

  1. npm run check && npm run test — all 1827 tests pass
  2. npm run build — production build succeeds
  3. Verify batch-delete rejects: { ids: [1], extraProp: true } → 400
  4. Verify batch-delete rejects: { ids: [1,2], excludeIds: [1] } → 400
  5. Verify batch-delete rejects: { filters: {} } → 400
  6. Verify campaign admin routes return 503 when table is unavailable

Closes #278, closes #280, closes #281

https://claude.ai/code/session_01QqzNhpF5Qy7W8vrxiFfmgD

Summary by CodeRabbit

  • Bug Fixes

    • Strengthened request validation for batch error log deletion operations, providing more consistent error responses and preventing invalid parameter combinations.
  • Improvements

    • Implemented service availability checks for automated campaign configuration features, returning appropriate service unavailable responses when the backend is not yet ready to serve requests.
  • Tests

    • Updated test suite to ensure robust validation of error log deletion and migration operations.

claude added 4 commits March 26, 2026 18:20
- #281: Replace fragile exact call-count assertion in migration test with
  toBeGreaterThan(0), relying on per-DDL string assertions for coverage
- #280: Add requireCampaignConfigsReady lazy-retry guard to all
  /api/admin/automated-campaigns routes, matching the existing
  requireConditionsReady pattern
- #278: Add Zod schema validation to POST /api/admin/error-logs/batch-delete
  request body, rejecting invalid types and unexpected properties with 400

https://claude.ai/code/session_01QqzNhpF5Qy7W8vrxiFfmgD
Add tests verifying that extra top-level and nested properties are
rejected by the strict Zod schema. Also apply .strict() to the nested
filters object for consistent validation.

https://claude.ai/code/session_01QqzNhpF5Qy7W8vrxiFfmgD
- Reject excludeIds when combined with ids (was silently ignored)
- Move empty-filters check into Zod refine for consistent error format
- Add test for excludeIds + ids rejection

https://claude.ai/code/session_01QqzNhpF5Qy7W8vrxiFfmgD
@github-actions github-actions bot added the fix label Mar 26, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 26, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR fixes three distinct issues: missing Zod schema validation on the batch-delete error-logs endpoint, a missing lazy-retry guard for automated campaign admin routes that fail silently after transient startup errors, and a fragile exact call-count assertion in migration tests that breaks when new ensure functions are added.

Changes

Cohort / File(s) Summary
Batch-delete error-log validation
server/routes.ts, server/routes.deleteErrorLog.test.ts
Introduced batchDeleteSchema Zod validation for request body (ids, filters, excludeIds), enforcing numeric constraints, array size caps, mutual exclusivity rules, and disallowing excludeIds with ids. Centralized validation returns 400 with flattened errors. Updated test assertions to expect unified "Invalid request body" responses and added strict schema tests for unexpected properties.
Campaign configs lazy-retry guard
server/routes.ts
Added requireCampaignConfigsReady(res) helper and converted campaignConfigsReady to mutable state, enabling per-request retry of ensureAutomatedCampaignConfigsTable(). Automated campaign admin routes now short-circuit with 503 SERVICE_UNAVAILABLE when configs remain unavailable after retry.
Migration test assertion
server/routes.migration.test.ts
Relaxed exact mockDbExecute call-count assertion from hardcoded 25 to > 0, relying instead on per-DDL substring assertions to verify individual migration statements were issued, eliminating manual count maintenance.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • PR #274: Adds "browserless" to ERROR_LOG_SOURCES and errorLogSourceSchema, which this PR now imports and uses in batch-delete validation.
  • PR #279: Modifies the same batch-delete error-log route and tests, tightening request validation.
  • PR #133: Implements requireConditionsReady lazy-retry pattern for monitor conditions; this PR applies an analogous pattern to campaign configs.

Suggested labels

fix

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title clearly identifies the PR as fixing three specific, numbered bugs (#278, #280, #281), directly mapping to the linked issues and main changes.
Linked Issues check ✅ Passed All three linked issues are addressed: #278 adds Zod batchDeleteSchema with strict validation and proper error responses; #280 adds requireCampaignConfigsReady lazy-retry guard; #281 relaxes fragile call-count assertion to toBeGreaterThan(0).
Out of Scope Changes check ✅ Passed All changes are scoped to the three linked issues: batch-delete validation schema, campaign admin route guards, and migration test assertion. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/fix-github-issues-PEdGa

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@bd73-com bd73-com merged commit 5d22c28 into main Mar 26, 2026
2 checks passed
@github-actions github-actions bot deleted the claude/fix-github-issues-PEdGa branch March 26, 2026 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants