Fix FunWithFlags boolean gate migration

guides/migration_guide.md

@@ -158,6 +158,21 @@ config :bandera,
   ]
 ```
 
+> **⚠ Boolean gate compatibility note.** FunWithFlags stored boolean gates with
+> a different internal sentinel than Bandera uses. If any flags had their boolean
+> state set while FunWithFlags was active, your table may have duplicate boolean
+> gate rows that cause the toggle to appear stuck. Run the one-time cleanup
+> migration after switching:
+>
+> ```
+> mix bandera.gen.fix_fun_with_flags_migration
+> mix ecto.migrate
+> ```
+>
+> This generates and runs a migration that normalises any legacy rows. It is safe
+> to run on a table that has already been fully migrated — it finds nothing to
+> change.
+
 **Option B: create a fresh Bandera table** and copy your rows over. Generate
 the table from a migration:
 

lib/bandera/ecto/migrations.ex

@@ -65,6 +65,55 @@ if Code.ensure_loaded?(Ecto.Migration) do
       :ok
     end
 
+    @doc """
+    Fixes boolean gate rows left by a FunWithFlags-to-Bandera migration.
+
+    FunWithFlags stored boolean gates with a legacy `target` value; Bandera uses
+    `"_bandera_none"`. Because the Ecto adapter's upsert conflict target is
+    `(flag_name, gate_type, target)`, toggling a flag via Bandera after migration
+    inserts a second boolean row rather than updating the legacy one. This leaves
+    two rows with contradictory `enabled` values, causing the dashboard toggle and
+    summary to disagree and making the toggle appear broken.
+
+    Run once from a migration after switching to Bandera:
+
+        defmodule MyApp.Repo.Migrations.FixFunWithFlagsBooleanGates do
+          use Ecto.Migration
+          def up, do: Bandera.Ecto.Migrations.fix_fun_with_flags_boolean_gates()
+          def down, do: :ok
+        end
+
+    Safe to run on a database that has already been fully migrated — it will find
+    nothing to change. Not reversible (`down` should be a no-op).
+    """
+    @spec fix_fun_with_flags_boolean_gates() :: :ok
+    def fix_fun_with_flags_boolean_gates do
+      table = Bandera.Config.ecto_table_name()
+      # `table` is developer-controlled config; '_bandera_none' is a fixed sentinel.
+
+      # When both a legacy row and a Bandera row exist for the same flag, delete
+      # the legacy row. The Bandera row reflects the most recent intent.
+      execute("""
+      DELETE FROM #{table}
+      WHERE gate_type = 'boolean'
+        AND target != '_bandera_none'
+        AND flag_name IN (
+          SELECT flag_name FROM #{table}
+          WHERE gate_type = 'boolean' AND target = '_bandera_none'
+        )
+      """)
+
+      # Rename any remaining legacy rows so future Bandera writes upsert correctly.
+      execute("""
+      UPDATE #{table}
+      SET target = '_bandera_none'
+      WHERE gate_type = 'boolean'
+        AND target != '_bandera_none'
+      """)
+
+      :ok
+    end
+
     @doc "Drops the flags table. Call from the `down/0` of your own migration."
     @spec down() :: :ok
     def down do

lib/bandera/store/persistent/ecto.ex

@@ -73,6 +73,26 @@ if Code.ensure_loaded?(Ecto.Adapters.SQL) do
       end
     end
 
+    @impl Bandera.Store.Persistent
+    def put(flag_name, %Gate{type: :boolean} = gate) do
+      # Delete any existing boolean row first — regardless of the `target` value
+      # stored there. Without this, migrating from FunWithFlags (which used
+      # `target = "boolean"`) leaves a stale row alongside Bandera's
+      # `target = "_bandera_none"` row, because the upsert conflict target is
+      # `[:flag_name, :gate_type, :target]` and the two targets differ.
+      name = to_string(flag_name)
+      row = Serializer.to_row(flag_name, gate)
+
+      repo().delete_all(
+        from(r in {table(), Record},
+          where: r.flag_name == ^name and r.gate_type == "boolean"
+        )
+      )
+
+      repo().insert_all({table(), Record}, [row])
+      get(flag_name)
+    end
+
     @impl Bandera.Store.Persistent
     def put(flag_name, %Gate{} = gate) do
       row = Serializer.to_row(flag_name, gate)

lib/mix/tasks/bandera.gen.fix_fun_with_flags_migration.ex

@@ -0,0 +1,63 @@
+defmodule Mix.Tasks.Bandera.Gen.FixFunWithFlagsMigration do
+  @moduledoc """
+  Generates an Ecto migration that fixes duplicate boolean gate rows left by a
+  FunWithFlags-to-Bandera migration.
+
+  ## Usage
+
+      mix bandera.gen.fix_fun_with_flags_migration [--path PATH]
+
+  ## Options
+
+    * `--path` — directory to write the migration file into.
+      Defaults to `priv/repo/migrations`.
+
+  ## What it generates
+
+      defmodule MyApp.Repo.Migrations.FixFunWithFlagsBooleanGates do
+        use Ecto.Migration
+        def up, do: Bandera.Ecto.Migrations.fix_fun_with_flags_boolean_gates()
+        def down, do: :ok
+      end
+
+  Run `mix ecto.migrate` after generating the file.
+  """
+  @shortdoc "Generate a migration to fix FunWithFlags boolean gate rows"
+  use Mix.Task
+
+  @default_path "priv/repo/migrations"
+
+  @impl Mix.Task
+  def run(args) do
+    {opts, _, _} = OptionParser.parse(args, strict: [path: :string])
+    path = Keyword.get(opts, :path, @default_path)
+    abs_path = Path.expand(path)
+
+    File.mkdir_p!(abs_path)
+
+    filename = "#{timestamp()}_fix_fun_with_flags_boolean_gates.exs"
+    filepath = Path.join(abs_path, filename)
+
+    File.write!(filepath, migration_content())
+    Mix.shell().info("Generated migration: #{filepath}")
+  end
+
+  defp timestamp do
+    {{y, m, d}, {hh, mm, ss}} = :calendar.universal_time()
+    "#{y}#{pad(m)}#{pad(d)}#{pad(hh)}#{pad(mm)}#{pad(ss)}"
+  end
+
+  defp pad(i) when i < 10, do: <<?0, ?0 + i>>
+  defp pad(i), do: to_string(i)
+
+  defp migration_content do
+    """
+    defmodule MyApp.Repo.Migrations.FixFunWithFlagsBooleanGates do
+      use Ecto.Migration
+
+      def up, do: Bandera.Ecto.Migrations.fix_fun_with_flags_boolean_gates()
+      def down, do: :ok
+    end
+    """
+  end
+end

test/bandera/ecto/migrations_test.exs

@@ -75,4 +75,97 @@ defmodule Bandera.Ecto.MigrationsTest do
 
     assert "value" in column_names(table)
   end
+
+  defmodule FixFunWithFlagsMigration do
+    use Ecto.Migration
+
+    @spec up() :: :ok
+    def up, do: Bandera.Ecto.Migrations.fix_fun_with_flags_boolean_gates()
+  end
+
+  describe "fix_fun_with_flags_boolean_gates/0" do
+    setup do
+      Bandera.TestRepo.query!("DELETE FROM schema_migrations WHERE version = 20260601000001")
+
+      on_exit(fn ->
+        Bandera.TestRepo.query!("DELETE FROM schema_migrations WHERE version = 20260601000001")
+      end)
+    end
+
+    test "renames a lone legacy boolean row to _bandera_none" do
+      Bandera.TestRepo.query!("""
+      INSERT INTO bandera_flags (flag_name, gate_type, target, enabled, value)
+      VALUES ('my_flag', 'boolean', 'boolean', true, NULL)
+      """)
+
+      run_fix_migration()
+
+      %{rows: rows} =
+        Bandera.TestRepo.query!(
+          "SELECT target, enabled FROM bandera_flags WHERE flag_name='my_flag' AND gate_type='boolean'"
+        )
+
+      assert rows == [["_bandera_none", 1]]
+    end
+
+    test "deletes legacy row when a Bandera row already exists, keeping Bandera row's enabled value" do
+      Bandera.TestRepo.query!("""
+      INSERT INTO bandera_flags (flag_name, gate_type, target, enabled, value)
+      VALUES ('my_flag', 'boolean', 'boolean', true, NULL),
+             ('my_flag', 'boolean', '_bandera_none', false, NULL)
+      """)
+
+      run_fix_migration()
+
+      %{rows: rows} =
+        Bandera.TestRepo.query!(
+          "SELECT target, enabled FROM bandera_flags WHERE flag_name='my_flag' AND gate_type='boolean'"
+        )
+
+      assert rows == [["_bandera_none", 0]]
+    end
+
+    test "does not touch actor or group rows" do
+      Bandera.TestRepo.query!("""
+      INSERT INTO bandera_flags (flag_name, gate_type, target, enabled, value)
+      VALUES ('my_flag', 'boolean', 'boolean', true, NULL),
+             ('my_flag', 'actor', 'user:1', true, NULL)
+      """)
+
+      run_fix_migration()
+
+      %{rows: rows} =
+        Bandera.TestRepo.query!(
+          "SELECT gate_type, target FROM bandera_flags WHERE flag_name='my_flag' ORDER BY gate_type"
+        )
+
+      assert rows == [["actor", "user:1"], ["boolean", "_bandera_none"]]
+    end
+
+    test "is a no-op when all boolean rows already use _bandera_none" do
+      Bandera.TestRepo.query!("""
+      INSERT INTO bandera_flags (flag_name, gate_type, target, enabled, value)
+      VALUES ('my_flag', 'boolean', '_bandera_none', true, NULL)
+      """)
+
+      run_fix_migration()
+
+      %{rows: rows} =
+        Bandera.TestRepo.query!(
+          "SELECT target, enabled FROM bandera_flags WHERE flag_name='my_flag' AND gate_type='boolean'"
+        )
+
+      assert rows == [["_bandera_none", 1]]
+    end
+
+    defp run_fix_migration do
+      Ecto.Migrator.run(
+        Bandera.TestRepo,
+        [{20_260_601_000_001, FixFunWithFlagsMigration}],
+        :up,
+        all: true,
+        log: false
+      )
+    end
+  end
 end

test/mix/tasks/bandera_gen_fix_fun_with_flags_migration_test.exs

@@ -0,0 +1,59 @@
+defmodule Mix.Tasks.Bandera.Gen.FixFunWithFlagsMigrationTest do
+  use ExUnit.Case, async: true
+  import ExUnit.CaptureIO
+
+  @task Mix.Tasks.Bandera.Gen.FixFunWithFlagsMigration
+
+  setup do
+    path =
+      Path.join(
+        System.tmp_dir!(),
+        "bandera_test_migrations_#{:erlang.unique_integer([:positive])}"
+      )
+
+    File.mkdir_p!(path)
+    on_exit(fn -> File.rm_rf!(path) end)
+    {:ok, path: path}
+  end
+
+  test "generates a migration file in the given path", %{path: path} do
+    capture_io(fn -> @task.run(["--path", path]) end)
+
+    files = File.ls!(path)
+    assert length(files) == 1
+    assert hd(files) =~ ~r/^\d{14}_fix_fun_with_flags_boolean_gates\.exs$/
+  end
+
+  test "generated file contains the correct module and helper call", %{path: path} do
+    capture_io(fn -> @task.run(["--path", path]) end)
+
+    content = path |> File.ls!() |> hd() |> then(&File.read!(Path.join(path, &1)))
+
+    assert content =~ "use Ecto.Migration"
+    assert content =~ "Bandera.Ecto.Migrations.fix_fun_with_flags_boolean_gates()"
+    assert content =~ "def up"
+    assert content =~ "def down, do: :ok"
+  end
+
+  test "prints the path of the generated file", %{path: path} do
+    output = capture_io(fn -> @task.run(["--path", path]) end)
+
+    assert output =~ path
+    assert output =~ "_fix_fun_with_flags_boolean_gates.exs"
+  end
+
+  test "defaults to priv/repo/migrations when no --path given" do
+    default_path = Path.join(File.cwd!(), "priv/repo/migrations")
+
+    on_exit(fn ->
+      File.ls!(default_path)
+      |> Enum.filter(&(&1 =~ "fix_fun_with_flags"))
+      |> Enum.each(&File.rm!(Path.join(default_path, &1)))
+    end)
+
+    output = capture_io(fn -> @task.run([]) end)
+
+    assert output =~ default_path
+    assert output =~ "_fix_fun_with_flags_boolean_gates.exs"
+  end
+end