diff --git a/PROGRESS.md b/PROGRESS.md index d471bd7..a7fbdb6 100644 --- a/PROGRESS.md +++ b/PROGRESS.md @@ -43,6 +43,11 @@ - Disk cache for variants (regenerable from DB) - `mix optimize_images` task for mockups - On-demand JPEG fallback generation +- Product image download pipeline (downloads Printify CDN images, processes through Media pipeline) + - ImageDownloadWorker downloads and links images to ProductImage + - PreviewData uses local images for responsive `` elements + - Startup recovery re-enqueues pending downloads + - `mix simpleshop.download_images` backfill task See: [docs/plans/image-optimization.md](docs/plans/image-optimization.md) for implementation details @@ -64,6 +69,12 @@ See: [docs/plans/image-optimization.md](docs/plans/image-optimization.md) for im - [ ] Add variant selector component (~2hr) #### Recently Completed +- [x] Product image download pipeline + - Downloads Printify CDN images via ImageDownloadWorker + - Processes through Media pipeline (WebP conversion, AVIF/WebP variants) + - PreviewData uses local images for responsive `` elements + - sync_product_images preserves image_id when URL unchanged + - Startup recovery and `mix simpleshop.download_images` backfill - [x] Wire shop LiveViews to Products context - PreviewData now uses real products when available - Fixed Printify image sync (position was string, not integer) diff --git a/lib/mix/tasks/simpleshop/download_images.ex b/lib/mix/tasks/simpleshop/download_images.ex new file mode 100644 index 0000000..7f4f809 --- /dev/null +++ b/lib/mix/tasks/simpleshop/download_images.ex @@ -0,0 +1,41 @@ +defmodule Mix.Tasks.Simpleshop.DownloadImages do + @moduledoc """ + Enqueues download jobs for product images that haven't been processed yet. + + ## Usage + + mix simpleshop.download_images + + This task finds all product images with a `src` URL but no linked `image_id` + and enqueues them for download via the ImageDownloadWorker. + + Use this to backfill existing products after enabling the image download pipeline. + """ + + use Mix.Task + + @shortdoc "Enqueue product image downloads" + + @impl Mix.Task + def run(_args) do + Mix.Task.run("app.start") + + alias SimpleshopTheme.Products + alias SimpleshopTheme.Sync.ImageDownloadWorker + + pending = Products.list_pending_downloads(limit: 10_000) + count = length(pending) + + if count == 0 do + Mix.shell().info("No pending product images to download.") + else + Mix.shell().info("Enqueueing #{count} product images for download...") + + Enum.each(pending, fn image -> + ImageDownloadWorker.enqueue(image.id) + end) + + Mix.shell().info("Done. Images will be processed by Oban workers.") + end + end +end diff --git a/lib/simpleshop_theme/images/variant_cache.ex b/lib/simpleshop_theme/images/variant_cache.ex index fc387c0..1a6727d 100644 --- a/lib/simpleshop_theme/images/variant_cache.ex +++ b/lib/simpleshop_theme/images/variant_cache.ex @@ -15,6 +15,8 @@ defmodule SimpleshopTheme.Images.VariantCache do alias SimpleshopTheme.Repo alias SimpleshopTheme.Media.Image, as: ImageSchema alias SimpleshopTheme.Images.{Optimizer, OptimizeWorker} + alias SimpleshopTheme.Products + alias SimpleshopTheme.Sync.ImageDownloadWorker import Ecto.Query @mockup_dir "priv/static/mockups" @@ -35,6 +37,7 @@ defmodule SimpleshopTheme.Images.VariantCache do ensure_database_image_variants() ensure_mockup_variants() + ensure_product_image_downloads() end defp ensure_database_image_variants do @@ -100,4 +103,15 @@ defmodule SimpleshopTheme.Images.VariantCache do dir = Path.dirname(source_path) File.exists?(Path.join(dir, "#{basename}-800.webp")) end + + defp ensure_product_image_downloads do + pending = Products.list_pending_downloads(limit: 500) + + if pending == [] do + Logger.info("[VariantCache] All product images downloaded") + else + Logger.info("[VariantCache] Enqueueing #{length(pending)} product images for download") + Enum.each(pending, fn image -> ImageDownloadWorker.enqueue(image.id) end) + end + end end diff --git a/lib/simpleshop_theme/products.ex b/lib/simpleshop_theme/products.ex index 972c71e..17e1955 100644 --- a/lib/simpleshop_theme/products.ex +++ b/lib/simpleshop_theme/products.ex @@ -294,6 +294,40 @@ defmodule SimpleshopTheme.Products do |> Repo.insert() end + @doc """ + Gets a single product image by ID. + """ + def get_product_image(id) do + Repo.get(ProductImage, id) + end + + @doc """ + Links a product image to a Media.Image by setting its image_id. + """ + def link_product_image(%ProductImage{} = product_image, image_id) do + product_image + |> ProductImage.changeset(%{image_id: image_id}) + |> Repo.update() + end + + @doc """ + Lists product images that need downloading (have src but no image_id). + + ## Options + + * `:limit` - maximum number of images to return (default: 100) + """ + def list_pending_downloads(opts \\ []) do + limit = Keyword.get(opts, :limit, 100) + + from(i in ProductImage, + where: not is_nil(i.src) and is_nil(i.image_id), + order_by: [asc: i.inserted_at], + limit: ^limit + ) + |> Repo.all() + end + @doc """ Deletes all images for a product. """ @@ -305,20 +339,60 @@ defmodule SimpleshopTheme.Products do @doc """ Syncs product images from a list of image data. - Deletes existing images and inserts new ones. + Preserves existing image_id references when the URL hasn't changed. + Returns a list of {:ok, image} tuples for images that need downloading. """ - def sync_product_images(%Product{id: product_id} = product, images) when is_list(images) do - delete_product_images(product) + def sync_product_images(%Product{id: product_id}, images) when is_list(images) do + # Build map of existing images by position + existing_by_position = + from(i in ProductImage, where: i.product_id == ^product_id) + |> Repo.all() + |> Map.new(&{&1.position, &1}) + incoming_positions = + images + |> Enum.with_index() + |> Enum.map(fn {image_data, index} -> image_data[:position] || index end) + |> MapSet.new() + + # Delete orphaned positions (images no longer in the list) + orphaned_ids = + existing_by_position + |> Enum.reject(fn {position, _img} -> MapSet.member?(incoming_positions, position) end) + |> Enum.map(fn {_position, img} -> img.id end) + + if orphaned_ids != [] do + from(i in ProductImage, where: i.id in ^orphaned_ids) |> Repo.delete_all() + end + + # Upsert incoming images images |> Enum.with_index() |> Enum.map(fn {image_data, index} -> - attrs = - image_data - |> Map.put(:product_id, product_id) - |> Map.put_new(:position, index) + position = image_data[:position] || index + src = image_data[:src] + existing = Map.get(existing_by_position, position) - create_product_image(attrs) + cond do + # Same URL at position - keep existing (preserve image_id) + existing && existing.src == src -> + {:ok, existing} + + # Different URL at position - update src, clear image_id (triggers re-download) + existing -> + existing + |> ProductImage.changeset(%{src: src, alt: image_data[:alt], image_id: nil}) + |> Repo.update() + + # New position - create new + true -> + attrs = + image_data + |> Map.put(:product_id, product_id) + |> Map.put(:position, position) + + create_product_image(attrs) + end end) end diff --git a/lib/simpleshop_theme/products/product_image.ex b/lib/simpleshop_theme/products/product_image.ex index 81b5369..ca5e5df 100644 --- a/lib/simpleshop_theme/products/product_image.ex +++ b/lib/simpleshop_theme/products/product_image.ex @@ -15,8 +15,10 @@ defmodule SimpleshopTheme.Products.ProductImage do field :src, :string field :position, :integer, default: 0 field :alt, :string + field :image_id, :binary_id belongs_to :product, SimpleshopTheme.Products.Product + belongs_to :image, SimpleshopTheme.Media.Image, define_field: false timestamps(type: :utc_datetime) end @@ -26,8 +28,9 @@ defmodule SimpleshopTheme.Products.ProductImage do """ def changeset(product_image, attrs) do product_image - |> cast(attrs, [:product_id, :src, :position, :alt]) + |> cast(attrs, [:product_id, :src, :position, :alt, :image_id]) |> validate_required([:product_id, :src]) |> foreign_key_constraint(:product_id) + |> foreign_key_constraint(:image_id) end end diff --git a/lib/simpleshop_theme/sync/image_download_worker.ex b/lib/simpleshop_theme/sync/image_download_worker.ex new file mode 100644 index 0000000..e2139b9 --- /dev/null +++ b/lib/simpleshop_theme/sync/image_download_worker.ex @@ -0,0 +1,139 @@ +defmodule SimpleshopTheme.Sync.ImageDownloadWorker do + @moduledoc """ + Oban worker for downloading product images from external URLs. + + Downloads images from Printify CDN, processes through the Media pipeline + (WebP conversion, AVIF/WebP variant generation), and links to ProductImage. + + ## Usage + + # Enqueue a download for a product image + ImageDownloadWorker.enqueue(product_image_id) + + ## Job Args + + * `product_image_id` - The ID of the ProductImage to download + """ + + use Oban.Worker, queue: :images, max_attempts: 3 + + alias SimpleshopTheme.Products + alias SimpleshopTheme.Media + + require Logger + + @impl Oban.Worker + def perform(%Oban.Job{args: %{"product_image_id" => product_image_id}}) do + case Products.get_product_image(product_image_id) do + nil -> + {:cancel, :product_image_not_found} + + %{image_id: image_id} when not is_nil(image_id) -> + # Already has a linked image, skip + :ok + + product_image -> + download_and_link(product_image) + end + end + + @doc """ + Enqueue an image download for a product image. + """ + def enqueue(product_image_id) do + %{product_image_id: product_image_id} + |> new() + |> Oban.insert() + end + + defp download_and_link(product_image) do + case download_image(product_image.src) do + {:ok, data, content_type} -> + upload_and_link(product_image, data, content_type) + + {:error, reason} -> + Logger.warning( + "[ImageDownloadWorker] Failed to download #{product_image.src}: #{inspect(reason)}" + ) + + {:error, reason} + end + end + + defp download_image(url) do + case Req.get(url, receive_timeout: 30_000) do + {:ok, %Req.Response{status: 200, body: body, headers: headers}} -> + content_type = get_content_type(headers) + {:ok, body, content_type} + + {:ok, %Req.Response{status: status}} -> + {:error, {:http_error, status}} + + {:error, reason} -> + {:error, reason} + end + end + + defp get_content_type(headers) do + headers + |> Enum.find(fn {k, _v} -> String.downcase(k) == "content-type" end) + |> case do + {_, value} when is_binary(value) -> value |> String.split(";") |> hd() |> String.trim() + {_, [value | _]} -> value |> String.split(";") |> hd() |> String.trim() + _ -> "image/jpeg" + end + end + + defp upload_and_link(product_image, data, content_type) do + filename = extract_filename(product_image.src, content_type) + + attrs = %{ + image_type: "product", + filename: filename, + content_type: content_type, + file_size: byte_size(data), + data: data + } + + case Media.upload_image(attrs) do + {:ok, image} -> + case Products.link_product_image(product_image, image.id) do + {:ok, _} -> + Logger.info( + "[ImageDownloadWorker] Downloaded and linked image for #{product_image.id}" + ) + + :ok + + {:error, reason} -> + Logger.error("[ImageDownloadWorker] Failed to link image: #{inspect(reason)}") + {:error, reason} + end + + {:error, reason} -> + Logger.error("[ImageDownloadWorker] Failed to upload image: #{inspect(reason)}") + {:error, reason} + end + end + + defp extract_filename(url, content_type) do + # Extract filename from URL path, fall back to generated name + uri = URI.parse(url) + path_parts = String.split(uri.path || "", "/") + basename = List.last(path_parts) || "image" + + # Ensure it has an extension + if Path.extname(basename) == "" do + ext = extension_for_content_type(content_type) + "#{basename}#{ext}" + else + basename + end + end + + defp extension_for_content_type("image/jpeg"), do: ".jpg" + defp extension_for_content_type("image/png"), do: ".png" + defp extension_for_content_type("image/webp"), do: ".webp" + defp extension_for_content_type("image/gif"), do: ".gif" + defp extension_for_content_type(_), do: ".jpg" +end diff --git a/lib/simpleshop_theme/sync/product_sync_worker.ex b/lib/simpleshop_theme/sync/product_sync_worker.ex index ec91c1a..a132911 100644 --- a/lib/simpleshop_theme/sync/product_sync_worker.ex +++ b/lib/simpleshop_theme/sync/product_sync_worker.ex @@ -20,6 +20,7 @@ defmodule SimpleshopTheme.Sync.ProductSyncWorker do alias SimpleshopTheme.Products alias SimpleshopTheme.Products.ProviderConnection alias SimpleshopTheme.Providers.Provider + alias SimpleshopTheme.Sync.ImageDownloadWorker require Logger @@ -157,7 +158,13 @@ defmodule SimpleshopTheme.Sync.ProductSyncWorker do } end) - Products.sync_product_images(product, images) + image_results = Products.sync_product_images(product, images) + + # Enqueue downloads for images without image_id + Enum.each(image_results, fn + {:ok, %{image_id: nil, id: id}} -> ImageDownloadWorker.enqueue(id) + _ -> :ok + end) # Sync variants variants = diff --git a/lib/simpleshop_theme/theme/preview_data.ex b/lib/simpleshop_theme/theme/preview_data.ex index 8b87ff7..674149d 100644 --- a/lib/simpleshop_theme/theme/preview_data.ex +++ b/lib/simpleshop_theme/theme/preview_data.ex @@ -185,7 +185,11 @@ defmodule SimpleshopTheme.Theme.PreviewData do end defp get_real_products do - Products.list_products(visible: true, status: "active", preload: [:images, :variants]) + Products.list_products( + visible: true, + status: "active", + preload: [images: :image, variants: []] + ) |> Enum.map(&product_to_map/1) end @@ -228,16 +232,22 @@ defmodule SimpleshopTheme.Theme.PreviewData do in_stock = Enum.any?(available_variants) on_sale = Enum.any?(product.variants, &SimpleshopTheme.Products.ProductVariant.on_sale?/1) + # Use local image if available, fall back to CDN URL + {image_url, image_id, source_width} = image_attrs(first_image) + {hover_image_url, hover_image_id, hover_source_width} = image_attrs(second_image) + %{ id: product.slug, name: product.title, description: product.description, price: if(cheapest_variant, do: cheapest_variant.price, else: 0), compare_at_price: if(cheapest_variant, do: cheapest_variant.compare_at_price, else: nil), - image_url: if(first_image, do: first_image.src, else: nil), - hover_image_url: if(second_image, do: second_image.src, else: nil), - source_width: nil, - hover_source_width: nil, + image_url: image_url, + image_id: image_id, + hover_image_url: hover_image_url, + hover_image_id: hover_image_id, + source_width: source_width, + hover_source_width: hover_source_width, category: product.category, slug: product.slug, in_stock: in_stock, @@ -246,6 +256,20 @@ defmodule SimpleshopTheme.Theme.PreviewData do } end + # Extract image attributes, preferring local Media.Image when available + defp image_attrs(nil), do: {nil, nil, nil} + + defp image_attrs(%{image_id: image_id, image: %{source_width: source_width}}) + when not is_nil(image_id) do + # Local image available - use image_id for responsive element + {nil, image_id, source_width} + end + + defp image_attrs(%{src: src}) do + # Fall back to CDN URL + {src, nil, nil} + end + # Default source width for mockup variants (max generated size) @mockup_source_width 1200 diff --git a/lib/simpleshop_theme_web/components/page_templates/pdp.html.heex b/lib/simpleshop_theme_web/components/page_templates/pdp.html.heex index 24a2916..a13d006 100644 --- a/lib/simpleshop_theme_web/components/page_templates/pdp.html.heex +++ b/lib/simpleshop_theme_web/components/page_templates/pdp.html.heex @@ -19,16 +19,24 @@
<.breadcrumb - items={[ - %{label: "Home", page: "home", href: "/"}, - %{ - label: @product.category, - page: "collection", - href: - "/collections/#{@product.category |> String.downcase() |> String.replace(" ", "-")}" - }, - %{label: @product.name, current: true} - ]} + items={ + [ + %{label: "Home", page: "home", href: "/"} + ] ++ + if @product.category do + [ + %{ + label: @product.category, + page: "collection", + href: + "/collections/#{@product.category |> String.downcase() |> String.replace(" ", "-")}" + } + ] + else + [] + end ++ + [%{label: @product.name, current: true}] + } mode={@mode} /> diff --git a/lib/simpleshop_theme_web/components/shop_components.ex b/lib/simpleshop_theme_web/components/shop_components.ex index 30dbab8..fc28661 100644 --- a/lib/simpleshop_theme_web/components/shop_components.ex +++ b/lib/simpleshop_theme_web/components/shop_components.ex @@ -1404,7 +1404,8 @@ defmodule SimpleshopThemeWeb.ShopComponents do src = if image_id do - "/images/#{image_id}/variant" + # Trailing slash so build_srcset produces /images/{id}/variant/800.webp + "/images/#{image_id}/variant/" else image_url end @@ -4012,10 +4013,15 @@ defmodule SimpleshopThemeWeb.ShopComponents do available = Optimizer.applicable_widths(assigns.source_width) default_width = Enum.max(available) + # Database images end with / (e.g., /images/{id}/variant/) + # Mockups use - separator (e.g., /mockups/product-1) + separator = if String.ends_with?(assigns.src, "/"), do: "", else: "-" + assigns = assigns |> assign(:available_widths, available) |> assign(:default_width, default_width) + |> assign(:separator, separator) ~H""" @@ -4030,7 +4036,7 @@ defmodule SimpleshopThemeWeb.ShopComponents do sizes={@sizes} /> {@alt} Enum.sort() - |> Enum.map(&"#{base}-#{&1}.#{format} #{&1}w") + |> Enum.map(&"#{base}#{separator}#{&1}.#{format} #{&1}w") |> Enum.join(", ") end end diff --git a/lib/simpleshop_theme_web/live/shop_live/product_show.ex b/lib/simpleshop_theme_web/live/shop_live/product_show.ex index 890fe2c..25e746b 100644 --- a/lib/simpleshop_theme_web/live/shop_live/product_show.ex +++ b/lib/simpleshop_theme_web/live/shop_live/product_show.ex @@ -34,13 +34,11 @@ defmodule SimpleshopThemeWeb.ShopLive.ProductShow do |> Enum.reject(fn p -> p.id == product.id end) |> Enum.take(4) - # Build gallery images (filter out nils) + # Build gallery images from local image_id or external URL gallery_images = [ - product.image_url, - product.hover_image_url, - product.image_url, - product.hover_image_url + image_src(product[:image_id], product[:image_url]), + image_src(product[:hover_image_id], product[:hover_image_url]) ] |> Enum.reject(&is_nil/1) @@ -70,6 +68,14 @@ defmodule SimpleshopThemeWeb.ShopLive.ProductShow do List.first(products) end + # Build image source URL - prefer local image_id, fall back to external URL + defp image_src(image_id, _url) when is_binary(image_id) do + "/images/#{image_id}/variant/1200.webp" + end + + defp image_src(_, url) when is_binary(url), do: url + defp image_src(_, _), do: nil + @impl true def render(assigns) do ~H""" diff --git a/priv/repo/migrations/20260131232618_add_image_id_to_product_images.exs b/priv/repo/migrations/20260131232618_add_image_id_to_product_images.exs new file mode 100644 index 0000000..78b67a1 --- /dev/null +++ b/priv/repo/migrations/20260131232618_add_image_id_to_product_images.exs @@ -0,0 +1,11 @@ +defmodule SimpleshopTheme.Repo.Migrations.AddImageIdToProductImages do + use Ecto.Migration + + def change do + alter table(:product_images) do + add :image_id, references(:images, type: :binary_id, on_delete: :nilify_all) + end + + create index(:product_images, [:image_id]) + end +end