You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/09/02 02:03:21 UTC

[GitHub] [dolphinscheduler] limaiwang opened a new pull request, #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

limaiwang opened a new pull request, #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743

   This pr will close https://github.com/apache/dolphinscheduler/issues/11742
   
   <!--Thanks very much for contributing to Apache DolphinScheduler. Please review https://dolphinscheduler.apache.org/en-us/community/development/pull-request.html before opening a pull request.-->
   
   
   ## Purpose of the pull request
   
   <!--(For example: This pull request adds checkstyle plugin).-->
   
   ## Brief change log
   For accounts other than admin, add dependent cannot discover the projects it created
   <!--*(for example:)*
     - *Add maven-checkstyle-plugin to root pom.xml*
   -->
   ## Verify this pull request
   
   <!--*(Please pick either of the following options)*-->
   
   This pull request is code cleanup without any test coverage.
   
   *(or)*
   
   This pull request is already covered by existing tests, such as *(please describe tests)*.
   
   (or)
   
   This change added tests and can be verified as follows:
   
   <!--*(example:)*
     - *Added dolphinscheduler-dao tests for end-to-end.*
     - *Added CronUtilsTest to verify the change.*
     - *Manually verified the change by testing locally.* -->
   
   (or)
   
   If your pull request contain incompatible change, you should also add it to `docs/docs/en/guide/upgrede/incompatible.md`
   


-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] limaiwang commented on a diff in pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
limaiwang commented on code in PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#discussion_r969258464


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -169,6 +169,13 @@
         <if test="userId != 0">
             right join t_ds_relation_project_user rpu on p.id = rpu.project_id
             where rpu.user_id = #{userId}
+            union
+            select
+            <include refid="baseSqlV2">
+                <property name="alias" value="p2"/>
+            </include>
+            from t_ds_project p2
+            where p2.user_id = #{userId}

Review Comment:
   @SbloodyS  I want to accept it, let me do it。Do I need to close this pr first?



-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#discussion_r969127291


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -169,6 +169,13 @@
         <if test="userId != 0">
             right join t_ds_relation_project_user rpu on p.id = rpu.project_id
             where rpu.user_id = #{userId}
+            union
+            select
+            <include refid="baseSqlV2">
+                <property name="alias" value="p2"/>
+            </include>
+            from t_ds_project p2
+            where p2.user_id = #{userId}

Review Comment:
   I found that `queryAllProject` is also used by the working group. And it should be permission controlled.
   
   In my opinion, The project list in dependent node's permissions should not be restricted. I've created a new issue to describe it #11902. We would like to release it in the next month's 3.1.0 version. Would you like to take it? If not, I'll take it by myself. @limaiwang 



-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#discussion_r969127291


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -169,6 +169,13 @@
         <if test="userId != 0">
             right join t_ds_relation_project_user rpu on p.id = rpu.project_id
             where rpu.user_id = #{userId}
+            union
+            select
+            <include refid="baseSqlV2">
+                <property name="alias" value="p2"/>
+            </include>
+            from t_ds_project p2
+            where p2.user_id = #{userId}

Review Comment:
   I found that `queryAllProject` is also used by the working group. And it should be permission controlled.
   
   In my opinion, The project list in dependent node's permissions should not be restricted. I've created a new issue to describe it. We would like to release it in the next month's 3.1.0 version. Would you like to take it? If not, I'll take it by myself. @limaiwang 



-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] fuchanghai commented on a diff in pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
fuchanghai commented on code in PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#discussion_r966517397


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -169,6 +169,13 @@
         <if test="userId != 0">
             right join t_ds_relation_project_user rpu on p.id = rpu.project_id
             where rpu.user_id = #{userId}
+            union
+            select
+            <include refid="baseSqlV2">
+                <property name="alias" value="p2"/>
+            </include>
+            from t_ds_project p2
+            where p2.user_id = #{userId}

Review Comment:
   @ruanwenjun @SbloodyS If you don't use union, it may not be easy to get pagination in the future. I'm thinking if the creator is also put into t_ds_relation_project_user and distinguishes whether to create a user or a normal user



-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] EricGao888 commented on a diff in pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
EricGao888 commented on code in PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#discussion_r969125478


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -169,6 +169,13 @@
         <if test="userId != 0">
             right join t_ds_relation_project_user rpu on p.id = rpu.project_id
             where rpu.user_id = #{userId}
