Merge remote-tracking branch 'origin/master' into 188127118-bar-graph…

…-link

README.md

@@ -59,84 +59,11 @@ To deploy a production release:
 
 ## Developing/deploying cloud functions
 
-CLUE uses several Google cloud functions to implement certain features that would be difficult (or impossible) to implement entirely client-side.
-|Function|Purpose|
-|--------|-------|
-|_getImageData_|Retrieves image data that may reside in other classes and hence is not accessible client-side, e.g. for supports published to multiple classes or documents retrieved via the teacher network.|
-|_getNetworkDocument_|Retrieves the contents of a document accessible to a teacher via the teacher network.|
-|_getNetworkResources_|Retrieves the list of resources (documents) available to a teacher via the teacher network.|
-|_postDocumentComment_|Posts a comment to a document in firestore, adding metadata for the document to firestore if necessary.|
-|_publishSupport_|Publishes a document as a support that is accessible to all of a teacher's classes (including any referenced images).|
-|_validateCommentableDocument_|Checks whether a specific commentable document exists in firestore and creates it if necessary.|
-
-The code for the functions is in the `functions` directory. You should be able to cd into the
-`functions` directory and perform basic development operations:
-```
-$ cd functions
-$ npm install     # install local dependencies
-$ npm run lint    # lint the functions code
-$ npm run test    # runs jest (unit) tests for the functions code
-$ npm run build   # build the functions code (transpile TypeScript)
-```
-### Note 1
-There seems to be an uneasy relationship between the `node_modules` folder in the
-`functions` directory and the one in the parent directory. I had to explicitly specify the
-path to typescript in the `build` function. There's probably a better configuration available,
-but in the meantime this seems to mostly work.
-
-### Note 2
-When running `npm run test` with node 16, the following error is shown
-```
-TypeError: Cannot read properties of undefined (reading 'INTERNAL')
-```
-This error is triggered by the following line in `test-utils.ts`
-```
-import { useEmulators } from "@firebase/rules-unit-testing";
-```
-The current work around is to use node 14 to run the tests.
-
-See functions/dependency-notes.md for more on this.
-
-### Testing cloud functions
-
-Google recommends (requires?) that [firebase-tools](https://www.npmjs.com/package/firebase-tools) be installed globally:
-```
-$ npm install -g firebase-tools
-```
-This should be run periodically to make sure you're running the latest version of the tools.
-
-#### Running tests locally (without running functions in the emulator)
-```
-$ npm run serve                 # build and then start the emulators
-$ npm run test                  # run all tests in `functions` directory
-$ npm run test -- some.test.ts  # run a particular test
-```
-The existing tests currently work this way. They test the basic functionality of the cloud functions by importing and calling them directly from node.js test code. This is a simple and efficient way of testing the basic functionality without all the overhead of the functions emulator. The downside is that the node.js test environment is not the same as the hosted function environment. For instance, it's possible to return objects in node.js that can't be JSON-stringified which will throw an error when the function is hosted. That said, you can't beat the convenience of simply calling the functions directly.
-
-#### Running local tests against functions hosted in the emulator
-To run jest tests against functions running in the emulator requires [serving functions using a Cloud Functions Shell](https://firebase.google.com/docs/functions/local-shell#serve_functions_using_a_cloud_functions_shell). Currently, all of our functions are `HTTPS Callable` functions, which [can be called](https://firebase.google.com/docs/functions/local-shell#invoke_https_callable_functions) in this shell mode, but:
->Emulation of context.auth is currently unavailable.
-
-#### Running CLUE against functions running locally in the emulator:
-```
-$ npm run serve   # build and then start the functions emulator
-```
-and launch CLUE with url parameter `functions=emulator`.
+CLUE uses several Google cloud functions to implement certain features that would be difficult (or impossible) to implement entirely client-side. There are two folders of functions `functions-v1` and `functions-v2`. We are trying to incrementally migrate the v1 functions into the v2 folder.
 
-### To deploy firebase functions to production:
-```
-$ npm run deploy                        # deploy all functions
-$ npm run deploy:getImageData           # deploy individual function
-$ npm run deploy:postDocumentComment    # deploy individual function
-```
-
-By convention, our firebase functions have an internal version number that is returned with any results. This should be incremented appropriately when new versions are deployed. This will allow us to determine whether the current code in GitHub has been deployed or not, for instance. Also by convention, our firebase functions accept parameters of `{ warmUp: true }` which can be issued in advance of any actual call to mitigate the google cloud function cold-start issue.
-
-### Serving CLUE from https://localhost
-To test the deployed function(s) from your local development environment, you may need to run your local dev server with https to avoid CORS errors. To do so, [create a certificate](https://www.matthewhoelter.com/2019/10/21/how-to-setup-https-on-your-local-development-environment-localhost-in-minutes.html) in your `~/.localhost-ssl` directory and name the files `localhost.pem` and `localhost.key`. To use the certificate:
-```
-$ npm run start:secure
-```
+Each folder has its own readme:
+- [functions-v2](functions-v2/README.md)
+- [functions-v1](functions-v1/README.md)
 
 ## Testing/Deploying database rules
 

firebase.json

@@ -20,19 +20,39 @@
       "port": 5001
     }
   },
-  "functions": {
-    "predeploy": [
-      "npm --prefix \"$RESOURCE_DIR\" run lint",
-      "npm --prefix \"$RESOURCE_DIR\" run build"
-    ],
-    "source": "functions",
-    "ignore": [
-      "*.log",
-      ".*",
-      ".git",
-      "coverage",
-      "node_modules",
-      "test"
-    ]
-  }
+  "functions": [
+    {
+      "source": "functions-v1",
+      "codebase": "functions-v1",
+      "predeploy": [
+        "npm --prefix \"$RESOURCE_DIR\" run lint",
+        "npm --prefix \"$RESOURCE_DIR\" run build"
+      ],
+      "ignore": [
+        "*.log",
+        ".*",
+        ".git",
+        "coverage",
+        "node_modules",
+        "test"
+      ]
+    },
+    {
+      "source": "functions-v2",
+      "codebase": "functions-v2",
+      "predeploy": [
+        "npm --prefix \"$RESOURCE_DIR\" run lint",
+        "npm --prefix \"$RESOURCE_DIR\" run build"
+      ],
+      "ignore": [
+        "node_modules",
+        ".*",
+        "firebase-debug.log",
+        "firebase-debug.*.log",
+        "*.local",
+        "*.log",
+        "coverage"
+      ]
+    }
+  ]
 }

