From 17441a9b30f0558944d47e810288768a5dda9905 Mon Sep 17 00:00:00 2001 From: Christian Vidal Wolf Date: Thu, 22 Jan 2026 11:37:53 +0100 Subject: [PATCH] fix: correct ad spend over-calculation in Weekly Grid - Refactored mergeSalesAndAdsData to aggregate by ASIN+Customer+Year+Week - Each ASIN/Week now produces exactly one output record - Updated pivotWeeklySalesData to use ASIN as primary key - Fixed double-counting when same ASIN had multiple SKUs - Added support for ads-only records (ASINs with spend but no sales) - Changed App.tsx to use filtered ads data for accurate totals --- App.tsx | 4 +- services/dataProcessor.ts | 182 ++++++++++++++++++++++---------------- 2 files changed, 109 insertions(+), 77 deletions(-) diff --git a/App.tsx b/App.tsx index 07643b7..d904ed5 100644 --- a/App.tsx +++ b/App.tsx @@ -248,8 +248,8 @@ const App: React.FC = () => { // Combine Sales & Ads Data dynamically based on current filters const combinedAdsData = useMemo(() => { - return mergeSalesAndAdsData(filteredData, adsData); - }, [filteredData, adsData]); + return mergeSalesAndAdsData(filteredData, filteredAdsData); + }, [filteredData, filteredAdsData]); // Derive Context Data (Product Line Context when drilling down) const contextAggregatedData = useMemo(() => { diff --git a/services/dataProcessor.ts b/services/dataProcessor.ts index 35fd188..5a5b30e 100644 --- a/services/dataProcessor.ts +++ b/services/dataProcessor.ts @@ -442,14 +442,61 @@ export const processAdsExcel = async (fileOrBuffer: File | ArrayBuffer): Promise // --- DATA MERGING --- export const mergeSalesAndAdsData = (salesData: SalesRecord[], adsData: AdsRecord[]): CombinedKPIs[] => { - // 1. Index Ads Data for fast lookup: Key = ASIN + Marketplace + Year + Week + // Key for both sales and ads: ASIN|Customer|Year|Week + const createKey = (asin: string, customer: string, year: number, week: number) => + `${asin.trim().toUpperCase()}|${customer.trim().toUpperCase()}|${year}|${week}`; + + // 1. Aggregate Sales by ASIN|Customer|Year|Week (combine all SKUs) + const salesMap = new Map(); + + salesData.forEach(sale => { + const weekNum = sale.week || 0; + if (weekNum === 0) return; // Skip records without week data for weekly analysis + + const key = createKey(sale.asin, sale.customer, sale.year, weekNum); + + if (salesMap.has(key)) { + const existing = salesMap.get(key)!; + existing.sellOut += sale.sellOut; + existing.units += sale.units; + // Keep the best metadata (longest title, first non-empty SKU) + if (sale.title && sale.title.length > (existing.title?.length || 0)) { + existing.title = sale.title; + } + if (sale.sku && !existing.sku) { + existing.sku = sale.sku; + } + } else { + salesMap.set(key, { + sellOut: sale.sellOut, + units: sale.units, + sku: sale.sku, + title: sale.title, + line: sale.line, + asin: sale.asin, + customer: sale.customer, + year: sale.year, + week: weekNum, + month: sale.month + }); + } + }); + + // 2. Aggregate Ads by ASIN|Customer|Year|Week const adsMap = new Map(); - adsData.forEach(ad => { - // Case-insensitive key using ASIN + Country + Year + Week - const key = `${ad.asin.trim().toUpperCase()}|${ad.country.trim().toUpperCase()}|${ad.year}|${ad.week}`; - - // If duplicates exist (e.g. multiple campaigns for same ASIN), sum them up + const key = createKey(ad.asin, ad.country, ad.year, ad.week); if (adsMap.has(key)) { const existing = adsMap.get(key)!; existing.cost += ad.cost; @@ -463,107 +510,88 @@ export const mergeSalesAndAdsData = (salesData: SalesRecord[], adsData: AdsRecor } }); - // 2. Iterate Sales Data and merge - const mergedData: CombinedKPIs[] = salesData.map(sale => { - // Use week from sales record if available - const weekNum = sale.week || 0; - const key = `${sale.asin.trim().toUpperCase()}|${sale.customer.trim().toUpperCase()}|${sale.year}|${weekNum}`; - const adData = adsMap.get(key) || { - country: sale.customer, - year: sale.year, - week: weekNum, - asin: sale.asin, - cost: 0, - clicks: 0, - impressions: 0, - cpc: 0, - ctr: 0, - acos: 0, - conversions: 0, - attributedSales30d: 0, - attributedUnits30d: 0 - }; + const mergedData: CombinedKPIs[] = []; + const processedKeys = new Set(); + + // 3. Create ONE record per ASIN/Customer/Year/Week from sales, attach ads if available + salesMap.forEach((sale, key) => { + processedKeys.add(key); + const ad = adsMap.get(key); + + const adCost = ad?.cost || 0; + const adClicks = ad?.clicks || 0; + const adImpressions = ad?.impressions || 0; + const adSales = ad?.attributedSales30d || 0; + const adUnits = ad?.attributedUnits30d || 0; const salesTotal = sale.sellOut; - const salesAds = adData.attributedSales30d; - // Logic: Organic = Total - Ads. Max(0) to avoid negative if attribution window logic differs vs finance dates - const salesOrganic = Math.max(0, salesTotal - salesAds); - const unitsTotal = sale.units; - const unitsAds = adData.attributedUnits30d; - const unitsOrganic = Math.max(0, unitsTotal - unitsAds); + const salesOrganic = Math.max(0, salesTotal - adSales); + const unitsOrganic = Math.max(0, unitsTotal - adUnits); // KPIs - const acos = salesAds > 0 ? (adData.cost / salesAds) * 100 : 0; - const tacos = salesTotal > 0 ? (adData.cost / salesTotal) * 100 : 0; - const roas = adData.cost > 0 ? salesAds / adData.cost : 0; - const ctr = adData.impressions > 0 ? (adData.clicks / adData.impressions) * 100 : 0; - const cpc = adData.clicks > 0 ? adData.cost / adData.clicks : 0; - // CVR (Units / Clicks) - const cvrUnits = adData.clicks > 0 ? (unitsAds / adData.clicks) * 100 : 0; + const acos = adSales > 0 ? (adCost / adSales) * 100 : 0; + const tacos = salesTotal > 0 ? (adCost / salesTotal) * 100 : 0; + const roas = adCost > 0 ? adSales / adCost : 0; + const ctr = adImpressions > 0 ? (adClicks / adImpressions) * 100 : 0; + const cpc = adClicks > 0 ? adCost / adClicks : 0; + const cvrUnits = adClicks > 0 ? (adUnits / adClicks) * 100 : 0; - const paidSalesShare = salesTotal > 0 ? (salesAds / salesTotal) * 100 : 0; - const organicSalesShare = salesTotal > 0 ? (salesOrganic / salesTotal) * 100 : 0; - - return { - id: sale.id, + mergedData.push({ + id: `merged-${key}`, marketplace: sale.customer, customer: sale.customer, month: sale.month, - week: sale.week || 0, // Preserve week info + week: sale.week, year: sale.year, asin: sale.asin, title: sale.title, line: sale.line, sku: sale.sku, - salesTotal, unitsTotal, - - salesAds, - unitsAds, - cost: adData.cost, - clicks: adData.clicks, - impressions: adData.impressions, - + salesAds: adSales, + unitsAds: adUnits, + cost: adCost, + clicks: adClicks, + impressions: adImpressions, salesOrganic, unitsOrganic, - - paidSalesShare, - organicSalesShare, - + paidSalesShare: salesTotal > 0 ? (adSales / salesTotal) * 100 : 0, + organicSalesShare: salesTotal > 0 ? (salesOrganic / salesTotal) * 100 : 0, acos, tacos, roas, ctr, cpc, cvrUnits - }; + }); }); - // 3. Include ads-only records (ASINs with ads but no sales) - const usedAdsKeys = new Set(); - salesData.forEach(sale => { - const weekNum = sale.week || 0; - const key = `${sale.asin.trim().toUpperCase()}|${sale.customer.trim().toUpperCase()}|${sale.year}|${weekNum}`; - usedAdsKeys.add(key); - }); + // 4. Add ads-only records (ASINs with ads but no sales in the filtered data) + adsMap.forEach((ad, key) => { + if (!processedKeys.has(key)) { + // Look up metadata from sales data for this ASIN (any week) + let meta: { sku: string; title: string; line: string } | undefined; + salesMap.forEach((sale, saleKey) => { + if (saleKey.startsWith(ad.asin.trim().toUpperCase() + '|' + ad.country.trim().toUpperCase())) { + if (!meta || sale.title?.length > meta.title?.length) { + meta = { sku: sale.sku, title: sale.title, line: sale.line }; + } + } + }); - adsData.forEach(ad => { - const key = `${ad.asin.trim().toUpperCase()}|${ad.country.trim().toUpperCase()}|${ad.year}|${ad.week}`; - if (!usedAdsKeys.has(key)) { - // Create a CombinedKPIs record for ads-only data mergedData.push({ - id: `ads-${key}`, + id: `ads-only-${key}`, marketplace: ad.country, customer: ad.country, - month: '', + month: 'N/A', week: ad.week, year: ad.year, asin: ad.asin, - title: '', - line: '', - sku: '', + title: meta?.title || ad.asin, + line: meta?.line || 'Unassigned', + sku: meta?.sku || '', salesTotal: 0, unitsTotal: 0, salesAds: ad.attributedSales30d, @@ -589,6 +617,7 @@ export const mergeSalesAndAdsData = (salesData: SalesRecord[], adsData: AdsRecor }; + // --- EXISTING HELPERS --- // Filter Ads Data by Country, Year, Week, and ASIN @@ -1327,7 +1356,8 @@ export const pivotWeeklySalesData = (data: CombinedKPIs[]): { const map = new Map(); data.forEach(record => { - const key = record.sku || record.asin || `${record.title}-${record.line}`; + // Use ASIN as primary key since mergeSalesAndAdsData outputs one record per ASIN/week + const key = record.asin || record.sku || `${record.title}-${record.line}`; if (!key) return; if (!map.has(key)) { @@ -1347,6 +1377,8 @@ export const pivotWeeklySalesData = (data: CombinedKPIs[]): { if (record.week) { const weekKey = `${record.year}-${String(record.week).padStart(2, '0')}`; row.unitsByWeek[weekKey] = (row.unitsByWeek[weekKey] || 0) + record.unitsTotal; + // Only add cost if we haven't already added it for this ASIN/week + // Since mergeSalesAndAdsData now outputs one record per ASIN/week, this should be clean row.spendByWeek[weekKey] = (row.spendByWeek[weekKey] || 0) + (record.cost || 0); } });