Merge pull request #210 from kufu/refactors/plan_controller

Extract PlanSchedule logics from PlansController to ItemsController

app/controllers/application_controller.rb

@@ -15,6 +15,7 @@ class ApplicationController < ActionController::Base
   rescue_from Exception, with: :server_error
   rescue_from ActiveRecord::RecordNotFound, with: :not_found
   rescue_from ActionController::RoutingError, with: :not_found
+  rescue_from ActionController::ParameterMissing, with: :bad_request # TODO: Remove in Rails8
 
   def set_user
     @user = User.find(session[:user_id]) if session[:user_id]
@@ -37,6 +38,12 @@ def not_found(err = nil)
     render template: 'errors/not_found', status: 404, layout: 'application', content_type: 'text/html'
   end
 
+  def bad_request(err = nil)
+    Rails.logger.debug("#{err}\n#{err.backtrace.join("\n")}") if err
+
+    render status: :bad_request, body: nil
+  end
+
   def server_error(err)
     print_error_if_test(err)
     Rails.logger.error("#{err}\n#{err.backtrace.join("\n")}")

app/controllers/items_controller.rb

@@ -0,0 +1,97 @@
+# frozen_string_literal: true
+
+class ItemsController < ApplicationController
+  include EventRouting
+  include ScheduleTable
+
+  before_action :set_event
+  before_action :set_item, only: %i[update destroy]
+  before_action :set_plan
+  before_action :build_item, only: %i[create]
+  before_action :check_user_owns_plan
+  before_action :check_item_belongs_to_event
+
+  def create
+    if @plan.valid?
+      @item.save!
+    else
+      flash[:error] = @plan.errors.messages[:schedules]
+      redirect_to event_plan_path(@plan, event_name: @event.name) and return
+    end
+    @plan.update!(initial: false) if @plan.initial
+
+    if request.format.turbo_stream?
+      set_attributes_for_turbo_stream
+      render 'update'
+    else
+      redirect_to event_plan_path(@plan, event_name: @event.name)
+    end
+  end
+
+  def update
+    @item.update!(item_params)
+    render 'schedules/_card', locals: { schedule: @item.schedule, mode: :plan, inactive: false }
+  end
+
+  def destroy
+    @item.destroy!
+    @plan.update!(initial: false)
+
+    case params[:mode]
+    when 'schedule'
+      set_attributes_for_turbo_stream
+      render 'update'
+    when 'plan'
+      redirect_to event_plan_path(@plan, event_name: @event.name)
+    end
+  end
+
+  private
+
+  def item_params
+    params.require(:plan_schedule).permit(:memo)
+  end
+
+  def set_item
+    @item = PlanSchedule.find(params[:id])
+  end
+
+  def build_item
+    @item = @plan.plan_schedules.build(schedule: Schedule.find(params[:schedule_id]))
+  end
+
+  def set_plan
+    @plan = @item ? @item.plan : Plan.find(params[:plan_id])
+  end
+
+  def check_user_owns_plan
+    render status: :bad_request, body: nil if @plan.nil?
+    render status: :forbidden, body: nil unless @user.plans.include?(@plan)
+  end
+
+  def check_item_belongs_to_event
+    render status: :not_found, body: nil unless @item.schedule.track.event == @event
+  end
+
+  # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+  def set_attributes_for_turbo_stream
+    @schedules = @event.schedules.includes(:speakers, :track).order(:start_at)
+    @schedule_table = Schedule::Tables.new(@schedules)
+
+    @row, @track_list = catch(:abort) do
+      @schedule_table.days.each do |day|
+        @table = @schedule_table[day]
+        @table.rows.each do |row|
+          throw :abort, [row, @table.track_list] if row.schedules.map(&:id).include?(params[:schedule_id])
+        end
+      end
+    end
+
+    return unless @user.profile
+
+    @friends_schedules_map = @user.profile.friend_profiles.to_h do |profile|
+      [profile.id, profile.user.plans.find_by(event: @event)&.plan_schedules&.map(&:schedule_id) || []]
+    end
+  end
+  # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+end

