Skip to content
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

Merged

Conversation

mabashian
Copy link
Member

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:

  1. User Form
    • Organization Lookup
  2. Application Form
    • Organization Lookup
  3. Inventory Form
    • Organization Lookup
  4. Smart Inventory Form
    • Organization Lookup
  5. Project Form
    • Organization Lookup
    • Red Hat Insights -> Credential Lookup
  6. Team Form
    • Organization Lookup
  7. Job Template Form
    • Project Lookup (this was already in place but I normalized the implementation)
  8. Inventory Source Form
    • Sourced from project -> Project Lookup (this was already in place but I normalized the implementation)
    • Google Compute Engine -> Credential Lookup
    • Microsoft Azure Resource Manager -> Credential Lookup
    • VMware vCenter -> Credential Lookup
    • Red Hat Satellite 6 -> Credential Lookup
    • OpenStack -> Credential Lookup
    • Red Hat Virtualization-> Credential Lookup
    • Ansible Tower -> Credential Lookup

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 in useCallback to prevent unnecessary extra requests being triggered in the lookup. The onChange 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. The useField 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 the useCallback hooks.

Here's a quick example of what it looks like:

autopoporg

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME
  • UI

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

@unlikelyzero
Copy link

@dsesami @tiagodread , @mabashian is going on PTO this week. I think we should give this some early feedback today.

Copy link
Contributor

@jlmitch5 jlmitch5 left a 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';

/**
Copy link
Contributor

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]
);
}
Copy link
Contributor

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.

@tiagodread
Copy link
Contributor

tiagodread commented Sep 15, 2020

@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:

  1. Create a organization, 2 projects
  2. Create a normal user and give him access as admin to this organization and projects
  3. Login as this user
  4. Navigate to Job template then click on Add button

Result:
This user can’t choose a project because de field is disabled

image

Expected Result:
in the old UI this user can choose a project and create a Job template

image

Extra information: Instead of creating 2 projects, create only one, the project field is auto populated but still disabled

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

Copy link
Contributor

@tiagodread tiagodread left a 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 👍🏽

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants