Feature/ability to see search result in index page's table #3712
Conversation
|
Code Climate has analyzed commit 81df621 and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
SahSantoshh
force-pushed
the
feature/ability_to_see_search_result_in_index_page
branch
from
26d949a to
7a5e338
Compare
|
This PR has been marked as stale because there was no activity for the past 15 days. |
Screen.Recording.2025-04-07.at.6.00.16.PM.mov@Paul-Bob @adrianthedev finally the search is working now, Please let me know what would nest step 😄 |
|
Issues with the branch
@Paul-Bob could you please take a look and guide me through |
|
Hi @SahSantoshh I'll have a look during this week, thank you for your patience! |
|
Hi @SahSantoshh I noticed that debounce was already working. I've just pushed a commit enabling pagination and URL update with the current search query! |
SahSantoshh
force-pushed
the
feature/ability_to_see_search_result_in_index_page
branch
from
14ecd75 to
589a142
Compare
ExecutionContext
# Conflicts: # app/controllers/avo/base_controller.rb
SahSantoshh
force-pushed
the
feature/ability_to_see_search_result_in_index_page
branch
from
589a142 to
0510c89
Compare
There was a problem hiding this comment.
Amazing work here @SahSantoshh!
I've left some comments with questions and some tweaks, let's discuss on those.
I tested it on index and seems to work as expected, how does it behave on association fields, on a has_many for example?
| turbo_stream.replace( | ||
| "#{@resource.model_key}_list", | ||
| partial: "avo/index/resource_table_component", | ||
| locals: { | ||
| resources: @resources, | ||
| resource: @resource, | ||
| reflection: @reflection, | ||
| parent_record: @parent_record, | ||
| parent_resource: @parent_resource, | ||
| pagy: @pagy, | ||
| query: @query, | ||
| actions: @actions | ||
| } | ||
| ), |
There was a problem hiding this comment.
Do we need to call the extra partial?
I wonder if we can use the partial code directly here, I give it a quick try and something was not working properly, could you please explore this a bit?
There was a problem hiding this comment.
yeah, tried some approach it didn't work. so pushing the changes a head
|
Issues:
|
View type is now fetched from the resource, |
|
@SahSantoshh, please re-request a review once the changes and discussion points have been addressed.
|
There was a problem hiding this comment.
Thanks for your patience on this PR, @SahSantoshh.
There's one question I'd like to clarify, I'll quote it from the previous review:
I tested it on index and seems to work as expected, how does it behave on association fields, on a has_many for example?
I've also left a comment on one of the discussions from the last review:
#3712 (comment)
SahSantoshh
force-pushed
the
feature/ability_to_see_search_result_in_index_page
branch
from
4a25fef to
e44936b
Compare
There was a problem hiding this comment.
@SahSantoshh I already left a review, let me know if you have any question
|
Main question is:
|
|
Sorry, I missed this one. |
|
@Paul-Bob could you please let me know some more details to deal with search for |
|
First, we need to verify the URL's proper transmission from the stimulus controller, I suspect it is not correctly seted for associations. Please review the previous search component's URL computation method for reference. |
Description
Fixes 2842
Checklist:
Screenshots & recording
Manual review steps
Manual reviewer: please leave a comment with output from the test if that's the case.