[Vendor] [Quality Management] Agentic review - Error Handling: QltyInspectionLine - No Pre-Condition Validation in OnModify Trigger

[PredragMaricic]

Describe the issue

ISSUE 6.8: No Pre-Condition Validation in OnModify Trigger

Severity: HIGH
Category: Validation & Data Integrity
Files: src/Document/QltyInspectionTestLine.Table.al

Problem:
The OnModify trigger attempts numeric conversion but doesn't validate the test line state or status before modification.

Code Location (Lines 218-226):

trigger OnModify()
begin
    if not Rec.IsTemporary() then
        TestStatusOpen();  // Only checks if test is open

    Rec."Numeric Value" := 0;
    if Evaluate(Rec."Numeric Value", Rec."Test Value") then;  // DEFECT: Ignored result
end;

Issues:

1. Silent Failure: Evaluate result ignored - "Numeric Value" stays 0 if conversion fails
2. No Field Validation: Doesn't check if "Test Value" field type is numeric
3. No Error Message: User has no idea conversion failed

Scenarios:

// SCENARIO 1: Text field treated as numeric
Field Type = "Text"
Test Value = "Some text"
OnModify: Evaluate fails silently
Numeric Value = 0 (wrong - should be null or error)

// SCENARIO 2: Invalid numeric format
Field Type = "Integer"
Test Value = "12.5.7"  // Invalid
OnModify: Evaluate fails silently
Numeric Value = 0 (wrong - should show error)

// SCENARIO 3: Overflow
Field Type = "Integer"
Test Value = "999999999999999"  // Exceeds integer max
OnModify: Evaluate fails silently
Numeric Value = 0 (wrong - data loss)

Recommended Fix:

trigger OnModify()
var
    ConversionSuccess: Boolean;
begin
    if not Rec.IsTemporary() then
        TestStatusOpen();

    // Only attempt numeric conversion for numeric field types
    Rec.CalcFields("Field Type");
    if Rec."Field Type" in [Rec."Field Type"::"Field Type Decimal", Rec."Field Type"::"Field Type Integer"] then begin
        Rec."Numeric Value" := 0;
       
        if Rec."Test Value" <> '' then begin
            ClearLastError();
            ConversionSuccess := Evaluate(Rec."Numeric Value", Rec."Test Value");
           
            if not ConversionSuccess then
                Error(InvalidNumericValueErr, Rec."Test Value", Rec."Field Code", Rec."Field Type");
        end;
    end else
        Rec."Numeric Value" := 0;  // Clear for non-numeric types
end;

var
    InvalidNumericValueErr: Label 'The value "%1" is not a valid %3 for field "%2". Please enter a valid numeric value.', Comment = '%1=entered value, %2=field code, %3=field type';

Expected behavior

Steps to reproduce

Additional context

I will provide a fix for a bug

• I will provide a fix for a bug

[insightworks007]

I think the issue description and proposed resolution are based on a misunderstanding of the purpose of the Numeric Value field.

Numeric Value is intentionally a derived, secondary field used for analysis and aggregation. Its role is to store a best-effort numeric representation of the recorded test value when such a representation is possible. It is not the authoritative source of truth for the test result, nor is it intended to enforce validation or block data entry.

For that reason, the conversion here must be silent and non-blocking:

• The conversion is opportunistic by design. If a numeric value can be extracted, it is stored; if it cannot, the field remains at a neutral value.
• Introducing an error or user-visible validation at this point would be inappropriate, as it would prevent valid test results from being recorded based solely on whether a numeric interpretation is possible.

Validation of whether a test value must be numeric already happens earlier and in intentional locations. When a test definition explicitly requires a decimal or integer (including ranges and tolerances), the appropriate data-type checks are already enforced during test value validation. This trigger is not the correct place to repeat or tighten those rules.

It is also important that conversion is not limited only to tests explicitly defined as decimal or integer. There are valid and common scenarios where a test value is stored as text, lookup, or option, but still benefits from a numeric projection for analysis. Examples include:
-Lookups or options that represent numeric magnitudes.
-Measurements that can return numeric values or textual indicators with tool limits are reached, such as “<-40”, “>+40”, “Out of range low”, or “Out of range high”.

In these cases, the textual result is the correct recorded value, while the numeric value, when derivable, remains useful for trend analysis, SPC, and reporting.

Although I understand the concern being raised, the current behavior is intentional and aligned with the design goals of separating data capture, validation, and analysis responsibilities. Adding blocking validation or errors here would regress valid use cases rather than improve data integrity.