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/19 01:57:19 UTC

[GitHub] [dolphinscheduler] limaiwang opened a new pull request, #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

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

   
   ## Purpose of the pull request
   
   close https://github.com/apache/dolphinscheduler/issues/11902
   ## Brief change log
   
   The project list in dependent node's permissions should not be restricted. Suggesting create a new API without project permission restrictions and provide it to dependent nodes in UI.
   
   ## 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 closed pull request #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

Posted by GitBox <gi...@apache.org>.
limaiwang closed pull request #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted. 
URL: https://github.com/apache/dolphinscheduler/pull/12034


-- 
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 #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12034)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12034&resolved=false&types=BUG) [![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=12034&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12034&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=12034&resolved=false&types=VULNERABILITY) [![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=12034&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12034&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=12034&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=12034&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12034&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=12034&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=12034&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12034&resolved=false&types=CODE_SMELL)
   
   [![81.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '81.8%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12034&metric=new_coverage&view=list) [81.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12034&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12034&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12034&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] limaiwang commented on a diff in pull request #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

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


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -180,4 +180,10 @@
             union select id as project_id  from t_ds_project where user_id=#{userId})
         </if>
     </select>
+
+    <select id="queryAllProjectForDependent" resultType="org.apache.dolphinscheduler.dao.entity.Project">
+        select
+        <include refid="baseSql"/>
+        from t_ds_project

Review Comment:
   fixed dependent node only gets item code and name, confirmed ui only gets code and name



-- 
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 #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProjectV2Controller.java:
##########
@@ -288,4 +288,23 @@ public ProjectListResponse queryAllProjectList(@ApiIgnore @RequestAttribute(valu
         Result result = projectService.queryAllProjectList(loginUser);
         return new ProjectListResponse(result);
     }
+
+    /**
+     * query all project list for dependent
+     *
+     * @param loginUser login user
+     * @return all project list
+     */
+    @ApiOperation(value = "queryAllProjectListForDependent", notes = "QUERY_ALL_PROJECT_LIST_FOR_DEPENDENT_NOTES")
+    @ApiImplicitParams({
+            @ApiImplicitParam(name = "loginUser", value = "LOGIN_USER", dataTypeClass = Object.class, example = "\"{id:100}\"", required = true)
+    })

Review Comment:
   It's useless. Please remove it.



-- 
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 #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProjectV2Controller.java:
##########
@@ -288,4 +288,23 @@ public ProjectListResponse queryAllProjectList(@ApiIgnore @RequestAttribute(valu
         Result result = projectService.queryAllProjectList(loginUser);
         return new ProjectListResponse(result);
     }
+
+    /**
+     * query all project list for dependent
+     *
+     * @param loginUser login user
+     * @return all project list
+     */
+    @ApiOperation(value = "queryAllProjectListForDependent", notes = "QUERY_ALL_PROJECT_LIST_FOR_DEPENDENT_NOTES")
+    @ApiImplicitParams({
+            @ApiImplicitParam(name = "loginUser", value = "LOGIN_USER", dataTypeClass = Object.class, example = "\"{id:100}\"", required = true)
+    })

Review Comment:
   removed



-- 
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] github-code-scanning[bot] commented on a diff in pull request #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

Posted by GitBox <gi...@apache.org>.
github-code-scanning[bot] commented on code in PR #12034:
URL: https://github.com/apache/dolphinscheduler/pull/12034#discussion_r975401683


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProjectV2Controller.java:
##########
@@ -288,4 +288,20 @@
         Result result = projectService.queryAllProjectList(loginUser);
         return new ProjectListResponse(result);
     }
+
+    /**
+     * query all project list for dependent
+     *
+     * @param loginUser login user
+     * @return all project list
+     */
+    @ApiOperation(value = "queryAllProjectListForDependent", notes = "QUERY_ALL_PROJECT_LIST_FOR_DEPENDENT_NOTES")
+    @GetMapping(value = "/list-dependent")
+    @ResponseStatus(HttpStatus.OK)
+    @ApiException(LOGIN_USER_QUERY_PROJECT_LIST_PAGING_ERROR)
+    @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
+    public ProjectListResponse queryAllProjectListForDependent(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser) {

Review Comment:
   ## Useless parameter
   
   The parameter loginUser is unused.
   
   [Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1494)



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProjectController.java:
##########
@@ -284,4 +284,19 @@
     public Result queryAllProjectList(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser) {
         return projectService.queryAllProjectList(loginUser);
     }
+
+    /**
+     * query all project list for dependent
+     *
+     * @param loginUser login user
+     * @return all project list
+     */
+    @ApiOperation(value = "queryAllProjectListForDependent", notes = "QUERY_ALL_PROJECT_LIST_FOR_DEPENDENT_NOTES")
+    @GetMapping(value = "/list-dependent")
+    @ResponseStatus(HttpStatus.OK)
+    @ApiException(LOGIN_USER_QUERY_PROJECT_LIST_PAGING_ERROR)
+    @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
+    public Result queryAllProjectListForDependent(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser) {

Review Comment:
   ## Useless parameter
   
   The parameter loginUser is unused.
   
   [Show more details](https://github.com/apache/dolphinscheduler/security/code-scanning/1493)



-- 
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 #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

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

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache-dolphinscheduler&pullRequest=12034)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12034&resolved=false&types=BUG) [![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=12034&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12034&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=12034&resolved=false&types=VULNERABILITY) [![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=12034&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12034&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=12034&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=12034&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=12034&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=12034&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=12034&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=12034&resolved=false&types=CODE_SMELL)
   
   [![81.8%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '81.8%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12034&metric=new_coverage&view=list) [81.8% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12034&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12034&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=12034&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] limaiwang commented on pull request #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

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

   Hi @SbloodyS  Help to review whether the Issue requirements are met


-- 
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 #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

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


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProjectV2Controller.java:
##########
@@ -288,4 +288,23 @@ public ProjectListResponse queryAllProjectList(@ApiIgnore @RequestAttribute(valu
         Result result = projectService.queryAllProjectList(loginUser);
         return new ProjectListResponse(result);
     }
+
+    /**
+     * query all project list for dependent
+     *
+     * @param loginUser login user
+     * @return all project list
+     */
+    @ApiOperation(value = "queryAllProjectListForDependent", notes = "QUERY_ALL_PROJECT_LIST_FOR_DEPENDENT_NOTES")
+    @ApiImplicitParams({
+            @ApiImplicitParam(name = "loginUser", value = "LOGIN_USER", dataTypeClass = Object.class, example = "\"{id:100}\"", required = true)
+    })
+    @GetMapping(value = "/list-dependent")
+    @ResponseStatus(HttpStatus.OK)
+    @ApiException(LOGIN_USER_QUERY_PROJECT_LIST_PAGING_ERROR)
+    @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
+    public Result queryAllProjectListForDependent(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser) {

Review Comment:
   Bug 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@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 #12034: [Improvement][Dependent] The project list in dependent node's permissions should not be restricted.

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


##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ProjectMapper.xml:
##########
@@ -180,4 +180,10 @@
             union select id as project_id  from t_ds_project where user_id=#{userId})
         </if>
     </select>
+
+    <select id="queryAllProjectForDependent" resultType="org.apache.dolphinscheduler.dao.entity.Project">
+        select
+        <include refid="baseSql"/>
+        from t_ds_project

Review Comment:
   It is recommended to query only the required fields, such as name and code.



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ProjectV2Controller.java:
##########
@@ -288,4 +288,23 @@ public ProjectListResponse queryAllProjectList(@ApiIgnore @RequestAttribute(valu
         Result result = projectService.queryAllProjectList(loginUser);
         return new ProjectListResponse(result);
     }
+
+    /**
+     * query all project list for dependent
+     *
+     * @param loginUser login user
+     * @return all project list
+     */
+    @ApiOperation(value = "queryAllProjectListForDependent", notes = "QUERY_ALL_PROJECT_LIST_FOR_DEPENDENT_NOTES")
+    @ApiImplicitParams({
+            @ApiImplicitParam(name = "loginUser", value = "LOGIN_USER", dataTypeClass = Object.class, example = "\"{id:100}\"", required = true)
+    })
+    @GetMapping(value = "/list-dependent")
+    @ResponseStatus(HttpStatus.OK)
+    @ApiException(LOGIN_USER_QUERY_PROJECT_LIST_PAGING_ERROR)
+    @AccessLogAnnotation(ignoreRequestArgs = "loginUser")
+    public Result queryAllProjectListForDependent(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser) {

Review Comment:
   ```suggestion
       public ProjectListResponse queryAllProjectListForDependent(@ApiIgnore @RequestAttribute(value = Constants.SESSION_USER) User loginUser) {
   ```



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