From 9a5a73ff02d19aa67e54633bb19d3668ba10ea38 Mon Sep 17 00:00:00 2001
From: Adam Kunicki <kunickiaj@gmail.com>
Date: Sat, 21 Jun 2025 09:38:45 -0700
Subject: [PATCH] Fix addTopic migration in notifications-backend plugin

The migration code fails when user_settings is non-empty
because it adds a non-nullable constraint before DML is applied.

This fixes that issue and adds a test case for that particular migration.

Signed-off-by: Adam Kunicki <kunickiaj@gmail.com>
---
 .changeset/red-trams-join.md                  |  5 +
 .../migrations/20250317_addTopic.js           | 12 +--
 .../src/tests/migrations.test.ts              | 99 +++++++++++++++++++
 3 files changed, 110 insertions(+), 6 deletions(-)
 create mode 100644 .changeset/red-trams-join.md
 create mode 100644 plugins/notifications-backend/src/tests/migrations.test.ts

diff --git a/.changeset/red-trams-join.md b/.changeset/red-trams-join.md
new file mode 100644
index 0000000000..3ee245e2a8
--- /dev/null
+++ b/.changeset/red-trams-join.md
@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-notifications-backend': patch
+---
+
+Fix `addTopic` migration when `user_settings` present
diff --git a/plugins/notifications-backend/migrations/20250317_addTopic.js b/plugins/notifications-backend/migrations/20250317_addTopic.js
index c8f78a5640..e17b8d0789 100644
--- a/plugins/notifications-backend/migrations/20250317_addTopic.js
+++ b/plugins/notifications-backend/migrations/20250317_addTopic.js
@@ -18,14 +18,10 @@ const crypto = require('crypto');
 exports.up = async function up(knex) {
   await knex.schema.alterTable('user_settings', table => {
     table.string('topic').nullable().after('origin');
-    table.string('settings_key_hash', 64).notNullable();
+    table.string('settings_key_hash', 64).nullable();
     table.dropUnique([], 'user_settings_unique_idx');
   });
 
-  await knex.schema.alterTable('user_settings', table => {
-    table.unique(['settings_key_hash'], 'user_settings_unique_idx');
-  });
-
   const rows = await knex('user_settings').select('user', 'channel', 'origin');
   for (const row of rows) {
     const rawKey = `${row.user}|${row.channel}|${row.origin}|}`;
@@ -35,10 +31,14 @@ exports.up = async function up(knex) {
         user: row.user,
         channel: row.channel,
         origin: row.origin,
-        topic: row.topic,
       })
       .update({ settings_key_hash: hash });
   }
+
+  await knex.schema.alterTable('user_settings', table => {
+    table.string('settings_key_hash', 64).notNullable().alter();
+    table.unique(['settings_key_hash'], 'user_settings_unique_idx');
+  });
 };
 
 exports.down = async function down(knex) {
diff --git a/plugins/notifications-backend/src/tests/migrations.test.ts b/plugins/notifications-backend/src/tests/migrations.test.ts
new file mode 100644
index 0000000000..aa3ba7ae9a
--- /dev/null
+++ b/plugins/notifications-backend/src/tests/migrations.test.ts
@@ -0,0 +1,99 @@
+/*
+ * Copyright 2025 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { Knex } from 'knex';
+import { TestDatabases } from '@backstage/backend-test-utils';
+import fs from 'fs';
+
+const migrationsDir = `${__dirname}/../../migrations`;
+const migrationsFiles = fs.readdirSync(migrationsDir).sort();
+
+async function migrateUpOnce(knex: Knex): Promise<void> {
+  await knex.migrate.up({ directory: migrationsDir });
+}
+
+async function migrateDownOnce(knex: Knex): Promise<void> {
+  await knex.migrate.down({ directory: migrationsDir });
+}
+
+async function migrateUntilBefore(knex: Knex, target: string): Promise<void> {
+  const index = migrationsFiles.indexOf(target);
+  if (index === -1) {
+    throw new Error(`Migration ${target} not found`);
+  }
+  for (let i = 0; i < index; i++) {
+    await migrateUpOnce(knex);
+  }
+}
+
+jest.setTimeout(60_000);
+
+describe('migrations', () => {
+  const databases = TestDatabases.create();
+
+  it.each(databases.eachSupportedId())(
+    '20221109192547_search_add_original_value_column.js, %p',
+    async databaseId => {
+      const knex = await databases.init(databaseId);
+
+      await migrateUntilBefore(knex, '20250317_addTopic.js');
+
+      await knex
+        .insert({
+          user: 'user1',
+          channel: 'channel1',
+          origin: 'origin1',
+          enabled: true,
+        })
+        .into('user_settings');
+
+      await migrateUpOnce(knex);
+
+      let rows = await knex('user_settings');
+      let normalized = rows.map(r => ({ ...r, enabled: !!r.enabled }));
+
+      expect(normalized).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            user: 'user1',
+            channel: 'channel1',
+            origin: 'origin1',
+            enabled: true,
+            settings_key_hash:
+              '73f97aff883b8b08a7f4e366234ef4f86827702b0016574ac4c1bf313c703d15',
+            topic: null,
+          }),
+        ]),
+      );
+
+      await migrateDownOnce(knex);
+
+      rows = await knex('user_settings');
+      normalized = rows.map(r => ({ ...r, enabled: !!r.enabled }));
+
+      expect(normalized).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            user: 'user1',
+            channel: 'channel1',
+            origin: 'origin1',
+            enabled: true,
+          }),
+        ]),
+      );
+    },
+  );
+});