-
Notifications
You must be signed in to change notification settings - Fork 62
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
Adds action/function suffix to tag names for action/function operations #642
Conversation
* Update operation tags generation * Refactor operation tag generation * Update unit tests * Update integration test files * Update release note * Update src/Microsoft.OpenApi.OData.Reader/Operation/EdmOperationOperationHandler.cs Co-authored-by: Andrew Omondi <[email protected]> * Account for nullability * Remove unnecessary code --------- Co-authored-by: Andrew Omondi <[email protected]>
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.
also lacks unit tests and integration tests updates
src/Microsoft.OpenApi.OData.Reader/Microsoft.OpenAPI.OData.Reader.csproj
Outdated
Show resolved
Hide resolved
src/Microsoft.OpenApi.OData.Reader/Operation/EdmOperationOperationHandler.cs
Outdated
Show resolved
Hide resolved
…tionHandler.cs Co-authored-by: Vincent Biret <[email protected]>
…of using Odata operation segment. Updated test files and regenerated yaml and json resource files
Quality Gate passedIssues Measures |
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.
Thank you for making the changes!
Can you please also put together the reflection of this PR for v2 so things move in sync and we don't have regressions with v2.
Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-4d93", operation2.OperationId); | ||
Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-a2b2", operation3.OperationId); | ||
Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-7bea", operation4.OperationId); | ||
Assert.Equal("Customers.Customer.MyFunction1.MyFunction2-6b6d", operation1.OperationId); |
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.
Looks like this change is resulting in duplicate operation ids as captured in the validation at microsoftgraph/msgraph-metadata#752
Ideally, the logic here should results in all 4 of the operationIds in the tests being unique
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.
@timayabi2020 can you prioritize a fix for this regression please?
Thank Andrew for catching this and sorry for not seeing it during the review
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.
No worries. Created #645 to resolve this one.
Fixes #640
Cherry picks from #586 and adds "Actions" or "Functions" as suffixes to the tag. This will
enable the powershell mappings to be updated to target the paths more precisely to fix the incorrect mappings.
enable the user functions/actions to still remain in their current modules by targeting operationIds like user.*.Actions.