Compare commits

...
2 Commits
Author SHA1 Message Date
twooey ed70207453 Fix two more bugs found in a third code-review pass
- SupplierOffer::delete_for_isbn() was the one Catalog method still
  missing a $wpdb failure check. There's no unique constraint on isbn13
  alone (schema only has a composite key on isbn13+status+base_price), so
  SyntheticOfferGenerator's delete-then-insert pattern relied entirely on
  the delete actually succeeding — a silent failure there left the stale
  row in place alongside the new insert, two rows for one ISBN, with
  best_offer_for_isbns() picking whichever was cheaper by chance and
  generate-offers reporting full success throughout. Verified with the
  same forced-failure reproduction as the review that found it (a BEFORE
  DELETE trigger): before this fix, that scenario left 2 rows for the
  ISBN; after, it throws immediately (caught by generate_all()'s existing
  per-ISBN handler) and the row count stays at 1.

- deploy/lib/env.sh's env_get() didn't strip a trailing \r or leading/
  trailing whitespace from extracted values. Consequential specifically
  because of restore.sh's DB drop/recreate (added earlier this session):
  DB_NAME_VALUE feeds directly into DROP DATABASE IF EXISTS/CREATE DATABASE
  for that name — a CRLF-saved or hand-edited-with-trailing-space .env file
  would silently target a different database name than the real one, with
  no error, defeating the "clean restore" guarantee that fix exists for.
  Also mattered for quote-stripping: a trailing \r landing after a closing
  quote made the end-with-quote check silently fail to match, leaking
  literal quote characters into the value. Verified against CRLF-terminated,
  trailing-whitespace, and quoted+CRLF combinations, including a byte-level
  check that no \r survives; confirmed no regression against normal
  LF-terminated unquoted/quoted values via a full deploy.sh dev run.
2026-08-27 17:03:00 -04:00
twooey b2fd0330a8 Fix book covers being cropped to squares in the shop grid
Reported: cover images looked cropped top/bottom. Confirmed directly —
WooCommerce's woocommerce_thumbnail size defaults to a hard 1:1 crop
(unset woocommerce_thumbnail_cropping option defaults to '1:1' in
WC core), so a 335x500 portrait cover was being force-cropped to 300x300
for the shop grid, cutting off the top and bottom of every cover.

woocommerce_thumbnail_cropping is a real, admin-configurable WooCommerce
setting (WooCommerce > Settings > Products > Display in wp-admin) — not a
WooCommerce code change. Set to 'uncropped', which makes the thumbnail
proportional instead (verified: same cover now generates a 300x448
woocommerce_thumbnail, matching its actual aspect ratio). wc_get_image_size()
caches the computed size in the object cache, so also flush the cache
right after — confirmed the option update alone did NOT take effect until
flushed (Redis was serving the stale pre-change size definition).

Added to deploy.sh alongside the other WooCommerce option settings it
already manages (HPOS, blog_public) so every environment gets this by
default rather than needing the same manual fix on each one. Verified via
a real dev deploy run (idempotent, no errors) and by regenerating the
already-synced covers' thumbnails and confirming their new dimensions.
2026-08-27 17:00:39 -04:00
3 changed files with 39 additions and 1 deletions

No files matched your search

+10
View File
@@ -112,6 +112,16 @@ echo "==> enabling WooCommerce HPOS (custom order tables)"
$WP option update woocommerce_custom_orders_table_enabled yes
$WP option update woocommerce_custom_orders_table_data_sync_enabled yes
# WooCommerce's default (unset -> '1:1') force-crops every product
# thumbnail to a square — confirmed directly: a 335x500 portrait book cover
# was landing at a hard-cropped 300x300 shop-grid thumbnail, cutting off
# the top and bottom of the cover. 'uncropped' makes it a proportional
# 300x448 instead. wc_get_image_size() caches the computed size in the
# object cache, so the option change alone doesn't take effect until flushed.
echo "==> uncropped product thumbnails (book covers are portrait, not square)"
$WP option update woocommerce_thumbnail_cropping uncropped
$WP cache flush
if [[ "$ENVIRONMENT" != "production" ]]; then
echo "==> discouraging search engines (non-production)"
$WP option update blog_public 0
+13
View File
@@ -9,6 +9,19 @@ env_get() {
local file="$1" key="$2" line val
line="$(grep -m1 -E "^${key}=" "$file" 2>/dev/null || true)"
val="${line#*=}"
# Strip a trailing \r (a CRLF-saved file — edited on Windows, or via some
# SFTP/GUI tools) and any leading/trailing whitespace BEFORE the
# quote-detection below. Order matters: a CRLF-terminated quoted value has
# its \r land after the closing quote, so the end-with-quote check below
# would silently fail to match and the literal quote characters would
# leak into the returned value instead of being stripped. Verified
# directly: left unstripped, a corrupted DB_NAME value fed into restore.sh's
# `DROP DATABASE IF EXISTS \`$DB_NAME_VALUE\`` targets a DIFFERENT
# database name than the real one — no error, just a "restore" that
# silently doesn't clean the actual target database first.
val="${val%$'\r'}"
val="${val#"${val%%[![:space:]]*}"}"
val="${val%"${val##*[![:space:]]}"}"
if [[ "$val" == \"*\" && "$val" == *\" ]]; then
val="${val#\"}"; val="${val%\"}"
elif [[ "$val" == \'*\' && "$val" == *\' ]]; then
@@ -74,6 +74,21 @@ class SupplierOffer
public static function delete_for_isbn(string $isbn13): void
{
global $wpdb;
$wpdb->delete(self::table(), ['isbn13' => $isbn13]);
$result = $wpdb->delete(self::table(), ['isbn13' => $isbn13]);
if ($result === false) {
// The one Catalog method still missing this check, and not a
// hypothetical gap: there's no unique constraint on isbn13
// alone (schema only has a composite KEY on isbn13+status+
// base_price), so SyntheticOfferGenerator's delete-then-insert
// pattern relies entirely on the delete actually succeeding.
// Verified directly (a forced delete failure via a BEFORE
// DELETE trigger): a silent no-op here left the stale row in
// place alongside the new insert — two rows for one ISBN, with
// best_offer_for_isbns() picking whichever was cheaper by
// chance, and generate-offers reporting full success the whole
// time. Throwing here routes it into generate_all()'s existing
// per-ISBN catch instead of a silent duplicate.
throw new \RuntimeException('bsc_supplier_offer delete failed: ' . $wpdb->last_error . ' (isbn13: ' . $isbn13 . ')');
}
}
}