-
Notifications
You must be signed in to change notification settings - Fork 104
Update dependency #153
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
Update dependency #153
Conversation
…cies # Conflicts: # .github/CODEOWNERS # README.md # package.json
RUN set -eux; \ | ||
apk update; \ | ||
# For private npm repo: | ||
# yarn config set registry http://host.docker.internal:4873; \ |
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.
add option for testing with private repo
@@ -8,11 +8,7 @@ const env = require('var'); | |||
let mongoConfig = { | |||
connection: `mongodb://${env.MONGO_HOSTNAME}`, | |||
db_name: env.MONGO_DB_NAME, | |||
options: { |
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.
depreciated
@@ -224,15 +223,15 @@ module.exports.searchById = (args) => | |||
let db = globals.get(CLIENT_DB); | |||
let collection = db.collection(`${COLLECTION.ALLERGYINTOLERANCE}_${base_version}`); | |||
// Query our collection for this observation | |||
collection.findOne({ id: id.toString() }, (err, allergyintolerance) => { | |||
if (err) { | |||
collection.findOne({ id: id.toString() }) |
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.
fix call w/ mongo driver
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.
There's an entire school of thought about returning error messages from an external system. See https://csf.tools/reference/nist-sp-800-53/r5/si/si-11/
I think it's worth updating the pattern with mongo to return a controlled message instead of the raw error from MongoDB.
I'll make another ticket for that. I believe it is beyond the scope of this PR |
Merge this pr after issue #157 is merge into this PR |
add mongo error handler
…cies # Conflicts: # .snyk # package.json # yarn.lock
.then( () => { | ||
return resolve({ | ||
id: id, | ||
created: res && res.lastErrorObject ? !res.lastErrorObject.updatedExisting : false, |
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.
this was updated to support null check and default to false. This was a bug previously where the first put would fail
@@ -3,18 +3,23 @@ name: Lint | |||
on: push | |||
|
|||
jobs: | |||
run-tests: |
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 dind for just lint
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.
Thanks for improving coverage and getting the error handling updated.
No description provided.