Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion app/datatables/volunteer_datatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def raw_records
.select(
<<-SQL
users.*,
COALESCE(users.display_name, users.email) AS default_sort_order,
COALESCE(supervisors.display_name, supervisors.email) AS supervisor_name,
supervisors.id AS supervisor_id,
transition_aged_youth_cases.volunteer_id IS NOT NULL AS has_transition_aged_youth_cases,
Expand Down Expand Up @@ -133,7 +134,7 @@ def hours_spent_in_days_subquery
end

def order_clause
@order_clause ||= build_order_clause || Arel.sql("COALESCE(users.display_name, users.email) ASC")
@order_clause ||= build_order_clause || Arel.sql("default_sort_order ASC")
end

def supervisor_filter
Expand Down
70 changes: 70 additions & 0 deletions spec/datatables/volunteer_datatable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -506,4 +506,74 @@
end
end
end

describe "extra_languages filter" do
let(:org) { build :casa_org }
let(:supervisor) { create :supervisor, casa_org: org }
let(:volunteer_with_language) { create :volunteer, casa_org: org, supervisor: supervisor }
let(:volunteer_without_language) { create :volunteer, casa_org: org, supervisor: supervisor }
let(:language) { create :language, casa_org: org }

before do
create :user_language, user: volunteer_with_language, language: language
end

context "when filtering for volunteers with extra languages with default ordering" do
# Use an invalid order_by to trigger the default COALESCE ordering
# which causes PG::InvalidColumnReference when combined with DISTINCT
let(:order_by) { "invalid_column" }
let(:additional_filters) do
{
active: %w[false true],
supervisor: [supervisor.id],
transition_aged_youth: %w[false true],
extra_languages: %w[true]
}
end

it "does not raise PG::InvalidColumnReference error" do
# Bug: PG::InvalidColumnReference: ERROR: for SELECT DISTINCT, ORDER BY
# expressions must appear in select list
# This occurs because extra_languages filter adds .distinct but the default
# ORDER BY COALESCE(users.display_name, users.email) is not in the SELECT list
expect { subject }.not_to raise_error
end

it "returns only volunteers with languages" do
expect(subject[:data].map { |d| d[:id].to_i }).to contain_exactly(volunteer_with_language.id)
end
end

context "when filtering for volunteers without extra languages with default ordering" do
let(:order_by) { "invalid_column" }
let(:additional_filters) do
{
active: %w[false true],
supervisor: [supervisor.id],
transition_aged_youth: %w[false true],
extra_languages: %w[false]
}
end

it "does not raise PG::InvalidColumnReference error" do
expect { subject }.not_to raise_error
end
Comment on lines +558 to +560
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
end

context "when filtering with multiple extra_languages options with default ordering" do
let(:order_by) { "invalid_column" }
let(:additional_filters) do
{
active: %w[false true],
supervisor: [supervisor.id],
transition_aged_youth: %w[false true],
extra_languages: %w[true false]
}
end

it "does not raise PG::InvalidColumnReference error" do
expect { subject }.not_to raise_error
end
Comment on lines +574 to +576
Copy link

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
end
end
end
Loading