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

Add search only fields to CreditMemo, PurchaseOrder, and WorkOrder #565

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cgunther
Copy link
Contributor

Both Invoice and CreditMemo records (as well as many other records) use the same search fields for the returned basic results, TransactionSearchRowBasic, so rather than repeat the fields for each record, extract them to a common module that can be shared between the respective records.

Since the search module may cover multiple records, it's fields will overlap with some of the standard fields on each record, so defining search only fields is now a no-op if the search only field already exists on the record as a standard field (rather than erroring). Previously for invoices, we defined search only fields as only the fields that didn't otherwise exist as standard fields, but now we need to include those standard fields potentially as search only as well as they may not be standard fields on another record that uses the same module. For this reason, it's necessary to include the search module after defining all the standard fields, otherwise we'll error if we try to define a standard field if it's already been defined as a search only field.

At this point, I only have use for searching invoices and credit memos, so the module is only included on those records, but it's the same search fields that could eventually be included on AssemblyBuild, BinTransfer, CashSale, etc. records, all of which generally seem to fall under "transaction".


Alternatively, I'm conflicted whether search should continue to be an action on the individual records, as it is now, or whether it'd be better off as a separate class, better mirroring how it works internally at NetSuite.

For example, Invoice.search will return all transaction types (ie. invoice, credit memo, etc.) unless you add a criteria on type to limit to just invoices. Similarly, InventoryItem.search will return all item types (ie. inventory, non-inventory, service, etc.) unless you add a criteria on type. And Customer.search will return all customer stages (ie. lead, prospect, customer) unless you add a criteria on stage. There's probably more examples that I haven't come across yet. This regularly surprises me.

Furthermore, Invoice.search may include a credit memo in it's results, but the instance of that record will be of type NetSuite::Records::Invoice, since that's the record class you searched on, as opposed to NetSuite::Records::CreditMemo, which is misleading.

Taking it a step further, Invoice.search may include the individual lines of each invoice as well, unless you set your criteria to mainline only, and now each of those lines is an instance of NetSuite::Records::Invoice, but that's a bit of a stretch.

One simple thought is that when you call Invoice.search, we could automatically add the criteria for type. This would align with the NetSuite UI where if you're looking at the list of invoices and click Search in the top-right, it defaults the resulting search to "Type: Invoice". Then we can guarantee all results from Invoice.search would actually be invoices and having them be instances of NetSuite::Records::Invoice would always be appropriate (though less so for the individual lines, unless we defaulted to mainline only, but that seems like a step too far).
Screen Shot 2022-09-29 at 2 54 46 PM

The downside there being you couldn't search across types, if you were so inclined, unless you searched on each separate class (ie invoice, credit memo, etc.) then combined them yourself. Perhaps a solution there is to have a class like TransactionSearch that you use, so now it's clear you're searching transactions generally, and if you want to narrow it down by type, you have to provide the criteria. Then the results could be instances of something like TransactionSearchRow, where we'd define the fields that could be returned aligning with NetSuite's TransactionSearchRowBasic. The downside there, however being given an instance of a result, it'd be on the developer to cast that into an instance of the proper record (ie. Invoice) if they needed to do some updates, or perhaps the row instance offers some helper method to convert to the record. Maybe Invoice.search could still exist, but just proxy to TransactionSearch with the appropriate criteria for type?

Those are just my thoughts on using NetSuite and this gem after a few years.

@cgunther cgunther force-pushed the credit-memo-search-only-fields branch from 14574ad to 9d0dbcc Compare September 29, 2022 19:10
@cgunther cgunther force-pushed the credit-memo-search-only-fields branch from 9d0dbcc to 2c15039 Compare November 10, 2022 21:47
@cgunther cgunther changed the title Add search only fields to CreditMemo Add search only fields to CreditMemo and PurchaseOrder Nov 10, 2022
@cgunther
Copy link
Contributor Author

Updated to add search only fields to PurchaseOrder as well.

@cgunther cgunther force-pushed the credit-memo-search-only-fields branch 2 times, most recently from f4faff5 to 35c591d Compare May 3, 2023 15:08
@cgunther cgunther force-pushed the credit-memo-search-only-fields branch from 35c591d to 84c688a Compare July 26, 2023 20:39
@cgunther cgunther changed the title Add search only fields to CreditMemo and PurchaseOrder Add search only fields to CreditMemo, PurchaseOrder, and WorkOrder Jul 26, 2023
@cgunther
Copy link
Contributor Author

Updated to add the search only fields to WorkOrder as well.

@cgunther cgunther force-pushed the credit-memo-search-only-fields branch from 84c688a to 693f04d Compare October 4, 2023 20:41
@cgunther cgunther force-pushed the credit-memo-search-only-fields branch from 693f04d to a72e2a9 Compare February 9, 2024 20:37
Both `Invoice`, `CreditMemo`, and `PurchaseOrder` records (as well as
many other records) use the same search fields for the returned basic
results, `TransactionSearchRowBasic`, so rather than repeat the fields
for each record, extract them to a common module that can be shared
between the respective records.

Since the search module may cover multiple records, it's fields will
overlap with some of the standard fields on each record, so defining
search only fields is now a no-op if the search only field already
exists on the record as a standard field. For this reason, it's
necessary to include the search module after defining all the standard
fields, otherwise we'll error if we try to define a standard field if
it's already been defined as a search only field.

At this point, I only have use for searching invoices, credit memos, and
purchase orders, so the module is only included on those records, but
it's the same search fields that could eventually be included on
`AssemblyBuild`, `BinTransfer`, `CashSale`, etc. records, all of which
generally seem to fall under "transaction".
@cgunther cgunther force-pushed the credit-memo-search-only-fields branch from a72e2a9 to d12de7d Compare February 12, 2024 18:22
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.

1 participant