oAuth input parameters UI #7325
Conversation
github-actions
bot
added
area/frontend
area/platform
| @@ -105,18 +109,54 @@ export function useRunOauthFlow( | |||
| done?: boolean; | |||
| run: () => void; | |||
| } { | |||
| const { values, setValues } = useFormikContext(); | |||
| const { | |||
There was a problem hiding this comment.
Validation shouldn't be part of runOauthFlow explicitly. If we are going to add validation here - this method should be refactored to be more explicit on where we set errors and where we actually run flow.
There was a problem hiding this comment.
I think to get rid of confusion, this method shouldn't have access to formik and instead accept required props explicitly. We could add 1 more wrapper that will be adapter between form state and logic.
There was a problem hiding this comment.
Awesome work @avida, but I have to reject such a PR as it requires significant changes for now and doesn't cover all UI cases.
First of all, useConnectorAuth is a service that is responsible only for auth aspect. Not for setting validation errors and etc. This part should be handled as separate flow if we are going to set up any auth errors.
Secondly, it doesn't cover all UI cases - check slack discussion where I provided detailed info, why it won't work.
- What happens if user decides to update one of dependent fields and oauth was not run again?
- What if user passes auth for one case but then change some of the fields?
- What if user tried to submit request, got errors in some fields and then tried to pass auth? It should be obvious which fields are required for auth and which not.
- What if form is long enough - some error should be displayed within oauthFlow ( above or below the button ) that some fields are required, so user will know why his flow is not starting.
What is more, I and @jrhizor confirmed that we will go next path:
We could build more of a wizard view based on the dependencies (would likely have more overlap with the future approach) purely on the frontend side
As it is easier for future refactoring.
Though if this PR is fixed properly, where we could easily adjust for future needs - I would be glad to accept it as temporary fix.
|
@jamakase Thanks!
It was supposed to be a temporary fix for e2e oAuth testing, I was not designing full UI experience. Ive addressed all the comments except of refactoring part (as it would be redo anyway and take a lot of time for me), please review it |
avida
force-pushed
the
drezchykov/oauth-input-ui
branch
from
0b925cc to
c0a85f7
Compare
avida
force-pushed
the
drezchykov/oauth-input-ui
branch
from
c0a85f7 to
4b9de4b
Compare
| @@ -1,6 +1,8 @@ | |||
| import { useFormikContext } from "formik"; | |||
| import { useCallback, useEffect, useMemo, useRef } from "react"; | |||
| import { useAsyncFn } from "react-use"; | |||
| import { flatten } from "flat"; | |||
| import { pick } from "lodash"; | |||
There was a problem hiding this comment.
First of all, we do not have lodash in our dependencies list. What is more, we do not want to import all lodash lib here and aim to import only what we are using.
You will need to add lodash.pick as we use other libraries.
* Oauth UI codestyle fixes
* oAuth input parameters UI * fix review comments part 1 * fix review comments part 2 * Oauth UI style fixes (airbytehq#7574) * Oauth UI codestyle fixes Co-authored-by: Artem Astapenko <jamakase54@gmail.com> Co-authored-by: Artem Astapenko <3767150+Jamakase@users.noreply.github.com>
What
UI part for #6971
All parameters list on oauthFlowInputFields are validated before sending get_consent_url reque
Here is how validation looks like:
How
Describe the solution
Recommended reading order
x.javay.pythonPre-merge Checklist
Expand the relevant checklist and delete the others.
New Connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/SUMMARY.mddocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampledocs/integrations/README.mdairbyte-integrations/builds.mdAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing./publishcommand described hereUpdating a connector
Community member or Airbyter
airbyte_secret./gradlew :airbyte-integrations:connectors:<name>:integrationTest.README.mdbootstrap.md. See description and examplesdocs/integrations/<source or destination>/<name>.mdincluding changelog. See changelog exampleAirbyter
If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.
/test connector=connectors/<name>command is passing./publishcommand described hereConnector Generator
-scaffoldin their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplatesthen checking in your changes