[Quality Management] Fix promoted-result matrix column visibility in pages

[JakovljevicDusan]

What & why

Fixes issue "Conditions disappears from the template once user selected new test of type Label" and related issues of UI inconsistency.

Fixes:

• For list pages, set visibility and captions in OnOpenPage
• Remove cached records that were not used
• Remove redundant calls, like repeated calls in OnAfterGetCurrRecord
• Improve condition visibility consistency after changing promoted Results

Linked work

Fixes AB#622771
Fixes AB#622770

[github-actions]

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

UpdateRowData Not Called for Each Row

The call to UpdateRowData() was removed from OnAfterGetRecord, which is the trigger that fires for every row as the subform list renders. OnAfterGetCurrRecord still calls it for the selected row only, so all non-current rows will display stale or empty CanEditTestValue, MeasurementNote, and condition cell data (MatrixArrayConditionCellData, MatrixSourceRecordId).

Recommendation:

• Restore the UpdateRowData() call inside OnAfterGetRecord so per-row data is populated for every visible inspection line.

    trigger OnAfterGetRecord()
    var
        QltySessionHelper: Codeunit "Qlty. Session Helper";
    begin
        QltySessionHelper.SetSessionValue(CurrentSelectedInspectionLineTok, Format(Rec.RecordId()));
        UpdateRowData();
    end;

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

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

---

JakovljevicDusan: Move to OnOpenPage, since it is setting visibility.

[github-actions]

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

EnsureResultsExist Removed from OnInsert

Removing the EnsureResultsExist(true) call from the OnInsert trigger means that when a new inspection template line is inserted, the expected default result records are no longer auto-created. Template lines may be left in an incomplete state with no associated results, breaking downstream inspection workflows that assume results exist.

Recommendation:

• Confirm whether result initialization is now handled elsewhere (e.g., in the calling page or a codeunit). If not, restore the call or document why it was intentionally removed.

        InitLineNoIfNeeded();
        EnsureResultsExist(true);
        Rec.CalcFields("Test Value Type");

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

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

JakovljevicDusan: A bit specific, since in the callstack this execution was skipped as the line is not yet in the database. So, it is removed since it is redundant.

[github-actions]

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

State Variables Not Refreshed After Field Validation

Removing OnAfterGetCurrRecord means that when a user changes a field such as Test Value Type on an open Test Card, the computed variables IsAllowableValuesEditable, IsLookupField, and EditableResult are not recalculated until the record is reloaded. OnAfterGetRecord alone does not cover in-place edits on the current record.

Recommendation:

• Re-add OnAfterGetCurrRecord to trigger UpdateRowData() after any field change, or handle recalculation inside the Test Value Type OnValidate trigger.

    trigger OnAfterGetCurrRecord()
    begin
        UpdateRowData();
    end;

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

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

[github-actions]

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

Fallback to Default Promoted Results Removed

The old UpdateRowData fell back to GetDefaultPromotedResults both when the test code was empty and when GetPromotedResultsForTest returned no visible results (MatrixVisibleState[1] = false). The new code skips the call entirely for empty codes and drops the visibility fallback, so tests with no promoted results configured will show no condition columns and EditableResult will evaluate to false based on stale Visible1 from OnOpenPage.

Recommendation:

• Restore the fallback: if Rec.Code = '' or after GetPromotedResultsForTest returns with MatrixVisibleState[1] = false, call GetDefaultPromotedResults to ensure columns and EditableResult are correctly initialised.

        if Rec.Code <> '' then begin
            QltyResultConditionMgmt.GetPromotedResultsForTest(Rec, MatrixSourceRecordId, MatrixArrayConditionCellData, MatrixArrayConditionDescriptionCellData, MatrixArrayCaptionSet, MatrixVisibleState);
            if not MatrixVisibleState[1] then
                QltyResultConditionMgmt.GetDefaultPromotedResults(false, MatrixSourceRecordId, MatrixArrayConditionCellData, MatrixArrayConditionDescriptionCellData, MatrixArrayCaptionSet, MatrixVisibleState);
        end else
            QltyResultConditionMgmt.GetDefaultPromotedResults(false, MatrixSourceRecordId, MatrixArrayConditionCellData, MatrixArrayConditionDescriptionCellData, MatrixArrayCaptionSet, MatrixVisibleState);

        EditableResult := (Rec.Code <> '') and (CurrPage.Editable) and (Visible1) and (MatrixArrayCaptionSet[1] <> '');

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

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

[github-actions]

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

EnsureResultsExist dropped from OnInsert

Removing EnsureResultsExist(true) from the table's OnInsert trigger means that template lines created programmatically (via API, codeunit automation, or test harness) without subsequently triggering the 'Test Code' field's OnValidate will have no result-condition records initialised. Any downstream code that relies on those records existing will silently find nothing, producing incorrect results or runtime errors.

