From f0dab7a5f9eaf798bf2635feb437d8f57d233398 Mon Sep 17 00:00:00 2001
From: cryptogopher <cryptogopher@noreply.me>
Date: Tue, 17 Dec 2024 01:50:57 +0100
Subject: [PATCH] Fix Units new/edit display on validation errors

Add test_new_and_edit_on_validation_error
Closes #41
---
 app/javascript/application.js        |  4 ++-
 app/views/units/new.turbo_stream.erb |  2 +-
 test/system/units_test.rb            | 37 +++++++++++++++++++++++++---
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/app/javascript/application.js b/app/javascript/application.js
index f7e595f..264c2c0 100644
--- a/app/javascript/application.js
+++ b/app/javascript/application.js
@@ -21,7 +21,7 @@ Turbo.StreamElement.prototype.removePreviousForm = function(form) {
   form.remove()
   if (row) {
     row.id = id
-    row.style.display = "revert"
+    row.removeAttribute("style")
   }
   if (form.hasAttribute("data-link-id")) {
     const link = document.getElementById(form.getAttribute("data-link-id"))
@@ -71,9 +71,11 @@ Turbo.StreamActions.after_form = function() {
 
 Turbo.StreamActions.replace_form = function() {
   this.targetElements.forEach((e) => {
+    const id = e.id;
     [...e.parentElement?.getElementsByClassName("form")].forEach((f) => {
       this.removePreviousForm(f)
     })
+    e = document.getElementById(id)
     e.style.display = "none"
     e.id = e.id + "_cached"
     e.parentElement?.insertBefore(this.templateContent, e.nextSibling)
diff --git a/app/views/units/new.turbo_stream.erb b/app/views/units/new.turbo_stream.erb
index a11eb9e..8cb7080 100644
--- a/app/views/units/new.turbo_stream.erb
+++ b/app/views/units/new.turbo_stream.erb
@@ -1,5 +1,4 @@
 <% link_id = dom_id(@unit.base || @unit, :add) %>
-<%= turbo_stream.disable link_id -%>
 
 <%= turbo_stream.replace :unit_form do %>
   <%= form_with model: @unit, html: {id: :unit_form} do %>
@@ -7,3 +6,4 @@
 <% end %>
 
 <%= turbo_stream.insert_form (@unit.base || :units), partial: 'form', locals: {link_id: link_id} %>
+<%= turbo_stream.disable link_id -%>
diff --git a/test/system/units_test.rb b/test/system/units_test.rb
index bf7ccef..ab30709 100644
--- a/test/system/units_test.rb
+++ b/test/system/units_test.rb
@@ -1,5 +1,10 @@
 require "application_system_test_case"
 
+# Fixture prerequisites:
+#  * user with multiple units + subunits
+#  * user with single unit
+#  * user with no units
+
 class UnitsTest < ApplicationSystemTestCase
   LINK_LABELS = {
     add_unit: t('units.index.add_unit'),
@@ -56,18 +61,44 @@ class UnitsTest < ApplicationSystemTestCase
       end
     end
 
+    assert_selector '.flash.notice', text: t('units.create.success', unit: Unit.last.symbol)
     within 'tbody' do
       assert_no_selector :fillable_field
       assert_selector 'tr', count: @user.units.count
     end
     assert_no_selector :element, :a, 'disabled': 'disabled',
       exact_text: Regexp.union(LINK_LABELS.fetch_values(:add_unit, :add_subunit))
-    assert_selector '.flash.notice', text: t('units.create.success', unit: Unit.last.symbol)
     assert_equal values, Unit.last.attributes.slice(*values.keys)
   end
 
-  # TODO: check proper form/button redisplay and flash messages on add/edit
-  test "new and edit form on validation error" do
+  test "new and edit on validation error" do
+    # It's not possible to cause validation error on :edit with single unit
+    LINK_LABELS.delete(:edit) unless @user.units.count > 1
+    type, label = LINK_LABELS.assoc(LINK_LABELS.keys.sample)
+    link = all(:link, exact_text: label).sample
+    link.click
+
+    get_values = -> { all(:field).map { |f| [f[:name], f[:value]] }.to_h }
+    values = nil
+    within 'tbody > tr:has(input[type=text])' do
+      # Provide duplicate :symbol as input invalidatable server side
+      fill_in 'unit[symbol]',
+        with: (@user.units.map(&:symbol) - [find_field('unit[symbol]').value]).sample
+      values = get_values[]
+      send_keys :enter
+    end
+
+    # Wait for flash before checking link :disabled status
+    assert_selector '.flash.alert'
+    if type == :edit
+      assert_no_selector :link, exact_text: link[:text]
+    else
+      assert_equal 'disabled', link[:disabled]
+    end
+
+    within 'tbody > tr:has(input[type=text])' do
+      assert_equal values, get_values[]
+    end
   end
 
   # TODO: add non-empty form closing warning