app/controllers/plans_controller.rb

@@ -13,19 +13,8 @@ def show
   end
 
   def update
-    target = switch_update_type_and_exec
-
-    if plan_add_or_remove? && params[:mode] == 'schedule'
-      set_attributes_for_turbo_stream
-      render 'update'
-    elsif plan_add_or_remove? && params[:mode] == 'plan'
-      redirect_to event_plan_path(@plan, event_name: @event.name)
-    elsif target
-      render 'schedules/_card',
-             locals: { schedule: target, mode: params[:edit_memo_schedule_id] ? :plan : :schedule, inactive: false }
-    else
-      redirect_to event_plan_url(@plan, event_name: @event.name)
-    end
+    @plan.update!(plan_params)
+    redirect_to event_plan_url(@plan, event_name: @event.name)
   end
 
   def editable
@@ -45,62 +34,19 @@ def editable
   def create
     create_and_set_user unless @user
     @plan = @user.plans.where(event: @event).create!(plan_params)
-    add_and_remove_plans if plan_add_or_remove?
+    add_plan(params[:plan][:add_schedule_id]) if params[:plan][:add_schedule_id]
     redirect_to event_schedules_path(event_name: @plan.event.name)
   end
 
   private
 
-  def switch_update_type_and_exec
-    if plan_add_or_remove?
-      add_and_remove_plans
-    elsif params[:edit_memo_schedule_id]
-      edit_memo
-    elsif params[:plan][:description]
-      edit_description
-    elsif params[:plan][:public]
-      edit_password_and_visibility
-    elsif params[:plan][:title]
-      edit_title
-    else
-      head :bad_request
-    end
-  end
-
   def plan_params
-    params.require(:plan).permit(:title, :description, :public, :initial)
-  end
-
-  def plan_add_or_remove?
-    if params[:plan]
-      params[:plan][:add_schedule_id] || params[:plan][:remove_schedule_id]
-    else
-      params[:add_schedule_id] || params[:remove_schedule_id]
-    end
-  end
-
-  def redirect_path_with_identifier(target)
-    identifier = target&.start_at&.strftime('%Y-%m-%d')
-    (request.referer || schedules_path) + (identifier ? "##{identifier}" : '')
+    params.require(:plan).permit(:title, :description, :public)
   end
 
   def set_plan
-    @plan = Plan.on_event(@event).find(params[:id] || params[:plan_id])
+    @plan = Plan.on_event(@event).find(params[:id])
     raise ActiveRecord::RecordNotFound if @plan.user != @user && !@plan.public?
-  rescue StandardError => e
-    @plan ||= Plan.new
-    raise e
-  end
-
-  def add_and_remove_plans
-    ret = nil
-    add_id = params[:add_schedule_id] || params.dig(:plan, :add_schedule_id)
-    remove_id = params[:remove_schedule_id] || params.dig(:plan, :remove_schedule_id)
-    ActiveRecord::Base.transaction do
-      ret = add_plan(add_id) if add_id
-      ret = remove_plan(remove_id) if remove_id
-    end
-    ret
   end
 
   def add_plan(id)
@@ -109,34 +55,13 @@ def add_plan(id)
       ps.save!
     else
       flash[:error] = @plan.errors.messages[:schedules]
-      redirect_to event_schedules_path(event_name: @plan.event.name) && return
+      redirect_to event_schedules_path(event_name: @plan.event.name) and return
     end
