Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix cancelling saving after attempting to close Workbench #26519

Merged
merged 4 commits into from Aug 7, 2019

Conversation

PhilColebrooke
Copy link
Contributor

Description of work.
When you attempt to close a project with unsaved changes on Workbench, a dialog box asks if you want to save. Clicking yes opens a window for choosing the file name and location. Previously clicking cancel on this window would cause Workbench to close without saving, but now it only closes the Save As dialog and Workbench remains open.

To test:

  1. Open Workbench and make changes to the project, e.g. load a workspace.
  2. Click the X in the corner to close Workbench.
  3. When asked if you want to save click Yes.
  4. When the Save As dialog opens, check that clicking cancel does not close Workbench.
  5. Check that the rest of the saving functionality works as intended, including saving via File -> Save Project As... and File -> Save Project.

Fixes #26515


Reviewer

Please comment on the following (full description):

Code Review
  • Is the code of an acceptable quality?
  • Does the code conform to the coding standards?
  • Are the unit tests small and test the class in isolation?
  • If there are changes in the release notes then do they describe the changes appropriately?
Functional Tests
  • Do changes function as described? Add comments below that describe the tests performed?
  • Do the changes handle unexpected situations, e.g. bad input?
  • Has the relevant (user and developer) documentation been added/updated?

Does everything look good? Mark the review as Approve. A member of @mantidproject/gatekeepers will take care of it.

@PhilColebrooke PhilColebrooke added the GUI Issues and pull requests specific to the Mantid Workbench GUI. label Jul 30, 2019
@PhilColebrooke PhilColebrooke added this to the Release 4.2 S1 milestone Jul 30, 2019
Copy link
Member

@gemmaguest gemmaguest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look good and it works as expected under all the saving possibilities I can think of, apart from the final issue mentioned in #26513 which will be dealt with separately. Just need to check that the new behaviour is unit-tested.

@@ -97,7 +97,7 @@ def test_offer_save_does_nothing_if_saved_is_true(self):

def test_offer_save_does_something_if_saved_is_false(self):
self.project._offer_save_message_box = mock.MagicMock(return_value=QMessageBox.Yes)
self.project.save = mock.MagicMock()
self.project.save = mock.MagicMock(return_value=None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is a new test required where the return value here is True?

@DanNixon DanNixon merged commit 2479f22 into master Aug 7, 2019
@DanNixon DanNixon deleted the 26515_cancel_save_workbench branch August 7, 2019 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI Issues and pull requests specific to the Mantid Workbench GUI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workbench: Clicking cancel after attempting to save an unsaved project after closing closes Workbench
3 participants