[Quality Management] Enforce rules and restrictions with indirect and implicit permissions

[JakovljevicDusan]

What & why

Implement InherentPermissions so users without QM permissions would not silently skip mandatory Quality Inspection creation or transaction restrictions.

Automatic Quality Inspection creation is now handled like this:

1. If there is no Quality Inspection Setup or there is no Generation Rule triggered on specific event, QM code is skipped and user does not need any QM permission set. InherentPermissions for these two tables is used.
2. However, if there is trigger, then user has to have QM permissions. Otherwise, user gets standard permission error, such as this:

3. Minimum is new permission set "QltyMgmt - I. Create"/'Quality Inspection - Create' which grants indirect permissions for creating inspections, without accessing or processing them further. - This permission set grants permission to run creation inspection manually as well, via "Create Quality Inspection" actions. But, due to AccessByPermission, View actions are hidden if user has just that QM permission set.

Linked work

Fixes AB#626330

[github-actions]

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

ReadPermission guard also removed from warehouse activity check

The same ReadPermission() removal issue exists in CommonCheckWarehouseActivityLineIsAllowed: QltyInspectionHeader.ReadPermission() and QltyInspectionResult.ReadPermission() guards were dropped, so quality-blocking logic now runs for all users performing warehouse activity line operations, even on tenants that have not licensed or configured Quality Management.

Recommendation:

• Apply the same fix as HandleOnAfterCheckItemTrackingInformation: add an explicit check that Quality Management is enabled before executing quality inspection enforcement logic.

if not QltyManagementSetup.GetSetupRecord() then
    exit;
if not QltyManagementSetup.IsEnabled() then
    exit;

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

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

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Security} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

Error message loses actionable remediation guidance

The FinishBeforeReinspectionErr label had its actionable guidance removed: the sentence directing users to ask their administrator to grant finish-inspection permission via the Quality Inspection Permissions page was deleted. Users encountering this error will now receive a dead-end message with no path to resolution.

Recommendation:

• Restore the administrator guidance sentence, or replace it with a more specific and up-to-date instruction (e.g. a link or page reference) so that users can self-serve or escalate effectively.

FinishBeforeReinspectionErr: Label 'An inspection must be finished before a Re-inspection can be made. This is done automatically, but you do not have permission to finish an inspection. Ask your administrator to grant you the Finish Inspection permission in the Quality Inspection Permissions page.';

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

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

[alexei-dobriansky]

Agentic PR Review - Round 1

Recommendation: Request Changes

What this PR does

This PR adds procedure-level implicit permissions to Quality Management integration subscribers so automatic inspection creation and item-tracking/warehouse restrictions run even when the current user lacks direct Quality Management permissions. That is the right enforcement direction for the described issue, and the tracking change intentionally removes the old ReadPermission() short-circuit. However, the diff still leaves at least two production source-update subscribers dependent on direct setup read permission, and there are no changed tests for a security/permission behavior change.

Suggestions

S1 - Add permission regression coverage
Add tests under src\Apps\W1\Quality Management\test\src that run the changed posting/restriction paths with restrictive or lowered permissions and assert both blocked and allowed behavior. This PR changes permission enforcement but modifies no test file, so there is no proof that the implicit permissions cover the intended paths without over-granting.

S2 - Cover production source update subscribers
QltyManufacturIntegration.Codeunit.al still calls QltyManagementSetup.GetSetupRecord() in HandleOnAfterToProdOrderLineModify and HandleOnAfterToProdOrderRtngLineInsert before reaching helper methods with implicit header permissions. Add the same implicit setup/header permissions, or otherwise remove the direct-permission dependency, so production line/routing source updates are not silently skipped for users without Quality Management read access.

S3 - Restore actionable reinspection remediation
QltyInspectionHeader.Table.al shortens FinishBeforeReinspectionErr while the code still errors when CanFinishInspection() is false before automatically finishing an inspection for re-inspection. Keep or replace the administrator guidance so users know which Quality Inspection permission to request when re-inspection creation is blocked.

