Refactor navigation item updates for Turbo and HTML

Author: rsmithlal

app/controllers/better_together/navigation_items_controller.rb

@@ -44,30 +44,13 @@ def create
       end
     end
 
-    def update # rubocop:todo Metrics/AbcSize, Metrics/MethodLength
+    def update
       authorize @navigation_item
 
-      respond_to do |format|
-        if @navigation_item.update(navigation_item_params)
-          flash[:notice] = t('navigation_item.updated')
-          format.html { redirect_to @navigation_area, notice: t('navigation_item.updated') }
-          format.turbo_stream do
-            redirect_to @navigation_area, only_path: true
-          end
-        else
-          flash.now[:alert] = t('navigation_item.update_failed')
-          format.html { render :edit, status: :unprocessable_entity }
-          format.turbo_stream do
-            render turbo_stream: [
-              turbo_stream.update('form_errors', partial: 'layouts/better_together/errors',
-                                                 locals: { object: @navigation_item }),
-              turbo_stream.update('navigation_item_form', partial: 'better_together/navigation_items/form',
-                                                          # rubocop:todo Layout/LineLength
-                                                          locals: { navigation_item: @navigation_item, navigation_area: @navigation_area })
-              # rubocop:enable Layout/LineLength
-            ]
-          end
-        end
+      if @navigation_item.update(navigation_item_params)
+        handle_update_success
+      else
+        handle_update_failure
       end
     end
 
@@ -80,6 +63,30 @@ def destroy
 
     private
 
+    def handle_update_success
+      flash[:notice] = t('navigation_item.updated')
+      respond_to do |format|
+        format.html { redirect_to @navigation_area, notice: flash[:notice] }
+        format.turbo_stream { redirect_to @navigation_area, only_path: true }
+      end
+    end
+
+    def handle_update_failure
+      flash.now[:alert] = t('navigation_item.update_failed')
+      respond_to do |format|
+        format.html { render :edit, status: :unprocessable_entity }
+        format.turbo_stream do
+          render turbo_stream: [
+            turbo_stream.update('form_errors', partial: 'layouts/better_together/errors',
+                                               locals: { object: @navigation_item }),
+            turbo_stream.update('navigation_item_form', partial: 'better_together/navigation_items/form',
+                                                        locals: { navigation_item: @navigation_item,
+                                                                  navigation_area: @navigation_area })
+          ]
+        end
+      end
+    end
+
     def available_parent_items
       BetterTogether::NavigationItem.where.not(id: @navigation_item.id).includes(
         :string_translations,

spec/requests/better_together/navigation_items_spec.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'NavigationItems', type: :request do
+  include BetterTogether::Engine.routes.url_helpers
+
+  let(:navigation_area) { create(:navigation_area) }
+  let(:navigation_item) { create(:navigation_item, navigation_area: navigation_area, title: 'Old Title') }
+
+  before do
+    allow_any_instance_of(BetterTogether::NavigationItemsController).to receive(:authorize)
+  end
+
+  describe 'PATCH /navigation_areas/:navigation_area_id/navigation_items/:id' do
+    context 'HTML format' do
+      it 'redirects on success' do
+        patch navigation_area_navigation_item_path(navigation_area, navigation_item, locale: I18n.default_locale),
+              params: { navigation_item: { title: 'New Title' } }
+
+        expect(response).to redirect_to(navigation_area_path(navigation_area, locale: I18n.default_locale))
+        expect(flash[:notice]).to be_present
+      end
+
+      it 'renders edit on failure' do
+        patch navigation_area_navigation_item_path(navigation_area, navigation_item, locale: I18n.default_locale),
+              params: { navigation_item: { title: '' } }
+
+        expect(response).to have_http_status(:unprocessable_entity)
+        expect(response.body).to include('navigation_item_form')
+      end
+    end
+
+    context 'Turbo Stream format' do
+      it 'redirects on success' do
+        patch navigation_area_navigation_item_path(navigation_area, navigation_item, locale: I18n.default_locale),
+              params: { navigation_item: { title: 'New Title' } }, as: :turbo_stream
+
+        expect(response).to redirect_to(navigation_area_path(navigation_area, locale: I18n.default_locale))
+      end
+
+      it 'renders turbo stream on failure' do
+        patch navigation_area_navigation_item_path(navigation_area, navigation_item, locale: I18n.default_locale),
+              params: { navigation_item: { title: '' } }, as: :turbo_stream
+
+        expect(response.media_type).to eq('text/vnd.turbo-stream.html')
+        expect(response.body).to include('form_errors')
+      end
+    end
+  end
+end