Skip to content

feat: resolve POS-receipt productId to webshop productId#17

Merged
gwillem merged 2 commits into
gwillem:mainfrom
aaearon:feat/resolve-pos-product-id
May 19, 2026
Merged

feat: resolve POS-receipt productId to webshop productId#17
gwillem merged 2 commits into
gwillem:mainfrom
aaearon:feat/resolve-pos-product-id

Conversation

@aaearon

@aaearon aaearon commented May 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds first-class POS → webshop product-id resolution to the receipts module. Client.GetReceipt now populates a new ReceiptItem.WebshopID so consumers can deep-link kassabon lines to ah.nl without an extra round-trip per item.

Why

PosReceiptProduct.externalIds is declared in the GraphQL schema but is always null in production, so ReceiptItem.ProductID (the POS-side integer printed on the kassabon) currently has no way to be turned into a navigable webshop id. The two id spaces are distinct: https://www.ah.nl/producten/product/wi767898 returns "Dit product bestaat niet (meer)" while the actual webshop id for that line is 171607 ("Coca-Cola Zero sugar zero caffeïne").

The official Appie iOS/Android app shows product cards on every kassabon line, so a resolver clearly exists. Probing the Query type surfaced productConvertId(sourceId: Int!): Int — a single-argument resolver that does exactly this. Verified empirically:

POS productId kassabon name productConvertId hydrated title
767898 COCA-COLA 171607 Coca-Cola Zero sugar zero caffeïne
823326 AH KWARK 231878 AH Magere kwark
552022 AH ZALMFILET 121453 AH Zalmfilet

The resolver returns -1 when no conversion exists (e.g. when the input is already a webshop id). It does not validate the source id space, so passing arbitrary integers can return real but unrelated webshop ids — only feed it ids that came from PosReceiptProduct.id. This caveat is captured in the ConvertPOSIDs doc comment and in doc/albertheijn_api.md.

What changed

  • receipts.go — new Client.ConvertPOSIDs(ctx, []int) (map[int]int, error) builds a single GraphQL doc with N aliased productConvertId subfields (so a whole receipt resolves in one round-trip), dedupes input, drops non-positive ids and the -1 sentinel. GetReceipt calls it best-effort (logs on failure, returns the receipt regardless) and fills Items[i].WebshopID. GetReceipts (list view) is unchanged — it doesn't return per-product data.
  • types.go — adds ReceiptItem.WebshopID int. Updates the ProductID doc comment to clarify it is the POS-side id, not the webshop id (the previous wording was misleading).
  • cmd/appie/receipt.goappie receipt show appends wi<id> next to each line when resolution succeeded; blank otherwise.
  • doc/albertheijn_api.md — adds a row to the GraphQL ops table and a #### productConvertId subsection covering signature, sentinel, caveat, and the aliased-batch example.
  • Tests — three new unit tests (TestGetReceiptResolvesWebshopID, TestConvertPOSIDsBatchShape with dedup verification, TestConvertPOSIDsErrorIsBestEffort) plus an integration assertion in TestGetReceiptIntegration that at least one item resolves and GetProduct(WebshopID) round-trips successfully.

Heads-up: pre-existing build break under -tags integration

Independent of this PR, main currently fails to compile under -tags integration: commit 515edfe added TestGetShoppingListItems to both shoppinglist_test.go (no tag) and appie_integration_test.go (under //go:build integration). To run the integration suite locally I had to rename one of them; the rename is not included in this PR. Happy to send a separate one-line fix if useful.

Test plan

  • just test passes (3 new unit tests + existing suite).
  • go test -tags integration -run 'TestGetReceiptIntegration' ./... passes against a real account; canonical receipt resolves all 8 lines.
  • appie receipt show <id> annotates every line with wi<id>.

Receipt items returned by GetReceipt carry a POS-side productId
(PosReceiptProduct.id) that is not navigable on ah.nl --
https://www.ah.nl/producten/product/wi767898 returns "product
bestaat niet" while the actual webshop id for that line is 171607.
PosReceiptProduct.externalIds is declared in the schema but always
null in production, so consumers had no way to deep-link or hydrate
kassabon lines.

Add Client.ConvertPOSIDs which calls the productConvertId(sourceId:
Int!) GraphQL field via aliased subfields, resolving an entire
receipt in one round-trip. GetReceipt invokes it best-effort and
populates the new ReceiptItem.WebshopID; conversion failures log
and continue rather than failing the receipt fetch. The CLI
receipt show command now prints "wi<id>" alongside each line.
Copilot AI review requested due to automatic review settings May 10, 2026 13:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds POS receipt product-id → AH webshop product-id resolution to the receipts workflow so GetReceipt can hydrate each receipt line with a navigable wi<id> (via GraphQL productConvertId) in a single additional GraphQL round-trip.

Changes:

  • Added ReceiptItem.WebshopID and clarified that ReceiptItem.ProductID is the POS-side id.
  • Implemented Client.ConvertPOSIDs (aliased GraphQL subfields) and wired it into Client.GetReceipt (best-effort).
  • Updated CLI output, docs, and tests (unit + integration) to cover the new behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
types.go Adds ReceiptItem.WebshopID and corrects ProductID documentation to reflect POS vs webshop id spaces.
receipts.go Implements POS→webshop id conversion and populates WebshopID during GetReceipt.
receipts_test.go Adds unit tests for conversion batching, dedupe/filtering, and best-effort failure behavior.
receipt_test.go Extends integration test to assert at least one WebshopID resolves and can round-trip via GetProduct.
doc/albertheijn_api.md Documents productConvertId, its sentinel behavior, caveats, and aliased batching pattern.
cmd/appie/receipt.go Shows resolved wi<id> next to each receipt line when available.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread receipts.go Outdated

var resp map[string]int
if err := c.DoGraphQL(ctx, b.String(), nil, &resp); err != nil {
return nil, fmt.Errorf("convert pos ids: %w", err)
Comment thread types.go Outdated
Comment on lines +302 to +303
// ah.nl), resolved from ProductID via productConvertId. 0 when
// resolution failed or the API returned its -1 sentinel.
Comment thread receipts_test.go Outdated
Comment on lines +182 to +183
// readGraphQLRequest reads and rewinds the body so the handler can both
// inspect and decode the request.
Comment thread receipts.go Outdated
Comment on lines +226 to +236
var resp map[string]int
if err := c.DoGraphQL(ctx, b.String(), nil, &resp); err != nil {
return nil, fmt.Errorf("convert pos ids: %w", err)
}

for i, id := range unique {
v, ok := resp[fmt.Sprintf("p%d", i)]
if !ok || v <= 0 {
continue
}
out[id] = v
- ConvertPOSIDs now returns the (always non-nil) out map alongside
  errors, so callers that ignore err can safely look up keys without
  nil-deref. Doc comment updated to reflect the guarantee.
- Decode productConvertId aliases through *int instead of int. The
  GraphQL field is declared `Int` (nullable); a single null alias
  would previously fail JSON unmarshal and torch the whole batch.
  Null is now treated identically to the -1 sentinel — covered by
  the new TestConvertPOSIDsTreatsNullAsUnresolved.
- Clarify ReceiptItem.WebshopID doc: the in-memory zero value 0
  means "unresolved", and JSON omits 0 entirely (omitempty).
- Fix readGraphQLRequest test helper comment — it does not rewind
  r.Body; document that callers must not read it again.
@gwillem gwillem merged commit 2e4ea21 into gwillem:main May 19, 2026
1 check passed
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.

3 participants