-
Notifications
You must be signed in to change notification settings - Fork 3.2k
fix(api): add validation entity type for policy creation #14955
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
Conversation
Bundle ReportBundle size has no change ✅ |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
We should prefer validation which works across all APIs. While this change protects GraphQL, it doesn't support OpenAPI or Restli. Implementing this logic as a standard validator allows for cross-API validation. |
46e5927
to
d6559d5
Compare
✅ Meticulous spotted 0 visual differences across 950 screens tested: view results. Meticulous evaluated ~8 hours of user flows against your PR. Expected differences? Click here. Last updated for commit d6559d5. This comment will update as new commits are pushed. |
d6559d5
to
d244b14
Compare
AspectPluginConfig.builder() | ||
.className(PolicyFieldTypeValidator.class.getName()) | ||
.enabled(true) | ||
.supportedOperations(List.of(CREATE, CREATE_ENTITY, UPSERT, UPDATE, PATCH)) |
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.
The implementation doesn't appear to support patch
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.
Remove or implement patch support. This would involve reading the existing aspect, applying the patch and then evaluate. Use the aspectRetriever interface from opContext to fetch the current aspect..
Otherwise we can get flooded with invalid policies that can cause these error logging