Skip to content

[Master] [Repair Item] Team Member unable to copy sales quotation when lines are associated to assembly - #9644

Open
Shikhverma wants to merge 21 commits into
mainfrom
bugs/Bug-630947-Master-TeamMemberUnableToCopySaleQuoteWhenAssemblyItemUsed
Open

[Master] [Repair Item] Team Member unable to copy sales quotation when lines are associated to assembly#9644
Shikhverma wants to merge 21 commits into
mainfrom
bugs/Bug-630947-Master-TeamMemberUnableToCopySaleQuoteWhenAssemblyItemUsed

Conversation

@Shikhverma

@Shikhverma Shikhverma commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Bug 630947: [ALL-E] [Repair Item] Team Member unable to copy sales quotation when lines are associated to assembly

Fixes AB#630947

@github-actions github-actions Bot added the Team: Integrations GitHub request for Integrations area label Jul 21, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone Jul 21, 2026
@Shikhverma Shikhverma added the Team: SCM GitHub request for SCM area label Jul 22, 2026
@github-actions github-actions Bot removed the Team: SCM GitHub request for SCM area label Jul 22, 2026
@Shikhverma Shikhverma added the Team: SCM GitHub request for SCM area label Jul 22, 2026
@github-actions github-actions Bot removed the Team: SCM GitHub request for SCM area label Jul 22, 2026
Comment thread src/Layers/W1/BaseApp/Utilities/CopyDocumentMgt.Codeunit.al Outdated
…Bug-630947-Master-TeamMemberUnableToCopySaleQuoteWhenAssemblyItemUsed
Comment thread src/Layers/ES/BaseApp/Utilities/CopyDocumentMgt.Codeunit.al Outdated
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Security}$

HandleAsmAttachedToSalesLine grants 'RIM' on tabledata "Assemble-to-Order Link", but along this code path the link is only read to discover an existing ATO relation and inserted when recreating that relation; the procedure never modifies an existing link row. Narrow the inherent permission to 'RI' so the method does not implicitly grant unnecessary modify access.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    [InherentPermissions(PermissionObjectType::TableData, Database::"Assemble-to-Order Link", 'RI', InherentPermissionsScope::Both)]

Knowledge:

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4

Comment thread src/Layers/ES/BaseApp/Utilities/CopyDocumentMgt.Codeunit.al
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Security}$

HandleAsmAttachedToSalesLine grants 'RIM' on tabledata "Assemble-to-Order Link", but along this code path the link is only read to discover an existing ATO relation and inserted when recreating that relation; the procedure never modifies an existing link row. Narrow the inherent permission to 'RI' so the method does not implicitly grant unnecessary modify access.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    [InherentPermissions(PermissionObjectType::TableData, Database::"Assemble-to-Order Link", 'RI', InherentPermissionsScope::Both)]

Knowledge:

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4

Comment thread src/Layers/IT/BaseApp/Utilities/CopyDocumentMgt.Codeunit.al
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Security}$

HandleAsmAttachedToSalesLine grants 'RIM' on tabledata "Assemble-to-Order Link", but along this code path the link is only read to discover an existing ATO relation and inserted when recreating that relation; the procedure never modifies an existing link row. Narrow the inherent permission to 'RI' so the method does not implicitly grant unnecessary modify access.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    [InherentPermissions(PermissionObjectType::TableData, Database::"Assemble-to-Order Link", 'RI', InherentPermissionsScope::Both)]

Knowledge:

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4

Comment thread src/Layers/NA/BaseApp/Utilities/CopyDocumentMgt.Codeunit.al
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Security}$

HandleAsmAttachedToSalesLine grants 'RIM' on tabledata "Assemble-to-Order Link", but along this code path the link is only read to discover an existing ATO relation and inserted when recreating that relation; the procedure never modifies an existing link row. Narrow the inherent permission to 'RI' so the method does not implicitly grant unnecessary modify access.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    [InherentPermissions(PermissionObjectType::TableData, Database::"Assemble-to-Order Link", 'RI', InherentPermissionsScope::Both)]

