arc_ecto icon indicating copy to clipboard operation
arc_ecto copied to clipboard

Scope id not available on insert with auto generated uuid

Open chrisjowen opened this issue 8 years ago • 9 comments

Hey,

So looking at the examples here is seem that everything is geared towards adding an image to an existing ecto model. I have an endpoint that uploads an image and some meta data and I would like to create the model and upload the image to S3 with the storage directory "uploads/#{scope.id}".

The problem is that at the time of insert the scope has no id, and I assumed that using server side generated uuids would fix this as the id is present before insertion. Im new to ecto/elixir/everything here so I dont understand the lifecycle that your working with, but is there some way to make the call to retrieve the storage_dir come after the auto generation of the uuid id?

Chris

chrisjowen avatar Feb 03 '16 00:02 chrisjowen

To be honest I haven't worked with UUIDs in Ecto yet, and you're right. The use cases I have built Arc with were mostly geared towards adding attachments or images to existing models.

Given that, I am open to having a better workflow for uploading during creation of the model itself, but I haven't had the time to investigate what a clean implementation would look like.

If you can get Ecto to assign the UUID prior to cast_attachments, then it should work fine. I don't know since I haven't tried though.

Alternatively, you can insert the model and then add the attachment immediately after, as in https://github.com/stavro/arc_ecto/issues/7

stavro avatar Feb 03 '16 01:02 stavro

Hi Stavro,

Thanks for the advice, adding the id in the changeset prior to cast_attachments as another cast did work but would have implications to updates as it always sets the id field. This isnt a problem for me as I wont be updating but its not really ideal.

I don't wanna start branching and adding features you disagree with (if I even can, as I said i'm an elixir/ecto newbie). But I thought that it would be useful was if the result of arc's store method returned a map with the filename and a generated uuid or some other unique id to be used in the definition methods. This would avoid having to have the models id available, so things like this could be used in the definition:

  def storage_dir(version, {file, scope}) do
    # "uploads/#{scope.id}" -- in this case scope is model and on insert has no id
    "uploads/#{file.file_identifier}" 
  end

This would need to be stored similar to the timestamp something like.

[image]|[UniqueIdentifier]?[Timestamp]

i.e. photo.jpg|c796ae4a-636f-4e03-9129-419ccdd47cdf?1234321

Not sure how you feel about such a solution, does it even make sense :) Whats your thoughts?

Chris

chrisjowen avatar Feb 03 '16 13:02 chrisjowen

@stavro So I had a stab at what I proposed, and it seems to work well for my use case. I'm hesitant to raise a PR both because my elixir skills suck and I was unsure if you even agreed with this idea. Anyway, for reference here are my forks:

https://github.com/stavro/arc_ecto/compare/master...chrisjowen:master https://github.com/stavro/arc/compare/master...chrisjowen:master

Summary is that Arc.File defstruct now has an identifier key. When its 'new' function is invoked it's given the value of a UUID. When Arc.Actions.Store.store is called it returns {file_name: xxx, identifier: xxx} instead of just the file name and arc_ecto uses this as I described before.

Chris

chrisjowen avatar Feb 05 '16 16:02 chrisjowen

@chrisjowen I added a uuid to the change set directly in my ecto model it works well enough. I just check to only set it if its not already set. https://github.com/digitalcake/digitalcakes_tmp/blob/master/web/models/post.ex#L27 https://github.com/digitalcake/digitalcakes_tmp/blob/master/web/uploaders/imagable.ex#L29

joshchernoff avatar Jul 09 '16 19:07 joshchernoff

@digitalcake cheers I thought of this in the first place. But I didn't like the idea of manually having to add a unique identifier to my model. If I added more files to the model I'd end up having several file identifiers properties. I just wanted this to be handled for me.

chrisjowen avatar Aug 21 '16 23:08 chrisjowen

@digitalcake I just went the same way and it worked perfectly. Thanks!

gmile avatar Aug 22 '16 04:08 gmile

@digitalcake do you still have those examples?

igorbelo avatar Jan 23 '17 23:01 igorbelo

@igorbelo Here is an example of model that works for me.

defmodule Hello.Client do
  use Hello.Web, :model
  use Arc.Ecto.Schema

  schema "clients" do
    field :name, :string
    field :age, :integer
    field :uuid, :string
    field :avatar, Hello.Avatar.Type

    timestamps()
  end

  @doc """
  Builds a changeset based on the `struct` and `params`.
  """
  def changeset(struct, params \\ %{}) do
    struct
    |> cast(params, [:name, :age, :uuid])
    |> check_uuid
    |> cast_attachments(params, [:avatar])
    |> validate_required([:name, :age])
  end

  defp check_uuid(changeset) do
    if get_field(changeset, :uuid) == nil do
      force_change(changeset, :uuid, UUID.uuid1)
    else
      changeset
    end
  end

end

And the Hello.Avatar module

defmodule Hello.Avatar do
  use Arc.Definition
  use Arc.Ecto.Definition

  def __storage, do: Arc.Storage.Local

  @versions [:original, :thumb]

  # Whitelist file extensions:
  # def validate({file, _}) do
  #   ~w(.jpg .jpeg .gif .png) |> Enum.member?(Path.extname(file.file_name))
  # end

  # Define a thumbnail transformation:
  def transform(:thumb, _) do
    {:convert, "-strip -thumbnail 100x100^ -gravity center -extent 100x100 -format png", :png}
  end

  # Override the persisted filenames:
  def filename(version, _) do
    version
  end

  # Override the storage directory:
  def storage_dir(version, {file, scope}) do
    if scope do
      "uploads/avatars/#{scope.uuid}"
    end
  end

  # Provide a default URL if there hasn't been a file uploaded
  # def default_url(version, scope) do
  #   "/images/avatars/default_#{version}.png"
  # end

  # Specify custom headers for s3 objects
  # Available options are [:cache_control, :content_disposition,
  #    :content_encoding, :content_length, :content_type,
  #    :expect, :expires, :storage_class, :website_redirect_location]
  #
  # def s3_object_headers(version, {file, scope}) do
  #   [content_type: Plug.MIME.path(file.file_name)]
  # end
end

Kurisu3 avatar Apr 07 '17 19:04 Kurisu3

For anyone who comes here finding that they can't replace their attachment in a changeset:

you'll get the previous attachment (you can access it in your changeset via changeset.data), delete it, and run another force_change(changeset, :uuid, UUID.uuid1) so that it knows it needs to upload this incoming file.

silentsilas avatar May 31 '19 20:05 silentsilas