diff --git a/MEMORY.md b/MEMORY.md index aa0ba3f..4cbef4c 100644 --- a/MEMORY.md +++ b/MEMORY.md @@ -12,7 +12,8 @@ This project (`product_inventory`) integrates Google Sheets with Shopify. It ser 1. **Documentation First**: Before implementing complex features, we update the plan and often the documentation (README/ARCHITECTURE). 2. **Safety First**: We use `SafeToAutoRun: false` for commands that deploy or modify external state until verified. 3. **Strict Typing**: We use TypeScript. No `any` unless absolutely necessary (and even then, we try to avoid it). -4. **Artifact Usage**: We use `task.md`, `implementation_plan.md`, and `walkthrough.md` to track state. +4. **TDD**: We follow Test Driven Development (Red/Green/Refactor). Write failing tests before implementing features. +5. **Artifact Usage**: We use `task.md`, `implementation_plan.md`, and `walkthrough.md` to track state. ## Key Technical Decisions - **Queue System**: We implemented `onEditQueue.ts` to batch edits. This prevents hitting Shopify API rate limits and Google Apps Script execution limits during rapid manual edits. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 13b73a8..2663bfc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -37,6 +37,18 @@ To avoid hitting Shopify API rate limits and Google Apps Script execution time l - Reads `pendingEdits`. - Filters for edits older than `BATCH_INTERVAL_MS` (30s) to allow for multiple quick edits to the same SKU. - Iterates through valid edits and calls `Product.UpdateShopifyProduct`. + - **SKU Validation**: Before any action, checks if the SKU is valid (not empty, `?`, or `n`). Aborts if invalid. + +### 2. Product Lifecycle Logic + +- **Creation**: + - Uses the **SKU** as the Shopify **Handle** (URL slug). + - Prevents creation if the SKU is a placeholder. + +- **Updates**: + - Prioritizes **ID-based lookup**. + - If the `shopify_id` column is populated, the system trusts this ID to locate the product in Shopify, even if the SKU has changed in the sheet. + - As a result, changing a SKU in the sheet and syncing will **rename** the existing product (handle/SKU) rather than creating a duplicate. ### 2. Shopify Integration (`src/shopifyApi.ts`) @@ -45,7 +57,9 @@ The project uses a hybrid approach for the Shopify Admin API: - **REST API**: Used primarily for fetching Orders (legacy support). - **GraphQL API**: Used for fetching and updating Products and Inventory. -The `Shop` class handles authentication using credentials stored in the "vars" sheet. +- **GraphQL API**: Used for fetching and updating Products and Inventory. + +The `Shop` class implements the `IShop` interface, handling authentication using credentials stored in the "vars" sheet. The interface decoupling facilitates robust unit testing via `MockShop`. ### 3. Configuration (`src/config.ts`) diff --git a/src/Product.ts b/src/Product.ts index 1f280cd..03b4d19 100644 --- a/src/Product.ts +++ b/src/Product.ts @@ -14,6 +14,7 @@ import * as shopify from "shopify-admin-api-typings" import { Config } from "./config" import { ISpreadsheetService } from "./interfaces/ISpreadsheetService" import { GASSpreadsheetService } from "./services/GASSpreadsheetService" +import { IShop } from "./interfaces/IShop" export class Product { shopify_id: string = "" @@ -87,8 +88,25 @@ export class Product { } } - MatchToShopifyProduct(shop: Shop): shopify.Product { - // TODO: Look for and match based on known gid before SKU lookup + MatchToShopifyProduct(shop: IShop): shopify.Product { + // Check if we have a known Shopify ID from the sheet + if (this.shopify_id && this.shopify_id !== "") { + console.log(`MatchToShopifyProduct: Checking ID '${this.shopify_id}' from sheet...`) + let productById = shop.GetProductById(this.shopify_id) + if (productById) { + console.log(`MatchToShopifyProduct: Found product by ID '${this.shopify_id}'`) + this.shopify_product = productById + this.shopify_id = this.shopify_product.id.toString() + this.shopify_default_variant_id = productById.variants.nodes[0].id + // We trust the ID, so we update the instantiated object with current Shopify data + // But we DO NOT overwrite the sheet's SKU/Title yet, because we might be pushing updates FROM sheet TO Shopify. + return + } else { + console.log(`MatchToShopifyProduct: Product with ID '${this.shopify_id}' not found. Falling back to SKU lookup.`) + } + } + + // Fallback to SKU lookup let product = shop.GetProductBySku(this.sku) if (product == undefined || product.id == undefined || product.id == "") { console.log("MatchToShopifyProduct: no product matched") @@ -140,7 +158,9 @@ export class Product { sps.category = this.ShopifyCategory() } sps.tags = this.tags + sps.tags = this.tags sps.title = this.title + sps.handle = this.sku sps.descriptionHtml = this.description sps.variants = [] let variant = new ShopifyVariant() @@ -159,8 +179,14 @@ export class Product { return sps } - UpdateShopifyProduct(shop: Shop) { + UpdateShopifyProduct(shop: IShop) { console.log("UpdateShopifyProduct()") + // SKU Validation + if (!this.sku || this.sku === "" || this.sku === "?" || this.sku === "n") { + console.log("UpdateShopifyProduct: Invalid or placeholder SKU. Aborting.") + return; + } + var newProduct = false let config = new Config() this.MatchToShopifyProduct(shop) @@ -215,7 +241,7 @@ export class Product { this.CreatePhotoFolder(); } - UpdateAllMetafields(shop: Shop) { + UpdateAllMetafields(shop: IShop) { console.log("UpdateAllMetafields()") if (!this.shopify_id) { console.log("Cannot update metafields without a Shopify Product ID.") @@ -326,7 +352,7 @@ export class Product { createPhotoFolderForSku(new(Config), this.sku, this.sheetService); } - PublishToShopifyOnlineStore(shop: Shop) { + PublishToShopifyOnlineStore(shop: IShop) { console.log("PublishToShopifyOnlineStore") let config = new Config() let query = /* GraphQL */ ` @@ -364,7 +390,7 @@ export class Product { return shop.shopifyGraphQLAPI(JSON.parse(j)) } - PublishShopifyProduct(shop: Shop) { + PublishShopifyProduct(shop: IShop) { //TODO: update product in sheet // TODO: status // TODO: shopify_status diff --git a/src/ProductLogic.test.ts b/src/ProductLogic.test.ts new file mode 100644 index 0000000..7b152ed --- /dev/null +++ b/src/ProductLogic.test.ts @@ -0,0 +1,106 @@ +import { Product } from "./Product"; +import { MockSpreadsheetService } from "./services/MockSpreadsheetService"; +import { MockShop } from "./test/MockShop"; + +describe("Product Logic (TDD)", () => { + let mockService: MockSpreadsheetService; + let mockShop: MockShop; + + beforeEach(() => { + mockService = new MockSpreadsheetService({ + product_inventory: [ + ["sku", "title", "price", "shopify_id", "product_type"], // Headers + ["TEST-SKU-1", "Test Product", 10.99, "", "Type A"] // Data + ], + values: [ + ["product_type", "shopify_category", "ebay_category_id"], + ["Type A", "Category A", "123"] + ] + }); + mockShop = new MockShop(); + }); + + test("UpdateShopifyProduct should abort if SKU is invalid", () => { + // Setup invalid SKU + mockService.setSheetData("product_inventory", [ + ["sku", "title"], + ["?", "Invalid Product"] + ]); + // Allow empty sku to be passed to constructor logic check, but Product constructor throws if sku not found. + // So we pass a valid sku that exists in sheet, but looks invalid? + // The requirement is "based on the product's title... If I have a placeholder value... ensure products are not created until they have a valid SKU". + // In `Product.ts`, constructor takes `sku`. + // If I pass `?` and it's in the sheet, it constructs. + + const product = new Product("?", mockService); + + // Attempt update + product.UpdateShopifyProduct(mockShop); + + // Verify no calls to creating product + // We expect MatchToShopifyProduct might be called (read only), but NOT productSet (writ) + // Actually our plan said "abort operation" at start of UpdateShopifyProduct. + expect(mockShop.productSetCalledWith).toBeNull(); + }); + + test("ToShopifyProductSet should set handle to SKU", () => { + const product = new Product("TEST-SKU-1", mockService); + const sps = product.ToShopifyProductSet(); + + // We expect sps to have a 'handle' property equal to the sku + // This will fail compilation initially as ShopifyProductSetInput doesn't have handle + expect((sps as any).handle).toBe("TEST-SKU-1"); + }); + + test("MatchToShopifyProduct should verify ID if present", () => { + // Setup data with shopify_id + mockService.setSheetData("product_inventory", [ + ["sku", "shopify_id"], + ["TEST-SKU-OLD", "123456789"] + ]); + const product = new Product("TEST-SKU-OLD", mockService); + + // Mock the response for GetProductById + mockShop.mockProductById = { + id: "123456789", + title: "Old Title", + variants: { nodes: [{ id: "gid://shopify/ProductVariant/123456789", sku: "TEST-SKU-OLD" }] }, + options: [{ id: "opt1", optionValues: [{ id: "optval1" }] }] + }; + + // We need to call Match, but it's called inside Update usually. + // We can call it directly for testing. + product.MatchToShopifyProduct(mockShop); + + // Expect GetProductById to have been called + expect(mockShop.getProductByIdCalledWith).toBe("123456789"); + expect(product.shopify_id).toBe("123456789"); + }); + + test("MatchToShopifyProduct should fall back to SKU if ID lookup fails", () => { + // Setup data with shopify_id that is invalid + mockService.setSheetData("product_inventory", [ + ["sku", "shopify_id"], + ["TEST-SKU-FAIL", "999999999"] + ]); + const product = new Product("TEST-SKU-FAIL", mockService); + + // Mock ID lookup failure (returns null/undefined) + mockShop.mockProductById = null; + + // Mock SKU lookup success + mockShop.mockProductBySku = { + id: "555555555", + title: "Found By SKU", + variants: { nodes: [{ id: "gid://shopify/ProductVariant/555555555", sku: "TEST-SKU-FAIL" }] }, + options: [{ id: "opt2", optionValues: [{ id: "optval2" }] }] + }; + + product.MatchToShopifyProduct(mockShop); + + expect(mockShop.getProductByIdCalledWith).toBe("999999999"); + // Should fall back to SKU + expect(mockShop.getProductBySkuCalledWith).toBe("TEST-SKU-FAIL"); + expect(product.shopify_id).toBe("555555555"); + }); +}); diff --git a/src/interfaces/IShop.ts b/src/interfaces/IShop.ts new file mode 100644 index 0000000..6764da7 --- /dev/null +++ b/src/interfaces/IShop.ts @@ -0,0 +1,13 @@ +import * as shopify from "shopify-admin-api-typings"; +import { Config } from "../config"; + +export interface IShop { + GetProductBySku(sku: string): any; // Return type is inferred as product node + GetProductById(id: string): any; // New method + GetInventoryItemBySku(sku: string): shopify.InventoryItem; + UpdateInventoryItemQuantity(item: shopify.InventoryItem, delta: number, config: Config): shopify.InventoryItem; + SetInventoryItemQuantity(item: shopify.InventoryItem, quantity: number, config: Config): any; + SetInventoryItemDefaults(item: shopify.InventoryItem, config: Config): shopify.InventoryItem; + SetInventoryItemWeight(item: shopify.InventoryItem, config: Config, weight: number, weight_unit: shopify.WeightUnit): shopify.InventoryItem; + shopifyGraphQLAPI(payload: any): any; +} diff --git a/src/shopifyApi.ts b/src/shopifyApi.ts index ab63da4..191421b 100644 --- a/src/shopifyApi.ts +++ b/src/shopifyApi.ts @@ -18,6 +18,7 @@ import { Config } from "./config" import * as shopify from "shopify-admin-api-typings" import gql from 'graphql-tag' +import { IShop } from "./interfaces/IShop" const ss = SpreadsheetApp.getActive() @@ -392,7 +393,7 @@ function parseLinkHeader(header) { return rels } -export class Shop { +export class Shop implements IShop { private shopifyApiKey: string private shopifyApiSecretKey: string private shopifyAdminApiAccessToken: string @@ -599,6 +600,34 @@ export class Shop { return product } + GetProductById(id: string) { + console.log("GetProductById('" + id + "')") + let gql = /* GraphQL */ ` + query productById { + product(id: "${id}") { + id + title + handle + variants(first: 1) { + nodes { + id + sku + } + } + } + } + ` + let query = buildGqlQuery(gql, {}) + let response = this.shopifyGraphQLAPI(query) + if (!response.content.data.product) { + console.log("GetProductById: no product matched") + return null; + } + let product = response.content.data.product + console.log("Product found:\n" + JSON.stringify(product, null, 2)) + return product + } + GetInventoryItemBySku(sku: string) { console.log('GetInventoryItemBySku("' + sku + '")') let gql = /* GraphQL */ ` @@ -1132,6 +1161,7 @@ export class ShopifyProductSetQuery { export class ShopifyProductSetInput { category: string descriptionHtml: string + handle: string id?: string productType: string redirectNewHandle: boolean = true diff --git a/src/test/MockShop.ts b/src/test/MockShop.ts new file mode 100644 index 0000000..04c9d40 --- /dev/null +++ b/src/test/MockShop.ts @@ -0,0 +1,52 @@ +import { IShop } from "../interfaces/IShop"; +import * as shopify from "shopify-admin-api-typings"; +import { Config } from "../config"; + +export class MockShop implements IShop { + // Mock methods to spy on calls + public getProductBySkuCalledWith: string | null = null; + public getProductByIdCalledWith: string | null = null; + public productSetCalledWith: any | null = null; + public shopifyGraphQLAPICalledWith: any | null = null; + + public mockProductBySku: any = null; + public mockProductById: any = null; + public mockResponse: any = {}; + + GetProductBySku(sku: string) { + this.getProductBySkuCalledWith = sku; + return this.mockProductBySku || {}; + } + + GetProductById(id: string) { + this.getProductByIdCalledWith = id; + return this.mockProductById; + } + + shopifyGraphQLAPI(payload: any): any { + this.shopifyGraphQLAPICalledWith = payload; + // Basic mock response structure if needed + if (payload.query && payload.query.includes("productSet")) { + this.productSetCalledWith = payload; + } + return { + content: { + data: { + productSet: { + product: { id: "mock-new-id" } + }, + products: { + edges: [] + } + } + } + }; + } + + // Stubs for other interface methods + GetInventoryItemBySku(sku: string): shopify.InventoryItem { return { id: "mock-inv-id" } as any; } + UpdateInventoryItemQuantity(item: shopify.InventoryItem, delta: number, config: Config): shopify.InventoryItem { return {} as any; } + SetInventoryItemQuantity(item: shopify.InventoryItem, quantity: number, config: Config): any { return {}; } + SetInventoryItemDefaults(item: shopify.InventoryItem, config: Config): shopify.InventoryItem { return {} as any; } + SetInventoryItemWeight(item: shopify.InventoryItem, config: Config, weight: number, weight_unit: shopify.WeightUnit): shopify.InventoryItem { return {} as any; } +} diff --git a/test_output.txt b/test_output.txt new file mode 100644 index 0000000..c92e8ab Binary files /dev/null and b/test_output.txt differ