update collision should show a better error

[moellep]

I've seen many cases where scientists will have lots of browser tabs open, and make a change to a sim, and then see the "simulation already modified" message. Now that message is gone, and the incorrect "Server upgraded" message is shown, and they are kicked out of the simulation back to the simulation list.

Instead it should show a "simulation already modified" error, possibly with a link to refresh the window, or refresh the simulation automatically. Otherwise they lose all their context and can't return to the simulation they want.

[robnagler]

Technically this could be detected by the JS. It's just not clear to me where and how it should be detected. The code before was in autoSave, and the refresh was forced with an alertText. We could do something similar catching a specific SRException and showing an alert. However, I don't understand the flow of the data at that point. You would need to know that you are in the same simulation, for example. If you flesh out the UI part and how it is to be called, I can link it up to the request infrastructure.

[robnagler]

@moellep LMK when you want to work on this.

[moellep]

This problem keeps coming up for users who have the simulation open in multiple tabs, or on a home computer. How about handling this on the client in handleSRException, so the current browser refreshes with the correct data after a message, and editing can continue:

    self.handleSRException = function(...) {
        //...
        if (e.routeName == "serverUpgraded" && e.params
            && e.params.reason === 'invalidSimulationSerial') {
            errorService.alertText(`
                This simulation has been updated outside of this browser.
                This page will refresh in 5 seconds.
            `);
            $timeout(() => {
                window.location.reload();
            }, 5000);
            return;
        }

[moellep]

Actually, it would be better to show a "Refresh" button in the alert message, so the refresh is manual - that way a separate instance of the simulation wouldn't also automatically refresh.

[robnagler]

We could add a routeName invalidSimulationSerial. It would be simpler.

Actually, it would be better to show a "Refresh" button in the alert message, so the refresh is manual - that way a separate instance of the simulation wouldn't also automatically refresh.

I think the automatic refresh is fine. That's what we did before. The window that has the correct serial, will not get the alert.

If you work on the UI, I'll work on the backend and the test.

[moellep]

I'm going to work on the UI now. I think we need a confirmation page for both the "version conflict" and the "server upgraded" message (it can be the same message). Something that warns the user that their recent change was not saved and the simulation will refresh now. Having a confirmation also prevents the same sim getting into an autoUpdate loop with both refreshing. Only the confirmed sim should be refreshed.

[robnagler]

I think we should have two different srExceptions, since they are different cases. The code then can differentiate the errors.

Having a confirmation also prevents the same sim getting into an autoUpdate loop with both refreshing. Only the confirmed sim should be refreshed.

Have you seen this happen? It seems like the one in conflict won't be allowed to update so there won't be a loop.

[moellep]

Here is an example where the loop could happen:
Open a radia sim to the Visualization tab - it runs several simulations. Wait for it to finish and minimize one of the windows (this will invoke autoSave later).
Then open the same sim in another browser window - it will also run several simulations.
The first simulation is invalid, but if it automatically refreshes on the serverUpgrade, it will become the new primary simulation, and the current window will be come invalid again. Showing a confirmation page when invalid will stop that old simulation from becoming the new primary.

[robnagler]

Ah so there are changes being saved that aren't actually differences.