Skip to content

Commit

Permalink
[SDP-1380] Allow disbursement.verification_field to be empty (#456)
Browse files Browse the repository at this point in the history
### What

Allow disbursement.verification_field to be empty

### Why

Address https://stellarorg.atlassian.net/browse/SDP-1380
  • Loading branch information
marcelosalloum authored Nov 4, 2024
1 parent e824a74 commit bc1384f
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
-- This migration drops the verification_field NOT NULL constraint from the disbursements table.

-- +migrate Up
ALTER TABLE disbursements
ALTER COLUMN verification_field DROP NOT NULL;

-- +migrate Down
13 changes: 7 additions & 6 deletions internal/data/disbursements.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/stellar/go/support/log"

"github.com/stellar/stellar-disbursement-platform-backend/db"
"github.com/stellar/stellar-disbursement-platform-backend/internal/utils"
)

type Disbursement struct {
Expand Down Expand Up @@ -75,15 +76,15 @@ func (d *DisbursementModel) Insert(ctx context.Context, disbursement *Disburseme
VALUES
($1, $2, $3, $4, $5, $6, $7, $8)
RETURNING id
`
var newId string
err := d.dbConnectionPool.GetContext(ctx, &newId, q,
`
var newID string
err := d.dbConnectionPool.GetContext(ctx, &newID, q,
disbursement.Name,
disbursement.Status,
disbursement.StatusHistory,
disbursement.Wallet.ID,
disbursement.Asset.ID,
disbursement.VerificationField,
utils.SQLNullString(string(disbursement.VerificationField)),
disbursement.ReceiverRegistrationMessageTemplate,
disbursement.RegistrationContactType,
)
Expand All @@ -95,7 +96,7 @@ func (d *DisbursementModel) Insert(ctx context.Context, disbursement *Disburseme
return "", fmt.Errorf("unable to create disbursement %s: %w", disbursement.Name, err)
}

return newId, nil
return newID, nil
}

func (d *DisbursementModel) GetWithStatistics(ctx context.Context, id string) (*Disbursement, error) {
Expand All @@ -118,7 +119,7 @@ const selectDisbursementQuery = `
d.name,
d.status,
d.status_history,
d.verification_field,
COALESCE(d.verification_field::text, '') as verification_field,
COALESCE(d.file_name, '') as file_name,
d.file_content,
d.created_at,
Expand Down
39 changes: 33 additions & 6 deletions internal/data/disbursements_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
func Test_DisbursementModelInsert(t *testing.T) {
dbt := dbtest.Open(t)
defer dbt.Close()

dbConnectionPool, err := db.OpenDBConnectionPool(dbt.DSN)
require.NoError(t, err)
defer dbConnectionPool.Close()
Expand All @@ -31,7 +30,7 @@ func Test_DisbursementModelInsert(t *testing.T) {
smsTemplate := "You have a new payment waiting for you from org x. Click on the link to register."

disbursement := Disbursement{
Name: "disbursement1",
Name: "disbursement",
Status: DraftDisbursementStatus,
StatusHistory: []DisbursementStatusHistoryEntry{
{
Expand All @@ -46,24 +45,27 @@ func Test_DisbursementModelInsert(t *testing.T) {
RegistrationContactType: RegistrationContactTypePhone,
}

t.Run("returns error when disbursement already exists", func(t *testing.T) {
t.Run("🔴 fails to insert disbursements with non-unique name", func(t *testing.T) {
defer DeleteAllDisbursementFixtures(t, ctx, dbConnectionPool)

_, err := disbursementModel.Insert(ctx, &disbursement)
require.NoError(t, err)
_, err = disbursementModel.Insert(ctx, &disbursement)
require.Error(t, err)
require.Equal(t, ErrRecordAlreadyExists, err)
})

t.Run("insert disbursement successfully", func(t *testing.T) {
disbursement.Name = "disbursement2"
t.Run("🟢 successfully insert disbursement", func(t *testing.T) {
defer DeleteAllDisbursementFixtures(t, ctx, dbConnectionPool)

id, err := disbursementModel.Insert(ctx, &disbursement)
require.NoError(t, err)
require.NotNil(t, id)

actual, err := disbursementModel.Get(ctx, dbConnectionPool, id)
require.NoError(t, err)

assert.Equal(t, "disbursement2", actual.Name)
assert.Equal(t, "disbursement", actual.Name)
assert.Equal(t, DraftDisbursementStatus, actual.Status)
assert.Equal(t, asset, actual.Asset)
assert.Equal(t, wallet, actual.Wallet)
Expand All @@ -73,6 +75,31 @@ func Test_DisbursementModelInsert(t *testing.T) {
assert.Equal(t, "user1", actual.StatusHistory[0].UserID)
assert.Equal(t, VerificationTypeDateOfBirth, actual.VerificationField)
})

t.Run("🟢 successfully insert disbursement (empty:[VerificationField,ReceiverRegistrationMessageTemplate])", func(t *testing.T) {
defer DeleteAllDisbursementFixtures(t, ctx, dbConnectionPool)

d := disbursement
d.ReceiverRegistrationMessageTemplate = ""
d.VerificationField = ""

id, err := disbursementModel.Insert(ctx, &d)
require.NoError(t, err)
require.NotNil(t, id)

actual, err := disbursementModel.Get(ctx, dbConnectionPool, id)
require.NoError(t, err)

assert.Equal(t, "disbursement", actual.Name)
assert.Equal(t, DraftDisbursementStatus, actual.Status)
assert.Equal(t, asset, actual.Asset)
assert.Equal(t, wallet, actual.Wallet)
assert.Empty(t, actual.ReceiverRegistrationMessageTemplate)
assert.Equal(t, 1, len(actual.StatusHistory))
assert.Equal(t, DraftDisbursementStatus, actual.StatusHistory[0].Status)
assert.Equal(t, "user1", actual.StatusHistory[0].UserID)
assert.Empty(t, actual.VerificationField)
})
}

func Test_DisbursementModelCount(t *testing.T) {
Expand Down
12 changes: 7 additions & 5 deletions internal/serve/httphandler/disbursement_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ func (d DisbursementHandler) validateRequest(req PostDisbursementRequest) *valid
"registration_contact_type",
fmt.Sprintf("registration_contact_type must be one of %v", data.AllRegistrationContactTypes()),
)
v.Check(
slices.Contains(data.GetAllVerificationTypes(), req.VerificationField),
"verification_field",
fmt.Sprintf("verification_field must be one of %v", data.GetAllVerificationTypes()),
)
if !req.RegistrationContactType.IncludesWalletAddress {
v.Check(
slices.Contains(data.GetAllVerificationTypes(), req.VerificationField),
"verification_field",
fmt.Sprintf("verification_field must be one of %v", data.GetAllVerificationTypes()),
)
}

return v
}
Expand Down
32 changes: 30 additions & 2 deletions internal/serve/httphandler/disbursement_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,13 @@ import (
)

func Test_DisbursementHandler_validateRequest(t *testing.T) {
testCases := []struct {
type TestCase struct {
name string
request PostDisbursementRequest
expectedErrors map[string]interface{}
}{
}

testCases := []TestCase{
{
name: "🔴 all fields are empty",
request: PostDisbursementRequest{},
Expand Down Expand Up @@ -82,6 +84,32 @@ func Test_DisbursementHandler_validateRequest(t *testing.T) {
},
}

for _, rct := range data.AllRegistrationContactTypes() {
var name string
var expectedErrors map[string]interface{}
if !rct.IncludesWalletAddress {
name = fmt.Sprintf("🔴[%s]registration_contact_type without wallet address REQUIRES verification_field", rct)
expectedErrors = map[string]interface{}{
"verification_field": fmt.Sprintf("verification_field must be one of %v", data.GetAllVerificationTypes()),
}
} else {
name = fmt.Sprintf("🟢[%s]registration_contact_type with wallet address DOES NOT REQUIRE registration_contact_type", rct)
}
newTestCase := TestCase{
name: name,
request: PostDisbursementRequest{
Name: "disbursement 1",
AssetID: "61dbfa89-943a-413c-b862-a2177384d321",
WalletID: "aab4a4a9-2493-4f37-9741-01d5bd31d68b",
RegistrationContactType: rct,
VerificationField: "",
},
expectedErrors: expectedErrors,
}

testCases = append(testCases, newTestCase)
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
handler := &DisbursementHandler{}
Expand Down
10 changes: 10 additions & 0 deletions internal/utils/sql.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package utils

import "database/sql"

func SQLNullString(s string) sql.NullString {
return sql.NullString{
String: s,
Valid: s != "",
}
}

0 comments on commit bc1384f

Please sign in to comment.