[FIX] Workflow saving: fix cancel, enforce ows extension

[krystofair]

I catched myself on doing that twice, when rename workflow file and cannot find it because of extension.
This change is about it. Add extension to filename if there is not.

This is one solution among two I think.
The second one is change visibility of files the user can choose from when opening saved workflow.
The first is simpler I guess and it seems more reasonable.

There is possibility about change type of filename when passed as pathlib.Path,
but this operation is safe in this place. Casting will not raise exception when
filename is pathlib.Path type indeed.

I changed it, because in my first commit basename was from original filename, so it can misguide.

Used Orange3 in version: 3.39

[krystofair]

Could be done in that way, but there can happen weird behaviour:

• Asking about replace file which exists, there is file with the same name without ext
• click [ok] - saved file, without overriding,
• user will have two files.

There is another bug:

orange-canvas-core 0.2.6

1. Open new workflow and writing name of it with file already exists in recent localization
2. Save when "modified" indicator is visible.
3. Click "Reject/Cancel"
4. Workflow is saved, no notification to user! existing file is replaced.

[krystofair]

Second commit fix mentioned bug.

[markotoplak]

Thank you for this PR, and sorry that we took so long to look into it.

I especially appreciate the fix for the correct treatment of clicking on the "Cancel" in the dialog. That was a nasty bug.

I like the idea of keeping the .ows extension, but now this PR introduced a new bug. Let's assume a file "a.ows" already exists. Then, the user saves their workflow into "a". Now, the code silently add ".ows" and this the user is not notified that they are overwriting an existing file. So that silent addition of ".ows" is dangerous.

We did attempt to handle this in the Save widget and the solution in owsavebase.py (from biolab/orange3) was to manually put up an additional dialog if needed. We could do something similar here. So the crux is this:

                if not os.path.exists(filename) or QMessageBox.question(
                        self, "Overwrite file?",
                        f"File {os.path.split(filename)[1]} already exists.\n"
                        "Overwrite?") == QMessageBox.Yes:

owsavebase.py handles different platforms separately, but I think the above "post-processing" of the dialog results should work with all platforms. Could you try a similar patch?

[markotoplak]

Please also rebase to master. If you perhaps don't have time to work on this PR, please tell us, so that we can add the check ourselves. Thank you!

[krystofair]

Yup, I know about that weird behaviour, but in that moment I cannot figure out where is the best place to do this. So what I was thinking, was compromise. 😎
Now that feature is in right place, I understood that thing about assigned 'path' when file was previously saved.

Thanks for this override snippet - added.

[krystofair]

it is, what is reverted

[krystofair]

/rebase

[krystofair]

I don't get one thing. When I run tests at my machine I have to still accept manually dialog with "override", even though the settings: AcceptSave and DontConfirmOverwrite are set. 😑

Maybe here will not ask, but if yes - any ideas?

[janezd]

Confirmation comes a few lines later in the code, where it calls QMessageBox. To avoid this, you need to patch the MessageBox.question, like this (the first patch is already there, just add the second line and the necessary imports):

        with patch("AnyQt.QtWidgets.QFileDialog.exec", exec), \
            patch("AnyQt.QtWidgets.QMessageBox.question", new=Mock(return_value=QMessageBox.Yes)):

[janezd]

@markotoplak, after you approve the PR (please also add a comment so I get an email), I can update the translations and merge.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.55%. Comparing base (005dc69) to head (feff7b5).

Files with missing lines	Patch %	Lines
orangecanvas/application/canvasmain.py	71.42%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #330      +/-   ##
==========================================
+ Coverage   76.54%   76.55%   +0.01%     
==========================================
  Files         102      102              
  Lines       21316    21313       -3     
==========================================
  Hits        16317    16317              
+ Misses       4999     4996       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[krystofair]

I don't like this mechanism. There is enough to write name as "title.o" to skip this at all and save file which can't be found...
But I think this is the most elegant solution without complicate things further.

Anyway that fix of "saving at cancel" is more important.