Don't allow to modify questions for started polls

Adding, modifiying, and/or deleting questions for an already started
poll is far away from being democratic and can lead to unwanted side
effects like missing votes in the results or stats.

So, from now on, only modifiying questions will be possible only if
the poll has not started yet.

app/components/admin/poll/questions/form_component.html.erb

@@ -12,10 +12,10 @@
         <%= f.hidden_field :poll_id, value: question.poll.id %>
       <% else %>
         <div class="small-12 medium-6 large-4 column">
-          <% select_options = Poll.all.map { |p| [p.name, p.id] } %>
           <%= f.select :poll_id,
                         options_for_select(select_options),
-                        prompt: t("admin.questions.index.select_poll") %>
+                        prompt: t("admin.questions.index.select_poll"),
+                        hint: t("admin.questions.form.poll_help") %>
         </div>
       <% end %>
     </div>

app/components/admin/poll/questions/form_component.rb

@@ -2,9 +2,18 @@ class Admin::Poll::Questions::FormComponent < ApplicationComponent
   include TranslatableFormHelper
   include GlobalizeHelper
   attr_reader :question, :url
+  delegate :can?, to: :helpers
 
   def initialize(question, url:)
     @question = question
     @url = url
   end
+
+  private
+
+    def select_options
+      Poll.all.select { |poll| can?(:create, Poll::Question.new(poll: poll)) }.map do |poll|
+        [poll.name, poll.id]
+      end
+    end
 end

app/components/admin/poll/questions/table_actions_component.html.erb

@@ -0,0 +1,3 @@
+<%= render Admin::TableActionsComponent.new(question, actions: actions) do |table_actions| %>
+  <%= table_actions.action(:answers, text: t("admin.polls.show.edit_answers")) %>
+<% end %>

app/components/admin/poll/questions/table_actions_component.rb

@@ -0,0 +1,14 @@
+class Admin::Poll::Questions::TableActionsComponent < ApplicationComponent
+  attr_reader :question
+  delegate :can?, to: :helpers
+
+  def initialize(question)
+    @question = question
+  end
+
+  private
+
+    def actions
+      [:edit, :destroy].select { |action| can?(action, question) }
+    end
+end

app/controllers/admin/poll/questions_controller.rb

@@ -3,7 +3,8 @@ class Admin::Poll::QuestionsController < Admin::Poll::BaseController
   include Translatable
 
   load_and_authorize_resource :poll
-  load_and_authorize_resource :question, class: "Poll::Question"
+  load_resource class: "Poll::Question"
+  authorize_resource except: [:new, :index]
 
   def index
     @polls = Poll.not_budget
@@ -17,6 +18,8 @@ class Admin::Poll::QuestionsController < Admin::Poll::BaseController
     proposal = Proposal.find(params[:proposal_id]) if params[:proposal_id].present?
     @question.copy_attributes_from_proposal(proposal)
     @question.poll = @poll
+
+    authorize! :create, @question
   end
 
   def create

app/models/abilities/administrator.rb

@@ -88,8 +88,13 @@ module Abilities
       can [:search, :create, :index, :destroy], ::Poll::Officer
       can [:create, :destroy, :manage], ::Poll::BoothAssignment
       can [:create, :destroy], ::Poll::OfficerAssignment
-      can [:read, :create, :update], Poll::Question
-      can :destroy, Poll::Question
+      can :read, Poll::Question
+      can [:create], Poll::Question do |question|
+        question.poll.blank? || !question.poll.started?
+      end
+      can [:update, :destroy], Poll::Question do |question|
+        !question.poll.started?
+      end
       can :manage, Poll::Question::Answer
       can :manage, Poll::Question::Answer::Video
       can [:create, :destroy], Image do |image|

app/models/poll.rb

@@ -71,6 +71,10 @@ class Poll < ApplicationRecord
     name
   end
 
+  def started?(timestamp = Time.current)
+    starts_at.present? && starts_at < timestamp
+  end
+
   def current?(timestamp = Time.current)
     starts_at <= timestamp && timestamp <= ends_at
   end

app/views/admin/poll/polls/_poll_header.html.erb

@@ -23,3 +23,7 @@
       </div>
   <% end %>
 </div>
+
+<div class="callout warning">
+  <strong><%= t("admin.questions.no_edit") %></strong>
+</div>

app/views/admin/poll/polls/_questions.html.erb

@@ -1,7 +1,9 @@
 <h3 class="inline-block"><%= t("admin.polls.show.questions_title") %></h3>
 
-<%= link_to t("admin.questions.index.create"), new_admin_question_path(poll_id: @poll.id),
-            class: "button float-right" %>
+<% if can?(:create, Poll::Question.new(poll: @poll)) %>
+  <%= link_to t("admin.questions.index.create"), new_admin_question_path(poll_id: @poll.id),
+              class: "button float-right" %>
+<% end %>
 
 <% if @poll.questions.empty? %>
   <div class="callout primary margin-top">
@@ -28,9 +30,7 @@
           <% end %>
         </td>
         <td>
-          <%= render Admin::TableActionsComponent.new(question) do |actions| %>
-            <%= actions.action(:answers, text: t("admin.polls.show.edit_answers")) %>
-          <% end %>
+          <%= render Admin::Poll::Questions::TableActionsComponent.new(question) %>
         </td>
       </tr>
     <% end %>

app/views/admin/poll/questions/show.html.erb

@@ -1,7 +1,9 @@
 <%= back_link_to admin_poll_path(@question.poll) %>
 
-<%= link_to t("admin.questions.show.edit_question"), edit_admin_question_path(@question),
-            class: "button hollow float-right" %>
+<% if can?(:update, @question) %>
+  <%= link_to t("admin.questions.show.edit_question"), edit_admin_question_path(@question),
+              class: "button hollow float-right" %>
+<% end %>
 
 <div class="clear"></div>
 

config/locales/en/admin.yml

@@ -1123,6 +1123,8 @@ en:
         poll_not_assigned: "Poll not assigned"
       edit:
         title: "Edit Question"
+      form:
+        poll_help: "You can only select polls that have not started yet"
       new:
         title: "Create question to poll %{poll}"
         title_proposal: "Create question"
@@ -1151,6 +1153,7 @@ en:
           documents_list: Documents list
           document_title: Title
           document_actions: Actions
+      no_edit: "Once the poll has started it will not be possible to create, edit or delete questions, answers or any content associated with the poll."
     answers:
       new:
         title: New answer

config/locales/es/admin.yml

@@ -1122,6 +1122,8 @@ es:
         poll_not_assigned: "Votación no asignada"
       edit:
         title: "Editar pregunta ciudadana"
+      form:
+        poll_help: "Solo se pueden seleccionar votaciones que no hayan empezado"
       new:
         title: "Crear pregunta ciudadana para la votación %{poll}"
         title_proposal: "Crear pregunta ciudadana"
@@ -1150,6 +1152,7 @@ es:
           documents_list: Lista de documentos
           document_title: Título
           document_actions: Acciones
+      no_edit: "Una vez comenzada la votación no será posible crear, editar o eliminar preguntas, respuestas o cualquier contenido asociado a la votación."
     answers:
       new:
         title: Nueva respuesta

spec/components/admin/poll/questions/form_component_spec.rb

@@ -0,0 +1,37 @@
+require "rails_helper"
+
+describe Admin::Poll::Questions::FormComponent do
+  before { sign_in(create(:administrator).user) }
+
+  context "question with a poll" do
+    let(:poll) { create(:poll) }
+    let(:question) { Poll::Question.new(poll: poll) }
+
+    it "does not display the poll selector" do
+      render_inline Admin::Poll::Questions::FormComponent.new(question, url: "/")
+
+      expect(page).not_to have_select "Poll"
+      expect(page).to have_field "poll_question[poll_id]", type: :hidden, with: poll.id
+    end
+  end
+
+  context "question without a poll" do
+    let(:question) { Poll::Question.new }
+
+    it "allows selecting polls which have not already started" do
+      create(:poll, :future, name: "Future poll")
+
+      render_inline Admin::Poll::Questions::FormComponent.new(question, url: "/")
+
+      expect(page).to have_select "Poll", options: ["Select Poll", "Future poll"]
+    end
+
+    it "does not allow selecting polls which have already started" do
+      create(:poll, name: "Already started poll")
+
+      render_inline Admin::Poll::Questions::FormComponent.new(question, url: "/")
+
+      expect(page).to have_select "Poll", options: ["Select Poll"]
+    end
+  end
+end

