feat: Restore dialog action #1702
Conversation
…ng granular opting out of individual features.
Completed Endpoint Removed comments
|
Warning Rate limit exceeded@oskogstad has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 25 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive implementation for dialog restoration in the Dialogporten system. The changes span multiple components, including the domain, application, infrastructure, and web API layers. Key modifications include adding a new restoration endpoint, creating domain events for dialog restoration, updating entity handling mechanisms, and introducing more granular filter controls for unit of work operations. The implementation allows for restoring soft-deleted dialogs while preserving their original state and generating appropriate domain events. Changes
Possibly related issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogRestoredDomainEvent.cs (1)
5-10: LGTM! Consider adding XML documentation.The domain event structure is well-designed with appropriate properties for tracking dialog restoration. Consider adding XML documentation to describe the purpose of each property, especially the relationship between
ProcessandPrecedingProcess.public sealed record DialogRestoredDomainEvent( + /// <summary> + /// The unique identifier of the restored dialog. + /// </summary> Guid DialogId, + /// <summary> + /// The service resource associated with the dialog. + /// </summary> string ServiceResource, + /// <summary> + /// The party involved in the dialog. + /// </summary> string Party, + /// <summary> + /// The current process identifier, if any. + /// </summary> string? Process, + /// <summary> + /// The preceding process identifier, if any. + /// </summary> string? PrecedingProcess) : DomainEvent, IProcessEvent;src/Digdir.Library.Entity.EntityFrameworkCore/IEntityOptions.cs (2)
3-47: Consider providing default values documentation.The interface is well-structured and documented. However, it would be beneficial to document the default behavior when these options are not explicitly set.
Add default value documentation to each property:
/// <summary> /// Gets a value indicating whether the soft deletable filter is enabled. + /// Default value: true /// </summary> bool EnableSoftDeletableFilter { get; }
6-47: Consider providing implementation guidance.The interface provides good flexibility for entity handling, but consider adding implementation examples or guidance in the interface documentation.
Add interface-level documentation:
/// <summary> /// Interface for configuring entity handling behavior options. +/// <example> +/// Typical implementation might look like: +/// <code> +/// public class EntityOptions : IEntityOptions +/// { +/// public bool EnableSoftDeletableFilter => true; +/// public bool EnableImmutableFilter => true; +/// // ... other properties +/// } +/// </code> +/// </example> /// </summary> public interface IEntityOptionstests/k6/tests/serviceowner/dialogRestore.js (1)
49-64: Add test for concurrent modification scenario.While the current test thoroughly verifies successful restoration, consider adding a test case for concurrent modification where an incorrect ETag is provided. This would verify the 412 Precondition Failed response.
describe('Restore with incorrect ETag', () => { let incorrectEtag = '00000000-0000-0000-0000-000000000000'; let r = restoreDialog(dialogId, incorrectEtag); expectStatusFor(r).to.equal(412); });src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/DialogDbContext.cs (1)
65-67: Add XML documentation explaining the IsModified state preservation.The logic for preserving the IsModified state is correct, but it would benefit from documentation explaining why this state needs to be preserved and its implications for concurrency control.
+ /// <summary> + /// Attempts to set the original revision while preserving the IsModified state. + /// This is crucial for maintaining proper concurrency control during operations + /// like restoration where we need to track both the original revision and any + /// modifications made during the current transaction. + /// </summary> internal bool TrySetOriginalRevision<TEntity>(
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/Digdir.Domain.Dialogporten.Application/Externals/IUnitOfWork.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Restore/RestoreDialogCommand.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogQuery.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs(2 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogRestoredDomainEvent.cs(1 hunks)src/Digdir.Domain.Dialogporten.Infrastructure/Persistence/DialogDbContext.cs(1 hunks)src/Digdir.Domain.Dialogporten.Infrastructure/UnitOfWork.cs(4 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Restore/RestoreDialogEndpoint.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Restore/RestoreDialogEndpointSummary.cs(1 hunks)src/Digdir.Library.Entity.EntityFrameworkCore/EntityLibraryEfCoreExtensions.cs(2 hunks)src/Digdir.Library.Entity.EntityFrameworkCore/Features/Aggregate/AggregateExtensions.cs(2 hunks)src/Digdir.Library.Entity.EntityFrameworkCore/IEntityOptions.cs(1 hunks)tests/k6/tests/serviceowner/all-tests.js(2 hunks)tests/k6/tests/serviceowner/dialogRestore.js(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Restore/RestoreDialogEndpointSummary.cs
🔇 Additional comments (17)
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Restore/RestoreDialogCommand.cs (1)
59-63: Verify the necessity of disabling multiple filtersLines 59-63 disable several entity framework filters before saving changes. Confirm that disabling
Updatable,SoftDeletable, and enabling concurrency checks are required for the restore operation and do not have unintended side effects on other entities or operations.src/Digdir.Domain.Dialogporten.Infrastructure/UnitOfWork.cs (2)
54-56: Review the impact of movingBeginTransactionAsyncThe
BeginTransactionAsyncmethod has been moved to a different location in the file (lines 54-56). Verify that this relocation does not affect the initialization order or any dependencies that rely on the method's position within the class.
113-116: Ensure proper usage ofHandleAuditableEntitiesIn lines 113-116,
HandleAuditableEntitiesis called with_saveChangesOptions. Confirm that_saveChangesOptionsis configured correctly and that the method handles all necessary entity types as intended.src/Digdir.Domain.Dialogporten.Application/Externals/IUnitOfWork.cs (1)
19-22: Breaking change: Verify all usages of WithoutAggregateSideEffects()The removal of
WithoutAggregateSideEffects()in favor of granular filter controls is a good architectural decision. However, this is a breaking change that requires verification of all existing usages.tests/k6/tests/serviceowner/all-tests.js (1)
12-12: LGTM! Test integration looks good.The dialogRestore test has been properly integrated into the test suite following the existing pattern.
Also applies to: 27-27
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Restore/RestoreDialogEndpoint.cs (3)
20-32: LGTM! Well-structured endpoint configuration.The endpoint follows RESTful conventions and properly documents response types. Authorization is correctly configured using the ServiceProvider policy.
34-50: LGTM! Robust request handling with proper error cases.The implementation correctly:
- Handles concurrency using ETags
- Returns appropriate status codes (204, 404, 412)
- Uses pattern matching for clean error handling
53-59: LGTM! Clean and focused request model.The request model correctly handles the dialog ID and optional ETag for concurrency control.
tests/k6/tests/serviceowner/dialogRestore.js (3)
14-23: LGTM! Well-structured test setup.The helper function and test variables are properly initialized. The restoreDialog function correctly handles the optional ETag parameter.
43-47: LGTM! Good idempotency test.The test correctly verifies that restoring a non-deleted dialog is idempotent by checking both the status code and unchanged ETag.
66-68: LGTM! Proper test cleanup.The cleanup ensures proper test isolation by purging the test dialog.
src/Digdir.Domain.Dialogporten.Domain/Dialogs/Entities/DialogEntity.cs (2)
22-23: LGTM! Appropriate interface implementations.The entity correctly implements IEventPublisher and IAggregateRestoredHandler to support the restoration functionality.
88-90: Consider handling related entities during restoration.While the basic restoration event is correctly published, consider whether related entities (Transmissions, Content, SearchTags, etc.) need special handling during restoration. For example, you might need to:
- Restore related entities' states
- Validate relationships
- Update timestamps
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogQuery.cs (1)
118-119: LGTM! Granular control over entity tracking.The replacement of
.WithoutAggregateSideEffects()with.DisableUpdatableFilter()and.DisableVersionableFilter()provides more precise control over which entity tracking features are disabled when saving changes.src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs (1)
129-130: LGTM! Consistent implementation of granular entity tracking.The changes align with the service owner implementation, maintaining consistency across the codebase in how entity tracking is controlled.
src/Digdir.Library.Entity.EntityFrameworkCore/Features/Aggregate/AggregateExtensions.cs (2)
18-18: LGTM! Enhanced flexibility with options parameter.The addition of
IEntityOptionsparameter allows for more configurable entity handling behavior.
27-57: LGTM! Well-structured conditional feature handling.The implementation properly segregates different entity features:
- Aggregate operations (create, update, delete, restore)
- Updatable entity handling
- Versionable entity handling
Each feature can now be independently controlled through the options parameter, improving modularity and maintainability.
...porten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Restore/RestoreDialogCommand.cs
Show resolved
Hide resolved
...porten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Restore/RestoreDialogCommand.cs
Show resolved
Hide resolved
src/Digdir.Library.Entity.EntityFrameworkCore/EntityLibraryEfCoreExtensions.cs
Show resolved
Hide resolved
src/Digdir.Library.Entity.EntityFrameworkCore/EntityLibraryEfCoreExtensions.cs
Show resolved
Hide resolved
|
Caution No docstrings were generated. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Digdir.Domain.Dialogporten.Infrastructure/UnitOfWork.cs (2)
57-79: Consider adding XML documentation for the filter methods.The implementation looks good and follows a consistent pattern. Consider adding XML documentation to describe the purpose and impact of disabling each filter, especially since these methods are part of the public API.
209-219: Consider documenting the impact of each filter.The
SaveChangesOptionsclass implementation looks good, but it would be helpful to document:
- The purpose of each filter
- The impact of disabling each filter
- Which scenarios might require disabling specific filters
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Digdir.Domain.Dialogporten.Infrastructure/UnitOfWork.cs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Dry run deploy apps / Deploy job sync-resource-policy-information-job to test
- GitHub Check: Dry run deploy apps / Deploy job sync-subject-resource-mappings-job to test
- GitHub Check: build / build-and-test
🔇 Additional comments (4)
src/Digdir.Domain.Dialogporten.Infrastructure/UnitOfWork.cs (4)
28-28: LGTM!The field initialization follows proper conventions and immutability principles.
54-56: LGTM!The method is concise and correctly handles transaction initialization using null-coalescing assignment.
205-208: Documentation improvement appreciated.The comment now better explains the rationale for limiting setters without the previous negative connotation.
113-116: Verify Entity library compatibility.The change in the
HandleAuditableEntitiesmethod signature suggests a breaking change in the Entity library. Please ensure that the Entity library version is compatible with this change.✅ Verification successful
Implementation is compatible with Entity library
The
HandleAuditableEntitiesimplementation inEntityLibraryEfCoreExtensions.csmatches the usage inUnitOfWork.cs. Since both the Entity library and its consumers are part of the same solution, the changes are properly coordinated.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check Entity library version and changes rg -l "HandleAuditableEntities.*IEntityOptions" . cat $(fd -g "*csproj" .) | grep -A 5 "Digdir.Library.Entity"Length of output: 3844
Script:
#!/bin/bash # Find HandleAuditableEntities implementation and related types ast-grep --pattern 'public $_ HandleAuditableEntities($_)' rg -A 5 "HandleAuditableEntities" . rg -l "interface.*EntityOptions" .Length of output: 1499
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Restore/RestoreDialogEndpointSummary.cs (1)
16-19: Consider adding 409 Conflict response.The endpoint should handle the case where a dialog cannot be restored because it's already active.
Add the following response code:
Responses[StatusCodes.Status204NoContent] = Constants.SwaggerSummary.Restored.FormatInvariant("aggregate"); Responses[StatusCodes.Status404NotFound] = Constants.SwaggerSummary.DialogNotFound; Responses[StatusCodes.Status412PreconditionFailed] = Constants.SwaggerSummary.RevisionMismatch; +Responses[StatusCodes.Status409Conflict] = "Dialog cannot be restored because it is already active.";docs/schema/V1/swagger.verified.json (4)
6373-6373: Consider enhancing the endpoint description.The description could be more detailed to explain:
- What happens when a dialog is restored
- Any side effects or constraints
- Any related activity events that are generated
6399-6401: Add more details to the 401 error description.The 401 error description should specify the required scope, similar to other endpoints in the API that mention "Requires a Maskinporten-token with the scope 'digdir:dialogporten.serviceprovider'".
6402-6404: Add more details to the 403 error description.The 403 error description should explain the authorization requirements, similar to other endpoints that mention "Unauthorized to restore the supplied dialog (not owned by authenticated organization or has additional scope requirements defined in policy)".
6396-6398: Add ETag header to successful response.For consistency with other endpoints that modify dialogs (create, update, delete), the 204 response should include the ETag header to return the new revision.
Apply this change to add the ETag header:
"204": { - "description": "The dialog aggregate was restored successfully." + "description": "The dialog aggregate was restored successfully.", + "headers": { + "Etag": { + "description": "The new UUID ETag of the dialog", + "example": "123e4567-e89b-12d3-a456-426614174000", + "schema": { + "type": "string" + } + } + } },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/schema/V1/swagger.verified.json(2 hunks)src/Digdir.Domain.Dialogporten.WebApi/Common/Constants.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Restore/RestoreDialogEndpointSummary.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build / build-and-test
🔇 Additional comments (5)
src/Digdir.Domain.Dialogporten.WebApi/Common/Constants.cs (1)
16-16: LGTM! Well-structured constant addition.The new
Restoredconstant follows the established pattern and maintains consistency with other dialog operation messages. It's appropriately positioned within theSwaggerSummaryclass and will integrate well with the new dialog restoration functionality.src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Restore/RestoreDialogEndpointSummary.cs (3)
1-5: LGTM! Clean and minimal imports.The imports are relevant and the namespace follows the project's structure.
7-10: LGTM! Follows FastEndpoints conventions.The class is appropriately sealed and inherits from the correct base class.
11-14: Update the documentation link.The description contains a placeholder "(link TBD)" that needs to be updated with the actual documentation link.
Would you like me to help create a documentation issue to track this? The PR objectives mention that documentation updates should be made either in the
docsdirectory, Altinnpedia, or through a separate PR in the Altinn studio documentation repository.docs/schema/V1/swagger.verified.json (1)
6371-6436: LGTM! The restore dialog endpoint is well-defined.The endpoint follows REST best practices:
- Uses POST for the restore action
- Follows the resource/action pattern in the URL
- Includes proper optimistic concurrency control via If-Match header
- Has appropriate response codes and descriptions
- Requires proper authentication and authorization
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/Common/Events/DomainEventsTests.cs (1)
256-256: Use consistent test data generation method.The test uses
GenerateSimpleFakeCreateDialogCommandwhile other tests useGenerateFakeCreateDialogCommand. Consider using the same method for consistency.Apply this diff to align with other tests:
- var createDialogCommand = DialogGenerator.GenerateSimpleFakeCreateDialogCommand(dialogId); + var createDialogCommand = DialogGenerator.GenerateFakeCreateDialogCommand( + id: dialogId, + attachments: [], + activities: []);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/schema/V1/swagger.verified.json(2 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/CloudEventTypes.cs(1 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs(1 hunks)src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogRestoredDomainEvent.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Common/Constants.cs(1 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/Common/Events/DomainEventsTests.cs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Digdir.Domain.Dialogporten.Domain/Dialogs/Events/DialogRestoredDomainEvent.cs
- src/Digdir.Domain.Dialogporten.WebApi/Common/Constants.cs
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Dry run deploy apps / Deploy job sync-resource-policy-information-job to test
- GitHub Check: Dry run deploy apps / Deploy job sync-subject-resource-mappings-job to test
🔇 Additional comments (3)
src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/CloudEventTypes.cs (1)
11-11: LGTM!The new cloud event type follows the established naming convention and is correctly placed in the "Dialog" section.
docs/schema/V1/swagger.verified.json (2)
6382-6447: New endpoint for restoring soft-deleted dialogsThe new endpoint
/api/v1/serviceowner/dialogs/{dialogId}/actions/restorehas been added with the following characteristics:
- HTTP Method: POST
- Path Parameters: dialogId (GUID)
- Headers: if-Match (optional GUID for optimistic concurrency)
- Authentication: JWT Bearer token required
- Response Codes:
- 204: Successful restoration
- 401: Unauthorized
- 403: Forbidden
- 404: Dialog not found
- 412: Precondition failed (if-Match header mismatch)
The endpoint follows RESTful principles and matches the pattern of other action endpoints in the API.
Let's verify the consistency of the restore functionality across the API:
✅ Verification successful
Restore functionality is implemented consistently.
- The
/api/v1/serviceowner/dialogs/{dialogId}/actions/restoreendpoint is properly defined with the expected POST method, parameters, and responses.- The domain events and activity types (e.g.,
DialogRestored) are integrated across the codebase—evidenced in the domain events, migrations, and tests.- The command handler (
RestoreDialogCommandHandler) and endpoint implementations confirm the correct implementation of restore logic and validations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the restore functionality is consistently implemented # Test 1: Check if restore is properly handled in activity types ast-grep --pattern 'DialogActivityType" { $$$ "DialogRestored" $$$ }' # Test 2: Check if restore is handled in domain events rg -A 5 "DialogRestored" # Test 3: Check for restore-related handlers and validators rg -A 5 "RestoreDialog"Length of output: 23743
7131-7135: Verify server URL configurationThe server URL is configured to point to the development environment. Ensure this is the intended configuration for the swagger documentation.
Let's check if there are any environment-specific configurations:
✅ Verification successful
Action: Confirm Intended Swagger Server URL Configuration
The shell script output shows that the development URL ("https://altinn-dev-api.azure-api.net/dialogporten") is consistently used not only in the Swagger documentation (docs/schema/V1/swagger.verified.json) but also across several test and configuration files. This indicates that the current configuration is intentionally pointing to the development environment.
- The Swagger file and multiple test configurations (e.g., in appsettings.test.json files and tests/k6 configurations) all use the same development URL.
- No conflicting environment-specific URLs were found in the relevant configurations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for environment-specific configurations # Test: Look for environment-specific URLs rg -A 5 "altinn-dev-api.azure-api.net"Length of output: 3346
...in.Dialogporten.Application.Integration.Tests/Features/V1/Common/Events/DomainEventsTests.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
...porten.Application/Features/V1/ServiceOwner/Dialogs/Commands/Restore/RestoreDialogCommand.cs
Show resolved
Hide resolved
...ialogporten.WebApi/Endpoints/V1/ServiceOwner/Dialogs/Restore/RestoreDialogEndpointSummary.cs
Show resolved
Hide resolved
Co-authored-by: Magnus Sandgren <5285192+MagnusSandgren@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Endpoints.verified.txt (1)
8-8: Formatting Consistency Notice – End Bracket.
It appears the closing bracket line has been modified (possibly removing or adjusting a trailing newline). Verify that this formatting aligns with the project’s style guidelines to maintain consistency across test files.tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Events.verified.txt (1)
8-8: Minor Formatting Change – Review End Bracket Syntax.
The change on the closing bracket (line 8) may indicate a modification in newline handling or spacing. Please confirm that this style conforms with other test files in the repository.src/Digdir.Domain.Dialogporten.Infrastructure/UnitOfWork.cs (1)
205-208: Improve comment clarity.The current comment is unclear and has grammatical issues. Consider this revision:
- // Although Digdir.Library.Entity.EntityFrameworkCore supports all the options, - // But we only have use cases for some of them. Therefore, - // only some of them have setters until the day we actually - // have a use case for them. + // Although Digdir.Library.Entity.EntityFrameworkCore supports all options, + // we currently only have use cases for some of them. Therefore, + // only the necessary options have setters enabled. Additional + // setters will be enabled when use cases arise.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Digdir.Domain.Dialogporten.Application/Features/V1/Common/Events/AltinnForwarders/DialogEventToAltinnForwarder.cs(3 hunks)src/Digdir.Domain.Dialogporten.Infrastructure/UnitOfWork.cs(4 hunks)tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/Common/Events/DomainEventsTests.cs(2 hunks)tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Endpoints.verified.txt(1 hunks)tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Events.verified.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Digdir.Domain.Dialogporten.Application.Integration.Tests/Features/V1/Common/Events/DomainEventsTests.cs
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Dry run deploy apps / Deploy job sync-resource-policy-information-job to test
- GitHub Check: Dry run deploy apps / Deploy job sync-subject-resource-mappings-job to test
- GitHub Check: build / build-and-test
🔇 Additional comments (3)
tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Endpoints.verified.txt (1)
5-5: Added New Restoration Event Entry.
The new eventDialogEventToAltinnForwarder_DialogRestoredDomainEventhas been inserted into the list, which aligns with the new dialog restoration feature. Please ensure that any related documentation and test references are updated accordingly.tests/Digdir.Domain.Dialogporten.Application.Unit.Tests/Features/V1/Common/Utils/ApplicationEventHandlerUtilsTests.Developer_Should_Use_Caution_When_Modifying_Events.verified.txt (1)
5-5: New Domain Event for Dialog Restoration Added.
The eventDigdir.Domain.Dialogporten.Domain.Dialogs.Events.DialogRestoredDomainEventhas been added to the events list. This change supports the restoration workflow by including the new domain event. Double-check that the event’s structure and its usage in the domain logic are consistent with existing events.src/Digdir.Domain.Dialogporten.Infrastructure/UnitOfWork.cs (1)
57-79: LGTM! Well-structured filter control implementation.The implementation of granular filter control is well-designed:
- Each filter has a dedicated method for disabling it
- Methods follow a consistent pattern
- Good use of fluent interface pattern with method chaining
- Clear separation of concerns
...orten.Application/Features/V1/Common/Events/AltinnForwarders/DialogEventToAltinnForwarder.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/schema/V1/swagger.verified.json (1)
6424-6424: Clarify error description for dialog restoreThe error description "The given dialog ID was not found or is already deleted" is ambiguous and could be improved. A deleted dialog should be restorable, so "already deleted" shouldn't be an error condition.
Consider updating the error description to be more precise:
-"description": "The given dialog ID was not found or is already deleted." +"description": "The given dialog ID was not found."
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/schema/V1/swagger.verified.json(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Common/Constants.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Digdir.Domain.Dialogporten.WebApi/Common/Constants.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Dry run deploy apps / Deploy job sync-subject-resource-mappings-job to test
🔇 Additional comments (1)
docs/schema/V1/swagger.verified.json (1)
6382-6447: New endpoint for restoring soft-deleted dialogsThe new endpoint
/api/v1/serviceowner/dialogs/{dialogId}/actions/restorehas been added with appropriate security controls and error handling:
- Authentication & Authorization:
- Requires JWT bearer authentication
- Proper 401/403 responses for unauthorized access
- Input Validation:
- Required dialogId path parameter
- Optional if-Match header for optimistic concurrency control
- Error Handling:
- 404 for non-existent dialogs
- 412 for concurrency conflicts
- Returns problem details for errors
- Response:
- 204 No Content on successful restore
Let's verify the endpoint's integration with the domain events system:
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [1.49.0](v1.48.5...v1.49.0) (2025-02-10) ### Features * Restore dialog action ([#1702](#1702)) ([331d492](331d492)) * **webapi:** Option to include deleted dialogs in ServiceOwner dialog search ([#1816](#1816)) ([5403063](5403063)) ### Miscellaneous Chores * **ci:** Increase container app verification timeout ([#1819](#1819)) ([fe5377a](fe5377a)) * **deps:** update dependency fastendpoints.swagger to 5.34.0 ([#1822](#1822)) ([8ce7e0e](8ce7e0e)) * **deps:** update dependency uuidnext to 4.1.0 ([#1823](#1823)) ([dfa5ce0](dfa5ce0)) * **deps:** update masstransit monorepo to 8.3.6 ([#1821](#1821)) ([6fbb41f](6fbb41f)) * refactor and add filters for telemetry ([#1813](#1813)) ([fd10351](fd10351)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Created new endpoint for restoration of soft deleted dialogs
Modified enitity librarty enabling granular opting out of individual features
Related Issue(s)
#1543
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)