Skip to content

fix(web): include start/end in webhook notifications query key - #268

Merged
vernu merged 4 commits into
vernu:devfrom
kalayciburak:fix/webhook-history-date-filter-query-key
Aug 3, 2026
Merged

fix(web): include start/end in webhook notifications query key#268
vernu merged 4 commits into
vernu:devfrom
kalayciburak:fix/webhook-history-date-filter-query-key

Conversation

@kalayciburak

@kalayciburak kalayciburak commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

useWebhookNotifications sent start and end as request query params but omitted them from the react-query queryKey. Changing the webhook history date filter therefore reused a cached response instead of refetching.

This moves the key into queryKeys.webhookNotifications (including start/end) and adds regression coverage.

Fixes #256

Test plan

  • npx vitest run lib/api/query-keys.test.ts lib/api/hooks.test.tsx lib/api/cache-invalidation.test.tsx (7/7)
  • npx vitest run in web/ (195/195)
  • npx tsc --noEmit in web/
  • npx eslint on touched files

Summary by CodeRabbit

  • Bug Fixes

    • Fixed webhook notification results being incorrectly reused when changing the start or end date filters.
    • Notification searches now refresh with the latest filter and pagination settings, ensuring results match the selected criteria.
  • Tests

    • Added coverage to verify that different date ranges produce distinct notification results and requests.

vernu and others added 4 commits July 22, 2026 17:34
Date filter changes for webhook history were ignored by react-query
because start and end were only sent as request params, not cache keys.

Fixes vernu#256
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

@kalayciburak is attempting to deploy a commit to the vernu's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The webhook notification hook now uses a centralized query-key factory that includes all filters, including start and end dates. Tests verify distinct keys and confirm that changing the start date triggers a new request.

Changes

Webhook notification cache invalidation

Layer / File(s) Summary
Webhook notification query-key contract
web/lib/api/query-keys.ts, web/lib/api/query-keys.test.ts
The query-key factory includes notification filters, pagination, and date values. Tests verify distinct keys and the expected key shape.
Hook integration and refetch validation
web/lib/api/hooks.ts, web/lib/api/hooks.test.tsx
useWebhookNotifications uses the centralized query-key factory. The regression test verifies that changing the start date sends a new request.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes add start and end to the query key and regression tests, satisfying issue #256.
Out of Scope Changes check ✅ Passed The changes are limited to the query-key fix and related regression coverage described in issue #256.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the fix to include start and end dates in the webhook notifications query key.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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

@vernu vernu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kalayciburak thanks for contributing

@vernu
vernu changed the base branch from main to dev August 3, 2026 09:39
@vernu
vernu merged commit 967ddd4 into vernu:dev Aug 3, 2026
1 of 2 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Aug 3, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Web: webhook history date filter never refetches

2 participants