Refactor QueryHelper and Models

[oyeb]

The QueryHelper brings convenience at a cost. It has polymorphic clauses for update, delete, get which accept integers/nil in addition to struct.

• This makes it hard to document and improve.
• We can reduce arity of most calls to QH if we only accept Ecto.Schema structs, because these struct have a lot of metadata.
• These methods that work with integer IDs have not been of much use, and I don't think they will ever be.

Tasks

• [ ] Make sure the last arg of all calls is a keyword list of options. This is important because all Ecto.Repo functions accept a keyword list of DB options.
• [ ] Reduce arity of all calls by
  • finding the Ecto.Repo at compile time (or fetch it from the keyword list)
  • finding the schema of struct from the metadata
• [ ] Remove commit_if_valid/2 because it serves no purpose.
• [ ] Update all Models to reflect changes made in QueryHelper.
• [ ] Add method call for soft delete.

[ramansah]

Proposing a refactor without breaking the existing build. So created some methods in Snitch.Data.Model which will help in reducing methods' arity in models.

model.ex

defmodule Snitch.Data.Model do
  defmacro __using__(_) do
    quote do

      def create(opts) do
        QH.create(get_module(), opts, Repo)
      end

      # Similar methods for update, get, delete

      defp get_module(), do: __MODULE__.get_schema()

      defoverridable create, 1

    end
  end
end

Best Case Scenario, User models becomes empty! user.ex

defmodule Snitch.Data.Model.User do

  def get_schema() do
    UserSchema
  end

end

Other models can ignore Repo and avoid repeated passing of schema

defmodule Snitch.Data.Model.Order do
  
  def get_schema() do
    Order
  end

  def create(params) do
    super(update_in(params, [:line_items], &update.../1))
  end

end

Tests are positive but since I have not gone through all the models in depth, there may be some use cases which require additional configuration or they may not support this design. Not now, but I did face some ambiguity errors with AsNestedSet in some model (Taxonomy maybe) when arity of create was same.

[arjun289]

@ramansah this looks good. Also, if you are trying to delve deeper into this then, please help us by removing the redundancies from QueryHelper.