claude spec for prod bug #6659
claude spec for prod bug #6659
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a production bug where filtering volunteers by extra languages with default ordering caused a PostgreSQL PG::InvalidColumnReference error. The error occurred because the extra_languages filter adds .distinct to the query, but the default ORDER BY COALESCE(users.display_name, users.email) expression wasn't included in the SELECT list, which PostgreSQL requires for SELECT DISTINCT queries.
Changes:
- Added the default sort expression as a named column alias in the SELECT clause
- Updated the order clause to reference the named alias instead of the raw expression
- Added comprehensive test coverage for all extra_languages filter scenarios with default ordering
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/datatables/volunteer_datatable.rb | Fixed PG::InvalidColumnReference by adding default_sort_order column alias to SELECT and referencing it in ORDER BY |
| spec/datatables/volunteer_datatable_spec.rb | Added tests verifying the bug fix across all extra_languages filter code paths |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| it "does not raise PG::InvalidColumnReference error" do | ||
| expect { subject }.not_to raise_error | ||
| end |
There was a problem hiding this comment.
Consider adding functional assertions to verify that the correct volunteers are returned when filtering for volunteers without extra languages. Similar to line 542-544, you could add an assertion like: expect(subject[:data].map { |d| d[:id].to_i }).to contain_exactly(volunteer_without_language.id) to ensure the filter logic works correctly, not just that it doesn't raise an error.
| it "does not raise PG::InvalidColumnReference error" do | ||
| expect { subject }.not_to raise_error | ||
| end |
There was a problem hiding this comment.
Consider adding functional assertions to verify that both volunteers are returned when filtering with multiple extra_languages options. You could add an assertion like: expect(subject[:data].map { |d| d[:id].to_i }).to contain_exactly(volunteer_with_language.id, volunteer_without_language.id) to ensure the filter logic works correctly when both true and false are selected.
What github issue is this PR for, if any?
Resolves #XXXX
What changed, and why?
How is this tested? (please write rspec and jest tests!) 💖💪
Note: if you see a flake in your test build in github actions, please post in slack #casa "Flaky test: " :) 💪
Note: We love capybara tests! If you are writing both haml/js and ruby, please try to test your work with tests at every level including system tests like https://github.com/rubyforgood/casa/tree/main/spec/system
Screenshots please :)
Run your local server and take a screenshot of your work! Try to include the URL of the page as well as the contents of the page.
Feelings gif (optional)
What gif best describes your feeling working on this issue? https://giphy.com/
How to embed:
