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

Issue with pivot table #4

Open
lokendrajoshi54 opened this issue Feb 11, 2019 · 14 comments
Open

Issue with pivot table #4

lokendrajoshi54 opened this issue Feb 11, 2019 · 14 comments
Labels
possible bug Seems to be a bug; requires additional checking requires upstream fix Requires fixing in the @adonis/lucid package

Comments

@lokendrajoshi54
Copy link

While fetching relations facing issue with pivot table. When I use "whereHas" on model it does not look deleted_at in pivot table. Please check below example:

Route.query().whereHas('regions', (builder) => {
builder.where('regions.id', queryParams.ward_id)
})
there is pivot table "route_regions" which holds "route_id" and "region_id" and for some rows in "route_regions" table there are some entries which have not null value for deleted_at. But above code fetching all records (including relations for which mappings are soft deleted).
I have to add condition for this in every query (Please check below):
Route.query().whereHas('regions', (builder) => {
builder.where('regions.id', queryParams.ward_id)
builder.whereNull('route_regions.deleted_at')
})
Is there anything I missed or Is there any workaround for this problem?

@radmen
Copy link
Owner

radmen commented Feb 13, 2019

Hey, thanks for feedback!

It's a longshot, but have you tried to use pivotModel? You should create one and attach to it @provider:Lucid/SoftDeletes trait.

@lokendrajoshi54
Copy link
Author

Hi, Thanks for reply,

I tried following but didn't work:
Model.addGlobalScope(
query => {
query.whereNull(${Model.table}.deleted_at)
if (query._PivotModel && query.relatedQuery) {
query.whereNull(${query._PivotModel.table}.deleted_at);
}
},
'soft_deletes'
)

@radmen
Copy link
Owner

radmen commented Feb 14, 2019

I'm not sure what you're trying to achieve. From the given code I don't see that you're using SoftDeletes trait. Also I'm not able to determine if you created a pivot model.

Could you give a more complete example of your models? Something that would allow me to recreate the problem? Thanks.

@lokendrajoshi54
Copy link
Author

lokendrajoshi54 commented Mar 6, 2019

Sorry for late reply. Please check example below in my case:
Model:

`const Model = use('Model')

class GpsDevice extends Model {
static boot () {
super.boot()
this.addTrait('@Provider:Lucid/SoftDeletes')
}

vehicles() {
return this.belongsToMany('App/Models/Vehicle').pivotModel('App/Models/VehicleGpsdevice')
}

}`

Query:
GpsDevice.query().whereHas('vehicles', (builder) => {
builder.where('vehicle_type_id', 2)
}).with(vehicles).fetch()

The above query fetching records with deleted mappings in vehiclegpsdevice model. It seems its not checking deleted_at on pivot table. Can you please help me in that. Thanks

@radmen
Copy link
Owner

radmen commented Mar 6, 2019

@lokendrajoshi54 thanks for the code example. Could you also post contents of VehicleGpsdevice model (at least boot() method of that model)? Thanks!

@lokendrajoshi54
Copy link
Author

lokendrajoshi54 commented Mar 6, 2019

Sure @radmen here is content of VehicleGpsdevice Model:
const Model = use('Model')

class VehicleGpsdevice extends Model {
static boot () {
super.boot()
this.addTrait('@Provider:Lucid/SoftDeletes')
}
..
..
}

module.exports = VehicleGpsdevice

Same issue I faced with withTrashed() method, I'm trying to figure out if I missed anything, if you can tell me it would be helpful.
Thanks in advance

@radmen
Copy link
Owner

radmen commented Mar 6, 2019

Thanks for the code. I'll try to check it tomorrow.

@radmen
Copy link
Owner

radmen commented Mar 7, 2019

@lokendrajoshi54 I did some testing with the examples you gave.

SoftDeletes trait works with pivot model when fetching related records. Here's the test case I used:

it('does not include deleted pivots', async () => {
const user = await User.create({ username: 'Jon' })
const tags = await Promise.all([
Tag.create({ title: 'Foo' }),
Tag.create({ title: 'Bar' })
])
await Promise.all(tags.map(model => user.tags().save(model)))
await lucid.db.table('user_tags')
.where({ tag_id: tags[0].id })
.update({ deleted_at: new Date() })
const userTags = await user.tags().fetch()
expect(userTags.rows.length).to.equal(1)
const freshUser = await User.query()
.with('tags')
.first()
expect(freshUser.getRelated('tags').rows.length).to.equal(1)
})

There's a problem with applying SoftDeletes global scope in whereHas(). Test case with example:

it.skip('appends scope to whereHas() statement', async () => {
const user = await User.create({ username: 'Jon' })
const tags = await Promise.all([
Tag.create({ title: 'Foo' }),
Tag.create({ title: 'Bar' })
])
await Promise.all(tags.map(model => user.tags().save(model)))
await lucid.db.table('user_tags')
.where({ tag_id: tags[0].id })
.update({ deleted_at: new Date() })
const freshUser = await User.query()
.whereHas('tags', (query) => {
query.where('title', 'Foo')
})
.first()
expect(freshUser).to.be.undefined // eslint-disable-line
})

So, TLDR:

  • trait should work without any problem when fetching related items
  • trait fails to apply its scope in whereHas() query; most likely it's a bug in Lucid

I'll try to verify if issues with whereHas() are indeed Lucid-related.

@radmen radmen added the possible bug Seems to be a bug; requires additional checking label Mar 7, 2019
@lokendrajoshi54
Copy link
Author

lokendrajoshi54 commented Mar 8, 2019

Hello @radmen

Thanks for your reply and looking into issue. Same thing is applicable for withTrashed() method.

Is there any way I can help in this?

@radmen
Copy link
Owner

radmen commented Mar 8, 2019

Same thing is applicable for withTrashed() method.

In which context does this apply? whereHas()?

Is there any way I can help in this?

First thing we need to determine is whether this is an issue with the package or Lucid. If you think that the problem is with the package you could try writing a test case which fails - this would help me finding the bug,j

@radmen
Copy link
Owner

radmen commented Mar 9, 2019

I was able to reproduce the problem on clean Adonis application. Its been reported: adonisjs/lucid#429

@lokendrajoshi54
Copy link
Author

@radmen Thanks for looking into this.

@lokendrajoshi54
Copy link
Author

@radmen withTrashed() method is not working inside with method. Here is the snippet:
let vehicles = await Vehicle.query().whereHas('gps_devices').with('gps_devices', (build) => {build.withTrashed()}).fetch()

@radmen
Copy link
Owner

radmen commented Mar 19, 2019

@lokendrajoshi54 I'm aware of that. In fact it seems that this is something that is not possible in Lucid.

I've asked about it, yet without any answer.

@radmen radmen added the requires upstream fix Requires fixing in the @adonis/lucid package label Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
possible bug Seems to be a bug; requires additional checking requires upstream fix Requires fixing in the @adonis/lucid package
Projects
None yet
Development

No branches or pull requests

2 participants