InformaticsMatters / portal

lac portal
Apache License 2.0
0 stars 0 forks source link

Shared notebook cannot be opened #15

Closed tdudgeon closed 8 years ago

tdudgeon commented 8 years ago

I'm hitting a problem with sharing. There was a problem with main site where shared notebooks were not showing up in your list of notebooks, but I fixed this. Now I see them but they can't be opened. This also happens when running in the IDE. You select the notebook in the list but it does not open in the canvas panel and the history panel does not show the right contents (just one editable with id=1 and created and updated as undefined). One thing is that the shared notebook needs to have a savepoint for sharing to make sense (1), but I made sure this is the case. For the user opening the shared notebook there would not initially be any editables, which is probably why nothing opens (though that should not stop the history panel showing the savepoints) . If this is the case we need to take some action, probably show a dialog to the user asking which savepoint they want to work with, and then create an editable from that one.

Could we work out what is the best approach to address this.

(1). we should also somehow prevent sharing of a notebook that does not have any savepoints

gabrielmoreno commented 8 years ago

Technically fixed. However, brings up new case. IE.: no user1 savepoint at all, to create editable on user2 results in 2 separate trees.

tdudgeon commented 8 years ago

Not working for me yet. When I open a shared notebook I get an exception because it tries to create a new editable without a parent, which the back end prevents by throwing exception. Seems like portal app is truing to create new un-rooted editable?

We should avoid the new case you describe by preventing a notebook to be shared unless it has at least one savepoint.

tdudgeon commented 8 years ago

Here is the relevant bits of the stacktrace:

portal_1       | java.io.IOException: Request failed: java.lang.NullPointerException: Editable must have a parent

...

portal_1       |    at org.squonk.core.client.AbstractHttpClient.checkResponse(AbstractHttpClient.java:246)
portal_1       |    at org.squonk.core.client.AbstractHttpClient.executePostAsInputStream(AbstractHttpClient.java:127)
portal_1       |    at org.squonk.core.client.AbstractHttpClient.executePostAsInputStream(AbstractHttpClient.java:120)
portal_1       |    at org.squonk.core.client.NotebookRestClient.createEditable(NotebookRestClient.java:168)
portal_1       |    at portal.notebook.webapp.NotebookSession.createRootEditable(NotebookSession.java:354)
portal_1       |    at portal.notebook.webapp.NotebookSession.loadCurrentNotebook(NotebookSession.java:93)
portal_1       |    at portal.notebook.webapp.NotebookSession$Proxy$_$$_WeldClientProxy.loadCurrentNotebook(Unknown Source)
portal_1       |    at portal.notebook.webapp.NotebookListPanel$1$4.onEvent(NotebookListPanel.java:129)
portal_1       |    at org.apache.wicket.ajax.AjaxEventBehavior.respond(AjaxEventBehavior.java:124)
portal_1       |    at org.apache.wicket.ajax.AbstractDefaultAjaxBehavior.onRequest(AbstractDefaultAjaxBehavior.java:633)
gabrielmoreno commented 8 years ago

Done. However, could be improved adding API to variable client

tdudgeon commented 8 years ago

Its not working for me. I get a "No default editable" error when I try to open a notebook (with savepoint) that a different user has shared. Stacktrace is:

java.lang.Exception: No default editable at portal.notebook.webapp.NotebookSession.loadCurrentNotebook(NotebookSession.java:93) at portal.notebook.webapp.NotebookSession$Proxy$_$$_WeldClientProxy.loadCurrentNotebook(Unknown Source)

Also, what additional API do you suggest to add?

gabrielmoreno commented 8 years ago

You should add a savepoint to the notebook as the owner or reset the DB. With the new changes that situation will never happen. Because notebooks without savepoints cannot be shared.

As per req.: "We should avoid the new case you describe by preventing a notebook to be shared unless it has at least one savepoint.", now the owner is not allowed to share a notebook without savepoints.

On API improvement: In order to disable sharing in a preemptive and efficient manner, we may want the NotebookVariableClient to provide info about whether the listed notebooks have savepoints or not (or if it´s shareable at all) along with the NotebookDTO. Otherwise we need to loop through the listed notebooks and list savepoints independently for each one just to find out whether there is at least one savepoint or not for a given notebook.

tdudgeon commented 8 years ago

The issue is that the shared notebook should be openable as it does contain a savepoint, but trying to open it (as the non-owner) results in the "No default editable" error and also nothing appears in the history panel.

I added some editable and savepoint counts to NotebookDTO so that you can find out if there are any savepoints present. https://github.com/InformaticsMatters/lac/commit/551909907d43c2e3651d54ad93db9a39f12980d2#diff-869377b25e267ed51552c73f0e1e2a12

In portal app I needed to modify MockNotebookVariableClient to allow for this (all counts are set to zero - you will need to change this). https://github.com/InformaticsMatters/portal/blob/19934f1cd5142e6c5f7dae81df4a7f253deba90f/portal-app/src/portal/notebook/service/MockNotebookVariableClient.java#L69

tdudgeon commented 8 years ago

Shared notebooks can now be opened.