feat(payments): reverse refunds issued outside the console
CI / changes (pull_request) Successful in 11s
CI / unit (pull_request) Successful in 22s
CI / integration (pull_request) Successful in 29s
CI / ui (pull_request) Successful in 1m27s
CI / conformance (pull_request) Successful in 19s
CI / gate (pull_request) Successful in 0s
CI / deploy (pull_request) Successful in 1m52s
CI / changes (pull_request) Successful in 11s
CI / unit (pull_request) Successful in 22s
CI / integration (pull_request) Successful in 29s
CI / ui (pull_request) Successful in 1m27s
CI / conformance (pull_request) Successful in 19s
CI / gate (pull_request) Successful in 0s
CI / deploy (pull_request) Successful in 1m52s
Two holes on the refund path, both found by asking what happens when a refund does not come from our own `/_gm` button. The merchant cabinet is a second entry point. An operator can refund there, and such a refund never passes through our API — so the money went back while the chips stayed credited, silently. Handle `refund.succeeded`: the refund is re-read from the API (the notification body is no more evidence here than it is for a payment), bound back to its order through the payment id recorded when the payment was minted, and reversed through the same engine. It is idempotent on (provider, refund id), so the event for a refund the console already recorded reverses nothing twice. The reversal engine is full-refund-only by design — it revokes exactly what the pack funded and rejects any other amount — so a partial refund is recorded as nothing at all and logged loudly for an operator. There is no non-arbitrary way to decide how many chips a part-refund costs, and guessing would be worse than asking a human. Second hole: a refund can still be canceled while pending, and the ledger is append-only. Recording on any non-empty refund id therefore risked revoking a customer's chips for money that stayed with us, with no way to take the row back. The console now records only a `succeeded` refund and tells the operator to press again otherwise — the idempotency key returns the same refund rather than paying twice. Tests: unit (GetRefund, the refund notification envelope, a non-final status surfaced to the caller); integration (a cabinet refund is reversed once and a redelivery is a no-op, the event after a console refund changes nothing, a partial refund records nothing, an unconfirmed refund reverses nothing, a pending refund records nothing until it settles and then does). The suite shares one database and the ledger dedupes refunds globally, so the fake provider now mints a refund id per payment — a constant id made one test's refund look like another's duplicate. Decisions D50 (amended) and D52; the notification subscription list in the deploy docs gains refund.succeeded.
This commit is contained in:
@@ -37,8 +37,12 @@ type fakeYooKassa struct {
|
||||
payments map[string]yookassa.Payment
|
||||
// refundFails makes POST /refunds answer 500, standing in for a provider that did not move money.
|
||||
refundFails bool
|
||||
// refundStatus is the status a created refund reports; empty means succeeded.
|
||||
refundStatus string
|
||||
// refundCalls counts the refunds actually requested.
|
||||
refundCalls int
|
||||
// refunds answers GET /refunds/{id}; a missing id is a 404.
|
||||
refunds map[string]yookassa.Refund
|
||||
// createdIdempotenceKey records the key sent on the last create-payment call.
|
||||
createdIdempotenceKey string
|
||||
// lastReceipt records the receipt object of the last create-payment call.
|
||||
@@ -48,7 +52,7 @@ type fakeYooKassa struct {
|
||||
// newFakeYooKassa starts the fake API and returns it with a shop config pointed at it.
|
||||
func newFakeYooKassa(t *testing.T) (*fakeYooKassa, yookassa.Config) {
|
||||
t.Helper()
|
||||
f := &fakeYooKassa{payments: map[string]yookassa.Payment{}}
|
||||
f := &fakeYooKassa{payments: map[string]yookassa.Payment{}, refunds: map[string]yookassa.Refund{}}
|
||||
srv := httptest.NewServer(http.HandlerFunc(f.serve))
|
||||
t.Cleanup(srv.Close)
|
||||
return f, yookassa.Config{ShopID: ykShopID, SecretKey: ykSecretKey, IsTest: true, BaseURL: srv.URL}
|
||||
@@ -94,14 +98,37 @@ func (f *fakeYooKassa) serve(w http.ResponseWriter, r *http.Request) {
|
||||
f.refundCalls++
|
||||
var body yookassa.RefundRequest
|
||||
_ = json.NewDecoder(r.Body).Decode(&body)
|
||||
_ = json.NewEncoder(w).Encode(yookassa.Refund{
|
||||
ID: "refund-1", Status: yookassa.StatusSucceeded, PaymentID: body.PaymentID, Amount: body.Amount,
|
||||
})
|
||||
status := f.refundStatus
|
||||
if status == "" {
|
||||
status = yookassa.StatusSucceeded
|
||||
}
|
||||
// The ledger dedupes refunds on (provider, refund id) across the whole database, so the id has
|
||||
// to be unique per payment or one test's refund looks like another's duplicate.
|
||||
refund := yookassa.Refund{ID: "refund-" + body.PaymentID, Status: status, PaymentID: body.PaymentID, Amount: body.Amount}
|
||||
f.refunds[refund.ID] = refund
|
||||
_ = json.NewEncoder(w).Encode(refund)
|
||||
case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/refunds/"):
|
||||
refund, ok := f.refunds[strings.TrimPrefix(r.URL.Path, "/refunds/")]
|
||||
if !ok {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
_, _ = w.Write([]byte(`{"type":"error","description":"not found"}`))
|
||||
return
|
||||
}
|
||||
_ = json.NewEncoder(w).Encode(refund)
|
||||
default:
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
}
|
||||
|
||||
// setRefund publishes a refund the API will report — how a test stands up a refund that was issued
|
||||
// somewhere other than through our own API, such as in the merchant cabinet.
|
||||
func (f *fakeYooKassa) setRefund(id string, refund yookassa.Refund) {
|
||||
f.mu.Lock()
|
||||
defer f.mu.Unlock()
|
||||
refund.ID = id
|
||||
f.refunds[id] = refund
|
||||
}
|
||||
|
||||
// setPayment overwrites what the API reports for a payment — how a test says "the money did (or did
|
||||
// not) really move", independently of what a notification claims.
|
||||
func (f *fakeYooKassa) setPayment(id string, mutate func(p *yookassa.Payment)) {
|
||||
@@ -383,8 +410,8 @@ func TestYooKassaConsoleRefundMovesMoneyThenRecords(t *testing.T) {
|
||||
acc).Scan(&provider, &refundID); err != nil {
|
||||
t.Fatalf("read refund ledger row: %v", err)
|
||||
}
|
||||
if provider != "yookassa" || refundID != "refund-1" {
|
||||
t.Errorf("refund ledger row = (%s, %s), want (yookassa, refund-1)", provider, refundID)
|
||||
if provider != "yookassa" || refundID != "refund-"+paymentID {
|
||||
t.Errorf("refund ledger row = (%s, %s), want (yookassa, refund-%s)", provider, refundID, paymentID)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -502,3 +529,182 @@ func TestYooKassaOrderRequiresAnEmailAnchor(t *testing.T) {
|
||||
t.Fatalf("order = %d (%s), want 403 email_required", rec.Code, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
// postYooKassaRefundNotify posts a refund notification to the intake and reports the status code.
|
||||
func postYooKassaRefundNotify(t *testing.T, srv *server.Server, refundID, paymentID string) int {
|
||||
t.Helper()
|
||||
body := fmt.Sprintf(`{"type":"notification","event":%q,"object":{"id":%q,"status":"succeeded","payment_id":%q}}`,
|
||||
yookassa.EventRefundSucceeded, refundID, paymentID)
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/v1/internal/payments/yookassa/notify", strings.NewReader(body))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
req.Header.Set("X-Forwarded-For", ykSenderIP)
|
||||
rec := httptest.NewRecorder()
|
||||
srv.Handler().ServeHTTP(rec, req)
|
||||
return rec.Code
|
||||
}
|
||||
|
||||
// fundedYooKassaOrder seeds an order and credits it, leaving an account with chips to reverse.
|
||||
func fundedYooKassaOrder(t *testing.T, f *fakeYooKassa, srv *server.Server, pay *payments.Service, acc uuid.UUID) (uuid.UUID, string) {
|
||||
t.Helper()
|
||||
orderID, paymentID := seedYooKassaOrder(t, f, pay, acc)
|
||||
if code := postYooKassaNotify(t, srv, ykSenderIP, yookassa.EventPaymentSucceeded, paymentID, orderID.String()); code != http.StatusOK {
|
||||
t.Fatalf("notify = %d, want 200", code)
|
||||
}
|
||||
return orderID, paymentID
|
||||
}
|
||||
|
||||
// TestYooKassaCabinetRefundIsReversed is why the refund event is handled at all: an operator can
|
||||
// refund in the merchant cabinet, which never passes through our API, so without this the money would
|
||||
// go back while the chips stayed credited.
|
||||
func TestYooKassaCabinetRefundIsReversed(t *testing.T) {
|
||||
f, shop := newFakeYooKassa(t)
|
||||
srv, pay := yookassaServer(t, shop)
|
||||
acc := provisionAccount(t)
|
||||
_, paymentID := fundedYooKassaOrder(t, f, srv, pay, acc)
|
||||
if got := readBalance(t, acc, "direct"); got != 100 {
|
||||
t.Fatalf("balance before the refund = %d, want 100", got)
|
||||
}
|
||||
|
||||
// The refund exists at the provider, issued outside our API.
|
||||
f.setRefund("cabinet-refund-1", yookassa.Refund{
|
||||
Status: yookassa.StatusSucceeded, PaymentID: paymentID,
|
||||
Amount: yookassa.Amount{Value: "149.00", Currency: "RUB"},
|
||||
})
|
||||
if code := postYooKassaRefundNotify(t, srv, "cabinet-refund-1", paymentID); code != http.StatusOK {
|
||||
t.Fatalf("refund notify = %d, want 200", code)
|
||||
}
|
||||
if got := readBalance(t, acc, "direct"); got != 0 {
|
||||
t.Errorf("balance = %d, want 0 — a cabinet refund did not revoke the chips", got)
|
||||
}
|
||||
if f.refundCalls != 0 {
|
||||
t.Errorf("provider refunds requested = %d, want 0 — the money had already moved", f.refundCalls)
|
||||
}
|
||||
var provider, refundID string
|
||||
if err := testDB.QueryRowContext(context.Background(),
|
||||
`SELECT provider, provider_payment_id FROM payments.ledger WHERE account_id=$1 AND kind='refund'`,
|
||||
acc).Scan(&provider, &refundID); err != nil {
|
||||
t.Fatalf("read refund ledger row: %v", err)
|
||||
}
|
||||
if provider != "yookassa" || refundID != "cabinet-refund-1" {
|
||||
t.Errorf("refund ledger row = (%s, %s), want (yookassa, cabinet-refund-1)", provider, refundID)
|
||||
}
|
||||
|
||||
// A redelivery reverses nothing a second time.
|
||||
if code := postYooKassaRefundNotify(t, srv, "cabinet-refund-1", paymentID); code != http.StatusOK {
|
||||
t.Fatalf("redelivered refund notify = %d, want 200", code)
|
||||
}
|
||||
if ledgerRows(t, acc, "refund") != 1 {
|
||||
t.Errorf("refund ledger rows = %d, want 1", ledgerRows(t, acc, "refund"))
|
||||
}
|
||||
}
|
||||
|
||||
// TestYooKassaRefundNotifyAfterConsoleRefundIsANoOp checks the two refund entry points cannot double
|
||||
// up: the event for a refund the console already recorded reverses nothing again.
|
||||
func TestYooKassaRefundNotifyAfterConsoleRefundIsANoOp(t *testing.T) {
|
||||
f, shop := newFakeYooKassa(t)
|
||||
srv, pay := yookassaServer(t, shop)
|
||||
acc := provisionAccount(t)
|
||||
orderID, paymentID := fundedYooKassaOrder(t, f, srv, pay, acc)
|
||||
|
||||
base := "http://admin.test/_gm/users/" + acc.String()
|
||||
if code, _ := consoleDo(srv.Handler(), http.MethodPost, base+"/refund", "order_id="+orderID.String(), "http://admin.test"); code != http.StatusOK {
|
||||
t.Fatalf("console refund = %d, want 200", code)
|
||||
}
|
||||
if got := readBalance(t, acc, "direct"); got != 0 {
|
||||
t.Fatalf("balance after the console refund = %d, want 0", got)
|
||||
}
|
||||
// YooKassa then notifies about that same refund.
|
||||
if code := postYooKassaRefundNotify(t, srv, "refund-"+paymentID, paymentID); code != http.StatusOK {
|
||||
t.Fatalf("refund notify = %d, want 200", code)
|
||||
}
|
||||
if ledgerRows(t, acc, "refund") != 1 {
|
||||
t.Errorf("refund ledger rows = %d, want 1 (reversed once)", ledgerRows(t, acc, "refund"))
|
||||
}
|
||||
if abuse, loss := readRisk(t, acc); abuse || loss != 0 {
|
||||
t.Errorf("risk = (abuse %v, loss %d), want none — the second reversal ran anyway", abuse, loss)
|
||||
}
|
||||
}
|
||||
|
||||
// TestYooKassaPartialRefundIsLeftToAnOperator: the reversal engine revokes exactly what the pack
|
||||
// funded and rejects any other amount, so a partial refund cannot be recorded without guessing how
|
||||
// many chips it costs. It must change nothing rather than guess.
|
||||
func TestYooKassaPartialRefundIsLeftToAnOperator(t *testing.T) {
|
||||
f, shop := newFakeYooKassa(t)
|
||||
srv, pay := yookassaServer(t, shop)
|
||||
acc := provisionAccount(t)
|
||||
_, paymentID := fundedYooKassaOrder(t, f, srv, pay, acc)
|
||||
|
||||
f.setRefund("partial-1", yookassa.Refund{
|
||||
Status: yookassa.StatusSucceeded, PaymentID: paymentID,
|
||||
Amount: yookassa.Amount{Value: "50.00", Currency: "RUB"}, // part of the 149.00 order
|
||||
})
|
||||
if code := postYooKassaRefundNotify(t, srv, "partial-1", paymentID); code != http.StatusOK {
|
||||
t.Fatalf("refund notify = %d, want 200", code)
|
||||
}
|
||||
if got := readBalance(t, acc, "direct"); got != 100 {
|
||||
t.Errorf("balance = %d, want 100 — a partial refund revoked chips", got)
|
||||
}
|
||||
if ledgerRows(t, acc, "refund") != 0 {
|
||||
t.Error("a partial refund wrote a refund ledger row")
|
||||
}
|
||||
}
|
||||
|
||||
// TestYooKassaRefundNotifyIsNotEvidence: as with a payment, the body only names a refund. One the
|
||||
// provider does not confirm reverses nothing.
|
||||
func TestYooKassaRefundNotifyIsNotEvidence(t *testing.T) {
|
||||
f, shop := newFakeYooKassa(t)
|
||||
srv, pay := yookassaServer(t, shop)
|
||||
acc := provisionAccount(t)
|
||||
_, paymentID := fundedYooKassaOrder(t, f, srv, pay, acc)
|
||||
|
||||
// A refund the provider has never heard of.
|
||||
if code := postYooKassaRefundNotify(t, srv, "invented-refund", paymentID); code != http.StatusOK {
|
||||
t.Fatalf("refund notify = %d, want 200", code)
|
||||
}
|
||||
// One that exists but is not final.
|
||||
f.setRefund("pending-refund", yookassa.Refund{
|
||||
Status: yookassa.StatusPending, PaymentID: paymentID,
|
||||
Amount: yookassa.Amount{Value: "149.00", Currency: "RUB"},
|
||||
})
|
||||
if code := postYooKassaRefundNotify(t, srv, "pending-refund", paymentID); code != http.StatusOK {
|
||||
t.Fatalf("refund notify = %d, want 200", code)
|
||||
}
|
||||
if got := readBalance(t, acc, "direct"); got != 100 {
|
||||
t.Errorf("balance = %d, want 100 — an unconfirmed refund revoked chips", got)
|
||||
}
|
||||
if ledgerRows(t, acc, "refund") != 0 {
|
||||
t.Error("an unconfirmed refund wrote a refund ledger row")
|
||||
}
|
||||
}
|
||||
|
||||
// TestYooKassaPendingRefundRecordsNothing: a refund can still be canceled while pending and the
|
||||
// ledger is append-only, so the console records only a completed one. Retrying is the way out.
|
||||
func TestYooKassaPendingRefundRecordsNothing(t *testing.T) {
|
||||
f, shop := newFakeYooKassa(t)
|
||||
srv, pay := yookassaServer(t, shop)
|
||||
acc := provisionAccount(t)
|
||||
orderID, _ := fundedYooKassaOrder(t, f, srv, pay, acc)
|
||||
|
||||
f.refundStatus = yookassa.StatusPending
|
||||
base := "http://admin.test/_gm/users/" + acc.String()
|
||||
_, body := consoleDo(srv.Handler(), http.MethodPost, base+"/refund", "order_id="+orderID.String(), "http://admin.test")
|
||||
if !strings.Contains(body, "not completed the refund yet") || !strings.Contains(body, "nothing was recorded") {
|
||||
t.Errorf("message = %q, want it to say the refund is not final and nothing was recorded", body)
|
||||
}
|
||||
if got := readBalance(t, acc, "direct"); got != 100 {
|
||||
t.Errorf("balance = %d, want 100 — chips were revoked for an unsettled refund", got)
|
||||
}
|
||||
if ledgerRows(t, acc, "refund") != 0 {
|
||||
t.Error("a pending refund wrote a refund ledger row")
|
||||
}
|
||||
|
||||
// Once the provider settles it, pressing again records the reversal — the idempotency key returns
|
||||
// the same refund rather than paying twice.
|
||||
f.refundStatus = yookassa.StatusSucceeded
|
||||
if code, body := consoleDo(srv.Handler(), http.MethodPost, base+"/refund", "order_id="+orderID.String(), "http://admin.test"); code != http.StatusOK || !strings.Contains(body, "revoked 100 chips") {
|
||||
t.Fatalf("retried refund = %d, body has 'revoked 100 chips' = %v", code, strings.Contains(body, "revoked 100 chips"))
|
||||
}
|
||||
if got := readBalance(t, acc, "direct"); got != 0 {
|
||||
t.Errorf("balance = %d, want 0", got)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user