loopbackio / loopback-next

LoopBack makes it easy to build modern API applications that require complex integrations.
https://loopback.io
Other
4.96k stars 1.07k forks source link

Docs: Update TodoList example and model relations pages to use `lb4 relation` command #3742

Closed ichtestemalwieder closed 5 years ago

ichtestemalwieder commented 5 years ago

Hello,

I rewrote the issue, as I realized, that LB V4 is still in it's infancy and will become more declarative in the future... But I am not the only one complaining, quoting:

"... An example? This is the code necessary to define a one-to-many relationship in Loopback 3:

{
  "name": "TodoList",
  "base": "PersistedModel",
  "relations": {
    "todos": {
      "type": "hasMany",
      "model": "Todo",
      "foreignKey": "todoListId"
    }
  }
}

Here is the same type of relationship defined in Loopback 4:

import {
  DefaultCrudRepository,
  HasManyRepositoryFactory,
  repository,
} from '@loopback/repository';
import {TodoList, Todo} from '../models';
import {DbDataSource} from '../datasources';
import {inject, Getter} from '@loopback/core';
import {TodoRepository} from './todo.repository';

export class TodoListRepository extends DefaultCrudRepository<
  TodoList,
  typeof TodoList.prototype.id
> {
  public readonly todos: HasManyRepositoryFactory<
    Todo,
    typeof TodoList.prototype.id
  >;

  constructor(
    @inject('datasources.db') dataSource: DbDataSource,
    @repository.getter(TodoRepository)
    protected todoRepositoryGetter: Getter<TodoRepository>,
  ) {
    super(TodoList, dataSource);
    this.todos = this._createHasManyRepositoryFactoryFor(
      'todos',
      todoRepositoryGetter,
    );
  }
}

..."

Examples normally showcase the "greatness" of something. But the concrete TODO List Example rather shows, that is way more complex to setup a oneToMany Relationship than in any other JS framework. Also in Spring Boot such an example is way more concise and intuitive. So the question is, how this can be solved.

A concrete example what feels so unintuitive (in the TODO List Example):

@model()
export class TodoList extends Entity {
  // ...properties defined by the CLI...

  @hasMany(() => Todo)
  todos?: Todo[];

  // ...constructor def...
}

export interface TodoListRelations {
  todos?: TodoWithRelations[];
}

export type TodoListWithRelations = TodoList & TodoListRelations;
@model()
export class Todo extends Entity {
  // ...properties defined by the CLI...

  @belongsTo(() => TodoList)
  todoListId: number;

  // ...constructor def...
}

export interface TodoRelations {
  todoList?: TodoListWithRelations;
}

export type TodoWithRelations = Todo & TodoRelations;

I am new to TS, but most likely this is more of an architectural problem: Why do we need extra interfaces for the relationships and type aliasing? Why is not information about persistance put into decorators and the models just model the relation on an object/entity level, so you can get rid of the type alias and the extra interfaces (like below in pseudo code)...?

@model()
export class TodoList extends Entity {
  // ...properties defined by the CLI...

  @hasMany(() => Todo)
  todos?: Todo[];

}
@model()
export class Todo extends Entity {
  // ...properties defined by the CLI...

  @belongsTo( ( field={todoListId: number}  ) => TodoList)
  todoList: TodoList;

}

This is not meant as a critique. I looked at many frameworks and was so happy finding one that has features like DI/IOC, Repositories, Decorators, TypeScript... But than looking at the examples I got so disappointed as it feels not intuitive. So the question is, how can this be improved. Thanks.

Acceptance Criteria

added by @dhmlau per this comment Now that we have lb4 relation, we should make sure our docs reflect this change.

dougal83 commented 5 years ago

You could always mount lb3 app in lb4. Personally I absolutely love Typescript implementation. JS is teribad.

dhmlau commented 5 years ago

@ichtestemalwieder, thanks for your feedback. The TodoList example was created before we have the tooling around relations, so the developer experience is a bit clumsy that way.

With the lb4 relation CLI, we've simplified the DX a bit. If there is Customer which hasMany Order, we can use the lb4 relation to specify the relationship after you've created the necessary model and repository class. It will add the @hasMany decorators (or whatever you pick in the CLI) in the model.

