-
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
Auto populate various required lookups on various forms #8104
Auto populate various required lookups on various forms #8104
Conversation
Build succeeded.
|
@dsesami @tiagodread , @mabashian is going on PTO this week. I think we should give this some early feedback today. |
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 good, aside from unused doc comment
@@ -0,0 +1,24 @@ | |||
import { useCallback, useRef } from 'react'; | |||
|
|||
/** |
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.
I think we should either fill this in or get rid of the comment
}, | ||
[populateLookupField] | ||
); | ||
} |
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 makes sense to me.
@mabashian As a normal user who has admin access to an Organization and Projects I can’t choose a project on Job template form because the lookup field is disabled. Steps to reproduce:
Result: Expected Result: Extra information: Instead of creating 2 projects, create only one, the project field is auto populated but still disabled |
…st as it wasn't actually testing the desired behavior.
8ec072a
to
10ae6c9
Compare
Build succeeded.
|
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.
Tests and stubs covered on 5530 👍🏽
Build succeeded (gate pipeline).
|
SUMMARY
link #4254
Auto populates the following fields on the following forms if the user only has access to one resource for the field in question:
In order to achieve this and make it as uniform as possible I added the useAutoPopulateLookup hook. This simple hook will trigger the callback to update the field value the first time through if enabled. This hook was added to the CredentialLookup, OrganizationLookup, and ProjectLookup components.
As a result of these changes, a lot of
onChange
functions in various forms needed to be wrapped inuseCallback
to prevent unnecessary extra requests being triggered in the lookup. TheonChange
function is now referenced inside the useRequest hook and as a result is in it's dependency array and shouldn't be changing with each render (hence useCallback). There was one snag with this pattern: jaredpalmer/formik#2268. TheuseField
helpers in Formik were causing the callback function to be re-generated with each render (and as result were causing numerous extra GET requests by the lookup components). Fortunately,setFieldValue
does not suffer the same fate: jaredpalmer/formik#2268 (comment). So I refactored a lot of our functions to use that inside theuseCallback
hooks.Here's a quick example of what it looks like:
ISSUE TYPE
COMPONENT NAME