[update] update functions should only update fields
Right now the update function updates the entire document supplied the mutation data. I think that the update should only update the fields sendt in the mutation.
How I know it updates only that fields which was passed via mutation. If you pass fieldname with null, it must save null to DB. If field not present in mutation it should keep it in DB unchanged.
@rlogachyov can you confirm this?
I have this function on Users scema:
UserSchema.pre('save', function (next) {
if (this.password || this.isNew){
this.password = bcryptjs.hashSync(this.password, 10)
}
next()
})
https://github.com/nodkz/graphql-compose-mongoose/blob/master/src/resolvers/updateById.js#L109-L116
Will get full document, update fields that is changed and save complete document back to database with all fields. This triggers crypt of a password that is not sendt in mutation.
I find http://mongoosejs.com/docs/api.html#document_Document-update to be more in the right place as it just sends a update to model containing just the fields to update.
😉
I can confirm when updating attributes on a subdocument i.e.,
{
_id: 'someId',
profile: {
...
}
}
the update wipes out fields that are not passed along with the query.
@smooJitter From MongoDB docs:
To update an embedded document or an array as a whole, specify the replacement value for the field. To update particular fields in an embedded document or in an array, use dot notation to specify the field.
I think, that it also will be true for using with mongoose. And it will work with graphql-compose-mongoose if we wrap updateById resolver where make record fields flat via dot-notation keys.
So partial updating of embedded docs is quite tricky. Maybe it will be nice to write additional standard resolver for graphql-compose-mongoose and call it like updatePartialById.
So we ran into this issue as well. Before writing a resolver, we found a work around just using updateMany(filter: {_id: ID}, record: { field: value}}). It works in most cases when you don't need the record back (since updateMany only returns numAffected).
I have this same issue is there a solution?
I know this issue is quite old now, but this was something that I wanted to achieve in a universal manner, and I found a way that seems to work nicely.
I created this before-save hook which can be applied to all my updateById. This goes in a file called "mergeFields.js"
function isObject(item) {
return item && typeof item === "object" && !Array.isArray(item);
}
export function merge(target, source) {
// sanity check - probably overkill
if (!isObject(source)) {
return source !== null ? source : undefined;
}
const output = target;
const entries = Object.entries(source);
for (let i = 0; i < entries.length; i += 1) {
const [key, value] = entries[i];
if (isObject(source[key])) {
if (target[key] === undefined) {
// add new embedded documents
output[key] = value;
} else {
// recursively merge existing embedded documents
output[key] = merge(target[key], value);
}
} else {
// remove existing fields with a `null` input
output[key] = value !== null ? value : undefined;
}
}
return output;
}
export default function mergeFields(resolvers) {
Object.keys(resolvers).forEach((k) => {
resolvers[k] = resolvers[k].wrapResolve(
(next: any) => async (rp) => {
rp.beforeRecordMutate = async (doc, rp) => {
const entries = Object.entries(rp.args.record);
for (let i = 0; i < entries.length; i += 1) {
const [key, value] = entries[i];
if (isObject(value) && !(value instanceof Date)) {
if (doc[key] === undefined) {
// add new embedded documents that don't exist yet
doc[key] = value;
} else {
// merge new embedded documents into existing ones
rp.args.record[key] = merge(doc[key], rp.args.record[key]);
}
} else if (value === null) {
// remove existing fields with a `null` input
rp.args.record[key] = undefined;
}
}
return doc;
};
return next(rp);
}
);
});
return resolvers;
}
Then when I add my resolvers:
import mergeFields from "./mergeFields";
export const NestedExampleMutation = {
...mergeFields({ updateNestedExample: NestedExampleTC.getResolver("updateById") }),
addNestedExample: NestedExampleTC.getResolver("createOne"),
};
If this approach actually works as well as it seems to in my testing, it might be nice to have it available as an option in UpdateByIdResolverOpts.