fix: Add missing return types for Transmissions and Activities in OpenAPI spec #1244
Conversation
📝 WalkthroughWalkthroughThe pull request introduces multiple new schemas and updates existing ones in the OpenAPI specification for the Dialogporten API. New schemas include various DTOs related to dialog activities and transmissions, enhancing the API's capability to provide detailed information. The endpoints have been updated to return these new schemas, and property descriptions and types have been refined for consistency. 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: 7
🧹 Outside diff range and nitpick comments (3)
src/Digdir.Domain.Dialogporten.GraphQL/EndUser/DialogById/MappingProfile.cs (1)
Line range hint
1-37: Summary: DTO renaming in AutoMapper configurationsThe changes in this file involve renaming two DTOs in the AutoMapper configurations:
GetDialogTransmissionAttachmentDtotoGetDialogDialogTransmissionAttachmentDtoGetDialogTransmissionAttachmentUrlDtotoGetDialogDialogTransmissionAttachmentUrlDtoThese modifications align with the PR objective of improving the clarity and completeness of the API documentation. The changes are consistent and don't introduce any logical alterations or new functionality.
To maintain consistency and prevent future naming discrepancies, consider implementing a naming convention guideline for DTOs and enforcing it through code reviews or static analysis tools.
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Get/GetDialogTransmissionDto.cs (2)
26-29: Improve the XML documentation for clarityThe summary for
IsAuthorizedhas a minor grammatical error and can be clarified. Consider rephrasing it as:"Flag indicating whether the authenticated user is authorized for this transmission. If not, the embedded content and attachments will not be available."
112-118: Ensure consistent collection initializationIn
GetDialogTransmissionAttachmentDto, the listsDisplayNameandUrlsare initialized using the shorthand syntax= [];. While this is acceptable in modern C#, ensure that this style is consistently used throughout the codebase and aligns with your team's coding standards.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Get/GetDialogTransmissionDto.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogDto.cs (3 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/MappingProfile.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/GetDialogTransmissionDto.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogDto.cs (3 hunks)
- src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/MappingProfile.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.GraphQL/EndUser/DialogById/MappingProfile.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogTransmissions/Get/GetDialogTransmissionSwaggerConfig.cs (2 hunks)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogTransmissions/Search/SearchDialogTransmissionSwaggerConfig.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Get/GetDialogTransmissionSwaggerConfig.cs (2 hunks)
🧰 Additional context used
🔇 Additional comments (18)
src/Digdir.Domain.Dialogporten.GraphQL/EndUser/DialogById/MappingProfile.cs (2)
33-34: LGTM. Verify renamed DTO usage and custom mapping.The mapping from
GetDialogDialogTransmissionAttachmentUrlDtotoAttachmentUrllooks correct, including the custom mapping forConsumerType. This change aligns with the renaming mentioned in the summary.To ensure consistency and correct functionality, let's verify the usage of the renamed DTO and the custom mapping across the codebase:
#!/bin/bash # Description: Check for any remaining occurrences of the old DTO name, confirm usage of the new name, and verify the custom mapping. # Test 1: Search for any remaining occurrences of the old name echo "Searching for any remaining occurrences of 'GetDialogTransmissionAttachmentUrlDto':" rg --type csharp 'GetDialogTransmissionAttachmentUrlDto' # Test 2: Confirm usage of the new name echo "Confirming usage of 'GetDialogDialogTransmissionAttachmentUrlDto':" rg --type csharp 'GetDialogDialogTransmissionAttachmentUrlDto' # Test 3: Verify the custom mapping for ConsumerType echo "Verifying custom mapping for ConsumerType:" rg --type csharp 'ConsumerType' -C 3
32-32: LGTM. Verify renamed DTO usage.The mapping from
GetDialogDialogTransmissionAttachmentDtotoAttachmentlooks correct. This change aligns with the renaming mentioned in the summary.To ensure consistency, let's verify the usage of the renamed DTO across the codebase:
✅ Verification successful
LGTM. Renamed DTO usage verified successfully.
No remaining occurrences of
GetDialogTransmissionAttachmentDtofound in the codebase. The mapping fromGetDialogDialogTransmissionAttachmentDtotoAttachmenthas been successfully updated.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining occurrences of the old DTO name and confirm usage of the new name. # Test 1: Search for any remaining occurrences of the old name echo "Searching for any remaining occurrences of 'GetDialogTransmissionAttachmentDto':" rg --type csharp 'GetDialogTransmissionAttachmentDto' # Test 2: Confirm usage of the new name echo "Confirming usage of 'GetDialogDialogTransmissionAttachmentDto':" rg --type csharp 'GetDialogDialogTransmissionAttachmentDto'Length of output: 131959
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogTransmissions/Get/GetDialogTransmissionSwaggerConfig.cs (2)
Line range hint
1-40: Summary: Changes align with PR objectives and improve API specification.The modifications in this file, particularly the explicit return type specification in
ProducesOneOf<GetDialogTransmissionDto>, directly address the PR objective of adding missing return types for Transmissions in the OpenAPI spec. TheOperationIdchange also contributes to API consistency.The
GetDialogTransmissionEndpointSummaryclass remains unchanged, which is appropriate as it already contained the necessary documentation.Overall, these changes enhance type safety, improve API documentation, and maintain consistency with other parts of the codebase.
13-13: Approve OperationId change, but verify impact.The change from "GetDialogTransmissionEU" to "GetDialogTransmission" looks good as it simplifies the operation ID. However, ensure that this change is reflected in the API documentation and that any client code relying on this operation ID is updated accordingly.
To verify the impact of this change, run the following script:
✅ Verification successful
OperationId Change Verified
The
OperationIdhas been successfully updated to "GetDialogTransmission" with no references to "GetDialogTransmissionEU" found in the codebase. Ensure that the API documentation and any external client code are updated accordingly.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for occurrences of the old operation ID in the codebase # Search for the old operation ID echo "Searching for 'GetDialogTransmissionEU':" rg --type-not cs "GetDialogTransmissionEU" # Search for potential client code using the old operation ID echo "Searching for potential client code using 'GetDialogTransmissionEU':" rg --type cs "GetDialogTransmissionEU" # Check if the new operation ID is used consistently echo "Checking for consistent use of 'GetDialogTransmission':" rg --type cs "GetDialogTransmission"Length of output: 5336
Script:
#!/bin/bash # Description: Confirm the usage of the new operation ID in the codebase echo "Searching for 'GetDialogTransmission':" rg --type cs "GetDialogTransmission"Length of output: 13242
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogTransmissions/Get/GetDialogTransmissionSwaggerConfig.cs (1)
17-19: LGTM! Improved Swagger configuration specificity.The change to
ProducesOneOf<GetDialogTransmissionDto>enhances the API documentation by explicitly defining the expected response type for the dialog transmission endpoint. This improvement aligns well with the PR objective of adding missing return types for Transmissions in the OpenAPI spec.Benefits:
- Clearer API contract for consumers
- Improved type safety in generated client code
- Better alignment with OpenAPI best practices
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/MappingProfile.cs (1)
65-66: Approve the mapping update with suggestions for improvement.The update to
GetDialogDialogTransmissionAttachmentUrlDtois consistent with the previous change and aligns with the PR objectives. However, consider the following points:
- The naming convention with repeated "Dialog" seems redundant. Consider simplifying it to
GetDialogTransmissionAttachmentUrlDtoor justifying the repetition.- Ensure that this change is consistently applied across the codebase to maintain coherence.
To verify the impact and consistency of this change, run the following script:
#!/bin/bash # Search for usages of the old and new DTO names echo "Searching for old DTO name (GetDialogTransmissionAttachmentUrlDto):" rg "GetDialogTransmissionAttachmentUrlDto" --type cs echo "\nSearching for new DTO name (GetDialogDialogTransmissionAttachmentUrlDto):" rg "GetDialogDialogTransmissionAttachmentUrlDto" --type cs echo "\nChecking for any remaining inconsistencies:" rg "DialogTransmissionAttachmentUrl.*Dto" --type cssrc/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/GetDialogDto.cs (4)
240-240: Improved naming consistency for transmission attachments.The change from
List<GetDialogTransmissionAttachmentDto>toList<GetDialogDialogTransmissionAttachmentDto>enhances the naming consistency. This new name more clearly indicates that these attachments are specifically for dialog transmissions.
Line range hint
639-654: Enhanced clarity in class naming for transmission attachments.The renaming of
GetDialogTransmissionAttachmentDtotoGetDialogDialogTransmissionAttachmentDtoand the update of theUrlsproperty type improve the clarity of the class hierarchy. These changes make it more explicit that these DTOs are specifically related to dialog transmissions.
657-657: Consistent renaming for transmission attachment URL DTO.The renaming of
GetDialogTransmissionAttachmentUrlDtotoGetDialogDialogTransmissionAttachmentUrlDtois consistent with the previous changes. This maintains a uniform naming convention throughout the related DTOs, enhancing the overall clarity of the code structure.
Line range hint
1-680: Summary: Improved naming consistency enhances code clarity.The changes in this file are part of a broader refactoring effort to improve naming consistency across the DTO classes related to dialog transmissions and attachments. By explicitly including "Dialog" in the class names, the relationship between these DTOs and the dialog context is now more apparent. This enhancement in naming conventions contributes to better code readability and maintainability, making it easier for developers to understand the purpose and context of each DTO.
While no functional changes were introduced, these naming improvements are valuable for the long-term maintainability of the codebase. Good job on this refactoring effort!
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/GetDialogDto.cs (4)
230-230: Property type updated for better consistencyThe property type for
Attachmentshas been updated fromGetDialogTransmissionAttachmentDtotoGetDialogDialogTransmissionAttachmentDto. This change improves naming consistency across the codebase.
Line range hint
632-647: Class and property type renamed for consistencyThe
GetDialogTransmissionAttachmentDtoclass has been renamed toGetDialogDialogTransmissionAttachmentDto, and theUrlsproperty type has been updated toGetDialogDialogTransmissionAttachmentUrlDto. These changes improve naming consistency across the codebase.
Line range hint
650-674: Class renamed for consistencyThe
GetDialogTransmissionAttachmentUrlDtoclass has been renamed toGetDialogDialogTransmissionAttachmentUrlDto. This change improves naming consistency across the codebase.
Line range hint
1-674: Summary: Improved naming consistency across DTO classesThe changes in this file focus on renaming classes and updating property types to improve naming consistency across the codebase. Specifically:
GetDialogTransmissionAttachmentDto→GetDialogDialogTransmissionAttachmentDtoGetDialogTransmissionAttachmentUrlDto→GetDialogDialogTransmissionAttachmentUrlDto- Updated property types to reflect the new class names
These changes enhance the readability and maintainability of the code without introducing any logical modifications or potential issues.
src/Digdir.Domain.Dialogporten.Application/Features/V1/ServiceOwner/DialogTransmissions/Queries/Get/GetDialogTransmissionDto.cs (3)
112-112:⚠️ Potential issueFix incorrect list initialization syntax for
UrlspropertyAt line 112, the
Urlsproperty is initialized using an empty array[], which is invalid in C#. To initialize a list, usenew List<GetDialogTransmissionAttachmentUrlDto>()ornew().Apply this diff to fix the issue:
- public List<GetDialogTransmissionAttachmentUrlDto> Urls { get; set; } = []; + public List<GetDialogTransmissionAttachmentUrlDto> Urls { get; set; } = new();Likely invalid or redundant comment.
107-107:⚠️ Potential issueFix incorrect list initialization syntax for
DisplayNamepropertyAt line 107, the
DisplayNameproperty is initialized using an empty array[], which is invalid in C#. To initialize a list, usenew List<LocalizationDto>()ornew().Apply this diff to fix the issue:
- public List<LocalizationDto> DisplayName { get; set; } = []; + public List<LocalizationDto> DisplayName { get; set; } = new();Likely invalid or redundant comment.
58-58:⚠️ Potential issueFix incorrect list initialization syntax for
AttachmentspropertyAt line 58, the
Attachmentsproperty is initialized using an empty array[], which is invalid in C#. To initialize a list, usenew List<GetDialogTransmissionAttachmentDto>()ornew().Apply this diff to fix the issue:
- public List<GetDialogTransmissionAttachmentDto> Attachments { get; set; } = []; + public List<GetDialogTransmissionAttachmentDto> Attachments { get; set; } = new();Likely invalid or redundant comment.
src/Digdir.Domain.Dialogporten.Application/Features/V1/EndUser/DialogTransmissions/Queries/Get/GetDialogTransmissionDto.cs (1)
139-145:⚠️ Potential issueRemove unnecessary null initialization for nullable properties
In
GetDialogTransmissionAttachmentUrlDto, the propertyMediaTypeis declared as nullable (string?) and is initialized tonull!. SinceMediaTypeis already nullable, the initialization tonull!is unnecessary and can be omitted.Apply this diff to fix the issue:
-public string? MediaType { get; set; } = null!; +public string? MediaType { get; set; }Likely invalid or redundant comment.
...Api/Endpoints/V1/EndUser/DialogTransmissions/Search/SearchDialogTransmissionSwaggerConfig.cs
Show resolved
Hide resolved
...en.WebApi/Endpoints/V1/EndUser/DialogTransmissions/Get/GetDialogTransmissionSwaggerConfig.cs
Show resolved
Hide resolved
...main.Dialogporten.Application/Features/V1/ServiceOwner/Dialogs/Queries/Get/MappingProfile.cs
Show resolved
Hide resolved
...ir.Domain.Dialogporten.Application/Features/V1/EndUser/Dialogs/Queries/Get/MappingProfile.cs
Show resolved
Hide resolved
....Application/Features/V1/EndUser/DialogTransmissions/Queries/Get/GetDialogTransmissionDto.cs
Show resolved
Hide resolved
....Application/Features/V1/EndUser/DialogTransmissions/Queries/Get/GetDialogTransmissionDto.cs
Show resolved
Hide resolved
....Application/Features/V1/EndUser/DialogTransmissions/Queries/Get/GetDialogTransmissionDto.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
docs/schema/V1/swagger.verified.json (1)
2561-2565: LGTM: Added id property to transmission attachment DTOsThe addition of the
idproperty with a GUID format to GetDialogTransmissionAttachmentDto, GetDialogTransmissionAttachmentDtoSO, GetDialogTransmissionAttachmentUrlDto, and GetDialogTransmissionAttachmentUrlDtoSO is a good improvement. This allows for unique identification of attachments and attachment URLs, enhancing tracking and management capabilities.Consider adding a brief description for the
idproperty in each DTO to clarify its purpose and usage, similar to other properties in the schema. For example:"id": { "description": "The unique identifier for the transmission attachment in UUIDv7 format.", "format": "guid", "type": "string" }Also applies to: 2592-2596
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- docs/schema/V1/swagger.verified.json (12 hunks)
🧰 Additional context used
🔇 Additional comments (3)
docs/schema/V1/swagger.verified.json (3)
1745-1846: LGTM: New transmission attachment DTOs addedThe new schemas for GetDialogDialogTransmissionAttachmentDto, GetDialogDialogTransmissionAttachmentDtoSO, GetDialogDialogTransmissionAttachmentUrlDto, and GetDialogDialogTransmissionAttachmentUrlDtoSO have been successfully added. These additions provide a clear structure for handling dialog transmission attachments, with appropriate separation between end-user and service owner DTOs.
1897-1897: LGTM: Updated references to new attachment DTOsThe GetDialogDialogTransmissionDto and GetDialogDialogTransmissionDtoSO have been updated to reference the newly added GetDialogDialogTransmissionAttachmentDto and GetDialogDialogTransmissionAttachmentDtoSO respectively. This change ensures consistency and provides a more detailed structure for transmission attachments.
Also applies to: 1966-1966
5492-5497: LGTM: Updated transmission endpoints to use new DTOsThe API endpoints for transmissions have been successfully updated to use the new DTO structures:
- GetDialogTransmissionList now returns an array of SearchDialogTransmissionDto
- GetDialogTransmission now returns a GetDialogTransmissionDto
- Corresponding changes have been made for the service owner (SO) versions of these endpoints
These updates provide more detailed and consistent information about transmissions, aligning with best practices for API design by using search DTOs for list endpoints and detailed DTOs for individual transmission retrieval.
Also applies to: 5546-5552, 6894-6899, 7053-7059
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Search/SearchDialogActivityEndpoint.cs (1)
9-9: LGTM! Consider adding XML documentation.The change to
Endpoint<SearchDialogActivityQuery, List<SearchDialogActivityDto>>is a good improvement. It explicitly defines the return type, enhancing type safety and API clarity.Consider adding XML documentation to the class to describe its purpose and the return type. This would further improve the API documentation:
/// <summary> /// Endpoint for searching dialog activities. /// </summary> /// <remarks> /// This endpoint returns a list of SearchDialogActivityDto objects. /// </remarks> public sealed class SearchDialogActivityEndpoint : Endpoint<SearchDialogActivityQuery, List<SearchDialogActivityDto>>src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/NotificationCondition/NotificationConditionSwaggerConfig.cs (2)
Line range hint
19-19: ImplementGetExamplemethod or provide a reason for the exception.The
GetExamplemethod is currently throwing aNotImplementedException. Consider implementing this method to provide an example for the Swagger documentation, or if it's intentionally left unimplemented, add a comment explaining why.Would you like assistance in implementing the
GetExamplemethod or adding an explanatory comment?
Line range hint
22-36: LGTM: SearchDialogActivityEndpointSummary implementation with a minor suggestion.The implementation of
SearchDialogActivityEndpointSummaryis well-structured and informative. The summary, description, and response definitions are clear and follow best practices.Consider extracting the formatted string for the 401 Unauthorized response into a constant or a separate method for better maintainability and reusability. For example:
private static string GetUnauthorizedMessage(string scope) => Constants.SwaggerSummary.ServiceOwnerAuthenticationFailure.FormatInvariant(scope); // Usage in constructor Responses[StatusCodes.Status401Unauthorized] = GetUnauthorizedMessage(AuthorizationScope.NotificationConditionCheck);This change would make it easier to update the message format in the future if needed.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Get/GetDialogActivitySwaggerConfig.cs (1)
Line range hint
24-39: Great addition: GetDialogActivityEndpointSummary class improves API documentation.The new
GetDialogActivityEndpointSummaryclass significantly enhances the API documentation by providing clear and concise descriptions for the endpoint and its various response scenarios. This addition:
- Improves the overall clarity of the API.
- Aligns with best practices for API documentation.
- Uses constants for common response descriptions, promoting consistency across the API.
Excellent work on improving the documentation!
Consider adding a brief example or more specific details about what information is returned in the 200 OK response. This could further enhance the usefulness of the documentation for API consumers.
docs/schema/V1/swagger.verified.json (1)
Line range hint
1-7289: Consistent naming conventions with room for description improvementsThe specification demonstrates consistent use of naming conventions:
- camelCase for properties
- PascalCase for schema names
Descriptions are provided for most properties and schemas, which is commendable. However, there's potential for enhancing some descriptions to provide more context or examples, particularly for complex properties or those with specific constraints.
Consider reviewing and potentially expanding descriptions for properties that might benefit from additional context or examples, especially for complex data types or properties with specific constraints or usage scenarios.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (8)
- docs/schema/V1/swagger.verified.json (22 hunks)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogActivities/Get/GetDialogActivityEndpoint.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogActivities/Get/GetDialogActivitySwaggerConfig.cs (2 hunks)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogActivities/Search/SearchDialogActivityEndpoint.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Get/GetDialogActivitySwaggerConfig.cs (2 hunks)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/NotificationCondition/NotificationConditionEndpoint.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/NotificationCondition/NotificationConditionSwaggerConfig.cs (1 hunks)
- src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Search/SearchDialogActivityEndpoint.cs (1 hunks)
🧰 Additional context used
🔇 Additional comments (17)
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Search/SearchDialogActivityEndpoint.cs (1)
9-9: Verify OpenAPI specification updateThe change in the return type should be reflected in the OpenAPI specification. Please ensure that the corresponding OpenAPI documentation has been updated to show that this endpoint returns a list of
SearchDialogActivityDtoobjects.Run the following script to check if the OpenAPI specification has been updated:
If the script doesn't return any results, it might indicate that the OpenAPI specification hasn't been updated to reflect the change in the return type.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogActivities/Get/GetDialogActivityEndpoint.cs (1)
9-9: LGTM! Consider updating the OpenAPI specification.The change to include
GetDialogActivityDtoas the return type in theEndpoint<>generic parameter is a good improvement. It enhances type safety and aligns with the PR objective of adding missing return types.To ensure consistency, please verify that the OpenAPI specification has been updated to reflect this change. You can run the following script to check for updates in the OpenAPI spec file:
If the OpenAPI spec hasn't been updated, consider adding this task to your PR checklist.
✅ Verification successful
Verified! The OpenAPI specification has been updated accordingly.
The
GetDialogActivityDtois correctly referenced in theswagger.verified.json, ensuring that the OpenAPI specification reflects the changes made in the endpoint. No further action is required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for updates in the OpenAPI spec file related to GetDialogActivityDto # Test: Search for GetDialogActivityDto in the OpenAPI spec file rg --type yaml 'GetDialogActivityDto' -C 5Length of output: 42
Script:
#!/bin/bash # Description: Comprehensive search for 'GetDialogActivityDto' in OpenAPI specification files (YAML and JSON) # Search in YAML and JSON files for 'GetDialogActivityDto' rg --type yaml --type json 'GetDialogActivityDto' -C 5Length of output: 2949
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogActivities/Search/SearchDialogActivityEndpoint.cs (3)
9-9: Improved type safety and API contract clarity.The updated class signature now includes the response type
List<SearchDialogActivityDto>. This change:
- Aligns with FastEndpoints best practices.
- Enhances type safety.
- Makes the API contract more explicit.
- Provides better documentation for API consumers.
Line range hint
28-35: LGTM: HandleAsync implementation consistent with updated signature.The
HandleAsyncmethod implementation is already aligned with the updated class signature. It correctly handles the list of DTOs returned by_sender.Send(req, ct)and uses appropriate pattern matching for different response scenarios. No changes were needed in the method body, which demonstrates good forward-thinking in the original design.
Line range hint
1-35: Summary: Improved API contract with minimal code changes.The update to
SearchDialogActivityEndpointenhances the API contract by explicitly specifying the response typeList<SearchDialogActivityDto>in the class signature. This change:
- Improves type safety and clarity.
- Aligns with FastEndpoints best practices.
- Requires no changes to the existing method implementations, indicating a well-designed initial structure.
The focused nature of this change minimizes the risk of introducing bugs while significantly improving the API's documentation and type safety.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/NotificationCondition/NotificationConditionEndpoint.cs (1)
9-9: Improved type safety and API documentationThe addition of
NotificationConditionDtoas a second generic type parameter to theEndpoint<>base class is a positive change. It explicitly defines the response type for this endpoint, which:
- Enhances type safety by ensuring the endpoint returns the expected DTO.
- Improves API documentation, aligning with the PR objective of adding missing return types for Transmissions in the OpenAPI spec.
- Makes the class signature consistent with the
HandleAsyncmethod implementation, which already returns a DTO.This change contributes to a more robust and self-documenting API design.
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/EndUser/DialogActivities/Get/GetDialogActivitySwaggerConfig.cs (3)
16-18: Excellent addition of the return type specification!The change to
ProducesOneOf<GetDialogActivityDto>()is a valuable improvement. It explicitly defines the return type for the GetDialogActivity operation, which enhances the clarity and accuracy of the API documentation. This modification aligns perfectly with the PR objective of adding missing return types for Transmissions in the OpenAPI spec.Benefits of this change:
- Improved API documentation
- Better type safety
- Enhanced developer experience for API consumers
Line range hint
20-20: Verify the intention ofGetExample()methodThe
GetExample()method is currently throwing aNotImplementedException. While this might be intentional, it's worth confirming if this is the desired behavior or if there's a plan to implement this method in the future.If it's intentional, consider adding a comment explaining why the method is not implemented. If it's meant to be implemented later, you might want to add a TODO comment.
Could you please clarify the intention behind this implementation?
Line range hint
1-39: Overall, the changes and existing code look goodThe modification to explicitly specify the return type in
ProducesOneOf<GetDialogActivityDto>()is a valuable improvement to the API documentation. The rest of the file, including the comprehensiveGetDialogActivityEndpointSummary, is well-structured and informative.The only suggestion for potential improvement is to clarify the intention behind the
GetExample()method implementation.Great job on enhancing the API documentation!
src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/NotificationCondition/NotificationConditionSwaggerConfig.cs (2)
13-18: LGTM: NotificationConditionSwaggerConfig implementation.The implementation of
NotificationConditionSwaggerConfiglooks good. TheOperationIdproperty andSetDescriptionmethod are well-defined and follow best practices.
Line range hint
1-36: Overall, the file is well-implemented with minor suggestions for improvement.The
NotificationConditionSwaggerConfigandSearchDialogActivityEndpointSummaryclasses are well-structured and follow best practices. The suggested improvements (implementingGetExampleand extracting the unauthorized message) are minor and would enhance maintainability without affecting functionality.src/Digdir.Domain.Dialogporten.WebApi/Endpoints/V1/ServiceOwner/DialogActivities/Get/GetDialogActivitySwaggerConfig.cs (2)
1-1: LGTM: New using statement added correctly.The new using statement for
Digdir.Domain.Dialogporten.Application.Features.V1.ServiceOwner.DialogActivities.Queries.Getis correctly added. This import is necessary for theGetDialogActivityDtoused in theProducesOneOfmethod.
17-19: Excellent: Return type specified for ProducesOneOf method.The
ProducesOneOfmethod has been updated to explicitly specifyGetDialogActivityDtoas the return type. This change:
- Improves the clarity of the API specification.
- Aligns with the PR objective of adding missing return types for Transmissions in the OpenAPI spec.
- Enhances type safety and documentation.
Great job on making this improvement!
docs/schema/V1/swagger.verified.json (4)
Line range hint
943-2984: New schemas added successfullyThe new schemas (GetDialogActivityDto, GetDialogActivityDtoSO, GetDialogActivityPerformedByActorDto, GetDialogActivityPerformedByActorDtoSO, GetDialogDialogTransmissionAttachmentDto, GetDialogDialogTransmissionAttachmentDtoSO, GetDialogDialogTransmissionAttachmentUrlDto, GetDialogDialogTransmissionAttachmentUrlDtoSO, GetDialogTransmissionContentDto, GetDialogTransmissionContentDtoSO, GetDialogTransmissionDto, and GetDialogTransmissionDtoSO) have been added correctly. They follow consistent naming conventions and have well-defined properties with appropriate types and descriptions.
Line range hint
2685-2984: Existing schemas updated appropriatelyThe modifications to existing schemas have been implemented correctly:
- GetDialogTransmissionAttachmentDto and GetDialogTransmissionAttachmentDtoSO now include an 'id' field, providing a unique identifier for attachments.
- GetDialogTransmissionDto and GetDialogTransmissionDtoSO have been updated to reference the newly defined attachment schemas, ensuring consistency with the new structure.
These changes improve the overall coherence of the API specification.
Line range hint
1-7289: Proper use of OpenAPI specification featuresThe OpenAPI specification demonstrates correct usage of OpenAPI 3.0.0 features:
- Well-structured components and schemas
- Properly defined path operations
- Correct use of $ref for schema reuse
- Appropriate definition and application of security schemes
The overall structure follows OpenAPI best practices, making the API documentation clear and easy to understand.
Line range hint
1-7289: Summary: Comprehensive and well-implemented API specification updatesThe changes made to the Dialogporten API OpenAPI specification are comprehensive and well-implemented. Key improvements include:
- Addition of new schemas for activities, transmissions, and related entities
- Updates to existing schemas to maintain consistency with the new structure
- Consistent use of naming conventions and descriptive property documentation
- Proper utilization of OpenAPI 3.0.0 features
These updates significantly enhance the clarity and completeness of the API documentation, which will benefit both API consumers and maintainers. The structure remains coherent, and the new additions integrate seamlessly with the existing specification.
Great job on improving the Dialogporten API documentation!
oskogstad
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
docs/schema/V1/swagger.verified.json (1)
Line range hint
1876-2097: Transmission schemas are well-structured, but consider simplifying namesThe new Transmission-related schemas (GetDialogDialogTransmissionAttachmentDto, GetDialogDialogTransmissionAttachmentDtoSO, etc.) are well-structured and provide detailed representations for transmissions, their attachments, and content. They follow the good practice of separating end-user and service owner views.
However, there's some redundancy in the naming. Consider simplifying the names by removing one instance of "Dialog" from each. For example:
GetDialogDialogTransmissionAttachmentDto->GetDialogTransmissionAttachmentDtoGetDialogDialogTransmissionContentDto->GetDialogTransmissionContentDtoThis would make the names more concise while still maintaining clarity.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- docs/schema/V1/swagger.verified.json (22 hunks)
🧰 Additional context used
🔇 Additional comments (2)
docs/schema/V1/swagger.verified.json (2)
943-1073: New Activity schemas look good!The newly added schemas (GetDialogActivityDto, GetDialogActivityDtoSO, GetDialogActivityPerformedByActorDto, GetDialogActivityPerformedByActorDtoSO) are well-structured and consistent with existing naming conventions. They provide a clear separation between end-user and service owner views, which is a good practice for maintaining different levels of information access.
Line range hint
2692-2930: Updates to existing Transmission schemas look goodThe updates to existing schemas (GetDialogTransmissionAttachmentDto, GetDialogTransmissionAttachmentDtoSO, GetDialogTransmissionDto, GetDialogTransmissionDtoSO) successfully incorporate references to the newly added schemas. These changes enhance the detail and structure of the transmission-related data while maintaining consistency with the new schemas.
The updates appear to be well-implemented and should improve the overall representation of transmission data in the API.
|
oskogstad
deleted the
fix/open-api-broken-return-type-on-transmissions
branch
🤖 I have created a release *beep* *boop* --- ## [1.23.0](v1.22.0...v1.23.0) (2024-10-10) ### Features * **infra:** upgrade postgresql SKU in test ([#1257](#1257)) ([5a751af](5a751af)) * **webAPI:** Add legacy HTML support for MainContentReference ([#1256](#1256)) ([482b38a](482b38a)) ### Bug Fixes * Add missing return types for Transmissions and Activities in OpenAPI spec ([#1244](#1244)) ([972870d](972870d)) * **graphQL:** Missing MediaType on dialog attachment url ([#1264](#1264)) ([3919343](3919343)) * Refactor probes and add more health checks ([#1159](#1159)) ([6889a96](6889a96)) * **webapi:** ensure correct health checks are used in probes ([#1249](#1249)) ([f951152](f951152)) --- 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
New Features
Bug Fixes