Loop through results and run another query for each result... Best practice? #838
Comments
|
Use the querychainer for multiple queries like that :) http://sequelizejs.com/documentation#utils-querychainer I would also escape pages[k].id or at the very least parseInt() it. To escape I believe you can do global.db.sequelize.Utils.format(['SELECT * FROM hello WHERE world = ?', 'Earth']) Which will escape earth :) |
|
You could also build your query up to do a where h.page_id IN (<array of pages[k].id>) as well :) This way you're only executing one query but that's less of sequelize and more of SQL in general. |
|
Beautiful, thanks @durango.
Can't thank you enough for the help. Saved me a lot of time and headache. |
|
Can someone please post a snippet how to achieve this using QueryChainer? I've tried myself, but I seem to not get it right. |
|
@jagged3dge at this point i'd suggest you use promises, we are moving away from the QueryChainer |
|
Thank you for the response Mick. I'm using ~2.0.0-beta.2 at this moment. I have tried to adapt from the promises example code on the documentation page. However, I'm still facing the same problem: the response is sent without the additional data. Here's what I've tried: db.User.sync()
.then(function() {
return req.User.getPosts({
include: [
//{ model: db.PostLikes },
{ model: db.User, attributes: ['displayName', 'name', 'profilePicture'] },
{ model: db.Game, attributes: ['name'] }
]
})
})
.then(function(posts) {
console.log(posts)
for (p in posts) {
db.PostLikes.sync()
.then(function() {
return db.PostLikes.count({ where: { PostId: posts[p].id } })
})
.then(function(likes) {
console.log('Blog #' + posts[p].id + ' has ' + likes + ' likes')
posts[p].likes = likes
console.log('posts[' + posts[p].id + '] = ', posts[p])
})
}
return posts
}).then(function(posts) {
// DEV: Send 404 if no 'likes' count has been added to the blogpost data. Remove for production.
if (posts[0].likes) {
res.json(posts)
} else {
res.send(404)
}
}) |
|
You need to return a promise in your second then since you're doing async var promises = []; On Wed, May 21, 2014 at 11:48 AM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
I tried your method, but I'm getting a "ReferenceError: Promise is not defined." I noticed that Sequelize uses Bluebird internally. What do I need to require to utilise the Promise object? |
|
Right you need to do something like var Promise = require('bluebird') or On Wed, May 21, 2014 at 1:20 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
I tried using var Promise = require('bluebird')
// throws: Cannot find module 'bluebird'
var Promise = db.Sequelize.Promise
//throws: Possibly unhandled TypeError: Cannot call method 'all' of undefined at D:\............\posts.js:58:22 |
|
Well you have to install bluebird. Sequelize.Promise is only available on On Wed, May 21, 2014 at 1:28 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
Thanks for the tip to install 'bluebird'. Apparently, I'm just a dork. LOL! Modified my code to the following. db.User.sync()
.then(function() {
return req.User.getPosts({
include: [
//{ model: db.PostLikes },
{ model: db.User, attributes: ['displayName', 'name', 'profilePicture'] },
{ model: db.Game, attributes: ['name'] }
]
})
})
.then(function(posts) {
for (p in posts) {
promises = []
promises.push(
db.PostLikes.sync()
.then(function() {
return db.PostLikes.count({ where: { PostId: posts[p].id } })
})
.then(function(likes) {
console.log('Blog #' + posts[p].id + ' has ' + likes + ' likes')
posts[p].likes = likes
console.log('posts[' + posts[p].id + '] = ', posts[p])
})
}
return Promise.all(promises)
}).then(function(posts) {
console.log(posts) // Should result in a list of posts with 'likes' attribute, but displays '[ undefined ]' instead
res.send(404) // Temporary response while testing
})The 'posts' parameter in the final '.then()' results in an undefined. I'm really new to Promises and it's tough trying to wrap my head around this. I'm sure it's something really simple that I'm missing here. Can you shed some light on this? |
|
var promises = [] should obviously be outside the loop :) Hmm, i just realized. Why are you calling sync for each post? Something is On Wed, May 21, 2014 at 2:08 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
.sync() should really only be called at the start of your app - It creates On Wed, May 21, 2014 at 2:08 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
Haha... yes, I realise calling the .sync() at each loop call is wrong. But, like I said earlier, I'm new to async code with promises, and I had to directly lift from the documentation. I couldn't figure a way to initiate the first promise. A copying mistake with the promises[] array. In my code, the array does get initialized outside the for loop. However, the result is like I said. The returning object in the final 'then()' is [ undefined ]. >.< Here's the corrected code: db.User.sync()
.then(function() {
return req.User.getPosts({
include: [
//{ model: db.PostLikes },
{ model: db.User, attributes: ['displayName', 'name', 'profilePicture'] },
{ model: db.Game, attributes: ['name'] }
]
})
})
.then(function(posts) {
promises = []
for (p in posts) {
promises.push(
db.PostLikes.sync()
.then(function() {
return db.PostLikes.count({ where: { PostId: posts[p].id } })
})
.then(function(likes) {
console.log('Blog #' + posts[p].id + ' has ' + likes + ' likes')
posts[p].likes = likes
console.log('posts[' + posts[p].id + '] = ', posts[p])
})
)
}
return Promise.all(promises)
}).then(function(posts) {
console.log(posts) // Should result in a list of posts with 'likes' attribute, but displays '[ undefined ]' instead
res.send(404) // Temporary response while testing
}) |
|
You might need return posts[p] in the final then() On Wed, May 21, 2014 at 2:19 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
Thank you so much Mick! Finally did it! Here's the working code: req.User.getPosts({
include: [
//{ model: db.PostLikes },
{ model: db.User, attributes: ['displayName', 'name', 'profilePicture'] },
{ model: db.Game, attributes: ['name'] }
]
})
.then(function(posts) {
promises = []
for (p in posts) {
promises.push(
db.PostLikes.count({ where: { PostId: posts[p].id } })
.then(function(likes) {
posts[p].dataValues.likes = likes // Gotcha! The json() string for each object only returns values from the 'dataValues' object. Adding it to the object itself doesn't return it in the JSON response
return posts[p] // vital for pushing the object into the result array
})
)
}
return Promise.all(promises)
}).then(function(posts) {
res.json(posts)
}) |
|
Manipulating dataValues is inadvisable. var post = posts[p].toJSON(); On Wed, May 21, 2014 at 2:43 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
Understood. Updated the code accordingly to pre-JSONify the individual post objects and add the attribute to the JSON, instead of modifying underlying dataValues object. Thanks for the pointer and all your help, Mick! You're awesome! |
|
Yeah that's the more future proof way atleast :) And should work for most Anytime :) We're also on irc, #sequelizejs @ freenode On Wed, May 21, 2014 at 3:04 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
Seems like my happiness was premature q_q The resultant JSON array has the right number of objects, however the objects themselves are copies of the last object in the original array. I have a gut-feeling this is happening due to the |
|
Hmm, can you provide me with a full copy of your code again? (use On Wed, May 21, 2014 at 3:22 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
Ah yeah this is a classic case for a closure. You either wan't posts.forEach(function (post) {}); or you want to create a for (p in posts) { On Wed, May 21, 2014 at 3:35 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
Awesome catch! Thanks a ton Mick :D Resolved the issue with |
|
You might want res.json(200, posts) rather than res.send, that sends the On Wed, May 21, 2014 at 4:05 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
Since I am converting the posts to JSON in the forEach() loop, I figured res.send() made more sense to send an array of pre-JSONified objects, instead of res.json(). Unless I'm wrong? I am testing the routes with POSTman and the responses seems to have the correct 'application/json' headers even with res.send(). One last thing before I rest this issue... How can I go about fetching and adding additional attributes, say for eg. number of Comments on the post? Would it be possible to do that as well in the same 'forEach' closure? If so, how? |
|
Afaik res.json just sets the right header and does JSON.stringify on your Well you can just use Promise.all() again inside your then and then map On Wed, May 21, 2014 at 4:47 PM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
I've been through all of bluebird's docs and tried looking around on SO as well... I'm having no luck understanding how to retrieve another attribute via Sequelize and map that onto. Most of the code snippets I've come across have one loop doing only 1 thing. I thought of making another array of promises for Comments and utilize that to fetch and update the Comments count. But my logic flunks out at the end when it comes to |
|
Inside your closure you could do: promises.push(Promise.all([ return post; On Thu, May 22, 2014 at 1:00 AM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
Fantastic! That works like a charm ... After this project's done, I'm gonna spend a few weeks poring over JS promises, I swear. Thanks again Mick! Updated the Gist too. |
|
I reckon a few hours would do the trick ;) On Thu, May 22, 2014 at 9:23 AM, jagged3dge notifications@github.comwrote:
Mick Hansen |
|
Nice thanks mick and jagge |
Hey all. I'm having trouble looping through a results set and then running another query for each result in order to attach some additional data.
Below is my code. "Pages" is a result from a Sequelize query. For each page, I'm trying to attach it's respective mentions. What's the best practice here?
Oddly, the below code actually renders the association to my view on localhost. But when pushed to Heroku, it does not.
The text was updated successfully, but these errors were encountered: