Merge pull request #5090 from rubyforgood/5070-update-banner-creation-ux

5070 - Update banner creation UX

app/controllers/banners_controller.rb

@@ -23,22 +23,30 @@ def create
     authorize :application, :admin_or_supervisor?
 
     @banner = current_organization.banners.build(banner_params)
-    if @banner.save
-      redirect_to banners_path
-    else
-      render :new
+
+    Banner.transaction do
+      deactivate_alternate_active_banner
+      @banner.save!
     end
+
+    redirect_to banners_path
+  rescue
+    render :new
   end
 
   def update
     authorize :application, :admin_or_supervisor?
 
     @banner = current_organization.banners.find(params[:id])
-    if @banner.update(banner_params)
-      redirect_to banners_path
-    else
-      render :edit
+
+    Banner.transaction do
+      deactivate_alternate_active_banner
+      @banner.update!(banner_params)
     end
+
+    redirect_to banners_path
+  rescue
+    render :edit
   end
 
   def destroy
@@ -53,4 +61,11 @@ def destroy
   def banner_params
     params.require(:banner).permit(:active, :content, :name).merge(user: current_user)
   end
+
+  def deactivate_alternate_active_banner
+    if banner_params[:active].to_i == 1
+      alternate_active_banner = current_organization.banners.where(active: true).where.not(id: @banner.id).first
+      alternate_active_banner&.update!(active: false)
+    end
+  end
 end

app/helpers/banner_helper.rb

@@ -0,0 +1,7 @@
+module BannerHelper
+  def conditionally_add_hidden_class(current_banner_is_active)
+    unless current_banner_is_active && current_organization.has_alternate_active_banner?(@banner.id)
+      "d-none"
+    end
+  end
+end

app/javascript/controllers/index.js

@@ -7,7 +7,9 @@ import { application } from './application'
 import DismissController from './dismiss_controller'
 import ExtendedNestedFormController from './extended_nested_form_controller'
 import HelloController from './hello_controller'
+import RevealController from 'stimulus-reveal-controller'
 
 application.register('dismiss', DismissController)
 application.register('extended-nested-form', ExtendedNestedFormController)
 application.register('hello', HelloController)
+application.register('reveal', RevealController)

app/models/casa_org.rb

@@ -87,6 +87,10 @@ def generate_contact_types_and_hearing_types
     end
   end
 
+  def has_alternate_active_banner?(current_banner_id)
+    banners.where(active: true).where.not(id: current_banner_id).exists?
+  end
+
   private
 
   def sanitize_svg

app/views/banners/_form.html.erb

@@ -21,9 +21,10 @@
       <%= form.label :name %>
       <%= form.text_field :name, required: true %>
     </div>
-    <div class="form-check checkbox-style mb-20">
-      <%= form.check_box :active, class: 'form-check-input' %>
+    <div data-controller="reveal" data-reveal-hidden-class="d-none" class="form-check checkbox-style mb-20">
+      <%= form.check_box :active, data: { action: current_organization.has_alternate_active_banner?(@banner.id) && 'click->reveal#toggle' }, class: 'form-check-input' %>
       <%= form.label :active, "Active?", class: 'form-check-label' %>
+      <span data-reveal-target="item" class="text-danger <%= conditionally_add_hidden_class(form.object.active) %>">Warning: This will replace your current active banner</span>
     </div>
     <div class="input-style-1">
       <%= form.label :content %>

app/views/banners/edit.html.erb

@@ -1 +1 @@
-<%= render 'banners/form', banner: @banner %>
+<%= render 'banners/form', banner: @banner, org_has_alternate_active_banner: @org_has_alternate_active_banner %>

app/views/banners/new.html.erb

@@ -1 +1 @@
-<%= render 'banners/form', banner: @banner %>
+<%= render 'banners/form', banner: @banner, org_has_alternate_active_banner: @org_has_alternate_active_banner %>

package.json

@@ -41,6 +41,7 @@
     "select2": "^4.0.13",
     "select2-bootstrap-5-theme": "^1.3.0",
     "stimulus-rails-nested-form": "^4.1.0",
