code review：crm 权限模块的实现

zhijiantianya@gmail.com
1 parent 68c9d563
Showing 18 changed files with 67 additions and 18 deletions
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/controller/admin/business/vo/CrmBusinessTransferReqVO.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/controller/admin/contact/vo/CrmContactTransferReqVO.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/controller/admin/contract/vo/CrmContractTransferReqVO.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/controller/admin/customer/vo/CrmCustomerTransferReqVO.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/controller/admin/permission/CrmPermissionController.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/controller/admin/permission/vo/CrmPermissionBaseVO.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/controller/admin/permission/vo/CrmPermissionRespVO.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/controller/admin/permission/vo/CrmPermissionUpdateReqVO.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/dal/dataobject/permission/CrmPermissionDO.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/dal/mysql/permission/CrmPermissionMapper.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/framework/core/annotations/CrmPermission.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/framework/core/aop/CrmPermissionAspect.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/framework/enums/CrmBizTypeEnum.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/framework/enums/CrmPermissionLevelEnum.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/service/permission/CrmPermissionServiceImpl.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/service/permission/bo/CrmPermissionCreateReqBO.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/service/permission/bo/CrmPermissionUpdateReqBO.java
yudao-module-crm/yudao-module-crm-biz/src/main/java/cn/iocoder/yudao/module/crm/service/permission/bo/CrmTransferPermissionReqBO.java
@@ -21,6 +21,7 @@ public class CrmBusinessTransferReqVO {
     @Schema(description = "新负责人的用户编号", requiredMode = Schema.RequiredMode.REQUIRED, example = "10430")
     private Long newOwnerUserId;
+    // TODO @puhui999：joinTeam 可以合并成 permissionLevel 里；oldOwnerPermissionLevel；这样 null 说明移除，因为都换负责人啦；
     /**
      * 老负责人是否加入团队，是/否
      */
@@ -21,13 +21,13 @@ public class CrmContactTransferReqVO {
     @Schema(description = "新负责人的用户编号", requiredMode = Schema.RequiredMode.REQUIRED, example = "10430")
     private Long newOwnerUserId;
+    // TODO @puhui999：joinTeam 可以合并成 permissionLevel 里；oldOwnerPermissionLevel；这样 null 说明移除，因为都换负责人啦；
     /**
      * 老负责人是否加入团队，是/否
      */
     @Schema(description = "老负责人是否加入团队", requiredMode = Schema.RequiredMode.REQUIRED, example = "false")
     @NotNull(message = "老负责人是否加入团队不能为空")
     private Boolean joinTeam;
-
     /**
      * 老负责人加入团队后的权限级别。如果 {@link #joinTeam} 为 false, permissionLevel 为 null
      * 关联 {@link CrmPermissionLevelEnum}
@@ -21,6 +21,7 @@ public class CrmContractTransferReqVO {
     @Schema(description = "新负责人的用户编号", requiredMode = Schema.RequiredMode.REQUIRED, example = "10430")
     private Long newOwnerUserId;
+    // TODO @puhui999：joinTeam 可以合并成 permissionLevel 里；oldOwnerPermissionLevel；这样 null 说明移除，因为都换负责人啦；
     /**
      * 老负责人是否加入团队，是/否
      */
@@ -21,6 +21,7 @@ public class CrmCustomerTransferReqVO {
     @Schema(description = "新负责人的用户编号", requiredMode = Schema.RequiredMode.REQUIRED, example = "10430")
     private Long newOwnerUserId;
+    // TODO @puhui999：joinTeam 可以合并成 permissionLevel 里；oldOwnerPermissionLevel；这样 null 说明移除，因为都换负责人啦；
     /**
      * 老负责人是否加入团队，是/否
      */
@@ -48,9 +48,11 @@ public class CrmPermissionController {
     @Resource
     private List<CrmPermissionValidateService> permissionValidateServices;
+    // TODO @puhui999：这个能不能使用 CrmPermission 注解替代？
     private void validatePermission(Integer bizType, Long bizId) {
         // 1. TODO 校验是否为超级管理员
         // 2. 防御一手，如果是超级管理员不校验权限还是得校验一下数据是否存在
+        // TODO @puhui999：是不是不用校验每个业务方的数据是否存在；其实不是很关键哈；简单一点~ 说白了，负责人只要在，它的数据就是存在~
         permissionValidateServices.forEach(item -> {
             if (!item.validateBizIdExists(bizType, bizId)) {
                 throw exception(CRM_PERMISSION_MODEL_NOT_EXISTS, getNameByType(bizType));
@@ -90,9 +92,11 @@ public class CrmPermissionController {
         return success(true);
     }
+    // TODO @puhui999：deletemapping
     @GetMapping("/delete")
     @Operation(summary = "移除团队成员")
     @Parameter(name = "id", description = "团队成员编号", required = true)
+    // TODO @puhui999：是不是 id 参数就够了？
     @PreAuthorize("@ss.hasPermission('crm:permission:delete')")
     public CommonResult<Boolean> deletePermission(@RequestParam("bizType") Integer bizType,
                                                   @RequestParam("bizId") Long bizId,
@@ -105,6 +109,8 @@ public class CrmPermissionController {
         return success(true);
     }
+    // TODO @puhui999：这个是哪个地方使用到哈？
+    // TODO @puhui999：是不是 deletemapping 呀；
     @GetMapping("/quit")
     @Operation(summary = "退出团队")
     @Parameters({
@@ -114,9 +120,10 @@ public class CrmPermissionController {
     @PreAuthorize("@ss.hasPermission('crm:permission:delete')")
     public CommonResult<Boolean> quitPermission(@RequestParam("bizType") Integer bizType,
                                                 @RequestParam("bizId") Long bizId) {
+        // 没有就不是团队成员
         CrmPermissionDO permission = crmPermissionService.getPermissionByBizTypeAndBizIdAndUserId(
                 bizType, bizId, getLoginUserId());
-        if (permission == null) { // 没有就不是团队成员
+        if (permission == null) {
             return success(false);
         }
         crmPermissionService.deletePermission(permission.getId());
@@ -136,6 +143,7 @@ public class CrmPermissionController {
         if (CollUtil.isEmpty(permission)) {
             return success(Collections.emptyList());
         }
+        // TODO @puhui999：池子的逻辑；
         permission.removeIf(item -> ObjUtil.equal(item.getUserId(), CrmPermissionDO.POOL_USER_ID)); // 排除
         // 拼接数据
@@ -30,6 +30,7 @@ public class CrmPermissionBaseVO {
     @NotNull(message = "Crm 类型数据编号不能为空")
     private Long bizId;
+    // TODO @puhui999：level；
     @Schema(description = "权限级别", requiredMode = Schema.RequiredMode.REQUIRED, example = "2")
     @InEnum(CrmPermissionLevelEnum.class)
     @NotNull(message = "权限级别不能为空")
@@ -12,9 +12,11 @@ public class CrmPermissionRespVO extends CrmPermissionBaseVO {
     @Schema(description = "数据权限编号", requiredMode = Schema.RequiredMode.REQUIRED, example = "13563")
     private Long id;
+    // TODO @puhui999：搞到字典里；
     @Schema(description = "团队级别", requiredMode = Schema.RequiredMode.REQUIRED, example = "负责人")
     private String permissionLevelName;
+    // TODO @puhui999：deptId、postIds 是不是要提供中文名哈；
     @Schema(description = "部门编号", requiredMode = Schema.RequiredMode.REQUIRED, example = "1")
     private Long deptId;
@@ -12,6 +12,9 @@ import lombok.ToString;
 public class CrmPermissionUpdateReqVO extends CrmPermissionBaseVO {
     @Schema(description = "数据权限编号", requiredMode = Schema.RequiredMode.REQUIRED, example = "13563")
+    // TODO @puhui999：非空判断；
     private Long id;
+    // TODO @puhui999：是不是只更新 permission？？？
+
 }
@@ -23,6 +23,7 @@ import lombok.*;
 @AllArgsConstructor
 public class CrmPermissionDO extends BaseDO {
+    // TODO puhui999：是不是公海的数据，就不插入了；
     /**
      * 当数据变为公海数据时，也就是数据团队成员中没有负责人的时候，将原本的负责人 userId 设置为 POOL_USER_ID 方便查询公海数据。
      * 也就是说每条数据到最后都有一个负责人，如果有人领取则 userId 为领取人
@@ -34,22 +35,35 @@ public class CrmPermissionDO extends BaseDO {
      */
     @TableId
     private Long id;
+
     /**
-     * 数据类型，关联 {@link CrmBizTypeEnum}
+     * 数据类型
+     *
+     * 枚举 {@link CrmBizTypeEnum}
      */
     private Integer bizType;
     /**
-     * 数据编号，关联 {@link CrmBizTypeEnum} 对应模块 DO#getId()
+     * 数据编号
+     *
+     * 关联 {@link CrmBizTypeEnum} 对应模块 DO 的 id 字段
      */
     private Long bizId;
+
     /**
-     * 团队成员，关联 AdminUser#id
+     * 团队成员
+     *
+     * 关联 AdminUser 的 id 字段
+     *
      * 如果为公海数据的话会干掉此数据的负责人后设置为 {@link #POOL_USER_ID}，领取人则上位负责人
      * 例：客户放入公海后会干掉团队成员中的负责人，而其他团队成员则不受影响
      */
     private Long userId;
+
+    // TODO @puhui999：是不是搞成 level 字段；简洁一点，主要表明已经 perssmion 实体里了；
     /**
-     * 权限级别，关联 {@link CrmPermissionLevelEnum}
+     * 权限级别
+     *
+     * 关联 {@link CrmPermissionLevelEnum}
      */
     private Integer permissionLevel;
@@ -18,6 +18,7 @@ import java.util.List;
 @Mapper
 public interface CrmPermissionMapper extends BaseMapperX<CrmPermissionDO> {
+    // TODO @puhui999：是不是不用谢这个注释；因为方法名，可以自解释；
     /**
      * 获取用户数据权限通过 数据类型 x 某个数据 x 用户编号
      *
@@ -33,6 +34,7 @@ public interface CrmPermissionMapper extends BaseMapperX&lt;CrmPermissionDO&gt; {
                 .eq(CrmPermissionDO::getUserId, userId));
     }
+    // TODO @puhui999：是不是不用谢这个注释；因为方法名，可以自解释；
     /**
      * 获取数据权限列表通过 数据类型 x 某个数据
      *
@@ -26,12 +26,14 @@ public @interface CrmPermission {
      */
     CrmBizTypeEnum bizType();
+    // TODO @puhui999：id，通过 spring el 表达式获取；
     /**
      * 数据编号获取来源类，确保数据 id 编号在此类中，不能在父类中。
      * 例：如果在 baseVO 中需要把 id 弄到 updateVO 中。
      */
     Class<?>[] getIdFor() default {};
+    // TODO @puhui999：是不是搞成 level 字段；简洁一点，主要表明已经 perssmion 实体里了；
     /**
      * 操作类型
      */
@@ -43,6 +43,7 @@ public class CrmPermissionAspect {
         return WebFrameworkUtils.getLoginUserId();
     }
+    // TODO @puhui999：id，通过 spring el 表达式获取；
     private Long getBizId(JoinPoint joinPoint, CrmPermission crmPermission) throws NoSuchFieldException, IllegalAccessException {
         Object[] args = joinPoint.getArgs();
         for (Object arg : args) {
@@ -61,6 +62,7 @@ public class CrmPermissionAspect {
         return null;
     }
+    // TODO @puhui999：一般核心的方法，放到最前面，private 放后面。主要是，主次要分出来哈；
     @Before("@annotation(crmPermission)")
     public void doBefore(JoinPoint joinPoint, CrmPermission crmPermission) {
         try {
@@ -75,12 +77,17 @@ public class CrmPermissionAspect {
             // 1. 获取数据权限
             List<CrmPermissionDO> bizPermissions = crmPermissionService.getPermissionByBizTypeAndBizId(bizType, bizId);
+            // TODO puhui999：这种情况下，最好是 CrmPermissionLevelEnum.isOwner
+            // 2.1 情况一：如果自己是负责人，则默认有所有权限
+            // TODO @puhui999：会不会存在空指针的问题？
             CrmPermissionDO userPermission = CollUtil.findOne(bizPermissions, item -> ObjUtil.equal(item.getUserId(), getUserId()));
-            if (isOwner(userPermission.getPermissionLevel())) { // 校验自己是否是负责人
+            if (isOwner(userPermission.getPermissionLevel())) {
                 return;
             }
-            if (isRead(permissionLevel)) { // 读权限
+            // 2.2 情况二：校验自己是否有读权限
+            if (isRead(permissionLevel)) {
                 // 如果没有数据权限或没有负责人则表示此记录为公海数据所有人都有只读权限可以领取成为负责人（团队成员领取的）
+                // TODO @puhui999：89 到 92 这块的逻辑，感觉可以不用 @CrmPermission，公海那自己 check 即可；
                 if (CollUtil.isEmpty(bizPermissions) || CollUtil.anyMatch(bizPermissions,
                         item -> ObjUtil.equal(item.getUserId(), CrmPermissionDO.POOL_USER_ID))) { // 详见 CrmPermissionDO.POOL_USER_ID 注释
                     return;
@@ -88,20 +95,23 @@ public class CrmPermissionAspect {
                 if (isRead(userPermission.getPermissionLevel())) { // 校验当前用户是否有读权限
                     return;
                 }
-                //如果查询数据的话拥有写权限的也能查询
+                // 如果查询数据的话拥有写权限的也能查询
                 if (isWrite(userPermission.getPermissionLevel())) { // 校验当前用户是否有写权限
                     return;
                 }
             }
-            if (isWrite(permissionLevel)) { // 写权限
+            // 2.3 情况三：校验自己是否有写权限
+            if (isWrite(permissionLevel)) {
                 if (isWrite(userPermission.getPermissionLevel())) { // 校验当前用户是否有写权限
                     return;
                 }
             }
-            // 2. 没通过结束，报错 {}操作失败，原因：没有权限
+            // 3. 没通过结束，报错 {} 操作失败，原因：没有权限
+            // TODO @puhui999：这里打个 info 日志，方便后续排查问题、审计；
             throw exception(CRM_PERMISSION_DENIED, crmPermission.bizType().getName());
         } catch (Exception ex) {
+            // TODO @puhui999：不用 catch 掉，就是系统异常；
             log.error("[doBefore][crmPermission({}) 数据校验错误]", toJsonString(crmPermission), ex);
             // TODO 报错抛个什么异常好呢
             throw exception(CRM_PERMISSION_DENIED, crmPermission.bizType().getName());
@@ -33,6 +33,7 @@ public enum CrmBizTypeEnum implements IntArrayValuable {
     private final String name;
     public static String getNameByType(Integer type) {
+        // TODO @puhui999：可以 findone，更简洁；另外，不存在返回 null 即可啦；
         for (CrmBizTypeEnum crmBizTypeEnum : CrmBizTypeEnum.values()) {
             if (ObjUtil.equal(crmBizTypeEnum.type, type)) {
                 return crmBizTypeEnum.name;
@@ -19,6 +19,7 @@ public enum CrmPermissionLevelEnum implements IntArrayValuable {
     OWNER(1, "负责人"),
     READ(2, "读"),
     WRITE(3, "写");
+
     public static final int[] ARRAYS = Arrays.stream(values()).mapToInt(CrmPermissionLevelEnum::getLevel).toArray();
     /**
@@ -48,6 +49,7 @@ public enum CrmPermissionLevelEnum implements IntArrayValuable {
     }
     public static String getNameByLevel(Integer level) {
+        // TODO @puhui999：可以 findone，更简洁；另外，不存在返回 null 即可啦；
         for (CrmPermissionLevelEnum levelEnum : CrmPermissionLevelEnum.values()) {
             if (ObjUtil.equal(levelEnum.level, level)) {
                 return levelEnum.name;
@@ -55,6 +55,7 @@ public class CrmPermissionServiceImpl implements CrmPermissionService {
     @Override
     @Transactional(rollbackFor = Exception.class)
     public void updatePermission(CrmPermissionUpdateReqBO updateBO) {
+        // TODO @puhui999：这里 1.1 1.2；下面 2.；这样更有序一点；
         // 1. 校验用户是否存在
         adminUserApi.validateUserList(Collections.singletonList(updateBO.getUserId()));
         // 2. 校验存在
@@ -91,7 +92,6 @@ public class CrmPermissionServiceImpl implements CrmPermissionService {
         }
     }
-
     @Override
     public void transferPermission(CrmTransferPermissionReqBO transferReqBO) {
         // 1. 校验数据权限-是否是负责人，只有负责人才可以转移
@@ -103,6 +103,7 @@ public class CrmPermissionServiceImpl implements CrmPermissionService {
             throw exception(CRM_PERMISSION_DENIED, crmName);
         }
+        // TODO @puhui999：这个顺序编号，看看调整下；2. 后面是 2.1 ，结果没 2.2 有点怪；
         // 2. 校验转移对象是否已经是该负责人
         if (ObjUtil.equal(transferReqBO.getNewOwnerUserId(), oldPermission.getUserId())) {
             throw exception(CRM_PERMISSION_MODEL_TRANSFER_FAIL_OWNER_USER_EXISTS, crmName);
@@ -27,16 +27,15 @@ public class CrmPermissionCreateReqBO {
     @NotNull(message = "Crm 类型不能为空")
     @InEnum(CrmBizTypeEnum.class)
     private Integer bizType;
-
     /**
      * 数据编号
      */
     @NotNull(message = "Crm 数据编号不能为空")
     private Long bizId;
+    // TODO @puhui999：简化成 level
     /**
      * 权限级别
-     * 关联 {@link CrmPermissionLevelEnum}
      */
     @NotNull(message = "权限级别不能为空")
     @InEnum(CrmPermissionLevelEnum.class)
@@ -17,7 +17,7 @@ import javax.validation.constraints.NotNull;
 public class CrmPermissionUpdateReqBO {
     /**
-     * 数据权限编号 {@link CrmPermissionDO#getId()}
+     * 数据权限编号
      */
     @NotNull(message = "Crm 数据权限编号不能为空")
     private Long id;
@@ -28,22 +28,22 @@ public class CrmPermissionUpdateReqBO {
     @NotNull(message = "用户编号不能为空")
     private Long userId;
+    // TODO @puhui999：id 字段，和 bizType + bizId 是否二选一；
     /**
      * Crm 类型
      */
     @NotNull(message = "Crm 类型不能为空")
     @InEnum(CrmBizTypeEnum.class)
     private Integer bizType;
-
     /**
      * 数据编号
      */
     @NotNull(message = "Crm 数据编号不能为空")
     private Long bizId;
+    // TODO @puhui999：简化成 level
     /**
      * 权限级别
-     * 关联 {@link CrmPermissionLevelEnum}
      */
     @NotNull(message = "权限级别不能为空")
     @InEnum(CrmPermissionLevelEnum.class)
@@ -7,6 +7,7 @@ import lombok.Data;
 import javax.validation.constraints.NotNull;
+// TODO @puhui999：CrmPermissionTransferReqBO
 /**
  * 数据权限转移 Request BO
  *
@@ -27,7 +28,6 @@ public class CrmTransferPermissionReqBO {
     @NotNull(message = "Crm 类型不能为空")
     @InEnum(CrmBizTypeEnum.class)
     private Integer bizType;
-
     /**
      * 数据编号
      */
@@ -40,6 +40,7 @@ public class CrmTransferPermissionReqBO {
     @NotNull(message = "新负责人的用户编号不能为空")
     private Long newOwnerUserId;
+    // TODO @puhui999：joinTeam 可以合并成 permissionLevel 里；oldOwnerPermissionLevel；这样 null 说明移除，因为都换负责人啦；
     /**
      * 老负责人是否加入团队，是/否
      */