Fix data directory copy fail

openlp/core/ui/advancedtab.py

@@ -184,18 +184,26 @@ class AdvancedTab(SettingsTab):
         :param pathlib.Path new_path: The new path
         :rtype: None
         """
-        # Make sure they want to change the data.
-        answer = QtWidgets.QMessageBox.question(self, translate('OpenLP.AdvancedTab', 'Confirm Data Directory Change'),
-                                                translate('OpenLP.AdvancedTab', 'Are you sure you want to change the '
-                                                          'location of the OpenLP data directory to:\n\n{path}'
-                                                          '\n\nThe data directory will be changed when OpenLP is '
-                                                          'closed.').format(path=new_path),
-                                                defaultButton=QtWidgets.QMessageBox.No)
-        if answer != QtWidgets.QMessageBox.Yes:
-            self.data_directory_path_edit.path = AppLocation.get_data_path()
-            return
         # Check if data already exists here.
         self.check_data_overwrite(new_path)
+        # Make sure they want to change the data.
+        if self.data_directory_copy_check_box.isChecked():
+            warning_string = translate('OpenLP.AdvancedTab', 'Are you sure you want to change the '
+                                                             'location of the OpenLP data directory to:\n\n{path}'
+                                                             '\n\nExisting files in this directory could be '
+                                                             'overwritten. The data directory will be changed when '
+                                                             'OpenLP is closed.').format(path=new_path)
+        else:
+            warning_string = translate('OpenLP.AdvancedTab', 'Are you sure you want to change the '
+                                                             'location of the OpenLP data directory to:\n\n{path}'
+                                                             '\n\nThe data directory will be changed when OpenLP is '
+                                                             'closed.').format(path=new_path)
+        answer = QtWidgets.QMessageBox.question(self, translate('OpenLP.AdvancedTab', 'Confirm Data Directory Change'),
+                                                warning_string, defaultButton=QtWidgets.QMessageBox.No)
+        if answer != QtWidgets.QMessageBox.Yes:
+            self.data_directory_path_edit.path = AppLocation.get_data_path()
+            self.new_data_directory_has_files_label.hide()
+            return
         # Save the new location.
         self.main_window.new_data_path = new_path
         self.data_directory_cancel_button.show()
@@ -226,7 +234,7 @@ class AdvancedTab(SettingsTab):
                                                              'WARNING: \n\nThe location you have selected \n\n{path}'
                                                              '\n\nappears to contain OpenLP data files. Do you wish to '
                                                              'replace these files with the current data '
-                                                             'files?'.format(path=data_path)),
+                                                             'files?').format(path=data_path),
                                                    QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Yes |
                                                                                          QtWidgets.QMessageBox.No),
                                                    QtWidgets.QMessageBox.No)

openlp/core/ui/mainwindow.py

@@ -1359,7 +1359,7 @@ class MainWindow(QtWidgets.QMainWindow, Ui_MainWindow, LogMixin, RegistryPropert
                 self.show_status_message(
                     translate('OpenLP.MainWindow', 'Copying OpenLP data to new data directory location - {path} '
                               '- Please wait for copy to finish').format(path=self.new_data_path))
-                shutil.copytree(str(old_data_path), str(self.new_data_path))
+                shutil.copytree(str(old_data_path), str(self.new_data_path), dirs_exist_ok=True)
                 self.log_info('Copy successful')
             except (OSError, shutil.Error) as why:
                 self.application.set_normal_cursor()

tests/openlp_core/ui/test_mainwindow.py

@@ -25,6 +25,8 @@ import os
 import pytest
 from pathlib import Path
 from unittest.mock import MagicMock, patch
+from shutil import rmtree
+from tempfile import mkdtemp
 
 from PyQt5 import QtCore, QtWidgets
 
@@ -313,3 +315,29 @@ def test_application_activate_event(mocked_is_macosx, main_window):
     # THEN:
     assert result is True, "The method should have returned True."
     assert main_window.isMinimized() is False
+
+
+@patch('openlp.core.app.QtWidgets.QMessageBox.critical')
+@patch('openlp.core.common.applocation.AppLocation.get_data_path')
+@patch('openlp.core.common.applocation.AppLocation.get_directory')
+def test_change_data_directory(mocked_get_directory, mocked_get_data_path, mocked_critical_box, main_window):
+    """
+    Test that changing the data directory works if the folder already exists
+    """
+    # GIVEN: an existing old and new data directory.
+    temp_folder = Path(mkdtemp())
+    mocked_get_data_path.return_value = temp_folder
+    main_window.copy_data = True
+    temp_new_data_folder = Path(mkdtemp())
+    main_window.new_data_path = temp_new_data_folder
+
+    # WHEN: running change_data_directory
+    result = main_window.change_data_directory()
+
+    # THEN: No error shouuld have occured
+    assert result is not False
+    mocked_critical_box.assert_not_called()
+
+    # Clean up
+    rmtree(temp_folder)
+    rmtree(temp_new_data_folder)