feat: add endpoint for cancelling orders (#2656)

* feat: add new endpoint for cancelling orders

* feat: extract order cancellation into method

* test: add tests for new and updated code

* fix: use sentinel error for reporting failed uuid parsing

* fix: use better method for error checking

* test: add pseudo-integration test

* test: add pseudo-integration test

services/grant/cmd/grant.go

@@ -507,6 +507,17 @@ func setupRouter(ctx context.Context, logger *zerolog.Logger) (context.Context,
 			)
 		}
 
+		corsMwrDelete := skus.NewCORSMwr(corsOpts, http.MethodDelete)
+
+		subr.Method(
+			http.MethodDelete,
+			"/{orderID}",
+			middleware.InstrumentHandler(
+				"CancelOrderNew",
+				corsMwrDelete(authMwr(handlers.AppHandler(orderh.Cancel))),
+			),
+		)
+
 		r.Mount("/v1/orders-new", subr)
 	}
 

services/skus/controllers.go

@@ -365,7 +365,7 @@ func CancelOrder(service *Service) handlers.AppHandler {
 			return handlers.WrapError(err, "Error validating auth merchant and caveats", http.StatusForbidden)
 		}
 
-		if err := service.CancelOrder(oid); err != nil {
+		if err := service.CancelOrderLegacy(oid); err != nil {
 			return handlers.WrapError(err, "Error retrieving the order", http.StatusInternalServerError)
 		}
 

services/skus/controllers_test.go

@@ -1899,6 +1899,45 @@ func (suite *ControllersTestSuite) TestWebhook_Radom() {
 	suite.Equal(model.OrderStatusPaid, order.Status)
 }
 
+func (suite *ControllersTestSuite) TestOrder_Cancel() {
+	ctx := context.Background()
+
+	oreq := &model.OrderNew{
+		Status: OrderStatusPaid,
+	}
+
+	ord, err := suite.service.orderRepo.Create(ctx, suite.service.Datastore.RawDB(), oreq)
+	suite.Require().NoError(err)
+
+	uri := "/v1/orders-new/" + ord.ID.String()
+
+	req := httptest.NewRequest(http.MethodDelete, uri, nil)
+
+	rw := httptest.NewRecorder()
+
+	oh := handler.NewOrder(suite.service)
+
+	rtr := chi.NewRouter()
+	subr := chi.NewRouter()
+	subr.Method(
+		http.MethodDelete,
+		"/{orderID}",
+		handlers.AppHandler(oh.Cancel),
+	)
+
+	rtr.Mount("/v1/orders-new", subr)
+
+	srv := &http.Server{Addr: ":8080", Handler: rtr}
+	srv.Handler.ServeHTTP(rw, req)
+
+	suite.Require().Equal(http.StatusOK, rw.Code)
+
+	ord2, err := suite.service.orderRepo.Get(ctx, suite.service.Datastore.RawDB(), ord.ID)
+	suite.Require().NoError(err)
+
+	suite.Equal(model.OrderStatusCanceled, ord2.Status)
+}
+
 // ReadSigningOrderRequestMessage reads messages from the unsigned order request topic
 func (suite *ControllersTestSuite) ReadSigningOrderRequestMessage(ctx context.Context, topic string) SigningOrderRequest {
 	kafkaReader, err := kafkautils.NewKafkaReader(ctx, test.RandomString(), topic)

services/skus/handler/cred.go

@@ -38,7 +38,7 @@ func (h *Cred) CountBatches(w http.ResponseWriter, r *http.Request) *handlers.Ap
 	if err != nil {
 		switch {
 		case errors.Is(err, context.Canceled):
-			return handlers.WrapError(err, "cliend ended request", model.StatusClientClosedConn)
+			return handlers.WrapError(err, "client ended request", model.StatusClientClosedConn)
 
 		case errors.Is(err, model.ErrOrderNotFound), errors.Is(err, model.ErrInvalidOrderNoItems), errors.Is(err, model.ErrOrderItemNotFound):
 			return handlers.WrapError(err, "order not found", http.StatusNotFound)

services/skus/handler/cred_test.go

@@ -82,7 +82,7 @@ func TestCred_CountBatches(t *testing.T) {
 				},
 			},
 			exp: tcExpected{
-				err: handlers.WrapError(context.Canceled, "cliend ended request", model.StatusClientClosedConn),
+				err: handlers.WrapError(context.Canceled, "client ended request", model.StatusClientClosedConn),
 			},
 		},
 

services/skus/handler/handler.go

@@ -8,7 +8,9 @@ import (
 	"net/http"
 
 	"github.com/asaskevich/govalidator"
+	"github.com/go-chi/chi"
 	"github.com/go-playground/validator/v10"
+	uuid "github.com/satori/go.uuid"
 
 	"github.com/brave-intl/bat-go/libs/handlers"
 	"github.com/brave-intl/bat-go/libs/logging"
@@ -24,6 +26,7 @@ const (
 type orderService interface {
 	CreateOrderFromRequest(ctx context.Context, req model.CreateOrderRequest) (*model.Order, error)
 	CreateOrder(ctx context.Context, req *model.CreateOrderRequestNew) (*model.Order, error)
+	CancelOrder(ctx context.Context, id uuid.UUID) error
 }
 
 type Order struct {
@@ -117,6 +120,29 @@ func (h *Order) CreateNew(w http.ResponseWriter, r *http.Request) *handlers.AppE
 	return handlers.RenderContent(ctx, result, w, http.StatusCreated)
 }
 
+func (h *Order) Cancel(w http.ResponseWriter, r *http.Request) *handlers.AppError {
+	ctx := r.Context()
+
+	orderID, err := uuid.FromString(chi.URLParamFromCtx(ctx, "orderID"))
+	if err != nil {
+		return handlers.ValidationError("request", map[string]interface{}{"orderID": model.ErrInvalidUUID})
+	}
+
+	lg := logging.Logger(ctx, "skus").With().Str("func", "CancelOrderNew").Logger()
+
+	if err := h.svc.CancelOrder(ctx, orderID); err != nil {
+		lg.Err(err).Str("order_id", orderID.String()).Msg("failed to cancel order")
+
+		if errors.Is(err, context.Canceled) {
+			return handlers.WrapError(model.ErrSomethingWentWrong, "client ended request", model.StatusClientClosedConn)
+		}
+
+		return handlers.WrapError(model.ErrSomethingWentWrong, "could not cancel order", http.StatusInternalServerError)
+	}
+
+	return handlers.RenderContent(ctx, struct{}{}, w, http.StatusOK)
+}
+
 func collectValidationErrors(err error) (map[string]string, bool) {
 	var verr validator.ValidationErrors
 	if !errors.As(err, &verr) {

services/skus/handler/handler_test.go

@@ -10,7 +10,9 @@ import (
 	"os"
 	"testing"
 
+	"github.com/go-chi/chi"
 	"github.com/rs/zerolog"
+	uuid "github.com/satori/go.uuid"
 	"github.com/shopspring/decimal"
 	should "github.com/stretchr/testify/assert"
 	must "github.com/stretchr/testify/require"
@@ -30,6 +32,7 @@ func TestMain(m *testing.M) {
 type mockOrderService struct {
 	fnCreateOrderFromRequest func(ctx context.Context, req model.CreateOrderRequest) (*model.Order, error)
 	fnCreateOrder            func(ctx context.Context, req *model.CreateOrderRequestNew) (*model.Order, error)
+	fnCancelOrder            func(ctx context.Context, id uuid.UUID) error
 }
 
 func (s *mockOrderService) CreateOrderFromRequest(ctx context.Context, req model.CreateOrderRequest) (*model.Order, error) {
@@ -48,6 +51,14 @@ func (s *mockOrderService) CreateOrder(ctx context.Context, req *model.CreateOrd
 	return s.fnCreateOrder(ctx, req)
 }
 
+func (s *mockOrderService) CancelOrder(ctx context.Context, id uuid.UUID) error {
+	if s.fnCancelOrder == nil {
+		return nil
+	}
+
+	return s.fnCancelOrder(ctx, id)
+}
+
 func TestOrder_Create(t *testing.T) {
 	type tcGiven struct {
 		svc  *mockOrderService
@@ -504,6 +515,129 @@ func TestOrder_CreateNew(t *testing.T) {
 	}
 }
 
+func TestOrder_Cancel(t *testing.T) {
+	type tcGiven struct {
+		ctx context.Context
+		svc *mockOrderService
+		oid uuid.UUID
+	}
+
+	type tcExpected struct {
+		err *handlers.AppError
+	}
+
+	type testCase struct {
+		name  string
+		given tcGiven
+		exp   tcExpected
+	}
+
+	tests := []testCase{
+		{
+			name: "invalid_orderID",
+			given: tcGiven{
+				ctx: context.WithValue(context.Background(), chi.RouteCtxKey, &chi.Context{
+					URLParams: chi.RouteParams{
+						Keys:   []string{"orderID"},
+						Values: []string{"invalid_id"},
+					},
+				}),
+				svc: &mockOrderService{},
+				oid: uuid.Nil,
+			},
+			exp: tcExpected{
+				err: handlers.ValidationError("request", map[string]interface{}{"orderID": model.ErrInvalidUUID}),
+			},
+		},
+
+		{
+			name: "context_cancelled",
+			given: tcGiven{
+				ctx: context.WithValue(context.Background(), chi.RouteCtxKey, &chi.Context{
+					URLParams: chi.RouteParams{
+						Keys:   []string{"orderID"},
+						Values: []string{"facade00-0000-4000-a000-000000000000"},
+					},
+				}),
+				svc: &mockOrderService{
+					fnCancelOrder: func(ctx context.Context, id uuid.UUID) error {
+						return context.Canceled
+					},
+				},
+				oid: uuid.Must(uuid.FromString("facade00-0000-4000-a000-000000000000")),
+			},
+			exp: tcExpected{
+				err: handlers.WrapError(model.ErrSomethingWentWrong, "client ended request", model.StatusClientClosedConn),
+			},
+		},
+
+		{
+			name: "something_went_wrong",
+			given: tcGiven{
+				ctx: context.WithValue(context.Background(), chi.RouteCtxKey, &chi.Context{
+					URLParams: chi.RouteParams{
+						Keys:   []string{"orderID"},
+						Values: []string{"facade00-0000-4000-a000-000000000000"},
+					},
+				}),
+				svc: &mockOrderService{
+					fnCancelOrder: func(ctx context.Context, id uuid.UUID) error {
+						return model.Error("any_error")
+					},
+				},
+				oid: uuid.Must(uuid.FromString("facade00-0000-4000-a000-000000000000")),
+			},
+			exp: tcExpected{
+				err: handlers.WrapError(model.ErrSomethingWentWrong, "could not cancel order", http.StatusInternalServerError),
+			},
+		},
+
+		{
+			name: "success",
+			given: tcGiven{
+				ctx: context.WithValue(context.Background(), chi.RouteCtxKey, &chi.Context{
+					URLParams: chi.RouteParams{
+						Keys:   []string{"orderID"},
+						Values: []string{"facade00-0000-4000-a000-000000000000"},
+					},
+				}),
+				svc: &mockOrderService{},
+				oid: uuid.Must(uuid.FromString("facade00-0000-4000-a000-000000000000")),
+			},
+			exp: tcExpected{},
+		},
+	}
+
+	for i := range tests {
+		tc := tests[i]
+
+		t.Run(tc.name, func(t *testing.T) {
+			h := handler.NewOrder(tc.given.svc)
+
+			uri := "http://localhost/v1/orders-new/" + tc.given.oid.String()
+			req := httptest.NewRequest(http.MethodDelete, uri, nil)
+			req = req.WithContext(tc.given.ctx)
+
+			rw := httptest.NewRecorder()
+			rw.Header().Set("content-type", "application/json")
+
+			actual1 := h.Cancel(rw, req)
+			must.Equal(t, tc.exp.err, actual1)
+
+			if tc.exp.err != nil {
+				actual1.ServeHTTP(rw, req)
+				resp := rw.Body.Bytes()
+
+				exp, err := json.Marshal(tc.exp.err)
+				must.NoError(t, err)
+
+				should.Equal(t, exp, bytes.TrimSpace(resp))
+				return
+			}
+		})
+	}
+}
+
 func mustDecimalFromString(v string) decimal.Decimal {
 	result, err := decimal.NewFromString(v)
 	if err != nil {

services/skus/model/model.go

@@ -28,6 +28,7 @@ const (
 	ErrInvalidOrderNoItems                    Error = "model: invalid order: no items"
 	ErrNoStripeCheckoutSessID                 Error = "model: order: no stripe checkout session id"
 	ErrInvalidOrderMetadataType               Error = "model: order: invalid metadata type"
+	ErrInvalidUUID                            Error = "model: invalid uuid"
 
 	ErrNumPerIntervalNotSet  Error = "model: invalid order: numPerInterval must be set"
 	ErrNumIntervalsNotSet    Error = "model: invalid order: numIntervals must be set"

services/skus/service.go

@@ -676,10 +676,26 @@ func (s *Service) updateOrderStripeSession(ctx context.Context, dbi sqlx.ExtCont
 	return s.renewOrderStripe(ctx, dbi, ord, sub.ID, expt, paidt)
 }
 
-// CancelOrder cancels an order, propagates to stripe if needed.
-//
-// TODO(pavelb): Refactor this.
-func (s *Service) CancelOrder(orderID uuid.UUID) error {
+func (s *Service) CancelOrder(ctx context.Context, id uuid.UUID) error {
+	tx, err := s.Datastore.RawDB().BeginTxx(ctx, nil)
+	if err != nil {
+		return err
+	}
+	defer func() { _ = tx.Rollback() }()
+
+	if err := s.cancelOrderTx(ctx, tx, id); err != nil {
+		return err
+	}
+
+	return tx.Commit()
+}
+
+func (s *Service) cancelOrderTx(ctx context.Context, dbi sqlx.ExecerContext, id uuid.UUID) error {
+	return s.orderRepo.SetStatus(ctx, dbi, id, model.OrderStatusCanceled)
+}
+
+// CancelOrderLegacy cancels an order, propagates to stripe if needed.
+func (s *Service) CancelOrderLegacy(orderID uuid.UUID) error {
 	// TODO: Refactor this later. Now here's a quick fix.
 	ord, err := s.Datastore.GetOrder(orderID)
 	if err != nil {
@@ -1671,7 +1687,7 @@ func (s *Service) processStripeNotificationTx(ctx context.Context, dbi sqlx.ExtC
 			return err
 		}
 
-		return s.orderRepo.SetStatus(ctx, dbi, oid, model.OrderStatusCanceled)
+		return s.cancelOrderTx(ctx, dbi, oid)
 
 	default:
 		return nil
@@ -1718,7 +1734,7 @@ func (s *Service) processAppStoreNotificationTx(ctx context.Context, dbi sqlx.Ex
 		return s.renewOrderWithExpPaidTimeTx(ctx, dbi, ord.ID, expt, paidt)
 
 	case ntf.shouldCancel():
-		return s.orderRepo.SetStatus(ctx, dbi, ord.ID, model.OrderStatusCanceled)
+		return s.cancelOrderTx(ctx, dbi, ord.ID)
 
 	default:
 		return nil
@@ -1769,11 +1785,11 @@ func (s *Service) processPlayStoreNotificationTx(ctx context.Context, dbi sqlx.E
 
 	// Sub cancellation.
 	case ntf.SubscriptionNtf != nil && ntf.SubscriptionNtf.shouldCancel():
-		return s.orderRepo.SetStatus(ctx, dbi, ord.ID, model.OrderStatusCanceled)
+		return s.cancelOrderTx(ctx, dbi, ord.ID)
 
 	// Voiding.
 	case ntf.VoidedPurchaseNtf != nil && ntf.VoidedPurchaseNtf.shouldProcess():
-		return s.orderRepo.SetStatus(ctx, dbi, ord.ID, model.OrderStatusCanceled)
+		return s.cancelOrderTx(ctx, dbi, ord.ID)
 
 	default:
 		return nil
@@ -2339,7 +2355,7 @@ func (s *Service) processRadomNotificationTx(ctx context.Context, dbi sqlx.ExtCo
 			return err
 		}
 
-		return s.orderRepo.SetStatus(ctx, dbi, ord.ID, model.OrderStatusCanceled)
+		return s.cancelOrderTx(ctx, dbi, ord.ID)
 
 	default:
 		return errRadomUnknownAction