feat: AI-categorized docs show in strategy sorts (PT-188476611)

[#188476611](https://www.pivotaltracker.com/story/show/188476611)

functions-v2/.gitignore

@@ -1,3 +1,7 @@
+# Local storage for Firebase Emulator Suite
+emulator-data/*
+!emulator-data/.gitkeep
+
 # Compiled JavaScript files
 lib/**/*.js
 lib/**/*.js.map

functions-v2/README.md

@@ -11,6 +11,7 @@ The functions are split into two folders `functions-v1` and `functions-v2`. This
 |_onAnalysisDocumentPending_|Monitors the queue for documents to analyze, and sends them to Shutterbug to create a screenshot of the document|
 |_onAnalysisDocumentImaged_|Sends new screenshots to ChatGPT for analysis and creates a comment on the original document|
 |_atMidnight_|Clears old Firebase roots for dev and qa instances|
+|_onDocumentTagged_|Updates metadata documents with strategies as needed whenever a comment is made|
 
 
 ## Operations
@@ -58,6 +59,12 @@ npm run emulator
 
 This will load the built function code into the emulator. The only function we have so far is one that monitors Firestore docs for changes. So with the function running in the emulator you can manually change some docs and see if the function responds correctly.
 
+To persist state between emulator runs, `npm run emulator` loads data from `functions-v2/emulator-data` when it starts, and saves data to the same location when it stops. It does this using the `--import` and `--export-on-exit` flags like so:
+
+```
+firebase emulators:start --project demo-test --import=./emulator-data --export-on-exit=./emulator-data
+```
+
 ## To deploy firebase functions to production:
 
 ```shell

functions-v2/package.json

@@ -4,7 +4,7 @@
     "lint": "eslint --ext .js,.ts .",
     "build": "tsc",
     "build:watch": "tsc --watch",
-    "emulator": "firebase emulators:start --project demo-test",
+    "emulator": "firebase emulators:start --project demo-test --import=./emulator-data --export-on-exit=./emulator-data",
     "emulator:online": "firebase emulators:start",
     "serve": "npm run build && firebase emulators:start --only functions",
     "shell": "npm run build && firebase functions:shell --project demo-test",

functions-v2/src/index.ts

@@ -1,5 +1,6 @@
 import * as admin from "firebase-admin";
 export {onUserDocWritten} from "./on-user-doc-written";
+export {onDocumentTagged} from "./on-document-tagged";
 export {atMidnight} from "./at-midnight";
 export {onAnalyzableTestDocWritten, onAnalyzableProdDocWritten} from "./on-analyzable-doc-written";
 export {onAnalysisDocumentPending} from "./on-analysis-document-pending";

functions-v2/src/on-document-tagged.ts

@@ -0,0 +1,55 @@
+import {FirestoreEvent, onDocumentWritten} from "firebase-functions/v2/firestore";
+import {Change, DocumentSnapshot} from "firebase-functions/lib/v2/providers/firestore";
+import * as admin from "firebase-admin";
+
+/*
+ * onDocumentTagged
+ * Listens for changes to comments and updates the strategies array in the commented-on document's metadata.
+*/
+export const onDocumentTagged = onDocumentWritten(
+  "{root}/{space}/documents/{documentId}/comments/{commentId}",
+  async (event: FirestoreEvent<Change<DocumentSnapshot> | undefined>) => {
+    if (!event.data) return;
+
+    const {root, space, documentId} = event.params;
+    const firestore = admin.firestore();
+    const docKey = await firestore.collection(`${root}/${space}/documents`).doc(documentId).get().then((doc) => {
+      return doc.data()?.key;
+    });
+    if (!docKey) return;
+
+    const collectionPath = `${root}/${space}/documents`;
+    const documentCollection = admin.firestore().collection(collectionPath);
+    const documentSnapshots = await documentCollection.where("key", "==", docKey).get();
+    const strategies: string[] = [];
+
+    // Get all tags from all comments on the document and build an array of unique strategy values from them.
+    for (const _documentSnapshot of documentSnapshots.docs) {
+      const commentsUrl = `${_documentSnapshot.ref.path}/comments`;
+      const commentCollection = admin.firestore().collection(commentsUrl);
+      const commentSnapshots = await commentCollection.get();
+
+      for (const _commentSnapshot of commentSnapshots.docs) {
+        const commentTags = _commentSnapshot.data()?.tags ?? [];
+
+        if (commentTags != null && !Array.isArray(commentTags)) {
+          console.warn("Found invalid comment tags", _commentSnapshot.ref.path, commentTags);
+          continue;
+        }
+
+        commentTags.forEach((tag: string) => {
+          if (tag && !strategies.includes(tag)) {
+            strategies.push(tag);
+          }
+        });
+      }
+    }
+
+    const metadataQuery = documentCollection.where("key", "==", docKey);
+    const querySnapshot = await metadataQuery.get();
+
+    for (const doc of querySnapshot.docs) {
+      const docRef = doc.ref;
+      await docRef.update({strategies: [...strategies]});
+    }
+  });

functions-v2/test/on-document-tagged.test.ts

@@ -0,0 +1,122 @@
+import * as admin from "firebase-admin";
+import {clearFirestoreData} from "firebase-functions-test/lib/providers/firestore";
+import {onDocumentTagged} from "../src/on-document-tagged";
+import {initialize, projectConfig} from "./initialize";
+
+type CollectionRef = admin.firestore.CollectionReference<admin.firestore.DocumentData, admin.firestore.DocumentData>;
+
+const {fft, cleanup} = initialize();
+
+describe("onDocumentTagged", () => {
+  let documentCollection: CollectionRef;
+
+  beforeEach(async () => {
+    // The three lines below seemed to help prevent an "open handle" warning after the tests ran. They don't
+    // actually solve the problem of Jest not exiting cleanly, but they do suppress the warning and so may be
+    // helpful in further attempts to fix the problem.
+    // const documents = await admin.firestore().collection("demo/test/documents").listDocuments();
+    // const deletePromises = documents.map((doc) => doc.delete());
+    // await Promise.all(deletePromises);
+
+    await clearFirestoreData(projectConfig);
+
+    documentCollection = admin.firestore().collection("demo/test/documents");
+    await documentCollection.doc("1234").set({
+      key: "doc-key",
+      strategies: [],
+    });
+  });
+
+  test("should add new values to a document's strategies array when a new comment is made", async () => {
+    const wrapped = fft.wrap(onDocumentTagged);
+    const commentRef = documentCollection.doc("1234").collection("comments").doc("5678");
+
+    await commentRef.set({tags: ["tag1", "tag2"]});
+    const event = {
+      params: {
+        root: "demo",
+        space: "test",
+        documentId: "1234",
+        commentId: "5678",
+      },
+    };
+    await wrapped(event);
+
+    const docSnapshot = await documentCollection.doc("1234").get();
+    const docData = docSnapshot.data();
+
+    expect(docData).toEqual({
+      key: "doc-key",
+      strategies: ["tag1", "tag2"],
+    });
+  });
+
+  test("should remove values from a document's strategies array when an existing comment is deleted", async () => {
+    const wrapped = fft.wrap(onDocumentTagged);
+    const commentRef1 = documentCollection.doc("1234").collection("comments").doc("5678");
+    const commentRef2 = documentCollection.doc("1234").collection("comments").doc("9012");
+
+    // Add a comment with one tag.
+    await commentRef1.set({tags: ["tag5"]});
+    const event1 = {
+      params: {
+        root: "demo",
+        space: "test",
+        documentId: "1234",
+        commentId: "5678",
+      },
+    };
+    await wrapped(event1);
+
+    let docSnapshot = await documentCollection.doc("1234").get();
+    let docData = docSnapshot.data();
+    expect(docData).toEqual({
+      key: "doc-key",
+      strategies: ["tag5"],
+    });
+
+    // Add a second comment with one redundant tag and one new tag.
+    await commentRef2.set({tags: ["tag5", "tag6"]});
+    const event2 = {
+      params: {
+        root: "demo",
+        space: "test",
+        documentId: "1234",
+        commentId: "9012",
+      },
+    };
+    await wrapped(event2);
+
+    docSnapshot = await documentCollection.doc("1234").get();
+    docData = docSnapshot.data();
+    expect(docData).toEqual({
+      key: "doc-key",
+      strategies: ["tag5", "tag6"],
+    });
+
+    // Delete the second comment.
+    await commentRef2.delete();
+    const event3 = {
+      params: {
+        root: "demo",
+        space: "test",
+        documentId: "1234",
+        commentId: "9012",
+      },
+    };
+    await wrapped(event3);
+
+    // Verify that only the redundant tag remains in the strategies array.
+    docSnapshot = await documentCollection.doc("1234").get();
+    docData = docSnapshot.data();
+
+    expect(docData).toEqual({
+      key: "doc-key",
+      strategies: ["tag5"],
+    });
+  });
+
+  afterAll(async () => {
+    await cleanup();
+  });
+});

src/hooks/document-comment-hooks.ts

@@ -109,7 +109,6 @@ type PostDocumentCommentUseMutationOptions =
 
 export const usePostDocumentComment = (options?: PostDocumentCommentUseMutationOptions) => {
   const queryClient = useQueryClient();
-  const [firestore] = useFirestore();
   const postDocumentComment = useFirebaseFunction<IPostDocumentCommentParams>("postDocumentComment_v1");
   const context = useUserContext();
   const postComment = useCallback((clientParams: IPostDocumentCommentClientParams) => {
@@ -122,39 +121,6 @@ export const usePostDocumentComment = (options?: PostDocumentCommentUseMutationO
       const { document, comment } = newCommentParams;
       const queryKey = getCommentsQueryKeyFromMetadata(document);
 
-      // update metadata document with the new tags
-      const tags = comment.tags || [];
-      const documentKey = isDocumentMetadata(document) ? document.key : undefined;
-      if (documentKey && tags.length > 0) {
-        // Just the document key is not enough for Firestore to know that we have permission
-        // to access the document. Providing a context_id gives the rules enough info to make
-        // sure we have access to the returned docs. The `context.classHash` used here is
-        // the current class that CLUE has been run in. However, a teacher might be commenting
-        // on a document from a different class. In that case the code below will not find
-        // the documents that need to have their strategies updated.
-        // Instead we should be using the context_id of the document that we are commenting on.
-        // CLUE tries to track the context_id when it loads a remote document. That
-        // information should be stored in the DocumentModel#remoteContext field. We don't
-        // have access to the DocumentModel here though, just document.metadata.
-        // FIXME: provide access to remoteContext here so we can update strategies on remote
-        // documents. Alternatively move this into a firebase function instead of doing this
-        // in the client.
-        // FIXME: in the case of exemplar documents, the metadata document won't exist
-        // until this mutation happens. That probably means metadataQuery.get() below
-        // will run before the document has been created so it will return an empty array of
-        // docs. This is another reason the firebase function approach to keep the strategies
-        // updated is a better way to go
-        const metadataQuery = firestore.collection("documents")
-          .where("key", "==", documentKey)
-          .where("context_id", "==", context.classHash);
-        metadataQuery.get().then(querySnapshot => {
-          querySnapshot.docs.forEach(doc => {
-            const docRef = doc.ref;
-            docRef.update({ strategies: firebase.firestore.FieldValue.arrayUnion(...tags) });
-          });
-        });
-      }
-
       // snapshot the current state of the comments in case we need to roll back on error
       const rollbackComments = queryKey && queryClient.getQueryData<CommentWithId[]>(queryKey);
       type CommentWithId = WithId<CommentDocument>;