Improve controllers and views

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

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

app/controllers/units_controller.rb

@@ -1,10 +1,9 @@
 class UnitsController < ApplicationController
-  before_action only: :new do
+  before_action ->{ @unit = current_user.units.find_by!(id: params[:id]) },
-    find_unit if params[:id].present?
+    only: [:new, :edit, :update, :rebase, :destroy],
-  end
+    unless: ->{ action_name == "new" && params[:id].nil? }
-  before_action :find_unit, only: [:edit, :update, :rebase, :destroy]
 
-  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

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]},

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) %>

app/views/units/edit.turbo_stream.erb

@@ -1,7 +1,7 @@
-<% ids = {row: dom_id(@unit, :edit),
+<% ids = {row: dom_target(:edit, @unit),
-          hidden_row: dom_id(@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 %>

app/views/units/new.turbo_stream.erb

@@ -1,8 +1,8 @@
-<% dom_obj = @unit.base || @unit %>
+<% target_id = dom_target(:new, @unit.base || :unit) %>
-<% ids = {row: dom_id(dom_obj, :new),
+<% ids = {row: target_id,
           hidden_row: nil,
-          link: dom_id(dom_obj, :new, :link),
+          link: dom_target(target_id, :link),
-          form_tag: dom_id(dom_obj, :new, :form)} %>
+          form_tag: dom_target(target_id, :form)} %>
 
 <%= turbo_stream.disable ids[:link] -%>
 

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 %>

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}"