rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Add relation support in cli

Open devsecur opened this issue 5 years ago • 15 comments

Following Issue 4695, I created a pull request for cli relation attribute support

https://github.com/devsecur/rfcs/blob/master/rfcs/0003-cli-generate-relations.md

devsecur avatar Dec 18 '19 07:12 devsecur

Hi @devsecur,

Thank you for this RFC.

I think this feature doesn't even require an RFC as it wouldn't change any of the core features of Strapi nor change any of the core concepts :)

We made some changes to the cli code recently that changes the examples you shared to make the cli simpler. And reworked the content-type-builder code to handle the creation of the models. You might be able to pull that logic outside of the plugin to reuse it for the cli ;)

alexandrebodin avatar Dec 18 '19 08:12 alexandrebodin

Hi @alexandrebodin,

that's awesome. I create a RFC following @lauriejim suggestion in 4695 after created an enhancement ticket on the main repository.

Which branch in the main repository contains the most recent changes of the cli? I would have a look at it and port the logic of the content-type-builder plugin if necessary to enhance the latest state of the cli - did i get you right?

Would I then make a pull on the main repository with an enhanced cli if successful?

devsecur avatar Dec 18 '19 09:12 devsecur

I suggest that to define the attribute format attribute_name:attribute_type currently. But what would it be for relations?

lauriejim avatar Dec 18 '19 09:12 lauriejim

For relations, we have a name, a type, the related api and the form of relation (many-to-many, one-to-many,...). This seems to be more complex.

Following the notation:

attibute_name:attribute_type:form:related

On the other hand, the cli has options. These could be used for attributes:

attribute_name:attribute_type --form form --related name

I have a bias for the second

devsecur avatar Dec 18 '19 09:12 devsecur

Using that complexe of a format is going to get really difficult to understand.

We might be better of just providing a --file option to pass a full schema. This would meet the need for automation and be a lot easier to use.

For a user wanting to do it manually I would go for a step by step prompt to be a lot more reliable and user friendly

alexandrebodin avatar Dec 18 '19 09:12 alexandrebodin

As the main propose for my request was automation, I created a json file containing the scheme and a script parsing this file and executing strapi cli. The file was the following:

{
  "api": {
    "person": {
      "name": {
        "type": "string"
        }
    },
    "location": {
      "owner": {
        "type": "relation",
        "target": "person",
        "form": "onetoone"
      }
    }
  },
  "controller": {},
  "model": {},
  "service": {},
  "policy": {},
  "plugin": {},
  "install": {},
  "uninstall": {}
}

As i didn't needed the other commands right now, I only used there the options following the cli documentation.

devsecur avatar Dec 18 '19 10:12 devsecur

To be more complete, maybe the attributes should be in an own "attribute" node within the api element and also add plugin and tpl

devsecur avatar Dec 18 '19 10:12 devsecur

For a user wanting to do it manually I would go for a step by step prompt to be a lot more reliable and user friendly

The second form

attribute_name:attribute_type --form form --related name

could trigger the step by step prompt, if the options are missing to be more reliable and user friendly but automatable at the same time

devsecur avatar Dec 18 '19 10:12 devsecur

Running a prompt if parameters are missing will create problem for automation. We should run by default in interactive mode or pass a file. or an encoded json maybe.

Using cli params like --form or --related will be a problem if you want to create multiple relations. I really don't think it makes sense to allow for such a complexe cli when we can just load from a file.

We could have 3 ways to run the generator

  • strapi generate:model would run with a prompte
  • strapi generate:model --name modelName -f model.json would parse the file
  • strapi generate:model --name modelName -json '{"title": {"type": "string"}, "rel": { "model": "user"}}'

We could even imagine feeding it as stdin like

cat model.json | strapi generate:model

This could be really useful for automation

alexandrebodin avatar Dec 18 '19 16:12 alexandrebodin

Yeah sounds great for me!

devsecur avatar Dec 18 '19 16:12 devsecur

Would you be willing to update the RFC to reflect the proposed changes. We need to figure out the way we can declare relations. You should look at how the content-type-buidler works to see that we delcare relation as follows:

Example model

{
  "attributes": {
    "category": {
      "nature": "oneToOne",
      "target": "application::category.category",
      "relatedAttribute": "restaurant"
    }
  }
}

alexandrebodin avatar Dec 18 '19 16:12 alexandrebodin

Would you be willing to update the RFC to reflect the proposed changes. We need to figure out the way we can declare relations. You should look at how the content-type-buidler works to see that we delcare relation as follows:

Sure @alexandrebodin , I tried to update the rfc with the consense on file and interactive cli. I also included typed following content-type-builder types and relations

But i'm not sure, if I understand the content-type-buidler plugin and the need of definition in rfc right - Is my adjustment sufficient? I didn't understand the type "component" and "dynamiczone"

I didn't understand some attributes in relations such as "columnName" and "targetColumnName" as well. Isn't the reverse attribute in the related api generated automatically following a naming convention? Do I have to name the relatedAttribute only in the relating api or do I have to create the attribute first in the related api and then relate the attribute in the relating api? Sorry for the confusion.

devsecur avatar Dec 19 '19 09:12 devsecur

Would you be willing to update the RFC to reflect the proposed changes. We need to figure out the way we can declare relations. You should look at how the content-type-buidler works to see that we delcare relation as follows:

Example model

{
  "attributes": {
    "category": {
      "nature": "oneToOne",
      "target": "application::category.category",
      "relatedAttribute": "restaurant"
    }
  }
}

@alexandrebodin I would love to see this schema change, it would make the definitions of relations so much more clear. Although the actual migration process for existing users will be.... painful. If possible we should also consider an automated script to handle this.

derrickmehaffy avatar Jun 22 '20 15:06 derrickmehaffy

CLA assistant check
All committers have signed the CLA.

strapi-cla avatar Jan 21 '21 13:01 strapi-cla

what is the status on this ?

Eggwise avatar Jul 03 '21 16:07 Eggwise