route-model-binding icon indicating copy to clipboard operation
route-model-binding copied to clipboard

Route model binding not working for HasManyThrough relations due to ambiguous column name

Open mr-feek opened this issue 2 years ago • 2 comments

Package version

1.0.1

Node.js and npm version

node -v
v20.5.1

npm -v
9.8.0

postgres version: 15.4

Issue

I'm getting a postgres error that the provided id column is ambiguous.

Sample Code (to reproduce the issue)

I have the following model setup:

An Organization hasMany Locations A Location hasMany Orders

Now I'm setting up a relationship so that I can fetch all orders that belong to an organization: An Organization hasManyThrough [Location, Order].

Here is the (reduced) code for my models:

Organization.ts

export default class Organization extends compose(BaseModel, SoftDeletes) {
    @column({ isPrimary: true })
    public id: string;

    @hasMany(() => Location, {
        foreignKey: 'organizationId',
    })
    public locations: HasMany<typeof Location>;
    
    @hasManyThrough([() => Order, () => Location])
    public orders: HasManyThrough<typeof Order>;
Location.ts

export default class Location extends compose(BaseModel, SoftDeletes, HasAutoGeneratedColumns) {
    @column({ isPrimary: true })
    public id: string;

    @column()
    public organizationId: string;

    @belongsTo(() => Organization, {
        foreignKey: 'organizationId',
    })
    public organization: BelongsTo<typeof Organization>;
    
    @hasMany(() => Order, {
        foreignKey: 'locationId',
    })
    public orders: HasMany<typeof Order>;
    
Order.ts

export default class Order extends compose(BaseModel, SoftDeletes, HasAutoGeneratedColumns) {
    @column({
        isPrimary: true,
    })
    public id: string;

    @column()
    public locationId: string;

    @belongsTo(() => Location, {
        foreignKey: 'locationId',
    })
    public location: BelongsTo<typeof Location>;

I'm declaring a route using route-model binding:

Route.put('organizations/:organization/orders/:>order', MyController);

And I'm declaring a controller like this:

@inject()
export default class MyController {
    constructor(private orderService: OrderService) {}
    
    @bind()
    public async handle(context: HttpContextContract, org: Organization, order: Order): Promise<OrderResource> {
        //
    }
}

When I hit this route, I get the following error:

Error: error: select "orders".*, "locations"."organization_id" as
   "through_organization_id" from "orders" inner join "locations" on "locations"."id" =
   "orders"."location_id" where "id" = $1 and "locations"."organization_id" = $2 and
   "orders"."deleted_at" is null limit $3 - column reference "id" is ambiguous

Notes

Manually adjusting the findRelatedForRequest query to prefix id with the table name works!

in Organization.ts:

public findRelatedForRequest(ctx, param, value) {
        /**
         * Have to do this weird dance because of
         * https://github.com/microsoft/TypeScript/issues/37778
         */
        const self = this as unknown as Organization;

        return self.related('orders').query().where('orders.id', value).firstOrFail();
    }

The table name (orders) prefix makes the query work.

BONUS (a sample repo to reproduce the issue)

I can set this up if need be! Please let me know.

mr-feek avatar Jan 08 '24 17:01 mr-feek

I think this could be fixed by prefixing the queries in the ResourceLoader with the model.tableName

IE

/**
         * Fallback to primaryKey lookup
         */
-        return relatedQuery.where(model.primaryKey, value).firstOrFail();
+       return relatedQuery.where(`${model.table}.${model.primaryKey}`).firstOrFail();

But I'm not certain if this should actually be handled within lucid perhaps. Just throwing this out there.

mr-feek avatar Jan 08 '24 17:01 mr-feek

Another workaround is to just set the routeLookupKey on the Order model.

/**
     * @see @see https://github.com/adonisjs/route-model-binding/issues/2
     */
    public static routeLookupKey = 'orders.id';

This makes it so that you don't have to manually handle all the other relationship lookups that might exist on the parent model

mr-feek avatar Jan 09 '24 19:01 mr-feek