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 @@ -72,8 +72,9 @@ WHERE userPrincipalName IS NOT NULL
-- 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
-- fall back to `assignedProducts`. A NULL flag is treated as unlicensed and
-- dropped too, per the #736 proposal. See #736.
AND coalesce(isLicensed, false) = 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 @@ -47,8 +47,9 @@ WHERE userPrincipalName IS NOT NULL
-- 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
-- fall back to `assignedProducts`. A NULL flag is treated as unlicensed and
-- dropped too, per the #736 proposal. See #736.
AND coalesce(isLicensed, false) = true
{% 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
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
name: m365
version: "2026.05.04"
# 1.0.0 (semver migration): adopt strict MAJOR.MINOR.PATCH (ADR-0015). The prior
# value "2026.05.04" is not semver, so reconcile classifies current→1.0.0 as a
# `migration` (republish + catalog re-discover, NO full-refresh). This only
# establishes the semver baseline. A follow-up MAJOR bump (1.0.0 → 2.0.0) will
# dispatch the one-shot `dbt --full-refresh` over `dbt_select` (tag:m365+) that
# purges collab rows materialized before the #736 unlicensed-user filter landed.
version: "1.0.0"
Comment on lines +2 to +8

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Update the documented reconcile baseline.

src/ingestion/README.md still describes 2026.05.04 as the current baseline, which will contradict this descriptor’s 1.0.0 version after merge. Update the operator-facing documentation in the same change.

🤖 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/descriptor.yaml` around lines 2 -
8, Update the operator-facing reconcile baseline in ingestion README
documentation from 2026.05.04 to 1.0.0, keeping it consistent with the
descriptor’s version and preserving the surrounding migration guidance.

schedule: 0 2 * * *
dbt_select: tag:m365+
workflow: sync
Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,30 @@
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)
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 filters them out with coalesce(isLicensed, false) = true, so an unlicensed OR unknown-license user never reaches gold; only an explicit isLicensed=true row is kept. #magic___^_^___line Setup — three Engineering people, each 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)
• grace — isLicensed=null, meetingsAttendedCount=77 → unknown, 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.
If the filter regressed, frank/grace would surface non-null values and this test would fail. alice proves the pipeline itself is healthy, so a failure isolates to the license predicate 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,}
- {$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}
- {$ref: templates/people.yaml#/templates/bamboohr_employee, id: e100, unique_key: bamboohr-test-e100, firstName: Grace, lastName: Ghost, displayName: Grace Ghost, workEmail: grace@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,}
- {$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}
- {$ref: templates/m365_teams.yaml#/templates/m365_teams, userPrincipalName: grace@example.com, reportRefreshDate: "2026-12-25", unique_key: tm-grace-20261225, isLicensed: null, meetingsAttendedCount: 77,}

cases:
- name: Unlicensed user excluded, licensed control present
- name: Unlicensed and unknown-license users excluded, licensed control present
request:
url: /v1/metric-results
method: POST
body:
entity: {type: person, ids: [alice@example.com, frank@example.com]}
entity: {type: person, ids: [alice@example.com, frank@example.com, grace@example.com]}
period: {from: "2026-12-01", to: "2026-12-31"}
metrics:
- metric_key: collab.meetings_count
Expand All @@ -34,8 +37,13 @@ cases:
view: period
find: {entity_id: alice@example.com}
equal: {value: 3}
# Unlicensed user: filtered out at silver → no gold observation → null.
# Explicitly unlicensed: filtered out at silver → no gold observation → null.
- metric: collab.meetings_count
view: period
find: {entity_id: frank@example.com}
equal: {value: null}
# Unknown license (isLicensed=null): coalesce(...,false) drops it too → null.
- metric: collab.meetings_count
view: period
find: {entity_id: grace@example.com}
equal: {value: null}
6 changes: 5 additions & 1 deletion src/ingestion/tests/e2e/metrics/templates/m365_teams.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,11 @@ templates:
reportPeriod: "D1"
reportRefreshDate: null
lastActivityDate: null
isLicensed: null
# Default to licensed: the meeting/chat feeders drop rows where
# coalesce(isLicensed, false) != true, so an unset flag would otherwise
# exclude every fixture row. Tests exercising the unlicensed path set this
# explicitly to false (or null). See #736.
isLicensed: true

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.

let's check all cases - true/false/null

teamChatMessageCount: null
privateChatMessageCount: null
postMessages: null
Expand Down
Loading