migration naming fixes, child_of/descendant_of imprv

Author: AlexGx

lib/nested_sets.ex

@@ -66,6 +66,9 @@ defmodule NestedSets do
   @type depth :: non_neg_integer() | nil
   @type delete_result :: {:ok, non_neg_integer()} | {:error, error_reason()}
 
+  @typep position :: :prepend | :append | :before | :after
+  @typep validation_result :: :ok | {:error, String.t()}
+
   @doc false
   def config(node) when is_struct(node), do: config(node.__struct__)
 
@@ -179,7 +182,9 @@ defmodule NestedSets do
 
       {:ok, count} = NestedSets.delete_with_children(Repo, node)
   """
-  @spec delete_with_children(Ecto.Repo.t(), ns_node()) :: {:ok, integer()} | {:error, term()}
+
+  @spec delete_with_children(Ecto.Repo.t(), ns_node()) ::
+          {:ok, non_neg_integer()} | {:error, term()}
   def delete_with_children(repo, node) do
     cfg = config(node)
 
@@ -214,6 +219,7 @@ defmodule NestedSets do
   """
   @spec delete_node(Ecto.Repo.t(), ns_node()) :: {:ok, ns_node()} | {:error, term()}
   def delete_node(repo, node) do
+    # @review: root node deletion
     if root?(node) do
       {:error, :cannot_delete_root}
     else
@@ -303,14 +309,14 @@ defmodule NestedSets do
   """
   @spec descendant_of?(ns_node(), ns_node()) :: boolean()
   def descendant_of?(node, potential_parent) do
-    cfg = config(node)
-
     if node.__struct__ != potential_parent.__struct__ do
       raise ArgumentError,
             "descendant_of?/2 expects both arguments to be structs of the same Schema, " <>
               "got #{inspect(node.__struct__)} and #{inspect(potential_parent.__struct__)}"
     end
 
+    cfg = config(node)
+
     node_lft = Map.get(node, cfg.lft)
     node_rgt = Map.get(node, cfg.rgt)
     parent_lft = Map.get(potential_parent, cfg.lft)
@@ -330,14 +336,14 @@ defmodule NestedSets do
   """
   @spec child_of?(ns_node(), ns_node()) :: boolean()
   def child_of?(node, potential_parent) do
-    cfg = config(node)
-
     if node.__struct__ != potential_parent.__struct__ do
       raise ArgumentError,
             "child_of?/2 expects both arguments to be structs of the same Schema, " <>
               "got #{inspect(node.__struct__)} and #{inspect(potential_parent.__struct__)}"
     end
 
+    cfg = config(node)
+
     same_scope? =
       if cfg.tree != false do
         Map.get(node, cfg.tree) == Map.get(potential_parent, cfg.tree)
@@ -370,9 +376,6 @@ defmodule NestedSets do
 
   # Private section
 
-  @typep position :: :prepend | :append | :before | :after
-  @typep validation_result :: :ok | {:error, String.t()}
-
   @spec loaded?(ns_node()) :: boolean()
   defp loaded?(%{__meta__: %{state: :loaded}}), do: true
   defp loaded?(_), do: false

lib/nested_sets/migration.ex

@@ -26,7 +26,7 @@ defmodule NestedSets.Migration do
 
   use Ecto.Migration
 
-  defmacro nested_sets_fields(opts \\ []) do
+  defmacro nested_sets_columns(opts \\ []) do
     quote bind_quoted: [opts: opts] do
       add :lft, :integer, null: false
       add :rgt, :integer, null: false

test/nested_sets/migration_test.exs

@@ -5,12 +5,12 @@ defmodule NestedSets.MigrationTest do
 
   alias NestedSets.Test.Repos
 
-  describe "nested_sets_fields/1" do
+  describe "nested_sets_columns/1" do
     for {db_type, repo, tags} <- Repos.list() do
       @describetag tags
 
       @tag db: db_type
-      test "creates basic nested set fields without tree (#{db_type})" do
+      test "creates basic nested set columns without tree (#{db_type})" do
         repo = unquote(repo)
         db_type = unquote(db_type)
         table = unique_table_name()
@@ -19,7 +19,7 @@ defmodule NestedSets.MigrationTest do
 
         create_table(repo, table, fn ->
           quote do
-            NestedSets.Migration.nested_sets_fields()
+            NestedSets.Migration.nested_sets_columns()
           end
         end)
 
@@ -41,7 +41,7 @@ defmodule NestedSets.MigrationTest do
 
         create_table(repo, table, fn ->
           quote do
-            NestedSets.Migration.nested_sets_fields(use_tree: true)
+            NestedSets.Migration.nested_sets_columns(use_tree: true)
           end
         end)
 
@@ -63,7 +63,7 @@ defmodule NestedSets.MigrationTest do
 
         create_table(repo, table, fn ->
           quote do
-            NestedSets.Migration.nested_sets_fields(use_tree: :organization_id)
+            NestedSets.Migration.nested_sets_columns(use_tree: :organization_id)
           end
         end)
 
@@ -83,7 +83,7 @@ defmodule NestedSets.MigrationTest do
 
         create_table(repo, table, fn ->
           quote do
-            NestedSets.Migration.nested_sets_fields(use_tree: :tenant_id, tree_null: false)
+            NestedSets.Migration.nested_sets_columns(use_tree: :tenant_id, tree_null: false)
           end
         end)
 
@@ -108,7 +108,7 @@ defmodule NestedSets.MigrationTest do
 
         create_table(repo, table, fn ->
           quote do
-            NestedSets.Migration.nested_sets_fields(
+            NestedSets.Migration.nested_sets_columns(
               use_tree: :organization_id,
               tree_type: :id,
               tree_references: unquote(ref_table)
@@ -140,7 +140,7 @@ defmodule NestedSets.MigrationTest do
 
         create_table(repo, table, fn ->
           quote do
-            NestedSets.Migration.nested_sets_fields(use_tree: false)
+            NestedSets.Migration.nested_sets_columns(use_tree: false)
           end
         end)
 
@@ -158,7 +158,7 @@ defmodule NestedSets.MigrationTest do
 
         create_table(repo, table, fn ->
           quote do
-            NestedSets.Migration.nested_sets_fields(use_tree: nil)
+            NestedSets.Migration.nested_sets_columns(use_tree: nil)
           end
         end)
 
@@ -175,7 +175,7 @@ defmodule NestedSets.MigrationTest do
 
         def change do
           create table(:test_invalid) do
-            NestedSets.Migration.nested_sets_fields(use_tree: 123)
+            NestedSets.Migration.nested_sets_columns(use_tree: 123)
           end
         end
       end
@@ -194,7 +194,7 @@ defmodule NestedSets.MigrationTest do
 
         def change do
           create table(:test_missing_ref) do
-            NestedSets.Migration.nested_sets_fields(use_tree: :org_id, tree_type: :id)
+            NestedSets.Migration.nested_sets_columns(use_tree: :org_id, tree_type: :id)
           end
         end
       end
@@ -213,10 +213,10 @@ defmodule NestedSets.MigrationTest do
     :"#{prefix}_#{System.unique_integer([:positive])}"
   end
 
-  defp create_table(repo, table_name, fields_fn) do
+  defp create_table(repo, table_name, columns_fn) do
     reset_migrations!(repo)
 
-    fields_ast = fields_fn.()
+    columns_ast = columns_fn.()
 
     migration_module_name =
       String.to_atom("Elixir.NestedSets.TestMigration#{System.unique_integer([:positive])}")
@@ -232,7 +232,7 @@ defmodule NestedSets.MigrationTest do
           def change do
             create table(unquote(table_name)) do
               add :name, :string
-              unquote(fields_ast)
+              unquote(columns_ast)
             end
           end
         end,
@@ -331,8 +331,8 @@ defmodule NestedSets.MigrationTest do
         %{name: name, type: normalize_type(type, :postgres), nullable: nullable == "YES"}
 
       Ecto.Adapters.SQLite3 ->
-        [_cid, name, type, notnull, _default, _pk] = row
-        %{name: name, type: normalize_type(type, :sqlite), nullable: notnull == 0}
+        [_cid, name, type, not_null, _default, _pk] = row
+        %{name: name, type: normalize_type(type, :sqlite), nullable: not_null == 0}
 
       Ecto.Adapters.MyXQL ->
         [name, type, nullable] = row

test/support/mysql/migrations/00_create_test_tables.exs

@@ -9,7 +9,7 @@ defmodule NestedSets.Test.MysqlRepo.Migrations.CreateTestTables do
       add :name, :string, null: false
       add :active, :boolean, default: true, null: false
 
-      nested_sets_fields()
+      nested_sets_columns()
 
       timestamps()
     end
@@ -21,7 +21,7 @@ defmodule NestedSets.Test.MysqlRepo.Migrations.CreateTestTables do
       add :name, :string, null: false
       add :active, :boolean, default: true, null: false
 
-      nested_sets_fields(use_tree: true)
+      nested_sets_columns(use_tree: true)
 
       timestamps()
     end

test/support/postgres/migrations/00_create_test_tables.exs

@@ -9,7 +9,7 @@ defmodule NestedSets.Test.Repo.Migrations.CreateTestTables do
       add :name, :string, null: false
       add :active, :boolean, default: true, null: false
 
-      nested_sets_fields()
+      nested_sets_columns()
 
       timestamps()
     end
@@ -21,7 +21,7 @@ defmodule NestedSets.Test.Repo.Migrations.CreateTestTables do
       add :name, :string, null: false
       add :active, :boolean, default: true, null: false
 
-      nested_sets_fields(use_tree: true)
+      nested_sets_columns(use_tree: true)
 
       timestamps()
     end

test/support/sqlite/migrations/00_create_test_tables.exs

@@ -9,7 +9,7 @@ defmodule NestedSets.Test.SqliteRepo.Migrations.CreateTestTables do
       add :name, :string, null: false
       add :active, :boolean, default: true, null: false
 
-      nested_sets_fields()
+      nested_sets_columns()
 
       timestamps()
     end
@@ -21,7 +21,7 @@ defmodule NestedSets.Test.SqliteRepo.Migrations.CreateTestTables do
       add :name, :string, null: false
       add :active, :boolean, default: true, null: false
 
-      nested_sets_fields(use_tree: true)
+      nested_sets_columns(use_tree: true)
 
       timestamps()
     end