diff --git a/edifikana/back-end/docs/datastore-test-catalog.md b/edifikana/back-end/docs/datastore-test-catalog.md index dd3556a35..c6f7fc0a5 100644 --- a/edifikana/back-end/docs/datastore-test-catalog.md +++ b/edifikana/back-end/docs/datastore-test-catalog.md @@ -513,7 +513,7 @@ more datastores — grounded in the actual data relationships observed in the en | DS-X-001 | **P0** | `SupabasePropertyDatastore.deleteProperty` (soft) → `SupabaseUnitDatastore.createUnit(propertyId = )` | The pre-check in `createUnit` filters `deletedAt isExact null` on the property lookup — confirm it correctly throws `NotFoundException`, blocking new units under a soft-deleted property (this is the one place in the module that actively defends against finding #3/#4-style orphaning by construction, at the *child creation* boundary — worth confirming it's not accidentally bypassable) | | DS-X-002 | **P0** | `SupabaseOrganizationDatastore.deleteOrganization` (soft) → `SupabasePropertyDatastore.createProperty(organizationId = )` | Fixed — `createProperty` now checks its parent organization is active, mirroring `createUnit`'s existing guard against its own parent `Property` | | DS-X-003 | P1 | `SupabaseEmployeeDatastore.deleteEmployee` (soft) → `SupabaseTaskDatastore.getTasks(propertyId, assigneeId = )` | Confirm tasks still assigned to a soft-deleted employee remain visible/filterable by that (now-gone) `assigneeId` — nothing in `TaskDatastore` reacts to employee deletion except the explicit, separately-invoked `unassignTasksForEmployee` (DS-TASK-005/006); if a caller forgets to invoke it, stale assignments persist indefinitely and silently | -| DS-X-004 | P1 | `SupabaseOccupantDatastore.createOccupant(unitId = )` | Unlike `Unit`/`Task`'s create paths, `Occupant`'s create has **no property/unit-active pre-check at all** — confirm an occupant can be created against a unit that itself may or may not still be active (Occupant only references `unitId`, never checks it) | +| DS-X-004 | P1 | `SupabaseOccupantDatastore.createOccupant(unitId = )` | Fixed for the unit-itself-soft-deleted case — `createOccupant` now checks its immediate parent `Unit` is active, mirroring `createUnit`/`createProperty`'s (`DS-X-002`) shallow guard pattern. The deeper case (unit active, but its parent property is soft-deleted) remains unguarded, consistent with `createUnit` not checking its own parent's parent either — see the multi-level-ancestor-checks follow-up noted in the `DS-X-004` plan | | DS-X-005 | P0 | `SupabaseMembershipDatastore.removeMember` → `SupabaseOrganizationDatastore.getUserRole` → (separately) `SupabaseMembershipDatastore.getMember` via `v_org_members` | Fixed alongside `DS-ORG-010` — the two read paths now agree post-removal, since `getUserRole` filters `status = ACTIVE` the same way `v_org_members` already did | | DS-X-006 | P1 | `SupabaseRentConfigDatastore.setRentConfig` → `SupabaseUnitDatastore.deleteUnit` (soft) → `SupabaseRentConfigDatastore.getRentConfig(unitId)` | Confirm the rent config row survives the unit's soft-delete unchanged (no cascading soft-delete relationship implemented in application code — any cascade would have to be a DB-level trigger or FK `ON DELETE`, which is out of scope for this Kotlin layer but worth confirming isn't silently relied upon) | | DS-X-007 | P1 | `SupabaseUserDatastore.deleteUser` → `SupabaseNotificationDatastore.getNotificationsForUser(deletedUserId)` | Confirm notifications tied to a soft-deleted user remain queryable by ID (nothing propagates the user's deletion into the notifications table) — consistent with `api-test-catalog.md` XAPI-023's observation about stale notifications, but verifiable here purely at the datastore level without needing the controller/RBAC layer at all | diff --git a/edifikana/back-end/src/integTest/kotlin/com/cramsan/edifikana/server/datastore/supabase/CrossDatastoreIntegrationTest.kt b/edifikana/back-end/src/integTest/kotlin/com/cramsan/edifikana/server/datastore/supabase/CrossDatastoreIntegrationTest.kt index a507d4393..4c636422e 100644 --- a/edifikana/back-end/src/integTest/kotlin/com/cramsan/edifikana/server/datastore/supabase/CrossDatastoreIntegrationTest.kt +++ b/edifikana/back-end/src/integTest/kotlin/com/cramsan/edifikana/server/datastore/supabase/CrossDatastoreIntegrationTest.kt @@ -111,28 +111,28 @@ class CrossDatastoreIntegrationTest : SupabaseIntegrationTest() { } @Test - fun `DS-X-004 createOccupant against a unit under a soft-deleted property has no active pre-check`(): Unit = + fun `DS-X-004 createOccupant against a soft-deleted unit should fail with NotFoundException`(): Unit = runBlocking { // Arrange val unitId = createTestUnit(propertyId!!, "${testPrefix}_OccupantUnit") - assertTrue(propertyDatastore.deleteProperty(propertyId!!).getOrThrow()) + assertTrue(unitDatastore.deleteUnit(unitId).getOrThrow()) - // Act - unlike Unit/Task's create paths, Occupant's create has no property/unit-active - // pre-check at all; it only references unitId + // Act - createOccupant now guards against this, mirroring createUnit's and + // createProperty's (DS-X-002) existing shallow parent-active checks val result = occupantDatastore.createOccupant( unitId = unitId, userId = null, addedBy = testUserId, - name = "${testPrefix}_OccupantUnderDeletedProperty", + name = "${testPrefix}_OccupantUnderDeletedUnit", email = null, occupantType = OccupantType.TENANT, isPrimary = true, startDate = LocalDate(2026, 1, 1), endDate = null, - ).registerOccupantForDeletion() + ) - // Assert - assertTrue(result.isSuccess) + // Assert - blocked by construction, matching DS-X-001/DS-X-002's precedent + assertTrue(result.isFailure) } @Test diff --git a/edifikana/back-end/src/main/kotlin/com/cramsan/edifikana/server/datastore/supabase/SupabaseOccupantDatastore.kt b/edifikana/back-end/src/main/kotlin/com/cramsan/edifikana/server/datastore/supabase/SupabaseOccupantDatastore.kt index c797e0162..534221ff9 100644 --- a/edifikana/back-end/src/main/kotlin/com/cramsan/edifikana/server/datastore/supabase/SupabaseOccupantDatastore.kt +++ b/edifikana/back-end/src/main/kotlin/com/cramsan/edifikana/server/datastore/supabase/SupabaseOccupantDatastore.kt @@ -15,6 +15,7 @@ import com.cramsan.edifikana.server.service.models.Occupant import com.cramsan.framework.annotations.BackendDatastore import com.cramsan.framework.core.runSuspendCatching import com.cramsan.framework.logging.logD +import com.cramsan.framework.utils.exceptions.ClientRequestExceptions import io.github.jan.supabase.postgrest.Postgrest import kotlinx.datetime.LocalDate import kotlin.time.Clock @@ -42,6 +43,18 @@ class SupabaseOccupantDatastore(private val postgrest: Postgrest, private val cl ): Result = runSuspendCatching(TAG) { logD(TAG, "Creating occupant for unit: %s", unitId) + + postgrest + .from(UnitEntity.COLLECTION) + .select { + filter { + UnitEntity::unitId eq unitId.unitId + UnitEntity::deletedAt isExact null + } + }.decodeSingleOrNull() ?: throw ClientRequestExceptions.NotFoundException( + "No unit with id $unitId found in database", + ) + val entity = CreateOccupantEntity( unitId = unitId,