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

Implement hasAndBelongsToMany relation #5383

Closed
wants to merge 1 commit into from
Closed

Implement hasAndBelongsToMany relation #5383

wants to merge 1 commit into from

Conversation

frbuceta
Copy link
Contributor

@frbuceta frbuceta commented May 11, 2020

I have been testing relationships for a while and have decided to take the step of creating a draft. So you can advise me.

9830483

Problems to solve:

  • Delete through entities when target entities are deleted
  • Build through repository inside

closes #2308
closes #2043

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

  • 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

👉 Check out how to submit a PR 👈

@frbuceta frbuceta marked this pull request as draft May 11, 2020 01:57
@dhmlau dhmlau added Relations Model relations (has many, etc.) community-contribution labels May 11, 2020
@agnes512
Copy link
Contributor

@frbuceta Hey! Thanks for starting on this 😄 I happen to work on HasManyThrough helpers & tests #5354. I think these two relations have a lot in common. It'd be good to check out to prevent duplication.

Also it's better to keep the scope of the PR small to make it easier for review, thanks!

@frbuceta
Copy link
Contributor Author

@frbuceta Hey! Thanks for starting on this 😄 I happen to work on HasManyThrough helpers & tests #5354. I think these two relations have a lot in common. It'd be good to check out to prevent duplication.

Also it's better to keep the scope of the PR small to make it easier for review, thanks!

I understand that the hasManyThrough relationship has data and the hasAndBelongsToMany does not

@bajtos
Copy link
Member

bajtos commented May 15, 2020

IIRC, in LoopBack 3, hasAndBelongsToMany relation is implemented as a special case of hasManyThrough relation, where the API for setting up hasAndBelongsToMany relation only defines a new through model and delegates all work to hasManyThrough implementation. I think we should follow the same design in LoopBack 4 too, unless there are compelling reasons why that would be a bad idea?

/cc @raymondfeng

@frbuceta
Copy link
Contributor Author

IIRC, in LoopBack 3, hasAndBelongsToMany relation is implemented as a special case of hasManyThrough relation, where the API for setting up hasAndBelongsToMany relation only defines a new through model and delegates all work to hasManyThrough implementation. I think we should follow the same design in LoopBack 4 too, unless there are compelling reasons why that would be a bad idea?

/cc @raymondfeng

Then, I close the PR and when the HasManyThrough is implemented I look at how to do If nobody does it before

@frbuceta frbuceta closed this May 23, 2020
@frbuceta frbuceta deleted the feat/has-and-belongs-to-many branch May 23, 2020 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Relations Model relations (has many, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Model Relation] hasAndBelongsToMany Many to many relation (hasAndBelongsToMany)
4 participants