Chore/834 consolidate campaign event hooks - #857
Conversation
|
@CHKM001 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Auto-review failed (API error). Leaving PR for human review. |
1 similar comment
|
Auto-review failed (API error). Leaving PR for human review. |
davidmaronio
left a comment
davidmaronio
left a comment
There was a problem hiding this comment.
the core extraction is good. useCampaignEvents faithfully carries over the dedupe set, cursor ref, visibility gating, callback ref pattern, and cleanup from both hooks, and the generic TEvent extends CampaignEvent signature keeps the call sites type-safe. this is the right shape for #834.
a few things need fixing before merge:
- src/hooks/useCampaignVoteEvents.ts:44 - behavior regression: the old hook called
onStreamingUnavailable?.()when a poll threw, so the UI could fall back when RPC started failing mid-session. the generic hook swallows fetch errors silently, so that signal is lost. consider an optionalonErrorcallback onuseCampaignEventsthat the vote hook wires toonStreamingUnavailable. - src/tests/lib/networkFee.test.ts - this file is unrelated to hook consolidation and is the exact diff already up in your #854. please drop it from this branch (looks like the branch is stacked); same for the quote-style churn in src/hooks/useDonationGracePeriod.ts.
- coordination: #849 touches the same two hooks and routes them through the shared
eventSubscriber(closes #833, dedupes polling across consumers). the two PRs conflict directly. #849 solves a different problem (one poll loop per campaign across all consumers) that this extraction does not, so whichever lands first forces a rework of the other. my suggestion: let the maintainers pick the ordering, and if #849 lands first this generic hook could live inside eventSubscriber's consumer instead. - the typecheck/build failures are the known stale base fixed by #875; a rebase is needed regardless.
|
Auto-review failed (API error). Leaving PR for human review. |
4a75472 to
d5e6d2c
Compare
|
Auto-review failed (API error). Leaving PR for human review. |
d5e6d2c to
8c80ba8
Compare
|
Auto-review failed (API error). Leaving PR for human review. |
…paignEvents (Iris-IV#834) Extract shared subscribe/unsubscribe/state-update logic from useCampaignContributionEvents and useCampaignVoteEvents into a single generic useCampaignEvents<TEvent> hook. Key changes: - New useCampaignEvents hook carries over the dedupe set, cursor ref, visibility gating, callback ref pattern, and cleanup from both hooks - Optional onError callback on UseCampaignEventsOptions so consumers can react to poll failures (wired in useCampaignVoteEvents to call onStreamingUnavailable, restoring behavior lost during extraction) - Both existing hooks become thin wrappers around the generic hook - eventSubscriber.ts: fix Timeout type annotation
8c80ba8 to
b626b0d
Compare
|
Auto-review failed (API error). Leaving PR for human review. |
3 participants
PR #834 # Consolidate duplicate campaign event hooks
Summary
useCampaignContributionEventsanduseCampaignVoteEventsshared near-identical polling/dedup/subscription lifecycle logic. Every bugfix to the subscription lifecycle had to be applied twice.Changes
src/hooks/useCampaignEvents.ts— generic hook that accepts:fetchEvents— the async fetch functiononUnseenEvents— callback for processed unseen eventspollIntervalMs— polling intervaluseMocksCheck— optional USE_MOCKS guarduseCampaignContributionEvents.ts— 37 lines (was 89), thin wrapper passingfetchContributionMadeEvents+ batch sum/invalidationuseCampaignVoteEvents.ts— 52 lines (was 92), thin wrapper passingfetchVoteCastEvents+ per-event parsing; streaming-availability check kept separate as voting-specific concernTesting
src/__tests__/hooks/useLiveCampaignFunding.test.tsx— 2/2 passsrc/__tests__/hooks/useLiveVoteTallies.test.tsx— 2/2 passCloses #834