$ lb4 relation
? Please select the relation type hasMany
? Please select source model Customer
? Please select target model Order
? Foreign key name to define on the target model customerId
? Source property name for the relation getter orders
identical src/controllers/customer-order.controller.ts

I think our docs should be updated to reflect that. If you agree, I'd like to convert this GitHub issue into a docs issue to update the TodoList example or possibly docs pages related to model relations. What do you think?

ichtestemalwieder commented 5 years ago

Thanks very much for the answer. This is great news, as it defenitely solves one part of the problem

Yes of course the ticket can be converted to a doc issue.

The Docs in general are great! But that concrete example lacks any background explanation why there are several dedicated Classes/Interfaces for the relations. And why it is implemented as "Constrained Repositories". I know what they are doing, but I still wonder why such a complex construct is used. (Wouldn't it be possible to have a normal Domain-Model and model the relation properties with a function that then does Lazy Loading on demand?...) If this would be explained, then maybe it would make sense and all be easier to understand/learn. The domain model (with extracting the relations) feels more like an anemic domain model... Thanks very much.

dhmlau commented 5 years ago

Thanks @ichtestemalwieder. I think another way to make the documentation clearer is to separate the usage and the minimal concept/info you need to know in order to use it from the detailed explanation. Anyway, I'd like to invite you to review the PR(s) coming from this task. :)

I've also added the acceptance criteria. Please let me know if I miss anything. Thanks.

sestienne commented 5 years ago

Hi, I follow the tutorial todo & todo list. I success to run it but I also find that relations are a little complex to understand but how it's designed offers a lot of possibilities. There is something what I failed to do until now : I would like thanks to the request : "POST /todo-lists" create a new todolist entry but directly with some todos in a single request. Is it possible ? For that, I changed the file "todo-list.controller.ts" by adding "includeRelations: true" in "getModelSchemaRef" function but when I call the request with for example this json : { "title": "shopping list", "color": "yellow", "todos": [ { "title": "tomato", "desc": "green zebra", "isComplete": false } ] } it returns : { "error": { "statusCode": 422, "name": "ValidationError", "message": "The 'TodoList' instance is not valid. Details: 'todos' is not defined in the model (value: undefined).", "details": { "context": "TodoList", "codes": { "todos": [ "unknown-property" ] }, "messages": { "todos": [ "is not defined in the model" ] } } } }

What is wrong in my changes ? Is it possible to do it ? Thanks!

dhmlau commented 5 years ago

@sestienne, we just finished the inclusionResolver for the model relations in https://github.com/strongloop/loopback-next/issues/3448 and https://github.com/strongloop/loopback-next/issues/3447, and will be updating the docs via PR https://github.com/strongloop/loopback-next/pull/3774.

Once we've published a release (possibly very soon), it's possible to do it.

cc @nabdelgadir @agnes512

agnes512 commented 5 years ago

@sestienne Hi! unfortunately we don't have such a way to create an instance along with its related instance in one post request. There are some reasons that we don't support. For example, users might use two different databases for two different models. So we want it to be more flexible.

As for the inclusion resolver @dhmlau mentioned above, the new feature allows us to query related instances easily.

{
   "title": "a todoList", 
  "id": 1
  "todos": [ 
    { "desc": "this todo is related to this todoList 1", "todoListId": 1 },
    { "desc": "this todo is related to this todoList 1", "todoListId": 1} 
  ]
} ,
{
   "title": "another  todoList", 
  "id": 2
  "todos": [ 
    { "desc": "this todo is related to this todoList 1", "todoListId": 2 }
  ]
} 

Please let me know if you have any questions.

sestienne commented 5 years ago

@dhmlau @agnes512 Thanks a lot for your messages, it's a very good news !

While I was waiting for a response, I've tried on a poc workaround but it's not clean nor graceful. Here is an example. The controller uses two repositories to do this :

let content = JSON.parse(JSON.stringify(news.content)); // get content
let shortNews = JSON.parse(JSON.stringify(news)); // clone object
delete shortNews.content; // remove content part from news
let resultNews = await this.newsRepository.create(shortNews); // create news in DB
content.news_id = resultNews.id; // retrieve news id
let resultContent = await this.contentRepository.create(content); // create content in DB
resultNews.content= resultContent; // merge result to return it
return resultNews;

Note that the block doesn't actually manage transactions.