Fix demo content installation when backup is declined

[pmoreno-rodriguez]

Change Description

This pull request addresses the issue raised by @erdnuesse in issue 18 of the Mundana Grav theme, regarding the behavior of installing the theme's demo content when the user chooses not to create a backup of the user/pages folder. Currently, if the user selects "no" when prompted to create a backup, the demo content installation is skipped entirely, which is counterintuitive since the user explicitly chose to install the demo content.

Current Behavior:

• When installing a theme with demo content, the user is prompted to create a backup of the existing user/pages folder.

• If the user selects yes, the user/pages folder is backed up and the demo content is installed.

• If the user selects "no", the installation process completely skips the installation of the demo content, leaving the existing user/pages folder intact.

Issue:

This behavior is illogical, as the user has already indicated their intention to install the demo content by continuing with the installation. Skipping the installation when the user declines to create a backup can lead to confusion and a poor user experience.

Proposed Changes:

• If the user selects "no" when prompted to create a backup:
• The existing user/pages folder is deleted (upon confirmation).
• The demo content is installed in the user/pages folder.
• This ensures that the demo content installation succeeds, even if the user chooses not to create a backup.

Testing Steps:

1. Install a new instance of Grav.

2. Install a theme with demo content (e.g., bin/gpm install mundana).

3. When prompted to create a backup of the user/pages folder, select "no".

4. Verify that:

• The existing user/pages folder has been deleted.
• The demo content has been successfully installed in the user/pages folder.

Example scenario:

• The user installs the Mundana theme and chooses to install the demo content.
• When asked "Do you want to backup your current user/pages folder?", the user responds "no".
• The existing user/pages folder has been deleted and the Mundana demo content has been installed, providing a clean and correct setup.

[pamtbaau]

IMHO, just deleting the pages folder when the users chooses not to create a backup of existing pages, as suggested in this PR, is way too destructive.

• Many users are novices to Grav.
• Many users have only 1 installation, which is both their "test" and "production" installation.
• In the PR:
  • Users are not being informed/warned that their pages will be permanently deleted, which cannot be undone.
  • There is no confirmation prompt (which should default to 'no') before deletion.
  • The PR changes the current cautious behaviour in a destructive behaviour.

The issue raised by the user is: "Skipping backup fails to install demo content".
The user is confused by the behaviour.

However, the installation does not fail, but works as designed. It implicitly mandates a backup to be made. Having said that, a proper notification might prevent the confusion raised.

Non-destructive suggestions:

• A simple improvement of the notification "Skipped", which notifies the user that the demo is not being installed.
• Or improving a prior prompt including that a backup is mandatory to install the demo pages would also do.

Just my 2 cents...

[pmoreno-rodriguez]

Well, @pamtbaau , as you know, I greatly value your input, although in this case I disagree with some aspects.

Even if a user clearly indicates they don't want to make a backup, they must do so; otherwise, the demo content won't install. So why ask them? Wouldn't it be better to do so?

A user might want to try out several themes and would have to make a backup of each one to install the demo content. Does that make sense? Well, I just don't know.

Every day we face situations in which we have to decide whether to delete something (a partition, a hard drive, a folder, etc.). The user should be free and responsible.

I agree with you that there could be a message warning the user that the demo content hasn't been installed because the backup was skipped.

What's clear, and what I think we can agree on, is that you can't skip installing the demo content without notifying users that it hasn't been installed because they haven't made a backup, even if they've previously expressed their willingness to install it. This isn't very logical either.

Finally, I hadn't considered the situation where users could have a single installation, which could be both a "test" and a "production" installation.

I'll try to improve this PR, taking into account your suggestions.

[pmoreno-rodriguez]

@pamtbaau, before modifying the PR, how about your feedback on these two alternatives?

OPTION 1. NON-AGGRESSIVE AND CONSERVATIVE

 /**
 * Prompt to install the demo content of a package
 *
 * @param Package $package The package containing demo content
 * @return void
 */
