hamiltop / rethinkdb_ecto

Shim library for using Ecto with RethinkDB. Not a full adapter.
24 stars 4 forks source link

date field #8

Closed thiagoc7 closed 8 years ago

thiagoc7 commented 8 years ago

Hi, thanks for this repo!

I can't insert a changeset that has a date filed:

** (Protocol.UndefinedError) protocol Enumerable not implemented for #Ecto.Date<2016-01-01>
    (elixir) lib/enum.ex:1: Enumerable.impl_for!/1
    (elixir) lib/enum.ex:116: Enumerable.reduce/3
    (elixir) lib/enum.ex:1477: Enum.reduce/3
    (elixir) lib/enum.ex:1092: Enum.map/2
             lib/rethinkdb/query/macros.ex:74: RethinkDB.Query.Macros.wrap/1
             lib/rethinkdb/query/macros.ex:75: anonymous fn/1 in RethinkDB.Query.Macros.wrap/1
    (elixir) lib/enum.ex:1092: anonymous fn/3 in Enum.map/2
    (stdlib) lists.erl:1262: :lists.foldl/3
    (elixir) lib/enum.ex:1092: Enum.map/2
             lib/rethinkdb/query/macros.ex:74: RethinkDB.Query.Macros.wrap/1
             lib/rethinkdb/query.ex:1288: RethinkDB.Query.insert/2
             lib/rethinkdb_ecto/connection.ex:63: RethinkDB.Ecto.Connection.do_insert/2

this is my dependencies:

defp deps do
    [{:phoenix, "~> 1.1.0"},
     {:phoenix_ecto, "~> 2.0"},
     {:postgrex, ">= 0.0.0"},
     {:phoenix_html, "~> 2.3"},
     {:phoenix_live_reload, "~> 1.0", only: :dev},
     {:gettext, "~> 0.9"},
     {:cowboy, "~> 1.0"},
     {:rethinkdb, "~> 0.2.1"},
     {:rethinkdb_ecto, github: "hamiltop/rethinkdb_ecto"}]
  end

is it really an issue, or am I doing something wrong?

Cheers!

hamiltop commented 8 years ago

Yeah, that's an issue.

Any interest is helping to fix it? I'd be more than happy to help walk you through it.

thiagoc7 commented 8 years ago

@hamiltop, thanks for the kind reply! I have to warn you. I'm learning elixir, this functional / process stuff is still entering in my head... And this issue is my first contact with rethinkdb!! But let's go, before you regret your offer...

this issue in related with this repo, or with rethink-elixir?

hamiltop commented 8 years ago

I think this is a good beginner issue. It doesn't have anything to do with processes or OTP. It has minimal FP stuff. You should be fine :smile:

The issue is that rethinkdb doesn't know what an Ecto.Date is. RethinkDB has their own datetime datatype:

iex(7)> RethinkDB.Query.time(2015, 1, 1, "Z") |> RethinkDB.Connection.run(conn)
%RethinkDB.Record{data: %RethinkDB.Pseudotypes.Time{epoch_time: 1420070400,
  timezone: "+00:00"}}

The RethinkDB datatype is %RethinkDB.Pseudotypes.Time{} it is made up of an epoch_time and a timezone.

So, first things: I have a branch named next. It adds a bunch of things (migrations anyone?), but the main thing is that I changed the name of RethinkDB.Ecto.Connection to RethinkDB.Ecto.Repo. Develop against that branch.

Now, look at https://github.com/hamiltop/rethinkdb_ecto/blob/next/lib/rethinkdb_ecto/repo.ex#L51 do_insert. This is where we apply the changeset and then do a ton of processing before we insert it into the db.

Somewhere in there, we need to iterate over all values in the model and if the value is an Ecto.Date, we need to convert it to a RethinkDB time. Either we can build the %RethinkDB.Pseudotypes.Time{} struct, or we can call one of the RethinkDB query functions to build the time object. For example, %{date: RethinkDB.Query.iso8601("1986-11-03T08:30:00-07:00")} is a valid ReQL object that can be inserted.

So the main pieces are "How do I convert from Ecto.Date to something ReQL understands?" and "How do I make sure this happens in the right places at the right time?"

Finally, writing a test: https://github.com/hamiltop/rethinkdb_ecto/blob/next/test/repo_test.exs#L24 is a wonderful starting point. You should be able to reproduce the error in a test fairly easily.

Join the #rethinkdb channel on the elixir Slack. I'm usually available to help there.

Good luck!

thiagoc7 commented 8 years ago

Thanks @hamiltop!!!

I did same minor refactor on tests structure, so it's easy for me to understand, don't know if you like.

Later today I'll start working on this. Cheers!

hamiltop commented 8 years ago

Love it.

On Wed, Jan 13, 2016, 3:38 AM Thiago notifications@github.com wrote:

Thanks @hamiltop https://github.com/hamiltop!!!

I did same minor refactor on tests structure https://github.com/thiagoc7/rethinkdb_ecto/blob/tests-refactoring/test/query_test.exs, so it's easy for me to understand, don't know if you like.

Later today I'll start working on this. Cheers!

— Reply to this email directly or view it on GitHub https://github.com/hamiltop/rethinkdb_ecto/issues/8#issuecomment-171262604 .

thiagoc7 commented 8 years ago

It's almost done, I'm having trouble with timezone. I could not find anything related in Ecto.Date docs. Can you help me? PR #9

hamiltop commented 8 years ago

https://github.com/elixir-lang/ecto/issues/535

On Wed, Jan 13, 2016, 7:17 AM Thiago notifications@github.com wrote:

It's almost done, I'm having trouble with timezone. I could not find anything related in Ecto.Date docs. Can you help me? PR #9 https://github.com/hamiltop/rethinkdb_ecto/pull/9

— Reply to this email directly or view it on GitHub https://github.com/hamiltop/rethinkdb_ecto/issues/8#issuecomment-171325910 .

willykaram commented 8 years ago

I can help with this on the weekend too, even if just ends up being reviewing/testing what's done. Much of what I need to do is date related, so I definitely have an interest in this. At first glance it seems as though "calling one of the RethinkDB query functions to build the time object" might be the way to go, as I've seen similar patterns before and we may be best to follow the DB's lead on this.

hamiltop commented 8 years ago

merged into branch next. I'm working with @ryanswapp to get his blog post updated and working with that code. Once he's ready, I'm going to merge that into master. Thank you for all the help!

Note: https://github.com/hamiltop/rethinkdb_ecto/issues/10 spun out of the PR.