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

[AAP-23162] Add credentials test button #2217

Merged
merged 6 commits into from May 21, 2024

Conversation

skochay330
Copy link
Member

@skochay330 skochay330 commented May 1, 2024

This PR adds a 'test' button to the credentials edit and create pages when an external credential type is selected. This button opens a modal which asks for some additional fields (dynamic depending on the credential type that is selected) and has a run button to run the test. Running the test then displays either a success or error alert on the page.

Jira issue: https://issues.redhat.com/browse/AAP-23162

Screenshot 2024-05-03 at 12 44 48 PM

Steps to see a mock successful response:
In AWX run:

DOCKER_HOST=`docker context inspect | jq -r ".[0].Endpoints.docker.Host"` VAULT=true COMPOSE_TAG=devel make docker-compose

Then, in a separate window (still in AWX):
ansible-playbook tools/docker-compose/ansible/plumb_vault.yml -e enable_ldap=false
ansible-playbook tools/docker-compose/ansible/unseal_vault.yml

Open a new window, in ansible-ui:
Point frontend to local backend using
export AWX_SERVER=https://127.0.0.1:8043
Npm run awx

In UI, go to credentials list
Vault lookup cred => edit => test button =>
Name of secret backend: my_engine
Path to secret: /my_root/my_folder
Key name: my_key

@github-actions github-actions bot added the AWX Label to indicate changes relevant to AWX label May 1, 2024
@skochay330 skochay330 force-pushed the AAP-23162_add-credential-form-test-modal branch from 5798a85 to 1945f87 Compare May 2, 2024 14:51
@github-actions github-actions bot added the AFW Ansible Framework label May 2, 2024
@nixocio nixocio requested a review from kialam May 2, 2024 19:39
@skochay330 skochay330 force-pushed the AAP-23162_add-credential-form-test-modal branch from 1945f87 to e25a692 Compare May 2, 2024 20:02
@skochay330 skochay330 changed the title WIP [AAP-23162] Add credentials test button [AAP-23162] Add credentials test button May 3, 2024
@skochay330 skochay330 force-pushed the AAP-23162_add-credential-form-test-modal branch 2 times, most recently from 4fef443 to 77d2c9b Compare May 3, 2024 19:08
@skochay330 skochay330 marked this pull request as ready for review May 3, 2024 19:09
@skochay330 skochay330 force-pushed the AAP-23162_add-credential-form-test-modal branch 4 times, most recently from c638065 to 55e0f08 Compare May 9, 2024 20:15
@github-actions github-actions bot removed the AFW Ansible Framework label May 9, 2024
@skochay330 skochay330 force-pushed the AAP-23162_add-credential-form-test-modal branch from 55e0f08 to 78ccffa Compare May 9, 2024 21:24
@skochay330 skochay330 force-pushed the AAP-23162_add-credential-form-test-modal branch from 78ccffa to 55d7f3c Compare May 13, 2024 16:18
@github-actions github-actions bot added E2E End-to-end testing CCT Component tests labels May 15, 2024
@skochay330 skochay330 force-pushed the AAP-23162_add-credential-form-test-modal branch from b214b0f to 08f8424 Compare May 20, 2024 19:35
@kialam
Copy link
Member

kialam commented May 21, 2024

This was a complicated piece of our code to nail down and implement, nice work. A suggestion to simplify/refactor some of the logic came to mind:
Use a new hook here to avoid some repetition and prop drilling in the CreateCredential and EditCredential sections of the code:

export const useTestButton = () => {
  const { t } = useTranslation();
  const openCredentialsExternalTestModal = useCredentialsTestModal()
  const [disableButton, setDisableButton] = useState(false);
  const [values, setValues] = useState({});

  const TestButton = () => (
    <Button
      variant="primary"
      onClick={() => {
        console.log('Test button clicked');
        console.log(values);
        openCredentialsExternalTestModal(...)
      }}
      isDisabled={disableButton}
    >
      {t('Test')}
    </Button>
  );
  return { TestButton, setDisableButton, setValues };
};

Then, within CreateCredential:

  const { TestButton, setDisableButton, setValues } = useTestButton();

Pass the setDisabledButton and setValues as props down to the CredentialInputs component.

Within the CredentialInputs component set a watcher for the form values to change/update:

const {
    formState: { isValid },
  } = useFormContext();
  const watchedValues = useWatch();

  useEffect(() => {
    setDisableButton(!isValid);
    setValues(watchedValues as initialValues);
  }, [isValid, setDisableButton, setValues, watchedValues]);

This would then give you the form values needed to render the Test Modal, as well as the validation logic to enable/disable the Test button within the form. We can then remove (I think):

  const [selectedCredentialTypeId, setSelectedCredentialTypeId] = useState<number>(0);
  const [watchedSubFormFields, setWatchedSubFormFields] = useState<unknown[]>([]);
  const [isTestButtonEnabled, setIsTestButtonEnabled] = useState(false);
  const [isTestButtonEnabledSubForm, setIsTestButtonEnabledSubForm] = useState(false);

Repeat for EditCredentials.

We can do this as a follow up PR as well, so I'll go ahead and approve this.

@skochay330 skochay330 force-pushed the AAP-23162_add-credential-form-test-modal branch from 08f8424 to 1cd2397 Compare May 21, 2024 15:04
@skochay330 skochay330 merged commit 5d1ba80 into main May 21, 2024
17 checks passed
@skochay330 skochay330 deleted the AAP-23162_add-credential-form-test-modal branch May 21, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AWX Label to indicate changes relevant to AWX CCT Component tests E2E End-to-end testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants