Merge pull request #1794 from manyfold3d/fix-model-editing

Fix model editing

app/controllers/models_controller.rb

@@ -40,8 +40,12 @@ def edit
   end
 
   def update
-    @model.update(model_params)
-    redirect_to [@model.library, @model]
+    if @model.update(model_params)
+      redirect_to [@model.library, @model]
+    else
+      edit # Load creators and collections
+      render :edit
+    end
   end
 
   def merge

app/helpers/application_helper.rb

@@ -56,10 +56,15 @@ def tag_class(state)
 
   def text_input_row(form, name)
     content_tag :div, class: "row mb-3 input-group" do
-      [
+      safe_join [
         form.label(name, class: "col-sm-2 col-form-label"),
-        form.text_field(name, class: "form-control col-auto")
-      ].join.html_safe
+        content_tag(:div, class: "col p-0") do
+          safe_join [
+            form.text_field(name, class: "form-control"),
+            errors_for(form.object, name)
+          ]
+        end
+      ]
     end
   end
 
@@ -85,4 +90,12 @@ def nav_link(ico, text, path, options = {})
       class: options[:style] || safe_join(["nav-link", (current_page?(path) ? "active" : "")], " ")
     )
   end
+
+  def errors_for(record, attribute)
+    return if record.nil? || attribute.nil?
+    return unless record.errors.include? attribute
+    content_tag(:div,
+      record.errors.full_messages_for(attribute).join("; "),
+      class: "invalid-feedback d-block")
+  end
 end

app/models/model.rb

@@ -19,6 +19,10 @@ class Model < ApplicationRecord
   before_validation :strip_separators_from_path, if: :path_changed?
   before_validation :slugify_name, if: :name_changed?
 
+  before_validation :normalize_license
+  # In Rails 7.1 we will be able to do this instead:
+  # normalizes :license, with: -> license { license.blank? ? nil : license }
+
   attr_reader :organize
   def organize=(value)
     @organize = ActiveRecord::Type::Boolean.new.cast(value)
@@ -105,6 +109,10 @@ def new?
 
   private
 
+  def normalize_license
+    self.license = nil if license.blank?
+  end
+
   def strip_separators_from_path
     self.path = path&.trim_path_separators
   end

app/views/models/_form.html.erb

@@ -34,7 +34,10 @@
 
   <div class="row mb-3 input-group">
     <%= form.label :license, class: "col-sm-2 col-form-label" %>
-    <%= form.select :license, license_select_options(selected: @model.license), {include_blank: true}, class: "form-control col-auto form-select" %>
+    <div class="col p-0">
+      <%= form.select :license, license_select_options(selected: @model.license), {include_blank: true}, class: "form-control form-select" %>
+      <%= errors_for @model, :license %>
+    </div>
   </div>
 
   <%= form.submit "Save", class: "btn btn-primary" %>

config/locales/en.yml

@@ -45,7 +45,7 @@ en:
     general:
       new: New Library
   licenses:
-    CC-BY-40: Creative Commons Attrbution
+    CC-BY-40: Creative Commons Attribution
     CC-BY-NC-40: Creative Commons Attribution NonCommercial
     CC-BY-NC-ND-40: Creative Commons Attribution NonCommercial NoDerivatives
     CC-BY-NC-SA-40: Creative Commons Attribution NonCommercial ShareAlike

spec/helpers/application_helper_spec.rb

@@ -51,7 +51,7 @@
       html = helper.text_input_row(form, :field)
       doc = Nokogiri::HTML(html)
       expect(doc.at("div.row")["class"]).to include("mb-3 input-group")
-      expect(doc.at("input")["class"]).to include("form-control col-auto")
+      expect(doc.at("input")["class"]).to include("form-control")
     end
   end
 

spec/models/model_spec.rb

@@ -48,6 +48,18 @@
       m = build(:model, license: "LicenseRef-Commercial")
       expect(m).to be_valid
     end
+
+    it "can remove a license on save" do
+      m = create(:model, license: "MIT")
+      m.license = nil
+      expect(m).to be_valid
+    end
+
+    it "normalizes blank licenses to nil" do
+      m = build(:model, license: "")
+      m.validate
+      expect(m.license).to be_nil
+    end
   end
 
   it "strips leading and trailing separators from paths" do