[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #2146: refactor(gitab-config-ui): gitlab multi conn

[GitBox <gi...@apache.org>]

e2corporation commented on code in PR #2146:
URL: https://github.com/apache/incubator-devlake/pull/2146#discussion_r896249893


##########
config-ui/src/pages/pipelines/create.jsx:
##########
@@ -236,7 +237,7 @@ const CreatePipeline = (props) => {
     // eslint-disable-next-line no-unused-vars
     getConnectionName
   } = useConnectionManager({
-    activeProvider: jiraIntegration
+    activeProvider: jiraIntegration, gitlabIntegration

Review Comment:
   This is probably undesirable and won't work as expected. `activeProvider` should be only one 1 value that corresponds to an object entry in the `Integrations.jsx` list, which defines the primary data provider. Normally there is only 1 active provider at play at a time (usually inferred by request params or set at runtime). This is just passing `gitlabIntegration` as named prop & value, which we probably don't want.
   
   Instead, we probably want to update the `fetchAllConnections` routine in the `useConnectionManager` Hook to accept a second parameter, `providerId` (defaults to null) so that you can control what provider you are fetching all connections for instead of only the _active_ provider. `setActiveProvider("$providerId")` could also be called with a new value before fetching all connections.
   
   The issue this relates to maybe exposed on the Create a Pipeline form when you are selecting when connection instance to use. I will confirm after visual test.
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org