Send typed broadcast updates for survey, job, and LOI writes

[rfontanarosa]

closes #2580

Summary

• Replaces the generic, empty-payload broadcastSurveyUpdate() with a typed broadcastUpdate() sent from the survey, job, and LOI write triggers.
• Each FCM message now carries type (survey | job | loi), the affected entity's id, and the triggering event's commit time (eventTime); LOI updates also carry deleted.
• Messages keep sharing one collapse key per survey, so a burst of writes (e.g. importing LOIs) still collapses into a single wake-up.
• Newly created LOIs now have their created/lastModified serverTimestamp corrected to the actual Firestore trigger event time, replacing the client-guessed value written at import time.
• Splits the lois/{loiId} trigger: onWriteLoi is replaced by onUpdateLoi and onDeleteLoi, and onCreateLoi now announces too. Both were previously registered on the same path, so every created LOI was broadcast twice, the first time before its generated properties had been written.
• A created LOI is now announced once, after it settles: onCreateLoi stays silent when it writes the fix-up and leaves the announcement to onUpdateLoi, and announces the LOI itself when there is nothing to fix up.
• Property enrichment moves to common/loi-properties.ts, so export-geojson no longer imports it from a trigger module.

Compatibility

Verified against ground-android: FirebaseMessagingService.onMessageReceived() only reads remoteMessage.from (the topic) to decide which survey to resync, and never inspects remoteMessage.data. The added fields are additive and ignored by the current app, so existing clients keep doing a full resync exactly as before. Newer clients can use the type/id hints to sync just what changed.

[gino-m]

I see we opted to stick with hard-delete via FCM (push) rather than soft delete + pull? What happens if many entites are deleted (e.g. an entire survey)? Wouldn't clients get flooded with messages? Same question on batch create and write - is there a way to debounce the create and write update messages?

[rfontanarosa]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the Location of Interest (LOI) event handling by splitting the generic onWriteLoi trigger into specific onUpdateLoi and onDeleteLoi triggers, introducing a more granular broadcast system, and correcting audit timestamps using Firestore event times. Feedback on these changes highlights several safety improvements: handling potential null or undefined values in propertiesPbToObject to prevent runtime crashes and avoid losing empty string properties, replacing risky non-null assertions on loiPb.geometry and jobDoc.data() with safe checks, and robustly parsing eventTime in withServerTimestamp to prevent NaN values from propagating to Firestore timestamps.

[gemini-code-assist]

The propertiesPbToObject function has two issues:

1. It does not handle a null or undefined pb argument, which will cause a runtime crash (TypeError: Cannot convert undefined or null to object) when loiPb.properties is missing or empty.
2. It uses the logical OR operator (||) to fall back to numericValue. If stringValue is an empty string (""), it is falsy, so the function will incorrectly fall back to numericValue (or undefined), causing empty string properties to be lost.

Using nullish coalescing (??) and adding a defensive check for pb resolves both issues.

Suggested change

	export function propertiesPbToObject(pb: {
	[k: string]: Pb.LocationOfInterest.IProperty;
	}): Properties {
	const properties: { [k: string]: string | number } = {};
	for (const k of Object.keys(pb).sort()) {
	const v = pb[k].stringValue || pb[k].numericValue;
	if (v !== null && v !== undefined) {
	properties[k] = v;
	}
	}
	return properties;
	}
	export function propertiesPbToObject(pb?: {
	[k: string]: Pb.LocationOfInterest.IProperty;
	} | null): Properties {
	if (!pb) return {};
	const properties: { [k: string]: string | number } = {};
	for (const k of Object.keys(pb).sort()) {
	const v = pb[k].stringValue ?? pb[k].numericValue;
	if (v !== null && v !== undefined) {
	properties[k] = v;
	}
	}
	return properties;
	}

[gemini-code-assist]

Using non-null assertions (!) on loiPb.geometry and jobDoc.data() is risky. If the LOI lacks geometry or if the referenced job does not exist in the database, this will cause a runtime crash. It is safer to check for their existence and handle the missing cases gracefully.

  if (!loiPb.geometry) {
    logger.error(`loiId=${loiId} has no geometry`);
    return {};
  }
  const geometry = toGeoJsonGeometry(loiPb.geometry);

  let properties = propertiesPbToObject(loiPb.properties);

  const jobDoc = await db.fetchJob(surveyId, loiPb.jobId);
  if (!jobDoc.exists) {
    logger.error(`loiId=${loiId} referenced job ${loiPb.jobId} not found`);
    return properties;
  }
  const jobPb = toMessage(jobDoc.data()!, Pb.Job) as Pb.Job;
  const enabledIntegrationIds = new Set(
    jobPb.enabledIntegrations.map(i => i.id)
  );

[gemini-code-assist]

If eventTime is not a valid ISO date string or is malformed, Date.parse(eventTime) will return NaN. This will propagate to toTimestampPb and result in an invalid Firestore timestamp. Adding a fallback to Date.now() ensures robustness.

export function withServerTimestamp(
  auditInfo: Pb.IAuditInfo,
  eventTime: string
): Pb.AuditInfo {
  const parsedTime = Date.parse(eventTime);
  const millis = isNaN(parsedTime) ? Date.now() : parsedTime;
  return new Pb.AuditInfo({
    userId: auditInfo.userId,
    displayName: auditInfo.displayName,
    photoUrl: auditInfo.photoUrl,
    emailAddress: auditInfo.emailAddress,
    clientTimestamp: auditInfo.clientTimestamp,
    serverTimestamp: toTimestampPb(millis),
  });
}