ppetzold / nestjs-paginate

Pagination and filtering helper method for TypeORM repositories or query builders using Nest.js framework :book::paperclip:
MIT License
438 stars 99 forks source link

[Feature Request] Nested relations filtering for one to many relations not eagerly loaded #814

Open Alsdrouf opened 11 months ago

Alsdrouf commented 11 months ago

If we use a filter that include a relation that is not eagery loaded or loaded it will make an error

it('should filter nested relations', async () => {
        const config: PaginateConfig<CatEntity> = {
            // relations: { home: { pillows: true } }, if we uncomment this relations it will work
            sortableColumns: ['id', 'name'],
            filterableColumns: { 'home.pillows.color': [FilterOperator.EQ] },
        }
        const query: PaginateQuery = {
            path: '',
            filter: { 'home.pillows.color': 'pink' },
        }

        const result = await paginate<CatEntity>(query, catRepo, config)

        const cat = clone(cats[1])
        const catHomesClone = clone(catHomes[1])
        const catHomePillowsClone = clone(catHomePillows[3])
        delete catHomePillowsClone.home

        catHomesClone.countCat = cats.filter((cat) => cat.id === catHomesClone.cat.id).length
        catHomesClone.pillows = [catHomePillowsClone]
        cat.home = catHomesClone
        delete cat.home.cat

        expect(result.meta.filter['home.pillows.color']).toStrictEqual('pink')
        expect(result.data).toStrictEqual([cat])
        expect(result.data[0].home).toBeDefined()
        expect(result.data[0].home.pillows).toStrictEqual(cat.home.pillows)
    })

Seems like this is an undocummented behavior, I tested version 6.1.0 and this isn't working so it's not a new update or something

Edit: It's my bad after testing out it was because I used vehicle=$null instead of vehicle.id=$null The error here is more about one to many relations filtering I think so it's a feature request

ppetzold commented 11 months ago

sorry I am bit lost. could you elaborate what's not working on one to many? what's current behavior vs what would you expect

Alsdrouf commented 11 months ago

sorry I am bit lost. could you elaborate what's not working on one to many? what's current behavior vs what would you expect

I took an existing test about nested relations and commented out the explicit relation loading in the paginate config.

And now the test fail probably because it's a one to many relations. So instead of doing like a where that would work with @vsamofal code, it currently fail to load the relations.

So I think the filter should work in that case of a not loaded relations that is filtered.

ppetzold commented 11 months ago

oh i see.. so an enhancement / fix for #770

ppetzold commented 11 months ago

ah misunderstood... you are referring to client side filtering. that should not be allowed for obvious security reasons. #770 refers to server side filtering / where clause

Alsdrouf commented 11 months ago

ah misunderstood... you are referring to client side filtering. that should not be allowed for obvious security reasons. #770 refers to server side filtering / where clause

I mean it's for allowed filterable columns so why would it be a security problem? So yes I think it's an improvement for #770 and a bug fix because for now it create an internal server as it is an allowed filterable columns but fail to join relation.

In the test I took, the test fail at paginate function not at expect, I forgot to mention that because I copied the whole test sorry

it('should filter nested relations', async () => {
        const config: PaginateConfig<CatEntity> = {
            // relations: { home: { pillows: true } }, if we uncomment this relations it will work
            sortableColumns: ['id', 'name'],
            filterableColumns: { 'home.pillows.color': [FilterOperator.EQ] },
        }
        const query: PaginateQuery = {
            path: '',
            filter: { 'home.pillows.color': 'pink' },
        }

        const result = await paginate<CatEntity>(query, catRepo, config)
        expect(result).toBeDefined()
})

so this test fail, but if we uncomment the relations config it work So in that case it will mean that we want all cat that is in a home that has a yellow pillow. But only the cat without the relations

vsamofal commented 11 months ago

yes, seems like a bug

Alsdrouf commented 11 months ago

Same thing for searchable columns, it's required to be loaded to be searchable without relations error

ppetzold commented 11 months ago

ah misunderstood... you are referring to client side filtering. that should not be allowed for obvious security reasons. #770 refers to server side filtering / where clause

I mean it's for allowed filterable columns so why would it be a security problem? So yes I think it's an improvement for #770 and a bug fix because for now it create an internal server as it is an allowed filterable columns but fail to join relation.

In the test I took, the test fail at paginate function not at expect, I forgot to mention that because I copied the whole test sorry


it('should filter nested relations', async () => {

        const config: PaginateConfig<CatEntity> = {

            // relations: { home: { pillows: true } }, if we uncomment this relations it will work

            sortableColumns: ['id', 'name'],

            filterableColumns: { 'home.pillows.color': [FilterOperator.EQ] },

        }

        const query: PaginateQuery = {

            path: '',

            filter: { 'home.pillows.color': 'pink' },

        }

        const result = await paginate<CatEntity>(query, catRepo, config)

        expect(result).toBeDefined()

})

so this test fail, but if we uncomment the relations config it work

So in that case it will mean that we want all cat that is in a home that has a yellow pillow. But only the cat without the relations

I would consider it a bad practice to allow client filter on fields which are not visible to him 😄

Would accept PR tho - since the current feature set anyway already allows to filter on hidden fields when using partial selection.