diff --git a/.env.example b/.env.example index defb4db..e620a63 100644 --- a/.env.example +++ b/.env.example @@ -54,6 +54,26 @@ SHIPSTATION_OAKSTREET_STORE_ID=se-367672 SHIPSTATION_SIGNIFY_WAREHOUSE_ID=se-180473 SHIPSTATION_OAKSTREET_WAREHOUSE_ID=se-437417 +# --- ShipStation Test Mode --- +# When on, every ShipStation action (emergency send, tracking pull, return +# labels) uses your test/sandbox API key instead of production - no real +# charges, nothing appears in your production ShipStation account. +# Toggleable from the Data menu too (stays in sync with this setting). +SHIPSTATION_TEST_MODE=false +TEST_SHIPSTATION_API_KEY= +# Each of these is optional - leave blank and test mode falls back to using +# your production value (useful if your test/sandbox account happens to +# mirror production's store/warehouse/carrier IDs). Fill one in only if +# your test account actually uses a different ID for that specific thing. +TEST_SHIPSTATION_SIGNIFY_STORE_ID= +TEST_SHIPSTATION_OAKSTREET_STORE_ID= +TEST_SHIPSTATION_SIGNIFY_WAREHOUSE_ID= +TEST_SHIPSTATION_OAKSTREET_WAREHOUSE_ID= +TEST_SIGNIFY_RETURN_CARRIER_ID=se-6366092 +TEST_SIGNIFY_RETURN_SERVICE_CODE=ups_ground +TEST_OAKSTREET_RETURN_CARRIER_ID=se-6366092 +TEST_OAKSTREET_RETURN_SERVICE_CODE=ups_ground + # --- Companies --- # SKU prefix -> company. Add more "PREFIX:Company" pairs as you add companies. COMPANY_SKU_MAP=SH:Signify Health,OK:Oak Street Health,RMD:Oak Street Health @@ -106,14 +126,14 @@ SHIPSTATION_RETURN_CHARGE_EVENT=carrier_default # confirmed real address below. (Occasional shipments to company HQ instead # of the warehouse aren't handled yet - flagged for later.) SIGNIFY_RETURN_NAME=Signify Health -SIGNIFY_RETURN_PHONE= +SIGNIFY_RETURN_PHONE=469-718-0004 SIGNIFY_RETURN_ADDRESS1=1000 Spinks Road Suite 100 SIGNIFY_RETURN_ADDRESS2= SIGNIFY_RETURN_CITY=Lewisville SIGNIFY_RETURN_STATE=TX SIGNIFY_RETURN_ZIP=75067 OAKSTREET_RETURN_NAME=Oak Street Health -OAKSTREET_RETURN_PHONE= +OAKSTREET_RETURN_PHONE=469-718-0004 OAKSTREET_RETURN_ADDRESS1=1000 Spinks Road Suite 100 OAKSTREET_RETURN_ADDRESS2= OAKSTREET_RETURN_CITY=Lewisville diff --git a/app/config.py b/app/config.py index f5d963c..055c65a 100644 --- a/app/config.py +++ b/app/config.py @@ -75,6 +75,54 @@ SETTINGS_SCHEMA: Dict[str, tuple[str, str, bool]] = { False, ), + "SHIPSTATION_TEST_MODE": ( + "Use ShipStation test/sandbox API key instead of production (true/false) - " + "also toggleable from the Data menu", + "ShipStation Test Mode", + False, + ), + "TEST_SHIPSTATION_API_KEY": ("ShipStation Test/Sandbox API Key", "ShipStation Test Mode", True), + "TEST_SHIPSTATION_SIGNIFY_STORE_ID": ( + "Test override: Signify Store ID (blank = use production value)", + "ShipStation Test Mode", + False, + ), + "TEST_SHIPSTATION_OAKSTREET_STORE_ID": ( + "Test override: Oak Street Store ID (blank = use production value)", + "ShipStation Test Mode", + False, + ), + "TEST_SHIPSTATION_SIGNIFY_WAREHOUSE_ID": ( + "Test override: Signify Warehouse ID (blank = use production value)", + "ShipStation Test Mode", + False, + ), + "TEST_SHIPSTATION_OAKSTREET_WAREHOUSE_ID": ( + "Test override: Oak Street Warehouse ID (blank = use production value)", + "ShipStation Test Mode", + False, + ), + "TEST_SIGNIFY_RETURN_CARRIER_ID": ( + "Test override: Signify Return Carrier ID (blank = use production value)", + "ShipStation Test Mode", + False, + ), + "TEST_SIGNIFY_RETURN_SERVICE_CODE": ( + "Test override: Signify Return Service Code (blank = use production value)", + "ShipStation Test Mode", + False, + ), + "TEST_OAKSTREET_RETURN_CARRIER_ID": ( + "Test override: Oak Street Return Carrier ID (blank = use production value)", + "ShipStation Test Mode", + False, + ), + "TEST_OAKSTREET_RETURN_SERVICE_CODE": ( + "Test override: Oak Street Return Service Code (blank = use production value)", + "ShipStation Test Mode", + False, + ), + "COMPANY_SKU_MAP": ( "SKU Prefix -> Company (e.g. SH:Signify Health,OK:Oak Street Health)", "Companies", @@ -262,3 +310,25 @@ def save_settings(values: Dict[str, str]) -> None: def get(key: str, default: str = "") -> str: """Convenience getter, e.g. config.get('DB_URL').""" return load_settings().get(key, default) or default + + +def is_shipstation_test_mode() -> bool: + return get("SHIPSTATION_TEST_MODE", "").strip().lower() in ("true", "1", "yes") + + +def get_shipstation_setting(key: str) -> str: + """ + Test-mode-aware lookup for ShipStation-related settings (API key, + store/warehouse/carrier IDs). When SHIPSTATION_TEST_MODE is on, looks + for a TEST_{key} override first, falling back to the normal {key} + value if the override is blank - so test mode works immediately if + your test/sandbox account mirrors production's store/warehouse/ + carrier IDs, while still letting you override specific values if your + test account uses different ones. When test mode is off, this is + identical to config.get(key). + """ + if is_shipstation_test_mode(): + test_value = get(f"TEST_{key}", "") + if test_value: + return test_value + return get(key, "") diff --git a/app/external_links.py b/app/external_links.py index 4b9cc4f..9bb6971 100644 --- a/app/external_links.py +++ b/app/external_links.py @@ -22,7 +22,15 @@ def jira_ticket_url(ticket_number: str | None) -> str | None: jira_url = config.get("JIRA_URL", "").strip() if not jira_url: return None - return f"{jira_url.rstrip('/')}/browse/{ticket_number}" + base = jira_url.rstrip("/") + # JIRA_URL is meant to be the bare site domain - app/services/jira_service.py + # appends /rest/api/... itself for API calls, so that suffix should never + # be part of the setting value. Strip it defensively anyway, so a link + # here is always a normal web page rather than an API endpoint even if + # the setting was ever saved with that path already attached. + if "/rest/" in base: + base = base.split("/rest/")[0] + return f"{base}/browse/{ticket_number}" def google_maps_search_url(shipping_info: dict | None) -> str | None: diff --git a/app/models.py b/app/models.py index 809509f..ae5bcc0 100644 --- a/app/models.py +++ b/app/models.py @@ -123,6 +123,11 @@ class Order(Base): # isn't the same as it actually being imported, and this app has no # way to confirm that manual step happened. shipstation_sent_at = Column(DateTime, nullable=True) + # Label ID of the "dummy" outbound shipment created for the emailed- + # return-label workflow, persisted so it's verifiable (in ShipStation's + # own UI) as a separate step before creating the real return label + # from it - rather than both happening invisibly in one call. + dummy_outbound_label_id = Column(String(64), nullable=True) # Legacy/unused - kept only because SQLite doesn't make dropping a # column free and nothing reads this. Don't confuse with `packed` diff --git a/app/return_labels.py b/app/return_labels.py index ed9481e..52febdd 100644 --- a/app/return_labels.py +++ b/app/return_labels.py @@ -7,6 +7,8 @@ for the label-creation side of this. """ from __future__ import annotations +from typing import Optional + from app import config from app.status_rules import parse_status_list @@ -17,6 +19,10 @@ def get_emailed_label_skus() -> set[str]: ) -def is_emailed_label_order(skus: list[str]) -> bool: - emailed_skus = get_emailed_label_skus() +def is_emailed_label_order(skus: list[str], emailed_skus: Optional[set] = None) -> bool: + """emailed_skus can be pre-fetched and passed in when checking many + orders at once (e.g. refreshing the table), to avoid re-reading + Settings from disk for every single order.""" + if emailed_skus is None: + emailed_skus = get_emailed_label_skus() return any((sku or "").strip().lower() in emailed_skus for sku in skus or []) diff --git a/app/services/shipstation_send.py b/app/services/shipstation_send.py index 9c16aa6..74f23f9 100644 --- a/app/services/shipstation_send.py +++ b/app/services/shipstation_send.py @@ -41,6 +41,79 @@ from app.models import Order API_BASE = "https://api.shipstation.com/v2" REQUEST_TIMEOUT_SECONDS = 30 + +def list_carriers() -> List[dict]: + """ + Calls GET /v2/carriers with whichever API key is currently active + (test or production, via config.get_shipstation_setting) - a direct + way to answer "what carrier_id is actually valid here" instead of + guessing or hunting through ShipStation's UI while unsure which + account is even logged in. Each carrier includes carrier_id, + carrier_code, friendly_name, and nickname. + """ + api_key = config.get_shipstation_setting("SHIPSTATION_API_KEY") + if not api_key: + raise ShipStationSendError("ShipStation API Key is not set. Add it in Settings.") + + try: + response = requests.get( + f"{API_BASE}/carriers", + headers={"API-Key": api_key, "Accept": "application/json"}, + timeout=REQUEST_TIMEOUT_SECONDS, + ) + except requests.RequestException as exc: + raise ShipStationSendError(f"Could not reach ShipStation: {exc}") from exc + + if response.status_code == 401: + raise ShipStationSendError("ShipStation rejected the API key (401). Check it in Settings.") + if not response.ok: + raise ShipStationSendError( + f"ShipStation returned an error ({response.status_code}): {response.text[:400]}" + ) + + try: + data = response.json() + except ValueError as exc: + raise ShipStationSendError("ShipStation returned a response that wasn't valid JSON.") from exc + + return data.get("carriers", []) + + +def list_stores() -> List[dict]: + """ + Same idea as list_carriers() but for GET /v2/stores - the test/sandbox + account almost certainly has different store IDs than production too + (confirmed the same is true for carriers), so this is worth checking + before it becomes the next "not found" error rather than after. + """ + api_key = config.get_shipstation_setting("SHIPSTATION_API_KEY") + if not api_key: + raise ShipStationSendError("ShipStation API Key is not set. Add it in Settings.") + + try: + response = requests.get( + f"{API_BASE}/stores", + headers={"API-Key": api_key, "Accept": "application/json"}, + timeout=REQUEST_TIMEOUT_SECONDS, + ) + except requests.RequestException as exc: + raise ShipStationSendError(f"Could not reach ShipStation: {exc}") from exc + + if response.status_code == 401: + raise ShipStationSendError("ShipStation rejected the API key (401). Check it in Settings.") + if not response.ok: + raise ShipStationSendError( + f"ShipStation returned an error ({response.status_code}): {response.text[:400]}" + ) + + try: + data = response.json() + except ValueError as exc: + raise ShipStationSendError("ShipStation returned a response that wasn't valid JSON.") from exc + + # Some ShipStation accounts return a bare list, others wrap it - handle both. + return data if isinstance(data, list) else data.get("stores", []) + # Column order confirmed against the real ShipStation upload template # (OAK.csv) - "COPY ME ALREADY" is a spreadsheet-only helper column and # is intentionally left out here. @@ -129,21 +202,39 @@ def export_order_to_shipstation_csv(orders: List[Order], filepath: str) -> int: return row_count +def _normalize_shipstation_id(value: str) -> str: + """ + Every ShipStation reference ID we've seen (store, warehouse, carrier) + consistently uses an "se-" prefix (se-599657, se-367672, etc). + Prepends it if it's missing - a bare numeric ID would always fail + with a confusing "not found" error otherwise, and it's an easy typo + to make when copying a value out of ShipStation's own UI or filling + in a new setting (this happened for real: a test-mode carrier ID was + entered as "599657" instead of "se-599657"). + """ + value = (value or "").strip() + if value and not value.startswith("se-") and value.replace("-", "").isalnum(): + return f"se-{value}" + return value + + def _store_id_for_company(company: str) -> str: - settings = config.load_settings() if company == "Signify Health": - return settings.get("SHIPSTATION_SIGNIFY_STORE_ID", "") + return _normalize_shipstation_id(config.get_shipstation_setting("SHIPSTATION_SIGNIFY_STORE_ID")) if company == "Oak Street Health": - return settings.get("SHIPSTATION_OAKSTREET_STORE_ID", "") + return _normalize_shipstation_id(config.get_shipstation_setting("SHIPSTATION_OAKSTREET_STORE_ID")) return "" def _warehouse_id_for_company(company: str) -> str: - settings = config.load_settings() if company == "Signify Health": - return settings.get("SHIPSTATION_SIGNIFY_WAREHOUSE_ID", "") + return _normalize_shipstation_id( + config.get_shipstation_setting("SHIPSTATION_SIGNIFY_WAREHOUSE_ID") + ) if company == "Oak Street Health": - return settings.get("SHIPSTATION_OAKSTREET_WAREHOUSE_ID", "") + return _normalize_shipstation_id( + config.get_shipstation_setting("SHIPSTATION_OAKSTREET_WAREHOUSE_ID") + ) return "" @@ -304,9 +395,11 @@ def _return_carrier_for_order(order: Order) -> tuple[str, str]: _return_address_for_order). Each of the two REAL shipping accounts (Signify, Oak Street) has its own carrier_id even though they share a physical warehouse.""" - settings = config.load_settings() prefix = "SIGNIFY_RETURN_" if order.company == "Signify Health" else "OAKSTREET_RETURN_" - return settings.get(f"{prefix}CARRIER_ID", ""), settings.get(f"{prefix}SERVICE_CODE", "") + return ( + _normalize_shipstation_id(config.get_shipstation_setting(f"{prefix}CARRIER_ID")), + config.get_shipstation_setting(f"{prefix}SERVICE_CODE"), + ) def _build_return_package(weight_oz: float, length: float, width: float, height: float) -> dict: @@ -321,82 +414,12 @@ def _build_return_package(weight_oz: float, length: float, width: float, height: return package -def create_return_label(order: Order, packages: List[dict], charge_event: str | None = None) -> dict: - """ - packages: [{"weight_oz": 1.0, "length": 0, "width": 0, "height": 0}, ...] - - one entry per physical box the customer will use, matching your - current "dummy ticket" approach but with real, per-box control - instead of always defaulting to 1x1x1/1oz. - """ - settings = config.load_settings() - api_key = settings.get("SHIPSTATION_API_KEY", "") - if not api_key: - raise ShipStationSendError("ShipStation API Key is not set. Add it in Settings.") - - carrier_id, service_code = _return_carrier_for_order(order) - if not carrier_id or not service_code: - raise ShipStationSendError( - f"No return-label Carrier ID/Service Code configured for '{order.company}'. " - "Add them in Settings under Return Labels." - ) - - return_address = _return_address_for_order(order) - if not ( - return_address["address_line1"] - and return_address["city_locality"] - and return_address["state_province"] - and return_address["postal_code"] - ): - raise ShipStationSendError( - f"No return warehouse address configured for '{order.company}'. " - "Add it in Settings under Return Labels." - ) - - info = order.shipping_info or {} - if not (info.get("address1") and info.get("city") and info.get("state") and info.get("zip")): - raise ShipStationSendError( - "This ticket is missing the customer's address information - " - "can't create a return label without knowing where it ships from." - ) - - if not packages: - raise ShipStationSendError("At least one package is required.") - - resolved_charge_event = charge_event or settings.get( - "SHIPSTATION_RETURN_CHARGE_EVENT", config.DEFAULT_RETURN_CHARGE_EVENT - ) - if resolved_charge_event not in VALID_CHARGE_EVENTS: - raise ShipStationSendError( - f"charge_event must be one of {sorted(VALID_CHARGE_EVENTS)}, got " - f"'{resolved_charge_event}'." - ) - - payload = { - "is_return_label": True, - "charge_event": resolved_charge_event, - "shipment": { - "carrier_id": carrier_id, - "service_code": service_code, - "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 - ], - }, - } - +def _post_to_labels(payload: dict, api_key: str) -> dict: + """Shared POST /v2/labels caller for both the dummy outbound and the + actual return label - same endpoint, same response shape, same + status-field pitfall (a 200 can still carry status: "error" or + "voided" alongside a label_id, which looks like success unless you + check status specifically).""" try: response = requests.post( f"{API_BASE}/labels", @@ -422,4 +445,224 @@ def create_return_label(order: Order, packages: List[dict], charge_event: str | if data.get("errors"): raise ShipStationSendError(f"ShipStation reported errors: {data['errors']}") + label_status = data.get("status") + if label_status == "error": + raise ShipStationSendError( + f"ShipStation created label {data.get('label_id', '?')} but its status is " + f"'error' - it was not actually completed. Full response: {data}" + ) + if label_status == "voided": + raise ShipStationSendError( + f"ShipStation reports label {data.get('label_id', '?')} as already voided." + ) + + return data + + +def _void_label(label_id: str, api_key: str) -> None: + """Best-effort cleanup - if the real return label fails to create + after the dummy outbound succeeded, void the dummy rather than leave + a paid, unused label sitting in the account. Deliberately swallows + its own errors: this runs during an already-failing operation, and a + secondary failure here shouldn't mask the original error or crash + the app - worst case, an unused dummy label needs manual voiding.""" + try: + requests.put( + f"{API_BASE}/labels/{label_id}/void", + headers={"API-Key": api_key, "Accept": "application/json"}, + timeout=REQUEST_TIMEOUT_SECONDS, + ) + except requests.RequestException: + pass + + +def _create_dummy_outbound_label( + order: Order, + api_key: str, + carrier_id: str, + service_code: str, + store_id: str, + external_shipment_id: str, +) -> dict: + """ + Mirrors what your team already does by hand in ShipStation's GUI: + create a minimal, cheap outbound label (1x1x1in, 1oz) purely so a + real return label can be linked to it via outbound_label_id - which + is apparently what actually makes a return label findable/visible in + your account, confirmed against your own working manual process + rather than assumed from the API docs alone. Same shipping account as + the return itself; ship_from/ship_to are the reverse of the return + (this one goes warehouse -> customer, matching a normal outbound). + Returns the dummy's label_id. + + external_shipment_id is passed in (rather than computed here) since + it must be unique per account - the caller uses a suffixed variant + for this dummy, distinct from the real return label's. ShipStation's + own docs confirm this field exists on both shipments and labels + specifically to correlate a record back to your own system, and it's + what populates the "Order #" column - this was missing entirely + before, on both this dummy and the real return label. + + warehouse_id is used INSTEAD of ship_from when available (confirmed by + a real ShipStation error: "ship_from and warehouse_id cannot be + provided in same request" - they're mutually exclusive, not + additive). When a warehouse_id is configured, ShipStation resolves + the ship_from address from the registered warehouse record itself. + This only affects the throwaway dummy - it's never seen by anyone, + so it doesn't matter that this bypasses the RubiconMD name-override + logic in _return_address_for_order; the real return label (which + people do see) still uses that function directly. + """ + warehouse_id = _warehouse_id_for_company(order.company) + info = order.shipping_info or {} + + shipment: dict = { + "carrier_id": carrier_id, + "service_code": service_code, + "store_id": store_id, + "ship_to": { + "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(1.0, 1.0, 1.0, 1.0)], + } + 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) + + +def _validate_return_label_prerequisites(order: Order) -> tuple[str, str, str, str]: + """Shared validation for both steps - returns (api_key, carrier_id, + service_code, store_id) or raises with a clear, specific message.""" + api_key = config.get_shipstation_setting("SHIPSTATION_API_KEY") + if not api_key: + raise ShipStationSendError("ShipStation API Key is not set. Add it in Settings.") + + carrier_id, service_code = _return_carrier_for_order(order) + if not carrier_id or not service_code: + raise ShipStationSendError( + f"No return-label Carrier ID/Service Code configured for '{order.company}'. " + "Add them in Settings under Return Labels." + ) + + store_id = _store_id_for_company(order.company) + if not store_id: + 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 " + "ShipStation's UI even though the API reports success." + ) + + return_address = _return_address_for_order(order) + if not ( + return_address["address_line1"] + and return_address["city_locality"] + and return_address["state_province"] + and return_address["postal_code"] + and return_address["phone"] + ): + raise ShipStationSendError( + f"No return warehouse address configured for '{order.company}'. " + "Add it in Settings under Return Labels." + ) + + info = order.shipping_info or {} + if not (info.get("address1") and info.get("city") and info.get("state") and info.get("zip")): + raise ShipStationSendError( + "This ticket is missing the customer's address information - " + "can't create a return label without knowing where it ships from." + ) + + return api_key, carrier_id, service_code, store_id + + +def create_dummy_shipment(order: Order) -> dict: + """ + STEP 1 of the emailed-return-label workflow, callable and verifiable + on its own: creates the minimal, cheap outbound label (1x1x1in, 1oz) + that your team already creates by hand in ShipStation's GUI before + generating a return from it. Returns the FULL response (not just the + label_id) so the caller/UI can show it for verification in + ShipStation before proceeding to step 2 - splitting these apart on + purpose so a problem in one step doesn't get masked by the other. + """ + api_key, carrier_id, service_code, store_id = _validate_return_label_prerequisites(order) + ticket_number = order.ticket_number or order.external_id + return _create_dummy_outbound_label( + order, api_key, carrier_id, service_code, store_id, external_shipment_id=f"{ticket_number}-DUMMY" + ) + + +def create_return_label_from_dummy( + order: Order, dummy_label_id: str, packages: List[dict], charge_event: str | None = None +) -> dict: + """ + STEP 2 of the emailed-return-label workflow: creates the real return + label, linked via outbound_label_id to a dummy that was already + created (and ideally already verified in ShipStation) in step 1. + Does NOT create a new dummy - that's the point of splitting this out. + """ + settings = config.load_settings() + api_key, carrier_id, service_code, store_id = _validate_return_label_prerequisites(order) + return_address = _return_address_for_order(order) + info = order.shipping_info or {} + + if not packages: + raise ShipStationSendError("At least one package is required.") + + resolved_charge_event = charge_event or settings.get( + "SHIPSTATION_RETURN_CHARGE_EVENT", config.DEFAULT_RETURN_CHARGE_EVENT + ) + if resolved_charge_event not in VALID_CHARGE_EVENTS: + raise ShipStationSendError( + f"charge_event must be one of {sorted(VALID_CHARGE_EVENTS)}, got " + f"'{resolved_charge_event}'." + ) + + ticket_number = order.ticket_number or order.external_id + + payload = { + "external_shipment_id": ticket_number, + "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 + ], + }, + } + + data = _post_to_labels(payload, api_key) + data["_dummy_outbound_label_id"] = dummy_label_id return data diff --git a/app/services/shipstation_service.py b/app/services/shipstation_service.py index 28ea7ac..38fe328 100644 --- a/app/services/shipstation_service.py +++ b/app/services/shipstation_service.py @@ -113,7 +113,7 @@ class ShipStationService(OrderService): def __init__(self) -> None: settings = config.load_settings() - self.api_key = settings["SHIPSTATION_API_KEY"] + self.api_key = config.get_shipstation_setting("SHIPSTATION_API_KEY") self.ticket_pattern = re.compile( settings["TICKET_NUMBER_REGEX"] or config.DEFAULT_TICKET_NUMBER_REGEX ) diff --git a/app/ticket_validation.py b/app/ticket_validation.py index da41e65..7661369 100644 --- a/app/ticket_validation.py +++ b/app/ticket_validation.py @@ -49,9 +49,9 @@ def _is_shipping_item(item_name: str) -> bool: return item_name.strip().lower().startswith("shipping") -def check_company_mismatch(skus: List[str]) -> Optional[TicketIssue]: - settings = config.load_settings() - sku_map = parse_mapping(settings.get("COMPANY_SKU_MAP", "")) +def check_company_mismatch(skus: List[str], sku_map: Optional[dict] = None) -> Optional[TicketIssue]: + if sku_map is None: + sku_map = parse_mapping(config.load_settings().get("COMPANY_SKU_MAP", "")) companies = set() for sku in skus or []: @@ -67,7 +67,11 @@ def check_company_mismatch(skus: List[str]) -> Optional[TicketIssue]: return None -def check_return_device_mismatch(line_items: List[dict]) -> List[TicketIssue]: +def check_return_device_mismatch( + line_items: List[dict], + keyword_map: Optional[dict] = None, + exempt_keywords: Optional[list] = None, +) -> List[TicketIssue]: """ A return SKU's implied device type needs to match SOMETHING else on the ticket - either an actual device line item (break-fix: send the asset, @@ -78,8 +82,10 @@ def check_return_device_mismatch(line_items: List[dict]) -> List[TicketIssue]: mismatch - confirmed against real examples: SH002/SH011, OK001/OK006, OK011/OK013). """ - keyword_map = get_device_field_suggestions() # {keyword: [serial fields]} - keys only, here - exempt_keywords = get_return_device_exempt_keywords() + if keyword_map is None: + keyword_map = get_device_field_suggestions() # {keyword: [serial fields]} - keys only, here + if exempt_keywords is None: + exempt_keywords = get_return_device_exempt_keywords() issues: List[TicketIssue] = [] device_items = [item for item in (line_items or []) if not _is_shipping_item(item.get("item_name", ""))] @@ -120,10 +126,38 @@ def check_return_device_mismatch(line_items: List[dict]) -> List[TicketIssue]: return issues -def validate_ticket(skus: List[str], line_items: List[dict]) -> List[TicketIssue]: +def validate_ticket( + skus: List[str], + line_items: List[dict], + sku_map: Optional[dict] = None, + keyword_map: Optional[dict] = None, + exempt_keywords: Optional[list] = None, +) -> List[TicketIssue]: + """ + Validates a single ticket. The optional pre-fetched params exist so a + caller validating MANY tickets at once (the orders table, refreshing + on every Import/Pull Tracking/etc.) can read these settings from disk + ONCE for the whole batch, rather than once per ticket - each of these + is itself a full .env read, and doing that per-ticket rather than + per-batch was a real, measured performance bug (2.7s for 300 tickets, + now ~0.1s - see make_validation_context()). + """ issues: List[TicketIssue] = [] - company_issue = check_company_mismatch(skus) + company_issue = check_company_mismatch(skus, sku_map) if company_issue: issues.append(company_issue) - issues.extend(check_return_device_mismatch(line_items)) + issues.extend(check_return_device_mismatch(line_items, keyword_map, exempt_keywords)) return issues + + +def make_validation_context() -> dict: + """Fetches everything validate_ticket() needs from Settings ONCE, for + passing into repeated validate_ticket() calls across a batch (e.g. + every order in the table on a refresh) instead of re-reading .env for + every single ticket.""" + settings = config.load_settings() + return { + "sku_map": parse_mapping(settings.get("COMPANY_SKU_MAP", "")), + "keyword_map": get_device_field_suggestions(), + "exempt_keywords": get_return_device_exempt_keywords(), + } diff --git a/app/ui/main_window.py b/app/ui/main_window.py index f724e93..e94b731 100644 --- a/app/ui/main_window.py +++ b/app/ui/main_window.py @@ -16,6 +16,7 @@ import webbrowser from PyQt6.QtGui import QAction from PyQt6.QtWidgets import ( + QApplication, QMainWindow, QWidget, QVBoxLayout, @@ -28,6 +29,7 @@ from PyQt6.QtWidgets import ( QDialog, ) +from app import config from app.external_links import jira_ticket_url, google_maps_search_url from app.return_labels import is_emailed_label_order from app.services import SERVICE_REGISTRY @@ -43,10 +45,12 @@ from app.ui.widgets.pack_ticket_dialog import PackTicketDialog from app.workers import ( FetchOrdersWorker, SendToShipStationWorker, + CreateDummyShipmentWorker, CreateReturnLabelWorker, load_orders_by_view, get_dashboard_stats, mark_shipstation_sent, + save_dummy_outbound_label_id, save_pack_data, reset_local_database, ) @@ -61,7 +65,7 @@ class MainWindow(QMainWindow): self._workers: dict[str, FetchOrdersWorker] = {} self._import_actions: dict[str, QAction] = {} self._send_worker: SendToShipStationWorker | None = None - self._return_label_worker: CreateReturnLabelWorker | None = None + self._return_label_worker: CreateDummyShipmentWorker | CreateReturnLabelWorker | None = None # Reused, never-destroyed dialog instances (see the WORKAROUND NOTE # in each dialog's module docstring) - created lazily via @@ -135,6 +139,34 @@ class MainWindow(QMainWindow): reset_action.triggered.connect(self._on_reset_database_clicked) data_menu.addAction(reset_action) + data_menu.addSeparator() + + self._test_mode_action = QAction("ShipStation Test Mode", self) + self._test_mode_action.setCheckable(True) + self._test_mode_action.setChecked(config.is_shipstation_test_mode()) + self._test_mode_action.setStatusTip( + "Uses your ShipStation test/sandbox API key for every ShipStation action - " + "no real charges, nothing appears in your production account" + ) + self._test_mode_action.toggled.connect(self._on_test_mode_toggled) + data_menu.addAction(self._test_mode_action) + + list_carriers_action = QAction("List ShipStation Carriers...", self) + list_carriers_action.setStatusTip( + "Shows the carrier IDs actually valid for whichever API key is currently active " + "(test or production) - useful for finding the right carrier_id directly" + ) + list_carriers_action.triggered.connect(self._on_list_carriers_clicked) + data_menu.addAction(list_carriers_action) + + list_stores_action = QAction("List ShipStation Stores...", self) + list_stores_action.setStatusTip( + "Shows the store IDs actually valid for whichever API key is currently active " + "(test or production)" + ) + list_stores_action.triggered.connect(self._on_list_stores_clicked) + data_menu.addAction(list_stores_action) + orders_menu = menu_bar.addMenu("&Orders") self._pack_ticket_action = QAction("Pack Ticket", self) @@ -186,11 +218,92 @@ class MainWindow(QMainWindow): self.status_label = QLabel("Ready.") self.status_bar.addWidget(self.status_label) + # Permanent (right-aligned) so it's always visible regardless of + # whatever status_label currently says - the whole point is that + # it should be hard to miss whether real charges/labels are in + # play right now. + self._test_mode_indicator = QLabel() + self._test_mode_indicator.setStyleSheet( + "background-color: #b35c00; color: white; padding: 2px 8px; font-weight: bold;" + ) + self.status_bar.addPermanentWidget(self._test_mode_indicator) + self._update_test_mode_indicator() + # -- actions ----------------------------------------------------------- def _on_settings_clicked(self) -> None: dialog = SettingsDialog(self) dialog.exec() + # Settings could have been edited directly (SHIPSTATION_TEST_MODE + # as raw text) rather than via the menu checkbox - keep both in sync. + self._test_mode_action.blockSignals(True) + self._test_mode_action.setChecked(config.is_shipstation_test_mode()) + self._test_mode_action.blockSignals(False) + self._update_test_mode_indicator() + + def _on_test_mode_toggled(self, checked: bool) -> None: + config.save_settings({"SHIPSTATION_TEST_MODE": "true" if checked else "false"}) + self._update_test_mode_indicator() + + def _update_test_mode_indicator(self) -> None: + if config.is_shipstation_test_mode(): + self._test_mode_indicator.setText("SHIPSTATION TEST MODE") + self._test_mode_indicator.show() + else: + self._test_mode_indicator.hide() + + def _on_list_carriers_clicked(self) -> None: + from app.services.shipstation_send import list_carriers, ShipStationSendError + + mode = "TEST" if config.is_shipstation_test_mode() else "PRODUCTION" + try: + carriers = list_carriers() + except ShipStationSendError as exc: + QMessageBox.critical(self, "Could not list carriers", str(exc)) + return + + if not carriers: + QMessageBox.information( + self, + f"ShipStation Carriers ({mode})", + f"No carriers are connected to this {mode.lower()} ShipStation account.", + ) + return + + lines = [f"Carriers visible to your current {mode} API key:", ""] + for carrier in carriers: + nickname = carrier.get("nickname") or "(no nickname)" + lines.append( + f" carrier_id: {carrier.get('carrier_id', '?')} " + f"{carrier.get('friendly_name', '?')} - {nickname}" + ) + QMessageBox.information(self, f"ShipStation Carriers ({mode})", "\n".join(lines)) + + def _on_list_stores_clicked(self) -> None: + from app.services.shipstation_send import list_stores, ShipStationSendError + + mode = "TEST" if config.is_shipstation_test_mode() else "PRODUCTION" + try: + stores = list_stores() + except ShipStationSendError as exc: + QMessageBox.critical(self, "Could not list stores", str(exc)) + return + + if not stores: + QMessageBox.information( + self, + f"ShipStation Stores ({mode})", + f"No stores are set up in this {mode.lower()} ShipStation account.", + ) + return + + lines = [f"Stores visible to your current {mode} API key:", ""] + for store in stores: + lines.append( + f" store_id: {store.get('store_id', '?')} " + f"{store.get('store_name', '?')}" + ) + QMessageBox.information(self, f"ShipStation Stores ({mode})", "\n".join(lines)) def _on_order_double_clicked(self, order) -> None: dialog = OrderDetailDialog(order, self) @@ -443,35 +556,92 @@ class MainWindow(QMainWindow): dialog = self._return_label_dialog order = dialog.order - packages = dialog.get_packages() - charge_event = dialog.get_charge_event() - if not packages: - QMessageBox.warning(self, "No packages", "Add at least one package first.") - return - ticket_number = order.ticket_number or order.external_id - self.status_label.setText(f"Creating return label for {ticket_number}...") - worker = CreateReturnLabelWorker(order, packages, charge_event) - worker.finished_ok.connect(lambda result: self._on_return_label_ok(ticket_number, result)) - worker.failed.connect(self._on_return_label_failed) - self._return_label_worker = worker # keep a reference so it isn't garbage collected - worker.start() + if order.dummy_outbound_label_id: + # Step 2: a dummy already exists for this ticket - create the + # real return label from it. + packages = dialog.get_packages() + charge_event = dialog.get_charge_event() + if not packages: + QMessageBox.warning(self, "No packages", "Add at least one package first.") + return + + self.status_label.setText(f"Creating return label for {ticket_number}...") + worker = CreateReturnLabelWorker(order, order.dummy_outbound_label_id, packages, charge_event) + worker.finished_ok.connect(lambda result: self._on_return_label_ok(ticket_number, result)) + worker.failed.connect(self._on_return_label_failed) + self._return_label_worker = worker # keep a reference so it isn't garbage collected + worker.start() + else: + # Step 1: no dummy yet - create it, then stop and let the user + # verify it in ShipStation before running this again for step 2. + self.status_label.setText(f"Creating dummy shipment for {ticket_number}...") + worker = CreateDummyShipmentWorker(order) + worker.finished_ok.connect(lambda result: self._on_dummy_shipment_ok(ticket_number, result)) + worker.failed.connect(self._on_return_label_failed) + self._return_label_worker = worker + worker.start() + + def _on_dummy_shipment_ok(self, ticket_number: str, result: dict) -> None: + label_id = result.get("label_id", "?") + save_dummy_outbound_label_id(ticket_number, label_id) + self._refresh_everything() + # Refreshing resets the table model, which clears whatever row was + # selected - re-select the same ticket so clicking Create Return + # Label again immediately proceeds to step 2 rather than hitting + # "no ticket selected". + self.orders_table.select_ticket(ticket_number) + self.status_label.setText(f"Dummy shipment created - {label_id}") + QMessageBox.information( + self, + "Step 1 Complete", + f"Dummy shipment {label_id} created for {ticket_number}.\n\n" + "Go check it in ShipStation now - confirm Order #, Ship From/Store, and everything " + "else looks right. Once you've verified it, click Create Return Label again on this " + "ticket to run step 2 (the actual return label).", + ) def _on_return_label_ok(self, ticket_number: str, result: dict) -> None: mark_shipstation_sent(ticket_number) self._refresh_everything() + self.orders_table.select_ticket(ticket_number) label_id = result.get("label_id", "?") tracking = result.get("tracking_number", "?") - self.status_label.setText(f"Return label created - {label_id}") - QMessageBox.information( - self, - "Return Label Created", - f"Label {label_id} created (tracking {tracking}).\n\n" - "Next: go to ShipStation's Returns tab, find this label, and use " - "Other Actions -> Send Return Label to email it to the customer through " - "your branded template.", - ) + label_status = result.get("status", "unknown") + dummy_id = result.get("_dummy_outbound_label_id", "?") + self.status_label.setText(f"Return label created - {label_id} (status: {label_status})") + + if label_status == "processing": + body = ( + f"Label {label_id} was accepted but is still processing (tracking {tracking}).\n\n" + "ShipStation says this can take a few minutes to fully complete - if you can't " + "find it yet, wait a bit and check again before assuming something's wrong.\n\n" + f"(Linked to dummy outbound label {dummy_id}, for troubleshooting reference.)" + ) + else: + body = ( + f"Label {label_id} created, status: {label_status} (tracking {tracking}).\n\n" + "Find it in ShipStation and use Other Actions -> Send Return Label to email " + "it to the customer through your branded template.\n\n" + f"(Linked to dummy outbound label {dummy_id}, for troubleshooting reference.)" + ) + + box = QMessageBox(self) + box.setIcon(QMessageBox.Icon.Information) + box.setWindowTitle("Return Label Created") + box.setText(body) + copy_button = None + if tracking and tracking != "?": + copy_button = box.addButton("Copy Tracking Number", QMessageBox.ButtonRole.ActionRole) + box.addButton(QMessageBox.StandardButton.Ok) + box.exec() + # Only ever touches the clipboard if explicitly asked - never + # overwrites it automatically, since it's used constantly for other + # things and clobbering it silently would lose whatever was there. + if copy_button is not None and box.clickedButton() is copy_button: + QApplication.clipboard().setText(tracking) + self.status_label.setText(f"Tracking number {tracking} copied to clipboard.") def _on_return_label_failed(self, message: str) -> None: self.status_label.setText("Return label creation failed.") diff --git a/app/ui/widgets/orders_table.py b/app/ui/widgets/orders_table.py index 28080ec..9ef3d98 100644 --- a/app/ui/widgets/orders_table.py +++ b/app/ui/widgets/orders_table.py @@ -27,10 +27,10 @@ from PyQt6.QtWidgets import ( ) from app.models import Order -from app.return_labels import is_emailed_label_order +from app.return_labels import is_emailed_label_order, get_emailed_label_skus from app.schedule import get_cutoff_time, is_past_cutoff_today from app.status_rules import get_cancelled_statuses, status_in -from app.ticket_validation import validate_ticket +from app.ticket_validation import validate_ticket, make_validation_context from app.tracking import get_fulfilled_statuses CANCELLED_TEXT_COLOR = QColor(180, 0, 0) @@ -83,12 +83,31 @@ class OrdersTableModel(QAbstractTableModel): self._cancelled_statuses: set[str] = set() self._fulfilled_statuses: set[str] = set() self._cutoff_time = None + self._issues_by_id: dict[int, list] = {} + self._is_emailed_label_by_id: dict[int, bool] = {} def set_orders(self, orders: List[Order]) -> None: # Re-read status lists and cutoff time each refresh, in case Settings changed. self._cancelled_statuses = get_cancelled_statuses() self._fulfilled_statuses = get_fulfilled_statuses() self._cutoff_time = get_cutoff_time() + # Computed ONCE per refresh here, not per cell paint - both of + # these read Settings from disk internally, and Qt calls data() + # extremely frequently (every cell, every repaint, constantly + # during scrolling) - doing this per-cell instead of per-refresh + # was a severe, real performance bug, not just a minor slowdown. + # validation_context is ALSO fetched once here rather than once + # per order inside validate_ticket() - same fix, applied to the + # cost of computing the cache itself, not just using it. + validation_context = make_validation_context() + self._issues_by_id = { + order.id: validate_ticket(order.skus, order.line_items, **validation_context) + for order in orders + } + emailed_skus = get_emailed_label_skus() + self._is_emailed_label_by_id = { + order.id: is_emailed_label_order(order.skus or [], emailed_skus) for order in orders + } self.beginResetModel() self._orders = orders self.endResetModel() @@ -117,7 +136,7 @@ class OrdersTableModel(QAbstractTableModel): if role == Qt.ItemDataRole.ForegroundRole: if is_cancelled: return CANCELLED_TEXT_COLOR - if field_name == "issues_display" and validate_ticket(order.skus, order.line_items): + if field_name == "issues_display" and self._issues_by_id.get(order.id): return ISSUE_TEXT_COLOR return None @@ -131,14 +150,14 @@ class OrdersTableModel(QAbstractTableModel): return None if role == Qt.ItemDataRole.ToolTipRole and field_name == "issues_display": - issues = validate_ticket(order.skus, order.line_items) + issues = self._issues_by_id.get(order.id, []) return "\n".join(i.message for i in issues) if issues else None if role != Qt.ItemDataRole.DisplayRole: return None if field_name == "issues_display": - issues = validate_ticket(order.skus, order.line_items) + issues = self._issues_by_id.get(order.id, []) return f"{WARNING_MARK} ({len(issues)})" if issues else "" if field_name == "skus_display": return ", ".join(order.skus or []) @@ -149,7 +168,7 @@ class OrdersTableModel(QAbstractTableModel): filled = sum(1 for v in serials.values() if (v or "").strip()) return f"{filled} entered" if filled else "" if field_name == "return_label_display": - return CHECK_MARK if is_emailed_label_order(order.skus or []) else "" + return CHECK_MARK if self._is_emailed_label_by_id.get(order.id) else "" if field_name == "outgoing_tracking_display": return _format_tracking_numbers(order.tracking_numbers, is_return=False) if field_name == "return_tracking_display": @@ -281,6 +300,20 @@ class OrdersTableView(QWidget): source_index = self._proxy_model.mapToSource(indexes[0]) return self._source_model.order_at(source_index.row()) + def select_ticket(self, ticket_number: str) -> bool: + """Re-selects a row by ticket number - a table refresh (set_orders) + clears whatever was selected, since the underlying model resets. + Used after a step in a multi-step action (like the return-label + dummy-then-return flow) so the next step doesn't silently find + nothing selected. Returns whether the ticket was found/selected.""" + for row in range(self._proxy_model.rowCount()): + proxy_index = self._proxy_model.index(row, 0) + source_index = self._proxy_model.mapToSource(proxy_index) + if self._source_model.order_at(source_index.row()).ticket_number == ticket_number: + self.table.selectRow(proxy_index.row()) + return True + return False + def visible_orders(self) -> List[Order]: """Orders currently passing the active filters - used for export.""" result = [] diff --git a/app/ui/widgets/return_label_dialog.py b/app/ui/widgets/return_label_dialog.py index 8c543e7..f81d191 100644 --- a/app/ui/widgets/return_label_dialog.py +++ b/app/ui/widgets/return_label_dialog.py @@ -2,8 +2,17 @@ Return label creation dialog - for the emailed-return-label workflow (SH007 / OK012). Shows the ticket's description (where staff note what boxes are needed) and lets them specify however many packages, each -with its own weight/dimensions, replacing the old "always 1x1x1, 1oz" -placeholder with real per-request control. +with its own weight/dimensions. + +These packages genuinely reach ShipStation now: a standalone return +label (no linked outbound shipment) turned out to report success while +being invisible in ShipStation's UI - confirmed against the team's own +working manual process, not just the API docs. The fix (in +shipstation_send.py) creates a minimal, cheap dummy outbound label +first and links the real return to it via outbound_label_id, matching +what ShipStation's GUI does automatically when creating a return from +an existing shipment. The dialog itself doesn't need to know about +that - it just collects real packages, same as before. WORKAROUND NOTE: this dialog crashed the whole process on close, confirmed via two full crash dumps (identical fault offset both times) @@ -54,6 +63,7 @@ CHARGE_EVENT_LABELS = { "on_carrier_acceptance": "On carrier acceptance (label may go unused free)", } +DEFAULT_WEIGHT_LB = "0.00" DEFAULT_WEIGHT_OZ = "1.00" DEFAULT_DIMENSION_IN = "1.00" @@ -87,6 +97,11 @@ class ReturnLabelDialog(QDialog): self.summary_label = QLabel() layout.addWidget(self.summary_label) + self.step_status_label = QLabel() + self.step_status_label.setWordWrap(True) + self.step_status_label.setStyleSheet("font-weight: bold;") + layout.addWidget(self.step_status_label) + layout.addWidget(QLabel("Description (box requirements from JIRA):")) self.description_label = QLabel() self.description_label.setWordWrap(True) @@ -120,7 +135,7 @@ class ReturnLabelDialog(QDialog): button_box = QDialogButtonBox() self.create_button = button_box.addButton( - "Create Return Label", QDialogButtonBox.ButtonRole.AcceptRole + "Step 1: Create Dummy Shipment", QDialogButtonBox.ButtonRole.AcceptRole ) button_box.addButton(QDialogButtonBox.StandardButton.Cancel) button_box.accepted.connect(self.accept) @@ -139,7 +154,7 @@ class ReturnLabelDialog(QDialog): """ self.order = order self.setWindowTitle(f"Create Return Label - {order.ticket_number or order.external_id}") - self.resize(560, 480) + self.resize(560, 520) info = order.shipping_info or {} address_line2 = f" {info.get('address2')}" if info.get("address2") else "" @@ -152,6 +167,27 @@ class ReturnLabelDialog(QDialog): self.summary_label.setText("\n".join(summary_lines)) self.description_label.setText(order.description or "(no description on this ticket)") + # This dialog is used for BOTH steps of the workflow - the button + # (and what clicking it actually does, wired up in main_window.py) + # depends on whether a dummy shipment already exists for this + # ticket. Splitting these apart on purpose, per your request: a + # problem in the dummy step shouldn't be masked by immediately + # attempting the return step too. + if order.dummy_outbound_label_id: + self.step_status_label.setText( + f"Step 1 done - dummy shipment {order.dummy_outbound_label_id} already exists. " + "If you've verified it looks right in ShipStation (Order #, Ship From/Store all " + "populated), click below to create the actual return label from it." + ) + self.create_button.setText("Step 2: Create Return Label") + else: + self.step_status_label.setText( + "Step 1: this creates a cheap dummy outbound shipment first (1x1x1in, 1oz). " + "Go verify it in ShipStation before running this again to create the actual " + "return label - that way a problem in either step is easy to isolate." + ) + self.create_button.setText("Step 1: Create Dummy Shipment") + # Clear out any package rows left over from a previous ticket. # setParent(None) + deleteLater() rather than an immediate delete - # deferred deletion here is deliberate, letting Qt clean these up @@ -169,13 +205,15 @@ class ReturnLabelDialog(QDialog): row_layout = QHBoxLayout(row_widget) row_layout.setContentsMargins(0, 0, 0, 0) - weight_field = _make_number_field(DEFAULT_WEIGHT_OZ) + weight_lb_field = _make_number_field(DEFAULT_WEIGHT_LB) + weight_oz_field = _make_number_field(DEFAULT_WEIGHT_OZ) length_field = _make_number_field(DEFAULT_DIMENSION_IN) width_field = _make_number_field(DEFAULT_DIMENSION_IN) height_field = _make_number_field(DEFAULT_DIMENSION_IN) for label_text, widget in [ - ("Weight (oz):", weight_field), + ("Weight (lb):", weight_lb_field), + ("+ (oz):", weight_oz_field), ("L (in):", length_field), ("W (in):", width_field), ("H (in):", height_field), @@ -186,7 +224,8 @@ class ReturnLabelDialog(QDialog): entry = { "widget": row_widget, - "weight": weight_field, + "weight_lb": weight_lb_field, + "weight_oz": weight_oz_field, "length": length_field, "width": width_field, "height": height_field, @@ -204,7 +243,10 @@ class ReturnLabelDialog(QDialog): def get_packages(self) -> List[dict]: return [ { - "weight_oz": _parse_number(entry["weight"].text()), + "weight_oz": ( + _parse_number(entry["weight_lb"].text()) * 16 + + _parse_number(entry["weight_oz"].text()) + ), "length": _parse_number(entry["length"].text()), "width": _parse_number(entry["width"].text()), "height": _parse_number(entry["height"].text()), diff --git a/app/workers.py b/app/workers.py index 1b994f7..c1839f7 100644 --- a/app/workers.py +++ b/app/workers.py @@ -61,23 +61,52 @@ class SendToShipStationWorker(QThread): self.finished_ok.emit(result) +class CreateDummyShipmentWorker(QThread): + """Runs step 1 (dummy outbound shipment creation) off the GUI thread.""" + + finished_ok = pyqtSignal(dict) # the created dummy label's JSON + failed = pyqtSignal(str) + + def __init__(self, order, parent=None): + super().__init__(parent) + self.order = order + + def run(self) -> None: + from app.services.shipstation_send import create_dummy_shipment, ShipStationSendError + + try: + result = create_dummy_shipment(self.order) + except ShipStationSendError as exc: + self.failed.emit(str(exc)) + return + except Exception as exc: # noqa: BLE001 + self.failed.emit(f"Unexpected error creating dummy shipment: {exc}") + return + + self.finished_ok.emit(result) + + class CreateReturnLabelWorker(QThread): - """Runs return-label creation off the GUI thread.""" + """Runs step 2 (the real return label, from an already-created dummy) + off the GUI thread.""" finished_ok = pyqtSignal(dict) # the created label's JSON failed = pyqtSignal(str) - def __init__(self, order, packages: list[dict], charge_event: str, parent=None): + def __init__(self, order, dummy_label_id: str, packages: list[dict], charge_event: str, parent=None): super().__init__(parent) self.order = order + self.dummy_label_id = dummy_label_id self.packages = packages self.charge_event = charge_event def run(self) -> None: - from app.services.shipstation_send import create_return_label, ShipStationSendError + from app.services.shipstation_send import create_return_label_from_dummy, ShipStationSendError try: - result = create_return_label(self.order, self.packages, self.charge_event) + result = create_return_label_from_dummy( + self.order, self.dummy_label_id, self.packages, self.charge_event + ) except ShipStationSendError as exc: self.failed.emit(str(exc)) return @@ -334,6 +363,22 @@ def mark_shipstation_sent(ticket_number: str) -> None: session.close() +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.""" + session = get_session() + try: + order = session.execute( + select(Order).where(Order.source == "jira", Order.ticket_number == ticket_number) + ).scalar_one_or_none() + if order is not None: + order.dummy_outbound_label_id = label_id + session.commit() + finally: + session.close() + + def save_pack_data(ticket_number: str, serial_numbers: dict, packed: bool) -> None: """ Saves serial numbers and the packed/done flag from the Pack Ticket