-
Notifications
You must be signed in to change notification settings - Fork 60
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
Multiple belongsTo relations to one model breaks #683
Comments
+1. I've created a gist for what sounds like the same issue. |
I found the offending code at controller/utils/params-to-query.js#L53-L58. Using I've got However, there's then a problem with building the SQL query in database/query/index.js#L372-L384. Lux builds the following SQL query, which throws an SELECT "users"."id" AS "id",
"users"."name" AS "name",
"users"."email" AS "email",
"images"."id" AS "avatar.id",
"images"."id" AS "coverImage.id"
FROM "users"
LEFT OUTER JOIN "images"
ON "users"."avatar_id" = "images"."id"
LEFT OUTER JOIN "images"
ON "users"."cover_image_id" = "images"."id"
ORDER BY users.created_at ASC,
users.id ASC
LIMIT 25 This seems to be an SQL aliasing issue, which is discussed below. |
Check out my fork of Lux (branch |
Nice, hope @zacharygolba has some spare time soon ;-) |
Here's the problem with the SQL aliasing. Unfortunately, when you put the LEFT_OUTER_JOIN alias as dot notation (e.g. SELECT "users"."id" AS "id",
"users"."avatar_id" AS "avatar.id", // Kept as string
FROM "users"
LEFT OUTER JOIN "images"
ON "users"."avatar_id" = "avatar"."id" // Dot notation parsed This appears to be a quirk in Knex and means the aliasing fails. On the Lux side of things, only keys with dot notation are properly resolved to records. So, I changed the SQL query to use Knex's |
Ok, I've got my project far enough to where I'm running into this now. @willviles / @zacharygolba - is there anything I can do to help push this fix along? |
@willviles & @zacharygolba - I believe I have, somewhat accidentally, found a workaround for this it is working for me to load the second belongsTo relationship (based on latest lux in master) context wise, I've got a the work-around?When I added I've been able to add and remove this and consistently get working/not working results, can you guys try and confirm this also works for you? I haven't thought thru all the downstream implications of just adding this field to the attributes but clearly it has some effect relevant to the conversations above so I thought I'd share. |
@jamemackson - you're right! Using the example of my gist, the following serializer config does indeed work: // serializers/user.js
attributes = [
...
'avatarId',
'coverImageId'
];
hasOne = [
'avatar',
'coverImage'
]; This is a good short-term workaround, but this isn't a particularly nice pattern. It also adds superfluous Currently, my fix-683 branch ensures records are correctly added to the payload from simply specifying the relationship in |
When model A has multiple belongsTo relations to model B, specified using:
When fetching model A with the relations, It will only do a LEFT OUTER JOIN for the first relation and thus never return the second in the JSON response (it's always null).
The text was updated successfully, but these errors were encountered: