Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,13 @@ SELECT
FROM {{ source('bronze_m365', 'teams_activity') }}
WHERE userPrincipalName IS NOT NULL
AND userPrincipalName != ''
-- Drop unlicensed users (guests, ex-employees, service accounts). Their Teams
-- activity inflates team-level collab counters and produces orphan gold rows
-- with no matching insight.people entry. `isLicensed` = "Selected if the user
-- is licensed to use Teams" (MS Graph getTeamsUserActivityUserDetail). Only the
-- teams_activity report exposes this flag; the email/onedrive/sharepoint feeders
-- fall back to `assignedProducts`. Conservative on NULL (unknown -> keep). See #736.
AND coalesce(isLicensed, true) = true
{% if is_incremental() %}
-- Watermark on the source EXTRACT time, not the business date (see zoom model header
-- for the backfill-strand failure mode this fixes). Re-pulled rows carry a fresh
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ SELECT
FROM {{ source('bronze_m365', 'onedrive_activity') }}
WHERE userPrincipalName IS NOT NULL
AND userPrincipalName != ''
-- Drop unlicensed users (see #736 / teams feeder). This MS Graph report
-- (getOneDriveActivityUserDetail) exposes no `isLicensed`; the only license signal
-- is `assignedProducts` — the products assigned to the user, empty for unlicensed
-- accounts. Airbyte stores the array as a JSON string, so an empty list is '[]'.
-- Conservative on NULL/unknown (keep); drop only explicitly-empty product lists.
AND (
assignedProducts IS NULL
OR replaceRegexpAll(assignedProducts, '[[:space:]]', '') NOT IN ('', '[]')
)
{% if is_incremental() %}
-- Watermark on the source EXTRACT time, not the business date (see zoom model header
-- for the backfill-strand failure mode this fixes). Re-pulled rows carry a fresh
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ SELECT
FROM {{ source('bronze_m365', 'sharepoint_activity') }}
WHERE userPrincipalName IS NOT NULL
AND userPrincipalName != ''
-- Drop unlicensed users (see #736 / teams feeder). This MS Graph report
-- (getSharePointActivityUserDetail) exposes no `isLicensed`; the only license signal
-- is `assignedProducts` — the products assigned to the user, empty for unlicensed
-- accounts. Airbyte stores the array as a JSON string, so an empty list is '[]'.
-- Conservative on NULL/unknown (keep); drop only explicitly-empty product lists.
AND (
assignedProducts IS NULL
OR replaceRegexpAll(assignedProducts, '[[:space:]]', '') NOT IN ('', '[]')
)
{% if is_incremental() %}
-- Watermark on the source EXTRACT time, not the business date (see zoom model header
-- for the backfill-strand failure mode this fixes). Re-pulled rows carry a fresh
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ SELECT
FROM {{ source('bronze_m365', 'email_activity') }}
WHERE userPrincipalName IS NOT NULL
AND userPrincipalName != ''
-- Drop unlicensed users (see #736 / teams feeder). This MS Graph report
-- (getEmailActivityUserDetail) exposes no `isLicensed`; the only license signal
-- is `assignedProducts` — the products assigned to the user, empty for unlicensed
-- accounts. Airbyte stores the array as a JSON string, so an empty list is '[]'.
-- Conservative on NULL/unknown (keep); drop only explicitly-empty product lists.
AND (
assignedProducts IS NULL
OR replaceRegexpAll(assignedProducts, '[[:space:]]', '') NOT IN ('', '[]')
)
{% if is_incremental() %}
-- Watermark on the source EXTRACT time, not the business date (see zoom model header
-- for the backfill-strand failure mode this fixes). Re-pulled rows carry a fresh
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ SELECT
FROM {{ source('bronze_m365', 'teams_activity') }}
WHERE userPrincipalName IS NOT NULL
AND userPrincipalName != ''
-- Drop unlicensed users (guests, ex-employees, service accounts). Their Teams
-- activity inflates team-level collab counters and produces orphan gold rows
-- with no matching insight.people entry. `isLicensed` = "Selected if the user
-- is licensed to use Teams" (MS Graph getTeamsUserActivityUserDetail). Only the
-- teams_activity report exposes this flag; the email/onedrive/sharepoint feeders
-- fall back to `assignedProducts`. Conservative on NULL (unknown -> keep). See #736.
AND coalesce(isLicensed, true) = true
Comment on lines +45 to +51

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== candidate files =="
git ls-files | rg 'src/ingestion/connectors/collaboration/m365/dbt/(m365__collab_meeting_activity|m365__collab_chat_activity|m365__collab_email_activity|m365__collab_document_activity_onedrive|m365__collab_document_activity_sharepoint)\.sql$'

echo
echo "== file outlines/contents relevant lines =="
for f in \
  src/ingestion/connectors/collaboration/m365/dbt/m365__collab_meeting_activity.sql \
  src/ingestion/connectors/collaboration/m365/dbt/m365__collab_chat_activity.sql \
  src/ingestion/connectors/collaboration/m365/dbt/m365__collab_email_activity.sql \
  src/ingestion/connectors/collaboration/m365/dbt/m365__collab_document_activity_onedrive.sql \
  src/ingestion/connectors/collaboration/m365/dbt/m365__collab_document_activity_sharepoint.sql
do
  echo "--- $f ---"
  wc -l "$f"
  sed -n '1,140p' "$f" | cat -n
done

echo
echo "== dbt incremental / unique keys across repo =="
rg -n "{% set|unique_key|incremental_strategy|insert_overwrite|merge|where\s*\(\).*incremental|unique_key|unique_key" . -g '*.sql' -g '*.yml' -g '*.yaml' | head -n 250

Repository: constructorfabric/insight

Length of output: 20094


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== candidate files =="
git ls-files | rg 'src/ingestion/connectors/collaboration/m365/dbt/(m365__collab_meeting_activity|m365__collab_chat_activity|m365__collab_email_activity|m365__collab_document_activity_onedrive|m365__collab_document_activity_sharepoint)\.sql$'

echo
echo "== file relevant contents =="
for f in \
  src/ingestion/connectors/collaboration/m365/dbt/m365__collab_meeting_activity.sql \
  src/ingestion/connectors/collaboration/m365/dbt/m365__collab_chat_activity.sql \
  src/ingestion/connectors/collaboration/m365/dbt/m365__collab_email_activity.sql \
  src/ingestion/connectors/collaboration/m365/dbt/m365__collab_document_activity_onedrive.sql \
  src/ingestion/connectors/collaboration/m365/dbt/m365__collab_document_activity_sharepoint.sql
do
  echo "--- $f ---"
  wc -l "$f"
  sed -n '1,140p' "$f" | cat -n
done

echo
echo "== dbt incremental / unique key indicators =="
rg -n "{% set|unique_key|incremental_strategy|insert_overwrite|merge|where\s*\(\).*incremental|unique_key|unique_key" . -g '*.sql' -g '*.yml' -g '*.yaml' | head -n 250 || true

Repository: constructorfabric/insight

Length of output: 20077


Add a one-time cleanup for newly excluded unlicensed activity.

These models are materialized='incremental', so the new license predicates filter future source selections but do not remove rows already materialized for isLicensed = false or explicitly empty assignedProducts. Add a cleanup/full refresh or explicit incremental deletion for all five models.

📍 Affects 5 files
  • src/ingestion/connectors/collaboration/m365/dbt/m365__collab_meeting_activity.sql#L45-L51 (this comment)
  • src/ingestion/connectors/collaboration/m365/dbt/m365__collab_chat_activity.sql#L70-L76
  • src/ingestion/connectors/collaboration/m365/dbt/m365__collab_email_activity.sql#L34-L42
  • src/ingestion/connectors/collaboration/m365/dbt/m365__collab_document_activity_onedrive.sql#L39-L47
  • src/ingestion/connectors/collaboration/m365/dbt/m365__collab_document_activity_sharepoint.sql#L39-L47
🤖 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
`@src/ingestion/connectors/collaboration/m365/dbt/m365__collab_meeting_activity.sql`
around lines 45 - 51, Add a one-time cleanup or explicit incremental deletion
for rows now excluded by the new license predicates, covering all five
incremental models:
src/ingestion/connectors/collaboration/m365/dbt/m365__collab_meeting_activity.sql
(lines 45-51), m365__collab_chat_activity.sql (lines 70-76),
m365__collab_email_activity.sql (lines 34-42),
m365__collab_document_activity_onedrive.sql (lines 39-47), and
m365__collab_document_activity_sharepoint.sql (lines 39-47). Remove previously
materialized activity with isLicensed = false or explicitly empty
assignedProducts while preserving the new filtering for future incremental runs.

{% if is_incremental() %}
-- Watermark on the source EXTRACT time, not the business date: a forward-only
-- `reportRefreshDate > max(date) - 3d` filter permanently strands backfilled /
Expand Down
3 changes: 3 additions & 0 deletions src/ingestion/scripts/create-bronze-placeholders.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1243,6 +1243,7 @@ CREATE TABLE IF NOT EXISTS bronze_m365.teams_activity (
reportRefreshDate Nullable(String),
reportPeriod Nullable(String),
lastActivityDate Nullable(String),
isLicensed Nullable(Bool),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Reconcile existing M365 bronze tables, not only fresh placeholders.

These columns are added only when the table is created. On a warm ClickHouse/E2E database, ch_table_exists skips the CREATE TABLE, leaving older tables without isLicensed or assignedProducts; the new dbt models then fail with UNKNOWN_IDENTIFIER. Add idempotent ALTER TABLE ... ADD COLUMN IF NOT EXISTS branches for existing tables.

Also applies to: 1310-1310, 1334-1334

🤖 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 `@src/ingestion/scripts/create-bronze-placeholders.sh` at line 1246, Add
idempotent ALTER TABLE ... ADD COLUMN IF NOT EXISTS handling for isLicensed and
assignedProducts in the existing-table branches of the M365 bronze table setup,
including the paths corresponding to all three referenced column definitions.
Preserve the current CREATE TABLE definitions for fresh tables while ensuring
warm databases reconcile missing columns before dbt models query them.

teamChatMessageCount Nullable(Decimal(38, 9)),
privateChatMessageCount Nullable(Decimal(38, 9)),
postMessages Nullable(Decimal(38, 9)),
Expand Down Expand Up @@ -1306,6 +1307,7 @@ CREATE TABLE IF NOT EXISTS bronze_m365.onedrive_activity (
reportRefreshDate Nullable(String),
reportPeriod Nullable(String),
lastActivityDate Nullable(String),
assignedProducts Nullable(String),
viewedOrEditedFileCount Nullable(Decimal(38, 9)),
syncedFileCount Nullable(Decimal(38, 9)),
sharedInternallyFileCount Nullable(Decimal(38, 9)),
Expand All @@ -1329,6 +1331,7 @@ CREATE TABLE IF NOT EXISTS bronze_m365.sharepoint_activity (
reportRefreshDate Nullable(String),
reportPeriod Nullable(String),
lastActivityDate Nullable(String),
assignedProducts Nullable(String),
viewedOrEditedFileCount Nullable(Decimal(38, 9)),
syncedFileCount Nullable(Decimal(38, 9)),
sharedInternallyFileCount Nullable(Decimal(38, 9)),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
spec_version: 1
description: >
Regression: unlicensed M365 users must NOT leak into collaboration metrics (#736). #magic___^_^___line The M365 teams_activity report includes unlicensed users (guests, ex-employees, service accounts) flagged `isLicensed=false`. The silver feeder m365__collab_meeting_activity now filters them out with `coalesce(isLicensed, true) = true`, so their activity never reaches gold. #magic___^_^___line Setup — two Engineering people, both with a Teams meeting row dated 2026-12-25:
• alice — isLicensed=true, meetingsAttendedCount=3 → licensed CONTROL, value 3
• frank — isLicensed=false, meetingsAttendedCount=99 → unlicensed, EXCLUDED (null)

If the license filter regressed, frank's period value would be 99 instead of null and this test would fail. alice proves the pipeline itself is healthy, so a failure isolates to the license filter rather than a broken build.

bronze:
bronze_bamboohr.employees:
- $ref: templates/people.yaml#/templates/alice
- {$ref: templates/people.yaml#/templates/bamboohr_employee, id: e099, unique_key: bamboohr-test-e099, firstName: Frank, lastName: Guest, displayName: Frank Guest, workEmail: frank@example.com,}

bronze_m365.teams_activity:
- {$ref: templates/m365_teams.yaml#/templates/m365_teams, userPrincipalName: alice@example.com, reportRefreshDate: "2026-12-25", unique_key: tm-alice-20261225, isLicensed: true, meetingsAttendedCount: 3}
- {$ref: templates/m365_teams.yaml#/templates/m365_teams, userPrincipalName: frank@example.com, reportRefreshDate: "2026-12-25", unique_key: tm-frank-20261225, isLicensed: false, meetingsAttendedCount: 99,}

cases:
- name: Unlicensed user excluded, licensed control present
request:
url: /v1/metric-results
method: POST
body:
entity: {type: person, ids: [alice@example.com, frank@example.com]}
period: {from: "2026-12-01", to: "2026-12-31"}
metrics:
- metric_key: collab.meetings_count
views:
- {view: period}
expect:
- assert: "status == 200"
# Licensed control: activity flows through as usual.
- metric: collab.meetings_count
view: period
find: {entity_id: alice@example.com}
equal: {value: 3}
# Unlicensed user: filtered out at silver → no gold observation → null.
- metric: collab.meetings_count
view: period
find: {entity_id: frank@example.com}
equal: {value: null}
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,19 @@ schemas:
type: object
additionalProperties: false
properties:
_airbyte_raw_id: { type: string }
_airbyte_extracted_at: { type: string, format: date-time }
_airbyte_meta: { type: string }
_airbyte_generation_id: { type: integer }
tenant_id: { type: [string, "null"] }
source_id: { type: [string, "null"] }
unique_key: { type: [string, "null"] }
userPrincipalName: { type: [string, "null"] }
reportRefreshDate: { type: [string, "null"] }
reportPeriod: { type: [string, "null"] }
lastActivityDate: { type: [string, "null"] }
viewedOrEditedFileCount: { type: [number, "null"] }
syncedFileCount: { type: [number, "null"] }
sharedInternallyFileCount: { type: [number, "null"] }
sharedExternallyFileCount: { type: [number, "null"] }
_airbyte_raw_id: {type: string}
_airbyte_extracted_at: {type: string, format: date-time}
_airbyte_meta: {type: string}
_airbyte_generation_id: {type: integer}
tenant_id: {type: [string, "null"]}
source_id: {type: [string, "null"]}
unique_key: {type: [string, "null"]}
userPrincipalName: {type: [string, "null"]}
reportRefreshDate: {type: [string, "null"]}
reportPeriod: {type: [string, "null"]}
lastActivityDate: {type: [string, "null"]}
assignedProducts: {type: [string, "null"]}
viewedOrEditedFileCount: {type: [number, "null"]}
syncedFileCount: {type: [number, "null"]}
sharedInternallyFileCount: {type: [number, "null"]}
sharedExternallyFileCount: {type: [number, "null"]}
Original file line number Diff line number Diff line change
Expand Up @@ -8,31 +8,32 @@ schemas:
type: object
additionalProperties: false
properties:
_airbyte_raw_id: { type: string }
_airbyte_extracted_at: { type: string, format: date-time }
_airbyte_meta: { type: string }
_airbyte_generation_id: { type: integer }
tenant_id: { type: [string, "null"] }
source_id: { type: [string, "null"] }
unique_key: { type: [string, "null"] }
userPrincipalName: { type: [string, "null"] }
reportRefreshDate: { type: [string, "null"] }
reportPeriod: { type: [string, "null"] }
lastActivityDate: { type: [string, "null"] }
teamChatMessageCount: { type: [number, "null"] }
privateChatMessageCount: { type: [number, "null"] }
postMessages: { type: [number, "null"] }
replyMessages: { type: [number, "null"] }
urgentMessages: { type: [number, "null"] }
callCount: { type: [number, "null"] }
meetingsAttendedCount: { type: [number, "null"] }
meetingsOrganizedCount: { type: [number, "null"] }
adHocMeetingsAttendedCount: { type: [number, "null"] }
adHocMeetingsOrganizedCount: { type: [number, "null"] }
scheduledOneTimeMeetingsAttendedCount: { type: [number, "null"] }
scheduledOneTimeMeetingsOrganizedCount: { type: [number, "null"] }
scheduledRecurringMeetingsAttendedCount: { type: [number, "null"] }
scheduledRecurringMeetingsOrganizedCount: { type: [number, "null"] }
audioDuration: { type: [string, "null"] }
videoDuration: { type: [string, "null"] }
screenShareDuration: { type: [string, "null"] }
_airbyte_raw_id: {type: string}
_airbyte_extracted_at: {type: string, format: date-time}
_airbyte_meta: {type: string}
_airbyte_generation_id: {type: integer}
tenant_id: {type: [string, "null"]}
source_id: {type: [string, "null"]}
unique_key: {type: [string, "null"]}
userPrincipalName: {type: [string, "null"]}
reportRefreshDate: {type: [string, "null"]}
reportPeriod: {type: [string, "null"]}
lastActivityDate: {type: [string, "null"]}
isLicensed: {type: [boolean, "null"]}
teamChatMessageCount: {type: [number, "null"]}
privateChatMessageCount: {type: [number, "null"]}
postMessages: {type: [number, "null"]}
replyMessages: {type: [number, "null"]}
urgentMessages: {type: [number, "null"]}
callCount: {type: [number, "null"]}
meetingsAttendedCount: {type: [number, "null"]}
meetingsOrganizedCount: {type: [number, "null"]}
adHocMeetingsAttendedCount: {type: [number, "null"]}
adHocMeetingsOrganizedCount: {type: [number, "null"]}
scheduledOneTimeMeetingsAttendedCount: {type: [number, "null"]}
scheduledOneTimeMeetingsOrganizedCount: {type: [number, "null"]}
scheduledRecurringMeetingsAttendedCount: {type: [number, "null"]}
scheduledRecurringMeetingsOrganizedCount: {type: [number, "null"]}
audioDuration: {type: [string, "null"]}
videoDuration: {type: [string, "null"]}
screenShareDuration: {type: [string, "null"]}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ templates:
reportPeriod: "D1"
reportRefreshDate: null
lastActivityDate: null
assignedProducts: null
viewedOrEditedFileCount: null
syncedFileCount: null
sharedInternallyFileCount: null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ templates:
reportPeriod: "D1"
reportRefreshDate: null
lastActivityDate: null
isLicensed: null
teamChatMessageCount: null
privateChatMessageCount: null
postMessages: null
Expand Down
Loading