Umbra-Bookshelf/umbra-bookshelf/docs/KNOWN-ISSUES.md

# Known issues

Defects and carried-over problems. For intended work, see `TASKS.md`.

Status at 1.1.0: the three issues that blocked the consolidation are resolved.
What remains is listed first.

---

## Open

### 1. Rating sync blocks a worker with `sleep()`

`includes/books/class-umbra-rating-sync.php`

`sync_all_ratings()` iterates up to 999 books and calls `sleep(2)` between each,
inside a cron run — roughly 33 minutes of a blocked PHP worker per tick.

Pre-existing. Deliberately left alone when Amazon scraping was removed, because
that change was scoped to dropping a source. Fix suggestion is in `TASKS.md` §3.

### 2. The two modules still model books differently

Making the catalogue schema canonical resolved the *table* conflict, but the
catalogue (row in `bd_books`) and the library (`book` custom post type with
per-post meta) are still independent models with no synchronisation between
them. Editing a book in one place does not update the other.

See `TASKS.md` §5 for the decision to make.

### 3. Set-aside legacy tables are never cleaned up

On a reconciled installation, `bd_books_legacy_library` and
`bd_series_legacy_library` remain indefinitely. Inert, but untidy. See
`TASKS.md` §4.

---

## Resolved in 1.1.0

### Catalogue schema is now canonical for `bd_books` and `bd_series`

**Was:** the catalogue and library modules both created `bd_books` and
`bd_series`, with different columns. Whichever activated first won; the other
either no-opped or errored, then read and wrote against the wrong shape. The
divergence was:

```
catalogue  wp_bd_books   id, title, slug, description, cover_image, isbn_*,
                         asin_*, amazon_asin, goodreads_id, rating_*, series_id
library    wp_bd_books   id, post_id, pen_name_id, series_id, volume_number,
                         asin, isbn, publish_date, sort_order
```

**Now:** `includes/class-umbra-schema-migration.php` runs before `dbDelta` on
activation and on update. If it finds a library-shaped table it renames it
aside (**never drops it**), creates the canonical table, and copies rows across:

* book **IDs are preserved**, which keeps `bd_book_meta`, `bd_reviews`,
  `bd_ratings` and `bd_review_shortlinks` valid
* `title`, `slug` and `description` are resolved from the linked post
* the library's single `isbn`/`asin` land on the ebook columns; `amazon_asin` is
  also populated so existing product links keep working
* `series_id` is preserved; the series row is migrated first so it still resolves
* post linkage is written back as `_bd_bookshelf_book_id` (preserved meta key)
* `publish_date` has no catalogue column, so it is kept as
  `_bd_bookshelf_publish_date` post meta
* goodreads/amazon values in `bd_ratings` are folded into the catalogue's
  `rating_goodreads` / `rating_amazon` columns

Rows whose post no longer exists are skipped with a log entry rather than
creating an unusable book. Duplicate slugs are de-duplicated, because the
catalogue's `slug` column is `UNIQUE`.

Verified by `tests/schema_migration_test.php` — 42 checks against a real
database, covering shape detection, row mapping, ID preservation, orphan
handling, rating folding, idempotency and slug collisions.

### Only Goodreads scraping remains

**Was:** three Goodreads implementations and two Amazon ones:

| Implementation | State before |
|---|---|
| `Umbra_Goodreads_Scraper` | complete, 325 lines, **never called** |
| `Umbra_Rating_Sync::fetch_goodreads_rating()` | working duplicate |
| library `refresh_goodreads_data()` | empty stub, but scheduled daily |
| `Umbra_Amazon_Scraper` | complete, 296 lines, **never called** |
| `Umbra_Rating_Sync::fetch_amazon_rating()` | working duplicate |

Both library "scrapers" were empty stubs attached to cron events, so
`bd_bookshelf_goodreads_refresh` (daily) and `bd_bookshelf_amazon_scrape`
(six-hourly) fired on schedule to do nothing.

**Now:** Goodreads only, and one implementation of it.
`Umbra_Amazon_Scraper` is deleted; the duplicate HTML parsing is gone from
`Umbra_Rating_Sync`, which now delegates to `Umbra_Goodreads_Scraper`. The two
library stubs and their cron registrations are removed. The surviving schedule
is the single `bd_bookshelf_sync_ratings` event every 12 hours.

The `rating_amazon` **column** is retained — it holds existing data and is
populated from `bd_ratings` during migration. Amazon retail *links* are
unaffected; only rating scraping was removed. The two former hooks are still
cleared on deactivation and uninstall so no stale schedules survive an upgrade.

### Licence is GPLv2 or later

**Was:** plugin headers said `GPLv2 or later` while `LICENSE`, `NOTICE` and 101
SPDX headers said Apache-2.0. The theme was worse: `style.css` declared GPLv2
while its PHP files carried Apache headers.

**Now:** GPL-2.0-or-later throughout, which is what WordPress.org requires and
what the theme already claimed. `LICENSE` is the full GPLv2 text; `NOTICE` is
removed (it was Apache-specific); every SPDX header reads
`GPL-2.0-or-later`.

`Old/` and `archive/pre-umbra-releases/` keep their original Apache headers on
purpose — they are frozen pre-consolidation snapshots, not shipped code.

### Default cover was missing

`Umbra_Bookshelf_Helper` returned `assets/images/default-cover.png` as the
fallback cover, but the repository contained no images at all, so every
cover-less book rendered a broken image. A real cover
(`WorkingTitleCover_512x815.png`, 512×815) is now committed at that path.
Replace with final artwork before release if this was only a working title.

### Email admin script did not exist

`Umbra_Simple_Email_Admin::enqueue_assets()` enqueued `email/assets/js/admin.js`
on every email admin page, but the file was never committed — a guaranteed 404.
An empty placeholder is now committed with a comment explaining its history.
Defining its behaviour is `TASKS.md` §1.

### `Umbra_SEO` was loaded but never instantiated

It registers its hooks from a constructor, and the bootstrap did not construct
it, so the class was dead code. It is now instantiated and its meta/Open
Graph/Twitter/sitemap hooks attach. Completing SEO for generated book detail
pages is `TASKS.md` §2.

---

## Fixed in 1.0.0 (the consolidation)

* **`Cannot declare class` fatal.** `class-bdm-theme-compatibility.php` declared
  `BD_BookShelfHelper`, the same class name as the library plugin's own helper.
  Latent while the two were separate plugins — it only fired when both were
  active — but an unconditional fatal once merged. The shim is now
  `Umbra_Theme_Compatibility`, with `class_alias()` preserving the legacy name
  so the unmodified theme keeps working.
* **Duplicate `bd_bookshelf_import_csv` AJAX handler.** `Umbra_Admin` and
  `Umbra_Migration` both registered it; the second silently replaced the first.
  The import page now lives solely in `Umbra_Admin` (the variant wired to
  `admin/js/umbra-bookshelf-admin.js`); `Umbra_Migration` keeps only the
  preview/run endpoints it uniquely provides.
* **Duplicate `bd-bookshelf-settings` submenu.** Catalogue and library both
  claimed the slug, leaving one Settings screen unreachable. Now
  `umbra-bookshelf-settings` and `umbra-bookshelf-library-settings`.
* **Duplicate admin menu.** All three modules called `add_menu_page`. Now one
  top-level menu, registered by the bootstrap at `admin_menu` priority 9.
* **Colliding asset handles.** Catalogue and library both enqueued
  `bd-bookshelf-admin`; the second enqueue was silently discarded. Handles are
  now unique per module.
* **Modules could miss their own `init` callbacks.** Constructing modules from
  inside an `init` callback means their `init` registrations race the hook being
  dispatched, so the `book` post type and `[bd-book]` shortcode could fail to
  register. Modules now construct on `plugins_loaded`, behind a re-entrancy
  guard.
* **Over-broad asset hook check.** The library module's
  `strpos($hook, 'bd-bookshelf')` matched every catalogue and email screen once
  all slugs shared the `umbra-bookshelf-` prefix. Now matched against its own
  slug list.