Skip to content

[PAN-2508] ShipBob <> Gorgias Sample Integration in Python - #11

Open
sarahtrefethen wants to merge 3 commits into
masterfrom
pan-2508-sb2gorgias-py
Open

[PAN-2508] ShipBob <> Gorgias Sample Integration in Python#11
sarahtrefethen wants to merge 3 commits into
masterfrom
pan-2508-sb2gorgias-py

Conversation

@sarahtrefethen

Copy link
Copy Markdown

@sarahtrefethen
sarahtrefethen force-pushed the pan-2508-sb2gorgias-py branch from 33da2a8 to 1bbbfd4 Compare July 16, 2026 21:17
@sarahtrefethen
sarahtrefethen force-pushed the pan-2508-sb2gorgias-py branch 4 times, most recently from e9b29ef to 6a6cea5 Compare August 5, 2026 20:54
@sarahtrefethen
sarahtrefethen force-pushed the pan-2508-sb2gorgias-py branch from 6a6cea5 to f3ab08d Compare August 5, 2026 21:26
@sarahtrefethen
sarahtrefethen force-pushed the pan-2508-sb2gorgias-py branch from eab44f5 to 29ccfa0 Compare August 10, 2026 22:28
@shanta3220
shanta3220 self-requested a review August 11, 2026 18:08
'https://authstage.shipbob.com': 'https://sandbox-api.shipbob.com/2026-01',
'https://auth.shipbob.com': 'https://api.shipbob.com/2026-01',
}
DEFAULT_BASE_URL = 'https://api.shipbob.com/1.0'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fallback base URL points to outdated ShipBob API version

logger.info('Processing updated order with id %s', order["id"])
process_order(order, gorgias, cache, newest_first)
# last_update_at is YYYY-MM-DDThh:mm:ss.sss+00:00; trim to 23 chars.
record['updated_order_start_date'] = shipbob.get_update_date(order, updated_cursor)[:23]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It unconditionally overwrites on every order in the loop, rather than taking a running min().

Failure scenario: Paging isn't guaranteed sorted across page boundaries, so the final cursor ends up being whatever the last order of the last page happened to have — not the true oldest processed update. The next run can start from a cursor that's newer than it should be, silently skipping orders updated in between. This contradicts the module's documented "never skips an update" guarantee.

Comment on lines +79 to +84
data.setdefault('pandium', {})
if not isinstance(data['pandium'].get('shipbob_orders'), list):
data['pandium']['shipbob_orders'] = []
cache[key] = {'id': existing['id'], 'data': data}
else:
cache[key] = gorgias.new_customer_payload(sb_order, key)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data.setdefault('pandium', {}) is a no-op if the key already exists with value None. The next line, data['pandium'].get('shipbob_orders'), then raises AttributeError on a NoneType.

Failure scenario: A Gorgias customer record with data: {"pandium": null} (e.g. edited via the Gorgias UI/API, or left over from a different integration) crashes the whole cron run for that page, uncaught before the surrounding try/except.

Comment on lines +52 to +57
orders.append(order_payload)
orders.sort(key=lambda o: o.get('id', 0), reverse=newest_first)
if len(orders) > MAX_ORDERS_TO_SYNC:
orders = orders[:MAX_ORDERS_TO_SYNC] if newest_first else orders[-MAX_ORDERS_TO_SYNC:]
return orders

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

order list sorted by id, not date. The sb2gorgias in the integrations repo sorted updated orders by actual last_update_at and relied on SortOrder=Oldest for new orders, then just sliced the trimmed list. This version re-sorts by o.get('id', 0) on every upsert, which may not have guaranteed relationship to update recency — an order just updated (lower id) can get bumped out of the kept 10 in favor of a higher-id order that wasn't recently touched. Suggest sorting by created_date / purchase_date instead (note: sort on the raw ISO value, before _format_date — the formatted %d/%m/%Y string doesn't sort chronologically).

Comment on lines +77 to +80

``customer_ref`` is what Gorgias should attach the ticket to — ``{'id': ...}`` for
a customer we resolved, or ``{'email': ...}`` to let Gorgias resolve it.
"""

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

customer_ref is always produced by resolve_customer, and that function only has two return statements — both {'id': ...}. Might need to update the comment.

deep_get(sb_order, 'recipient.address.address1', '') or '',
deep_get(sb_order, 'recipient.address.city', '') or '',
deep_get(sb_order, 'recipient.address.country', '') or '',
]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: we could reduce traversals here:

address = deep_get(sb_order, 'recipient.address', {})
deep_get(sb_order, 'recipient.name', '')
address.get('address1') or ''
address.get('city') or ''
address.get('country') or ''

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants