Repository navigation
STF-1906: Add phone verification inputs #1969
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
Changes from all commits
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 |
|---|---|---|
| @@ -1,4 +1,6 @@ | ||
| import { PhoneVerificationMethod } from '../constants.js'; | ||
| import { ArgumentError } from '../errors.js'; | ||
| import { assertValidDate } from '../utils.js'; | ||
|
|
||
| export interface LocationProps { | ||
| /** | ||
|
|
@@ -39,13 +41,29 @@ export interface LocationProps { | |
| */ | ||
| postal?: string; | ||
| /** | ||
| * The phone country code for the phone number associated with the address. | ||
| * The phone number, without the country code, associated with the address. | ||
| */ | ||
| phoneNumber?: string; | ||
| /** | ||
| * The phone number, without the country code, associated with the address. | ||
| * The phone country code for the phone number associated with the address. | ||
| */ | ||
| phoneCountryCode?: string; | ||
| /** | ||
| * The most recent method used to verify the phone number. | ||
| */ | ||
| phoneVerificationMethod?: PhoneVerificationMethod; | ||
|
oschwald marked this conversation as resolved.
Resolved
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. Medium: The type only helps TypeScript callers. A plain-JS caller can pass
🤖 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 042133f. Reply by Claude Opus 5.5 on Greg's behalf. |
||
| /** | ||
| * The date and time of the most recent verification of the phone number. | ||
|
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: No Consider one helper in 🤖 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 042133f. Reply by Claude Opus 5.5 on Greg's behalf. |
||
| * The time must not be before 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. | ||
| */ | ||
| phoneVerificationTime?: Date; | ||
|
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. Medium: An invalid
The PHP, Python and Ruby clients reject a bad time. Suggest: in the 🤖 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 042133f. Reply by Claude Opus 5.5 on Greg's behalf. |
||
| /** | ||
| * Whether the most recent verification of the phone number succeeded. Omit | ||
| * this if no verification was attempted. | ||
| */ | ||
| phoneWasVerificationSuccessful?: boolean; | ||
|
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. Low: A plain-JS caller can pass a string from a form, query string or env var, for example Suggest: throw 🤖 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 042133f. Reply by Claude Opus 5.5 on Greg's behalf. |
||
| } | ||
|
|
||
| const countryRegex = /^[A-Z]{2}$/; | ||
|
|
@@ -76,6 +94,12 @@ export default class Location implements LocationProps { | |
| public phoneNumber?: string; | ||
| /** @inheritDoc LocationProps.phoneCountryCode */ | ||
| public phoneCountryCode?: string; | ||
| /** @inheritDoc LocationProps.phoneVerificationMethod */ | ||
| public phoneVerificationMethod?: PhoneVerificationMethod; | ||
| /** @inheritDoc LocationProps.phoneVerificationTime */ | ||
| public phoneVerificationTime?: Date; | ||
|
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. Low: If the caller reuses the const t = new Date();
const billing = new Billing({ phoneVerificationTime: t });
t.setTime(otherEventTime); // the request now sends otherEventTimeThe new test in 🤖 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 042133f. The constructor copies the Reply by Claude Opus 5.5 on Greg's behalf. |
||
| /** @inheritDoc LocationProps.phoneWasVerificationSuccessful */ | ||
| public phoneWasVerificationSuccessful?: boolean; | ||
|
|
||
| public constructor(location: LocationProps) { | ||
|
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. Low: The new fields do not follow the repo's
🤖 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 042133f. The constructor validates the three inputs, and each has an invalid-input test. Reply by Claude Opus 5.5 on Greg's behalf. |
||
| if (location.country != null && !countryRegex.test(location.country)) { | ||
|
|
@@ -84,6 +108,38 @@ export default class Location implements LocationProps { | |
| ); | ||
| } | ||
|
|
||
| if ( | ||
| location.phoneVerificationMethod != null && | ||
| !Object.values(PhoneVerificationMethod).includes( | ||
| location.phoneVerificationMethod | ||
| ) | ||
| ) { | ||
| throw new ArgumentError( | ||
| `"${location.phoneVerificationMethod}" is an invalid value for "phoneVerificationMethod"` | ||
| ); | ||
| } | ||
|
|
||
| if ( | ||
| location.phoneWasVerificationSuccessful != null && | ||
| typeof location.phoneWasVerificationSuccessful !== 'boolean' | ||
| ) { | ||
| throw new ArgumentError( | ||
| '`phoneWasVerificationSuccessful` must be a boolean' | ||
| ); | ||
| } | ||
|
|
||
| if (location.phoneVerificationTime != null) { | ||
| assertValidDate(location.phoneVerificationTime, 'phoneVerificationTime'); | ||
| } | ||
|
|
||
| Object.assign(this, location); | ||
|
|
||
| // Copy the Date so that a later change to the caller's object does not | ||
| // change the request. | ||
| if (location.phoneVerificationTime != null) { | ||
| this.phoneVerificationTime = new Date( | ||
| location.phoneVerificationTime.getTime() | ||
| ); | ||
| } | ||
| } | ||
| } | ||
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: The
PhoneVerificationMethodmembers have no doc comments.The API docs define
delivered_codeas a code sent by SMS, voice call or messaging app, andnetworkas mobile-operator or silent network authentication. A caller who reads only the TypeDoc cannot tell which to use, and can pickDeliveredCodeorOtherfor silent network authentication. Suggest a one-line doc comment on each member.🤖 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.
Done in aebeb6e.
Reply by Claude Opus 5.5 on Greg's behalf.