Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

test case for filtering null and not null on relation #539

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

daniel-maegerli
Copy link

As discussed, I've written a new test case for checking $null on relations, and changed the existing test case for checking properly on $not:$null on relations. The first test case is failing as expected/described within the issue. Since this is my first time working on this repo (and submitting a PR to an open source project in general), let me know if this is fine.

Issue #538

@ppetzold
Copy link
Owner

Oh I see. I think the issue is simply that we always LEFT JOIN. So nulled relations will be listed as well. I suppose it must be INNER JOIN to achieve what you expect.

I am currently only working on a sequelize project. Sequelize has the require property on joins. Looking over TypeORM's docs I cannot find the equivalent. Do you know what TypeORMs notation for this is? If so, we can try to imitate.

@Helveg Helveg force-pushed the issue-538-test-case-null-on-relations branch from 58ee944 to be99407 Compare September 29, 2024 14:04
@Helveg
Copy link
Collaborator

Helveg commented Oct 4, 2024

@ppetzold I fixed the null queries on relationships by storing that when we use a relationship in a filter that it should be inner joined. On top of that I allow the user to configure the default join method, and the join method per column, since this all converged on the same approach I needed to extend for building the relationships either way.

Oh and and !!! IMPORTANT !

Important

It also fixes an error in the following scenario:

{ filterableColumns: ['home.countCat'] }

Using that filter without including the home in relations used to throw errors like:

column "__root_home_rel_countCat" does not exist

That's fixed now! Whenever you use a relationship in filter it is automatically inner joined :)

This PR is ready for review :)

@Helveg Helveg requested review from Helveg and ppetzold October 4, 2024 16:22
@Helveg
Copy link
Collaborator

Helveg commented Jan 6, 2025

@ppetzold can you take a look here?

@Helveg
Copy link
Collaborator

Helveg commented Feb 19, 2025

@ppetzold ping

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants