implemented property management - #316
Open
henry-casper wants to merge 28 commits into
Open
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @henry-casper, your pull request is larger than the review limit of 150000 diff characters
Contributor
Reviewer's GuideImplements end-to-end property management capabilities across API, persistence, GraphQL, admin UI, and verification tests, including permissions, soft-delete semantics, and test-time event handler wiring. Sequence diagram for soft-deleted property removal via GraphQLsequenceDiagram
actor AdminUser
participant AdminUI as AdminUI_Properties
participant GraphQL as GraphQL_Server
participant Resolvers as PropertyResolvers
participant Service as PropertyApplicationService
participant Repo as PropertyRepository
participant DB as MongoDB
AdminUser->>AdminUI: click RemoveProperty
AdminUI->>GraphQL: propertyDelete(input.id)
GraphQL->>Resolvers: Mutation.propertyDelete
Resolvers->>Service: requestDelete({ id })
Service->>Repo: getById(id)
Repo->>DB: findById(id).populate(['community','owner'])
Repo-->>Service: Property aggregate
Service-->>Repo: aggregate.requestDelete()
Repo->>DB: save({ isDeleted: true })
Repo-->>Service: deleted aggregate
Service-->>Resolvers: PropertyMutationResult{ status.success }
Resolvers-->>GraphQL: propertyDelete payload
GraphQL-->>AdminUI: success, property removed from list
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end property management across the domain, persistence, GraphQL API, community-admin UI, and verification suites.
Changes:
- Adds property CRUD, permissions, role resolution, and soft deletion.
- Adds guarded admin property list/create/detail pages.
- Adds extensive Storybook, acceptance, and E2E coverage plus dependency security overrides.
Reviewed changes
Copilot reviewed 128 out of 129 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
pnpm-workspace.yaml |
Updates security overrides and audit exceptions. |
packages/ocom/ui-community-route-admin/src/section-layout.graphql |
Queries property-management permission. |
packages/ocom/ui-community-route-admin/src/pages/properties.tsx |
Adds property routes. |
packages/ocom/ui-community-route-admin/src/pages/properties.stories.tsx |
Tests guarded property page states. |
packages/ocom/ui-community-route-admin/src/pages/properties-list.tsx |
Adds property-list page layout. |
packages/ocom/ui-community-route-admin/src/pages/properties-list.stories.tsx |
Covers list page states. |
packages/ocom/ui-community-route-admin/src/pages/properties-detail.tsx |
Adds property-detail page. |
packages/ocom/ui-community-route-admin/src/pages/properties-detail.stories.tsx |
Covers detail page states. |
packages/ocom/ui-community-route-admin/src/pages/properties-create.tsx |
Adds property-create page. |
packages/ocom/ui-community-route-admin/src/pages/properties-create.stories.tsx |
Covers create page rendering. |
packages/ocom/ui-community-route-admin/src/index.tsx |
Registers property menu and route. |
packages/ocom/ui-community-route-admin/src/components/properties-route-guard.container.tsx |
Enforces route permission. |
packages/ocom/ui-community-route-admin/src/components/properties-route-guard.container.stories.tsx |
Covers guard outcomes. |
packages/ocom/ui-community-route-admin/src/components/properties-list.tsx |
Renders the property table. |
packages/ocom/ui-community-route-admin/src/components/properties-list.stories.tsx |
Covers property-table states. |
packages/ocom/ui-community-route-admin/src/components/properties-list.container.tsx |
Loads and navigates properties. |
packages/ocom/ui-community-route-admin/src/components/properties-list.container.stories.tsx |
Covers list-container behavior. |
packages/ocom/ui-community-route-admin/src/components/properties-list.container.graphql |
Defines property-list query. |
packages/ocom/ui-community-route-admin/src/components/properties-detail.tsx |
Adds edit and removal form. |
packages/ocom/ui-community-route-admin/src/components/properties-detail.stories.tsx |
Covers detail interactions. |
packages/ocom/ui-community-route-admin/src/components/properties-detail.container.tsx |
Handles update and deletion. |
packages/ocom/ui-community-route-admin/src/components/properties-detail.container.stories.tsx |
Covers detail-container flows. |
packages/ocom/ui-community-route-admin/src/components/properties-detail.container.graphql |
Defines detail CRUD operations. |
packages/ocom/ui-community-route-admin/src/components/properties-create.tsx |
Adds property creation form. |
packages/ocom/ui-community-route-admin/src/components/properties-create.stories.tsx |
Covers create-form validation. |
packages/ocom/ui-community-route-admin/src/components/properties-create.container.tsx |
Handles property creation. |
packages/ocom/ui-community-route-admin/src/components/properties-create.container.stories.tsx |
Covers creation outcomes. |
packages/ocom/ui-community-route-admin/src/components/properties-create.container.graphql |
Defines create mutation. |
packages/ocom/persistence/src/datasources/readonly/property/property/property.read-repository.ts |
Adds filtered property reads. |
packages/ocom/persistence/src/datasources/readonly/property/property/property.read-repository.test.ts |
Tests read filtering and population. |
packages/ocom/persistence/src/datasources/readonly/property/property/property.data.ts |
Defines property data source. |
packages/ocom/persistence/src/datasources/readonly/property/property/index.ts |
Exposes property repository. |
packages/ocom/persistence/src/datasources/readonly/property/index.ts |
Builds property read context. |
packages/ocom/persistence/src/datasources/readonly/index.ts |
Registers property read context. |
packages/ocom/persistence/src/datasources/domain/property/property/property.repository.ts |
Adds population and soft-delete saving. |
packages/ocom/persistence/src/datasources/domain/property/property/property.repository.soft-delete.test.ts |
Tests soft-delete persistence. |
packages/ocom/graphql/src/schema/types/property.resolvers.ts |
Adds property query and mutation resolvers. |
packages/ocom/graphql/src/schema/types/property.graphql |
Defines property GraphQL API. |
packages/ocom/graphql/src/schema/types/member.resolvers.ts |
Adds role lookup fallback. |
packages/ocom/graphql/src/schema/types/member.resolvers.additional.test.ts |
Updates role resolver coverage. |
packages/ocom/graphql/src/schema/types/end-user-role.graphql |
Exposes property permissions. |
packages/ocom/domain/src/domain/contexts/property/property/index.ts |
Exports property domain types. |
packages/ocom/data-sources-mongoose-models/src/models/property/property.model.ts |
Adds deletion flag and location changes. |
packages/ocom/application-services/src/index.ts |
Registers property services. |
packages/ocom/application-services/src/contexts/property/property/update.ts |
Implements property updates. |
packages/ocom/application-services/src/contexts/property/property/request-delete.ts |
Implements deletion requests. |
packages/ocom/application-services/src/contexts/property/property/query-by-id.ts |
Adds property lookup. |
packages/ocom/application-services/src/contexts/property/property/query-by-community-id.ts |
Adds community property lookup. |
packages/ocom/application-services/src/contexts/property/property/index.ts |
Composes property operations. |
packages/ocom/application-services/src/contexts/property/property/create.ts |
Implements property creation. |
packages/ocom/application-services/src/contexts/property/index.ts |
Builds property service context. |
packages/ocom/application-services/src/contexts/community/member/query-by-id-with-role.ts |
Adds populated member lookup. |
packages/ocom/application-services/src/contexts/community/member/index.ts |
Registers member-role lookup. |
packages/ocom-verification/verification-shared/src/scenarios/property/property-management.feature |
Specifies property CRUD behavior. |
packages/ocom-verification/verification-shared/src/scenarios/property/property-authorization.feature |
Specifies authorization behavior. |
packages/ocom-verification/verification-shared/src/pages/property-form.page.ts |
Adds shared property-form page object. |
packages/ocom-verification/verification-shared/src/pages/properties-list.page.ts |
Adds shared property-list page object. |
packages/ocom-verification/verification-shared/src/pages/index.ts |
Exports property page objects. |
packages/ocom-verification/e2e-tests/src/step-definitions/index.ts |
Registers property E2E steps. |
packages/ocom-verification/e2e-tests/src/shared/support/graphql-response.ts |
Adds GraphQL response helpers. |
packages/ocom-verification/e2e-tests/src/contexts/property/tasks/view-property-details.ts |
Adds detail-view task. |
packages/ocom-verification/e2e-tests/src/contexts/property/tasks/view-properties-list.ts |
Adds list-view task. |
packages/ocom-verification/e2e-tests/src/contexts/property/tasks/update-property.ts |
Adds update task. |
packages/ocom-verification/e2e-tests/src/contexts/property/tasks/ensure-property-exists.ts |
Adds conditional creation task. |
packages/ocom-verification/e2e-tests/src/contexts/property/tasks/delete-property.ts |
Adds removal task. |
packages/ocom-verification/e2e-tests/src/contexts/property/tasks/create-property.ts |
Adds creation task. |
packages/ocom-verification/e2e-tests/src/contexts/property/tasks/become-property-manager.ts |
Provisions E2E property managers. |
packages/ocom-verification/e2e-tests/src/contexts/property/step-definitions/index.ts |
Loads property steps. |
packages/ocom-verification/e2e-tests/src/contexts/property/questions/property-screen.ts |
Adds property-screen assertions. |
packages/ocom-verification/e2e-tests/src/contexts/property/notes/property-notes.ts |
Defines E2E property state. |
packages/ocom-verification/e2e-tests/src/contexts/property/interactions/submit-property-save.ts |
Captures update outcomes. |
packages/ocom-verification/e2e-tests/src/contexts/property/interactions/submit-property-create.ts |
Captures creation outcomes. |
packages/ocom-verification/e2e-tests/src/contexts/property/interactions/record-property-notes.ts |
Records list baselines. |
packages/ocom-verification/e2e-tests/src/contexts/property/interactions/open-property-detail.ts |
Opens property details. |
packages/ocom-verification/e2e-tests/src/contexts/property/interactions/open-properties-list.ts |
Opens property lists. |
packages/ocom-verification/e2e-tests/src/contexts/property/interactions/open-create-property-form.ts |
Opens creation form. |
packages/ocom-verification/e2e-tests/src/contexts/property/interactions/open-admin-portal.ts |
Opens provisioned admin portal. |
packages/ocom-verification/e2e-tests/src/contexts/property/interactions/fill-property-form.ts |
Fills property forms. |
packages/ocom-verification/e2e-tests/src/contexts/property/interactions/confirm-property-removal.ts |
Confirms property deletion. |
packages/ocom-verification/e2e-tests/src/contexts/property/abilities/admin-portal-page.ts |
Adds property navigation helpers. |
packages/ocom-verification/acceptance-ui/tsconfig.json |
Includes admin route sources. |
packages/ocom-verification/acceptance-ui/src/step-definitions/index.ts |
Registers property UI steps. |
packages/ocom-verification/acceptance-ui/src/contexts/property/tasks/properties-screen.ts |
Renders property acceptance screens. |
packages/ocom-verification/acceptance-ui/src/contexts/property/tasks/manage-property.ts |
Implements UI CRUD tasks. |
packages/ocom-verification/acceptance-ui/src/contexts/property/step-definitions/index.ts |
Loads property UI steps. |
packages/ocom-verification/acceptance-ui/src/contexts/property/questions/property-screen.ts |
Adds UI screen assertions. |
packages/ocom-verification/acceptance-ui/src/contexts/property/questions/property-outcome.ts |
Adds mocked outcome questions. |
packages/ocom-verification/acceptance-ui/src/contexts/property/notes/property-ui-notes.ts |
Defines UI scenario state. |
packages/ocom-verification/acceptance-api/src/world.ts |
Registers property API abilities. |
packages/ocom-verification/acceptance-api/src/step-definitions/index.ts |
Registers property API steps. |
packages/ocom-verification/acceptance-api/src/shared/graphql/property-operations.ts |
Defines verification GraphQL operations. |
packages/ocom-verification/acceptance-api/src/shared/abilities/update-property.ts |
Adds update ability. |
packages/ocom-verification/acceptance-api/src/shared/abilities/provision-resident-member.ts |
Provisions unauthorized residents. |
packages/ocom-verification/acceptance-api/src/shared/abilities/index.ts |
Exports property abilities. |
packages/ocom-verification/acceptance-api/src/shared/abilities/graphql-client.ts |
Adds principal context headers. |
packages/ocom-verification/acceptance-api/src/shared/abilities/delete-property.ts |
Adds deletion ability. |
packages/ocom-verification/acceptance-api/src/shared/abilities/create-property.ts |
Adds creation ability. |
packages/ocom-verification/acceptance-api/src/shared/abilities/actor-auth.ts |
Tracks end-user tokens and context. |
packages/ocom-verification/acceptance-api/src/servers/api-graphql-test-server.ts |
Passes test principal context. |
packages/ocom-verification/acceptance-api/src/mock-application-services.ts |
Registers handlers and end-user validation. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/view-property-details.ts |
Adds API detail-view task. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/view-properties-list.ts |
Adds API list-view task. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/update-property.ts |
Adds API update task. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/update-property-input.ts |
Maps update inputs. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/provision-resident-member.ts |
Arranges resident actors. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/delete-property.ts |
Adds API deletion task. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/create-property.ts |
Adds API creation task. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/become-property-manager.ts |
Arranges property managers. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/attempt-update-property.ts |
Captures rejected updates. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/attempt-delete-property.ts |
Captures rejected deletions. |
packages/ocom-verification/acceptance-api/src/contexts/property/tasks/attempt-create-property.ts |
Captures rejected creations. |
packages/ocom-verification/acceptance-api/src/contexts/property/step-definitions/index.ts |
Loads property API steps. |
packages/ocom-verification/acceptance-api/src/contexts/property/questions/viewed-property.ts |
Reads viewed property data. |
packages/ocom-verification/acceptance-api/src/contexts/property/questions/property-retrievable.ts |
Checks post-deletion retrieval. |
packages/ocom-verification/acceptance-api/src/contexts/property/questions/property-operation-outcome.ts |
Reads operation outcomes. |
packages/ocom-verification/acceptance-api/src/contexts/property/questions/property-named.ts |
Finds properties by name. |
packages/ocom-verification/acceptance-api/src/contexts/property/questions/property-manager-permission.ts |
Verifies role permission. |
packages/ocom-verification/acceptance-api/src/contexts/property/questions/property-field.ts |
Reads property fields. |
packages/ocom-verification/acceptance-api/src/contexts/property/questions/properties-list.ts |
Queries community properties. |
packages/ocom-verification/acceptance-api/src/contexts/property/notes/property-notes.ts |
Defines API scenario state. |
packages/ocom-verification/acceptance-api/package.json |
Adds verification dependencies. |
codegen.yml |
Maps GraphQL Property to domain type. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…scade npm latest (4.13.2, also 4.13.1) point at CDN artifacts that 404 (Azure.Functions.Cli.linux-x64.<version>.zip missing), breaking the unpinned global install. 4.13.0 is the newest release with a working artifact (verified via ranged GET -> HTTP 206). Also add succeeded() to the func-tools/Playwright install conditions and replace always() on the Playwright verify step, so a failed install no longer cascades into misleading 'pnpm: command not found' errors. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…unique name index to active properties
- getById now treats soft-deleted properties as not found, preventing
update/delete mutations against hidden records (PR review P1)
- getAll filters out soft-deleted documents
- unique {community, propertyName} index is now partial on
{isDeleted: false} so deleted property names can be reused (PR review P2)
- added compensating {community, isDeleted} index for listing queries
- covered by repository unit tests, index contract tests, and two new
acceptance-api scenarios
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tibility
Reverts the partial {isDeleted: false} unique index and the compensating
{community, isDeleted} index so the PR requires no manual index migration
on deployed databases (createIndex with changed options would conflict
with the existing index). Deleted property names remain reserved.
Keeps the P1 fix: soft-deleted properties are still excluded from the
write repository (getById/getAll), so they cannot be mutated.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Scope property reads to community members: property/propertiesByCommunityId now verify the actor's membership in the target community (Unauthorized otherwise) - Require canManageProperties for admin property updates via a public assertCanManageProperties guard on the Property aggregate - Forward explicit nulls for bedrooms/bathrooms/squareFeet so numeric listing details can be cleared end to end (UI container, resolver, command) - Evict deleted properties from the Apollo cache after propertyDelete - Resolve Property.owner through the member read model so nested account fields are GraphQL-safe - Pin func-tools CI cache to exact version key; inexact hits no longer skip installation of the pinned Core Tools version - Drain in-flight integration event handlers before per-scenario DB reset and skip the mock server dev seed under tests (SKIP_DEV_SEED) to stop acceptance cross-scenario contamination - Note: member navigation finding was a false positive (MemberReadRepo.isAdmin already includes canManageProperties) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The spec mandates all admin-side property queries enforce propertyPermissions.canManageProperties. The previous fix only verified community membership, letting residents without the permission read the property directory. Reads now load the acting member's role and require canManageProperties in the target community; the contradictory resident-can-view scenario is replaced with rejection scenarios for both list and details. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ry application services Property read authorization now lives in the application services and is bound to the request's current member/community context: - Expose the request-scoped passport on DataSources so application services can evaluate domain visas on read operations. - Guard Property queryById/queryByCommunityId with the property visa (canManageProperties or system account). The member passport is built from the request's x-member-id/x-community-id hints and the MemberPropertyVisa denies cross-community roots, so a manager acting under a different community context is rejected even if they hold manage permissions elsewhere. - Drop the resolver-level membership lookup that authorized via any membership matching the requested community; resolvers now only require a verified user and delegate authorization to the services. - New acceptance scenarios: a manager who switches communities can no longer view their original community's list or property details. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 211 out of 226 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/ocom/graphql/src/schema/types/member.resolvers.ts:111
- This fallback introduces an N+1 query for every list that requests
Member.role.membersForCurrentEndUseris built by an aggregation that does not populate roles, so each returned member reaches this line and executes a separatequeryByIdWithRolecall. Use a request-scoped DataLoader backed by the newly addedqueryByIdsWithRole, or populate roles in the originating list query, so all role lookups are batched.
…n owner options, flaky story
- Add derived EndUserRolePermissions.isNonPropertyAdmin SDL field (resolver
mirrors MemberReadRepositoryImpl.isAdmin minus canManageProperties) with
unit tests; gate the admin Members/Settings menu entries on it so
property-only managers no longer see unrelated admin sections.
- canAccessAdminPortal: admins holding any non-property admin permission keep
the legacy portal entry points (even with canManageProperties and no
ACCEPTED account); the ACCEPTED-account requirement now applies only to
property-only managers. Updated helper tests and nav queries/mocks.
- Replace the member-management members query in the property create/detail
containers with a minimal AdminPropertiesOwnerOptions operation (id +
memberName only) so property managers no longer receive member-management
data (accounts, profile email/bio); updated all stories and the
acceptance-ui mock backend.
- De-flake the "Submitting Blocks Duplicate Creates" story: raise the mocked
create delay to 2s, guard the duplicate click against post-navigation
detachment, and give the final navigation assertion a generous timeout.
- Reset the property form's Save & Close submit intent on validation failure
(and consume it on submit) so a later Enter-key submit stays on the page.
- Fix impossible __typename ('PropertyPermissions' ->
'EndUserRolePropertyPermissions') in community-list story and ui-community
storybook apollo mocks.
- Pin the nanoid override to 3.3.18 (was >=3.3.17 which resolved to ESM-only
nanoid 6.x for CJS consumers such as postcss).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 212 out of 228 changed files in this pull request and generated 10 comments.
Suppressed comments (4)
Previously missed (3) — in code that hasn't changed since the last review.
packages/ocom/graphql/src/schema/types/property.graphql:166
- This field returns every property in the community, while the UI's table pagination is only client-side. Property data and owner resolution therefore grow without bound in one request. Add server-side cursor/page arguments and return a paginated result so large communities do not produce increasingly expensive responses.
packages/cellix/serenity-framework/src/servers/process-test-server.ts:38 - This adds a public
@cellix/serenity-framework/serversoption that controls spawned-process behavior, butprocess-test-server.test.tshas no coverage for it. Add a test proving overrides reach the child process while inherited variables remain available and inheritedNODE_OPTIONSis still stripped.
packages/ocom/ui-community-route-admin/src/components/property-form.validation.ts:20 - This comment is now incorrect:
normalizeTagsthrows when more than 50 normalized tags are supplied; it no longer silently keeps the first 50. Update the mirrored-domain description so future validation changes are based on the actual contract.
packages/ocom/ui-community-route-admin/src/index.tsx:55
hasPermissionsonly hides this menu item;MenuComponentnever guards the matching route. A property-only manager can still deep-link to/members(and/settings) because those<Route>elements remain unconditional, and the member query only requires an authenticated JWT. Add a route-level non-property-admin guard, as the Properties section does.
…batch member roles - Add packages/ocom/ui-community-route-admin/src/components/countries.ts to sonar.cpd.exclusions: it is static ISO country/subdivision data, and its 1,503 "duplicated" lines were the sole cause of the PR duplication gate failure (19.4% vs 4% limit). - Add propertyOwnerOptions(communityId) root query authorized by the property visa (canManageProperties via ensureCommunityPropertiesViewable) and point the AdminPropertiesOwnerOptions operation at it, so owner dropdown options are no longer served by the JWT-only membersByCommunityId field that let any authenticated caller enumerate member ids/names of arbitrary communities. New app service queryOwnerOptionsByCommunityId + resolver/unit tests. - Batch-load roles once in membersForCurrentEndUser via queryByIdsWithRole: the external-id read is an aggregation without populated roles, so role-selecting queries previously fell back to one role lookup per member (N+1) in the Member.role field resolver. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 212 out of 231 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
packages/ocom/ui-community-route-admin/src/components/format-display-address.ts:29
- The formatter accepts and fetches
countrybut drops it from the rendered address. International properties can therefore display an ambiguous address—orN/Awhen country is the only populated location field. Include the trimmed country inparts.
packages/ocom/ui-community-route-admin/src/index.tsx:55
hasPermissionsis only consulted byMenuComponentwhen building navigation; it does not guard the matching React Router route, andSectionLayoutrenders<Outlet />unconditionally. A property-only manager can therefore deep-link to/members(whosemembersByCommunityIdresolver only requires a verified JWT) or/settingseven though these menu items are hidden. Add authorization guards to the route elements themselves, not just the menu entries.
…ered portal links
- Return a dedicated PropertyOwnerOption { id, memberName } type from
propertyOwnerOptions instead of full Member records, and map to that DTO in
the application service, so a property-only manager cannot select accounts,
profile, role, or nested end-user data through the owner dropdown query.
- Add NonPropertyAdminRouteGuardContainer around the Members and Settings
routes: hiding the navigation entries did not stop a property-only manager
from deep linking to those URLs. The guard mirrors the menu gating
(isNonPropertyAdmin) and rejects member ids from other communities.
- Look up each community's member group by community id in the accounts
community list: the members prop is parallel to the unfiltered communities
array, so filtered rows previously used the wrong index and produced portal
links pairing one community with another community's member ids.
- Make the Azure Functions Core Tools install self-verifying and drop its
Cache@2 task: npm installs the tools into Node's global prefix rather than
the cached /opt/hostedtoolcache/func path, so a cache hit skipped the
install without providing func. The step now checks `func --version` and
installs only when the pinned 4.13.0 is absent.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 210 out of 233 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
packages/ocom/ui-community-route-admin/src/components/properties-detail.container.tsx:42
network-onlybypasses the cache only when this query executes; it does not make the request's community/member headers dependencies. React Router can reuse this component when those route params change whileidstays the same, so no new authorization check runs and the detail from the previous route remains rendered. Explicitly rerun/remount the query when the route context changes (and ensure the Apollo link has the new headers first).
packages/ocom/ui-community-route-admin/src/components/format-display-address.ts:29countryis accepted by this formatter but never added toparts. Consequently a country-only address displaysN/A, and every international address silently omits its country. Include the trimmed country in the output and update the formatter tests accordingly.
Property.owner returned the unrestricted Member type, so a caller authorized
only to manage properties could traverse owner { accounts profile role ... }
and reach member data the restricted PropertyOwnerOption boundary was added
to protect. The field now returns PropertyOwnerOption and the resolver
projects the batch-loaded member down to id + memberName, keeping the
per-request DataLoader batching intact. All owner-selecting operations
already request only id/memberName, so no client behavior changes.
Also bumps the fast-uri override to ^4.1.3 (with a matching
minimumReleaseAgeExclude entry): Snyk began flagging four new high-severity
CVEs against 4.1.2 (SNYK-JS-FASTURI-19256867/-69/-71/-73), which failed the
pre-commit dependency scan; 4.1.3 is the coordinated patch release.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 210 out of 233 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
packages/ocom/ui-community-route-admin/src/components/property-form.tsx:713
- The domain now rejects a 51st bedroom-detail row, but this form leaves the Add button enabled indefinitely and has no list-level validation. Users can build an invalid form that only fails after submission; disable adding once
fields.lengthreaches 50 (and surface the limit inline).
This issue also appears on line 772 of the same file.
packages/cellix/serenity-framework/src/servers/process-test-server.ts:38
- This adds a public
ProcessTestServerbehavior used to control child-process seeding, but the existingprocess-test-server.test.tssuite does not verify that overrides reach the spawned process or that inherited environment variables remain intact. Add a contract test that starts a child with an override and asserts the child observes it, including theNODE_OPTIONSsanitization behavior.
packages/ocom/ui-community-route-admin/src/components/property-form.tsx:778
- The backend caps additional-amenity rows at 50, but this button can keep adding rows past that invariant. Prevent the 51st row in the form (and expose the limit inline) so a user cannot prepare a submission that is guaranteed to be rejected.
…tier Tier-label gates alone still allowed privilege escalation through the permissions payload: a Staff.CaseManager updating a CaseManager-classified role could set canManageTechAdmin, canManageAllCommunities, or finance permissions, and staff passports derive capabilities from the persisted role flags, so the elevated permissions applied on the caller's next request. The command mapper now derives a per-tier grantable-flag allow-list that mirrors the domain default role definitions (TechAdmin: all flags; CaseManager/SupportLead: their default six; Finance: its default set plus the finance group) and rejects any flag requested true that is outside the caller's tiers, unless that flag is already true on the persisted role (full-payload re-saves keep working) — revocations always pass. Both staffRoleCreate and staffRoleUpdate are gated; staffRoleUpdate passes the persisted role's permissions so unchanged elevated flags survive. Includes 11 new mapper unit tests, 2 resolver tests, and an @api-only acceptance scenario proving a case manager cannot grant canManageTechAdmin on the Default Case Manager role. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 208 out of 233 changed files in this pull request and generated 3 comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/ocom/ui-community-route-admin/src/components/format-display-address.ts:29
countryis accepted and fetched for every row but is never included inparts. As a result, a country-only address renders asN/A, and international addresses omit their country entirely. Append the trimmed country to the display parts and update the country-only test expectation.
Move persisted-state authorization for staffRoleUpdate and staffUserAssignRole out of resolver pre-reads and into the application service unit of work, closing the TOCTOU window where a concurrent role promotion could bypass a stale pre-check. Both commands now carry a callerContext (allowed enterprise app role tiers, unclassified-role allowance, grantable permission flags) computed from the verified JWT at the API boundary and validated against the same role snapshot that gets mutated or assigned. The mapper keeps input-only gates (blank/over-tier requested labels, create-side grant gate); error messages are unchanged. Collapse property update/requestDelete failures for unknown ids and properties the caller cannot manage into one indistinguishable "Property not found" error, so mutation responses can no longer be used to probe property ids across communities (write-side counterpart of the read-side deny-by-omission). PropertyRepository.getById now throws the seedwork NotFoundError (same message) so callers can classify missing ids by error name. New acceptance scenarios pin the identical message for cross-community and unknown-id update/delete attempts. The image-size GHSA review finding was false: no patched release exists (first_patched_version is null for both advisories, npm latest is 2.0.2), so the audit ignores stay with refreshed comments. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Invalidate stale owner options, secure member role reads, harden validation scenarios, and handle missing property timestamps. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 219 out of 250 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
packages/ocom/data-sources-mongoose-models/src/models/property/property.model.ts:214
- The read paths treat a missing
isDeletedas active ($ne: true), but this index includes only documents where the field is exactlyfalse. Existing properties created before this field was added therefore remain visible while being excluded from the uniqueness constraint, allowing a new active property with the same community/name. A schema declaration also does not replace the previous same-key unique index, which can continue blocking name reuse. Add a deployment migration that backfillsisDeleted: falseand explicitly replaces the old index before relying on this constraint.
packages/ocom/application-services/src/contexts/property/property/apply-property-fields.ts:149 - An omitted or blank
categorystill creates and persists an additional-amenity row because the value-object setter is skipped. This bypassesCategory's minimum-length validation and violates the entity's requiredcategoryproperty. Assign the value object for every new row so incomplete rows are rejected.
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
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 by Sourcery
Implement end-to-end property management with full-field CRUD, community authorization, soft deletion, and comprehensive verification coverage.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests:
Chores: