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.

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

Add content-type and filename params for file upload #47

Merged
merged 6 commits into from
Dec 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/channels.md
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,6 @@ require 'rocketchat'

rocket_server = RocketChat::Server.new('http://your.server.address/')
session = rocket_server.login('username', 'password')
session.channels.upload_file(room_id: 'GENERAL', file: File, msg: "Optional Message", description: "Optional Description", tmid: "Optional thread message id")
session.channels.upload_file(room_id: 'GENERAL', file: File, filename: "Optional. The name of the file to use.", content_type: "Optional. The content type of the uploaded file", msg: "Optional Message", description: "Optional Description", tmid: "Optional thread message id")

```
2 changes: 1 addition & 1 deletion docs/groups.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,6 @@ require 'rocketchat'

rocket_server = RocketChat::Server.new('http://your.server.address/')
session = rocket_server.login('username', 'password')
session.groups.upload_file(room_id: 'GENERAL', file: File, msg: "Optional Message", description: "Optional Description", tmid: "Optional thread message id")
session.groups.upload_file(room_id: 'GENERAL', file: File, filename: "Optional. The name of the file to use.", content_type: "Optional. The content type of the uploaded file", msg: "Optional Message", description: "Optional Description", tmid: "Optional thread message id")

```
14 changes: 11 additions & 3 deletions lib/rocket_chat/messages/room.rb
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ def upload_file(room_id:, file:, **rest_params)
response = session.request_json(
"#{API_PREFIX}/rooms.upload/#{room_id}",
method: :post,
form_data: file_upload_hash(file: file, **rest_params)
form_data: file_upload_array(file: file, **rest_params)
)

RocketChat::Message.new response['message'] if response['success']
Expand All @@ -345,9 +345,17 @@ def validate_attribute(attribute)
self.class.settable_attributes.include?(attribute)
end

def file_upload_hash(**params)
def file_upload_array(**params)
permited_keys_for_file_upload = %i[file msg description tmid]
Util.slice_hash(params, *permited_keys_for_file_upload)
hash = Util.slice_hash(params, *permited_keys_for_file_upload).compact

# NOTE: https://docs.ruby-lang.org/en/master/Net/HTTPHeader.html#method-i-set_form
file_options = params.slice(:filename, :content_type).compact
hash.map do |key, value|
next [key.to_s, value, file_options] if key == :file && file_options.keys.any?

[key.to_s, value]
end
end
end
end
Expand Down
3 changes: 1 addition & 2 deletions lib/rocket_chat/request_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def create_request(path, options)
req = Net::HTTP::Post.new(path, headers)
add_body(req, body) if body

form_data = reject_nils(options[:form_data])
form_data = options[:form_data]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would appear to be a breaking change. Can you explain why you've made it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it will be a breaking change and here is why:

  1. reject_nils expect hash, form data in this update will be array.
  2. form_data was added in my prev PR with file upload and wasn't supported previously in any requests. reject_nils for json-bodies are still at row 107
  3. form_data is added in lib/rocket_chat/messages/room.rb:322 by transforming provided hash to an array of arrays.
    3.1. If some params passed by user are nil they will be cleaned out by compact at lib/rocket_chat/messages/room.rb:350
    3.2. If file-related optional params (filename, content-type) are nil they will be cleared at lib/rocket_chat/messages/room.rb:353
    3.3. I don't see any possible way to receive an array with nil element at lib/rocket_chat/messages/room.rb:354 with 3.1. and 3.2.

On the other hand, for future development, part of file_upload_array at lines lib/rocket_chat/messages/room.rb:352-358 could be moved to request helper to cover all form_data requests from any future endpoints. I could do this in separate PR if this is ok

add_form_data(req, form_data) if form_data
else
uri = path
Expand All @@ -131,7 +131,6 @@ def add_body(request, body)
end

def add_form_data(request, form_data)
form_data = form_data.transform_keys(&:to_s) if form_data.is_a? Hash
request.set_form(form_data, 'multipart/form-data')
end

Expand Down
15 changes: 15 additions & 0 deletions spec/shared/room_behaviors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,21 @@
it { expect(upload).to be_a(RocketChat::Message) }
end

context 'with content_type params' do
subject(:upload) { scope.upload_file(room_id: room_id, file: file, content_type: content_type, **rest_params) }

let(:content_type) { 'image/png' }
let(:file) { File.open('spec/fixtures/files/image.png') }
let(:response) { png_upload_response(room_id: room_id) }

before do
stub_authed_request(:post, path).to_return(body: response, status: 200)
end

it { expect { upload }.not_to raise_error }
it { expect(upload).to be_a(RocketChat::Message) }
end

context 'when not accepted error is raised' do
before do
stub_authed_request(:post, path).to_return(body: response, status: 400)
Expand Down
Loading