-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: updated TypeNameConverter #1900
Conversation
📝 WalkthroughWalkthroughThis pull request updates an OpenAPI schema reference and refines type name conversion logic. The schema used for filtering dialogs has been renamed from "V1Common_dFilter" to "V1Common_DeletedFilter" in the API specification, clarifying its purpose. Additionally, the Changes
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/Digdir.Domain.Dialogporten.WebApi/Common/Swagger/TypeNameConverter.cs (1)
113-115
: LGTM! Consider improving readability.The changes correctly fix the issue of removing incomplete words from prefixes, ensuring terms like "Deleted" and "Patched" remain intact.
Consider these readability improvements:
+ // Constants for character checks + private const char LowercaseA = 'a'; + private const char LowercaseZ = 'z'; private static ReadOnlySpan<char> ExcludePrefix(this ReadOnlySpan<char> name, IEnumerable<string> prefixes) { + // Helper method to check if character is lowercase + static bool IsLowercase(char c) => c >= LowercaseA && c <= LowercaseZ; foreach (var prefix in prefixes) { if (name.StartsWith(prefix) && (name.Length == prefix.Length || - (name.Length > prefix.Length && (name[prefix.Length] < 'a' || name[prefix.Length] > 'z')))) + (name.Length > prefix.Length && !IsLowercase(name[prefix.Length]))))docs/schema/V1/swagger.verified.json (3)
7187-7191
: Consider adding more server environments.The servers array currently only includes the development environment. Consider adding other environments like staging, production, etc., to make the API specification more complete.
Add additional server environments:
"servers": [ { "url": "https://altinn-dev-api.azure-api.net/dialogporten" }, + { + "url": "https://altinn-staging-api.azure-api.net/dialogporten", + "description": "Staging environment" + }, + { + "url": "https://altinn-prod-api.azure-api.net/dialogporten", + "description": "Production environment" + } ]
4776-4779
: Add more descriptive API information.The API information section could be enhanced with more details to better describe the API's purpose and version.
Add more descriptive information:
"info": { "title": "Dialogporten", + "description": "API for managing dialogs between service owners and end users in Altinn", + "contact": { + "name": "Altinn Support", + "url": "https://www.altinn.no/en/about-altinn/contact/" + }, "version": "v1" }
4767-4775
: Consider enhancing security scheme documentation.The security scheme documentation could be improved to provide more details about the JWT bearer authentication.
Enhance the security scheme documentation:
"securitySchemes": { "JWTBearerAuth": { "bearerFormat": "JWT", - "description": "Enter a JWT token to authorize the requests...", + "description": "Enter a Maskinporten JWT token to authorize the requests. The token must contain the appropriate scopes for the requested operation. For more information about obtaining tokens, see the Maskinporten documentation.", "scheme": "bearer", "type": "http" } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/schema/V1/swagger.verified.json
(3 hunks)src/Digdir.Domain.Dialogporten.WebApi/Common/Swagger/TypeNameConverter.cs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Dry run deploy apps / Deploy web-api-so to test
- GitHub Check: build / build-and-test
🔇 Additional comments (3)
docs/schema/V1/swagger.verified.json (3)
330-343
: LGTM! Schema definition forV1Common_DeletedFilter
is well-structured.The schema definition for
V1Common_DeletedFilter
is clear and follows best practices:
- Proper enum values: "Exclude", "Include", "Only"
- Appropriate type: "string"
- Includes x-enumNames for better code generation
- Empty description field is acceptable as the enum values are self-descriptive
5731-5743
: Schema reference updated correctly for thedeleted
query parameter.The query parameter definition for
deleted
properly references theV1Common_DeletedFilter
schema and includes:
- Clear description explaining each enum value's behavior
- Nullable property to make the parameter optional
- Proper schema reference using
$ref
1-4775
:❓ Verification inconclusive
Verify API specification against OpenAPI standards.
Let's verify that the API specification follows OpenAPI standards and best practices.
🏁 Script executed:
#!/bin/bash # Description: Validate OpenAPI specification against standards # Install spectral if not already installed if ! command -v spectral &> /dev/null; then npm install -g @stoplight/spectral-cli fi # Run spectral linting spectral lint docs/schema/V1/swagger.verified.jsonLength of output: 1861
Action Required: Manual Verification of OpenAPI Specification via Spectral
The initial script failed due to permission issues when installing the global Spectral CLI, and as a result, no linting output was produced. To proceed with proper validation, please run the following command manually or via a corrected script:
npx --yes @stoplight/spectral-cli lint docs/schema/V1/swagger.verified.jsonThis approach uses npx to bypass global installation issues and should provide proper feedback on whether the API specification adheres to OpenAPI standards.
🤖 I have created a release *beep* *boop* --- ## [1.53.0](v1.52.0...v1.53.0) (2025-02-19) ### Features * Enforce minimum auth level requirements on dialogs ([#1875](#1875)) ([37febf6](37febf6)) ### Bug Fixes * **graphql:** Typo in auth level error type name ([#1904](#1904)) ([b3d9ad8](b3d9ad8)) * Return new revision ETag on system label update ([#1903](#1903)) ([2e763cd](2e763cd)) ### Miscellaneous Chores * **deps:** update dependency testcontainers.postgresql to 4.2.0 ([#1908](#1908)) ([9f76f69](9f76f69)) * **deps:** update jaegertracing/all-in-one docker tag to v1.66.0 ([#1910](#1910)) ([29877d7](29877d7)) * **deps:** update microsoft dependencies ([#1907](#1907)) ([c916575](c916575)) * updated TypeNameConverter ([#1900](#1900)) ([f68c112](f68c112)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Updated TypeNameConverter to not remove half words from Prefixes (Delete from Delted, Patch from Patched)
Related Issue(s)
Verification
Documentation
docs
-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)