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

[ACL-242] Add CreditableAt in GetPaymentResult #237

Merged
merged 2 commits into from
Dec 11, 2024
Merged

Conversation

tl-Roberto-Mancinelli
Copy link
Collaborator

@tl-Roberto-Mancinelli tl-Roberto-Mancinelli commented Dec 11, 2024

Added CreditableAt in Authorized, Executed and Settled case of GetPayment.
Added test for GetPayment when status is Settled

@tl-Roberto-Mancinelli tl-Roberto-Mancinelli marked this pull request as ready for review December 11, 2024 16:15
/// </summary>
/// <returns></returns>
[JsonDiscriminator("authorized")]
public record Authorized : PaymentDetails;
public record Authorized(DateTime? CreditableAt) : PaymentDetails;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct? It does not seems so based on our public docs? I can't see the field on the authorized payment details on the API ref

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, it's quite new, probably the docs are not updated, I'm checking with Risk

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's confirmed, we need to update docs

_httpClient = httpClient;
}

public async Task<Uri> AuthorisePaymentAsync(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we do something hacky already for similar purposes, for payments and mandates

Could we make this function usable for every resource type (mandates and payments) and reuse this everywhere?
Not needed now, we can do this in another PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, I was planning to do a refactor later, we can optimise several part of the tests.
The problem with this method was that returns a GetPaymentResponse.PaymentDetails and we need the specific type of the GetPaymentResponse for this test.

@tl-Roberto-Mancinelli tl-Roberto-Mancinelli merged commit 6ea53a4 into main Dec 11, 2024
1 check passed
@tl-Roberto-Mancinelli tl-Roberto-Mancinelli deleted the ACL-242 branch December 11, 2024 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants