Don't allow to modify answer's images for started polls

Note that the `create` action doesn't create an image but updates an
answer instead. We're removing the references to `:create` in the
abilities since it isn't used.

In the future we might change the form to add an image to an answer
because it's been broken for ages since it shows all the attached
images.

app/controllers/admin/poll/questions/answers/images_controller.rb

@@ -2,6 +2,7 @@ class Admin::Poll::Questions::Answers::ImagesController < Admin::Poll::BaseContr
   include ImageAttributes
 
   load_and_authorize_resource :answer, class: "::Poll::Question::Answer"
+  load_and_authorize_resource only: [:destroy]
 
   def index
   end
@@ -11,6 +12,7 @@ class Admin::Poll::Questions::Answers::ImagesController < Admin::Poll::BaseContr
 
   def create
     @answer.attributes = images_params
+    authorize! :update, @answer
 
     if @answer.save
       redirect_to admin_answer_images_path(@answer),
@@ -21,7 +23,6 @@ class Admin::Poll::Questions::Answers::ImagesController < Admin::Poll::BaseContr
   end
 
   def destroy
-    @image = ::Image.find(params[:id])
     @image.destroy!
 
     respond_to do |format|

app/models/abilities/administrator.rb

@@ -103,8 +103,8 @@ module Abilities
       can [:create, :update, :destroy], Poll::Question::Answer::Video do |video|
         can?(:update, video.answer)
       end
-      can [:create, :destroy], Image do |image|
+      can [:destroy], Image do |image|
-        image.imageable_type == "Poll::Question::Answer"
+        image.imageable_type == "Poll::Question::Answer" && can?(:update, image.imageable)
       end
 
       can :manage, SiteCustomization::Page

app/models/abilities/common.rb

@@ -74,7 +74,9 @@ module Abilities
         document.documentable&.author_id == user.id
       end
 
-      can [:destroy], Image, imageable: { author_id: user.id }
+      can [:destroy], Image do |image|
+        image.imageable_type != "Poll::Question::Answer" && image.imageable&.author_id == user.id
+      end
 
       can [:create, :destroy], DirectUpload
 

app/views/admin/poll/questions/answers/images/index.html.erb

@@ -1,8 +1,20 @@
 <%= back_link_to admin_question_path(@answer.question) %>
 
-<%= link_to t("admin.questions.answers.images.add_image"),
+<div class="clear"></div>
-            new_admin_answer_image_path(@answer),
+
-            class: "button hollow float-right" %>
+<h2 class="inline-block">
+  <%= t("admin.answers.images.index.title") %>
+</h2>
+
+<% if can?(:update, @answer) %>
+  <%= link_to t("admin.questions.answers.images.add_image"),
+              new_admin_answer_image_path(@answer),
+              class: "button hollow float-right" %>
+<% else %>
+  <div class="callout warning">
+    <strong><%= t("admin.questions.no_edit") %></strong>
+  </div>
+<% end %>
 
 <ul class="breadcrumbs margin-top">
   <li><%= @answer.question.title %></li>
@@ -13,11 +25,13 @@
   <div class="small-12 medium-4 column end">
     <%= render_image(image, :large, true) if image.present? %>
 
-    <%= link_to t("images.remove_image"),
+    <% if can?(:destroy, image) %>
-                admin_image_path(image),
+      <%= link_to t("images.remove_image"),
-                class: "delete float-right",
+                  admin_image_path(image),
-                method: :delete,
+                  class: "delete float-right",
-                remote: true,
+                  method: :delete,
-                data: { confirm: t("admin.actions.confirm_action", action: t("images.remove_image"), name: image.title) } %>
+                  remote: true,
+                  data: { confirm: t("admin.actions.confirm_action", action: t("images.remove_image"), name: image.title) } %>
+    <% end %>
   </div>
 <% end %>

config/locales/en/admin.yml

@@ -1161,6 +1161,9 @@ en:
         title: Edit answer
       destroy:
         success_notice: "Answer deleted successfully"
+      images:
+        index:
+          title: Images
       videos:
         index:
           title: Videos

config/locales/es/admin.yml

@@ -1160,6 +1160,9 @@ es:
         title: Editar respuesta
       destroy:
         success_notice: "Respuesta eliminada correctamente"
+      images:
+        index:
+          title: Imágenes
       videos:
         index:
           title: Vídeos

spec/controllers/admin/poll/questions/answers/images_controller_spec.rb

@@ -0,0 +1,52 @@
+require "rails_helper"
+
+describe Admin::Poll::Questions::Answers::ImagesController, :admin do
+  let(:current_answer) { create(:poll_question_answer, poll: create(:poll)) }
+  let(:future_answer) { create(:poll_question_answer, poll: create(:poll, :future)) }
+
+  describe "POST create" do
+    let(:answer_attributes) do
+      {
+        images_attributes: {
+          "0" => {
+            attachment: fixture_file_upload("clippy.jpg"),
+            title: "Title",
+            user_id: User.last.id
+          }
+        }
+      }
+    end
+
+    it "is not possible for an already started poll" do
+      post :create, params: { poll_question_answer: answer_attributes, answer_id: current_answer }
+
+      expect(flash[:alert]).to eq "You do not have permission to carry out the action 'update' on Answer."
+      expect(Image.count).to eq 0
+    end
+
+    it "is possible for a not started poll" do
+      post :create, params: { poll_question_answer: answer_attributes, answer_id: future_answer }
+
+      expect(response).to redirect_to admin_answer_images_path(future_answer)
+      expect(flash[:notice]).to eq "Image uploaded successfully"
+      expect(Image.count).to eq 1
+    end
+  end
+
+  describe "DELETE destroy" do
+    it "is not possible for an already started poll" do
+      current_image = create(:image, imageable: current_answer)
+      delete :destroy, xhr: true, params: { id: current_image }
+
+      expect(flash[:alert]).to eq "You do not have permission to carry out the action 'destroy' on Image."
+      expect(Image.count).to eq 1
+    end
+
+    it "is possible for a not started poll" do
+      future_image = create(:image, imageable: future_answer)
+      delete :destroy, xhr: true, params: { id: future_image }
+
+      expect(Image.count).to eq 0
+    end
+  end
+end

spec/factories/polls.rb

@@ -80,6 +80,10 @@ FactoryBot.define do
     trait :with_video do
       after(:create) { |answer| create(:poll_answer_video, answer: answer) }
     end
+
+    factory :future_poll_question_answer do
+      poll { association(:poll, :future) }
+    end
   end
 
   factory :poll_answer_video, class: "Poll::Question::Answer::Video" do

spec/models/abilities/administrator_spec.rb

@@ -24,7 +24,8 @@ describe Abilities::Administrator do
   let(:future_poll_question_answer) { create(:poll_question_answer, poll: future_poll) }
   let(:current_poll_answer_video) { create(:poll_answer_video, answer: current_poll_question_answer) }
   let(:future_poll_answer_video) { create(:poll_answer_video, answer: future_poll_question_answer) }
-  let(:answer_image) { build(:image, imageable: current_poll_question_answer) }
+  let(:current_poll_answer_image) { build(:image, imageable: current_poll_question_answer) }
+  let(:future_poll_answer_image) { build(:image, imageable: future_poll_question_answer) }
 
   let(:past_process) { create(:legislation_process, :past) }
   let(:past_draft_process) { create(:legislation_process, :past, :not_published) }
@@ -141,8 +142,8 @@ describe Abilities::Administrator do
   it { should_not be_able_to(:update, current_poll_answer_video) }
   it { should_not be_able_to(:destroy, current_poll_answer_video) }
 
