N°9205 - Show progress messages in setup sequencer

[Lenaick]

Base information

Question	Answer
Related to a SourceForge thread / Another PR / A GitHub Issue / Combodo ticket?	N°9205
Type of change?	Enhancement

Objective

Show progress messages in setup sequencer

Proposed solution

Display a message confirming that the previous step was successful each time we move on to the next step

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 without digging in the code?

[greptile-apps]

Greptile Summary

This PR adds per-step progress messages to the setup wizard, displaying a green success banner each time a sequencer step completes successfully, for both the application-install and data-audit flows.

• StepSequencer::GetNextStep gains a $sPrevStepSuccessMessage parameter; ApplicationInstallSequencer exposes a SUCCESS_LABELS map and a ComputeNextStep helper to populate it automatically; DataAuditSequencer passes inline strings to each GetNextStep call.
• WizStepInstall::AddPrevStepSuccessMessage appends a .message-valid div to #progress_content via a jQuery add_ready_script, called in both the in-progress and completion branches of AsyncAction.
• css/setup.scss gains an #progress_content block that sets min-height, overflow: auto, and a 1.5rem top-margin for the first success message appended after a non-.message element.

Confidence Score: 5/5

Safe to merge — the change is additive and well-scoped, touching only the sequencer return values and the wizard JS output logic.

All changed code paths follow existing patterns; the new SUCCESS_LABELS map and ComputeNextStep helper in ApplicationInstallSequencer are straightforward, StepSequencer::GetNextStep defaults the new parameter to an empty string preserving backward compatibility, and the CSS additions are isolated to #progress_content. No existing behavior is altered.

No files require special attention.

Important Files Changed

Filename	Overview
setup/sequencers/ApplicationInstallSequencer.php	Added SUCCESS_LABELS constant mapping each step to its completion message, and a ComputeNextStep helper that feeds the correct prev-step-success-message into GetNextStep; no logic errors detected.
setup/sequencers/DataAuditSequencer.php	Each GetNextStep call now passes an inline success message string as $sPrevStepSuccessMessage; the Copying... string passed as $sMessage in the copy case is unused in both web and CLI OK paths but causes no runtime issue.
setup/sequencers/StepSequencer.php	Added $sPrevStepSuccessMessage parameter to GetNextStep and includes it as prev-step-success-message in the returned array; backward-compatible since it defaults to an empty string.
setup/wizardsteps/WizStepInstall.php	New AddPrevStepSuccessMessage method appends a green .message-valid banner to #progress_content; call sites use static:: (late static binding) which correctly supports subclass overrides.
css/setup.scss	Added #progress_content block with min-height, overflow: auto, and a 1.5rem top-margin rule for the first .message that follows a non-.message element; CSS compiled into setup.css.

_{Reviews (3): Last reviewed commit: "N°9205 - Show progress messages in setup..." | Re-trigger Greptile}

[Hipska]

Do you have an example how it would look?

[Molkobain]

Do you have an example how it would look?

Curious as well :)

[odain-cbd]

great job. sequencer behaviour has been enhanced. please adapt their tests. i guess they will fail otherwise...

[Lenaick]

Do you have an example how it would look?

I've added some screenshots to the PR description, the messages still need to be approved by our PO

[Hipska]

Looks good. Is that "verification" step something new?

[Lenaick]

Looks good. Is that "verification" step something new?

@Hipska yes, this is a new step included in the process for uninstalling extensions. It checks whether there is any data that needs to be cleared before running the setup

[Hipska]

I have doubts about the "Data consistency check"..

What about custom extensions that do a whole datamodel rework/migration in their 2.0 version? I currently solve consistency problems in BeforeDatabaseCreation, and AfterDatabaseCreation, but that check seems to be done even before that, so consistency will fail?

[BenGrenoble]

@Hipska

This data consistency is a new step checking if a subclass has been removed.
If yes it'll suggest to clean the data before uninstalling the extension.
You will still be able to use BeforeDatabaseCreation and AfterDatabaseCreation.
Moreover if you remove a subclass and want to migrate the data AfterDatabaseCreation it's still possible by choosing ignore and continue.

[Hipska]

So it will give useless and frightening warnings to the users, even while there are perfect, clean migration steps under the hood?

This will give also warnings for all extensions that somehow use ModuleInstallerAPI::RenameClassInDB?

[odain-cbd]

I have doubts about the "Data consistency check"..

What about custom extensions that do a whole datamodel rework/migration in their 2.0 version? I currently solve consistency problems in BeforeDatabaseCreation, and AfterDatabaseCreation, but that check seems to be done even before that, so consistency will fail?

data audit step occurs in the middle of iTop setup. after compilation and before real application upgrade (where BeforeDatabaseCreation/AfterDatabaseCreation will be executed among others).

in case of data possible errors setup hangs and remains as before (unless you force it to go further). admin can switch to data cleanup page. this not a under the hood process...

[Hipska]

in case of data possible errors setup hangs and remains as before (unless you force it to go further). admin can switch to data cleanup page. this not a under the hood process...

So the user will see the consistency check failing, but still able to click to continue setup (where it will run BeforeDatabaseCreation and AfterDatabaseCreation methods)?

Or how should I see that, do you have a screenshot of such "setup hangs" scenario? And what is that "admin can switch to data cleanup page"?

[BenGrenoble]

It's really good to know that you're able to do this "under the hood" but most of our community is only using XML to customize iTop. The uninstallation functionality has a real value for them.
Also it will allow to remove lots of existing modules and extensions without having to code a specific algorithm for each of them.

[Hipska]

I don't feel like that's an answer to my questions.

There is no way to do DB migrations with XML.