-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Solution] Replace PrebuiltRuleAsset schema construction with Zod transform #188092
Conversation
/ci |
⏳ Build in-progress, with failures
Failed CI StepsTest Failures
Historycc @jpdjere |
Pinging @elastic/security-detections-response (Team:Detections and Resp) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management) |
/ci |
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.
🎉
const TypeSpecificFields = TypeSpecificCreateProps.transform((val) => { | ||
switch (val.type) { | ||
case 'query': { | ||
const { response_actions: _, ...rest } = val; | ||
return rest; | ||
} | ||
case 'saved_query': { | ||
const { response_actions: _, ...rest } = val; | ||
return rest; | ||
} | ||
default: | ||
return val; | ||
} | ||
}); |
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.
Very cool, this is much simpler than reconstructing type specific props from scratch. And it even creates a correct type for TypeSpecificFields
which I didn't expect! Also it's extensible: this way we could omit any other fields from other rule types. Very-very cool!
Summary
Pending work from: #186615
PrebuiltRuleAsset
with some RuleResponse fields ommited from it had the disadvantage of being built with a discriminated union where all rule types had to be re-listed. If a new type was added, then it would have required manually adding the type to that union as well, which would have been surely forgotten.For maintainers