From eae0ee9d20d04bae68795f261ad244bcbfea027c Mon Sep 17 00:00:00 2001 From: cryptogopher Date: Tue, 28 Jul 2026 00:46:33 +0200 Subject: [PATCH] Improve controllers and views --- app/controllers/default/units_controller.rb | 6 +++--- app/controllers/quantities_controller.rb | 6 +++--- app/controllers/units_controller.rb | 21 +++++++------------ app/models/quantity.rb | 1 + app/views/units/create.turbo_stream.erb | 2 +- app/views/units/edit.turbo_stream.erb | 6 +++--- app/views/units/new.turbo_stream.erb | 8 +++---- app/views/units/update.turbo_stream.erb | 2 +- .../record_identifier_with_suffix.rb | 1 + 9 files changed, 25 insertions(+), 28 deletions(-) diff --git a/app/controllers/default/units_controller.rb b/app/controllers/default/units_controller.rb index 1dabae9..06e9d25 100644 --- a/app/controllers/default/units_controller.rb +++ b/app/controllers/default/units_controller.rb @@ -17,7 +17,7 @@ class Default::UnitsController < ApplicationController def import @unit.port!(current_user) - flash.now[:notice] = t('.success', unit: @unit) + flash.now.notice = t('.success', unit: @unit) ensure run_and_render :index end @@ -30,14 +30,14 @@ class Default::UnitsController < ApplicationController def export @unit.port!(nil) - flash.now[:notice] = t('.success', unit: @unit) + flash.now.notice = t('.success', unit: @unit) ensure run_and_render :index end def destroy @unit.destroy! - flash.now[:notice] = t('.success', unit: @unit) + flash.now.notice = t('.success', unit: @unit) ensure run_and_render :index end diff --git a/app/controllers/quantities_controller.rb b/app/controllers/quantities_controller.rb index 86dc12d..47bce3b 100644 --- a/app/controllers/quantities_controller.rb +++ b/app/controllers/quantities_controller.rb @@ -21,7 +21,7 @@ class QuantitiesController < ApplicationController if @quantity.save @before = @quantity.successive @ancestors = @quantity.ancestors - flash.now[:notice] = t('.success', quantity: @quantity) + flash.now.notice = t('.success', quantity: @quantity) else render :new end @@ -33,7 +33,7 @@ class QuantitiesController < ApplicationController def update if @quantity.update(quantity_params.except(:parent_id)) @ancestors = @quantity.ancestors - flash.now[:notice] = t('.success', quantity: @quantity) + flash.now.notice = t('.success', quantity: @quantity) else render :edit end @@ -57,7 +57,7 @@ class QuantitiesController < ApplicationController def destroy @quantity.destroy! @ancestors = @quantity.ancestors - flash.now[:notice] = t('.success', quantity: @quantity) + flash.now.notice = t('.success', quantity: @quantity) end private diff --git a/app/controllers/units_controller.rb b/app/controllers/units_controller.rb index b064773..97f116f 100644 --- a/app/controllers/units_controller.rb +++ b/app/controllers/units_controller.rb @@ -1,10 +1,9 @@ class UnitsController < ApplicationController - before_action only: :new do - find_unit if params[:id].present? - end - before_action :find_unit, only: [:edit, :update, :rebase, :destroy] + before_action ->{ @unit = current_user.units.find_by!(id: params[:id]) }, + only: [:new, :edit, :update, :rebase, :destroy], + unless: ->{ action_name == "new" && params[:id].nil? } - before_action except: :index do + before_action except: [:index] do raise AccessForbidden unless current_user.at_least(:active) end @@ -20,7 +19,7 @@ class UnitsController < ApplicationController @unit = current_user.units.new(params.expect(Unit::ATTRIBUTES)) if @unit.save @before = @unit.successive - flash.now[:notice] = t('.success', unit: @unit) + flash.now.notice = t('.success', unit: @unit) else render_errors @unit end @@ -31,7 +30,7 @@ class UnitsController < ApplicationController def update if @unit.update(params.except(:base_id).expect(Unit::ATTRIBUTES)) - flash.now[:notice] = t('.success', unit: @unit) + flash.now.notice = t('.success', unit: @unit) else render_errors @unit end @@ -48,18 +47,14 @@ class UnitsController < ApplicationController @before = @unit.successive if @unit.multiplier_previously_changed? - flash.now[:notice] = t(".multiplier_reset", unit: @unit) + flash.now.notice = t(".multiplier_reset", unit: @unit) end end def destroy @unit.destroy! - flash.now[:notice] = t('.success', unit: @unit) + flash.now.notice = t('.success', unit: @unit) end private - - def find_unit - @unit = current_user.units.find_by!(id: params[:id]) - end end diff --git a/app/models/quantity.rb b/app/models/quantity.rb index ae64c8a..f1bcbef 100644 --- a/app/models/quantity.rb +++ b/app/models/quantity.rb @@ -13,6 +13,7 @@ class Quantity < ApplicationRecord errors.add(:parent, :self_reference) if id == parent_id end validate if: ->{ parent.present? }, on: :update do + # TODO: should not report error for (parent == self) errors.add(:parent, :descendant_reference) if ancestor_of?(parent) end validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]}, diff --git a/app/views/units/create.turbo_stream.erb b/app/views/units/create.turbo_stream.erb index 75fb4f5..5d71001 100644 --- a/app/views/units/create.turbo_stream.erb +++ b/app/views/units/create.turbo_stream.erb @@ -1,4 +1,4 @@ -<%= turbo_stream.close_form dom_id(@unit.base || Unit, :new) %> +<%= turbo_stream.close_form dom_target(:new, @unit.base || :unit) %> <%= turbo_stream.remove :no_items %> <%= turbo_stream.replace @unit.base if @unit.base_id? %> <%= @before ? turbo_stream.before(@before, @unit) : turbo_stream.append(:units, @unit) %> diff --git a/app/views/units/edit.turbo_stream.erb b/app/views/units/edit.turbo_stream.erb index 0ec4f9d..b3cbf79 100644 --- a/app/views/units/edit.turbo_stream.erb +++ b/app/views/units/edit.turbo_stream.erb @@ -1,7 +1,7 @@ -<% ids = {row: dom_id(@unit, :edit), - hidden_row: dom_id(@unit), +<% ids = {row: dom_target(:edit, @unit), + hidden_row: dom_target(@unit), link: nil, - form_tag: dom_id(@unit, :edit, :form)} %> + form_tag: dom_target(:edit, @unit, :form)} %> <%= turbo_stream.append :unit_form do %> <%- tabular_form_with model: @unit, html: {id: ids[:form_tag]} do %> diff --git a/app/views/units/new.turbo_stream.erb b/app/views/units/new.turbo_stream.erb index 8540537..1397e56 100644 --- a/app/views/units/new.turbo_stream.erb +++ b/app/views/units/new.turbo_stream.erb @@ -1,8 +1,8 @@ -<% dom_obj = @unit.base || @unit %> -<% ids = {row: dom_id(dom_obj, :new), +<% target_id = dom_target(:new, @unit.base || :unit) %> +<% ids = {row: target_id, hidden_row: nil, - link: dom_id(dom_obj, :new, :link), - form_tag: dom_id(dom_obj, :new, :form)} %> + link: dom_target(target_id, :link), + form_tag: dom_target(target_id, :form)} %> <%= turbo_stream.disable ids[:link] -%> diff --git a/app/views/units/update.turbo_stream.erb b/app/views/units/update.turbo_stream.erb index 99d7adc..9be69ba 100644 --- a/app/views/units/update.turbo_stream.erb +++ b/app/views/units/update.turbo_stream.erb @@ -1,3 +1,3 @@ -<%= turbo_stream.close_form dom_id(@unit, :edit) %> +<%= turbo_stream.close_form dom_target(:edit, @unit) %> <%= turbo_stream.replace @unit.base if @unit.base_id? %> <%= turbo_stream.replace @unit %> diff --git a/lib/core_ext/action_view/record_identifier_with_suffix.rb b/lib/core_ext/action_view/record_identifier_with_suffix.rb index 2e5138b..5e2a853 100644 --- a/lib/core_ext/action_view/record_identifier_with_suffix.rb +++ b/lib/core_ext/action_view/record_identifier_with_suffix.rb @@ -1,4 +1,5 @@ module CoreExt::ActionView::RecordIdentifierWithSuffix + # TODO: replace dom_id with dom_target, then remove this override def dom_id(object, prefix = nil, suffix = nil) if suffix "#{super(object, prefix)}#{::ActionView::RecordIdentifier::JOIN}#{suffix}"