Knowledge:

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4

Comment thread src/Layers/NL/BaseApp/Utilities/CopyDocumentMgt.Codeunit.al
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Security}$

HandleAsmAttachedToSalesLine grants 'RIM' on tabledata "Assemble-to-Order Link", but along this code path the link is only read to discover an existing ATO relation and inserted when recreating that relation; the procedure never modifies an existing link row. Narrow the inherent permission to 'RI' so the method does not implicitly grant unnecessary modify access.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    [InherentPermissions(PermissionObjectType::TableData, Database::"Assemble-to-Order Link", 'RI', InherentPermissionsScope::Both)]

Knowledge:

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4

Comment thread src/Layers/RU/BaseApp/Utilities/CopyDocumentMgt.Codeunit.al
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Security}$

HandleAsmAttachedToSalesLine grants 'RIM' on tabledata "Assemble-to-Order Link", but along this code path the link is only read to discover an existing ATO relation and inserted when recreating that relation; the procedure never modifies an existing link row. Narrow the inherent permission to 'RI' so the method does not implicitly grant unnecessary modify access.

Suggested fix (apply manually — could not be anchored as a one-click suggestion):

    [InherentPermissions(PermissionObjectType::TableData, Database::"Assemble-to-Order Link", 'RI', InherentPermissionsScope::Both)]

Knowledge:

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

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.35.4

Comment thread src/Layers/W1/BaseApp/Utilities/CopyDocumentMgt.Codeunit.al
Comment thread src/Layers/W1/Tests/SCM/SCMCopyDocumentMgt.Codeunit.al
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 3

Recommendation: Accept

What this PR does

The latest changes keep the Team Member permission fix for copying sales quotes with assemble-to-order lines, and add the same coverage for archived sales quotes. The inherent permissions are now placed on the copy paths that read or recreate assembly data, and the direct copy helper only grants insert/read on the assemble-to-order link, which matches the current code path. The localized copies stay aligned with W1.

Problem-solution fit

Fit: Strong

The reported problem is that a restricted Team Member user cannot copy a sales quote when an assemble-to-order link is present. The code now grants the needed assembly-table access around both normal and archived quote-copy paths, and the tests exercise resource, item-component, and archived quote scenarios.

Status of previous suggestions

No open suggestions were carried from round 2.

New observations (commits since round 2)

None - the new commit narrows the direct copy permission and adds archive coverage without introducing a new review finding.

Risk assessment and necessity

Risk: The touched area copies sales document lines and recreate assemble-to-order assembly records, so wrong permissions could either leave Team Member users blocked or grant too much access. The current diff keeps the grant narrow on the direct ATO-copy helper, keeps the archive path covered, and applies the same change across W1 and the localized copies.

Necessity: Without this change, Team Member users can remain blocked from copying affected sales quotes. The scope is limited to the copy-document assembly paths needed for that scenario, and the added tests cover the meaningful variants in this PR.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=9644 round=3 by=alexei-dobriansky at=2026-08-31T22:17:35Z lastSha=70a20c25e08fd73e2468253ba74471e2d5f920f1 reviewKey=51bfb229cae509405277c986c75a051d2cdbf0cbae88c12f5823bd30d4b2f36b suggestions= parentRound=2

@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 4

Recommendation: Accept

What this PR does

The latest commit changes only the archived quote regression test. It now uses recalculation for the archived copy because the archived sales line does not keep the assemble-to-order quantity, while the main quote-copy code and the localized permission changes stay unchanged. The normal quote path still has item-component coverage, and the archived path still runs under the restricted plan and verifies that the target quote gets the copied line and assemble-to-order link.

Problem-solution fit

Fit: Strong

The reported scenario is still clear: a restricted Team Member user must be able to copy a sales quote with an assemble-to-order line. The code grants the assembly-table operations needed by that copy path, and the tests now cover the main quote path, the item-component variant, and the archived quote path without changing the production fix.

Status of previous suggestions

No open suggestions were carried from round 3.

New observations (commits since round 3)

None - the new commit only adjusts the archived regression test setup and does not introduce a new review finding.

