Skip to content

Add integration event OnAfterGetRecordOnAfterConfirmAndModify in the General Posting Setup dataitem OnAfterGetRecord trigger after Modify. - #10334

Open
Jatin Kumar (Explorer986) wants to merge 1 commit into
mainfrom
private/kumarjatin/646562
Open

Add integration event OnAfterGetRecordOnAfterConfirmAndModify in the General Posting Setup dataitem OnAfterGetRecord trigger after Modify.#10334
Jatin Kumar (Explorer986) wants to merge 1 commit into
mainfrom
private/kumarjatin/646562

Conversation

@Explorer986

@Explorer986 Jatin Kumar (Explorer986) commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

What & why

Report 87 "Copy - General Posting Setup" only calls Modify when the user confirms, and there was no event inside that block. So, partners could not run follow up logic that should happen only on Yes and only after the record is modified.

This adds the integration event OnAfterGetRecordOnGeneralPostingSetupOnAfterConfirmAndModify right after Modify in the confirmation block. It is purely additive and changes no existing behavior.

Fixes AB#646562

How I validated this

  • I read the full diff and it contains only changes I intended.
  • I built the affected app(s) locally with no new analyzer warnings.
  • I ran the change in Business Central and confirmed it behaves as expected.
  • I added or updated tests for the new behavior or explained below why none are needed.

What I tested and the outcome

No test added since this is an additive event with no behavior change, following the usual practice for event requests.

Risk & compatibility

Low. Adds one integration event, nothing else. IncludeSender is true to match the sibling OnAfterCopyGenPostingSetup. No breaking changes.

@Explorer986
Jatin Kumar (Explorer986) requested a review from a team August 18, 2026 06:33
@github-actions github-actions Bot added the Finance GitHub request for Finance area label Aug 18, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Aug 18, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 1

Recommendation: Accept with Suggestions

Change type: Extensibility / integration-event request (low-risk, purely additive).

What it does: Adds OnAfterGetRecordOnGeneralPostingSetupOnAfterConfirmAndModify in report 87 "Copy - General Posting Setup", raised inside the confirm block right after Modify(). This lets partners run follow-up logic only when the user confirms and only after the target record is persisted. The declaration mirrors the sibling OnAfterCopyGenPostingSetup: [IntegrationEvent(true, false)] (IncludeSender = true), var ToGeneralPostingSetup + FromGeneralPostingSetup, with XML doc comments.

Assessment: The placement is correct (after Modify(), inside the if ConfirmManagement.GetResponseOrDefault(...) block), the begin..end wrapping is right, and the signature/convention matches the existing event in the same object. No existing behavior changes. No posting/financial/data-integrity path is left uncovered, so the absence of a test is acceptable for a pure event request. No blocking issues found.

Suggestions

S1 - Reconsider the event name before merge (Low, naming)
Integration-event names are a permanent public contract - renaming after release is a breaking change, so it is worth getting right now. OnAfterGetRecordOnGeneralPostingSetupOnAfterConfirmAndModify is long and stacks several context tokens (OnAfterGetRecord + OnGeneralPostingSetup + OnAfterConfirmAndModify). Consider a shorter, convention-aligned name such as OnAfterGetRecordOnAfterConfirmAndModify or one closer to the sibling OnAfterCopyGenPostingSetup. Not blocking - keep as-is if the current name is intentional.

S2 - Align the PR title with the declared name (Low, consistency)
The PR title says OnGeneralPostingSetupOnAfterGetRecordOnAfterConfirmAndModify, but the code declares OnAfterGetRecordOnGeneralPostingSetupOnAfterConfirmAndModify. Update the title so the event is searchable by its real name. Cosmetic only.


A never approves or rejects - a human makes the final call.

[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10334 round=1 by=alexei-dobriansky at=2026-08-18T07:05:40Z lastSha=673ac4d704f7a5af38eca5a027bce15a99aae29d reviewKey=2b1876577386d38aa025de89c848abbff3bd40a44dfbf075365b5fe30691ba82 suggestions=S1@c4b882c5,S2@81ebd7d2

…General Posting Setup dataitem OnAfterGetRecord trigger after Modify
@Explorer986 Jatin Kumar (Explorer986) changed the title Add integration event OnGeneralPostingSetupOnAfterGetRecordOnAfterConfirmAndModify in the General Posting Setup dataitem OnAfterGetRecord trigger after Modify. Add integration event OnAfterGetRecordOnAfterConfirmAndModify in the General Posting Setup dataitem OnAfterGetRecord trigger after Modify. Aug 18, 2026
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Agentic PR Review - Round 2

Recommendation: Accept

What this PR does

This PR adds the integration event OnAfterGetRecordOnAfterConfirmAndModify in report 87, Copy - General Posting Setup. The event is raised only after the user confirms the copy and after Modify() persists the target General Posting Setup record.

The current diff matches the event-request scenario from AB#646562 and the PR description. The new event is inside the existing confirm block, so it does not run on Cancel. It is raised after the same Modify() call that the request depends on, and it passes the target setup by �ar plus the source setup by value. This is a narrow, additive extension point and does not change existing copy behavior.

Status of previous suggestions
ID Title Status Author response
S1 Reconsider the event name before merge Addressed The event is now named OnAfterGetRecordOnAfterConfirmAndModify, which removes the extra OnGeneralPostingSetup token.
S2 Align the PR title with the declared name Addressed The PR title now uses the same event name as the code.
New observations (commits since round 1)

None - the new head addresses the previous naming and title suggestions. The prior SHA is no longer reachable from the fetched PR history, so I reviewed the full current three-dot diff against main and only attributed the one net PR hunk.

Risk assessment and necessity

Risk: This is a public integration event in a financial setup report, so the main risk is freezing the event name and timing as a long-term extension contract. The final placement after Modify() and inside the confirm block matches the requested contract. Adding an event publisher is additive and does not break existing subscribers.

Necessity: The change is justified because the existing OnAfterCopyGenPostingSetup event runs before the user confirms and before the record is modified. Partners need a hook that runs only for the confirmed, persisted copy. No test is needed for this pure event request because it does not change behavior or guard a posting, financial, or data-integrity path.


[AI-PR-REVIEW] version=1 promptVersion=2 system=github pr=10334 round=2 by=alexei-dobriansky at=2026-08-18T13:11:36Z lastSha=4cd8e0f44ab85ae610ae4f7c53a5986ae0364019 reviewKey=ad0512b433217989f19aab01388618d478ff3c3c56fb9788e59789e91a88d474 suggestions=S1@c4b882c5:addressed,S2@81ebd7d2:addressed parentRound=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Finance GitHub request for Finance area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants