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/20 01:15:47 UTC
[GitHub] [incubator-devlake] likyh opened a new pull request, #3496: Issues/3329 entity
likyh opened a new pull request, #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496
# Summary
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes.
Please fill out as many sections below as possible.
-->
### Does this close any open issues?
Closes xx
### Screenshots
Include any relevant screenshots here.
### Other Information
Any other information that is important to this PR.
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013006336
##########
config-ui/src/components/blueprints/create-workflow/DataScopes.jsx:
##########
@@ -67,22 +65,20 @@ const DataScopes = (props) => {
cardStyle = {}
} = props
- const selectedBoards = useMemo(
- () => boards[configuredConnection.id],
- [boards, configuredConnection?.id]
+ const selectedScopeEntities = useMemo(
+ () => scopeEntitiesGroup[configuredConnection.id],
+ [scopeEntitiesGroup, configuredConnection?.id]
)
- const selectedProjects = useMemo(
- () => projects[configuredConnection.id],
- [projects, configuredConnection?.id]
- )
-
- useEffect(() => {
- console.log('>> OVER HERE!!!', selectedBoards)
- }, [selectedBoards])
- useEffect(() => {
- console.log('>> OVER HERE FOR Projects!!!', selectedProjects)
- }, [selectedProjects])
+ const setScopeEntities = useCallback(
+ (scopeEntities) => {
+ setScopeEntitiesGroup((g) => ({
+ ...g,
+ [configuredConnection.id]: scopeEntities
Review Comment:
Emm, `configuredConnection.id` is index instead of `connection.id`. So I haven't found it.
https://github.com/apache/incubator-devlake/blob/3b24e450ed2a56d905b6b66c11e520894d47ea10/config-ui/src/hooks/useConnectionManager.jsx#L622-L628
I also think it's not a good practice. Let's tidy up these models in next refactor PR. (next milestone)
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1012948802
##########
config-ui/src/components/blueprints/ProviderTransformationSettings.jsx:
##########
@@ -45,24 +44,7 @@ const withTransformationSettings = (
)
const ProviderTransformationSettings = (props) => {
- const {
Review Comment:
To clarify, while `props` is being spread and these props are not technically in use, I think it's useful to reference these props in case this file is further extended and this makes it clear from a documentation standpoint _what_ props we expected `ProviderTransformationSettings` to depend on. If you choose to remove them, move the props to a DocBlock comment instead.
It's cleaner to to just spread `props` from a development standpoint in case we want to extend dependencies to all transformation settings components, we only have to make that change in one place (where ProviderTransforationSettings is implemented)
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013044265
##########
config-ui/src/components/blueprints/ProviderTransformationSettings.jsx:
##########
@@ -84,7 +82,20 @@ const ProviderTransformationSettings = (props) => {
provider?.id && TransformationComponents[provider?.id]
? TransformationComponents[provider?.id]
: null,
- { ...props, entities: props.entities[props?.connection?.id] }
+ {
+ dataDomains: dataDomainsGroup[connection?.id],
+ provider,
Review Comment:
Either way is fine, both are correct.
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1003249779
##########
config-ui/src/models/Connection.js:
##########
@@ -35,7 +35,7 @@
* @property {string|object?} plugin
* @property {string|object?} provider
* @property {string?} providerId
- * @property {<Array<DataEntity>>} entities
+ * @property {<Array[DataDomain]>} dataDomains
Review Comment:
Is this valid JSDoc format for specifying array of type?
```suggestion
* @property {<Array<DataDomain>>} dataDomains
```
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1003247196
##########
config-ui/src/hooks/data-scope/useTransformationsManager.jsx:
##########
@@ -94,33 +94,38 @@ const getDefaultTransformations = (provider) => {
const useTransformationsManager = () => {
const [transformations, setTransformations] = useState({})
- const generateKey = (
- connectionProvider,
- connectionId,
- projectNameOrBoard
- ) => {
+ /**
+ * @param {string} connectionProvider
+ * @param {string} connectionId
+ * @param {Entity} entity
+ * @return {string}
+ */
+ const generateKey = (connectionProvider, connectionId, entity) => {
let key = `not-distinguished`
switch (connectionProvider) {
case Providers.GITHUB:
case Providers.GITLAB:
case Providers.JENKINS:
- key = projectNameOrBoard?.id
+ key = entity?.id
break
case Providers.JIRA:
- key = projectNameOrBoard?.id
+ key = entity?.id
Review Comment:
```suggestion
key = entity?.getConfiguredEntityId()
```
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013043585
##########
config-ui/src/components/blueprints/ProviderTransformationSettings.jsx:
##########
@@ -84,7 +82,20 @@ const ProviderTransformationSettings = (props) => {
provider?.id && TransformationComponents[provider?.id]
? TransformationComponents[provider?.id]
: null,
- { ...props, entities: props.entities[props?.connection?.id] }
+ {
+ dataDomains: dataDomainsGroup[connection?.id],
+ provider,
Review Comment:
ok, although I don't agree with usage like this, fixed.
--
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
[GitHub] [incubator-devlake] e2corporation commented on pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#issuecomment-1288943976
@likyh This PR will be on-hold in the queue until Part 1 of Plugin Registry (#2886) is completed & merged.
--
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
[GitHub] [incubator-devlake] likyh commented on pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#issuecomment-1298080311
> This PR content is really too much.😂
😂 Yeah, lots of refactors are related. You can review commits one by one. I split them into different commits.
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1012980538
##########
config-ui/src/components/blueprints/DataScopesGrid.jsx:
##########
@@ -144,39 +144,29 @@ const DataScopesGrid = (props) => {
whiteSpace: 'nowrap'
}}
>
- {[Providers.GITLAB, Providers.GITHUB].includes(
- c.provider?.id
- ) && (
+ {[
Review Comment:
use `c.plugin.getMultiConnection`? But I'm not sure if `azure/bitbucket/gitee/tapd` have a title.
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1009344879
##########
config-ui/src/hooks/data-scope/useTransformationsManager.jsx:
##########
@@ -94,33 +94,38 @@ const getDefaultTransformations = (provider) => {
const useTransformationsManager = () => {
const [transformations, setTransformations] = useState({})
- const generateKey = (
- connectionProvider,
- connectionId,
- projectNameOrBoard
- ) => {
+ /**
+ * @param {string} connectionProvider
+ * @param {string} connectionId
+ * @param {Entity} entity
+ * @return {string}
+ */
+ const generateKey = (connectionProvider, connectionId, entity) => {
let key = `not-distinguished`
switch (connectionProvider) {
case Providers.GITHUB:
case Providers.GITLAB:
case Providers.JENKINS:
- key = projectNameOrBoard?.id
+ key = entity?.id
break
case Providers.JIRA:
- key = projectNameOrBoard?.id
+ key = entity?.id
Review Comment:
changed by other PR.
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013021179
##########
config-ui/src/components/blueprints/ProviderTransformationSettings.jsx:
##########
@@ -84,7 +82,20 @@ const ProviderTransformationSettings = (props) => {
provider?.id && TransformationComponents[provider?.id]
? TransformationComponents[provider?.id]
: null,
- { ...props, entities: props.entities[props?.connection?.id] }
+ {
+ dataDomains: dataDomainsGroup[connection?.id],
+ provider,
Review Comment:
One of the reasons for using the HOC function is so that spreading `...props` is much cleaner and less code, and there will be no need to maintain these props manually.
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013042236
##########
config-ui/src/components/blueprints/DataScopesGrid.jsx:
##########
@@ -144,39 +144,29 @@ const DataScopesGrid = (props) => {
whiteSpace: 'nowrap'
}}
>
- {[Providers.GITLAB, Providers.GITHUB].includes(
- c.provider?.id
- ) && (
+ {[
Review Comment:
fixed
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1009530269
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -93,65 +78,35 @@ const DataTransformations = (props) => {
() =>
[Providers.TAPD].includes(configuredConnection?.provider) ||
([Providers.GITLAB].includes(configuredConnection?.provider) &&
- dataEntities[configuredConnection?.id].every(
- (e) => e.value !== DataEntityTypes.DEVOPS
+ dataDomainsGroup[configuredConnection?.id].every(
+ (e) => e.value !== DataDomainTypes.DEVOPS
)),
[
configuredConnection?.provider,
configuredConnection?.id,
- dataEntities,
+ dataDomainsGroup,
Providers.TAPD,
Providers.GITLAB
]
)
- const boardsAndProjects = useMemo(
+ const scopeEntities = useMemo(
() => [
- ...(Array.isArray(boards[configuredConnection?.id])
- ? boards[configuredConnection?.id]
- : []),
- ...(Array.isArray(projects[configuredConnection?.id])
- ? projects[configuredConnection?.id]
+ ...(Array.isArray(scopeEntitiesGroup[configuredConnection?.id])
+ ? scopeEntitiesGroup[configuredConnection?.id]
: [])
],
- [projects, boards, configuredConnection?.id]
+ [scopeEntitiesGroup, configuredConnection?.id]
)
- const [entityList, setEntityList] = useState(
- boardsAndProjects?.map((e, eIdx) => ({
- id: eIdx,
- value: e?.value,
- title: e?.title,
- entity: e,
- type: e.variant
- }))
- )
- const [activeEntity, setActiveEntity] = useState()
-
- useEffect(() => {
- console.log('>>> PROJECT/BOARD SELECT LIST DATA...', entityList)
- setActiveEntity(Array.isArray(entityList) ? entityList[0] : null)
- }, [entityList])
-
useEffect(() => {
+ console.log('>>> SCOPE ENTITIES SELECT LIST DATA...', scopeEntities)
if (useDropdownSelector) {
- console.log('>>>>> PROJECT / BOARD ENTITY SELECTED!', activeEntity)
- switch (activeEntity?.type) {
- case Variants.BOARD:
- addBoardTransformation(activeEntity?.entity)
- break
- case Variants.PROJECT:
- default:
- addProjectTransformation(activeEntity?.entity)
- break
- }
+ setConfiguredScopeEntity(
+ Array.isArray(scopeEntities) ? scopeEntities[0] : null
Review Comment:
The original intent here was the dropdown selector should select the Active Scope entity being configured, if `scopeEntities[0]` is chosen here wouldn't this just select the first one in the list instead of the `activeEntity?.entity` ?
##########
config-ui/src/models/Entity.js:
##########
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+/**
+ * @typedef {object} GitHubProject
Review Comment:
```suggestion
* @typedef {object} Entity
```
##########
config-ui/src/components/blueprints/DataScopesGrid.jsx:
##########
@@ -144,39 +144,29 @@ const DataScopesGrid = (props) => {
whiteSpace: 'nowrap'
}}
>
- {[Providers.GITLAB, Providers.GITHUB].includes(
- c.provider?.id
- ) && (
+ {[
Review Comment:
Going forward we'll need to cleanup some of the Grouped Provider arrays to be more dynamic behavior in accordance with Plugin Registry. Can we perform a check like `entity?.getConfiguredEntityId() !== null` as a better means of determining whether we should list scope entities?
Either that or another bool prop added to the base `Entity` Model, `hasScopeEntities (true|false)` that way we can check this prop to determine UI features related to entities?
##########
config-ui/src/components/blueprints/create-workflow/DataScopes.jsx:
##########
@@ -67,22 +65,20 @@ const DataScopes = (props) => {
cardStyle = {}
} = props
- const selectedBoards = useMemo(
- () => boards[configuredConnection.id],
- [boards, configuredConnection?.id]
+ const selectedScopeEntities = useMemo(
+ () => scopeEntitiesGroup[configuredConnection.id],
+ [scopeEntitiesGroup, configuredConnection?.id]
)
- const selectedProjects = useMemo(
- () => projects[configuredConnection.id],
- [projects, configuredConnection?.id]
- )
-
- useEffect(() => {
- console.log('>> OVER HERE!!!', selectedBoards)
- }, [selectedBoards])
- useEffect(() => {
- console.log('>> OVER HERE FOR Projects!!!', selectedProjects)
- }, [selectedProjects])
+ const setScopeEntities = useCallback(
+ (scopeEntities) => {
+ setScopeEntitiesGroup((g) => ({
+ ...g,
+ [configuredConnection.id]: scopeEntities
Review Comment:
@likyh Have you had a chance to review this scenario with regards to storing entities by Connection ID?
##########
config-ui/src/components/blueprints/create-workflow/DataScopes.jsx:
##########
@@ -67,22 +65,20 @@ const DataScopes = (props) => {
cardStyle = {}
} = props
- const selectedBoards = useMemo(
- () => boards[configuredConnection.id],
- [boards, configuredConnection?.id]
+ const selectedScopeEntities = useMemo(
+ () => scopeEntitiesGroup[configuredConnection.id],
+ [scopeEntitiesGroup, configuredConnection?.id]
)
- const selectedProjects = useMemo(
- () => projects[configuredConnection.id],
- [projects, configuredConnection?.id]
- )
-
- useEffect(() => {
- console.log('>> OVER HERE!!!', selectedBoards)
- }, [selectedBoards])
- useEffect(() => {
- console.log('>> OVER HERE FOR Projects!!!', selectedProjects)
- }, [selectedProjects])
+ const setScopeEntities = useCallback(
+ (scopeEntities) => {
+ setScopeEntitiesGroup((g) => ({
+ ...g,
+ [configuredConnection.id]: scopeEntities
Review Comment:
@likyh Now that `projects` and `boards` and `jobs` all share the same map for **Scope Entities**, we can no longer key this by just `connection.id` alone. Consider the scenario where we have a new DevLake installation and user creates a multi-connection Blueprint. If the user Adds a New Data Connection for each Provider, they will all have a **Connection ID** of **1**. This will create an undesired scenario where Establishing entities for Connection A will be overwritten when entities for Connection B are configured, creating a bug.
I think we need to re-key this with a similar generated key (`providerId`+`connectionId`) like the one used to store Transformations, and refactor all related references to use the generated key.
##########
config-ui/src/components/blueprints/ProviderTransformationSettings.jsx:
##########
@@ -45,24 +44,7 @@ const withTransformationSettings = (
)
const ProviderTransformationSettings = (props) => {
- const {
Review Comment:
@likyh These props are **not** unused, do not remove them. These props are spread within the HOC on `Ln 69` (`...props`). Though not accessed directly at the moment, it's easier to see what props are being injected to each transformation settings component without having to look at where `<ProviderTransformationSettings>` is being implemented.
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013003855
##########
config-ui/src/components/blueprints/DataScopesGrid.jsx:
##########
@@ -144,39 +144,29 @@ const DataScopesGrid = (props) => {
whiteSpace: 'nowrap'
}}
>
- {[Providers.GITLAB, Providers.GITHUB].includes(
- c.provider?.id
- ) && (
+ {[
Review Comment:
In this particular scenario it's not related to multi-connection, we mainly need to know If this provider has a concept of Entity. So this is why we can simply call a check for the `coniguredEntityId()` or extend the models with a new prop to represent this trait.
Another refactor idea for replacing all of these is to add set of memo consts to **Integrations Manager Hook** that represents groups of Providers that **Have** certain traits or that **Can** do certain actions or **Use** certain features, this way the arrays are centralized in one place. Ideally we should stop using these types of arrays except in limited feature scenarios, and resort to other alternatives that can work dynamically for all plugins in the Registry. This would also be done by extending the Entity Data Model props where needed to add more prop checks.
```jsx
// Sample Array Replacements for Provider Grouped Traits (Abstract)
function useIntegrations() {
....
// Grouped Entity Traits & Behaviors
const ProvidersThatHaveEntities = useMemo(() => [Providers.GITHUB, Providers.JIRA], [Providers])
const ProvidersThatHaveWebhooks = useMemo(() => [Providers.DORA], [Providers])
const ProvidersThatCanDeploy = useMemo(() => [Providers.GITHUB, Providers.GITLAB], [Providers])
const ProvidersThatCanExpire = useMemo(() => [Providers.GITEE, Providers.TAPD], [Providers])
....
}
```
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013014446
##########
config-ui/src/components/blueprints/DataScopesGrid.jsx:
##########
@@ -144,39 +144,29 @@ const DataScopesGrid = (props) => {
whiteSpace: 'nowrap'
}}
>
- {[Providers.GITLAB, Providers.GITHUB].includes(
- c.provider?.id
- ) && (
+ {[
Review Comment:
`coniguredEntityId()` is defined in entity. But here `c` means `DataScopeConnection`.
Defined a new function in `DataScopeConnection`?
```
function hasScopeEntities() {
return this.scopeEntities > 0
}
```
I agree with `ProvidersThatXXXX`.
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1003270108
##########
config-ui/src/components/blueprints/create-workflow/DataScopes.jsx:
##########
@@ -144,27 +140,22 @@ const DataScopes = (props) => {
disabled={isRunning}
placeholder='username/repo, username/another-repo'
values={
- projects[configuredConnection.id]?.map(
- (p) => p.value
- ) || []
+ selectedScopeEntities?.map((p) => p.value) || []
}
fill={true}
onChange={(values) =>
- setProjects((p) => ({
- ...p,
- [configuredConnection.id]: [
- ...values.map(
- (v, vIdx) =>
- new GitHubProject({
- id: v,
- key: v,
- title: v,
- value: v,
- type: 'string'
- })
- )
- ]
- }))
+ setScopeEntities([
+ ...values.map(
+ (v, vIdx) =>
+ new GitHubProject({
Review Comment:
@likyh I see, it looks like the `TagInput` component I created earlier is limited for Github use only. Ideally this `<TagInput>.....` sub-component should be extracted in the future to `ProjectsSelector`
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1003248582
##########
config-ui/src/components/blueprints/create-workflow/DataScopes.jsx:
##########
@@ -144,27 +140,22 @@ const DataScopes = (props) => {
disabled={isRunning}
placeholder='username/repo, username/another-repo'
values={
- projects[configuredConnection.id]?.map(
- (p) => p.value
- ) || []
+ selectedScopeEntities?.map((p) => p.value) || []
}
fill={true}
onChange={(values) =>
- setProjects((p) => ({
- ...p,
- [configuredConnection.id]: [
- ...values.map(
- (v, vIdx) =>
- new GitHubProject({
- id: v,
- key: v,
- title: v,
- value: v,
- type: 'string'
- })
- )
- ]
- }))
+ setScopeEntities([
+ ...values.map(
+ (v, vIdx) =>
+ new GitHubProject({
Review Comment:
https://github.com/apache/incubator-devlake/pull/3496/files#diff-14f5220fdc068ccea4c51287ebc5ba3b26882d1e542d87daff33c5470207525cR132-R134
Here is the github repo input. There is no other entity
--
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
[GitHub] [incubator-devlake] likyh merged pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh merged PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013022150
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -93,65 +78,35 @@ const DataTransformations = (props) => {
() =>
[Providers.TAPD].includes(configuredConnection?.provider) ||
([Providers.GITLAB].includes(configuredConnection?.provider) &&
- dataEntities[configuredConnection?.id].every(
- (e) => e.value !== DataEntityTypes.DEVOPS
+ dataDomainsGroup[configuredConnection?.id].every(
+ (e) => e.value !== DataDomainTypes.DEVOPS
)),
[
configuredConnection?.provider,
configuredConnection?.id,
- dataEntities,
+ dataDomainsGroup,
Providers.TAPD,
Providers.GITLAB
]
)
- const boardsAndProjects = useMemo(
+ const scopeEntities = useMemo(
() => [
- ...(Array.isArray(boards[configuredConnection?.id])
- ? boards[configuredConnection?.id]
- : []),
- ...(Array.isArray(projects[configuredConnection?.id])
- ? projects[configuredConnection?.id]
+ ...(Array.isArray(scopeEntitiesGroup[configuredConnection?.id])
+ ? scopeEntitiesGroup[configuredConnection?.id]
: [])
],
- [projects, boards, configuredConnection?.id]
+ [scopeEntitiesGroup, configuredConnection?.id]
)
- const [entityList, setEntityList] = useState(
- boardsAndProjects?.map((e, eIdx) => ({
- id: eIdx,
- value: e?.value,
- title: e?.title,
- entity: e,
- type: e.variant
- }))
- )
- const [activeEntity, setActiveEntity] = useState()
-
- useEffect(() => {
- console.log('>>> PROJECT/BOARD SELECT LIST DATA...', entityList)
- setActiveEntity(Array.isArray(entityList) ? entityList[0] : null)
- }, [entityList])
-
useEffect(() => {
+ console.log('>>> SCOPE ENTITIES SELECT LIST DATA...', scopeEntities)
if (useDropdownSelector) {
- console.log('>>>>> PROJECT / BOARD ENTITY SELECTED!', activeEntity)
- switch (activeEntity?.type) {
- case Variants.BOARD:
- addBoardTransformation(activeEntity?.entity)
- break
- case Variants.PROJECT:
- default:
- addProjectTransformation(activeEntity?.entity)
- break
- }
+ setConfiguredScopeEntity(
+ Array.isArray(scopeEntities) ? scopeEntities[0] : null
Review Comment:
Yes. I test that situation. I have a blueprint with 4 plugins and 2 scopes. Each plugin have a scope with all transformation and another scope without any transformation. They can display, edit and submit success.
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013003855
##########
config-ui/src/components/blueprints/DataScopesGrid.jsx:
##########
@@ -144,39 +144,29 @@ const DataScopesGrid = (props) => {
whiteSpace: 'nowrap'
}}
>
- {[Providers.GITLAB, Providers.GITHUB].includes(
- c.provider?.id
- ) && (
+ {[
Review Comment:
In this particular scenario it's not related to multi-connection, we mainly need to know If this provider has a concept of Entity. So this is why we can simply call a check for the `coniguredEntityId()` or extend the models with a new prop to represent this trait.
Another refactor idea for replacing all of these is to add set of memo consts to Transformations Manager that represents groups of Providers that **Have** certain traits or that **Can** do certain actions or **Use** certain features, this way the arrays are centralized in one place. Ideally we should stop using these types of arrays except in limited feature scenarios, and resort to other alternatives that can work dynamically for all plugins in the Registry. This would also be done by extending the Entity Data Model props where needed to add more prop checks.
```jsx
// Sample Array Replacements for Provider Grouped Traits (Abstract)
function useTransformationsManager() {
....
// Grouped Entity Traits & Behaviors
const ProvidersThatHaveEntities = useMemo(() => [Providers.GITHUB, Providers.JIRA], [Providers])
const ProvidersThatHaveWebhooks = useMemo(() => [Providers.DORA], [Providers])
const ProvidersThatCanDeploy = useMemo(() => [Providers.GITHUB, Providers.GITLAB], [Providers])
const ProvidersThatCanExpire = useMemo(() => [Providers.GITEE, Providers.TAPD], [Providers])
....
}
```
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013012612
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -93,65 +78,35 @@ const DataTransformations = (props) => {
() =>
[Providers.TAPD].includes(configuredConnection?.provider) ||
([Providers.GITLAB].includes(configuredConnection?.provider) &&
- dataEntities[configuredConnection?.id].every(
- (e) => e.value !== DataEntityTypes.DEVOPS
+ dataDomainsGroup[configuredConnection?.id].every(
+ (e) => e.value !== DataDomainTypes.DEVOPS
)),
[
configuredConnection?.provider,
configuredConnection?.id,
- dataEntities,
+ dataDomainsGroup,
Providers.TAPD,
Providers.GITLAB
]
)
- const boardsAndProjects = useMemo(
+ const scopeEntities = useMemo(
() => [
- ...(Array.isArray(boards[configuredConnection?.id])
- ? boards[configuredConnection?.id]
- : []),
- ...(Array.isArray(projects[configuredConnection?.id])
- ? projects[configuredConnection?.id]
+ ...(Array.isArray(scopeEntitiesGroup[configuredConnection?.id])
+ ? scopeEntitiesGroup[configuredConnection?.id]
: [])
],
- [projects, boards, configuredConnection?.id]
+ [scopeEntitiesGroup, configuredConnection?.id]
)
- const [entityList, setEntityList] = useState(
- boardsAndProjects?.map((e, eIdx) => ({
- id: eIdx,
- value: e?.value,
- title: e?.title,
- entity: e,
- type: e.variant
- }))
- )
- const [activeEntity, setActiveEntity] = useState()
-
- useEffect(() => {
- console.log('>>> PROJECT/BOARD SELECT LIST DATA...', entityList)
- setActiveEntity(Array.isArray(entityList) ? entityList[0] : null)
- }, [entityList])
-
useEffect(() => {
+ console.log('>>> SCOPE ENTITIES SELECT LIST DATA...', scopeEntities)
if (useDropdownSelector) {
- console.log('>>>>> PROJECT / BOARD ENTITY SELECTED!', activeEntity)
- switch (activeEntity?.type) {
- case Variants.BOARD:
- addBoardTransformation(activeEntity?.entity)
- break
- case Variants.PROJECT:
- default:
- addProjectTransformation(activeEntity?.entity)
- break
- }
+ setConfiguredScopeEntity(
+ Array.isArray(scopeEntities) ? scopeEntities[0] : null
Review Comment:
If you Edit a blueprint's Data Connection with multiple projects/boards, is the project dropdown selector's initial rendering correctly? If so this comment can be resolved/dismissed.
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013015818
##########
config-ui/src/components/blueprints/DataScopesGrid.jsx:
##########
@@ -144,39 +144,29 @@ const DataScopesGrid = (props) => {
whiteSpace: 'nowrap'
}}
>
- {[Providers.GITLAB, Providers.GITHUB].includes(
- c.provider?.id
- ) && (
+ {[
Review Comment:
Yea adding something to a Model in scope of this logic would be fine as well.
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1009437791
##########
config-ui/src/models/Connection.js:
##########
@@ -35,7 +35,7 @@
* @property {string|object?} plugin
* @property {string|object?} provider
* @property {string?} providerId
- * @property {<Array<DataEntity>>} entities
+ * @property {<Array[DataDomain]>} dataDomains
Review Comment:
fixed
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1012986884
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -93,65 +78,35 @@ const DataTransformations = (props) => {
() =>
[Providers.TAPD].includes(configuredConnection?.provider) ||
([Providers.GITLAB].includes(configuredConnection?.provider) &&
- dataEntities[configuredConnection?.id].every(
- (e) => e.value !== DataEntityTypes.DEVOPS
+ dataDomainsGroup[configuredConnection?.id].every(
+ (e) => e.value !== DataDomainTypes.DEVOPS
)),
[
configuredConnection?.provider,
configuredConnection?.id,
- dataEntities,
+ dataDomainsGroup,
Providers.TAPD,
Providers.GITLAB
]
)
- const boardsAndProjects = useMemo(
+ const scopeEntities = useMemo(
() => [
- ...(Array.isArray(boards[configuredConnection?.id])
- ? boards[configuredConnection?.id]
- : []),
- ...(Array.isArray(projects[configuredConnection?.id])
- ? projects[configuredConnection?.id]
+ ...(Array.isArray(scopeEntitiesGroup[configuredConnection?.id])
+ ? scopeEntitiesGroup[configuredConnection?.id]
: [])
],
- [projects, boards, configuredConnection?.id]
+ [scopeEntitiesGroup, configuredConnection?.id]
)
- const [entityList, setEntityList] = useState(
- boardsAndProjects?.map((e, eIdx) => ({
- id: eIdx,
- value: e?.value,
- title: e?.title,
- entity: e,
- type: e.variant
- }))
- )
- const [activeEntity, setActiveEntity] = useState()
-
- useEffect(() => {
- console.log('>>> PROJECT/BOARD SELECT LIST DATA...', entityList)
- setActiveEntity(Array.isArray(entityList) ? entityList[0] : null)
- }, [entityList])
-
useEffect(() => {
+ console.log('>>> SCOPE ENTITIES SELECT LIST DATA...', scopeEntities)
if (useDropdownSelector) {
- console.log('>>>>> PROJECT / BOARD ENTITY SELECTED!', activeEntity)
- switch (activeEntity?.type) {
- case Variants.BOARD:
- addBoardTransformation(activeEntity?.entity)
- break
- case Variants.PROJECT:
- default:
- addProjectTransformation(activeEntity?.entity)
- break
- }
+ setConfiguredScopeEntity(
+ Array.isArray(scopeEntities) ? scopeEntities[0] : null
Review Comment:
`activeTransformation` is deps on `configuredScopeEntity`, I guess.
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013044265
##########
config-ui/src/components/blueprints/ProviderTransformationSettings.jsx:
##########
@@ -84,7 +82,20 @@ const ProviderTransformationSettings = (props) => {
provider?.id && TransformationComponents[provider?.id]
? TransformationComponents[provider?.id]
: null,
- { ...props, entities: props.entities[props?.connection?.id] }
+ {
+ dataDomains: dataDomainsGroup[connection?.id],
+ provider,
Review Comment:
Either way is fine, both are correct. You can change to manual props later if you desire.
--
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
[GitHub] [incubator-devlake] likyh commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1012979802
##########
config-ui/src/components/blueprints/ProviderTransformationSettings.jsx:
##########
@@ -45,24 +44,7 @@ const withTransformationSettings = (
)
const ProviderTransformationSettings = (props) => {
- const {
Review Comment:
fixed
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013003855
##########
config-ui/src/components/blueprints/DataScopesGrid.jsx:
##########
@@ -144,39 +144,29 @@ const DataScopesGrid = (props) => {
whiteSpace: 'nowrap'
}}
>
- {[Providers.GITLAB, Providers.GITHUB].includes(
- c.provider?.id
- ) && (
+ {[
Review Comment:
In this particular scenario it's not related to multi-connection, we mainly need to know If this provider has a concept of Entity. So this is why we can simply call a check for the `coniguredEntityId()` or extend the models with a new prop to represent this trait.
Another refactor idea for replacing all of these is to add set of memo consts to **Integrations Manager Hook** that represents groups of Providers that **Have** certain traits or that **Can** do certain actions or **Use** certain features, this way the arrays are centralized in one place. Ideally we should stop using these types of arrays except in limited feature scenarios, and resort to other alternatives that can work dynamically for all plugins in the Registry. This would also be done by extending the Entity Data Model props where needed to add more prop checks.
```jsx
// Sample Array Replacements for Provider Grouped Traits (Abstract)
// @hooks/useIntegrations.jsx
function useIntegrations() {
....
// Grouped Entity Traits & Behaviors
const ProvidersThatHaveEntities = useMemo(() => [Providers.GITHUB, Providers.JIRA], [Providers])
const ProvidersThatHaveWebhooks = useMemo(() => [Providers.DORA], [Providers])
const ProvidersThatCanDeploy = useMemo(() => [Providers.GITHUB, Providers.GITLAB], [Providers])
const ProvidersThatCanExpire = useMemo(() => [Providers.GITEE, Providers.TAPD], [Providers])
....
}
```
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013010340
##########
config-ui/src/components/blueprints/create-workflow/DataScopes.jsx:
##########
@@ -67,22 +65,20 @@ const DataScopes = (props) => {
cardStyle = {}
} = props
- const selectedBoards = useMemo(
- () => boards[configuredConnection.id],
- [boards, configuredConnection?.id]
+ const selectedScopeEntities = useMemo(
+ () => scopeEntitiesGroup[configuredConnection.id],
+ [scopeEntitiesGroup, configuredConnection?.id]
)
- const selectedProjects = useMemo(
- () => projects[configuredConnection.id],
- [projects, configuredConnection?.id]
- )
-
- useEffect(() => {
- console.log('>> OVER HERE!!!', selectedBoards)
- }, [selectedBoards])
- useEffect(() => {
- console.log('>> OVER HERE FOR Projects!!!', selectedProjects)
- }, [selectedProjects])
+ const setScopeEntities = useCallback(
+ (scopeEntities) => {
+ setScopeEntitiesGroup((g) => ({
+ ...g,
+ [configuredConnection.id]: scopeEntities
Review Comment:
Ok, this was another reason why I noted Index ID was important within the code comments when I setup the all provider connections map. I think it would be ideal to use a generated key like we do with transformations manager for improved consistency. As long as Connection **Index ID** with respect to the all connections list is being used this is ok for now.
--
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
[GitHub] [incubator-devlake] likyh commented on pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
likyh commented on PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#issuecomment-1297197409
@e2corporation rebased, need your review
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1003244896
##########
config-ui/src/components/blueprints/create-workflow/DataScopes.jsx:
##########
@@ -144,27 +140,22 @@ const DataScopes = (props) => {
disabled={isRunning}
placeholder='username/repo, username/another-repo'
values={
- projects[configuredConnection.id]?.map(
- (p) => p.value
- ) || []
+ selectedScopeEntities?.map((p) => p.value) || []
}
fill={true}
onChange={(values) =>
- setProjects((p) => ({
- ...p,
- [configuredConnection.id]: [
- ...values.map(
- (v, vIdx) =>
- new GitHubProject({
- id: v,
- key: v,
- title: v,
- value: v,
- type: 'string'
- })
- )
- ]
- }))
+ setScopeEntities([
+ ...values.map(
+ (v, vIdx) =>
+ new GitHubProject({
Review Comment:
This would mean all scope entities are GitHub Projects? Models need to be instantiated by the Provider associated with the entity. ie. `GitHubProject` vs `GitlabProject` vs `JenkinsJob` etc.
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1003246898
##########
config-ui/src/hooks/data-scope/useTransformationsManager.jsx:
##########
@@ -94,33 +94,38 @@ const getDefaultTransformations = (provider) => {
const useTransformationsManager = () => {
const [transformations, setTransformations] = useState({})
- const generateKey = (
- connectionProvider,
- connectionId,
- projectNameOrBoard
- ) => {
+ /**
+ * @param {string} connectionProvider
+ * @param {string} connectionId
+ * @param {Entity} entity
+ * @return {string}
+ */
+ const generateKey = (connectionProvider, connectionId, entity) => {
let key = `not-distinguished`
switch (connectionProvider) {
case Providers.GITHUB:
case Providers.GITLAB:
case Providers.JENKINS:
- key = projectNameOrBoard?.id
+ key = entity?.id
Review Comment:
We should be using the accessory function instead, `entity.getConfiguredEntityId()`
```suggestion
key = entity?.getConfiguredEntityId()
```
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013010340
##########
config-ui/src/components/blueprints/create-workflow/DataScopes.jsx:
##########
@@ -67,22 +65,20 @@ const DataScopes = (props) => {
cardStyle = {}
} = props
- const selectedBoards = useMemo(
- () => boards[configuredConnection.id],
- [boards, configuredConnection?.id]
+ const selectedScopeEntities = useMemo(
+ () => scopeEntitiesGroup[configuredConnection.id],
+ [scopeEntitiesGroup, configuredConnection?.id]
)
- const selectedProjects = useMemo(
- () => projects[configuredConnection.id],
- [projects, configuredConnection?.id]
- )
-
- useEffect(() => {
- console.log('>> OVER HERE!!!', selectedBoards)
- }, [selectedBoards])
- useEffect(() => {
- console.log('>> OVER HERE FOR Projects!!!', selectedProjects)
- }, [selectedProjects])
+ const setScopeEntities = useCallback(
+ (scopeEntities) => {
+ setScopeEntitiesGroup((g) => ({
+ ...g,
+ [configuredConnection.id]: scopeEntities
Review Comment:
Ok, this was another reason why I noted Index ID was important within the code comments when I setup the all provider connections map. I think it would be ideal to use a generated key like we do with transformations manager for improved consistency.
--
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
[GitHub] [incubator-devlake] e2corporation commented on pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#issuecomment-1302236814
@likyh PR #3666 should be merged before this one Ideally.
--
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
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3496: Issues/3329 entity
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3496:
URL: https://github.com/apache/incubator-devlake/pull/3496#discussion_r1013021179
##########
config-ui/src/components/blueprints/ProviderTransformationSettings.jsx:
##########
@@ -84,7 +82,20 @@ const ProviderTransformationSettings = (props) => {
provider?.id && TransformationComponents[provider?.id]
? TransformationComponents[provider?.id]
: null,
- { ...props, entities: props.entities[props?.connection?.id] }
+ {
+ dataDomains: dataDomainsGroup[connection?.id],
+ provider,
Review Comment:
The reason for using the HOC function is so that spreading `...props` is much cleaner and less code, and there will be no need to maintain these props manually.
--
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