+    "stimulus-reveal-controller": "^4.1.0",
     "strftime": "^0.10.2",
     "sweetalert2": "^11.3.5",
     "trix": "^2.0.5",

spec/helpers/banner_helper_spec.rb

@@ -0,0 +1,38 @@
+require "rails_helper"
+
+RSpec.describe BannerHelper do
+  describe "#conditionally_add_hidden_class" do
+    it "returns d-none if current banner is inactive" do
+      current_organization = double
+      allow(helper).to receive(:current_organization).and_return(current_organization)
+      banner = double(id: 1)
+      assign(:banner, banner)
+
+      allow(current_organization).to receive(:has_alternate_active_banner?).and_return(true)
+
+      expect(helper.conditionally_add_hidden_class(false)).to eq("d-none")
+    end
+
+    it "returns d-none if current banner is active and org does not have an alternate active banner" do
+      current_organization = double
+      allow(helper).to receive(:current_organization).and_return(current_organization)
+      banner = double(id: 1)
+      assign(:banner, banner)
+
+      allow(current_organization).to receive(:has_alternate_active_banner?).and_return(false)
+
+      expect(helper.conditionally_add_hidden_class(true)).to eq("d-none")
+    end
+
+    it "returns nil if current banner is active and org has an alternate active banner" do
+      current_organization = double
+      allow(helper).to receive(:current_organization).and_return(current_organization)
+      banner = double(id: 1)
+      assign(:banner, banner)
+
+      allow(current_organization).to receive(:has_alternate_active_banner?).and_return(true)
+
+      expect(helper.conditionally_add_hidden_class(true)).to eq(nil)
+    end
+  end
+end

spec/system/banners/new_spec.rb

@@ -0,0 +1,89 @@
+require "rails_helper"
+
+RSpec.describe "Banners", type: :system, js: true do
+  let(:admin) { create(:casa_admin) }
+  let(:organization) { admin.casa_org }
+
+  it "adds a banner" do
+    sign_in admin
+
+    visit banners_path
+    click_on "New Banner"
+    fill_in "Name", with: "Volunteer Survey Announcement"
+    check "Active?"
+    fill_in_rich_text_area "banner_content", with: "Please fill out this survey."
+    click_on "Submit"
+
+    visit banners_path
+    expect(page).to have_text("Volunteer Survey Announcement")
+
+    visit banners_path
+    within "#banners" do
+      click_on "Edit", match: :first
+    end
+    fill_in "Name", with: "Better Volunteer Survey Announcement"
+    click_on "Submit"
+
+    visit banners_path
+    expect(page).to have_text("Better Volunteer Survey Announcement")
+
+    visit root_path
+    expect(page).to have_text("Please fill out this survey.")
+  end
+
+  describe "when an organization has an active banner" do
+    let(:admin) { create(:casa_admin) }
+    let(:organization) { create(:casa_org) }
+    let(:active_banner) { create(:banner, casa_org: organization) }
+
+    context "when a banner is submitted as active" do
+      it "deactivates and replaces the current active banner" do
+        active_banner
+
+        sign_in admin
+
+        visit banners_path
+        expect(page).to have_text(active_banner.content.body.to_plain_text)
+        click_on "New Banner"
+        fill_in "Name", with: "New active banner name"
+        check "Active?"
+        fill_in_rich_text_area "banner_content", with: "New active banner content."
+        click_on "Submit"
+
+        visit banners_path
+        within("table#banners") do
+          already_existing_banner_row = find("tr", text: active_banner.name)
+
+          expect(already_existing_banner_row).to have_selector("td.min-width", text: "No")
+        end
+
+        expect(page).to have_text("New active banner content.")
+      end
+    end
+
+    context "when a banner is submitted as inactive" do
+      it "does not deactivate the current active banner" do
+        active_banner
+
+        sign_in admin
+
+        visit banners_path
+        expect(page).to have_text(active_banner.content.body.to_plain_text)
+        click_on "New Banner"
+        fill_in "Name", with: "New active banner name"
+        fill_in_rich_text_area "banner_content", with: "New active banner content."
+        click_on "Submit"
+
+        visit banners_path
+
+        within("table#banners") do
+          already_existing_banner_row = find("tr", text: active_banner.name)
+
+          expect(already_existing_banner_row).to have_selector("td.min-width", text: "Yes")
+        end
+
+        expect(page).to have_text(active_banner.content.body.to_plain_text)
+      end
+    end
+  end
+end

