-
Notifications
You must be signed in to change notification settings - Fork 907
GlideRecord Query Helper #1754
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
GlideRecord Query Helper #1754
Conversation
WillemZeiler
left a comment
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.
Thank you for your submission. In general I would be careful with script includes that wrap the ootb GlideRecord. The syntax is deviating to what every ServiceNow developer knows, it requires additional code execution, and the default functions are not available in the wrapper (f.e. updating a record).
| GlideRecordHelper.prototype = { | ||
| initialize: function(tableName) { | ||
| if (!tableName) { | ||
| throw new Error("Table name is required."); |
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.
What is your reason to handle the error in this way?
|
|
||
| for (var key in queryObj) { | ||
| if (queryObj.hasOwnProperty(key)) { | ||
| gr.addQuery(key, queryObj[key]); |
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.
I would expect error handling here as well. If an object value is now empty it will query on empty resulting in unexpected behavior that will be difficult to spot.
| }, | ||
|
|
||
| getRecords: function(queryObj) { | ||
| var gr = new GlideRecord(this.tableName); |
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.
If you want to go the wrapper route, it would make sense to add additional error handling, security, and input validations to make it more robust.
| var records = helper.getRecords({ priority: 1, active: true }); | ||
|
|
||
| records.forEach(function(record) { | ||
| gs.info(record.number); |
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.
What if, based on the GlideRecord we did, we want to update one or multiple values on the record?
|
Thank you for your submission. I have added comments to the code. I would suggest not using wrapper functions like this, but if you want to continue with the code, you can use the comments. |
PR Description:
Pull Request Checklist
Overview
Code Quality
Repository Structure Compliance
Core ServiceNow APIs/Server-Side Components/Client-Side Components/Modern Development/Integration/Specialized Areas/Documentation
Restrictions