Skip to content

[Quality Management] Fix missing validation of "Allowable Values" on Quality Test card #8287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JakovljevicDusan wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[Quality Management] Fix missing validation of "Allowable Values" on Quality Test card #8287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e36b97
Fix
JakovljevicDusan May 21, 2026
afff4e8
Support "Value Type Boolean" and add tests
JakovljevicDusan May 22, 2026
3abebdf
Code review suggestion
JakovljevicDusan May 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 107 additions & 2 deletions src/Apps/W1/Quality Management/app/src/Configuration/Result/QltyResultEvaluation.Codeunit.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ codeunit 20410 "Qlty. Result Evaluation"
IsDefaultTextTok: Label '<>''''', Locked = true;
InvalidDataTypeErr: Label 'The value "%1" is not allowed for %2, it is not a %3.', Comment = '%1=the value, %2=field name,%3=field type.';
NotInAllowableValuesErr: Label 'The value "%1" is not allowed for %2, it must be in the range of "%3".', Comment = '%1=the value, %2=field name,%3=field type.';
InvalidAllowableValuesFormatErr: Label 'The allowable values "%1" are not a valid filter expression for %2 of type %3.', Comment = '%1=the allowable values, %2=field name, %3=field type.';

trigger OnRun()
var
Expand Down Expand Up @@ -324,6 +325,110 @@ codeunit 20410 "Qlty. Result Evaluation"
ValidateAllowableValuesOnText(TestNameForError, QltyTest."Default Value", QltyTest."Allowable Values", QltyTest."Test Value Type", TempBufferQltyTestLookupValue, QltyCaseSensitivity);
end;

/// <summary>
/// Validates that the allowable values expression is a valid filter for the given test value type.
/// </summary>
/// <param name="QltyTest">The test whose allowable values expression should be validated.</param>
internal procedure ValidateAllowableValuesFormat(var QltyTest: Record "Qlty. Test")
var
TestNameForError: Text;
begin
if QltyTest.Description <> '' then
TestNameForError := QltyTest.Description
else
TestNameForError := QltyTest.Code;

ValidateAllowableValuesFormat(TestNameForError, QltyTest."Allowable Values", QltyTest."Test Value Type");
end;

/// <summary>
/// Validates that the allowable values expression is a valid filter for the given test value type.
/// </summary>
/// <param name="NumberOrNameOfTestNameForError">Friendly identifier used in error messages.</param>
/// <param name="AllowableValues">The allowable values filter expression to validate.</param>
/// <param name="QltyTestValueType">The test value type that the filter expression must match.</param>
local procedure ValidateAllowableValuesFormat(NumberOrNameOfTestNameForError: Text; AllowableValues: Text; QltyTestValueType: Enum "Qlty. Test Value Type")
var
IsValid: Boolean;
begin
if AllowableValues = '' then
exit;

if IsBlankOrEmptyCondition(AllowableValues) then
exit;

if IsAnythingExceptEmptyCondition(AllowableValues) then
exit;

// Allowable values may contain inline expressions like [Field] that are resolved at evaluation time.
// Defer validation until resolution.
if AllowableValues.Contains('[') then
exit;

IsValid := true;
case QltyTestValueType of
QltyTestValueType::"Value Type Decimal":
IsValid := TryApplyDecimalFilter(AllowableValues);
QltyTestValueType::"Value Type Integer":
IsValid := TryApplyIntegerFilter(AllowableValues);
QltyTestValueType::"Value Type Boolean":
IsValid := TryApplyBooleanFilter(AllowableValues);
QltyTestValueType::"Value Type Date":
IsValid := TryApplyDateFilter(AllowableValues);
QltyTestValueType::"Value Type DateTime":
IsValid := TryApplyDateTimeFilter(AllowableValues);
end;

if not IsValid then
Error(InvalidAllowableValuesFormatErr, AllowableValues, NumberOrNameOfTestNameForError, QltyTestValueType);
end;

[TryFunction]
local procedure TryApplyDecimalFilter(FilterExpression: Text)
var
TempQltyInspectionLine: Record "Qlty. Inspection Line" temporary;
begin
TempQltyInspectionLine.SetFilter("Derived Numeric Value", FilterExpression);
if TempQltyInspectionLine.IsEmpty() then;
end;

[TryFunction]
local procedure TryApplyIntegerFilter(FilterExpression: Text)
var
TempInteger: Record "Integer" temporary;
begin
TempInteger.SetFilter(Number, FilterExpression);
if TempInteger.IsEmpty() then;
end;

[TryFunction]
local procedure TryApplyBooleanFilter(FilterExpression: Text)
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Error() called inside [TryFunction] with empty placeholders

TryApplyBooleanFilter is marked [TryFunction] but raises Error() explicitly with empty-string placeholders ('' for field name and literal 'Boolean' for type). While AL [TryFunction] does catch explicit Error() calls and returns false, the inner error message is set with missing context, and the outer ValidateAllowableValuesFormat then raises its own error — meaning two error messages are generated internally before the user-visible one, which can confuse debugging and telemetry.

Recommendation:

  • Remove the explicit Error() from inside TryApplyBooleanFilter and instead return without error from the try function; let the caller (ValidateAllowableValuesFormat) raise the user-visible error with full context. Alternatively, validate boolean values in a regular (non-try) procedure.
Suggested change
local procedure TryApplyBooleanFilter(FilterExpression: Text)
[TryFunction]
local procedure TryApplyBooleanFilter(FilterExpression: Text)
var
QltyBooleanParsing: Codeunit "Qlty. Boolean Parsing";
begin
if not QltyBooleanParsing.CanTextBeInterpretedAsBooleanIsh(FilterExpression) then
Error('');
end;

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

var
QltyBooleanParsing: Codeunit "Qlty. Boolean Parsing";
begin
// Valid boolean allowable values are: Yes, No, True, False, 1, 0, On, Off (and variations)
if not QltyBooleanParsing.CanTextBeInterpretedAsBooleanIsh(FilterExpression) then
Error(InvalidAllowableValuesFormatErr, FilterExpression, '', 'Boolean');
end;

[TryFunction]
local procedure TryApplyDateFilter(FilterExpression: Text)
var
TempDateLookupBuffer: Record "Date Lookup Buffer" temporary;
begin
TempDateLookupBuffer.SetFilter("Period Start", FilterExpression);
if TempDateLookupBuffer.IsEmpty() then;
end;

[TryFunction]
local procedure TryApplyDateTimeFilter(FilterExpression: Text)
var
TempQltyInspectionHeader: Record "Qlty. Inspection Header" temporary;
begin
TempQltyInspectionHeader.SetFilter("Finished Date", FilterExpression);
if TempQltyInspectionHeader.IsEmpty() then;
end;

local procedure ValidateAllowableValuesOnText(NumberOrNameOfTestNameForError: Text; var TextToValidate: Text[250]; AllowableValues: Text; QltyTestValueType: Enum "Qlty. Test Value Type"; var TempBufferQltyTestLookupValue: Record "Qlty. Test Lookup Value" temporary; QltyCaseSensitivity: Enum "Qlty. Case Sensitivity")
var
QltyBooleanParsing: Codeunit "Qlty. Boolean Parsing";
Expand Down Expand Up @@ -393,9 +498,8 @@ codeunit 20410 "Qlty. Result Evaluation"
if not (IsBlankOrEmptyCondition(AllowableValues) and (TextToValidate = '')) then
if not QltyResultEvaluation.CheckIfValueIsString(TextToValidate, ConvertStr(AllowableValues, ',', '|'), QltyCaseSensitivity) then
Error(NotInAllowableValuesErr, TextToValidate, NumberOrNameOfTestNameForError, AllowableValues);

QltyTestValueType::"Value Type Option",
QltyTestValueType::"Value Type Table Lookup":
QltyTestValueType::"Value Type Table Lookup":
begin
TextToValidate := CopyStr(TextToValidate.Trim(), 1, MaxStrLen(TextToValidate));

Expand All @@ -419,6 +523,7 @@ codeunit 20410 "Qlty. Result Evaluation"
Error(NotInAllowableValuesErr, TextToValidate, NumberOrNameOfTestNameForError, AllowableValues);
end;
end;

OnAfterValidateAllowableValuesOnText(NumberOrNameOfTestNameForError, TextToValidate, AllowableValues, QltyTestValueType, TempBufferQltyTestLookupValue, QltyCaseSensitivity);
end;

Expand Down
13 changes: 13 additions & 0 deletions src/Apps/W1/Quality Management/app/src/Configuration/Template/Test/QltyTest.Table.al
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,11 @@ table 20401 "Qlty. Test"
{
Caption = 'Allowable Values';
ToolTip = 'Specifies an expression for the range of values you can enter or select for the Test. Depending on the Test Value Type, the expression format varies. For example if you want a measurement such as a percentage that collects between 0 and 100 you would enter 0..100. This is not the pass or acceptable condition, these are just the technically possible values that the inspector can enter. You would then enter a passing condition in your result conditions. If you had a result of Pass being 80 to 100, you would then configure 80..100 for that result.';

trigger OnValidate()
begin
Rec.ValidateAllowableValuesFormat();

if Rec."Test Value Type" in [Rec."Test Value Type"::"Value Type Option", Rec."Test Value Type"::"Value Type Table Lookup"] then
Rec."Allowable Values" := CopyStr(Rec."Allowable Values".Replace(', ', ','), 1, MaxStrLen(Rec."Allowable Values"));
end;
Expand Down Expand Up @@ -472,6 +475,16 @@ table 20401 "Qlty. Test"
QltyResultEvaluation.ValidateAllowableValuesOnTest(Rec);
end;

/// <summary>
/// Validates that the allowable values expression is a valid filter for the test value type.
/// </summary>
procedure ValidateAllowableValuesFormat()
var
QltyResultEvaluation: Codeunit "Qlty. Result Evaluation";
begin
QltyResultEvaluation.ValidateAllowableValuesFormat(Rec);
end;

/// <summary>
/// Code = the unique code
/// Description = raw description.
Expand Down
Loading
Loading