spec/components/admin/poll/questions/table_actions_component_spec.rb

@@ -0,0 +1,25 @@
+require "rails_helper"
+
+describe Admin::Poll::Questions::TableActionsComponent, controller: Admin::BaseController do
+  before { sign_in(create(:administrator).user) }
+
+  it "displays the edit and destroy actions when the poll has not started" do
+    question = create(:poll_question, poll: create(:poll, :future))
+
+    render_inline Admin::Poll::Questions::TableActionsComponent.new(question)
+
+    expect(page).to have_link "Edit answers"
+    expect(page).to have_link "Edit"
+    expect(page).to have_button "Delete"
+  end
+
+  it "does not display the edit and destroy actions when the poll has started" do
+    question = create(:poll_question, poll: create(:poll))
+
+    render_inline Admin::Poll::Questions::TableActionsComponent.new(question)
+
+    expect(page).to have_link "Edit answers"
+    expect(page).not_to have_link "Edit"
+    expect(page).not_to have_button "Delete"
+  end
+end

spec/controllers/admin/poll/questions_controller_spec.rb

@@ -0,0 +1,105 @@
+require "rails_helper"
+
+describe Admin::Poll::QuestionsController, :admin do
+  let(:current_poll) { create(:poll) }
+  let(:future_poll) { create(:poll, :future) }
+
+  describe "POST create" do
+    it "is not possible for an already started poll" do
+      post :create, params: {
+        poll_question: {
+          translations_attributes: {
+            "0" => {
+              locale: "en",
+              title: "Question from started poll"
+            }
+          },
+          poll_id: current_poll
+        }
+      }
+
+      expect(flash[:alert]).to eq "You do not have permission to carry out the action 'create' on Question."
+      expect(Poll::Question.count).to eq 0
+    end
+
+    it "is possible for a not started poll" do
+      post :create, params: {
+        poll_question: {
+          translations_attributes: {
+            "0" => {
+              locale: "en",
+              title: "Question from future poll"
+            }
+          },
+          poll_id: future_poll
+        }
+      }
+
+      expect(response).to redirect_to admin_question_path(Poll::Question.last)
+      expect(Poll::Question.last.title).to eq "Question from future poll"
+      expect(Poll::Question.count).to eq 1
+    end
+  end
+
+  describe "PATCH update" do
+    it "is not possible for an already started poll" do
+      current_question = create(:poll_question, poll: current_poll, title: "Sample title")
+
+      patch :update, params: {
+        poll_question: {
+          translations_attributes: {
+            "0" => {
+              locale: "en",
+              title: "New title",
+              id: current_question.translations.first.id
+            }
+          }
+        },
+        id: current_question
+      }
+
+      expect(flash[:alert]).to eq "You do not have permission to carry out the action 'update' on Question."
+      expect(current_question.reload.title).to eq "Sample title"
+    end
+
+    it "is possible for a not started poll" do
+      future_question = create(:poll_question, poll: future_poll)
+
+      patch :update, params: {
+        poll_question: {
+          translations_attributes: {
+            "0" => {
+              locale: "en",
+              title: "New title",
+              id: future_question.translations.first.id
+            }
+          }
+        },
+        id: future_question
+      }
+
+      expect(response).to redirect_to admin_question_path(future_question)
+      expect(flash[:notice]).to eq "Changes saved"
+      expect(future_question.reload.title).to eq "New title"
+    end
+  end
+
+  describe "DELETE destroy" do
+    it "is not possible for an already started poll" do
+      current_question = create(:poll_question, poll: current_poll)
+      delete :destroy, params: { id: current_question }
+
+      expect(flash[:alert]).to eq "You do not have permission to carry out the action 'destroy' on Question."
+      expect(Poll::Question.count).to eq 1
+    end
+
+    it "is possible for a not started poll" do
+      future_question = create(:poll_question, poll: future_poll)
+      delete :destroy, params: { id: future_question }
+
+      expect(response).to redirect_to admin_poll_path(future_poll)
+      expect(flash[:notice]).to eq "Question deleted successfully"
+      expect(Poll::Question.count).to eq 0
+    end
+  end
+end

