Skip to content

[Quality Management] Init values in the test if there are two or more promoted quality results#8255

Open
JakovljevicDusan wants to merge 1 commit into
mainfrom
bugs/QM-DefaultConditionsInTestForBlankValue
Open

[Quality Management] Init values in the test if there are two or more promoted quality results#8255
JakovljevicDusan wants to merge 1 commit into
mainfrom
bugs/QM-DefaultConditionsInTestForBlankValue

Conversation

@JakovljevicDusan
Copy link
Copy Markdown
Contributor

@JakovljevicDusan JakovljevicDusan commented May 21, 2026

What & why

Handle default condition in Test for blank value

Linked work

Fixes AB#622769

@JakovljevicDusan JakovljevicDusan requested a review from a team as a code owner May 21, 2026 00:55
@github-actions github-actions Bot added the AL: Apps (W1) Add-on apps for W1 label May 21, 2026
@github-actions github-actions Bot added this to the Version 29.0 milestone May 21, 2026
ToTestQltyIResultConditConf."Condition Description" := CopyStr(Condition, 1, MaxStrLen(ToTestQltyIResultConditConf."Condition Description"));
ToTestQltyIResultConditConf.Insert();
end else
if AlwaysUpdateExistingCondition or (OnlyOverwriteIfADefaultCondition and (ToTestQltyIResultConditConf.Condition in [QltyInspectionResult."Default Boolean Condition", QltyInspectionResult."Default Number Condition", QltyInspectionResult."Default Text Condition"])) then begin
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.

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

Empty condition silently overwrites on upgrade

Adding '' to the in list means any existing record whose Condition field is blank will now be treated as a default and overwritten during configuration seeding — even if the blank value was intentionally left by a user or previous customization. This silent data mutation can occur on every upgrade run that calls this code path with OnlyOverwriteIfADefaultCondition = true.

Recommendation:

  • Confirm whether a blank Condition should truly be equated with the named default values. If so, document this assumption. If not, keep the guard to the three explicit default constants and handle blank conditions separately (e.g., only overwrite blank on fresh insert). At minimum, add a comment explaining why '' is included alongside the named defaults.
Suggested change
if AlwaysUpdateExistingCondition or (OnlyOverwriteIfADefaultCondition and (ToTestQltyIResultConditConf.Condition in [QltyInspectionResult."Default Boolean Condition", QltyInspectionResult."Default Number Condition", QltyInspectionResult."Default Text Condition"])) then begin
if AlwaysUpdateExistingCondition or
(OnlyOverwriteIfADefaultCondition and
(ToTestQltyIResultConditConf.Condition in
// '' treated as uninitialized; safe to overwrite just like named defaults
['',
QltyInspectionResult."Default Boolean Condition",
QltyInspectionResult."Default Number Condition",
QltyInspectionResult."Default Text Condition"])) then begin

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

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

Labels

AL: Apps (W1) Add-on apps for W1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant