Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

field updated_at waffle.Ecto.type in a embed schema #19

Open
mrdotb opened this issue Jun 15, 2020 · 6 comments · May be fixed by #44
Open

field updated_at waffle.Ecto.type in a embed schema #19

mrdotb opened this issue Jun 15, 2020 · 6 comments · May be fixed by #44

Comments

@mrdotb
Copy link

mrdotb commented Jun 15, 2020

Hello,

When I write some tests I found a weird behavior from waffle_ecto
The updated_at type return from insert differ from select. (NaiveDateTime to String)
I did an example with phoenix since it happen when the data is inserted in postgres.

Basically I got a embeds_many who use Waffle.Ecto.Schema

defmodule Raven.Post do
  use Ecto.Schema
  import Ecto.Changeset

  schema "posts" do
    field :title, :string
    embeds_many :media, Raven.Media
  end

  def changeset(post, attrs) do
    post
    |> cast(attrs, ~w(title)a)
    |> cast_embed(:media, with: &Raven.Media.changeset/2)
  end
end

defmodule Raven.Media do
  use Ecto.Schema
  use Waffle.Ecto.Schema
  import Ecto.Changeset

  alias Raven.Uploaders.Media

  @primary_key false
  embedded_schema do
    field :src, Media.Type
    field :alt, :string
  end

  @doc false
  def changeset(media, attrs) do
    media
    |> cast(attrs, ~w(alt)a)
    |> cast_attachments(attrs, ~w(src)a)
  end
end

# I omit some code clone the example repo to get all

test "datetime not parsed" do
  upload = build_upload(@img)
  attrs = %{
    title: "Some title",
    media: [%{
      alt: "Some alt",
      src: upload
    },
    %{
      alt: "Some alt",
      src: upload
    }]
  }
  assert {:ok, post} = App.create_post(attrs)
  assert App.list_posts() == [post]
end

In the test result the updated_at field in the embeds_many is a NaiveDateTime on insert and a String on select
Is this behavior expected ? (since it's stored in text format and not timestamp)

     left:  [
              %Raven.Post{
                __meta__: #Ecto.Schema.Metadata<:loaded, "posts">,
                id: 16,
                media: [
                  %Raven.Media{alt: "Some alt", src: %{file_name: "pixel.png", updated_at: "2020-06-15T16:48:58"}},
                  %Raven.Media{alt: "Some alt", src: %{file_name: "pixel.png", updated_at: "2020-06-15T16:48:58"}}
                ],
                title: "Some title"
              }
            ]
     right: [
              %Raven.Post{
                __meta__: #Ecto.Schema.Metadata<:loaded, "posts">,
                id: 16,
                media: [
                  %Raven.Media{alt: "Some alt", src: %{file_name: "pixel.png", updated_at: ~N[2020-06-15 16:48:58]}},
                  %Raven.Media{alt: "Some alt", src: %{file_name: "pixel.png", updated_at: ~N[2020-06-15 16:48:58]}}
                ],
                title: "Some title"
              }
            ]
     stacktrace:
       test/raven/app_test.exs:41: (test)

Thanks for your time.
@achempion
Copy link
Member

Hi, thank your for reporting this issue and providing an example.

updated_at attribute should be the NaiveDateTime everywhere, I'll look into your repo.

@mrdotb
Copy link
Author

mrdotb commented Sep 15, 2020

Hello, did you have the time to check ?
I can dive in ?

@achempion
Copy link
Member

@mrdotb PR is welcome

@Faymir
Copy link

Faymir commented Feb 3, 2022

Hello.
Is there any news about the correction of this bug?

timadevelop added a commit to timadevelop/waffle_ecto that referenced this issue Dec 12, 2022
@timadevelop
Fix Waffle.Ecto.Type load and dump in embeds
009571c 
- fixes elixir-waffle#19

By default Ecto uses :self strategy which encodes the type without
calling load/1 or dump/1 on custom type. So Waffle.Ecto.Type was
encoded and decoded as a json, with native Ecto tools -> it was stored
as an actual json in DB instead of using `filename_with_timestamp`
string representation defined in Waffle.Ecto.Type

BREAKING CHANGE: this commit will "break" loading existing embedded
fieds from DB that used previous waffle versions. Now Waffle.Ecto.Type
expects `value` to be a string, but old representation will feed it
a map. Depending on your ecto adapter, you can fix it by:
a) re-dumping all values to your db properly
b) adding Waffle.Ecto.Type.load/2 for a json Map values and parsing
"updated_at" map value to NaiveDateTime from a representation specific
to your adapter
@timadevelop timadevelop linked a pull request Dec 12, 2022 that will close this issue
Open
@timadevelop
Copy link

@mrdotb @Faymir I made a PR, but if you need to fix this asap - you can write your own version of Waffle.Ecto.Definition that would implement embed_as/1 to return :dump value.

Honestly, it's quite sad that things like this bug are not being fixed in the elixir community for years... I wonder why. Because nobody stores filenames inside embeds? Maybe nobody uses this in production? Or everyone's using another lib for file uploading?

@mrdotb
Copy link
Author

mrdotb commented Dec 12, 2022

I guess not much people use it as embed. Glad you found the root cause. I tried a bit and I ended up not using waffle_ecto and roll my own embedded schema.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix Waffle.Ecto.Type load and dump in embeds timadevelop/waffle_ecto
4 participants
@achempion @mrdotb @timadevelop @Faymir