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

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

Open
mr-feek opened this issue Jan 8, 2024 · 2 comments

Comments

@mr-feek
Copy link

mr-feek commented Jan 8, 2024

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
Copy link
Author

mr-feek commented Jan 8, 2024

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
Copy link
Author

mr-feek commented Jan 9, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant