[EDIFIKANA] Org-wide property list + assigned-staff endpoints (#549)#556
Open
CRamsan wants to merge 7 commits into
Open
[EDIFIKANA] Org-wide property list + assigned-staff endpoints (#549)#556CRamsan wants to merge 7 commits into
CRamsan wants to merge 7 commits into
Conversation
Replaces the blanket authenticated_all_properties policy with an org-scoped one matching the org_scoped_units/org_scoped_common_areas pattern, closing a cross-tenant read gap on the properties table. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…es; add getEmployeesForProperty (#549) PropertyApi.getAssignedProperties becomes getProperties(organizationId), returning all properties in an org instead of only the caller's individually-mapped ones. EmployeeApi gains getEmployeesForProperty for the property-detail assigned-staff feature. Both use an explicit `path` sub-segment (by-organization / by-property) to avoid colliding with the existing single-entity routes at the same base path, following the precedent in CommonAreaApi.getCommonAreasForProperty. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Datastore/Service/Controller for Property now source getProperties from the raw properties table filtered by organization_id (instead of the v_user_properties view filtered by user_id), with an EMPLOYEE-role-or- higher RBAC gate the old user-assignment-scoped version didn't need. Removes the now-unused v_user_properties view usage and UserPropertyViewEntity. Updates existing controller/service/integration tests to match, adding a negative RBAC case and a cross-org isolation case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
) New Datastore/Service/Controller method returning all non-deleted employees assigned to a property, RBAC-gated at MANAGER role or higher to match the existing single-property detail gate. Backs the property-detail assigned-staff feature planned for the frontend slice. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…st callers (#549) Repurposing the backend's getProperties to be org-scoped breaks every front-end consumer of PropertyService/PropertyManager.getPropertyList(). Updates all of them found while implementing this slice — three more than the plan anticipated, since PropertiesOverviewScreen and MyOrganizationsViewModel landed in #548 after this issue was planned: - PropertyHomeViewModel (Properties tab) - resolves the active org via OrganizationManager, same pattern already used elsewhere. - PropertiesOverviewScreen/ViewModel (Dashboard tab's Properties sub-tab) - now takes organizationId from OrganizationHomeScreen, which already had it in scope but wasn't passing it to this screen. - PropertyManager.removeProperty - needed organizationId threaded from PropertyDetailViewModel (added to PropertyDetailUIState) to re-fetch the list after deletion. - MyOrganizationsViewModel - previously computed per-org property counts from a single cross-org query; now queries per-org (matches the org-scoped endpoint and, incidentally, fixes an undercount for any org where the current user didn't personally create every property). Also removes two pieces of dead test code discovered while fixing compilation: HomeViewModelTest.kt (a complete duplicate of PropertyHomeViewModelTest.kt testing the same ViewModel) and an unused propertyManager mock in SplashViewModelTest.kt (never wired into the ViewModel under test). Includes the regenerated openapi.yaml reflecting the Part 1 API changes (side effect of rebuilding after they'd already landed in an earlier commit). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The backend's Postgrest client is a single service_role-keyed singleton (DatastoreModule.kt) shared across all datastores, with no per-request JWT impersonation anywhere — every backend query already bypasses RLS by design (service_role has BYPASSRLS), and the front-end has no Postgrest client at all, so nothing ever queries these tables with a real user JWT. The RBAC check in PropertyController.getProperties is the actual and only enforcement for this path; the RLS policy added in d9efc6a8f had no effect on it. Reverting to the pre-existing blanket "authenticated_all_properties" policy, consistent with every other backend-only table today. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Per review feedback: PropertyApi.getProperties and
EmployeeApi.getEmployeesForProperty were each hard-shaped to search by
exactly one field via a path param. Redesigned both as generic
query-param-based list operations (GetPropertiesQueryParams,
GetEmployeesForPropertyQueryParams), matching the existing
Get<Entity>QueryParams convention (GetTasksQueryParams,
GetDocumentsQueryParams) — one required field today, room for more
optional filters later without another route redesign.
getProperties now lives at the bare GET /property (no custom path
needed — query params don't factor into route matching, so it doesn't
collide with getProperty's GET /property/{id}). getEmployeesForProperty
keeps its `by-property` sub-path since the existing unfiltered
getEmployees already claims GET /employee; kept it as its own endpoint
rather than merging into getEmployees since the two have different
authorization rules (unfiltered vs MANAGER+-gated) and merging would
mean query-param-dependent auth branching in one handler.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CRamsan
force-pushed
the
cramsan/549-property-list-backend-rls
branch
from
July 23, 2026 00:16
ec0f82a to
98953c1
Compare
gaaliciA1990
self-requested a review
July 23, 2026 00:17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PropertyApi.getAssignedProperties→getProperties: now returns all properties in an organization (query-paramGetPropertiesQueryParams(organizationId)), not just the ones the caller is individually mapped to viauser_property_mapping. RBAC-gated atEMPLOYEErole or higher.EmployeeApi.getEmployeesForProperty(GetEmployeesForPropertyQueryParams(propertyId)): returns the manager/security/cleaning staff assigned to a property, RBAC-gated atMANAGERrole or higher. Kept separate from the existing unfilteredgetEmployeessince the two have different authorization rules.GetTasksQueryParams/GetDocumentsQueryParamsconvention) rather than single-field path params — room for more optional filters later without another redesign.organizationIdrequirement through every front-end caller of the old signature — five call sites in total, not the one originally anticipated:PropertyHomeViewModel,PropertiesOverviewViewModel/OrganizationHomeScreen,PropertyManager.removeProperty/PropertyDetailViewModel, andMyOrganizationsViewModel(whose per-org property counts now come from a per-org query instead of one cross-org query — incidentally fixes a latent undercount for orgs where the current user didn't personally create every property).HomeViewModelTest.kt(a full duplicate ofPropertyHomeViewModelTest.kt) and an unused mock inSplashViewModelTest.kt.Demo
N/A — no visual/UI changes. The touched ViewModels change data scoping only (org-wide vs. user-assigned), not screen appearance.
References
~/.claude/plans/issue-356-part1-property-list-backend.md~/.claude/plans/issue-356-part1-property-list-backend-verification.mdTest plan
PropertyControllerTest,EmployeeControllerTest— new/updated cases for both endpoints, success + 401 paths.PropertyServiceTest,EmployeeServiceTest— delegation updated for the new signatures.SupabasePropertyDatastoreIntegrationTest,SupabaseEmployeeDatastoreIntegrationTest— new cross-org / cross-property isolation cases, run against a live Supabase instance (./gradlew :edifikana:back-end:integTest) — all passing.PropertyHomeViewModelTest,PropertiesOverviewViewModelTest,PropertyDetailViewModelTest,MyOrganizationsViewModelTest,PropertyManagerTest,PropertyServiceImplTest— updated for theorganizationIdplumbing../gradlew :edifikana:api:release :edifikana:back-end:release :edifikana:front-end:app:release --quiet— all green.