Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add inline images to Delegate Reports #10086

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions app/controllers/delegate_reports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -77,6 +86,7 @@ def update
:wic_feedback_requested,
:wic_incidents,
*DelegateReport::AVAILABLE_SECTIONS,
setup_images: [],
)
end

Expand Down
21 changes: 20 additions & 1 deletion app/models/delegate_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
14 changes: 14 additions & 0 deletions app/views/delegate_reports/_delegate_report.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
Expand Down
23 changes: 23 additions & 0 deletions app/views/delegate_reports/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
Expand Down

This file was deleted.

12 changes: 12 additions & 0 deletions app/views/delegate_reports/working_group_2024/_venue_default.md
Original file line number Diff line number Diff line change
@@ -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.]
2 changes: 2 additions & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down Expand Up @@ -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. 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: ""
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,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
Expand Down
31 changes: 30 additions & 1 deletion lib/tasks/delegate_report_version.rake
Original file line number Diff line number Diff line change
@@ -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!
Expand All @@ -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
10 changes: 6 additions & 4 deletions spec/controllers/delegate_reports_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand Down
22 changes: 22 additions & 0 deletions spec/factories/delegate_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions spec/features/delegate_report_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion spec/models/competition_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions spec/models/delegate_report_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down