lux icon indicating copy to clipboard operation
lux copied to clipboard

Multiple belongsTo relations to one model breaks

Open ralfschimmel opened this issue 9 years ago • 8 comments

When model A has multiple belongsTo relations to model B, specified using:

class ModelA extends Model {
    static belongsTo = {
       aB: {
            model: 'modelB',
            inverse: 'aModels'
        },
        bB: {
            model: 'modelB',
            inverse: 'bModels'
        }
   };
}

class ModelB extends Model {
    static hasMany = {
       aModels: {
            model: 'modelA',
            inverse: 'aB'
        },
        bModels: {
            model: 'modelA',
            inverse: 'bB'
        }
   };
}

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).

ralfschimmel avatar Mar 01 '17 20:03 ralfschimmel

+1.

I've created a gist for what sounds like the same issue.

willviles avatar Mar 06 '17 21:03 willviles

I found the offending code at controller/utils/params-to-query.js#L53-L58. Using find returns just the first relationship per related model.

I've got includedFields returning all relationships by looping over each relationship.

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 SQLITE_ERROR: ambiguous column name: images.id.

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.

willviles avatar Mar 09 '17 11:03 willviles

Check out my fork of Lux (branch fix-683) to test this issue:

https://github.com/willviles/lux/tree/fix-683

willviles avatar Mar 09 '17 12:03 willviles

Nice, hope @zacharygolba has some spare time soon ;-)

ralfschimmel avatar Mar 11 '17 17:03 ralfschimmel

Here's the problem with the SQL aliasing. Unfortunately, when you put the LEFT_OUTER_JOIN alias as dot notation (e.g. "avatar.id"), it gets parsed as "avatar"."id", whereas in the SELECT part of the query, it's maintained as a string:

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 joinRaw to manually write the join SQL. The records are resolved as expected in the payload's relationships object. However when you try to include the relationships (e.g. /users/?include=avatar,coverImage), the records' attributes are not populated.

willviles avatar Mar 17 '17 17:03 willviles

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?

jamemackson avatar Mar 28 '17 19:03 jamemackson

@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 products model that has a colorChart and an alternateColorChart belongsTo relationships, both relating to the same resource called color-chart. Today I got the second relationship wired up and could set the value but then on a fresh get request the alternateColorChart would be null in relationships which landed me back here as is the crux of this issue. I already had colorChart and alternateColorChart in the hasOne array on the product serializer.

the work-around?

When I added alternateColorChartId to the attributes array on the serializer, then the alternateColorChart relationship also began populating in addition to the initial colorChart relationship.

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 avatar Mar 28 '17 20:03 jamemackson

@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 avatar-id and cover-image-id attrs to the attributes payload.

Currently, my fix-683 branch ensures records are correctly added to the payload from simply specifying the relationship in hasOne. However included data returns blank records.

willviles avatar Mar 28 '17 20:03 willviles