search

ruckus / quickbooks-ruby

Quickbooks Online REST API V3 - Ruby
MIT License
374 stars 302 forks source link

URI.unescape is obsolete in roxml gem. #548

Closed joshuapinter closed 3 years ago

joshuapinter commented 3 years ago

Seeing lots and lots of URI.unescape is obsolete warnings when doing anything. 

/Users/me/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/roxml-4.1.1/lib/roxml/xml/references.rb:50: warning: URI.escape is obsolete
/Users/me/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/roxml-4.1.1/lib/roxml/xml/references.rb:50: warning: URI.unescape is obsolete
/Users/me/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/roxml-4.1.1/lib/roxml/xml/references.rb:50: warning: URI.escape is obsolete
/Users/me/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/roxml-4.1.1/lib/roxml/xml/references.rb:50: warning: URI.unescape is obsolete
/Users/me/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/roxml-4.1.1/lib/roxml/xml/references.rb:50: warning: URI.escape is obsolete
/Users/me/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/roxml-4.1.1/lib/roxml/xml/references.rb:50: warning: URI.unescape is obsolete
/Users/me/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/roxml-4.1.1/lib/roxml/xml/references.rb:50: warning: URI.escape is obsolete
/Users/me/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/roxml-4.1.1/lib/roxml/xml/references.rb:50: warning: URI.unescape is obsolete

It looks like it's an issue with the roxml gem and Ruby 2.7 deprecations:

https://github.com/Empact/roxml/issues/71

Doesn't look like a fix is merged in yet but I wanted to add this here so it's on your radar and when an update is merged, we can get an updated version cut with high minimum requirements of the roxml gem. Maybe something like '~> 4.1' or whatever they come out with.

Thanks for all your effort with this library!

parkerleblanc01 commented 3 years ago

I believe this is related to the following error I'm getting with Ruby 3.0.1: undefined method 'escape' for URI:Module

Looks like it will be a required fix for Ruby 3.0.1 support

anthonyhamou commented 3 years ago

Same problem here for a Ruby 3.0.1 version ... WARN: Quickbooks::IntuitRequestException: Error parsing XML: undefined method `escape' for URI:Module

WARN: /app/vendor/bundle/ruby/3.0.0/gems/quickbooks-ruby-1.0.17/lib/quickbooks/service/base_service.rb:160:in `rescue in parse_collection'

jacobjlevine commented 3 years ago

Hey folks! I just got in touch with the creator of the roxml gem, and he's open to giving an additional contributor admin permissions for the gem. Since this is one of the biggest downstream projects I know of, I was thinking maybe there's a developer at Intuit - or at least someone with a vested interest in the success of the quickbooks-ruby gem - who'd be open to taking on that responsibility. Any takers?

joshuapinter commented 3 years ago

@jacobjlevine Probably best to ask this on the roxml gem repo instead of here. People there would be more familiar with the library and open to taking it on.

jacobjlevine commented 3 years ago

@joshuapinter fair enough. I'll post there as well.

ruckus commented 3 years ago

Thanks @jacobjlevine and @joshuapinter @anthonyhamou . It does feel sketchy as f**k to be dependent on another gem to do the heavy lifting. At the time it made total sense as Intuit/QBO API was XML-based. There was a time back in the day when QB Desktop users had a client-side app that pushed all data to the cloud and there was a single "zone" of input/output. Which was jacked and they fell back to the Web Connector. And also pushed everyone to move to QB Online. Which is still not feature complete (the biggest issue is lack of multiple inventory locations).

Anyways I asked Intuit Dev if they were ever going to deprecate XML. They push JSON and all docs are JSON focused, and they said no; they have no plans to deprecate XML. But I feel uneasy, all docs are in JSON.

In hindsight I should not have relied on roxml gem and should have implemented the heavy lifting natively. But that gem rocks and did so much, so seemed valid at the time. Whatcha going to do?

anthonyhamou commented 3 years ago

Hi Guys ! It is so good to hear from you @ruckus @joshuapinter @jacobjlevine I am so dependent of that gem quickbooks-ruby !! Everyday my clients ask me where to find their monthly invoice in my platform and I can not explain them the problem I agree with @joshuapinter we need to ask to the roxml team to update their gem in order to go forward with this one so useful and badass I am not used with that kind of stuff. Do you all agree with this needing and who can care of it ?

jacobjlevine commented 3 years ago

@ruckus do you work for Intuit? Or did you develop this gem externally? And do you have any interest in being a direct contributor to the roxml gem to make sure it doesn't end up bottlenecking progress for quickbooks-ruby in the future? The creator of roxml is open to bringing on additional administrators moving forward. You can see the current conversation here: https://github.com/Empact/roxml/issues/71

ruckus commented 3 years ago

Hi @jacobjlevine no, I don't work for Intuit and I developed this gem on my own because my own SaaS needed to integrate with QBO back in 2011 and there was nothing else available.

And do you have any interest in being a direct contributor to the roxml

To be super honest, I don't. Largely because I just don't have the time to take on that additional responsibility.

But I do recognize that this gem relies heavily on roxml and this URI issue is causing this issue downstream. (in hindsight I wish I had gone the JSON route and this would never have been an issue at all and quickbooks-ruby would never have used roxml in the first place. Ah 20/20 hindsight).

It does sound like two others have volunteered so that's super positive. Extreme worse-case scenario if Empact cannot provide a way forward is to fork that repo/gem.

anthonyhamou commented 3 years ago

Hi guys ! It seems that Empact/roxml#77 has fixed the warning of deprecation URI.escape and URI.unescape for Ruby > 2.7 So what is the next step for us now ?

ruckus commented 3 years ago

@anthonyhamou next steps are to get Empact/roxml to cut a gem release that we can include our gemspec.

anthonyhamou commented 3 years ago

Hi @ruckus if I'm not mistaken It seems to me that Empact has fixed the roxml gem. So can you help us to go forward please ?

ruckus commented 3 years ago

@anthonyhamou Team ROXML needs to push an updated gem release to RubyGems that we can then reference in this gemspec. The latest version of roxml is 4.1.1 from July 6, 2020

https://rubygems.org/gems/roxml

anthonyhamou commented 3 years ago

Hi all Hi @ruckus Goods news ! The gem release 4.2.0 Roxml - September 09, 2021 has just got out ! https://rubygems.org/gems/roxml

safeforge commented 3 years ago

bundle update roxml works for me.

ruckus commented 3 years ago

@anthonyhamou great, I just pushed 1.0.18 with the gemspec updated 

  gem.add_dependency 'roxml', '~> 4.2'
joshuapinter commented 3 years ago

Great job everyone! And thanks @ruckus for cutting a new gem so quickly. 👏 👏 👏