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

CLI scaffolding - "New model" #741

Closed
adrians5j opened this issue Mar 4, 2020 · 5 comments
Closed

CLI scaffolding - "New model" #741

adrians5j opened this issue Mar 4, 2020 · 5 comments
Assignees
Labels
no-issue-activity stale-issue This label is automatically assigned to issues that have no activity for at least 60 days.

Comments

@adrians5j
Copy link
Member

adrians5j commented Mar 4, 2020

This is:

  • Feature request

Detailed Description

This is an extension of issue #733. It defines a new scaffolding template - "New model".

Basically, we need a new scaffolding template, which will enable users to immediately jump into defining their data models, without doing the repetitive initial setup.

So basically, the user selects the template, and enters the name of the model. I think for v1 this should be more than enough, but feel free to add anything if needed.

The folder that the template creates might look like this:
image

The contents of files

api/pets/src/models/pet.model.ts:

import { compose, withName, withFields, string, boolean, number } from "@webiny/commodo";

export default ({ createBase }) => {
    const Pet = compose(
        withName("Pet"),
        withFields({
            name: string(),
            age: number(),
            active: boolean()
			// The rest of the fields...
        })
    )(createBase());

    return Pet;
};

api/pets/src/models/trick.model.ts:

import { compose, withName, withFields, string, boolean, number } from "@webiny/commodo";

export default ({ createBase }) => {
    const Trick = compose(
        withName("Trick"),
        withFields({
            name: string(),
            age: number(),
            active: boolean()
			// The rest of the fields...
        })
    )(createBase());

    return Trick;
};

api/pets/src/models/index.ts:

export { default as pet } from "./pet.model";
export { default as trick } from "./trick.model";

And finally, the api/pets/src/models.ts:

import { compose, withFields, withStorage, withSoftDelete, withCrudLogs } from "@webiny/commodo";
import { pet, trick } from "./models";

export default () => ({
    name: "graphql-context-models",
    type: "graphql-context",
    apply(context) {
        const driver = context.commodo && context.commodo.driver;

        if (!driver) {
            throw Error(
                `Commodo driver is not configured! Make sure you add a Commodo driver plugin to your service.`
            );
        }

		// If needed, you can add additional functionality here.
        const createBase = ({ context, driver }) =>
            flow(
                withFields({
                    id: context.commodo.fields.id()
                }),
                withStorage({ driver }),
                withSoftDelete(),
                withCrudLogs()
            )();

        context.models = {
            Pet: pet({ createBase }),
            Trick: trick({ createBase }),
            base: createBase
        };
    }
});

##### Notes

  1. If api/pets/src/models.ts already exists, the scaffolding template should be smart enough to amend it. The same goes for api/pets/src/models/index.ts

  2. I'm not sure about the api/pets/src/index.ts, should the tool import and add the plugins there too? Or should the user do this manually? An example:

image

Other actions

Except for the files above, there are a few other actions the scaffolding template must take.

It must add this plugin:
image

So the snippet is:

  - factory: "@webiny/api-plugin-commodo-db-proxy"
        options:
          functionArn: ${dbProxy.arn}

This is required because it puts a configured Commodo driver into the graphql context. In other words, because of this plugin, we can do const driver = context.commodo && context.commodo.driver; in api/pets/src/models.ts.

I realize that this is hard-coding the db-proxy driver. Maybe it's enough for this stage? Later we can ask the user which driver he wants to use and generate needed resources in the serverless.yml.


And that's it basically. We'll see along the way if something is missing. There are a few things we could, of course, improve here, for example, ask the user which fields he'd like to immediately insert into the new model. But I think it's not super important at this stage. Feel free to leave your thoughts.

@adrians5j
Copy link
Member Author

The only thing I'm questioning now... how should these models be wired with the GraphQL plugins? Currently, the user will have to do it by himself.

Should we offer a wiring option to the user? For example, we could ask him if he'd like we create basic GraphQL CRUD fields?

@Pavel910
Copy link
Collaborator

Pavel910 commented Mar 4, 2020

Some feedback from me:

  1. I would call it New data model, or New Commodo model, as Commodo is kind of a brand of its own here.

  2. Regarding this snippet of code:

context.models = {
    Pet: pet({ createBase }),
    Trick: trick({ createBase }),
    base: createBase
};

If the scaffolding template for the whole apollo service generates this base setup (talking about graphql-context-models plugin here), which only creates the base, then the New model scaffolding can reliably use AST to add new models on the fly. I strongly recommend going the AST way, and not string replacement.

So my idea is to generate the entire skeleton for the apollo service, including the models boilerplate. Then users can run the New model scaffolding to add models one by one.

  1. GraphQL can't really be generated if you don't have model fields. We could only generate an empty graphql-schema plugin, with types and everything but let the user fill in the fields himself. Or even leave this one out for v1.

  2. The db-proxy plugin should, in my opinion, be generated by the apollo-service template, not the model template. All of our apollo-services use DB, so I would go with the assumption that most services WILL use DB.

@adrians5j
Copy link
Member Author

adrians5j commented Mar 4, 2020

  1. Cool.
  2. Oh, the apollo-service scaffolding template is generating the models.ts file too? Okay cool, I think it makes sense.
  3. Let's leave it out for v1.
  4. This is linked with the point number 2, and me not realizing that the base apollo-service scaffolding template will also prepare the foundation for models. Your proposal sounds good as well.

@Fsalker Fsalker mentioned this issue Mar 5, 2020
@adrians5j
Copy link
Member Author

Please reopen this issue once you add more information and updates here. If this is not the case and you need some help, feel free to seek help from our Slack Community or ping one of the reviewers. Thank you for your contributions!

@webiny-bot
Copy link
Collaborator

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@webiny-bot webiny-bot added the stale-issue This label is automatically assigned to issues that have no activity for at least 60 days. label Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-issue-activity stale-issue This label is automatically assigned to issues that have no activity for at least 60 days.
Projects
None yet
Development

No branches or pull requests

4 participants