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

Enable configuring search_count per resource #2930

Merged
merged 6 commits into from
Jul 9, 2024

Conversation

binarygit
Copy link
Contributor

@binarygit binarygit commented Jul 2, 2024

Description

This PR allows setting the search count on a per resource basis. It makes sure that the resource specific count takes precedence over the global configuration.

Fixes #2901

Checklist:

  • 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

Screenshots & recording

Manual review steps

  1. In the dummy app, inside resources/user.rb, I set count to 5.
  2. Navigated to /admin/resources/users.
  3. Typed 'a'. Verified that only 5 search results were displayed out of 39.

  1. In the dummy app, inside resources/user.rb, I set count to -> { 3 }.
  2. Navigated to /admin/resources/users.
  3. Typed 'a'. Verified that only 3 search results were displayed out of 39.

Manual reviewer: please leave a comment with output from the test if that's the case.

@binarygit binarygit force-pushed the configure-search-count-per-resource branch from e4224b2 to e3d83ab Compare July 2, 2024 15:46
Copy link

codeclimate bot commented Jul 2, 2024

Code Climate has analyzed commit 98f0e65 and detected 0 issues on this pull request.

View more on Code Climate.

@binarygit
Copy link
Contributor Author

binarygit commented Jul 2, 2024

@Paul-Bob I think there should be tests for this. However I'm not sure where the search related tests are written. If you could tell me, then I'd be happy to write them.

@binarygit
Copy link
Contributor Author

Not sure why CI's failing 🤷 . I checked the failing specs locally and they pass
passing

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Jul 3, 2024

It's looking great! Thanks @binarygit

spec/features/avo/search_has_many_scope_spec.rb is an example of a search controller test.

We have some helpers for system tests if you want to go that way.

Don't worry about the CI tests I'll have a look, it seems unrelated.

@adrianthedev
Copy link
Collaborator

hey @binarygit.
I'd like to write something to you.
How can I reach you? can you share your email, or are you on our discord?

@binarygit
Copy link
Contributor Author

binarygit commented Jul 4, 2024

Hey adrian. Here’s my email @adrianthedev : [email protected]

I’m not in the discord yet!

@binarygit
Copy link
Contributor Author

@Paul-Bob I added the tests. Let me know what you think 🙏

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Jul 9, 2024

Thank you for the contribution @binarygit !

@Paul-Bob Paul-Bob merged commit eb0260d into avo-hq:main Jul 9, 2024
17 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configure search_count per resource
3 participants