You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "s7monk (via GitHub)" <gi...@apache.org> on 2024/01/17 10:49:58 UTC
[PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
s7monk opened a new pull request, #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148
<!-- Please specify the module before the PR name: [core] ... or [flink] ... -->
### Purpose
close: https://github.com/apache/incubator-paimon-webui/issues/79
This PR improves the user management interface and fixes some problems.
### Tests
UserControllerTest
### API and Format
<!-- Does this change affect API or storage format -->
### Documentation
<!-- Does this change introduce a new feature -->
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "s7monk (via GitHub)" <gi...@apache.org>.
s7monk commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1478002945
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -41,19 +53,76 @@ public class UserController {
@Autowired private UserService userService;
/**
- * get user by id.
+ * Get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUser(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Get user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> listUsers(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.listUsers(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Add a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Update an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:update")
+ @PutMapping
+ public R<Void> update(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.updateUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Delete one or more users by user ID.
+ *
+ * @param userIds an array of user IDs to be deleted
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:delete")
+ @DeleteMapping("/{userIds}")
+ public R<Void> delete(@PathVariable Integer[] userIds) {
+ return userService.deleteUserByIds(userIds) > 0 ? R.succeed() : R.failed();
Review Comment:
> What's the behavior that the same user is multiple deleted?
I think there should be no situation where a User is deleted multiple times, because this is transmitted from the front end. If a User is deleted, it will not be displayed in the User list, so the front end cannot pass the deleted UserId to the back end. Endless.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1482622522
##########
paimon-web-server/src/test/java/org/apache/paimon/web/server/controller/UserControllerTest.java:
##########
@@ -0,0 +1,176 @@
+/*
+ * 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.
+ */
+
+package org.apache.paimon.web.server.controller;
+
+import org.apache.paimon.web.server.data.model.User;
+import org.apache.paimon.web.server.data.result.PageR;
+import org.apache.paimon.web.server.data.result.R;
+import org.apache.paimon.web.server.data.vo.UserVO;
+import org.apache.paimon.web.server.util.ObjectMapperUtils;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.http.MediaType;
+import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
+import org.springframework.test.web.servlet.result.MockMvcResultHandlers;
+import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/** Test for {@link UserController}. */
+@SpringBootTest
+@AutoConfigureMockMvc
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
+public class UserControllerTest extends ControllerTestBase {
+
+ private static final String userPath = "/api/user";
+
+ private static final int userId = 3;
+ private static final String username = "test";
+
+ @Test
+ @Order(1)
+ public void testAddUser() throws Exception {
+ User user = new User();
+ user.setId(userId);
+ user.setUsername(username);
+ user.setNickname(username);
+ user.setUserType(0);
+ user.setEnabled(true);
+ user.setIsDelete(false);
+
+ mockMvc.perform(
+ MockMvcRequestBuilders.post(userPath)
+ .cookie(cookie)
+ .content(ObjectMapperUtils.toJSON(user))
+ .contentType(MediaType.APPLICATION_JSON_VALUE)
+ .accept(MediaType.APPLICATION_JSON_VALUE))
+ .andExpect(MockMvcResultMatchers.status().isOk())
+ .andDo(MockMvcResultHandlers.print());
+ }
+
+ @Test
+ @Order(2)
+ public void testGetUser() throws Exception {
+ String responseString =
+ mockMvc.perform(
+ MockMvcRequestBuilders.get(userPath + "/" + userId)
+ .cookie(cookie)
+ .contentType(MediaType.APPLICATION_JSON_VALUE)
+ .accept(MediaType.APPLICATION_JSON_VALUE))
+ .andExpect(MockMvcResultMatchers.status().isOk())
+ .andDo(MockMvcResultHandlers.print())
+ .andReturn()
+ .getResponse()
+ .getContentAsString();
+
+ R<UserVO> r = ObjectMapperUtils.fromJSON(responseString, new TypeReference<R<UserVO>>() {});
+ assertEquals(200, r.getCode());
+ assertNotNull(r.getData());
+ assertEquals(r.getData().getUsername(), username);
+ }
+
+ @Test
+ @Order(3)
+ public void testUpdateUser() throws Exception {
+ String newUserName = username + "-edit";
+ User user = new User();
+ user.setId(userId);
+ user.setUsername(newUserName);
+ user.setNickname(newUserName);
+ user.setUserType(0);
+ user.setEnabled(true);
+ user.setIsDelete(false);
+
+ mockMvc.perform(
+ MockMvcRequestBuilders.put(userPath)
+ .cookie(cookie)
+ .content(ObjectMapperUtils.toJSON(user))
+ .contentType(MediaType.APPLICATION_JSON_VALUE)
+ .accept(MediaType.APPLICATION_JSON_VALUE))
+ .andExpect(MockMvcResultMatchers.status().isOk());
+
+ String responseString =
+ mockMvc.perform(
+ MockMvcRequestBuilders.get(userPath + "/" + userId)
+ .cookie(cookie)
+ .contentType(MediaType.APPLICATION_JSON_VALUE)
+ .accept(MediaType.APPLICATION_JSON_VALUE))
+ .andExpect(MockMvcResultMatchers.status().isOk())
+ .andDo(MockMvcResultHandlers.print())
+ .andReturn()
+ .getResponse()
+ .getContentAsString();
+
+ R<UserVO> r = ObjectMapperUtils.fromJSON(responseString, new TypeReference<R<UserVO>>() {});
+ assertEquals(200, r.getCode());
+ assertNotNull(r.getData());
+ assertEquals(r.getData().getUsername(), newUserName);
+ }
+
+ @Test
+ @Order(4)
+ public void testListUsers() throws Exception {
+ String responseString =
+ mockMvc.perform(
+ MockMvcRequestBuilders.get(userPath + "/list")
+ .cookie(cookie)
+ .contentType(MediaType.APPLICATION_JSON_VALUE)
+ .accept(MediaType.APPLICATION_JSON_VALUE))
+ .andExpect(MockMvcResultMatchers.status().isOk())
+ .andDo(MockMvcResultHandlers.print())
+ .andReturn()
+ .getResponse()
+ .getContentAsString();
+
+ PageR<?> r = ObjectMapperUtils.fromJSON(responseString, PageR.class);
+ assertNotNull(r);
+ assertTrue(
Review Comment:
Asset the actual data content.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "s7monk (via GitHub)" <gi...@apache.org>.
s7monk commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1460761209
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/service/impl/UserServiceImpl.java:
##########
@@ -61,6 +70,35 @@ public class UserServiceImpl extends ServiceImpl<UserMapper, User> implements Us
@Autowired private RoleMenuService roleMenuService;
@Autowired private SysMenuService sysMenuService;
@Autowired private TenantService tenantService;
+ @Autowired private UserRoleMapper userRoleMapper;
+
+ @Override
+ public UserVO getUserById(Integer id) {
+ UserWithRolesDTO userWithRolesDTO = userMapper.selectUserWithRolesById(id);
+ if (Objects.nonNull(userWithRolesDTO)) {
+ return toVo(userWithRolesDTO);
+ }
+ return null;
+ }
+
+ @Override
+ public List<UserVO> selectUserList(IPage<User> page, User user) {
+ return userMapper.selectUserList(page, user).stream()
+ .map(this::toVo)
+ .collect(Collectors.toList());
+ }
+
+ @Override
+ public List<UserVO> listUsers() {
+ return this.list().stream().map(this::toVo).collect(Collectors.toList());
+ }
+
+ @Override
+ public boolean checkUserNameUnique(User user) {
+ int roleId = user.getId() == null ? -1 : user.getId();
Review Comment:
When user.getId is null, assign a value that does not exist in the user table. Here, -1 is assigned.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1469047355
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -41,19 +53,88 @@ public class UserController {
@Autowired private UserService userService;
/**
- * get user by id.
+ * Get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUser(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Get user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> listUsers(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.listUsers(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Add a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Update an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:update")
+ @PutMapping
+ public R<Void> update(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.updateUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Delete one or more users by user ID.
+ *
+ * @param userIds an array of user IDs to be deleted
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:delete")
+ @DeleteMapping("/{userIds}")
+ public R<Void> delete(@PathVariable Integer[] userIds) {
+ return userService.deleteUserByIds(userIds) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Allocate a role to a user.
+ *
+ * @param user the user to whom the role is to be allocated
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:allocate")
+ @PostMapping("/allocate")
+ public R<Void> allocateRole(@RequestBody User user) {
+ return userService.allocateRole(user) > 0 ? R.succeed() : R.failed();
Review Comment:
Why does this interface add in `UserController` not `SysRoleController`?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "s7monk (via GitHub)" <gi...@apache.org>.
s7monk commented on PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#issuecomment-1902559395
Thanks for your review, I have updated this PR. @SteNicholas PTAL.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "s7monk (via GitHub)" <gi...@apache.org>.
s7monk commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1460742527
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -44,16 +56,108 @@ public class UserController {
* get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUserById(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Gets user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> selectUserList(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.selectUserList(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Obtain a list of user selection boxes.
+ *
+ * @return {@link R} with a list of {@link UserVO} objects
+ */
+ @SaCheckPermission("system:user:query")
+ @GetMapping("/all")
+ public R<List<UserVO>> all() {
+ return R.succeed(userService.listUsers());
+ }
+
+ /**
+ * Adds a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:role:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Edits an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:edit")
+ @PutMapping
+ public R<Void> edit(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.updateUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Changes the status of a user.
+ *
+ * @param user the user whose status is to be updated, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:edit")
+ @PutMapping("/changeStatus")
+ public R<Void> changeStatus(@Validated @RequestBody User user) {
Review Comment:
update may modify the role assigned to the user, involving simultaneous modification of the user_role table. changeStatus only modifies the user status in the user table.
Similar to the following：
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [paimon-webui]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas merged PR #148:
URL: https://github.com/apache/paimon-webui/pull/148
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "s7monk (via GitHub)" <gi...@apache.org>.
s7monk commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1460742527
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -44,16 +56,108 @@ public class UserController {
* get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUserById(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Gets user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> selectUserList(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.selectUserList(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Obtain a list of user selection boxes.
+ *
+ * @return {@link R} with a list of {@link UserVO} objects
+ */
+ @SaCheckPermission("system:user:query")
+ @GetMapping("/all")
+ public R<List<UserVO>> all() {
+ return R.succeed(userService.listUsers());
+ }
+
+ /**
+ * Adds a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:role:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Edits an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:edit")
+ @PutMapping
+ public R<Void> edit(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.updateUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Changes the status of a user.
+ *
+ * @param user the user whose status is to be updated, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:edit")
+ @PutMapping("/changeStatus")
+ public R<Void> changeStatus(@Validated @RequestBody User user) {
Review Comment:
update may modify the role assigned to the user, involving simultaneous modification of the user_role table. changeStatus only modifies the user status in the user table.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "s7monk (via GitHub)" <gi...@apache.org>.
s7monk commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1477999017
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -41,19 +53,76 @@ public class UserController {
@Autowired private UserService userService;
/**
- * get user by id.
+ * Get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUser(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Get user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> listUsers(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.listUsers(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Add a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Update an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:update")
+ @PutMapping
+ public R<Void> update(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.updateUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Delete one or more users by user ID.
+ *
+ * @param userIds an array of user IDs to be deleted
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:delete")
+ @DeleteMapping("/{userIds}")
+ public R<Void> delete(@PathVariable Integer[] userIds) {
+ return userService.deleteUserByIds(userIds) > 0 ? R.succeed() : R.failed();
Review Comment:
It has been modified. This should not be roleId.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "s7monk (via GitHub)" <gi...@apache.org>.
s7monk commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1460740602
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -44,16 +56,108 @@ public class UserController {
* get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUserById(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Gets user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> selectUserList(User user) {
Review Comment:
all obtains all user data, including is_delete = true, selectUserList does not include, selectUserList is obtained in paging,I will merge it into one interface.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1480944914
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -41,19 +53,76 @@ public class UserController {
@Autowired private UserService userService;
/**
- * get user by id.
+ * Get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUser(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Get user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> listUsers(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.listUsers(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Add a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Update an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:update")
+ @PutMapping
+ public R<Void> update(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.updateUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Delete one or more users by user ID.
+ *
+ * @param userIds an array of user IDs to be deleted
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:delete")
+ @DeleteMapping("/{userIds}")
+ public R<Void> delete(@PathVariable Integer[] userIds) {
+ return userService.deleteUserByIds(userIds) > 0 ? R.succeed() : R.failed();
Review Comment:
@s7monk, when the same user is multiple deleted, we should return succeed instead of failed. The backend interface has nothing with frontend.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "s7monk (via GitHub)" <gi...@apache.org>.
s7monk commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1481277587
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -41,19 +53,76 @@ public class UserController {
@Autowired private UserService userService;
/**
- * get user by id.
+ * Get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUser(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Get user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> listUsers(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.listUsers(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Add a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Update an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:update")
+ @PutMapping
+ public R<Void> update(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.updateUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Delete one or more users by user ID.
+ *
+ * @param userIds an array of user IDs to be deleted
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:delete")
+ @DeleteMapping("/{userIds}")
+ public R<Void> delete(@PathVariable Integer[] userIds) {
+ return userService.deleteUserByIds(userIds) > 0 ? R.succeed() : R.failed();
Review Comment:
done.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [paimon-webui]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #148:
URL: https://github.com/apache/paimon-webui/pull/148#discussion_r1549220810
##########
paimon-web-server/src/test/java/org/apache/paimon/web/server/controller/PermissionTest.java:
##########
@@ -54,7 +54,7 @@ public void testNoPermission() throws Exception {
.getResponse()
.getContentAsString();
- R<User> r = ObjectMapperUtils.fromJSON(responseString, new TypeReference<R<User>>() {});
+ R<UserVO> r = ObjectMapperUtils.fromJSON(responseString, new TypeReference<R<UserVO>>() {});
assertEquals(Status.SUCCESS.getCode(), r.getCode());
Review Comment:
Asset the actual data content.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1460712765
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -44,16 +56,108 @@ public class UserController {
* get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUserById(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Gets user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> selectUserList(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.selectUserList(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Obtain a list of user selection boxes.
+ *
+ * @return {@link R} with a list of {@link UserVO} objects
+ */
+ @SaCheckPermission("system:user:query")
+ @GetMapping("/all")
+ public R<List<UserVO>> all() {
+ return R.succeed(userService.listUsers());
+ }
+
+ /**
+ * Adds a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:role:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Edits an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:edit")
Review Comment:
```suggestion
@SaCheckPermission("system:user:update")
```
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -44,16 +56,108 @@ public class UserController {
* get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUserById(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Gets user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> selectUserList(User user) {
Review Comment:
What's the difference between `selectUserList` and `all`? Why not merge to one interface?
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/service/UserService.java:
##########
@@ -22,14 +22,48 @@
import org.apache.paimon.web.server.data.model.User;
import org.apache.paimon.web.server.data.result.exception.BaseException;
import org.apache.paimon.web.server.data.vo.UserInfoVO;
+import org.apache.paimon.web.server.data.vo.UserVO;
+import com.baomidou.mybatisplus.core.metadata.IPage;
import com.baomidou.mybatisplus.extension.service.IService;
import java.util.List;
/** User Service. */
public interface UserService extends IService<User> {
+ /**
+ * Gets a user by ID.
+ *
+ * @param id the user ID
+ * @return the UserVO or null if not found
+ */
+ UserVO getUserById(Integer id);
+
+ /**
+ * Selects users with pagination.
+ *
+ * @param page the pagination information
+ * @param user the filter criteria
+ * @return list of UserVO
+ */
+ List<UserVO> selectUserList(IPage<User> page, User user);
Review Comment:
Combine `selectUserList` and `listUsers` to `listUsers`.
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -44,16 +56,108 @@ public class UserController {
* get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUserById(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Gets user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> selectUserList(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.selectUserList(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Obtain a list of user selection boxes.
+ *
+ * @return {@link R} with a list of {@link UserVO} objects
+ */
+ @SaCheckPermission("system:user:query")
+ @GetMapping("/all")
+ public R<List<UserVO>> all() {
+ return R.succeed(userService.listUsers());
+ }
+
+ /**
+ * Adds a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:role:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Edits an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:edit")
+ @PutMapping
+ public R<Void> edit(@Validated @RequestBody User user) {
Review Comment:
```suggestion
public R<Void> update(@Validated @RequestBody User user) {
```
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/service/impl/UserServiceImpl.java:
##########
@@ -61,6 +70,35 @@ public class UserServiceImpl extends ServiceImpl<UserMapper, User> implements Us
@Autowired private RoleMenuService roleMenuService;
@Autowired private SysMenuService sysMenuService;
@Autowired private TenantService tenantService;
+ @Autowired private UserRoleMapper userRoleMapper;
+
+ @Override
+ public UserVO getUserById(Integer id) {
+ UserWithRolesDTO userWithRolesDTO = userMapper.selectUserWithRolesById(id);
+ if (Objects.nonNull(userWithRolesDTO)) {
+ return toVo(userWithRolesDTO);
+ }
+ return null;
+ }
+
+ @Override
+ public List<UserVO> selectUserList(IPage<User> page, User user) {
+ return userMapper.selectUserList(page, user).stream()
+ .map(this::toVo)
+ .collect(Collectors.toList());
+ }
+
+ @Override
+ public List<UserVO> listUsers() {
+ return this.list().stream().map(this::toVo).collect(Collectors.toList());
+ }
+
+ @Override
+ public boolean checkUserNameUnique(User user) {
+ int roleId = user.getId() == null ? -1 : user.getId();
Review Comment:
Why is `roleId` is -1 when `user.getId` is null?
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -44,16 +56,108 @@ public class UserController {
* get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUserById(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Gets user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> selectUserList(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.selectUserList(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Obtain a list of user selection boxes.
+ *
+ * @return {@link R} with a list of {@link UserVO} objects
+ */
+ @SaCheckPermission("system:user:query")
+ @GetMapping("/all")
+ public R<List<UserVO>> all() {
+ return R.succeed(userService.listUsers());
+ }
+
+ /**
+ * Adds a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:role:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Edits an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:edit")
+ @PutMapping
+ public R<Void> edit(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.updateUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Changes the status of a user.
+ *
+ * @param user the user whose status is to be updated, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:edit")
+ @PutMapping("/changeStatus")
+ public R<Void> changeStatus(@Validated @RequestBody User user) {
Review Comment:
What's the difference between `update` and `changeStatus`?
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "s7monk (via GitHub)" <gi...@apache.org>.
s7monk commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1481286402
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -41,19 +53,76 @@ public class UserController {
@Autowired private UserService userService;
/**
- * get user by id.
+ * Get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUser(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Get user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> listUsers(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.listUsers(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Add a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Update an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:update")
+ @PutMapping
+ public R<Void> update(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.updateUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Delete one or more users by user ID.
+ *
+ * @param userIds an array of user IDs to be deleted
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:delete")
+ @DeleteMapping("/{userIds}")
+ public R<Void> delete(@PathVariable Integer[] userIds) {
+ return userService.deleteUserByIds(userIds) > 0 ? R.succeed() : R.failed();
Review Comment:
> What's the behavior that the same user is multiple deleted?
In my test, I tested passing in the same userId, and the result was 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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1463109862
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -44,16 +56,97 @@ public class UserController {
* get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUserById(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Gets user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> selectUserList(User user) {
Review Comment:
```suggestion
public PageR<UserVO> listUsers(User 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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [paimon-webui]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #148:
URL: https://github.com/apache/paimon-webui/pull/148#discussion_r1549226373
##########
paimon-web-server/src/test/java/org/apache/paimon/web/server/controller/UserControllerTest.java:
##########
@@ -0,0 +1,178 @@
+/*
+ * 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.
+ */
+
+package org.apache.paimon.web.server.controller;
+
+import org.apache.paimon.web.server.data.model.User;
+import org.apache.paimon.web.server.data.result.PageR;
+import org.apache.paimon.web.server.data.result.R;
+import org.apache.paimon.web.server.data.vo.UserVO;
+import org.apache.paimon.web.server.util.ObjectMapperUtils;
+
+import com.fasterxml.jackson.core.type.TypeReference;
+import org.junit.jupiter.api.MethodOrderer;
+import org.junit.jupiter.api.Order;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestMethodOrder;
+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.http.MediaType;
+import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
+import org.springframework.test.web.servlet.result.MockMvcResultHandlers;
+import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/** Test for {@link UserController}. */
+@SpringBootTest
+@AutoConfigureMockMvc
+@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
+public class UserControllerTest extends ControllerTestBase {
+
+ private static final String userPath = "/api/user";
+
+ private static final int userId = 3;
+ private static final String username = "test";
+
+ @Test
+ @Order(1)
+ public void testAddUser() throws Exception {
+ User user = new User();
+ user.setId(userId);
+ user.setUsername(username);
+ user.setNickname(username);
+ user.setUserType(0);
+ user.setEnabled(true);
+ user.setIsDelete(false);
+
+ mockMvc.perform(
+ MockMvcRequestBuilders.post(userPath)
+ .cookie(cookie)
+ .content(ObjectMapperUtils.toJSON(user))
+ .contentType(MediaType.APPLICATION_JSON_VALUE)
+ .accept(MediaType.APPLICATION_JSON_VALUE))
+ .andExpect(MockMvcResultMatchers.status().isOk())
+ .andDo(MockMvcResultHandlers.print());
+ }
+
+ @Test
+ @Order(2)
+ public void testGetUser() throws Exception {
+ String responseString =
+ mockMvc.perform(
+ MockMvcRequestBuilders.get(userPath + "/" + userId)
+ .cookie(cookie)
+ .contentType(MediaType.APPLICATION_JSON_VALUE)
+ .accept(MediaType.APPLICATION_JSON_VALUE))
+ .andExpect(MockMvcResultMatchers.status().isOk())
+ .andDo(MockMvcResultHandlers.print())
+ .andReturn()
+ .getResponse()
+ .getContentAsString();
+
+ R<UserVO> r = ObjectMapperUtils.fromJSON(responseString, new TypeReference<R<UserVO>>() {});
+ assertEquals(200, r.getCode());
+ assertNotNull(r.getData());
+ assertEquals(r.getData().getUsername(), username);
+ }
+
+ @Test
+ @Order(3)
+ public void testUpdateUser() throws Exception {
+ String newUserName = username + "-edit";
+ User user = new User();
+ user.setId(userId);
+ user.setUsername(newUserName);
+ user.setNickname(newUserName);
+ user.setUserType(0);
+ user.setEnabled(true);
+ user.setIsDelete(false);
+
+ mockMvc.perform(
+ MockMvcRequestBuilders.put(userPath)
+ .cookie(cookie)
+ .content(ObjectMapperUtils.toJSON(user))
+ .contentType(MediaType.APPLICATION_JSON_VALUE)
+ .accept(MediaType.APPLICATION_JSON_VALUE))
+ .andExpect(MockMvcResultMatchers.status().isOk());
+
+ String responseString =
+ mockMvc.perform(
+ MockMvcRequestBuilders.get(userPath + "/" + userId)
+ .cookie(cookie)
+ .contentType(MediaType.APPLICATION_JSON_VALUE)
+ .accept(MediaType.APPLICATION_JSON_VALUE))
+ .andExpect(MockMvcResultMatchers.status().isOk())
+ .andDo(MockMvcResultHandlers.print())
+ .andReturn()
+ .getResponse()
+ .getContentAsString();
+
+ R<UserVO> r = ObjectMapperUtils.fromJSON(responseString, new TypeReference<R<UserVO>>() {});
+ assertEquals(200, r.getCode());
+ assertNotNull(r.getData());
+ assertEquals(r.getData().getUsername(), newUserName);
+ }
+
+ @Test
+ @Order(4)
+ public void testListUsers() throws Exception {
+ String responseString =
+ mockMvc.perform(
+ MockMvcRequestBuilders.get(userPath + "/list")
+ .cookie(cookie)
+ .contentType(MediaType.APPLICATION_JSON_VALUE)
+ .accept(MediaType.APPLICATION_JSON_VALUE))
+ .andExpect(MockMvcResultMatchers.status().isOk())
+ .andDo(MockMvcResultHandlers.print())
+ .andReturn()
+ .getResponse()
+ .getContentAsString();
+
+ PageR<UserVO> r =
+ ObjectMapperUtils.fromJSON(responseString, new TypeReference<PageR<UserVO>>() {});
+ assertTrue(
+ r.getData() != null
+ && ((r.getTotal() > 0 && r.getData().size() > 0)
+ || (r.getTotal() == 0 && r.getData().size() == 0)));
+ assertEquals(r.getData().get(0).getUsername(), "admin");
+ assertEquals(r.getData().get(1).getUsername(), "common");
Review Comment:
Please assert all field of user like `nickName`, `userType`.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "s7monk (via GitHub)" <gi...@apache.org>.
s7monk commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1460751036
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/service/UserService.java:
##########
@@ -22,14 +22,48 @@
import org.apache.paimon.web.server.data.model.User;
import org.apache.paimon.web.server.data.result.exception.BaseException;
import org.apache.paimon.web.server.data.vo.UserInfoVO;
+import org.apache.paimon.web.server.data.vo.UserVO;
+import com.baomidou.mybatisplus.core.metadata.IPage;
import com.baomidou.mybatisplus.extension.service.IService;
import java.util.List;
/** User Service. */
public interface UserService extends IService<User> {
+ /**
+ * Gets a user by ID.
+ *
+ * @param id the user ID
+ * @return the UserVO or null if not found
+ */
+ UserVO getUserById(Integer id);
+
+ /**
+ * Selects users with pagination.
+ *
+ * @param page the pagination information
+ * @param user the filter criteria
+ * @return list of UserVO
+ */
+ List<UserVO> selectUserList(IPage<User> page, User user);
Review Comment:
ok.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1463110641
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -44,16 +56,97 @@ public class UserController {
* get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUserById(@PathVariable("id") Integer id) {
Review Comment:
```suggestion
public R<UserVO> getUser(@PathVariable("user") User 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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Improvement] Improves the user management interface [incubator-paimon-webui]
Posted by "SteNicholas (via GitHub)" <gi...@apache.org>.
SteNicholas commented on code in PR #148:
URL: https://github.com/apache/incubator-paimon-webui/pull/148#discussion_r1477168666
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/controller/UserController.java:
##########
@@ -41,19 +53,76 @@ public class UserController {
@Autowired private UserService userService;
/**
- * get user by id.
+ * Get user by id.
*
* @param id user-id
- * @return {@link R} with {@link User}
+ * @return {@link R} with {@link UserVO}
*/
@SaCheckPermission("system:user:query")
@GetMapping("/{id}")
- public R<User> getUserById(@PathVariable("id") Long id) {
- User user = userService.getById(id);
+ public R<UserVO> getUser(@PathVariable("id") Integer id) {
+ UserVO user = userService.getUserById(id);
if (user == null) {
return R.failed(USER_NOT_EXIST);
}
- user.setPassword(null);
return R.succeed(user);
}
+
+ /**
+ * Get user views with pagination.
+ *
+ * @param user filter conditions
+ * @return paginated user view objects
+ */
+ @SaCheckPermission("system:user:list")
+ @GetMapping("/list")
+ public PageR<UserVO> listUsers(User user) {
+ IPage<User> page = PageSupport.startPage();
+ List<UserVO> list = userService.listUsers(page, user);
+ return PageR.<UserVO>builder().success(true).total(page.getTotal()).data(list).build();
+ }
+
+ /**
+ * Add a new user.
+ *
+ * @param user the user to be added, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:add")
+ @PostMapping
+ public R<Void> add(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.insertUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Update an existing user's details.
+ *
+ * @param user the user with updated details, must not be null
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:update")
+ @PutMapping
+ public R<Void> update(@Validated @RequestBody User user) {
+ if (!userService.checkUserNameUnique(user)) {
+ return R.failed(Status.USER_NAME_ALREADY_EXISTS, user.getUsername());
+ }
+
+ return userService.updateUser(user) > 0 ? R.succeed() : R.failed();
+ }
+
+ /**
+ * Delete one or more users by user ID.
+ *
+ * @param userIds an array of user IDs to be deleted
+ * @return a {@code R<Void>} response indicating success or failure
+ */
+ @SaCheckPermission("system:user:delete")
+ @DeleteMapping("/{userIds}")
+ public R<Void> delete(@PathVariable Integer[] userIds) {
+ return userService.deleteUserByIds(userIds) > 0 ? R.succeed() : R.failed();
Review Comment:
What's the behavior that the same user is multiple deleted?
##########
paimon-web-server/src/main/java/org/apache/paimon/web/server/service/impl/UserServiceImpl.java:
##########
@@ -61,6 +70,35 @@ public class UserServiceImpl extends ServiceImpl<UserMapper, User> implements Us
@Autowired private RoleMenuService roleMenuService;
@Autowired private SysMenuService sysMenuService;
@Autowired private TenantService tenantService;
+ @Autowired private UserRoleMapper userRoleMapper;
+
+ @Override
+ public UserVO getUserById(Integer id) {
+ UserWithRolesDTO userWithRolesDTO = userMapper.selectUserWithRolesById(id);
+ if (Objects.nonNull(userWithRolesDTO)) {
+ return toVo(userWithRolesDTO);
+ }
+ return null;
+ }
+
+ @Override
+ public List<UserVO> selectUserList(IPage<User> page, User user) {
+ return userMapper.selectUserList(page, user).stream()
+ .map(this::toVo)
+ .collect(Collectors.toList());
+ }
+
+ @Override
+ public List<UserVO> listUsers() {
+ return this.list().stream().map(this::toVo).collect(Collectors.toList());
+ }
+
+ @Override
+ public boolean checkUserNameUnique(User user) {
+ int roleId = user.getId() == null ? -1 : user.getId();
Review Comment:
@s7monk, I don't think the role id is -1 when the id of user is null. The default role id could be set via role service.
--
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: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org