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

HasManyThrough using HasMany #2359

Conversation

clayrisser
Copy link
Contributor

@clayrisser clayrisser commented Feb 9, 2019

This pull request attempts to set up the hasManyThrough relation using the hasMany relation. It does it using a through property that can be provided to the @hasMany() decorator.

I chose to setup the hasManyThrough this way in an attempt to stick to the existing loopback hasManyThrough spec.

https://loopback.io/doc/en/lb3/HasManyThrough-relations.html#defining-a-hasmanythrough-relation

import { Entity, model, hasMany, property } from '@loopback/repository';
import { Patient, Appointment } from '../models';

@model()
export class Physician extends Entity {
  @property({
    type: 'string',
    id: true
  })
  id?: string;

  @hasMany(() => Patient, { through: () => Appointment })
  patients: Patient[];
}

See also #2264
See also #2356

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@clayrisser
Copy link
Contributor Author

I've separated the hasMany and hasManyThrough repository factories. Please reference the following two comments.

#2264 (comment)

#2264 (comment)

@clayrisser clayrisser force-pushed the codejamninja/has-many-through-using-has-many branch from 7a47883 to 38bebcb Compare February 12, 2019 03:10
@clayrisser clayrisser force-pushed the codejamninja/has-many-through-using-has-many branch from 38bebcb to 38e2ab0 Compare February 12, 2019 03:11
@bajtos bajtos changed the title Codejamninja/has many through using has many HasManyThrough using HasMany Feb 12, 2019
@bajtos bajtos added community-contribution Relations Model relations (has many, etc.) labels Feb 12, 2019
@bajtos
Copy link
Member

bajtos commented Feb 12, 2019

@codejamninja thank you for the pull request, this is great effort!

I'll need to set aside larger chunk of time to review the changes, hopefully later this week 🤞

I've separated the hasMany and hasManyThrough repository factories.

Please add integration-level tests for the new repository factory to https://github.com/strongloop/loopback-next/blob/master/packages/repository/src/__tests__/integration/repositories/relation.factory.integration.ts

I think we should also copy https://github.com/strongloop/loopback-next/blob/master/packages/repository/src/__tests__/acceptance/has-many.relation.acceptance.ts into has-many-through.relation.acceptance.ts and write tests to cover the new "through" mode.

@clayrisser
Copy link
Contributor Author

Ok, thanks @bajtos

@clayrisser
Copy link
Contributor Author

I just added integration level tests and acceptance tests. They are all passing for me.

@clayrisser
Copy link
Contributor Author

Anyone wanting to get early access to the hasManyThrough relation (or anyone wanting to test it), you can install it the following way.

npm install --save @loopback/repository@git+https://[email protected]/codejamninja/loopback-next.git#npm/has-many-through

or add the following to your package.json file

{
  "dependencies": {
    "@loopback/repository": "git+https://[email protected]/codejamninja/loopback-next.git#npm/has-many-through"
  }
}

@clayrisser
Copy link
Contributor Author

I just added some basic usage docs for hasManyThrough

@raymondfeng
Copy link
Contributor

@codejamninja Thank you for the contribution. I'll try to find some time to go over the PR.

@bajtos bajtos self-assigned this Feb 14, 2019
Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start, @codejamninja!

I like @hasMany(() => Customer, {through () => Order}) API and also the repository APIs as used by relation consumers (e.g. in Controllers) 👍

However, I feel we need to rework and improve the design and the interactions between the repository factory, the has-many-through constrain and HasManyThroughRepository, PTAL at my last comment below.

@TomerSalton
Copy link

TomerSalton commented Feb 28, 2019

Hey,
I wanted to ask what is the difference in the functionality (not implementation) between hasManyThrough and hasAndBelongsToMany?

Edit:
Shouldn't we add a function 'findSourceByTargetID'?

@bajtos
Copy link
Member

bajtos commented Feb 28, 2019

I wanted to ask what is the difference in the functionality (not implementation) between hasManyThrough and hasAndBelongsToMany?

This is my understanding:

  • hasManyThrough requires the user to explicitly provide the "through" (junction) model.
  • hasAndBelongsToMany creates the "through" model automatically for the user.

@raymondfeng is that correct?

@TomerSalton
Copy link

I wanted to ask what is the difference in the functionality (not implementation) between hasManyThrough and hasAndBelongsToMany?

This is my understanding:

  • hasManyThrough requires the user to explicitly provide the "through" (junction) model.
  • hasAndBelongsToMany creates the "through" model automatically for the user.

@raymondfeng is that correct?

You are right in that aspect. What about the API though?

@bajtos
Copy link
Member

bajtos commented Feb 28, 2019

What about the API though?

I think that's something we need to find out while working on hasAndBelongsToMany relation. Do you anticipate any problems if we don't consider hasAndBelongsToMany requirements while working on hasManyThrough? If not, then let's move discussion about hasAndBelongsToMany to a more appropriate place please, to keep this PR focused on hasManyThrough.

@clayrisser
Copy link
Contributor Author

I don't anticipate any problems.

@bajtos
Copy link
Member

bajtos commented Mar 11, 2019

@codejamninja what's the status of this pull request? Do you need any additional information or feedback from us? I just want to ensure the progress is not blocked by us.

@clayrisser
Copy link
Contributor Author

@bajtos, thanks for asking. I am not blocked. The holdup is my schedule. I should get back to this sometime towards the end of the week.

@mhmnemati
Copy link

When this feature will merge to master ?

At the moment, this work is a community effort. @KalleV was kind enough to contribute the first step - see #4247.

Would you @koliberr136a1 like to join the forces and help @KalleV with the next steps?

I like a lot, but unfortunately I haven't the required informations about how to implement this feature

KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Dec 16, 2019
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Dec 17, 2019
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Dec 17, 2019
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Dec 17, 2019
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Dec 17, 2019
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Dec 17, 2019
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Jan 6, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Jan 6, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Jan 6, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Jan 6, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Jan 8, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Jan 8, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Jan 13, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Jan 14, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
KalleV pushed a commit to KalleV/loopback-next that referenced this pull request Jan 14, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
derdeka pushed a commit that referenced this pull request Jan 14, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of #2359 and implements a step from #2359 (comment).
dougal83 pushed a commit to dougal83/loopback-next that referenced this pull request Jan 23, 2020
The hasManyThrough repository interface defines the actions that can be performed on a many-to-many model relation.

This PR is a continuation of loopbackio#2359 and implements a step from loopbackio#2359 (comment).
@MacGyver26
Copy link

Hello guys !

Do you know when this feature is going to be available in the framework ? thanks in advance.

@dhmlau
Copy link
Member

dhmlau commented Mar 31, 2020

@MacGyver26, I believe #4438 takes over the implementation of the hasManyThrough relation.

@agnes512
Copy link
Contributor

The implementation and documentation are merged, closing the issue.

@agnes512 agnes512 closed this Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted major Relations Model relations (has many, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.