spec/models/abilities/administrator_spec.rb

@@ -16,8 +16,10 @@ describe Abilities::Administrator do
   let(:budget_investment) { create(:budget_investment) }
   let(:finished_investment) { create(:budget_investment, budget: create(:budget, :finished)) }
   let(:legislation_question) { create(:legislation_question) }
-  let(:poll) { create(:poll) }
-  let(:poll_question) { create(:poll_question) }
+  let(:current_poll) { create(:poll) }
+  let(:future_poll) { create(:poll, :future) }
+  let(:current_poll_question) { create(:poll_question) }
+  let(:future_poll_question) { create(:poll_question, poll: future_poll) }
   let(:poll_question_answer) { create(:poll_question_answer) }
   let(:answer_image) { build(:image, imageable: poll_question_answer) }
 
@@ -27,7 +29,7 @@ describe Abilities::Administrator do
 
   let(:proposal_document) { build(:document, documentable: proposal, user: proposal.author) }
   let(:budget_investment_document) { build(:document, documentable: budget_investment) }
-  let(:poll_question_document) { build(:document, documentable: poll_question) }
+  let(:poll_question_document) { build(:document, documentable: current_poll_question) }
 
   let(:proposal_image) { build(:image, imageable: proposal, user: proposal.author) }
   let(:budget_investment_image) { build(:image, imageable: budget_investment) }
@@ -74,8 +76,8 @@ describe Abilities::Administrator do
   it { should be_able_to(:comment_as_administrator, legislation_question) }
   it { should_not be_able_to(:comment_as_moderator, legislation_question) }
 
-  it { should be_able_to(:comment_as_administrator, poll) }
-  it { should_not be_able_to(:comment_as_moderator, poll) }
+  it { should be_able_to(:comment_as_administrator, current_poll) }
+  it { should_not be_able_to(:comment_as_moderator, current_poll) }
 
   it { should be_able_to(:summary, past_process) }
   it { should_not be_able_to(:summary, past_draft_process) }
@@ -113,8 +115,12 @@ describe Abilities::Administrator do
   it { should be_able_to(:manage, Dashboard::Action) }
 
   it { should be_able_to(:read, Poll::Question) }
-  it { should be_able_to(:create, Poll::Question) }
-  it { should be_able_to(:update, Poll::Question) }
+  it { should be_able_to(:create, future_poll_question) }
+  it { should be_able_to(:update, future_poll_question) }
+  it { should be_able_to(:destroy, future_poll_question) }
+  it { should_not be_able_to(:create, current_poll_question) }
+  it { should_not be_able_to(:update, current_poll_question) }
+  it { should_not be_able_to(:destroy, current_poll_question) }
 
   it { should be_able_to(:manage, Poll::Question::Answer) }
 

spec/system/admin/poll/questions_spec.rb

@@ -2,9 +2,9 @@ require "rails_helper"
 
 describe "Admin poll questions", :admin do
   scenario "Index" do
-    poll1 = create(:poll)
-    poll2 = create(:poll)
-    poll3 = create(:poll)
+    poll1 = create(:poll, :future)
+    poll2 = create(:poll, :future)
+    poll3 = create(:poll, :future)
     proposal = create(:proposal)
     question1 = create(:poll_question, poll: poll1)
     question2 = create(:poll_question, poll: poll2)
@@ -55,25 +55,32 @@ describe "Admin poll questions", :admin do
     expect(page).to have_content question.author.name
   end
 
-  scenario "Create" do
-    poll = create(:poll, name: "Movies")
-    title = "Star Wars: Episode IV - A New Hope"
+  describe "Create" do
+    scenario "Is possible for a not started poll" do
+      poll = create(:poll, :future, name: "Movies")
 
-    visit admin_poll_path(poll)
-    click_link "Create question"
+      visit admin_poll_path(poll)
+      click_link "Create question"
 
-    expect(page).to have_content("Create question to poll Movies")
-    expect(page).to have_selector("input[id='poll_question_poll_id'][value='#{poll.id}']",
-                                   visible: :hidden)
-    fill_in "Question", with: title
+      expect(page).to have_content("Create question to poll Movies")
+      expect(page).to have_selector("input[id='poll_question_poll_id'][value='#{poll.id}']",
+                                    visible: :hidden)
 