Recommendation:

• If the removal is intentional for UI paths (to avoid redundant writes), keep EnsureResultsExist in the table OnInsert with ForceOverwriteConditions := false so programmatic inserts are still safe, and suppress it from the page trigger only.

trigger OnInsert()
begin
    if Rec."Line No." = 0 then
        exit;

    InitLineNoIfNeeded();
    EnsureResultsExist(false);  // ensure records exist on programmatic insert without overwriting
    Rec.CalcFields("Test Value Type");
end;

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

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

[github-actions]

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

OnAfterGetCurrRecord removed from card page

The removed OnAfterGetCurrRecord trigger was the only mechanism that refreshed EditableResult, IsAllowableValuesEditable, IsLookupField, and IsExpressionFormulaEditable when the current record changed in scenarios where OnAfterGetRecord does not re-fire (e.g., after CurrPage.Update() or after an external modification refreshes the page). Fields bound to these expressions will display stale editable states in those scenarios.

Recommendation:

• Reintroduce OnAfterGetCurrRecord calling UpdateRowData(), or verify that every code path that can change the current record without triggering OnAfterGetRecord is accounted for.

trigger OnAfterGetCurrRecord()
begin
    UpdateRowData();
end;

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

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

[github-actions]

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

EnsureResultsExist removed from OnInsert

The call to EnsureResultsExist(true) has been removed from the OnInsert trigger. If this method was responsible for automatically creating associated result records when a new template line is inserted, omitting it will leave newly inserted template lines without the required result records, potentially causing downstream errors when the template is used.

Recommendation:

• Verify whether EnsureResultsExist must be called during insert to maintain data integrity. If result records must exist for every template line, restore the call (or invoke it from a more appropriate trigger). Document the decision if this is intentionally deferred to another point in the workflow.

InitLineNoIfNeeded();
EnsureResultsExist(true);
Rec.CalcFields("Test Value Type");

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

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

[github-actions]

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

EnsureResultsExist removed from OnNewRecord

The OnNewRecord trigger, which previously called Rec.EnsureResultsExist(false), has been completely removed and replaced with OnOpenPage. If EnsureResultsExist is required to initialise result records for each new template line created through this subform, users will no longer get those records initialised, leading to incomplete data.

Recommendation:

• Move the EnsureResultsExist(false) call to a suitable trigger that still fires when a new record is initialised through the subform (e.g. restore OnNewRecord in addition to OnOpenPage, or call it from OnInsert on the table).

trigger OnNewRecord(BelowxRec: Boolean)
begin
    Rec.EnsureResultsExist(false);
end;

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

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

[github-actions]

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

OnAfterGetCurrRecord removed; navigation skips row-data refresh

OnAfterGetCurrRecord has been removed. This trigger fires when the user navigates to an existing record without a fresh record fetch (e.g. moving focus within the subform). OnAfterGetRecord alone will not cover all navigation scenarios, so UpdateRowData — which refreshes per-row cell data and the editable flags — will silently be skipped when navigating between rows.

Recommendation:

• Restore OnAfterGetCurrRecord calling UpdateRowData(), or verify through testing that all navigation paths in the subform still correctly refresh the per-row condition data and editable states.

trigger OnAfterGetCurrRecord()
begin
    UpdateRowData();
end;

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

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

[github-actions]

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

OnInsertRecord removed; UI state stale on new record

OnInsertRecord previously called UpdateRowData() so that the matrix columns and editable flags were initialised as soon as a new test record was inserted. Removing this trigger means the UI will show stale/default state until the user navigates away and back.

Recommendation:

• Restore OnInsertRecord with a call to UpdateRowData(), or ensure another trigger (e.g. OnAfterGetRecord) covers the newly inserted record scenario.

trigger OnInsertRecord(BelowxRec: Boolean): Boolean
begin
    UpdateRowData();
end;

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

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

[github-actions]

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

Local MatrixVisibleState in UpdateRowData is unused

UpdateRowData() declares a local MatrixVisibleState: array[10] of Boolean and passes it to GetPromotedResultsForTest as an out-parameter, but never reads from it afterwards. This means the visible-state output of GetPromotedResultsForTest is silently discarded on every row fetch, and the AL compiler may emit an unused-variable warning.

Recommendation:

• Either remove the MatrixVisibleState local variable if the visible state from GetPromotedResultsForTest is intentionally unused (and document why), or use it to update Visible1..10 so per-test column visibility is correctly reflected.

local procedure UpdateRowData()
var
    MatrixVisibleState: array[10] of Boolean;