functions-v1/README.md

@@ -0,0 +1,82 @@
+# Firebase Functions
+The functions are split into two folders `functions-v1` and `functions-v2`. This folder `functions-v1` contains the legacy functions. We are hoping to incrementally migrate these legacy functions into the newer `functions-v2`.
+
+
+## Available Functions
+
+|Function|Purpose|
+|--------|-------|
+|_getImageData_|Retrieves image data that may reside in other classes and hence is not accessible client-side, e.g. for supports published to multiple classes or documents retrieved via the teacher network.|
+|_getNetworkDocument_|Retrieves the contents of a document accessible to a teacher via the teacher network.|
+|_getNetworkResources_|Retrieves the list of resources (documents) available to a teacher via the teacher network.|
+|_postDocumentComment_|Posts a comment to a document in firestore, adding metadata for the document to firestore if necessary.|
+|_publishSupport_|Publishes a document as a support that is accessible to all of a teacher's classes (including any referenced images).|
+|_validateCommentableDocument_|Checks whether a specific commentable document exists in firestore and creates it if necessary.|
+
+Here are the basic development operations you can do after you cd into the `functions-v1` directory:
+```
+$ cd functions-v1
+$ npm install     # install local dependencies
+$ npm run lint    # lint the functions code
+$ npm run test    # runs jest (unit) tests for the functions code
+$ npm run build   # build the functions code (transpile TypeScript)
+```
+### Note 1
+There seems to be an uneasy relationship between the `node_modules` folder in the
+`functions-v1` directory and the one in the parent directory. I had to explicitly specify the
+path to typescript in the `build` function. There's probably a better configuration available,
+but in the meantime this seems to mostly work.
+
+### Note 2
+When running `npm run test` with node 16, the following error is shown
+```
+TypeError: Cannot read properties of undefined (reading 'INTERNAL')
+```
+This error is triggered by the following line in `test-utils.ts`
+```
+import { useEmulators } from "@firebase/rules-unit-testing";
+```
+The current work around is to use node 14 to run the tests.
+
+See functions/dependency-notes.md for more on this.
+
+### Testing cloud functions
+
+Google recommends (requires?) that [firebase-tools](https://www.npmjs.com/package/firebase-tools) be installed globally:
+```
+$ npm install -g firebase-tools
+```
+This should be run periodically to make sure you're running the latest version of the tools.
+
+#### Running tests locally (without running functions in the emulator)
+```
+$ npm run serve                 # build and then start the emulators
+$ npm run test                  # run all tests in `functions` directory
+$ npm run test -- some.test.ts  # run a particular test
+```
+The existing tests currently work this way. They test the basic functionality of the cloud functions by importing and calling them directly from node.js test code. This is a simple and efficient way of testing the basic functionality without all the overhead of the functions emulator. The downside is that the node.js test environment is not the same as the hosted function environment. For instance, it's possible to return objects in node.js that can't be JSON-stringified which will throw an error when the function is hosted. That said, you can't beat the convenience of simply calling the functions directly.
+
+#### Running local tests against functions hosted in the emulator
+To run jest tests against functions running in the emulator requires [serving functions using a Cloud Functions Shell](https://firebase.google.com/docs/functions/local-shell#serve_functions_using_a_cloud_functions_shell). Currently, all of our functions are `HTTPS Callable` functions, which [can be called](https://firebase.google.com/docs/functions/local-shell#invoke_https_callable_functions) in this shell mode, but:
+>Emulation of context.auth is currently unavailable.
+
+#### Running CLUE against functions running locally in the emulator:
+```
+$ npm run serve   # build and then start the functions emulator
+```
+and launch CLUE with url parameter `functions=emulator`.
+
+### To deploy firebase functions to production:
+```
+$ npm run deploy                        # deploy all functions
+$ npm run deploy:getImageData           # deploy individual function
+$ npm run deploy:postDocumentComment    # deploy individual function
+```
+
+By convention, our firebase functions have an internal version number that is returned with any results. This should be incremented appropriately when new versions are deployed. This will allow us to determine whether the current code in GitHub has been deployed or not, for instance. Also by convention, our firebase functions accept parameters of `{ warmUp: true }` which can be issued in advance of any actual call to mitigate the google cloud function cold-start issue.
+
+### Serving CLUE from https://localhost
+To test the deployed function(s) from your local development environment, you may need to run your local dev server with https to avoid CORS errors. To do so, [create a certificate](https://www.matthewhoelter.com/2019/10/21/how-to-setup-https-on-your-local-development-environment-localhost-in-minutes.html) in your `~/.localhost-ssl` directory and name the files `localhost.pem` and `localhost.key`. To use the certificate:
+```
+$ npm run start:secure
+```

functions/package.json → functions-v1/package.json

@@ -1,5 +1,5 @@
 {
-  "name": "functions",
+  "name": "functions-v1",
   "scripts": {
     "lint": "eslint \"src/**/*\" \"test/**/*\"",
     "build": "npm-run-all clean build:prod",
@@ -10,7 +10,7 @@
     "serve:functions": "npm run build && firebase emulators:start --only functions",
     "shell": "npm run build && firebase functions:shell",
     "start": "npm run shell",
-    "deploy:all": "firebase deploy --only functions",
+    "deploy:all": "firebase deploy --only functions:functions-v1",
     "deploy:getImageData": "firebase deploy --only functions:getImageData_v1",
     "deploy:getNetworkDocument": "firebase deploy --only functions:getNetworkDocument_v1",
     "deploy:getNetworkResources": "firebase deploy --only functions:getNetworkResources_v1",

functions/src/canonicalize-url.ts → functions-v1/src/canonicalize-url.ts

@@ -1,5 +1,5 @@
 import * as admin from "firebase-admin";
-import { buildFirebaseImageUrl, parseFirebaseImageUrl } from "./shared-utils";
+import { buildFirebaseImageUrl, parseFirebaseImageUrl } from "../../shared/shared-utils";
 
 export async function canonicalizeUrl(url: string, defaultClassHash: string, firestoreRoot: string) {
   const { imageClassHash, imageKey } = parseFirebaseImageUrl(url);

functions/src/get-image-data.ts → functions-v1/src/get-image-data.ts

@@ -1,6 +1,6 @@
 import * as admin from "firebase-admin";
 import * as functions from "firebase-functions";
-import { IGetImageDataUnionParams, isWarmUpParams } from "./shared";
+import { IGetImageDataUnionParams, isWarmUpParams } from "../../shared/shared";
 import { validateUserContext } from "./user-context";
 
 // update this when deploying updates to this function

functions/src/get-network-document.ts → functions-v1/src/get-network-document.ts

@@ -2,7 +2,7 @@ import * as admin from "firebase-admin";
 import * as functions from "firebase-functions";
 import { canonicalizeUrl } from "./canonicalize-url";
 import { parseDocumentContent } from "./parse-document-content";
-import { IGetNetworkDocumentUnionParams, isWarmUpParams } from "./shared";
+import { IGetNetworkDocumentUnionParams, isWarmUpParams } from "../../shared/shared";
 import { validateUserContext } from "./user-context";
 
 // update this when deploying updates to this function

functions/src/get-network-resources.ts → functions-v1/src/get-network-resources.ts

@@ -3,7 +3,7 @@ import * as functions from "firebase-functions";
 import {
   IGetNetworkResourcesUnionParams, INetworkResourceClassResponse, INetworkResourceOfferingResponse,
   INetworkResourceTeacherClassResponse, INetworkResourceTeacherOfferingResponse, isWarmUpParams
-} from "./shared";
+} from "../../shared/shared";
 import { validateUserContext } from "./user-context";
 
 // update this when deploying updates to this function

functions/src/parse-document-content.ts → functions-v1/src/parse-document-content.ts

@@ -1,5 +1,5 @@
-import { IDocumentContent } from "./shared";
-import { matchAll, parseFirebaseImageUrl, replaceAll, safeJsonParse } from "./shared-utils";
+import { IDocumentContent } from "../../shared/shared";
+import { matchAll, parseFirebaseImageUrl, replaceAll, safeJsonParse } from "../../shared/shared-utils";
 
 // regular expression for identifying firebase image urls in document content
 // In some tile state the image URLS are inside of a double escaped JSON. This means

functions/src/post-document-comment.ts → functions-v1/src/post-document-comment.ts

@@ -2,7 +2,7 @@ import * as admin from "firebase-admin";
 import * as functions from "firebase-functions";
 import {
   IPostDocumentCommentUnionParams, isCurriculumMetadata, isDocumentMetadata, isWarmUpParams
-} from "./shared";
+} from "../../shared/shared";
 import { validateUserContext } from "./user-context";
 import { createCommentableDocumentIfNecessary } from "./validate-commentable-document";
 

functions/src/publish-support.ts → functions-v1/src/publish-support.ts

@@ -2,8 +2,8 @@ import * as admin from "firebase-admin";
 import * as functions from "firebase-functions";
 import { canonicalizeUrl } from "./canonicalize-url";
 import { parseDocumentContent } from "./parse-document-content";
-import { IPublishSupportUnionParams, isWarmUpParams } from "./shared";
-import { parseFirebaseImageUrl } from "./shared-utils";
+import { IPublishSupportUnionParams, isWarmUpParams } from "../../shared/shared";
+import { parseFirebaseImageUrl } from "../../shared/shared-utils";
 import { validateUserContext } from "./user-context";
 
 // update this when deploying updates to this function

functions/src/user-context.ts → functions-v1/src/user-context.ts

@@ -1,5 +1,5 @@
 import { AuthData } from "firebase-functions/lib/common/providers/https";
-import { escapeKey, IUserContext } from "./shared";
+import { escapeKey, IUserContext } from "../../shared/shared";
 
 export interface IValidatedUserContext {
   isValid: boolean;

functions/src/validate-commentable-document.ts → functions-v1/src/validate-commentable-document.ts

@@ -3,7 +3,7 @@ import * as functions from "firebase-functions";
 import {
   ICommentableDocumentParams, ICommentableDocumentUnionParams, isCurriculumMetadata, isDocumentMetadata,
   isWarmUpParams, networkDocumentKey
-} from "./shared";
+} from "../../shared/shared";
 import { validateUserContext } from "./user-context";
 
 // update this when deploying updates to this function

functions/test/canonicalize-url.test.ts → functions-v1/test/canonicalize-url.test.ts

@@ -1,5 +1,5 @@
 import { canonicalizeUrl } from "../src/canonicalize-url";
-import { buildFirebaseImageUrl } from "../src/shared-utils";
+import { buildFirebaseImageUrl } from "../../shared/shared-utils";
 
 describe("canonicalizeUrl", () => {
   it("should simply return invalid urls", async () => {

functions/test/get-image-data.test.ts → functions-v1/test/get-image-data.test.ts

@@ -1,6 +1,6 @@
 import { apps, clearFirestoreData, initializeAdminApp } from "@firebase/rules-unit-testing";
 import { getImageData } from "../src/get-image-data";
-import { IGetImageDataParams, IUserContext } from "../src/shared";
+import { IGetImageDataParams, IUserContext } from "../../shared/shared";
 import { validateUserContext } from "../src/user-context";
 import {
   configEmulators,

functions/test/get-network-document.test.ts → functions-v1/test/get-network-document.test.ts

@@ -1,8 +1,8 @@
 import {
   apps, clearFirestoreData, initializeAdminApp} from "@firebase/rules-unit-testing";
 import { getNetworkDocument } from "../src/get-network-document";
-import { IGetNetworkDocumentParams } from "../src/shared";
-import { buildFirebaseImageUrl, parseFirebaseImageUrl } from "../src/shared-utils";
+import { IGetNetworkDocumentParams } from "../../shared/shared";
+import { buildFirebaseImageUrl, parseFirebaseImageUrl } from "../../shared/shared-utils";
 import { validateUserContext } from "../src/user-context";
 import {
   configEmulators,

functions/test/get-network-resources.test.ts → functions-v1/test/get-network-resources.test.ts

@@ -1,7 +1,7 @@
 import {
   apps, clearFirestoreData, initializeAdminApp} from "@firebase/rules-unit-testing";
 import { getNetworkResources } from "../src/get-network-resources";
-import { IGetNetworkResourcesParams } from "../src/shared";
+import { IGetNetworkResourcesParams } from "../../shared/shared";
 import {
   configEmulators, kClassHash, kOffering1Id, kOffering2Id, kOtherClassHash, kProblemPath,
   kTeacherName, kTeacherNetwork, kUserId, specAuth, specUserContext

functions/test/parse-document-content.test.ts → functions-v1/test/parse-document-content.test.ts

@@ -1,5 +1,5 @@
 import { parseDocumentContent } from "../src/parse-document-content";
-import { buildFirebaseImageUrl, parseFirebaseImageUrl, replaceAll } from "../src/shared-utils";
+import { buildFirebaseImageUrl, parseFirebaseImageUrl, replaceAll } from "../../shared/shared-utils";
 import { specDocumentContent } from "./test-utils";
 import sharedDatasetExample from "./shared-dataset-example";
 

functions/test/post-document-comment.test.ts → functions-v1/test/post-document-comment.test.ts

@@ -4,7 +4,7 @@ import { postDocumentComment } from "../src/post-document-comment";
 import {
   ICurriculumMetadata, IDocumentMetadata, IPostDocumentCommentParams, isCurriculumMetadata,
   IUserContext, networkDocumentKey
-} from "../src/shared";
+} from "../../shared/shared";
 import {
   configEmulators, kCanonicalPortal, kCurriculumKey, kDemoName, kDocumentKey, kDocumentType, kFirebaseUserId,
   kTeacherName, kTeacherNetwork, kUserId, specAuth, specUserContext

functions/test/publish-support.test.ts → functions-v1/test/publish-support.test.ts

@@ -1,7 +1,7 @@
 import { apps, clearFirestoreData, initializeAdminApp } from "@firebase/rules-unit-testing";
 import { publishSupport } from "../src/publish-support";
-import { IPublishSupportParams } from "../src/shared";
-import { buildFirebaseImageUrl, parseFirebaseImageUrl, replaceAll } from "../src/shared-utils";
+import { IPublishSupportParams } from "../../shared/shared";
+import { buildFirebaseImageUrl, parseFirebaseImageUrl, replaceAll } from "../../shared/shared-utils";
 import {
   configEmulators,
   kCanonicalPortal, kClassHash, kOtherClassHash, kPortal, kTeacherNetwork,