-
-
Notifications
You must be signed in to change notification settings - Fork 278
Opportunity record #216
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
Opportunity record #216
Conversation
lib/netsuite/records/opportunity.rb
Outdated
| initialize_from_attributes_hash(attributes) | ||
| end | ||
|
|
||
| def self.search_class_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for a search_Class_name here... this is auto-generated. This is only required when the search name is different from the class name.
4272c18 to
3932720
Compare
| @@ -0,0 +1,30 @@ | |||
| module NetSuite | |||
| module Records | |||
| class OpportunityItemList < Support::Sublist | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jheth OpportunityItemList shouldn't contain any logic aside a namespace definition and a sublist call. Ex:
netsuite/lib/netsuite/records/invoice_item_list.rb
Lines 4 to 6 in de60d36
| include Namespaces::TranSales | |
| sublist :item, InvoiceItem |
|
Looking good! One additional comment: any new records should be added to the basic record spec: https://github.com/NetSweet/netsuite/blob/master/spec/netsuite/records/basic_record_spec.rb |
3932720 to
5a3aa37
Compare
5a3aa37 to
518264d
Compare
|
Thanks @iloveitaly. Just added to basic_record_spec. Should I add an opportunity_spec.rb as well? |
I've been bearish on individual specs for records. I'm fine with just a If there is no unique record functionality, they add a lot of maintinance weight with very little value. What do you think? Thanks for the PR! |
|
Merged with 371baf9 |
No description provided.