Merge pull request #2435 from sarken/AO3-4547

AO3-4547 Allow banner updates WITHOUT turning banner back on
otwcode · May 28, 2016 · a0c94e3 · a0c94e3
2 parents 6924aad + 51b0079
commit a0c94e3
Show file tree

Hide file tree

Showing 4 changed files with 84 additions and 19 deletions.
diff --git a/app/controllers/admin/banners_controller.rb b/app/controllers/admin/banners_controller.rb
@@ -4,7 +4,7 @@ class Admin::BannersController < ApplicationController
 
   # GET /admin/banners
   def index   
-    @admin_banners = AdminBanner.order("id DESC").paginate(:page => params[:page])
+    @admin_banners = AdminBanner.order("id DESC").paginate(page: params[:page])
   end
 
   # GET /admin/banners/1
@@ -43,16 +43,19 @@ def create
   def update
     @admin_banner = AdminBanner.find(params[:id])
 
-    if @admin_banner.update_attributes(params[:admin_banner])
+    if !@admin_banner.update_attributes(params[:admin_banner])
+      render action: 'edit'
+    elsif params[:admin_banner_minor_edit]
+      flash[:notice] = ts('Updating banner for users who have not already dismissed it. This may take some time.')
+      redirect_to @admin_banner      
+    else
       if @admin_banner.active?
         AdminBanner.banner_on
         flash[:notice] = ts('Setting banner back on for all users. This may take some time.')
       else
         flash[:notice] = ts('Banner successfully updated.')
       end
       redirect_to @admin_banner
-    else
-      render action: 'edit'
     end
   end
 

diff --git a/app/views/admin/banners/_banner_form.html.erb b/app/views/admin/banners/_banner_form.html.erb
@@ -35,6 +35,11 @@
       <dt><%= f.check_box :active %></dt>
       <dd><%= f.label :active, ts('Active') %></dd>
 
+      <% if @admin_banner.active? %>
+        <dt><%= check_box_tag :admin_banner_minor_edit %></dt>
+        <dd><%= label_tag :admin_banner_minor_edit, ts('This is a minor update (Do not turn the banner back on for users who have dismissed it)') %></dd>
+      <% end %>
+
     </dl>
   </fieldset>
 

diff --git a/features/other_a/banner_general.feature b/features/other_a/banner_general.feature
@@ -8,7 +8,7 @@ Scenario: Banner is blank until admin sets it
 
 Scenario: Admin can set a banner
   Given there are no banners
-    And an admin creates an active banner
+  When an admin creates an active banner
   Then a logged-in user should see the banner
     And a logged-out user should see the banner
 
@@ -21,7 +21,7 @@ Scenario: Admin can set an alert banner
 
 Scenario: Admin can set an event banner
   Given there are no banners
-    And an admin creates an active "event" banner
+  When an admin creates an active "event" banner
   Then a logged-in user should see the "event" banner
     And a logged-out user should see the "event" banner
 
@@ -43,42 +43,42 @@ Scenario: User can turn off banner using "×" button
   Given there are no banners
     And an admin creates an active banner
   When I turn off the banner
-  Then I should not see "This is some banner text"
+  Then the page should not have a banner
 
 Scenario: Banner stays off when logging out and in again
   Given there are no banners
     And an admin creates an active banner
     And I turn off the banner
   When I am logged out
     And I am logged in as "newname"
-  Then I should not see "This is some banner text"
+  Then the page should not have a banner
 
 Scenario: Logged out user can turn off banner
   Given there are no banners
     And an admin creates an active banner
     And I am logged out
   When I follow "×"
-  Then I should not see "This is some banner text"
+  Then the page should not have a banner
 
 Scenario: User can turn off banner in preferences
   Given there are no banners
     And an admin creates an active banner
     And I am logged in as "banner_tester"
     And I set my preferences to turn off the banner showing on every page
   When I go to my user page
-  Then I should not see "This is some banner text"
+  Then the page should not have a banner
 
 Scenario: User can turn off banner in preferences, but will still see a banner when an admin deactivates the existing banner and sets a new banner
   Given there are no banners
     And an admin creates an active banner
     And I am logged in as "banner_tester_2"
   When I set my preferences to turn off the banner showing on every page
     And I go to my user page
-  Then I should not see "This is some banner text"
+  Then the page should not have a banner
   When an admin deactivates the banner
     And an admin creates a different active banner
   When I am logged in as "banner_tester_2"
