Merge pull request #348 from ucam-department-of-psychiatry/report-fai…

…led-database-deletion

Better handling of database deletion when user forgets password

.github/workflows/build-and-install-packages.yml

@@ -21,8 +21,8 @@ jobs:
                 os: [ubuntu-20.04, ubuntu-22.04]
         runs-on: ${{ matrix.os }}
         steps:
-            - uses: actions/checkout@v3
-            - uses: actions/setup-python@v4
+            - uses: actions/checkout@v4
+            - uses: actions/setup-python@v5
               with:
                   python-version: ${{ matrix.python-version }}
             - name: Change apt mirror

.github/workflows/build-qt.yml

@@ -24,15 +24,15 @@ jobs:
                       os: windows-2019
         runs-on: ${{ matrix.os }}
         steps:
-            - uses: actions/checkout@v3
+            - uses: actions/checkout@v4
             - name: Free up disk space
               # The runner on Ubuntu will run out of space so we remove some of the things
               # we don't need.
               if: runner.os == 'Linux'
               run: |
                   set -eux -o pipefail
                   ${GITHUB_WORKSPACE}/.github/scripts/free_up_disk_space.sh
-            - uses: actions/setup-python@v4
+            - uses: actions/setup-python@v5
               with:
                   python-version: 3.8
             - name: Common Linux prerequisites

.github/workflows/cpp-tests.yml

@@ -20,7 +20,7 @@ jobs:
     cpp-tests:
         runs-on: ubuntu-22.04
         steps:
-            - uses: actions/checkout@v3
+            - uses: actions/checkout@v4
             - name: Qt version check
               run: |
                   set -euxo pipefail

.github/workflows/database-upgrade-downgrade.yml

@@ -14,8 +14,8 @@ jobs:
     database-upgrade-downgrade:
         runs-on: ubuntu-20.04
         steps:
-            - uses: actions/checkout@v3
-            - uses: actions/setup-python@v4
+            - uses: actions/checkout@v4
+            - uses: actions/setup-python@v5
               with:
                   python-version: 3.8
             - name: Set up MySQL

.github/workflows/docker.yml

@@ -15,7 +15,7 @@ jobs:
     docker:
         runs-on: ubuntu-latest
         steps:
-            - uses: actions/checkout@v3
+            - uses: actions/checkout@v4
 
             - name: Build
               run: |

.github/workflows/docs.yml

@@ -22,8 +22,8 @@ jobs:
                       python-version: "3.10"
         runs-on: ${{ matrix.os }}
         steps:
-            - uses: actions/checkout@v3
-            - uses: actions/setup-python@v4
+            - uses: actions/checkout@v4
+            - uses: actions/setup-python@v5
               with:
                   python-version: ${{ matrix.python-version }}
             - name: Change apt mirror

.github/workflows/installer.yml

@@ -19,7 +19,7 @@ jobs:
     installer:
         runs-on: ubuntu-latest
         steps:
-            - uses: actions/checkout@v3
+            - uses: actions/checkout@v4
             - name: Run installer
               run: |
                 set -eux -o pipefail

.github/workflows/launch-cherrypy.yml

@@ -19,8 +19,8 @@ jobs:
                 os: [ubuntu-22.04]
         runs-on: ${{ matrix.os }}
         steps:
-            - uses: actions/checkout@v3
-            - uses: actions/setup-python@v4
+            - uses: actions/checkout@v4
+            - uses: actions/setup-python@v5
               with:
                   python-version: ${{ matrix.python-version }}
             - name: Set up MySQL

.github/workflows/missing-migration-check.yml

@@ -14,8 +14,8 @@ jobs:
       migration-check:
           runs-on: ubuntu-20.04
           steps:
-            - uses: actions/checkout@v3
-            - uses: actions/setup-python@v4
+            - uses: actions/checkout@v4
+            - uses: actions/setup-python@v5
               with:
                   python-version: 3.8
             - name: Missing migration check

.github/workflows/precommit.yml

@@ -12,8 +12,8 @@ jobs:
     precommit-checks:
         runs-on: ubuntu-20.04
         steps:
-            - uses: actions/checkout@v3
-            - uses: actions/setup-python@v4
+            - uses: actions/checkout@v4
+            - uses: actions/setup-python@v5
               with:
                   python-version: 3.8
             - name: Pre-commit checks

.github/workflows/python-package-checks.yml

@@ -17,8 +17,8 @@ jobs:
                 os: [ubuntu-20.04, ubuntu-22.04]
         runs-on: ${{ matrix.os }}
         steps:
-            - uses: actions/checkout@v3
-            - uses: actions/setup-python@v4
+            - uses: actions/checkout@v4
+            - uses: actions/setup-python@v5
               with:
                   python-version: ${{ matrix.python-version }}
             - name: Python package checks

.github/workflows/python-tests.yml

@@ -19,8 +19,8 @@ jobs:
                 os: [ubuntu-20.04, ubuntu-22.04]
         runs-on: ${{ matrix.os }}
         steps:
-            - uses: actions/checkout@v3
-            - uses: actions/setup-python@v4
+            - uses: actions/checkout@v4
+            - uses: actions/setup-python@v5
               with:
                   python-version: ${{ matrix.python-version }}
             - name: Change apt mirror

.github/workflows/release.yml

@@ -14,7 +14,7 @@ jobs:
             contents: write
         steps:
             - name: Check out repository
