Merge pull request #5473 from rubyforgood/5428-banner-dismissed-forev…

…er-bug

Banner Dismissed Forever Bug fix

app/controllers/application_controller.rb

@@ -9,6 +9,7 @@ class ApplicationController < ActionController::Base
   before_action :set_current_user
   before_action :set_timeout_duration
   before_action :set_current_organization
+  before_action :set_active_banner
   after_action :verify_authorized, except: :index, unless: :devise_controller?
   # after_action :verify_policy_scoped, only: :index
 
@@ -30,6 +31,13 @@ def after_sign_out_path_for(resource_or_scope)
     end
   end
 
+  def set_active_banner
+    return nil unless current_organization
+
+    @active_banner = current_organization.banners.active.first
+    @active_banner = nil if session[:dismissed_banner] == @active_banner&.id
+  end
+
   protected
 
   def handle_short_url(url_list)

app/controllers/banners_controller.rb

@@ -1,5 +1,6 @@
 class BannersController < ApplicationController
-  after_action :verify_authorized
+  after_action :verify_authorized, except: %i[dismiss]
+  before_action :set_banner, only: %i[edit update destroy dismiss]
 
   def index
     authorize :application, :admin_or_supervisor?
@@ -15,8 +16,11 @@ def new
 
   def edit
     authorize :application, :admin_or_supervisor?
+  end
 
-    @banner = current_organization.banners.find(params[:id])
+  def dismiss
+    session[:dismissed_banner] = @banner.id
+    render json: {status: :ok}
   end
 
   def create
@@ -37,8 +41,6 @@ def create
   def update
     authorize :application, :admin_or_supervisor?
 
-    @banner = current_organization.banners.find(params[:id])
-
     Banner.transaction do
       deactivate_alternate_active_banner
       @banner.update!(banner_params)
@@ -52,12 +54,16 @@ def update
   def destroy
     authorize :application, :admin_or_supervisor?
 
-    current_organization.banners.find(params[:id]).destroy
+    @banner.destroy
     redirect_to banners_path
   end
 
   private
 
+  def set_banner
+    @banner = current_organization.banners.find(params[:id])
+  end
+
   def banner_params
     params.require(:banner).permit(:active, :content, :name).merge(user: current_user)
   end

app/javascript/controllers/dismiss_controller.js

@@ -2,12 +2,19 @@ import { Controller } from '@hotwired/stimulus'
 
 export default class extends Controller {
   static targets = ['element']
+  static values = {
+    url: String
+  }
 
   dismiss (event) {
     event.preventDefault()
 
-    const { id } = event.params
-    document.cookie = `dismiss_${id}=true`
-    this.elementTarget.classList.add('d-none')
+    fetch(this.urlValue)
+      .then(response => response.json())
+      .then(data => {
+        if (data.status === 'ok') {
+          this.elementTarget.classList.add('d-none')
+        }
+      })
   }
 }

app/views/layouts/_banner.html.erb

@@ -1,14 +1,10 @@
-<% banner = current_organization.banners.active.first %>
-<% if banner %>
-  <% cookie_id = "banner_#{banner.id}" %>
-  <% if !cookies["dismiss_#{cookie_id}"] %>
-    <div class="bg-secondary-100 text-xl p-5 d-flex" data-controller="dismiss" data-dismiss-target="element">
-      <div class="">
-        <%= banner.content %>
-      </div>
-      <div class="ms-auto">
-        <a href="#" data-action="click->dismiss#dismiss" data-dismiss-id-param="<%= cookie_id %>">Dismiss</a>
-      </div>
+<% if @active_banner %>
+  <div class="bg-secondary-100 text-xl p-5 d-flex" data-controller="dismiss" data-dismiss-target="element" data-dismiss-url-value="<%= dismiss_banner_path(@active_banner) %>">
+    <div class="">
+      <%= @active_banner.content %>
     </div>
-  <% end %>
+    <div class="ms-auto">
+      <a href="#" data-action="click->dismiss#dismiss">Dismiss</a>
+    </div>
+  </div>
 <% end %>

config/routes.rb

@@ -116,7 +116,11 @@
   resources :learning_hour_topics, only: %i[new create edit update]
   resources :followup_reports, only: :index
   resources :placement_reports, only: :index
-  resources :banners, only: %i[index new edit create update destroy]
+  resources :banners, except: %i[show] do
+    member do
+      get :dismiss
+    end
+  end
   resources :bulk_court_dates, only: %i[new create]
   resources :case_groups, only: %i[index new edit create update destroy]
   resources :learning_hours, only: %i[index show new create edit update destroy]

spec/requests/banners_spec.rb

@@ -0,0 +1,50 @@
+require "rails_helper"
+
+RSpec.describe "Banners", type: :request do
+  let!(:casa_org) { create(:casa_org) }
+  let!(:active_banner) { create(:banner, casa_org: casa_org) }
+  let(:volunteer) { create(:volunteer, casa_org: casa_org) }
+
+  context "when user dismisses a banner" do
+    subject do
+      get dismiss_banner_path(active_banner)
+    end
+
+    it "sets session variable" do
+      sign_in volunteer
+      subject
+      expect(session[:dismissed_banner]).to eq active_banner.id
+    end
+
+    it "does not display banner on page reloads" do
+      sign_in volunteer
+      get casa_cases_path
+      expect(response.body).to include "Please fill out this survey"
+
+      subject
+      get casa_cases_path
+      expect(response.body).not_to include "Please fill out this survey"
+    end
+
+    context "when user logs out and back in" do
+      it "nils out session variable" do
+        sign_in volunteer
+        subject
+        get destroy_user_session_path
+        sign_in volunteer
+
+        expect(session[:dismissed_banner]).to be_nil
+      end
+
+      it "displays banner" do
+        sign_in volunteer
+        subject
+        get destroy_user_session_path
+        sign_in volunteer
+
+        get casa_cases_path
+        expect(response.body).to include "Please fill out this survey"
+      end
+    end
+  end
+end

spec/system/banners/dismiss_spec.rb

@@ -0,0 +1,19 @@
+require "rails_helper"
+
+RSpec.describe "banners/dismiss", type: :system, js: true do
+  let!(:casa_org) { create(:casa_org) }
+  let!(:active_banner) { create(:banner, casa_org: casa_org) }
+  let(:volunteer) { create(:volunteer, casa_org: casa_org) }
+
+  context "when user dismisses a banner" do
+    it "hides banner" do
+      sign_in volunteer
+
+      visit root_path
+      expect(page).to have_text("Please fill out this survey")
+
+      click_on "Dismiss"
+      expect(page).not_to have_text("Please fill out this survey")
+    end
+  end
+end