Make count query optimisation work when using decorate_with #5811
Conversation
|
Is it possible to add a test? |
|
@deivid-rodriguez I'm not really sure what would be a good test case for that. I could add a unit test: # spec/unit/resource_controller/decorators_spec.rb
it 'delegates offset method' do
expect(applied).to respond_to(:offset)
endBut I don't think this provides much value. I would appreciate some guidance on how to best test this change 🙏 |
|
Yeah, I see how a proper test can be difficult. I guess ideally we should test that when using I'll look into this manually and consider merging without a test. One question that comes to mind at first sight is whether we can undo #4068 if we include this patch. |
I will look into that. |
|
I tried to investigate if the issue fixed by #4068 still exists with this change but it feels a little like #4068 got merged without any deeper understanding of why the issue occurred. That makes it super hard to reproduce so I didn't really come up with an answer to the question if we can revert the change ( |
|
@deivid-rodriguez I reverted #4068 and tests are passing. |
|
Thanks @irmela! I'll have a look at this soon 👍. |
f3fac34 to
652ead2
Compare
652ead2 to
8e24320
Compare
|
I added a test for this change! Setting this as ready. |
#3848 introduced a great performance improvement for pagination total_count. After #4068 this optimization was not available when using
decorate_with.This is another approach of #5389 with minimal change. Please let me know in case I missed something.