feature: Always use index_query when loading a resource

[sriddbs]

Description

Avo::AssociationsController will now always consider index_query to load a resource

Fixes #2445

Checklist:

• [x] I have performed a self-review of my own code
• [ ] I have commented my code, particularly in hard-to-understand areas
• [ ] I have made corresponding changes to the documentation
• [ ] I have added tests that prove my fix is effective or that my feature works

[codeclimate[bot]]

Code Climate has analyzed commit 6bc4a47e and detected 0 issues on this pull request.

View more on Code Climate.

[sriddbs]

hey @Paul-Bob let me know if this is all what is required 😄 can you also point me where to add the specs for this please?

[sriddbs]

Thank you @Paul-Bob for the detailed explanation 👍

How to distinguish on the index_query block where we're on an association (with the current implementation i guess we can do @parent.present?)

am making the changes in app/controllers/avo/associations_controller.rb, so do we still need the @parent.present? check?

The example can be added anywhere that you think it fits on spec/dummy app.

yes, I added the index_query on a resource which is also loaded via association and the changes I made seem working but as you suggested I will add the specs as well 👍

[adrianthedev]

I'm going to let you guys jam on this. I have a couple of questions.

1. Is this going to be a breaking change for some existing users?
2. Do we really want to apply the index_query to associations too? Why?

[Paul-Bob]

1. Is this going to be a breaking change for some existing users?

Code will not break, the self.index_query will start to apply when rendering a resource through a has_* association field so it may affect the records shown. In order to keep 0 changes we can add the association_query option like:

self.index_query = -> {
  query.order(position: :asc)
}

self.association_query = -> {
  query.order(position: :asc)
}

Here you can even reuse code:

def order_query
  -> {
    query.order(position: :asc)
  }
end

self.index_query = order_query
self.association_query = order_query

2. Do we really want to apply the index_query to associations too? Why?

There is a linked issue on the PR description with more details and with more linked discussions. Has fields are basically scoped index views of a resource.

Lets take an example where we want to order the users always ascending by position on Avo::Resources::User everywhere on the app:

self.index_query = -> {
  query.order(position: :asc)
}

Now when visiting the users page the order is respected but when have something like:

field :users, as: :has_many

The order will not getting applied. Sure there is the possibility to field :users, as: :has_many, scope: -> { query.order(position: :asc) } but you'll need to apply this scope on all your users fields and I'm giving here a simple ordering example, it easily get messy when you have a multiple lines logic and need to change one little detail.

[Paul-Bob]

am making the changes in app/controllers/avo/associations_controller.rb, so do we still need the @parent.present? check?

@sriddbs I mean on the Proc, we should be able to separate the query when it's executed as association:

self.index_query = -> {
  if on_association? # fictive method I think we can do @parent.present?
    query.do_this...
  else
    query.do_that...
  end
}

Makes sense? There are cases when we want to apply different logic to the query when is loaded as association.

[sriddbs]

I mean on the Proc, we should be able to separate the query when it's executed as association

Ahh got it 👍

[adrianthedev]

@sriddbs what is the use-case on your end? What would you like to apply from index_query to the association?

[github-actions[bot]]

This PR has been marked as stale because there was no activity for the past 15 days.

[github-actions[bot]]

This PR has been marked as stale because there was no activity for the past 15 days.

[adrianthedev]

Closing for now as we haven't gotten the use-case for it. Please feel free to reopen.

[Paul-Bob]

Linked discussions:

• https://github.com/avo-hq/avo/issues/2443
• https://discord.com/channels/740892036978442260/1217489935007879239/1217512469170491412

Workaround: use scope, if it repeats all over the code create a helper for it