N°9121 - CSV Import : The advanced mode option no longer works as bef…

[bdalsass]

…ore.

Base information

Question	Answer
Related to a SourceForge thread / Another PR / Combodo ticket?	https://support.combodo.com/pages/UI.php?operation=details&class=Bug&id=9121
Type of change?	Bug fix

Symptom (bug) / Objective (enhancement)

CSV import advanced button doen't works as before

[greptile-apps]

Greptile Summary

This PR fixes the CSV Import advanced mode by replacing the previous full-page-reload approach (which was causing loss of state) with an AJAX-based workflow: toggling the advanced checkbox now fires a new display_classes_select AJAX call to rebuild the class dropdown in-place, and the helper logic is neatly extracted into a new ImportHelper class.

Key points:

• ImportHelper::GetClassesSelectUIBlock correctly migrates the previous page-scoped function into a proper, autoloaded class.
• The new display_classes_select AJAX endpoint uses proper sanitization (ENUM_SANITIZATION_FILTER_CLASS) and boolean-string conversion for the advanced flag.
• Race condition in DoAdvanced(): UpdateClassesSelect() is asynchronous, but DoMapping() is called immediately after without waiting for the callback. If the user had an advanced-only class selected and toggles off advanced mode, DoMapping will fire with the stale class name while the dropdown asynchronously removes it — leaving the mapping area inconsistent with the visible select.
• is_null($iActionCode) in ImportHelper is dead code since the parameter is typed as non-nullable int.
• No .fail() handler exists in UpdateClassesSelect, so a failed AJAX request leaves the select in a permanently blocked state.

Confidence Score: 4/5

Safe to merge for the common case, but the async race condition in DoAdvanced() can leave the mapping area inconsistent when an advanced-only class is selected and the user disables advanced mode.

One P1 finding remains: DoMapping() is called immediately after the async UpdateClassesSelect(), which can produce stale UI state for edge-case users who had an advanced-only class selected. The two P2 findings (missing .fail() handler and dead is_null check) are minor and don't block the primary user path. The refactoring itself is clean and the new AJAX endpoint is correctly secured.

pages/csvimport.php — the DoAdvanced() JavaScript function and UpdateClassesSelect() AJAX callback ordering.

Important Files Changed

Filename	Overview
pages/csvimport.php	Replaces full-page reload on advanced-mode toggle with AJAX calls; DoAdvanced() calls DoMapping() synchronously before the async UpdateClassesSelect() completes, creating a race condition where stale class/mapping data can be shown.
pages/ajax.csvimport.php	Adds new display_classes_select AJAX case that rebuilds the classes select via ImportHelper; parameter handling and sanitisation look correct.
sources/Application/Helper/ImportHelper.php	New helper class extracting GetClassesSelectUIBlock from csvimport.php into a reusable static method; is_null($iActionCode) is dead code since the parameter is typed as non-nullable int.
lib/composer/autoload_classmap.php	Registers ImportHelper in the Composer classmap; straightforward and correct.
lib/composer/autoload_static.php	Registers ImportHelper in the static autoloader; straightforward and correct.

Sequence Diagram

sequenceDiagram
    participant User
    participant Browser as "Browser (csvimport.php)"
    participant AjaxCSV as "ajax.csvimport.php"
    participant ImportHelper

    User->>Browser: "Toggle Advanced checkbox"
    Browser->>Browser: "DoAdvanced()"

    Browser->>AjaxCSV: "POST display_classes_select {class_name, advanced}"
    AjaxCSV->>ImportHelper: "GetClassesSelectUIBlock(name, class, action, bAdvanced)"
    ImportHelper-->>AjaxCSV: "Select UI block"
    AjaxCSV-->>Browser: "Rendered select HTML"
    Browser->>Browser: "replaceWith(data) — rebind change to DoMapping()"

    Note over Browser: DoMapping() called immediately before replaceWith completes
    Browser->>AjaxCSV: "POST display_mapping_form {class_name (possibly stale), advanced}"
    AjaxCSV-->>Browser: "Mapping form HTML"
    Browser->>Browser: "Update #mapping (may use stale class)"

Loading

_{Reviews (1): Last reviewed commit: "N°9121 - CSV Import : The advanced mode ..." | Re-trigger Greptile}

[Hipska]

Could you be a little more specific? "doesn't work as before" is not really helpful.

[bdalsass]

Could you be a little more specific? "doesn't work as before" is not really helpful.

As mentionned in the related ticket, iTop doesn't automaticly select id anymore (on Data Mapping stage)

[Hipska]

Thanks, I'm not able to see that related ticket (I already asked a few times for partners to also be able to see Bugs on the support portal).