Skip to content

3 AL/BC patterns: Insert/Delete trigger defaults on master data and declined Confirm in OnValidate - #209

Open
Michael Dieringer (MichaelDieringer) wants to merge 3 commits into
microsoft:mainfrom
Curabis:community-contribution/runtrigger-and-declined-confirm
Open

Michael Dieringer (MichaelDieringer) wants to merge 3 commits into
microsoft:mainfrom
Curabis:community-contribution/runtrigger-and-declined-confirm

Conversation

@MichaelDieringer

@MichaelDieringer Michael Dieringer (MichaelDieringer) commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Three articles about silent defaults on record writes and confirmations. All three compile without a diagnostic and pass review by eye.

  • data-modeling/master-data-must-be-inserted-with-trigger.md
    • Record.Insert() does not run OnInsert, because RunTrigger defaults to false (Learn).
    • On a master table that skips the initialization: No./No. Series when No. is blank, the conditional contact and salesperson defaults, Global Dimension 1/2 overwrite, UpdateReferencedIds, and timestamps.
    • In BCApps W1 BaseApp, master-table inserts use Insert(true) 34 times against 6 bare Insert(). All 6 fall under the stated exceptions: temporary records, XMLport round-trips (ExportItemData:405, MfgExportItemData:415), CatalogItemManagement, and CopyItem.
    • Microsoft's DataMigration facades and Templ. Mgt. codeunits all use Insert(true), so migration from an external source is explicitly not an exception.
    • Framed as the "trigger does work the caller depends on" case of pass-false-to-insert-when-trigger-not-needed.md, so the decision stays per call. There is deliberately no Modify counterpart, because Base App uses Modify() without trigger routinely.
  • data-modeling/delete-master-data-with-trigger.md
    • Delete()/DeleteAll() without true, called from outside the owning table, skips its OnDelete guard. Example: Currency.OnDelete (lines 797-820) blocks deletion while open Cust./Vendor/Employee ledger entries exist, and Customer.OnDelete runs codeunit 361 MoveEntries.
    • A scan of all ~3,000 BaseApp delete calls found trigger-less deletes on these tables outside the owning table only in codeunit 1812. That is a no-postings data-migration rebuild and is listed as an exception.
    • Also out of scope:
      • the owning table's own cascade (Currency.OnDelete calls CurrExchRate.DeleteAll())
      • same-key delete-and-reinsert (JobArchiveManagement restore)
      • temporary records
    • Notes that table-extension OnBeforeDelete/OnAfterDelete still run (Learn).
  • error-handling/declined-confirm-must-abort-not-partially-apply.md
    • In OnValidate, the field already holds its new value. A declined Confirm that only skips a side effect therefore commits the new value while leaving state tied to the old value orphaned.
    • Scoped to side effects whose absence orphans state. Optional follow-ups are explicit non-findings: Sales Header "Do you want to update the lines?" → end else exit, and Opportunity."Campaign No.".
    • if not Confirm(...) then Error('') is shown as the standard abort (InteractionTemplate), and restoring xRec as the alternative (To-do "Team Code").
    • Links avoid-user-prompts-inside-transactions.md and job-queue-handlers-must-not-require-ui.md.

Relationship to existing articles: reconciled with and cross-linked to:

  • pass-false-to-insert-when-trigger-not-needed
  • master-table-no-from-number-series-in-oninsert
  • owning-table-must-delete-dependents-in-ondelete
  • do-not-modify-or-delete-posted-ledger-entries
  • datatransfer-skips-triggers-and-subscribers

None are restated.

Verification

  • Every fixture was compiled with alc 30.0 against Base App 28.2. The only warnings are AA0137 on two intentionally empty stub parameters in the Confirm sample.
  • Every BCApps citation was re-opened at main 837ef8024.
  • Learn pages checked: Record.Insert(Boolean), Delete, DeleteAll, OnValidate, Confirm.

Wiring

  • al-data-modeling-review.md and al-error-handling-review.md gain targeted checks.
  • Each check lists its exclusions in the cue itself: owning-table cascade, same-key reinsert, migration rebuild, XMLport round-trip, and optional-follow-up confirms.
  • All three are registered in evaluation/review-fixtures.json.

Test plan

  • validate_frontmatter.py: 0 errors (2 pre-existing warnings in unrelated files)
  • Test-ReviewFixtures.ps1: 222 cases / 111 paired articles
  • Test-ReviewContract.ps1, Test-SkillIndex.ps1, Test-KnowledgeIndex.ps1, Test-KnowledgeRetrieval.ps1: pass
  • CLA: license/cla check passes (CURABIS ApS company agreement on file)
  • Domain owner review (microsoft/knowledge/data-modeling/, microsoft/knowledge/error-handling/)

🤖 Generated with Claude Code

Three Microsoft-layer articles with good/bad samples:
- error-handling/declined-confirm-must-abort-not-partially-apply
- data-modeling/delete-master-data-with-trigger
- data-modeling/master-data-must-be-inserted-with-trigger

Wire targeted worklist cues into al-error-handling-review and
al-data-modeling-review and register the samples in review-fixtures.json.

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

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.

Reviewed 4e2a3ead4d246ef7f8bef74186c2c04219029379. The master insert/delete guidance accurately reflects trigger defaults and current Customer/Currency behavior while preserving temporary/staging, owning-table cascade, same-key restore, explicit initialization, and no-postings rebuild exceptions. The declined-confirm rule is scoped to provably orphaned old state, not optional follow-ups or safe pre-write cancellation, and the good sample aborts before the side effect. The package is consistent with existing performance/ownership guidance and all three pairs are registered. Exact-head local frontmatter, knowledge index/retrieval, skill/schema, review-contract, and changed-path fixture validation pass (222 cases). No merge-critical issue found. GitHub validation workflows are awaiting approval and should run before merge.

@JesperSchulz

Copy link
Copy Markdown
Contributor

Michael Dieringer (@MichaelDieringer), could you resolve the conflict? Looks good otherwise!



Insert-only conflicts in al-data-modeling-review.md (scope line, token list,
not-applicable list) and evaluation/review-fixtures.json: both sides kept.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@MichaelDieringer

Copy link
Copy Markdown
Contributor Author

Resolved, thanks! Merged current main (insert-only overlaps with #203/#207 in al-data-modeling-review.md and review-fixtures.json, both sides kept); all local validators pass.

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.

2 participants