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

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

Closed
2 tasks
ichtestemalwieder opened this issue Sep 14, 2019 · 8 comments · Fixed by #4126
Closed
2 tasks
Assignees
Labels
developer-experience Issues affecting ease of use and overall experience of LB users Docs

Comments

@ichtestemalwieder
Copy link

ichtestemalwieder commented Sep 14, 2019

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

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

@ichtestemalwieder ichtestemalwieder changed the title Feature Request: Make Loopback (V4) easy and fun again Feature Request: Make Loopback (V4) (Examples) more intuitive Sep 15, 2019
@dhmlau
Copy link
Member

dhmlau commented Sep 18, 2019

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

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 dhmlau added Docs developer-experience Issues affecting ease of use and overall experience of LB users and removed feature labels Sep 18, 2019
@dhmlau
Copy link
Member

dhmlau commented Sep 18, 2019

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.

@dhmlau dhmlau added the 2019Q4 label Sep 18, 2019
@dhmlau dhmlau changed the title Feature Request: Make Loopback (V4) (Examples) more intuitive Docs: Update TodoList example and model relations pages to use lb4 relation command Sep 18, 2019
@sestienne
Copy link

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

dhmlau commented Sep 23, 2019

@sestienne, we just finished the inclusionResolver for the model relations in #3448 and #3447, and will be updating the docs via PR #3774.

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

cc @nabdelgadir @agnes512

@agnes512
Copy link
Contributor

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

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Issues affecting ease of use and overall experience of LB users Docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants