N°9639 - Uninstallation Analysis result doesn't list all the object

[eespie]

N°9639 - Uninstallation Analysis result doesn't list all the object

Refactor the count algorithm to have a result much faster

[greptile-apps]

Greptile Summary

This PR refactors the uninstallation analysis (deletion plan) from an ORM-based object-by-object count to a SQL-driven ID-tracking approach (StaticDeletionPlan / DeletionPlanEntity / DeletionPlanItem), fixing the bug where the summary table omitted classes that appeared only as collateral deletions. It also tightens the setup wizard's token flow by replacing direct chmod calls with SetupUtils::CreateSetupToken().

• New StaticDeletionPlan service: walks the MetaModel reference graph recursively, collecting IDs to delete/update/issue via direct SQL queries instead of loading full iTop objects; backed by a new StaticDeletionPlanTest suite covering cascades, circular refs, and multiple ext-keys.
• Setup wizard hardening: AjaxController::LaunchSetup() now calls CreateSetupToken() instead of chmod; wizard.php defers token creation until after the config-file writability check; WizStepWelcome injects the "Keep current choices" button into the shared wizard button bar via jQuery.
• CMDBSource::QueryToCol: drops the error-suppressing @ in favour of an explicit null guard, ensuring null column values are skipped cleanly.

Confidence Score: 4/5

The core deletion-plan computation is correct for typical acyclic datamodels and is well-covered by the new test suite; a few edge cases around recursive ID scoping and circular references raised in prior review threads remain open.

The new StaticDeletionPlan logic is a significant algorithmic change that drives a data-sensitive workflow (deciding what gets deleted during a module uninstall). The constructor issue in DeletionPlanEntity is benign today but is a latent maintenance trap, and concerns about ID-scope correctness across loop iterations in DeletionPlanForReferencingClasses are not yet fully resolved. The setup wizard token changes are straightforward and low-risk.

datamodels/2.x/combodo-data-feature-removal/src/Service/StaticDeletionPlan.php and datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php deserve a second look before merge.

Important Files Changed

Filename	Overview
datamodels/2.x/combodo-data-feature-removal/src/Service/StaticDeletionPlan.php	New service replacing DataCleanupService with a SQL-based, ID-tracking deletion plan. Core logic is sound but has open questions around $sIdsToRemove scoping, non-nullable circular FK chains, and DEL_MOVEUP on non-hierarchical keys (all noted in prior review threads).
datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanEntity.php	New entity class with a constructor that declares no parameters but references three undefined variables via ??; phpdoc @param annotations describe parameters that are never actually accepted.
datamodels/2.x/combodo-data-feature-removal/src/Entity/DeletionPlanItem.php	New lightweight value object wrapping an ID array with Merge, Count, and FilterBy helpers; straightforward and correct.
datamodels/2.x/combodo-data-feature-removal/src/Controller/DataFeatureRemovalController.php	Switches GetDeletionPlanSummaryTable to use the new StaticDeletionPlan; adds a GetStaticDeletionPlan call whose result is computed but never used (already flagged in a prior review thread).
core/cmdbsource.class.inc.php	Replaces the error-suppressing @ with an explicit null check; correctly skips null column values while preserving the same return behaviour for callers.
setup/wizard.php	Removes EraseSetupToken() from the config-not-writable error branch; safe because the else branch only runs when IsSessionSetupTokenValid() is false, so no valid token exists to erase.
setup/wizardsteps/WizStepWelcome.php	Moves 'Keep current choices' button into a hidden form and injects it into the shared button bar via a positional jQuery selector; works when JS is available but the selector is fragile.
setup/wizardcontroller.class.inc.php	Removes the PreFormDisplay() call; the base method was a no-op so removal has no functional impact.
datamodels/2.x/itop-core-update/src/Controller/AjaxController.php	Replaces the direct chmod of the config file with SetupUtils::CreateSetupToken(), aligning the launch-setup flow with the new token-based handshake.
datamodels/2.x/itop-core-update/src/Controller/UpdateController.php	Drops the redundant chmod call from OperationRunSetup; token check is preserved, redirect behaviour is unchanged.
tests/php-unit-tests/unitary-tests/datamodels/2.x/combodo-data-feature-removal/StaticDeletionPlanTest.php	New comprehensive test suite covering initial deletion plan, nullable/non-nullable ext-keys, cascades, issues (DEL_MANUAL), multiple ext-keys, and the circular-refs scenario.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GetStaticDeletionPlan aClasses] --> B[For each sClass in aClasses]
    B --> C[GetInitialClassDeletionPlan SELECT all IDs]
    C --> D[Create DeletionPlanEntity Merge IDs into oDelete]
    D --> E[aDeletionPlan sClass = entity]
    E --> F[DeletionPlanForReferencingClasses sClass]
    F --> G[EnumReferencingClasses]
    G --> H{For each sRemoteClass}
    H --> I{IsNullAllowed?}
    I -- Yes --> J[UpdateExtKeyNullable oUpdate.Merge]
    I -- No --> K[GetRemoteIdsForExtKey]
    K --> L{DEL_MANUAL?}
    L -- Yes --> M[oIssue.Merge stop]
    L -- No --> N{DEL_MOVEUP + Hierarchical?}
    N -- Yes --> O[UpdateHierarchicalExtKey oUpdate.Merge]
    N -- No/After --> P{aRemoteIds not empty?}
    O --> P
    P -- Yes --> Q[oDelete.Merge new IDs]
    Q --> R[Recurse DeletionPlanForReferencingClasses sRemoteClass]
    R --> H
    P -- No --> H
    H --> S[Return aDeletionPlan]
    S --> T[GetCleanupSummary FilterUpdatesByDeletes Build summary]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[GetStaticDeletionPlan aClasses] --> B[For each sClass in aClasses]
    B --> C[GetInitialClassDeletionPlan SELECT all IDs]
    C --> D[Create DeletionPlanEntity Merge IDs into oDelete]
    D --> E[aDeletionPlan sClass = entity]
    E --> F[DeletionPlanForReferencingClasses sClass]
    F --> G[EnumReferencingClasses]
    G --> H{For each sRemoteClass}
    H --> I{IsNullAllowed?}
    I -- Yes --> J[UpdateExtKeyNullable oUpdate.Merge]
    I -- No --> K[GetRemoteIdsForExtKey]
    K --> L{DEL_MANUAL?}
    L -- Yes --> M[oIssue.Merge stop]
    L -- No --> N{DEL_MOVEUP + Hierarchical?}
    N -- Yes --> O[UpdateHierarchicalExtKey oUpdate.Merge]
    N -- No/After --> P{aRemoteIds not empty?}
    O --> P
    P -- Yes --> Q[oDelete.Merge new IDs]
    Q --> R[Recurse DeletionPlanForReferencingClasses sRemoteClass]
    R --> H
    P -- No --> H
    H --> S[Return aDeletionPlan]
    S --> T[GetCleanupSummary FilterUpdatesByDeletes Build summary]

Loading

_{Reviews (2): Last reviewed commit: "N°9639 - Remove unused deletion plan fro..." | Re-trigger Greptile}

[eespie]

@greptileai