-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fixed the hard coded sorting for profiles search #10693
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
Fixed the hard coded sorting for profiles search #10693
Conversation
Thanks for opening this pull request! This space is protected by our Code of Conduct - and we're here to help. |
Codecov Report
@@ Coverage Diff @@
## main #10693 +/- ##
==========================================
+ Coverage 82.43% 82.45% +0.01%
==========================================
Files 98 98
Lines 5990 5996 +6
==========================================
+ Hits 4938 4944 +6
Misses 1052 1052
|
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
app/services/search_service.rb
Outdated
user_scope.joins(:revisions) | ||
.where("node_revisions.status = 1") | ||
.order("node_revisions.timestamp #{search_criteria.order_direction}") | ||
user_scope.joins(:revisions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use 2 (not 0) spaces for indentation.
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
Hi @jstrickl-tunes, do you think you could add functional tests for this feature? Take a look at some test examples you can use as a guide plots2/test/functional/api/search_api_test.rb Lines 11 to 30 in dc60919
Great job!! |
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
Code Climate has analyzed commit ca27d6d and detected 0 issues on this pull request. View more on Code Climate. |
This pull request generated screenshots of many common pages in the running app. You should be able to download and view them here: |
Hi 😄, this issue has been automatically marked as stale because it has not had recent activity. Don't worry you can continue to work on this and ask @publiclab/reviewers to add |
The profile search was hard coded to perform only one type of sorting, so I removed that hard code in the controller and added in getting the users choice from the parameters.
Fixes #9334
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test
@publiclab/reviewers
for help, in a comment below