From 82cb8d4c5215d09fc64fbde6df53f33e461d9ee1 Mon Sep 17 00:00:00 2001
From: iagirre <itziar.agirre@m2c.es>
Date: Wed, 24 Jan 2018 16:23:12 +0100
Subject: [PATCH] Changes suggested in PR: * Delete all things related to
 banner images and styles (in code) * Add a new test to check that the banner
 is showing correctly * Update the specs accordingly to match the changes

Update dev_seed to set a random background_color and a font_color for banners (and remove everything about image and style)
Add a rake task to migrate the banner style to backgroun_color and font_color (so that the banners have the same colors than before)
---
 app/assets/javascripts/banners.js.coffee    | 12 +----
 app/controllers/admin/banners_controller.rb |  4 +-
 app/views/admin/banners/_form.html.erb      | 12 ++---
 app/views/admin/banners/index.html.erb      | 12 ++---
 app/views/shared/_banner.html.erb           | 12 ++---
 lib/tasks/banners.rake                      | 20 ++++++++
 spec/factories.rb                           |  4 +-
 spec/features/admin/banners_spec.rb         | 57 +++++++++++++--------
 8 files changed, 75 insertions(+), 58 deletions(-)
 create mode 100644 lib/tasks/banners.rake

diff --git a/app/assets/javascripts/banners.js.coffee b/app/assets/javascripts/banners.js.coffee
index e7748c148..d38587083 100644
--- a/app/assets/javascripts/banners.js.coffee
+++ b/app/assets/javascripts/banners.js.coffee
@@ -24,21 +24,13 @@ App.Banners =
       change: ->
         App.Banners.update_banner("#js-banner-description", $(this).val())
 
-    $("#banner_style").on
-      change: ->
-        App.Banners.update_style("#js-banner-style", $(this).val())
-
-    $("#banner_image").on
-      change: ->
-        App.Banners.update_style("#js-banner-image", $(this).val())
-
     $("#banner_background_color_picker").on
       change: ->
-        App.Banners.update_background_color("#js-banner-style", "#banner_background_color", $(this).val());
+        App.Banners.update_background_color("#js-banner-background", "#banner_background_color", $(this).val());
 
     $("#banner_background_color").on
       change: ->
-        App.Banners.update_background_color("#js-banner-style", "#banner_background_color_picker", $(this).val());
+        App.Banners.update_background_color("#js-banner-background", "#banner_background_color_picker", $(this).val());
 
     $("#banner_font_color_picker").on
       change: ->
diff --git a/app/controllers/admin/banners_controller.rb b/app/controllers/admin/banners_controller.rb
index 802110063..25038b80f 100644
--- a/app/controllers/admin/banners_controller.rb
+++ b/app/controllers/admin/banners_controller.rb
@@ -2,8 +2,6 @@ class Admin::BannersController < Admin::BaseController
 
   has_filters %w{all with_active with_inactive}, only: :index
 
-  before_action :banner_styles, only: [:edit, :new, :create, :update]
-  before_action :banner_imgs, only: [:edit, :new, :create, :update]
   before_action :banner_sections, only: [:edit, :new, :create, :update]
 
   respond_to :html, :js
@@ -39,7 +37,7 @@ class Admin::BannersController < Admin::BaseController
   private
 
     def banner_params
