ruanwenjun commented on code in PR #15933:
URL: 
https://github.com/apache/dolphinscheduler/pull/15933#discussion_r1593439147


##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/UiPluginController.java:
##########
@@ -85,4 +89,22 @@ public Result queryUiPluginDetailById(@Parameter(hidden = 
true) @RequestAttribut
         Map<String, Object> result = 
uiPluginService.queryUiPluginDetailById(pluginId);
         return returnDataList(result);
     }
+
+    /**
+     * obtain project version and address
+     *
+//     * @param loginUser login user
+//     * @param userId token for user
+     * @return product info
+     */
+    @Operation(summary = "queryProductInfo", description = 
"QUERY_PRODUCT_INFO")
+    @PostMapping(value = "/queryProductInfo")
+    @ResponseStatus(HttpStatus.OK)
+    @ApiException(VERSION_INFO_STATE_ERROR)
+    public Result<ProductInfoDto> queryProductInfo(
+            @Parameter(hidden = true) @RequestAttribute(value = 
Constants.SESSION_USER) User loginUser,
+            @RequestParam(value = "userId") Integer userId) {
+        ProductInfoDto result = uiPluginService.queryProductInfo(loginUser, 
userId);
+        return Result.success(result);

Review Comment:
   ```suggestion
       public Result<ProductInfoDto> queryProductInfo(
               @Parameter(hidden = true) @RequestAttribute(value = 
Constants.SESSION_USER) User loginUser) {
           ProductInfoDto result = uiPluginService.queryProductInfo(loginUser);
           return Result.success(result);
   ```



##########
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/UiPluginControllerTest.java:
##########
@@ -91,4 +103,31 @@ public void testQueryUiPluginDetailById() throws Exception {
                 
JSONUtils.parseObject(mvcResult.getResponse().getContentAsString(), 
Result.class);
         
assertThat(actualResponseContent.toString()).isEqualTo(expectResponseContent.toString());
     }
+
+    @Test
+    public void testQueryProductInfo() throws Exception {
+        ProductInfoDto mockResult = new ProductInfoDto();
+        Mockito.when(uiPluginService.queryProductInfo(Mockito.any(), 
Mockito.anyInt())).thenReturn(mockResult);
+
+        MultiValueMap<String, String> paramsMap = new LinkedMultiValueMap<>();
+        paramsMap.add("userId", "1");
+
+        MvcResult mvcResult = 
mockMvc.perform(post("/ui-plugins/queryProductInfo")
+                        .header(SESSION_ID, sessionId)
+                        .params(paramsMap))
+                        .andExpect(status().isOk())
+                        
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
+                        .andReturn();
+
+        Result result = 
JSONUtils.parseObject(mvcResult.getResponse().getContentAsString(), 
Result.class);
+        Assertions.assertEquals(Status.SUCCESS.getCode(), 
result.getCode().intValue());
+        logger.info(mvcResult.getResponse().getContentAsString());

Review Comment:
   Don't use log in UT



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UiPluginServiceImpl.java:
##########
@@ -82,4 +92,24 @@ public Map<String, Object> queryUiPluginDetailById(int id) {
         return result;
     }
 
+    @Override
+    public ProductInfoDto queryProductInfo(User loginUser, int userId) {
+
+        // check if user is existed
+        if (userId <= 0 || !(loginUser.getId() == userId)) {
+            throw new ServiceException(Status.REQUEST_PARAMS_NOT_VALID_ERROR,
+                    "User id: " + userId + " should not less than or equals to 
0.");
+        }
+        // persist to the database
+        DsVersion dsVersion = dsVersionMapper.selectById(1);
+
+        if(StringUtils.isBlank(dsVersion.getVersion())){
+            throw new ServiceException(Status.VERSION_INFO_STATE_ERROR);
+        }
+        ProductInfoDto result = new ProductInfoDto();
+        result.setId(dsVersion.getId());
+        result.setVersion(dsVersion.getVersion());
+        return result;

Review Comment:
   Don't query the version by id, id might changed.



##########
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UiPluginService.java:
##########
@@ -30,4 +32,6 @@ public interface UiPluginService {
 
     Map<String, Object> queryUiPluginDetailById(int id);
 
+    ProductInfoDto queryProductInfo(User loginUser, int userId);
+

Review Comment:
   It's weird that there exists a method `queryProductInfo` in uiPluginServer.



##########
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/UiPluginControllerTest.java:
##########
@@ -91,4 +103,31 @@ public void testQueryUiPluginDetailById() throws Exception {
                 
JSONUtils.parseObject(mvcResult.getResponse().getContentAsString(), 
Result.class);
         
assertThat(actualResponseContent.toString()).isEqualTo(expectResponseContent.toString());
     }
+
+    @Test
+    public void testQueryProductInfo() throws Exception {
+        ProductInfoDto mockResult = new ProductInfoDto();
+        Mockito.when(uiPluginService.queryProductInfo(Mockito.any(), 
Mockito.anyInt())).thenReturn(mockResult);
+
+        MultiValueMap<String, String> paramsMap = new LinkedMultiValueMap<>();
+        paramsMap.add("userId", "1");
+
+        MvcResult mvcResult = 
mockMvc.perform(post("/ui-plugins/queryProductInfo")
+                        .header(SESSION_ID, sessionId)
+                        .params(paramsMap))
+                        .andExpect(status().isOk())
+                        
.andExpect(content().contentType(MediaType.APPLICATION_JSON))
+                        .andReturn();
+
+        Result result = 
JSONUtils.parseObject(mvcResult.getResponse().getContentAsString(), 
Result.class);
+        Assertions.assertEquals(Status.SUCCESS.getCode(), 
result.getCode().intValue());
+        logger.info(mvcResult.getResponse().getContentAsString());
+    }
+
+    private User getLoginUser() {
+        User user = new User();
+        user.setId(1);
+        user.setUserName("admin");
+        return user;
+    }

Review Comment:
   Don't add useless code



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to