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

Add relation support in cli #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

devsecur
Copy link

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

@alexandrebodin
Copy link
Member

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 ;)

@devsecur
Copy link
Author

devsecur commented Dec 18, 2019

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?

@lauriejim
Copy link

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

@devsecur
Copy link
Author

devsecur commented Dec 18, 2019

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

@alexandrebodin
Copy link
Member

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

@devsecur
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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

@alexandrebodin
Copy link
Member

alexandrebodin commented Dec 18, 2019

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

@devsecur
Copy link
Author

Yeah sounds great for me!

@alexandrebodin
Copy link
Member

alexandrebodin commented Dec 18, 2019

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"
    }
  }
}

@devsecur
Copy link
Author

devsecur commented Dec 19, 2019

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.

@derrickmehaffy
Copy link
Member

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.

@alexandrebodin alexandrebodin changed the title Add rfc for relation support in cli Add relation support in cli Sep 15, 2020
@strapi-cla
Copy link

strapi-cla commented Jan 21, 2021

CLA assistant check
All committers have signed the CLA.

@Eggwise
Copy link

Eggwise commented Jul 3, 2021

what is the status on this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants