Fix analytics accuracy using tenant-scoped queries and cache-based deduplication#469
Open
Yogender-verma wants to merge 1 commit into
Open
Fix analytics accuracy using tenant-scoped queries and cache-based deduplication#469Yogender-verma wants to merge 1 commit into
Yogender-verma wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughAdds cache-based event deduplication (48-hour TTL) for bounce, reply, open, and click email events. A new ChangesEmail Event Deduplication
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/campaigns/tasks.py`:
- Around line 452-458: The bounce and reply task loops set the dedupe cache key
before the state transition finishes, so if processing fails the event is lost
on retry; update the bounce path around _mark_campaign_lead_bounced() and the
reply-processing block to remove the dedupe key whenever an exception occurs
during marking, saving, or routing, then re-raise/continue so the next run can
retry. Use the existing dedupe_key and cache.add() flow in
backend/campaigns/tasks.py, and apply the same cleanup pattern in both task
loops.
In `@backend/campaigns/tests.py`:
- Around line 1976-1985: The test setup in the User.objects.create_user calls is
using hardcoded password literals that are never used, so remove the password
argument from the user_a and user_b fixture creation in this test class. Keep
the create_user calls for the existing User model setup, but eliminate the
unused password= values to satisfy Ruff and avoid unnecessary test data.
In `@backend/campaigns/views.py`:
- Around line 389-392: The dedupe marker is being set too early in the webhook
flow, so a failure after cache.add() in the campaign event handler can cause
valid retries to be treated as duplicates. Update the dedupe handling around the
relevant campaign webhook processing path in views.py (the logic using
dedupe_key, cache.add, and the subsequent save/branch helpers) so the marker is
only committed after successful processing, or is explicitly removed if an
exception occurs before completion. Keep the duplicate check behavior in place,
but ensure failures do not leave a stale 48-hour marker behind.
- Around line 384-390: The deduplication key for bounce/reply terminal events is
inconsistent between the webhook path and the task path because `dedupe_key` in
the webhook logic prefers `provider_event_id` while the task guards use
`last_sent_message_id`. Update the dedupe-key construction in the webhook
handling around `cache.add` to use the same identifier precedence as the task
logic, so both paths generate identical `evt_dedupe` keys for the same event.
Verify the symbols `dedupe_key`, `provider_event_id`, `message_id`, and
`last_sent_message_id` all resolve to one shared scheme for bounce/reply events.
- Around line 501-516: Require a single trusted lead match before any processing
in the webhook handler. In the view logic around the `org_id` filtering and
`cleads = list(base_qs)`, do not treat unauthenticated payload tenant fields
(`organization_id`, `org_id`, `orgId`) as sufficient to resolve ambiguity; only
narrow the queryset when a trusted server-generated identifier like `message_id`
or `campaign_id` is present. Update the `CampaignLead` selection path so that if
the filtered result still yields multiple rows, the request is ignored rather
than iterating through all matches in the `for cl in cleads` block, and keep the
existing ambiguity warning/response for `AllowAny` requests.
- Around line 854-859: The click redirect flow in the code path that calls
process_email_event() should remain best-effort even if analytics processing
fails. Update the click handling block around process_email_event in the lead
redirect logic to catch and ignore transient exceptions from
analytics/cache/database work, while still only special-casing
CampaignLead.DoesNotExist as needed, so the destination redirect can continue
even when event processing fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f6ae228c-25bc-410b-8282-9e2ac36171f0
📒 Files selected for processing (3)
backend/campaigns/tasks.pybackend/campaigns/tests.pybackend/campaigns/views.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
🔗 Related Issue
Closes #20
📝 Summary of Changes
🏷️ Type of Change
🧪 Testing
How this was tested:
python manage.py test)✅ Checklist
Summary by CodeRabbit