Corrected FirstRunDialog accept/reject, added db backup dir

main.cpp

@@ -48,7 +48,6 @@ int main(int argc, char *argv[])
     }
 
     ASettings::setup();
-    //Debug firstrundialog
     ASettings::write(ASettings::Setup::SetupComplete, false);
 
     aDB()->connect();
@@ -59,7 +58,7 @@ int main(int argc, char *argv[])
             QProcess::startDetached(qApp->arguments()[0], qApp->arguments());
         } else {
             DEB "First run not accepted. Exiting.";
-            return 0;
+            return 1;
         }
     }
 
@@ -89,7 +88,7 @@ int main(int argc, char *argv[])
     ARunGuard guard(QStringLiteral("opl_single_key"));
         if ( !guard.tryToRun() ){
             DEB << "Another Instance is already running. Exiting.";
-            return 0;
+            return 2;
         }
 
     MainWindow w;

src/classes/astandardpaths.cpp

@@ -1,4 +1,4 @@
-#include "src/astandardpaths.h"
+#include "src/classes/astandardpaths.h"
 
 //QMap<QStandardPaths::StandardLocation, QString> AStandardPaths::paths;
 QMap<AStandardPaths::Dirs, QString> AStandardPaths::paths;
@@ -7,27 +7,28 @@ void AStandardPaths::setup()
 {
     auto settings_location = QStandardPaths::AppConfigLocation;
     auto data_location = QStandardPaths::AppDataLocation;
-    paths = {
+    paths = {  // [G]: potential rename to `dirs`
         {Database, QStandardPaths::writableLocation(data_location)},
         {Templates, QDir(QStandardPaths::writableLocation(data_location)).filePath("templates")},
         {Settings, QStandardPaths::writableLocation(settings_location)},
+        {DatabaseBackup, QDir(QStandardPaths::writableLocation(data_location)).filePath("backup")}
     };
     DEB << "Paths created: " << paths;
 }
 
-QString AStandardPaths::getPath(Dirs loc)
+QString AStandardPaths::pathTo(Dirs loc)
 {
     return paths[loc];
 }
 
-QMap<AStandardPaths::Dirs, QString> AStandardPaths::getPaths()
+QMap<AStandardPaths::Dirs, QString> AStandardPaths::allPaths()
 {
     return paths;
 }
 
 void AStandardPaths::scan_paths()
 {
-    for(auto& path : paths.values()){
+    for(auto& path : paths){
         auto dir = QDir(path);
         DEB << "Scanning " << dir.path();
         if(!dir.exists()) {
@@ -39,7 +40,7 @@ void AStandardPaths::scan_paths()
 
 bool AStandardPaths::validate_paths()
 {
-    for(auto& path : paths.values()){
+    for(auto& path : paths){
         DEB << "Validating " << path;
         if(false)  // determine path as valid (scan contents and parse for correctness)
             return false;

src/classes/astandardpaths.h

@@ -18,6 +18,7 @@ public:
         Database,
         Templates,
         Settings,
+        DatabaseBackup
     };
 private:
     static
@@ -26,8 +27,12 @@ public:
     /// Initialise paths with corresponding StandardLocation paths
     static void setup();
 
-    static QString getPath(Dirs loc);
-    static QMap<Dirs, QString> getPaths();
+    // [G]: Subjective opinion:
+    // We should move away from getThis getThat functions.
+    // I believe we can give better namings while avoiding this
+    // OOP cliche of getEverything
+    static QString pathTo(Dirs loc);
+    static QMap<Dirs, QString> allPaths();
 
     /// Ensure standard app paths exist, if not mkdir them.
     static void scan_paths();

src/database/adatabase.cpp

@@ -51,8 +51,8 @@ ADatabase* ADatabase::getInstance()
 }
 
 ADatabase::ADatabase()
-    : databaseDir(QDir(AStandardPaths::getPath(AStandardPaths::Database))),
-      databasePath(databaseDir.filePath(QStringLiteral("logbook.db")))
+    : databaseDir(QDir(AStandardPaths::pathTo(AStandardPaths::Database))),
+      databaseFile(QFileInfo(databaseDir.filePath(QStringLiteral("logbook.db"))))
 {}
 
 /*!
@@ -74,7 +74,7 @@ bool ADatabase::connect()
         return false;
 
     QSqlDatabase db = QSqlDatabase::addDatabase(SQL_DRIVER);
-    db.setDatabaseName(databasePath);
+    db.setDatabaseName(databaseFile.completeBaseName());
 
     if (!db.open())
         return false;

src/database/adatabase.h

@@ -92,7 +92,7 @@ public:
 
     ADatabaseError lastError;
     const QDir databaseDir;
-    const QString databasePath;
+    const QFileInfo databaseFile;
 
     /*!
      * \brief Connect to the database and populate database information.

src/database/adatabasesetup.cpp

@@ -281,7 +281,7 @@ bool ADataBaseSetup::createDatabase()
 
 bool ADataBaseSetup::downloadTemplates()
 {
-    QDir template_dir(AStandardPaths::getPath(AStandardPaths::Templates));
+    QDir template_dir(AStandardPaths::pathTo(AStandardPaths::Templates));
     DEB << template_dir;
     for (const auto& table : templateTables) {
         QEventLoop loop;
@@ -298,14 +298,16 @@ bool ADataBaseSetup::downloadTemplates()
 
 bool ADataBaseSetup::backupOldData()
 {
-    // back up old database
-    auto oldDatabase = QFile(aDB()->databasePath);
-    if (oldDatabase.exists()) {
-        auto dateString = QDateTime::currentDateTime().toString(Qt::ISODate);
-        DEB << "Backing up old database as: " << "logbook-backup-" + dateString;
-        if (!oldDatabase.rename("data/logbook-backup-" + dateString)) {
-            DEB << "Warning: Creating backup of old database has failed.";
+    auto database_file = aDB()->databaseFile;
+    if(database_file.exists()){
+        auto date_string = QDateTime::currentDateTime().toString(Qt::ISODate);
+        auto backup_dir = QDir(AStandardPaths::pathTo(AStandardPaths::DatabaseBackup));
+        auto backup_name = database_file.baseName() + "-backup-" + date_string + ".bak";
+        if(!backup_dir.mkpath(backup_name)){
+            DEB << "Could not create file " << backup_name << " at: " << backup_dir.path();
+            return false;
         }
+        DEB << "Backing up old database as: " << backup_name;
     }
     return true;
 }
@@ -321,7 +323,7 @@ bool ADataBaseSetup::importDefaultData()
             DEB << "Error: " << query.lastError().text();
         }
         //fill with data from csv
-        if (!commitData(aReadCsv(AStandardPaths::getPath(AStandardPaths::Templates)
+        if (!commitData(aReadCsv(AStandardPaths::pathTo(AStandardPaths::Templates)
                                  % QLatin1Char('/')
                                  % table % QStringLiteral(".csv")),
                         table)) {

src/gui/dialogues/firstrundialog.cpp

@@ -59,9 +59,9 @@ void FirstRunDialog::on_nextPushButton_clicked()
         if(ui->firstnameLineEdit->text().isEmpty()
            || ui->lastnameLineEdit->text().isEmpty())
         {
-            QMessageBox(QMessageBox::Warning,QStringLiteral("Error"),
-                             QStringLiteral("Please enter first and last name")
-                             ).exec();
+            QMessageBox(QMessageBox::Warning, QStringLiteral("Error"),
+                        QStringLiteral("Please enter first and last name")
+                        ).exec();
             return;
         }
         ui->previousPushButton->setEnabled(true);
@@ -70,7 +70,10 @@ void FirstRunDialog::on_nextPushButton_clicked()
         ui->nextPushButton->setText(QStringLiteral("Done"));
         break;
     case 2:
-        finish();
+        if(!finish())
+            QDialog::reject();
+        else
+            QDialog::accept();
         return;
     }
     ui->stackedWidget->setCurrentIndex(current_idx + 1);
@@ -81,7 +84,7 @@ void FirstRunDialog::on_themeGroup_toggled(int id)
     ASettings::write(ASettings::Main::Theme, id);
 }
 
-void FirstRunDialog::finish()
+bool FirstRunDialog::finish()
 {
     ASettings::write(ASettings::UserData::LastName, ui->lastnameLineEdit->text());
     ASettings::write(ASettings::UserData::FirstName, ui->firstnameLineEdit->text());
@@ -115,21 +118,19 @@ void FirstRunDialog::finish()
     // why do you write setup complete twice?
     if (!setupDatabase()) {
         db_fail_msg_box.exec();
+        return false;
     }
-    ASettings::write(ASettings::Setup::SetupComplete, true);
-
     aDB()->disconnect(); // reset db connection to refresh layout after initial setup.
     aDB()->connect();
 
     auto pilot = APilotEntry(1);
     pilot.setData(data);
-    if (aDB()->commit(pilot)) {
-        ASettings::write(ASettings::Setup::SetupComplete, true);
-        QDialog::accept();
-    } else {
+    if(!aDB()->commit(pilot)){
         db_fail_msg_box.exec();
-        QDialog::reject();
+        return false;
     }
+    ASettings::write(ASettings::Setup::SetupComplete, true);
+    return true;
 }
 
 bool FirstRunDialog::setupDatabase()

src/gui/dialogues/firstrundialog.h

@@ -34,7 +34,7 @@ private:
 
     void reject() override;
     bool setupDatabase();
-    void finish();
+    bool finish();
 
 };
 

src/gui/widgets/debugwidget.cpp

@@ -169,7 +169,7 @@ void DebugWidget::on_importCsvPushButton_clicked()
 
 void DebugWidget::on_debugPushButton_clicked()
 {
-    DEB << AStandardPaths::getPaths()[AStandardPaths::Database	];
+    DEB << AStandardPaths::allPaths()[AStandardPaths::Database	];
 }
 
 /* //Comparing two functions template