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

Global minorversion #518

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Global minorversion #518

wants to merge 6 commits into from

Conversation

ruckus
Copy link
Owner

@ruckus ruckus commented May 8, 2020

No description provided.

lib/quickbooks/service/preferences.rb Outdated Show resolved Hide resolved
lib/quickbooks/service/item.rb Outdated Show resolved Hide resolved
lib/quickbooks/service/account.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@drewish drewish left a comment

Choose a reason for hiding this comment

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

Oh noticed a few more we can whack

@@ -9,17 +9,16 @@ def delete(customer)

def url_for_resource(resource)
url = super(resource)
"#{url}?minorversion=#{Quickbooks::Model::Customer::MINORVERSION}"
url
end
Copy link
Contributor

Choose a reason for hiding this comment

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

this one seems like it could go too right?

@@ -7,17 +7,16 @@ def delete(invoice)
end

def url_for_resource(resource)
url = super(resource)
super(resource)
Copy link
Contributor

Choose a reason for hiding this comment

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

This too?

fetch_object(model, url, params)
end

def url_for_query(query = nil, start_position = 1, max_results = 20, options = {})
url = super(query, start_position, max_results, options)
"#{url}&minorversion=#{Quickbooks::Model::Invoice::MINORVERSION}"
super(query, start_position, max_results, options)
Copy link
Contributor

Choose a reason for hiding this comment

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

and this?

fetch_object(model, url, params)
end

def url_for_query(query = nil, start_position = 1, max_results = 20, options = {})
url = super(query, start_position, max_results, options)
"#{url}&minorversion=#{Quickbooks::Model::Customer::MINORVERSION}"
super(query, start_position, max_results, options)
Copy link
Contributor

Choose a reason for hiding this comment

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

this too?

@drewish
Copy link
Contributor

drewish commented May 12, 2020

Do we have to worry about setting the global version introducing breaking changes? Like if I'm upgrading do I want to lock this at a lower version?

@ruckus
Copy link
Owner Author

ruckus commented May 12, 2020

@drewish

Do we have to worry about setting the global version introducing breaking changes? Like if I'm upgrading do I want to lock this at a lower version?

Good question, I wondered about that too. From what I can tell QBO uses minorversions on data reads, potentially returning more info than before. I have yet to see if alter the format / payload depending on a version change. In other words, bumping the minorversion tells QBO to return additional data than before (e.g. Customer#customer_type_ref). I dont think its used on writes.

Thus, I think it should be safe to use and not introduce backwards incompatibilities?

@drewish
Copy link
Contributor

drewish commented May 15, 2020

That makes sense. It kind of an interesting versioning strategy if only adds additions. Typically versions are used to introduce breaking changes... so glad they haven't had to do that I guess?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants