refactor: provide review feedback (#2325)

services/skus/controllers.go

@@ -626,11 +626,16 @@ func deleteItemCreds(svc *Service) handlers.AppHandler {
 		isSigned := r.URL.Query().Get("isSigned") == "true"
 
 		if err := svc.DeleteOrderCreds(ctx, orderID, reqID, isSigned); err != nil {
-			if errors.Is(err, model.ErrOrderNotFound) || errors.Is(err, ErrOrderHasNoItems) {
+			lg.Error().Err(err).Msg("failed to delete the order credentials")
+
+			switch {
+			case errors.Is(err, model.ErrOrderNotFound), errors.Is(err, ErrOrderHasNoItems):
 				return handlers.WrapError(err, "order or item not found", http.StatusNotFound)
+			case errors.Is(err, errExceededMaxActiveOrderCreds):
+				return handlers.WrapError(err, errExceededMaxActiveOrderCreds.Error(), http.StatusUnprocessableEntity)
+			default:
+				return handlers.WrapError(err, "error deleting credentials", http.StatusInternalServerError)
 			}
-			lg.Error().Err(err).Msg("failed to delete the order credentials")
-			return handlers.WrapError(err, "error deleting credentials", http.StatusInternalServerError)
 		}
 
 		return handlers.RenderContent(ctx, nil, w, http.StatusOK)

services/skus/credentials.go

@@ -44,7 +44,7 @@ var (
 	errItemDoesNotExist       model.Error = "order item does not exist for order"
 	errCredsAlreadySubmitted  model.Error = "credentials already submitted"
 
-	errMaxActiveOrderCredsExceeded model.Error = "maximum active order credentials exceeded"
+	errExceededMaxActiveOrderCreds model.Error = "maximum active order credentials exceeded"
 
 	defaultExpiresAt = time.Now().Add(17532 * time.Hour) // 2 years
 	retryPolicy      = retrypolicy.DefaultRetry
@@ -752,7 +752,7 @@ func (s *Service) deleteTLV2(ctx context.Context, dbi sqlx.ExtContext, order *mo
 			return fmt.Errorf("failed to get count of active order credentials: %w", err)
 		}
 		if activeCreds > maxTLV2ActiveOrderCreds {
-			return errMaxActiveOrderCredsExceeded
+			return errExceededMaxActiveOrderCreds
 		}
 		return s.Datastore.DeleteTimeLimitedV2OrderCredsByOrderTx(ctx, dbi, order.ID, reqID)
 	}

services/skus/datastore.go

@@ -1036,15 +1036,13 @@ func (pg *Postgres) GetTLV2Creds(ctx context.Context, dbi sqlx.QueryerContext, o
 	return result, nil
 }
 
-// GetCountActiveOrderCreds returns the count of order creds currently active on an order
+// GetCountActiveOrderCreds returns the count of order creds currently active on an order.
 func (pg *Postgres) GetCountActiveOrderCreds(ctx context.Context, dbi sqlx.ExtContext, orderID uuid.UUID, now time.Time) (int, error) {
-	query := `
-		select count(1) from time_limited_v2_order_creds
-		where order_id = $1 and $2 > valid_from and valid_to > $2 group by request_id
-	`
+	const q = `SELECT COUNT(1) FROM time_limited_v2_order_creds
+		WHERE order_id = $1 AND valid_from < $2 AND valid_to > $2 GROUP BY request_id`
 
 	var activeCredCount int
-	if err := sqlx.GetContext(ctx, dbi, &activeCredCount, query, orderID, now); err != nil {
+	if err := sqlx.GetContext(ctx, dbi, &activeCredCount, q, orderID, now); err != nil {
 		return 0, fmt.Errorf("error getting active credential count: %w", err)
 	}
 

services/skus/datastore_test.go

@@ -155,18 +155,19 @@ func (suite *PostgresTestSuite) TestCountActiveOrderCreds_Success() {
 	env := os.Getenv("ENV")
 	ctx := context.WithValue(context.Background(), appctx.EnvironmentCTXKey, env)
 
-	// create paid order with two order items
-	ctx = context.WithValue(ctx, appctx.WhitelistSKUsCTXKey, []string{devBraveFirewallVPNPremiumTimeLimited,
-		devBraveSearchPremiumYearTimeLimited})
+	ctx = context.WithValue(ctx, appctx.WhitelistSKUsCTXKey, []string{
+		devBraveFirewallVPNPremiumTimeLimited,
+		devBraveSearchPremiumYearTimeLimited,
+	})
 
-	orderCredentials := suite.createTimeLimitedV2OrderCreds(suite.T(), ctx, devBraveFirewallVPNPremiumTimeLimited,
-		devBraveSearchPremiumYearTimeLimited)
+	creds := suite.createTimeLimitedV2OrderCreds(suite.T(), ctx, devBraveFirewallVPNPremiumTimeLimited, devBraveSearchPremiumYearTimeLimited)
 
-	// both order items have same orderID so can use the first element to retrieve all order creds
-	currentlyActiveOrderCredentialsCount, err := suite.storage.GetCountActiveOrderCreds(ctx, suite.storage.RawDB(), orderCredentials[0].OrderID, time.Now())
+	actual, err := suite.storage.GetCountActiveOrderCreds(ctx, suite.storage.RawDB(), creds[0].OrderID, time.Now())
 	suite.Require().NoError(err)
 
-	suite.Assert().Equal(2, currentlyActiveOrderCredentialsCount)
+	const expected = 2
+
+	suite.Assert().Equal(expected, actual)
 }
 
 func (suite *PostgresTestSuite) TestGetTimeLimitedV2OrderCredsByOrder_Success() {