Repository navigation
STF-1906: Add phone verification inputs #723
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d1ec55e
73f863d
828dec1
8a481f7
d2ba1c8
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 |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ | |
|
|
||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import com.maxmind.minfraud.AbstractModel; | ||
| import java.time.ZonedDateTime; | ||
| import java.util.Locale; | ||
| import java.util.regex.Pattern; | ||
|
|
||
| /** | ||
|
|
@@ -20,6 +22,9 @@ public abstract sealed class AbstractLocation extends AbstractModel | |
| private final String postal; | ||
| private final String phoneNumber; | ||
| private final String phoneCountryCode; | ||
| private final PhoneVerificationMethod phoneVerificationMethod; | ||
| private final Boolean phoneWasVerificationSuccessful; | ||
| private final ZonedDateTime phoneVerificationTime; | ||
|
|
||
| protected AbstractLocation(AbstractLocation.Builder builder) { | ||
| firstName = builder.firstName; | ||
|
|
@@ -33,6 +38,9 @@ protected AbstractLocation(AbstractLocation.Builder builder) { | |
| postal = builder.postal; | ||
| phoneNumber = builder.phoneNumber; | ||
| phoneCountryCode = builder.phoneCountryCode; | ||
| phoneVerificationMethod = builder.phoneVerificationMethod; | ||
| phoneWasVerificationSuccessful = builder.phoneWasVerificationSuccessful; | ||
| phoneVerificationTime = builder.phoneVerificationTime; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -56,6 +64,9 @@ abstract static class Builder<T extends AbstractLocation.Builder<T>> { | |
| String postal; | ||
| String phoneNumber; | ||
| String phoneCountryCode; | ||
| PhoneVerificationMethod phoneVerificationMethod; | ||
| Boolean phoneWasVerificationSuccessful; | ||
| ZonedDateTime phoneVerificationTime; | ||
|
|
||
| /** | ||
| * @param name The first name associated with the address | ||
|
|
@@ -162,6 +173,39 @@ public final T phoneNumber(String number) { | |
| return (T) this; | ||
| } | ||
|
|
||
| /** | ||
| * @param method The most recent method used to verify the phone number associated with | ||
| * the address. Do not set this if no verification was attempted. | ||
| * @return The builder object. | ||
| */ | ||
| public final T phoneVerificationMethod(PhoneVerificationMethod method) { | ||
|
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. Nit: The Javadoc for 馃 Comment by Claude Opus 5.5.
Member
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 in 828dec1, also in the CHANGELOG. Reply by Claude Opus 5.5 on Greg's behalf. |
||
| phoneVerificationMethod = method; | ||
| return (T) this; | ||
| } | ||
|
|
||
| /** | ||
| * @param wasSuccessful Whether the most recent verification of the phone number | ||
| * associated with the address succeeded. Do not set this if no | ||
| * verification was attempted. | ||
| * @return The builder object. | ||
| */ | ||
| public final T phoneWasVerificationSuccessful(Boolean wasSuccessful) { | ||
| phoneWasVerificationSuccessful = wasSuccessful; | ||
| return (T) this; | ||
| } | ||
|
|
||
| /** | ||
| * @param time The date and time of the most recent verification of the phone number | ||
| * associated with the address. The time must not be before | ||
|
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. Nit: I saw the earlier thread about a range check, and I agree with the decision not to add one. But this Javadoc now states a rule that the client does not enforce. A reader can think that the builder checks the range. Suggestion: tell the reader what occurs, as dev-site does. For example: "The web service ignores a time outside this range and returns a warning." 馃 Comment by Claude Opus 5.5.
Member
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 in 828dec1. Reply by Claude Opus 5.5 on Greg's behalf. |
||
| * 1970-01-01T00:00:00Z or more than two days in the future. The web | ||
| * service ignores a time outside this range and returns a warning. | ||
| * @return The builder object. | ||
| */ | ||
| public final T phoneVerificationTime(ZonedDateTime time) { | ||
|
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. Question: 馃 Comment by Claude Opus 5.5.
Member
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.
Reply by Claude Opus 5.5 on Greg's behalf. |
||
| phoneVerificationTime = time; | ||
| return (T) this; | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| abstract AbstractLocation build(); | ||
| } | ||
|
|
||
|
|
@@ -257,4 +301,56 @@ public final String phoneNumber() { | |
| public final String phoneCountryCode() { | ||
| return phoneCountryCode; | ||
| } | ||
|
|
||
| /** | ||
| * @return The most recent method used to verify the phone number associated with the address | ||
| */ | ||
| @JsonProperty("phone_verification_method") | ||
| public final PhoneVerificationMethod phoneVerificationMethod() { | ||
| return phoneVerificationMethod; | ||
| } | ||
|
|
||
| /** | ||
| * @return Whether the most recent verification of the phone number associated with the | ||
| * address succeeded | ||
| */ | ||
| @JsonProperty("phone_was_verification_successful") | ||
| public final Boolean phoneWasVerificationSuccessful() { | ||
| return phoneWasVerificationSuccessful; | ||
| } | ||
|
|
||
| /** | ||
| * @return The date and time of the most recent verification of the phone number associated | ||
| * with the address | ||
| */ | ||
| @JsonProperty("phone_verification_time") | ||
| public final ZonedDateTime phoneVerificationTime() { | ||
|
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. Minor: On line 326, Jackson serializes I checked this on the branch. This is an edge case. 馃 Comment by Claude Opus 5.5.
Member
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. We will keep the caller's offset. This is an edge case, and Reply by Claude Opus 5.5 on Greg's behalf. |
||
| return phoneVerificationTime; | ||
| } | ||
|
|
||
| /** | ||
| * Enumerated phone verification methods. | ||
| */ | ||
| public enum PhoneVerificationMethod { | ||
| /** | ||
| * A code delivered to the phone, such as by SMS, voice call, or messaging app | ||
| */ | ||
| DELIVERED_CODE, | ||
| /** | ||
| * Verification through the mobile network operator, such as silent network | ||
| * authentication | ||
| */ | ||
| NETWORK, | ||
| /** | ||
| * Another verification method | ||
| */ | ||
| OTHER; | ||
|
|
||
| /** | ||
| * @return a string representation of the object. | ||
| */ | ||
| public String toString() { | ||
| return this.name().toLowerCase(Locale.ROOT); | ||
| } | ||
| } | ||
| } | ||
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.
Nit:
PhoneVerificationMethodis declared inAbstractLocation. The README (here and at line 260) andRequestTestHelperget it through the subclasses asBilling.PhoneVerificationMethodandShipping.PhoneVerificationMethod. A reader can think that these are two different types. Also, the enum does not show in theBillingorShippingJavadoc. Some IDE inspections flag a static member that is accessed through a subclass.Suggestion: use
AbstractLocation.PhoneVerificationMethodin the README, and give that name in the CHANGELOG.馃 Comment by Claude Opus 5.5.
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, mentioning AbstractLocation seems odd though.