Fix Waffle.Ecto.Type load and dump in embeds #44
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.TypeBREAKING 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 properlyb) 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
@achempion I'm not sure how you approach breaking changes in this project, but I believe handling old (and wrong) data representation inside the library would be messy, so I didn't. E.g. we would end up having code like
Seems like it's better to tell lib users who need this to handle their specific situation properly.
I'm not an expert in elixir or ecto though, so if someone knows a better way to make this backward compatible - feel free to add more commits.