Skip to content

[CLI] add lb4 relation command #1359

New issue
New issue
Closed
Closed
[CLI] add lb4 relation command#1359
Assignees
TomerSaltonorshlom
Labels
CLIJugglerRelationsModel relations (has many, etc.)feature parity
Milestone
May 2019 milestone

Description

@dhmlau
dhmlau
opened on May 24, 2018

Description / Steps to reproduce / Feature proposal

  • CLI command lb4 relation or the likes to wire up related models (think of LB3)
    • Creating CLI templates which expose relation related CRUD methods during controller method generation
    • Adding CLI prompts which create/decorate relational properties etc.

See also #695

LoopBack 3 has models relations definition in JSON file or in JavaScript code
We would like to define models relations in LoopBack 4 as well. Definition of relations need to auto generate the api services.

cc @b-admike

Acceptance Criteria (@virkt25)

  • Add a new lb4 relation command
  • Prompt the user to select relation type (current support is: hasMany, belongsTo in progress).
  • For hasMany ask the following prompts:
    • Model to create a relationship FROM: Provide list of available models.
    • Model to create a relationship TO: Provide list of available models (minus the one selected in the above).
    • Property name for the relation: Recommend the plural form of the TO model.
    • Enter name of custom foreign key (optional).
  • Prompt the user to see if they want to create / update a Repository (see [CLI] add lb4 repository command #1588 for prompts you should ask / need to generate a repository)
  • Prompt the user to see if they want to create a controller to expose the relation over API
  • Add the new property to the FROM model that is decorated with the hasMany decorator. Will need to import the TO model here.
  • If a custom foreign key is defined, check to see if it exists in the TO model. If not, add an optional property.
  • If the want to create / update repository: Check if a related repository exists. If not, create one which has the HasManyRepositoryFactory and it's implementation. If it exists, use AST to add the new property and code related to power the HasMany relation. See: https://github.com/strongloop/loopback-next/blob/master/examples/todo-list/src/repositories/todo-list.repository.ts for details.
  • If a new controller is wanted, create a new controller fromModel-toModel.controller.ts that exposes GET, POST, PATCH, DEL. Template can be based on https://github.com/strongloop/loopback-next/blob/master/examples/todo-list/src/controllers/todo-list-todo.controller.ts.
  • Tests + Docs + --config support (JSON Definition of Model) and --yes support (Skip relation property name, foreign key, assume they want to create/update repo, create controller).
  • Should also support the following CLI Syntax: lb4 relation fromModel hasMany toModel ... with --yes this is 🔥
  • Blog

Note For Estimation
CHALLENGE: Updating a pre-defined Model is tricky if we are manipulating the file as text ... user could've defined multiple Classes in a single file, etc. The best approach will be to use Abstract Syntax Tree (AST).

Activity

  1. dhmlau
    added
    non-DP3
    on May 24, 2018
  2. bajtos
    changed the title [-][CLI] Relation [/-] [+][CLI] Relations[/+] on Jun 25, 2018
  3. bajtos
    added
    Core-GA
    CLI
    p2
    Juggler
    on Jun 25, 2018
  4. dhmlau
    added
    needs grooming
    on Aug 11, 2018

  5. bajtos commented on Aug 13, 2018

    @bajtos
    bajtos
    on Aug 13, 2018
    Member

    As for the controller generator, our current approach is to have one controller for each relation. For example, when Customer has many orders, there will be three controllers: CustomerController, OrderController and CustomerOrderController.

    The reason for this split is to keep application code clean and avoid controllers with too many methods. Consider the case where a model has 3-5 relations. If we kept all relation endpoints in a single controller, the controller could easily end up with more than 20 methods.

    As a nice side effect, our code generator will be also simpler to implement because adding a relation means adding a new controller file instead of editing existing controller classes.

  6. virkt25
    changed the title [-][CLI] Relations[/-] [+][CLI] `lb4 relation` command[/+] on Aug 21, 2018
  7. virkt25
    changed the title [-][CLI] `lb4 relation` command[/-] [+][CLI] add `lb4 relation` command[/+] on Aug 21, 2018
  8. dhmlau
    removed
    needs grooming
    non-DP3
    on Aug 22, 2018

  9. virkt25 commented on Aug 28, 2018

    @virkt25
    virkt25
    on Aug 28, 2018
    Contributor

    Rejecting for now. While the task is well groomed, we need consensus on if we are going with AST Approach, Declarative Support, etc. Also need a spike on AST to figure out feasibility and build expertise. Also, this task should be broken down into smaller tasks.

  10. raymondfeng commented on Aug 29, 2018

    @raymondfeng
    raymondfeng
    on Aug 29, 2018
    Contributor

    We can possibly leverage prettier for TypeScript source code manipulation. I built a quick PoC as follows:

    const prettier = require('prettier');
const util = require('util');

async function transform() {
  const prog = await prettier.format(
    `/** class */
    class X {
      a: string;
      b(): string {
        return '1';
      }
    }`,
    {
      parser(text, {typescript}) {
        const ast = typescript(text);
        console.log(util.inspect(ast, {depth: 5}));
        ast.body[0].id.name = 'Y';
        return ast;
      },
    },
  );
  console.log(prog);
}

transform();

    You'll now get:

    /** class */
class Y {
  a: string;
  b(): string {
    return "1";
  }
}

  11. marioestradarosa commented on Aug 29, 2018

    @marioestradarosa
    marioestradarosa
    on Aug 29, 2018
    Contributor

    How about these ones ?

    https://github.com/SBoudrias/AST-query
    https://github.com/estools/esquery

  12. 7 remaining items

  13. dhmlau commented on Oct 27, 2018

    @dhmlau
    dhmlau
    on Oct 27, 2018
    MemberAuthor

    Per the discussion in our previous estimation meeting, this task would involve changing of the ts file. Therefore, we've created the spike #1656 to see what's the best approach. With this, I'm moving this task back to Needs Priority.

  14. dhmlau
    removed
    post-GA
    on Oct 27, 2018
  15. dhmlau
    added
    feature parity
    on Nov 15, 2018
  16. dhmlau
    added
    RelationsModel relations (has many, etc.)
    on Nov 28, 2018

  17. orshlom commented on Dec 16, 2018

    @orshlom
    orshlom
    on Dec 16, 2018

    Hi @dhmlau,
    I did some research on creating additional relations for models and repositories using TypeScript AST manipulation, following #1656 spike, and made a comment with my findings.

    I am more than confident I can implement the CLI command for relations following your guidelines.
    I would be happy to be contribute and be assigned on this issue.

  18. orshlom commented on Dec 27, 2018

    @orshlom
    orshlom
    on Dec 27, 2018

    @dhmlau
    If a user wants to specify in the FROM model a property other than its ID property (keyFrom attribute), should it be supported in the CLI as well?

  19. bajtos
    assigned
    orshlom
    on Jan 7, 2019

  20. bajtos commented on Jan 7, 2019

    @bajtos
    bajtos
    on Jan 7, 2019
    Member

    I am more than confident I can implement the CLI command for relations following your guidelines.
    I would be happy to be contribute and be assigned on this issue.

    Sounds great, assigning the issue to you :)

    If a user wants to specify in the FROM model a property other than its ID property (keyFrom attribute), should it be supported in the CLI as well?

    Personally, I prefer to work in small increments and start with the simplest possible solution. I think that means no user-configurable FROM/keyFrom property, what do you think?

    For longer term, I think it makes sense to eventually extend lb4 relation command to allow callers to customize all aspects of the relation to be created, including a custom keyFrom setting.

  21. bajtos
    assigned
    TomerSalton
    on Jan 10, 2019

  22. TomerSalton commented on Jan 10, 2019

    @TomerSalton
    TomerSalton
    on Jan 10, 2019

    Thanks for assigning me into this task :)
    I have a question to have a better understanding of the relationships design.

    From the documentation and from some tests I performed in my environment I realized:

    1. hasMany doesn't require belongsTo relation on the target model.
    2. hasOne requires belongsTo relation on the target model.
    3. belongsTo doesn't require hasMany/hasOne on the target model.

    I guess my question is why hasMany is different from hasOne in the aspect of requiring the belongsTo in the target model.

  23. dhmlau
    added
    community contribution
    on Jan 11, 2019

  24. elv1s commented on Jan 29, 2019

    @elv1s
    elv1s
    on Jan 29, 2019
    Contributor

    What is special about belongsTo that it is a requirement of hasOne? I'm pretty sure that wasn't a requirement in LB3.

  25. bajtos commented on May 24, 2019

    @bajtos
    bajtos
    on May 24, 2019
    Member

    Closing as done via #2426

  26. bajtos
    closed this as completedon May 24, 2019
  27. bajtos
    added this to the May 2019 milestone milestone on May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

CLIJugglerRelationsModel relations (has many, etc.)feature parity

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions