Allow not logged in users to interact with temporary files (#2727)

* Allow specifying temporary files via tryit notebooks. For example:
  `tryit?iomd=1234&file=abcd&filename=abcd.txt`
* Allow uploading files when not logged in (they will be stored
  in memory and uploaded if the user ever authenticates / logs in) -
  fixes #2680
* As part of this work, refactor files modal to use redux

server/notebooks/templates/notebook.html

@@ -20,5 +20,8 @@
 <script>
   window.evalFrameOrigin = "{{ eval_frame_origin }}";
 </script>
+{% for filename, filecontent, mimetype in files %}
+<script id="file-{{ filename }}" type="application/base64" mimetype="{{ mimetype }}">{{ filecontent }}</script>
+{% endfor %}
 <script src="/iodide.js"></script>
 {% endblock %}

server/notebooks/views.py

@@ -1,5 +1,8 @@
+import base64
+import mimetypes
 import urllib.parse
 
+from django.core.exceptions import ValidationError
 from django.db import transaction
 from django.http import HttpResponseBadRequest
 from django.shortcuts import get_object_or_404, redirect, render
@@ -135,17 +138,35 @@ def _get_new_notebook_content(iomd):
     return get_template("new_notebook_content.iomd").render()
 
 
-def _get_iomd_params(iomd):
-    if iomd:
-        return f"?iomd={urllib.parse.quote_plus(iomd)}"
-    return ""
+def _get_files_from_request(request, base64_encode=False):
+    filenames = request.GET.getlist("filename")
+    file_contents = request.GET.getlist("file")
+
+    if len(file_contents) != len(filenames):
+        raise ValidationError("Number of file contents must match filenames")
+
+    # re-encode file_contents in the appropriate format
+    file_contents = [bytes(file_content, "utf-8") for file_content in file_contents]
+    if base64_encode:
+        file_contents = [
+            base64.b64encode(file_content).decode("utf-8") for file_content in file_contents
+        ]
+
+    return zip(
+        filenames, file_contents, [mimetypes.guess_type(filename)[0] for filename in filenames],
+    )
 
 
 @ensure_csrf_cookie
 def new_notebook_view(request):
-    iomd = request.GET.get("iomd")
     if not request.user.is_authenticated:
-        return redirect(reverse("try-it") + _get_iomd_params(iomd))
+        return redirect(reverse("try-it") + "?" + request.META["QUERY_STRING"])
+
+    try:
+        iomd = request.GET.get("iomd")
+        files = _get_files_from_request(request)
+    except ValidationError as e:
+        return HttpResponseBadRequest(content=e)
 
     # create a new notebook and redirect to its view
     with transaction.atomic():
@@ -156,6 +177,8 @@ def new_notebook_view(request):
             title=get_random_compound(),
             is_draft=True,
         )
+        for (filename, content, _) in files:
+            File.objects.create(notebook=notebook, filename=filename, content=content)
     return redirect(notebook)
 
 
