shrinerb / shrine

File Attachment toolkit for Ruby applications
https://shrinerb.com
MIT License
3.18k stars 275 forks source link

RSpec Rails and Shrine test examples for requests don't work #557

Closed jahseng-lee closed 2 years ago

jahseng-lee commented 3 years ago

Brief Description

I've set up a test case to try to test my controller, which uploads a (nested) photo within the params. I use the given example in the docs and everything, which works for manually creating an upload in test but does not for sending it over a request.

Expected behavior

I would expect post some_path, params: { photo: { image: TestData.uploaded_image } } to work correctly

Actual behavior

JSON::ParserError:
    783: unexpected token at '#<Shrine::UploadedFile:0x000000013d791c90>'

So it looks like the post request sends though an "Uploaded File" as a string, as opposed to an object shrine can interpret.

Simplest self-contained example code to demonstrate issue

In my code:

# Controller
  def create
    # ...

    photo = Photo.create!(create_display_photo_params.delete(:photo)) # Error occurs here
    DisplayPhoto.create!(
      photo: photo,
      property: @property
    )
  end

  # ...

  def create_display_photo_params
    params.require(:display_photo).permit(
      photo: [:image],
    )
  end

# Model
  class Photo < ApplicationRecord
    include ImageUploader::Attachment(:image)
  end

In my request spec:

    describe "#create" do
      context "with valid params" do
        let(:valid_params) do
          {
            display_photo: {
              photo: {
                # I've also tried fixture_file_upload to a similar result FWIW
                image: TestImageData.uploaded_image # copy-paste of https://shrinerb.com/docs/testing#test-data
              }
            }
          }
        end

        it "creates a DisplayPhoto with a Photo attached" do
          expect{
            post property_display_photos_path(
              property_id: property_id,
              params: valid_params
            )
          }.to  change{ DisplayPhoto.count }.by(1)
           .and change{ Photo.count }.by(1)

          expect(response.status).to eq 201
          expect(DisplayPhoto.last.photo).to be_present
        end
      end
    end

System configuration

Ruby version: ruby 2.7.2p137 Rails 6.0.3.6

Shrine version: gem 'shrine', '~> 3.0'

benkoshy commented 3 years ago

I think i've faced a similar issue before: are you using a string or a jsonb column?

Would you able to try using attacher.column_data / or attacher.data according to your particular case?

image

jahseng-lee commented 3 years ago

I think i've faced a similar issue before: are you using a string or a jsonb column?

Would you able to try using attacher.column_data / or attacher.data according to your particular case?

Same behaviour unfortunately 🙁

FWIW I'm using a string in the db, not JSONB

benkoshy commented 3 years ago

I have reproduced the problem here: https://github.com/benkoshy/debugging-shrine-example/tree/photo-test-data-problem

Note: please change the path of the Shrine gemfile to use github, rather than a local repository.

It is perhaps best answered by the maintainers.

janko commented 3 years ago

Sending a Shrine::UploadedFile object in request params will cause the string representation of the object to be sent. You want to send a JSON string or a hash instead, which you can do by calling .to_json or .data on the uploaded file object.

jahseng-lee commented 3 years ago

Sending a Shrine::UploadedFile object in request params will cause the string representation of the object to be sent. You want to send a JSON string or a hash instead, which you can do by calling .to_json or .data on the uploaded file object.

Thanks! This worked... sorta.

So the key parts are: Updating my params to return #data:

        let(:valid_params) do
          {
            display_photo: {
              photo: {
                image: TestImageData.uploaded_image.data # updated this to use data now
              }
            }
          }
        end

Updating uploaded_file in the Shrine example to use :cache:

module TestData
  # ...

  def uploaded_image
    # ...

    # NOTE: VERY important you use `:cache` here. Shrine 3.0 does NOT support `:storage` option
    #             see: https://github.com/shrinerb/shrine/blob/master/doc/upgrading_to_3.md#assigning
    uploaded_file = Shrine.upload(file, :cache, metadata: false)

Updating photo_params to use nested attributes:

  # NOTE: https://shrinerb.com/docs/getting-started#tab-group-13-content-14
  #             DOESN'T work. display_photo_params[:photo] seems to return `{}`.
  def display_photo_params
    params.require(:display_photo).permit(
      photo: [
        image: [
          :id,
          :storage,
          metadata: [
            :filename,
            :mime_type,
            :size,
          ]
        ]
      ],
    )
  end

I am wondering now, given my examples above, if a maintainer wants to create more generic versions of said code snippets and update the docs?

janko commented 3 years ago

Yes, it would definitely be good for the docs to show testing attaching cached files. I will try to get to it at some point, but I would also appreciate a PR 🙂

jahseng-lee commented 3 years ago

@janko I won't get around to it today but I'll keep this tab open and see if I have some time in the near future 🙂