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

"snipeit:restore" wipes the database and migrates #15363

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

uberbrady
Copy link
Collaborator

This is a follow-up on #15296 - this embeds the db:wipe, and the migrate command into the CLI, and removes them from the Settings controller. It also exposes the option in the web GUI so you can disable the db:wipe before the restore, if for some reason you want that.

I tried to make this PR against the previous PR, but it wouldn't let me do that without putting the PR in my own repo, which I did not want.

So, instead, it just embeds the contents of the previous PR in it. Which is weird, but, well, that's what it does.

This is mostly untested, but I'm going to be doing said testing shortly. Once I've tested all of the various modes, I'll pull it out of 'Draft'.

Copy link

what-the-diff bot commented Aug 21, 2024

PR Summary

  • Addition of New Environment Variable
    An extra variable, DB_SANITIZE_BY_DEFAULT, has been introduced to the environment setup. Its default value is false.

  • Improvements to the 'Restore From Backup' process
    The 'Restore From Backup' command in the application's Console Commands is beefed up by:

    • Adding a new option --do-not-wipe that gives more flexibility.
    • Taking care of running database migrations automatically.
  • Updates in Settings Controller
    We've updated the method signature of the postRestore to now include a Request parameter. This allows better handling of the --do-not-wipe option and also takes care of database migrations.

  • Updated configuration
    A new configuration property, sanitize_by_default, is introduced in the backup.php config file. Here, its default value is false.

  • Code Refactoring
    We've improved the code in snipeit.js under resources by cleaning out unused variables and functions.

  • Integration of New Translations
    New translations are introduced in the general.php file under the settings directory.

  • Backups Settings UI Enhancement
    The backups settings page now has a fresh look with a new modal for confirming backup restoration. This modal offers options to clean the backed-up database and perform a "wipe" before restoring. As a part of this change, the previous JavaScript event handling code is replaced by the new structure.

@uberbrady
Copy link
Collaborator Author

This seems to mostly work, but I'm having a little trouble when I try to restore without doing the cleaning, but with doing the db:wipe. The results are weird - I am restoring a file that has prefixes, so I would expect to see just my prefixed tables in the DB. I'm getting those prefixed tables, but also I'm getting the 'correct' unprefixed tables. But those tables are mostly empty - except for users - just one user record is in there. I think that one user is the restored one as part of the web GUI restore process.

What's weird is I am seeing the debugging messages output saying that it's doing the wipe. Maybe it's using some other record of what the schema is? I'm not sure.

I'm going to keep messing with it until I can get an explanation, or maybe a fix. Maybe I'll try a die() or a dd() right after the wipe to make sure there are no tables at that point...

@uberbrady
Copy link
Collaborator Author

I blanked a database and created a fake table, then did a restore via CLI with the --do-not-wipe option, and the fake table remained. So the CLI-part is working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant