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

(Resolve) Tech debt of the ConnectorForm #14250

Closed
4 of 8 tasks
timroes opened this issue Jun 29, 2022 · 4 comments · Fixed by #20146
Closed
4 of 8 tasks

(Resolve) Tech debt of the ConnectorForm #14250

timroes opened this issue Jun 29, 2022 · 4 comments · Fixed by #20146
Assignees
Labels
area/frontend Related to the Airbyte webapp team/extensibility technical-debt issues to fix code smell

Comments

@timroes
Copy link
Collaborator

timroes commented Jun 29, 2022

Brainstomring issue about problem with the ServiceForm component. Please feel free to list all issues that should be addressed to make this component more stable and easier to maintain:

  • Pull out auth field hiding from FormSection to a higher level when generating from the spec which fields should be rendered.
  • values.connectionConfiguration plus flattened values are confusing (for a flattened form)
  • Rethink default behavior (should it be sending the value if the field is empty?), since it's working different depending on your field type atm -> extra issue
@timroes timroes added technical-debt issues to fix code smell area/frontend Related to the Airbyte webapp ui/connectors labels Jun 29, 2022
@octavia-squidington-iii
Copy link
Collaborator

cc @airbytehq/frontend

@timroes timroes changed the title Tech debt of the ServiceForm Tech debt of the ConnectorForm Nov 15, 2022
@sherifnada sherifnada changed the title Tech debt of the ConnectorForm (Resolve) Tech debt of the ConnectorForm Dec 1, 2022
@flash1293
Copy link
Contributor

flash1293 commented Dec 3, 2022

current selection for oneOf does not need additional state because the documentation states that each oneOf has a const property which is the same for each of the conditions (also checked via SAT): https://docs.airbyte.com/connector-development/connector-specification-reference/#using-oneof

Instead of storing this separately we can do the same thing the SAT is doing and search for the shared const field in each condition to determine the currently chosen path. This has the downside of breaking connectors which don't satisfy this SAT rule, but it seems justifiable in this case.

The derived formFields representation of the schema should include the detected "selected condition" keys for each condition node in the form, so the condition form node component itself as well as the yup schema building logic can look it up easily.

@flash1293
Copy link
Contributor

Last of the stacked PRs is in review and received feedback already, should be merged soon

@flash1293
Copy link
Contributor

Approved, waiting for the code freeze

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp team/extensibility technical-debt issues to fix code smell
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants