Skip to content

Resolve BCQuality #208 merge conflicts - #5

Merged
MichaelDieringer merged 8 commits into
Curabis:community-contribution/tablerelation-length-and-recordref-tempfrom
microsoft:resolve-pr-208-conflicts
Oct 5, 2026
Merged

MichaelDieringer merged 8 commits into
Curabis:community-contribution/tablerelation-length-and-recordref-tempfrom
microsoft:resolve-pr-208-conflicts

Conversation

@JesperSchulz

Copy link
Copy Markdown

This sync merges current microsoft/BCQuality main (02e7ab15b040be712ff8b694f65ae82257aac0db) into the source branch of microsoft#208, preserving the approved TableRelation and RecordRef guidance.

Validated merge commit: 1d140cb4fee0cf2962bd6c3fb20307832b910c43.

The only textual conflict was in al-data-modeling-review.md; its applicability, tokens and not-applicable definition were resolved as a union of main's master insert/delete guidance and this branch's TableRelation field-length guidance. All six canonical knowledge/sample files and the testing skill remain identical to the approved head. The manifest retains every main registration plus the two approved pairs.

Frontmatter, actual changed-path review-fixture preparation (248 cases), review contract, skill/schema, knowledge index/retrieval and git diff --check passed. The final diff against main contains exactly the nine original PR paths.

Please merge this sync PR using Create a merge commit, rather than squash-merging, to retain the main ancestry and clear the original PR's conflicts. Once the source branch is synchronized, microsoft#208 is ready for the maintainers' merge. We cannot push directly to the fork because GitHub rejects the maintainer push with HTTP 403.

MichaelDieringer and others added 8 commits October 5, 2026 14:42
…ord should take it as var Record (#216)

Adds mutating-procedure-for-a-page-caller-takes-var-record with good/bad
samples, a worklist cue in al-style-review, and registration in the style
review-fixtures override.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…eclined Confirm in OnValidate (#209)

* Add trigger-default and declined-Confirm knowledge with review cues

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 <noreply@anthropic.com>

* Tighten declined-confirm, delete and insert trigger articles after review

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 89dba8c8-6529-4b60-956f-875a59be499d
… for distinct values and duplicates (#215)

Adds use-grouped-query-for-distinct-values-and-duplicates with good/bad
samples, a worklist cue in al-performance-review, and registration in the
performance review-fixtures override.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…ue (#213)

* knowledge(events): database trigger setup flags may only be set to true

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 <noreply@anthropic.com>

* knowledge(events): address review of database trigger setup flags article

- 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 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Reject fragment-bearing finding IDs and cap uncited confidence and severity in the shared schema. Require final-source verification before emission and cover leaf/root compatibility and fail-closed source bounds.

Co-authored-by: wenjiefan <wenjiefan@microsoft.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* Add retention policy knowledge to the privacy domain

Two articles covering retention policies for extension-owned tables,
the gap that lets high-volume log tables grow unbounded:

- register-owned-log-tables-for-retention-policies: an extension's own
  log tables must be added to the allowed-tables list from install AND
  upgrade code, guarded by IsAllowedTable plus an upgrade tag, with a
  mandatory minimum retention where audit needs one.
- ship-a-default-retention-policy-setup: registration only makes a table
  selectable; nothing is deleted until a Retention Policy Setup record
  exists, so ship one (disabled by default) as the platform's own
  Retention Policy Installer does.

Each ships good/bad AL samples. Claims verified against the BC admin
docs and the Retention Policy module in microsoft/BCApps.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011gvTjm746MtJEVWeRTbG46

* Address review feedback on retention policy knowledge

- Scope both articles to bc-version [17..] (retention policies shipped in v17).
- Allowed-tables sample: add OnRefreshAllowedTables subscriber with a
  ForceUpdate path; the upgrade tag now gates one-time setup only.
- Default-policy sample: use Retention Policy Setup.FindOrCreateRetentionPeriod
  instead of a hand-rolled lookup-then-insert that can collide on code.
- Anti-pattern now keys on append-only tables rather than table names.
- al-privacy-review: add retention-policy tokens and deterministic routing
  for both articles, with a bounded per-table text search for delete and
  registration paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Address second review round on retention policy knowledge

- Scope both articles to bc-version [22..]: FindOrCreateRetentionPeriod
  first appears in the 21.1 System Application and OnRefreshAllowedTables
  in 22.
- Reframe the default-setup article as optional guidance; registration
  without a setup is valid. The anti-pattern and review routing now cover
  only false claims that registration alone cleans up data.
- Make all four samples self-contained: declare Contoso Activity Log in
  each, add the Retention Policy Setup permission, and guard the default
  setup on IsAllowedTable.
- Let evaluation overrides list additionalArticles and register both
  retention pairs as extra privacy cases (38 cases, existing IDs unchanged).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Revert evaluation harness change for multiple articles per domain

The harness intentionally evaluates one paired article per domain. Keep it
as designed; how the retention pairs join privacy evaluation is left to the
maintainers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Register retention policy pairs in the privacy evaluation override

Use main's articles override so both retention article pairs get positive
and clean cases alongside no-pii-in-telemetry-message-string.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Jeremy Vyska <jeremy@sparebrained.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolve data-modeling applicability, token, and not-applicable conflicts by retaining TableRelation guidance alongside master-record insert/delete routing. Preserve approved canonical knowledge and samples, RecordRef exclusions, and all fixture registrations.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@MichaelDieringer
MichaelDieringer merged commit 9409fa5 into Curabis:community-contribution/tablerelation-length-and-recordref-temp Oct 5, 2026
2 checks passed
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.

4 participants