-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Eliziario/hubspot oauth #7279
Eliziario/hubspot oauth #7279
Conversation
airbyte-oauth/src/main/java/io/airbyte/oauth/BaseOAuthFlow.java
Outdated
Show resolved
Hide resolved
… returns both access_token and refresh_token
@@ -96,31 +94,6 @@ public String getDestinationConsentUrl(final UUID workspaceId, final UUID destin | |||
return formatConsentUrl(destinationDefinitionId, getClientIdUnsafe(oAuthParamConfig), redirectUrl); | |||
} | |||
|
|||
protected String formatConsentUrl(String clientId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this removed from the base class? other oauth flows don't depend on this method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it WAS not used. Basically all the derived classes re-implement this with a lot of commonality, but also with quite a few subtle differences. It can and it should be refactored so derived classes didn't have all those copies of only-slightly-dissimilar code, but I was planning to have a specific PR just for this and other useful refactors as adding this here was going to change too many unrelated connectors. So, it is better to have a refactors-specific PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sgtm
/test connector:source-hubspot
|
/test connector=source-hubspot
|
* Hubspot OAuth backend implementation * Hubspot OAuth backend implementation - post master merge fixes * Hubspot OAuth backend implementation - missing factory * Review changes - return only refresh_token when consent flow callback returns both access_token and refresh_token * Missing import for OAuthImplementationFactory * unit test fix after merge for HubspotOAuthFlowTest * unit test fix after merge for HubspotOAuthFlowTest
What
Implements backend OAuth for Hubspot connector
#7113
How
Inherit from BaseOAuthFlow, add new subclass to factory method, create integration test and unit tests
Recommended reading order
x.java
Pre-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.md
bootstrap.md
. See description and examplesdocs/SUMMARY.md
docs/integrations/<source or destination>/<name>.md
including changelog. See changelog exampledocs/integrations/README.md
airbyte-integrations/builds.md
Airbyter
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./publish
command described hereUpdating a connector
Community member or Airbyter
airbyte_secret
./gradlew :airbyte-integrations:connectors:<name>:integrationTest
.README.md
bootstrap.md
. See description and examplesdocs/integrations/<source or destination>/<name>.md
including 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./publish
command described hereConnector Generator
-scaffold
in their name) have been updated with the latest scaffold by running./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates
then checking in your changes