Skip to content

Conversation

@pospischil
Copy link
Contributor

No description provided.

…ult set (look for an array, and if it's not one make it one)
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious... why was this needed? What situation broke the existing implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When you have more than one page of items and the last page has only a single item.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh, got it. Nice catch! Merged this into master.

@iloveitaly
Copy link
Member

Thanks for the PR!

I was considering removing the static WSDLs from the gem completely. Do you find the static WSDLs useful? Why do you find using them helpful as opposed to just pointing to the NS hosted WSDLs?

@pospischil
Copy link
Contributor Author

Not really (honestly, I don't even know how they are used). Just included
it because I was in there anyway and was using the newer endpoint.

On Thu, Jun 19, 2014 at 7:33 PM, Michael Bianco [email protected]
wrote:

Thanks for the PR!

I was considering removing the static WSDLs from the gem completely. Do
you find the static WSDLs useful? Why do you find using them helpful as
opposed to just pointing to the NS hosted WSDLs?


Reply to this email directly or view it on GitHub
#82 (comment)
.

Ensure that response.body has :search_row_list AND that the value is not itself null
@pospischil
Copy link
Contributor Author

any intention of merging this? Happy to make any changes, just let me know.

@iloveitaly
Copy link
Member

Could you remove the 2014_1 WSDL?

I'd like to get this merged, just haven't had enough time to take a deeper look. Hopefully I'll have time soon!

@pospischil
Copy link
Contributor Author

Done!

I also:

  • added some null protection to fix some issues we ran into
  • added some new attributes to sales_order

Let me know if you have questions, would love to have this merged in

Copy link
Member

Choose a reason for hiding this comment

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

Why were these additional SSL configuration options needed? I'm hesitant to merge them in unless there is a strong general use case.

@iloveitaly
Copy link
Member

Whoa, sorry about the delay on this!

Merged in some of the commits here with 5faab91...64e8a17

Couple other notes:

diegopolido pushed a commit to penrosehill/netsuite that referenced this pull request Oct 7, 2021
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.

3 participants