Skip to content

knowledge(events): database trigger setup flags may only be set to true - #213

Open
Michael Dieringer (MichaelDieringer) wants to merge 2 commits into
microsoft:mainfrom
Curabis:community-contribution/r4-events
Open

Michael Dieringer (MichaelDieringer) wants to merge 2 commits into
microsoft:mainfrom
Curabis:community-contribution/r4-events

Conversation

@MichaelDieringer

Copy link
Copy Markdown
Contributor

Summary

  • events/database-trigger-setup-flags-may-only-be-set-to-true. GetDatabaseTableTriggerSetup on the system codeunit Global Triggers and OnAfterGetDatabaseTableTriggerSetup on codeunit 49 GlobalTriggerManagement pass the same four var Boolean flags through every subscriber, and subscribers run in no particular order. A subscriber that assigns false, or a lookup result such as OnDatabaseModify := MySetup.Get(TableId), can clear flags set by Dataverse sync, API webhooks, data archive, or another app. A direct Global Triggers subscriber can also clear the change log flags. The compiler does not catch this.
    • Change log protection is narrower than it looks. GlobalTriggerManagement.Codeunit.al lines 51-65: codeunit 49 asks the change log last, and only in the normal execution context (62-64), next to its "don't allow anyone to disable change log" comment. That protects only against OnAfterGetDatabaseTableTriggerSetup subscribers.
    • BCApps subscribers only ever set the flags to true: Change Log Management 85-88 (with or), API webhooks 224-227, CRM 3748-3753, Master Data Management 1511-1516, Data Archive 27-32, GP migration 11-21. The no-op if not Delete then Delete := false; is carved out.
    • Two places in BCApps do clear flags, and the article names them: the demo data tool (W1 343-344, CZ 353, IN 357) and the Backup Management test library (470-476). They are the same mechanism in demo and test sessions, where clearing is acceptable. The review check carves them out.
    • One premise is inferred, and the article says so. Learn does not state that the events are raised only when the flag is true. The article infers it from:
      • Learn's integration-record refactoring page: subscribing to global triggers disables bulk SQL insert, modify and delete for that specific table.
      • The per-table cache comment in the BCApps change log test.
      • No Transactions Subscriber, which turns on every flag in order to see every write.
    • Recommended path: codeunit 49's integration events, following Learn's transition guidance against subscribing directly to system codeunits 2000000001..2000000010. When the set of tables is known, use table-specific mechanisms instead.
  • Wiring: added to al-events-review (tokens plus an event-design check) and registered in the events review-fixtures override.
  • Samples: good and bad samples checked with AL compiler 30.0 against Base Application 28.4 symbols.

Applicable versions: 15 and later.

Test plan

  • validate_frontmatter.py: 0 errors (2 warnings, both in files this PR doesn't touch)
  • Test-KnowledgeIndex.ps1, Test-SkillIndex.ps1, Test-KnowledgeRetrieval.ps1, Test-ReviewContract.ps1
  • Test-ReviewFixtures.ps1: 230 cases

🤖 Generated with Claude Code

Add an events article with compiled good/bad samples:
GetDatabaseTableTriggerSetup (Global Triggers) and
OnAfterGetDatabaseTableTriggerSetup (GlobalTriggerManagement) share four
var Booleans across all subscribers, which run in no particular order.
Assigning false, or a lookup result without or-ing in the current value,
clears flags other features set (Dataverse sync, API webhooks, data
archive, and, for direct Global Triggers subscribers, the change log).
Recommends the codeunit 49 integration events per Learn's guidance on
system codeunits 2000000001..2000000010 and handler-side table filtering.

Wired into al-events-review tokens and an event-design check, with
carve-outs for conditional := true, Flag := Flag or ..., and the no-op
"if not Flag then Flag := false" found in BCApps. Registered in the
events review-fixtures override.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…icle

- Name the BCApps code that clears flags (demo data tool W1/CZ/IN,
  Backup Management test library) as the mechanism in demo/test-only
  sessions; carve such code out of the al-events-review check.
- Label "events are raised only when the flag is true" as inferred and
  cite the supporting sources (Learn integration-record refactoring on
  disabled bulk SQL operations, ChangeLog test cache comment,
  No Transactions Subscriber); fold the bulk-SQL cost into Best Practice.
- Drop the "handler without opt-in" signal from article and check.
- Qualify change log protection as normal execution context only.
- Drop the unverifiable codeunit ID; describe Global Triggers by its
  2000000001..2000000010 range and mark the transition page as v14-era.
- Order-dependent wording for flag overwrites; add GP and
  No Transactions as further direct subscribers; link the references.

Samples rechecked with alc 30.0 against Base Application 28.4 symbols.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.

1 participant