Skip to content

feat(formbuilder): ModelDeletedEvent consumer for broken Relation Picker refs#102

Merged
phuongnse merged 10 commits into
mainfrom
cursor/form-model-reference-consumers-e85a
May 25, 2026
Merged

feat(formbuilder): ModelDeletedEvent consumer for broken Relation Picker refs#102
phuongnse merged 10 commits into
mainfrom
cursor/form-model-reference-consumers-e85a

Conversation

@phuongnse

@phuongnse phuongnse commented May 24, 2026

Copy link
Copy Markdown
Owner

Summary

FormBuilder consumes ModelDeletedEvent for broken Relation Picker refs (US-033 / E05), with form_model_references read model, sync on field changes, IsBroken on GET form, and 409 when active form references block model delete.

This revision addresses PR review module-boundary feedback: deletion guard no longer calls FormBuilder Application repositories from Axis.Api. DataModeling validates via FormBuilder gRPC (CountActiveModelReferences). Gateway endpoints no longer inject Identity repositories (MediatR queries/handlers instead). GatewayBoundaryTests enforces Axis.Api cannot depend on other modules' Application layers.

Linked spec

Requirements

  • ModelDeletedHandler consumes ModelDeletedEvent (Contracts-only ref to DataModeling)
  • FormFieldDto.IsBroken exposed on GET form
  • DELETE model returns 409 when form Relation Picker refs exist (via gRPC guard)
  • EF migration AddFormModelReferences
  • No in-process cross-module Application/Repositories from Axis.Api (gRPC + gateway fitness tests)
  • Application + architecture tests pass

Deferred: WorkflowBuilder record.* trigger broken refs; DataModeling relation-field broken flags on other models; frontend broken indicators.

Open in Web Open in Cursor 

Summary by CodeRabbit

  • New Features

    • Form fields now show a "broken" indicator when they reference deleted models
    • Form model references are tracked and kept in sync when fields are added/removed
  • Bug Fixes

    • Model deletion is blocked when active form references exist (returns HTTP 409)
    • User sessions are revoked upon deactivation
  • Documentation

    • Implementation progress and epic/readme status updated for DataModeling and FormBuilder

Review Change Stack

…er refs

- form_model_references read model + sync on field add/remove
- ModelDeletedHandler flags broken fields; GET form returns isBroken
- Block DELETE model when active form references exist (US-033)
- EF migration AddFormModelReferences

Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3bf5a898-bc1f-4513-9087-a89c95396272

📥 Commits

Reviewing files that changed from the base of the PR and between 4df257c and 2761ff9.

📒 Files selected for processing (11)
  • src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Extensions/DataModelingInfrastructureExtensions.cs
  • src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Grpc/FormModelDeletionGuard.cs
  • src/Modules/Identity/Axis.Identity.Application/Commands/DeactivateUser/DeactivateUserHandler.cs
  • src/Modules/Identity/Axis.Identity.Application/Queries/GetUserTokenClaims/GetUserTokenClaimsHandler.cs
  • tests/Api/Axis.Api.Tests/Axis.Api.Tests.csproj
  • tests/Api/Axis.Api.Tests/Helpers/ApiTestFixture.cs
  • tests/Architecture/Axis.Architecture.Tests/GatewayBoundaryTests.cs
  • tests/Modules/Identity/Axis.Identity.Application.Tests/Commands/DeactivateUserHandlerTests.cs
  • tests/Modules/Identity/Axis.Identity.Application.Tests/Queries/GetCurrentUserProfileHandlerTests.cs
  • tests/Modules/Identity/Axis.Identity.Application.Tests/Queries/GetInvitationByTokenHandlerTests.cs
  • tests/Modules/Identity/Axis.Identity.Application.Tests/Queries/GetUserTokenClaimsHandlerTests.cs
✅ Files skipped from review due to trivial changes (1)
  • tests/Api/Axis.Api.Tests/Axis.Api.Tests.csproj

📝 Walkthrough

Walkthrough

This PR adds persistent form-model reference tracking (DB+EF migration), a ModelDeleted handler that flags broken references, a gRPC count API and FormModelDeletionGuard used by DataModeling to block deletions, refactors several Identity endpoints to use MediatR queries/handlers, and introduces architectural gateway tests.

Changes

Form-Model Reference Tracking and Deletion Guard

