Add inline images to Delegate Reports (thewca#10086)

* Add image support for DR directly

* Add verification for image count

* Image edit styling

* Localization

* Only verify image count upon posting the report

* (Re-)introduce venue section for DR2024 template

* Display venue pictures below 'venue' template block

* Make sure images are only required when there is a 'venue' section

* Only run image count validation when images are required

* Add DR2024 venue section migration script

* Fix Venue template equipment section line break

* Add dummy images in DR test

* Test case fixes (1)

* Test case fixes (2)

* Clarify image setup description

app/controllers/delegate_reports_controller.rb

@@ -66,6 +66,15 @@ def update
     end
   end
 
+  def delete_image
+    image = ActiveStorage::Attachment.find(params[:image_id])
+    image.purge
+
+    flash[:success] = "Image deleted successfully."
+
+    redirect_to delegate_report_edit_path(competition_from_params)
+  end
+
   private def delegate_report_params
     params.require(:delegate_report).permit(
       :discussion_url,
@@ -77,6 +86,7 @@ def update
       :wic_feedback_requested,
       :wic_incidents,
       *DelegateReport::AVAILABLE_SECTIONS,
+      setup_images: [],
     )
   end
 

app/models/delegate_report.rb

@@ -19,6 +19,10 @@ class DelegateReport < ApplicationRecord
 
   enum :version, [:legacy, :working_group_2024], suffix: true, default: :working_group_2024
 
+  has_many_attached :setup_images do |attachable|
+    attachable.variant :preview, resize_to_limit: [100, 100]
+  end
+
   attr_accessor :current_user
 
   before_create :set_discussion_url
@@ -50,6 +54,13 @@ def md_section_defaults!
   validates :wrc_incidents, presence: true, if: :wrc_feedback_requested
   validates :wic_incidents, presence: true, if: :wic_feedback_requested
 
+  validate :setup_image_count, if: [:posted?, :requires_setup_images?]
+  private def setup_image_count
+    if self.setup_images.count < self.required_setup_images_count
+      errors.add(:setup_images, "Needs at least #{self.required_setup_images_count} images")
+    end
+  end
+
   def schedule_and_discussion_urls_required?
     posted? && created_at > Date.new(2019, 7, 21)
   end
@@ -62,7 +73,7 @@ def uses_section?(section)
     case section
     when :summary
       self.working_group_2024_version?
-    when :venue
+    when :equipment
       self.legacy_version?
     else
       true
@@ -73,6 +84,14 @@ def md_sections
     AVAILABLE_SECTIONS.filter { |section| self.uses_section?(section) }
   end
 
+  def requires_setup_images?
+    self.uses_section?(:venue) && self.required_setup_images_count > 0
+  end
+
+  def required_setup_images_count
+    self.working_group_2024_version? ? 2 : 0
+  end
+
   def can_see_submit_button?(current_user)
     !posted? && competition.staff_delegates.include?(current_user)
   end

app/views/delegate_reports/_delegate_report.html.erb

@@ -77,6 +77,20 @@
       <h2><%= section.capitalize %></h2>
       <%=md delegate_report.read_attribute(section) %>
     </div>
+
+    <% if section == :venue && delegate_report.setup_images.attached? %>
+      <div>
+        <h2>Setup Images</h2>
+        <p>(Click to get a larger view)</p>
+        <div style="display: flex; flex-wrap: wrap; justify-content: space-around; align-items: center">
+          <% delegate_report.setup_images.each do |image| %>
+            <%= link_to url_for(image), target: :_blank do %>
+              <%= image_tag image.variant(:preview).processed %>
+            <% end %>
+          <% end %>
+        </div>
+      </div>
+    <% end %>
   <% end %>
 
   <%# Technically, 'remarks' is also a Markdown section but it's the only one that doesn't have a template %>

app/views/delegate_reports/edit.html.erb

@@ -17,7 +17,30 @@
 
     <% @delegate_report.md_sections.each do |section| %>
       <%= f.input section, input_html: { class: "markdown-editor markdown-editor-image-upload" } %>
+
+      <% if section == :venue && @delegate_report.requires_setup_images? %>
+        <% @delegate_report.setup_images.each do |image| %>
+          <%= f.hidden_field :setup_images, multiple: true, value: image.signed_id %>
+        <% end %>
+
+        <%= f.input :setup_images, multiple: true, hint: t('simple_form.hints.delegate_report.setup_images', image_count: @delegate_report.required_setup_images_count), input_html: { multiple: true, accept: 'image/*' } %>
+
+        <% if @delegate_report.setup_images.attached? %>
+          <div class="row">
+            <% @delegate_report.setup_images.each do |image| %>
+              <div class="col-sm-1 thumb">
+                <%= image_tag image.variant(:preview).processed %>
+
+                <% if @delegate_report.setup_images.count > @delegate_report.required_setup_images_count %>
+                  <%= link_to 'Delete', delegate_report_delete_image_path(competition_id: @delegate_report.competition_id, image_id: image.id), method: :delete %>
+                <% end %>
+              </div>
+            <% end %>
+          </div>
+        <% end %>
+      <% end %>
     <% end %>
+
     <%= f.input :wrc_feedback_requested, as: :boolean %>
     <%= f.input :wrc_incidents %>
     <%= f.input :wic_feedback_requested, as: :boolean %>

app/views/delegate_reports/working_group_2024/_venue_default.md

@@ -0,0 +1,12 @@
+* **Equipment:** [Please fill in the counts for all corresponding competition gear below]
+
+Gen 3 Timers:
+Gen 4 Timers:
+Gen 5 Timers:
+
+Speed Stacks Displays:
+QiYi Displays:
+
+* **Venue notes** [Type of venue used, were any complaints made about aspects of the venue such as lighting or temperature, was there anything noteworthy or unique about the venue?]
+
+* **Setup:** [Please write a brief description of the competition set up. Photo(s) of the scrambling area and competition setup are to be uploaded using the upload button below.]

config/locales/en.yml

@@ -331,6 +331,7 @@ en:
         venue: "Venue"
         organization: "Organization"
         incidents: "Incidents"
+        setup_images: "Competition setup images"
         wrc_feedback_requested: "I would like feedback from the WRC on some of the incidents above."
         wrc_incidents: "Incidents Needing WRC Feedback"
         wic_feedback_requested: "I would like feedback from the WIC on some of the incidents above."
@@ -502,6 +503,7 @@ en:
         venue: "The listed points above are just for guidance for a complete report. Use them at your discretion for what you see fit. Please remove unused parts!"
         organization: "The listed points above are just for guidance for a complete report. Use them at your discretion for what you see fit. Please remove unused parts!"
         incidents: "Please report all incidents which are non-standard cases. Number the incidents starting from 1., so that other Delegates can comment referring to them by their numbers. Even if everything seemed generic, just list at least the most noteworthy cases. Please check off the appropriate boxes below if you are actively seeking feedback."
+        setup_images: "Please choose at least %{image_count} images which illustrate the competition setup (scrambling area, competition area, etc). You can select multiple files at once. You can delete images only when enough other images have been uploaded (if you want to replace an image, you need to upload the new one first, and then delete the old one)."
         wrc_feedback_requested: ""
         wrc_incidents: "List of numbers referencing any incidents that you would like the WRC's feedback on."
         wic_feedback_requested: ""

config/routes.rb

@@ -134,6 +134,7 @@
   get 'competitions/:competition_id/report/edit' => 'delegate_reports#edit', as: :delegate_report_edit
   get 'competitions/:competition_id/report' => 'delegate_reports#show', as: :delegate_report
   patch 'competitions/:competition_id/report' => 'delegate_reports#update'
+  delete 'competitions/:competition_id/report/:image_id' => 'delegate_reports#delete_image', as: :delegate_report_delete_image
 
   # Stripe needs this special redirect URL during OAuth, see the linked controller method for details
   get 'stripe-connect' => 'competitions#stripe_connect', as: :competitions_stripe_connect

lib/tasks/delegate_report_version.rake

@@ -1,7 +1,9 @@
 # frozen_string_literal: true
 
+DR2024_OLD_EQUIPMENT = "Gen 3 Timers:\nGen 4 Timers:\nGen 5 Timers:\n\nSpeed Stacks Displays:\nQiYi Displays:\n"
+
 namespace :delegate_reports do
-  desc "Copy devise_two_factor OTP secret from old format to new format"
+  desc "Migrate untouched 'legacy' reports to 'dr2024' format"
   task migrate_wg2024: [:environment] do
     template_report = DelegateReport.new(version: :legacy)
     template_report.md_section_defaults!
@@ -23,4 +25,31 @@ namespace :delegate_reports do
       end
     end
   end
+
+  desc "Introduce 'venue' section for reports in 'dr2024' format"
+  task migrate_wg2024_venue: [:environment] do
+    template_report = DelegateReport.new(version: :working_group_2024)
+    template_report.md_section_defaults!
+
+    venue_section_template = template_report.venue
+                                            .gsub("\r\n", "\n")
+                                            .gsub("\r", "\n")
+
+    # Only consider reports that haven't been posted yet
+    DelegateReport.where(posted_at: nil, version: :working_group_2024)
+                  .find_each do |dr|
+      puts "Migrating #{dr.id} (Competition '#{dr.competition_id}')"
+
+      dr.venue = venue_section_template
+
+      if dr.equipment.present?
+        puts "  Overriding 'equipment' part in venue section"
+        dr.venue.gsub!(DR2024_OLD_EQUIPMENT, dr.equipment)
+      end
+
+      dr.equipment = nil
+
+      dr.save!
+    end
+  end
 end

spec/controllers/delegate_reports_controller_spec.rb

@@ -6,9 +6,9 @@
   let(:delegate) { FactoryBot.create :delegate }
   let(:trainee_delegate) { FactoryBot.create :trainee_delegate }
   let(:comp) { FactoryBot.create(:competition, delegates: [delegate, trainee_delegate], starts: 2.days.ago) }
-  let!(:delegate_report1) { FactoryBot.create :delegate_report, competition: comp, schedule_url: "http://example.com" }
+  let!(:delegate_report1) { FactoryBot.create :delegate_report, :with_images, competition: comp, schedule_url: "http://example.com" }
   let(:pre_delegate_reports_form_comp) { FactoryBot.create(:competition, delegates: [delegate], starts: Date.new(2015, 1, 1)) }
-  let!(:delegate_report2) { FactoryBot.create :delegate_report, competition: pre_delegate_reports_form_comp, schedule_url: "http://example.com" }
+  let!(:delegate_report2) { FactoryBot.create :delegate_report, :with_images, competition: pre_delegate_reports_form_comp, schedule_url: "http://example.com" }
   let!(:wrc_members) { FactoryBot.create_list :user, 3, :wrc_member }
 
   context "not logged in" do
@@ -85,7 +85,8 @@
       post :update, params: { competition_id: comp.id, delegate_report: { remarks: "My newer remarks", posted: true } }
 
       expect(response).to redirect_to(delegate_report_path(comp))
-      assert_enqueued_jobs 3
+      assert_enqueued_jobs 2, queue: :mailers
+      assert_enqueued_jobs 1, only: SendWrcReportNotification
       expect(flash[:info]).to eq "Your report has been posted and emailed!"
       comp.reload
       expect(comp.delegate_report.remarks).to eq "My newer remarks"
@@ -114,7 +115,8 @@
       post :update, params: { competition_id: pre_delegate_reports_form_comp.id, delegate_report: { remarks: "My newer remarks", posted: true } }
       expect(response).to redirect_to(delegate_report_path(pre_delegate_reports_form_comp))
       expect(flash[:info]).to eq "Your report has been posted but not emailed because it is for a pre June 2016 competition."
-      assert_enqueued_jobs 0
+      assert_enqueued_jobs 0, queue: :mailers
+      assert_enqueued_jobs 0, only: SendWrcReportNotification
     end
   end
 

spec/factories/delegate_report.rb

@@ -8,10 +8,32 @@
       schedule_url { "http://example.com" }
       posted_at { Time.now }
       posted_by_user { FactoryBot.create(:user) }
+      upload_files { true }
+    end
+
+    trait :with_images do
+      upload_files { true }
+    end
+
+    transient do
+      upload_files { false }
     end
 
     initialize_with do
       competition.delegate_report
     end
+
+    after(:build, :create) do |dr, evaluator|
+      if evaluator.upload_files
+        dr.required_setup_images_count.times do |i|
+          default_io = File.open(Rails.root.join('app', 'assets', 'images', 'og-wca_logo.png'), 'rb')
+
+          dr.setup_images.attach(
+            io: default_io,
+            filename: "venue_setup_#{i}.png",
+          )
+        end
+      end
+    end
   end
 end

spec/features/delegate_report_spec.rb

@@ -2,10 +2,10 @@
 
 require "rails_helper"
 
-RSpec.feature "Registration management" do
+RSpec.feature "Delegate Report" do
   let!(:delegate) { FactoryBot.create :delegate, name: "Jeremy on Bart" }
   let(:competition) { FactoryBot.create :competition, :with_valid_submitted_results, delegates: [delegate], name: "Submit Report 2017" }
-  let!(:delegate_report) { FactoryBot.create :delegate_report, competition: competition, schedule_url: "http://example.com" }
+  let!(:delegate_report) { FactoryBot.create :delegate_report, :with_images, competition: competition, schedule_url: "http://example.com" }
   let!(:wrc_members) { FactoryBot.create_list :user, 3, :wrc_member }
 
   context "when signed in as competition delegate" do

spec/models/competition_spec.rb

@@ -419,7 +419,8 @@
 
     it "does not warn for posted reports" do
       competition = FactoryBot.create :competition, :visible, :with_delegate, starts: 2.days.ago
-      competition.delegate_report.update!(schedule_url: "http://example.com", posted: true)
+      posted_dummy_dr = FactoryBot.create :delegate_report, :posted, competition: competition
+      competition.delegate_report.update!(schedule_url: "http://example.com", posted: true, setup_images: posted_dummy_dr.setup_images_blobs)
       delegate = competition.delegates.first
       expect(competition.user_should_post_delegate_report?(delegate)).to eq false
     end

spec/models/delegate_report_spec.rb

@@ -19,7 +19,7 @@
   end
 
   it "schedule_url is required when posted" do
-    dr = FactoryBot.build :delegate_report, schedule_url: nil
+    dr = FactoryBot.build :delegate_report, :with_images, schedule_url: nil
     expect(dr).to be_valid
 
     dr.posted = true
@@ -79,7 +79,8 @@
       end
 
       it "can view delegate report with posted report" do
-        competition.delegate_report.update!(schedule_url: "http://example.com", posted: true)
+        posted_dummy_dr = FactoryBot.create :delegate_report, :posted, competition: competition
+        competition.delegate_report.update!(schedule_url: "http://example.com", posted: true, setup_images: posted_dummy_dr.setup_images_blobs)
 
         expect(other_delegate.can_view_delegate_report?(competition.delegate_report)).to eq true
       end