begin
    QltyResultConditionMgmt.GetPromotedResultsForTest(Rec, MatrixSourceRecordId, MatrixArrayConditionCellData, MatrixArrayConditionDescriptionCellData, MatrixArrayCaptionSet, MatrixVisibleState);
    // If column visibility should be per-row, assign Visible1 := MatrixVisibleState[1]; etc.
end;

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

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

[github-actions]

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

EnsureResultsExist removed from OnInsert

The EnsureResultsExist(true) call was removed from the table's OnInsert trigger. Any programmatic code path that inserts a Qlty. Inspection Template Line record without subsequently calling Validate("Test Code") will silently skip result-record creation, leaving the line in a broken state that only manifests downstream.

Recommendation:

• Either restore EnsureResultsExist(true) in the OnInsert trigger, or add it after InitLineNoIfNeeded() if the intent is to run it unconditionally on insert, similar to how OnModify still calls EnsureResultsExist(false).

trigger OnInsert()
begin
    if Rec.IsTemporary() then
        exit;

    InitLineNoIfNeeded();
    EnsureResultsExist(true);
    Rec.CalcFields("Test Value Type");
end;

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

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

[github-actions]

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

UpdateRowData removed from OnAfterGetCurrRecord

OnAfterGetCurrRecord no longer calls UpdateRowData(), so when a user navigates between rows in the inspection subform without triggering a full page refresh, the page-level MatrixSourceRecordId and condition cell arrays retain values from the previously focused row. This means OnAssistEdit and OnValidate handlers on matrix cells could operate on the wrong underlying result-condition record.

Recommendation:

• Re-add a call to UpdateRowData() inside OnAfterGetCurrRecord so that per-row matrix data is refreshed whenever the current record changes.

trigger OnAfterGetCurrRecord()
var
    QltySessionHelper: Codeunit "Qlty. Session Helper";
begin
    QltySessionHelper.SetSessionValue(CurrentSelectedInspectionLineTok, Format(Rec.RecordId()));
    UpdateRowData();
end;

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

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

[github-actions]

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

Redundant CalcFields call per subform row

Rec.CalcFields("Test Value Type") is called at line 326 inside OnAfterGetRecord and again at line 351 inside UpdateRowData(), producing two separate SQL round-trips for every row rendered in the inspection subform. On inspections with many lines this doubles the database load for this field.

Recommendation:

• Remove the duplicate CalcFields call from UpdateRowData() (line 351) since the value is already fetched before UpdateRowData() is called. Pass the already-computed value or rely on the result from the OnAfterGetRecord call.

local procedure UpdateRowData()
var
    MatrixVisibleState: array[10] of Boolean;
    Index: Integer;
begin
    MeasurementNote := Rec.GetMeasurementNote();
    // Remove redundant CalcFields - already called in OnAfterGetRecord
    CanEditTestValue := GetCanEditTestValue();
    ...

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

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

[github-actions]

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

Iterator increments for results without conditions

Iterator is unconditionally incremented for every promoted QltyInspectionResult, even when no matching QltyIResultConditConf exists. This wastes matrix slots, so if several results lack configs the useful conditions may be pushed past MaxResultConditions and silently dropped from the matrix.

Recommendation:

• Only increment Iterator when a matching condition config is actually found, or track a separate 'ResultSlot' counter for position and a 'FilledCount' counter for the cap check.

repeat
    QltyIResultConditConf.SetRange("Result Code", QltyInspectionResult.Code);
    if QltyIResultConditConf.FindFirst() then begin
        Iterator += 1;
        if Iterator > GetMaxResultConditions() then
            break;
        MatrixVisibleStateToSet[Iterator] := true;
        // ... rest of assignments
    end;
until (QltyInspectionResult.Next() = 0);

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

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

[github-actions]

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

EnsureResultsExist removed from OnInsert

The call to EnsureResultsExist(true) was removed from the OnInsert trigger. Any code that inserts a new Qlty. Inspection Template Line and then immediately reads or processes its result records will now find no results exist, potentially causing downstream errors or silent data-integrity gaps.

Recommendation:

• Restore the EnsureResultsExist call in OnInsert, or add a comment explaining the new contract under which callers are expected to trigger result creation themselves.

trigger OnInsert()
begin
    // ...
    InitLineNoIfNeeded();
    EnsureResultsExist(true);
    Rec.CalcFields("Test Value Type");
end;

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

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

[github-actions]

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

UpdateRowData removed from OnAfterGetCurrRecord

UpdateRowData() was removed from the OnAfterGetCurrRecord trigger body in QltyInspectionSubform. This means CanEditTestValue, MeasurementNote, and matrix condition cell data will not refresh when the user navigates between subform rows by clicking, causing stale data to remain displayed for the newly selected row.

Recommendation:

