From 0b83be68377c8aaced7c9d9fbae51de17fae5035 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Wed, 12 Aug 2020 14:45:45 +0200
Subject: [PATCH] Apply banner default colors to dev seeds

Banners created through the admin form were getting the default color.
However, banners created by other means (like the `db:dev_seed` rake
task) were not getting these default values.

This feature was originally implemented when we were using Rails 4.
With Rails 5, we can provide default values to all new banners and
simplify the code at the same time thanks to its `attribute` method.

Now, when creating a new banner, instead of getting a blank space, we
get an empty line with the banner's default background color, which most
users won't know what it's about until they fill in the banner's title.
So we're not displaying the content of the banner when it's empty,
thanks to the `:empty` CSS pseudoclass.
---
 app/assets/stylesheets/layout.scss     |  4 ++++
 app/helpers/banners_helper.rb          | 16 ----------------
 app/models/banner.rb                   |  3 +++
 app/views/admin/banners/_form.html.erb |  6 ++----
 spec/models/banner_spec.rb             |  7 +++++++
 5 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/app/assets/stylesheets/layout.scss b/app/assets/stylesheets/layout.scss
index 05e8189e0..dabfb2330 100644
--- a/app/assets/stylesheets/layout.scss
+++ b/app/assets/stylesheets/layout.scss
@@ -2292,6 +2292,10 @@ table {
   a > * {
     @include grid-row;
     padding: 0 rem-calc(16);
+
+    &:empty {
+      display: none;
+    }
   }
 
   + .budget.expanded,
diff --git a/app/helpers/banners_helper.rb b/app/helpers/banners_helper.rb
index c43bc7e13..42cdb240d 100644
--- a/app/helpers/banners_helper.rb
+++ b/app/helpers/banners_helper.rb
@@ -3,22 +3,6 @@ module BannersHelper
     @banners.present? && @banners.count > 0
   end
 
-  def banner_default_bg_color
-    "#e7f2fc"
-  end
-
-  def banner_default_font_color
-    "#222222"
-  end
-
-  def banner_bg_color_or_default
-    @banner.background_color.presence || banner_default_bg_color
-  end
-
-  def banner_font_color_or_default
-    @banner.font_color.presence || banner_default_font_color
-  end
-
   def banner_target_link(banner)
     link_to banner.target_url do
       tag.h2(banner.title, style: "color:#{banner.font_color}") +
diff --git a/app/models/banner.rb b/app/models/banner.rb
index f4b9b1d09..b5484e397 100644
--- a/app/models/banner.rb
+++ b/app/models/banner.rb
@@ -2,6 +2,9 @@ class Banner < ApplicationRecord
   acts_as_paranoid column: :hidden_at
   include ActsAsParanoidAliases
 
+  attribute :background_color, default: "#e7f2fc"
+  attribute :font_color, default: "#222222"
+
   translates :title,       touch: true
   translates :description, touch: true
   include Globalizable
diff --git a/app/views/admin/banners/_form.html.erb b/app/views/admin/banners/_form.html.erb
index bf8fc89b7..e6b607650 100644
--- a/app/views/admin/banners/_form.html.erb
+++ b/app/views/admin/banners/_form.html.erb
@@ -56,8 +56,7 @@
       <p class="help-text"><%= t("admin.shared.color_help") %></p>
       <div class="row collapse">
         <div class="small-12 medium-6 column">
-          <%= f.text_field :background_color, label: false, type: :color,
-                                              value: banner_bg_color_or_default %>
+          <%= f.text_field :background_color, label: false, type: :color %>
         </div>
         <div class="small-12 medium-6 column">
           <%= f.text_field :background_color, label: false, id: "background_color_input" %>
@@ -70,8 +69,7 @@
       <p class="help-text"><%= t("admin.shared.color_help") %></p>
       <div class="row collapse">
         <div class="small-12 medium-6 column">
-          <%= f.text_field :font_color, label: false, type: :color,
-                                        value: banner_font_color_or_default %>
+          <%= f.text_field :font_color, label: false, type: :color %>
         </div>
         <div class="small-12 medium-6 column">
           <%= f.text_field :font_color, label: false, id: "font_color_input" %>
diff --git a/spec/models/banner_spec.rb b/spec/models/banner_spec.rb
index 92e3ef407..75c05d918 100644
--- a/spec/models/banner_spec.rb
+++ b/spec/models/banner_spec.rb
@@ -11,4 +11,11 @@ describe Banner do
   it "is valid" do
     expect(banner).to be_valid
   end
+
+  it "assigns default values to new banners" do
+    banner = Banner.new
+
+    expect(banner.background_color).to be_present
+    expect(banner.font_color).to be_present
+  end
 end