-      attributes = [:title, :description, :target_url, :style, :image,
+      attributes = [:title, :description, :target_url,
                     :post_started_at, :post_ended_at,
                     :background_color, :font_color,
                     web_section_ids: []]
diff --git a/app/views/admin/banners/_form.html.erb b/app/views/admin/banners/_form.html.erb
index dc0fb0634..d9ab0e609 100644
--- a/app/views/admin/banners/_form.html.erb
+++ b/app/views/admin/banners/_form.html.erb
@@ -77,12 +77,10 @@
       <%= f.submit(class: "button expanded", value: t("admin.banners.edit.form.submit_button")) %>
     </div>
   </div>
-  <div id="js-banner-style" class="<%= @banner.style %> banner" style="background-color:<%= @banner.background_color %>">
-    <div id="js-banner-image" class="<%= @banner.image %>">
-      <%= link_to @banner.target_url do %>
-        <h2 id="js-banner-title" style="color:<%= @banner.font_color %>"><%= @banner.title %></h2>
-        <h3 id="js-banner-description" style="color:<%= @banner.font_color %>"><%= @banner.description %></h3>
-      <% end %>
-    </div>
+  <div id="js-banner-background" class="banner" style="background-color:<%= @banner.background_color %>">
+    <%= link_to @banner.target_url do %>
+      <h2 id="js-banner-title" style="color:<%= @banner.font_color %>"><%= @banner.title %></h2>
+      <h3 id="js-banner-description" style="color:<%= @banner.font_color %>"><%= @banner.description %></h3>
+    <% end %>
   </div>
 <% end %>
diff --git a/app/views/admin/banners/index.html.erb b/app/views/admin/banners/index.html.erb
index c0e8186d2..bc1cec4e8 100644
--- a/app/views/admin/banners/index.html.erb
+++ b/app/views/admin/banners/index.html.erb
@@ -37,13 +37,11 @@
       </tr>
       <tr>
         <td colspan="3">
-          <div class="<%= banner.style %> banner" style="background-color:<%= banner.background_color %>">
-            <div class="<%= banner.image %>">
-              <%= link_to banner.target_url do %>
-                <h2 style="color:<%= banner.font_color %>"><%= banner.title %></h2>
-                <h3 style="color:<%= banner.font_color %>"><%= banner.description %></h3>
-              <% end %>
-            </div>
+          <div class="banner" style="background-color:<%= banner.background_color %>">
+            <%= link_to banner.target_url do %>
+              <h2 style="color:<%= banner.font_color %>"><%= banner.title %></h2>
+              <h3 style="color:<%= banner.font_color %>"><%= banner.description %></h3>
+            <% end %>
           </div>
         </td>
       </tr>
diff --git a/app/views/shared/_banner.html.erb b/app/views/shared/_banner.html.erb
index 0ba7a2bd1..eff05b46f 100644
--- a/app/views/shared/_banner.html.erb
+++ b/app/views/shared/_banner.html.erb
@@ -1,9 +1,7 @@
 <% banner = @banners.sample %>
-<div class="<%= banner.style %>">
-  <div class="<%= banner.image %>">
-    <%= link_to banner.target_url do %>
-      <h2><%=banner.title %></h2>
-      <h3><%=banner.description %></h3>
-    <% end %>
-  </div>
+<div class="banner" style="background-color:<%= banner.background_color %>">
+  <%= link_to banner.target_url do %>
+    <h2 style="color:<%= banner.font_color %>"><%=banner.title %></h2>
+    <h3 style="color:<%= banner.font_color %>"><%=banner.description %></h3>
+  <% end %>
 </div>
diff --git a/lib/tasks/banners.rake b/lib/tasks/banners.rake
new file mode 100644
index 000000000..eb2cc7d9c
--- /dev/null
+++ b/lib/tasks/banners.rake
@@ -0,0 +1,20 @@
+namespace :banners do
+
+  desc "Migrate styles to background_color and font_color"
+  task migrate_style: :environment do
+
+    Banner.all.each do |banner|
+      banner.font_color = '#FFFFFF'
+      case banner.style
+      when "banner-style banner-style-one"
+        banner.background_color = '#004a83'
+      when "banner-style banner-style-two"
+        banner.background_color = '#7e328a'
+      when "banner-style banner-style-three"
+        banner.background_color = '#33dadf'
+      end
+      banner.save
+    end
+  end
+
+end
diff --git a/spec/factories.rb b/spec/factories.rb
index dacde8385..2b192813d 100644
--- a/spec/factories.rb
+++ b/spec/factories.rb
@@ -738,11 +738,11 @@ FactoryBot.define do
   factory :banner do
     sequence(:title) { |n| "Banner title #{n}" }
     sequence(:description) { |n| "This is the text of Banner #{n}" }
-    style {["banner-style-one", "banner-style-two", "banner-style-three"].sample}
-    image {["banner.banner-img-one", "banner.banner-img-two", "banner.banner-img-three"].sample}
     target_url {["/proposals", "/debates" ].sample}
     post_started_at { Time.current - 7.days }
     post_ended_at { Time.current + 7.days }
+    background_color '#FF0000'
+    font_color '#FFFFFF'
   end
 
   factory :proposal_notification do
diff --git a/spec/features/admin/banners_spec.rb b/spec/features/admin/banners_spec.rb
index e58c82f20..668a42a28 100644
--- a/spec/features/admin/banners_spec.rb
+++ b/spec/features/admin/banners_spec.rb
@@ -11,42 +11,42 @@ feature 'Admin banners magement' do
       @banner1 = create(:banner, title: "Banner number one",
                   description:  "This is the text of banner number one and is not active yet",
                   target_url:  "http://www.url.com",
-                  style: "banner-style.banner-one",
-                  image: "banner-img.banner-one",
                   post_started_at: (Time.current + 4.days),
-                  post_ended_at:   (Time.current + 10.days))
+                  post_ended_at:   (Time.current + 10.days),
+                  background_color: '#FF0000',
+                  font_color: '#FFFFFF')
 
       @banner2 = create(:banner, title: "Banner number two",
                   description:  "This is the text of banner number two and is not longer active",
                   target_url:  "http://www.url.com",
-                  style: "banner-style.banner-two",
-                  image: "banner-img.banner-two",
                   post_started_at: (Time.current - 10.days),
-                  post_ended_at:   (Time.current - 3.days))
+                  post_ended_at:   (Time.current - 3.days),
+                  background_color: '#00FF00',
+                  font_color: '#FFFFFF')
 
       @banner3 = create(:banner, title: "Banner number three",
                   description:  "This is the text of banner number three and has style banner-three",
                   target_url:  "http://www.url.com",
-                  style: "banner-style.banner-three",
-                  image: "banner-img.banner-three",
                   post_started_at: (Time.current - 1.day),
-                  post_ended_at:   (Time.current + 10.days))
+                  post_ended_at:   (Time.current + 10.days),
+                  background_color: '#0000FF',
+                  font_color: '#FFFFFF')
 
       @banner4 = create(:banner, title: "Banner number four",
                   description:  "This is the text of banner number four and has style banner-one",
                   target_url:  "http://www.url.com",
-                  style: "banner-style.banner-one",
-                  image: "banner-img.banner-one",
                   post_started_at: (DateTime.current - 10.days),
-                  post_ended_at:   (DateTime.current + 10.days))
+                  post_ended_at:   (DateTime.current + 10.days),
+                  background_color: '#FFF000',
+                  font_color: '#FFFFFF')
 
       @banner5 = create(:banner, title: "Banner number five",
                   description:  "This is the text of banner number five and has style banner-two",
                   target_url:  "http://www.url.com",
-                  style: "banner-style.banner-one",
-                  image: "banner-img.banner-one",
                   post_started_at: (DateTime.current - 10.days),
-                  post_ended_at:   (DateTime.current + 10.days))
+                  post_ended_at:   (DateTime.current + 10.days),
+                  background_color: '#FFFF00',
+                  font_color: '#FFFFFF')
     end
 
     scenario 'Index show active banners' do
