feat: Add default membership flag for test groups

config/dev.exs

@@ -72,4 +72,6 @@ config :phoenix, :stacktrace_depth, 20
 config :phoenix, :plug_init_mode, :runtime
 
 # Don't report issues with migrations from before we introduced excellent_migrations
-config :excellent_migrations, start_after: "20230908133314"
+config :excellent_migrations,
+  start_after: "20230908133314",
+  skip_checks: [:column_added_with_default]

lib/skate/settings/db/test_group.ex

@@ -5,11 +5,13 @@ defmodule Skate.Settings.Db.TestGroup do
   import Ecto.Changeset
 
   alias Skate.Settings.Db.TestGroupUser, as: DbTestGroupUser
+  alias Skate.Settings.TestGroupOverride
 
   @type t() :: %__MODULE__{}
 
   schema "test_groups" do
     field(:name, :string)
+    field(:override, TestGroupOverride, default: :none)
     timestamps()
 
     has_many(:test_group_users, DbTestGroupUser, on_replace: :delete_if_exists)
@@ -18,7 +20,7 @@ defmodule Skate.Settings.Db.TestGroup do
 
   def changeset(test_group, attrs \\ %{}) do
     test_group
-    |> cast(attrs, [:name])
+    |> cast(attrs, [:name, :override])
     |> cast_assoc(:test_group_users)
     |> validate_required([:name])
     |> unique_constraint(:name, name: :test_groups_unique_index)

lib/skate/settings/test_group.ex

@@ -4,15 +4,17 @@ defmodule Skate.Settings.TestGroup do
   alias Skate.Settings.Db.TestGroup, as: DbTestGroup
   alias Skate.Settings.Db.User, as: DbUser
 
+  import Ecto.Query, only: [from: 2]
+
   @type t() :: %__MODULE__{
           id: integer(),
           name: String.t(),
           users: [DbUser.t()]
         }
 
-  @enforce_keys [:id, :name, :users]
+  @enforce_keys [:id, :name, :users, :override]
 
-  defstruct [:id, :name, users: []]
+  defstruct [:id, :name, users: [], override: :none]
 
   @spec create(String.t()) :: {:ok, t()} | {:error, Ecto.Changeset.t()}
   def create(name) do
@@ -50,6 +52,13 @@ defmodule Skate.Settings.TestGroup do
     |> Enum.map(&convert_from_db_test_group(&1))
   end
 
+  @spec get_override_enabled() :: [t()]
+  def get_override_enabled() do
+    from(tg in DbTestGroup, where: tg.override == :enabled)
+    |> Skate.Repo.all()
+    |> Enum.map(&convert_from_db_test_group(&1))
+  end
+
   @spec update(t()) :: t()
   def update(test_group) do
     existing_test_group =
@@ -80,7 +89,8 @@ defmodule Skate.Settings.TestGroup do
     %__MODULE__{
       id: db_test_group.id,
       name: db_test_group.name,
-      users: db_test_group.users
+      users: db_test_group.users,
+      override: db_test_group.override
     }
   end
 

lib/skate/settings/test_group_override.ex

@@ -0,0 +1,25 @@
+defmodule Skate.Settings.TestGroupOverride do
+  @moduledoc false
+
+  use Ecto.Type
+
+  @type t :: :enabled | :disabled | :none
+
+  @impl true
+  def type, do: :string
+
+  @valid_states [:enabled, :disabled, :none]
+
+  @impl true
+  def cast(term) when term in @valid_states, do: {:ok, term}
+  def cast(_), do: :error
+
+  @impl true
+  def dump(term) when term in @valid_states, do: {:ok, Atom.to_string(term)}
+  def dump(_), do: :error
+
+  @impl true
+  def load(term) do
+    term |> String.to_existing_atom() |> cast()
+  end
+end

lib/skate/settings/user.ex

@@ -2,6 +2,7 @@ defmodule Skate.Settings.User do
   @moduledoc false
 
   alias Skate.Settings.Db.User, as: DbUser
+  alias Skate.Settings.TestGroup
 
   import Ecto.Query
   require Logger
@@ -75,12 +76,20 @@ defmodule Skate.Settings.User do
   end
 
   def is_in_test_group(user_id, test_group_name) do
-    %{test_groups: user_test_groups} =
-      DbUser
-      |> Skate.Repo.get(user_id)
-      |> Skate.Repo.preload(:test_groups)
+    user_id
+    |> get_by_id!()
+    |> all_test_group_names()
+    |> Enum.member?(test_group_name)
+  end
+
+  def all_test_group_names(user) do
+    user = Skate.Repo.preload(user, :test_groups)
+
+    enabled_test_groups = TestGroup.get_override_enabled()
 
-    Enum.any?(user_test_groups, &(&1.name == test_group_name))
+    (enabled_test_groups ++ user.test_groups)
+    |> Enum.map(& &1.name)
+    |> Enum.dedup()
   end
 
   defp users_for_route_ids_query(route_ids) do

lib/skate_web/controllers/page_controller.ex

@@ -9,11 +9,7 @@ defmodule SkateWeb.PageController do
   def index(conn, _params) do
     %{id: user_id} = AuthManager.Plug.current_resource(conn)
 
-    %{username: username} =
-      user =
-      user_id
-      |> User.get_by_id!()
-      |> Skate.Repo.preload(:test_groups)
+    %{username: username} = user = User.get_by_id!(user_id)
 
     _ = Logger.info("uid=#{username}")
 
@@ -38,7 +34,7 @@ defmodule SkateWeb.PageController do
       base: Application.get_env(:skate, :base_tileset_url),
       satellite: Application.get_env(:skate, :satellite_tileset_url)
     })
-    |> assign(:user_test_groups, Enum.map(user.test_groups, & &1.name))
+    |> assign(:user_test_groups, User.all_test_group_names(user))
     |> assign(:map_limits, map_limits)
     |> assign(:sentry_org_slug, Application.get_env(:skate, :sentry_org_slug))
     |> render("index.html")

lib/skate_web/controllers/test_group_controller.ex

@@ -53,6 +53,7 @@ defmodule SkateWeb.TestGroupController do
       |> assign(:test_group_name, test_group.name)
       |> assign(:test_group_id, test_group.id)
       |> assign(:test_group_users, test_group_users)
+      |> assign(:test_group_override, test_group.override)
       |> put_layout()
       |> render(:test_group)
     else
@@ -135,6 +136,20 @@ defmodule SkateWeb.TestGroupController do
     redirect(conn, to: ~p"/test_groups")
   end
 
+  @spec enable_override(Plug.Conn.t(), map()) :: Plug.Conn.t()
+  def enable_override(conn, %{"id" => id}) do
+    test_group = TestGroup.get(id)
+    TestGroup.update(%{test_group | override: :enabled})
+    redirect(conn, to: ~p"/test_groups/#{id}")
+  end
+
+  @spec remove_override(Plug.Conn.t(), map()) :: Plug.Conn.t()
+  def remove_override(conn, %{"id" => id}) do
+    test_group = TestGroup.get(id)
+    TestGroup.update(%{test_group | override: :none})
+    redirect(conn, to: ~p"/test_groups/#{id}")
+  end
+
   defp put_layout(conn) do
     conn
     |> put_layout(html: {SkateWeb.Layouts, :barebones})

lib/skate_web/controllers/test_group_html/test_group.html.heex

@@ -1,4 +1,24 @@
 <h1><%= @test_group_name %></h1>
+<.form
+  :if={@test_group_override != :enabled}
+  for={@conn}
+  action={~p"/test_groups/#{@test_group_id}/enable_override"}
+>
+  <%= submit("Enable for all users") %>
+</.form>
+
+<div :if={@test_group_override != :none}>
+  <em>
+    There is currently an override in place, which means that all
+    users are treated as though they are members of <%= @test_group_name %>.  You can still add and remove users
+    below, but that will only have an effect if you remove the
+    override.
+  </em>
+  <.form for={@conn} action={~p"/test_groups/#{@test_group_id}/remove_override"}>
+    <%= submit("Remove override") %>
+  </.form>
+</div>
+
 <ul>
   <%= for {user_id, user_email} <- @test_group_users do %>
     <li>
@@ -12,6 +32,7 @@
 </ul>
 <.link href={~p"/test_groups/#{@test_group_id}/add_user"}>Add user</.link>
 <.link href={~p"/test_groups"}>Back to all test groups</.link>
+
 <.form for={@conn} action={~p"/test_groups/#{@test_group_id}"} method="DELETE">
   <%= submit("Delete group", onclick: "return confirm('Are you sure? This cannot be undone.')") %>
 </.form>

lib/skate_web/router.ex

@@ -111,6 +111,8 @@ defmodule SkateWeb.Router do
     get "/test_groups/:id/add_user", TestGroupController, :add_user_form
     post "/test_groups/:id/add_user", TestGroupController, :add_user
     post "/test_groups/:id/remove_user", TestGroupController, :remove_user
+    post "/test_groups/:id/enable_override", TestGroupController, :enable_override
+    post "/test_groups/:id/remove_override", TestGroupController, :remove_override
   end
 
   scope "/api", SkateWeb do

priv/repo/migrations/20231220204054_add_override_for_test_groups.exs

@@ -0,0 +1,15 @@
+defmodule Skate.Repo.Migrations.AddOverrideForTestGroups do
+  use Ecto.Migration
+
+  def change do
+    # excellent_migrations:safety-assured-for-this-file raw_sql_executed
+    execute(
+      "CREATE TYPE test_group_override AS ENUM ('none', 'enabled', 'disabled')",
+      "DROP TYPE test_group_override"
+    )
+
+    alter table(:test_groups) do
+      add(:override, :test_group_override, default: "none", null: false)
+    end
+  end
+end

test/skate/settings/test_group_test.exs

@@ -52,7 +52,7 @@ defmodule Skate.Settings.TestGroupTest do
     end
   end
 
-  describe "get_all/1" do
+  describe "get_all/0" do
     test "gets all test groups" do
       {:ok, group1} = TestGroup.create("group 1")
       {:ok, group2} = TestGroup.create("group 2")
@@ -66,6 +66,28 @@ defmodule Skate.Settings.TestGroupTest do
     end
   end
 
+  describe "get_override_enabled/0" do
+    test "only retrieves test groups with the :enabled override" do
+      {:ok, group1} = TestGroup.create("group 1")
+      {:ok, _group2} = TestGroup.create("group 2")
+      {:ok, _group3} = TestGroup.create("group 3")
+      {:ok, group4} = TestGroup.create("group 4")
+
+      TestGroup.update(%{group1 | override: :enabled})
+      TestGroup.update(%{group4 | override: :enabled})
+
+      groups = TestGroup.get_override_enabled()
+
+      assert Enum.count(groups) == 2
+
+      refute groups |> Enum.find(fn group -> group.name == "group 1" end) |> is_nil()
+      refute groups |> Enum.find(fn group -> group.name == "group 4" end) |> is_nil()
+
+      assert groups |> Enum.find(fn group -> group.name == "group 2" end) |> is_nil()
+      assert groups |> Enum.find(fn group -> group.name == "group 3" end) |> is_nil()
+    end
+  end
+
   describe "update/1" do
     test "updates name" do
       {:ok, test_group} = TestGroup.create("name 1")
@@ -97,6 +119,20 @@ defmodule Skate.Settings.TestGroupTest do
       assert user2 in users_update_2
       assert user3 in users_update_2
     end
+
+    test "can set override to enabled" do
+      {:ok, test_group} = TestGroup.create("group name")
+
+      new_test_group = TestGroup.update(%{test_group | override: :enabled})
+
+      assert new_test_group.override == :enabled
+    end
+
+    test "override defaults to :none" do
+      {:ok, test_group} = TestGroup.create("group name")
+
+      assert test_group.override == :none
+    end
   end
 
   describe "delete/1" do

test/skate/settings/user_test.exs

@@ -131,5 +131,57 @@ defmodule Skate.Settings.UserTest do
       assert User.is_in_test_group(user_1.id, target_test_group.name)
       refute User.is_in_test_group(user_1.id, other_test_group.name)
     end
+
+    test "returns true if the test group has an override enabled" do
+      user_1 = User.upsert(@username, @email)
+      user_2 = User.upsert("otheruser", "otheruser@test.com")
+      {:ok, overridden_test_group} = TestGroup.create("overridden_test_group")
+
+      overridden_test_group = TestGroup.update(%{overridden_test_group | override: :enabled})
+      assert User.is_in_test_group(user_1.id, overridden_test_group.name)
+      assert User.is_in_test_group(user_2.id, overridden_test_group.name)
+    end
+  end
+
+  describe "all_test_group_names/1" do
+    test "returns an empty array if there are no test groups" do
+      user = User.upsert("user", "user@test.com")
+
+      assert User.all_test_group_names(User.get_by_id(user.id)) == []
+    end
+
+    test "returns a test group if the user is a member of it" do
+      {:ok, test_group} = TestGroup.create("test-group")
+      user = User.upsert("user", "user@test.com")
+
+      TestGroup.update(%{test_group | users: [user]})
+
+      assert User.all_test_group_names(User.get_by_id(user.id)) == ["test-group"]
+    end
+
+    test "does not include test groups that the user is not a member of" do
+      TestGroup.create("test-group")
+      user = User.upsert("user", "user@test.com")
+
+      assert User.all_test_group_names(User.get_by_id(user.id)) == []
+    end
+
+    test "includes test groups that have an :enabled override" do
+      {:ok, test_group} = TestGroup.create("test-group")
+      user = User.upsert("user", "user@test.com")
+
+      TestGroup.update(%{test_group | override: :enabled})
+
+      assert User.all_test_group_names(User.get_by_id(user.id)) == ["test-group"]
+    end
+
+    test "does not include duplicate test group records" do
+      {:ok, test_group} = TestGroup.create("test-group")
+      user = User.upsert("user", "user@test.com")
+
+      TestGroup.update(%{test_group | override: :enabled, users: [user]})
+
+      assert User.all_test_group_names(User.get_by_id(user.id)) == ["test-group"]
+    end
   end
 end