Risk assessment and necessity

Risk: The changed subscribers sit on purchase, sales return, transfer, assembly, manufacturing, warehouse posting/registering, and item-tracking validation paths. An incomplete permission fix can either continue bypassing mandatory inspections/restrictions or unexpectedly block operational transactions; the unannotated manufacturing line/routing callbacks are a concrete remaining gap.

Necessity: The scenario is important because users without direct Quality Management permissions should not bypass mandatory inspection creation or transaction restrictions. AB#626330 is linked, but the work item was not independently verified; the PR body and diff still show a valid enforcement need.

---

[AI-PR-REVIEW] version=1 system=github pr=8556 round=1 by=alexei-dobriansky at=2026-06-10T15:49:07Z lastSha=794939cd7756d1912ce0d0edb76b7fbe323c5074 suggestions=S1,S2,S3

[alexei-dobriansky]

@JakovljevicDusan, please check if the suggestions are valid.

[github-actions]

⚠️ 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.

[github-actions]

Copilot PR Review

Iteration 1 · Outcome: completed

Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630

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

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

_{Findings produced by the Copilot CLI agent against BCQuality at 822cae1b2771ac25f665f73369f69093bd4fd630. Reply 👎 on any inline comment to flag false positives.}

[alexei-dobriansky]

Agentic PR Review - Round 2

Recommendation: Request Changes

What this PR does

Since round 1, the PR changed the permission model. Automatic enforcement now checks setup and generation rules with inherent permissions, then requires the user to have the new minimum Quality Inspection create permissions when a rule applies. The manufacturing source-update gap is fixed, and the new page fallback avoids opening inspection pages when the user cannot read them. The remaining concern is proof: this is a broad permission and posting behavior change, but the current PR diff still has no Quality Management test changes.

Status of previous suggestions

ID	Title	Status	Author response
S1	Add permission regression coverage	Not addressed	No Quality Management test files are changed in the current PR diff.
S2	Cover production source update subscribers	Addressed	The two production source-update subscribers now have setup read inherent permissions, and the called update helpers keep inspection header read/modify permissions.
S3	Restore actionable reinspection remediation	Not addressed	FinishBeforeReinspectionErr still removes the guidance about which Quality Inspection permission to request.

New observations (commits since round 1)

S4 - Align manual create access check
The PR says QltyMgmt - I. Create should allow manual Create Quality Inspection actions, but the Qlty. Create Inspection report still requires R on Qlty. Inspection Header through AccessByPermission. The new permission set grants indirect read and direct insert on the header, so use an insert-based access check or grant read deliberately, then cover this with a permission test.

Risk assessment and necessity

Risk: This still touches purchase receipt/release, sales return receipt, transfer receipt, assembly, manufacturing, warehouse, and item-tracking checks. A mistake can either let mandatory inspections and blocking rules be skipped, or can block normal operational posting with permission errors. The new minimum permission set also changes which QM actions are visible and runnable for users without full QM access.

Necessity: The change is necessary because mandatory Quality Management enforcement must not depend on broad direct QM access. The scope is now closer to the stated concept, but permission behavior needs regression coverage before merge because the failure modes are security and posting regressions.

---

[AI-PR-REVIEW] version=1 system=github pr=8556 round=2 by=alexei-dobriansky at=2026-07-06T08:04:41Z lastSha=db5239935de470d30f4472dcf553d67ba3b3805a suggestions=S1:notaddressed,S2:addressed,S3:notaddressed,S4:new parentRound=1

[alexei-dobriansky]

@JakovljevicDusan please check if the new suggestion is valid.

[JakovljevicDusan]

S4 - Align manual create access check
The PR says QltyMgmt - I. Create should allow manual Create Quality Inspection actions, but the Qlty. Create Inspection report still requires R on Qlty. Inspection Header through AccessByPermission. The new permission set grants indirect read and direct insert on the header, so use an insert-based access check or grant read deliberately, then cover this with a permission test.

Answer: AccessByPermission is there just to avoid showing report in search/tell me.