Optimize media manager polling and product info retrieval

- Remove recursive polling in MediaManager.html; context is now loaded once at startup.
- Optimize getSelectedProductInfo in mediaHandlers.ts to reduce SpreadsheetApp API calls.
- Update related tests to match new optimization.
This commit is contained in:
Ben Miller
2025-12-31 08:53:39 -07:00
parent ad67dd9ab5
commit 8487df3ea0
3 changed files with 57 additions and 5 deletions

View File

@ -1206,8 +1206,8 @@
*/ */
var controller = { var controller = {
init() { init() {
// Start polling for SKU selection // Initialize by checking SKU once
setInterval(() => this.checkSku(), 2000); // Since this is a modal, the selection cannot change during the session.
this.checkSku(); this.checkSku();
}, },
@ -1222,10 +1222,19 @@
this.loadMedia(); this.loadMedia();
} else if (!sku && !state.sku) { } else if (!sku && !state.sku) {
if (document.getElementById('error-ui').style.display !== 'flex') { if (document.getElementById('error-ui').style.display !== 'flex') {
this.loadMedia(); this.loadMedia(); // Likely to trigger error UI
} }
} }
}) })
.withFailureHandler(e => {
console.warn("SKU check failed", e);
// If it fails once at startup, we probably should alert or retry once,
// but for now let's just leave it. If it fails, the UI might hang on "Loading..."
// potentially better to trigger error UI?
if (document.getElementById('loading-ui').style.display !== 'none') {
alert("Failed to load product info: " + e.message);
}
})
.getSelectedProductInfo(); .getSelectedProductInfo();
}, },

View File

@ -449,6 +449,29 @@ describe("mediaHandlers", () => {
}) })
test("getSelectedProductInfo should return sku and title from sheet", () => { test("getSelectedProductInfo should return sku and title from sheet", () => {
// Mock SpreadsheetApp behavior specifically for the optimized implementation
// The implementation calls:
// 1. sheet.getRange(1, 1, 1, lastCol).getValues()[0] (headers)
// 2. sheet.getRange(row, 1, 1, lastCol).getValues()[0] (values)
const mockRange = {
getValues: jest.fn()
};
const mockSheet = {
getName: jest.fn().mockReturnValue("product_inventory"),
getLastColumn: jest.fn().mockReturnValue(3),
getActiveRange: jest.fn().mockReturnValue({ getRow: () => 5 }),
getRange: jest.fn().mockReturnValue(mockRange)
};
(global.SpreadsheetApp.getActiveSheet as jest.Mock).mockReturnValue(mockSheet);
// First call: Headers
mockRange.getValues.mockReturnValueOnce([["sku", "title", "thumbnail"]]);
// Second call: Row Values
mockRange.getValues.mockReturnValueOnce([["TEST-SKU", "Test Product Title", "thumb.jpg"]]);
const info = getSelectedProductInfo() const info = getSelectedProductInfo()
expect(info).toEqual({ sku: "TEST-SKU", title: "Test Product Title" }) expect(info).toEqual({ sku: "TEST-SKU", title: "Test Product Title" })
}) })

View File

@ -17,14 +17,34 @@ export function showMediaManager() {
export function getSelectedProductInfo(): { sku: string, title: string } | null { export function getSelectedProductInfo(): { sku: string, title: string } | null {
const ss = new GASSpreadsheetService() const ss = new GASSpreadsheetService()
// Optimization: Direct usage to avoid multiple service calls overhead
// Use SpreadsheetApp only once if possible to get active context
const sheet = SpreadsheetApp.getActiveSheet() const sheet = SpreadsheetApp.getActiveSheet()
if (sheet.getName() !== "product_inventory") return null if (sheet.getName() !== "product_inventory") return null
const row = sheet.getActiveRange().getRow() const row = sheet.getActiveRange().getRow()
if (row <= 1) return null // Header if (row <= 1) return null // Header
const sku = ss.getCellValueByColumnName("product_inventory", row, "sku") // Optimization: Get the whole row values in one go
const title = ss.getCellValueByColumnName("product_inventory", row, "title") // We need to know which index is SKU and Title.
// Getting headers once is cheaper than searching by name twice if we cache or just linear scan once.
// Actually, getCellValueByColumnName does: getSheet -> getHeaders (read) -> getRowData (read).
// Doing it twice = 6 operations.
// Let's do it manually efficiently:
const headers = sheet.getRange(1, 1, 1, sheet.getLastColumn()).getValues()[0] as string[];
const skuIdx = headers.indexOf("sku");
const titleIdx = headers.indexOf("title");
if (skuIdx === -1) return null; // No SKU column
// Read the specific row
// getRange(row, 1, 1, lastCol)
const rowValues = sheet.getRange(row, 1, 1, sheet.getLastColumn()).getValues()[0];
const sku = rowValues[skuIdx];
const title = titleIdx !== -1 ? rowValues[titleIdx] : "";
return sku ? { sku: String(sku), title: String(title || "") } : null return sku ? { sku: String(sku), title: String(title || "") } : null
} }