N°8758 - Fix mandatory caselog in transition requiring double confirmation

[steffunky]

Base information

Question	Answer
Related to a SourceForge thread / Another PR / Combodo ticket?	N°8758
Type of change?	Bug fix

Symptom (bug) / Objective (enhancement)

When a caselog attribute is set mandatory in a transition, even after adding content to it a "Please fill all mandatory fields" error appears, a second submit is necessary

Reproduction procedure (bug)

Use this delta, then try to close a work order on iTop 3.2.2

<class id="WorkOrder" _delta="merge">
      <lifecycle>
        <states>
          <state id="closed">
            <flags>
              <attribute id="log" _delta="define">
                <mandatory/>
                <must_prompt/>
              </attribute>
            </flags>
          </state>
        </states>
      </lifecycle>
    </class>

Cause (bug)

When updating to CKEditor5, ValidateCKEditField was updated but not ValidateCaseLogField that's used in transitions

Proposed solution (bug and enhancement)

Factorize ValidateCKEditField code and use it in ValidateCaseLogField

Checklist before requesting a review

• I have performed a self-review of my code
• I have tested all changes I made on an iTop instance
• I have added a unit test, otherwise I have explained why I couldn't
• Is the PR clear and detailed enough so anyone can understand digging in the code?

[greptile-apps]

Greptile Summary

This PR fixes a double-confirmation bug on mandatory caselog fields in lifecycle transitions by refactoring the CKEditor validation logic into a shared EvaluateCKEditorValidation helper. ValidateCaseLogField now reads content directly from the CKEditor instance (via getData()) instead of the underlying textarea value, which is the correct fix—the old textarea approach was not synchronised with CKEditor5's internal state, causing the "field is empty" false positive that required a second submit.

Key changes:

• New EvaluateCKEditorValidation(oOptions) helper centralises field lookup, CKEditor instance acquisition, placeholder/image detection, and the change:data event listener.
• ValidateCKEditField and ValidateCaseLogField are both refactored to delegate to this helper via callbacks (validate, onRetry, onChange).
• The old 500 ms polling timer in ValidateCaseLogField is removed in favour of the event-driven approach already used by ValidateCKEditField.

Issues found:

• P1 — broken originalValue comparison for CaseLog: ValidateCaseLogField passes originalValue: to the options object (line 445), but EvaluateCKEditorValidation reads oOptions.sOriginalValue. This mismatch means sTextOriginalContents is always undefined, so the "value must be changed" validation branch in ValidateCaseLogField is silently dead. ValidateCKEditField correctly uses sOriginalValue:.
• P2 — minor style: Missing blank line between ValidateCKEditField and ResetPwd, and one line in ValidateCKEditField uses spaces instead of tabs.

Confidence Score: 4/5

Safe to merge after fixing the originalValue → sOriginalValue key typo, which silently disables the "value must be changed" check for caselog fields.

The core fix (reading from CKEditor instead of the textarea) is sound and directly addresses the reported double-confirmation bug. However, the property key mismatch (originalValue vs sOriginalValue) is a present defect that breaks a validation path for caselog fields — if a transition requires the caselog value to change, that check will now silently pass regardless of content. This warrants a P1 and prevents a 5/5.

js/forms-json-utils.js — specifically the originalValue key on line 445 inside ValidateCaseLogField.

Important Files Changed

Filename	Overview
js/forms-json-utils.js	Refactors CKEditor validation into a shared EvaluateCKEditorValidation helper and fixes ValidateCaseLogField to read directly from CKEditor instead of the underlying textarea; however, ValidateCaseLogField passes originalValue instead of sOriginalValue to the shared helper, silently breaking the "value must be changed" validation path for case log fields.

Sequence Diagram

sequenceDiagram
    participant Form
    participant ValidateCaseLogField
    participant EvaluateCKEditorValidation
    participant CombodoCKEditorHandler
    participant CKEditor

    Form->>ValidateCaseLogField: validate(sFieldId, bMandatory, ...)
    ValidateCaseLogField->>EvaluateCKEditorValidation: call with options {sFieldId, sFormId, originalValue ⚠️, validate, onRetry, onChange}
    EvaluateCKEditorValidation->>CombodoCKEditorHandler: GetInstanceSynchronous(#sFieldId)
    alt CKEditor not yet ready
        CombodoCKEditorHandler-->>EvaluateCKEditorValidation: undefined
        EvaluateCKEditorValidation->>CombodoCKEditorHandler: GetInstance(#sFieldId).then(onRetry)
        EvaluateCKEditorValidation-->>ValidateCaseLogField: return false
    else CKEditor ready
        CombodoCKEditorHandler-->>EvaluateCKEditorValidation: oCKEditor instance
        EvaluateCKEditorValidation->>CKEditor: getData()
        CKEditor-->>EvaluateCKEditorValidation: sFormattedContent
        Note over EvaluateCKEditorValidation: Extract sTextContent from HTML
        Note over EvaluateCKEditorValidation: oOptions.sOriginalValue is undefined ⚠️ so sTextOriginalContents = undefined
        EvaluateCKEditorValidation->>ValidateCaseLogField: validate(sTextContent, undefined)
        ValidateCaseLogField-->>EvaluateCKEditorValidation: {bValid, sExplain}
        EvaluateCKEditorValidation->>CKEditor: once('change:data', onChange)
        EvaluateCKEditorValidation->>Form: ReportFieldValidationStatus(...)
        EvaluateCKEditorValidation-->>ValidateCaseLogField: return bValid
    end

Loading

_{Reviews (1): Last reviewed commit: "Bring back lost methods" | Re-trigger Greptile}