Risk assessment and necessity

Risk: The touched area copies sales document lines and recreates assemble-to-order assembly data, so wrong permissions could either leave Team Member users blocked or grant more access than needed. The latest commit does not add production risk; the permission grants remain aligned across W1 and localized copies.

Necessity: Without the production change, restricted users can remain blocked from copying affected sales quotes. The latest test adjustment is also reasonable because it matches how archived sales lines rebuild assemble-to-order quantity during copy.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=9644 round=4 by=alexei-dobriansky at=2026-09-01T22:11:38Z lastSha=2b45c1171be02764df63e591bfe243b85efe97ab reviewKey=30abaa5f9962eb2e5d01a0c8a8aff0cc6e14ff6574773047e26709579923fd71 suggestions= parentRound=3

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.

The Team Member regression is covered across live quotations with resource and item BOM components, as well as archived quotations. The inherent permissions are scoped to the assembly records read or created by these copy paths. No blocking findings.

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.

The Team Member regression is covered for live quotations with resource and item BOM components, as well as archived quotations. The inherent permissions are scoped to the assembly records used by these copy paths, and the relevant CI checks pass. No blocking findings.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

⚠️ Stale Status Check Deleted

The Pull Request Build workflow run for this PR was older than 72 hours and has been deleted.

📋 Why was it deleted?

Status checks that are too old may no longer reflect the current state of the target branch. To ensure this PR is validated against the latest code and passes up-to-date checks, a fresh build is required.


🔄 How to trigger a new status check:

  1. 📤 Push a new commit to the PR branch, or
  2. 🔁 Close and reopen the PR

This will automatically trigger a new Pull Request Build workflow run.

Comment thread src/Layers/ES/BaseApp/Utilities/CopyDocumentMgt.Codeunit.al Outdated
…Bug-630947-Master-TeamMemberUnableToCopySaleQuoteWhenAssemblyItemUsed
Comment thread src/Layers/ES/BaseApp/Utilities/CopyDocumentMgt.Codeunit.al Outdated
Comment thread src/Layers/W1/Tests/SCM/SCMCopyDocumentMgt.Codeunit.al
@alexei-dobriansky

Copy link
Copy Markdown
Contributor

Good Sense Reviewer - Round 5

Recommendation: Accept

What this PR does

The latest commits move the assembly-table permission elevation to the codeunit permission block in each affected BaseApp layer copy. This still covers the quote and archived-quote copy paths that read the source assemble-to-order data and recreate the target assembly header, lines, and link. The existing regression tests continue to exercise the restricted Team Member path for the normal quote copy, an item-component variant, and an archived quote copy.

Problem-solution fit

Fit: Strong

The reported scenario is narrow and clear: a restricted Team Member user must be able to copy a sales quote that has an assemble-to-order line. The final diff grants the assembly-table operations needed by that copy flow and keeps coverage for both the main and archived variants.

Status of previous suggestions

No open suggestions were carried from round 4.

New observations (commits since round 4)

None - the new commits adjust where the permission grant is declared and keep it aligned across the affected layer copies. The changed span is present in the net PR diff and does not introduce a new review finding.

Risk assessment and necessity

Risk: The touched area is the sales copy-document flow for assemble-to-order lines. The permission grant is broader than a single procedure attribute because it is now on the codeunit, but it is limited to the three assembly tables needed by the copy logic, and it does not change a public signature or event contract.

Necessity: Without the production change, restricted users can remain blocked from copying affected sales quotes. The scope remains appropriate because the change grants only the assembly-table read, insert, and modify operations used while recreating the copied assemble-to-order data.


[AI-PR-REVIEW] version=1 promptVersion=4 system=github pr=9644 round=5 by=alexei-dobriansky at=2026-09-08T22:08:34Z lastSha=848d5eb42601668593d5464b1707993e58572ecd reviewKey=a94880821d66d279ae6c8258733a70deb18a44011c0ac08840519b67fcfc1e07 suggestions=none parentRound=4

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

Labels

Team: Integrations GitHub request for Integrations area

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants