Merge pull request #2397 from concord-consortium/188211348-put-functi…

…on-in-own-file

rename and move function to its own file

functions-v2/jest.config.js

@@ -1,6 +1,7 @@
 module.exports = {
   preset: 'ts-jest',
   testEnvironment: 'node',
+  testPathIgnorePatterns: ['lib/', 'node_modules/'],
 };
 
 // This is configured here because the clearFirebaseData function from

functions-v2/src/index.ts

@@ -1,82 +1,4 @@
-import {onDocumentWritten} from "firebase-functions/v2/firestore";
-import * as logger from "firebase-functions/logger";
 import * as admin from "firebase-admin";
+export {onUserDocWritten} from "./on-user-doc-written";
 
 admin.initializeApp();
-
-export const updateClassDocNetworksOnUserChange =
-  onDocumentWritten("{root}/{space}/users/{userId}", async (event) => {
-    const {root, space, userId} = event.params;
-
-    const classesResult = await admin.firestore()
-      .collection(`${root}/${space}/classes`)
-      .where("teachers", "array-contains", userId)
-      .get();
-
-    // For every class of this teacher update the networks.
-    // We could do something more efficient in the case where a network was
-    // added. Or in the case that networks were not changed.
-    // That can be figured out by looking at the event.data.before and
-    // event.data.after documents.
-    // However to keep the code more simple we just always do the scan
-    // of classes and teachers. This is required when a network is deleted
-    // because we need to figure out if another teacher in the class still has
-    // the deleted network.
-
-    // To optimize this we collect all of the teachers we care about
-    // and make one request for them instead of requesting the teachers for each
-    // class separately.
-
-    const teacherIdSet = new Set<string>();
-    classesResult.forEach((classDoc) => {
-      const {teachers} = classDoc.data() as {teachers: string[]};
-      if (!Array.isArray(teachers)) return;
-      teachers.forEach((id) => teacherIdSet.add(id));
-    });
-
-    const teacherIds = [...teacherIdSet];
-
-    const teacherNetworks: Record<string, string[]|undefined> = {};
-
-    // Need to use batching incase the number of teacherIds is larger than 30
-    const batchSize = 30;
-    for (let i = 0; i < teacherIds.length; i += batchSize) {
-      const batch = teacherIds.slice(i, i + batchSize);
-      const teachersResult = await admin.firestore()
-        .collection(`${root}/${space}/users`)
-        .where("uid", "in", batch)
-        .get();
-
-      teachersResult.forEach((teacherDoc) => {
-        const teacherData = teacherDoc.data();
-        teacherNetworks[teacherData.uid] = teacherData.networks;
-      });
-    }
-
-    const classUpdatePromises: Promise<unknown>[] = [];
-    classesResult.forEach((classDoc) => {
-      // Update each class with the networks of each teacher in the class
-      const {teachers, networks} = classDoc.data() as {teachers: string[], networks: string[] | undefined};
-      if (!Array.isArray(teachers)) return;
-      const classNetworks = new Set<string>();
-      teachers.forEach((teacher) => {
-        const networks = teacherNetworks[teacher];
-        if (!networks) return;
-        networks.forEach((network) => classNetworks.add(network));
-      });
-      const orderedNetworks = [...classNetworks].sort();
-      if (isArrayEqual(networks, orderedNetworks)) return;
-
-      classUpdatePromises.push(
-        classDoc.ref.update({networks: orderedNetworks})
-      );
-    });
-
-    await Promise.all(classUpdatePromises);
-
-    logger.info("User updated", event.document);
-  });
-
-function isArrayEqual(array1: string[] | undefined, array2: string[]) {
-  return array1?.length === array2.length && array1.every((value, index) => value === array2[index]);
-}

functions-v2/src/on-user-doc-written.ts

@@ -0,0 +1,79 @@
+import {onDocumentWritten} from "firebase-functions/v2/firestore";
+import * as logger from "firebase-functions/logger";
+import * as admin from "firebase-admin";
+import {isArrayEqual} from "./utils";
+
+export const onUserDocWritten =
+  onDocumentWritten("{root}/{space}/users/{userId}", async (event) => {
+    const {root, space, userId} = event.params;
+
+    const classesResult = await admin.firestore()
+      .collection(`${root}/${space}/classes`)
+      .where("teachers", "array-contains", userId)
+      .get();
+
+    // For every class of this teacher update the networks.
+    // We could do something more efficient in the case where a network was
+    // added. Or in the case that networks were not changed.
+    // That can be figured out by looking at the event.data.before and
+    // event.data.after documents.
+    // However to keep the code more simple we just always do the scan
+    // of classes and teachers. This is required when a network is deleted
+    // because we need to figure out if another teacher in the class still has
+    // the deleted network.
+
+    // To optimize this we collect all of the teachers we care about
+    // and make one request for them instead of requesting the teachers for each
+    // class separately.
+
+    const teacherIdSet = new Set<string>();
+    classesResult.forEach((classDoc) => {
+      const {teachers} = classDoc.data() as {teachers: string[]};
+      if (!Array.isArray(teachers)) return;
+      teachers.forEach((id) => teacherIdSet.add(id));
+    });
+
+    const teacherIds = [...teacherIdSet];
+
+    const teacherNetworks: Record<string, string[]|undefined> = {};
+
+    // Need to use batching incase the number of teacherIds is larger than 30
+    const batchSize = 30;
+    for (let i = 0; i < teacherIds.length; i += batchSize) {
+      const batch = teacherIds.slice(i, i + batchSize);
+      const teachersResult = await admin.firestore()
+        .collection(`${root}/${space}/users`)
+        .where("uid", "in", batch)
+        .get();
+
+      teachersResult.forEach((teacherDoc) => {
+        const teacherData = teacherDoc.data();
+        teacherNetworks[teacherData.uid] = teacherData.networks;
+      });
+    }
+
+    const classUpdatePromises: Promise<unknown>[] = [];
+    classesResult.forEach((classDoc) => {
+      // Update each class with the networks of each teacher in the class
+      const {teachers, networks} = classDoc.data() as {teachers: string[], networks: string[] | undefined};
+      if (!Array.isArray(teachers)) return;
+      const classNetworks = new Set<string>();
+      teachers.forEach((teacher) => {
+        const networks = teacherNetworks[teacher];
+        if (!networks) return;
+        networks.forEach((network) => classNetworks.add(network));
+      });
+      const orderedNetworks = [...classNetworks].sort();
+      if (isArrayEqual(networks, orderedNetworks)) return;
+
+      classUpdatePromises.push(
+        classDoc.ref.update({networks: orderedNetworks})
+      );
+    });
+
+    await Promise.all(classUpdatePromises);
+
+    logger.info("User updated", event.document);
+  });
+
+

functions-v2/src/utils.ts

@@ -0,0 +1,3 @@
+export function isArrayEqual(array1: string[] | undefined, array2: string[]) {
+  return array1?.length === array2.length && array1.every((value, index) => value === array2[index]);
+}

functions-v2/test/index.test.ts → functions-v2/test/on-user-doc-written.test.ts

@@ -4,21 +4,19 @@ import {
 } from "firebase-functions-test/lib/providers/firestore";
 import * as logger from "firebase-functions/logger";
 import * as admin from "firebase-admin";
-
-// We cannot import the function here because we need to call
-// initializeFFT first in order to set things up before the
-// initializeApp is called in the function module.
-// import {updateClassDocNetworksOnUserChange} from "../src";
+import {onUserDocWritten} from "../src/on-user-doc-written";
 
 jest.mock("firebase-functions/logger");
 
 process.env["FIRESTORE_EMULATOR_HOST"]="127.0.0.1:8088";
 const projectConfig = {projectId: "demo-test"};
 const fft = initializeFFT(projectConfig);
 
-// We can't initialize the firebase admin here because that
-// can only happen once and the function module needs to do it.
-// admin.initializeApp(projectConfig);
+// When the function is running in the cloud initializeApp is called by index.ts
+// Here we are importing the function's module directly so we can call
+// initializeApp ourselves. This is beneficial since initializeApp needs to
+// be called after initializeFFT above.
+admin.initializeApp();
 
 type CollectionRef = admin.firestore.CollectionReference<
   admin.firestore.DocumentData, admin.firestore.DocumentData
@@ -29,14 +27,14 @@ describe("functions", () => {
     await clearFirestoreData(projectConfig);
   });
 
-  describe("updateClassDocNetworksOnUserChange", () => {
+  describe("onUserDocWritten", () => {
     let classesCollection: CollectionRef;
     let usersCollection: CollectionRef;
 
-    function init() {
+    beforeEach(() => {
       classesCollection = admin.firestore().collection("demo/test/classes");
       usersCollection = admin.firestore().collection("demo/test/users");
-    }
+    });
 
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     function writeClassDocs(classDocs: any[]) {
@@ -57,17 +55,7 @@ describe("functions", () => {
     }
 
     test("add new network", async () => {
-      // The function module has to be imported after initializeFFT is called.
-      // The initializeFFT sets up environment vars so the
-      // admin.initializeApp() in index.ts will have the same project
-      // settings.
-      const {updateClassDocNetworksOnUserChange} = await import("../src");
-
-      // We can't use beforeEach because this needs to happen after the import.
-      // And we can't put the import in beforeEach because it would be hard to
-      // get the imported function typed properly.
-      init();
-      const wrapped = fft.wrap(updateClassDocNetworksOnUserChange);
+      const wrapped = fft.wrap(onUserDocWritten);
 
       const event = {
         params: {
@@ -144,9 +132,7 @@ describe("functions", () => {
     });
 
     test("remove network", async () => {
-      const {updateClassDocNetworksOnUserChange} = await import("../src");
-      init();
-      const wrapped = fft.wrap(updateClassDocNetworksOnUserChange);
+      const wrapped = fft.wrap(onUserDocWritten);
 
       const event = {
         params: {
@@ -225,9 +211,7 @@ describe("functions", () => {
     // If there is overlap between the networks of the co-teachers then removing
     // a network from one co-teacher might not change the networks of the class
     test("no network change in a class", async () => {
-      const {updateClassDocNetworksOnUserChange} = await import("../src");
-      init();
-      const wrapped = fft.wrap(updateClassDocNetworksOnUserChange);
+      const wrapped = fft.wrap(onUserDocWritten);
 
       const event = {
         params: {