Fix external_shipment_id placement and test-mode store_id ticket persistence in the return-label flow
external_shipment_id was at the top level of the POST /v2/labels payload in both
_create_dummy_outbound_label() and create_return_label_from_dummy() - ShipStation's
schema only supports it nested under shipment, so it was silently ignored and
auto-generated ("SEAuto-...") on every label this flow ever created. Moved it inside
the shipment dict in both functions. Confirmed end-to-end on a real production
ticket (AR-166098): Order # now correctly shows the real ticket number, and
ship_from/ship_to are correct for a return.
Also: store_id is now only required in production - ShipStation's sandbox
environment cannot have stores/Order Sources at all (confirmed in the real sandbox
dashboard), so test mode omits it from the request instead of requiring an
impossible value. And three per-ticket write-backs (mark_shipstation_sent,
save_dummy_outbound_label_id, save_pack_data) were hardcoded to source == "jira",
silently no-opping for source == "test" tickets created by Create Test Shipment -
now match on ticket_number alone, since source is only ever "jira" or "test" and
ticket_number is already unique across both.
CLAUDE.md records the full investigation and closes out the long-standing
"return-label flow real-world verification" known-pending item.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
Claude Sonnet 5
@@ -623,7 +623,16 @@ def _create_dummy_outbound_label(
|
||||
shipment: dict = {
|
||||
"carrier_id": carrier_id,
|
||||
"service_code": service_code,
|
||||
"store_id": store_id,
|
||||
# Confirmed against ShipStation's own request schema for POST /v2/labels:
|
||||
# external_shipment_id is a field ON the shipment object
|
||||
# (shipment.external_shipment_id), there is no top-level equivalent for
|
||||
# this endpoint. Putting it at the top level (as this code used to) means
|
||||
# ShipStation silently ignores it and auto-generates its own "SEAuto-..."
|
||||
# placeholder instead - confirmed live: a real production dummy shipment
|
||||
# came back with external_shipment_id "SEAuto-..." instead of the ticket
|
||||
# number we sent, which is almost certainly why "Order #" never showed up
|
||||
# in ShipStation's UI for labels from this flow specifically.
|
||||
"external_shipment_id": external_shipment_id,
|
||||
"ship_to": {
|
||||
"name": info.get("name", ""),
|
||||
"phone": info.get("phone", ""),
|
||||
@@ -636,13 +645,17 @@ def _create_dummy_outbound_label(
|
||||
},
|
||||
"packages": [_build_return_package(1.0, 1.0, 1.0, 1.0)],
|
||||
}
|
||||
# Left out entirely when blank (test mode only - see
|
||||
# _validate_return_label_prerequisites) rather than sent as an empty string,
|
||||
# matching the confirmed-working shape from a live probe.
|
||||
if store_id:
|
||||
shipment["store_id"] = store_id
|
||||
if warehouse_id:
|
||||
shipment["warehouse_id"] = warehouse_id
|
||||
else:
|
||||
shipment["ship_from"] = _return_address_for_order(order)
|
||||
|
||||
payload = {
|
||||
"external_shipment_id": external_shipment_id,
|
||||
"shipment": shipment,
|
||||
}
|
||||
return _post_to_labels(payload, api_key)
|
||||
@@ -662,8 +675,17 @@ def _validate_return_label_prerequisites(order: Order) -> tuple[str, str, str, s
|
||||
"Add them in Settings under Return Labels."
|
||||
)
|
||||
|
||||
# store_id is only enforced in PRODUCTION. Confirmed directly in ShipStation's own
|
||||
# dashboard: the Sandbox environment cannot connect/create Order Sources ("stores")
|
||||
# at all - it explicitly says to switch to Production to do that. So no valid
|
||||
# TEST_SHIPSTATION_*_STORE_ID can ever exist; requiring one in test mode would make
|
||||
# this flow permanently untestable in sandbox. Confirmed via a live API probe that
|
||||
# omitting store_id entirely still succeeds (schema-optional, not just a guess) -
|
||||
# see _create_dummy_outbound_label()/create_return_label_from_dummy() for where it's
|
||||
# left out of the request when blank. Still required in production, where it's
|
||||
# achievable and matters for real (see the error message below).
|
||||
store_id = _store_id_for_company(order.company)
|
||||
if not store_id:
|
||||
if not store_id and not config.is_shipstation_test_mode():
|
||||
raise ShipStationSendError(
|
||||
f"No ShipStation Store ID configured for '{order.company}'. Add it in Settings "
|
||||
"under ShipStation. A label created without one may not show up anywhere in "
|
||||
@@ -738,33 +760,42 @@ def create_return_label_from_dummy(
|
||||
|
||||
ticket_number = order.ticket_number or order.external_id
|
||||
|
||||
payload = {
|
||||
shipment: dict = {
|
||||
"carrier_id": carrier_id,
|
||||
"service_code": service_code,
|
||||
# Nested here, not top-level - see _create_dummy_outbound_label() for why
|
||||
# (confirmed against ShipStation's own schema; a top-level
|
||||
# external_shipment_id is silently ignored on POST /v2/labels).
|
||||
"external_shipment_id": ticket_number,
|
||||
"ship_to": return_address,
|
||||
"ship_from": {
|
||||
"name": info.get("name", ""),
|
||||
"phone": info.get("phone", ""),
|
||||
"address_line1": info.get("address1", ""),
|
||||
"address_line2": info.get("address2", "") or None,
|
||||
"city_locality": info.get("city", ""),
|
||||
"state_province": info.get("state", ""),
|
||||
"postal_code": info.get("zip", ""),
|
||||
"country_code": "US",
|
||||
},
|
||||
"packages": [
|
||||
_build_return_package(
|
||||
p.get("weight_oz", 1.0), p.get("length", 0), p.get("width", 0), p.get("height", 0)
|
||||
)
|
||||
for p in packages
|
||||
],
|
||||
}
|
||||
# Left out entirely when blank (test mode only - see
|
||||
# _validate_return_label_prerequisites), matching the confirmed-working shape
|
||||
# from a live probe, rather than sent as an empty string.
|
||||
if store_id:
|
||||
shipment["store_id"] = store_id
|
||||
|
||||
payload = {
|
||||
"is_return_label": True,
|
||||
"outbound_label_id": dummy_label_id,
|
||||
"charge_event": resolved_charge_event,
|
||||
"shipment": {
|
||||
"carrier_id": carrier_id,
|
||||
"service_code": service_code,
|
||||
"store_id": store_id,
|
||||
"ship_to": return_address,
|
||||
"ship_from": {
|
||||
"name": info.get("name", ""),
|
||||
"phone": info.get("phone", ""),
|
||||
"address_line1": info.get("address1", ""),
|
||||
"address_line2": info.get("address2", "") or None,
|
||||
"city_locality": info.get("city", ""),
|
||||
"state_province": info.get("state", ""),
|
||||
"postal_code": info.get("zip", ""),
|
||||
"country_code": "US",
|
||||
},
|
||||
"packages": [
|
||||
_build_return_package(
|
||||
p.get("weight_oz", 1.0), p.get("length", 0), p.get("width", 0), p.get("height", 0)
|
||||
)
|
||||
for p in packages
|
||||
],
|
||||
},
|
||||
"shipment": shipment,
|
||||
}
|
||||
|
||||
data = _post_to_labels(payload, api_key)
|
||||
|
||||
+16
-5
@@ -443,11 +443,16 @@ def load_orders_by_view() -> Tuple[List[Order], List[Order], List[Order]]:
|
||||
|
||||
def mark_shipstation_sent(ticket_number: str) -> None:
|
||||
"""Stamps shipstation_sent_at after a CONFIRMED emergency API send -
|
||||
called once ShipStation's own response confirms creation succeeded."""
|
||||
called once ShipStation's own response confirms creation succeeded.
|
||||
|
||||
Matches by ticket_number alone, not source == "jira" - source is only
|
||||
ever "jira" or "test" (synthetic tickets from create_test_shipment_order()),
|
||||
and ticket_number is already unique across both, so restricting to "jira"
|
||||
here just means this silently no-ops for test tickets instead of erroring."""
|
||||
session = get_session()
|
||||
try:
|
||||
order = session.execute(
|
||||
select(Order).where(Order.source == "jira", Order.ticket_number == ticket_number)
|
||||
select(Order).where(Order.ticket_number == ticket_number)
|
||||
).scalar_one_or_none()
|
||||
if order is not None:
|
||||
order.shipstation_sent_at = dt.datetime.now()
|
||||
@@ -459,11 +464,14 @@ def mark_shipstation_sent(ticket_number: str) -> None:
|
||||
def save_dummy_outbound_label_id(ticket_number: str, label_id: str) -> None:
|
||||
"""Persists step 1's result (the dummy shipment's label_id) so step 2
|
||||
(the real return label) can be triggered separately - including in a
|
||||
later session, after the dummy has been verified in ShipStation."""
|
||||
later session, after the dummy has been verified in ShipStation.
|
||||
|
||||
Matches by ticket_number alone - see mark_shipstation_sent() above for why
|
||||
this must not be restricted to source == "jira"."""
|
||||
session = get_session()
|
||||
try:
|
||||
order = session.execute(
|
||||
select(Order).where(Order.source == "jira", Order.ticket_number == ticket_number)
|
||||
select(Order).where(Order.ticket_number == ticket_number)
|
||||
).scalar_one_or_none()
|
||||
if order is not None:
|
||||
order.dummy_outbound_label_id = label_id
|
||||
@@ -478,11 +486,14 @@ def save_pack_data(ticket_number: str, serial_numbers: dict, packed: bool) -> No
|
||||
dialog. Staff-entered data, not sourced from JIRA - this is the
|
||||
beginning of the eventual end-of-day push back to JIRA (deferred for
|
||||
now), so nothing here gets overwritten by a JIRA re-import.
|
||||
|
||||
Matches by ticket_number alone, not source == "jira" - see
|
||||
mark_shipstation_sent() for why.
|
||||
"""
|
||||
session = get_session()
|
||||
try:
|
||||
order = session.execute(
|
||||
select(Order).where(Order.source == "jira", Order.ticket_number == ticket_number)
|
||||
select(Order).where(Order.ticket_number == ticket_number)
|
||||
).scalar_one_or_none()
|
||||
if order is None:
|
||||
return
|
||||
|
||||
Reference in New Issue
Block a user