Skip to content

feat!: remove the BI event-type override - #14

Open
fateeand wants to merge 5 commits into
mainfrom
13-remove-the-bi-event-type-override
Open

fateeand wants to merge 5 commits into
mainfrom
13-remove-the-bi-event-type-override

Conversation

@fateeand

@fateeand fateeand commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Removes the per-event eventType override from UwtBITelemetryService.track(). Every BI event is now recorded under the single {ns}.bi type, with eventName as a field.

The override existed so dashboards built on older event types could keep working during migration. We don't need that. It also let an event leave the one-type model, and a custom type ending in .bi couldn't be told apart from a standard BI event.

Changes

  • API (breaking): UwtBIEventDetail no longer has eventType, only scenarioId and feature. It's now the single interface for these fields; the unexported UwtBIEventCorrelation it aliased is removed, and UwtBIEvent extends UwtBIEventDetail.
  • BI service: always records {ns}.bi. The event type is no longer part of the duplicate-detection key.
  • Docs: DESIGN.md's "One event type" section loses the override example, and the duplicate-detection rule, exports table and classifier docs are updated. The classifier's unknown kind is now described as "any other event type".
  • Tests: the override tests are replaced with one checking that BI events always use {ns}.bi. The delimiter-collision test now uses scenarioId instead of eventType.

Breaking change

Callers passing eventType in track()'s third argument get a TypeScript error and must remove it. Those events are then recorded as {ns}.bi.


Closes #13

@fateeand fateeand linked an issue Oct 16, 2026 that may be closed by this pull request
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Coverage report

St.❔
Category Percentage Covered / Total
🟢 Statements
96.98% (-0% 🔻)
1221/1259
🟢 Branches
95.4% (-0.03% 🔻)
663/695
🟢 Functions 96.84% 306/316
🟢 Lines
97.36% (-0% 🔻)
1179/1211
Show files with reduced coverage 🔻
St.❔
File Statements Branches Functions Lines
🟢
... / uwt-bi-telemetry.service.ts
100%
96.15% (-0.51% 🔻)
100% 100%

Test suite run success

723 tests passing in 23 suites.

Report generated by 🧪jest coverage report action from bb92283

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

One or more issues must be addressed before approval.

0 open findings

What changed in this PR

Removes per-event BI event-type overrides so all BI events use {namespace}.bi, updates deduplication and public models, and aligns documentation/tests.

Changes:

  • Simplifies UwtBIEventDetail and UwtBIEvent.
  • Always emits BI events under the namespace BI type.
  • Updates deduplication, tests, and documentation.
File Description
projects/​ngx-ui-watchtower/​src/​lib/​utils/​uwt-telemetry-event.util/​uwt-telemetry-event.util.ts Updated as part of this pull request.
projects/​ngx-ui-watchtower/​src/​lib/​services/​uwt-bi-telemetry.service/​uwt-bi-telemetry.service.ts Updated as part of this pull request.
projects/​ngx-ui-watchtower/​src/​lib/​services/​uwt-bi-telemetry.service/​uwt-bi-telemetry.service.spec.ts Updated as part of this pull request.
projects/​ngx-ui-watchtower/​src/​lib/​models/​uwt-bi.models/​uwt-bi.models.ts Updated as part of this pull request.
projects/​ngx-ui-watchtower/​src/​lib/​config/​uwt-bi-telemetry.config/​uwt-bi-telemetry.config.ts Updated as part of this pull request.
projects/​ngx-ui-watchtower/​DESIGN.md Updated as part of this pull request.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@fateeand
fateeand marked this pull request as ready for review October 19, 2026 08:11
@fateeand
fateeand requested a lite review from Copilot October 19, 2026 08:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Add test coverage confirming differing legacy overrides now deduplicate into one event.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Test deduplication when legacy eventType values differ

projects/​ngx-ui-watchtower/​src/​lib/​services/​uwt-bi-telemetry.service/​uwt-bi-telemetry.service.spec.ts:663

The production key no longer includes eventType, but the replacement test only verifies the emitted type for a single legacy-shaped call. It does not protect the changed deduplication contract: two otherwise identical events carrying different legacy eventType values must now collapse to one. Please add a second track call with a different override and assert that only one event is recorded.

Review effort: Lite

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

No unresolved review comments remain, and all reviewed changes are aligned.

No open issues 🎉

Review effort: Lite

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.

Remove the BI event-type override

2 participants