Harden assignment sync to prevent disappearing assignments

functions/levante-admin/README_DISAPPEARING_ASSIGNMENTS.md

@@ -0,0 +1,67 @@
+# Fix disappearing assignments
+
+## Issue summary
+
+Some assignment creation flows appeared to succeed in the UI, but one or more user assignment documents were missing under:
+
+- `users/{uid}/assignments/{administrationId}`
+
+This was reported as "disappearing assignments".
+
+## What we found
+
+Two different behaviors were mixed together:
+
+1. Expected non-assignment due to conditions
+   - Some administrations intentionally assign only users that match an `assigned` condition (for example `userType == student`).
+   - In those cases, parent/teacher users are correctly not assigned.
+
+2. Real backend risk in async fan-out
+   - Assignment fan-out runs asynchronously in `updateAssignmentsForOrgChunk`.
+   - If the payload includes undefined values or malformed org arrays, Firestore writes can fail and prevent some assignment docs from being written.
+
+## Why this can look like a UI success
+
+Administration creation and assignment fan-out are separate phases:
+
+1. administration doc is created/updated
+2. fan-out tasks are enqueued and processed later
+
+If phase 1 succeeds and phase 2 partially fails, users can see successful creation while assignment docs are incomplete.
+
+## Changes in this fix
+
+### 1) Harden upsert payloads (`src/upsertAdministration.ts`)
+
+- Normalize org ID arrays (`districts`, `schools`, `classes`, `groups`) to non-empty string IDs only.
+- Strip undefined recursively from assessments and legal fields.
+- Normalize assessment `params` to an object.
+- Validate required assessment fields (`taskId`, `variantId`, `variantName`).
+- Reject invalid dates explicitly.
+- Ensure update payload preserves a valid `createdBy` value.
+
+These changes reduce malformed data entering downstream fan-out.
+
+### 2) Harden assignment writes (`src/assignments/assignment-utils.ts`)
+
+- Use cleaned assessments when building assignment docs.
+- Sanitize assignment documents before `transaction.set(...)` in both add and update flows.
+
+This prevents transaction failures caused by nested undefined fields.
+
+### 3) Improve recursive sanitizer (`src/utils/utils.ts`)
+
+- `removeUndefinedFields()` now recursively cleans nested arrays, not only top-level array items.
+
+This closes a gap where undefined values inside array objects could survive and break Firestore writes.
+
+## Why this fixes disappearing assignments
+
+The core failure mode was bad shape/undefined data reaching Firestore write paths in async fan-out.
+By normalizing and validating input early and sanitizing assignment payloads immediately before write, task execution is significantly less likely to fail partway through chunk processing.
+
+## Operational recommendation
+
+- Monitor `updateAssignmentsForOrgChunk` error rates and alert on repeated payload-shape failures.
+- Keep an audit test that compares expected eligible users against actual assignment docs.
+

functions/levante-admin/src/assignments/assignment-utils.ts

