feat(app): Add ExternalReference to Transmission #2348
Conversation
📝 WalkthroughWalkthroughA new nullable string property, Changes
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
| @@ -11,5 +11,7 @@ public void Configure(EntityTypeBuilder<DialogTransmission> builder) | |||
| builder.HasOne(x => x.RelatedTransmission) | |||
| .WithMany(x => x.RelatedTransmissions) | |||
| .OnDelete(DeleteBehavior.SetNull); | |||
|
|
|||
| builder.HasIndex(x => x.ExternalReference); | |||
There was a problem hiding this comment.
Lag et partial filter
There was a problem hiding this comment.
@MagnusSandgren
Ble vi enige om å droppe index her? 🤔
Hvis det ikke skal inn som søkeparameter
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20250526153145_AddTransmissionExternalReference.Designer.csis excluded by!**/Migrations/**/*Designer.cssrc/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/DialogDbContextModelSnapshot.csis excluded by!**/Migrations/DialogDbContextModelSnapshot.cs
📒 Files selected for processing (12)
docs/schema/V1/swagger.verified.json(6 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Search/TransmissionDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/Validators/CreateDialogDialogTransmissionDtoValidator.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/Validators/UpdateDialogDialogTransmissionDtoValidator.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/DialogDto.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/Transmissions/DialogTransmission.cs(1 hunks)src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20250526153145_AddTransmissionExternalReference.cs(1 hunks)src/Digdir.Library.Dialogporten.WebApiClient/Features/V1/RefitterInterface.cs(5 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/CreateTransmissionTests.cs(2 hunks)tests/Digdir.Domain.Dialogporten.GraphQl.Unit.Tests/ObjectTypes/TransmissionTests.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/Digdir.Domain.Dialogporten.GraphQl.Unit.Tests/ObjectTypes/TransmissionTests.cs (1)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/Transmissions/DialogTransmission.cs (1)
DialogTransmission(14-67)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Dry run deploy apps / Deploy graphql to test
- GitHub Check: Dry run deploy apps / Deploy service to test
- GitHub Check: Dry run deploy apps / Deploy web-api-eu to test
- GitHub Check: Dry run deploy apps / Deploy web-api-so to test
- GitHub Check: build / build-and-test
🔇 Additional comments (20)
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/Validators/CreateDialogDialogTransmissionDtoValidator.cs (1)
27-28: LGTM! Validation follows established patterns.The validation rule for
ExternalReferencecorrectly applies a maximum length constraint and follows the same pattern as other string property validations in this validator.src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/Validators/UpdateDialogDialogTransmissionDtoValidator.cs (1)
27-28: Excellent consistency between validators.The validation rule for
ExternalReferencematches the create validator implementation, ensuring consistent validation behavior across create and update operations.tests/Digdir.Domain.Dialogporten.GraphQl.Unit.Tests/ObjectTypes/TransmissionTests.cs (1)
19-20: Verify that excluding ExternalReference from GraphQL is intentional.The test correctly excludes
ExternalReferencefrom GraphQL property matching. Please confirm this design decision aligns with requirements - isExternalReferenceintended to be an internal/administrative field not exposed to GraphQL consumers?src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/Transmissions/DialogTransmission.cs (1)
25-25: Clean domain entity property addition.The
ExternalReferenceproperty is well-placed and appropriately typed as a nullable string for an optional external reference field.src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Create/CreateDialogDto.cs (4)
57-59: LGTM: Consistent implementation of ExternalReference propertyThe addition of the
ExternalReferenceproperty to theCreateDialogDtoclass is well-implemented with clear documentation and appropriate nullable string type for an optional service-specific reference.
196-199: LGTM: Consistent property addition to TransmissionDtoThe
ExternalReferenceproperty is consistently implemented in theTransmissionDtoclass with matching documentation and type definition. The positioning near other optional properties is logical.
57-60: Well-implemented feature addition.The
ExternalReferenceproperty is properly implemented with clear documentation and appropriate nullable string type. The placement alongsideExtendedStatusmakes logical sense for service-specific metadata.
196-200: Consistent implementation in TransmissionDto.The
ExternalReferenceproperty follows the same pattern as the main dialog DTO with consistent documentation and type. The implementation maintains consistency across the nested DTO structure.src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Update/UpdateDialogDto.cs (4)
28-30: LGTM: Consistent ExternalReference implementation across DTOsThe
ExternalReferenceproperty addition toUpdateDialogDtomaintains consistency with the create DTO, using the same type, documentation, and logical positioning.
147-150: LGTM: Aligned implementation in TransmissionDtoThe
ExternalReferenceproperty implementation in the updateTransmissionDtois consistent with the create version, ensuring API coherence across different operations.
28-31: Consistent implementation across create and update DTOs.The
ExternalReferenceproperty implementation mirrors the create DTO with identical documentation and type. This maintains consistency across the command operations.
147-151: Proper transmission DTO consistency.The nested
TransmissionDtoimplementation maintains the same pattern as the create command, ensuring consistent API behavior across all dialog operations.src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Search/TransmissionDto.cs (2)
31-34: LGTM: Consistent ExternalReference in query DTOThe
ExternalReferenceproperty implementation in the search queryTransmissionDtomaintains consistency with command DTOs, ensuring the property is available across all transmission-related operations.
31-35: Query DTO maintains consistency with command operations.The
ExternalReferenceproperty implementation in the query DTO is consistent with the command DTOs, ensuring uniform API behavior across all transmission-related operations.src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/Migrations/20250526153145_AddTransmissionExternalReference.cs (1)
1-29: LGTM! Well-implemented EF Core migration.The migration correctly adds the
ExternalReferencecolumn to theDialogTransmissiontable with appropriate constraints:
- Nullable string allowing optional external references
- 255 character limit provides reasonable length for external system identifiers
- Proper rollback capability in the
Downmethod- Follows EF Core migration best practices
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/CreateTransmissionTests.cs (2)
174-198: Excellent test coverage for ExternalReference functionality.This test effectively validates the core functionality by:
- Creating transmissions with distinct external reference values
- Verifying persistence through the full create-retrieve cycle
- Using appropriate assertions to confirm both transmissions are stored correctly
- Following the established AAA pattern and testing conventions
200-219:✅ Verification successful
Good validation test for max length constraint.
The test properly validates the ExternalReference length constraint by creating an oversized string and verifying the validation error response. The test follows established patterns and uses appropriate assertions.
Verify that
Constants.DefaultMaxStringLengthaligns with the migration's 255 character limit:
🏁 Script executed:
#!/bin/bash # Description: Verify that the constant used in tests matches the migration constraint # Expected: Constants.DefaultMaxStringLength should be 255 to match the migration # Search for the constant definition rg -A 2 "DefaultMaxStringLength.*=" --type cs # Search for any other references to this constant in relation to ExternalReference rg "DefaultMaxStringLength.*ExternalReference|ExternalReference.*DefaultMaxStringLength" --type csLength of output: 664
Constant Alignment Verified: DefaultMaxStringLength = 255
The test’s use of
Constants.DefaultMaxStringLength + 1correctly exceeds the 255-character limit defined in the database migration. No changes needed.• Constant defined in
src/Digdir.Domain.Dialogporten.Domain/Common/Constants.cs
• Referenced intests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/ServiceOwner/Transmissions/Commands/CreateTransmissionTests.cssrc/Digdir.Library.Dialogporten.WebApiClient/Features/V1/RefitterInterface.cs (1)
926-931: LGTM! Consistent auto-generated ExternalReference property additions.The auto-generated code correctly adds the
ExternalReferenceproperty across all transmission-related DTOs with consistent:
- Type definition (nullable string)
- JSON serialization (
[JsonPropertyName("externalReference")])- Documentation describing it as a service-specific external reference
- Additive changes that maintain backwards compatibility
The pattern is consistently applied across search, create, update, and get operations for transmissions.
Also applies to: 1430-1435, 1913-1918, 3133-3138, 3922-3927
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/DialogDto.cs (2)
85-91: Enhanced documentation provides good user guidance.The
ExternalReferenceproperty implementation includes helpful additional documentation directing users to service-specific documentation. This enhances the API usability while maintaining consistency with other DTOs.
227-231: Complete feature implementation across all DTO levels.The
DialogTransmissionDtoimplementation completes the consistent addition ofExternalReferenceacross all dialog and transmission DTOs, ensuring comprehensive feature coverage.
Description
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)