diff --git a/openlp/core/ui/advancedtab.py b/openlp/core/ui/advancedtab.py index 4ec2bc892..f37718227 100644 --- a/openlp/core/ui/advancedtab.py +++ b/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) diff --git a/openlp/core/ui/mainwindow.py b/openlp/core/ui/mainwindow.py index 7cedf9d2a..34cb8fa49 100644 --- a/openlp/core/ui/mainwindow.py +++ b/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() diff --git a/tests/functional/openlp_core/ui/test_mainwindow.py b/tests/openlp_core/ui/test_mainwindow.py similarity index 93% rename from tests/functional/openlp_core/ui/test_mainwindow.py rename to tests/openlp_core/ui/test_mainwindow.py index 8397cac9a..9896c7e5e 100644 --- a/tests/functional/openlp_core/ui/test_mainwindow.py +++ b/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)