feat: add Home Depot provider - #77
Conversation
Adds a new `homedepot` subcommand that syncs Home Depot purchases into Monarch, mirroring the walmart/costco/amazon providers. Design (per proposal in eshaffer321#31): - Companion Go client github.com/fnziman/homedepot-go handles the internal /oms/customer/order/v1 API via cookie replay (THD_CUSTOMER cookie decoded for the auth token). itemize depends on tagged release v0.1.1. - Provider adapter at internal/adapters/providers/homedepot/ wraps the client's OrderDetail + LineItem types to satisfy providers.Order + providers.OrderItem. - Both online and in-store schemas supported. Online orders use orderNumber as the dedup ID; in-store use a composite hd-instore-{store}-{transactionId} since orderNumber is empty. Registration: - internal/cli/providers.go: NewHomeDepotProvider — mirrors the costco/walmart factory shape, threads a scoped *slog.Logger through to homedepot-go. - cmd/itemize/main.go: `case "homedepot":` in the subcommand switch; printUsage rows for the command and the HOMEDEPOT_* env vars. - internal/infrastructure/config/config.go: HomeDepotConfig struct, env-var defaults in LoadFromEnv (HOMEDEPOT_LOOKBACK_DAYS, HOMEDEPOT_MAX_ORDERS, HOMEDEPOT_COOKIE_FILE). - config.yaml: `homedepot:` block. Merchant matching: no interface change needed. fetch.go matches against DisplayName(), which is "Home Depot" — case-insensitive substring-matches Monarch's "THE HOME DEPOT" cleanly. Tests: 95.1% coverage on the provider package. Mock hdClient interface; every FetchOrders / GetOrder / HealthCheck / GetItems / GetSKU / GetQuantity path tested. All Order/OrderItem interface compliance asserted at compile time. Docs: README provider row + full walkthrough at docs/homedepot-specifics.md (cookie export, MFA re-auth, 24-month history cap, troubleshooting table). Attribution to joshellissh/homedepot-history in the client's README. Refs eshaffer321#31 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #77 +/- ##
==========================================
+ Coverage 64.34% 65.06% +0.71%
==========================================
Files 49 51 +2
Lines 6575 6761 +186
==========================================
+ Hits 4231 4399 +168
- Misses 2037 2046 +9
- Partials 307 316 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
eshaffer321
left a comment
eshaffer321
left a comment
There was a problem hiding this comment.
Thanks for the well-structured provider and thorough unit coverage. I verified the exact current-main merge locally: the Home Depot package reports 95.1% coverage, and go test ./... -race, go vet ./..., the build, and the companion client tests all pass. GitHub checks are green as well.
I am requesting changes for the safety/correctness issues called out inline. Two additional repository-level gaps are not attachable to changed lines:
- [P2]
internal/cli/serve.gostill registers only Walmart, Costco, and Amazon, soPOST /api/syncwithhomedepotreturnsinvalid provider. Please add the Home Depot factory/log entry and an API/service test, or explicitly scope and document this provider as CLI-only. - The integrated live dry-run checkbox is still open. After the fixes, please run a telemetry-disabled, sanitized
itemize homedepot -dry-run -days 14 -max 1 -verboseagainst the latest branch and report whether order totals, item totals, merchant matching, and proposed split sums agree. If the account contains both shapes, please validate one online and one in-store purchase; do not post cookies, IDs, or financial details.
| return o.detail.OrderNumber | ||
| } | ||
| if o.summary.TransactionID != "" { | ||
| return fmt.Sprintf("hd-instore-%s-%s", o.summary.StoreNumber, o.summary.TransactionID) |
There was a problem hiding this comment.
[P1] Please make the in-store dedup key include the full stable receipt identity. The companion client needs registerNumber, salesDate, storeNumber, and transactionId to retrieve an in-store order, but this key drops register and date. Two summaries that differ only by those fields therefore produce the same ID; processing_records.order_id is globally unique and IsProcessed checks only that ID, so a legitimate second purchase can be skipped permanently. Please include register and normalized sale date, reject incomplete identifiers, and add a test with same store/transaction but different register/date. This is safest to fix before release because changing persisted IDs later can cause reprocessing.
| } | ||
|
|
||
| detail, err := p.client.GetOrder(ctx, summary) | ||
| if err != nil { |
There was a problem hiding this comment.
[P1] Please distinguish isolated detail failures from systemic failures instead of turning every error into a successful skip. Expired auth, ErrRateLimited, network/context failures, and API/schema errors all land here; if every detail request fails, this method returns an empty slice with nil, and the sync reports success with zero errors. The detail requests are also issued back-to-back: GetRateLimit() is not consumed by the orchestrator, and homedepot-go only pauses between history pages. Please add actual pacing/retry, abort on systemic errors, aggregate any isolated skips, and cover the all-auth-fail/all-rate-limited cases.
| Logger: hdLogger, | ||
| }) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create Home Depot client: %w", err) |
There was a problem hiding this comment.
[P1] Please sanitize Home Depot construction/auth errors before they reach telemetry. homedepot-go file errors include the cookie filename, while main sends provider-creation failures through telemetry.CaptureError; the current scrubber does not remove filesystem paths, so a username or custom secret path can leave the machine. Home Depot API errors also include response-body excerpts when sync failures are captured. Please return a telemetry-safe typed error or bypass telemetry for these auth/file errors, and add a regression test for path/body redaction.
| }, | ||
| HomeDepot: HomeDepotConfig{ | ||
| Enabled: true, | ||
| LookbackDays: getEnvInt("HOMEDEPOT_LOOKBACK_DAYS", 14), |
There was a problem hiding this comment.
[P2] These advertised environment values currently have no effect on a sync. ParseSyncFlags always defaults -days/-max to 14/0, ToSyncOptions uses those flag values, and no code reads cfg.Providers.HomeDepot.LookbackDays or MaxOrders. Likewise, only CookieFile is consumed from HomeDepotConfig; enabled, rate_limit, and debug are ignored. Please wire the provider config into runtime defaults/behavior or remove the unsupported settings and documentation.
| func (p *Provider) GetOrderDetails(ctx context.Context, orderID string) (providers.Order, error) { | ||
| p.logger.Warn("GetOrderDetails called without OrderSummary context", | ||
| slog.String("order_id", orderID)) | ||
| return nil, fmt.Errorf("GetOrderDetails not supported for Home Depot without OrderSummary") |
There was a problem hiding this comment.
[P2] The common CLI advertises -order-id as the blast-radius limiter for syncs, but Home Depot always fails when it is used. Please implement a targeted path (direct online lookup and/or list-and-match for in-store IDs), or reject/document this flag specifically for Home Depot before shared clients and storage are initialized.
2 participants
Adds a new
homedepotsubcommand that syncs Home Depot purchases into Monarch, mirroring the walmart/costco/amazon providers.Closes / references #31. Design follows the plan you signed off on in that thread.
Design
github.com/fnziman/homedepot-go— MIT, tagged release v0.1.1 — handles the internal/oms/customer/order/v1/user/{userId}/orderhistory+/orderdetailsendpoints via cookie replay (decodesTHD_CUSTOMERfor the auth token). The client itself is scrubbed-fixtures-only (no real cookies/tokens/PII), 85.6% covered,go 1.25.12to match this repo's CI pin.internal/adapters/providers/homedepot/. Wraps the client'sOrderDetail/LineItemtypes to satisfyproviders.Order/providers.OrderItem. Both online and in-store orders supported. Online orders useorderNumberas the dedup ID; in-store use a compositehd-instore-{store}-{transactionId}(sinceorderNumberis empty for in-store).internal/cli/providers.go:NewHomeDepotProvider(mirrors the costco/walmart shape, threads a scoped*slog.Loggerviahdgo.Config{Logger: hdLogger}— matches the walmart/costcoConfig-struct pattern per your ask), newcase "homedepot":incmd/itemize/main.go,HomeDepotConfiginconfig.go,homedepot:block inconfig.yaml, env-var fallbacks (HOMEDEPOT_LOOKBACK_DAYS,HOMEDEPOT_MAX_ORDERS,HOMEDEPOT_COOKIE_FILE).fetch.gomatches onDisplayName()which is"Home Depot"— that case-insensitively substring-matches Monarch's canonical"THE HOME DEPOT".SupportsRefunds() = false; refund handling can go in a follow-up.Test plan
go test ./... -race— all green, provider package at 95.1% coverage, categorizer/adapter packages unchangedgo vet ./...— cleangolangci-lint run ./...— clean for touched files (two pre-existingQF1012warnings oncategorizer.go:285,290predate this PR)go build -o itemize ./cmd/itemize/—./itemizeshowshomedepot Sync Home Depot ordersin usage andHOMEDEPOT_*env vars in helpgo.modstays ongo 1.25.12(verified aftergo mod tidy)HOMEDEPOT_COOKIE_FILE=~/.homedepot-api/cookies.json ./itemize homedepot -dry-run -days 14 -verbose— happy to run this and report back once you've had a chance to look at the codeDocs
homedepotrow + example usagedocs/homedepot-specifics.mdcovers cookie export walkthrough (DevTools snippet), the online/in-store distinction, 24-month history cap, MFA re-auth flow, and a troubleshooting tableDeferred / follow-ups (not in this PR per your "keep it focused" note)
OrderDetail.returnTotalis available; just not wired)LICENSEfile on this repodocs/adding-providers.mdrefresh (still references the pre-rename module path andinternal/providers/)Happy to iterate — let me know if you'd like any part of this reshaped before merge.