att / rcloud.flexdashboard

flexdashboard from an RCloud notebook
Other
4 stars 10 forks source link

for anonymous user, flexdashboard.html doesn't display any error message #13

Closed gordonwoodhull closed 7 years ago

gordonwoodhull commented 8 years ago

From @Prateek032 on November 28, 2016 10:02

If the notebook is unpublished and anonymous user tries to open it using shareable links(shiny, mini, view) it throws the following error message.

image

But for the flexdashboard, it doesn't display any error.

Copied from original issue: att/rcloud#2365

gordonwoodhull commented 8 years ago

Another good one, thanks @Prateek032!

Moving to att/rcloud.flexdashboard.

gordonwoodhull commented 8 years ago

The error is identical to #12 (rcloud._ocaps.load_module_package is not a function).

I didn't know about that ocap and I'm not sure if it's safe to open it up to anonymous users. Anyway we'll need to reorganize this package a bit so that it does something equivalent to the view.js/mini.js stuff that uses load_notebook to check if the notebook is accessible.

There's in general a little too much on the client side in this package. While stuff like window.RCloudFlexDashboard.renderFlexDashboard and rcloud._ocaps.load_module_package do work, they don't feel idiomatic. The latter should be exported in rcloud.js (I guess), and I'm not sure what to do about the former. I'll have to return to this later.

It's all normal code-review and testing that we didn't have time for because we were in a rush to release. Hopefully we can set more reasonable schedules in 2017.

gaborcsardi commented 7 years ago

I agree that this is hacky, and if it does not work for the anonymous user, then we need to change it, obviously. You cannot really use the same approach as in mini.html here, because we are not running the notebook here at all, but some custom R code that works on the notebook.

Basically all we need is a way to trigger an R package load from the JS, that's all I am using load_module_package for. If there is something else for this, that's an easy fix then. If not, then it should be added I think.

I am not sure what is wrong with window.RCloudFlexDashboard.renderFlexDashboard, I create this in the .onLoad() function of the package. Is there another way to pass an OCAP to the JS side?

gordonwoodhull commented 7 years ago

Agree, I think we should expose an rcloud.load_module_package and on a second look window.RCloudFlexDashboard.* makes sense to me too. We might be able to generalize this in the future because there are probably other circumstances where you'd want to process the notebook in R instead of running it for its output/side effects.

(BTW I think this package could be a general-purpose RMarkdown renderer for notebooks, enabling slidify and stuff like that, rather than just for flexdashboard.)

gaborcsardi commented 7 years ago

(BTW I think this package could be a general-purpose RMarkdown renderer for notebooks, enabling slidify and stuff like that, rather than just for flexdashboard.)

Indeed, there is actually nothing flexdashboard specific in this package, and it does not even have to import flexdashboard, it uses no code from it. As it is today, it could be merged into the rcloud.rmd package. But this might change in the future, and we might add flexdashboard specific stuff.

gordonwoodhull commented 7 years ago

An error is displayed, so this is fixed by #17. However, we still need to allow anonymous users somehow to enable the OCAP which is needed to allow them to view flexdashboards (#12).