-    click_button "Save"
+      fill_in "Question", with: "Star Wars: Episode IV - A New Hope"
+      click_button "Save"
 
-    expect(page).to have_content(title)
+      expect(page).to have_content "Star Wars: Episode IV - A New Hope"
+    end
+
+    scenario "Is not possible for an already started poll" do
+      visit admin_poll_path(create(:poll))
+
+      expect(page).not_to have_link "Create question"
+    end
   end
 
   scenario "Create from proposal" do
-    create(:poll, name: "Proposals")
+    create(:poll, :future, name: "Proposals")
     proposal = create(:proposal)
 
     visit admin_proposal_path(proposal)
@@ -84,7 +91,7 @@ describe "Admin poll questions", :admin do
     expect(page).to have_current_path(new_admin_question_path, ignore_query: true)
     expect(page).to have_field("Question", with: proposal.title)
 
-    select "Proposals", from: "poll_question_poll_id"
+    select "Proposals", from: "Poll"
 
     click_button "Save"
 
@@ -92,7 +99,7 @@ describe "Admin poll questions", :admin do
   end
 
   scenario "Create from successful proposal" do
-    create(:poll, name: "Proposals")
+    create(:poll, :future, name: "Proposals")
     proposal = create(:proposal, :successful)
 
     visit admin_proposal_path(proposal)
@@ -103,7 +110,7 @@ describe "Admin poll questions", :admin do
     expect(page).to have_current_path(new_admin_question_path, ignore_query: true)
     expect(page).to have_field("Question", with: proposal.title)
 
-    select "Proposals", from: "poll_question_poll_id"
+    select "Proposals", from: "Poll"
 
     click_button "Save"
 
@@ -115,29 +122,29 @@ describe "Admin poll questions", :admin do
   end
 
   scenario "Update" do
-    poll = create(:poll)
-    question1 = create(:poll_question, poll: poll)
+    poll = create(:poll, :future)
+    question = create(:poll_question, poll: poll)
+    old_title = question.title
+    new_title = "Vegetables are great and everyone should have one"
 
     visit admin_poll_path(poll)
 
-    within("#poll_question_#{question1.id}") do
+    within("#poll_question_#{question.id}") do
       click_link "Edit"
     end
 
     expect(page).to have_link "Go back", href: admin_poll_path(poll)
-    old_title = question1.title
-    new_title = "Potatoes are great and everyone should have one"
     fill_in "Question", with: new_title
 
     click_button "Save"
 
     expect(page).to have_content "Changes saved"
     expect(page).to have_content new_title
-    expect(page).not_to have_content(old_title)
+    expect(page).not_to have_content old_title
   end
 
   scenario "Destroy" do
-    poll = create(:poll)
+    poll = create(:poll, :future)
     question1 = create(:poll_question, poll: poll)
     question2 = create(:poll_question, poll: poll)
 
@@ -156,7 +163,7 @@ describe "Admin poll questions", :admin do
 
   context "Poll select box" do
     scenario "translates the poll name in options" do
-      poll = create(:poll, name_en: "Name in English", name_es: "Nombre en Español")
+      poll = create(:poll, :future, name_en: "Name in English", name_es: "Nombre en Español")
       proposal = create(:proposal)
 
       visit admin_proposal_path(proposal)
@@ -172,7 +179,7 @@ describe "Admin poll questions", :admin do
 
     scenario "uses fallback if name is not translated to current locale",
              if: Globalize.fallbacks(:fr).reject { |locale| locale.match(/fr/) }.first == :es do
-      poll = create(:poll, name_en: "Name in English", name_es: "Nombre en Español")
+      poll = create(:poll, :future, name_en: "Name in English", name_es: "Nombre en Español")
       proposal = create(:proposal)
 
       visit admin_proposal_path(proposal)

spec/system/admin/translatable_spec.rb

@@ -383,7 +383,7 @@ describe "Admin edit translatable records", :admin do
   end
 
   context "Remove a translation with invalid data" do
-    let(:translatable) { create(:poll_question) }
+    let(:translatable) { create(:poll_question, poll: create(:poll, :future)) }
     let(:path) { edit_admin_question_path(translatable) }
 
     scenario "Doesn't remove the translation" do