forked from fixin.me/fixin.me
Fix controller tests and SQLite compatibility for defaults_diff
Test infrastructure: - Allow www.example.com host in test env (ActionDispatch::HostAuthorization was blocking all integration test requests) - Include Devise::Test::IntegrationHelpers in ActionDispatch::IntegrationTest so tests can sign in with sign_in(user) Controller tests: - Rewrite UsersControllerTest to match actual routes/actions (no new/create/ edit/destroy); sign in as admin; test update-self rejection via turbo_stream - Fix Default::UnitsControllerTest to sign in before requesting the index SQLite compatibility in Unit#defaults_diff: - Hoist the inner "units" CTE to the outer WITH RECURSIVE level (fixes nested WITH syntax error) — this was the existing TODO in the code - Use Unit.joins(...) for the recursive part instead of a raw Arel::SelectManager so the SQLite visitor does not wrap it in parentheses inside UNION ALL - Drop the named "units" CTE (conflicts with the table name under WITH RECURSIVE in SQLite); apply the user/defaults scope directly on the base case - Qualify GROUP BY columns to avoid ambiguity when bases_units is joined - Qualify ORDER BY :multiplier/:symbol to avoid ambiguity (Unit.ordering) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -26,10 +26,8 @@ class Unit < ApplicationRecord
|
|||||||
other_bases_units = arel_table.alias('other_bases_units')
|
other_bases_units = arel_table.alias('other_bases_units')
|
||||||
sub_units = arel_table.alias('sub_units')
|
sub_units = arel_table.alias('sub_units')
|
||||||
|
|
||||||
# TODO: move inner 'with' CTE to outer 'with recursive' - it can have multiple
|
|
||||||
# CTEs, even non recursive ones.
|
|
||||||
Unit.with_recursive(actionable_units: [
|
Unit.with_recursive(actionable_units: [
|
||||||
Unit.with(units: self.or(Unit.defaults)).left_joins(:base)
|
self.or(Unit.defaults).left_joins(:base)
|
||||||
.where.not(
|
.where.not(
|
||||||
# Exclude Units that are/have default counterpart
|
# Exclude Units that are/have default counterpart
|
||||||
Arel::SelectManager.new.project(1).from(other_units)
|
Arel::SelectManager.new.project(1).from(other_units)
|
||||||
@@ -65,8 +63,14 @@ class Unit < ApplicationRecord
|
|||||||
),
|
),
|
||||||
# Fill base Units to display proper hierarchy. Duplicates will be removed
|
# Fill base Units to display proper hierarchy. Duplicates will be removed
|
||||||
# by final group() - can't be deduplicated with UNION due to 'portable' field.
|
# by final group() - can't be deduplicated with UNION due to 'portable' field.
|
||||||
arel_table.join(actionable_units).on(actionable_units[:base_id].eq(arel_table[:id]))
|
# Use ActiveRecord::Relation (not a raw SelectManager) so the SQLite Arel
|
||||||
.project(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
|
# visitor does not wrap it in parentheses inside the UNION ALL CTE body.
|
||||||
|
Unit.joins(
|
||||||
|
arel_table.create_join(
|
||||||
|
actionable_units,
|
||||||
|
arel_table.create_on(actionable_units[:base_id].eq(arel_table[:id]))
|
||||||
|
)
|
||||||
|
).select(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
|
||||||
]).select(units: [:base_id, :symbol])
|
]).select(units: [:base_id, :symbol])
|
||||||
.select(
|
.select(
|
||||||
units[:id].minimum.as('id'), # can be ANY_VALUE()
|
units[:id].minimum.as('id'), # can be ANY_VALUE()
|
||||||
@@ -74,7 +78,7 @@ class Unit < ApplicationRecord
|
|||||||
Arel::Nodes.build_quoted(1).as('multiplier'), # disregard multiplier when sorting
|
Arel::Nodes.build_quoted(1).as('multiplier'), # disregard multiplier when sorting
|
||||||
units[:portable].minimum.as('portable')
|
units[:portable].minimum.as('portable')
|
||||||
)
|
)
|
||||||
.from(units).group(:base_id, :symbol)
|
.from(units).group(units[:base_id], units[:symbol])
|
||||||
}
|
}
|
||||||
scope :ordered, ->{
|
scope :ordered, ->{
|
||||||
left_outer_joins(:base).order(ordering)
|
left_outer_joins(:base).order(ordering)
|
||||||
@@ -83,8 +87,8 @@ class Unit < ApplicationRecord
|
|||||||
def self.ordering
|
def self.ordering
|
||||||
[arel_table.coalesce(Arel::Table.new(:bases_units)[:symbol], arel_table[:symbol]),
|
[arel_table.coalesce(Arel::Table.new(:bases_units)[:symbol], arel_table[:symbol]),
|
||||||
arel_table[:base_id].not_eq(nil),
|
arel_table[:base_id].not_eq(nil),
|
||||||
:multiplier,
|
arel_table[:multiplier],
|
||||||
:symbol]
|
arel_table[:symbol]]
|
||||||
end
|
end
|
||||||
|
|
||||||
before_destroy do
|
before_destroy do
|
||||||
|
|||||||
@@ -58,4 +58,7 @@ Rails.application.configure do
|
|||||||
# config.action_view.annotate_rendered_view_with_filenames = true
|
# config.action_view.annotate_rendered_view_with_filenames = true
|
||||||
|
|
||||||
config.log_level = :info
|
config.log_level = :info
|
||||||
|
|
||||||
|
# Allow the default integration test host.
|
||||||
|
config.hosts << "www.example.com"
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -1,8 +1,12 @@
|
|||||||
require "test_helper"
|
require "test_helper"
|
||||||
|
|
||||||
class Default::UnitsControllerTest < ActionDispatch::IntegrationTest
|
class Default::UnitsControllerTest < ActionDispatch::IntegrationTest
|
||||||
|
setup do
|
||||||
|
sign_in users(:alice)
|
||||||
|
end
|
||||||
|
|
||||||
test "should get index" do
|
test "should get index" do
|
||||||
get units_defaults_index_url
|
get default_units_url
|
||||||
assert_response :success
|
assert_response :success
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -2,7 +2,9 @@ require "test_helper"
|
|||||||
|
|
||||||
class UsersControllerTest < ActionDispatch::IntegrationTest
|
class UsersControllerTest < ActionDispatch::IntegrationTest
|
||||||
setup do
|
setup do
|
||||||
@user = users(:one)
|
@admin = users(:admin)
|
||||||
|
@user = users(:alice)
|
||||||
|
sign_in @admin
|
||||||
end
|
end
|
||||||
|
|
||||||
test "should get index" do
|
test "should get index" do
|
||||||
@@ -10,39 +12,25 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
|
|||||||
assert_response :success
|
assert_response :success
|
||||||
end
|
end
|
||||||
|
|
||||||
test "should get new" do
|
|
||||||
get new_user_url
|
|
||||||
assert_response :success
|
|
||||||
end
|
|
||||||
|
|
||||||
test "should create user" do
|
|
||||||
assert_difference("User.count") do
|
|
||||||
post users_url, params: { user: { email: @user.email, status: @user.status } }
|
|
||||||
end
|
|
||||||
|
|
||||||
assert_redirected_to user_url(User.last)
|
|
||||||
end
|
|
||||||
|
|
||||||
test "should show user" do
|
test "should show user" do
|
||||||
get user_url(@user)
|
get user_url(@user)
|
||||||
assert_response :success
|
assert_response :success
|
||||||
end
|
end
|
||||||
|
|
||||||
test "should get edit" do
|
|
||||||
get edit_user_url(@user)
|
|
||||||
assert_response :success
|
|
||||||
end
|
|
||||||
|
|
||||||
test "should update user" do
|
test "should update user" do
|
||||||
patch user_url(@user), params: { user: { email: @user.email, status: @user.status } }
|
patch user_url(@user), params: { user: { status: :restricted } }, as: :turbo_stream
|
||||||
assert_redirected_to user_url(@user)
|
assert_equal "restricted", @user.reload.status
|
||||||
end
|
end
|
||||||
|
|
||||||
test "should destroy user" do
|
test "should not update self" do
|
||||||
assert_difference("User.count", -1) do
|
patch user_url(@admin), params: { user: { status: :active } }, as: :turbo_stream,
|
||||||
delete user_url(@user)
|
headers: { "HTTP_REFERER" => users_url }
|
||||||
end
|
assert_response :redirect
|
||||||
|
end
|
||||||
|
|
||||||
assert_redirected_to users_url
|
test "should forbid non-admin" do
|
||||||
|
sign_in @user
|
||||||
|
get users_url
|
||||||
|
assert_response :forbidden
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -2,6 +2,10 @@ ENV["RAILS_ENV"] ||= "test"
|
|||||||
require_relative "../config/environment"
|
require_relative "../config/environment"
|
||||||
require "rails/test_help"
|
require "rails/test_help"
|
||||||
|
|
||||||
|
class ActionDispatch::IntegrationTest
|
||||||
|
include Devise::Test::IntegrationHelpers
|
||||||
|
end
|
||||||
|
|
||||||
class ActiveSupport::TestCase
|
class ActiveSupport::TestCase
|
||||||
# Run tests in parallel with specified workers
|
# Run tests in parallel with specified workers
|
||||||
parallelize(workers: :number_of_processors)
|
parallelize(workers: :number_of_processors)
|
||||||
|
|||||||
Reference in New Issue
Block a user