code review：优惠劵发放

YunaiV
1 parent af0771a1
Showing 9 changed files with 43 additions and 52 deletions
yudao-module-mall/yudao-module-promotion-api/src/main/java/cn/iocoder/yudao/module/promotion/enums/ErrorCodeConstants.java
yudao-module-mall/yudao-module-promotion-api/src/main/java/cn/iocoder/yudao/module/promotion/enums/coupon/CouponTakeTypeEnum.java
yudao-module-mall/yudao-module-promotion-biz/src/main/java/cn/iocoder/yudao/module/promotion/controller/admin/coupon/CouponController.java
yudao-module-mall/yudao-module-promotion-biz/src/main/java/cn/iocoder/yudao/module/promotion/controller/admin/coupon/CouponTemplateController.java
yudao-module-mall/yudao-module-promotion-biz/src/main/java/cn/iocoder/yudao/module/promotion/convert/coupon/CouponConvert.java
yudao-module-mall/yudao-module-promotion-biz/src/main/java/cn/iocoder/yudao/module/promotion/dal/mysql/coupon/CouponMapper.java
yudao-module-mall/yudao-module-promotion-biz/src/main/java/cn/iocoder/yudao/module/promotion/dal/mysql/coupon/CouponTemplateMapper.java
yudao-module-mall/yudao-module-promotion-biz/src/main/java/cn/iocoder/yudao/module/promotion/service/coupon/CouponService.java
yudao-module-mall/yudao-module-promotion-biz/src/main/java/cn/iocoder/yudao/module/promotion/service/coupon/CouponServiceImpl.java
@@ -27,8 +27,8 @@ public interface ErrorCodeConstants {
     // ========== 优惠劵模板 1013004000 ==========
     ErrorCode COUPON_TEMPLATE_NOT_EXISTS = new ErrorCode(1013004000, "优惠劵模板不存在");
     ErrorCode COUPON_TEMPLATE_TOTAL_COUNT_TOO_SMALL = new ErrorCode(1013004001, "发放数量不能小于已领取数量({})");
-    ErrorCode COUPON_TEMPLATE_TASK_EMPTY = new ErrorCode(1013004002, "当前剩余数量不够领取");
-    ErrorCode COUPON_TEMPLATE_USER_TASKED = new ErrorCode(1013004003, "用户已领取过此优惠券");
+    ErrorCode COUPON_TEMPLATE_NOT_ENOUGH = new ErrorCode(1013004002, "当前剩余数量不够领取");
+    ErrorCode COUPON_TEMPLATE_USER_ALREADY_TAKE = new ErrorCode(1013004003, "用户已领取过此优惠券");
     ErrorCode COUPON_TEMPLATE_EXPIRED = new ErrorCode(1013004004, "优惠券已过期");
     ErrorCode COUPON_TEMPLATE_CANNOT_TAKE = new ErrorCode(1013004005, "领取方式不正确");
  
@@ -14,7 +14,8 @@ import java.util.Arrays;
 @AllArgsConstructor
 @Getter
 public enum CouponTakeTypeEnum implements IntArrayValuable {
-    COMMON(0, "通用"),
+
+    COMMON(0, "通用"), // TODO @疯狂：要不去掉“通用"和“兑换”，保持和 crmeb 一致；就手动领取、指定发送、新人券
     BY_USER(1, "直接领取"), // 用户可在首页、每日领劵直接领取
     BY_ADMIN(2, "指定发放"), // 后台指定会员赠送优惠劵
     BY_REGISTER(3, "新人券"), // 注册时自动领取
@@ -22,7 +22,6 @@ import org.springframework.web.bind.annotation.*;
 import javax.annotation.Resource;
 import javax.validation.Valid;
 import java.util.Map;
-import java.util.Set;
  
 import static cn.iocoder.yudao.framework.common.pojo.CommonResult.success;
 import static cn.iocoder.yudao.framework.common.util.collection.CollectionUtils.convertSet;
@@ -65,9 +64,9 @@ public class CouponController {
         if (CollUtil.isEmpty(pageResulVO.getList())) {
             return success(pageResulVO);
         }
+
         // 读取用户信息，进行拼接
-        Set<Long> userIds = convertSet(pageResult.getList(), CouponDO::getUserId);
-        Map<Long, MemberUserRespDTO> userMap = memberUserApi.getUserMap(userIds);
+        Map<Long, MemberUserRespDTO> userMap = memberUserApi.getUserMap(convertSet(pageResult.getList(), CouponDO::getUserId));
         pageResulVO.getList().forEach(itemRespVO -> MapUtils.findAndThen(userMap, itemRespVO.getUserId(),
                 userRespDTO -> itemRespVO.setNickname(userRespDTO.getNickname())));
         return success(pageResulVO);
@@ -80,4 +79,5 @@ public class CouponController {
         Boolean result = couponService.sendCoupon(reqVO.getTemplateId(), reqVO.getUserIds());
         return success(result);
     }
+
 }
@@ -79,6 +79,7 @@ public class CouponTemplateController {
         return success(CouponTemplateConvert.INSTANCE.convertPage(pageResult));
     }
  
+    // TODO @疯狂：是不是可以合并到 getCouponTemplatePage 接口，作为一个参数选择
     @GetMapping("/can-take-page")
     @Operation(summary = "获得可用于领取的优惠劵模板分页")
     @PreAuthorize("@ss.hasPermission('promotion:coupon-template:query')")
 package cn.iocoder.yudao.module.promotion.convert.coupon;
  
-import cn.iocoder.yudao.framework.common.enums.CommonStatusEnum;
 import cn.iocoder.yudao.framework.common.pojo.PageResult;
 import cn.iocoder.yudao.module.promotion.api.coupon.dto.CouponRespDTO;
 import cn.iocoder.yudao.module.promotion.controller.admin.coupon.vo.coupon.CouponPageItemRespVO;
@@ -55,6 +55,7 @@ public interface CouponMapper extends BaseMapperX&lt;CouponDO&gt; {
                 .eq(CouponDO::getStatus, status));
     }
  
+    // TODO @疯狂：要 selectList 哈；
     default List<CouponDO> selectByTemplateIdAndUserId(Long templateId, Collection<Long> userIds) {
         return selectList(new LambdaQueryWrapperX<CouponDO>()
                 .eq(CouponDO::getTemplateId, templateId)
@@ -34,17 +34,11 @@ public interface CouponTemplateMapper extends BaseMapperX&lt;CouponTemplateDO&gt; {
     default PageResult<CouponTemplateDO> selectCanTakePage(CouponTemplatePageReqVO reqVO, Collection<Integer> takeTypes) {
         // 构建可领取的查询条件, 好啰嗦  ( ╯-_-)╯┴—┴
         Consumer<LambdaQueryWrapper<CouponTemplateDO>> canTakeConsumer = w ->
-                // 1.状态为可用的
-                w.eq(CouponTemplateDO::getStatus, CommonStatusEnum.ENABLE.getStatus())
-                        // 2.领取方式一致
-                        .in(CouponTemplateDO::getTakeType, takeTypes)
-                        // 3.未过期
-                        .and(ww -> ww.isNull(CouponTemplateDO::getValidEndTime)
-                                .or()
-                                .gt(CouponTemplateDO::getValidEndTime, LocalDateTime.now()))
-                        // 4.剩余数量大于0
-                        .apply(" take_count < total_count ");
-
+                w.eq(CouponTemplateDO::getStatus, CommonStatusEnum.ENABLE.getStatus()) // 1. 状态为可用的
+                        .in(CouponTemplateDO::getTakeType, takeTypes) // 2. 领取方式一致
+                        .and(ww ->  ww.isNull(CouponTemplateDO::getValidEndTime)  // 3. 未过期
+                                .or().gt(CouponTemplateDO::getValidEndTime, LocalDateTime.now()))
+                        .apply(" take_count < total_count "); // 4. 剩余数量大于 0
         return selectPage(reqVO, new LambdaQueryWrapperX<CouponTemplateDO>()
                 .likeIfPresent(CouponTemplateDO::getName, reqVO.getName())
                 .eqIfPresent(CouponTemplateDO::getDiscountType, reqVO.getDiscountType())
@@ -78,18 +78,20 @@ public interface CouponService {
      */
     Long getUnusedCouponCount(Long userId);
  
+    // TODO @疯狂：可以返回 void；因为都是 true = =
     /**
      * 领取优惠券
      *
      * @param templateId 优惠券模板编号
      * @param userIds    用户编号列表
-     * @param takType    领取方式
+     * @param takeType    领取方式
      * @return 领取结果
      */
-    Boolean takeCoupon(Long templateId, Set<Long> userIds, CouponTakeTypeEnum takType);
+    Boolean takeCoupon(Long templateId, Set<Long> userIds, CouponTakeTypeEnum takeType);
  
+    // TODO @疯狂：感觉 3 个方法的命名，改成 takeCouponByAdmin；takeCouponByUser；takeCouponByRegister 会更容易理解哈；现在两个都叫 sendCoupon ，感觉不太好懂
     /**
-     * 【管理员】 给用户发送优惠券
+     * 【管理员】给用户发送优惠券
      *
      * @param templateId 优惠券模板编号
      * @param userIds    用户编号列表
@@ -100,7 +102,7 @@ public interface CouponService {
     }
  
     /**
-     * 【会员】 领取优惠券
+     * 【会员】领取优惠券
      *
      * @param templateId 优惠券模板编号
      * @param userId     用户编号
@@ -111,7 +113,7 @@ public interface CouponService {
     }
  
     /**
-     * 【系统】 给用户发送新人券
+     * 【系统】给用户发送新人券
      *
      * @param templateId 优惠券模板编号
      * @param userId     用户编号列表
@@ -134,20 +134,20 @@ public class CouponServiceImpl implements CouponService {
     }
  
     @Override
-    public Boolean takeCoupon(Long templateId, Set<Long> userIds, CouponTakeTypeEnum takType) {
+    public Boolean takeCoupon(Long templateId, Set<Long> userIds, CouponTakeTypeEnum takeType) {
+        // 1. 校验并过滤用户
         CouponTemplateDO template = couponTemplateService.getCouponTemplate(templateId);
-        // 校验并过滤用户
-        userIds = validateAndFilterTakeUserId(template, userIds, takType);
+        userIds = validateAndFilterTakeUserId(template, userIds, takeType);
  
+        // 2. 批量保存优惠劵
+        // TODO @疯狂：这里可以使用 CollectionUtils.convertList 更简洁；stream 可以简化很多代码，常用的 stream 操作，使用 util 可以进一步简洁，同时提升可读性
         List<CouponDO> couponList = userIds.stream()
                 .map(userId -> CouponConvert.INSTANCE.convert(template, userId))
                 .collect(Collectors.toList());
-        // 批量保存
         couponMapper.insertBatch(couponList);
  
-        // 增加优惠劵模板的领取数量
+        // 3. 增加优惠劵模板的领取数量
         couponTemplateService.updateCouponTemplateTakeCount(templateId, userIds.size());
-
         return true;
     }
  
@@ -159,55 +159,48 @@ public class CouponServiceImpl implements CouponService {
      * @param takeType       领取方式
      * @return 可领取此券的用户列表
      */
+    // TODO @疯狂：我建议哈，校验模版，和过滤用户分成两个方法；混在一起，有点小重，后续单测可能也比较难写哈；
     private Set<Long> validateAndFilterTakeUserId(CouponTemplateDO couponTemplate, Set<Long> userIds, CouponTakeTypeEnum takeType) {
-        // 校验模板
+        // 1.1 校验模板
         if (couponTemplate == null) {
             throw exception(COUPON_TEMPLATE_NOT_EXISTS);
         }
-
-        if (couponTemplate.getTotalCount() > 0) {
-            // 校验剩余数量
-            if (couponTemplate.getTakeCount() + userIds.size() > couponTemplate.getTotalCount()) {
-                throw exception(COUPON_TEMPLATE_TASK_EMPTY);
-            }
+        // 1.2 校验剩余数量
+        if (couponTemplate.getTakeCount() + userIds.size() > couponTemplate.getTotalCount()) {
+            throw exception(COUPON_TEMPLATE_NOT_ENOUGH);
         }
-
-        // 校验"固定日期"的有效期类型是否过期
+        // 1.3 校验"固定日期"的有效期类型是否过期
         if (CouponTemplateValidityTypeEnum.DATE.getType().equals(couponTemplate.getValidityType())) {
             if (LocalDateTimeUtils.beforeNow(couponTemplate.getValidEndTime())) {
                 throw exception(COUPON_TEMPLATE_EXPIRED);
             }
         }
-
-        // 校验领取方式
+        // 1.4 校验领取方式
+        // TODO @疯狂：如果要做这样的判断，使用 !ObjectUtils.equalsAny() 会更简洁
         if (!CouponTakeTypeEnum.COMMON.getValue().equals(couponTemplate.getTakeType())) {
             if (ObjectUtil.notEqual(couponTemplate.getTakeType(), takeType.getValue())) {
                 throw exception(COUPON_TEMPLATE_CANNOT_TAKE);
             }
         }
  
-        // 获取领取过此券的用户
-        List<CouponDO> takedList = couponMapper.selectByTemplateIdAndUserId(couponTemplate.getId(), userIds);
-
+        // 2.1 过滤掉，已经领取到上限的用户
+        List<CouponDO> alreadyTakeCoupons = couponMapper.selectByTemplateIdAndUserId(couponTemplate.getId(), userIds);
         // 校验新人券
+        // TODO @疯狂：我在想，这个判断，是不是和下面的 couponTemplate.getTakeLimitCount() > 0 冗余了；可以先都过滤，然后最终去判断 userIds 是不是空了；
         if (CouponTakeTypeEnum.BY_REGISTER.equals(takeType)) {
-            if (!takedList.isEmpty()) {
-                throw exception(COUPON_TEMPLATE_USER_TASKED);
+            if (!alreadyTakeCoupons.isEmpty()) {
+                throw exception(COUPON_TEMPLATE_USER_ALREADY_TAKE);
             }
         }
-
         // 校验领取数量限制
         if (couponTemplate.getTakeLimitCount() > 0) {
-            // 统计用户的领取数量
-            Map<Long, Integer> userTakeCountMap = CollStreamUtil.groupBy(takedList, CouponDO::getUserId, Collectors.summingInt(c -> 1));
-            //过滤掉达到领取数量限制的用户
+            Map<Long, Integer> userTakeCountMap = CollStreamUtil.groupBy(alreadyTakeCoupons, CouponDO::getUserId, Collectors.summingInt(c -> 1));
             userIds.removeIf(userId -> MapUtil.getInt(userTakeCountMap, userId, 0) >= couponTemplate.getTakeLimitCount());
-            // 用户全部领取过此优惠券
+            // 2.2 如果所有用户都领取过，则抛出异常
             if (userIds.isEmpty()) {
-                throw exception(COUPON_TEMPLATE_USER_TASKED);
+                throw exception(COUPON_TEMPLATE_USER_ALREADY_TAKE);
             }
         }
-
         return userIds;
     }