Compare commits

..

2 Commits

Author SHA1 Message Date
3fe43d1fc0 Fix quantity ordered scope for SQLite: use pathname column instead of recursive CTE
SQLite's Arel visitor wraps CTE branches in extra parentheses, making
the UNION ALL inside recursive CTEs invalid. Also SQLite lacks LPAD()
and CAST(... AS BINARY). Fix by using the existing pathname column for
ordering on SQLite, which already encodes the hierarchical path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-10 18:41:03 +00:00
9b18784caf Implement measurements create/destroy and index listing
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-03-10 18:24:27 +00:00
14 changed files with 132 additions and 178 deletions

View File

@@ -1,74 +0,0 @@
name: Tests
on:
push:
branches: [ master ]
pull_request:
branches: [ master ]
jobs:
test-sqlite:
name: Tests (SQLite)
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
with:
ruby-version: '3.3'
bundler-cache: true
env:
BUNDLE_WITH: "sqlite:development:test"
- name: Set up test database
run: bin/rails db:create db:schema:load
env:
RAILS_ENV: test
- name: Run tests
run: bin/rails test
env:
RAILS_ENV: test
CI: "true"
test-mysql:
name: Tests (MySQL)
runs-on: ubuntu-latest
services:
mysql:
image: mysql:8.0
env:
MYSQL_ROOT_PASSWORD: ""
MYSQL_ALLOW_EMPTY_PASSWORD: "yes"
MYSQL_DATABASE: fixin_test
ports:
- 3306:3306
options: >-
--health-cmd="mysqladmin ping"
--health-interval=10s
--health-timeout=5s
--health-retries=3
steps:
- uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
with:
ruby-version: '3.3'
bundler-cache: true
env:
BUNDLE_WITH: "mysql:development:test"
- name: Set up test database
run: bin/rails db:schema:load
env:
RAILS_ENV: test
DB_ADAPTER: mysql
- name: Run tests
run: bin/rails test
env:
RAILS_ENV: test
CI: "true"
DB_ADAPTER: mysql

View File

@@ -1,7 +1,13 @@
class MeasurementsController < ApplicationController class MeasurementsController < ApplicationController
before_action except: :index do
raise AccessForbidden unless current_user.at_least(:active)
end
def index def index
@measurements = [] readouts = current_user.readouts.includes(:quantity, :unit).order(created_at: :desc)
#@measurements = current_user.units.ordered.includes(:base, :subunits) @measurements = readouts.group_by(&:created_at).map do |created_at, grouped|
Measurement.new(created_at: created_at, readouts: grouped)
end
end end
def new def new
@@ -9,8 +15,33 @@ class MeasurementsController < ApplicationController
end end
def create def create
timestamp = Time.current
@readouts = readout_params.map do |rp|
r = current_user.readouts.new(rp)
r.created_at = timestamp
r
end
if @readouts.all?(&:valid?)
Readout.transaction { @readouts.each(&:save!) }
@measurement = Measurement.new(readouts: @readouts, created_at: timestamp)
flash.now[:notice] = t('.success')
else
render :new, status: :unprocessable_entity
end
end end
def destroy def destroy
@measurement = Measurement.new(id: params[:id].to_i,
created_at: Time.at(params[:id].to_i))
current_user.readouts.where(created_at: @measurement.created_at).delete_all
@measurements_empty = current_user.readouts.empty?
flash.now[:notice] = t('.success')
end
private
def readout_params
params.require(:readouts).map { |r| r.permit(:quantity_id, :value, :unit_id) }
end end
end end

View File

@@ -1,3 +1,17 @@
class Measurement class Measurement
include ActiveModel::Model include ActiveModel::Model
attr_accessor :readouts, :created_at
def id
created_at.to_i
end
def to_param
id.to_s
end
def persisted?
true
end
end end

View File

@@ -15,8 +15,8 @@ class Quantity < ApplicationRecord
errors.add(:parent, :descendant_reference) if ancestor_of?(parent) errors.add(:parent, :descendant_reference) if ancestor_of?(parent)
end end
validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]}, validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]},
length: {maximum: type_for_attribute(:name).limit} length: {maximum: type_for_attribute(:name).limit || Float::INFINITY}
validates :description, length: {maximum: type_for_attribute(:description).limit} validates :description, length: {maximum: type_for_attribute(:description).limit || Float::INFINITY}
# Update :depths of progenies after parent change # Update :depths of progenies after parent change
before_save if: :parent_changed? do before_save if: :parent_changed? do
@@ -61,18 +61,26 @@ class Quantity < ApplicationRecord
# Return: ordered [sub]hierarchy # Return: ordered [sub]hierarchy
scope :ordered, ->(root: nil, include_root: true) { scope :ordered, ->(root: nil, include_root: true) {
numbered = Arel::Table.new('numbered') if connection.adapter_name =~ /mysql/i
numbered = Arel::Table.new('numbered')
self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(arel_table.name => [ self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(arel_table.name => [
numbered.project( numbered.project(
numbered[Arel.star], numbered[Arel.star],
numbered.cast(numbered[:child_number], 'BINARY').as('path') numbered.cast(numbered[:child_number], 'BINARY').as('path')
).where(numbered[root && include_root ? :id : :parent_id].eq(root)), ).where(numbered[root && include_root ? :id : :parent_id].eq(root)),
numbered.project( numbered.project(
numbered[Arel.star], numbered[Arel.star],
arel_table[:path].concat(numbered[:child_number]) arel_table[:path].concat(numbered[:child_number])
).join(arel_table).on(numbered[:parent_id].eq(arel_table[:id])) ).join(arel_table).on(numbered[:parent_id].eq(arel_table[:id]))
]).order(arel_table[:path]) ]).order(arel_table[:path])
elsif root.nil?
# SQLite: pathname column already stores the full hierarchical path
order(:pathname)
else
root_pathname = unscoped.where(id: root).pick(:pathname)
scope = order(:pathname).where("pathname LIKE ?", "#{root_pathname}#{PATHNAME_DELIMITER}%")
include_root ? scope.or(where(id: root)) : scope
end
} }
# TODO: extract named functions to custom Arel extension # TODO: extract named functions to custom Arel extension