@@ -244,7 +244,7 @@ const prepareNewAssignment = async (
       sequential: administrationData.sequential ?? false,
       assigningOrgs: usersAssigningOrgs,
       readOrgs: userReadOrgs,
-      assessments,
+      assessments: cleanedAssessments,
       progress,
       userData: userDataCopy,
       testData: administrationData.testData ?? false,
@@ -287,10 +287,13 @@ export const addAssignmentToUsers = async (
 
   return _map(assignments, ([assignmentRef, assignmentData]) => {
     if (assignmentRef && assignmentData) {
+      const cleanedAssignmentData = removeUndefinedFields(assignmentData);
       logger.debug(`Adding new assignment at ${assignmentRef.path}`, {
-        assignmentSummary: summarizeAssignmentForLog(assignmentData),
+        assignmentSummary: summarizeAssignmentForLog(cleanedAssignmentData),
+      });
+      return transaction.set(assignmentRef, cleanedAssignmentData, {
+        merge: true,
       });
-      return transaction.set(assignmentRef, assignmentData, { merge: true });
     } else {
       return transaction;
     }
@@ -668,10 +671,13 @@ export const updateAssignmentForUsers = async (
 
   return _map(assignments, ([assignmentRef, assignmentData]) => {
     if (assignmentRef && assignmentData) {
+      const cleanedAssignmentData = removeUndefinedFields(assignmentData);
       logger.info(`Updating or creating assignment ${assignmentRef.path}`, {
-        assignmentSummary: summarizeAssignmentForLog(assignmentData),
+        assignmentSummary: summarizeAssignmentForLog(cleanedAssignmentData),
+      });
+      return transaction.set(assignmentRef, cleanedAssignmentData, {
+        merge: true,
       });
-      return transaction.set(assignmentRef, assignmentData, { merge: true });
     } else if (assignmentRef) {
       return transaction.delete(assignmentRef);
     } else {

functions/levante-admin/src/upsertAdministration.ts

@@ -8,6 +8,7 @@ import { HttpsError } from "firebase-functions/v2/https";
 import { logger } from "firebase-functions/v2";
 import type { IAssessment, IOrgsList } from "./interfaces.js"; // Assuming necessary types/helpers are in common
 import type { Class, Group, School } from "../firestore-schema.js";
+import { removeUndefinedFields } from "./utils/utils.js";
 
 interface UpsertAdministrationData {
   name: string;
@@ -51,6 +52,39 @@ interface IAdministrationDoc {
   creatorName: string;
 }
 
+const normalizeIdList = (ids: unknown): string[] => {
+  if (!Array.isArray(ids)) return [];
+  return [...new Set(ids)]
+    .filter((id): id is string => typeof id === "string")
+    .map((id) => id.trim())
+    .filter((id) => id.length > 0);
+};
+
+const normalizeOrgs = (orgs?: IOrgsList): Required<IOrgsList> => {
+  return {
+    districts: normalizeIdList(orgs?.districts),
+    schools: normalizeIdList(orgs?.schools),
+    classes: normalizeIdList(orgs?.classes),
+    groups: normalizeIdList(orgs?.groups),
+  };
+};
+
+const normalizeAssessments = (assessments: IAssessment[]): IAssessment[] => {
+  return assessments.map((assessment) => {
+    const cleanedAssessment = removeUndefinedFields(assessment) as IAssessment;
+    const params =
+      cleanedAssessment.params &&
+      typeof cleanedAssessment.params === "object" &&
+      !Array.isArray(cleanedAssessment.params)
+        ? cleanedAssessment.params
+        : {};
+    return {
+      ...cleanedAssessment,
+      params,
+    };
+  });
+};
+
 export const upsertAdministrationHandler = async (
   callerAdminUid: string,
   data: UpsertAdministrationData
@@ -67,23 +101,27 @@ export const upsertAdministrationHandler = async (
     dateOpen,
     dateClose,
     sequential = true,
-    orgs = {
-      districts: [],
-      schools: [],
-      classes: [],
-      groups: [],
-    },
+    orgs,
     tags = [],
     administrationId,
     isTestData = false,
     legal,
     creatorName,
   } = data as UpsertAdministrationData;
 
+  const normalizedOrgs = normalizeOrgs(orgs);
+  const rawAssessments = Array.isArray(assessments) ? assessments : [];
+  const cleanedAssessments = normalizeAssessments(
+    removeUndefinedFields(rawAssessments) as IAssessment[]
+  );
+  const cleanedLegal = removeUndefinedFields(legal ?? {});
+  const cleanedTags = normalizeIdList(tags);
+
   if (
     !name ||
-    !assessments ||
-    !Array.isArray(assessments) ||
+    !cleanedAssessments ||
+    !Array.isArray(cleanedAssessments) ||
+    cleanedAssessments.length === 0 ||
     !dateOpen ||
     !dateClose
   ) {
@@ -93,11 +131,33 @@ export const upsertAdministrationHandler = async (
     );
   }
 
+  const hasInvalidAssessment = cleanedAssessments.some(
+    (assessment) =>
+      typeof assessment.taskId !== "string" ||
+      !assessment.taskId.trim() ||
+      typeof assessment.variantId !== "string" ||
+      !assessment.variantId.trim() ||
+      typeof assessment.variantName !== "string" ||
+      !assessment.variantName.trim()
+  );
+  if (hasInvalidAssessment) {
+    throw new HttpsError(
+      "invalid-argument",
+      "Assessments must include taskId, variantId, and variantName."
+    );
+  }
+
   let dateOpenedTs: Timestamp;
   let dateClosedTs: Timestamp;
   try {
     const dateOpenObj = new Date(dateOpen);
     const dateCloseObj = new Date(dateClose);
+    if (
+      Number.isNaN(dateOpenObj.getTime()) ||
+      Number.isNaN(dateCloseObj.getTime())
+    ) {
+      throw new Error("Invalid date");
+    }
 
     dateOpenedTs = Timestamp.fromDate(dateOpenObj);
     dateClosedTs = Timestamp.fromDate(dateCloseObj);
@@ -134,43 +194,44 @@ export const upsertAdministrationHandler = async (
             `Administration with ID ${administrationId} not found for update.`
           );
         }
+        const existingData = existingDoc.data() as Partial<IAdministrationDoc>;
 
         // Prepare data for update (merge: true will handle partial updates)
-        const updateData: Partial<IAdministrationDoc> = {
+        const updateData: Partial<IAdministrationDoc> = removeUndefinedFields({
           // Use Partial for updates
           name,
           publicName: publicName ?? name,
           normalizedName,
-          // createdBy should not be updated
-          groups: orgs.groups ?? [],
-          classes: orgs.classes ?? [],
-          schools: orgs.schools ?? [],
-          districts: orgs.districts ?? [],
+          createdBy: existingData.createdBy ?? callerAdminUid,
+          groups: normalizedOrgs.groups,
+          classes: normalizedOrgs.classes,
+          schools: normalizedOrgs.schools,
+          districts: normalizedOrgs.districts,
           // dateCreated should not be updated
           dateOpened: dateOpenedTs,
           dateClosed: dateClosedTs,
-          assessments: assessments,
+          assessments: cleanedAssessments,
           sequential: sequential,
-          tags: tags,
-          legal: legal,
+          tags: cleanedTags,
+          legal: cleanedLegal,
           testData: isTestData ?? false,
           // Explicitly construct org lists for update
           readOrgs: {
             // Re-enabled
-            districts: orgs.districts ?? [],
-            schools: orgs.schools ?? [],
-            classes: orgs.classes ?? [],
-            groups: orgs.groups ?? [],
+            districts: normalizedOrgs.districts,
+            schools: normalizedOrgs.schools,
+            classes: normalizedOrgs.classes,
+            groups: normalizedOrgs.groups,
           },
           minimalOrgs: {
             // Re-enabled
-            districts: orgs.districts ?? [],
-            schools: orgs.schools ?? [],
-            classes: orgs.classes ?? [],
-            groups: orgs.groups ?? [],
+            districts: normalizedOrgs.districts,
+            schools: normalizedOrgs.schools,
+            classes: normalizedOrgs.classes,
+            groups: normalizedOrgs.groups,
           },
           updatedAt: FieldValue.serverTimestamp() as Timestamp,
-        };
+        });
 
         // --- Write 1 (Update Path) --- Update administration doc using transaction.update()
         transaction.update(administrationDocRef, updateData); // Switched from set with merge to update
@@ -186,30 +247,30 @@ export const upsertAdministrationHandler = async (
         const siteId = data.siteId;
 
         // Prepare Administration Data for creation
-        const administrationData: IAdministrationDoc = {
+        const administrationData: IAdministrationDoc = removeUndefinedFields({
           name,
           publicName: publicName ?? name,
           normalizedName,
           createdBy: callerAdminUid,
-          creatorName: creatorName,
-          groups: orgs.groups ?? [],
-          classes: orgs.classes ?? [],
-          schools: orgs.schools ?? [],
-          districts: orgs.districts ?? [],
+          creatorName: creatorName ?? "",
+          groups: normalizedOrgs.groups,
+          classes: normalizedOrgs.classes,
+          schools: normalizedOrgs.schools,
+          districts: normalizedOrgs.districts,
           dateCreated: FieldValue.serverTimestamp() as Timestamp,
           dateOpened: dateOpenedTs,
           dateClosed: dateClosedTs,
-          assessments: assessments,
+          assessments: cleanedAssessments,
           sequential: sequential,
-          tags: tags,
-          legal: legal,
+          tags: cleanedTags,
+          legal: cleanedLegal,
           testData: isTestData ?? false,
-          readOrgs: orgs,
-          minimalOrgs: orgs,
+          readOrgs: normalizedOrgs,
+          minimalOrgs: normalizedOrgs,
           siteId,
           createdAt: FieldValue.serverTimestamp() as Timestamp,
           updatedAt: FieldValue.serverTimestamp() as Timestamp,
-        };
+        });
 
         // --- Write 1 (Create Path) --- Create administration doc
         transaction.set(administrationDocRef, administrationData); // Use set without merge for creation

functions/levante-admin/src/utils/utils.ts

@@ -196,7 +196,9 @@ export const doesDocExist = async (docRef, transaction) => {
  */
 export const removeUndefinedFields = (obj: any): any => {
   if (Array.isArray(obj)) {
-    return _without(obj, undefined);
+    return obj
+      .map((value) => removeUndefinedFields(value))
+      .filter((value) => value !== undefined);
   } else if (obj && typeof obj === "object") {
     return Object.entries(obj).reduce((acc, [key, value]) => {
       const cleanedValue = removeUndefinedFields(value);