@@ -112,14 +112,27 @@ feature 'Admin banners magement' do
     expect(page.find_field("banner_font_color_picker").value).to eq('#ffb2b2')
   end
 
+  scenario "Created banner is correctly shown in the web" do
+    banner = create(:banner, title: 'My banner', description: 'My description of the banner', target_url: '/debates', background_color: '#00FF00')
+
+    visit proposals_path
+
+    expect(find('.banner')[:style]).to eq("background-color:#00FF00")
+
+    within('.banner') do
+      expect(find('h2')[:style]).to eq("color:#{banner.font_color}")
+      expect(find('h3')[:style]).to eq("color:#{banner.font_color}")
+    end
+  end
+
   scenario 'Edit banner with live refresh', :js do
     banner1 = create(:banner, title: 'Hello',
                               description: 'Wrong text',
                               target_url:  'http://www.url.com',
-                              style: 'banner-style.banner-one',
-                              image: 'banner-img.banner-one',
                               post_started_at: (Time.current + 4.days),
-                              post_ended_at:   (Time.current + 10.days))
+                              post_ended_at:   (Time.current + 10.days),
+                              background_color: '#FF0000',
+                              font_color: '#FFFFFF')
 
     visit admin_root_path
 
@@ -135,7 +148,7 @@ feature 'Admin banners magement' do
     select 'Banner style 1', from: 'banner_style'
     select 'Banner image 2', from: 'banner_image'
 
-    within('div#js-banner-style') do
+    within('div#js-banner-background') do
       expect(page).to have_selector('h2', text: 'Modified title')
       expect(page).to have_selector('h3', text: 'Edited text')
     end
@@ -154,10 +167,10 @@ feature 'Admin banners magement' do
     create(:banner, title: 'Ugly banner',
                     description: 'Bad text',
                     target_url:  'http://www.url.com',
-                    style: 'banner-style.banner-one',
-                    image: 'banner-img.banner-one',
                     post_started_at: (Time.current + 4.days),
-                    post_ended_at:   (Time.current + 10.days))
+                    post_ended_at:   (Time.current + 10.days),
+                    background_color: '#FF0000',
+                    font_color: '#FFFFFF')
 
     visit admin_root_path