-  it { should be_able_to(:create, answer_image) }
+  it { should be_able_to(:destroy, future_poll_answer_image) }
-  it { should be_able_to(:destroy, answer_image) }
+  it { should_not be_able_to(:destroy, current_poll_answer_image) }
 
   it { is_expected.to be_able_to :manage, Dashboard::AdministratorTask }
   it { is_expected.to be_able_to :manage, dashboard_administrator_task }

spec/system/admin/poll/questions/answers/images/images_spec.rb

@@ -1,8 +1,11 @@
 require "rails_helper"
 
 describe "Images", :admin do
+  let(:future_poll) { create(:poll, :future) }
+  let(:current_poll) { create(:poll) }
+
   it_behaves_like "nested imageable",
-                  "poll_question_answer",
+                  "future_poll_question_answer",
                   "new_admin_answer_image_path",
                   { answer_id: "id" },
                   nil,
@@ -30,34 +33,63 @@ describe "Images", :admin do
     end
   end
 
-  scenario "Add image to answer" do
+  describe "Add image to answer" do
-    answer = create(:poll_question_answer)
+    scenario "Is possible for a not started poll" do
+      answer = create(:poll_question_answer, poll: future_poll)
 
-    visit admin_answer_images_path(answer)
+      visit admin_answer_images_path(answer)
-    expect(page).not_to have_css("img[title='clippy.jpg']")
-    expect(page).not_to have_content("clippy.jpg")
 
-    visit new_admin_answer_image_path(answer)
+      expect(page).not_to have_css "img[title='clippy.jpg']"
-    imageable_attach_new_file(file_fixture("clippy.jpg"))
+      expect(page).not_to have_content "clippy.jpg"
-    click_button "Save image"
 
-    expect(page).to have_css("img[title='clippy.jpg']")
+      click_link "Add image"
-    expect(page).to have_content("clippy.jpg")
+      expect(page).to have_content "Descriptive image"
-  end
 
-  scenario "Remove image from answer" do
+      imageable_attach_new_file(file_fixture("clippy.jpg"))
-    answer = create(:poll_question_answer)
+      click_button "Save image"
-    image = create(:image, imageable: answer)
 
-    visit admin_answer_images_path(answer)
+      expect(page).to have_content "Image uploaded successfully"
-    expect(page).to have_css("img[title='#{image.title}']")
+      expect(page).to have_css "img[title='clippy.jpg']"
-    expect(page).to have_content(image.title)
+      expect(page).to have_content "clippy.jpg"
-
-    accept_confirm "Are you sure? Remove image \"#{image.title}\"" do
-      click_link "Remove image"
     end
 
-    expect(page).not_to have_css("img[title='#{image.title}']")
+    scenario "Is not possible for an already started poll" do
-    expect(page).not_to have_content(image.title)
+      answer = create(:poll_question_answer, poll: current_poll)
+
+      visit admin_answer_images_path(answer)
+
+      expect(page).not_to have_link "Add image"
+      expect(page).to have_content "Once the poll has started it will not be possible to create, edit or"
+    end
+  end
+
+  describe "Remove image from answer" do
+    scenario "Is possible for a not started poll" do
+      answer = create(:poll_question_answer, poll: future_poll)
+      image = create(:image, imageable: answer)
+
+      visit admin_answer_images_path(answer)
+      expect(page).to have_css "img[title='#{image.title}']"
+      expect(page).to have_content image.title
+
+      accept_confirm "Are you sure? Remove image \"#{image.title}\"" do
+        click_link "Remove image"
+      end
+
+      expect(page).not_to have_css "img[title='#{image.title}']"
+      expect(page).not_to have_content image.title
+    end
+
+    scenario "Is not possible for an already started poll" do
+      answer = create(:poll_question_answer, poll: current_poll)
+      image = create(:image, imageable: answer)
+
+      visit admin_answer_images_path(answer)
+      expect(page).to have_css "img[title='#{image.title}']"
+      expect(page).to have_content image.title
+
+      expect(page).not_to have_link "Remove image"
+      expect(page).to have_content "Once the poll has started it will not be possible to create, edit or"
+    end
   end
 end