search

openworm / org.geppetto.simulation

Generic simulation bundle for Geppetto
http://www.geppetto.org/
Other
9 stars 10 forks source link

disallow set experiment view if no write permission #131

Closed mattearnshaw closed 7 years ago

mattearnshaw commented 7 years ago

this fixes issue with OSB sample projects having their experiment state written (since they are persisted, see comodl because osb.org has a temporary fix in place; for example every time you open HH there are more popups because the script runs, and the ones from before have been saved)

tarelli commented 7 years ago

@gidili can you review this?

mattearnshaw commented 7 years ago

see also https://github.com/openworm/org.geppetto.frontend/pull/648

gidili commented 7 years ago

@mattearnshaw @tarelli issue: if I understand correctly the change - this might cause tons of exceptions to be raised in the case of non-persisted projects with local storage enabled because the setView call always hits the server, if we don't have the persistence bundle or the project is not persisted we store the view server side in the volatile project object so that when the project is downloaded (download project feature) the views are also serialized(*). The only case the server does not get hit at the moment is if local storage is disabled and there is no persistence (code here).

As I understand it this change is meant to fix the case where the project is persisted BUT the user does not have write access. Is this correct @mattearnshaw and when is it happening? Or are you worried about it happening manually (like from console etc)?

Either way unless I am missing something it might a simple case of adding that check on the client side on this line here adding && GEPPETTO.userController.hasWritePermissions() (we can then remove GEPPETTO.UserController.persistence since the other check includes that).

(*) You might think "this is a bad idea" but we had lengthy conversations at the time and decided that this implementation was better than serailizing the project without views and add them client side tampering on the fly with the json (which was the initial implementation before we decided against it), this way everything gets serialized in one go.

mattearnshaw commented 7 years ago

@gidili I added the client side check already here :) I will remove the redundant condition

mattearnshaw commented 7 years ago

obviated by https://github.com/openworm/org.geppetto.frontend/pull/648