Conversation
| when :add | ||
| self.send(:include, NetSuite::Actions::Add::Support) | ||
| when :add | ||
| self.send(:include, NetSuite::Actions::AddList::Support) |
There was a problem hiding this comment.
@aom do we know that every record that supports add also supports add_list? My hunch would be this isn't the case, but I could be wrong here.
If every record does support add_list we should eliminate the double when :add and add add and add_list under the same when.
There was a problem hiding this comment.
This was not intentional. I'll change it to when :add_list.
AFAIK the addList is widely supported by all record types. At least I can't find any exceptions from NetSuite help section: https://system.eu2.netsuite.com/app/help/helpcenter.nl?fid=section_N3481360.html
There was a problem hiding this comment.
Ok, good to know! Awesome work on this PR. Excited to get it merged!
|
@aom Thanks for the contribution! Apologies for the delayed response. Left a comment on the PR, otherwise this looks awesome. |
| r.external_id == attr[:@external_id] | ||
| end | ||
|
|
||
| record.instance_variable_set('@internal_id', attr[:@internal_id]) |
There was a problem hiding this comment.
@aom what happens here if you don't set an externalID on the record you are running through addList? record would be nil and throw an exception, right?
Are the results returned by addList just a internal and external ID, or is the whole record returned?
Also feels weird to use instance_variable_set here, but without modifying every record in the gem there isn't a great alternative right now. The only thing I'd like to explore here is if we can use the results from addList to generate a new list of records to return. This would eliminate the find usage and instance_variable_set.
What do you think? I haven't worked with addList so I could be wrong here.
There was a problem hiding this comment.
@iloveitaly You might confusing AddList with UpsertList. You don't need to define externalId for AddList records because they are always added to NetSuite unless other validation errors occur. With UpsertList you'll need to define a unique externalId for new records or internalId for existing records.
Yes, unfortunately AddList returns just the reference to NetSuite Record with internalId and externalId. No luck there.
But you did point out a bug!
I'll include another test case without externalId. Because instance_variable_set will throw an exception if it is called for nil record. As I said above, the externalId is not mandatory for AddList records (I use it anyway on my queries so I didn't fall into this trap).
There was a problem hiding this comment.
I added a safe guard in case add_list is being called without external_id.
Update fork
iloveitaly
force-pushed
the
master
branch
2 times, most recently
from
7185ffc to
df796ee
Compare
|
@iloveitaly rebased this branch as well to resolve conflicts |
2 participants
NetSuite supports AddList operation so this'll add it into the gem.
You could do same with UpsertList with the exception that if you're using externalId's you will possibly mutate existing records. AddList will throw duplicate error instead.
I took the liberty to update NetSuite version numbers in fixtures related to this new spec.