You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by "VampireAchao (via GitHub)" <gi...@apache.org> on 2023/05/10 15:46:27 UTC

[GitHub] [incubator-streampark] VampireAchao opened a new pull request, #2739: optimize user locking, resource transfer, and user unlocking

VampireAchao opened a new pull request, #2739:
URL: https://github.com/apache/incubator-streampark/pull/2739

   1. Keep resource transfer and user lock as one transaction.
   2. Optimize the front-end interaction for user locking/unlocking.
   3. Any operation is disabled instantly after the user is locked.
   4. The prompt message in the transfer dialog box is changed to be shorter.
   5. Users cannot lock themselves.


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

[GitHub] [incubator-streampark] zhoulii commented on pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "zhoulii (via GitHub)" <gi...@apache.org>.
zhoulii commented on PR #2739:
URL: https://github.com/apache/incubator-streampark/pull/2739#issuecomment-1543398392

   Hi @VampireAchao,  Thanks for your contribution, I build and deploy sp with this commit, and the fe cannot be visited. 
   
   ![Snipaste_2023-05-11_14-00-40](https://github.com/apache/incubator-streampark/assets/23091870/38ba15df-8cad-4627-aece-1887a1ea7feb)
   
   This may be related to [UserStatusCheckInterceptor.java#L40](https://github.com/apache/incubator-streampark/blob/0f9b849a574b27676ad55d8ead96692a281b1d85/streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/base/interceptor/UserStatusCheckInterceptor.java#L40). I will continue the review when this issue is 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: issues-unsubscribe@streampark.apache.org

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

Re: [PR] [WIP] [Improve] optimize user locking, resource transfer, and user unlocking [incubator-streampark]

Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys closed pull request #2739: [WIP] [Improve] optimize user locking, resource transfer, and user unlocking
URL: https://github.com/apache/incubator-streampark/pull/2739


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

[GitHub] [incubator-streampark] zhoulii commented on a diff in pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "zhoulii (via GitHub)" <gi...@apache.org>.
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

[GitHub] [incubator-streampark] VampireAchao closed pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "VampireAchao (via GitHub)" <gi...@apache.org>.
VampireAchao closed pull request #2739: optimize user locking, resource transfer, and user unlocking
URL: https://github.com/apache/incubator-streampark/pull/2739


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

Re: [PR] [WIP] [Improve] optimize user locking, resource transfer, and user unlocking [incubator-streampark]

Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on PR #2739:
URL: https://github.com/apache/incubator-streampark/pull/2739#issuecomment-1836235466

   There are numerous conflicting files. I suggest initiating a new discussion and resubmitting the PR


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

[GitHub] [incubator-streampark] VampireAchao commented on pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "VampireAchao (via GitHub)" <gi...@apache.org>.
VampireAchao commented on PR #2739:
URL: https://github.com/apache/incubator-streampark/pull/2739#issuecomment-1565288766

   Thanks for the review. I'll make the code changes later.


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

[GitHub] [incubator-streampark] wolfboys commented on pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on PR #2739:
URL: https://github.com/apache/incubator-streampark/pull/2739#issuecomment-1543355562

   Thanks for your contribution, I will review it later


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

[GitHub] [incubator-streampark] VampireAchao closed pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "VampireAchao (via GitHub)" <gi...@apache.org>.
VampireAchao closed pull request #2739: optimize user locking, resource transfer, and user unlocking
URL: https://github.com/apache/incubator-streampark/pull/2739


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

[GitHub] [incubator-streampark] VampireAchao commented on pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "VampireAchao (via GitHub)" <gi...@apache.org>.
VampireAchao commented on PR #2739:
URL: https://github.com/apache/incubator-streampark/pull/2739#issuecomment-1549507003

   > Hi @VampireAchao, Thanks for your contribution, the create user function is not available in this pr, can you fix it ? I will review later.
   > 
   > ![image](https://user-images.githubusercontent.com/23091870/238585445-4a261978-a0ae-4022-9ccf-c95aaa6592aa.png)
   
   Hang on


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

[GitHub] [incubator-streampark] VampireAchao closed pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "VampireAchao (via GitHub)" <gi...@apache.org>.
VampireAchao closed pull request #2739: optimize user locking, resource transfer, and user unlocking
URL: https://github.com/apache/incubator-streampark/pull/2739


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

[GitHub] [incubator-streampark] VampireAchao commented on pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "VampireAchao (via GitHub)" <gi...@apache.org>.
VampireAchao commented on PR #2739:
URL: https://github.com/apache/incubator-streampark/pull/2739#issuecomment-1544210729

   > Hi @VampireAchao, Thanks for your contribution, I build and deploy sp with this commit, and the fe cannot be visited.
   > 
   > ![Snipaste_2023-05-11_14-00-40](https://user-images.githubusercontent.com/23091870/237589269-38ba15df-8cad-4627-aece-1887a1ea7feb.png)
   > 
   > This may be related to [UserStatusCheckInterceptor.java#L40](https://github.com/apache/incubator-streampark/blob/0f9b849a574b27676ad55d8ead96692a281b1d85/streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/base/interceptor/UserStatusCheckInterceptor.java#L40). I will continue the review when this issue is fixed.🙂
   
   Thx for ur review, I'll fix ASAP.


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

[GitHub] [incubator-streampark] wolfboys commented on pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "wolfboys (via GitHub)" <gi...@apache.org>.
wolfboys commented on PR #2739:
URL: https://github.com/apache/incubator-streampark/pull/2739#issuecomment-1545931864

   Thanks for your hard work, I will review later


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

[GitHub] [incubator-streampark] zhoulii commented on pull request #2739: optimize user locking, resource transfer, and user unlocking

Posted by "zhoulii (via GitHub)" <gi...@apache.org>.
zhoulii commented on PR #2739:
URL: https://github.com/apache/incubator-streampark/pull/2739#issuecomment-1549242526

   Hi @VampireAchao, Thanks for your contribution, the create user function is not available in this pr, can you fix it ? I will review later.
   
   ![image](https://github.com/apache/incubator-streampark/assets/23091870/4a261978-a0ae-4022-9ccf-c95aaa6592aa)
   


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