From 803fe95fb107578ca10e05f1d0c6547ee4c47ebc Mon Sep 17 00:00:00 2001 From: Roman Mitasov Date: Thu, 23 Jul 2026 20:35:02 +0800 Subject: [PATCH] fix(m365): treat unknown Teams license as unlicensed; adopt semver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #736 (shipped in #1869), addressing the CodeRabbit review. 1. Teams feeders (meeting, chat): switch `coalesce(isLicensed, true)` to `coalesce(isLicensed, false) = true`, so rows with a NULL/unknown license flag are dropped alongside explicit `isLicensed=false` — matching the #736 proposal. The e2e teams fixture template now defaults `isLicensed: true` so existing fixtures stay counted; the regression test adds an `isLicensed=null` case asserting it is excluded. The email/onedrive/sharepoint feeders keep their conservative `assignedProducts` filter (drop only explicitly-empty lists, keep NULL): those reports expose no `isLicensed`, and the JSON-string serialization is inferred rather than verified, so a NULL there means "unknown/unsynced" and must not nuke the whole feeder. 2. descriptor.yaml: migrate version "2026.05.04" -> "1.0.0" (strict semver, ADR-0015). Because the current deployed value is non-semver, reconcile classifies this as a `migration` (republish + catalog re-discover, NO full-refresh) — it 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 filter landed, since incremental models do not retro-delete already-written rows. Refs #736. Signed-off-by: Roman Mitasov Co-Authored-By: Claude Opus 4.8 --- .../m365/dbt/m365__collab_chat_activity.sql | 5 ++-- .../dbt/m365__collab_meeting_activity.sql | 5 ++-- .../collaboration/m365/descriptor.yaml | 8 +++++- ...llab_meeting_unlicensed_excluded.test.yaml | 26 ++++++++++++------- .../e2e/metrics/templates/m365_teams.yaml | 6 ++++- 5 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/ingestion/connectors/collaboration/m365/dbt/m365__collab_chat_activity.sql b/src/ingestion/connectors/collaboration/m365/dbt/m365__collab_chat_activity.sql index 2c9b42561..a82825b28 100644 --- a/src/ingestion/connectors/collaboration/m365/dbt/m365__collab_chat_activity.sql +++ b/src/ingestion/connectors/collaboration/m365/dbt/m365__collab_chat_activity.sql @@ -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 diff --git a/src/ingestion/connectors/collaboration/m365/dbt/m365__collab_meeting_activity.sql b/src/ingestion/connectors/collaboration/m365/dbt/m365__collab_meeting_activity.sql index 79ddabcf3..a8b8d7aa4 100644 --- a/src/ingestion/connectors/collaboration/m365/dbt/m365__collab_meeting_activity.sql +++ b/src/ingestion/connectors/collaboration/m365/dbt/m365__collab_meeting_activity.sql @@ -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 / diff --git a/src/ingestion/connectors/collaboration/m365/descriptor.yaml b/src/ingestion/connectors/collaboration/m365/descriptor.yaml index 68ce5e2ec..e9c99940a 100644 --- a/src/ingestion/connectors/collaboration/m365/descriptor.yaml +++ b/src/ingestion/connectors/collaboration/m365/descriptor.yaml @@ -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" schedule: 0 2 * * * dbt_select: tag:m365+ workflow: sync diff --git a/src/ingestion/tests/e2e/metrics/collab_meeting_unlicensed_excluded.test.yaml b/src/ingestion/tests/e2e/metrics/collab_meeting_unlicensed_excluded.test.yaml index 1e958cce1..c7c1d6773 100644 --- a/src/ingestion/tests/e2e/metrics/collab_meeting_unlicensed_excluded.test.yaml +++ b/src/ingestion/tests/e2e/metrics/collab_meeting_unlicensed_excluded.test.yaml @@ -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 @@ -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} diff --git a/src/ingestion/tests/e2e/metrics/templates/m365_teams.yaml b/src/ingestion/tests/e2e/metrics/templates/m365_teams.yaml index f1e978f38..c616726cf 100644 --- a/src/ingestion/tests/e2e/metrics/templates/m365_teams.yaml +++ b/src/ingestion/tests/e2e/metrics/templates/m365_teams.yaml @@ -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 teamChatMessageCount: null privateChatMessageCount: null postMessages: null