-
Notifications
You must be signed in to change notification settings - Fork 11
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
Client API are updated to not require an IP address for transaction reporting #246
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
50925ff
Fix formatting tags
ugexe 1accde3
Correct TransactionReport ToString() output
ugexe 301047f
Make IP address optional when reporting transactions
ugexe 62141ad
Treat 0 UUID minfraudId the same as a null minfraudId
ugexe da0e1c6
Avoid requiring ipAddress to be specified
ugexe 60da300
Use non-deprecated constructor in test
ugexe b88a7c7
Test the empty values fail identifier validation
ugexe 98647e5
Bump major version
ugexe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,58 +46,93 @@ public sealed class TransactionReport | |
private string? _maxmindId; | ||
|
||
/// <summary> | ||
/// Constructor. | ||
/// Constructor with validation. | ||
/// </summary> | ||
/// <param name="ipAddress">The IP address reported to MaxMind for the | ||
/// transaction.</param> | ||
/// <param name="tag">The <c>TransactionReportTag</c> indicating the | ||
/// type of report being made.</param> | ||
/// <param name="chargebackCode">A string which is provided by your | ||
/// payment processor indicating <a href="https://en.wikipedia.org/wiki/Chargeback#Reason_codes"> | ||
/// the reason for the chargeback</a>.</param> | ||
/// <param name="ipAddress">The IP address reported to MaxMind for the | ||
/// transaction. This field is not required if you provide at least | ||
/// one of the transaction's <c>minfraudId</c>, <c>maxmindId</c>, | ||
/// or <c>transactionId></c>. You are encouraged to provide it, | ||
/// if possible.</param> | ||
/// <param name="maxmindId">A unique eight character string identifying | ||
/// a minFraud Standard or Premium request. These IDs are returned | ||
/// in the <c>maxmindID</c> field of a response for a successful | ||
/// minFraud request. This field is not required, but you are | ||
/// encouraged to provide it, if possible.</param> | ||
/// minFraud request. This field is not required if you provide at | ||
/// least one of the transaction's <c>ipAddress</c>, | ||
/// <c>minfraudId</c>, or <c>transactionId></c>. You are encouraged | ||
/// to provide it, if possible.</param> | ||
/// <param name="minfraudId">A UUID that identifies a minFraud Score, | ||
/// minFraud Insights, or minFraud Factors request. This ID is | ||
/// returned at <c>/id</c> in the response. This field is not | ||
/// required, but you are encouraged to provide it if the request | ||
/// was made to one of these services.</param> | ||
/// required if you provide at least one of the transaction's | ||
/// <c>ipAddress</c>, <c>maxmindId</c>, or <c>transactionId></c>. | ||
/// You are encouraged to provide it, if possible.</param> | ||
/// <param name="notes">Your notes on the fraud tag associated with the | ||
/// transaction. We manually review many reported transactions to | ||
/// improve our scoring for you so any additional details to help | ||
/// us understand context are helpful.</param> | ||
/// <param name="transactionId">The transaction ID you originally passed | ||
/// to minFraud. This field is not required, but you are encouraged | ||
/// to provide it or the transaction's <c>>maxmindId</c> or | ||
/// <c>minfraudId</c>.</param> | ||
/// to minFraud. This field is not required if you provide at least | ||
/// one of the transaction's <c>ipAddress</c>, <c>maxmindId</c>, | ||
/// or <c>minfraudId></c>. You are encouraged to provide it, if | ||
/// possible.</param> | ||
public TransactionReport( | ||
IPAddress ipAddress, | ||
TransactionReportTag tag, | ||
string? chargebackCode = null, | ||
IPAddress? ipAddress = null, | ||
string? maxmindId = null, | ||
Guid? minfraudId = null, | ||
string? notes = null, | ||
string? transactionId = null | ||
) | ||
{ | ||
if (ipAddress == null && (minfraudId == null || minfraudId == Guid.Empty) | ||
&& string.IsNullOrEmpty(maxmindId) && string.IsNullOrEmpty(transactionId)) | ||
{ | ||
throw new ArgumentException( | ||
"The user must pass at least one of the following: " + | ||
"ipAddress, minfraudId, maxmindId, transactionId." | ||
); | ||
} | ||
|
||
IPAddress = ipAddress; | ||
Tag = tag; | ||
ChargebackCode = chargebackCode; | ||
MaxMindId = maxmindId; | ||
MinFraudId = minfraudId; | ||
Notes = notes; | ||
TransactionId = transactionId; | ||
|
||
if(minfraudId != Guid.Empty) { | ||
MinFraudId = minfraudId; | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// Constructor for backwards compatibility. | ||
/// </summary> | ||
[Obsolete] | ||
public TransactionReport( | ||
IPAddress? ipAddress, | ||
TransactionReportTag tagObsolete, | ||
string? chargebackCode = null, | ||
string? maxmindId = null, | ||
Guid? minfraudId = null, | ||
string? notes = null, | ||
string? transactionId = null | ||
): this(tagObsolete, chargebackCode, ipAddress, maxmindId, minfraudId, notes, transactionId) | ||
{ | ||
} | ||
|
||
/// <summary> | ||
/// The IP address reported to MaxMind for the transaction. | ||
/// </summary> | ||
[JsonPropertyName("ip_address")] | ||
[JsonConverter(typeof(IPAddressConverter))] | ||
public IPAddress IPAddress { get; init; } | ||
public IPAddress? IPAddress { get; init; } | ||
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. Hmm, this seems like a breaking change. I don't know if is likely to cause actual breakage for anyone though. Maybe we should do a major version bump to be safe though. |
||
|
||
/// <summary> | ||
/// The <c>TransactionReportTag</c> indicating the type of report | ||
|
@@ -156,7 +191,7 @@ public string? MaxMindId | |
/// <summary> | ||
/// The transaction ID you originally passed to minFraud. This field | ||
/// is not required, but you are encouraged to provide it or the | ||
/// transaction's <c>>maxmindId</c> or <c>minfraudId</c>. | ||
/// transaction's <c>maxmindId</c> or <c>minfraudId</c>. | ||
/// </summary> | ||
[JsonPropertyName("transaction_id")] | ||
public string? TransactionId { get; init; } | ||
|
@@ -167,7 +202,7 @@ public string? MaxMindId | |
/// <returns>A string that represents the current object.</returns> | ||
public override string ToString() | ||
{ | ||
return $"IPAddress: {IPAddress}, Tag: {Tag}, ChargebackCode: {ChargebackCode}, MaxMindId: {MaxMindId}" | ||
return $"IPAddress: {IPAddress}, Tag: {Tag}, ChargebackCode: {ChargebackCode}, MaxMindId: {MaxMindId}, " | ||
+ $"MinFraudId: {MinFraudId}, Notes: {Notes}, TransactionId: {TransactionId}"; | ||
} | ||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hmm, this interacts poorly with init properties, but I don't have an immediate solution for that. Although we encourage init properties for minFraud requests, we don't really encourage them for the report transaction API. I suppose we can worry about this in the future.