From ef1c875f6ec1e02657006cb09cd0b1d868ec2fc0 Mon Sep 17 00:00:00 2001
From: Paul Nothaft <paul@MacStudio-von-Paul.local>
Date: Sat, 2 May 2026 00:04:06 +0200
Subject: [PATCH] fix(events): stop mapping branding_logo_position onto
 hero_logo_position
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two settings with overlapping names but different value sets were being
conflated:

- branding_logo_position (header bar, horizontal): 'left'|'center'|'right'
- hero_logo_position    (hero block, vertical):   'top'|'center'|'bottom'

getBrandingDefaults() copied the global branding value over the per-event
hero value when seeding new events. Any admin with branding logo set to
'left' (the most common choice) created events with hero_logo_position
= 'left' written to the DB. Subsequent PUTs to /admin/events/:id then
failed validation with "Invalid value (field: hero_logo_position)" — the
validator only accepts top/center/bottom.

Fix:

1. Drop the bogus mapping. branding_logo_position is no longer read by
   getBrandingDefaults — it doesn't belong there. The fallback default
   ('top') is used unless the request body explicitly provides
   hero_logo_position, which is independently validated.

2. Migration 084_fix_hero_logo_position normalises any existing rows
   whose hero_logo_position is outside ('top','center','bottom') back
   to 'top'. Without this, affected events would continue to 400 on
   every save until the admin manually picks a valid option.

Reproduction: admin sets branding logo position to 'left' under global
branding, creates an event, opens the event detail page, clicks Save
without changing anything → 400. After this fix, save succeeds and new
events default to 'top' regardless of branding-bar position.
---
 .../core/084_fix_hero_logo_position.js        | 21 +++++++++++++++++++
 backend/src/routes/adminEvents.js             | 15 +++++++------
 2 files changed, 30 insertions(+), 6 deletions(-)
 create mode 100644 backend/migrations/core/084_fix_hero_logo_position.js

diff --git a/backend/migrations/core/084_fix_hero_logo_position.js b/backend/migrations/core/084_fix_hero_logo_position.js
new file mode 100644
index 00000000..d687ee64
--- /dev/null
+++ b/backend/migrations/core/084_fix_hero_logo_position.js
@@ -0,0 +1,21 @@
+/**
+ * Heal events whose hero_logo_position contains a branding-style value
+ * (left/right) caused by a prior bug in adminEvents.js getBrandingDefaults
+ * that mapped branding_logo_position (left/center/right) onto
+ * hero_logo_position (top/center/bottom). Any non-canonical value is
+ * reset to 'top' so subsequent PUTs no longer fail validation.
+ */
+
+exports.up = async function up(knex) {
+  const hasColumn = await knex.schema.hasColumn('events', 'hero_logo_position');
+  if (!hasColumn) return;
+
+  await knex('events')
+    .whereNotIn('hero_logo_position', ['top', 'center', 'bottom'])
+    .update({ hero_logo_position: 'top' });
+};
+
+exports.down = async function down() {
+  // Data correction is not reversible — the original (incorrect) values
+  // are not preserved.
+};
diff --git a/backend/src/routes/adminEvents.js b/backend/src/routes/adminEvents.js
index 7ea50898..a5f0effd 100644
--- a/backend/src/routes/adminEvents.js
+++ b/backend/src/routes/adminEvents.js
@@ -138,14 +138,20 @@ const getDownloadProtectionDefaults = async () => {
   return { enable_devtools_protection: await readBooleanSetting('enable_devtools_protection') };
 };
 
-// Helper to get branding defaults for new events (Feature 7: Branding Inheritance)
+// Helper to get branding defaults for new events (Feature 7: Branding Inheritance).
+//
+// Note: `branding_logo_position` (header bar — left/center/right) is a
+// different concept from `hero_logo_position` (hero block — top/center/
+// bottom) and must NOT be mapped here. A previous version copied the
+// branding value over, which wrote 'left'/'right' into per-event
+// hero_logo_position columns and broke any subsequent PUT validation
+// (#357). Migration 084 heals existing rows.
 const getBrandingDefaults = async () => {
   try {
     const settings = await db('app_settings')
       .whereIn('setting_key', [
         'branding_logo_display_hero',
-        'branding_logo_size',
-        'branding_logo_position'
+        'branding_logo_size'
       ])
       .select('setting_key', 'setting_value');
 
@@ -166,9 +172,6 @@ const getBrandingDefaults = async () => {
       if (s.setting_key === 'branding_logo_size' && value) {
         defaults.hero_logo_size = value;
       }
-      if (s.setting_key === 'branding_logo_position' && value) {
-        defaults.hero_logo_position = value;
-      }
     });
 
     return defaults;