Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: unable to generate subscription query unless mutation operation in model is set to true #2139

Merged
merged 4 commits into from Oct 3, 2020

Conversation

RinkiyaKeDad
Copy link
Contributor

@RinkiyaKeDad RinkiyaKeDad commented Oct 1, 2020

I've updated the if conditions and it now depends only on the subXXX knobs. This should fix issue #2135. Please review the code. Thanks.

Copy link
Contributor

@machi1990 machi1990 left a comment

@RinkiyaKeDad thanks you for the PR. It should be okay for graphql generated files. I think we should do the same for .ts generated files:
See

  • export const createSubscriptionsTS = (types: ModelDefinition[]) => {
    const subscriptions = []
    types.forEach((model: ModelDefinition) => {
    const t = model.graphqlType;
    const name = model.graphqlType.name;
    if (model.crudOptions.create && model.crudOptions.subCreate) {
    const operation = getSubscriptionName(name, GraphbackOperationType.CREATE);
    const inputTypeField = getInputTypeName(model.graphqlType.name, GraphbackOperationType.SUBSCRIPTION_CREATE)
    subscriptions.push({
    name: operation,
    implementation: subscriptionTS(t, operation, inputTypeField)
    })
    }
    if (model.crudOptions.update && model.crudOptions.subUpdate) {
    const operation = getSubscriptionName(name, GraphbackOperationType.UPDATE);
    const inputTypeField = getInputTypeName(model.graphqlType.name, GraphbackOperationType.SUBSCRIPTION_UPDATE)
    subscriptions.push({
    name: operation,
    implementation: subscriptionTS(t, operation, inputTypeField)
    })
    }
    if (model.crudOptions.delete && model.crudOptions.subDelete) {
    const operation = getSubscriptionName(name, GraphbackOperationType.DELETE);
    const inputTypeField = getInputTypeName(model.graphqlType.name, GraphbackOperationType.SUBSCRIPTION_DELETE)
    subscriptions.push({
    name: operation,
    implementation: subscriptionTS(t, operation, inputTypeField)
    })
    }
    })
    return subscriptions
    }

Once that is done, can you add a snapshot test: one for ts and gql?

You can base them on

test('Test plugin engine ts', async () => {
const crudMethods = {
"create": true,
"update": true,
"findOne": true,
"find": true,
"delete": true,
}
const metadata = new GraphbackCoreMetadata({ crudMethods }, buildSchema(schemaText))
const plugin = new ClientCRUDPlugin({ outputFile: './tmp/generated.ts', fragmentOnly: false });
expect(plugin.getDocuments(metadata)).toMatchSnapshot();
});
test('Test plugin engine graphql', async () => {
const crudMethods = {
"create": true,
"update": true,
"findOne": true,
"find": true,
"delete": true,
}
const metadata = new GraphbackCoreMetadata({ crudMethods }, buildSchema(schemaText))
const plugin = new ClientCRUDPlugin({ outputFile: './tmp/generated.graphql', fragmentOnly: false });
expect(plugin.getDocuments(metadata)).toMatchSnapshot();
.

For these added tests, you can use

 const crudMethods = {
    "create": false,
    "update": false,
    "findOne": true,
    "find": true,
    "delete": false,
    "subCreate": true,
    "subUpdate": true,
    "subDelete": true
  }

as a configuration.

Once that is done, we should be good to go.

Thanks.

@RinkiyaKeDad
Copy link
Contributor Author

RinkiyaKeDad commented Oct 3, 2020

@machi1990 Thanks for reviewing my PR. I have never added tests before so there might be some errors. Can you please point me where I could read more about these tests and how to add them properly? Nevertheless I've tried adding them based on the ones already present. I've named the two tests and their output files arbitrarily, please tell me what would be an appropriate name for them. Thanks a lot.

@machi1990
Copy link
Contributor

machi1990 commented Oct 3, 2020

@machi1990 Thanks for reviewing my PR. I have never added tests before so there might be some errors. Can you please point me where I could read more about these tests and how to add them properly? Nevertheless I've tried adding them based on the ones already present. I've named the two tests and their output files arbitrarily, please tell me what would be an appropriate name for them. Thanks a lot.

Thanks for the followup. The added tests and naming looks okay to me. This is good to go.

@machi1990 machi1990 merged commit fbdf8f7 into aerogear:master Oct 3, 2020
@craicoverflow craicoverflow added the bug Something isn't working label Oct 6, 2020
@RinkiyaKeDad RinkiyaKeDad deleted the subs-fix branch Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Hacktoberfest
Projects
None yet
3 participants