Cancelling an invalid problem edit then saying "save" discards all changes.

[clevengr]

Describe the issue:

If the Admin makes changes while editing a problem, then pushes the "Cancel" button, a dialog saying "Problem modified, save changes? [Yes] [No] [Cancel]" is presented. If the user presses "Yes", the expectation is that the changes which were entered will be saved. This works correctly if all the changes were valid (that is, if the changes result in a "valid problem state".

If however the changes leave the problem in an invalid state, pressing "Yes" discards all changes -- even valid ones.

For example, if the user changes the problem time limit and also changes the Output Validator to "PC2" (from something other than "PC2") but fails to select a "PC2 Validator Option", the Admin returns to the main screen having discarded all changes, including the legitimate change of the problem time limit. Confusingly, it also subsequently displays a popup warning "PC2 Validator is selected; you must select a Validator Mode Option" -- even though the Edit screen is gone and the user cannot select that option any more without re-opening the Edit Problem pane -- but the user has lost all other changes made (even though they said "Yes, save changes").

To Reproduce:

• Start a server with a contest having at least one problem.

• Start an Admin, select a problem, press the "Edit Problem" button.

• Select the "Output Validator" tab. If the problem already has "Use PC2 Validator" selected:

  • select the "Do not use Validator" radio button.
  • press Update (this is to make sure the problem starts with NOT having the PC2 validator, so we can switch to the PC2 validator).
  • Select "Edit Problem" again.

• Change the "Run Timeout Limit" (on the General tab) to some new value (remember the new value).

• Select the "Output Validator" tab.

• Select the Use PC2 Validator radio button.

• Make sure that the "PC2 Validator Mode" drop-down says "None selected" (change it to "None selected" if not).

At this point you have made two changes: setting a new Run Timeout Limit and selecting the PC2 Output Validator (with option "None" selected). You might want to make all sorts of additional changes -- judging type, data files, input validator, etc. -- these would represent all sorts of changes a user might also make. However, no more changes are necessary to demonstrate the bug. Continue:

• Press the Cancel button. This displays a dialog "Problem modified, save changes?".
• Press the Yes button.

Expected behavior:

The system should prompt that the current configuration is invalid (because no PC2 Option has been selected), and should leave the user on the Edit Problem screen allowing them to fix the invalid data while not losing all the other changes they've made.

Actual behavior:

• The system discards ALL changes and returns to the "Problems Pane". The fact that ALL changes (even valid ones) were discarded even though the user said "Yes, save changes" can be verified by looking at the "Run Timeout Limit" value -- it will have its OLD value, not the new changed value.
• A delayed Message dialog saying "PC^2 Validator is selected; you must select a Validator Mode option" pops up -- but dismissing that Message dialog doesn't give the user the option to fix the problem; rather, all changes have been discarded just as if the user had selected "No, do not save changes".

Environment:

• This was observed on Windows 8.1 and 10; I'm pretty sure it will manifest on ANY OS platform.
• This was observed in PC2 Version 9.7.0-develop (that is, on the Develop branch in my GitHub fork, which is up-to-date with Develop in the master repo.

Additional context:

Method EditProblemPane.handleCancelButton() calls updateProblem(), which in turn calls validateProblemFromFields(), which contains the following logic block:

        if (getUsePC2ValidatorRadioButton().isSelected()) {
            if (getPc2ValidatorOptionComboBox().getSelectedIndex() < 1) {
                showMessage("PC^2 Validator is selected; you must select a Validator Mode option (\"Output Validator\" tab)");
                return false;
            }
        }

Method showMessage() calls SwingUtilities.invokeLater() to display the "invalid-state" message (which explains why the message shows up later). validateProblemFromFields() then returns false, which causes updateProblem() to terminate the attempt to update the problem (thus discarding all changes, including legitimate ones).

The issue, then, is why doesn't the Edit Problem pane remain visible at that point so the user can fix the problem?

Further note: the above uses "selecting the PC2 validator without selecting a validator option" as an example of the issue -- but the problem isn't strictly related to the PC2 Validator Option. For example, on the "i118_Integrate_VIVA" branch, the exact same issue occurs if you select an invalid Input Validator combination -- for example, selecting the "Use VIVA" button but failing to enter a VIVA pattern -- then press "Cancel" then press "Yes, save changes": ALL changes are lost and a delayed popup Message dialog appears. I'm pretty sure that ANY combination of "multiple changes where one of the changes leaves the problem in an invalid state" will cause this bug to manifest.

[troy2914]

If you hit close, and answer yes, and the state is invalid, you want the edit problem to not go away?

[clevengr]

If you hit close, and answer yes, and the state is invalid, you want the edit problem to not go away?

If the user makes a change to a problem and then is presented with a message saying "Problem modified - save changes?" and the user clicks "Yes", and the dialog goes away, then the user's presumption is going to be that those changes were saved. However, this doesn't happen if there were some changes which are invalid (see the Issue writeup for a detailed example). Instead of either saving only the valid changes (which I think would also be misleading), or refusing to save anything unless everything is valid, the valid changes are discarded.

Since I don't think the user should be misled by answering "yes" to "Save changes?" and then those changes aren't saved, the answer to your question is "Yes -- I think the Edit Problem dialog should not go away" -- instead, a new dialog identifying the "invalid" state should be displayed and the user should be told to correct the problem.