Required properties in Create Resolvers

[israelglar]

Reposting in right repo from graphql-compose/graphql-compose#151

I have a User schema

const UserSchema = new Mongoose.Schema(
  {
    email: {type: String, unique: true, required: true},
    name: {type: String, required: true},
  },
  {
    timestamps: true,
  },
);

After using composeWithMongoose and exposing all CRUD resolvers the result for the Create methods are:

type CreateOneUserInput {
  email: String
  name: String
  updatedAt: Date
  createdAt: Date
}
type CreateManyUserInput {
  email: String
  name: String
  updatedAt: Date
  createdAt: Date
}

The properties email and name should be required.

I just recently started using the library, so maybe there is something in docs that I didn't see.
If it's a bug, I can contribute with a fix with some guidance.
Thanks

[israelglar]

As I see it, the issue comes from this function and the function is calls:

graphql-compose-mongoose/src/fieldsConverter.js

Lines 190 to 218 in 04e82c3

	export function convertFieldToGraphQL(
	field: MongooseFieldT,
	prefix?: string = '',
	schemaComposer: SchemaComposer<any>
	): ComposeOutputType {
	if (!schemaComposer.has('MongoID')) {
	schemaComposer.set('MongoID', GraphQLMongoID);
	}
	const complexType = deriveComplexType(field);
	switch (complexType) {
	case ComplexTypes.SCALAR:
	return scalarToGraphQL(field);
	case ComplexTypes.ARRAY:
	return arrayToGraphQL(field, prefix, schemaComposer);
	case ComplexTypes.EMBEDDED:
	return embeddedToGraphQL(field, prefix, schemaComposer);
	case ComplexTypes.ENUM:
	return enumToGraphQL(field, prefix, schemaComposer);
	case ComplexTypes.REFERENCE:
	return referenceToGraphQL(field);
	case ComplexTypes.DOCUMENT_ARRAY:
	return (documentArrayToGraphQL(field, prefix, schemaComposer): any);
	case ComplexTypes.MIXED:
	return 'JSON';
	default:
	return scalarToGraphQL(field);
	}
	}

It doesn't check if the field is required and handles it

[nodkz]

I suggest to you use programmatic way. When types were generated then you may modify them as you wish. In your case you need to run the following code:

UserTC.getResolver('createMany').getArgTC('record'). makeRequired(['email', 'name']);

[israelglar]

I understand that this is possible, my question is more about why isn't it supported by default with the composeWithMongoose function, since that information is already present in the mongoose Schema.

I'm doing some tests with the source code and something like this can be done:

graphql-compose-mongoose/src/fieldsConverter.js LN138

Object.keys(mongooseFields).forEach(fieldName => {
    const mongooseField = mongooseFields[fieldName];

    let fieldType = convertFieldToGraphQL(mongooseField, typeName, schemaComposer)

    if (mongooseField.isRequired) {
      if (typeof fieldType === "string") {
        fieldType = fieldType + '!'
      }
    }

    graphqlFields[fieldName] = {
      type: fieldType,
      description: _getFieldDescription(mongooseField),
    };

That is something I'm doing right now and it works for some types that are typeof string.
I understand that not all results from convertFieldToGraphQL are typeof string, but I hope you can see the idea.

What do you think about that?

[nodkz]

Brilliant! Can your create Pull Request with tests?

[israelglar]

Sure 👍
I will create a PR with support for other types(not only string) and make the tests :)

[nodkz]

🎉 This issue has been resolved in version 5.2.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[israelglar]

I just realized something. Adding the required fields that way forces all resolvers to have that field required.

That's not good because it breaks some filter logic (eg: findOne, findMany, Count, Remove). This logic only really makes sense for 'create' Resolvers.

What do you think about that?

[nodkz]

Wow good catch. Urgently need to fix this.

[israelglar]

What do you think it's a good solution to that?

We can always go back to what was before, but I still think that making the fields required when the schema changes is nice thing to have.

Maybe alter the resolver so that only the "Create" ones have that check of required fields?

[nodkz]

For now only Create resolvers should have required fields. All other args should be as they were for backward compatibility for current users.

It's very bad to update a package and got a broken queries at runtime.

[israelglar]

Agree with you. I will fix that tomorrow so that only Create Resolvers have required fields

[israelglar]

Created a new PR: #146