-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Add organization when creating/editing an execution environments #8324
Add organization when creating/editing an execution environments #8324
Conversation
awx/ui_next/src/screens/ExecutionEnvironment/shared/ExecutionEnvironmentForm.jsx
Outdated
Show resolved
Hide resolved
awx/ui_next/src/screens/ExecutionEnvironment/shared/ExecutionEnvironmentForm.jsx
Outdated
Show resolved
Hide resolved
: () => undefined, | ||
}); | ||
|
||
const { setFieldValue } = useFormikContext(); |
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.
You don't need this you can access the functions you need to set the field value via that useStates above
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.
This was the same piece of code used in other forms, I will keep like that to be consistent. In case we decide to a general change later on. Thanks for your review. Updated the other suggestions.
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, see comment here related to the useField
: #8104
2b07625
to
3b407ef
Compare
8988079
to
39f878d
Compare
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.
You addressed all of my concerns. Looks good Thanks!
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 solid. rebase the org list PR after this merges, thanks!
Unable to freeze job graph: Job awx-push-new-schema depends on awx-ui which was not run. |
regate |
Unable to freeze job graph: Job awx-push-new-schema depends on awx-ui which was not run. |
Add organization as part of creating/editing an execution environments If one is a `system admin` the Organization is an optional field. Not providing an Organization makes the execution environment globally available. If one is a `org admin` the Organization is a required field. See: ansible#7887
39f878d
to
26dec3b
Compare
PR rebased @dsesami. |
recheck |
Add organization when creating/editing an execution environments.
If one is a
system admin
the Organization is an optional field. Notproviding an Organization makes the execution environment globally
available.
If one is a
org admin
the Organization is a required field.See: #7887
System Admin:
Org Admin