Skip to content

Commit 0b981f7

Browse files
unklehonodkz
authored andcommitted
fix(findByIds): Allowed to be id of any type
According to the documentation, `_id` field can be of any type. https://docs.mongodb.com/manual/core/document/#the-id-field "The field name _id is reserved for use as a primary key; its value must be unique in the collection, is immutable, and may be of any type other than an array." Thanks @unkleho for his great first PR 🎉 Closes #4
1 parent 34e1744 commit 0b981f7

File tree

4 files changed

+77
-12
lines changed

4 files changed

+77
-12
lines changed

src/__mocks__/postModel.js

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { mongoose, Schema } from './mongooseCommon';
2+
3+
const PostSchema = new Schema(
4+
{
5+
_id: {
6+
type: Number,
7+
unique: true,
8+
},
9+
title: {
10+
type: String,
11+
description: 'Post title',
12+
},
13+
14+
// createdAt, created via option `timastamp: true` (see bottom)
15+
// updatedAt, created via option `timastamp: true` (see bottom)
16+
}
17+
);
18+
19+
const PostModel = mongoose.model('Post', PostSchema);
20+
21+
export {
22+
PostSchema,
23+
PostModel,
24+
};

src/resolvers/__tests__/findById-test.js

+21
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,17 @@ import { expect } from 'chai';
44
import { GraphQLNonNull } from 'graphql';
55
import { Resolver } from 'graphql-compose';
66
import { UserModel } from '../../__mocks__/userModel';
7+
import { PostModel } from '../../__mocks__/postModel';
78
import findById from '../findById';
89
import GraphQLMongoID from '../../types/mongoid';
910
import { composeWithMongoose } from '../../composeWithMongoose';
1011

1112
const UserTypeComposer = composeWithMongoose(UserModel);
13+
const PostTypeComposer = composeWithMongoose(PostModel);
1214

1315
describe('findById() ->', () => {
1416
let user;
17+
let post;
1518

1619
before('clear UserModel collection', (done) => {
1720
UserModel.collection.drop(() => {
@@ -24,6 +27,17 @@ describe('findById() ->', () => {
2427
user.save(done);
2528
});
2629

30+
before('clear PostModel collection', (done) => {
31+
PostModel.collection.drop(() => {
32+
done();
33+
});
34+
});
35+
36+
before('add test post document with integer _id to mongoDB', (done) => {
37+
post = new PostModel({ _id: 1, title: 'Post 1' });
38+
post.save(done);
39+
});
40+
2741
it('should return Resolver object', () => {
2842
const resolver = findById(UserModel, UserTypeComposer);
2943
expect(resolver).to.be.instanceof(Resolver);
@@ -67,5 +81,12 @@ describe('findById() ->', () => {
6781
});
6882
expect(result).instanceof(UserModel);
6983
});
84+
85+
it('should return mongoose Post document', async () => {
86+
const result = await findById(PostModel, PostTypeComposer).resolve({
87+
args: { _id: 1 },
88+
});
89+
expect(result).instanceof(PostModel);
90+
});
7091
});
7192
});

src/resolvers/__tests__/findByIds-test.js

+30-7
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,21 @@ import { expect } from 'chai';
44
import { GraphQLNonNull, GraphQLList } from 'graphql';
55
import { Resolver } from 'graphql-compose';
66
import { UserModel } from '../../__mocks__/userModel';
7+
import { PostModel } from '../../__mocks__/postModel';
78
import findByIds from '../findByIds';
89
import GraphQLMongoID from '../../types/mongoid';
910
import { composeWithMongoose } from '../../composeWithMongoose';
1011

1112
const UserTypeComposer = composeWithMongoose(UserModel);
13+
const PostTypeComposer = composeWithMongoose(PostModel);
1214

1315
describe('findByIds() ->', () => {
1416
let user1;
1517
let user2;
1618
let user3;
19+
let post1;
20+
let post2;
21+
let post3;
1722

1823
before('clear UserModel collection', (done) => {
1924
UserModel.collection.drop(() => {
@@ -33,6 +38,24 @@ describe('findByIds() ->', () => {
3338
]).then(() => done());
3439
});
3540

41+
before('clear PostModel collection', (done) => {
42+
PostModel.collection.drop(() => {
43+
done();
44+
});
45+
});
46+
47+
before('add test post documents with integer _id to mongoDB', (done) => {
48+
post1 = new PostModel({ _id: 1, title: 'Post 1' });
49+
post2 = new PostModel({ _id: 2, title: 'Post 2' });
50+
post3 = new PostModel({ _id: 3, title: 'Post 3' });
51+
52+
Promise.all([
53+
post1.save(),
54+
post2.save(),
55+
post3.save(),
56+
]).then(() => done());
57+
});
58+
3659
it('should return Resolver object', () => {
3760
const resolver = findByIds(UserModel, UserTypeComposer);
3861
expect(resolver).to.be.instanceof(Resolver);
@@ -71,13 +94,6 @@ describe('findByIds() ->', () => {
7194
expect(result).to.be.empty;
7295
});
7396

74-
it('should return empty array if args._ids is not valid objectIds', async () => {
75-
const result = await findByIds(UserModel, UserTypeComposer)
76-
.resolve({ args: { _ids: ['d', 'e'] } });
77-
expect(result).to.be.instanceOf(Array);
78-
expect(result).to.be.empty;
79-
});
80-
8197
it('should return array of documents', async () => {
8298
const result = await findByIds(UserModel, UserTypeComposer)
8399
.resolve({ args: { _ids: [user1._id, user2._id, user3._id] } });
@@ -97,6 +113,13 @@ describe('findByIds() ->', () => {
97113
expect(result).to.have.lengthOf(1);
98114
});
99115

116+
it('should return array of documents if args._ids are integers', async () => {
117+
const result = await findByIds(PostModel, PostTypeComposer)
118+
.resolve({ args: { _ids: [1, 2, 3] } });
119+
expect(result).to.be.instanceOf(Array);
120+
expect(result).to.have.lengthOf(3);
121+
});
122+
100123
it('should return mongoose documents', async () => {
101124
const result = await findByIds(UserModel, UserTypeComposer)
102125
.resolve({ args: { _ids: [user1._id, user2._id] } });

src/resolvers/findByIds.js

+2-5
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,8 @@ export default function findByIds(
5353
return Promise.resolve([]);
5454
}
5555

56-
const selector = {};
57-
selector._id = {
58-
$in: args._ids
59-
.filter(id => mongoose.Types.ObjectId.isValid(id))
60-
.map(id => mongoose.Types.ObjectId(id)), // eslint-disable-line
56+
const selector = {
57+
_id: { $in: args._ids },
6158
};
6259

6360
resolveParams.query = model.find(selector); // eslint-disable-line

0 commit comments

Comments
 (0)