Cld 1835 netsuite null fields #481

gmike11 · 2021-07-13T15:59:54Z

No description provided.

WHY: previously could not utilize soap action attach This adds the attach basic reference functionality into the gem

[CLOUD-737-attach-functionality]

Why: When adding a file from a workflow, even though it will sometimes be 'succuessful' it was still returning a 500 to the platform This change addresses the need by: made an update to the gem to return true/200 if a file is created and to return the file name if it is successfully created so we can query file by that attribute to create attachment Ticket [CLOUD-737]

[hotfix-CLOUD-737]

Why: * no nested if else statements! This change addresses the need by: * learned about try Ticket * [CLOUD-]

[HOTFIX-CLOUD-737-add-action-for-file]

*WHY The sales order record did not have the intercompany transaction field or the intercompany status field *This addresses the need by Adding those fields to the sales order record -caveat: I tried to test this with multiple payloads provided by the automation engineer but have yet to get a successful post. I have communicated the two errors we are getting, either a permission error or wrong enity for record depending on how the payload is formatted. The automation engineer is checking with customer, so I'm not sure if we want to wait to merge this in until the errors have been confirmed or not. [CLD-597]

Why: *The expense report object did not exist in the netsuite ruby gem or in our connector This change addresses the need by: *updating the gem and connector to include the expense report object. This ticket is dependent on the pr for branch cld-571 in the netsuite ruby connector Ticket * [CLD-571]

[CLD-597-intercompany-updates]

[CLD-571-expense-report]

*Why: Inventory subsidiary and inventory location were not present fields on the sales order line item *This change addresses the need by: Added those two fields

[CLD-659-sales-order-lines]

*Why: The vendor credit object in the gem previously had record reference fields defined differently than the majority of the other objects in the gem. Because of this, when we call '.record_refs' in the connector, it was erroring *This addresses the change by: Adding in the record ref support and redefined how the record reference fields are defined. [CLD-669]

[CLD-669-vendorcredit-record-refs]

Why: *The purchase order expenses were not supported within the gem, so they were being returned in the soap response but not in the json response returned by the gem. With this object missing, it would not have allowed posting a purchase order with an expense list either. This change addresses the need by: *Added the purchase order expense list record and the purchase order expense record Ticket * [CLD-725]

[CLD-725-expenses-on-purchaseorders]

Why: *There was not support for expense lines to show up on item receipts in Netsuite This change addresses the need by: *added in the item receipt expense list and item receipt expense models to the gem Ticket * [CLD-737]

[CLD-737-item-receipt-expenses]

Why: *expense categories were needed for posting an expense report This change addresses the need by: *adding in the expense category record to support searching for expense category Ticket * [CLD-756]

[CLD-756-Add-Expense-Categories]

*Why: We were experiencing 500s when making certain requests to netsuite and needed to accomodate them *How: Increased the timeout when configuring the savon client [CLD-882]

[CLD-882-large-response]

*Why: The gem previously supported versions up to 2018 but later versions were not supported *How: Updated the netsuite savon config to set the wsdl and the endpoint level [CLD-878]

[CLD-878 Update config for versions]

*Why: Earlier versions of netsuite did not have the field class on the vendor credit expense line level or the vendor credit item line level *This addresses the need by: Added the class field to both the vendor credit expense record class and the vendor credit item record class. Added the methods to those classes required for setting the record references appropriately [CLD-917]

[CLD-917-vendor-credit-klass]

update readme to include Cloudsnap additions

[L3-109] fix multiselect bug

added employee address records

added default address field on employee class

* netsuite gem lacked support for null_field_list This change addresses the need by: * Added null_field_lest record * added nullFieldList fields to applicable objects * Added check for nullFieldList element and code to swap namespace Ticket * [CLD-1835]

iloveitaly · 2021-07-19T14:04:22Z

@gmike11 there are some great changes here, mind submitting another PR?

gmike11 · 2021-07-19T20:55:29Z

Hi Michael, Sure, no problem. I'm sure you noticed that I only updated a few of the records files (just the ones we have needed for our project, so far). Thanks, Mike

…

________________________________ From: Michael Bianco ***@***.***> Sent: Monday, July 19, 2021 09:04 To: NetSweet/netsuite ***@***.***> Cc: Mike Good ***@***.***>; Mention ***@***.***> Subject: Re: [NetSweet/netsuite] Cld 1835 netsuite null fields (#481) @gmike11<https://github.com/gmike11> there are some great changes here, mind submitting another PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#481 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AEPALFTVUCZJ6X76XNFUSATTYQWHBANCNFSM5AJOGWSA>.

iloveitaly

Wow! This is a huge number of awesome changes. Thank you!

I'd love to get this merged, but I'd like to investigate some of the changes a bit more:

The regex logic on the update call
The errors returned on get
Modified logic on add
Configuration changes

Could you extract those changes into a separate PR and update this one?

Also, there are some merge conflicts in this branch—could you fix those as well?

iloveitaly · 2021-07-21T21:15:40Z

lib/netsuite/actions/add.rb

          @errors = response.errors

          if response.success?
-            @internal_id = response.body[:@internal_id]
+            @internal_id = response.body.dig(:@internal_id) unless response.body.class == Nori::StringIOFile


What is response.body.class == Nori::StringIOFile protecting against?

#495 likely adds some more context around where Nori::StringIOFile is coming from and attempts to address it in a way that still ensures the internal_id is extracted.

iloveitaly · 2021-07-21T21:16:23Z

lib/netsuite/actions/update.rb

+        if inner.keys.grep(/(.*):nullFieldList?/).any?
+          null_field_key = inner.keys.grep(/(.*):nullFieldList?/)
+          hash["platformMsgs:record"][:content!]["platformCore:nullFieldList"] = hash["platformMsgs:record"][:content!].delete null_field_key[0]
+        end


Could you explain what's going on here? Could you pull this into a separate PR?

I needed to extract the NullFieldList stuff and this part makes sure the updated_record.record_type isn't used for the nullFieldList

iloveitaly · 2021-07-21T21:17:19Z

lib/netsuite/records/custom_field_list.rb

+              if attrs[:value].is_a?(Array)
+                attrs[:value] = custom_field_data[:value].map { |entry| CustomRecordRef.new(entry) }
+              else
+                attrs[:value] = CustomRecordRef.new(custom_field_data[:value])


This was recently fixed: 3377c97

Can you remove this change from your PR?

iloveitaly · 2021-07-21T21:17:33Z

lib/netsuite/records/employee.rb

@@ -17,12 +17,14 @@ class Employee
             :bill_pay, :comments, :date_created, :direct_deposit, :entity_id, :password, :password2,
             :expense_limit, :fax, :is_job_resource, :job_description, :labor_cost, :last_modified_date, :mobile_phone, :pay_frequency,
             :phonetic_name, :purchase_order_approval_limit, :purchase_order_approver, :purchase_order_limit, :release_date,
-             :resident_status, :salutation, :social_security_number, :visa_exp_date, :visa_type
+             :resident_status, :salutation, :social_security_number, :visa_exp_date, :visa_type, :default_address


👍 on all of these missing fields!

iloveitaly · 2021-07-21T21:19:11Z

Gemfile

@@ -5,6 +5,7 @@ gem 'simplecov', :require => false

 gem 'pry-nav'
 gem 'pry-rescue'
+gem 'pry'


Why did you need to include this directly?

iloveitaly · 2021-07-21T21:19:29Z

README.md

+If you see the record in the netsuite schema browser, you can easily add it to the records in this gem.  Use the fields and sub lists mentioned in the Netsuite schema browser to build out the record file following the same format as the other record files.
+
+### Other Cloudsnap Changes
+We made a few changes to the configuration.rb file.  The check credentials method and set attributes methods were added to support the updated way of passing credentials.  In the connection method, we also added the ability to set the wsdl and the endpoint level for the Savon client that is configured.  This was required to connect to Netsuite api versions 2020+


After this is merged, I'll go ahead and edit these readme changes a bit to fit within the existing readme structure.

iloveitaly · 2021-07-21T21:23:37Z

lib/netsuite/actions/get.rb

+      def errors
+        error_obj = response_hash.dig(:status,:status_detail)
+        OpenStruct.new(status: 404, status_detail: error_obj)
+      end


Can we pull this logic into a separate PR?

iloveitaly · 2021-07-21T21:25:06Z

lib/netsuite/configuration.rb

@@ -15,6 +15,8 @@ def attributes
    end

    def connection(params={}, credentials={})
+      check_credentials(credentials)


Why were these changes required in your case? This smells like something specific to your application and not something we'd want to include in the netsuite gem.

iloveitaly · 2021-07-21T21:26:04Z

lib/netsuite/configuration.rb

@@ -27,10 +29,28 @@ def connection(params={}, credentials={})
        log_level: log_level,
        log: !silent, # turn off logging entirely if configured
      }.update(params))
+      client.wsdl.endpoint = client.wsdl.endpoint.to_s.sub('//webservices.netsuite.com/', "//#{wsdl_domain}/")


This should be fixed by this PR #473

iloveitaly · 2021-08-10T13:29:09Z

@gmike11 friendly reminder on this one! Would be great to handle some of my comments and get this merged.

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 NetSweet#481: NetSweet#481 (comment) However the fix in NetSweet#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.

#495) 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: #481 (comment) 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.

@gmike11

NullFieldList is used to either clear an existing value on update, or avoid a default being applied on adding a record. My use case only required support for invoices, so I started there, but this could likely be added to most records. This was heavily inspired by @gmike11's work in NetSweet#481, but that seems to lump a bunch of changes together. This also solves NetSweet#398. Where NetSweet#481 manipulated the XML request body to address that the `<platformCore:nullFieldList>` needs to always use the `platformCore` namespace, as opposed to the records normal namespace (ie `transSale` for invoice) as happens for any other field, I tried to address that on the `#to_record` side.

@gmike11

NullFieldList is used to either clear an existing value on update, or avoid a default being applied on adding a record. My use case only required support for invoices, so I started there, but this could likely be added to most records. This was heavily inspired by @gmike11's work in NetSweet#481, but that seems to lump a bunch of changes together. This also solves NetSweet#398. Where NetSweet#481 manipulated the XML request body to address that the `<platformCore:nullFieldList>` needs to always use the `platformCore` namespace, as opposed to the records normal namespace (ie `transSale` for invoice) as happens for any other field, I tried to address that on the `#to_record` side.

@gmike11

NullFieldList is used to either clear an existing value on update, or avoid a default being applied on adding a record. My use case only required support for credit memos and invoices, so I started there, but this could likely be added to most records. This was heavily inspired by @gmike11's work in NetSweet#481, but that seems to lump a bunch of changes together. This also solves NetSweet#398. Where NetSweet#481 manipulated the XML request body to address that the `<platformCore:nullFieldList>` needs to always use the `platformCore` namespace, as opposed to the record's normal namespace (ie `transSale` for invoice) as happens for any other field, I tried to address that on the `#to_record` side.

@gmike11

NullFieldList is used to either clear an existing value on update, or avoid a default being applied on adding a record. My use case only required support for credit memos and invoices, so I started there, but this could likely be added to most records. This was heavily inspired by @gmike11's work in NetSweet#481, but that seems to lump a bunch of changes together. This also solves NetSweet#398. Where NetSweet#481 manipulated the XML request body to address that the `<platformCore:nullFieldList>` needs to always use the `platformCore` namespace, as opposed to the record's normal namespace (ie `transSale` for invoice) as happens for any other field, I tried to address that on the `#to_record` side.

@gmike11