• Keep UpdateRowData() in OnAfterGetCurrRecord (or ensure an unchanged OnAfterGetRecord covers all navigation paths) so that row-level data is always current.

trigger OnAfterGetCurrRecord()
begin
    QltySessionHelper.SetSessionValue(CurrentSelectedInspectionLineTok, Format(Rec.RecordId()));
    UpdateRowData();
end;

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

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

[github-actions]

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

FindFirst silently drops multi-condition configs

The old code iterated QltyIResultConditConf and could expose multiple condition configurations per result code as separate matrix columns. The new code calls FindFirst() and only surfaces the single highest-priority config, silently discarding any others that previously appeared in separate slots.

Recommendation:

• Document explicitly that only the top-priority condition per result code is now surfaced, and verify there are no callers or templates that relied on seeing multiple configs for the same result in the matrix.

// Add a comment above FindFirst:
// Note: Only the highest-priority condition config for each Result Code is used (FindFirst with Priority-descending sort).
if QltyIResultConditConf.FindFirst() then begin

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

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

[github-actions]

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

Column visibility set once; never refreshed

Visible1-10 are populated once in OnOpenPage from GetDefaultPromotedResults. If the set of promoted inspection results changes while the page is open (e.g., another user edits the result configuration), the column visibility will be stale until the page is reopened.

Recommendation:

• Consider calling the visibility update from OnAfterGetRecord as well, or add a refresh action so users can sync visibility without reopening the page.

// In OnAfterGetRecord, after UpdateRowData():
QltyResultConditionMgmt.GetDefaultPromotedResults(true, MatrixSourceRecordId, MatrixArrayConditionCellData, MatrixArrayConditionDescriptionCellData, MatrixArrayCaptionSet, MatrixVisibleState);
Visible1 := MatrixVisibleState[1];
// ... etc.

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

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

[github-actions]

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

Default promoted-result fallback removed

The old UpdateRowData fell back to GetDefaultPromotedResults when GetPromotedResultsForTest returned no visible state (MatrixVisibleState[1] was false). The new code calls GetPromotedResultsForTest unconditionally and discards its visible-state output. Tests that previously relied on the fallback to display the default matrix columns will now show empty columns.

Recommendation:

• Retain the fallback: after GetPromotedResultsForTest, check whether any results were found and call GetDefaultPromotedResults if not.

QltyResultConditionMgmt.GetPromotedResultsForTest(Rec, MatrixSourceRecordId, MatrixArrayConditionCellData, MatrixArrayConditionDescriptionCellData, DummyMatrixArrayCaptionSet, DummyMatrixVisibleState);
if not DummyMatrixVisibleState[1] then
    QltyResultConditionMgmt.GetDefaultPromotedResults(false, MatrixSourceRecordId, MatrixArrayConditionCellData, MatrixArrayConditionDescriptionCellData, DummyMatrixArrayCaptionSet, DummyMatrixVisibleState);

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

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

[github-actions]

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

EditableResult depends on stale Visible1

EditableResult is computed as (Rec.Code <> '') and (CurrPage.Editable) and (Visible1) and (MatrixArrayCaptionSet[1] <> ''). Visible1 is now fixed at page-open time. If the promoted-results configuration changes, Visible1 will be stale, potentially keeping EditableResult false even when editing should be allowed.

Recommendation:

• Either refresh Visible1 per-record in UpdateRowData, or derive the editability check from the per-record DummyMatrixVisibleState returned by GetPromotedResultsForTest.

EditableResult := (Rec.Code <> '') and (CurrPage.Editable) and DummyMatrixVisibleState[1] and (MatrixArrayCaptionSet[1] <> '');

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

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

[github-actions]

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

OnAfterGetCurrRecord removed from lookup page

Both OnAfterGetRecord and OnAfterGetCurrRecord previously called UpdateRowData() in QltyTestLookup. OnAfterGetCurrRecord is removed, so IsAllowableValuesEditable and the matrix condition data will not refresh when a user navigates between rows in the lookup without triggering a full record re-read.

Recommendation:

• Restore OnAfterGetCurrRecord calling UpdateRowData() to keep editable state and cell data in sync during navigation.

trigger OnAfterGetCurrRecord()
begin
    UpdateRowData();
end;

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

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

[github-actions]

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

OnAfterGetCurrRecord removed from list page

OnAfterGetCurrRecord was removed from QltyTests (a list page). In list pages, OnAfterGetCurrRecord fires when the user navigates between rows without re-reading from the database. Relying solely on OnAfterGetRecord means condition cell data will not refresh on navigation, leaving stale values visible.

Recommendation:

• Add an OnAfterGetCurrRecord trigger that also calls UpdateRowData(), matching the original behaviour.

trigger OnAfterGetCurrRecord()
begin
    UpdateRowData();
end;

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

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