Skip to content

Commit

Permalink
馃獰 馃悰 New OAuth connectors should never say "re-authenticate" (#18487)
Browse files Browse the repository at this point in the history
* look for empty credential values, not empty credential keys

* use a more generalizable solution, move it to the service

* cleanup

* use getIn instead of get

* Update airbyte-webapp/src/views/Connector/ServiceForm/useAuthentication.tsx

Co-authored-by: Tim Roes <tim@airbyte.io>

Co-authored-by: Tim Roes <tim@airbyte.io>
  • Loading branch information
teallarson and timroes committed Nov 4, 2022
1 parent 51ac4b5 commit 6e94249
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { useMemo, useState } from "react";
import { ConnectorDefinitionSpecification } from "core/domain/connector";
import { AuthSpecification } from "core/request/AirbyteClient";
import { useRunOauthFlow } from "hooks/services/useConnectorAuth";
import { useAuthentication } from "views/Connector/ServiceForm/useAuthentication";

import { useServiceForm } from "../../../serviceFormContext";
import { ServiceFormValues } from "../../../types";
Expand Down Expand Up @@ -51,13 +52,12 @@ function useFormikOauthAdapter(connector: ConnectorDefinitionSpecification): {

const { run, loading, done } = useRunOauthFlow(connector, onDone);
const preparedValues = useMemo(() => getValues<Credentials>(values), [getValues, values]);
const connectionObjectEmpty = preparedValues?.connectionConfiguration?.credentials
? Object.keys(preparedValues.connectionConfiguration?.credentials).length <= 1
: true;

const { hasAuthFieldValues } = useAuthentication();

return {
loading,
done: done || !connectionObjectEmpty,
done: done || hasAuthFieldValues,
hasRun,
run: async () => {
const oauthInputProperties =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ interface AuthenticationHook {
* the passed in field, and false otherwise.
*/
shouldShowAuthButton: (fieldPath: string) => boolean;
/**
* This will return true if any of the hidden auth fields have values. This determines
* whether we will render "authenticate" or "re-authenticate" on the OAuth button text
*/
hasAuthFieldValues: boolean;
}

export const useAuthentication = (): AuthenticationHook => {
Expand Down Expand Up @@ -202,9 +207,14 @@ export const useAuthentication = (): AuthenticationHook => {
[advancedAuth, isAuthButtonVisible, legacyOauthSpec]
);

const hasAuthFieldValues: boolean = useMemo(() => {
return implicitAuthFieldPaths.some((path) => getIn(values, path) !== undefined);
}, [implicitAuthFieldPaths, values]);

return {
isHiddenAuthField,
hiddenAuthFieldErrors,
shouldShowAuthButton,
hasAuthFieldValues,
};
};

0 comments on commit 6e94249

Please sign in to comment.