Ups support #8
Ups support #8
Conversation
… different tracking numbers
Have to update test username because too many invalid username attempts cause us to be locked out and return a different error.
carriers/ups.js
Outdated
| function filter(res) { | ||
| const packageInfo = res.body.TrackResponse.Shipment.Package; | ||
| var activitiesList = []; | ||
| if(packageInfo === undefined){ |
There was a problem hiding this comment.
if( should be if ( (needs a space)
I'm wondering if the ESLint rules for this project are missing/incorrect. There are a few lines (like this one) that should be throwing ESLint errors per our current style.
@freshlogic space-after-if and no-space-after-function seem to be missing (I made those names up), right?
carriers/ups.js
Outdated
| } else { | ||
| packageInfo.forEach((package) => { | ||
| activitiesList.push(getActivities(package)); | ||
| }) |
carriers/ups.js
Outdated
| return callback(null, results); | ||
| } | ||
|
|
||
| if (res && res.body && res.body.Fault && res.body.Fault.detail && res.body.Fault.detail.Errors && res.body.Fault.detail.Errors.ErrorDetail && res.body.Fault.detail.Errors.ErrorDetail.PrimaryErrorCode && res.body.Fault.detail.Errors.ErrorDetail.PrimaryErrorCode.Description) { |
There was a problem hiding this comment.
I can't think of one at the moment, but I wish there was a better way to do this that didn't involve so many expressions in the conditional. We do this sort of stuff often, but it's usually not so deep.
Not sure we need to change it, but I couldn't not say anything?
carriers/ups.js
Outdated
| activitiesList.forEach(activities => { | ||
| locations = Array.from(new Set(activities.map(activity => activity.location))); | ||
| getResults(locations, callback, results, activities); | ||
| }) |
util/confirmUpsFreight.js
Outdated
| if (checkDigit(trackingNumber, [3, 1, 7], 10)) { return [true, true]; } | ||
| return [false, false]; | ||
|
|
||
|
|
There was a problem hiding this comment.
Run auto-format on this? (extra lines and 1-line if on line 6 will probably get auto-fixed)
carriers/ups.js
Outdated
| activity.location = geography.addressToString(activity.address); | ||
| } else { | ||
| activity.address = { | ||
| city: '', |
There was a problem hiding this comment.
Don't use empty string to represent null or undefined: https://coding.abel.nu/2013/01/on-null-or-why-empty-strings-are-not-same-as-null/
carriers/ups.js
Outdated
| activitiesList.forEach(activity => { | ||
| if (activity.ActivityLocation != undefined) { | ||
| activity.address = { | ||
| city: activity.ActivityLocation.Address === undefined ? (activity.ActivityLocation.City === undefined ? '' : activity.ActivityLocation.City) : (activity.ActivityLocation.Address.City === undefined ? '' : activity.ActivityLocation.Address.City), |
There was a problem hiding this comment.
This is very similar to the code on line 67 but doesn't look quite identical. This is usually the sign of a subtle bug. What is this code trying to accomplish?
carriers/ups.js
Outdated
| const firstChar = `${(trackingNumber.charCodeAt(0) - 63) % 10}`; | ||
| const remaining = trackingNumber.slice(1); | ||
| trackingNumber = `${firstChar}${remaining}`; | ||
| if (checkDigit(trackingNumber, [3, 1, 7], 10)) { return [true, true]; } |
| @@ -94,6 +94,8 @@ function USPS(options) { | |||
|
|
|||
| const trackDetailList = data.TrackResponse.TrackInfo[0].TrackDetail; | |||
|
|
|||
| //console.log(data.TrackResponse.TrackInfo[0].TrackDetail); | |||
There was a problem hiding this comment.
Remove console.logs (even if they're comments)
| @@ -4,7 +4,7 @@ | |||
| "async": "~3.1.0", | |||
| "moment-timezone": "~0.5.25", | |||
| "node-geocoder": "~3.23.0", | |||
| "petty-cache": "~2.4.1", | |||
| "petty-cache": "^2.4.1", | |||
…into ups-support
No description provided.