You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by GitBox <gi...@apache.org> on 2022/10/28 03:19:06 UTC

[GitHub] [incubator-devlake] mintsweet commented on a diff in pull request #2886: feat: setup configuration-ui plugin registry

mintsweet commented on code in PR #2886:
URL: https://github.com/apache/incubator-devlake/pull/2886#discussion_r1007566589


##########
config-ui/src/components/Sidebar.jsx:
##########
@@ -29,35 +35,46 @@ import { ReactComponent as Logo } from '@/images/devlake-logo.svg'
 import { ReactComponent as LogoText } from '@/images/devlake-textmark.svg'
 
 import '@/styles/sidebar.scss'
-import UIContext from '../store/UIContext'
+import UIContext from '@/store/UIContext'
 
-const Sidebar = () => {
+const Sidebar = (props) => {
+  const { integrations = [] } = props
   const activeRoute = useRouteMatch()
   const uiContext = useContext(UIContext)
 
-  const [menu, setMenu] = useState(MenuConfiguration(activeRoute))
+  const getMenu = useCallback(
+    () => MenuConfiguration(activeRoute, integrations),
+    [activeRoute, integrations]
+  )
+
+  const ActiveMenu = useMemo(() => getMenu(), [getMenu])
+
+  const [menu, setMenu] = useState(ActiveMenu)
   const [versionTag, setVersionTag] = useState('')
 
-  useEffect(() => {
-    setMenu(MenuConfiguration(activeRoute))
-  }, [activeRoute])
+  // useEffect(() => {
+  //   setMenu(ActiveMenu)
+  // }, [ActiveMenu])
 
   useEffect(() => {
-    const fetchVersion = async () => {
-      try {
-        const versionUrl = `${DEVLAKE_ENDPOINT}/version`
-        const res = await request.get(versionUrl).catch((e) => {
-          console.log('>>> API VERSION ERROR...', e)
-          setVersionTag('')
-        })
-        setVersionTag(res?.data ? res.data?.version : '')
-      } catch (e) {
-        setVersionTag('')
-      }
-    }
-    fetchVersion()
+    // @todo: re-enable version fetch
+    // const fetchVersion = async () => {
+    //   try {
+    //     const versionUrl = `${DEVLAKE_ENDPOINT}/version`
+    //     const res = await request.get(versionUrl).catch((e) => {
+    //       console.log('>>> API VERSION ERROR...', e)
+    //       setVersionTag('')
+    //     })
+    //     setVersionTag(res?.data ? res.data?.version : '')
+    //   } catch (e) {
+    //     setVersionTag('')
+    //   }
+    // }
+    // fetchVersion()
   }, [])
 
+  // useEffect(() => {}, [integrations])

Review Comment:
   I think since it is a refactoring, these commented-out codes can be deleted.



##########
config-ui/src/hooks/useDataScopesManager.jsx:
##########
@@ -363,27 +397,31 @@ function useDataScopesManager({
     []
   )
 
-  const getDefaultEntities = useCallback((providerId) => {
-    let entities = []
-    switch (providerId) {
-      case Providers.GITHUB:
-      case Providers.GITLAB:
-        entities = DEFAULT_DATA_ENTITIES
-        break
-      case Providers.JIRA:
-        entities = DEFAULT_DATA_ENTITIES.filter(
-          (d) => d.name === 'issue-tracking' || d.name === 'cross-domain'
-        )
-        break
-      case Providers.JENKINS:
-        entities = DEFAULT_DATA_ENTITIES.filter((d) => d.name === 'ci-cd')
-        break
-      case Providers.TAPD:
-        entities = DEFAULT_DATA_ENTITIES.filter((d) => d.name === 'ci-cd')
-        break
-    }
-    return entities
-  }, [])
+  const getDefaultEntities = useCallback(
+    (providerId) => {
+      console.log('GET ENTITIES FOR PROVIDER =', providerId)
+      let entities = []
+      switch (providerId) {
+        case Providers.GITHUB:
+        case Providers.GITLAB:
+          entities = DEFAULT_DATA_ENTITIES
+          break
+        case Providers.JIRA:
+          entities = DEFAULT_DATA_ENTITIES.filter(
+            (d) => d.name === 'issue-tracking' || d.name === 'cross-domain'
+          )
+          break
+        case Providers.JENKINS:
+          entities = DEFAULT_DATA_ENTITIES.filter((d) => d.name === 'ci-cd')
+          break
+        case Providers.TAPD:
+          entities = DEFAULT_DATA_ENTITIES.filter((d) => d.name === 'ci-cd')

Review Comment:
   **entities** can also be defined in the plugin so that the conditional judgment here can be omitted.



##########
config-ui/src/components/Sidebar.jsx:
##########
@@ -29,35 +35,46 @@ import { ReactComponent as Logo } from '@/images/devlake-logo.svg'
 import { ReactComponent as LogoText } from '@/images/devlake-textmark.svg'
 
 import '@/styles/sidebar.scss'
-import UIContext from '../store/UIContext'
+import UIContext from '@/store/UIContext'
 
-const Sidebar = () => {
+const Sidebar = (props) => {
+  const { integrations = [] } = props
   const activeRoute = useRouteMatch()
   const uiContext = useContext(UIContext)
 
-  const [menu, setMenu] = useState(MenuConfiguration(activeRoute))
+  const getMenu = useCallback(
+    () => MenuConfiguration(activeRoute, integrations),
+    [activeRoute, integrations]
+  )
+
+  const ActiveMenu = useMemo(() => getMenu(), [getMenu])

Review Comment:
   `useMemo(() =>  MenuConfiguration(activeRoute, integrations), [...])`
   
   Wouldn't this save the **useCallback** above?



##########
config-ui/src/hooks/data-scope/useTransformationsManager.jsx:
##########
@@ -15,103 +15,109 @@
  * limitations under the License.
  *
  */
-import { useCallback, useState } from 'react'
-import { Providers } from '@/data/Providers'
+import { useCallback, useState, useContext } from 'react'
+import IntegrationsContext from '@/store/integrations-context'
+// import { Providers } from '@/data/Providers'
 import TransformationSettings from '@/models/TransformationSettings'
 import { isEqual } from 'lodash'
 
-// TODO separate to each plugin
-const getDefaultTransformations = (provider) => {
-  let transforms = {}
-  switch (provider) {
-    case Providers.GITHUB:
-      transforms = {
-        prType: '',
-        prComponent: '',
-        prBodyClosePattern: '',
-        issueSeverity: '',
-        issueComponent: '',
-        issuePriority: '',
-        issueTypeRequirement: '',
-        issueTypeBug: '',
-        issueTypeIncident: '',
-        refdiff: null,
-        productionPattern: '',
-        deploymentPattern: ''
-        // stagingPattern: '',
-        // testingPattern: ''
-      }
-      break
-    case Providers.JIRA:
-      transforms = {
-        epicKeyField: '',
-        typeMappings: {},
-        storyPointField: '',
-        remotelinkCommitShaPattern: '',
-        bugTags: [],
-        incidentTags: [],
-        requirementTags: [],
-        // @todo: verify if jira utilizes deploy tag(s)?
-        productionPattern: '',
-        deploymentPattern: ''
-        // stagingPattern: '',
-        // testingPattern: ''
-      }
-      break
-    case Providers.JENKINS:
-      transforms = {
-        productionPattern: '',
-        deploymentPattern: ''
-        // stagingPattern: '',
-        // testingPattern: ''
-      }
-      break
-    case Providers.GITLAB:
-      transforms = {
-        productionPattern: '',
-        deploymentPattern: ''
-        // stagingPattern: '',
-        // testingPattern: ''
-      }
-      break
-    case Providers.TAPD:
-      // @todo: complete tapd transforms #2673
-      transforms = {
-        issueTypeRequirement: '',
-        issueTypeBug: '',
-        issueTypeIncident: '',
-        productionPattern: '',
-        deploymentPattern: ''
-        // stagingPattern: '',
-        // testingPattern: ''
-      }
-      break
-  }
-  return transforms
-}
-
 // manage transformations in one place
 const useTransformationsManager = () => {
+  const { Providers, ProviderTransformations, Integrations } =
+    useContext(IntegrationsContext)
   const [transformations, setTransformations] = useState({})
 
-  const generateKey = (
-    connectionProvider,
-    connectionId,
-    projectNameOrBoard
-  ) => {
-    let key = `not-distinguished`
-    switch (connectionProvider) {
-      case Providers.GITHUB:
-      case Providers.GITLAB:
-      case Providers.JENKINS:
-        key = projectNameOrBoard?.id
-        break
-      case Providers.JIRA:
-        key = projectNameOrBoard?.id
-        break
-    }
-    return `${connectionProvider}/${connectionId}/${key}`
-  }
+  const getDefaultTransformations = useCallback(
+    (provider) => {
+      // let transforms = {}
+      const transforms = ProviderTransformations[provider] || {}
+      // @note: Default Transformations configured in Plugin Registry! (see @src/registry/plugins)
+      // switch (provider) {
+      //   case Providers.GITHUB:
+      //     transforms = {
+      //       prType: '',
+      //       prComponent: '',
+      //       prBodyClosePattern: '',
+      //       issueSeverity: '',
+      //       issueComponent: '',
+      //       issuePriority: '',
+      //       issueTypeRequirement: '',
+      //       issueTypeBug: '',
+      //       issueTypeIncident: '',
+      //       refdiff: null,
+      //       productionPattern: '',
+      //       deploymentPattern: ''
+      //       // stagingPattern: '',
+      //       // testingPattern: ''
+      //     }
+      //     break
+      //   case Providers.JIRA:
+      //     transforms = {
+      //       epicKeyField: '',
+      //       typeMappings: {},
+      //       storyPointField: '',
+      //       remotelinkCommitShaPattern: '',
+      //       bugTags: [],
+      //       incidentTags: [],
+      //       requirementTags: [],
+      //       // @todo: verify if jira utilizes deploy tag(s)?
+      //       productionPattern: '',
+      //       deploymentPattern: ''
+      //       // stagingPattern: '',
+      //       // testingPattern: ''
+      //     }
+      //     break
+      //   case Providers.JENKINS:
+      //     transforms = {
+      //       productionPattern: '',
+      //       deploymentPattern: ''
+      //       // stagingPattern: '',
+      //       // testingPattern: ''
+      //     }
+      //     break
+      //   case Providers.GITLAB:
+      //     transforms = {
+      //       productionPattern: '',
+      //       deploymentPattern: ''
+      //       // stagingPattern: '',
+      //       // testingPattern: ''
+      //     }
+      //     break
+      //   case Providers.TAPD:
+      //     // @todo: complete tapd transforms #2673
+      //     transforms = {
+      //       issueTypeRequirement: '',
+      //       issueTypeBug: '',
+      //       issueTypeIncident: '',
+      //       productionPattern: '',
+      //       deploymentPattern: ''
+      //       // stagingPattern: '',
+      //       // testingPattern: ''
+      //     }
+      //     break
+      // }
+      return transforms
+    },
+    [ProviderTransformations]
+  )
+
+  const generateKey = useCallback(
+    (connectionProvider, connectionId, projectNameOrBoard) => {
+      let key = `not-distinguished`
+      switch (connectionProvider) {
+        case Providers.GITHUB:
+        case Providers.GITLAB:
+        case Providers.JENKINS:
+          key = projectNameOrBoard?.id
+          break
+        case Providers.JIRA:
+          key = projectNameOrBoard?.id
+          break

Review Comment:
   I found this place before, and I didn't see the difference between the two conditions.



##########
config-ui/src/hooks/usePipelineManager.jsx:
##########
@@ -47,18 +49,9 @@ function usePipelineManager(
   const [activePipeline, setActivePipeline] = useState(NullPipelineRun)
   const [lastRunId, setLastRunId] = useState(null)
   const [pipelineRun, setPipelineRun] = useState(NullPipelineRun)
-  const [allowedProviders, setAllowedProviders] = useState([
-    Providers.JIRA,
-    Providers.GITLAB,
-    Providers.JENKINS,
-    Providers.GITHUB,
-    Providers.REFDIFF,
-    Providers.GITEXTRACTOR,
-    Providers.FEISHU,
-    Providers.AE,
-    Providers.DBT,
-    Providers.TAPD
-  ])
+  const [allowedProviders, setAllowedProviders] = useState(

Review Comment:
   **Config UI** doesn't seem to support all plugins yet, is this judgment unnecessary?



-- 
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