Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(schema): Check for undefined value in resolveQueryObjectId resolver #2914

Merged
merged 2 commits into from
Dec 11, 2022

Conversation

philipimperato
Copy link
Contributor

@philipimperato philipimperato commented Dec 7, 2022

Summary

Insures that a undefined or null field within the mongodb objectId resolver (resolveQueryObjectId) doesn't throw an exception and returns undefined.

@netlify
Copy link

netlify bot commented Dec 7, 2022

‼️ Deploy request for feathers-dove rejected.

Name Link
🔨 Latest commit 5127272

@@ -19,6 +19,10 @@ export async function resolveQueryObjectId(
): Promise<IdQueryObject<ObjectId>>
export async function resolveQueryObjectId(value: ObjectIdParam): Promise<ObjectId>
export async function resolveQueryObjectId(value: ObjectIdParam | IdQueryObject<ObjectIdParam>) {
if (value === undefined || value === null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a scenario where we can actually just do if (!value) since the value has to be a string or an object, and neither of those can be 0.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is ok. Otherwise it would still error if you pass null as the value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I already made the changes prior to reading this. Resubmitted.

@daffl daffl changed the title Check for undefined value in resolveQueryObjectId resolver fix(schema): Check for undefined value in resolveQueryObjectId resolver Dec 11, 2022
@daffl daffl merged commit d9449fa into feathersjs:dove Dec 11, 2022
@daffl
Copy link
Member

daffl commented Dec 11, 2022

Great, thank you! Will go out shortly.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants