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

Fix search failing test #1213

Merged
merged 1 commit into from Oct 7, 2023
Merged

Conversation

JuanVqz
Copy link
Contributor

@JuanVqz JuanVqz commented Oct 3, 2023

This PR is just for fixing the latest merge that introduces a failing
test.

When initializing a new Search model it has some default values,
especially the results_count one.
https://github.com/JuanVqz/lobsters/blob/5338e5a4b335b94337636a59b7bbc83529dfce81/app/models/search.rb#L42

When doing Search.new({}, nil) in the ignore_searx method
https://github.com/JuanVqz/lobsters/blob/5338e5a4b335b94337636a59b7bbc83529dfce81/app/controllers/search_controller.rb#L30

It takes the -1 value by default which in this if/else statement

https://github.com/JuanVqz/lobsters/blob/5338e5a4b335b94337636a59b7bbc83529dfce81/app/views/search/index.html.erb#L40

is rendering this else block

https://github.com/JuanVqz/lobsters/blob/5338e5a4b335b94337636a59b7bbc83529dfce81/app/views/search/index.html.erb#L157-L167

and that doesn't render the 0 results but it renders the Search hints:

if that is the required behaviour then this PR is solving it.
otherwise, let me know and I'll arrange it as desired.

@JuanVqz JuanVqz marked this pull request as ready for review October 3, 2023 21:29
@pushcx
Copy link
Member

pushcx commented Oct 4, 2023

Thanks for fixing this. I guess I’d prefer to return 0 results without an error message. This isn’t currently possible with the Search model. Do you see an unintrusive way to make that possible?

This PR is just for fixing the latest merge that introduces a failing
test.

When initializing a new Search model it has some default values,
specially the results_count one.
https://github.com/JuanVqz/lobsters/blob/5338e5a4b335b94337636a59b7bbc83529dfce81/app/models/search.rb#L42

When doing Search.new({}, nil) in the ignore_searx method
https://github.com/JuanVqz/lobsters/blob/5338e5a4b335b94337636a59b7bbc83529dfce81/app/controllers/search_controller.rb#L30

It takes the -1 value by default which in this if/else statement

https://github.com/JuanVqz/lobsters/blob/5338e5a4b335b94337636a59b7bbc83529dfce81/app/views/search/index.html.erb#L40

is rendering this else block

https://github.com/JuanVqz/lobsters/blob/5338e5a4b335b94337636a59b7bbc83529dfce81/app/views/search/index.html.erb#L157-L167

and that doesn't render the 0 results but it renders the Search hints:

if that is the decired behaviour then this PR is solving it.
@JuanVqz
Copy link
Contributor Author

JuanVqz commented Oct 5, 2023

Thanks for fixing this. I guess I’d prefer to return 0 results without an error message. This isn’t currently possible with the Search model. Do you see an unintrusive way to make that possible?

@pushcx I updated it, let me know what you think about it.

Another option could be to redirect to another page instead of rendering index

@pushcx pushcx merged commit d316c3b into lobsters:master Oct 7, 2023
1 check passed
@pushcx
Copy link
Member

pushcx commented Oct 7, 2023

This works nicely, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants