jondkinney/docusign_rest

docusign_rest breaks with multipart-post 2.0.0

Closed this issue · 4 comments

As the title says, we upgraded multipart-post gem to 2.0.0 and now get an Argument Error (4 for 3) when using the create_envelope_from_document method (only one tested). Downgrading to multipart-post 1.2.0 seems to fix the issue. Has anyone else had this problem?

Hi @davenguyen,

It looks like I specified:

gem.add_dependency('multipart-post', '>= 1.1.5')

Which is probably a mistake, since it's letting you update to 2.0 when I guess it's incompatible with the current code in the gem. I'm using multipart-post up to 1.2 in my current app that uses this gem, I haven't tried to go any further than that, but I don't think it'll be too tough to figure out what is going on and make it compatible. I should probably lock the gemspec to ~>1.2 for now until I can investigate further though.

Also, I still need to release a new version of this gem to rubygems. I'm referencing this repo's master in my gemfile for my current project. 👎

Thanks for the bug report. I'll try to get to it this week. In the mean time, stick to multipart-post 1.2 if you can.

Thanks!

So it looks like the new multipart-post gem now allows the content-type to be set dynamically via an options hash. YAY! This was the only reason I had to monkey patch it before. Here's the new code in multipart-post 2.0.

def build_part(boundary, name, value, headers = {})
  part = ''
  part << "--#{boundary}\r\n"
  part << "Content-Disposition: form-data; name=\"#{name.to_s}\"\r\n"
  part << "Content-Type: #{headers["Content-Type"]}\r\n" if headers["Content-Type"]
  part << "\r\n"
  part << "#{value}\r\n"
end

Versus my monkey patch which just slams the content-type right in there...also note the lack of the 4th headers param, which is now required in 2.0.

def build_part(boundary, name, value)
  part = ''
  part << "--#{boundary}\r\n"
  part << "Content-Disposition: form-data; name=\"#{name.to_s}\"\r\n"
  part << "Content-Type: application/json\r\n" #Add the content type which isn't present in the multipart-post gem, but DocuSign requires
  part << "\r\n"
  part << "#{value}\r\n"
end

However, I am as of yet unable to figure out how to actually tap into that headers hash and pass along the content-type header. I did update the monkeypatch to take that additional parameter though, so the gem should now be compatible with multipart-post 2.0.

I'm going to have another look at this soon, and I know right where in the code I need to figure out what is going on, I'm just not quite sure the best way to decipher it.

See here: https://github.com/nicksieger/multipart-post/blob/master/lib/multipartable.rb#L12

So that's looking for a :parts key to a hash in the headers. I was messing with passing in:

parts: {'Content-Type' => 'application/json'}

Since that seems to be the only way to get parts_headers actually populated. But then when it iterates through the params and tries to index into the parts_headers hash based on the key of the param and that's where I get stuck. I need to get the content-type header through to the Part.new constructor but I'm not following that code fully.

If you have any insight let me know. Thanks.

Turns out I was able to support both multipart-post 1.2 and 2.0 by just adding that optional 4th headers hash argument. So I've left the gemspec capable of upgrading beyond 1.2 to 2.0, but I still need to support 1.2 for my app. We're using another gem that requires multipart-post version 1.2 max, so couldn't abandon 1.2 support for docusign_rest just yet. Luckily there is no need!

I also pushed a new version, 0.1.1, of the gem to rubygems.