Fix Date / Datetime issues

hamiltop commented 8 years ago

Does it handle dates for update as well?

thiagoc7 commented 8 years ago

Thanks for the code review, I feel that now it's better, but wait for your call. For green tests I need to convert it back to Ecto.Date after find.

I'll work on this in a couple hours, but can you point me to right place?

hamiltop commented 8 years ago

load_model is where the magic happens. Right now it's using Ecto.Schema.__load__. It's probably the most fragile part of the codebase, as it's a public but undocumented function in Ecto. So, "here be dragons".

There's a private function called load that is passed into __load__. Right now that function just returns {:ok, data}. I think if you pattern match on the RethinkDB date type, you can return {:ok, convert_date(data)} or something of the sort.

thiagoc7 commented 8 years ago

Yeah, load will do!! I'm just having a hard time to get create a Ecto.Date%{}.

iex(10)> value_from_db = %RethinkDB.Pseudotypes.Time{epoch_time: 1452643200,  timezone: "+00:00"}
iex(11)> RethinkDB.Query.day(value_from_db)
%RethinkDB.Q{query: [130,
  [%{:epoch_time => 1452643200,
     :timezone => "+00:00",
     "$reql_type$" => "TIME"}]]}

hamiltop commented 8 years ago

http://hexdocs.pm/ecto/Ecto.Date.html#cast/1

hamiltop commented 8 years ago

Hmm... cast won't do epoch time... :calendar.seconds_to_time is probably what you want

thiagoc7 commented 8 years ago

uhu, all green!! Solution came from this blog. Maybe it would be nice to add this date conversion to rethinkdb_elixir.

hamiltop commented 8 years ago

I want the driver to be pretty limited in data types. That's what libs like this are for. Some helpers to and from core datetime structures and RethinkDB structures might make sense for the driver though.

hamiltop commented 8 years ago

Maybe adding the equivalent to Ecto.Date.to_erl and Ecto.Date.from_erl in the driver would be useful. What do you think?

thiagoc7 commented 8 years ago

Maybe adding the equivalent to Ecto.Date.to_erl and Ecto.Date.from_erl in the driver would be useful. > What do you think?

I think that it's that the library should deal with date, at least to_erl. Maybe it can be an attr in the struct.

hamiltop commented 8 years ago

Maybe it can be an attr in the struct.

Not sure what you mean by that. But let's open up an issue on that repo and discuss it.

thiagoc7 commented 8 years ago

I'm glad to help! I'm not a professional, code is a hobby, do this just because I like. But I don't get involved with other programmers as much. So for me it's really cool helping you this way, and I learn a lot.

I end up here because I get a little disappointed after finding out how phoenix's channels work... I was expecting something more firebase, or meteor. So I looked for a solution, and changefeeds seems to fit. Maybe in the future we could work on something to make phoenix realtime features as easy as firebase.

Tomorrow I'll finish the tests. And please let me know when you'll merge, so I rebase all this commits. Thanks for the warm welcome!

Cheers!

thiagoc7 commented 8 years ago

I created all CRUD tests, but there are two that are not working:

get many - I don't know how to make db insert work in transactions, so I can't control what is in db
query - where does table function came from? It's in readme.

hamiltop commented 8 years ago

@thiagoc7 for get_many, in the past I've had a unique table per test. That gets the desired sandboxing. Or delete everything in the table before inserting.

For table, it's in RethinkDB.Query.

thiagoc7 commented 8 years ago

Hi @hamiltop, thanks for the tips! Now just the query test is failing. I'll let it crash on travis, so you can see the error:

     test/query_test.exs:46
     ** (UndefinedFunctionError) undefined function RethinkDB.Q.fetch/2
     stacktrace:
       (rethinkdb) RethinkDB.Q.fetch(%RethinkDB.Q{query: [10, [#Reference<0.0.4.503>]]}, :title)
       (elixir) lib/access.ex:77: Access.get/3
       test/query_test.exs:51: anonymous fn/1 in QueryTest.test filtered queries work/1
       (rethinkdb) lib/rethinkdb/query.ex:1808: RethinkDB.Query.func/1
       (rethinkdb) lib/rethinkdb/query.ex:1035: RethinkDB.Query.filter/2
       test/query_test.exs:50

hamiltop commented 8 years ago

Ahh... My example was flawed. You need to use the lambda macro from RethinkDB.Lambda like this:

import RethinkDB.Lambda

filter(lambda fn post -> post[:title] == "yayay" end)

Could you update the readme too?

On Thu, Jan 14, 2016, 8:35 AM Thiago notifications@github.com wrote:

Hi @hamiltop https://github.com/hamiltop, thanks for the tips! Now just the query test is failing. I'll let it crash on travis, so you can see the error:
 test/query_test.exs:46
 ** (UndefinedFunctionError) undefined function RethinkDB.Q.fetch/2
 stacktrace:
   (rethinkdb) RethinkDB.Q.fetch(%RethinkDB.Q{query: [10, [#Reference<0.0.4.503>]]}, :title)
   (elixir) lib/access.ex:77: Access.get/3
   test/query_test.exs:51: anonymous fn/1 in QueryTest.test filtered queries work/1
   (rethinkdb) lib/rethinkdb/query.ex:1808: RethinkDB.Query.func/1
   (rethinkdb) lib/rethinkdb/query.ex:1035: RethinkDB.Query.filter/2
   test/query_test.exs:50
— Reply to this email directly or view it on GitHub https://github.com/hamiltop/rethinkdb_ecto/pull/9#issuecomment-171692906 .

hamiltop commented 8 years ago

Actually, I'm going to redo the readme anyway, I can take care of it.

On Thu, Jan 14, 2016, 8:59 AM Peter Hamilton peterghamilton@gmail.com wrote:

Ahh... My example was flawed. You need to use the lambda macro from RethinkDB.Lambda like this:

import RethinkDB.Lambda

filter(lambda fn post -> post[:title] == "yayay" end)

Could you update the readme too?

On Thu, Jan 14, 2016, 8:35 AM Thiago notifications@github.com wrote:
Hi @hamiltop https://github.com/hamiltop, thanks for the tips! Now just the query test is failing. I'll let it crash on travis, so you can see the error:
 test/query_test.exs:46
 ** (UndefinedFunctionError) undefined function RethinkDB.Q.fetch/2
 stacktrace:
   (rethinkdb) RethinkDB.Q.fetch(%RethinkDB.Q{query: [10, [#Reference<0.0.4.503>]]}, :title)
   (elixir) lib/access.ex:77: Access.get/3
   test/query_test.exs:51: anonymous fn/1 in QueryTest.test filtered queries work/1
   (rethinkdb) lib/rethinkdb/query.ex:1808: RethinkDB.Query.func/1
   (rethinkdb) lib/rethinkdb/query.ex:1035: RethinkDB.Query.filter/2
   test/query_test.exs:50
— Reply to this email directly or view it on GitHub https://github.com/hamiltop/rethinkdb_ecto/pull/9#issuecomment-171692906 .

thiagoc7 commented 8 years ago

now it's all green!

I noticed that you are not validating before insert. the date test's shouldn't work, because I set title as a required field on changesets

hamiltop commented 8 years ago

So I think this is good to go. I'll go ahead and merge and close this.

Re: validating schema. Let's have a discussion on https://github.com/hamiltop/rethinkdb_ecto/issues/10 around that.

Thanks for the help!

hamiltop / rethinkdb_ecto

Fix Date / Datetime issues #9