From c8de30f78cf9e67521f9da3379a3ee3093a02e2c Mon Sep 17 00:00:00 2001 From: Chang lue Tsen Date: Wed, 23 Jul 2025 11:45:06 -0400 Subject: [PATCH] fix(order): improve code quality and fix critical bugs in order processing logic - Fix inconsistent logging calls across all order logic files - Fix critical gift amount deduction logic bug in renewal process - Fix variable shadowing errors in database transactions - Add comprehensive Go-standard documentation comments - Improve log prefix consistency for better debugging - Remove redundant discount validation code --- internal/logic/public/order/getDiscount.go | 1 + .../logic/public/order/preCreateOrderLogic.go | 20 +++++++++---- internal/logic/public/order/purchaseLogic.go | 29 ++++++++++++------- internal/logic/public/order/renewalLogic.go | 18 +++++++----- 4 files changed, 44 insertions(+), 24 deletions(-) diff --git a/internal/logic/public/order/getDiscount.go b/internal/logic/public/order/getDiscount.go index c645e89..34c16a9 100644 --- a/internal/logic/public/order/getDiscount.go +++ b/internal/logic/public/order/getDiscount.go @@ -10,5 +10,6 @@ func getDiscount(discounts []types.SubscribeDiscount, inputMonths int64) float64 finalDiscount = discount.Discount } } + return float64(finalDiscount) / float64(100) } diff --git a/internal/logic/public/order/preCreateOrderLogic.go b/internal/logic/public/order/preCreateOrderLogic.go index 1f974da..4e16715 100644 --- a/internal/logic/public/order/preCreateOrderLogic.go +++ b/internal/logic/public/order/preCreateOrderLogic.go @@ -24,7 +24,8 @@ type PreCreateOrderLogic struct { svcCtx *svc.ServiceContext } -// Pre create order +// NewPreCreateOrderLogic creates a new pre-create order logic instance for order preview operations. +// It initializes the logger with context and sets up the service context for database operations. func NewPreCreateOrderLogic(ctx context.Context, svcCtx *svc.ServiceContext) *PreCreateOrderLogic { return &PreCreateOrderLogic{ Logger: logger.WithContext(ctx), @@ -33,12 +34,21 @@ func NewPreCreateOrderLogic(ctx context.Context, svcCtx *svc.ServiceContext) *Pr } } +// PreCreateOrder calculates order pricing preview including discounts, coupons, gift amounts, and fees +// without actually creating an order. It validates subscription plans, coupons, and payment methods +// to provide accurate pricing information for the frontend order preview. func (l *PreCreateOrderLogic) PreCreateOrder(req *types.PurchaseOrderRequest) (resp *types.PreOrderResponse, err error) { u, ok := l.ctx.Value(constant.CtxKeyUser).(*user.User) if !ok { logger.Error("current user is not found in context") return nil, errors.Wrapf(xerr.NewErrCode(xerr.InvalidAccess), "Invalid Access") } + + if req.Quantity <= 0 { + l.Debugf("[PreCreateOrder] Quantity is less than or equal to 0, setting to 1") + req.Quantity = 1 + } + // find subscribe plan sub, err := l.svcCtx.SubscribeModel.FindOne(l.ctx, req.SubscribeId) if err != nil { @@ -52,9 +62,7 @@ func (l *PreCreateOrderLogic) PreCreateOrder(req *types.PurchaseOrderRequest) (r discount = getDiscount(dis, req.Quantity) } price := sub.UnitPrice * req.Quantity - if discount == 0 { - discount = 1 - } + amount := int64(float64(price) * discount) discountAmount := price - amount var couponAmount int64 @@ -75,7 +83,7 @@ func (l *PreCreateOrderLogic) PreCreateOrder(req *types.PurchaseOrderRequest) (r }) if err != nil { - l.Logger.Error("[PreCreateOrder] Database query error", logger.Field("error", err.Error()), logger.Field("user_id", u.Id), logger.Field("coupon", req.Coupon)) + l.Errorw("[PreCreateOrder] Database query error", logger.Field("error", err.Error()), logger.Field("user_id", u.Id), logger.Field("coupon", req.Coupon)) return nil, errors.Wrapf(xerr.NewErrCode(xerr.DatabaseQueryError), "find coupon error: %v", err.Error()) } @@ -106,7 +114,7 @@ func (l *PreCreateOrderLogic) PreCreateOrder(req *types.PurchaseOrderRequest) (r if req.Payment != 0 { payment, err := l.svcCtx.PaymentModel.FindOne(l.ctx, req.Payment) if err != nil { - l.Logger.Error("[PreCreateOrder] Database query error", logger.Field("error", err.Error()), logger.Field("payment", req.Payment)) + l.Errorw("[PreCreateOrder] Database query error", logger.Field("error", err.Error()), logger.Field("payment", req.Payment)) return nil, errors.Wrapf(xerr.NewErrCode(xerr.DatabaseQueryError), "find payment method error: %v", err.Error()) } // Calculate the handling fee diff --git a/internal/logic/public/order/purchaseLogic.go b/internal/logic/public/order/purchaseLogic.go index b3b124d..cabeecc 100644 --- a/internal/logic/public/order/purchaseLogic.go +++ b/internal/logic/public/order/purchaseLogic.go @@ -31,7 +31,8 @@ const ( CloseOrderTimeMinutes = 15 ) -// NewPurchaseLogic purchase Subscription +// NewPurchaseLogic creates a new purchase logic instance for subscription purchase operations. +// It initializes the logger with context and sets up the service context for database operations. func NewPurchaseLogic(ctx context.Context, svcCtx *svc.ServiceContext) *PurchaseLogic { return &PurchaseLogic{ Logger: logger.WithContext(ctx), @@ -40,6 +41,9 @@ func NewPurchaseLogic(ctx context.Context, svcCtx *svc.ServiceContext) *Purchase } } +// Purchase processes new subscription purchase orders including validation, discount calculation, +// coupon processing, gift amount deduction, fee calculation, and order creation with database transaction. +// It handles the complete purchase workflow from user validation to order creation and task scheduling. func (l *PurchaseLogic) Purchase(req *types.PurchaseOrderRequest) (resp *types.PurchaseOrderResponse, err error) { u, ok := l.ctx.Value(constant.CtxKeyUser).(*user.User) @@ -47,6 +51,12 @@ func (l *PurchaseLogic) Purchase(req *types.PurchaseOrderRequest) (resp *types.P logger.Error("current user is not found in context") return nil, errors.Wrapf(xerr.NewErrCode(xerr.InvalidAccess), "Invalid Access") } + + if req.Quantity <= 0 { + l.Debugf("[Purchase] Quantity is less than or equal to 0, setting to 1") + req.Quantity = 1 + } + // find user subscription userSub, err := l.svcCtx.UserModel.QueryUserSubscribe(l.ctx, u.Id) if err != nil { @@ -89,9 +99,6 @@ func (l *PurchaseLogic) Purchase(req *types.PurchaseOrderRequest) (resp *types.P _ = json.Unmarshal([]byte(sub.Discount), &dis) discount = getDiscount(dis, req.Quantity) } - if discount == 0 { - discount = 1 - } price := sub.UnitPrice * req.Quantity // discount amount amount := int64(float64(price) * discount) @@ -145,7 +152,7 @@ func (l *PurchaseLogic) Purchase(req *types.PurchaseOrderRequest) (resp *types.P // find payment method payment, err := l.svcCtx.PaymentModel.FindOne(l.ctx, req.Payment) if err != nil { - l.Logger.Error("[Purchase] Database query error", logger.Field("error", err.Error()), logger.Field("payment", req.Payment)) + l.Errorw("[Purchase] Database query error", logger.Field("error", err.Error()), logger.Field("payment", req.Payment)) return nil, errors.Wrapf(xerr.NewErrCode(xerr.DatabaseQueryError), "find payment method error: %v", err.Error()) } var feeAmount int64 @@ -183,8 +190,8 @@ func (l *PurchaseLogic) Purchase(req *types.PurchaseOrderRequest) (resp *types.P // update user deduction && Pre deduction ,Return after canceling the order if orderInfo.GiftAmount > 0 { // update user deduction && Pre deduction ,Return after canceling the order - if e := l.svcCtx.UserModel.Update(l.ctx, u, db); err != nil { - l.Errorw("[Purchase] Database update error", logger.Field("error", err.Error()), logger.Field("user", u)) + if e := l.svcCtx.UserModel.Update(l.ctx, u, db); e != nil { + l.Errorw("[Purchase] Database update error", logger.Field("error", e.Error()), logger.Field("user", u)) return e } // create deduction record @@ -198,7 +205,7 @@ func (l *PurchaseLogic) Purchase(req *types.PurchaseOrderRequest) (resp *types.P } if e := db.Model(&user.GiftAmountLog{}).Create(&giftAmountLog).Error; e != nil { l.Errorw("[Purchase] Database insert error", - logger.Field("error", err.Error()), + logger.Field("error", e.Error()), logger.Field("deductionLog", giftAmountLog), ) return e @@ -216,14 +223,14 @@ func (l *PurchaseLogic) Purchase(req *types.PurchaseOrderRequest) (resp *types.P } val, err := json.Marshal(payload) if err != nil { - l.Errorw("[CreateOrder] Marshal payload error", logger.Field("error", err.Error()), logger.Field("payload", payload)) + l.Errorw("[Purchase] Marshal payload error", logger.Field("error", err.Error()), logger.Field("payload", payload)) } task := asynq.NewTask(queue.DeferCloseOrder, val, asynq.MaxRetry(3)) taskInfo, err := l.svcCtx.Queue.Enqueue(task, asynq.ProcessIn(CloseOrderTimeMinutes*time.Minute)) if err != nil { - l.Errorw("[CreateOrder] Enqueue task error", logger.Field("error", err.Error()), logger.Field("task", task)) + l.Errorw("[Purchase] Enqueue task error", logger.Field("error", err.Error()), logger.Field("task", task)) } else { - l.Infow("[CreateOrder] Enqueue task success", logger.Field("TaskID", taskInfo.ID)) + l.Infow("[Purchase] Enqueue task success", logger.Field("TaskID", taskInfo.ID)) } return &types.PurchaseOrderResponse{ diff --git a/internal/logic/public/order/renewalLogic.go b/internal/logic/public/order/renewalLogic.go index b70b5b3..6692628 100644 --- a/internal/logic/public/order/renewalLogic.go +++ b/internal/logic/public/order/renewalLogic.go @@ -27,7 +27,7 @@ type RenewalLogic struct { svcCtx *svc.ServiceContext } -// NewRenewalLogic Renewal Subscription +// NewRenewalLogic creates a new renewal logic instance for subscription renewal operations func NewRenewalLogic(ctx context.Context, svcCtx *svc.ServiceContext) *RenewalLogic { return &RenewalLogic{ Logger: logger.WithContext(ctx), @@ -36,12 +36,19 @@ func NewRenewalLogic(ctx context.Context, svcCtx *svc.ServiceContext) *RenewalLo } } +// Renewal processes subscription renewal orders including discount calculation, +// coupon validation, gift amount deduction, fee calculation, and order creation func (l *RenewalLogic) Renewal(req *types.RenewalOrderRequest) (resp *types.RenewalOrderResponse, err error) { u, ok := l.ctx.Value(constant.CtxKeyUser).(*user.User) if !ok { logger.Error("current user is not found in context") return nil, errors.Wrapf(xerr.NewErrCode(xerr.InvalidAccess), "Invalid Access") } + if req.Quantity <= 0 { + l.Debugf("[Renewal] Quantity is less than or equal to 0, setting to 1") + req.Quantity = 1 + } + orderNo := tool.GenerateTradeNo() // find user subscribe userSubscribe, err := l.svcCtx.UserModel.FindOneUserSubscribe(l.ctx, req.UserSubscribeID) @@ -64,9 +71,6 @@ func (l *RenewalLogic) Renewal(req *types.RenewalOrderRequest) (resp *types.Rene _ = json.Unmarshal([]byte(sub.Discount), &dis) discount = getDiscount(dis, req.Quantity) } - if discount == 0 { - discount = 1 - } price := sub.UnitPrice * req.Quantity amount := int64(float64(price) * discount) discountAmount := price - amount @@ -103,7 +107,7 @@ func (l *RenewalLogic) Renewal(req *types.RenewalOrderRequest) (resp *types.Rene payment, err := l.svcCtx.PaymentModel.FindOne(l.ctx, req.Payment) if err != nil { l.Errorw("[Renewal] Database query error", logger.Field("error", err.Error()), logger.Field("payment", req.Payment)) - return nil, errors.Wrapf(err, "find payment error: %v", err.Error()) + return nil, errors.Wrapf(xerr.NewErrCode(xerr.DatabaseQueryError), "find payment error: %v", err.Error()) } amount -= coupon @@ -112,8 +116,8 @@ func (l *RenewalLogic) Renewal(req *types.RenewalOrderRequest) (resp *types.Rene if u.GiftAmount > 0 { if u.GiftAmount >= amount { deductionAmount = amount + u.GiftAmount -= deductionAmount amount = 0 - u.GiftAmount -= amount } else { deductionAmount = u.GiftAmount amount -= u.GiftAmount @@ -155,7 +159,7 @@ func (l *RenewalLogic) Renewal(req *types.RenewalOrderRequest) (resp *types.Rene if orderInfo.GiftAmount > 0 { // update user deduction && Pre deduction ,Return after canceling the order if err := l.svcCtx.UserModel.Update(l.ctx, u, db); err != nil { - l.Errorw("[Purchase] Database update error", logger.Field("error", err.Error()), logger.Field("user", u)) + l.Errorw("[Renewal] Database update error", logger.Field("error", err.Error()), logger.Field("user", u)) return err } // create deduction record