[SDP-961] Change `POST /disbursements` to accept Verification Type #103

ziyliu · 2023-11-21T18:58:55Z

What

Add verification types to /disbursement endpoint

Why

Finally leverage validation of verification type for the above endpoint

Known limitations

[TODO or N/A]

Checklist

PR Structure

This PR has reasonably narrow scope (if not, break it down into smaller PRs).
This PR does not mix refactoring changes with feature changes (split into two PRs otherwise).
This PR's title starts with the name of the package, area, or subject affected by the change.

Thoroughness

This PR adds tests for the new functionality or fixes.
This PR contains the link to the Jira ticket it addresses.

Configs and Secrets

No new CONFIG variables are required -OR- the new required ones were added to the helmchart's values.yaml file.
No new CONFIG variables are required -OR- the new required ones were added to the deployments (pr-preview, dev, demo, prd).
No new SECRETS variables are required -OR- the new required ones were mentioned in the helmchart's values.yaml file.
No new SECRETS variables are required -OR- the new required ones were added to the deployments (pr-preview secrets, dev secrets, demo secrets, prd secrets).

Release

This is not a breaking change.
This is ready for production.. If your PR is not ready for production, please consider opening additional complementary PRs using this one as the base. Only merge this into develop or main after it's ready for production!

Deployment

Does the deployment work after merging?

stellar-jenkins · 2023-11-21T19:00:03Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

stellar-jenkins · 2023-11-21T23:07:38Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

stellar-jenkins · 2023-11-21T23:28:36Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

stellar-jenkins · 2023-11-21T23:37:02Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

ziyliu · 2023-11-21T23:37:17Z

internal/serve/validators/disbursement_validator.go

+	VERIFICATION_FIELD_PIN_MIN_LENGTH = 4
+	VERIFICATION_FIELD_PIN_MAX_LENGTH = 8
+
+	VERIFICATION_FIELD_MAX_ID_LENGTH = 50


I checked with @torisamples on these values

stellar-jenkins · 2023-11-22T18:14:10Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

stellar-jenkins · 2023-11-22T18:15:06Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

stellar-jenkins · 2023-11-22T18:48:50Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

internal/data/disbursements.go

stellar-jenkins · 2023-11-28T19:01:21Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

marwen-abid · 2023-11-28T18:59:49Z

internal/serve/httphandler/disbursement_handler.go

@@ -52,16 +54,21 @@ func (d DisbursementHandler) PostDisbursement(w http.ResponseWriter, r *http.Req
 		return
 	}

-	// validate request
-	v := validators.NewValidator()
+	iv := validators.NewDisbursementInstructionsValidator(disbursementRequest.VerificationField)


We don't need the NewDisbursementInstructionsValidator as we're not validating instructions at this stage. We're just validating the 5 fields posted when creating a new disbursement.

We should revert this back to v := validators.NewValidator() and just make sure that VerificationField is not empty.

v := validators.NewValidator() v.Check(disbursementRequest.Name != "", "name", "name is required") v.Check(disbursementRequest.CountryCode != "", "country_code", "country_code is required") v.Check(disbursementRequest.WalletID != "", "wallet_id", "wallet_id is required") v.Check(disbursementRequest.AssetID != "", "asset_id", "asset_id is required") v.Check(disbursementRequest.VerificationField != "", "verification_field", "verification_field is required") if v.HasErrors() { httperror.BadRequest("Request invalid", err, v.Errors).Render(w) return }

@marwen-abid my thinking is that wouldn't we want to make sure that VerificationField is one of the three existing types? i.e. we decide to add a new verification type but not use it for this endpoint. I know we're technically not validating instructions in this handler but the additional ValidateAndGetVerificationType call on the NewDisbursementInstructionsValidator instance provides an additional error messaging for when the type is invalid.

I don't think this shortcut is worth it.

the new method you created ValidateAndGetVerificationType is only used here, so I think it makes more sense to create a new validator called DisbursementRequestValidator that takes the whole disbursementRequest and validates it. You can move ValidateAndGetVerificationType to that new validator.

internal/serve/httphandler/disbursement_handler_test.go

stellar-jenkins · 2023-11-28T19:25:25Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

stellar-jenkins · 2023-11-28T19:43:22Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