@@ -166,10 +189,14 @@ def tryit_view(request):
 
     If user is logged in, redirect to `/new/`
     """
-    iomd = request.GET.get("iomd")
-
     if request.user.is_authenticated:
-        return redirect(reverse(new_notebook_view) + _get_iomd_params(iomd))
+        return redirect(reverse(new_notebook_view) + "?" + request.META["QUERY_STRING"])
+
+    try:
+        iomd = request.GET.get("iomd")
+        files = _get_files_from_request(request, base64_encode=True)
+    except ValidationError as e:
+        return HttpResponseBadRequest(content=e)
 
     return render(
         request,
@@ -182,6 +209,7 @@ def tryit_view(request):
                 "title": "Untitled notebook",
             },
             "iomd": _get_new_notebook_content(iomd),
+            "files": files,
             "iframe_src": _get_iframe_src(),
             "eval_frame_origin": EVAL_FRAME_ORIGIN,
         },

server/tests/test_notebook_view.py

@@ -101,7 +101,7 @@ def test_new_notebook_view(client, fake_user, logged_in, iomd):
         client.force_login(fake_user)
 
     path = reverse("new-notebook")
-    params = f"?iomd={urllib.parse.quote_plus(iomd)}" if iomd else ""
+    params = f"?iomd={urllib.parse.quote_plus(iomd)}" if iomd else "?"
     path += params
 
     response = client.get(path, follow=True)

src/__tests__/__snapshots__/storyshots.test.js.snap

@@ -907,7 +907,7 @@ exports[`Storyshots File sources pane one file source: user can't save 1`] = `
 </div>
 `;
 
-exports[`Storyshots Files pane no files: user can save 1`] = `
+exports[`Storyshots Files pane no files 1`] = `
 <div
   className="css-1tqhp7d-BodyWrapper e198ef500"
 >
@@ -929,23 +929,7 @@ exports[`Storyshots Files pane no files: user can save 1`] = `
 </div>
 `;
 
-exports[`Storyshots Files pane no files: user can't save 1`] = `
-<div
-  className="css-1tqhp7d-BodyWrapper e198ef500"
->
-  <div
-    className="css-qy57zi-Body e198ef501"
-  >
-    <span
-      className="css-1l4unqs-NoFilesNotice emgvl1y0"
-    >
-      No files are attached to this notebook
-    </span>
-  </div>
-</div>
-`;
-
-exports[`Storyshots Files pane one file: user can save 1`] = `
+exports[`Storyshots Files pane one file 1`] = `
 <div
   className="css-1tqhp7d-BodyWrapper e198ef500"
 >
@@ -962,74 +946,9 @@ exports[`Storyshots Files pane one file: user can save 1`] = `
       className="css-1h6cnob-List edroo1r3"
     >
       <div
-        className="saved first-visible last-visible css-1e1cs9a-ListItem-FileListItem eclaraf0"
-        id={null}
-      >
-        <a
-          className="css-16plg5q-FilenameLink eclaraf1"
-          href="http://localhost/files/foobar.jsx"
-          target="_blank"
-        >
-          foobar.jsx
-        </a>
-        <div
-          className="css-1361dfe-FileActions eclaraf5"
-        >
-          <button
-            className="file-action file-action-delete eclaraf6 css-nz5qwq-TextButtonContainer-GenericButton-TextButtonContainer-FileAction e1ac55ls2"
-            onClick={[Function]}
-          >
-            delete
-          </button>
-        </div>
-      </div>
-    </div>
-    <footer
-      className="css-k9pcj0-FileFooter evif4vm0"
-    >
-      These files can be loaded by
-
-      <a
-        href="https://iodide-project.github.io/docs/iomd/#fetch-chunks-fetch"
-      >
-        fetch chunks
-      </a>
-
-      and manipulated via the
-
-      <a
-        href="https://iodide-project.github.io/docs/api/#iodidefile"
-      >
-        file API
-      </a>
-      .
-    </footer>
-  </div>
-</div>
-`;
-
-exports[`Storyshots Files pane one file: user can't save 1`] = `
-<div
-  className="css-1tqhp7d-BodyWrapper e198ef500"
->
-  <div
-    className="css-qy57zi-Body e198ef501"
-  >
-    <div
-      className="css-1h6cnob-List edroo1r3"
-    >
-      <div
-        className="saved first-visible last-visible css-1e1cs9a-ListItem-FileListItem eclaraf0"
-        id={null}
-      >
-        <a
-          className="css-16plg5q-FilenameLink eclaraf1"
-          href="http://localhost/files/foobar.jsx"
-          target="_blank"
-        >
-          foobar.jsx
-        </a>
-      </div>
+        className=" first-visible css-8ttp8x-ListItem-FileListItem eclaraf0"
+        id="file-215"
+      />
     </div>
     <footer
       className="css-k9pcj0-FileFooter evif4vm0"

src/editor/actions/__tests__/file-actions.test.js

@@ -1,7 +1,8 @@
 import configureMockStore from "redux-mock-store";
 import thunk from "redux-thunk";
 
-import { getFiles } from "../file-actions";
+import { updateFiles, deleteFile } from "../file-actions";
+import * as CALLBACKS from "../file-request-callbacks";
 import * as FILE_OPS from "../../../shared/utils/file-operations";
 
 const mockStore = configureMockStore([thunk]);
@@ -10,12 +11,26 @@ const initialState = () => {
     userData: { name: "this-user" },
     notebookInfo: {
       connectionMode: "SERVER",
-      username: "this-user"
+      username: "this-user",
+      files: [
+        {
+          filename: "test.csv",
+          id: 0,
+          lastUpdated: "2019-04-03T16:51:45.075609+00:00",
+          status: "saved"
+        },
+        {
+          filename: "test2.csv",
+          id: 0,
+          lastUpdated: "2019-04-04T16:51:45.075609+00:00",
+          status: "saved"
+        }
+      ]
     }
   };
 };
 
-describe("getFiles (editor action)", () => {
+describe("updateFiles", () => {
   it("translates a response from the server as expected", async () => {
     const getFilesMock = jest.spyOn(FILE_OPS, "getFilesForNotebookFromServer");
     getFilesMock.mockImplementation(() => {
@@ -30,7 +45,7 @@ describe("getFiles (editor action)", () => {
     });
     const store = mockStore(initialState());
 
-    const request = store.dispatch(getFiles());
+    const request = store.dispatch(updateFiles());
 
     await expect(request).resolves.toBe(undefined);
 
@@ -39,8 +54,110 @@ describe("getFiles (editor action)", () => {
     expect(store.getActions()).toEqual([
       {
         type: "UPDATE_FILES_FROM_SERVER",
-        files: [{ filename: "test.csv", id: 1, lastUpdated: "a-date-string" }]
+        serverFiles: [
+          {
+            filename: "test.csv",
+            id: 1,
+            lastUpdated: "a-date-string",
+            status: "saved"
+          }
+        ]
       }
     ]);
   });
 });
+
+describe("deleteFile", () => {
+  let deleteFileMock;
+  let postSuccessMock;
+  let postErrorMock;
+  let store;
+
+  beforeEach(() => {
+    deleteFileMock = jest.spyOn(FILE_OPS, "deleteFileOnServer");
+    deleteFileMock.mockReset();
+    postSuccessMock = jest.spyOn(CALLBACKS, "postFileOperationSuccessMessage");
+    postSuccessMock.mockReset();
+    postErrorMock = jest.spyOn(CALLBACKS, "postFileOperationErrorMessage");
+    postErrorMock.mockReset();
+    store = mockStore(initialState());
+  });
+
+  ["from-editor", "from-eval-frame"].forEach(variant => {
+    const options =
+      variant === "from-eval-frame"
+        ? { fileRequestID: "some-file-request-id" }
+        : {};
+
+    it(`fails if attempting to delete a non-existent file (${variant})`, async () => {
+      await expect(
+        store.dispatch(deleteFile("does-not-exist.csv", options))
+      ).resolves.toBe(undefined);
+
+      if (variant === "from-editor") {
+        expect(postErrorMock).toHaveBeenCalledTimes(0);
+        expect(store.getActions()).toEqual([
+          {
+            type: "UPDATE_FILE",
+            status: "error",
+            filename: "does-not-exist.csv",
+            errorMessage: '(delete) file "does-not-exist.csv" does not exist'
+          }
+        ]);
+      } else {
+        expect(postErrorMock).toHaveBeenCalledTimes(1);
+        expect(store.getActions()).toEqual([{ type: "NOOP" }]);
+      }
+    });
+
+    it(`successfully handles a server error on deleteFileFromServer (${variant})`, async () => {
+      deleteFileMock.mockImplementation(() => {
+        return Promise.reject(new Error("artificial error"));
+      });
+
+      await expect(
+        store.dispatch(deleteFile("test.csv", options))
+      ).resolves.toBe(undefined);
+
+      expect(deleteFileMock).toHaveBeenCalledTimes(1);
+      if (variant === "from-editor") {
+        expect(postErrorMock).toHaveBeenCalledTimes(0);
+        expect(store.getActions()).toEqual([
+          {
+            type: "UPDATE_FILE",
+            status: "error",
+            filename: "test.csv",
+            errorMessage: "artificial error"
+          }
+        ]);
+      } else {
+        expect(postErrorMock).toHaveBeenCalledTimes(1);
+        expect(store.getActions()).toEqual([{ type: "NOOP" }]);
+      }
+    });
+
+    it(`successfully calls deleteFileFromServer & dispatches DELETE_FILE (${variant})`, async () => {
+      deleteFileMock.mockImplementation(() => {
+        return Promise.resolve(undefined);
+      });
+
+      await expect(
+        store.dispatch(deleteFile("test.csv", options))
+      ).resolves.toBe(undefined);
+
+      expect(deleteFileMock).toHaveBeenCalled();
+      expect(store.getActions()).toEqual([
+        {
+          filename: "test.csv",
+          status: "deleted",
+          type: "UPDATE_FILE"
+        }
+      ]);
+      if (variant === "from-editor") {
+        expect(postSuccessMock).toHaveBeenCalledTimes(0);
+      } else {
+        expect(postSuccessMock).toHaveBeenCalledTimes(1);
+      }
+    });
+  });
+});