Refactor Media Manager sync logic and fix duplication bugs
This major refactor addresses improper image matching and duplication: - Implemented strict ID-based matching in 'MediaService', removing the greedy filename matching fallback. - Redesigned synchronization pipeline to treat Google Drive as the Source of Truth, supporting orphan adoption (Shopify -> Drive) and secure uploads. - Implemented 'gallery_order' using Drive file properties (supporting both v2 and v3 APIs) for stable, drag-and-drop global ordering. - Added conditional file renaming using timestamps to enforce '_' naming convention without unnecessary renames. - Fixed runtime errors in 'MediaService' loops and updated 'ShopifyMediaService' GraphQL mutations to match correctly schema. - Rewrote 'MediaService.test.ts' with robust test cases for strict matching, adoption, sorting, and reordering.
This commit is contained in:
Ben Miller
@ -23,6 +23,48 @@ export class MediaService {
|
||||
|
||||
|
||||
|
||||
|
||||
getDiagnostics(sku: string, shopifyProductId: string) {
|
||||
const results = {
|
||||
drive: { status: 'pending', fileCount: 0, folderId: null, folderUrl: null, error: null },
|
||||
shopify: { status: 'pending', mediaCount: 0, id: shopifyProductId, adminUrl: null, error: null },
|
||||
matching: { status: 'pending', error: null }
|
||||
}
|
||||
|
||||
// 1. Unsafe Drive Check
|
||||
try {
|
||||
const folder = this.driveService.getOrCreateFolder(sku, this.config.productPhotosFolderId)
|
||||
results.drive.folderId = folder.getId()
|
||||
results.drive.folderUrl = folder.getUrl()
|
||||
const files = this.driveService.getFiles(folder.getId())
|
||||
results.drive.fileCount = files.length
|
||||
results.drive.status = 'ok'
|
||||
} catch (e) {
|
||||
results.drive.status = 'error'
|
||||
results.drive.error = e.toString()
|
||||
}
|
||||
|
||||
// 2. Unsafe Shopify Check
|
||||
try {
|
||||
if (shopifyProductId) {
|
||||
const media = this.shopifyMediaService.getProductMedia(shopifyProductId)
|
||||
results.shopify.mediaCount = media.length
|
||||
// Admin URL construction (Best effort)
|
||||
// Assuming standard Shopify admin pattern
|
||||
const domain = this.shopifyMediaService.getShopDomain? this.shopifyMediaService.getShopDomain() : 'admin.shopify.com';
|
||||
results.shopify.adminUrl = `https://${domain.replace('.myshopify.com','')}.myshopify.com/admin/products/${shopifyProductId.split('/').pop()}`
|
||||
results.shopify.status = 'ok'
|
||||
} else {
|
||||
results.shopify.status = 'skipped' // Not linked yet
|
||||
}
|
||||
} catch (e) {
|
||||
results.shopify.status = 'error'
|
||||
results.shopify.error = e.toString()
|
||||
}
|
||||
|
||||
return results
|
||||
}
|
||||
|
||||
getUnifiedMediaState(sku: string, shopifyProductId: string): any[] {
|
||||
console.log(`MediaService: Getting unified state for SKU ${sku}`)
|
||||
|
||||
@ -41,30 +83,33 @@ export class MediaService {
|
||||
const matchedShopifyIds = new Set<string>()
|
||||
|
||||
// Map of Drive Files
|
||||
const driveMap = new Map<string, {file: GoogleAppsScript.Drive.File, shopifyId: string | null}>()
|
||||
|
||||
driveFiles.forEach(f => {
|
||||
const driveFileStats = driveFiles.map(f => {
|
||||
let shopifyId = null
|
||||
let galleryOrder = 9999
|
||||
try {
|
||||
// Expensive lookup for properties:
|
||||
if (typeof Drive !== 'undefined') {
|
||||
const advFile = (Drive as any).Files.get(f.getId(), { fields: 'appProperties' })
|
||||
if (advFile.appProperties && advFile.appProperties['shopify_media_id']) {
|
||||
shopifyId = advFile.appProperties['shopify_media_id']
|
||||
}
|
||||
const props = this.driveService.getFileProperties(f.getId())
|
||||
if (props['shopify_media_id']) {
|
||||
shopifyId = props['shopify_media_id']
|
||||
}
|
||||
if (props['gallery_order']) {
|
||||
galleryOrder = parseInt(props['gallery_order'])
|
||||
}
|
||||
} catch (e) {
|
||||
console.warn(`Failed to get properties for ${f.getName()}`)
|
||||
}
|
||||
|
||||
driveMap.set(f.getId(), { file: f, shopifyId })
|
||||
return { file: f, shopifyId, galleryOrder }
|
||||
})
|
||||
|
||||
// Match Logic
|
||||
driveFiles.forEach(f => {
|
||||
const d = driveMap.get(f.getId())
|
||||
if (!d) return
|
||||
// Sort: Gallery Order ASC, then Filename ASC
|
||||
driveFileStats.sort((a, b) => {
|
||||
if (a.galleryOrder !== b.galleryOrder) {
|
||||
return a.galleryOrder - b.galleryOrder
|
||||
}
|
||||
return a.file.getName().localeCompare(b.file.getName())
|
||||
})
|
||||
|
||||
// Match Logic (Strict ID Match Only)
|
||||
driveFileStats.forEach(d => {
|
||||
let match = null
|
||||
|
||||
// 1. ID Match
|
||||
@ -73,23 +118,17 @@ export class MediaService {
|
||||
if (match) matchedShopifyIds.add(match.id)
|
||||
}
|
||||
|
||||
// 2. Filename Match (if no ID match)
|
||||
if (!match) {
|
||||
match = shopifyMedia.find(m =>
|
||||
!matchedShopifyIds.has(m.id) &&
|
||||
(m.filename === f.getName() || (m.preview && m.preview.image && m.preview.image.originalSrc && m.preview.image.originalSrc.includes(f.getName())))
|
||||
)
|
||||
if (match) matchedShopifyIds.add(match.id)
|
||||
}
|
||||
// NO Filename Fallback matching per new design "Strict Linkage"
|
||||
|
||||
unifiedState.push({
|
||||
id: f.getId(), // Use Drive ID as primary key for "Synced" or "Drive" items
|
||||
driveId: f.getId(),
|
||||
id: d.file.getId(), // Use Drive ID as primary key
|
||||
driveId: d.file.getId(),
|
||||
shopifyId: match ? match.id : null,
|
||||
filename: f.getName(),
|
||||
filename: d.file.getName(),
|
||||
source: match ? 'synced' : 'drive_only',
|
||||
thumbnail: `data:image/jpeg;base64,${Utilities.base64Encode(f.getThumbnail().getBytes())}`, // Expensive?
|
||||
status: 'active'
|
||||
thumbnail: `data:image/jpeg;base64,${Utilities.base64Encode(d.file.getThumbnail().getBytes())}`,
|
||||
status: 'active',
|
||||
galleryOrder: d.galleryOrder
|
||||
})
|
||||
})
|
||||
|
||||
@ -100,10 +139,12 @@ export class MediaService {
|
||||
id: m.id, // Use Shopify ID keys for orphans
|
||||
driveId: null,
|
||||
shopifyId: m.id,
|
||||
filename: "Shopify Media", // TODO: extract real name
|
||||
filename: "Orphaned Media", // Shopify doesn't always expose filename cleanly in same way
|
||||
// Try to get filename if possible or fallback
|
||||
source: 'shopify_only',
|
||||
thumbnail: m.preview?.image?.originalSrc || "",
|
||||
status: 'active'
|
||||
status: 'active',
|
||||
galleryOrder: 10000 // End of list
|
||||
})
|
||||
}
|
||||
})
|
||||
@ -111,79 +152,106 @@ export class MediaService {
|
||||
return unifiedState
|
||||
}
|
||||
|
||||
processMediaChanges(sku: string, finalState: any[], shopifyProductId: string) {
|
||||
processMediaChanges(sku: string, finalState: any[], shopifyProductId: string): string[] {
|
||||
const logs: string[] = []
|
||||
logs.push(`Starting processing for SKU ${sku}`)
|
||||
console.log(`MediaService: Processing changes for SKU ${sku}`)
|
||||
|
||||
// 0. Service Availability Check & Local Capture (Fixing 'undefined' context issues)
|
||||
const shopifySvc = this.shopifyMediaService
|
||||
const driveSvc = this.driveService
|
||||
|
||||
if (!shopifySvc) throw new Error("MediaService Error: shopifyMediaService is undefined")
|
||||
if (!driveSvc) throw new Error("MediaService Error: driveService is undefined")
|
||||
|
||||
// 1. Get Current State (for diffing deletions)
|
||||
const currentState = this.getUnifiedMediaState(sku, shopifyProductId)
|
||||
const finalIds = new Set(finalState.map(f => f.id))
|
||||
|
||||
// 2. Process Deletions
|
||||
// 2. Process Deletions (Orphans not in final state are removed from Shopify)
|
||||
const toDelete = currentState.filter(c => !finalIds.has(c.id))
|
||||
if (toDelete.length === 0) logs.push("No deletions found.")
|
||||
|
||||
toDelete.forEach(item => {
|
||||
console.log(`Deleting item: ${item.filename}`)
|
||||
const msg = `Deleting item: ${item.filename}`
|
||||
logs.push(msg)
|
||||
console.log(msg)
|
||||
if (item.shopifyId) {
|
||||
this.shopifyMediaService.productDeleteMedia(shopifyProductId, item.shopifyId)
|
||||
shopifySvc.productDeleteMedia(shopifyProductId, item.shopifyId)
|
||||
logs.push(`- Deleted from Shopify (${item.shopifyId})`)
|
||||
}
|
||||
if (item.driveId) {
|
||||
this.driveService.trashFile(item.driveId)
|
||||
driveSvc.trashFile(item.driveId)
|
||||
logs.push(`- Trashed in Drive (${item.driveId})`)
|
||||
}
|
||||
})
|
||||
|
||||
// 3. Process Backfills (Shopify Only -> Drive)
|
||||
// 3. Process Adoptions (Shopify Orphans -> Drive)
|
||||
// Identify items that are source='shopify_only' but are KEPT in the final state.
|
||||
// These need to be downloaded to become the source of truth in Drive.
|
||||
finalState.forEach(item => {
|
||||
if (item.source === 'shopify_only' && item.shopifyId) {
|
||||
console.log(`Backfilling item: ${item.filename}`)
|
||||
// Download using global UrlFetchApp for blob access if generic interface is limited?
|
||||
// Actually implementation of INetworkService returns HTTPResponse which has getBlob().
|
||||
// But item.thumbnail usually is a URL.
|
||||
const resp = this.networkService.fetch(item.thumbnail, { method: 'get' })
|
||||
const blob = resp.getBlob()
|
||||
const file = this.driveService.createFile(blob)
|
||||
const msg = `Adopting Orphan: ${item.filename}`
|
||||
logs.push(msg)
|
||||
console.log(msg)
|
||||
|
||||
// Move to correct folder
|
||||
const folder = this.driveService.getOrCreateFolder(sku, this.config.productPhotosFolderId)
|
||||
const driveFile = this.driveService.getFileById(file.getId())
|
||||
// GASDriveService must handle move? Standard File has moveTo?
|
||||
// "moveTo" is standard GAS.
|
||||
// But we used interface `IDriveService` which returns `GoogleAppsScript.Drive.File`.
|
||||
// So we can assume `driveFile.moveTo(folder)` works if it's a real GAS object.
|
||||
// TypeScript might complain if `IDriveService` returns a wrapper?
|
||||
// Interface says `GoogleAppsScript.Drive.File`. That is the native type.
|
||||
// Native type has `moveTo(destination: Folder)`.
|
||||
driveFile.moveTo(folder)
|
||||
try {
|
||||
// Download
|
||||
const resp = this.networkService.fetch(item.thumbnail, { method: 'get' })
|
||||
const blob = resp.getBlob()
|
||||
blob.setName(`${sku}_adopted_${Date.now()}.jpg`) // Safety rename
|
||||
const file = driveSvc.createFile(blob)
|
||||
|
||||
this.driveService.updateFileProperties(file.getId(), { shopify_media_id: item.shopifyId })
|
||||
// Move to correct folder
|
||||
const folder = driveSvc.getOrCreateFolder(sku, this.config.productPhotosFolderId)
|
||||
const driveFile = driveSvc.getFileById(file.getId())
|
||||
// driveFile.moveTo(folder) // GAS Hack: make sure to add parents/remove parents if needed, or create in place
|
||||
// Mock/GAS adapter should handle folder placement correctly if possible, or we assume create puts in root and we move.
|
||||
// For this refactor, let's assume `createFile` puts it where it needs to be or we accept root for now.
|
||||
// ACTUALLY: The GASDriveService implementation uses DriveApp.createFile which puts in root.
|
||||
// We should move it strictly.
|
||||
folder.addFile(driveFile)
|
||||
DriveApp.getRootFolder().removeFile(driveFile)
|
||||
|
||||
// Update item refs for subsequent steps
|
||||
item.driveId = file.getId()
|
||||
item.source = 'synced'
|
||||
|
||||
driveSvc.updateFileProperties(file.getId(), { shopify_media_id: item.shopifyId })
|
||||
|
||||
// Update item refs for subsequent steps
|
||||
item.driveId = file.getId()
|
||||
item.source = 'synced'
|
||||
logs.push(`- Adopted to Drive (${file.getId()})`)
|
||||
} catch (e) {
|
||||
logs.push(`- Failed to adopt ${item.filename}: ${e}`)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
// 4. Process Uploads (Drive Only -> Shopify)
|
||||
const toUpload = finalState.filter(item => item.source === 'drive_only' && item.driveId)
|
||||
if (toUpload.length > 0) {
|
||||
console.log(`Uploading ${toUpload.length} new items from Drive`)
|
||||
|
||||
const msg = `Uploading ${toUpload.length} new items from Drive`
|
||||
logs.push(msg)
|
||||
const uploads = toUpload.map(item => {
|
||||
const f = this.driveService.getFileById(item.driveId)
|
||||
const f = driveSvc.getFileById(item.driveId)
|
||||
return {
|
||||
filename: f.getName(),
|
||||
mimeType: f.getMimeType(),
|
||||
resource: "IMAGE",
|
||||
httpMethod: "POST",
|
||||
file: f
|
||||
file: f,
|
||||
originalItem: item
|
||||
}
|
||||
})
|
||||
|
||||
// ... (Existing upload logic logic, simplified for brevity in plan, but fully implemented here)
|
||||
// Batch Staged Uploads
|
||||
const stagedInput = uploads.map(u => ({
|
||||
filename: u.filename,
|
||||
mimeType: u.mimeType,
|
||||
resource: u.resource,
|
||||
httpMethod: u.httpMethod
|
||||
}))
|
||||
const stagedResp = this.shopifyMediaService.stagedUploadsCreate(stagedInput)
|
||||
const stagedResp = shopifySvc.stagedUploadsCreate(stagedInput)
|
||||
const targets = stagedResp.stagedTargets
|
||||
|
||||
const mediaToCreate = []
|
||||
@ -192,9 +260,7 @@ export class MediaService {
|
||||
const payload = {}
|
||||
target.parameters.forEach((p: any) => payload[p.name] = p.value)
|
||||
payload['file'] = u.file.getBlob()
|
||||
|
||||
this.networkService.fetch(target.url, { method: "post", payload: payload })
|
||||
|
||||
mediaToCreate.push({
|
||||
originalSource: target.resourceUrl,
|
||||
alt: u.filename,
|
||||
@ -202,49 +268,73 @@ export class MediaService {
|
||||
})
|
||||
})
|
||||
|
||||
// Create Media (Updated to return IDs)
|
||||
const createdMedia = this.shopifyMediaService.productCreateMedia(shopifyProductId, mediaToCreate)
|
||||
const createdMedia = shopifySvc.productCreateMedia(shopifyProductId, mediaToCreate)
|
||||
if (createdMedia && createdMedia.media) {
|
||||
createdMedia.media.forEach((m: any, i: number) => {
|
||||
const originalItem = toUpload[i]
|
||||
const originalItem = uploads[i].originalItem
|
||||
if (m.status === 'FAILED') {
|
||||
console.error("Media create failed", m)
|
||||
logs.push(`- Failed to create media for ${originalItem.filename}: ${m.message}`)
|
||||
return
|
||||
}
|
||||
if (m.id) {
|
||||
this.driveService.updateFileProperties(originalItem.driveId, { shopify_media_id: m.id })
|
||||
driveSvc.updateFileProperties(originalItem.driveId, { shopify_media_id: m.id })
|
||||
originalItem.shopifyId = m.id
|
||||
originalItem.source = 'synced'
|
||||
logs.push(`- Created in Shopify (${m.id}) and linked`)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// 5. Process Reordering
|
||||
const moves: any[] = []
|
||||
// 5. Sequential Reordering & Renaming
|
||||
// Now that we have Drive IDs and Shopify IDs for everything (orphans adopted, new files uploaded)
|
||||
// We update the gallery_order on ALL Drive files to match the finalState order (0-indexed).
|
||||
// And we check filenames.
|
||||
|
||||
const reorderMoves: any[] = []
|
||||
|
||||
finalState.forEach((item, index) => {
|
||||
if (item.shopifyId) {
|
||||
moves.push({ id: item.shopifyId, newPosition: index.toString() })
|
||||
if (!item.driveId) return // Should not happen if adoption worked, but safety check
|
||||
|
||||
try {
|
||||
const file = driveSvc.getFileById(item.driveId)
|
||||
|
||||
// A. Update Gallery Order
|
||||
driveSvc.updateFileProperties(item.driveId, { gallery_order: index.toString() })
|
||||
|
||||
// B. Conditional Renaming
|
||||
const currentName = file.getName()
|
||||
const expectedPrefix = `${sku}_`
|
||||
// If name doesn't start with SKU_ or looks like "SKU_timestamp.ext" pattern enforcement
|
||||
// The requirement: "Files will only be renamed if they do not conform to the expected pattern"
|
||||
// Pattern: startWith sku + "_"
|
||||
if (!currentName.startsWith(expectedPrefix)) {
|
||||
const ext = currentName.includes('.') ? currentName.split('.').pop() : 'jpg'
|
||||
// Use file creation time or now for unique suffix
|
||||
const timestamp = new Date().getTime()
|
||||
const newName = `${sku}_${timestamp}.${ext}`
|
||||
driveSvc.renameFile(item.driveId, newName)
|
||||
logs.push(`- Renamed ${currentName} -> ${newName} (Non-conforming)`)
|
||||
}
|
||||
|
||||
// C. Prepare Shopify Reorder
|
||||
if (item.shopifyId) {
|
||||
reorderMoves.push({ id: item.shopifyId, newPosition: index.toString() })
|
||||
}
|
||||
|
||||
} catch (e) {
|
||||
logs.push(`- Error updating ${item.filename}: ${e}`)
|
||||
}
|
||||
})
|
||||
if (moves.length > 0) {
|
||||
this.shopifyMediaService.productReorderMedia(shopifyProductId, moves)
|
||||
|
||||
// 6. Execute Shopify Reorder
|
||||
if (reorderMoves.length > 0) {
|
||||
shopifySvc.productReorderMedia(shopifyProductId, reorderMoves)
|
||||
logs.push("Reordered media in Shopify.")
|
||||
}
|
||||
|
||||
// 6. Rename Drive Files
|
||||
finalState.forEach((item, index) => {
|
||||
if (item.driveId) {
|
||||
const paddedIndex = (index + 1).toString().padStart(4, '0')
|
||||
const ext = item.filename.includes('.') ? item.filename.split('.').pop() : 'jpg'
|
||||
const newName = `${sku}_${paddedIndex}.${ext}`
|
||||
|
||||
// Avoid unnecessary rename validation calls if possible, but renameFile is fast
|
||||
const startName = this.driveService.getFileById(item.driveId).getName()
|
||||
if (startName !== newName) {
|
||||
this.driveService.renameFile(item.driveId, newName)
|
||||
}
|
||||
}
|
||||
})
|
||||
logs.push("Processing Complete.")
|
||||
return logs
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user