-  Then I should see "This is new banner text"
+  Then the page should have the different banner
 
 Scenario: Admin can delete a banner and it will no longer be shown to users
   Given there are no banners
@@ -90,3 +90,33 @@ Scenario: Admin can delete a banner and it will no longer be shown to users
   Then I should see "Banner successfully deleted."
     And a logged-in user should not see a banner
     And a logged-out user should not see a banner
+
+Scenario: Admin should not have option to make minor updates on a new banner
+  Given there are no banners
+    And I am logged in as an admin
+  When I am on the new_admin_banner page
+  Then I should not see "This is a minor update (Do not turn the banner back on for users who have dismissed it)"
+
+Scenario: Admin should not have option to make minor updates on banner that is not active
+  Given there are no banners
+    And an admin creates a banner
+  When I am logged in as an admin
+    And I am on the admin_banners page
+    And I follow "Edit"
+  Then I should not see "This is a minor update (Do not turn the banner back on for users who have dismissed it)"
+
+Scenario: Admin can make minor changes to the text of an active banner without turning it back on for users who have already dismissed it
+  Given there are no banners
+    And an admin creates an active banner
+    And I am logged in as "banner_tester_3"
+    And I set my preferences to turn off the banner showing on every page
+    And an admin makes a minor edit to the active banner
+  When I am logged in as "banner_tester_3"
+  Then I should not see the banner with minor edits
+    And the page should not have a banner
+  When I am logged out
+  Then I should see the banner with minor edits
+  When I am logged in as "banner_tester_4"
+  Then I should see the banner with minor edits
+
+
diff --git a/features/step_definitions/banner_steps.rb b/features/step_definitions/banner_steps.rb
@@ -8,10 +8,10 @@
 
 ### WHEN
 
-When /^an admin creates an active(?: "([^\"]*)")? banner$/ do |banner_type|
+When /^an admin creates an?( active)?(?: "([^\"]*)")? banner$/ do |active, banner_type|
   step %{I am logged in as an admin}
   visit(new_admin_banner_path)
-  fill_in("admin_banner_content", :with => "This is some banner text")
+  fill_in("admin_banner_content", with: "This is some banner text")
   if banner_type.present?
     if banner_type == "alert"
       choose("admin_banner_banner_type_alert")
@@ -21,9 +21,9 @@
       choose("admin_banner_banner_type_")
     end
   end
-  check("admin_banner_active")
+  check("admin_banner_active") unless active.blank?
   click_button("Create Banner")
-  step %{I should see "Setting banner back on for all users. This may take some time."}
+  step %{I should see "Setting banner back on for all users. This may take some time."} unless active.blank?
 end
 
 When /^an admin deactivates the banner$/ do
@@ -39,15 +39,25 @@
   step %{I am logged in as an admin}
   visit(admin_banners_path)
   step %{I follow "Edit"}
-  fill_in("admin_banner_content", :with => "This is some edited banner text")
+  fill_in("admin_banner_content", with: "This is some edited banner text")
   click_button("Update Banner")
   step %{I should see "Setting banner back on for all users. This may take some time."}
 end
 
+When /^an admin makes a minor edit to the active banner$/ do
+  step %{I am logged in as an admin}
+  visit(admin_banners_path)
+  step %{I follow "Edit"}
+  fill_in("admin_banner_content", with: "This is some banner text!")
+  check("admin_banner_minor_edit")
+  click_button("Update Banner")
+  step %{I should see "Updating banner for users who have not already dismissed it. This may take some time."}
+end
+
 When /^an admin creates a different active banner$/ do
   step %{I am logged in as an admin}
   visit(new_admin_banner_path)
-  fill_in("admin_banner_content", :with => "This is new banner text")
+  fill_in("admin_banner_content", with: "This is new banner text")
   check("admin_banner_active")
   click_button("Create Banner")
   step %{I should see "Setting banner back on for all users. This may take some time."}
@@ -128,4 +138,21 @@
 Then /^I should see the first login popup$/ do
   step %{I should see "Here are some tips to help you get started."}
     step %{I should see "To log in, locate and fill in the log in link"}
-end
+end
+
+Then /^I should see the banner with minor edits$/ do
+  step %{I should see "This is some banner text!"}
+end
+
+Then /^I should not see the banner with minor edits$/ do
+  step %{I should not see "This is some banner text!"}
+end
+
+Then /^the page should have the different banner$/ do
+  step %{I should see "This is new banner text"}
+end
+
+Then /^the page should not have a banner$/ do
+  page.should_not have_xpath("//div[@class=\"announcement group\"]")
+end
+