fix(todo-list tutorial): add missing code to the TodoRepository

[hayk94]

Code for adding todoList property to the TodoRepository was missing.

[bajtos]

Thank you for the pull request, @hayk94.

IIRC from #3171, it was never our intention to show the full repository class in this doc page. When the code shows the full source file, it's difficult to see which part is important (added while introducing inclusion of related models) and which parts are just the support.

Could you please provide more details about your intent and what is your reasoning for the proposed change? What did you try to accomplish and what was missing in our documentation?

[hayk94]

@bajtos Hey!
The intent was not the the full code.

The issue is here
https://loopback.io/doc/en/lb4/todo-list-tutorial-repository.html

When it says

Let’s do the same on the TodoRepository:

src/repositories/todo.repository.ts

async find(
  filter?: Filter<Todo>,
  options?: Options,
): Promise<TodoWithRelations[]> {
  // Prevent juggler for applying "include" filter
  // Juggler is not aware of LB4 relations
  const include = filter && filter.include;
  filter = {...filter, include: undefined};

  const result = await super.find(filter, options);

  // poor-mans inclusion resolver, this should be handled by DefaultCrudRepo
    // and use `inq` operator to fetch related todo-lists in fewer DB queries
    // this is a temporary implementation, please see
    // https://github.com/strongloop/loopback-next/issues/3195
  if (include && include.length && include[0].relation === 'todoList') {
    await Promise.all(
      result.map(async r => {
        r.todoList = await this.todoList(r.id);
      }),
    );
  }

  return result;
}

async findById(
  id: typeof Todo.prototype.id,
  filter?: Filter<Todo>,
  options?: Options,
): Promise<TodoWithRelations> {
  // Prevent juggler for applying "include" filter
  // Juggler is not aware of LB4 relations
  const include = filter && filter.include;
  filter = {...filter, include: undefined};

  const result = await super.findById(id, filter, options);

  // poor-mans inclusion resolver, this should be handled by DefaultCrudRepo
  // and use `inq` operator to fetch related todo-lists in fewer DB queries
  // this is a temporary implementation, please see
  // https://github.com/strongloop/loopback-next/issues/3195
  if (include && include.length && include[0].relation === 'todoList') {
    result.todoList = await this.todoList(result.id);
  }

  return result;
}

On line result.todoList = await this.todoList(result.id); we get error that this.todoList property is missing in the class. And in the tutorial there is no way mentioned how to add this property.

After looking at the exapmle app I found that this piece of code is missing.

export class TodoRepository extends DefaultCrudRepository<
  Todo,
  typeof Todo.prototype.id,
  TodoRelations
> {
  public readonly todoList: BelongsToAccessor<
    TodoList,
    typeof Todo.prototype.id
    >;

  constructor(
    @inject('datasources.db') dataSource: DbDataSource,
    @repository.getter('TodoListRepository')
    protected todoListRepositoryGetter: Getter<TodoListRepository>,
  ) {
    super(Todo, dataSource);
    this.todoList = this.createBelongsToAccessorFor(
      'todoList',
      todoListRepositoryGetter,
    );
  }

Here we define the todos property with BelongsToAccessor, and there was nothing mentioned about this in the doc.

[bajtos]

Thank you @hayk94 for the explanation, I understand the issue you encountered and I agree with you to improve our docs!

At the moment, we are working on a better solution for inclusion of related models, we have already few building blocks in place. It's not necessary to override all find* methods, repository classes can use the recently introduced method includeRelatedModels instead.

Setting that aside, we still need to initialize this.todoList property. We already have instructions how to initialize this.todo property in TodoListRepository, see https://loopback.io/doc/en/lb4/todo-list-tutorial-repository.html#create-your-repository. Can we perhaps simply add a short paragraph explaining that relational properties need to be defined in the constructor and refer readers to the text above?

@nabdelgadir @agnes512 thoughts?

[agnes512]

Hi @hayk94! Thank you for the contribution. I agree with you that even the idea of hasMany and belongsTo are similar, it's better to have it in the tutorial.

Let's rebase it, and simplify it as @bajtos suggested. Thanks!

[nabdelgadir]

Hi @hayk94, if you need any help, let us know.

[hayk94]

Hey guys!
@nabdelgadir @agnes512

Is this still actual? It seems the tutorial already changed?

[agnes512]

We've updated the docs for the latest code and new features. It should be more consistent now.

[nabdelgadir]

Hi @hayk94! So we've made some changes to the file, but we still want to include initializing this.todoList as from @bajtos' comment:

Setting that aside, we still need to initialize this.todoList property. We already have instructions how to initialize this.todo property in TodoListRepository, see https://loopback.io/doc/en/lb4/todo-list-tutorial-repository.html#create-your-repository

Would it be possible for you to rebase your PR and simplify it to do the above? Thanks!

[dhmlau]

@hayk94, you're right. I'm afraid the tutorial has changed quite a bit since you first created the PR. The tutorial makes use of the lb4 relation command now to specify the relation instead of modifying the code. I'd like to close this PR because the content is no longer applicable. Thank you so much for your PR anyway. If you see anything the tutorial needs to improve, please feel free to submit a new PR. Thanks again!