refactor(formulus): remove legacy observation id mapping

formulus/src/database/repositories/WatermelonDBRepo.ts

@@ -253,20 +253,6 @@ export class WatermelonDBRepo implements LocalRepoInterface {
     });
   }
 
-  /** Resolve a row by Watermelon id or by observation_id column (legacy ingested rows - TODO: Legacy support to be removed in vNextMinor). */
-  private async findObservationRecord(
-    stableId: string,
-  ): Promise<ObservationModel | null> {
-    try {
-      return await this.observationsCollection.find(stableId);
-    } catch {
-      const rows = await this.observationsCollection
-        .query(Q.where('observation_id', stableId))
-        .fetch();
-      return rows[0] ?? null;
-    }
-  }
-
   /**
    * Get an observation by its ID
    * @param id The unique identifier for the observation
@@ -275,28 +261,8 @@ export class WatermelonDBRepo implements LocalRepoInterface {
   async getObservation(id: string): Promise<Observation | null> {
     return profileActivity.run('Database: getObservation', async () => {
       try {
-        // First try direct lookup by ID (WatermelonDB's internal ID)
-        try {
-          const observation = await this.observationsCollection.find(id);
-          return this.mapObservationModelToInterface(observation);
-        } catch {
-          // ID not found, continue to next approach
-        }
-
-        // If not found by ID, try to find by observationId field
-        // Force a database sync before querying to ensure we have the latest data
-        await this.database.get('observations').query().fetch();
-
-        const observations = await this.observationsCollection
-          .query(Q.where('observation_id', id))
-          .fetch();
-
-        if (observations.length > 0) {
-          const observation = observations[0];
-          return this.mapObservationModelToInterface(observation);
-        }
-
-        return null;
+        const observation = await this.observationsCollection.find(id);
+        return this.mapObservationModelToInterface(observation);
       } catch (error) {
         console.error(
           'Error getting observation:',
@@ -502,12 +468,9 @@ export class WatermelonDBRepo implements LocalRepoInterface {
   async updateObservation(input: UpdateObservationInput): Promise<boolean> {
     return profileActivity.run('Database: updateObservation', async () => {
       try {
-        const record = await this.findObservationRecord(input.observationId);
-
-        if (!record) {
-          console.error('Observation not found with ID:', input.observationId);
-          return false;
-        }
+        const record = await this.observationsCollection.find(
+          input.observationId,
+        );
 
         // Update the record
         let success = false;
@@ -557,12 +520,7 @@ export class WatermelonDBRepo implements LocalRepoInterface {
   async deleteObservation(id: string): Promise<boolean> {
     return profileActivity.run('Database: deleteObservation', async () => {
       try {
-        const record = await this.findObservationRecord(id);
-
-        if (!record) {
-          console.error('Observation not found with ID:', id);
-          return false;
-        }
+        const record = await this.observationsCollection.find(id);
 
         // Mark the record as deleted (soft delete)
         let success = false;
@@ -600,31 +558,7 @@ export class WatermelonDBRepo implements LocalRepoInterface {
       'Database: markObservationAsSynced',
       async () => {
         try {
-          // Find the observation using our improved lookup approach
-          let record: ObservationModel | null = null;
-
-          // Try to find by direct ID first
-          try {
-            record = await this.observationsCollection.find(id);
-          } catch {
-            // not found by primary key
-          }
-
-          // If not found by ID, try to find by observationId field
-          if (!record) {
-            const observations = await this.observationsCollection
-              .query(Q.where('observation_id', id))
-              .fetch();
-
-            if (observations.length > 0) {
-              record = observations[0];
-            }
-          }
-
-          if (!record) {
-            console.error('Observation not found with ID:', id);
-            return false;
-          }
+          const record = await this.observationsCollection.find(id);
 
           // Update the syncedAt timestamp
           let success = false;
@@ -823,17 +757,9 @@ export class WatermelonDBRepo implements LocalRepoInterface {
         }
         const now = new Date();
         await this.database.write(async () => {
-          const byPk = await this.observationsCollection
+          const records = await this.observationsCollection
             .query(Q.where('id', Q.oneOf(ids)))
             .fetch();
-          const byObservationColumn = await this.observationsCollection
-            .query(Q.where('observation_id', Q.oneOf(ids)))
-            .fetch();
-          const merged = new Map<string, ObservationModel>();
-          for (const r of [...byPk, ...byObservationColumn]) {
-            merged.set(r.id, r);
-          }
-          const records = [...merged.values()];
 
           const batchOps = records.map(record =>
             record.prepareUpdate(rec => {

formulus/src/database/repositories/__tests__/WatermelonDBRepo.test.ts

@@ -577,16 +577,8 @@ describe('WatermelonDBRepo', () => {
   });
 
   /**
-   * Regression: observations ingested via applyServerChanges must keep the server's
-   * observation_id as the canonical id exposed to the app and to Synkronus push payloads.
-   *
-   * Why earlier tests missed it:
-   * - All existing WatermelonDBRepo tests only used saveObservation(), which sets both
-   *   Watermelon record id and observation_id to the same obs_${timestamp}_ value.
-   * - applyServerChanges was never covered (only a synchronize() todo).
-   * - prepareCreate for pull sets observation_id but leaves Watermelon to assign a
-   *   different internal id; mappers incorrectly used model.id as domain observationId,
-   *   so pushes could use a random uuid as observation_id and duplicate server rows.
+   * Pulled observations keep the server observation_id as the stable identifier
+   * across the WatermelonDB row, domain model, lookup path, and API payload.
    */
   test('applyServerChanges (new row): domain observationId matches server observation_id', async () => {
     const serverObservationId = 'obs_pulled_from_sync_1001';

formulus/src/mappers/ObservationMapper.ts

@@ -89,15 +89,6 @@ export class ObservationMapper {
     };
   }
 
-  /**
-   * Canonical observation id for API/sync: the `observation_id` column (server id).
-   * Falls back to Watermelon row id for empty legacy rows.
-   */
-  static observationIdFromDBModel(model: ObservationModel): string {
-    const col = model.observationId?.trim();
-    return col || model.id;
-  }
-
   // DB Model -> Domain
   static fromDBModel(model: ObservationModel): DomainObservation {
     let geolocation: ObservationGeolocation | null = null;
@@ -122,7 +113,7 @@ export class ObservationMapper {
     }
 
     return {
-      observationId: ObservationMapper.observationIdFromDBModel(model),
+      observationId: model.observationId,
       formType: model.formType,
       formVersion: model.formVersion,
       data:

formulus/src/mappers/__tests__/ObservationMapper.syncIdentity.test.ts

@@ -2,18 +2,15 @@ import { ObservationMapper } from '../ObservationMapper';
 import type { ObservationModel } from '../../database/models/ObservationModel';
 
 /**
- * Documents the contract broken when domain observationId is taken from WatermelonDB's
- * internal row id instead of the observation_id column (see WatermelonDBRepo.applyServerChanges prepareCreate).
- *
- * Why this was not caught earlier: WatermelonDBRepo tests only exercised saveObservation(),
- * which sets _raw.id === observation_id, so model.id and model.observationId were always equal.
+ * Observations use the same stable identifier for both the WatermelonDB row id
+ * and the observation_id column throughout persistence and synchronization.
  */
-describe('ObservationMapper sync identity (regression)', () => {
-  test('fromDBModel must use observation_id column when it differs from Watermelon row id', () => {
-    const serverId = 'obs_pulled_from_sync_1001';
+describe('ObservationMapper sync identity', () => {
+  test('preserves the stable observation id from database to API', () => {
+    const observationId = 'obs_pulled_from_sync_1001';
     const rowAsModel = {
-      id: 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee',
-      observationId: serverId,
+      id: observationId,
+      observationId,
       formType: 'register_coffee',
       formVersion: '1.0',
       data: JSON.stringify({ k: 'v' }),
@@ -27,12 +24,12 @@ describe('ObservationMapper sync identity (regression)', () => {
       syncedAt: new Date('2025-01-03T00:00:00.000Z'),
     } as ObservationModel;
 
-    const domain = ObservationMapper.fromDBModel(rowAsModel);
+    expect(rowAsModel.id).toBe(rowAsModel.observationId);
 
-    expect(domain.observationId).toBe(serverId);
-    expect(domain.observationId).not.toBe(rowAsModel.id);
+    const domain = ObservationMapper.fromDBModel(rowAsModel);
+    expect(domain.observationId).toBe(observationId);
 
     const api = ObservationMapper.toApi(domain);
-    expect(api.observation_id).toBe(serverId);
+    expect(api.observation_id).toBe(observationId);
   });
 });