Fixes save crash issue (#169)

Author: StephenNneji

installer/linux/build_installer.sh

@@ -43,7 +43,7 @@ if [ -n "$HELP" ]; then
   echo "-l <dir>, --local <dir>	     Clone SScanSS 2 from local directory (requires git)"
   echo "-d <dir>, --build-dir <dir>  Specify build directory (temp directory will be used if not provided)"
   echo "-t <arg>, --tag <arg>	     Clone specific tag of SScanSS 2 from local (requires git) or web"
-  echo "-r, --remote		     Clone SScanSS 2 from Github repo"
+  echo "-r, --remote		     Clone SScanSS 2 from GitHub repo"
   exit 0
 fi
 

sscanss/app/window/presenter.py

@@ -33,7 +33,7 @@ def __init__(self, view):
             raise FileNotFoundError("No instrument description file was found.")
 
         self.worker = None
-
+        self.can_discard = False  # Need a better solution
         self.recent_list_size = 10  # Maximum size of the recent project list
 
     def notifyError(self, message, exception):
@@ -119,34 +119,42 @@ def projectCreationError(self, exception, args):
 
         self.notifyError(msg, exception)
 
-    def saveProject(self, save_as=False):
+    def saveProject(self, save_as=False, callback=None):
         """Saves a project to a file. A file dialog should be opened for the first save
         after which the function will save to the same location. if save_as is True a dialog is
         opened every time
 
         :param save_as: indicates if file dialog should be used
         :type save_as: bool
+        :param callback: callback to call after success save
+        :type callback: Optional[Callable[None, None]]
         """
         # Avoids saving when there are no changes
         if self.view.undo_stack.isClean() and self.model.save_path and not save_as:
             return
-
         filename = self.model.save_path
         if save_as or not filename:
             filename = self.view.showSaveDialog('hdf5 File (*.h5)', title='Save Project')
             if not filename:
                 return
-
         error_msg = f'An error occurred while attempting to save this project ({filename}).'
+
+        def on_success():
+            self._saveProjectSuccess()
+            if callback is not None:
+                callback()
+
         self.useWorker(self._saveProjectHelper, [filename],
                        message='Saving Project to File',
                        on_failure=lambda e: self.notifyError(error_msg, e),
-                       on_complete=self.view.progress_dialog.close)
+                       on_success=on_success)
 
     def _saveProjectHelper(self, filename):
         self.model.saveProjectData(filename)
         self.updateRecentProjects(filename)
         self.model.save_path = filename
+
+    def _saveProjectSuccess(self):
         self.view.showProjectName()
         self.view.undo_stack.setClean()
 
@@ -157,9 +165,6 @@ def openProject(self, filename=''):
         :param filename: full path of file
         :type filename: str
         """
-        if not self.confirmSave():
-            return
-
         if not filename:
             filename = self.view.showOpenDialog('hdf5 File (*.h5)',
                                                 title='Open Project',
@@ -206,29 +211,26 @@ def projectOpenError(self, exception, args):
 
         self.notifyError(msg, exception)
 
-    def confirmSave(self):
+    def confirmSave(self, callback):
         """Checks if the project is saved and asks the user to save if necessary
 
-        :return: indicates if the project is saved or user chose to discard changes
-        :rtype: bool
+        :param callback: callback to call if the project is saved or discarded
+        :type callback: Callable[None, None]
         """
         if self.model.project_data is None or self.view.undo_stack.isClean():
-            return True
+            callback()
+            return
 
+        self.can_discard = False
         reply = self.view.showSaveDiscardMessage(self.model.project_data['name'])
-
         if reply == MessageReplyType.Save:
             if self.model.save_path:
-                self.saveProject()
-                return True
+                self.saveProject(callback=callback)
             else:
-                self.saveProject(save_as=True)
-                return self.view.undo_stack.isClean()
-
+                self.saveProject(save_as=True, callback=callback)
         elif reply == MessageReplyType.Discard:
-            return True
-        else:
-            return False
+            self.can_discard = True
+            callback()
 
     def updateRecentProjects(self, filename):
         """Adds a filename entry to the front of the recent projects list