…s) (#529) NullFieldList is used to either clear an existing value on update, or avoid a default being applied on adding a record. My use case only required support for credit memos and invoices, so I started there, but this could likely be added to most records. This was heavily inspired by @gmike11's work in #481, but that seems to lump a bunch of changes together. This also solves #398. Where #481 manipulated the XML request body to address that the `<platformCore:nullFieldList>` needs to always use the `platformCore` namespace, as opposed to the record's normal namespace (ie `transSale` for invoice) as happens for any other field, I tried to address that on the `#to_record` side.

reginad1 added 30 commits November 8, 2019 14:11

[CLOUD-737-attach-functionality]

cf1338c

WHY: previously could not utilize soap action attach This adds the attach basic reference functionality into the gem

Merge pull request #1 from CloudsnapInc/add-attach-functionlity

55bea99

[CLOUD-737-attach-functionality]

Merge pull request #2 from CloudsnapInc/fix-add-action-file

633895a

[hotfix-CLOUD-737]

[HOTFIX-CLOUD-737-add-action-for-file]

f4e02c9

Why: * no nested if else statements! This change addresses the need by: * learned about try Ticket * [CLOUD-]

Merge pull request #3 from CloudsnapInc/add-action-for-file

247a8db

[HOTFIX-CLOUD-737-add-action-for-file]

Merge pull request #4 from CloudsnapInc/CLD-597-sales-order

3d77d54

[CLD-597-intercompany-updates]

Merge pull request #5 from CloudsnapInc/CLD-571-expense-report

656ca69

[CLD-571-expense-report]

[CLD-659-sales-order-lines]

439e871

*Why: Inventory subsidiary and inventory location were not present fields on the sales order line item *This change addresses the need by: Added those two fields

Merge pull request #6 from CloudsnapInc/CLD-619-sales-order-item

3ef6d6c

[CLD-659-sales-order-lines]

Merge pull request #7 from CloudsnapInc/CLD-669-vendorcredit

fe96eff

[CLD-669-vendorcredit-record-refs]

Merge pull request #8 from CloudsnapInc/CLD-725-purchase-order-expense

f9b05f7

[CLD-725-expenses-on-purchaseorders]

[CLD-737-item-receipt-expenses]

7fbe5b2

Why: *There was not support for expense lines to show up on item receipts in Netsuite This change addresses the need by: *added in the item receipt expense list and item receipt expense models to the gem Ticket * [CLD-737]

Merge pull request #9 from CloudsnapInc/CLD-737-expense-item-receipt

bc79ecf

[CLD-737-item-receipt-expenses]

[CLD-756-Add-Expense-Categories]

0423790

Why: *expense categories were needed for posting an expense report This change addresses the need by: *adding in the expense category record to support searching for expense category Ticket * [CLD-756]

Merge pull request #10 from CloudsnapInc/CLD-756-Add-Expense-Category

99b040d

[CLD-756-Add-Expense-Categories]

[CLD-882-large-response]

3a3c747

*Why: We were experiencing 500s when making certain requests to netsuite and needed to accomodate them *How: Increased the timeout when configuring the savon client [CLD-882]

pry

811e421

remove pry

3d0f984

pry

1a2045e

remove pry

e188a5d

Merge pull request #11 from CloudsnapInc/increase-timeout

28125a4

[CLD-882-large-response]

[CLD-878-support-later-version]

e20132a

*Why: The gem previously supported versions up to 2018 but later versions were not supported *How: Updated the netsuite savon config to set the wsdl and the endpoint level [CLD-878]

Merge pull request #12 from CloudsnapInc/update-config-for-versions

16371f7

[CLD-878 Update config for versions]

Merge pull request #13 from CloudsnapInc/cld-917-class-at-vendorcredit

7db5d5d

[CLD-917-vendor-credit-klass]

reginad1 and others added 11 commits January 22, 2021 11:49

update readme to include Cloudsnap additions

cdc3e50

Merge pull request #21 from CloudsnapInc/update-readme

d74d3be

update readme to include Cloudsnap additions

fix multiselect bug

67a0215

Merge pull request #23 from CloudsnapInc/multi_select_support

a4460fe

[L3-109] fix multiselect bug

added employee address records

4f0cc2c

Merge pull request #24 from CloudsnapInc/employee_address_support

1776652

added employee address records

added default address field on employee class

1ac62f8

Merge pull request #25 from CloudsnapInc/L3-199_adding_default_address

f7fd347

added default address field on employee class

Initial commit WIP

6227de5

removed commented fields, using NullFieldList class

d8ff880

Why:

bb7f976

* netsuite gem lacked support for null_field_list This change addresses the need by: * Added null_field_lest record * added nullFieldList fields to applicable objects * Added check for nullFieldList element and code to swap namespace Ticket * [CLD-1835]

gmike11 closed this Jul 13, 2021

gmike11 reopened this Jul 21, 2021

iloveitaly requested changes Jul 21, 2021

View reviewed changes

cgunther mentioned this pull request Oct 19, 2021

Fix "undefined method `[]' for #<Nori::StringIOFile>" when adding File #495

Merged

cgunther mentioned this pull request Apr 6, 2022

Add support for NullFieldList (starting with credit memos and invoices) #529

Merged

gmike11 closed this Apr 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cld 1835 netsuite null fields #481

Cld 1835 netsuite null fields #481

gmike11 commented Jul 13, 2021

iloveitaly commented Jul 19, 2021

gmike11 commented Jul 19, 2021 via email

iloveitaly left a comment •

edited

Loading

iloveitaly Jul 21, 2021

cgunther Oct 19, 2021

iloveitaly Jul 21, 2021

mcfearsome Dec 2, 2021

iloveitaly Jul 21, 2021

iloveitaly Jul 21, 2021

iloveitaly Jul 21, 2021

iloveitaly Jul 21, 2021

iloveitaly Jul 21, 2021

iloveitaly Jul 21, 2021

iloveitaly Jul 21, 2021

iloveitaly commented Aug 10, 2021

Cld 1835 netsuite null fields #481

Cld 1835 netsuite null fields #481

Conversation

gmike11 commented Jul 13, 2021

iloveitaly commented Jul 19, 2021

gmike11 commented Jul 19, 2021 via email

iloveitaly left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iloveitaly commented Aug 10, 2021

iloveitaly left a comment •

edited

Loading