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

Relationship conditions #4963

Closed
lewispham opened this issue Nov 26, 2023 · 11 comments · Fixed by #5210
Closed

Relationship conditions #4963

lewispham opened this issue Nov 26, 2023 · 11 comments · Fixed by #5210
Labels
enhancement New feature or request

Comments

@lewispham
Copy link

lewispham commented Nov 26, 2023

Is your feature request related to a problem? Please describe.
It looks like mikro-orm does not support default conditions for a relationship. At this time, we'll have to pass the condition as init options when we want to load a collection under some conditions.

const errors = await collection.init({
  where: {
    type: "ERROR"
  }
})

It would be ways more convenient to be able to pass the condition beforehand, when we define the relationship for example.
Describe the solution you'd like
Pass the condition as Collection options

  @OneToMany(() => Log, log => log.user)
  errors = new Collection<Candidate>(this, {
    where: {
      type: "ERROR"
    }
  });

Describe alternatives you've considered
Define the condition as relationship definition

  @OneToMany({
    entity: () => Log,
    mappedBy: log => log.user,
    where: {
      type: "ERROR"
    }
  })
  errors = new Collection<Candidate>(this);
@lewispham lewispham added the enhancement New feature or request label Nov 26, 2023
@B4nan
Copy link
Member

B4nan commented Nov 26, 2023

Why don't you use a filter instead? This is exactly the use case for filters, I am not sure if we need more.

@lewispham
Copy link
Author

@B4nan Using filters still requires passing filter name into every single query. While my proposed approach requires no further steps. It's also a bit less readable since filter definition is at entity level, not property level.

@B4nan
Copy link
Member

B4nan commented Nov 26, 2023

You can enable a filter by default...

@B4nan
Copy link
Member

B4nan commented Nov 26, 2023

There are indeed some gotchas with filters, if you would use them here, they would always join the target relation, while what you are proposing would be applied only if you load the collection, right?

Btw we will need to go with the alternative approach, the second parameter of the constructor is already occupied, and it feels more correct anyway - it is metadata and as such it should be part of the entity definition, a property initializer is a runtime concept - you won't know anything about that unless you create some entity instance.

@lewispham
Copy link
Author

There are indeed some gotchas with filters, if you would use them here, they would always join the target relation, while what you are proposing would be applied only if you load the collection, right?

Yes, that's right.

Btw we will need to go with the alternative approach, the second parameter of the constructor is already occupied, and it feels more correct anyway - it is metadata and as such it should be part of the entity definition, a property initializer is a runtime concept - you won't know anything about that unless you create some entity instance.

I think that makes sense. You can consider supporting all query options instead of just where.

  @OneToMany({
    entity: () => Log,
    mappedBy: log => log.user,
    conditions: {
      where: {
        type: "ERROR"
      },
      orderBy: {id: "DESC"},
      limit: 10
    }
  })
  errors = new Collection<Candidate>(this);

@B4nan
Copy link
Member

B4nan commented Nov 26, 2023

You can already specify orderBy, for limit it would be pretty hard for anything else than the init call, this would require window functions or subquery joins.

@lewispham
Copy link
Author

Ah I see. It should be ok with just where and orderBy then.

@lewispham
Copy link
Author

Hi @B4nan, is there any chance that this feature will be implemented soon?

@B4nan
Copy link
Member

B4nan commented Jan 14, 2024

Yes, there is a chance, e.g. when someone sends a PR :]

I will need to prioritize bug fixing in the upcoming weeks and have a short vacation planned too, I won't have time fore feature development.

@lewispham
Copy link
Author

Ok I see. Thanks for the info.

@pohnean
Copy link

pohnean commented Jan 23, 2024

+1

B4nan added a commit that referenced this issue Feb 4, 2024
Collections can also represent only a subset of the target entities:

```ts
@entity()
class Author {

  @OneToMany(() => Book, b => b.author)
  books = new Collection<Book>(this);

  @OneToMany(() => Book, b => b.author, { where: { favorite: true } })
  favoriteBooks = new Collection<Book>(this);

}
```

This works also for M:N relations. Note that if you want to declare more relations mapping to the same pivot table, you need to explicitly specify its name (or use the same pivot entity):

```ts
@entity()
class Book {

  @manytomany(() => BookTag)
  tags = new Collection<BookTag>(this);

  @manytomany({
    entity: () => BookTag,
    pivotTable: 'book_tags',
    where: { popular: true },
  })
  popularTags = new Collection<BookTag>(this);

}
```

Closes #4963
B4nan added a commit that referenced this issue Feb 4, 2024
)

Collections can also represent only a subset of the target entities:

```ts
@entity()
class Author {

  @OneToMany(() => Book, b => b.author)
  books = new Collection<Book>(this);

  @OneToMany(() => Book, b => b.author, { where: { favorite: true } })
  favoriteBooks = new Collection<Book>(this);

}
```

This works also for M:N relations. Note that if you want to declare more
relations mapping to the same pivot table, you need to explicitly
specify its name (or use the same pivot entity):

```ts
@entity()
class Book {

  @manytomany(() => BookTag)
  tags = new Collection<BookTag>(this);

  @manytomany({
    entity: () => BookTag, 
    pivotTable: 'book_tags',
    where: { popular: true },
  })
  popularTags = new Collection<BookTag>(this);

}
```

Closes #4963
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants