Fix analytics when combining folder scope with excluded links#3867
Fix analytics when combining folder scope with excluded links#3867pepeladeira wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR implements workspace-scoped link ID filtering by normalizing workspace-link parameters in the analytics TypeScript layer, tightening the early-return condition for all-time aggregates, and propagating consistent ChangesWorkspace-Scoped Link ID Filtering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/tinybird/pipes/v4_group_by.pipe (1)
141-170:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the
linkIdfiltering fix tov4_group_by_link_metadata.pipeevent nodesThe
group_by_link_metadata_clicks,group_by_link_metadata_leads, andgroup_by_link_metadata_salesnodes have linkId filtering logic that should apply the same fix asv4_group_by.pipe. When combining scope filters (folderId, domain, partnerId, groupId, partnerTagId, tenantId, tagId, programId, root) with excluded linkId filtering (NOT IN), addAND link_id IN (SELECT link_id FROM workspace_links)to ensure correct data accuracy.Example: clicks node
{% if defined(linkId) %} {% if defined(linkIdOperator) and String(linkIdOperator) == 'NOT IN' %} AND ce.link_id NOT IN {{ Array(linkId, 'String') }} {% if defined(programId) or defined(partnerId) or defined(groupId) or defined(partnerTagId) or defined(tenantId) or defined(folderId) or defined(domain) or defined(tagId) or defined(root) %} AND ce.link_id IN (SELECT link_id FROM workspace_links) {% end %} {% else %} AND ce.link_id IN {{ Array(linkId, 'String') }} {% if defined(programId) or defined(partnerId) or defined(groupId) or defined(partnerTagId) or defined(tenantId) or defined(folderId) or defined(domain) or defined(tagId) or defined(root) %} AND ce.link_id IN (SELECT link_id FROM workspace_links) {% end %} {% end %} {% end %}Apply the same pattern to the
leadsandsalesnodes, adjusting the table alias (le.link_idandse.link_idrespectively).🤖 Prompt for 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. In `@packages/tinybird/pipes/v4_group_by.pipe` around lines 141 - 170, The clicks/leads/sales event nodes (group_by_link_metadata_clicks, group_by_link_metadata_leads, group_by_link_metadata_sales) need the same linkId filtering fix as v4_group_by.pipe: when linkId is defined and linkIdOperator == 'NOT IN' and any scope filter (programId, partnerId, groupId, partnerTagId, tenantId, folderId, domain, tagId, programId, root) is present, add "AND <alias>.link_id IN (SELECT link_id FROM workspace_links)"; likewise add that same IN-check when linkId is defined and not NOT IN; update the conditions around linkId, linkIdOperator and the scope checks in each node replacing the plain aliases (ce.link_id, le.link_id, se.link_id) accordingly so the NOT IN branch and the IN branch both append the workspace_links IN clause when any scope filter is present.
🤖 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.
Outside diff comments:
In `@packages/tinybird/pipes/v4_group_by.pipe`:
- Around line 141-170: The clicks/leads/sales event nodes
(group_by_link_metadata_clicks, group_by_link_metadata_leads,
group_by_link_metadata_sales) need the same linkId filtering fix as
v4_group_by.pipe: when linkId is defined and linkIdOperator == 'NOT IN' and any
scope filter (programId, partnerId, groupId, partnerTagId, tenantId, folderId,
domain, tagId, programId, root) is present, add "AND <alias>.link_id IN (SELECT
link_id FROM workspace_links)"; likewise add that same IN-check when linkId is
defined and not NOT IN; update the conditions around linkId, linkIdOperator and
the scope checks in each node replacing the plain aliases (ce.link_id,
le.link_id, se.link_id) accordingly so the NOT IN branch and the IN branch both
append the workspace_links IN clause when any scope filter is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: add15672-91b3-46e9-ad33-f568426b1abb
📒 Files selected for processing (5)
apps/web/lib/analytics/get-analytics.tspackages/tinybird/pipes/v4_count.pipepackages/tinybird/pipes/v4_events.pipepackages/tinybird/pipes/v4_group_by.pipepackages/tinybird/pipes/v4_timeseries.pipe
Summary by CodeRabbit
Release Notes