private function installDemoContent(Package $package): void
{
    $io = $this->getIO();
    $demo_dir = $this->destination . DS . $package->install_path . DS . '_demo';

    if (file_exists($demo_dir)) {
        $dest_dir = $this->destination . DS . 'user';
        $pages_dir = $dest_dir . DS . 'pages';

        // Notify about available demo content
        $io->writeln("<white>Attention: </white><cyan>{$package->name}</cyan> contains demo content");

        // First confirmation - install demo content?
        $question = new ConfirmationQuestion('Do you wish to install this demo content? [y|N] ', false);
        $answer = $io->askQuestion($question);

        if (!$answer) {
            $io->writeln("  '- <yellow>Demo content installation skipped by user!</yellow>");
            $io->newLine();
            return;
        }

        // Check if demo contains pages
        if (file_exists($demo_dir . DS . 'pages')) {
            $pages_backup = 'pages.' . date('m-d-Y-H-i-s');

            // Strong warning about requirement
            $io->writeln("\n<yellow>IMPORTANT:</yellow> Demo installation REQUIRES a backup of your current pages.");
            $io->writeln("<yellow>Without backup, demo content WILL NOT be installed.</yellow>");

            $backupQuestion = new ConfirmationQuestion(
                'Backup your current `user/pages` to `user/'.$pages_backup.'` and continue? [Y|n] ',
                true
            );

            $backupAnswer = $this->all_yes ? true : $io->askQuestion($backupQuestion);

            if (!$backupAnswer) {
                // User refused backup - abort installation
                $io->writeln("\n<red>DEMO CONTENT NOT INSTALLED!</red>");
                $io->writeln("<yellow>Backup is required to install demo pages.</yellow>");
                $io->writeln("  '- <red>Operation cancelled</red>");
                $io->newLine();
                return;
            }

            // Attempt backup
            if (file_exists($dest_dir)) {
                if (rename($pages_dir, $dest_dir . DS . $pages_backup)) {
                    $io->writeln('  |- Backing up pages...    <green>ok</green>');
                    // Create fresh pages directory for demo
                    Folder::create($pages_dir);
                } else {
                    $io->writeln('  |- Backing up pages...    <red>failed</red>');
                    $io->writeln("\n<red>DEMO CONTENT NOT INSTALLED!</red>");
                    $io->writeln("<yellow>Backup failed - cannot proceed with installation.</yellow>");
                    $io->writeln("  '- <red>Operation aborted</red>");
                    $io->newLine();
                    return;
                }
            }
        }

        // Proceed with installation after successful backup
        $io->writeln('  |- Installing demo content...');
        Folder::rcopy($demo_dir, $dest_dir);
        $io->writeln("  '- <green>Demo content successfully installed!</green>");
        $io->writeln("<yellow>Note:</yellow> Your original pages were backed up to user/{$pages_backup}");
        $io->newLine();
    }
}

OPTION 2. AGGRESSIVE, BUT WITH DOUBLE DELETE CONFIRMATION

/**
 * Prompt to install the demo content of a package
 *
 * @param Package $package The package containing demo content
 * @return void
 */
private function installDemoContent(Package $package): void
{
    $io = $this->getIO();
    $demo_dir = $this->destination . DS . $package->install_path . DS . '_demo';

    // Check if demo content exists in the package
    if (file_exists($demo_dir)) {
        $dest_dir = $this->destination . DS . 'user';
        $pages_dir = $dest_dir . DS . 'pages';

        // Notify user about available demo content
        $io->writeln("<white>Attention: </white><cyan>{$package->name}</cyan> contains demo content");

        // Ask if user wants to install demo content
        $question = new ConfirmationQuestion('Do you wish to install this demo content? [y|N] ', false);
        $answer = $io->askQuestion($question);

        if (!$answer) {
            $io->writeln("  '- <yellow>Demo content installation skipped!</yellow>");
            $io->newLine();
            return;
        }

        // Check if demo contains pages folder
        if (file_exists($demo_dir . DS . 'pages')) {
            $pages_backup = 'pages.' . date('m-d-Y-H-i-s');

            // Warn about page modifications and ask about backup
            $io->writeln("<yellow>WARNING:</yellow> Installing demo content will modify your pages.");
            $backupQuestion = new ConfirmationQuestion(
                'Do you want to backup your current `user/pages` to `user/'.$pages_backup.'`? [Y|n] ',
                true
            );

            $backupAnswer = $this->all_yes ? true : $io->askQuestion($backupQuestion);

            if ($backupAnswer) {
                // If backup is requested
                if (file_exists($dest_dir)) {
                    if (rename($pages_dir, $dest_dir . DS . $pages_backup)) {
                        $io->writeln('  |- Backing up pages...    <green>ok</green>');
                        // Create fresh pages directory for demo
                        Folder::create($pages_dir);
                    } else {
                        $io->writeln('  |- Backing up pages...    <red>failed</red>');
                        $io->writeln("  '- <red>Installation aborted due to backup failure!</red>");
                        $io->newLine();
                        return;
                    }
                }
            } else {
                // If no backup requested - show strong warning
                $io->writeln("\n<red>WARNING:</red> You have chosen NOT to create a backup.");
                $io->writeln("<red>ALL existing pages will be PERMANENTLY DELETED and replaced with demo content!</red>");

                // Require explicit confirmation for destructive action
                $confirmQuestion = new ConfirmationQuestion(
                    'Are you ABSOLUTELY sure you want to DELETE ALL PAGES and install the demo? [y|N] ',
                    false
                );

                $confirmAnswer = $this->all_yes ? false : $io->askQuestion($confirmQuestion);

                if (!$confirmAnswer) {
                    $io->writeln("  '- <yellow>Installation cancelled by user</yellow>");
                    $io->newLine();
                    return;
                }

                // User confirmed - proceed with complete deletion
                if (file_exists($pages_dir)) {
                    Folder::delete($pages_dir); // Completely wipe pages directory
                    Folder::create($pages_dir); // Create fresh empty directory
                    $io->writeln('  |- Deleting all pages...    <green>done</green>');
                }
            }
        }

        // Proceed with demo content installation
        $io->writeln('  |- Installing demo content...');
        Folder::rcopy($demo_dir, $dest_dir); // Recursively copy demo content
        $io->writeln("  '- <green>Demo content successfully installed!</green>");
        $io->newLine();
    }
}

