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

Fileds filter not seem to be working properly with nested relations #5314

Closed
pratikjaiswal15 opened this issue May 3, 2020 · 6 comments
Closed
Assignees
Labels
Relations Model relations (has many, etc.)

Comments

@pratikjaiswal15
Copy link

pratikjaiswal15 commented May 3, 2020

Note
This issue was spun off from a Slack discussion and StackOverflow question:


I have four models- purchase, purchase products, vendors, and product.

The purchase has many purchase products and vice versa
purchase products belong to product and vice versa
Vendors has many purchase and vice vera
Now with the following code, I am getting all purchases, purchase-products, and vendors(with field filter) but not able to use fields filter with the product model.
Is it a bug or I am doing it wrong way

async find(

  ): Promise<Purchase[]> {
    return this.purchaseRepository.find(
      {

        include: [{
          relation: 'purchaseProducts',
          scope: {
            fields: { name: false, product_id: false}, // not working
            include: [{ relation: 'products' }],
          }

        }, {

          relation: 'vendors',
          scope: {
            fields: { address_line1: false, city: false, state: false, pincode: false, gst_number: false, // working }
          }
        }]
      }
    );

  }


And for relation vendors fields name : true is not working, so I have to fields name : false.
Thank you in advance

purchase.model.ts

import { Entity, model, property, hasMany, belongsTo } from '@loopback/repository';
import { PurchaseProduct } from './purchase-product.model';
import { Vendor } from './vendor.model';

@model({ settings: { strict: true } })
export class Purchase extends Entity {


  @property({
    type: 'string',
    id: true,

  })
  purchase_id: string;


  @property({
    type: 'date',
  })
  date?: string;


  @property({
    type: 'number',
    required: true,

  })
  totalprice?: number;
  @hasMany(() => PurchaseProduct, { keyTo: 'purchase_id' })
  purchaseProducts: PurchaseProduct[];

  @belongsTo(() => Vendor, { name: 'vendors' })
  vendor_id: number;
  // Define well-known properties here

  // Indexer property to allow additional data
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  [prop: string]: any;

  constructor(data?: Partial<Purchase>) {
    super(data);
  }
}

export interface PurchaseRelations {
  // describe navigational properties here
}

export type PurchaseWithRelations = Purchase & PurchaseRelations;


purchase-products.model.ts

import { Entity, model, property, belongsTo } from '@loopback/repository';
import { Purchase } from './purchase.model';
import { Product } from './product.model';

@model({ settings: { strict: true } })
export class PurchaseProduct extends Entity {
  @property({
    type: 'number',
    id: true,
    generated: true,
  })
  id?: number;

  @property({
    type: 'number',
    required: true,
  })
  quantity: number;

  @property({
    type: 'number',
    required: true,
  })
  price: number;

  @belongsTo(() => Purchase, { name: 'purchases' })
  purchase_id: string;

  @belongsTo(() => Product, { name: 'products' })
  product_id: number;
  // Define well-known properties here

  // Indexer property to allow additional data
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  [prop: string]: any;

  constructor(data?: Partial<PurchaseProduct>) {
    super(data);
  }
}

export interface PurchaseProductRelations {
  // describe navigational properties here
}

export type PurchaseProductWithRelations = PurchaseProduct & PurchaseProductRelations;

product.model.ts

import { Entity, model, property, hasMany, hasOne, belongsTo } from '@loopback/repository';
import { Purchase } from './purchase.model';
import { OrderedProducts } from './ordered-products.model';
import {PurchaseProduct} from './purchase-product.model';

@model({ settings: { strict: true } })
export class Product extends Entity {
  @property({
    type: 'number',
    id: true,
    generated: true,

  })
  product_id?: number;

  @property({
    type: 'string',
    required: true,
    index: {
      unique: true
    }

  })
  name: string;

  @property({
    type: 'string',
    required: true,
  })
  image_url: string;

  @property({
    type: 'string',
  })
  description?: string;



  @hasMany(() => OrderedProducts, { keyTo: 'product_id' })
  orderedProducts: OrderedProducts[];

  @hasOne(() => ProductPrice, { keyTo: 'product_id' })
  productPrices: ProductPrice;

  @hasMany(() => PurchaseProduct, {keyTo: 'product_id'})
  purchaseProducts: PurchaseProduct[];
  // Define well-known properties here

  // Indexer property to allow additional data
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  [prop: string]: any;

  constructor(data?: Partial<Product>) {
    super(data);
  }
}

export interface ProductRelations {
  // describe navigational properties here
}

export type ProductWithRelations = Product & ProductRelations;

vendor.model.ts

import { Entity, model, property, hasMany } from '@loopback/repository';
import { Purchase } from './purchase.model';

@model({ settings: { strict: true } })
export class Vendor extends Entity {
  @property({
    type: 'number',
    id: true,
    generated: true,
  })
  vendor_id?: number;

  @property({
    type: 'string',
    required: true,
  })
  name: string;

  @property({
    type: 'string',
    required: true,
    index: {
      unique: true
    }
  })
  mobile_no: string;

  @property({
    type: 'string',
    index: {
      unique: true
    }
  })
  email?: string;


  @hasMany(() => Purchase, { keyTo: 'vendor_id' })
  purchases: Purchase[];

  // Define well-known properties here

  // Indexer property to allow additional data
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  [prop: string]: any;

  constructor(data?: Partial<Vendor>) {
    super(data);
  }
}

export interface VendorRelations {
  // describe navigational properties here
}

export type VendorWithRelations = Vendor & VendorRelations;

@achrinza achrinza added the Relations Model relations (has many, etc.) label May 3, 2020
@pratikjaiswal15
Copy link
Author

Please find the updated code. I have added all models

@pratikjaiswal15
Copy link
Author

Anyone please?

@agnes512 agnes512 self-assigned this May 6, 2020
@agnes512
Copy link
Contributor

agnes512 commented May 6, 2020

@pratikjaiswal15

but not able to use fields filter with the product model.

Do you mean that product_id still shows up, or the inclusion doesn't work for products?

Anyhow this won't work as expected:

include: [{
          relation: 'purchaseProducts',
          scope: {
            fields: { name: false, product_id: false}, // not working
            include: [{ relation: 'products' }],
          }
...

PurchaseProduct belongs to a Product, and the foreign key is product_id. This won't work because the foreign key product_id is being excluded, so that the inclusion doesn't work as the product_id is undefined. For relations, primary keys and foreign keys need to be included. I think we should make it clear in the relation docs.

If you would like to hind the product_id from the response, the hidden properties might help.

@InvictusMB
Copy link
Contributor

InvictusMB commented May 11, 2020

@agnes512
To be honest this is very counter-intuitive, especially for REST API consumers.
Also it is exposing the implementation details of model relations onto those REST API consumers. They have to know the foreign key name even though it is never exposed to them (in case it is marked as hidden to work around this issue in the first place).
I believe this is something that should be handled under the hood. The way I understand the codebase so far, findByForeignKeys could fill this for you and it won't be a breaking change.

P.S. Is it a best practice then to mark foreign keys as hidden in models?
And if it is, shouldn't the relation generator do it for you?

@agnes512
Copy link
Contributor

@InvictusMB I totally understand. Unfortunately it's the limitations of current design :( The filtered (excludes fk in this case) data is passed to the next query. And the nested relation needs the fk for findByForeignKeys. That's why it doesn't work.

And if it is, shouldn't the relation generator do it for you?

I agree that fields should be more intuitive. Besides adding options to the generator and enhancing the docs, is there any room to improve the design? @raymondfeng

@InvictusMB
Copy link
Contributor

@agnes512 I don't see where this limitation comes from.
findByForeignKeys helper has access to foreign key metadata
https://github.com/strongloop/loopback-next/blob/a81ce7e1193f7408d30d984d0c3ddcec74f7c316/packages/repository/src/relations/relation.helpers.ts#L57
and it already modifies the filter
https://github.com/strongloop/loopback-next/blob/a81ce7e1193f7408d30d984d0c3ddcec74f7c316/packages/repository/src/relations/relation.helpers.ts#L61

It could enrich the fields with fkName in the same manner if it's missing.
Am I missing something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Relations Model relations (has many, etc.)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants