"Undefined index: site_url" warning when installing mautic

[gramakri]

Bug Description

When installing mautic there is a warning Undefined index: site_url

Q	A
Mautic version	3.1.0
PHP version	7.3
Browser	CLI

Steps to reproduce

1. Install mautic via the CLI. I am pasting the command from the Cloudron package:

    sudo -u www-data --preserve-env php /app/code/bin/console mautic:install \
        --db_host="${CLOUDRON_MYSQL_HOST}" \
        --db_port="${CLOUDRON_MYSQL_PORT}" \
        --db_name="${CLOUDRON_MYSQL_DATABASE}" \
        --db_user="${CLOUDRON_MYSQL_USERNAME}" \
        --db_password="${CLOUDRON_MYSQL_PASSWORD}" \
        --db_table_prefix=null \
        --db_backup_tables=false \
        --admin_email=admin@cloudron.local \
        --admin_password=changeme123 \
        -- "${CLOUDRON_APP_ORIGIN}"

2. See warning messages like below:

  Mautic Install
==============

Parsing options and arguments...
0 - Checking installation requirements...
Ready to Install!
1 - Creating database...
PHP Notice:  Undefined index: site_url in /app/code/app/bundles/InstallBundle/Command/InstallCommand.php on line 500
1.1 - Creating schema...
PHP Notice:  Undefined index: site_url in /app/code/app/bundles/InstallBundle/Command/InstallCommand.php on line 500
1.2 - Loading fixtures...
PHP Notice:  Undefined index: site_url in /app/code/app/bundles/InstallBundle/Command/InstallCommand.php on line 500
2 - Creating admin user...
PHP Notice:  Undefined index: site_url in /app/code/app/bundles/InstallBundle/Command/InstallCommand.php on line 500
3 - Email configuration and final steps...

================
Install complete
================

3. Note that mautic appears to install fine despite above warnings.

[gramakri]

I have tried to debug this a bit:

1. The message comes because of https://github.com/mautic/mautic/blob/staging/app/bundles/InstallBundle/Command/InstallCommand.php#L500 $siteUrl = $params['site_url'];

2. The above function gets called from https://github.com/mautic/mautic/blob/staging/app/bundles/InstallBundle/Command/InstallCommand.php#L345, https://github.com/mautic/mautic/blob/staging/app/bundles/InstallBundle/Command/InstallCommand.php#L384 . Both dbParams and adminParam do not set have the site_url set. This is the reason for the above warning.

It looks like https://github.com/mautic/mautic/blob/staging/app/bundles/InstallBundle/Command/InstallCommand.php#L499 should only be done with EMAIL_STEP, no?

[mabumusa1]

@gramakri Thanks for reporting this issue. Would like to make a PR for this issue to fix it? I can support you in case you need help

[mabumusa1]

@gramakri following up on this issue? do you mind contribute your code changes?

[gramakri]

@mabumusa1 Yes, I am interested in getting this fixed. I found this issue when packaging mautic 3 for Cloudron (just recently announced - https://forum.cloudron.io/topic/3165/mautic-3-major-update). I will see if we hit any other issues, so I can work on the items together.

[mabumusa1]

@gramakri thanks for the contribution, please submit a PR as soon as possible, hopefully, we can get it merged into the next release within a week

[RCheesley]

Also pinging @madmath03 as I think we flagged up this warning when we were testing the install at CLI PR but decided to look into it at a later date.

Bumping this to 3.1.2 - @gramakri if you'd like to work on this we'd need the PR to be submitted and tested before the 18th October.

[madmath03]

Indeed we saw this warning during tests of #7395 with @RCheesley but left it for later as this warning does not seem to impact the install process.

As @gramakri mentioned, the final step could be moved into email step, but I think adding a new index for the final step might be more flexible.
I'm thinking about something like that:

/**
 * Class InstallService.
 */
class InstallService
{
    const CHECK_STEP    = 0;
    const DOCTRINE_STEP = 1;
    const USER_STEP     = 2;
    const EMAIL_STEP    = 3;
    const FINAL_STEP    = 4;

//...

/**
 * CLI Command to install Mautic.
 * Class InstallCommand.
 */
class InstallCommand extends ContainerAwareCommand
{
    /**
     * {@inheritdoc}
     */
    protected function configure()
    {
        $this
            ->setName('mautic:install')
            ->setDescription('Installs Mautic')
            ->setHelp('This command allows you to trigger the install process.')
            ->addArgument(
                'site_url',
                InputArgument::REQUIRED,
                'Site URL.',
                null
            )
            ->addArgument(
                'step',
                InputArgument::OPTIONAL,
                'Install process start index. 0 for requirements check, 1 for database, 2 for admin, 3 for configuration, 4 for final step. Each successful step will trigger the next until completion.',
                0
            )
//...

    /**
     * {@inheritdoc}
     */
    protected function execute(InputInterface $input, OutputInterface $output)
    {
//...

        switch ($step) {
//...

                // no break
            case InstallService::EMAIL_STEP:
                $output->writeln($step.' - Email configuration step...');
                $messages = $this->stepAction($installer, $allParams, $step);
                if (is_array($messages) && !empty($messages)) {
                    $output->writeln('Errors in email configuration:');
                    $this->handleInstallerErrors($output, $messages);

                    $output->writeln('Install canceled');

                    return -$step;
                }
                // Keep on with next step
                $step = InstallService::FINAL_STEP;

                // no break
            case InstallService::FINAL_STEP:
                $output->writeln($step.' - Final step..');
                $siteUrl  = $allParams['site_url'];
                $messages = $installer->createFinalConfigStep($siteUrl);

                if (is_bool($messages) && true === $messages) {
                    $installer->finalMigrationStep();
                } else {
                    $output->writeln('Errors in final migration:');
                    $this->handleInstallerErrors($output, $messages);

                    $output->writeln('Install canceled');

                    return -$step;
                }

        }
//...

I can submit a PR for this if you're interested.

[gramakri]

@madmath03 I am happy to test it if you can submit a PR. Thanks!

[RCheesley]

Please do submit a PR if you are able - would be great to deal with the notice rather than leave folk worrying that something isn't going to work/something broke!

[madmath03]

@RCheesley I already did the modification mentioned before but did not had the time to send PR yet.
Will do asap: #9250

[RCheesley]

@madmath03 thanks for taking a look at that and making the PR! As this is technically a bugfix we could potentially consider it for 3.1.2 if we have enough testing etc :)

[gramakri]

Thanks for the PR @madmath03 , I will test this today.

[npracht]

fixed in 3.1.2