Repository navigation
STF-1906: Add phone verification inputs #475
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
8e61dc1
2e99c0c
cf3ab80
4f6a9b1
12450c7
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 |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
| import re | ||
| import urllib.parse | ||
| import uuid | ||
| from datetime import datetime | ||
| from decimal import Decimal | ||
| from typing import Any as AnyType | ||
|
|
||
|
|
@@ -21,6 +22,7 @@ | |
| All, | ||
| Any, | ||
| In, | ||
| Invalid, | ||
| Match, | ||
| MultipleInvalid, | ||
| Range, | ||
|
|
@@ -79,8 +81,30 @@ def _hostname(hostname: str) -> str: | |
| raise ValueError | ||
|
|
||
|
|
||
| def _datetime_exists(s: str) -> str: | ||
| # Reject impossible values such as Feb 30 or 24:00. The regex already | ||
| # checked the fraction and the offset, so parse only the part before them. | ||
| try: | ||
| datetime.strptime(s[:19], "%Y-%m-%dT%H:%M:%S") # noqa: DTZ007 | ||
| except ValueError as e: | ||
| msg = "invalid date or time" | ||
| raise Invalid(msg) from e | ||
| return s | ||
|
|
||
|
|
||
| _rfc3339_datetime = All( | ||
| Match( | ||
| r"(?a)\A\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?" | ||
| r"(?:Z|[+-](?:[01]\d|2[0-3]):[0-5]\d)\Z", | ||
| ), | ||
| _datetime_exists, | ||
| ) | ||
|
|
||
|
|
||
| _delivery_speed = In(["same_day", "overnight", "expedited", "standard"]) | ||
|
|
||
| _phone_verification_method = In(["delivered_code", "network", "other"]) | ||
|
|
||
| _address = { | ||
| "address": str, | ||
| "address_2": str, | ||
|
|
@@ -91,6 +115,9 @@ def _hostname(hostname: str) -> str: | |
| "last_name": str, | ||
| "phone_country_code": _telephone_country_code, | ||
| "phone_number": str, | ||
| "phone_verification_method": _phone_verification_method, | ||
| "phone_verification_time": _rfc3339_datetime, | ||
|
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.
The lower bound does not depend on the clock, so the client can check it with no clock-skew risk. The upper bound depends on the client clock, so skipping it is defensible, but the decision should be the same in all the sibling client PRs. The client also does not check the one-year range 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. No client checks this range, the same as for Reply by Claude Opus 5.5 on Greg's behalf. |
||
| "phone_was_verification_successful": bool, | ||
| "postal": str, | ||
| "region": _subdivision_iso_code, | ||
| } | ||
|
|
@@ -296,11 +323,6 @@ def _credit_card_token(s: str) -> str: | |
| raise ValueError | ||
|
|
||
|
|
||
| _rfc3339_datetime = Match( | ||
| r"(?a)\A\d{4}-\d{2}-\d{2}[Tt]\d{2}:\d{2}:\d{2}(\.\d+)?(?:[Zz]|[+-]\d{2}:\d{2})\Z", | ||
| ) | ||
|
|
||
|
|
||
| _event_party = In(["agent", "customer"]) | ||
|
|
||
| _event_type = In( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| from __future__ import annotations | ||
|
|
||
| import unittest | ||
| from datetime import datetime, timezone | ||
| from decimal import Decimal | ||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
|
|
@@ -11,6 +12,39 @@ | |
| if TYPE_CHECKING: | ||
| from collections.abc import Callable | ||
|
|
||
| GOOD_TIMES = ( | ||
| "2015-05-08T16:07:56+00:00", | ||
| "2015-05-08T16:07:56Z", | ||
| "2026-10-01T14:30:00Z", | ||
| "2026-10-01T14:30:00.123456789Z", | ||
| "2026-10-01T14:30:00+23:59", | ||
| "2024-02-29T00:00:00-05:00", | ||
| ) | ||
|
|
||
| BAD_TIMES = ( | ||
| "2015-05-08T16:07:56", | ||
| "2015-05-08 16:07:56Z", | ||
| "2026-02-30T25:61:61Z", | ||
|
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. No If 馃 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. Added in 4f6a9b1. Reply by Claude Opus 5.5 on Greg's behalf. |
||
| "2026-13-45T10:00:00Z", | ||
| "0000-00-00T00:00:00Z", | ||
| "2026-10-01T14:30:00+24:00", | ||
| "2026-10-01T14:30:00+99:99", | ||
| "2026-10-01T24:00:00Z", | ||
| "2026-10-01T14:30:60Z", | ||
| "2026-10-01t14:30:00Z", | ||
| "2026-10-01T14:30:00z", | ||
| "2026-10-01T14:30:00+25:00", | ||
| "2026-10-01T14:30:00+05:60", | ||
| "2026-02-30T10:00:00Z", | ||
| "2025-02-29T00:00:00Z", | ||
| "2026-04-31T00:00:00Z", | ||
| ) | ||
|
|
||
| NON_STRING_TIMES = ( | ||
| 1, | ||
| datetime(2026, 10, 1, 14, 30, tzinfo=timezone.utc), | ||
| ) | ||
|
|
||
|
|
||
| class ValidationBase(unittest.TestCase): | ||
| def setup_transaction(self, transaction: dict[str, Any]) -> None: | ||
|
|
@@ -140,6 +174,31 @@ def test_phone_country_code(self) -> None: | |
| for invalid in ("", "12345", "U"): | ||
| self.check_invalid_transaction({self.type: {"phone_country_code": invalid}}) | ||
|
|
||
| def test_phone_verification_method(self) -> None: | ||
| for good in ("delivered_code", "network", "other"): | ||
| self.check_transaction({self.type: {"phone_verification_method": good}}) | ||
| for bad in ("sms", 1, ""): | ||
| self.check_invalid_transaction( | ||
| {self.type: {"phone_verification_method": bad}}, | ||
| ) | ||
|
|
||
| def test_phone_was_verification_successful(self) -> None: | ||
| self.check_bool(self.type, "phone_was_verification_successful") | ||
|
|
||
| def test_phone_verification_time(self) -> None: | ||
|
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: this test covers only string inputs. A Python caller can easily pass a 馃 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. Added in 4f6a9b1. Reply by Claude Opus 5.5 on Greg's behalf. |
||
| for good in GOOD_TIMES: | ||
| self.check_transaction({self.type: {"phone_verification_time": good}}) | ||
| for bad in BAD_TIMES: | ||
| self.check_invalid_transaction( | ||
| {self.type: {"phone_verification_time": bad}}, | ||
| ) | ||
|
|
||
| def test_phone_verification_time_not_string(self) -> None: | ||
| for bad in NON_STRING_TIMES: | ||
| self.check_invalid_transaction( | ||
| {self.type: {"phone_verification_time": bad}}, | ||
| ) | ||
|
|
||
|
|
||
| class TestBillingAddress(AddressBase): | ||
| type = "billing" | ||
|
|
@@ -327,11 +386,22 @@ def test_shop_id(self) -> None: | |
| self.check_transaction_str_type("event", "shop_id") | ||
|
|
||
| def test_time(self) -> None: | ||
| for good in ("2015-05-08T16:07:56+00:00", "2015-05-08T16:07:56Z"): | ||
| for good in GOOD_TIMES: | ||
| self.check_transaction({"event": {"time": good}}) | ||
| for bad in ("2015-05-08T16:07:56", "2015-05-08 16:07:56Z"): | ||
| for bad in BAD_TIMES: | ||
| self.check_invalid_transaction({"event": {"time": bad}}) | ||
|
|
||
| def test_time_not_string(self) -> None: | ||
| for bad in NON_STRING_TIMES: | ||
| self.check_invalid_transaction({"event": {"time": bad}}) | ||
|
|
||
| def test_time_impossible_date_message(self) -> None: | ||
| transaction: dict[str, Any] = {"event": {"time": "2026-02-30T10:00:00Z"}} | ||
| self.setup_transaction(transaction) | ||
| with self.assertRaises(MultipleInvalid) as cm: | ||
| validate_transaction(transaction) | ||
| self.assertEqual(cm.exception.errors[0].msg, "invalid date or time") | ||
|
|
||
| def test_type(self) -> None: | ||
| for good in ( | ||
| "account_creation", | ||
|
|
||
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.
The fraction group
(\.\d+)?has no length limit. The server callsgetString(v, 255, false)before it parses the time, so it rejects a value longer than 255 characters."2026-10-01T14:30:00." + "1" * 300 + "Z"passesvalidate_transactionforevent.timeandphone_verification_time. The server rejects it with "the string is too long" and ignores the field. This is an edge case, but it conflicts with the changelog claim. A bound such as(\.\d{1,9})?(nanosecond precision, which Go keeps) would close it.馃 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.
We will leave this. It is an edge case, and the HISTORY entry no longer claims a full match.
Reply by Claude Opus 5.5 on Greg's behalf.