-    @plan.update!(initial: false)
-    ps.schedule
-  end
-
-  def remove_plan(id)
-    schedule = Schedule.find(id)
-    @plan.plan_schedules.find_by(schedule:).destroy!
-    @plan.update!(initial: false)
-    schedule
-  end
-
-  def edit_memo
-    schedule = Schedule.find(params[:edit_memo_schedule_id])
-    plan_schedule = @plan.plan_schedules.find_by(schedule:)
-    plan_schedule.update!(memo: params[:memo])
-    schedule
-  end
-
-  def edit_description
-    @plan.update!(description: params[:plan][:description])
-    nil
   end
 
   def check_user_owns_plan
-    return render :bad_request if @plan.nil?
-    return render :bad_request unless @user.plans.include?(@plan)
+    render status: :bad_request, body: nil if @plan.nil?
+    render status: :forbidden, body: nil unless @user.plans.include?(@plan)
   end
 
   def edit_password_and_visibility
@@ -145,36 +70,4 @@ def edit_password_and_visibility
     @plan.save!
     nil
   end
-
-  def edit_title
-    @plan.update(title: params[:plan][:title])
-    nil
-  end
-
-  # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
-  def set_attributes_for_turbo_stream
-    @schedules = @event.schedules.includes(:speakers, :track).order(:start_at)
-    @schedule_table = Schedule::Tables.new(@schedules)
-
-    target_schedule_id = params[:add_schedule_id] ||
-                         params.dig(:plan, :add_schedule_id) ||
-                         params[:remove_schedule_id] ||
-                         params.dig(:plan, :remove_schedule_id)
-
-    @row, @track_list = catch(:abort) do
-      @schedule_table.days.each do |day|
-        @table = @schedule_table[day]
-        @table.rows.each do |row|
-          throw :abort, [row, @table.track_list] if row.schedules.map(&:id).include?(target_schedule_id)
-        end
-      end
-    end
-
-    return unless @user.profile
-
-    @friends_schedules_map = @user.profile.friend_profiles.to_h do |profile|
-      [profile.id, profile.user.plans.find_by(event: @event)&.plan_schedules&.map(&:schedule_id) || []]
-    end
-  end
-  # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
 end

app/helpers/items_helper.rb

@@ -0,0 +1,4 @@
+# frozen_string_literal: true
+
+module ItemsHelper
+end

app/views/items/create.html.erb

@@ -0,0 +1,4 @@
+<div>
+  <h1 class="font-bold text-4xl">Items#create</h1>
+  <p>Find me in app/views/items/create.html.erb</p>
+</div>

app/views/items/destroy.html.erb

@@ -0,0 +1,4 @@
+<div>
+  <h1 class="font-bold text-4xl">Items#destroy</h1>
+  <p>Find me in app/views/items/destroy.html.erb</p>
+</div>

app/views/items/update.turbo_stream.erb

@@ -0,0 +1,8 @@
+<%= turbo_stream.replace @row.turbo_stream_id do %>
+  <%= render partial: 'schedules/table_row', locals: { row: @row, plan: @plan, track_list: @track_list} %>
+<% end %>
+<%= turbo_stream.update "mobile-table-#{@table.day}" do %>
+  <% @table.rows.each do |row| %>
+    <%= render partial: 'schedules/mobile_table_row', locals: { row: row, plan: @plan, track_list: @table.track_list } %>
+  <% end %>
+<% end %>

app/views/schedules/_card.html.erb

@@ -60,14 +60,13 @@
               <%= I18n.t('button.update_memo') %>
             </button>
             <dialog id="<%= memo_dialog_element_id %>" class="dialog max-w-[min(100vw-16px,800px)]">
-              <%= form_with(url: event_plan_path(@plan, event_name: @plan.event.name), method: :patch) do |f| %>
+              <%= form_with(url: event_item_url(@plan.plan_schedules.find { _1.schedule == schedule }, event_name: @plan.event.name), scope: 'plan_schedule', method: :patch) do |f| %>
                 <div data-controller="word-counter" data-word-counter-max-value="<%= plan_memo_max_length %>" class="">
                   <div class="border-b-[1px] border-[rgb(214,211,208)] py-4 px-6 flex flex-col justify-start">
                     <p class="text-xl"><%= I18n.t('dialog.edit_memo', title: schedule.title) %></p>
                   </div>
                   <div class="max-h-[calc(100vh-212px)] overflow-auto">
                     <div class="w-full p-6">
