Skip to content

removed access internal to edocument mailing#9527

Open
GOB-Wijaya wants to merge 1 commit into
microsoft:mainfrom
GOB-Wijaya:gobwijaya/8566-edocumentmailingtoglobal
Open

removed access internal to edocument mailing#9527
GOB-Wijaya wants to merge 1 commit into
microsoft:mainfrom
GOB-Wijaya:gobwijaya/8566-edocumentmailingtoglobal

Conversation

@GOB-Wijaya

@GOB-Wijaya GOB-Wijaya commented Jul 16, 2026

Copy link
Copy Markdown

Removed access internal in Codeunit "E-Document Emailing"

Fixes #8566

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.

@GOB-Wijaya
GOB-Wijaya requested review from a team July 16, 2026 08:54
@github-actions github-actions Bot added AL: Apps (W1) Add-on apps for W1 From Fork Pull request is coming from a fork labels Jul 16, 2026
@GOB-Wijaya

Copy link
Copy Markdown
Author

@Groenbech96 Can you look at the Pull Request? Thanks in advance!

@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Breaking\ Changes} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Access = Internal; was removed from codeunit 6188 "E-Document Emailing" with no replacement scoping.

The codeunit's default access is now Public, and its two exposed procedures (SendEDocumentEmail, SetAttachments) carry no explicit access modifier of their own, so they become part of the platform's supported public API surface by default rather than by deliberate design. The commit message ('removed access internal to edocument mailing') gives no rationale, and no procedure was marked internal/local to keep the internal helpers (CreateSourceLists, CreateZipArchiveWithEDocAttachments, etc. are already local and unaffected) out of the contract. Per BCQuality guidance, access should be the narrowest that works, and public should be reserved for entry points intentionally committed to as a stable, maintained API.

Recommendation:

  • either keep Access = Internal; if the codeunit is not meant to be a supported external entry point, or, if publicizing SendEDocumentEmail/SetAttachments is intentional, do so explicitly and document/justify the new public contract (and consider whether the widened surface needs a compatibility/versioning note).

Knowledge:

Line mapping was unavailable, so this was posted as an issue comment.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why

@github-actions

Copy link
Copy Markdown
Contributor

Copilot PR Review

Iteration 1 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@186d8a131465475c79244d994acb872cd5c0d4bf

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain Findings Knowledge-backed Agent Inline Fallback
Breaking Changes 1 1 0 0 1

Totals: 1 knowledge-backed · 0 agent findings.

Orchestrator pre-filter (2 file(s) excluded)

  • layer-disabled (knowledge) : 2 file(s)

Findings produced by the AL review agent v1.7.3. Reply 👎 on any inline comment to flag false positives.

@JesperSchulz JesperSchulz added the Integration GitHub request for Integration area label Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AL: Apps (W1) Add-on apps for W1 From Fork Pull request is coming from a fork Integration GitHub request for Integration area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Extensibility] [E-Document] Publisher in "E-Document Mailing"

3 participants