-
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
[Fleet] Allow global data tags to be readonly #187161
[Fleet] Allow global data tags to be readonly #187161
Conversation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
Forms are complex. IMO it's a losing battle trying to engineer a comprehensive solution against all possible permutations. That said, we do have enough usage examples of "disable entire form" to give to the EUI team for them to brainstorm possible EUI solutions that could work across Kibana. |
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.
Code LGTM
The form implementation provided by EUI is opinionated in some ways, but is generally not opinionated with how form state is handled. This is where libraries like Formik or https://react-hook-form.com/ could be considered useful. We'd still need to build meaningful abstractions around whatever library we put in place to do things like disabling forms when certain permissions are not met, so I think we'd still need new patterns in place to prevent things like this in the future. I agree with the sentiment though that any time we build a CRUD form for a particular resource it should be impossible for the form to be editable without the core permissions required to edit that resource. |
@elasticmachine merge upstream |
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @nchaulet |
Summary
Resolve #187045
Implement a disabled state for global data tags.
UI Changes
With a user that has read only access to agent policies
Test
This change is covered by unit tests
Further discusssions
@elastic/fleet, @cmacknz had a really good point on how we can prevent this kind of bugs to appear? this one is a little specific as it is a really custom component, but in general maybe we could benefits of unifying a little how we build our forms and support disabled forms, consider using a form library something like formik? it looks like there is so many ways form are implemented in Kibana