fix(webapi): Return 410 GONE for sub-resources on soft-deleted dialogs #1564
Conversation
📝 WalkthroughWalkthroughThis pull request introduces several modifications to the Dialogporten API, primarily focusing on enhancing the OpenAPI specification and response handling for various endpoints. Key updates include the addition of the Changes
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: 3
🧹 Outside diff range and nitpick comments (4)
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Search/SearchDialogActivityEndpointSummary.cs (1)
20-21: Consider documenting the soft-deletion behavior in API guidelines.The consistent implementation of 410 Gone across endpoints is excellent. Consider documenting this behavior in your API guidelines to ensure:
- Other teams understand when to expect 410 vs 404
- Future endpoints maintain this consistency
- API consumers understand the implications of soft-deletion on sub-resources
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Search/SearchDialogSeenLogEndpoint.cs (1)
27-28: LGTM! Consider adding XML documentation.The addition of
Status410Goneto the response description is correct and aligns with the PR objectives for handling soft-deleted dialogs.Consider adding XML documentation to describe the conditions under which each status code is returned:
/// <summary> /// Search dialog seen logs /// </summary> /// <response code="200">Returns the list of seen logs</response> /// <response code="404">Dialog not found</response> /// <response code="410">Dialog has been deleted</response>src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/GetTransmissionQuery.cs (1)
64-68: LGTM: Consistent implementation of soft-delete handlingThe implementation follows the same pattern as other handlers, maintaining consistency across the codebase.
Consider extracting the common soft-delete check pattern into a shared extension method or base class, as this pattern is repeated across multiple query handlers. This would improve maintainability and reduce the risk of inconsistent implementations.
Example extension method:
public static class DialogExtensions { public static OneOf<T, EntityNotFound, EntityDeleted> HandleDeletedDialog<T>( this Dialog dialog, Guid dialogId) { if (dialog is null) return new EntityNotFound<DialogEntity>(dialogId); if (dialog.Deleted) return new EntityDeleted<DialogEntity>(dialogId); return default; } }docs/schema/V1/swagger.verified.json (1)
6582-6584: Add descriptive messages for 410 Gone responses.The 410 Gone responses have been correctly added to all sub-resource endpoints, but some are missing descriptive messages. Consider adding consistent descriptions across all endpoints.
Apply this pattern to all 410 responses:
"410": { - "description": "Entity with the given key(s) is removed." + "description": "The requested resource exists but has been soft-deleted. The client should not request this resource again." }Also applies to: 6643-6644, 6709-6710, 6766-6767, 6940-6941
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (19)
docs/schema/V1/swagger.verified.json(5 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Get/GetActivityQuery.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Search/SearchActivityQuery.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Get/GetSeenLogQuery.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Search/SearchSeenLogQuery.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/GetTransmissionQuery.cs(3 hunks)src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Search/SearchTransmissionQuery.cs(2 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Get/GetDialogActivityEndpoint.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Get/GetDialogActivitySwaggerConfig.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Search/SearchDialogActivityEndpoint.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Search/SearchDialogActivityEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Get/GetDialogSeenLogEndpoint.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Get/GetDialogSeenLogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Search/SearchDialogSeenLogEndpoint.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Search/SearchDialogSeenLogEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Get/GetDialogTransmissionEndpoint.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Get/GetDialogTransmissionEndpointSummary.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Search/SearchDialogTransmissionEndpoint.cs(1 hunks)src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Search/SearchDialogTransmissionEndpointSummary.cs(1 hunks)
🔇 Additional comments (32)
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Get/GetDialogSeenLogEndpointSummary.cs (1)
17-18: LGTM! Appropriate status codes added for resource state handling
The addition of both 404 (Not Found) and 410 (Gone) status codes is correct and follows HTTP specifications. Using 410 Gone for deleted dialogs provides better semantics than a generic 404, as it explicitly indicates that the resource existed but was removed.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Search/SearchDialogSeenLogEndpointSummary.cs (1)
18-19: LGTM! Consistent implementation across endpoints
The changes maintain consistency with the Get endpoint by adding the same status codes (404 and 410) and using the same swagger summary constants. This ensures uniform behavior across the API for handling non-existent and deleted resources.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Search/SearchDialogActivityEndpoint.cs (1)
30-31: LGTM! Comprehensive response handling implemented
The Match pattern correctly handles all possible cases (success, not found, and deleted) with appropriate response status codes. The use of extension methods ensures consistent response formatting across the API.
Let's verify that this pattern is consistently implemented across all similar endpoints:
✅ Verification successful
Pattern is consistently implemented across all endpoints
The verification shows that the GoneAsync response pattern is consistently implemented across all relevant endpoints in the codebase. All endpoints that handle deleted resources use the same pattern:
- Service Owner endpoints use
gone => this.GoneAsync(gone, ct) - End User endpoints use
deleted => this.GoneAsync(deleted, ct)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent implementation of GoneAsync across endpoints
# Expected: All endpoints handling deleted resources should use GoneAsync
# Search for Match patterns in endpoint files
rg -A 3 "await result\.Match\(" "src/Digdir.Domain.Dialogporten.WebApi/Endpoints/"
# Search for GoneAsync usage
rg "GoneAsync" "src/Digdir.Domain.Dialogporten.WebApi/Endpoints/"
Length of output: 19129
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Get/GetDialogActivitySwaggerConfig.cs (1)
20-20: LGTM! Verify documentation constant.
The addition of the 410 Gone status code is appropriate for indicating soft-deleted dialogs. This aligns with REST API best practices and the PR objectives.
Let's verify the constant's message is appropriate:
✅ Verification successful
Documentation constant is appropriate and consistent
The constant Constants.SwaggerSummary.DialogDeleted is defined with a clear and accurate message: "Entity with the given key(s) is removed." This description is appropriate for a 410 Gone response and aligns well with REST API conventions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the DialogDeleted constant definition
rg -l "DialogDeleted\s*=" --type cs
Length of output: 244
Script:
#!/bin/bash
# Get the content of the DialogDeleted constant
rg "DialogDeleted\s*=" -A 1 --type cs
Length of output: 746
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Get/GetDialogTransmissionEndpointSummary.cs (1)
20-20: LGTM! Consistent implementation.
The addition of the 410 Gone status maintains consistency with other endpoints and follows the same pattern, which is crucial for API predictability.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Search/SearchDialogActivityEndpointSummary.cs (1)
21-21: LGTM! Verify handling in search implementation.
The addition of 410 Gone status is consistent with other endpoints. Since this is a search endpoint, ensure the corresponding endpoint implementation correctly handles soft-deleted dialogs.
Let's verify the endpoint implementation:
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Search/SearchDialogTransmissionEndpointSummary.cs (1)
21-21: LGTM! Verify the DialogDeleted message content.
The addition of the 410 Gone response is consistent with the PR objectives and follows the established pattern for response documentation.
Let's verify the content of the DialogDeleted constant to ensure it provides a clear message:
✅ Verification successful
LGTM! The DialogDeleted constant message is clear and appropriate.
The constant DialogDeleted in Constants.SwaggerSummary is defined with a clear and descriptive message: "Entity with the given key(s) is removed." This message appropriately describes the 410 Gone status code scenario and aligns well with REST API conventions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the DialogDeleted constant definition
rg -l "DialogDeleted\s*=" --type cs
Length of output: 244
Script:
#!/bin/bash
# Get the content of DialogDeleted constant
rg "DialogDeleted\s*=" -A 1 --type cs
Length of output: 746
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Get/GetDialogSeenLogEndpoint.cs (2)
27-28: LGTM! Response configuration is properly documented.
The addition of Status410Gone to ProducesOneOf ensures proper OpenAPI documentation.
36-37: Verify the GoneAsync extension method implementation.
The pattern matching with GoneAsync extension method looks good, but let's verify its implementation for consistency.
✅ Verification successful
The GoneAsync extension method implementation is verified and consistent.
The implementation is found in src/Digdir.Domain.Dialogporten.WebApi/Common/Extensions/EndpointExtensions.cs and correctly:
- Takes an
EntityDeletedparameter and optional cancellation token - Returns HTTP 410 Gone status code
- Converts the deleted entity to validation results
- Sends the error response asynchronously
The method is used consistently across multiple endpoints in the codebase for handling deleted entity scenarios.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the GoneAsync extension method implementation
ast-grep --pattern 'static $_ GoneAsync$_($_) { $$$ }'
Length of output: 56
Script:
#!/bin/bash
# Let's try a broader search for GoneAsync using ripgrep
rg -l "GoneAsync" --type-add 'csharp:*.cs' --type csharp
# Then let's look at the content of any files that contain this method
rg "GoneAsync" --type-add 'csharp:*.cs' --type csharp -C 5
Length of output: 31238
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Get/GetDialogActivityEndpoint.cs (1)
26-27: LGTM! Implementation maintains consistency across endpoints.
The changes follow the same pattern as other endpoints, which is excellent for maintainability:
- Properly documents 410 Gone in ProducesOneOf
- Consistently handles deleted case using pattern matching
Also applies to: 35-36
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogSeenLogs/Search/SearchDialogSeenLogEndpoint.cs (1)
36-37: LGTM! Proper handling of deleted case.
The implementation correctly handles the deleted case using the Match pattern, maintaining consistency with the API's error handling approach.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Get/GetDialogTransmissionEndpoint.cs (1)
36-37: LGTM! Proper error handling implementation.
The implementation correctly handles both not found and deleted cases using the Match pattern.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Search/SearchDialogTransmissionEndpoint.cs (1)
36-37: LGTM! Consistent error handling.
The implementation correctly handles both not found and deleted cases, maintaining consistency with other endpoints.
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Search/SearchActivityQuery.cs (3)
19-19: LGTM: Result type extension
The addition of EntityDeleted to OneOfBase aligns with the PR objective to handle soft-deleted dialogs.
41-42: LGTM: Improved formatting
The line break in the WhereIf clause improves code readability.
51-54: LGTM: Soft deletion handling
Correctly implements soft deletion check to return EntityDeleted status for deleted dialogs.
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Search/SearchSeenLogQuery.cs (3)
19-19: LGTM: Result type extension
The addition of EntityDeleted to OneOfBase maintains consistency with other query handlers.
46-47: LGTM: Improved formatting
The line break in the WhereIf clause maintains consistent formatting across files.
56-59: LGTM: Soft deletion handling
Correctly implements soft deletion check, maintaining consistency with other query handlers.
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogSeenLogs/Queries/Get/GetSeenLogQuery.cs (4)
20-20: LGTM: Result type extension
The addition of EntityDeleted to OneOfBase maintains consistency with other query handlers.
48-49: LGTM: Improved formatting
The line break in the WhereIf clause maintains consistent formatting across files.
58-61: LGTM: Soft deletion handling
Correctly implements soft deletion check before the seenLog existence check, ensuring that deleted dialogs return EntityDeleted status regardless of sub-resource existence.
58-61: Verify consistent implementation across all dialog sub-resource handlers
Let's ensure all dialog sub-resource query handlers consistently implement the soft deletion check.
✅ Verification successful
Soft deletion check is consistently implemented across all dialog sub-resource handlers
The verification shows that the soft deletion check is properly implemented across all dialog sub-resource handlers. The pattern if (dialog.Deleted) { return new EntityDeleted<DialogEntity>(request.DialogId); } is consistently used in:
- All dialog query handlers (both ServiceOwner and EndUser)
- All dialog transmission handlers
- All dialog activity handlers
- All dialog seen log handlers
- All dialog system label handlers
- All dialog label assignment log handlers
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all dialog-related query handlers to verify consistent implementation
# Search for query handler classes that might need the soft deletion check
echo "Searching for dialog-related query handlers..."
rg -l "IRequestHandler.*Dialog.*Query" --type cs
# Search for existing implementations of soft deletion checks
echo -e "\nVerifying soft deletion checks..."
rg "if.*dialog\.Deleted.*return.*new EntityDeleted" --type cs
Length of output: 726
Script:
#!/bin/bash
# Let's examine the content of these query handlers to check for soft deletion handling
# First, let's check the content of all dialog-related query handlers
echo "Content of GetDialogQuery.cs (ServiceOwner):"
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogQuery.cs
echo -e "\nContent of GetDialogQuery.cs (EndUser):"
cat src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogQuery.cs
# Let's also search for any other deletion checks with different patterns
echo -e "\nSearching for other deletion check patterns:"
rg "dialog.*Deleted" --type cs -A 2
Length of output: 29318
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogActivities/Queries/Get/GetActivityQuery.cs (3)
21-21: LGTM: Result type extension properly handles soft-deleted entities
The addition of EntityDeleted to GetActivityResult aligns with the PR objective and maintains consistency with other query results.
47-48: Formatting change only
57-61: LGTM: Proper handling of soft-deleted dialogs
The implementation correctly returns EntityDeleted for soft-deleted dialogs before checking for the activity, which aligns with the PR objective.
Let's verify that this change is consistently implemented across all dialog-related endpoints:
✅ Verification successful
Soft-delete handling is consistently implemented across all dialog-related endpoints
The verification shows that the dialog.Deleted check is properly implemented across all dialog-related endpoints in both ServiceOwner and EndUser features, consistently returning EntityDeleted<DialogEntity> with the correct dialog ID. This includes:
- All Activity queries (Get, Search, NotificationCondition)
- All Transmission queries (Get, Search)
- All SeenLog queries (Get, Search)
- Dialog queries
- LabelAssignmentLog queries
- SystemLabel commands
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for dialog.Deleted checks in query handlers
ast-grep --pattern 'if (dialog.Deleted)
{
return new EntityDeleted<DialogEntity>($$$)
}'
Length of output: 9876
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Search/SearchTransmissionQuery.cs (3)
19-19: LGTM: Consistent result type extension
The addition of EntityDeleted maintains consistency with other query results in the codebase.
51-52: Formatting change only
56-66: LGTM: Well-structured error handling
The implementation properly separates concerns:
- First checks for null dialog
- Then checks for soft-deleted dialog
- Finally proceeds with transmission mapping
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/GetTransmissionQuery.cs (2)
21-21: LGTM: Consistent result type extension
The addition of EntityDeleted maintains consistency with other query results.
54-55: Formatting change only
docs/schema/V1/swagger.verified.json (1)
Line range hint 1-7000: Implementation follows REST principles and standards.
The addition of 410 Gone responses for sub-resources of soft-deleted dialogs is well-implemented and follows REST principles and RFC 7231. The changes maintain a clear distinction between:
- 404 Not Found: Resource never existed
- 410 Gone: Resource existed but was deleted
The implementation is consistent across all sub-resource endpoints:
- /activities/{activityId}
- /seenlog
- /seenlog/{seenLogId}
- /transmissions
- /transmissions/{transmissionId}
...en.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Get/GetDialogTransmissionEndpoint.cs
Show resolved
Hide resolved
...Api/Endpoints/V1/ServiceOwner/DialogTransmissions/Search/SearchDialogTransmissionEndpoint.cs
Show resolved
Hide resolved
oskogstad
deleted the
fix/get-on-sub-resource-returns-404-for-deleted-dialogs
branch
🤖 I have created a release *beep* *boop* --- ## [1.41.1](v1.41.0...v1.41.1) (2024-12-09) ### Bug Fixes * **webapi:** Return 410 GONE for sub-resources on soft-deleted dialogs ([#1564](#1564)) ([bb601a9](bb601a9)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)Summary by CodeRabbit
Release Notes
New Features
410 Gonefor various endpoints, indicating resources that have been permanently removed.Bug Fixes
410 Gonewhen resources are deleted, enhancing clarity for API consumers.Documentation