From e54827dbda33d170432c55813122ebc85bda78a9 Mon Sep 17 00:00:00 2001 From: Chris Gunther Date: Tue, 19 Oct 2021 17:18:07 -0400 Subject: [PATCH] Fix "undefined method `[]' for #" when adding File When calling `File#add`, the response XML sets a `type="file"` attribute on the `baseRef` element, where "file" corresponds to the NetSuite record. This then causes Nori (the XML parser used by Savon, the SOAP client) to interpret that element as representing a base64 encoded file, so it tries to get fancy about how it parses that element into a hash by returning an instance of it's `StringIOFile` class: https://github.com/savonrb/nori/blob/f59f8e18b0e53285c87d75a635058745e66b0bfb/lib/nori/xml_utility_node.rb#L131-L136 Either NetSuite's doing something non-standard with it's use of the `type` attribute that Nori is trying to enforce, or Nori is over-aggressive in trying to typecast to aid the developer. The end result was that when we tried to extract the `internal_id` from the response, the `body` was actually an instance of `StringIOFile`, not a hash: https://github.com/NetSweet/netsuite/blob/f0e46a076d0e7cb2abd5e9001ccbfd4bbb3d35c3/lib/netsuite/actions/add.rb#L80 To work around this, as I don't see a way to disable such behavior in Savon/Nori, if we detect the `baseRef` element was parsed to `StringIOFile`, we'll then take the raw XML and parse out the `baseRef` ourselves, returning a hash with the `internal_id` as we'd exect from non-`File` responses. I'm not thrilled with this solution. If we ever needed something else from the `baseRef` element, this effectively drops all other attributes for file records. It also introduces explicit references to `Nori` and `Nokogiri`, both of which are dependencies of Savon, but I'm not sure if that means this gem should list them as explicit dependencies to guard against Savon replacing them in a future update. Listing them as dependencies would then require keeping their version constraints in sync with Savon most likely. I believe this answers a question from #481: https://github.com/NetSweet/netsuite/pull/481#discussion_r674351298 However the fix in #481 solves it by not trying to extract the `internal_id`, which would create a problem if someone wanted to add a file then save the ID in their own database for future use. --- HISTORY.md | 2 +- lib/netsuite/actions/add.rb | 6 ++++- spec/netsuite/actions/add_spec.rb | 36 ++++++++++++++++++++++++++ spec/support/fixtures/add/add_file.xml | 20 ++++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 spec/support/fixtures/add/add_file.xml diff --git a/HISTORY.md b/HISTORY.md index ac317b1b2..a43e30dec 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -7,7 +7,7 @@ ### Fixed -* +* Fix "undefined method `[]` for #" when adding File (#495) * ## 0.8.9 diff --git a/lib/netsuite/actions/add.rb b/lib/netsuite/actions/add.rb index 4cc352661..8acc99e86 100644 --- a/lib/netsuite/actions/add.rb +++ b/lib/netsuite/actions/add.rb @@ -49,7 +49,11 @@ def success? end def response_body - @response_body ||= response_hash[:base_ref] + @response_body ||= if response_hash[:base_ref].is_a?(Nori::StringIOFile) + { :@internal_id => Nokogiri::XML(@response.to_s).remove_namespaces!.at_xpath('//baseRef')[:internalId] } + else + response_hash[:base_ref] + end end def response_errors diff --git a/spec/netsuite/actions/add_spec.rb b/spec/netsuite/actions/add_spec.rb index c55403c28..2be80d272 100644 --- a/spec/netsuite/actions/add_spec.rb +++ b/spec/netsuite/actions/add_spec.rb @@ -114,4 +114,40 @@ end end + context 'File' do + let(:file) do + NetSuite::Records::File.new(name: 'foo.pdf', content: 'abc123') + end + + context 'when successful' do + before do + savon.expects(:add).with(:message => { + 'platformMsgs:record' => { + :content! => { + 'fileCabinet:name' => 'foo.pdf', + 'fileCabinet:content' => 'abc123', + }, + '@xsi:type' => 'fileCabinet:File' + }, + }).returns(File.read('spec/support/fixtures/add/add_file.xml')) + end + + it 'makes a valid request to the NetSuite API' do + NetSuite::Actions::Add.call([file]) + end + + it 'returns a valid Response object' do + response = NetSuite::Actions::Add.call([file]) + expect(response).to be_kind_of(NetSuite::Response) + expect(response).to be_success + end + + it 'properly extracts internal ID from response' do + file.add + + expect(file.internal_id).to eq('23556') + end + end + end + end diff --git a/spec/support/fixtures/add/add_file.xml b/spec/support/fixtures/add/add_file.xml new file mode 100644 index 000000000..99c9962b5 --- /dev/null +++ b/spec/support/fixtures/add/add_file.xml @@ -0,0 +1,20 @@ + + + + + REDACTED + + + + + + + + false + + + + + + +