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

224/mk/post quote update #230

Merged
merged 18 commits into from
Dec 9, 2022
Merged

224/mk/post quote update #230

merged 18 commits into from
Dec 9, 2022

Conversation

mkurapov
Copy link
Contributor

@mkurapov mkurapov commented Dec 8, 2022

Changes proposed in this pull request

During quote creation, we should either expect
a) just the receiver
b) receiver & receiveAmount
c) receiver & sendAmount

I wanted to enforce this at the API level, as currently, receiver is a required property, but receiveAmount and sendAmount are just optional properties.

After some fiddling around, I found the best way to enforce this was providing 3 possible oneOf schemas for the quote creation request body.

https://open-payments-integration.readme.io/reference/create-quote

Context

Fixes #224

receiver:
$ref: ./schemas.yaml#/components/schemas/receiver
receiveAmount:
description: All amounts are maxima, i.e. multiple payments can be created under a grant as long as the total amounts of these payments do not exceed the maximum amount per interval as specified in the grant.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description doesn't seem applicable to quotes

Suggested change
description: All amounts are maxima, i.e. multiple payments can be created under a grant as long as the total amounts of these payments do not exceed the maximum amount per interval as specified in the grant.

Copy link
Contributor Author

@mkurapov mkurapov Dec 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also change the description of schemas.yaml -> components -> schemas -> amount?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, maybe we relocate that description to

sendAmount:
$ref: '#/components/schemas/amount'
receiveAmount:
$ref: '#/components/schemas/amount'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating the issue!

receiver:
$ref: ./schemas.yaml#/components/schemas/receiver
sendAmount:
description: All amounts are maxima, i.e. multiple payments can be created under a grant as long as the total amounts of these payments do not exceed the maximum amount per interval as specified in the grant.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: All amounts are maxima, i.e. multiple payments can be created under a grant as long as the total amounts of these payments do not exceed the maximum amount per interval as specified in the grant.

@mkurapov mkurapov marked this pull request as ready for review December 9, 2022 14:51
receiver:
$ref: ./schemas.yaml#/components/schemas/receiver
receiveAmount:
description: The maximum amount that should be paid into the receiving payment pointer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: The maximum amount that should be paid into the receiving payment pointer.
description: The fixed amount that would be paid into the receiving payment pointer.

with maybe some caveat about assuming a successful subsequent outgoing payment.

receiver:
$ref: ./schemas.yaml#/components/schemas/receiver
sendAmount:
description: The maximum amount that should be sent from the sending payment pointer.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: The maximum amount that should be sent from the sending payment pointer.
description: The fixed amount that would be sent from the sending payment pointer.

@mkurapov mkurapov merged commit ae0e924 into main Dec 9, 2022
@mkurapov mkurapov deleted the 224/mk/post-quote-update branch December 9, 2022 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update POST quote request to have oneOf receiveAmount or sendAmount
2 participants