Add "name" field to Display Ads (#18396)

* feat: create a migration to add the display ad name column to the tanle

* feat: add a display ad name to the interface

* feat: add a display ad name after create

* feat: whoops the migration file

* feat: add a DUS to update previous Display Ads names

* fix: test

* refactor: update the name after_save

* fix: add name to params

app/controllers/admin/display_ads_controller.rb

@@ -60,7 +60,7 @@ module Admin
     private
 
     def display_ad_params
-      params.permit(:organization_id, :body_markdown, :placement_area, :published, :approved)
+      params.permit(:organization_id, :body_markdown, :placement_area, :published, :approved, :name)
     end
 
     def authorize_admin

app/models/display_ad.rb

@@ -17,6 +17,7 @@ class DisplayAd < ApplicationRecord
                              inclusion: { in: ALLOWED_PLACEMENT_AREAS }
   validates :body_markdown, presence: true
   before_save :process_markdown
+  after_save :generate_display_ad_name
 
   scope :approved_and_published, -> { where(approved: true, published: true) }
 
@@ -36,6 +37,13 @@ class DisplayAd < ApplicationRecord
 
   private
 
+  def generate_display_ad_name
+    return unless name.nil?
+
+    self.name = "Display Ad #{id}"
+    save!
+  end
+
   def process_markdown
     renderer = Redcarpet::Render::HTMLRouge.new(hard_wrap: true, filter_html: false)
     markdown = Redcarpet::Markdown.new(renderer, Constants::Redcarpet::CONFIG)

app/views/admin/display_ads/_form.html.erb

@@ -1,5 +1,10 @@
 <div class="grid l:grid-cols-2 gap-6 mb-4">
   <div class="flex flex-col gap-4">
+    <div class="crayons-field">
+      <%= label_tag :name, "Name:", class: "crayons-field__label" %>
+      <%= text_field_tag :name, @display_ad.name, class: "crayons-textfield" %>
+    </div>
+
     <div class="crayons-field">
       <%= label_tag :organization_id, "Organization ID:", class: "crayons-field__label" %>
       <%= text_field_tag :organization_id, @display_ad.organization_id, class: "crayons-textfield", placeholder: "1234" %>

app/views/admin/display_ads/index.html.erb

@@ -20,7 +20,7 @@
   <table class="crayons-table" width="100%">
     <thead>
       <tr>
-        <th scope="col">ID</th>
+        <th scope="col">Name</th>
         <th scope="col">Placement Area</th>
         <th scope="col">Published</th>
         <th scope="col">Approved</th>
@@ -30,7 +30,7 @@
     <tbody class="crayons-card">
       <% @display_ads.each do |display_ad| %>
           <tr data-row-id="<%= display_ad.id %>">
-            <td><%= link_to display_ad.id, edit_admin_display_ad_path(display_ad) %></td>
+            <td><%= link_to display_ad.name, edit_admin_display_ad_path(display_ad) %></td>
             <td><%= display_ad.human_readable_placement_area %></td>
             <td><%= display_ad.published %></td>
             <td><%= display_ad.approved %></td>

db/migrate/20220830141143_add_name_to_display_ad.rb

@@ -0,0 +1,5 @@
+class AddNameToDisplayAd < ActiveRecord::Migration[7.0]
+  def change
+    add_column :display_ads, :name, :string
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2022_08_04_135751) do
+ActiveRecord::Schema[7.0].define(version: 2022_08_30_141143) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
   enable_extension "pg_stat_statements"
@@ -460,6 +460,7 @@ ActiveRecord::Schema[7.0].define(version: 2022_08_04_135751) do
     t.integer "clicks_count", default: 0
     t.datetime "created_at", precision: nil, null: false
     t.integer "impressions_count", default: 0
+    t.string "name"
     t.bigint "organization_id"
     t.string "placement_area"
     t.text "processed_html"

lib/data_update_scripts/20220830153942_generate_display_ad_names.rb

@@ -0,0 +1,9 @@
+module DataUpdateScripts
+  class GenerateDisplayAdNames
+    def run
+      DisplayAd.where(name: nil).find_each do |display_ad|
+        display_ad.update(name: "Display Ad #{display_ad.id}")
+      end
+    end
+  end
+end

spec/lib/data_update_scripts/generate_display_ad_names_spec.rb

@@ -0,0 +1,25 @@
+require "rails_helper"
+require Rails.root.join(
+  "lib/data_update_scripts/20220830153942_generate_display_ad_names.rb",
+)
+
+describe DataUpdateScripts::GenerateDisplayAdNames do
+  context "when there is no name for a Display Ad" do
+    it "generates a name for an existing Display Ad" do
+      display_ad = create(:display_ad, name: nil)
+
+      described_class.new.run
+      expect(display_ad.reload.name).to eq("Display Ad #{display_ad.id}")
+    end
+  end
+
+  context "when there is a name for the Display Ad" do
+    it "does not change the name" do
+      display_ad = create(:display_ad, name: "Test")
+
+      expect do
+        described_class.new.run
+      end.not_to change { display_ad.reload.name }
+    end
+  end
+end

spec/models/display_ad_spec.rb

@@ -52,6 +52,16 @@ RSpec.describe DisplayAd, type: :model do
     end
   end
 
+  describe "after_create callbacks" do
+    it "generates a name when one does not exist" do
+      display_ad = create(:display_ad, name: nil)
+      display_ad_with_name = create(:display_ad, name: "Test")
+
+      expect(display_ad.name).to eq("Display Ad #{display_ad.id}")
+      expect(display_ad_with_name.name).to eq("Test")
+    end
+  end
+
   describe ".for_display" do
     let!(:display_ad) { create(:display_ad, organization_id: organization.id) }