fix: imports actually import books #30

Merged

jfro merged 3 commits from jtk/import-books into main 2026-10-18 17:07:29 +00:00

Conversation 0 Commits 3 Files changed 8 +732 -54

jfro commented 2026-10-18 16:27:33 +00:00

Owner

Copy link

Fixes #13: the import previewed the file, "started" and reported success, but perform_import was a stub that only marked the job complete.

What changed

• FuzzyCatalog.ImportExport.Importer reads the JSON and CSV files Exporter writes (CSV via NimbleCSV, so quoted commas, quotes and newlines survive).
  • Each row's book is matched by ISBN-13, ISBN-10, UPC or ASIN, then title + author (case-insensitive). Otherwise it's created.
  • Then it's added to the collection with the row's media type, unless it's already there. Unknown media types become unspecified, and added_at is kept.
  • Existing books are never changed. Only whitelisted book fields are taken from the file, never id or cover_image_key.
  • Invalid rows are skipped and counted. The job shows "Import Result" (books added / matched / already in collection / rows skipped), and the first 5 skipped rows with reasons go in its error field.
  • Progress is updated every 25 rows. Logger lines record start, finish, duration and counts, plus a warning for each skipped row.
• Uploads get cleaned up. The temp upload is deleted when the import finishes (whether it succeeded or failed), and also when a pending import job is deleted. Before this, delete joined the absolute temp path onto the storage dir, so uploads were never removed. Upload names now get a unique suffix.
• Supervised tasks. Imports and exports run under a new FuzzyCatalog.TaskSupervisor instead of bare spawn/1.
• CSV preview uses the same parser as the import, so it shows the right title, author and media type.
• Upload crash fixed. Uploading an import file crashed on main (Ecto.CastError, mixed atom and string keys). File info now uses string keys.

New dependency: nimble_csv (Dashbit; Req already lists it as optional).

Tests

test/fuzzy_catalog/import_export/importer_test.exs covers JSON and CSV imports, matching without changing existing books, running the same import twice, ignored id/cover_image_key, invalid rows, malformed JSON, missing CSV headers, upload removal, and JSON and CSV export → import round trips. import_export_controller_test.exs covers upload → preview → confirm → completed job, and deleting a pending import.

mix precommit: 191 tests, 0 failures.

Merge notes

• application.ex also changes on #17 (ebook). Both add children to the list, so expect a small conflict in whichever merges second.
• The import/export routes are still only behind require_authenticated_user on main. require_admin comes with #17 (#8).

Fixes #13: the import previewed the file, "started" and reported success, but `perform_import` was a stub that only marked the job complete. ## What changed - **`FuzzyCatalog.ImportExport.Importer`** reads the JSON and CSV files `Exporter` writes (CSV via NimbleCSV, so quoted commas, quotes and newlines survive). - Each row's book is matched by ISBN-13, ISBN-10, UPC or ASIN, then title + author (case-insensitive). Otherwise it's created. - Then it's added to the collection with the row's media type, unless it's already there. Unknown media types become `unspecified`, and `added_at` is kept. - Existing books are never changed. Only whitelisted book fields are taken from the file, never `id` or `cover_image_key`. - Invalid rows are skipped and counted. The job shows "Import Result" (books added / matched / already in collection / rows skipped), and the first 5 skipped rows with reasons go in its error field. - Progress is updated every 25 rows. Logger lines record start, finish, duration and counts, plus a warning for each skipped row. - **Uploads get cleaned up.** The temp upload is deleted when the import finishes (whether it succeeded or failed), and also when a pending import job is deleted. Before this, delete joined the absolute temp path onto the storage dir, so uploads were never removed. Upload names now get a unique suffix. - **Supervised tasks.** Imports and exports run under a new `FuzzyCatalog.TaskSupervisor` instead of bare `spawn/1`. - **CSV preview** uses the same parser as the import, so it shows the right title, author and media type. - **Upload crash fixed.** Uploading an import file crashed on main (`Ecto.CastError`, mixed atom and string keys). File info now uses string keys. New dependency: `nimble_csv` (Dashbit; Req already lists it as optional). ## Tests `test/fuzzy_catalog/import_export/importer_test.exs` covers JSON and CSV imports, matching without changing existing books, running the same import twice, ignored `id`/`cover_image_key`, invalid rows, malformed JSON, missing CSV headers, upload removal, and JSON and CSV export → import round trips. `import_export_controller_test.exs` covers upload → preview → confirm → completed job, and deleting a pending import. `mix precommit`: 191 tests, 0 failures. ## Merge notes - `application.ex` also changes on #17 (ebook). Both add children to the list, so expect a small conflict in whichever merges second. - The import/export routes are still only behind `require_authenticated_user` on main. `require_admin` comes with #17 (#8).

jfro added 1 commit 2026-10-18 16:27:33 +00:00

fix: imports actually import books ... af396b311e

perform_import was a stub that marked the job complete. The new Importer
reads the JSON and CSV files the exporter writes (CSV through NimbleCSV,
so quoted commas and newlines survive), matches books by ISBN, UPC, ASIN
or title and author, creates the rest and adds collection items, and
records a summary and skipped rows on the job. Uploads are deleted when
the import finishes or the job is deleted. Imports and exports run under
a Task.Supervisor instead of bare spawn.

Uploading an import file also crashed before this: the upload info had
atom keys merged into string-keyed params.

Fixes #13

jfro reviewed 2026-10-18 17:02:04 +00:00

jfro left a comment

Copy link

Review by Nye (PR review / audit) — APPROVE @af396b3

CI green. Checked: importer only takes @book_fields through Book.changeset (no id, cover_image_key or cover_url, so no cover download/SSRF path), per-row transactions, never updates existing books, Map.merge(job_params, file_info) puts the server's file_path last so the form can't override it, the CastError fix (string keys), Task.Supervisor, upload cleanup on finish/fail/delete, and good Logger context.

Merge note: conflicts with #17 only in lib/fuzzy_catalog/application.ex: keep #17's children list and add {Task.Supervisor, name: FuzzyCatalog.TaskSupervisor} before the Endpoint.

**Review by Nye (PR review / audit) — APPROVE** @af396b3 CI green. Checked: importer only takes `@book_fields` through `Book.changeset` (no id, cover_image_key or cover_url, so no cover download/SSRF path), per-row transactions, never updates existing books, `Map.merge(job_params, file_info)` puts the server's file_path last so the form can't override it, the CastError fix (string keys), Task.Supervisor, upload cleanup on finish/fail/delete, and good Logger context. Merge note: conflicts with #17 only in `lib/fuzzy_catalog/application.ex`: keep #17's children list and add `{Task.Supervisor, name: FuzzyCatalog.TaskSupervisor}` before the Endpoint.

jfro added 2 commits 2026-10-18 17:05:09 +00:00

Merge remote-tracking branch 'origin/main' into jtk/import-books ... fb5e7bd547

# Conflicts:
#	lib/fuzzy_catalog/application.ex

test: log in as an admin for the import/export controller tests 7aaca84272

jfro merged commit 92145d4982 into main 2026-10-18 17:07:29 +00:00

jfro deleted branch jtk/import-books 2026-10-18 17:07:29 +00:00

jfro referenced this pull request from a commit 2026-10-18 17:07:30 +00:00

fix: imports actually import books (#30)

jfro referenced this pull request 2026-10-18 18:15:22 +00:00

Import upload from the admin page always fails with 400 #45

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

  

critical

  

ebook-branch

  

high

  

low

  

main

  

medium

  

release-blocker

  

security

No labels

bug

critical

ebook-branch

high

low

main

medium

release-blocker

security

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

jfro/fuzzy_catalog!30

No description provided.