Merge pull request #2403 from concord-consortium/188211348-local-stor…

…age-dev-root

refactor firebase root id computation

firestore.rules

@@ -426,10 +426,10 @@ service cloud.firestore {
       allow read, write: if isAuthed();
     }
 
-    match /dev/{userId}/{restOfPath=**} {
-      allow read: if isAuthed();
-      // users can only write to their own folders
-      allow write: if matchFirebaseUserId(userId);
+    // In the future, developers might use a random unique key instead of
+    // the firebase user id. So this rule is relaxed in order to allow that.
+    match /dev/{devId}/{restOfPath=**} {
+      allow read, write: if isAuthed();
     }
 
     match /qa/{userId}/{restOfPath=**} {

src/clue/components/clue-app-header.tsx

@@ -26,8 +26,16 @@ export const ClueAppHeaderComponent: React.FC<IProps> = observer(function ClueAp
   const { showGroup } = props;
   const { appConfig, appMode, appVersion, db, user, problem, groups, investigation, ui, unit } = useStores();
   const myGroup = showGroup ? groups.getGroupById(user.currentGroupId) : undefined;
-  const userTitle = appMode !== "authed" && appMode !== "demo"
-                      ? `Firebase UID: ${db.firebase.userId}` : undefined;
+  const getUserTitle = () => {
+    switch(appMode){
+      case "dev":
+      case "qa":
+      case "test":
+        return `Firebase UID: ${db.firebase.userId}`;
+      default:
+        return undefined;
+    }
+  };
 
   const renderPanelButtons = () => {
     const { panels, onPanelChange, current} = props;
@@ -150,7 +158,7 @@ export const ClueAppHeaderComponent: React.FC<IProps> = observer(function ClueAp
         </div>
         <div className="right">
           <div className="version">Version {appVersion}</div>
-          <div className="user teacher" title={userTitle}>
+          <div className="user teacher" title={getUserTitle()}>
             <div className="class" data-test="user-class">
               <ClassMenuContainer />
             </div>
@@ -191,7 +199,7 @@ export const ClueAppHeaderComponent: React.FC<IProps> = observer(function ClueAp
           <NetworkStatus user={user}/>
           <div className="version">Version {appVersion}</div>
           {myGroup ? renderGroup(myGroup) : null}
-          <div className="user" title={userTitle}>
+          <div className="user" title={getUserTitle()}>
             <div className="user-contents">
               <div className="name" data-test="user-name">{user.name}</div>
               <div className="class" data-test="user-class">{user.className}</div>

src/lib/db.ts

@@ -40,6 +40,7 @@ import { urlParams } from "../utilities/url-params";
 import { firebaseConfig } from "./firebase-config";
 import { UserModelType } from "../models/stores/user";
 import { logExemplarDocumentEvent } from "../models/document/log-exemplar-document-event";
+import { AppMode } from "../models/stores/store-types";
 import { DEBUG_FIRESTORE } from "./debug";
 
 export type IDBConnectOptions = IDBAuthConnectOptions | IDBNonAuthConnectOptions;
@@ -55,7 +56,7 @@ export interface IDBAuthConnectOptions extends IDBBaseConnectOptions {
   rawFirebaseJWT: string;
 }
 export interface IDBNonAuthConnectOptions extends IDBBaseConnectOptions {
-  appMode: "dev" | "test" | "demo" | "qa";
+  appMode: Exclude<AppMode, "authed">;
 }
 export interface UserGroupMap {
   [key: string]: {

src/lib/firebase.test.ts

@@ -0,0 +1,46 @@
+import { DB } from "./db";
+import { Firebase } from "./firebase";
+
+const mockStores = {
+  appMode: "authed",
+  demo: { name: "demo" },
+  user: { portal: "test-portal" }
+};
+const mockDB = {
+  stores: mockStores
+} as DB;
+
+describe("Firebase class", () => {
+  describe("initialization", () => {
+    it("should create a valid Firebase object", () => {
+      const firebase = new Firebase(mockDB);
+      expect(firebase).toBeDefined();
+    });
+  });
+  describe("getRootFolder", () => {
+    it("should handle authed mode", () => {
+      const firebase = new Firebase(mockDB);
+      expect(firebase.getRootFolder()).toBe("/authed/portals/test-portal/");
+    });
+    describe("should handle the demo appMode", () => {
+      it("handles basic demo name", () => {
+        const stores = {...mockStores,
+          appMode: "demo", demo: { name: "test-demo" }};
+        const firebase = new Firebase({stores} as DB);
+        expect(firebase.getRootFolder()).toBe("/demo/test-demo/portals/test-portal/");
+      });
+      it("handles empty demo name", () => {
+        const stores = {...mockStores,
+          appMode: "demo", demo: { name: "" }};
+        const firebase = new Firebase({stores} as DB);
+        expect(firebase.getRootFolder()).toBe("/demo/test-portal/portals/test-portal/");
+      });
+      it("handles empty demo name and empty portal", () => {
+        const stores = {...mockStores,
+          appMode: "demo", demo: { name: "" }, user: { portal: ""}};
+        const firebase = new Firebase({stores} as DB);
+        expect(firebase.getRootFolder()).toBe("/demo/demo/portals//");
+      });
+    });
+  });
+});

src/lib/firebase.ts

@@ -8,6 +8,7 @@ import { DB } from "./db";
 import { escapeKey } from "./fire-utils";
 import { urlParams } from "../utilities/url-params";
 import { DocumentModelType } from "src/models/document/document";
+import { getRootId } from "./root-id";
 
 // Set this during database testing in combination with the urlParam testMigration=true to
 // override the top-level Firebase key regardless of mode. For example, setting this to "authed-copy"
@@ -65,21 +66,15 @@ export class Firebase {
     // dev: /dev/<firebaseUserId>/portals/localhost <as portalDomain>
     // qa: /qa/<firebaseUserId>/portals/qa <as portalDomain>
     // test: /test/<firebaseUserId>/portals/<arbitraryString as portalDomain>
-    const { appMode, demo: { name: demoName }, user } = this.db.stores;
+    const { appMode, user } = this.db.stores;
 
     const parts = [];
     if (urlParams.testMigration === "true" && FIREBASE_ROOT_OVERRIDE) {
       parts.push(FIREBASE_ROOT_OVERRIDE);
     } else {
       parts.push(`${appMode}`);
-      if ((appMode === "dev") || (appMode === "test") || (appMode === "qa")) {
-        parts.push(this.userId);
-      }
-      else if (appMode === "demo") {
-        const slug = demoName && demoName.length > 0 ? escapeKey(demoName) : "";
-        if (slug.length > 0) {
-          parts.push(slug);
-        }
+      if (appMode !== "authed") {
+        parts.push(getRootId(this.db.stores, this.userId));
       }
     }
     parts.push("portals");

src/lib/firestore.test.ts

@@ -56,8 +56,35 @@ describe("Firestore class", () => {
     it("should create a valid Firestore object", () => {
       const firestore = new Firestore(mockDB);
       expect(firestore).toBeDefined();
+    });
+  });
+
+  describe("getRootFolder", () => {
+    beforeEach(() => resetMocks());
+    it("should handle the authed appMode", () => {
+      const firestore = new Firestore(mockDB);
       expect(firestore.getRootFolder()).toBe("/authed/test-portal/");
     });
+    describe("should handle the demo appMode", () => {
+      it("handles basic demo name", () => {
+        const stores = {...mockStores,
+          appMode: "demo", demo: { name: "test-demo" }};
+        const firestore = new Firestore({stores} as DB);
+        expect(firestore.getRootFolder()).toBe("/demo/test-demo/");
+      });
+      it("handles empty demo name", () => {
+        const stores = {...mockStores,
+          appMode: "demo", demo: { name: "" }};
+        const firestore = new Firestore({stores} as DB);
+        expect(firestore.getRootFolder()).toBe("/demo/test-portal/");
+      });
+      it("handles empty demo name and empty portal", () => {
+        const stores = {...mockStores,
+          appMode: "demo", demo: { name: "" }, user: { portal: ""}};
+        const firestore = new Firestore({stores} as DB);
+        expect(firestore.getRootFolder()).toBe("/demo/demo/");
+      });
+    });
   });
 
   describe("setFirebaseUser", () => {

src/lib/firestore.ts

@@ -1,6 +1,7 @@
 import firebase from "firebase/app";
 import "firebase/firestore";
 import { DB } from "./db";
+import { getRootId } from "./root-id";
 import { escapeKey } from "./fire-utils";
 import { UserDocument } from "./firestore-schema";
 
@@ -40,25 +41,8 @@ export class Firestore {
   //
 
   public getRootFolder() {
-    const { appMode, demo: { name: demoName }, user: { portal } } = this.db.stores;
-    let rootDocId: string;
-    const escapedPortal = portal ? escapeKey(portal) : portal;
-
-    // `authed/${escapedPortalDomain}`
-    if (appMode === "authed") {
-      rootDocId = escapedPortal;
-    }
-    // `demo/${escapedDemoName}`
-    else if ((appMode === "demo") && (demoName?.length > 0)) {
-      const escapedDemoName = demoName ? escapeKey(demoName) : demoName;
-      rootDocId = escapedDemoName || escapedPortal || "demo";
-    }
-    // `${appMode}/${userId}`
-    else {  // (appMode === "dev") || (appMode === "test") || (appMode === "qa")
-      rootDocId = this.userId;
-    }
-
-    return `/${appMode}/${rootDocId}/`;
+    const { appMode } = this.db.stores;
+    return `/${appMode}/${getRootId(this.db.stores, this.userId)}/`;
   }
 
   public getFullPath(path = "") {

src/lib/root-id.ts

@@ -0,0 +1,30 @@
+import { IStores } from "../models/stores/stores";
+import { escapeKey } from "./fire-utils";
+
+type IRootDocIdStores = Pick<IStores, "appMode" | "demo" | "user">;
+
+export function getRootId(stores: IRootDocIdStores, firebaseUserId: string) {
+  const { appMode, demo: { name: demoName }, user: { portal } } = stores;
+  const escapedPortal = portal ? escapeKey(portal) : portal;
+
+  switch (appMode) {
+    case "authed": {
+      return escapedPortal;
+    }
+    case "demo": {
+      // Legacy Note: Previously if the demoName was "", the root id in the realtime database
+      // and Firestore would be different. The paths would end up being:
+      // database: /demo/portals/demo/ (root id is skipped)
+      // firestore: /demo/{firebaseUserId}/
+      // Now the root id will be the default "demo" in this case so the paths will be:
+      // database: /demo/demo/portals/demo/
+      // firestore: /demo/demo/
+      const escapedDemoName = demoName ? escapeKey(demoName) : demoName;
+      return escapedDemoName || escapedPortal || "demo";
+    }
+    // "dev", "qa", and "test"
+    default: {
+      return firebaseUserId;
+    }
+  }
+}

src/models/stores/user-context-provider.ts

@@ -11,6 +11,10 @@ export class UserContextProvider {
     this.stores = stores;
   }
 
+  /**
+   * This user context is sent to the Firebase functions so they know the context of the
+   * request.
+   */
   get userContext() {
     const appMode = this.stores.appMode;
     const { name: demoName } = this.stores.demo;