@@ -767,7 +769,7 @@ def alignSample(self, matrix):
 
     def alignSampleWithPose(self, pose):
         """Aligns the sample on instrument using specified 6D pose. Pose contains 3D translation
-        (X, Y, Z) and 3D orientation (XYZ euler angles)
+        (X, Y, Z) and 3D orientation (ZYX euler angles)
 
         :param pose: position and orientation
         :type pose: List[float]

sscanss/app/window/view.py

@@ -20,6 +20,23 @@
 MAIN_WINDOW_TITLE = 'SScanSS 2'
 
 
+class RecentFileCallback:
+    """A callback to open a project with the given path
+
+    :param view: instance of main window
+    :type view: MainWindow
+    :param filename: path of the file
+    :type filename: str
+    """
+    def __init__(self, view, filename):
+
+        self.view = view
+        self.filename = filename
+
+    def __call__(self):
+        return self.view.presenter.openProject(self.filename)
+
+
 class MainWindow(QtWidgets.QMainWindow):
     """Creates the main view for the sscanss app"""
     def __init__(self):
@@ -67,13 +84,13 @@ def createActions(self):
         self.new_project_action.setStatusTip('Create a new project')
         self.new_project_action.setIcon(QtGui.QIcon(IconEngine('file.png')))
         self.new_project_action.setShortcut(QtGui.QKeySequence.StandardKey.New)
-        self.new_project_action.triggered.connect(self.showNewProjectDialog)
+        self.new_project_action.triggered.connect(lambda: self.presenter.confirmSave(self.showNewProjectDialog))
 
         self.open_project_action = QtGui.QAction('&Open Project', self)
         self.open_project_action.setStatusTip('Open an existing project')
         self.open_project_action.setIcon(QtGui.QIcon(IconEngine('folder-open.png')))
         self.open_project_action.setShortcut(QtGui.QKeySequence.StandardKey.Open)
-        self.open_project_action.triggered.connect(lambda: self.presenter.openProject())
+        self.open_project_action.triggered.connect(lambda: self.presenter.confirmSave(self.presenter.openProject()))
 
         self.save_project_action = QtGui.QAction('&Save Project', self)
         self.save_project_action.setStatusTip('Save project')
@@ -717,14 +734,22 @@ def populateRecentMenu(self):
         if self.recent_projects:
             for project in self.recent_projects:
                 recent_project_action = QtGui.QAction(project, self)
-                recent_project_action.triggered.connect(lambda ignore, p=project: self.presenter.openProject(p))
+                callback = RecentFileCallback(self, project)
+                recent_project_action.triggered.connect(lambda ignore, c=callback: self.presenter.confirmSave(c))
                 self.recent_menu.addAction(recent_project_action)
         else:
             recent_project_action = QtGui.QAction('None', self)
             self.recent_menu.addAction(recent_project_action)
 
+    def canClose(self):
+        if self.undo_stack.isClean() or self.presenter.can_discard:
+            return True
+        else:
+            self.presenter.confirmSave(self.close)
+            return False
+
     def closeEvent(self, event):
-        if self.presenter.confirmSave():
+        if self.canClose():
             settings.system.setValue(settings.Key.Geometry.value, self.saveGeometry())
             if self.recent_projects:
                 settings.system.setValue(settings.Key.Recent_Projects.value, self.recent_projects)
@@ -836,9 +861,6 @@ def __createChangeCollimatorAction(self, name, active, detector):
 
     def showNewProjectDialog(self):
         """Opens the new project dialog"""
-        if not self.presenter.confirmSave():
-            return
-
         self.createNonModalDialog(ProjectDialog)
         self.non_modal_dialog.updateRecentProjects(self.recent_projects)
         self.non_modal_dialog.show()
@@ -1157,7 +1179,7 @@ def __init__(self, parent):
         self.worker.job_failed.connect(self.onFailure)
 
     def check(self, startup=False):
-        """Asynchronously checks for new release using the Github release API and notifies the user when
+        """Asynchronously checks for new release using the GitHub release API and notifies the user when
         update is found, not found or an error occurred. When startup is true, the user will
         only be notified if update is found.
 

tests/helpers.py

@@ -170,7 +170,7 @@ def click_check_box(check_box):
     :param check_box: widget to click
     :type check_box: QtWidgets.QCheckBox
     """
