From c1c84507b8b9026dc16b199c74dafec1a0e935b2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 6 Jan 2021 14:12:38 +0100
Subject: [PATCH] Make card title mandatory

We didn't add any validation rules to the card model. At the very least,
the title should be mandatory.

The fact that the label field is marked as optional in the form but the
other fields are not probably means description and link should be
mandatory as well. However, since there might be institutions using
cards with descriptions but no link or cards with links but no
description, so we're keeping these fields optional for compatibility
reasons. We might change our minds in the future, though.
---
 app/models/widget/card.rb                    |  2 ++
 app/views/admin/widget/cards/_form.html.erb  |  6 +++++-
 spec/models/widget/card_spec.rb              |  4 ++++
 spec/support/common_actions/notifications.rb |  2 +-
 spec/system/admin/widgets/cards_spec.rb      | 18 ++++++++++++++++++
 5 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/app/models/widget/card.rb b/app/models/widget/card.rb
index 64a840231..4b67f8896 100644
--- a/app/models/widget/card.rb
+++ b/app/models/widget/card.rb
@@ -14,6 +14,8 @@ class Widget::Card < ApplicationRecord
   translates :link_text,   touch: true
   include Globalizable
 
+  validates_translation :title, presence: true
+
   def self.header
     where(header: true)
   end
diff --git a/app/views/admin/widget/cards/_form.html.erb b/app/views/admin/widget/cards/_form.html.erb
index ab0762859..e0afe9e47 100644
--- a/app/views/admin/widget/cards/_form.html.erb
+++ b/app/views/admin/widget/cards/_form.html.erb
@@ -1,6 +1,7 @@
 <%= render "shared/globalize_locales", resource: @card %>
 
 <%= translatable_form_for [:admin, @card] do |f| %>
+  <%= render "shared/errors", resource: @card %>
 
   <div class="row">
     <%= f.translatable_fields do |translations_form| %>
@@ -49,7 +50,10 @@
       </div>
     </div>
     <div class="column">
-      <%= f.submit(t("admin.homepage.#{action_name}.#{@card.header? ? "submit_header" : "submit_card"}"), class: "button success") %>
+      <%= f.submit(
+        t("admin.homepage.#{admin_submit_action(@card)}.#{@card.header? ? "submit_header" : "submit_card"}"),
+        class: "button success"
+      ) %>
     </div>
   </div>
 <% end %>
diff --git a/spec/models/widget/card_spec.rb b/spec/models/widget/card_spec.rb
index 8f925fefd..e7b01ddfe 100644
--- a/spec/models/widget/card_spec.rb
+++ b/spec/models/widget/card_spec.rb
@@ -9,6 +9,10 @@ describe Widget::Card do
     it "is valid" do
       expect(card).to be_valid
     end
+
+    it "is not valid without a title" do
+      expect(build(:widget_card, title: "")).not_to be_valid
+    end
   end
 
   describe "#header" do
diff --git a/spec/support/common_actions/notifications.rb b/spec/support/common_actions/notifications.rb
index 4db1cc833..572a4d3c8 100644
--- a/spec/support/common_actions/notifications.rb
+++ b/spec/support/common_actions/notifications.rb
@@ -56,7 +56,7 @@ module Notifications
   def error_message(resource_model = nil)
     resource_model ||= "(.*)"
     field_check_message = "Please check the marked fields to know how to correct them:"
-    /\d errors? prevented this #{resource_model} from being saved. #{field_check_message}/
+    /\d errors? prevented this #{resource_model} from being saved.(\n| )#{field_check_message}/
   end
 
   def fill_in_admin_notification_form(options = {})
diff --git a/spec/system/admin/widgets/cards_spec.rb b/spec/system/admin/widgets/cards_spec.rb
index 969a9b0ca..a0315118a 100644
--- a/spec/system/admin/widgets/cards_spec.rb
+++ b/spec/system/admin/widgets/cards_spec.rb
@@ -27,6 +27,15 @@ describe "Cards", :admin do
     end
   end
 
+  scenario "Create with errors", :js do
+    visit admin_homepage_path
+    click_link "Create card"
+    click_button "Create card"
+
+    expect(page).to have_text error_message
+    expect(page).to have_button "Create card"
+  end
+
   scenario "Index" do
     3.times { create(:widget_card) }
 
@@ -130,6 +139,15 @@ describe "Cards", :admin do
       end
     end
 
+    scenario "Create with errors", :js do
+      visit admin_homepage_path
+      click_link "Create header"
+      click_button "Create header"
+
+      expect(page).to have_text error_message
+      expect(page).to have_button "Create header"
+    end
+
     context "Page card" do
       let!(:custom_page) { create(:site_customization_page, :published) }