-              uses: actions/checkout@v3
+              uses: actions/checkout@v4
             - name: Work out tag
               id: vars
               run: echo ::set-output name=tag::${GITHUB_REF#refs/*/}

.github/workflows/report-missing-client-translations.yml

@@ -20,7 +20,7 @@ jobs:
     report-missing-client-translations:
         runs-on: ubuntu-22.04
         steps:
-            - uses: actions/checkout@v3
+            - uses: actions/checkout@v4
             - name: Qt version check
               run: |
                   set -euxo pipefail
@@ -34,7 +34,7 @@ jobs:
                   target: desktop
                   arch: gcc_64
                   archives: "qtbase qtdeclarative qttools qttranslations icu"
-            - uses: actions/setup-python@v4
+            - uses: actions/setup-python@v5
               with:
                   python-version: "3.10"
             - name: Report missing client translations

docs/source/changelog.rst

@@ -3895,3 +3895,8 @@ Current C++/SQLite client, Python/SQLAlchemy server
   the previous registration, if available. This should reduce the amount of data
   entry needed following a network or registration failure.
   https://github.com/ucam-department-of-psychiatry/camcops/issues/104
+
+- Provide more information if the app cannot delete the SQLite databases when a user
+  has forgotten their password. Fix a bug where if the initial password dialog was
+  aborted, the next attempt to set up a password would fail.
+  https://github.com/ucam-department-of-psychiatry/camcops/issues/346

server/setup.py

@@ -86,8 +86,8 @@
     "Faker==4.1.1",  # create fake data; for test and dummy database creation
     # FHIR export, our fork until https://github.com/smart-on-fhir/client-py/pull/105 is merged  # noqa
     "fhirclient @ git+https://github.com/ucam-department-of-psychiatry/client-py@128bbe3c2194a51ba6ff8cf880ef2fdb9bfcc2d6#egg=fhirclient-4.0.0.1",  # noqa: E501
-    "flower==1.2.0",  # monitor for Celery
-    "gunicorn==20.1.0",  # web server (Unix only)
+    "flower==2.0.1",  # monitor for Celery
+    "gunicorn==21.2.0",  # web server (Unix only)
     "hl7==0.3.5",  # For HL7 export
     # Celery dependency for Python <= 3.7; workaround import error
     # https://github.com/celery/celery/issues/7783; scheduled to be fixed in

tablet_qt/core/camcopsapp.cpp

@@ -1279,7 +1279,15 @@ bool CamcopsApp::connectDatabaseEncryption(QString& new_user_password,
                         new_pw_text, new_pw_title,
                         false /* require_old_password */,
                         dummy_old_password, new_user_password, nullptr)) {
+
+                // The user quit without setting a password.
+                // If we don't delete the database here, the next attempt to
+                // set up a password will fail (canReadDatabase() calls below
+                // will return false) and the user will be forced to set up
+                // another one.
+                deleteDatabases();
                 user_cancelled_please_quit = true;
+
                 return false;
             }
             qInfo() << "Encrypting databases for the first time...";
@@ -1336,7 +1344,12 @@ bool CamcopsApp::connectDatabaseEncryption(QString& new_user_password,
                 if (!userConfirmedRetryPassword()) {
                     if (userConfirmedDeleteDatabases()) {
                         qInfo() << "... deleting databases.";
-                        deleteDatabases();
+                        const bool ok = deleteDatabases();
+                        if (!ok)
+                        {
+                            user_cancelled_please_quit = true;
+                            return false;
+                        }
                         qInfo() << "... recreating databases.";
                         openOrCreateDatabases();
                     }
@@ -1390,16 +1403,55 @@ bool CamcopsApp::userConfirmedDeleteDatabases() const
 }
 
 
-void CamcopsApp::deleteDatabases()
+bool CamcopsApp::deleteDatabases()
 {
-    const QString data_filename = dbFullPath(dbfunc::DATA_DATABASE_FILENAME);
-    const QString sys_filename = dbFullPath(dbfunc::SYSTEM_DATABASE_FILENAME);
+    QString data_error_string;
+    QString sys_error_string;
+
+    const bool data_ok = deleteDatabase(dbfunc::DATA_DATABASE_FILENAME,
+                                        data_error_string);
+    const bool sys_ok = deleteDatabase(dbfunc::SYSTEM_DATABASE_FILENAME,
+                                       sys_error_string);
+
+    if (data_ok && sys_ok) {
+        return true;
+    }
+
+    QString error_string;
+
+    if (!data_ok) {
+        error_string = data_error_string;
+    }
 
-    QFile data_file(data_filename);
-    data_file.remove();
+    if (!sys_ok) {
+        error_string += "\n" + sys_error_string;
+    }
+    uifunc::alert(
+        tr("CamCOPS could not delete its databases:\n\n"
+           "%1\n"
+           "Please try to delete these files manually and restart CamCOPS\n"
+    ).arg(error_string));
+
+    return false;
+}
+
+
+bool CamcopsApp::deleteDatabase(const QString& filename, QString& error_string)
+{
+    const QString fullpath = dbFullPath(filename);
+    QFile file(fullpath);
+    const bool ok = file.remove();
+
+    if (!ok) {
+        error_string = tr(
+            "Failed to delete file:\n"
+            "%1\n"
+            "because of this error:\n"
+            "%2\n"
+        ).arg(fullpath, file.errorString());
+    }
 
-    QFile sys_file(sys_filename);
-    sys_file.remove();
+    return ok;
 }
 
 

tablet_qt/core/camcopsapp.h

@@ -169,8 +169,11 @@ class CamcopsApp : public QApplication
     // Open our pair of databases, or create them if they don't exist.
     void openOrCreateDatabases();
 
-    // Delete databases
-    void deleteDatabases();
+    // Delete databases, returning true if successful
+    bool deleteDatabases();
+
+    // Delete the named database, returning true if successful
+    bool deleteDatabase(const QString& filename, QString& error_string);
 
     // Close our databases.
     void closeDatabases();