-    pos = QPoint(2, check_box.height() // 2)
+    pos = QPoint(10, check_box.height() // 2)
     QTest.mouseClick(check_box, Qt.MouseButton.LeftButton, pos=pos)
 
 

tests/test_main_presenter.py

@@ -175,41 +175,50 @@ def testUpdateRecentProjects(self):
             self.assertEqual(self.view_mock.recent_projects, [r'C:\folder\test.png'])
 
     def testConfirmSave(self):
-        # confirmSave should return True when project_data is None
+        callback = mock.Mock()
+        # confirmSave should call callback when project_data is None
         self.model_mock.return_value.project_data = None
-        self.assertTrue(self.presenter.confirmSave())
+        self.presenter.confirmSave(callback)
+        callback.assert_called_once()
         self.view_mock.showSaveDiscardMessage.assert_not_called()
 
-        # confirmSave should return True when there are no unsaved changes
+        # confirmSave should call callback when there are no unsaved changes
         # and the save-discard message should not be called
         self.model_mock.return_value.project_data = self.test_project_data
         self.view_mock.undo_stack.setClean()
-        self.assertTrue(self.presenter.confirmSave())
+        self.presenter.confirmSave(callback)
+        self.assertEqual(callback.call_count, 2)
         self.view_mock.showSaveDiscardMessage.assert_not_called()
 
         # confirmSave should return False when user selects cancel on
         # the save-discard message box
         self.view_mock.undo_stack.resetClean()
         self.view_mock.showSaveDiscardMessage.return_value = MessageReplyType.Cancel
-        self.assertFalse(self.presenter.confirmSave())
+        self.presenter.confirmSave(callback)
+        self.assertEqual(callback.call_count, 2)
 
         # confirmSave should return True when user selects discard on
         # the save-discard message box
+        self.assertFalse(self.presenter.can_discard)
         self.view_mock.showSaveDiscardMessage.return_value = MessageReplyType.Discard
-        self.assertTrue(self.presenter.confirmSave())
+        self.presenter.confirmSave(callback)
+        self.assertEqual(callback.call_count, 3)
+        self.assertTrue(self.presenter.can_discard)
 
         # confirmSave should call save (if save path exist) then return True
         # when user selects save on the save-discard message box
         self.model_mock.return_value.save_path = self.test_filename_1
         self.presenter.saveProject = mock.create_autospec(self.presenter.saveProject)
         self.view_mock.showSaveDiscardMessage.return_value = MessageReplyType.Save
-        self.assertTrue(self.presenter.confirmSave())
-        self.presenter.saveProject.assert_called_with()
+        self.presenter.confirmSave(callback)
+        self.assertEqual(callback.call_count, 3)
+        self.assertFalse(self.presenter.can_discard)
+        self.presenter.saveProject.assert_called_with(callback=callback)
 
         # confirmSave should call save_as (if save_path does not exist)
         self.model_mock.return_value.save_path = ""
-        self.presenter.confirmSave()
-        self.presenter.saveProject.assert_called_with(save_as=True)
+        self.presenter.confirmSave(callback)
+        self.presenter.saveProject.assert_called_with(save_as=True, callback=callback)
 
     def testConfirmClearStack(self):
         self.view_mock.undo_stack = mock.Mock()