Skip to content

Commit 99742d2

Browse files
committed
feat(removeById, removeOne): Add support beforeRecordUpdate. Now remove* resolvers work like updat
1 parent 2ac8237 commit 99742d2

File tree

4 files changed

+167
-44
lines changed

4 files changed

+167
-44
lines changed

src/resolvers/__tests__/removeById-test.js

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/* @flow */
2+
/* eslint-disable no-param-reassign */
23

34
import { expect } from 'chai';
45
import { GraphQLNonNull, GraphQLObjectType } from 'graphql';
5-
import { Query } from 'mongoose';
66
import { Resolver, TypeComposer } from 'graphql-compose';
77
import { UserModel } from '../../__mocks__/userModel';
88
import removeById from '../removeById';
@@ -74,14 +74,14 @@ describe('removeById() ->', () => {
7474
expect(result).have.property('recordId', user.id);
7575
});
7676

77-
it('should return payload.recordId even document not exists', async () => {
77+
it('should return error if document does not exist', () => {
7878
const unexistedId = '500000000000000000000000';
79-
const result = await removeById(UserModel, UserTypeComposer).resolve({
79+
const promise = removeById(UserModel, UserTypeComposer).resolve({
8080
args: {
8181
_id: unexistedId,
8282
},
8383
});
84-
expect(result).have.property('recordId', unexistedId);
84+
return expect(promise).to.eventually.be.rejectedWith('Document not found');
8585
});
8686

8787

@@ -108,25 +108,62 @@ describe('removeById() ->', () => {
108108
expect(result).have.deep.property('record.id', user.id);
109109
});
110110

111+
it('should pass empty projection to findById and got full document data', async () => {
112+
const result = await removeById(UserModel, UserTypeComposer).resolve({
113+
args: {
114+
_id: user.id,
115+
},
116+
projection: {
117+
record: {
118+
name: true,
119+
},
120+
},
121+
});
122+
expect(result).have.deep.property('record.id', user.id);
123+
expect(result).have.deep.property('record.name', user.name);
124+
expect(result).have.deep.property('record.gender', user.gender);
125+
});
126+
111127
it('should return mongoose document', async () => {
112128
const result = await removeById(UserModel, UserTypeComposer).resolve({
113129
args: { _id: user.id },
114130
});
115131
expect(result.record).instanceof(UserModel);
116132
});
117133

118-
it('should call `beforeQuery` method with non-executed `query` as arg', async () => {
119-
let beforeQueryCalled = false;
134+
it('should call `beforeRecordMutate` method with founded `record` and `resolveParams` as args', async () => {
135+
let beforeMutationId;
120136
const result = await removeById(UserModel, UserTypeComposer).resolve({
121137
args: { _id: user.id },
122-
beforeQuery: (query) => {
123-
expect(query).instanceof(Query);
124-
beforeQueryCalled = true;
125-
return query;
126-
}
138+
context: { ip: '1.1.1.1' },
139+
beforeRecordMutate: (record, rp) => {
140+
beforeMutationId = record.id;
141+
record.someDynamic = rp.context.ip;
142+
return record;
143+
},
127144
});
128-
expect(beforeQueryCalled).to.be.true;
129145
expect(result.record).instanceof(UserModel);
146+
expect(result).have.deep.property('record.someDynamic', '1.1.1.1');
147+
expect(beforeMutationId).to.equal(user.id);
148+
149+
const empty = await UserModel.collection.findOne({ _id: user._id });
150+
expect(empty).to.equal(null);
151+
});
152+
153+
it('`beforeRecordMutate` may reject operation', async () => {
154+
const result = removeById(UserModel, UserTypeComposer).resolve({
155+
args: { _id: user.id },
156+
context: { readOnly: true },
157+
beforeRecordMutate: (record, rp) => {
158+
if (rp.context.readOnly) {
159+
return Promise.reject(new Error('Denied due context ReadOnly'));
160+
}
161+
return record;
162+
},
163+
});
164+
await expect(result).be.rejectedWith(Error, 'Denied due context ReadOnly');
165+
const exist = await UserModel.collection.findOne({ _id: user._id });
166+
expect(exist.name).to.equal(user.name);
130167
});
131168
});
132169

src/resolvers/__tests__/removeOne-test.js

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
/* @flow */
2+
/* eslint-disable no-param-reassign */
23

34
import { expect } from 'chai';
45
import { GraphQLObjectType } from 'graphql';
56
import { Resolver, TypeComposer } from 'graphql-compose';
6-
import { Query } from 'mongoose';
77
import { UserModel } from '../../__mocks__/userModel';
88
import removeOne from '../removeOne';
99
import GraphQLMongoID from '../../types/mongoid';
@@ -139,25 +139,67 @@ describe('removeOne() ->', () => {
139139
expect(result2).have.deep.property('record.age', user3.age);
140140
});
141141

142+
it('should pass empty projection to findOne and got full document data', async () => {
143+
const result = await removeOne(UserModel, UserTypeComposer).resolve({
144+
args: {
145+
filter: { _id: user1.id },
146+
},
147+
projection: {
148+
record: {
149+
name: true,
150+
},
151+
},
152+
});
153+
expect(result).have.deep.property('record.id', user1.id);
154+
expect(result).have.deep.property('record.name', user1.name);
155+
expect(result).have.deep.property('record.gender', user1.gender);
156+
});
157+
142158
it('should return mongoose document', async () => {
143159
const result = await removeOne(UserModel, UserTypeComposer).resolve({
144-
args: { },
160+
args: { filter: { _id: user1.id } },
145161
});
146162
expect(result.record).instanceof(UserModel);
147163
});
148164

149-
it('should call `beforeQuery` method with non-executed `query` as arg', async () => {
150-
let beforeQueryCalled = false;
165+
it('should rejected with Error if args.filter is empty', async () => {
166+
const result = removeOne(UserModel, UserTypeComposer).resolve({ args: {} });
167+
await expect(result).be.rejectedWith(Error, 'at least one value in args.filter');
168+
});
169+
170+
it('should call `beforeRecordMutate` method with founded `record` and `resolveParams` as args', async () => {
171+
let beforeMutationId;
151172
const result = await removeOne(UserModel, UserTypeComposer).resolve({
152-
args: { },
153-
beforeQuery: (query) => {
154-
expect(query).instanceof(Query);
155-
beforeQueryCalled = true;
156-
return query;
157-
}
173+
args: { filter: { _id: user1.id } },
174+
context: { ip: '1.1.1.1' },
175+
beforeRecordMutate: (record, rp) => {
176+
beforeMutationId = record.id;
177+
record.someDynamic = rp.context.ip;
178+
return record;
179+
},
158180
});
159-
expect(beforeQueryCalled).to.be.true;
160181
expect(result.record).instanceof(UserModel);
182+
expect(result).have.deep.property('record.someDynamic', '1.1.1.1');
183+
expect(beforeMutationId).to.equal(user1.id);
184+
185+
const empty = await UserModel.collection.findOne({ _id: user1._id });
186+
expect(empty).to.equal(null);
187+
});
188+
189+
it('`beforeRecordMutate` may reject operation', async () => {
190+
const result = removeOne(UserModel, UserTypeComposer).resolve({
191+
args: { filter: { _id: user1.id } },
192+
context: { readOnly: true },
193+
beforeRecordMutate: (record, rp) => {
194+
if (rp.context.readOnly) {
195+
return Promise.reject(new Error('Denied due context ReadOnly'));
196+
}
197+
return record;
198+
},
199+
});
200+
await expect(result).be.rejectedWith(Error, 'Denied due context ReadOnly');
201+
const exist = await UserModel.collection.findOne({ _id: user1._id });
202+
expect(exist.name).to.equal(user1.name);
161203
});
162204
});
163205

src/resolvers/removeById.js

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
GraphQLNonNull,
77
} from 'graphql';
88
import { Resolver, TypeComposer } from 'graphql-compose';
9+
import findById from './findById';
910
import { projectionHelper } from './helpers/projection';
1011
import GraphQLMongoID from '../types/mongoid';
1112
import typeStorage from '../typeStorage';
@@ -33,6 +34,8 @@ export default function removeById(
3334
);
3435
}
3536

37+
const findByIdResolver = findById(model, typeComposer);
38+
3639
const outputTypeName = `RemoveById${typeComposer.getTypeName()}Payload`;
3740
const outputType = typeStorage.getOrSet(
3841
outputTypeName,
@@ -64,6 +67,7 @@ export default function removeById(
6467
type: new GraphQLNonNull(GraphQLMongoID),
6568
},
6669
},
70+
// $FlowFixMe
6771
resolve: (resolveParams: ExtendedResolveParams) => {
6872
const args = resolveParams.args || {};
6973

@@ -73,15 +77,29 @@ export default function removeById(
7377
);
7478
}
7579

76-
resolveParams.query = model.findByIdAndRemove(args._id);
77-
projectionHelper(resolveParams);
80+
// We should get all data for document, cause Mongoose model may have hooks/middlewares
81+
// which required some fields which not in graphql projection
82+
// So empty projection returns all fields.
83+
resolveParams.projection = {};
7884

79-
return (
80-
resolveParams.beforeQuery
81-
? Promise.resolve(resolveParams.beforeQuery(resolveParams.query))
82-
: resolveParams.query.exec()
83-
)
84-
.then(record => {
85+
// FlowFixMe
86+
return findByIdResolver.resolve(resolveParams)
87+
.then((doc) => {
88+
// $FlowFixMe
89+
if (resolveParams.beforeRecordMutate) {
90+
return resolveParams.beforeRecordMutate(doc, resolveParams);
91+
}
92+
return doc;
93+
})
94+
// remove record from DB
95+
.then((doc) => {
96+
if (!doc) {
97+
return Promise.reject(new Error('Document not found'));
98+
}
99+
return doc.remove();
100+
})
101+
// prepare output payload
102+
.then((record) => {
85103
if (record) {
86104
return {
87105
record,

src/resolvers/removeOne.js

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ import { GraphQLObjectType } from 'graphql';
55
import { Resolver, TypeComposer } from 'graphql-compose';
66
import GraphQLMongoID from '../types/mongoid';
77
import typeStorage from '../typeStorage';
8-
import { filterHelperArgs, filterHelper } from './helpers/filter';
9-
import { sortHelperArgs, sortHelper } from './helpers/sort';
10-
import { projectionHelper } from './helpers/projection';
8+
import { filterHelperArgs } from './helpers/filter';
9+
import { sortHelperArgs } from './helpers/sort';
10+
import findOne from './findOne';
1111
import type {
1212
MongooseModelT,
1313
ExtendedResolveParams,
@@ -32,6 +32,8 @@ export default function removeOne(
3232
);
3333
}
3434

35+
const findOneResolver = findOne(model, typeComposer, opts);
36+
3537
const outputTypeName = `RemoveOne${typeComposer.getTypeName()}Payload`;
3638
const outputType = typeStorage.getOrSet(
3739
outputTypeName,
@@ -68,18 +70,42 @@ export default function removeOne(
6870
...(opts && opts.sort),
6971
}),
7072
},
73+
// $FlowFixMe
7174
resolve: (resolveParams: ExtendedResolveParams) => {
72-
resolveParams.query = model.findOneAndRemove({});
73-
filterHelper(resolveParams);
74-
sortHelper(resolveParams);
75-
projectionHelper(resolveParams);
75+
const filterData = (resolveParams.args && resolveParams.args.filter) || {};
76+
77+
if (!(typeof filterData === 'object')
78+
|| Object.keys(filterData).length === 0
79+
) {
80+
return Promise.reject(
81+
new Error(`${typeComposer.getTypeName()}.removeOne resolver requires `
82+
+ 'at least one value in args.filter')
83+
);
84+
}
85+
86+
// We should get all data for document, cause Mongoose model may have hooks/middlewares
87+
// which required some fields which not in graphql projection
88+
// So empty projection returns all fields.
89+
resolveParams.projection = {};
7690

77-
return (
78-
resolveParams.beforeQuery
79-
? Promise.resolve(resolveParams.beforeQuery(resolveParams.query))
80-
: resolveParams.query.exec()
81-
)
82-
.then(record => {
91+
// $FlowFixMe
92+
return findOneResolver.resolve(resolveParams)
93+
.then((doc) => {
94+
// $FlowFixMe
95+
if (resolveParams.beforeRecordMutate) {
96+
return resolveParams.beforeRecordMutate(doc, resolveParams);
97+
}
98+
return doc;
99+
})
100+
// remove record from DB
101+
.then((doc) => {
102+
if (!doc) {
103+
return Promise.reject(new Error('Document not found'));
104+
}
105+
return doc.remove();
106+
})
107+
// prepare output payload
108+
.then((record) => {
83109
if (record) {
84110
return {
85111
record,

0 commit comments

Comments
 (0)