stellar-jenkins · 2023-11-28T21:56:14Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

marwen-abid

Thank you! This looks great 🚀

stellar-jenkins · 2023-11-28T22:06:15Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

more feedback rename lint

stellar-jenkins · 2023-11-28T22:15:30Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

stellar-jenkins · 2023-11-28T22:17:03Z

stellar-disbursement-platform-backend-preview is available here:
SDP: https://sdp-backend-pr103.previews.kube001.services.stellar-ops.com/health
AP: https://sdp-ap-pr103.previews.kube001.services.stellar-ops.com/health
Frontend: https://sdp-backend-dashboard-pr103.previews.kube001.services.stellar-ops.com

ziyliu temporarily deployed to Anchor Integration Tests November 21, 2023 18:59 — with GitHub Actions Inactive

ziyliu had a problem deploying to Receiver Registration - E2E Integration Tests November 21, 2023 18:59 — with GitHub Actions Failure

ziyliu marked this pull request as draft November 21, 2023 18:59

ziyliu temporarily deployed to Anchor Integration Tests November 21, 2023 23:06 — with GitHub Actions Inactive

ziyliu had a problem deploying to Receiver Registration - E2E Integration Tests November 21, 2023 23:06 — with GitHub Actions Failure

ziyliu force-pushed the erica/sdp-961 branch from 6d4d1b1 to b9ee6eb Compare November 21, 2023 23:27

ziyliu temporarily deployed to Anchor Integration Tests November 21, 2023 23:27 — with GitHub Actions Inactive

ziyliu had a problem deploying to Receiver Registration - E2E Integration Tests November 21, 2023 23:27 — with GitHub Actions Failure

ziyliu temporarily deployed to Anchor Integration Tests November 21, 2023 23:36 — with GitHub Actions Inactive

ziyliu had a problem deploying to Receiver Registration - E2E Integration Tests November 21, 2023 23:36 — with GitHub Actions Failure

ziyliu commented Nov 21, 2023

View reviewed changes

ziyliu marked this pull request as ready for review November 21, 2023 23:37

ziyliu requested review from marwen-abid and ceciliaromao November 21, 2023 23:37

ziyliu self-assigned this Nov 21, 2023

ziyliu temporarily deployed to Anchor Integration Tests November 22, 2023 18:12 — with GitHub Actions Inactive

ziyliu temporarily deployed to Receiver Registration - E2E Integration Tests November 22, 2023 18:12 — with GitHub Actions Inactive

ziyliu temporarily deployed to Anchor Integration Tests November 22, 2023 18:14 — with GitHub Actions Inactive

ziyliu temporarily deployed to Receiver Registration - E2E Integration Tests November 22, 2023 18:14 — with GitHub Actions Inactive

ziyliu force-pushed the erica/sdp-961 branch from 70ff1df to d6c9466 Compare November 22, 2023 18:48

ziyliu temporarily deployed to Receiver Registration - E2E Integration Tests November 22, 2023 18:48 — with GitHub Actions Inactive

ziyliu temporarily deployed to Anchor Integration Tests November 22, 2023 18:48 — with GitHub Actions Inactive

marwen-abid requested changes Nov 22, 2023

View reviewed changes

internal/data/disbursements.go Outdated Show resolved Hide resolved

internal/data/disbursements.go Outdated Show resolved Hide resolved

ziyliu temporarily deployed to Anchor Integration Tests November 28, 2023 19:00 — with GitHub Actions Inactive

ziyliu temporarily deployed to Receiver Registration - E2E Integration Tests November 28, 2023 19:00 — with GitHub Actions Inactive

marwen-abid requested changes Nov 28, 2023

View reviewed changes

feedback

54a5174

ziyliu temporarily deployed to Anchor Integration Tests November 28, 2023 19:24 — with GitHub Actions Inactive

ziyliu temporarily deployed to Receiver Registration - E2E Integration Tests November 28, 2023 19:24 — with GitHub Actions Inactive

ziyliu temporarily deployed to Receiver Registration - E2E Integration Tests November 28, 2023 19:42 — with GitHub Actions Inactive

ziyliu temporarily deployed to Anchor Integration Tests November 28, 2023 19:42 — with GitHub Actions Inactive

ziyliu requested a review from marwen-abid November 28, 2023 19:43

ziyliu temporarily deployed to Anchor Integration Tests November 28, 2023 21:55 — with GitHub Actions Inactive

ziyliu temporarily deployed to Receiver Registration - E2E Integration Tests November 28, 2023 21:55 — with GitHub Actions Inactive

ziyliu temporarily deployed to Receiver Registration - E2E Integration Tests November 28, 2023 22:05 — with GitHub Actions Inactive

ziyliu temporarily deployed to Anchor Integration Tests November 28, 2023 22:05 — with GitHub Actions Inactive

marwen-abid approved these changes Nov 28, 2023

View reviewed changes

fix

dbce4d7

more feedback rename lint

ziyliu force-pushed the erica/sdp-961 branch from de1714f to dbce4d7 Compare November 28, 2023 22:14

ziyliu temporarily deployed to Anchor Integration Tests November 28, 2023 22:14 — with GitHub Actions Inactive

ziyliu temporarily deployed to Receiver Registration - E2E Integration Tests November 28, 2023 22:14 — with GitHub Actions Inactive

rm method

48ed862

ziyliu temporarily deployed to Receiver Registration - E2E Integration Tests November 28, 2023 22:16 — with GitHub Actions Inactive

ziyliu temporarily deployed to Anchor Integration Tests November 28, 2023 22:16 — with GitHub Actions Inactive

ziyliu merged commit a665505 into develop Nov 28, 2023
6 checks passed

ziyliu deleted the erica/sdp-961 branch November 28, 2023 22:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SDP-961] Change `POST /disbursements` to accept Verification Type #103

[SDP-961] Change `POST /disbursements` to accept Verification Type #103

ziyliu commented Nov 21, 2023 •

edited

Loading

stellar-jenkins commented Nov 21, 2023

stellar-jenkins commented Nov 21, 2023

stellar-jenkins commented Nov 21, 2023

stellar-jenkins commented Nov 21, 2023

ziyliu Nov 21, 2023

stellar-jenkins commented Nov 22, 2023

stellar-jenkins commented Nov 22, 2023

stellar-jenkins commented Nov 22, 2023

stellar-jenkins commented Nov 28, 2023

marwen-abid Nov 28, 2023

ziyliu Nov 28, 2023 •

edited

Loading

marwen-abid Nov 28, 2023

stellar-jenkins commented Nov 28, 2023

stellar-jenkins commented Nov 28, 2023

stellar-jenkins commented Nov 28, 2023

marwen-abid left a comment

stellar-jenkins commented Nov 28, 2023

stellar-jenkins commented Nov 28, 2023

stellar-jenkins commented Nov 28, 2023

[SDP-961] Change POST /disbursements to accept Verification Type #103

[SDP-961] Change POST /disbursements to accept Verification Type #103

Conversation

ziyliu commented Nov 21, 2023 • edited Loading

What

Why

Known limitations

Checklist

PR Structure

Thoroughness

Configs and Secrets

Release

Deployment

stellar-jenkins commented Nov 21, 2023

stellar-jenkins commented Nov 21, 2023

stellar-jenkins commented Nov 21, 2023

stellar-jenkins commented Nov 21, 2023

ziyliu Nov 21, 2023

Choose a reason for hiding this comment

stellar-jenkins commented Nov 22, 2023

stellar-jenkins commented Nov 22, 2023

stellar-jenkins commented Nov 22, 2023

stellar-jenkins commented Nov 28, 2023

marwen-abid Nov 28, 2023

Choose a reason for hiding this comment

ziyliu Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

marwen-abid Nov 28, 2023

Choose a reason for hiding this comment

stellar-jenkins commented Nov 28, 2023

stellar-jenkins commented Nov 28, 2023

stellar-jenkins commented Nov 28, 2023

marwen-abid left a comment

Choose a reason for hiding this comment

stellar-jenkins commented Nov 28, 2023

stellar-jenkins commented Nov 28, 2023

stellar-jenkins commented Nov 28, 2023

[SDP-961] Change `POST /disbursements` to accept Verification Type #103

[SDP-961] Change `POST /disbursements` to accept Verification Type #103

ziyliu commented Nov 21, 2023 •

edited

Loading

ziyliu Nov 28, 2023 •

edited

Loading