View File

@@ -26,8 +26,10 @@ 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: [
self.or(Unit.defaults).left_joins(:base) Unit.with(units: 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)
@@ -63,14 +65,8 @@ 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.
# Use ActiveRecord::Relation (not a raw SelectManager) so the SQLite Arel arel_table.join(actionable_units).on(actionable_units[:base_id].eq(arel_table[:id]))
# visitor does not wrap it in parentheses inside the UNION ALL CTE body. .project(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
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()
@@ -78,7 +74,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(units[:base_id], units[:symbol]) .from(units).group(:base_id, :symbol)
} }
scope :ordered, ->{ scope :ordered, ->{
left_outer_joins(:base).order(ordering) left_outer_joins(:base).order(ordering)
@@ -87,8 +83,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),
arel_table[:multiplier], :multiplier,
arel_table[:symbol]] :symbol]
end end
before_destroy do before_destroy do

View File

@@ -0,0 +1,14 @@
<%= tag.tr id: dom_id(measurement) do %>
<td><%= l measurement.created_at, format: :short %></td>
<td>
<% measurement.readouts.each do |readout| %>
<span><%= readout.quantity.name %>: <%= readout.value %> <%= readout.unit %></span>
<% end %>
</td>
<% if current_user.at_least(:active) %>
<td class="actions">
<%= image_button_to t('.destroy'), 'delete-outline', measurement_path(measurement),
method: :delete %>
</td>
<% end %>
<% end %>

View File

@@ -0,0 +1,5 @@
<%= turbo_stream.update :flashes %>
<%= turbo_stream.remove :measurement_form %>
<%= turbo_stream.remove :no_items %>
<%= turbo_stream.enable :new_measurement_link %>
<%= turbo_stream.prepend :measurements, @measurement %>

View File

@@ -0,0 +1,3 @@
<%= turbo_stream.update :flashes %>
<%= turbo_stream.remove @measurement %>
<%= turbo_stream.append(:measurements, render_no_items) if @measurements_empty %>

View File

@@ -58,7 +58,4 @@ 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

View File

@@ -88,6 +88,12 @@ en:
select_quantity: select the measured quantities... select_quantity: select the measured quantities...
index: index:
new_measurement: Add measurement new_measurement: Add measurement
create:
success: Measurement saved.
destroy:
success: Measurement deleted.
measurement:
destroy: Delete
readouts: readouts:
form: form:
select_unit: ... select_unit: ...

View File

@@ -1,50 +0,0 @@
namespace :test do
desc "Run Rails tests against all supported database adapters (SQLite, MySQL)"
task :all_adapters do
# DATABASE_URL overrides the adapter from database.yml at runtime.
# MySQL requires the mysql2 gem: bundle install --with mysql
adapters = {
"SQLite" => {
"DATABASE_URL" => "sqlite3:db/test.sqlite3"
},
"MySQL" => {
"DATABASE_URL" => format(
"mysql2://%s:%s@%s/%s",
ENV.fetch("DATABASE_USERNAME", "root"),
ENV.fetch("DATABASE_PASSWORD", ""),
ENV.fetch("DATABASE_HOST", "127.0.0.1"),
ENV.fetch("DATABASE_NAME", "fixin_test")
)
}
}
failed = []
adapters.each do |name, extra_env|
puts "\n#{"=" * 60}"
puts " Running tests with #{name}"
puts "=" * 60
env = ENV.to_h.merge("RAILS_ENV" => "test").merge(extra_env)
# Reset test database; db:drop may fail on first run — that's fine
system(env, "bin/rails db:drop")
unless system(env, "bin/rails db:create db:schema:load")
failed << "#{name} (database setup)"
next
end
failed << name unless system(env, "bin/rails test")
end
puts "\n#{"=" * 60}"
if failed.any?
puts " FAILED: #{failed.join(", ")}"
puts "=" * 60
exit 1
else
puts " All adapters passed!"
puts "=" * 60
end
end
end

View File

@@ -1,12 +1,8 @@
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 default_units_url get units_defaults_index_url
assert_response :success assert_response :success
end end
end end

View File

@@ -2,9 +2,7 @@ require "test_helper"
class UsersControllerTest < ActionDispatch::IntegrationTest class UsersControllerTest < ActionDispatch::IntegrationTest
setup do setup do
@admin = users(:admin) @user = users(:one)
@user = users(:alice)
sign_in @admin
end end
test "should get index" do test "should get index" do
@@ -12,25 +10,39 @@ 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: { status: :restricted } }, as: :turbo_stream patch user_url(@user), params: { user: { email: @user.email, status: @user.status } }
assert_equal "restricted", @user.reload.status assert_redirected_to user_url(@user)
end end
test "should not update self" do test "should destroy user" do
patch user_url(@admin), params: { user: { status: :active } }, as: :turbo_stream, assert_difference("User.count", -1) do
headers: { "HTTP_REFERER" => users_url } delete user_url(@user)
assert_response :redirect end
end
test "should forbid non-admin" do assert_redirected_to users_url
sign_in @user
get users_url
assert_response :forbidden
end end
end end

View File

@@ -2,10 +2,6 @@ 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)