+            union
+            select
+            <include refid="baseSqlV2">
+                <property name="alias" value="p2"/>
+            </include>
+            from t_ds_project p2
+            where p2.user_id = #{userId}

Review Comment:
   I prefer to doing this in service level instead of with sql. If we have complex logic in both service and sql levels, it is much easier to introduce new bugs.



-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] caishunfeng commented on pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
caishunfeng commented on PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#issuecomment-1245033397

   this pr fix #11517 too


-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#issuecomment-1244864695

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11743)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=BUG) [![D](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D-16px.png 'D')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=BUG) [2 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=VULNERABILITY) [![B](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/B-16px.png 'B')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=VULNERABILITY) [1 Vulnerability](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=CODE_SMELL) [103 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=CODE_SMELL)
   
   [![53.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png '53.5%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11743&metric=new_coverage&view=list) [53.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11743&metric=new_coverage&view=list)  
   [![29.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/20plus-16px.png '29.3%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11743&metric=new_duplicated_lines_density&view=list) [29.3% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11743&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] ruanwenjun commented on a diff in pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#discussion_r966138736


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -169,6 +169,13 @@
         <if test="userId != 0">
             right join t_ds_relation_project_user rpu on p.id = rpu.project_id
             where rpu.user_id = #{userId}
+            union
+            select
+            <include refid="baseSqlV2">
+                <property name="alias" value="p2"/>
+            </include>
+            from t_ds_project p2
+            where p2.user_id = #{userId}

Review Comment:
   It's not recommend to use union, I hope we can do all the union, join operation in memory, we need to make the sql easier.



-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#issuecomment-1244861375

   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/11743?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#11743](https://codecov.io/gh/apache/dolphinscheduler/pull/11743?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fcca07c) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/f95cdd350a864730fac3a0a20062b22468e7e633?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f95cdd3) will **decrease** coverage by `0.00%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev   #11743      +/-   ##
   ============================================
   - Coverage     39.65%   39.65%   -0.01%     
   + Complexity     4693     4688       -5     
   ============================================
     Files          1008     1008              
     Lines         37878    37861      -17     
     Branches       4237     4232       -5     
   ============================================
   - Hits          15022    15012      -10     
   + Misses        21250    21240      -10     
   - Partials       1606     1609       +3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/11743?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...er/master/dispatch/host/assign/RandomSelector.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9kaXNwYXRjaC9ob3N0L2Fzc2lnbi9SYW5kb21TZWxlY3Rvci5qYXZh) | `77.77% <0.00%> (-5.56%)` | :arrow_down: |
   | [...dolphinscheduler/remote/future/ResponseFuture.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1yZW1vdGUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcmVtb3RlL2Z1dHVyZS9SZXNwb25zZUZ1dHVyZS5qYXZh) | `81.96% <0.00%> (-1.64%)` | :arrow_down: |
   | [...r/plugin/task/sqoop/parameter/SqoopParameters.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stc3Fvb3Avc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svc3Fvb3AvcGFyYW1ldGVyL1Nxb29wUGFyYW1ldGVycy5qYXZh) | `55.12% <0.00%> (-1.29%)` | :arrow_down: |
   | [...pache/dolphinscheduler/common/utils/DateUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0RhdGVVdGlscy5qYXZh) | `74.21% <0.00%> (-0.79%)` | :arrow_down: |
   | [...che/dolphinscheduler/common/utils/HadoopUtils.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvY29tbW9uL3V0aWxzL0hhZG9vcFV0aWxzLmphdmE=) | `15.86% <0.00%> (-0.73%)` | :arrow_down: |
   | [...uler/api/service/impl/DataAnalysisServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9EYXRhQW5hbHlzaXNTZXJ2aWNlSW1wbC5qYXZh) | `81.51% <0.00%> (-0.31%)` | :arrow_down: |
   | [...api/service/impl/ProcessDefinitionServiceImpl.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL3NlcnZpY2UvaW1wbC9Qcm9jZXNzRGVmaW5pdGlvblNlcnZpY2VJbXBsLmphdmE=) | `32.30% <0.00%> (-0.01%)` | :arrow_down: |
   | [...e/dolphinscheduler/server/master/MasterServer.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1tYXN0ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL21hc3Rlci9NYXN0ZXJTZXJ2ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...e/dolphinscheduler/server/worker/WorkerServer.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci13b3JrZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvc2VydmVyL3dvcmtlci9Xb3JrZXJTZXJ2ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [.../dolphinscheduler/plugin/task/datax/DataxTask.java](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci10YXNrLXBsdWdpbi9kb2xwaGluc2NoZWR1bGVyLXRhc2stZGF0YXgvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvcGx1Z2luL3Rhc2svZGF0YXgvRGF0YXhUYXNrLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | ... and [3 more](https://codecov.io/gh/apache/dolphinscheduler/pull/11743/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#issuecomment-1244867096

   SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=11743)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=BUG) [![D](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D-16px.png 'D')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=BUG) [2 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=VULNERABILITY) [![B](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/B-16px.png 'B')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=VULNERABILITY) [1 Vulnerability](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=CODE_SMELL) [103 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=11743&resolved=false&types=CODE_SMELL)
   
   [![53.5%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50-16px.png '53.5%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11743&metric=new_coverage&view=list) [53.5% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11743&metric=new_coverage&view=list)  
   [![29.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/20plus-16px.png '29.3%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11743&metric=new_duplicated_lines_density&view=list) [29.3% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=11743&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] EricGao888 commented on a diff in pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
EricGao888 commented on code in PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#discussion_r969125478


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -169,6 +169,13 @@
         <if test="userId != 0">
             right join t_ds_relation_project_user rpu on p.id = rpu.project_id
             where rpu.user_id = #{userId}
+            union
+            select
+            <include refid="baseSqlV2">
+                <property name="alias" value="p2"/>
+            </include>
+            from t_ds_project p2
+            where p2.user_id = #{userId}

Review Comment:
   I prefer to doing this in service level instead of with sql. If we have complex logic in both service and sql level, it is much easier to introduce new bugs.



-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] caishunfeng closed pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
caishunfeng closed pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created
URL: https://github.com/apache/dolphinscheduler/pull/11743


-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] limaiwang commented on a diff in pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
limaiwang commented on code in PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#discussion_r965413424


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -169,6 +169,13 @@
         <if test="userId != 0">
             right join t_ds_relation_project_user rpu on p.id = rpu.project_id
             where rpu.user_id = #{userId}
+            union
+            select
+            <include refid="baseSqlV2">
+                <property name="alias" value="p2"/>
+            </include>
+            from t_ds_project p2
+            where p2.user_id = #{userId}

Review Comment:
   > I think we should refactor the api used by the dependent node instead of using union all. cc @caishunfeng @zhongjiajie @EricGao888 @ruanwenjun
   Hi @SbloodyS  Is your idea to get all the projects in two api's? One to get the projects authorized by the user and one to get the projects created by the user itself. when processing the dependent module to display all the projects, we need to find out the data in both api's and then process them to display
   
   



-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#discussion_r964360571


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -169,6 +169,13 @@
         <if test="userId != 0">
             right join t_ds_relation_project_user rpu on p.id = rpu.project_id
             where rpu.user_id = #{userId}
+            union
+            select
+            <include refid="baseSqlV2">
+                <property name="alias" value="p2"/>
+            </include>
+            from t_ds_project p2
+            where p2.user_id = #{userId}

Review Comment:
   I think we should refactor the api used by the dependent node instead of using union all. cc @caishunfeng @zhongjiajie @EricGao888 @ruanwenjun 



-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] caishunfeng commented on pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
caishunfeng commented on PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#issuecomment-1248819810

   close by #11961


-- 
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@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] SbloodyS commented on a diff in pull request #11743: [Bug] [Dependent] For accounts other than admin, add dependent cannot discover the projects it created

Posted by GitBox <gi...@apache.org>.
SbloodyS commented on code in PR #11743:
URL: https://github.com/apache/dolphinscheduler/pull/11743#discussion_r969260828


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -169,6 +169,13 @@
         <if test="userId != 0">
             right join t_ds_relation_project_user rpu on p.id = rpu.project_id
             where rpu.user_id = #{userId}
+            union
+            select
+            <include refid="baseSqlV2">
+                <property name="alias" value="p2"/>
+            </include>
+            from t_ds_project p2
+            where p2.user_id = #{userId}

Review Comment:
   > @SbloodyS I want to accept it, let me do it。Do I need to close this pr first?
   
   Yes. Please reply in the issue #11902 so that I can assign to you.



-- 
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@dolphinscheduler.apache.org

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