OpenApi @@id support #1757
OpenApi @@id support #1757
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PostLike
participant post_Item
participant RESTfulOpenAPIGenerator
User->>RESTfulOpenAPIGenerator: Request to generate OpenAPI spec
RESTfulOpenAPIGenerator->>PostLike: Check for relationships
PostLike-->>RESTfulOpenAPIGenerator: Return relationship data
RESTfulOpenAPIGenerator->>post_Item: Check for likes field
post_Item-->>RESTfulOpenAPIGenerator: Return likes field data
RESTfulOpenAPIGenerator->>User: Generate OpenAPI spec with relationships
User-->>RESTfulOpenAPIGenerator: Validate generated spec
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
packages/plugins/openapi/tests/openapi-restful.test.ts (3)
38-38: Consider adding a description to thelikesfield in theUsermodelAdding a description to the
likesfield will enhance the generated OpenAPI documentation, making it clearer for API consumers.
59-59: Consider adding a description to thelikesfield in thepost_ItemmodelProviding a description for the
likesfield will improve the OpenAPI documentation and help users understand its purpose in the API.
371-371: Remove unnecessaryconsole.logstatementThe
console.log(JSON.stringify(parsed));statement may clutter the test output. Consider removing it or using a logger with appropriate log levels.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/plugins/openapi/tests/baseline/rest-3.0.0.baseline.yamlis excluded by!**/*.yamlpackages/plugins/openapi/tests/baseline/rest-3.1.0.baseline.yamlis excluded by!**/*.yaml
📒 Files selected for processing (2)
- packages/plugins/openapi/src/rest-generator.ts (3 hunks)
- packages/plugins/openapi/tests/openapi-restful.test.ts (4 hunks)
🧰 Additional context used
🔇 Additional comments (7)
packages/plugins/openapi/tests/openapi-restful.test.ts (3)
111-113:likesrelationship paths forUsermodel are correctly testedThe added test cases effectively verify that the OpenAPI paths for
/user/{id}/relationships/likesare properly generated and accessible viaGET,POST, andPATCHmethods.
117-119:likesrelationship paths forpost_Itemmodel are correctly testedThe new test assertions confirm that the OpenAPI specification includes the correct paths for
/post_Item/{id}/relationships/likes, ensuring they supportGET,POST, andPATCHmethods as expected.
343-371: Test case for compound ID fields is well-implementedThe test accurately checks that individual fields from a compound ID (
role,company) are exposed as attributes in the OpenAPI schema for theUsermodel.packages/plugins/openapi/src/rest-generator.ts (4)
412-413: Ensure consistent use ofhasMultipleIdsThe variable
hasMultipleIdscorrectly identifies if the model has multiple ID fields. Verify that this variable is consistently used throughout the method to handle filter parameters appropriately for models with compound IDs.
421-422: Correctly filter ID fields based on the number of IDsThe condition
if (isIdField(field) && !hasMultipleIds)ensures that ID fields are treated as regular filterable fields when the model has multiple IDs. This change allows clients to filter by each component of a compound ID, which enhances query flexibility.
849-851: Expose all fields in the schema for models with compound IDsBy setting
fieldstomodel.fieldswhen there are multiple ID fields, all fields, including ID fields, are included in the schema. This adjustment ensures that each component of a compound ID is exposed as a separate field in the API schema, which is necessary for clients to construct and interpret compound IDs correctly.
894-895: Confirm ID field requirements in create requestsThe logic adds the
idproperty to the required fields if all ID fields lack a@defaultattribute. For models with compound IDs without default values, this enforces that all ID components must be provided during creation. Ensure that this behavior aligns with the expected API usage and that clients are aware of this requirement.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/plugins/openapi/tests/openapi-restful.test.ts (1)
343-370: LGTM: New test case for compound ID fieldsThis new test case is a valuable addition to the test suite. It verifies that the OpenAPI generator correctly exposes individual fields from a compound ID as attributes in the generated specification. The test is well-structured and covers an important edge case in OpenAPI generation.
Suggestions for enhancement:
- Consider adding assertions to verify the data types of the 'role' and 'company' fields in the generated specification.
- It might be beneficial to add a similar test case for the REST API endpoints to ensure they correctly handle compound IDs in URL parameters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/plugins/openapi/tests/baseline/rest-3.0.0.baseline.yamlis excluded by!**/*.yamlpackages/plugins/openapi/tests/baseline/rest-3.1.0.baseline.yamlis excluded by!**/*.yaml
📒 Files selected for processing (1)
- packages/plugins/openapi/tests/openapi-restful.test.ts (4 hunks)
🧰 Additional context used
🔇 Additional comments (5)
packages/plugins/openapi/tests/openapi-restful.test.ts (5)
38-38: LGTM: New relationship field added to User modelThe addition of the
likes PostLike[]field to the User model correctly establishes a relationship with the new PostLike model. This change is consistent with the PR objectives and enhances the model's functionality.
59-59: LGTM: New relationship field added to post_Item modelThe addition of the
likes PostLike[]field to the post_Item model correctly establishes a relationship with the new PostLike model. This change aligns with the PR objectives and enhances the model's functionality.
66-72: LGTM: New PostLike model addedThe new PostLike model is well-structured with appropriate relationships to post_Item and User models. The use of a composite primary key
@@id([postId, userId])is correct for this many-to-many relationship.Note: A previous suggestion to add timestamp fields was deemed unnecessary for testing purposes by the author.
111-113: LGTM: New assertions added for likes relationshipsThe new assertions correctly validate the generation of API paths for the likes relationship in both User and post_Item models. They cover the necessary HTTP methods (GET, POST, PATCH) and ensure that the OpenAPI specification accurately reflects the new model relationships.
Also applies to: 117-119
Line range hint
1-385: Overall assessment: Well-structured and comprehensive test additionsThe changes in this file effectively enhance the test coverage for the OpenAPI Plugin RESTful functionality. The additions include:
- New model relationships for PostLike
- Assertions for API paths related to the new relationships
- A new test case for compound ID fields
These changes align well with the PR objectives and provide robust testing for the new features. The test suite now offers more comprehensive coverage of the OpenAPI specification generation process.
|
I'm merging it and will include the change in the upcoming 2.7.0 release. |
Companion to #1754
@ymc9 how do I best update the baseline files with the extra model needed for new tests?