-
Notifications
You must be signed in to change notification settings - Fork 50
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.
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
Not compatible with ActiveStorage #10
Comments
Oh that ActiveStorage... we use shrine for our applications, so I don't know if we add support for ActiveStorage in the nearest future. |
I'm running into this issue too. @bastengao have you found a solution? Also @fuelen, would you be open to PRs that add ActiveStorage compatibility? |
@awinograd yes, sure. |
@fuelen still trying to wrap my head around the code as I haven't worked with middlewares or Looks like it was introduced in awinograd@0a9e17d to solve a different but |
@awinograd without I think the only way for now to add support is to monkey-patch |
Ahh I see. I knew there must have been a reason but I had trouble figuring out what the history was around that change. I'll likely take some more time to look at this when I'm trying to productionize the system. Then I can take some time to dig into the ActiveStorage internals and see what fix might work. Thanks for the info! |
After digging through the initializer for
( Hope this helps with those who are struggling with ActiveStorage. Not sure if this the best way to handle it, but it might help to expose a helper method in |
I also hit another issue with this now, while using ActiveData which has type checks, and FWIW, it is possible to obtain the unwrapped object via |
I used @liang3404814 solution for own scalar to move the logic out of the mutation.
It would be good to support ActionStorage out of the box in the final version. I guess that just the key names are different. |
this means there is not support for active storage now. right? @fuelen and I didn't get code that you mentioned. do I need to write that code in resolver? |
Yes, no support for active storage. |
I have tried that also that did not work. @karensg and whenever we try to access file it is coming as null in variable like variables: {'id': 1, file: null} the how can we access args[:file] @liang3404814 |
@vimox-shah was file really sent? |
Do you have |
yes I have already integrated |
I have uploaded sample code here https://stackoverflow.com/questions/50705123/how-can-i-pass-multipart-form-data-and-file-upload-using-graphql-ruby-at-servers can you please check and verify is it right or wrong? @karensg |
yes file was really sent. can you please share your code how did you manage upload file? @karensg |
@vimox-shah could you show parameters from logs? |
yes you can see logs in attached screenshot @fuelen |
@bastengao besides doing all the basic project.file.attach(io: args[:file].to_io,
filename: args[:file].original_filename) |
thanks @djGrill, that worked for me! With or |
I think we can do something like this in
Could anyone test this approach? |
module ApolloUploadServer
class GraphQLDataBuilder
def assign_file(field, splited_path, file)
if field.is_a? Hash
field.merge!(splited_path.last => file)
elsif field.is_a? Array
field[splited_path.last.to_i] = file
end
end
end
remove_const :Upload
Upload = GraphQL::ScalarType.define do
name "Upload"
coerce_input ->(value, _ctx) { value }
coerce_result lambda { |value, _ctx|
return if value.nil?
def value.as_json(options = nil)
instance_values.except("tempfile").as_json(options)
end
value
}
end
end Putting this monkey patch in config/initializer worked for me, but is there any reason not making |
@yskkin We don't remember that reason. |
Just for sake of completeness: rails/rails#25250 |
I make a patch, unwrap ApolloUploadServer::Upload = ApolloUploadServer::Upload.redefine do
coerce_input ->(value, _ctx) do
value.is_a?(ApolloUploadServer::Wrappers::UploadedFile) ? value.__getobj__ : value
end
end |
Bump this because it's still an issue, and one that's particularly annoying to debug since the error message produced is mind-bendingly confusing (the wrapper object being transparent to all logging).
(Included above for searchability from Google.) We've worked around this temporarily by using: argument :image, ApolloUploadServer::Upload, required: false, prepare: -> (upload, _) {
upload&.__getobj__ # Unwrap value for ActiveStorage
} This is less than ideal however, since the argument needs to be prepared every time we accept a file. With ActiveStorage being the out-of-box file management solution for Rails, this issue is probably going to cause a lot of headaches. EDIT: Still an issue in 2024. As a recommendation for quality of life, it's possible to extend the GraphQL DSL of input objects to work around this, module Types
class BaseInputObject < GraphQL::Schema::InputObject
# ...
def self.upload_argument(name, *args, **options)
argument(name, ApolloUploadServer::Upload, *args, **options, prepare: -> (upload, _) {
upload&.__getobj__ # Unwrap value for ActiveStorage
})
end
end
end Which slightly cleans up the argument preparation to: upload_argument :image, required: false This bit is up to personal preference, of course. |
@mintyfresh Yeah, the error message is quite confusing and took me a while to identify the actual problem. In our new Rails projects, we've switched from Carrierwave to ActiveStorage. So far, we haven't found a straightforward solution to resolve it in an elegant manner. |
ActiveStorage accept
ActionDispatch::Http::UploadedFile
generally, see https://github.com/rails/rails/blob/master/activestorage/lib/active_storage/attached.rb#L18 , but uploaded file type isApolloUploadServer::Wrappers:: UploadedFile
, it is not accepted by ActiveStorage.The text was updated successfully, but these errors were encountered: