-
Notifications
You must be signed in to change notification settings - Fork 3
Conversation
@GeoffThorpeFT, not sure if you want to look over this |
lib/fetch-system-codes.js
Outdated
// We're only interested in system codes | ||
return response.body | ||
.filter(entry => entry.systemCode) | ||
.map(entry => entry.systemCode); | ||
}) |
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 assume the above is aiming to extract just the field named "systemCode" from the rather long list of fields returned by the v3/items/system call. However, due to a legacy issue with data uploads (too long to explain) the systemcode is actually held in the key field "dataItemID" not "systemCode". You may find some "systemCode" fields but its no longer used.
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.
Excellent, thanks Geoff! Glad I asked :) will change
test/unit/lib/fetch-system-codes.js
Outdated
{value: 'bar'} | ||
{systemCode: 'foo'}, | ||
{noSystemCode: 'this should not appear'}, | ||
{systemCode: 'bar'} | ||
] |
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.
Due to above issue with the "systemCode" field I recommend changing these mockups to populate "dataItemID" since that is the real system code field.
As @GeoffThorpeFT mentioned, systemCode is no longer used. We now get a more complete list of codes by using dataItemId.
We've had to switch endpoints for the move from v2 to v3 of CMDB. The intention is to support filtering in the URL at a later date which may mean we can increase the per-page limit and only request the data we need.