[erdnuesse]

To me, the demo content is most useful for anything I try out, especially on fresh installs, as I specifically had created a dev instance to try out multiple themes. (many subdomains, many folders in my webspace)

First up:
Creating backups is 100% useful for any case where content is present, I agree wholeheartedly.
However, when there has been no content added, there's no backup to be made. Why give an option, if it's mandatory based on a step, that comes after I was asked to create a backup in the first place?

In that specific case, I would have expected to be informed, that without backup the demo content wouldn't work - 1 step prior.

However it's going to be resolved, at least the information "why" would at least clear the misconceptions of an unsuspecting user.

The options how I see them, from a UX perspective:

1. Information
   a) Backup selection: "Be advised, that without backup, it will not be possible to install the demo content in the following step"
   b) Demo Content: "Be advised, that the demo content will not be installed, if you have selected no for the backup previously."

2. Safe overwrite
   a) When demo content is chosen, create the backup regardless of the previous backup selection.
   b) Notify that a backup has been made, because of the selection, and how the rollback can be performed.

3. Hard overwrite
   a) Add a warning to the Demo content install, when no backup has been chosen: "Caution, your content will be overwritten, to install the demo content. You have been warned."

The reason is the following:
When you ask for 2 choices with yes/no, it results in 4 cases.

Backup, and demo content
No backup and demo content
Backup, no demo content
No backup, no demo content

If any one thing depends on the other - it's a decision tree, rather than two separate choices. (Backup yes -> Demo content yes/no | No backup)

The question to solve is not only how it should be made clear to the user, but how it's going to be handled; if the design decision is a decision tree, user interface / information needs fixing, and if those decisions are separate from each other, each of the 4 cases should be handled differently and accordingly - as a separate case.
Those two questions (3 vs 4 cases, and how to inform) can't be separated from each other.

Lastly; As I have no stakes in the matter, To me it was a bug, because I made a choice and it didn't do what I expected it to.

• if it works as designed, it's designed badly (see 3 vs 4 cases).
• if it's too destructive, just override the users choice (say backup will be created regardless of the decision prior) - If you want to hold the user's hand, at least make it so, that you give him what he wants.
• overwriting without prior notice IS bad, so is doing something else, than what the user specified.
• Anyhow, In the spirit of open source, giving the user freedom to ~# sudo rm -rf / their machine should be possible, just tell them with great power comes great responsibility. A big warning sign and all options should be available.

But that's just my 2ct. Thanks for considering the issue. (And sorry that it's become an essay at this point.)

[pmoreno-rodriguez]

Well, in the end, I chose the first, less aggressive option, requiring the user to create a backup before installing the demo content and improving the warning messages for the various options.

Thanks @pamtbaau and @erdnuesse for your suggestions.

@rhukster, I hope Grav's team takes these changes into account

[Karmalakas]

IMHO, if there's something about to be deleted, there must be a confirmation with a clear message. No matter the case

[hughbris]

Irrelevant late comment but I have to disagree with one of the early foundations from @pamtbaau:

• Many users have only 1 installation, which is both their "test" and "production" installation.

I have almost no sympathy for developers that do not do anything to protect their work, whether backups, version control, or setting up safe environments. Grav supports many layers, but the latter is universal across coding practice. I don't think it should be a priority to support that kind of carelessness. The Discourse forum has many posts of users who didn't (and often won't even consider) set up a safe development/sandbox environment. I know it sounds like gatekeeping, but if you are that kind of developer and stubbornly stick to your unsafe workflows and configurations, then Grav is not going to work out for you.

That said, I agree there is a problem here with surprising CLI behaviour that can be addressed.