-                      <%= f.hidden_field :edit_memo_schedule_id, value: schedule.id, id: "#{schedule.id}-#{memo_dialog_element_id}" %>
                       <%= f.text_area :memo, value: @plan.plan_schedules.find { _1.schedule == schedule }&.memo, id: SecureRandom.uuid, class: "border opacity-100 rounded-md border border-[rgb(214,211,208)] bg-white p-2 text-[rgb(35,34,30)] w-full", data: { "word-counter-target": "source", action: "input->word-counter#calc" } %>
                       <div class="font-xs mt-2 text-[rgb(112,109,101)]">
                         <span data-word-counter-target="counter"></span>/<%= plan_memo_max_length %>
@@ -89,9 +88,7 @@
         <% unless mode == :team %>
           <div>
             <% if @plan.plan_schedules.map(&:schedule).include?(schedule) # to avoid n+1 %>
-              <%= form_with(url: event_plan_url(@plan, event_name: @plan.event.name), method: 'PATCH' ) do |form| %>
-                <%= form.hidden_field :remove_schedule_id, value: schedule.id, id: SecureRandom.uuid %>
-                <%= form.hidden_field :mode, value: mode, id: SecureRandom.uuid %>
+              <%= form_with(url: event_item_url(@plan.plan_schedules.find_by(schedule:), event_name: @plan.event.name), method: :delete ) do |form| %>
                 <%= form.submit I18n.t('card.remove'), class: "remove-plan-button p-2 text-sm font-bold min-h-5 normal-button", data: { turbo_frame: "event_#{@event.id}" } %>
               <% end %>
             <% elsif @plan.new_record? %>
@@ -120,7 +117,7 @@
                   <div class="flex flex-col border-solid border-t border-[rgb(214,211,208)] py-4 px-6">
                     <div class="flex justify-end">
                       <button class="normal-button" data-action="click->dialog#close"><%= I18n.t('button.close') %></button>
-                      <%= form_with(url: event_plans_url(@plan, event_name: @plan.event.name), model: @plan, method: 'POST') do |form| %>
+                      <%= form_with(url: event_plans_url(@plan, event_name: @plan.event.name), model: @plan, method: :post) do |form| %>
                         <%= form.hidden_field :title %>
                         <%= form.hidden_field :description %>
                         <%= form.hidden_field :public %>
@@ -134,9 +131,8 @@
                 </dialog>
               </div>
             <% else %>
-              <%= form_with(url: event_plan_url(@plan, event_name: @plan.event.name), method: 'PATCH' ) do |form| %>
-                <%= form.hidden_field :add_schedule_id, value: schedule.id %>
-                <%= form.hidden_field :mode, value: mode %>
+              <%= form_with(url: event_plan_items_url(plan_id: @plan.id, event_name: @plan.event.name), method: :post ) do |form| %>
+                <%= form.hidden_field :schedule_id, value: schedule.id %>
                 <%= form.submit inactive ? I18n.t('card.inactive') : I18n.t('card.add'), class: "add-plan-button p-2 text-sm font-bold min-h-5 normal-button", disabled: inactive, data: { turbo_frame: "event_#{@event.id}"} %>
               <% end %>
             <% end %>

config/routes.rb

@@ -24,9 +24,15 @@
       scope module: :plans do
         resource :ogp, only: %i[show]
       end
+
+      resources :items, only: %i[create]
     end
+
+    resources :items, only: %i[update destroy]
   end
 
+  resolve('PlanSchedule') { %i[event item] }
+
   resources :teams, except: :index do
     resources :members, only: %i[create update destroy]
   end