You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by "zhoulii (via GitHub)" <gi...@apache.org> on 2023/05/25 09:26:04 UTC
[GitHub] [incubator-streampark] zhoulii commented on a diff in pull request #2739: optimize user locking, resource transfer, and user unlocking
zhoulii commented on code in PR #2739:
URL: https://github.com/apache/incubator-streampark/pull/2739#discussion_r1205188069
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/UserServiceImpl.java:
##########
@@ -263,8 +251,31 @@ public Map<String, Object> generateFrontendUserInfo(User user, Long teamId, JWTT
@Override
@Transactional(rollbackFor = Exception.class)
- public void transferResource(Long userId, Long targetUserId) {
- applicationService.changeOwnership(userId, targetUserId);
- resourceService.changeOwnership(userId, targetUserId);
+ public boolean lockUser(Long userId, Long transferToUserId) {
+ boolean hasResource =
+ applicationService.existsByUserId(userId) || resourceService.existsByUserId(userId);
Review Comment:
There are some other resources binding to a user, like variable, alertconfig...
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/system/service/impl/UserServiceImpl.java:
##########
@@ -263,8 +251,31 @@ public Map<String, Object> generateFrontendUserInfo(User user, Long teamId, JWTT
@Override
@Transactional(rollbackFor = Exception.class)
- public void transferResource(Long userId, Long targetUserId) {
- applicationService.changeOwnership(userId, targetUserId);
- resourceService.changeOwnership(userId, targetUserId);
+ public boolean lockUser(Long userId, Long transferToUserId) {
+ boolean hasResource =
+ applicationService.existsByUserId(userId) || resourceService.existsByUserId(userId);
+ if (hasResource && transferToUserId == null) {
+ return true;
+ }
+ if (transferToUserId != null) {
Review Comment:
`userId` and `transferToUserId` cannot be same, and `transferToUserId` cannot be a locked user.
##########
streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/base/exception/UserLogoutException.java:
##########
@@ -0,0 +1,24 @@
+/*
+ * 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.streampark.console.base.exception;
+
+public class UserLogoutException extends ApiAlertException {
Review Comment:
```suggestion
public class UserLockedException extends ApiAlertException {
```
##########
streampark-console/streampark-console-webapp/src/views/system/user/User.vue:
##########
@@ -82,11 +102,48 @@
showIndexColumn: false,
canResize: false,
actionColumn: {
- width: 150,
+ width: 200,
title: t('component.table.operation'),
dataIndex: 'action',
},
});
+
+ const [transferForm, { resetFields: resetTransferFields, validate: transferValidate }] =
+ useForm({
+ layout: 'vertical',
+ showActionButtonGroup: false,
+ baseColProps: { lg: 22, md: 22 },
+ schemas: [
+ {
+ field: 'userId',
+ label: t('system.user.form.transferResource'),
+ component: 'ApiSelect',
+ componentProps: {
+ api: async () => {
+ let { records } = await fetchMemberList({
+ page: 1,
+ pageSize: 999999,
+ teamId: userStore.getTeamId || '',
+ });
+ return records.filter((user) => user.userName !== userName.value);
Review Comment:
userName.value is the current logined user, not the user to be locked. Besides, we should also exclude users who are already locked.
##########
streampark-console/streampark-console-webapp/src/locales/lang/zh-CN/system/user.ts:
##########
@@ -44,7 +49,7 @@ export default {
edit: '编辑用户',
view: '查看用户',
notice: '提示',
- transferResource: '请将需要被禁用的用户资源转移到新的用户上'
+ transferResource: '请将资源转移到新的用户上',
Review Comment:
```suggestion
transferResource: '请将资源转移到其他用户',
```
##########
streampark-console/streampark-console-webapp/src/locales/lang/en/system/user.ts:
##########
@@ -21,6 +21,11 @@ export default {
modify: 'modify user',
reset: 'reset password',
resetTip: 'reset password, are you sure',
+ lock: 'lock user',
+ lockTip: 'lock user, are you sure',
Review Comment:
```suggestion
lockTip: 'Are you sure to lock this user ?',
```
##########
streampark-console/streampark-console-webapp/src/locales/lang/en/system/user.ts:
##########
@@ -44,7 +49,7 @@ export default {
edit: 'Edit User',
view: 'View User',
notice: 'Notice',
- transferResource: 'Please transfer the resources of the user who needs to be disabled to a new user'
+ transferResource: 'Please transfer the resources to a new user',
Review Comment:
```suggestion
transferResource: 'Please transfer the resources to another user',
```
##########
streampark-console/streampark-console-webapp/src/views/system/user/User.vue:
##########
@@ -113,6 +170,24 @@
confirm: handleReset.bind(null, record),
},
},
+ {
+ icon: 'ant-design:lock-outlined',
+ color: 'error',
+ auth: 'user:delete',
+ ifShow: record.username !== 'admin' && record.status === '1',
Review Comment:
We'd better hide lock button for the logined 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@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org