Adds flag to be unable to preload pivot duplicated data

[hjJunior]

Fix #98

[tejainece]

Thanks for the PR. I will review it and merge it soon.

[jaumard]

For me this is not ok, let me explain first. What you're trying to achieve in #98 is really a specific case, why trying to make it generic under the ORM itself, doesn't make sens. If you really need this just override fetchByOrder and patch it like you want/need.

Because here if we start adding specific cases inside the ORM itself it will become a nightmare to maintain and evolved... Because what about someone who'll have the case that he has 3 foreign keys but want duplicate from the first 2. Or what if it's not from the first one like you did on your PR but the second one instead ? I don't think we can support this for all use cases, the ORM allow you to overrides methods for that purpose imho.

[hjJunior]

Well, let me try to add a justificate on it... I agree in parts, for example, if someone has 3 or more foreign keys and just want to fetch duplicate from one of this relation will fetch a lot of data unnecessary, but I disagree that this is a really specific case, because generally ORM that I use fetch duplicates data, I know that is a complex structure behind of it, like cache data...

So I was thinking maybe generator write a custom function for each relation many to many to fetch duplicates data, in this way we don't prejudice the performance, what do u think about it?

[jaumard]

Do you have a link of an existing ORM with such feature ? All the one I know manage that by custom queries, not internally.

Because for me first it's not really duplicate data as you set multiple primary key and want to query from only one of them, so it's not technically a duplicate. For me it's typically the kind of request you have to do on your own as the generator can't provide an answer for all case (example of 3 foreign keys or just don't want to fetch from the first one but the second...) why should we support half a feature that will generate of questions and issues than just saying "you want this just override fetchByOrder on your bean and provide it yourself", it's not that complicated.

Even if we provide dedicated methods to fetch those stuff, it still doesn't cover half of the use cases like this one...

[hjJunior]

I know that javascript its a different world, but here is an example that I usually use https://adonisjs.com/docs/4.0/relationships#_belongs_to_many

And here I'll add a basic example:

/// App/Models/Order
class Order extends Model {
  products() {
    return this.belongsToMany('App/Models/Product').pivotModel('App/Models/OrderProduct')
  }
}

/// App/Models/OrderProduct
class OrderProduct extends Model {}

/// App/Models/Product
class Product extends Model {
  orders() {
    return this.belongsToMany('App/Models/Order').pivotModel('App/Models/OrderProduct')
  }
}

And on route

Route.get('/orders', async () => {
  const orderFounded = await Order.find(1);
  return await orderFounded.products().fetch()
})

Result it on

[
  {
    "id": 1,
    "created_at": null,
    "updated_at": null,
    "pivot": {
      "product_id": 1,
      "order_id": 1
    }
  },
  {
    "id": 1,
    "created_at": null,
    "updated_at": null,
    "pivot": {
      "product_id": 1,
      "order_id": 1
    }
  }
]

[jaumard]

This is not the same use case here :) because this you can implement it with jaguar just like:

class Order {
  @PrimaryKey()
  String id;

  @ManyToMany(OrderProductBean, ProductBean)
  List<Product> products;
}

class Product {
  @PrimaryKey()
  String id;

  @ManyToMany(OrderProductBean, OrderBean)
  List<Order> orders;
}

class OrderProduct {
  @BelongsTo.many(OrderBean)
  String orderId;

  @BelongsTo.many(ProductBean)
  String productId;
}

And then:

OrderBean().find(1, preload: true);

PS: didn't put bean code as it's just the definition nothing relavent on them

[hjJunior]

I did this, if look at the issue #98 it works, but just returns distinct data on pivot table, so the example above I have on pivot table two data with the same foreign keys

What I have on database

Jaguar ORM

Pivot table

[OrderItem {id: 1, orderId: 3, produtoId: 4 }, OrderItem {id: 2, orderId: 3, produtoId: 4 }]

But after the OR filter to get on product table it gets only product because of this:

SELECT * products WHERE id = 1 OR id = 1

I'll returns only one product with id 1 (if exists)

If exists twice or more times the same product the list of products will add only one time

[jaumard]

I think I understand what you mean, generally you want to avoir duplicates on many to many relations that's why I was a bit lost ^^

So what you're trying to achieve is more OrderProductBean().findByOrder(1) ?

[hjJunior]

Yep, and the problem is findByOrder

      final exp = Or();
      for (final t in pivots) {
        exp.or(productBean.id.eq(t.produtoId));
      }
      return await productBean.findWhere(exp);

Will ignore if product is duplicated, because of the query SELECT * products WHERE id = 1 OR id = 1 OR ...

[jaumard]

Ok understood ! Can you put here what the example we take will generate with your PR please ? Kind of hard to guess from the code ^^ Or better, add an example under orm/example/many_to_many/fetchduplicate it will be easier to see what's generated and how use it.

[hjJunior]

Of course! The generator will adds a flag for methods fetchDuplicates

  Future<Order> preload(Order model,
      {bool cascade: false, bool fetchDuplicates: false}) async {

and preload all the same

  Future<List<Order>> preloadAll(List<Order> models,
      {bool cascade: false, bool fetchDuplicates: false}) async {

So I just pass this flag to fetch method

    model.orderProducts = await orderItemBean.fetchByOrder(model,
        fetchDuplicates: fetchDuplicates);

So after the generator the fetch method will look like this, just verifying if have flagged it if not use the custom standart of jaguar ORM

  Future<List<Product>> fetchByOrder(Order model,
      {bool fetchDuplicates: false}) async {
    final pivots = await findByOrder(model.id);
    if (!fetchDuplicates) {
      final exp = Or();
      for (final t in pivots) {
        exp.or(productBean.id.eq(t.produtoId));
      }
      return await productBean.findWhere(exp);
    } else {
      final List<Product> returnList = [];
      final groupedData = groupBy(pivots, (pivot) => pivot.orderId);
      for (int i = 0; i < groupedData.values.length; i++) {
        final data = await productBean.find(groupedData.keys.elementAt(i));
        for (int j = 0; j < groupedData.values.elementAt(i).length; j++) {
          returnList.add(data);
        }
      }
      return returnList;
    }
  }

[jaumard]

Has fetchDuplicates is only for many to many relation, either generate fetchDuplicates only when their a many to many relation, or instead generate a specific fetchDuplicates methods generate only on many to many. What do you think @hjJunior @tejainece, I don't want to have fetchDuplicates on other relations as it will do nothing

[hjJunior]

I'll work on it, I think the best solution it's create fetchDuplicates for each Many to Many relationships, what do u think @jaumard ? like fetchProducts and fetchOrders

[jaumard]

I not sure about the naming for this... @tejainece have you an idea ? Maybe better to keep the method as it is and just add fetchDuplicates param when there a many to many setup... Might be the less messy solution or for many to many we'll have fetchProducts, fetchByProducts, findProducts ect... people will be totally lost on what method is for...

once we decide and it's done please regenerate also the embedded example, it will be easier to see first if it works (generate something that at least compile) and will be able to see more easily what the output of this. Thanks

[tejainece]

I will review it, test it and publish it over the coming weekend. Sorry for the delay.