Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.0] Add a "progress" system #18296

Merged
merged 33 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
fbefab0
First draft of refactoring install process with a progress display sy…
Pierstoval Nov 12, 2024
d365d13
Remove InstallTweaks and move its code to install.php
Pierstoval Nov 15, 2024
0535840
Remove "echo" statement in Toolbox::createSchema, no longer needed
Pierstoval Nov 15, 2024
1e4a23d
Removed unused session install flag removal in install.php
Pierstoval Nov 15, 2024
c8aca33
Remove query length check in DBmysql
Pierstoval Nov 15, 2024
97e62ba
Rename Step7Controller to InstallController and update db setup routes
Pierstoval Nov 15, 2024
218aea5
Add ProgressChecker::hasProgress
Pierstoval Nov 15, 2024
00ac828
Enhance install UI
Pierstoval Nov 15, 2024
1e3d043
Add error catch for db requests in case it fails
Pierstoval Nov 18, 2024
bb47be6
Move further in progress bar definition for installation
Pierstoval Nov 20, 2024
38de7d9
Finish remaining small issues for progress in install
Pierstoval Nov 21, 2024
80ab668
Isolate progress function more
Pierstoval Nov 21, 2024
ab8efca
Fix missing checks on progress success
Pierstoval Nov 21, 2024
044b94f
Move check progress outside install script
Pierstoval Nov 22, 2024
3106825
More things about progress are standardized to create_progress_bar.js
Pierstoval Nov 22, 2024
feb0e2c
Simplify UI and its setup and make sure stop has less side-effects
Pierstoval Nov 22, 2024
2f35b89
Small fixes for CS and error handling
Pierstoval Nov 22, 2024
14bf01c
Small cs fix
Pierstoval Nov 25, 2024
ed39b63
Attempt at fixing sonarcube issue
Pierstoval Nov 25, 2024
4043807
Updates after review
Pierstoval Nov 25, 2024
35ba790
Add a timeout detection mechanism
Pierstoval Nov 26, 2024
927a89e
Convert install scripts to modules
Pierstoval Nov 26, 2024
8921ab7
Fail on http >=400 instead of 300
Pierstoval Nov 26, 2024
b6be68c
Keep progress bar args as object but destructure it
Pierstoval Nov 26, 2024
b874a55
CS fix
Pierstoval Nov 27, 2024
78106a5
Take timeout in account
Pierstoval Nov 27, 2024
f04ee10
Add success callback to progress
Pierstoval Nov 27, 2024
f1556b5
Fixes after review
Pierstoval Nov 27, 2024
2a37c9a
Rename db url
Pierstoval Nov 27, 2024
678c3ac
small enhancements/fixes; code clean; renamings
cedric-anne Nov 27, 2024
5ad8d32
Display a progress bar on CLI install
cedric-anne Nov 27, 2024
444a724
rename manager -> storage
cedric-anne Nov 27, 2024
b528727
simplify progress bar handling
cedric-anne Nov 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dependency_injection/services.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
$services->load('Glpi\Controller\\', $projectDir . '/src/Glpi/Controller');
$services->load('Glpi\Http\\', $projectDir . '/src/Glpi/Http');
$services->load('Glpi\DependencyInjection\\', $projectDir . '/src/Glpi/DependencyInjection');
$services->load('Glpi\Progress\\', $projectDir . '/src/Glpi/Progress');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is is alright to load the full Progress directory here ?

SessionProgress is not a service, shouldn't loading it in the dependency injection trigger an error as its constructor expect some parameters ?


/**
* Override Symfony's logger.
Expand Down
87 changes: 53 additions & 34 deletions install/install.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,22 @@
use Glpi\Toolbox\Filesystem;

/**
* @var array $CFG_GLPI
* @var \Psr\SimpleCache\CacheInterface $GLPI_CACHE
*/
global $GLPI_CACHE;
global $CFG_GLPI, $GLPI_CACHE;

// Force `root_doc` value
$request = \Symfony\Component\HttpFoundation\Request::createFromGlobals();
$CFG_GLPI['root_doc'] = $request->getBasePath();

$GLPI_CACHE = (new CacheManager())->getInstallerCacheInstance();

if (isset($_POST["language"]) && isset($CFG_GLPI["languages"][$_POST["language"]])) {
$_SESSION["glpilanguage"] = $_POST["language"];
Session::loadLanguage(with_plugins: false);
}

Session::checkCookieSecureConfig();

//Print a correct Html header for application
Expand Down Expand Up @@ -227,36 +237,41 @@ public function __construct($dbh)
//Step 4 Create and fill database.
function step4($databasename, $newdatabasename)
{
/**
* @var array $CFG_GLPI
*/
global $CFG_GLPI;

$host = $_SESSION['db_access']['host'];
$user = $_SESSION['db_access']['user'];
$password = $_SESSION['db_access']['password'];

//display the form to return to the previous step.
echo "<h3>" . __s('Initialization of the database') . "</h3>";
echo "<br />";

$prev_form = function ($host, $user, $password) {
echo "<br><form action='install.php' method='post'>";
$prev_form = function ($host, $user, $password, bool $disabled = false) {
echo "<form action='install.php' method='post' class='d-inline'>";
echo "<input type='hidden' name='db_host' value='" . htmlescape($host) . "'>";
echo "<input type='hidden' name='db_user' value='" . htmlescape($user) . "'>";
echo " <input type='hidden' name='db_pass' value='" . htmlescape(rawurlencode($password)) . "'>";
echo "<input type='hidden' name='update' value='no'>";
echo "<input type='hidden' name='install' value='Etape_2'>";
echo "<p class='submit'><input type='submit' name='submit' class='submit' value='" .
__s('Back') . "'></p>";
echo "<button type='submit' name='submit' class='btn btn-warning' " . ($disabled ? 'disabled="disabled"' : '') . ">";
echo "<i class='fas fa-chevron-left me-1 fa-2x alert-icon'></i>";
echo __s("Back");
echo "</button>";
Html::closeForm();
};

//Display the form to go to the next page
$next_form = function () {
(new CacheManager())->getInstallerCacheInstance();

echo "<br><form action='install.php' method='post'>";
$next_form = function (bool $disabled = false) {
echo "<form action='install.php' method='post' class='d-inline'>";
echo "<input type='hidden' name='install' value='Etape_4'>";
echo "<button type='submit' name='submit' class='btn btn-primary'>
" . __s('Continue') . "
<i class='fas fa-chevron-right ms-1'></i>
</button>";
echo "<button type='submit' name='submit' class='btn btn-primary' " . ($disabled ? 'disabled="disabled"' : '') . ">";
echo __s('Continue');
echo "<i class='fas fa-chevron-right ms-1'></i>";
echo "</button>";
Html::closeForm();
};

Expand Down Expand Up @@ -297,7 +312,7 @@ public function __construct($dbh)

if (
!$link->select_db($databasename)
&& !$link->query("CREATE DATABASE IF NOT EXISTS `" . $databasename . "`")
&& !$link->query(\sprintf("CREATE DATABASE IF NOT EXISTS `%s`;", $databasename))
) {
echo __s('Error in creating database!');
echo "<br>" . sprintf(__s('The server answered: %s'), htmlescape($link->error));
Expand Down Expand Up @@ -329,22 +344,28 @@ public function __construct($dbh)

if ($success) {
echo "<p>" . __s('Initializing database tables and default data...') . "</p>";
try {
Toolbox::createSchema($_SESSION["glpilanguage"]);
} catch (\Throwable $e) {
echo "<p>"
. sprintf(
__s('An error occurred during the database initialization. The error was: %s'),
'<br />' . htmlescape($e->getMessage())
)
. "</p>";
@unlink(GLPI_CONFIG_DIR . '/config_db.php'); // try to remove the config file, to be able to restart the process
$prev_form($host, $user, $password);
return;
}
echo "<p>" . __s('OK - database was initialized') . "</p>";

$next_form();
echo '<div id="glpi_install_messages_container"></div>';

echo '<div class="text-center">';
echo '<div id="glpi_install_back" class="d-none">';
$prev_form($host, $user, $password, disabled: true);
echo '</div>';
echo '<div id="glpi_install_success" class="d-none">';
$next_form(disabled: true);
echo '</div>';
echo '</div>';

echo \sprintf(
<<<HTML
<script defer type="module">
import { init_database } from '%s/js/modules/GlpiInstall.js';
init_database("%s");
</script>
HTML,
$CFG_GLPI['root_doc'],
\Glpi\Controller\InstallController::PROGRESS_KEY_INIT_DATABASE,
);
} else { // can't create config_db file
echo "<p>" . __s('Impossible to write the database setup file') . "</p>";
$prev_form($host, $user, $password);
Expand Down Expand Up @@ -424,6 +445,7 @@ function step8()

function update1($dbname)
{
$_SESSION['is_installing'] = false;

$host = $_SESSION['db_access']['host'];
$user = $_SESSION['db_access']['user'];
Expand Down Expand Up @@ -478,11 +500,6 @@ function update1($dbname)
Session::start();
error_reporting(0); // we want to check system before affraid the user.

if (isset($_POST["language"]) && isset($CFG_GLPI["languages"][$_POST["language"]])) {
$_SESSION["glpilanguage"] = $_POST["language"];
}

Session::loadLanguage('', false);

/**
* @since 0.84.2
Expand Down Expand Up @@ -525,6 +542,8 @@ function checkConfigFile()
$_POST["db_pass"] = rawurldecode($_POST["db_pass"]);
}

$_SESSION['is_installing'] = true;

switch ($_POST["install"]) {
case "lang_select": // lang ok, go accept licence
checkConfigFile();
Expand Down
84 changes: 84 additions & 0 deletions js/modules/GlpiInstall.js
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think this whole file should be avoided.

3rd party UI components like bootstrap's dropdown or modals are configured only using html's data attribute and IMO we should be able to do the same here as it make them much more convenient to use.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a back and a continue button that have to be displayed at the end of the process. This will probably be different in other usages of this component. Also, the endpoint to fetch may require some parameters, and it may be difficult to pass them via data-attributes. For instance, for the massive actions, we will want to send data in JSON format to remove the current limitation related to the form format (max_input_vars limitation).

Maybe the error message display could be mutualize, we will see when we will use this component in other contexts.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a back and a continue button that have to be displayed at the end of the process.

The buttons are already identified by some unique identifier like glpi_install_success.
It wouldn't be too hard to change it to something like data-glpi-progress-show-on-success.

This will probably be different in other usages of this component.

I agree that there may be rare edge cases where it will be needed to have some custom code.

But we should still be able to provide something simple to use for the common cases (I suppose most progress bar will either display something or redirect the user once they are finished) through data attributes.

Taking the boostrap example once again, they have data attribute for common cases and emit some events so that specific behavior can be implemented by adding listeners.

For instance, for the massive actions, we will want to send data in JSON format to remove the current limitation related to the form format (max_input_vars limitation)

I think this should be handled by using the appropriate HTML elements.

For example, the install progress display a <progress data-glpi-progess='...'></progress> item that is executed when the page is displayed.

The massive actions would instead use a <form data-glpi-progress='...'></form> item instead that is executed when the form is submitted (thus passing the JSON along).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not against mutualizing code. It is something we could try to achieve when we will use the progress bar system in massive actions.

Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* ---------------------------------------------------------------------
*
* GLPI - Gestionnaire Libre de Parc Informatique
*
* http://glpi-project.org
*
* @copyright 2015-2024 Teclib' and contributors.
* @licence https://www.gnu.org/licenses/gpl-3.0.html
*
* ---------------------------------------------------------------------
*
* LICENSE
*
* This file is part of GLPI.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*
* ---------------------------------------------------------------------
*/

import { ProgressBar } from './ProgressBar.js';

export async function init_database(progress_key)
{
function message(message_list_element, text) {
const alert = document.createElement('div');
alert.setAttribute('class', 'alert alert-important alert-danger my-2 mx-4');
alert.setAttribute('role', 'alert');
alert.innerHTML = `
<i class="fas fa-2x fa-exclamation-triangle align-middle"></i>
${text}
`;
message_list_element.appendChild(alert);
}

const messages_container = document.getElementById('glpi_install_messages_container');
const success_container = document.getElementById('glpi_install_success');
const back_button_container = document.getElementById('glpi_install_back');

const message_list_element = document.createElement('div');

const progress = new ProgressBar({
key: progress_key,
container: messages_container,
success_callback: () => {
success_container.querySelector('button[type="submit"]').removeAttribute('disabled');
success_container.setAttribute('class', 'd-inline');
},
error_callback: (msg) => {
back_button_container.querySelector('button[type="submit"]').removeAttribute('disabled');
back_button_container.setAttribute('class', 'd-inline');
message(message_list_element, msg);
},
});

progress.init();

messages_container.appendChild(message_list_element);

setTimeout(() => {
progress.start();
}, 1500);

try {
await fetch(`${CFG_GLPI.root_doc}/install/init_database`, {method: 'POST'});
} catch {
// DB installation is really long and can result in a `Proxy timeout` error.
// It does not mean that the process is killed, it just mean that the proxy did not wait for the response
// and send an error to the client.
// Here we catch any error to make it silent, but we will handle it with the ProgressBar error_callback.
}
}
Loading