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

Support non-record field results in saved search responses #426

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

Conversation

davidlaprade
Copy link
Contributor

@davidlaprade davidlaprade commented Mar 20, 2019

TL;DR -- This exposes some additional search result information that is currently being dropped


Sometimes NS makes information available within saved search results that are not record fields. For example, you can run an InventoryItem search through the GUI that returns:

  • Location Available (the quantity available at the default location)
  • Location Reorder Point (quantity at which you should reorder at the location)
  • Location On Order (quantity on order for the location)

image

These aren't record fields, as you can see with a quick CTRL-f through the NS schema docs. There's no field named locationQuantityAvailable in the schema -- even though one comes back in the XML response to the saved search:

<platformCommon:locationQuantityAvailable>
  <platformCore:searchValue>2.0</platformCore:searchValue>
  <platformCore:customLabel>Qty - Available</platformCore:customLabel>
</platformCommon:locationQuantityAvailable>

They also aren't custom Formula results either (which NS doesn't seem to expose through Suitetalk).

Nor are they custom fields. This information is just default, standard-issue NetSuite.

For that reason, I think it's perfectly reasonable to have a search that returns them as results, and thus also reasonable to expect to be able to access them through the API.

I have a need to support them in my own projects, and think others would benefit from having access to them as well.

EDIT: tests pass for me locally
image

The "records" in the search group are hashes with "keys" that are the
attribute names and "values" that the data returned by the search for
those attributes (not necessarily the attribute values, though). For
example, you'll often see things like this:

```
{
  :department=>{
    :search_value=>{:@internal_id=>"113"},
    :custom_label=>"Business Unit"
  }
}
```

I think using `attr_name` and `search_result` instead of `k` and `v`,
respectively, makes it clearer what the parts of this data constitute.
Sometimes NS makes information available within saved search results
that are not record fields. For example, you can run an InventoryItem
search through the GUI that returns:

* `Location Available` (the quantity available at the default location)
* `Location Reorder Point` (quantity at which you should reorder)
* `Location On Order` (quantity on order for the location)

These are not record fields, as you can see with a quick CTRL-f through
the NS schema docs:

http://www.netsuite.com/help/helpcenter/en_US/srbrowser/Browser2018_2/schema/record/inventoryitem.html

They aren't custom formula results either (which NS doesn't seem to expose
through the API).

And they also aren't _custom fields_. This information is just default,
standard-issue NetSuite. It's perfectly reasonable to have a search that
returns them as results, and thus also reasonable to expect to be able
to access them through the API.
@iloveitaly iloveitaly force-pushed the master branch 2 times, most recently from 7185ffc to df796ee Compare May 28, 2019 15:55
@cgunther
Copy link
Contributor

I hit a similar problem on my integration, though not via a saved search, just a basic search through the API.

Searching invoices, TransactionSearchRowBasic has a field closeDate, however Invoice doesn't, so searching via this gem essentially drops the column since it's not a field defined on the record class.

This PR seems to be a step towards making these search-only fields accessible, albeit faking them as custom fields, though I hit a few snags using it.

First, if the search is returning internalId (or probably externalId too), this PR seems to treat them as custom fields. That's likely because this gem doesn't define these as fields, so when this PR compares the returned columns to fields, it doesn't see a match, thus assuming they're "custom fields":
davidlaprade@bb09391#diff-68d7940fabf39ec307b04a06b31bc6faea22cf6405ffb43594618640c04e79dcR78

That line likely needs to check if attr_name is internal_id (or external_id) too.

Secondly, this PR seems to store the field name as internal_id of the custom field:
davidlaprade@bb09391#diff-68d7940fabf39ec307b04a06b31bc6faea22cf6405ffb43594618640c04e79dcR88

But for newer integrations (2013.2 and newer?), I believe custom fields should be referenced by script_id, based on:

@reference_id_type ||= Configuration.api_version >= '2013_2' ? :script_id : :internal_id

Therefore, where I'm using 2020.2, if I try using this PR, the custom fields returned have a nil script_id but the internal_id matches the field name (:close_date), but because of my version, the CustomFieldList is assuming it'll receive custom fields with a script_id set, not internal_id:

if reference_id = custom_field.send(reference_id_type)
@custom_fields_assoc[reference_id.to_sym] = custom_field
end

Perhaps this PR needs to conditionally set the field name to either script_id or internal_id based on the version used?

Alternatively, I wonder if it'd be better to model the search results as proper classes where you could define the fields each has, as opposed to an approach like this. I guess it'd come down to whether you'd expect to get instances of the record (ie. Invoice) back from a search, or instances of a search result (ie. TransactionSearchRowBasic) back. The former certainly makes it easier to then push changes back to NetSuite by manipulating the returned instance.

@iloveitaly
Copy link
Member

@cgunther if you can rebase this PR and fix it up to work with the conditions you mentioned, I can take a look at getting this merged.

@iloveitaly
Copy link
Member

Pretty sure this is handled by:

Still need to double check and ensure there's nothing unique in this PR that needs to be included.

@cgunther
Copy link
Contributor

I'd agree, this is predominantly addressed by 676a618.

cgunther added a commit to cgunther/netsuite that referenced this pull request Aug 11, 2021
Attempt to document NetSweet#483.

I wasn't sure how best to convey this given it affects consuming the search results, not performing the search itself. It should also be unsurprising that you'd access these search-only fields the same way you'd access regular or read-only fields.

Had we buried these search-only fields in custom fields, like NetSweet#426 first attempted, then I'd find that more surprising and warranting clearer documentation.
iloveitaly pushed a commit that referenced this pull request Aug 11, 2021
Attempt to document #483.

I wasn't sure how best to convey this given it affects consuming the search results, not performing the search itself. It should also be unsurprising that you'd access these search-only fields the same way you'd access regular or read-only fields.

Had we buried these search-only fields in custom fields, like #426 first attempted, then I'd find that more surprising and warranting clearer documentation.
diegopolido pushed a commit to penrosehill/netsuite that referenced this pull request Oct 7, 2021
Attempt to document NetSweet#483.

I wasn't sure how best to convey this given it affects consuming the search results, not performing the search itself. It should also be unsurprising that you'd access these search-only fields the same way you'd access regular or read-only fields.

Had we buried these search-only fields in custom fields, like NetSweet#426 first attempted, then I'd find that more surprising and warranting clearer documentation.
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