spec/views/banners/new.html.erb_spec.rb

@@ -0,0 +1,66 @@
+require "rails_helper"
+
+RSpec.describe "banners/new", type: :view do
+  context "when new banner is marked as inactive" do
+    it "does not warn that current active banner will be deactivated" do
+      user = build_stubbed(:casa_admin)
+      current_organization = user.casa_org
+      current_organization_banner = build(:banner, active: true, casa_org: current_organization)
+
+      allow(view).to receive(:current_user).and_return(user)
+      allow(view).to receive(:current_organization).and_return(current_organization)
+      allow(current_organization).to receive(:has_alternate_active_banner?).and_return(true)
+
+      assign :banners, [current_organization_banner]
+      assign :banner, Banner.new(active: false)
+
+      render template: "banners/new"
+
+      expect(rendered).not_to have_checked_field("banner_active")
+      expect(rendered).to have_css("span.d-none", text: "Warning: This will replace your current active banner")
+    end
+  end
+
+  context "when organization has an active banner" do
+    context "when new banner is marked as active" do
+      it "warns that current active banner will be deactivated" do
+        user = build_stubbed(:casa_admin)
+        current_organization = user.casa_org
+        current_organization_banner = build(:banner, active: true, casa_org: current_organization)
+
+        allow(view).to receive(:current_user).and_return(user)
+        allow(view).to receive(:current_organization).and_return(current_organization)
+        allow(current_organization).to receive(:has_alternate_active_banner?).and_return(true)
+
+        assign :banners, [current_organization_banner]
+        assign :banner, Banner.new(active: true)
+
+        render template: "banners/new"
+
+        expect(rendered).to have_checked_field("banner_active")
+        expect(rendered).not_to have_css("span.d-none", text: "Warning: This will replace your current active banner")
+      end
+    end
+  end
+
+  context "when organization has no active banner" do
+    context "when new banner is marked as active" do
+      it "does not warn that current active banner will be deactivated" do
+        user = build_stubbed(:casa_admin)
+        current_organization = user.casa_org
+
+        allow(view).to receive(:current_user).and_return(user)
+        allow(view).to receive(:current_organization).and_return(current_organization)
+        allow(current_organization).to receive(:has_alternate_active_banner?).and_return(false)
+
+        assign :banners, []
+        assign :banner, Banner.new(active: true)
+
+        render template: "banners/new"
+
+        expect(rendered).to have_checked_field("banner_active")
+        expect(rendered).to have_css("span.d-none", text: "Warning: This will replace your current active banner")
+      end
+    end
+  end
+end

yarn.lock

@@ -5137,6 +5137,11 @@ stimulus-rails-nested-form@^4.1.0:
   resolved "https://registry.yarnpkg.com/stimulus-rails-nested-form/-/stimulus-rails-nested-form-4.1.0.tgz#bfce185cff908170a4eb9973875b72517c3bc83a"
   integrity sha512-ORqcTsg3sa4PGFEyUkbvcPG56F4K2fx1qJCUQIgngO1GaW5taKcvDkT0HvdTqtQAFe/1lN4CpJAqoSCt+nYF/Q==
 
+stimulus-reveal-controller@^4.1.0:
+  version "4.1.0"
+  resolved "https://registry.yarnpkg.com/stimulus-reveal-controller/-/stimulus-reveal-controller-4.1.0.tgz#bf0fb4c2706f22d41544b5b02e2fbd794f608575"
+  integrity sha512-cPpTLV/+IQgiE+J3iBMjf3kD3H9ZOeoRJjyhvcsjyPE82mdcsuWxlzpI1pwSJPN66qSud4hVkhNH5w4xadyOfA==
+
 stream-combiner@~0.0.4:
   version "0.0.4"
   resolved "https://registry.npmjs.org/stream-combiner/-/stream-combiner-0.0.4.tgz"