Layer / File(s) Summary
Documentation updates
docs/PROGRESS.md, docs/epics/E03-data-modeling/features/F01-model-definition.md, docs/epics/E05-form-builder/README.md
Progress/epic notes updated to reflect form_model_references, ModelDeleted handling, and revised retrofit status.
Application contracts & domain read model
src/Modules/FormBuilder/Axis.FormBuilder.Application/Repositories/IFormModelReferenceRepository.cs, src/Modules/FormBuilder/Axis.FormBuilder.Application/Services/IFormModelReferenceSync.cs, src/Modules/FormBuilder/Axis.FormBuilder.Domain/ReadModels/FormModelReference.cs
Add IFormModelReferenceRepository, IFormModelReferenceSync, and FormModelReference read model with state transitions.
EF Core mapping, DbContext, migration
src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Persistence/FormBuilderDbContext.cs, .../Persistence/Configurations/FormModelReferenceConfiguration.cs, .../Migrations/20260524150108_AddFormModelReferences.*, .../Migrations/FormBuilderDbContextModelSnapshot.cs
Add DbSet, entity configuration, and migration creating form_model_references table with composite PK and indexes.
Repository and sync implementation
src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Repositories/FormModelReferenceRepository.cs, src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Services/FormModelReferenceSync.cs
Implement queries for broken field IDs and active-reference counts; implement sync service to upsert/delete/retarget references from RelationPicker fields.
Form handlers and DTO
src/Modules/FormBuilder/Axis.FormBuilder.Application/Commands/AddFieldToForm/AddFieldToFormHandler.cs, .../RemoveFieldFromForm/RemoveFieldFromFormHandler.cs, .../Queries/GetFormById/FormFieldDto.cs, .../Queries/GetFormById/GetFormByIdHandler.cs
Add IFormModelReferenceSync to add/remove handlers and call SyncRelationPickerReferencesAsync; extend FormFieldDto with IsBroken and populate from repository.
ModelDeletedHandler
src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Handlers/ModelDeletedHandler.cs
Handler consumes ModelDeletedEvent, creates/marks broken FormModelReference records for matching RelationPicker fields, and persists when changes occurred.
gRPC contract, service, and infra
src/Modules/FormBuilder/Axis.FormBuilder.Contracts/*.proto, src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Grpc/FormModelReferenceGrpcService.cs, .../Axis.FormBuilder.Infrastructure.csproj, .../Extensions/FormBuilderInfrastructureExtensions.cs
Add Protobuf count RPC, implement Grpc service, add csproj tooling, register service and map gRPC endpoints.
Deletion guard and DataModeling wiring
src/Modules/DataModeling/Axis.DataModeling.Application/Services/IModelDeletionGuard.cs, src/Modules/DataModeling/Axis.DataModeling.Infrastructure/*
Add IModelDeletionGuard and FormModelDeletionGuard that calls FormBuilder gRPC CountActiveModelReferences; register gRPC client in DataModeling infra.
DeleteModel handler & API mapping
src/Modules/DataModeling/Axis.DataModeling.Application/Commands/DeleteModel/DeleteModelHandler.cs, src/Axis.Api/Endpoints/ModelEndpoints.cs, src/Axis.Api/Program.cs, src/Axis.Api/appsettings.json
Inject deletion guard into DeleteModelHandler and call it before deletion; endpoint documents 409 response; app maps FormBuilder gRPC and provides GrpcUrl config.
Tests: form, ModelDeleted, deletion guard
tests/Modules/FormBuilder/..., tests/Modules/FormBuilder.Infrastructure.Tests/Handlers/ModelDeletedHandlerTests.cs, tests/Modules/DataModeling/.../DeleteModelHandlerTests.cs
Update/add unit and EF-backed tests to assert sync calls, broken-field flags, ModelDeleted behavior, and DeleteModel guard blocking on active references.
Test infra
tests/Api/Axis.Api.Tests/*
Wire Grpc.Net.ClientFactory into in-process test server and set test Modules:FormBuilder:GrpcUrl for integration tests.

Identity Module Mediator Refactoring

Layer / File(s) Summary
Identity queries & handlers
src/Modules/Identity/Axis.Identity.Application/Queries/*
Add GetCurrentUserProfile, GetInvitationByToken, and GetUserTokenClaims queries and handlers with DTOs to centralize claim/profile/invitation resolution.
DeactivateUser command & handler
src/Modules/Identity/Axis.Identity.Application/Commands/DeactivateUser/*
Remove AdminRoleId from command; handler now resolves Admin role via IRoleRepository and revokes sessions via ISessionStore after deactivation.
Endpoints refactored
src/Axis.Api/Endpoints/ConnectEndpoints.cs, src/Axis.Api/Endpoints/InvitationEndpoints.cs, src/Axis.Api/Endpoints/UserEndpoints.cs
Switch token issuance, GetInvitation, and GetMe flows to use MediatR queries/commands instead of direct repository calls; UpdateStatus uses DeactivateUserCommand via mediator.
Identity tests
tests/Modules/Identity/Axis.Identity.Application.Tests/*
Add tests for new query handlers and update DeactivateUserHandler tests for new dependencies and signature.

Architectural Enforcement

Layer / File(s) Summary
Gateway boundary tests
tests/Architecture/Axis.Architecture.Tests/GatewayBoundaryTests.cs, tests/Architecture/Axis.Architecture.Tests/README.md
Add tests enforcing that Axis.Api endpoints/infrastructure do not directly depend on other modules' Application repositories/services.

Sequence Diagram(s)

sequenceDiagram
  participant DataModeling
  participant Kafka
  participant ModelDeletedHandler
  participant FormDB
  participant FormGrpc
  participant DeleteGuard

  DataModeling->>Kafka: Publish ModelDeletedEvent

  Kafka->>ModelDeletedHandler: Deliver ModelDeletedEvent
  ModelDeletedHandler->>FormDB: Load forms and RelationPicker fields
  ModelDeletedHandler->>FormDB: Create/Mark FormModelReference rows (IsBroken=true)
  ModelDeletedHandler->>FormDB: SaveChangesAsync

  DeleteGuard->>FormGrpc: CountActiveModelReferences(modelId, orgId)
  FormGrpc-->>DeleteGuard: ActiveReferenceCount
  alt ActiveReferenceCount > 0
    DeleteGuard-->>DataModeling: Return Conflict failure
  else
    DeleteGuard-->>DataModeling: Return Success
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • phuong-labs/axis#25: Related — earlier FormBuilder Application handlers/DTOs that this PR extends with reference sync and broken-field support.
  • phuong-labs/axis#101: Related — introduces DataModeling model-deleted event contract consumed by ModelDeletedHandler.
  • phuong-labs/axis#37: Related — overlaps on DeleteModel handler/test-level changes and cross-organization test scenarios.

Poem

🐰 I patched the fields where the pickers would roam,

Marked broken connections so models stay home.
Through gRPC whispers and migrations applied,
Guards now check references before models are fried.
The rabbit hums—ask first, don't assume—use mediator with pride.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cursor/form-model-reference-consumers-e85a

cursoragent and others added 2 commits May 24, 2026 14:49
…ests

Register IFormModelReferenceRepository and IFormModelReferenceSync in DI
so AddFieldToFormHandler resolves in API tests.

Refactor ModelDeletedHandler to use injected DbContext (test schema).
Block DELETE model when active form references exist (409).

Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
…signer

Manual migration lacked .Designer.cs so EF never applied it in tests/CI.
Regenerated via dotnet ef migrations add; table form_model_references is
now in the migration chain.

Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
@phuongnse phuongnse marked this pull request as ready for review May 24, 2026 15:12

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/Axis.Api/Endpoints/ModelEndpoints.cs (1)

71-79: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Declare the new 409 response in endpoint metadata.

The route can now return conflict, but OpenAPI metadata for MapDelete does not include .ProducesProblem(409).

Suggested patch
         group.MapDelete("/{modelId:guid}", DeleteModel)
             .RequireAuthorization(Permissions.DataModeling.ModelDelete)
             .WithName("DeleteModel")
             .WithSummary("Soft-delete a data model")
             .WithTags("DataModeling")
             .Produces(204)
             .ProducesProblem(401)
             .ProducesProblem(403)
+            .ProducesProblem(409)
             .ProducesProblem(404);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Axis.Api/Endpoints/ModelEndpoints.cs` around lines 71 - 79, The OpenAPI
metadata for the DeleteModel endpoint is missing the 409 conflict response;
update the MapDelete("/{modelId:guid}", DeleteModel) endpoint fluent chain to
include a .ProducesProblem(409) so the DeleteModel route reflects that it can
return a 409 Conflict; ensure you add the call alongside the existing
.ProducesProblem(401/.403/.404) in the same chain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/Axis.Api/Endpoints/ModelEndpoints.cs`:
- Line 4: The DeleteModel endpoint currently contains a deletion guard and a
direct dependency on Axis.FormBuilder.Application.Repositories; remove that
domain validation and the using directive from the endpoint and instead perform
the reference check inside the application command handler (e.g., the
DeleteModelCommand/handler) so the handler returns a failure Result when
references exist; from the endpoint, only call mediator.Send(deleteModelCommand)
and map the returned Result to an HTTP response with ToProblemDetails(),
ensuring no in-process calls into other modules from the endpoint.

In
`@tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Commands/AddFieldToFormHandlerTests.cs`:
- Around line 16-17: The tests create a mock IFormModelReferenceSync but never
assert it was invoked; update the success tests (e.g.,
Handle_WhenFormExists_AddsFieldAndReturnsFieldId) to verify the handler calls
the sync service after adding a field by asserting the mock
_formModelReferenceSync received the expected call (use
Received/ReceivedWithAnyArgs and match on the FormModelReference or formId
argument), ensuring the AddFieldToFormHandler triggers
IFormModelReferenceSync.Sync (or the actual method name) once after a successful
add; apply the same verification to the other similar test(s).

In
`@tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Commands/RemoveFieldFromFormHandlerTests.cs`:
- Around line 17-18: The test currently never verifies that
IFormModelReferenceSync is invoked; update the success test
Handle_WhenFieldExists_RemovesAndSaves in RemoveFieldFromFormHandlerTests to
assert that the mock IFormModelReferenceSync (the _formModelReferenceSync field)
was called once after handler.Handle(...) completes—use Substitute's Received(1)
on the appropriate method (the sync method that updates FormModelReference) with
the expected form id (or matching argument) to ensure the handler calls the sync
service after removing the field.

In
`@tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Queries/GetFormByIdHandlerTests.cs`:
- Around line 14-22: Add a test that verifies broken-field detection: configure
the mocked IFormModelReferenceRepository (_formModelReferenceRepo) to return a
HashSet containing the specific FormField.Id, arrange the repository (_repo) to
return a FormDefinition with that field, invoke GetFormByIdHandler.Handle via
the GetFormByIdQuery, and assert the returned FormDetailDto.Fields contains the
field and its IsBroken property is true (suggested test name:
Handle_WhenFieldIsBroken_MarksDtoAsBroken). Ensure you use existing
helpers/constructors (FormDefinition.Create, FormDefinition.AddField) and the
handler instance _handler created in the test fixture.

In
`@tests/Modules/FormBuilder/Axis.FormBuilder.Infrastructure.Tests/Handlers/ModelDeletedHandlerTests.cs`:
- Around line 37-65: The test currently saves a form with a RelationPicker field
directly to the DbContext, so no FormModelReference is created and
ModelDeletedHandler has nothing to mark; modify the test to create the
FormModelReference before invoking the handler by either (a) invoking the real
AddFieldToFormHandler (or the code path that uses IFormModelReferenceSync) to
add the field so the reference is created, or (b) directly create and save a
FormModelReference record for the form/targetModelId in the test DbContext prior
to calling CreateHandler(...).Handle(BuildEvent(targetModelId), ...), then
assert that the existing reference IsBroken is true.

---

Outside diff comments:
In `@src/Axis.Api/Endpoints/ModelEndpoints.cs`:
- Around line 71-79: The OpenAPI metadata for the DeleteModel endpoint is
missing the 409 conflict response; update the MapDelete("/{modelId:guid}",
DeleteModel) endpoint fluent chain to include a .ProducesProblem(409) so the
DeleteModel route reflects that it can return a 409 Conflict; ensure you add the
call alongside the existing .ProducesProblem(401/.403/.404) in the same chain.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2718fe70-8a3f-4dc6-abcd-58c85e54d5e3

📥 Commits

Reviewing files that changed from the base of the PR and between 59912bb and 618699e.

📒 Files selected for processing (25)
  • docs/PROGRESS.md
  • docs/epics/E03-data-modeling/features/F01-model-definition.md
  • docs/epics/E05-form-builder/README.md
  • src/Axis.Api/Endpoints/ModelEndpoints.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Application/Commands/AddFieldToForm/AddFieldToFormHandler.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Application/Commands/RemoveFieldFromForm/RemoveFieldFromFormHandler.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Application/Queries/GetFormById/FormFieldDto.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Application/Queries/GetFormById/GetFormByIdHandler.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Application/Repositories/IFormModelReferenceRepository.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Application/Services/IFormModelReferenceSync.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Domain/ReadModels/FormModelReference.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Axis.FormBuilder.Infrastructure.csproj
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Extensions/FormBuilderInfrastructureExtensions.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Handlers/ModelDeletedHandler.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Migrations/20260524150108_AddFormModelReferences.Designer.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Migrations/20260524150108_AddFormModelReferences.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Migrations/FormBuilderDbContextModelSnapshot.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Persistence/Configurations/FormModelReferenceConfiguration.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Persistence/FormBuilderDbContext.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Repositories/FormModelReferenceRepository.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Services/FormModelReferenceSync.cs
  • tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Commands/AddFieldToFormHandlerTests.cs
  • tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Commands/RemoveFieldFromFormHandlerTests.cs
  • tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Queries/GetFormByIdHandlerTests.cs
  • tests/Modules/FormBuilder/Axis.FormBuilder.Infrastructure.Tests/Handlers/ModelDeletedHandlerTests.cs

Comment thread src/Axis.Api/Endpoints/ModelEndpoints.cs Outdated
cursoragent and others added 2 commits May 24, 2026 15:25
- Move US-033 form-reference check to DeleteModelHandler via IModelDeletionGuard
- FormModelDeletionGuard in Axis.Api composition root (no FormBuilder ref in endpoint)
- OpenAPI: ProducesProblem(409) on DELETE /models
- Assert IFormModelReferenceSync in add/remove field handler tests
- Add GetFormById broken-field test; extend ModelDeletedHandler tests

Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/Axis.Api/Infrastructure/FormModelDeletionGuard.cs`:
- Around line 8-17: FormModelDeletionGuard is making an in-process call to
another module via IFormModelReferenceRepository
(CountActiveReferencesToModelAsync) inside ValidateCanDeleteAsync, which breaks
the module-boundary rule; replace this direct repository usage with a
cross-module integration: inject and call a transport-oriented client (e.g.,
IFormModelReferenceClient or IFormModelReferenceGrpcClient/KafkaPublisher) that
implements the contract for querying reference counts via gRPC/Kafka, update
ValidateCanDeleteAsync to call that client method instead of
CountActiveReferencesToModelAsync, and adjust DI registration to bind the new
transport client implementation so all cross-module communication goes through
the contract layer rather than in-process repository calls.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 9da405b0-4e2d-47f9-ae18-bd2446b5f624

📥 Commits

Reviewing files that changed from the base of the PR and between 618699e and 1df4d38.

📒 Files selected for processing (10)
  • src/Axis.Api/Endpoints/ModelEndpoints.cs
  • src/Axis.Api/Infrastructure/FormModelDeletionGuard.cs
  • src/Axis.Api/Program.cs
  • src/Modules/DataModeling/Axis.DataModeling.Application/Commands/DeleteModel/DeleteModelHandler.cs
  • src/Modules/DataModeling/Axis.DataModeling.Application/Services/IModelDeletionGuard.cs
  • tests/Modules/DataModeling/Axis.DataModeling.Application.Tests/Commands/DeleteModelHandlerTests.cs
  • tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Commands/AddFieldToFormHandlerTests.cs
  • tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Commands/RemoveFieldFromFormHandlerTests.cs
  • tests/Modules/FormBuilder/Axis.FormBuilder.Application.Tests/Queries/GetFormByIdHandlerTests.cs
  • tests/Modules/FormBuilder/Axis.FormBuilder.Infrastructure.Tests/Handlers/ModelDeletedHandlerTests.cs
✅ Files skipped from review due to trivial changes (1)
  • src/Axis.Api/Endpoints/ModelEndpoints.cs

Comment on lines +8 to +17
internal sealed class FormModelDeletionGuard(IFormModelReferenceRepository formModelReferences)
: IModelDeletionGuard
{
public async Task<Result> ValidateCanDeleteAsync(
Guid modelId,
Guid organizationId,
CancellationToken cancellationToken = default)
{
int formReferenceCount = await formModelReferences.CountActiveReferencesToModelAsync(
modelId, organizationId, cancellationToken);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Cross-module in-process repository call violates the module boundary rule.

Line 16 invokes IFormModelReferenceRepository from another module’s Application layer directly. This creates synchronous in-process coupling; this check should cross module boundaries via Kafka/gRPC contracts instead of direct Application/Infrastructure calls.

As per coding guidelines, "No in-process method calls into another module's Application or Infrastructure layers; use Kafka events or gRPC calls for cross-module communication".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Axis.Api/Infrastructure/FormModelDeletionGuard.cs` around lines 8 - 17,
FormModelDeletionGuard is making an in-process call to another module via
IFormModelReferenceRepository (CountActiveReferencesToModelAsync) inside
ValidateCanDeleteAsync, which breaks the module-boundary rule; replace this
direct repository usage with a cross-module integration: inject and call a
transport-oriented client (e.g., IFormModelReferenceClient or
IFormModelReferenceGrpcClient/KafkaPublisher) that implements the contract for
querying reference counts via gRPC/Kafka, update ValidateCanDeleteAsync to call
that client method instead of CountActiveReferencesToModelAsync, and adjust DI
registration to bind the new transport client implementation so all cross-module
communication goes through the contract layer rather than in-process repository
calls.

cursoragent and others added 3 commits May 25, 2026 00:45
Shared fixture schema caused IX_form_definitions_organization_id_name
violation when both tests used the same form name "Intake".

Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
Replace in-process FormModelDeletionGuard with FormBuilder gRPC contract.
Move Identity repository usage out of Api endpoints into queries/handlers.
Add GatewayBoundaryTests to catch future Axis.Api cross-module DI.

Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Extensions/DataModelingInfrastructureExtensions.cs (1)

27-32: ⚡ Quick win

Fail fast when Modules:FormBuilder:GrpcUrl is missing.

Defaulting to http://localhost:5280 can hide misconfiguration and route calls to a non-existent/local endpoint outside dev environments.

Suggested change
-        string formBuilderGrpcUrl = configuration["Modules:FormBuilder:GrpcUrl"]
-            ?? "http://localhost:5280";
+        string? formBuilderGrpcUrl = configuration["Modules:FormBuilder:GrpcUrl"];
+        if (string.IsNullOrWhiteSpace(formBuilderGrpcUrl))
+        {
+            throw new InvalidOperationException("Missing configuration 'Modules:FormBuilder:GrpcUrl'.");
+        }
         services.AddGrpcClient<FormModelReferenceService.FormModelReferenceServiceClient>(opts =>
         {
             opts.Address = new Uri(formBuilderGrpcUrl);
         });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Extensions/DataModelingInfrastructureExtensions.cs`
around lines 27 - 32, The code defaults Modules:FormBuilder:GrpcUrl to
"http://localhost:5280", which can hide misconfiguration; update
DataModelingInfrastructureExtensions to fail fast by validating the
configuration key before calling services.AddGrpcClient: read
configuration["Modules:FormBuilder:GrpcUrl"] into formBuilderGrpcUrl and if it's
null or empty throw an InvalidOperationException (or ConfigurationException)
with a clear message instructing to set Modules:FormBuilder:GrpcUrl (mentioning
the configuration key), otherwise use the value when calling
AddGrpcClient<FormModelReferenceService.FormModelReferenceServiceClient>.
src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Grpc/FormModelDeletionGuard.cs (1)

63-63: ⚡ Quick win

Use lowercase gRPC metadata key for auth header

  • Grpc.Core.Metadata.Add normalizes keys to lowercase, so "Authorization" won’t be rejected due to case alone; switching to "authorization" keeps consistency with gRPC metadata conventions and avoids potential interop edge cases.
Proposed fix
-            headers.Add("Authorization", authorization);
+            headers.Add("authorization", authorization);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Grpc/FormModelDeletionGuard.cs`
at line 63, In FormModelDeletionGuard.cs change the gRPC metadata key to
lowercase by updating the call that adds the header (headers.Add(...)) to use
"authorization" instead of "Authorization"; locate the headers.Add invocation
within the FormModelDeletionGuard class/method and replace the header key so it
follows gRPC metadata conventions and avoids interop edge cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/Modules/FormBuilder/Axis.FormBuilder.Contracts/Protos/axis/formbuilder/v1/form_model_reference_service.proto`:
- Line 5: Buf lint fails PACKAGE_DIRECTORY_MATCH because the proto package
declarations axis.formbuilder.v1 and axis.identity.v1 are not located under the
configured Buf root path; either update your buf.yaml or buf.work.yaml
build.roots (or module input roots) so the directory containing axis/ is the
root for the module, or move the proto files so their filesystem paths match
axis/formbuilder/v1 and axis/identity/v1; after adjusting build.roots or
relocating the files, re-run buf lint to confirm PACKAGE_DIRECTORY_MATCH errors
for axis.formbuilder.v1 and axis.identity.v1 are resolved.

In
`@src/Modules/Identity/Axis.Identity.Application/Commands/DeactivateUser/DeactivateUserHandler.cs`:
- Around line 26-33: The code currently falls back to Guid.Empty when
roleRepo.GetByNameAsync("Admin", ...) returns null, which can bypass the
last-admin protection; update DeactivateUserHandler to fail fast if adminRole is
null (throw or return a Result.Failure) instead of using Guid.Empty, so the
subsequent logic that calls CountAdminsAsync(command.OrganizationId,
adminRoleId, ...) and checks user.RoleIds.Contains(adminRoleId) only runs with a
valid adminRole.Id; reference adminRole, roleRepo.GetByNameAsync, adminRoleId,
userRepo.CountAdminsAsync, and user.RoleIds.Contains when making the change.

In
`@src/Modules/Identity/Axis.Identity.Application/Queries/GetUserTokenClaims/GetUserTokenClaimsHandler.cs`:
- Around line 25-27: Ensure GetUserTokenClaimsHandler validates organization
scope: do not let query.OrganizationId silently override user.OrganizationId.
Before calling roleRepo.GetByIdsAsync, check if query.OrganizationId is non-null
and differs from user.OrganizationId; if so, reject the request (throw an
appropriate authorization/validation exception or return an error) rather than
proceeding. Then set orgId to the validated value (user.OrganizationId or the
allowed query.OrganizationId) and only after that call roleRepo.GetByIdsAsync
with the validated orgId.

In `@tests/Architecture/Axis.Architecture.Tests/GatewayBoundaryTests.cs`:
- Around line 25-27: The filter that builds endpointTypes currently checks
t.Namespace == "Axis.Api.Endpoints" which excludes nested namespaces like
Axis.Api.Endpoints.Admin; update the predicate used on api!.GetTypes() (the
endpointTypes assignment) to include nested namespaces by checking t.Namespace
is not null and either equals "Axis.Api.Endpoints" or starts with
"Axis.Api.Endpoints." (e.g., use t.Namespace?.StartsWith("Axis.Api.Endpoints")
or an explicit equals OR starts-with check) so all Endpoint namespaces under
Axis.Api.Endpoints are included for validation.

---

Nitpick comments:
In
`@src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Extensions/DataModelingInfrastructureExtensions.cs`:
- Around line 27-32: The code defaults Modules:FormBuilder:GrpcUrl to
"http://localhost:5280", which can hide misconfiguration; update
DataModelingInfrastructureExtensions to fail fast by validating the
configuration key before calling services.AddGrpcClient: read
configuration["Modules:FormBuilder:GrpcUrl"] into formBuilderGrpcUrl and if it's
null or empty throw an InvalidOperationException (or ConfigurationException)
with a clear message instructing to set Modules:FormBuilder:GrpcUrl (mentioning
the configuration key), otherwise use the value when calling
AddGrpcClient<FormModelReferenceService.FormModelReferenceServiceClient>.

In
`@src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Grpc/FormModelDeletionGuard.cs`:
- Line 63: In FormModelDeletionGuard.cs change the gRPC metadata key to
lowercase by updating the call that adds the header (headers.Add(...)) to use
"authorization" instead of "Authorization"; locate the headers.Add invocation
within the FormModelDeletionGuard class/method and replace the header key so it
follows gRPC metadata conventions and avoids interop edge cases.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6663a51f-fb46-4903-aa34-6e37ec0e41f6

📥 Commits

Reviewing files that changed from the base of the PR and between d7047f0 and 4df257c.

📒 Files selected for processing (26)
  • Directory.Packages.props
  • src/Axis.Api/Endpoints/ConnectEndpoints.cs
  • src/Axis.Api/Endpoints/InvitationEndpoints.cs
  • src/Axis.Api/Endpoints/UserEndpoints.cs
  • src/Axis.Api/Program.cs
  • src/Axis.Api/appsettings.json
  • src/Modules/DataModeling/Axis.DataModeling.Application/Services/IModelDeletionGuard.cs
  • src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Axis.DataModeling.Infrastructure.csproj
  • src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Extensions/DataModelingInfrastructureExtensions.cs
  • src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Grpc/FormModelDeletionGuard.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Contracts/Axis.FormBuilder.Contracts.csproj
  • src/Modules/FormBuilder/Axis.FormBuilder.Contracts/Protos/axis/formbuilder/v1/form_model_reference_service.proto
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Axis.FormBuilder.Infrastructure.csproj
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Extensions/FormBuilderInfrastructureExtensions.cs
  • src/Modules/FormBuilder/Axis.FormBuilder.Infrastructure/Grpc/FormModelReferenceGrpcService.cs
  • src/Modules/Identity/Axis.Identity.Application/Commands/DeactivateUser/DeactivateUserCommand.cs
  • src/Modules/Identity/Axis.Identity.Application/Commands/DeactivateUser/DeactivateUserHandler.cs
  • src/Modules/Identity/Axis.Identity.Application/Queries/GetCurrentUserProfile/GetCurrentUserProfileHandler.cs
  • src/Modules/Identity/Axis.Identity.Application/Queries/GetCurrentUserProfile/GetCurrentUserProfileQuery.cs
  • src/Modules/Identity/Axis.Identity.Application/Queries/GetInvitationByToken/GetInvitationByTokenHandler.cs
  • src/Modules/Identity/Axis.Identity.Application/Queries/GetInvitationByToken/GetInvitationByTokenQuery.cs
  • src/Modules/Identity/Axis.Identity.Application/Queries/GetUserTokenClaims/GetUserTokenClaimsHandler.cs
  • src/Modules/Identity/Axis.Identity.Application/Queries/GetUserTokenClaims/GetUserTokenClaimsQuery.cs
  • tests/Architecture/Axis.Architecture.Tests/GatewayBoundaryTests.cs
  • tests/Architecture/Axis.Architecture.Tests/README.md
  • tests/Modules/Identity/Axis.Identity.Application.Tests/Commands/DeactivateUserHandlerTests.cs
✅ Files skipped from review due to trivial changes (2)
  • src/Modules/DataModeling/Axis.DataModeling.Infrastructure/Axis.DataModeling.Infrastructure.csproj
  • Directory.Packages.props


option csharp_namespace = "Axis.FormBuilder.Contracts.Grpc";

package axis.formbuilder.v1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify Buf configuration roots and reproduce the lint error locally.
fd -i -a 'buf.yaml'
fd -i -a 'buf.work.yaml'
fd -a 'form_model_reference_service.proto'

if command -v buf >/dev/null 2>&1; then
  buf lint
fi

Repository: phuong-labs/axis

Length of output: 812


🏁 Script executed:

#!/bin/bash
set -euo pipefail

fd -i 'buf.yaml' || true
fd -i 'buf.work.yaml' || true
fd -i 'form_model_reference_service.proto' || true

command -v buf >/dev/null 2>&1 && buf lint

Repository: phuong-labs/axis

Length of output: 793


🏁

Length of output: 25


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show Buf roots / inputs that determine the "relative to root" directory used by PACKAGE_DIRECTORY_MATCH
fd -i 'buf.yaml' -a 2>/dev/null || true
fd -i 'buf.work.yaml' -a 2>/dev/null || true

for f in $(fd -i 'buf.yaml' -a 2>/dev/null || true) $(fd -i 'buf.work.yaml' -a 2>/dev/null || true); do
  echo "---- $f ----"
  rg -n '^(.*(build|roots|workspace|modules|breaking|lint|deps|input).*)$' "$f" || true
done

# Re-run lint to ensure the fix addresses the same error
command -v buf >/dev/null 2>&1 && buf lint

Repository: phuong-labs/axis

Length of output: 680


🏁 Script executed:

#!/bin/bash
set -euo pipefail

fd -i 'buf.yaml' -a --type f || true
fd -i 'buf.work.yaml' -a --type f || true

for f in $(fd -i 'buf.yaml' --type f 2>/dev/null || true) $(fd -i 'buf.work.yaml' --type f 2>/dev/null || true); do
  echo "---- $f ----"
  rg -n '(build\.roots|build\.root|modules|workspace|lint|breaking|deps|input[s]?:|roots:)' "$f" || true
done

command -v buf >/dev/null 2>&1 && buf lint || true

Repository: phuong-labs/axis

Length of output: 678


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate any buf config files (buf.yaml/buf.work.yaml or others matching buf*.yml)
mapfile -t cfgs < <(fd -i -g 'buf*.y*ml' --type f .)

for f in "${cfgs[@]}"; do
  echo "---- $f ----"
  rg -n '(build\.roots|build\.root|roots:|input[s]?:|workspace:|modules:)' "$f" || true
done

command -v buf >/dev/null 2>&1 && buf lint

Repository: phuong-labs/axis

Length of output: 117


Fix Buf PACKAGE_DIRECTORY_MATCH for axis.formbuilder.v1 (also affects axis.identity.v1).

buf lint fails because package "axis.formbuilder.v1" must be in axis/formbuilder/v1 relative to the configured Buf root, but the file is under src/Modules/FormBuilder/Axis.FormBuilder.Contracts/Protos/axis/formbuilder/v1/ (and similarly axis.identity.v1 is under src/Modules/Identity/Axis.Identity.Contracts/Protos/axis/identity/v1/). Update buf.yaml/buf.work.yaml build.roots / module input roots so axis/... is at the expected level, or move the proto files to match axis/formbuilder/v1 and axis/identity/v1.

  • Check buf.yaml / buf.work.yaml for build.roots (and related input/module root settings) and align the root so axis/formbuilder/v1 and axis/identity/v1 are directly under it.
  • Re-run buf lint and ensure the PACKAGE_DIRECTORY_MATCH errors are gone.
    [/request_verification]
🧰 Tools
🪛 Buf (1.69.0)

[error] 5-5: Files with package "axis.formbuilder.v1" must be within a directory "axis/formbuilder/v1" relative to root but were in directory "src/Modules/FormBuilder/Axis.FormBuilder.Contracts/Protos/axis/formbuilder/v1".

(PACKAGE_DIRECTORY_MATCH)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/Modules/FormBuilder/Axis.FormBuilder.Contracts/Protos/axis/formbuilder/v1/form_model_reference_service.proto`
at line 5, Buf lint fails PACKAGE_DIRECTORY_MATCH because the proto package
declarations axis.formbuilder.v1 and axis.identity.v1 are not located under the
configured Buf root path; either update your buf.yaml or buf.work.yaml
build.roots (or module input roots) so the directory containing axis/ is the
root for the module, or move the proto files so their filesystem paths match
axis/formbuilder/v1 and axis/identity/v1; after adjusting build.roots or
relocating the files, re-run buf lint to confirm PACKAGE_DIRECTORY_MATCH errors
for axis.formbuilder.v1 and axis.identity.v1 are resolved.

Comment thread tests/Architecture/Axis.Architecture.Tests/GatewayBoundaryTests.cs
cursoragent and others added 2 commits May 25, 2026 03:45
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
Co-authored-by: Phuong Nguyen <phuongnse@users.noreply.github.com>
@phuongnse phuongnse merged commit 343bc33 into main May 25, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants