zven21 / ancestry Goto Github PK

  def arrange(record, _opts, module) do
    case module.has_children?(record) do
      true ->
        Map.merge(record, %{children: do_build_children(record, module)})

      false ->
        record
    end
  end

  defp do_build_children(record, module) do
    record
    |> module.children()
    |> Enum.map(fn x ->
      case module.has_children?(x) do
        true ->
          Map.merge(x, %{children: do_build_children(x, module)})

        false ->
          x
      end
    end)
  end

If a record has 100 children but no grandchildren, We will hit the database 101 times, if every child has 100 children, we will hit the database more than 10000 times, the cost is huge.

The like statement is not accurate, example if id of the record is 1 it will find all subrecord having an ancestry of 10.., 100...

# https://github.com/zven21/ancestry/blob/master/lib/ancestry.ex#L265
query =
  from(
    u in unquote(module),
    where:
      fragment(
        unquote("#{opts[:ancestry_column]} LIKE ?"),
        ^"#{query_string}%"
      )
  )

A quick fix could be something like

query =
  from(
    u in unquote(module),
    where: fragment("#{opts[:ancestry_column]} = ?", "#{query_string}") or
      fragment(
        unquote("#{opts[:ancestry_column]} LIKE ?"),
        ^"#{query_string}/%"
      )
  )

Right now, Ancestry.Repo.delete/3 breaks convention by using the unsafe Repo.delete!/2.

Please, provide an alternate version of the function that uses Repo.delete/2 instead. Also, rename the current function to reflect the convention.

This "safe" version of this function, should probably accept and pass on the optional opts keyword list which allows us to treat exceptional cases like :stale_error_field.

category = Repo.get!(Category, id)

# This could be replaced with an exclusion happening in another process or request
Repo.delete!(category) 

# throws unexpected "** (Ecto.StaleEntryError) attempted to delete a stale struct:"
Category.delete(category)

If the struct has been removed from db prior to call, Ecto.StaleEntryError will be raised.

Since by the documentation, this is the default behavior. Ancestry.Repo.delete/3, should either treat the exclusion process completely (and safely) or provide an API that allows it.