Skip to content

fix: ignore imported data when querying goals with custom props#6441

Open
ukutaht wants to merge 2 commits into
masterfrom
fix-imported-goals-with-custom-props
Open

fix: ignore imported data when querying goals with custom props#6441
ukutaht wants to merge 2 commits into
masterfrom
fix-imported-goals-with-custom-props

Conversation

@ukutaht

@ukutaht ukutaht commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Changes

Fixes APP-217

Goals with custom props are not considered in the imported data query logic. Filtering or breaking down by such a goal on a site with imported data either:

  • crashes — the goal filter emits meta.value[indexOf(meta.key, ...)] conditions against imported_custom_events, which has no meta columns → Ch.Error: UNKNOWN_IDENTIFIER, or
  • over-counts — the event:goal breakdown matches imported rows by event name / page path only, attributing all imported events to a prop-restricted goal

Changes

  • Imported.Base: skip imported data (with the standard unsupported_query warning) when any goal matching a top-level filter has custom props. Applied to all three table-decision paths, mirroring the existing scroll-goal handling.
  • Stats.Goals.goal_join_data/1: emit "" into types and event_names_imports (consumed only by the imported merge) for custom-props goals, so the unfiltered goal breakdown no longer counts imported rows toward them — other goals in the breakdown keep their imported data.

@ukutaht ukutaht requested review from a team and aerosol June 10, 2026 12:31
goal_type = Plausible.Goal.type(goal)
{prop_keys, prop_values} = Enum.unzip(goal.custom_props)

# `types` and `event_names_imports` are only used for matching goals

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd consider creating a separate function like goal_join_imported_data and return only fields that are relevant in each respective context. As is, this is turning very hairball-like.

%{
indices: [idx | acc.indices],
types: [to_string(goal_type) | acc.types],
types: [if(queryable_from_imports?, do: to_string(goal_type), else: "") | acc.types],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Provided we split the logic, wouldn't it make more sense to filter out the custom prop goals right at the start?

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.

2 participants