Skip to content

Support for other types of ids#1

Open
surfdude75 wants to merge 5 commits into
hemerajs:masterfrom
surfdude75:master
Open

Support for other types of ids#1
surfdude75 wants to merge 5 commits into
hemerajs:masterfrom
surfdude75:master

Conversation

@surfdude75

Copy link
Copy Markdown

I have a legacy mongo database where not all ids are of type ObjectId.

It would be great if the plugin could try do not use ObjectId if the id is not compatible with the ObjectID.

Thanks and congrats for you great work.

@StarpTech

Copy link
Copy Markdown
Member

Hi @surfdude75 thanks for your contribution. I understand your concern.

Comment thread store.js Outdated
removeById(req) {
var id
try {
id = this.ObjectID(req.id)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This kind of check is really bad. We can't trust in that an exception is thrown. Please check the format via regex and swap it out in a separate function to avoid duplicate code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is even better

var mongodb = require("mongodb"),
    objectid = mongodb.BSONPure.ObjectID;

console.log(objectid.isValid('53fbf4615c3b9f41c381b6a3'));

@surfdude75

Copy link
Copy Markdown
Author

Hi @StarpTech I hope it is as you requested.

Let me know if it is necessary any other adjustment.

Thanks.

@StarpTech

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants