-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/issue 236 #282
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
Feature/issue 236 #282
Changes from 4 commits
47ace23
a65e661
b17bb23
4acf866
d87ef4c
6047320
2eb92d4
9e5d7be
535fe38
c2ccab9
d09d1e8
d9105de
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| using FluentResults; | ||
| using MediatR; | ||
| using VictoryCenter.BLL.DTOs.Payment.Donation; | ||
|
|
||
| namespace VictoryCenter.BLL.Commands.Donation.Common; | ||
|
|
||
| public record DonationCommand(DonationRequestDto Request) : IRequest<Result<DonationResponseDto>>; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| using MediatR; | ||
|
|
||
| namespace VictoryCenter.BLL.Commands.Donation.Common; | ||
|
|
||
| public interface IDonationCommandHandler<in TRequest, TResult> : IRequestHandler<TRequest, TResult> | ||
| where TRequest : IRequest<TResult> | ||
| { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| using System.Globalization; | ||
| using System.Net; | ||
| using System.Security.Cryptography; | ||
| using System.Text; | ||
| using FluentResults; | ||
| using Microsoft.Extensions.Options; | ||
| using VictoryCenter.BLL.Commands.Donation.Common; | ||
| using VictoryCenter.BLL.Constants; | ||
| using VictoryCenter.BLL.DTOs.Payment.Donation; | ||
| using VictoryCenter.BLL.DTOs.Payment.Way4Pay; | ||
| using VictoryCenter.BLL.Options.Donation; | ||
|
|
||
| namespace VictoryCenter.BLL.Commands.Donation.Way4Pay; | ||
|
|
||
| public class Way4PayDonationCommandHandler : IDonationCommandHandler<DonationCommand, Result<DonationResponseDto>> | ||
| { | ||
| private readonly IOptions<Way4PayOptions> _way4PayOptions; | ||
| private readonly IHttpClientFactory _httpClientFactory; | ||
|
|
||
| public Way4PayDonationCommandHandler(IOptions<Way4PayOptions> way4PayOptions, IHttpClientFactory httpClientFactory) | ||
| { | ||
| _way4PayOptions = way4PayOptions; | ||
| _httpClientFactory = httpClientFactory; | ||
| } | ||
|
|
||
| public async Task<Result<DonationResponseDto>> Handle(DonationCommand request, CancellationToken cancellationToken) | ||
| { | ||
| var orderReference = Guid.CreateVersion7(); | ||
| var orderDate = DateTimeOffset.UtcNow.ToUnixTimeSeconds(); | ||
| var merchantSignature = GenerateMerchantSignature(request, orderReference, orderDate); | ||
|
|
||
| var purchaseRequest = new Way4PayPurchaseRequest() | ||
| { | ||
| Amount = request.Request.Amount, | ||
| Currency = request.Request.Currency, | ||
| MerchantAccount = _way4PayOptions.Value.MerchantLogin, | ||
| MerchantDomainName = _way4PayOptions.Value.MerchantDomainName, | ||
| OrderDate = orderDate, | ||
| OrderReference = orderReference.ToString(), | ||
| ProductCount = [1], | ||
| ProductName = [PaymentConstants.ProductName], | ||
| ProductPrice = [request.Request.Amount], | ||
| MerchantSignature = merchantSignature, | ||
| ReturnUrl = request.Request.ReturnUrl | ||
| }; | ||
|
|
||
| if (request.Request.IsSubscription) | ||
| { | ||
| purchaseRequest.RegularBehavior = "preset"; | ||
| purchaseRequest.RegularAmount = request.Request.Amount; | ||
| purchaseRequest.RegularMode = "monthly"; | ||
| purchaseRequest.RegularOn = true; | ||
| } | ||
|
|
||
| var keyValues = new Dictionary<string, string> | ||
| { | ||
| ["merchantAccount"] = purchaseRequest.MerchantAccount, | ||
| ["merchantDomainName"] = purchaseRequest.MerchantDomainName, | ||
| ["orderReference"] = purchaseRequest.OrderReference, | ||
| ["orderDate"] = purchaseRequest.OrderDate.ToString(), | ||
| ["amount"] = purchaseRequest.Amount.ToString(CultureInfo.InvariantCulture), | ||
| ["currency"] = purchaseRequest.Currency, | ||
| ["productName[]"] = purchaseRequest.ProductName[0], | ||
| ["productCount[]"] = purchaseRequest.ProductCount[0].ToString(CultureInfo.InvariantCulture), | ||
| ["productPrice[]"] = purchaseRequest.ProductPrice[0].ToString(CultureInfo.InvariantCulture), | ||
| ["merchantSignature"] = purchaseRequest.MerchantSignature, | ||
| }; | ||
|
|
||
| if (purchaseRequest.RegularOn.HasValue && purchaseRequest.RegularOn.Value) | ||
| { | ||
| keyValues["regularOn"] = "1"; | ||
| keyValues["regularAmount"] = purchaseRequest.RegularAmount?.ToString()!; | ||
| keyValues["regularMode"] = purchaseRequest.RegularMode!; | ||
| keyValues["regularBehavior"] = purchaseRequest.RegularBehavior!; | ||
| keyValues["regularCount"] = "12"; | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| if (!string.IsNullOrWhiteSpace(purchaseRequest.ReturnUrl)) | ||
| { | ||
| keyValues["returnUrl"] = purchaseRequest.ReturnUrl; | ||
| } | ||
|
|
||
| var content = new FormUrlEncodedContent(keyValues); | ||
|
|
||
| var client = _httpClientFactory.CreateClient("Way4PayClient"); | ||
|
|
||
| var httpRequestMessage = new HttpRequestMessage() | ||
| { | ||
| RequestUri = new Uri(_way4PayOptions.Value.ApiUrl), | ||
| Method = HttpMethod.Post, | ||
| Content = content | ||
| }; | ||
|
|
||
| var response = await client.SendAsync(httpRequestMessage, cancellationToken); | ||
|
|
||
| if (response.StatusCode is HttpStatusCode.Found or HttpStatusCode.SeeOther or HttpStatusCode.Moved) | ||
| { | ||
| var redirectUrl = response.Headers.Location?.ToString(); | ||
| if (!string.IsNullOrEmpty(redirectUrl)) | ||
| { | ||
| return Result.Ok(new DonationResponseDto() | ||
| { | ||
| PaymentUrl = redirectUrl | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return Result.Fail(response.ReasonPhrase); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| private string GenerateMerchantSignature(DonationCommand request, Guid orderReference, long orderDate) | ||
| { | ||
| var concatenatedValues = string.Join( | ||
| ';', | ||
| _way4PayOptions.Value.MerchantLogin, | ||
| _way4PayOptions.Value.MerchantDomainName, | ||
| orderReference, | ||
| orderDate, | ||
| request.Request.Amount, | ||
| request.Request.Currency, | ||
| PaymentConstants.ProductName, | ||
| 1, | ||
| request.Request.Amount); | ||
|
|
||
| var secretKeyBytes = Encoding.UTF8.GetBytes(_way4PayOptions.Value.MerchantSecretKey); | ||
| var signatureBytes = Encoding.UTF8.GetBytes(concatenatedValues); | ||
|
|
||
| using var hmac = new HMACMD5(secretKeyBytes); | ||
|
|
||
| var bytes = hmac.ComputeHash(signatureBytes); | ||
| var sb = new StringBuilder(); | ||
| foreach (var b in bytes) | ||
| { | ||
| sb.Append(b.ToString("x2")); | ||
| } | ||
|
|
||
| return sb.ToString(); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,7 @@ | ||||||||||
| namespace VictoryCenter.BLL.Constants; | ||||||||||
|
|
||||||||||
| public static class PaymentConstants | ||||||||||
| { | ||||||||||
| public static string ChosenPaymentSystemIsNotSupported = "Chosen payment system is not supported"; | ||||||||||
| public static string ProductName = "Донат"; | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Make the fields
- public static string ChosenPaymentSystemIsNotSupported = "Chosen payment system is not supported";
- public static string ProductName = "Донат";
+ public const string ChosenPaymentSystemIsNotSupported = "Chosen payment system is not supported";
+ public const string ProductName = "Донат";📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @NovickVitaliy Great! Thank you for making those fields 🐰 🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,10 @@ | ||||||
| namespace VictoryCenter.BLL.DTOs.Payment.Donation; | ||||||
|
|
||||||
| public record DonationRequestDto | ||||||
| { | ||||||
| public decimal Amount { get; init; } | ||||||
| public string Currency { get; init; } = null!; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Replace null-forgiveness with the Using - public string Currency { get; init; } = null!;
+ public required string Currency { get; init; }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need because the dto is validated before usage
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @NovickVitaliy Thanks for the clarification! You're absolutely right that the validation layer handles this appropriately. I can see from the codebase that the The validation-before-usage approach is a valid architectural choice here, and since the validation is enforced at the service layer, the 🧠 Learnings used |
||||||
| public bool IsSubscription { get; init; } | ||||||
| public PaymentSystem PaymentSystem { get; init; } | ||||||
| public string? ReturnUrl { get; init; } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| namespace VictoryCenter.BLL.DTOs.Payment.Donation; | ||
|
|
||
| public record DonationResponseDto | ||
| { | ||
| public string PaymentUrl { get; init; } = null!; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| using System.Text.Json.Serialization; | ||
|
|
||
| namespace VictoryCenter.BLL.DTOs.Payment; | ||
|
|
||
| [JsonConverter(typeof(JsonStringEnumConverter))] | ||
| public enum PaymentSystem | ||
| { | ||
| Way4Pay | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| namespace VictoryCenter.BLL.DTOs.Payment.Way4Pay; | ||
|
|
||
| public class Way4PayPurchaseRequest | ||
| { | ||
| required public string MerchantAccount { get; init; } | ||
| required public string MerchantDomainName { get; init; } | ||
| required public string MerchantSignature { get; init; } | ||
| required public string OrderReference { get; init; } | ||
| required public long OrderDate { get; init; } | ||
| required public decimal Amount { get; init; } | ||
| required public string Currency { get; init; } | ||
| required public string[] ProductName { get; init; } | ||
| required public decimal[] ProductPrice { get; init; } | ||
| required public int[] ProductCount { get; init; } | ||
|
Comment on lines
+5
to
+
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Parallel arrays risk index-mismatch bugs—model products as a single collection
- required public string[] ProductName { get; init; }
- required public decimal[] ProductPrice { get; init; }
- required public int[] ProductCount { get; init; }
+ // Keeps related fields together and eliminates alignment issues
+ required public IReadOnlyCollection<Way4PayProductItem> Items { get; init; }
+
+public sealed record Way4PayProductItem(
+ string Name,
+ decimal Price,
+ int Count);🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I do so then the object is not going to be serialized properly
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @NovickVitaliy You're absolutely right! Payment APIs like Way4Pay typically expect very specific field names and formats. Changing to a collection would alter the serialization structure and likely break the API integration. Here are some alternative approaches that preserve the current serialization while still addressing the alignment risk: Option 1: Add validation in the factory/builder public static class Way4PayPurchaseRequestFactory
{
public static Way4PayPurchaseRequest Create(/* params */)
{
if (productNames.Length != productPrices.Length ||
productPrices.Length != productCounts.Length)
{
throw new ArgumentException("Product arrays must have equal lengths");
}
return new Way4PayPurchaseRequest { /* ... */ };
}
}Option 2: Add validation attributes [ArrayLengthsEqual(nameof(ProductPrice), nameof(ProductCount))]
public class Way4PayPurchaseRequest
{
// existing properties...
}Since the Way4Pay API contract dictates the serialization format, keeping the parallel arrays is the right approach. The validation should happen at the service layer where the request is constructed. ✏️ Learnings added
🧠 Learnings used |
||
| public string? ReturnUrl { get; set; } | ||
| public string? ServiceUrl { get; set; } | ||
| public string? RegularBehavior { get; set; } | ||
| public string? RegularMode { get; set; } | ||
| public decimal? RegularAmount { get; set; } | ||
| public bool? RegularOn { get; set; } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| using FluentResults; | ||
| using Microsoft.Extensions.Options; | ||
| using VictoryCenter.BLL.Commands.Donation.Common; | ||
| using VictoryCenter.BLL.Commands.Donation.Way4Pay; | ||
| using VictoryCenter.BLL.DTOs.Payment; | ||
| using VictoryCenter.BLL.DTOs.Payment.Donation; | ||
| using VictoryCenter.BLL.Factories.Donation.Interfaces; | ||
| using VictoryCenter.BLL.Options.Donation; | ||
|
|
||
| namespace VictoryCenter.BLL.Factories.Donation.Implementations; | ||
|
|
||
| public class Way4PayDonationFactory : IDonationFactory | ||
| { | ||
| private readonly IOptions<Way4PayOptions> _way4PayOptions; | ||
| private readonly IHttpClientFactory _httpClientFactory; | ||
|
|
||
| public Way4PayDonationFactory(IOptions<Way4PayOptions> way4PayOptions, IHttpClientFactory httpClientFactory) | ||
| { | ||
| _way4PayOptions = way4PayOptions; | ||
| _httpClientFactory = httpClientFactory; | ||
| } | ||
|
|
||
| public PaymentSystem PaymentSystem => PaymentSystem.Way4Pay; | ||
|
|
||
| public IDonationCommandHandler<DonationCommand, Result<DonationResponseDto>> GetRequestHandler() | ||
| { | ||
| return new Way4PayDonationCommandHandler(_way4PayOptions, _httpClientFactory); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| using FluentResults; | ||
| using VictoryCenter.BLL.Commands.Donation.Common; | ||
| using VictoryCenter.BLL.DTOs.Payment; | ||
| using VictoryCenter.BLL.DTOs.Payment.Donation; | ||
|
|
||
| namespace VictoryCenter.BLL.Factories.Donation.Interfaces; | ||
|
|
||
| public interface IDonationFactory | ||
| { | ||
| PaymentSystem PaymentSystem { get; } | ||
| IDonationCommandHandler<DonationCommand, Result<DonationResponseDto>> GetRequestHandler(); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| using FluentResults; | ||
| using VictoryCenter.BLL.DTOs.Payment.Donation; | ||
|
|
||
| namespace VictoryCenter.BLL.Interfaces.PaymentService; | ||
|
|
||
| public interface IDonationService | ||
| { | ||
| Task<Result<DonationResponseDto>> CreateDonation(DonationRequestDto request, CancellationToken cancellationToken); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| using System.ComponentModel.DataAnnotations; | ||
|
|
||
| namespace VictoryCenter.BLL.Options.Donation; | ||
|
|
||
| public class Way4PayOptions | ||
| { | ||
| public const string Position = "PaymentSystemsConfigurations:Way4Pay"; | ||
|
|
||
| [Required] | ||
| public string MerchantLogin { get; init; } = null!; | ||
|
|
||
| [Required] | ||
| public string MerchantSecretKey { get; init; } = null!; | ||
|
|
||
| [Required] | ||
| public string MerchantDomainName { get; init; } = null!; | ||
|
|
||
| [Required] | ||
| public string ApiUrl { get; init; } = null!; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| using FluentResults; | ||
| using FluentValidation; | ||
| using VictoryCenter.BLL.Commands.Donation.Common; | ||
| using VictoryCenter.BLL.Constants; | ||
| using VictoryCenter.BLL.DTOs.Payment.Donation; | ||
| using VictoryCenter.BLL.Factories.Donation.Interfaces; | ||
| using VictoryCenter.BLL.Interfaces.PaymentService; | ||
|
|
||
| namespace VictoryCenter.BLL.Services.PaymentService; | ||
|
|
||
| public class DonationService : IDonationService | ||
| { | ||
| private readonly IEnumerable<IDonationFactory> _donationFactories; | ||
| private readonly IValidator<DonationRequestDto> _validator; | ||
|
|
||
| public DonationService(IEnumerable<IDonationFactory> donationFactories, IValidator<DonationRequestDto> validator) | ||
| { | ||
| _donationFactories = donationFactories; | ||
| _validator = validator; | ||
| } | ||
|
|
||
| public async Task<Result<DonationResponseDto>> CreateDonation(DonationRequestDto request, CancellationToken cancellationToken) | ||
| { | ||
| var validationResult = await _validator.ValidateAsync(request, cancellationToken); | ||
| if (!validationResult.IsValid) | ||
| { | ||
| return Result.Fail(validationResult.Errors.Select(x => x.ErrorMessage)); | ||
| } | ||
|
|
||
| var donationFactory = _donationFactories.SingleOrDefault(df => df.PaymentSystem == request.PaymentSystem); | ||
| if (donationFactory is null) | ||
| { | ||
| return Result.Fail(PaymentConstants.ChosenPaymentSystemIsNotSupported); | ||
| } | ||
|
|
||
| var commandHandler = donationFactory.GetRequestHandler(); | ||
|
|
||
| return await commandHandler.Handle(new DonationCommand(request), CancellationToken.None); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| using FluentValidation; | ||
| using VictoryCenter.BLL.Constants; | ||
| using VictoryCenter.BLL.DTOs.Payment.Donation; | ||
|
|
||
| namespace VictoryCenter.BLL.Validators.Donation; | ||
|
|
||
| public class DonationRequestValidator : AbstractValidator<DonationRequestDto> | ||
| { | ||
| private const int CurrencyCodeLenght = 3; | ||
| private const string InvalidCurrencyCode = "Invalid currency code"; | ||
| private const string CurrencyExpression = "^[A-Z]{3}$"; | ||
|
|
||
| public DonationRequestValidator() | ||
| { | ||
| RuleFor(x => x.Amount) | ||
| .GreaterThan(0).WithMessage(ErrorMessagesConstants.PropertyMustBeGreaterThan(nameof(DonationRequestDto.Amount), 0)); | ||
|
|
||
| RuleFor(x => x.Currency) | ||
| .NotEmpty().WithMessage(ErrorMessagesConstants.PropertyIsRequired(nameof(DonationRequestDto.Currency))) | ||
| .Matches(CurrencyExpression).WithMessage(InvalidCurrencyCode) | ||
| .Length(CurrencyCodeLenght).WithMessage(ErrorMessagesConstants.PropertyMustHaveALengthOfNCharacters(nameof(DonationRequestDto.Currency), CurrencyCodeLenght)); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| RuleFor(x => x.PaymentSystem) | ||
| .NotNull().WithMessage(ErrorMessagesConstants.PropertyIsRequired(nameof(DonationRequestDto.PaymentSystem))); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider validating the donation amount.
The amount is used directly without validation. Consider adding a check to ensure it's positive.
Add validation before creating the purchase request:
📝 Committable suggestion
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is validated in the
DonationServicebefore calling the handler.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NovickVitaliy, you're absolutely right! I can see that the
DonationServicevalidates the request usingDonationRequestValidatorbefore calling any handler, and the validator ensures the amount is greater than zero. This follows a good separation of concerns where validation happens at the service boundary rather than being duplicated in each handler. Thank you for the clarification!✏️ Learnings added
🧠 Learnings used