Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change transfer_airtime to fail when no exact matching amount #1289

Merged
merged 1 commit into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions flows/actions/testdata/transfer_airtime.json
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@
"uuid": "ad154980-7bf7-4ab8-8728-545fd6378912",
"amounts": {
"RWF": 500,
"USD": 3.5
"USD": 3
},
"result_name": "Reward Transfer"
},
Expand All @@ -201,8 +201,7 @@
"sender": "tel:+17036975131",
"recipient": "tel:+12065551212?channel=57f1078f-88aa-46f4-a59a-948a5739c03d&id=123",
"currency": "USD",
"desired_amount": 3.5,
"actual_amount": 3,
"amount": 3,
"http_logs": [
{
"url": "https://dvs-api.dtone.com/v1/lookup/mobile-number",
Expand Down Expand Up @@ -301,8 +300,7 @@
"sender": "tel:+17036975131",
"recipient": "tel:+12065551212?channel=57f1078f-88aa-46f4-a59a-948a5739c03d&id=123",
"currency": "",
"desired_amount": 0,
"actual_amount": 0,
"amount": 0,
"http_logs": [
{
"url": "https://dvs-api.dtone.com/v1/lookup/mobile-number",
Expand Down
35 changes: 16 additions & 19 deletions flows/events/airtime_transferred.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ const TypeAirtimeTransferred string = "airtime_transferred"
// "sender": "tel:4748",
// "recipient": "tel:+1242563637",
// "currency": "RWF",
// "desired_amount": 120,
// "actual_amount": 100,
// "amount": 100,
// "http_logs": [
// {
// "url": "https://dvs-api.dtone.com/v1/sync/transactions",
Expand All @@ -41,27 +40,25 @@ const TypeAirtimeTransferred string = "airtime_transferred"
type AirtimeTransferredEvent struct {
BaseEvent

TransferUUID flows.AirtimeTransferUUID `json:"transfer_uuid"`
ExternalID string `json:"external_id"`
Sender urns.URN `json:"sender"`
Recipient urns.URN `json:"recipient"`
Currency string `json:"currency"`
DesiredAmount decimal.Decimal `json:"desired_amount"`
ActualAmount decimal.Decimal `json:"actual_amount"`
HTTPLogs []*flows.HTTPLog `json:"http_logs"`
TransferUUID flows.AirtimeTransferUUID `json:"transfer_uuid"`
ExternalID string `json:"external_id"`
Sender urns.URN `json:"sender"`
Recipient urns.URN `json:"recipient"`
Currency string `json:"currency"`
Amount decimal.Decimal `json:"amount"`
HTTPLogs []*flows.HTTPLog `json:"http_logs"`
}

// NewAirtimeTransferred creates a new airtime transferred event
func NewAirtimeTransferred(t *flows.AirtimeTransfer, httpLogs []*flows.HTTPLog) *AirtimeTransferredEvent {
return &AirtimeTransferredEvent{
BaseEvent: NewBaseEvent(TypeAirtimeTransferred),
TransferUUID: t.UUID,
ExternalID: t.ExternalID,
Sender: t.Sender,
Recipient: t.Recipient,
Currency: t.Currency,
DesiredAmount: t.DesiredAmount,
ActualAmount: t.ActualAmount,
HTTPLogs: httpLogs,
BaseEvent: NewBaseEvent(TypeAirtimeTransferred),
TransferUUID: t.UUID,
ExternalID: t.ExternalID,
Sender: t.Sender,
Recipient: t.Recipient,
Currency: t.Currency,
Amount: t.Amount,
HTTPLogs: httpLogs,
}
}
16 changes: 7 additions & 9 deletions flows/events/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,12 @@ func TestEventMarshaling(t *testing.T) {
{
events.NewAirtimeTransferred(
&flows.AirtimeTransfer{
UUID: "4c2d9b7a-e02c-4e6a-ab18-06df4cb5666d",
ExternalID: "98765432",
Sender: urns.URN("tel:+593979099111"),
Recipient: urns.URN("tel:+593979099222"),
Currency: "USD",
DesiredAmount: decimal.RequireFromString("1.20"),
ActualAmount: decimal.RequireFromString("1.00"),
UUID: "4c2d9b7a-e02c-4e6a-ab18-06df4cb5666d",
ExternalID: "98765432",
Sender: urns.URN("tel:+593979099111"),
Recipient: urns.URN("tel:+593979099222"),
Currency: "USD",
Amount: decimal.RequireFromString("1.00"),
},
[]*flows.HTTPLog{
{
Expand All @@ -82,10 +81,9 @@ func TestEventMarshaling(t *testing.T) {
},
),
`{
"actual_amount": 1,
"amount": 1,
"created_on": "2018-10-18T14:20:30.000123456Z",
"currency": "USD",
"desired_amount": 1.2,
"external_id": "98765432",
"http_logs": [
{
Expand Down
13 changes: 6 additions & 7 deletions flows/services.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,12 @@ const (

// AirtimeTransfer is the result of an attempted airtime transfer
type AirtimeTransfer struct {
UUID AirtimeTransferUUID
ExternalID string
Sender urns.URN
Recipient urns.URN
Currency string
DesiredAmount decimal.Decimal
ActualAmount decimal.Decimal
UUID AirtimeTransferUUID
ExternalID string
Sender urns.URN
Recipient urns.URN
Currency string
Amount decimal.Decimal
}

// AirtimeService provides airtime functionality to the engine
Expand Down
40 changes: 14 additions & 26 deletions services/airtime/dtone/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,11 @@ func NewService(httpClient *http.Client, httpRetries *httpx.RetryConfig, key, se

func (s *service) Transfer(sender urns.URN, recipient urns.URN, amounts map[string]decimal.Decimal, logHTTP flows.HTTPLogCallback) (*flows.AirtimeTransfer, error) {
transfer := &flows.AirtimeTransfer{
UUID: flows.AirtimeTransferUUID(uuids.NewV4()),
Sender: sender,
Recipient: recipient,
Currency: "",
DesiredAmount: decimal.Zero,
ActualAmount: decimal.Zero,
UUID: flows.AirtimeTransferUUID(uuids.NewV4()),
Sender: sender,
Recipient: recipient,
Currency: "",
Amount: decimal.Zero,
}
recipientPhoneNumber := recipient.Path()
if !strings.HasPrefix(recipientPhoneNumber, "+") {
Expand Down Expand Up @@ -70,34 +69,24 @@ func (s *service) Transfer(sender urns.URN, recipient urns.URN, amounts map[stri
return transfer, fmt.Errorf("product fetch failed: %w", err)
}

// closest product for each currency we have a desired amount for
closestProducts := make(map[string]*Product, len(amounts))

// find a matching product in any currency we have a desired amount for
var product *Product
for currency, desiredAmount := range amounts {
for _, product := range products {
if product.Destination.Unit == currency {
closest := closestProducts[currency]
prodAmount := product.Destination.Amount

if (closest == nil || prodAmount.GreaterThan(closest.Destination.Amount)) && prodAmount.LessThanOrEqual(desiredAmount) {
closestProducts[currency] = product
for _, p := range products {
if p.Destination.Unit == currency {
if p.Destination.Amount.Equal(desiredAmount) {
product = p
break
}
}
}
}
if len(closestProducts) == 0 {
if product == nil {
return transfer, fmt.Errorf("unable to find a suitable product for operator '%s'", operator.Name)
}

// it's possible we have more than one supported currency/product.. use any
var product *Product
for i := range closestProducts {
product = closestProducts[i]
break
}

transfer.Currency = product.Destination.Unit
transfer.DesiredAmount = amounts[transfer.Currency]
transfer.Amount = product.Destination.Amount

// request asynchronous confirmed transaction for this product
tx, trace, err := s.client.TransactionAsync(string(transfer.UUID), product.ID, recipientPhoneNumber)
Expand All @@ -113,7 +102,6 @@ func (s *service) Transfer(sender urns.URN, recipient urns.URN, amounts map[stri
}

transfer.ExternalID = fmt.Sprintf("%d", tx.ID)
transfer.ActualAmount = product.Destination.Amount

return transfer, nil
}
27 changes: 16 additions & 11 deletions services/airtime/dtone/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,19 @@ func TestServiceWithSuccessfulTranfer(t *testing.T) {
urns.URN("tel:+593979000000"),
urns.URN("tel:+593979123456"),
map[string]decimal.Decimal{
"USD": decimal.RequireFromString("3.5"),
"USD": decimal.RequireFromString("3"),
"RWF": decimal.RequireFromString("5000"),
},
httpLogger.Log,
)
assert.NoError(t, err)
assert.Equal(t, &flows.AirtimeTransfer{
UUID: "1ae96956-4b34-433e-8d1a-f05fe6923d6d",
ExternalID: "2237512891",
Sender: urns.URN("tel:+593979000000"),
Recipient: urns.URN("tel:+593979123456"),
Currency: "USD",
DesiredAmount: decimal.RequireFromString("3.5"),
ActualAmount: decimal.RequireFromString("3"), // closest product
UUID: "1ae96956-4b34-433e-8d1a-f05fe6923d6d",
ExternalID: "2237512891",
Sender: urns.URN("tel:+593979000000"),
Recipient: urns.URN("tel:+593979123456"),
Currency: "USD",
Amount: decimal.RequireFromString("3"),
}, transfer)

assert.Equal(t, 3, len(httpLogger.Logs))
Expand All @@ -85,12 +84,14 @@ func TestServiceFailedTransfers(t *testing.T) {
httpx.NewMockResponse(200, nil, []byte(lookupNumberResponse)),
httpx.NewMockResponse(200, nil, []byte(lookupNumberResponse)),
httpx.NewMockResponse(200, nil, []byte(lookupNumberResponse)),
httpx.NewMockResponse(200, nil, []byte(lookupNumberResponse)),
},
"https://dvs-api.dtone.com/v1/products?type=FIXED_VALUE_RECHARGE&operator_id=1596&per_page=100": {
httpx.NewMockResponse(400, nil, errorResp(1003001, "Product is not available in your account")),
httpx.NewMockResponse(200, nil, []byte(`[]`)), // no products
httpx.NewMockResponse(200, nil, []byte(productsResponse)),
httpx.NewMockResponse(200, nil, []byte(productsResponse)),
httpx.NewMockResponse(200, nil, []byte(productsResponse)),
},
"https://dvs-api.dtone.com/v1/async/transactions": {
httpx.NewMockResponse(400, nil, errorResp(1003001, "Something went wrong")),
Expand All @@ -106,15 +107,14 @@ func TestServiceFailedTransfers(t *testing.T) {
svc := dtone.NewService(http.DefaultClient, nil, "key123", "sesame")

httpLogger := &flows.HTTPLogger{}
amounts := map[string]decimal.Decimal{"USD": decimal.RequireFromString("3.5")}
amounts := map[string]decimal.Decimal{"USD": decimal.RequireFromString("3")}

// try when phone number lookup gives a connection error
transfer, err := svc.Transfer(urns.URN("tel:+593979000000"), urns.URN("tel:+593979123456"), amounts, httpLogger.Log)
assert.EqualError(t, err, "number lookup failed: unable to connect to server")
assert.Equal(t, urns.URN("tel:+593979000000"), transfer.Sender)
assert.Equal(t, urns.URN("tel:+593979123456"), transfer.Recipient)
assert.Equal(t, decimal.Zero, transfer.DesiredAmount)
assert.Equal(t, decimal.Zero, transfer.ActualAmount)
assert.Equal(t, decimal.Zero, transfer.Amount)

// try when phone number lookup fails
transfer, err = svc.Transfer(urns.URN("tel:+593979000000"), urns.URN("tel:+593979123456"), amounts, httpLogger.Log)
Expand All @@ -136,6 +136,11 @@ func TestServiceFailedTransfers(t *testing.T) {
assert.EqualError(t, err, "unable to find a suitable product for operator 'Claro Ecuador'")
assert.NotNil(t, transfer)

// try when we can't find any suitable products (there are products but none match the amount)
transfer, err = svc.Transfer(urns.URN("tel:+593979000000"), urns.URN("tel:+593979123456"), map[string]decimal.Decimal{"USD": decimal.RequireFromString("2")}, httpLogger.Log)
assert.EqualError(t, err, "unable to find a suitable product for operator 'Claro Ecuador'")
assert.NotNil(t, transfer)

// try when transaction request errors
transfer, err = svc.Transfer(urns.URN("tel:+593979000000"), urns.URN("tel:+593979123456"), amounts, httpLogger.Log)
assert.EqualError(t, err, "transaction creation failed: Something went wrong")
Expand Down
11 changes: 5 additions & 6 deletions test/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,11 @@ func (s *airtimeService) Transfer(sender urns.URN, recipient urns.URN, amounts m
}

transfer := &flows.AirtimeTransfer{
UUID: flows.AirtimeTransferUUID(uuids.NewV4()),
Sender: sender,
Recipient: recipient,
Currency: s.fixedCurrency,
DesiredAmount: amount,
ActualAmount: amount,
UUID: flows.AirtimeTransferUUID(uuids.NewV4()),
Sender: sender,
Recipient: recipient,
Currency: s.fixedCurrency,
Amount: amount,
}

return transfer, nil
Expand Down
2 changes: 1 addition & 1 deletion test/testdata/runner/airtime.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
"uuid": "8720f157-ca1c-432f-9c0b-2014ddc77094",
"amounts": {
"RWF": 5000,
"USD": 3.5
"USD": 3
},
"result_name": "Transfer"
}
Expand Down
6 changes: 2 additions & 4 deletions test/testdata/runner/airtime.test_successful_transfer.json
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,9 @@
{
"events": [
{
"actual_amount": 3,
"amount": 3,
"created_on": "2018-07-06T12:30:08.123456789Z",
"currency": "USD",
"desired_amount": 3.5,
"external_id": "2237512891",
"http_logs": [
{
Expand Down Expand Up @@ -214,10 +213,9 @@
"created_on": "2018-07-06T12:30:00.123456789Z",
"events": [
{
"actual_amount": 3,
"amount": 3,
"created_on": "2018-07-06T12:30:08.123456789Z",
"currency": "USD",
"desired_amount": 3.5,
"external_id": "2237512891",
"http_logs": [
{
Expand Down
Loading