Skip to content

Commit b295791

Browse files
committed
fix(Resolvers): Provide documents with all fields to updateById and updateOne
Before this fix mongoose load only that field which was in graphql projection. But mongoose models may have hooks that require fields which not loaded. So now update resolvers load full documents from database.
1 parent 09ad79a commit b295791

File tree

6 files changed

+42
-44
lines changed

6 files changed

+42
-44
lines changed

src/resolvers/__tests__/updateById-test.js

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,7 @@ describe('updateById() ->', () => {
2020
let user1;
2121
let user2;
2222

23-
before('clear UserModel collection', (done) => {
24-
UserModel.collection.drop(() => {
25-
done();
26-
});
27-
});
28-
29-
before('add test user document to mongoDB', () => {
23+
beforeEach('init UserModel collection', (done) => {
3024
user1 = new UserModel({
3125
name: 'userName1',
3226
skills: ['js', 'ruby', 'php', 'python'],
@@ -41,10 +35,12 @@ describe('updateById() ->', () => {
4135
relocation: true,
4236
});
4337

44-
return Promise.all([
45-
user1.save(),
46-
user2.save(),
47-
]);
38+
UserModel.collection.drop(() => {
39+
Promise.all([
40+
user1.save(),
41+
user2.save(),
42+
]).then(() => done());
43+
});
4844
});
4945

5046
beforeEach(() => {
@@ -131,24 +127,22 @@ describe('updateById() ->', () => {
131127
expect(result).have.deep.property('record.name', checkedName);
132128
});
133129

134-
it('should extract projection from record for findById', async () => {
135-
const checkedName = 'anyName123';
130+
it('should pass empty projection to findById and got full document data', async () => {
136131
const result = await updateById(UserModel, UserTypeComposer).resolve({
137132
args: {
138133
record: {
139134
_id: user1.id,
140-
name: checkedName,
141135
},
142136
},
143137
projection: {
144138
record: {
145139
name: true,
146-
gender: true,
147140
},
148141
},
149142
});
150143
expect(result).have.deep.property('record.id', user1.id);
151-
expect(result).have.deep.property('record.gender');
144+
expect(result).have.deep.property('record.name', user1.name);
145+
expect(result).have.deep.property('record.gender', user1.gender);
152146
});
153147

154148
it('should return mongoose document', async () => {

src/resolvers/__tests__/updateOne-test.js

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,7 @@ describe('updateOne() ->', () => {
1515
let user1;
1616
let user2;
1717

18-
before('clear UserModel collection', (done) => {
19-
UserModel.collection.drop(() => {
20-
done();
21-
});
22-
});
23-
24-
before('add test user document to mongoDB', () => {
18+
beforeEach('init UserModel collection', (done) => {
2519
user1 = new UserModel({
2620
name: 'userName1',
2721
skills: ['js', 'ruby', 'php', 'python'],
@@ -36,10 +30,12 @@ describe('updateOne() ->', () => {
3630
relocation: true,
3731
});
3832

39-
return Promise.all([
40-
user1.save(),
41-
user2.save(),
42-
]);
33+
UserModel.collection.drop(() => {
34+
Promise.all([
35+
user1.save(),
36+
user2.save(),
37+
]).then(() => done());
38+
});
4339
});
4440

4541
beforeEach(() => {
@@ -158,20 +154,20 @@ describe('updateOne() ->', () => {
158154
expect(result1.record.id).to.not.equal(result2.record.id);
159155
});
160156

161-
it('should extract projection from record for findOne', async () => {
157+
it('should pass empty projection to findOne and got full document data', async () => {
162158
const result = await updateOne(UserModel, UserTypeComposer).resolve({
163159
args: {
164160
filter: { _id: user1.id },
165161
},
166162
projection: {
167163
record: {
168164
name: true,
169-
gender: true,
170165
},
171166
},
172167
});
173168
expect(result).have.deep.property('record.id', user1.id);
174-
expect(result).have.deep.property('record.gender');
169+
expect(result).have.deep.property('record.name', user1.name);
170+
expect(result).have.deep.property('record.gender', user1.gender);
175171
});
176172

177173
it('should return mongoose document', async () => {
@@ -188,7 +184,7 @@ describe('updateOne() ->', () => {
188184
beforeRecordMutate: (record) => {
189185
beforeMutationId = record.id;
190186
return record;
191-
}
187+
},
192188
});
193189
expect(result.record).instanceof(UserModel);
194190
expect(beforeMutationId).to.equal(user1.id);

src/resolvers/helpers/projection.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type {
44
ExtendedResolveParams,
55
} from '../../definition';
66

7-
export function projectionHelper(resolveParams: ExtendedResolveParams): void {
7+
export function projectionHelper(resolveParams: ExtendedResolveParams): void { // eslint-disable-line
88
const projection = resolveParams.projection;
99
if (projection) {
1010
const flatProjection = {};
@@ -17,6 +17,9 @@ export function projectionHelper(resolveParams: ExtendedResolveParams): void {
1717
flatProjection[key] = !!projection[key];
1818
}
1919
});
20-
resolveParams.query = resolveParams.query.select(flatProjection); // eslint-disable-line
20+
21+
if (Object.keys(flatProjection).length > 0) {
22+
resolveParams.query = resolveParams.query.select(flatProjection); // eslint-disable-line
23+
}
2124
}
2225
}

src/resolvers/updateById.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,18 +88,21 @@ export default function updateById(
8888

8989
resolveParams.args._id = recordData._id;
9090
delete recordData._id;
91-
resolveParams.projection =
92-
(resolveParams.projection && resolveParams.projection.record) || {};
91+
92+
// We should get all data for document, cause Mongoose model may have hooks/middlewares
93+
// which required some fields which not in graphql projection
94+
// So empty projection returns all fields.
95+
resolveParams.projection = {};
9396

9497
return findByIdResolver.resolve(resolveParams)
95-
.then(doc => {
98+
.then((doc) => {
9699
if (resolveParams.beforeRecordMutate) {
97100
return resolveParams.beforeRecordMutate(doc);
98101
}
99102
return doc;
100103
})
101104
// save changes to DB
102-
.then(doc => {
105+
.then((doc) => {
103106
if (!doc) {
104107
return Promise.reject('Document not found');
105108
}
@@ -110,7 +113,7 @@ export default function updateById(
110113
return doc;
111114
})
112115
// prepare output payload
113-
.then(record => {
116+
.then((record) => {
114117
if (record) {
115118
return {
116119
record,

src/resolvers/updateMany.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ export default function updateMany(
101101
? Promise.resolve(resolveParams.beforeQuery(resolveParams.query))
102102
: resolveParams.query.exec()
103103
)
104-
.then(res => {
104+
.then((res) => {
105105
if (res.ok) {
106106
return {
107107
numAffected: res.nModified,

src/resolvers/updateOne.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,26 +94,28 @@ export default function updateOne(
9494
);
9595
}
9696

97-
resolveParams.projection =
98-
(resolveParams.projection && resolveParams.projection.record) || {};
97+
// We should get all data for document, cause Mongoose model may have hooks/middlewares
98+
// which required some fields which not in graphql projection
99+
// So empty projection returns all fields.
100+
resolveParams.projection = {};
99101

100102
return findOneResolver.resolve(resolveParams)
101-
.then(doc => {
103+
.then((doc) => {
102104
if (resolveParams.beforeRecordMutate) {
103105
return resolveParams.beforeRecordMutate(doc);
104106
}
105107
return doc;
106108
})
107109
// save changes to DB
108-
.then(doc => {
110+
.then((doc) => {
109111
if (recordData) {
110112
doc.set(recordData);
111113
return doc.save();
112114
}
113115
return doc;
114116
})
115117
// prepare output payload
116-
.then(record => {
118+
.then((record) => {
117119
if (record) {
118120
return {
119121
record,

0 commit comments

Comments
 (0)