Improvements for search test coverage #1513
Conversation
…function & added unit test for it
Oaphi
force-pushed
the
0valt/1509/search_tests
branch
from
b8cc3fb to
71dc1af
Compare
…s note) as 'get' prefix is not idiomatic
app/helpers/search_helper.rb
Outdated
| def accessible_posts_for(user) | ||
| (user&.is_moderator || user&.is_admin ? Post : Post.undeleted) | ||
| .qa_only.list_includes | ||
| end |
There was a problem hiding this comment.
Should this also check category access settings here? We've got an open security report that posts in private categories sometimes show in search; this might be an ideal place to check that.
There was a problem hiding this comment.
Yup, looks like just the place to do so - will update once I am done with basic test coverage for the helper (want to make sure there are no other surprises first)
There was a problem hiding this comment.
Or actually sooner than that as properly testing the :category qualifier requires the check to work correctly.
There was a problem hiding this comment.
After reading through the security report itself, I realized that it doesn't seem to be related to search, @ArtOfCode-, am I missing something? I abstracted the check for category access into accessible_categories_for just in case, but the tests show that search handles category access correctly.
There was a problem hiding this comment.
That's a different report - there's another one open in the support tickets that hasn't made it to a GH advisory yet.
There was a problem hiding this comment.
Ah, got it, there is another one. Hmm, looks like that one might crop up when there is no filter by category. Will take a look
part of #1509