From 5011d4745bdc5dc2bc64a33400f89a2cfa7492b3 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 7 Sep 2022 13:41:26 +0200 Subject: [PATCH 01/19] Remove related code for unused actions from admin polls controller Actions :search_booths and :search_officers in admin polls controller are moved to other controllers since commit 20e31133a for :search_booths and commit 19ec7f93b for :search_officers. This then allows us to remove the code that references these actions in the controller and in the administrator abilities. --- app/controllers/admin/poll/polls_controller.rb | 9 --------- app/models/abilities/administrator.rb | 2 +- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/app/controllers/admin/poll/polls_controller.rb b/app/controllers/admin/poll/polls_controller.rb index 8cfa00136..5e9c84c90 100644 --- a/app/controllers/admin/poll/polls_controller.rb +++ b/app/controllers/admin/poll/polls_controller.rb @@ -4,7 +4,6 @@ class Admin::Poll::PollsController < Admin::Poll::BaseController include ReportAttributes load_and_authorize_resource - before_action :load_search, only: [:search_booths, :search_officers] before_action :load_geozones, only: [:new, :create, :edit, :update] def index @@ -86,14 +85,6 @@ class Admin::Poll::PollsController < Admin::Poll::BaseController [*attributes, *report_attributes, translation_params(Poll)] end - def search_params - params.permit(:poll_id, :search) - end - - def load_search - @search = search_params[:search] - end - def resource @poll ||= Poll.find(params[:id]) end diff --git a/app/models/abilities/administrator.rb b/app/models/abilities/administrator.rb index 0f492cd0b..44201bdf4 100644 --- a/app/models/abilities/administrator.rb +++ b/app/models/abilities/administrator.rb @@ -83,7 +83,7 @@ module Abilities can [:index, :create, :update, :destroy], Geozone - can [:read, :create, :update, :destroy, :add_question, :search_booths, :search_officers, :booth_assignments], Poll + can [:read, :create, :update, :destroy, :add_question, :booth_assignments], Poll can [:read, :create, :update, :destroy, :available], Poll::Booth can [:search, :create, :index, :destroy], ::Poll::Officer can [:create, :destroy, :manage], ::Poll::BoothAssignment From 38b6cf36a2ca476cf008289da82de694c6f17b3e Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 7 Sep 2022 14:25:15 +0200 Subject: [PATCH 02/19] Remove unused add_question action from admin polls controller Since commit adf18ee756 this action no longer makes sense. --- app/controllers/admin/poll/polls_controller.rb | 12 ------------ app/models/abilities/administrator.rb | 2 +- config/locales/en/admin.yml | 3 --- config/locales/es/admin.yml | 3 --- config/routes/admin.rb | 1 - 5 files changed, 1 insertion(+), 20 deletions(-) diff --git a/app/controllers/admin/poll/polls_controller.rb b/app/controllers/admin/poll/polls_controller.rb index 5e9c84c90..71156ecf5 100644 --- a/app/controllers/admin/poll/polls_controller.rb +++ b/app/controllers/admin/poll/polls_controller.rb @@ -42,18 +42,6 @@ class Admin::Poll::PollsController < Admin::Poll::BaseController end end - def add_question - question = ::Poll::Question.find(params[:question_id]) - - if question.present? - @poll.questions << question - notice = t("admin.polls.flash.question_added") - else - notice = t("admin.polls.flash.error_on_question_added") - end - redirect_to admin_poll_path(@poll), notice: notice - end - def booth_assignments @polls = Poll.current.created_by_admin end diff --git a/app/models/abilities/administrator.rb b/app/models/abilities/administrator.rb index 44201bdf4..9647f970f 100644 --- a/app/models/abilities/administrator.rb +++ b/app/models/abilities/administrator.rb @@ -83,7 +83,7 @@ module Abilities can [:index, :create, :update, :destroy], Geozone - can [:read, :create, :update, :destroy, :add_question, :booth_assignments], Poll + can [:read, :create, :update, :destroy, :booth_assignments], Poll can [:read, :create, :update, :destroy, :available], Poll::Booth can [:search, :create, :index, :destroy], ::Poll::Officer can [:create, :destroy, :manage], ::Poll::BoothAssignment diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index f615458c3..5e3e52896 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1103,9 +1103,6 @@ en: table_title: "Title" edit_answers: Edit answers see_proposal: "(See proposal)" - flash: - question_added: "Question added to this poll" - error_on_question_added: "Question could not be assigned to this poll" destroy: alert: "This action will remove the poll and all its associated questions." success_notice: "Poll deleted successfully" diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index aa861ab09..d50c807da 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1102,9 +1102,6 @@ es: table_title: "Título" edit_answers: Editar respuestas see_proposal: "(Ver propuesta)" - flash: - question_added: "Pregunta añadida a esta votación" - error_on_question_added: "No se pudo asignar la pregunta" destroy: alert: "Esta acción eliminará la votación y todas sus preguntas asociadas." success_notice: "Votación eliminada correctamente" diff --git a/config/routes/admin.rb b/config/routes/admin.rb index bae512e69..c3ee8f142 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -141,7 +141,6 @@ namespace :admin do scope module: :poll do resources :polls do get :booth_assignments, on: :collection - patch :add_question, on: :member resources :booth_assignments, only: [:index, :show, :create, :destroy] do get :search_booths, on: :collection From cc4b22ee3713187fed799653d5e77f508da9f70e Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 7 Sep 2022 14:35:43 +0200 Subject: [PATCH 03/19] Use resource from load_and_authorize_resource in admin polls controller --- app/controllers/admin/poll/polls_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/poll/polls_controller.rb b/app/controllers/admin/poll/polls_controller.rb index 71156ecf5..e42db27db 100644 --- a/app/controllers/admin/poll/polls_controller.rb +++ b/app/controllers/admin/poll/polls_controller.rb @@ -7,18 +7,18 @@ class Admin::Poll::PollsController < Admin::Poll::BaseController before_action :load_geozones, only: [:new, :create, :edit, :update] def index - @polls = Poll.not_budget.created_by_admin.order(starts_at: :desc) + @polls = @polls.not_budget.created_by_admin.order(starts_at: :desc) end def show - @poll = Poll.find(params[:id]) end def new end def create - @poll = Poll.new(poll_params.merge(author: current_user)) + @poll.author = current_user + if @poll.save notice = t("flash.actions.create.poll") if @poll.budget.present? From ad9362399eab35dcd34998038fc6fe8df5733334 Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 8 Sep 2022 16:40:08 +0200 Subject: [PATCH 04/19] After destroy question redirect to his poll show page After removing a question from a poll it makes more sense to redirect to your own poll show page in order to manage their questions. Currently it is redirecting to the questions index page where all the questions from all the polls are displayed and takes you completely out of the context of the poll you are in. In the future we will remove this index question page. --- app/controllers/admin/poll/questions_controller.rb | 2 +- spec/system/admin/poll/questions_spec.rb | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/poll/questions_controller.rb b/app/controllers/admin/poll/questions_controller.rb index bb07a5ac2..85673b936 100644 --- a/app/controllers/admin/poll/questions_controller.rb +++ b/app/controllers/admin/poll/questions_controller.rb @@ -48,7 +48,7 @@ class Admin::Poll::QuestionsController < Admin::Poll::BaseController else notice = t("flash.actions.destroy.error") end - redirect_to admin_questions_path, notice: notice + redirect_to admin_poll_path(@question.poll), notice: notice end private diff --git a/spec/system/admin/poll/questions_spec.rb b/spec/system/admin/poll/questions_spec.rb index f7062ea67..bbf0eb77e 100644 --- a/spec/system/admin/poll/questions_spec.rb +++ b/spec/system/admin/poll/questions_spec.rb @@ -147,8 +147,9 @@ describe "Admin poll questions", :admin do end end - expect(page).not_to have_content(question1.title) - expect(page).to have_content(question2.title) + expect(page).not_to have_content question1.title + expect(page).to have_content question2.title + expect(page).to have_current_path admin_poll_path(poll) end context "Poll select box" do From 8b4cd13675287855474f46981b234cf4db3ba103 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 9 Sep 2022 11:52:50 +0200 Subject: [PATCH 05/19] Unify with the rest of application destroy method in questions controller We also add a missing translation. --- app/controllers/admin/poll/questions_controller.rb | 8 ++------ config/locales/en/admin.yml | 2 ++ config/locales/es/admin.yml | 2 ++ 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/poll/questions_controller.rb b/app/controllers/admin/poll/questions_controller.rb index 85673b936..0bef8fb0b 100644 --- a/app/controllers/admin/poll/questions_controller.rb +++ b/app/controllers/admin/poll/questions_controller.rb @@ -43,12 +43,8 @@ class Admin::Poll::QuestionsController < Admin::Poll::BaseController end def destroy - if @question.destroy - notice = "Question destroyed succesfully" - else - notice = t("flash.actions.destroy.error") - end - redirect_to admin_poll_path(@question.poll), notice: notice + @question.destroy! + redirect_to admin_poll_path(@question.poll), notice: t("admin.questions.destroy.notice") end private diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 5e3e52896..d0fe1c394 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1126,6 +1126,8 @@ en: new: title: "Create question to poll %{poll}" title_proposal: "Create question" + destroy: + notice: "Question deleted succesfully" answers: images: add_image: "Add image" diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index d50c807da..47399d97d 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1125,6 +1125,8 @@ es: new: title: "Crear pregunta ciudadana para la votación %{poll}" title_proposal: "Crear pregunta ciudadana" + destroy: + notice: "Pregunta eliminada correctamente" answers: images: add_image: "Añadir imagen" From 01005b50cbd644c1c5f8e43c0f3923210cf9db4f Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 9 Sep 2022 10:57:53 +0200 Subject: [PATCH 06/19] Load question from load_and_authorize_resource in answers controller --- app/controllers/admin/poll/questions/answers_controller.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/poll/questions/answers_controller.rb b/app/controllers/admin/poll/questions/answers_controller.rb index 5e224a514..5713ea99a 100644 --- a/app/controllers/admin/poll/questions/answers_controller.rb +++ b/app/controllers/admin/poll/questions/answers_controller.rb @@ -7,12 +7,11 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController load_and_authorize_resource :question, class: "::Poll::Question" def new - @answer = ::Poll::Question::Answer.new + @answer = @question.answers.new end def create - @answer = ::Poll::Question::Answer.new(answer_params) - @question = @answer.question + @answer = @question.answers.new(answer_params) if @answer.save redirect_to admin_question_path(@question), @@ -30,7 +29,7 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController def update if @answer.update(answer_params) - redirect_to admin_question_path(@answer.question), + redirect_to admin_question_path(@question), notice: t("flash.actions.save_changes.notice") else render :edit From 405b37f6054f6fefe086622717c9150b80da5934 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 9 Sep 2022 15:56:36 +0200 Subject: [PATCH 07/19] Load answer through question in answers controller We are simplifying the load answer and we can remove the ambiguous hidden field from answer form. --- .../admin/poll/questions/answers_controller.rb | 17 ++++++----------- .../admin/poll/questions/answers/_form.html.erb | 2 -- .../poll/questions/answers/documents.html.erb | 2 +- .../admin/poll/questions/answers/edit.html.erb | 2 +- .../admin/poll/questions/answers/show.html.erb | 2 +- app/views/admin/poll/questions/show.html.erb | 2 +- config/routes/admin.rb | 3 ++- .../poll/questions/answers/answers_spec.rb | 2 +- spec/system/admin/translatable_spec.rb | 2 +- 9 files changed, 14 insertions(+), 20 deletions(-) diff --git a/app/controllers/admin/poll/questions/answers_controller.rb b/app/controllers/admin/poll/questions/answers_controller.rb index 5713ea99a..1d4dd4982 100644 --- a/app/controllers/admin/poll/questions/answers_controller.rb +++ b/app/controllers/admin/poll/questions/answers_controller.rb @@ -2,17 +2,16 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController include Translatable include DocumentAttributes - before_action :load_answer, only: [:show, :edit, :update, :documents] - load_and_authorize_resource :question, class: "::Poll::Question" + load_resource class: "::Poll::Question::Answer", + through: :question, + through_association: :question_answers, + except: :documents def new - @answer = @question.answers.new end def create - @answer = @question.answers.new(answer_params) - if @answer.save redirect_to admin_question_path(@question), notice: t("flash.actions.create.poll_question_answer") @@ -37,6 +36,7 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController end def documents + @answer = ::Poll::Question::Answer.find(params[:answer_id]) @documents = @answer.documents render "admin/poll/questions/answers/documents" @@ -54,16 +54,11 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController end def allowed_params - attributes = [:title, :description, :given_order, :question_id, - documents_attributes: document_attributes] + attributes = [:title, :description, :given_order, documents_attributes: document_attributes] [*attributes, translation_params(Poll::Question::Answer)] end - def load_answer - @answer = ::Poll::Question::Answer.find(params[:id] || params[:answer_id]) - end - def resource load_answer unless @answer @answer diff --git a/app/views/admin/poll/questions/answers/_form.html.erb b/app/views/admin/poll/questions/answers/_form.html.erb index 53eceae17..ef32b93e5 100644 --- a/app/views/admin/poll/questions/answers/_form.html.erb +++ b/app/views/admin/poll/questions/answers/_form.html.erb @@ -7,8 +7,6 @@ <%= f.hidden_field :given_order, value: @answer.persisted? ? @answer.given_order : @answer.class.last_position(@answer.question_id || @question.id) + 1 %> - <%= f.hidden_field :question_id, value: @answer.question_id || @question.id %> -
<%= f.translatable_fields do |translations_form| %>
diff --git a/app/views/admin/poll/questions/answers/documents.html.erb b/app/views/admin/poll/questions/answers/documents.html.erb index 4c0396849..7a94fd8e6 100644 --- a/app/views/admin/poll/questions/answers/documents.html.erb +++ b/app/views/admin/poll/questions/answers/documents.html.erb @@ -9,7 +9,7 @@
<%= form_for(Poll::Question::Answer.new, - url: admin_answer_path(@answer), + url: admin_question_answer_path(@answer.question, @answer), method: :put) do |f| %> <%= render "shared/errors", resource: @answer %> diff --git a/app/views/admin/poll/questions/answers/edit.html.erb b/app/views/admin/poll/questions/answers/edit.html.erb index 46a8784cc..def21279d 100644 --- a/app/views/admin/poll/questions/answers/edit.html.erb +++ b/app/views/admin/poll/questions/answers/edit.html.erb @@ -10,5 +10,5 @@
- <%= render "form", form_url: admin_answer_path(@answer) %> + <%= render "form", form_url: admin_question_answer_path(@question, @answer) %>
diff --git a/app/views/admin/poll/questions/answers/show.html.erb b/app/views/admin/poll/questions/answers/show.html.erb index 8244687b2..00bc72b02 100644 --- a/app/views/admin/poll/questions/answers/show.html.erb +++ b/app/views/admin/poll/questions/answers/show.html.erb @@ -1,6 +1,6 @@ <%= back_link_to %> -<%= link_to t("admin.answers.show.edit"), edit_admin_answer_path(@answer), +<%= link_to t("admin.answers.show.edit"), edit_admin_question_answer_path(@answer.question, @answer), class: "button hollow float-right" %>
    diff --git a/app/views/admin/poll/questions/show.html.erb b/app/views/admin/poll/questions/show.html.erb index 4072a0614..0c45f0234 100644 --- a/app/views/admin/poll/questions/show.html.erb +++ b/app/views/admin/poll/questions/show.html.erb @@ -52,7 +52,7 @@ <% @question.question_answers.each do |answer| %> - <%= link_to answer.title, admin_answer_path(answer) %> + <%= link_to answer.title, admin_question_answer_path(@question, answer) %> <%= wysiwyg(answer.description) %> (<%= answer.images.count %>) diff --git a/config/routes/admin.rb b/config/routes/admin.rb index c3ee8f142..dde11dbc9 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -169,7 +169,8 @@ namespace :admin do end resources :questions, shallow: true do - resources :answers, except: [:index, :destroy], controller: "questions/answers" do + resources :answers, except: [:index, :destroy], controller: "questions/answers", shallow: false + resources :answers, only: [], controller: "questions/answers" do resources :images, controller: "questions/answers/images" resources :videos, controller: "questions/answers/videos" get :documents, to: "questions/answers#documents" diff --git a/spec/system/admin/poll/questions/answers/answers_spec.rb b/spec/system/admin/poll/questions/answers/answers_spec.rb index 262ed7d7f..76d936fc8 100644 --- a/spec/system/admin/poll/questions/answers/answers_spec.rb +++ b/spec/system/admin/poll/questions/answers/answers_spec.rb @@ -36,7 +36,7 @@ describe "Answers", :admin do answer = create(:poll_question_answer, question: question, title: "Answer title", given_order: 2) create(:poll_question_answer, question: question, title: "Another title", given_order: 1) - visit admin_answer_path(answer) + visit admin_question_answer_path(question, answer) click_link "Edit answer" diff --git a/spec/system/admin/translatable_spec.rb b/spec/system/admin/translatable_spec.rb index fe9d80a8d..09602ae16 100644 --- a/spec/system/admin/translatable_spec.rb +++ b/spec/system/admin/translatable_spec.rb @@ -216,7 +216,7 @@ describe "Admin edit translatable records", :admin do context "CKEditor fields" do let(:translatable) { create(:poll_question_answer) } - let(:path) { edit_admin_answer_path(translatable) } + let(:path) { edit_admin_question_answer_path(translatable.question, translatable) } scenario "Changes the existing translation" do visit path From cb2958e1b017dfdd42e13bf24e2a51aa77e45515 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 9 Sep 2022 16:29:04 +0200 Subject: [PATCH 08/19] Add load_and_authorize_resource to answers controller --- .../admin/poll/questions/answers_controller.rb | 8 ++++---- app/models/abilities/administrator.rb | 1 + spec/models/abilities/administrator_spec.rb | 2 ++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/poll/questions/answers_controller.rb b/app/controllers/admin/poll/questions/answers_controller.rb index 1d4dd4982..77e48e46e 100644 --- a/app/controllers/admin/poll/questions/answers_controller.rb +++ b/app/controllers/admin/poll/questions/answers_controller.rb @@ -3,10 +3,10 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController include DocumentAttributes load_and_authorize_resource :question, class: "::Poll::Question" - load_resource class: "::Poll::Question::Answer", - through: :question, - through_association: :question_answers, - except: :documents + load_and_authorize_resource class: "::Poll::Question::Answer", + through: :question, + through_association: :question_answers, + except: :documents def new end diff --git a/app/models/abilities/administrator.rb b/app/models/abilities/administrator.rb index 9647f970f..4fb5474cf 100644 --- a/app/models/abilities/administrator.rb +++ b/app/models/abilities/administrator.rb @@ -90,6 +90,7 @@ module Abilities can [:create, :destroy], ::Poll::OfficerAssignment can [:read, :create, :update], Poll::Question can :destroy, Poll::Question + can :manage, Poll::Question::Answer can :manage, SiteCustomization::Page can :manage, SiteCustomization::Image diff --git a/spec/models/abilities/administrator_spec.rb b/spec/models/abilities/administrator_spec.rb index 424638a1b..71a12ad71 100644 --- a/spec/models/abilities/administrator_spec.rb +++ b/spec/models/abilities/administrator_spec.rb @@ -114,6 +114,8 @@ describe Abilities::Administrator do it { should be_able_to(:create, Poll::Question) } it { should be_able_to(:update, Poll::Question) } + it { should be_able_to(:manage, Poll::Question::Answer) } + it { is_expected.to be_able_to :manage, Dashboard::AdministratorTask } it { is_expected.to be_able_to :manage, dashboard_administrator_task } From ec861ca8e67e3a257635a0280acea86e9f043994 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 14 Sep 2022 09:25:17 +0200 Subject: [PATCH 09/19] Extract documents action from Answer controller This way we have a controller just to manage Poll::Question::Answer related documents in the same way we have for videos and images. --- .../documents/index_component.html.erb} | 15 ++++------ .../answers/documents/index_component.rb | 7 +++++ .../questions/answers/documents_controller.rb | 29 +++++++++++++++++++ .../poll/questions/answers_controller.rb | 13 ++------- .../answers/documents/index.html.erb | 1 + config/routes/admin.rb | 2 +- .../answers/documents/documents_spec.rb | 12 ++++++++ 7 files changed, 58 insertions(+), 21 deletions(-) rename app/{views/admin/poll/questions/answers/documents.html.erb => components/admin/poll/questions/answers/documents/index_component.html.erb} (77%) create mode 100644 app/components/admin/poll/questions/answers/documents/index_component.rb create mode 100644 app/controllers/admin/poll/questions/answers/documents_controller.rb create mode 100644 app/views/admin/poll/questions/answers/documents/index.html.erb diff --git a/app/views/admin/poll/questions/answers/documents.html.erb b/app/components/admin/poll/questions/answers/documents/index_component.html.erb similarity index 77% rename from app/views/admin/poll/questions/answers/documents.html.erb rename to app/components/admin/poll/questions/answers/documents/index_component.html.erb index 7a94fd8e6..729d718e3 100644 --- a/app/views/admin/poll/questions/answers/documents.html.erb +++ b/app/components/admin/poll/questions/answers/documents/index_component.html.erb @@ -3,16 +3,13 @@

    <%= t("admin.questions.show.answers.documents_list") %>

      -
    • <%= @answer.question.title %>
      • -
    • <%= @answer.title %>
      • +
    • <%= answer.question.title %>
      • +
    • <%= answer.title %>
    - <%= form_for(Poll::Question::Answer.new, - url: admin_question_answer_path(@answer.question, @answer), - method: :put) do |f| %> - - <%= render "shared/errors", resource: @answer %> + <%= form_for(Poll::Question::Answer.new, url: admin_answer_documents_path(answer)) do |f| %> + <%= render "shared/errors", resource: answer %>
    <%= render "documents/nested_documents", f: f %> @@ -23,14 +20,14 @@
    <% end %> - <% if @answer.documents.present? %> + <% if answer.documents.present? %> - <% @answer.documents.each do |document| %> + <% answer.documents.each do |document| %> + <% @question.question_answers.each do |answer| %> - + + <% end %> diff --git a/config/initializers/routes_hierarchy.rb b/config/initializers/routes_hierarchy.rb index 7fda048ff..e66c8386c 100644 --- a/config/initializers/routes_hierarchy.rb +++ b/config/initializers/routes_hierarchy.rb @@ -25,7 +25,7 @@ module ActionDispatch::Routing::UrlFor def namespaced_polymorphic_path(namespace, resource, options = {}) if %w[Budget::Group Budget::Heading Poll::Booth Poll::BoothAssignment Poll::Officer - Poll::Question Poll::Question::Answer::Video Poll::Shift + Poll::Question Poll::Question::Answer Poll::Question::Answer::Video Poll::Shift SDG::LocalTarget].include?(resource.class.name) resolve = resolve_for(resource) resolve_options = resolve.pop diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index d0fe1c394..cfc5bf653 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1154,12 +1154,6 @@ en: answers: new: title: New answer - show: - title: Title - description: Description - images: Images - images_list: Images list - edit: Edit answer edit: title: Edit answer videos: diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index 47399d97d..8e997a466 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1153,12 +1153,6 @@ es: answers: new: title: Nueva respuesta - show: - title: Título - description: Descripción - images: Imágenes - images_list: Lista de imágenes - edit: Editar respuesta edit: title: Editar respuesta videos: diff --git a/config/routes/admin.rb b/config/routes/admin.rb index f61ab41a8..f3471c58c 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -169,7 +169,7 @@ namespace :admin do end resources :questions, shallow: true do - resources :answers, except: [:index, :destroy], controller: "questions/answers", shallow: false + resources :answers, except: [:index, :show, :destroy], controller: "questions/answers", shallow: false resources :answers, only: [], controller: "questions/answers" do resources :images, controller: "questions/answers/images" resources :videos, controller: "questions/answers/videos" @@ -323,6 +323,10 @@ resolve "Poll::Officer" do |officer, options| [:officer, options.merge(id: officer)] end +resolve "Poll::Question::Answer" do |answer, options| + [:question, :answer, options.merge(question_id: answer.question, id: answer)] +end + resolve "Poll::Question::Answer::Video" do |video, options| [:video, options.merge(id: video)] end diff --git a/spec/system/admin/poll/questions/answers/answers_spec.rb b/spec/system/admin/poll/questions/answers/answers_spec.rb index 76d936fc8..1b9722e69 100644 --- a/spec/system/admin/poll/questions/answers/answers_spec.rb +++ b/spec/system/admin/poll/questions/answers/answers_spec.rb @@ -33,15 +33,13 @@ describe "Answers", :admin do scenario "Update" do question = create(:poll_question) - answer = create(:poll_question_answer, question: question, title: "Answer title", given_order: 2) + create(:poll_question_answer, question: question, title: "Answer title", given_order: 2) create(:poll_question_answer, question: question, title: "Another title", given_order: 1) - visit admin_question_answer_path(question, answer) - - click_link "Edit answer" + visit admin_question_path(question) + within("tr", text: "Answer title") { click_link "Edit" } fill_in "Answer", with: "New title" - click_button "Save" expect(page).to have_content "Changes saved" From 3d1a9501aac4c55290593da900089a882c50e02f Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 9 Sep 2022 10:55:33 +0200 Subject: [PATCH 12/19] Remove unused resource controller methods These methods aren't necessary since commit 71601bd3f. --- app/controllers/admin/poll/polls_controller.rb | 4 ---- app/controllers/admin/poll/questions/answers_controller.rb | 5 ----- app/controllers/admin/poll/questions_controller.rb | 4 ---- 3 files changed, 13 deletions(-) diff --git a/app/controllers/admin/poll/polls_controller.rb b/app/controllers/admin/poll/polls_controller.rb index e42db27db..68ba02a01 100644 --- a/app/controllers/admin/poll/polls_controller.rb +++ b/app/controllers/admin/poll/polls_controller.rb @@ -72,8 +72,4 @@ class Admin::Poll::PollsController < Admin::Poll::BaseController [*attributes, *report_attributes, translation_params(Poll)] end - - def resource - @poll ||= Poll.find(params[:id]) - end end diff --git a/app/controllers/admin/poll/questions/answers_controller.rb b/app/controllers/admin/poll/questions/answers_controller.rb index 6d6b8a759..c015f1ebe 100644 --- a/app/controllers/admin/poll/questions/answers_controller.rb +++ b/app/controllers/admin/poll/questions/answers_controller.rb @@ -46,9 +46,4 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController [*attributes, translation_params(Poll::Question::Answer)] end - - def resource - load_answer unless @answer - @answer - end end diff --git a/app/controllers/admin/poll/questions_controller.rb b/app/controllers/admin/poll/questions_controller.rb index 0bef8fb0b..598e664e7 100644 --- a/app/controllers/admin/poll/questions_controller.rb +++ b/app/controllers/admin/poll/questions_controller.rb @@ -62,8 +62,4 @@ class Admin::Poll::QuestionsController < Admin::Poll::BaseController def search_params params.permit(:poll_id, :search) end - - def resource - @poll_question ||= Poll::Question.find(params[:id]) - end end From 1ec793863b1c5fd4687dcec68ca42d305de2e45e Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 9 Sep 2022 12:54:03 +0200 Subject: [PATCH 13/19] Add missing system specs for poll question answer videos Minor change for replace @video.answer_id to @video.answer. --- .../questions/answers/videos_controller.rb | 3 +- .../questions/answers/videos/index.html.erb | 2 +- .../questions/answers/videos/videos_spec.rb | 47 +++++++++++++++---- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/app/controllers/admin/poll/questions/answers/videos_controller.rb b/app/controllers/admin/poll/questions/answers/videos_controller.rb index 2df3ea80c..0fb13ee46 100644 --- a/app/controllers/admin/poll/questions/answers/videos_controller.rb +++ b/app/controllers/admin/poll/questions/answers/videos_controller.rb @@ -25,8 +25,7 @@ class Admin::Poll::Questions::Answers::VideosController < Admin::Poll::BaseContr def update if @video.update(video_params) - redirect_to admin_answer_videos_path(@video.answer_id), - notice: t("flash.actions.save_changes.notice") + redirect_to admin_answer_videos_path(@video.answer), notice: t("flash.actions.save_changes.notice") else render :edit end diff --git a/app/views/admin/poll/questions/answers/videos/index.html.erb b/app/views/admin/poll/questions/answers/videos/index.html.erb index 2889db68a..cedfe1b4d 100644 --- a/app/views/admin/poll/questions/answers/videos/index.html.erb +++ b/app/views/admin/poll/questions/answers/videos/index.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to admin_question_path(@answer.question_id) %> +<%= back_link_to admin_question_path(@answer.question) %>
    diff --git a/spec/system/admin/poll/questions/answers/videos/videos_spec.rb b/spec/system/admin/poll/questions/answers/videos/videos_spec.rb index d3ced3080..1488b4a73 100644 --- a/spec/system/admin/poll/questions/answers/videos/videos_spec.rb +++ b/spec/system/admin/poll/questions/answers/videos/videos_spec.rb @@ -1,26 +1,53 @@ require "rails_helper" describe "Videos", :admin do - scenario "Create" do - question = create(:poll_question) - answer = create(:poll_question_answer, question: question) - video_title = "'Magical' by Junko Ohashi" - video_url = "https://www.youtube.com/watch?v=-JMf43st-1A" + let!(:question) { create(:poll_question) } + let!(:answer) { create(:poll_question_answer, question: question) } + let(:title) { "'Magical' by Junko Ohashi" } + let(:url) { "https://www.youtube.com/watch?v=-JMf43st-1A" } + scenario "Create" do visit admin_question_path(question) within("#poll_question_answer_#{answer.id}") do click_link "Video list" end - click_link "Add video" - fill_in "poll_question_answer_video_title", with: video_title - fill_in "poll_question_answer_video_url", with: video_url + fill_in "Title", with: title + fill_in "External video", with: url click_button "Save" - expect(page).to have_content(video_title) - expect(page).to have_content(video_url) + expect(page).to have_content title + expect(page).to have_content url + end + + scenario "Update" do + video = create(:poll_answer_video, answer: answer) + + visit edit_admin_video_path(video) + + fill_in "Title", with: title + fill_in "External video", with: url + + click_button "Save" + + expect(page).to have_content title + expect(page).to have_content url + end + + scenario "Destroy" do + video = create(:poll_answer_video, answer: answer) + + visit admin_answer_videos_path(answer) + + within("#poll_question_answer_video_#{video.id}") do + accept_confirm("Are you sure? This action will delete \"#{video.title}\" and can't be undone.") do + click_button "Delete" + end + end + + expect(page).to have_content "Answer video deleted successfully." end end From 919534fbc518f1050e2c6f63c4558bed7385b959 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 9 Sep 2022 12:54:42 +0200 Subject: [PATCH 14/19] Unify with the rest of application destroy method in videos controller --- .../admin/poll/questions/answers/videos_controller.rb | 9 +++------ config/locales/en/responders.yml | 1 - config/locales/es/responders.yml | 1 - 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/app/controllers/admin/poll/questions/answers/videos_controller.rb b/app/controllers/admin/poll/questions/answers/videos_controller.rb index 0fb13ee46..1abbdb625 100644 --- a/app/controllers/admin/poll/questions/answers/videos_controller.rb +++ b/app/controllers/admin/poll/questions/answers/videos_controller.rb @@ -32,12 +32,9 @@ class Admin::Poll::Questions::Answers::VideosController < Admin::Poll::BaseContr end def destroy - notice = if @video.destroy - t("flash.actions.destroy.poll_question_answer_video") - else - t("flash.actions.destroy.error") - end - redirect_back(fallback_location: (request.referer || root_path), notice: notice) + @video.destroy! + notice = t("flash.actions.destroy.poll_question_answer_video") + redirect_to admin_answer_videos_path(@video.answer), notice: notice end private diff --git a/config/locales/en/responders.yml b/config/locales/en/responders.yml index 7fb544b97..371cbaf71 100644 --- a/config/locales/en/responders.yml +++ b/config/locales/en/responders.yml @@ -32,7 +32,6 @@ en: translation: "Translation updated successfully" destroy: budget_investment: "Investment project deleted succesfully." - error: "Could not delete" topic: "Topic deleted successfully." poll_question_answer_video: "Answer video deleted successfully." valuator_group: "Valuator group deleted successfully" diff --git a/config/locales/es/responders.yml b/config/locales/es/responders.yml index 6092cf9ff..4a0b7d415 100644 --- a/config/locales/es/responders.yml +++ b/config/locales/es/responders.yml @@ -32,7 +32,6 @@ es: translation: "Traducción actualizada correctamente" destroy: budget_investment: "Proyecto de gasto eliminado." - error: "No se pudo borrar" topic: "Tema eliminado." poll_question_answer_video: "Vídeo de respuesta eliminado." valuator_group: "Grupo de evaluadores eliminado correctamente" From b17b03099ccbb7b77931c6de4312f9d4e6e99980 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 9 Sep 2022 14:27:29 +0200 Subject: [PATCH 15/19] Load videos through answer in all actions --- .../questions/answers/videos_controller.rb | 21 +++++-------------- .../questions/answers/videos/_form.html.erb | 2 -- .../questions/answers/videos/edit.html.erb | 2 +- .../questions/answers/videos/index.html.erb | 4 ++-- .../questions/answers/videos/new.html.erb | 2 +- config/routes/admin.rb | 4 ++-- spec/routing/polymorphic_routes_spec.rb | 2 +- .../questions/answers/videos/videos_spec.rb | 2 +- 8 files changed, 13 insertions(+), 26 deletions(-) diff --git a/app/controllers/admin/poll/questions/answers/videos_controller.rb b/app/controllers/admin/poll/questions/answers/videos_controller.rb index 1abbdb625..e4191e0dd 100644 --- a/app/controllers/admin/poll/questions/answers/videos_controller.rb +++ b/app/controllers/admin/poll/questions/answers/videos_controller.rb @@ -1,17 +1,14 @@ class Admin::Poll::Questions::Answers::VideosController < Admin::Poll::BaseController - before_action :load_answer, only: [:index, :new, :create] - before_action :load_video, only: [:edit, :update, :destroy] + load_resource :answer, class: "::Poll::Question::Answer" + load_resource class: "::Poll::Question::Answer::Video", through: :answer def index end def new - @video = ::Poll::Question::Answer::Video.new end def create - @video = ::Poll::Question::Answer::Video.new(video_params) - if @video.save redirect_to admin_answer_videos_path(@answer), notice: t("flash.actions.create.poll_question_answer_video") @@ -25,7 +22,7 @@ class Admin::Poll::Questions::Answers::VideosController < Admin::Poll::BaseContr def update if @video.update(video_params) - redirect_to admin_answer_videos_path(@video.answer), notice: t("flash.actions.save_changes.notice") + redirect_to admin_answer_videos_path(@answer), notice: t("flash.actions.save_changes.notice") else render :edit end @@ -34,7 +31,7 @@ class Admin::Poll::Questions::Answers::VideosController < Admin::Poll::BaseContr def destroy @video.destroy! notice = t("flash.actions.destroy.poll_question_answer_video") - redirect_to admin_answer_videos_path(@video.answer), notice: notice + redirect_to admin_answer_videos_path(@answer), notice: notice end private @@ -44,14 +41,6 @@ class Admin::Poll::Questions::Answers::VideosController < Admin::Poll::BaseContr end def allowed_params - [:title, :url, :answer_id] - end - - def load_answer - @answer = ::Poll::Question::Answer.find(params[:answer_id]) - end - - def load_video - @video = ::Poll::Question::Answer::Video.find(params[:id]) + [:title, :url] end end diff --git a/app/views/admin/poll/questions/answers/videos/_form.html.erb b/app/views/admin/poll/questions/answers/videos/_form.html.erb index 0381706e0..57d2757d5 100644 --- a/app/views/admin/poll/questions/answers/videos/_form.html.erb +++ b/app/views/admin/poll/questions/answers/videos/_form.html.erb @@ -2,8 +2,6 @@ <%= render "shared/errors", resource: @video %> - <%= f.hidden_field :answer_id, value: @video.answer_id || @answer.id %> -
    diff --git a/app/views/admin/poll/questions/answers/videos/edit.html.erb b/app/views/admin/poll/questions/answers/videos/edit.html.erb index 31b59c2dd..9e6f06a13 100644 --- a/app/views/admin/poll/questions/answers/videos/edit.html.erb +++ b/app/views/admin/poll/questions/answers/videos/edit.html.erb @@ -5,5 +5,5 @@
    - <%= render "form", form_url: admin_video_path(@video) %> + <%= render "form", form_url: admin_answer_video_path(@answer, @video) %>
    diff --git a/app/views/admin/poll/questions/answers/videos/index.html.erb b/app/views/admin/poll/questions/answers/videos/index.html.erb index cedfe1b4d..bd436d88a 100644 --- a/app/views/admin/poll/questions/answers/videos/index.html.erb +++ b/app/views/admin/poll/questions/answers/videos/index.html.erb @@ -7,7 +7,7 @@ <%= link_to t("admin.answers.videos.index.add_video"), - new_admin_answer_video_path, + new_admin_answer_video_path(@answer), class: "button success float-right" %>
    @@ -22,7 +22,7 @@
    - <% @answer.videos.each do |video| %> + <% @videos.each do |video| %> diff --git a/app/views/admin/poll/questions/answers/videos/new.html.erb b/app/views/admin/poll/questions/answers/videos/new.html.erb index 564817e3f..9552916ea 100644 --- a/app/views/admin/poll/questions/answers/videos/new.html.erb +++ b/app/views/admin/poll/questions/answers/videos/new.html.erb @@ -5,5 +5,5 @@
    - <%= render "form", form_url: admin_answer_videos_path %> + <%= render "form", form_url: admin_answer_videos_path(@answer) %>
    diff --git a/config/routes/admin.rb b/config/routes/admin.rb index f3471c58c..84d3e6b68 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -172,7 +172,7 @@ namespace :admin do resources :answers, except: [:index, :show, :destroy], controller: "questions/answers", shallow: false resources :answers, only: [], controller: "questions/answers" do resources :images, controller: "questions/answers/images" - resources :videos, controller: "questions/answers/videos" + resources :videos, controller: "questions/answers/videos", shallow: false resources :documents, only: [:index, :create], controller: "questions/answers/documents" end post "/answers/order_answers", to: "questions/answers#order_answers" @@ -328,5 +328,5 @@ resolve "Poll::Question::Answer" do |answer, options| end resolve "Poll::Question::Answer::Video" do |video, options| - [:video, options.merge(id: video)] + [:answer, :video, options.merge(answer_id: video.answer, id: video)] end diff --git a/spec/routing/polymorphic_routes_spec.rb b/spec/routing/polymorphic_routes_spec.rb index 814e67b01..5ff9bef09 100644 --- a/spec/routing/polymorphic_routes_spec.rb +++ b/spec/routing/polymorphic_routes_spec.rb @@ -97,7 +97,7 @@ describe "Polymorphic routes" do it "routes poll answer videos" do video = create(:poll_answer_video) - expect(admin_polymorphic_path(video)).to eq admin_video_path(video) + expect(admin_polymorphic_path(video)).to eq admin_answer_video_path(video.answer, video) end it "routes milestones for resources with no hierarchy" do diff --git a/spec/system/admin/poll/questions/answers/videos/videos_spec.rb b/spec/system/admin/poll/questions/answers/videos/videos_spec.rb index 1488b4a73..8735e2529 100644 --- a/spec/system/admin/poll/questions/answers/videos/videos_spec.rb +++ b/spec/system/admin/poll/questions/answers/videos/videos_spec.rb @@ -26,7 +26,7 @@ describe "Videos", :admin do scenario "Update" do video = create(:poll_answer_video, answer: answer) - visit edit_admin_video_path(video) + visit edit_admin_answer_video_path(answer, video) fill_in "Title", with: title fill_in "External video", with: url From 99495a21d2ed37067cf09626a2ac3873d9239ed5 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 9 Sep 2022 16:45:09 +0200 Subject: [PATCH 16/19] Add load_and_authorize_resource to videos controller --- .../admin/poll/questions/answers/videos_controller.rb | 4 ++-- app/models/abilities/administrator.rb | 1 + spec/models/abilities/administrator_spec.rb | 2 ++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/poll/questions/answers/videos_controller.rb b/app/controllers/admin/poll/questions/answers/videos_controller.rb index e4191e0dd..c198540b1 100644 --- a/app/controllers/admin/poll/questions/answers/videos_controller.rb +++ b/app/controllers/admin/poll/questions/answers/videos_controller.rb @@ -1,6 +1,6 @@ class Admin::Poll::Questions::Answers::VideosController < Admin::Poll::BaseController - load_resource :answer, class: "::Poll::Question::Answer" - load_resource class: "::Poll::Question::Answer::Video", through: :answer + load_and_authorize_resource :answer, class: "::Poll::Question::Answer" + load_and_authorize_resource class: "::Poll::Question::Answer::Video", through: :answer def index end diff --git a/app/models/abilities/administrator.rb b/app/models/abilities/administrator.rb index 4fb5474cf..57cef7552 100644 --- a/app/models/abilities/administrator.rb +++ b/app/models/abilities/administrator.rb @@ -91,6 +91,7 @@ module Abilities can [:read, :create, :update], Poll::Question can :destroy, Poll::Question can :manage, Poll::Question::Answer + can :manage, Poll::Question::Answer::Video can :manage, SiteCustomization::Page can :manage, SiteCustomization::Image diff --git a/spec/models/abilities/administrator_spec.rb b/spec/models/abilities/administrator_spec.rb index 71a12ad71..6ba8c79ff 100644 --- a/spec/models/abilities/administrator_spec.rb +++ b/spec/models/abilities/administrator_spec.rb @@ -116,6 +116,8 @@ describe Abilities::Administrator do it { should be_able_to(:manage, Poll::Question::Answer) } + it { should be_able_to(:manage, Poll::Question::Answer::Video) } + it { is_expected.to be_able_to :manage, Dashboard::AdministratorTask } it { is_expected.to be_able_to :manage, dashboard_administrator_task } From 83e0d4c6b779b1918a3e1b2dfe4594f48fabe63c Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 9 Sep 2022 17:14:55 +0200 Subject: [PATCH 17/19] Add load_and_authorize_resource to images controller --- .../admin/poll/questions/answers/images_controller.rb | 6 +----- app/models/abilities/administrator.rb | 3 +++ app/views/admin/poll/questions/answers/images/new.html.erb | 4 +--- spec/models/abilities/administrator_spec.rb | 5 +++++ 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/app/controllers/admin/poll/questions/answers/images_controller.rb b/app/controllers/admin/poll/questions/answers/images_controller.rb index f91389daa..a209b058f 100644 --- a/app/controllers/admin/poll/questions/answers/images_controller.rb +++ b/app/controllers/admin/poll/questions/answers/images_controller.rb @@ -1,7 +1,7 @@ class Admin::Poll::Questions::Answers::ImagesController < Admin::Poll::BaseController include ImageAttributes - before_action :load_answer, except: :destroy + load_and_authorize_resource :answer, class: "::Poll::Question::Answer" def index end @@ -38,8 +38,4 @@ class Admin::Poll::Questions::Answers::ImagesController < Admin::Poll::BaseContr def allowed_params [:answer_id, images_attributes: image_attributes] end - - def load_answer - @answer = ::Poll::Question::Answer.find(params[:answer_id]) - end end diff --git a/app/models/abilities/administrator.rb b/app/models/abilities/administrator.rb index 57cef7552..e9a455c3c 100644 --- a/app/models/abilities/administrator.rb +++ b/app/models/abilities/administrator.rb @@ -92,6 +92,9 @@ module Abilities can :destroy, Poll::Question can :manage, Poll::Question::Answer can :manage, Poll::Question::Answer::Video + can [:create, :destroy], Image do |image| + image.imageable_type == "Poll::Question::Answer" + end can :manage, SiteCustomization::Page can :manage, SiteCustomization::Image diff --git a/app/views/admin/poll/questions/answers/images/new.html.erb b/app/views/admin/poll/questions/answers/images/new.html.erb index 0ba27107b..e4ddaa769 100644 --- a/app/views/admin/poll/questions/answers/images/new.html.erb +++ b/app/views/admin/poll/questions/answers/images/new.html.erb @@ -1,7 +1,5 @@
    - <%= form_for(@answer, - url: admin_answer_images_path(@answer), - method: :post) do |f| %> + <%= form_for(@answer, url: admin_answer_images_path(@answer), method: :post) do |f| %> <%= render "shared/errors", resource: @answer %>
    diff --git a/spec/models/abilities/administrator_spec.rb b/spec/models/abilities/administrator_spec.rb index 6ba8c79ff..72e1722ee 100644 --- a/spec/models/abilities/administrator_spec.rb +++ b/spec/models/abilities/administrator_spec.rb @@ -18,6 +18,8 @@ describe Abilities::Administrator do let(:legislation_question) { create(:legislation_question) } let(:poll) { create(:poll) } let(:poll_question) { create(:poll_question) } + let(:poll_question_answer) { create(:poll_question_answer) } + let(:answer_image) { build(:image, imageable: poll_question_answer) } let(:past_process) { create(:legislation_process, :past) } let(:past_draft_process) { create(:legislation_process, :past, :not_published) } @@ -118,6 +120,9 @@ describe Abilities::Administrator do it { should be_able_to(:manage, Poll::Question::Answer::Video) } + it { should be_able_to(:create, answer_image) } + it { should be_able_to(:destroy, answer_image) } + it { is_expected.to be_able_to :manage, Dashboard::AdministratorTask } it { is_expected.to be_able_to :manage, dashboard_administrator_task } From 2d353ba893674ec0f5ddb5a59a8ca2552d111ba2 Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 12 Sep 2022 17:52:00 +0200 Subject: [PATCH 18/19] Remove unused routes related to documents These routes aren't used since commits (2993ef8707, 88a7a29d27) In parallel while these routes were being removed, the route file was being split into different files, and they were included again in the commit 1cd47da9d4. --- config/routes/document.rb | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/config/routes/document.rb b/config/routes/document.rb index 164153b50..03aca1e64 100644 --- a/config/routes/document.rb +++ b/config/routes/document.rb @@ -1,7 +1 @@ -resources :documents, only: [:new, :create, :destroy] do - collection do - get :new_nested - delete :destroy_upload - post :upload - end -end +resources :documents, only: [:destroy] From 86ff4eeff94e9a0512361323aa9460d5f1058ad3 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 14 Sep 2022 11:38:01 +0200 Subject: [PATCH 19/19] Add consistency to back_links In some cases (e.g. after editing or creating a resource with errors) the default back_link did not redirect to the expected page. Now we force the back_links to the index pages, so we always get the desired redirect. --- .../questions/answers/documents/index_component.html.erb | 2 +- app/views/admin/poll/polls/edit.html.erb | 2 +- app/views/admin/poll/polls/new.html.erb | 2 +- app/views/admin/poll/questions/answers/edit.html.erb | 2 +- app/views/admin/poll/questions/answers/new.html.erb | 2 +- app/views/admin/poll/questions/answers/videos/edit.html.erb | 2 +- app/views/admin/poll/questions/edit.html.erb | 2 +- app/views/admin/poll/questions/show.html.erb | 2 +- spec/system/admin/poll/polls_spec.rb | 2 ++ spec/system/admin/poll/questions/answers/answers_spec.rb | 4 ++++ .../poll/questions/answers/documents/documents_spec.rb | 1 + .../admin/poll/questions/answers/videos/videos_spec.rb | 2 ++ spec/system/admin/poll/questions_spec.rb | 6 ++++-- 13 files changed, 21 insertions(+), 10 deletions(-) diff --git a/app/components/admin/poll/questions/answers/documents/index_component.html.erb b/app/components/admin/poll/questions/answers/documents/index_component.html.erb index 729d718e3..9c6b95c20 100644 --- a/app/components/admin/poll/questions/answers/documents/index_component.html.erb +++ b/app/components/admin/poll/questions/answers/documents/index_component.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= back_link_to admin_question_path(@answer.question) %>

    <%= t("admin.questions.show.answers.documents_list") %>

    diff --git a/app/views/admin/poll/polls/edit.html.erb b/app/views/admin/poll/polls/edit.html.erb index f7bd93212..2fd150b35 100644 --- a/app/views/admin/poll/polls/edit.html.erb +++ b/app/views/admin/poll/polls/edit.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= back_link_to admin_polls_path %>

    <%= t("admin.polls.edit.title") %>

    diff --git a/app/views/admin/poll/polls/new.html.erb b/app/views/admin/poll/polls/new.html.erb index 528b7ff69..899de8223 100644 --- a/app/views/admin/poll/polls/new.html.erb +++ b/app/views/admin/poll/polls/new.html.erb @@ -1,6 +1,6 @@
    - <%= back_link_to %> + <%= back_link_to admin_polls_path %>

    <%= t("admin.polls.new.title") %>

    diff --git a/app/views/admin/poll/questions/answers/edit.html.erb b/app/views/admin/poll/questions/answers/edit.html.erb index def21279d..3abeef3b6 100644 --- a/app/views/admin/poll/questions/answers/edit.html.erb +++ b/app/views/admin/poll/questions/answers/edit.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= back_link_to admin_question_path(@question) %>
    • <%= @answer.title %>
      • diff --git a/app/views/admin/poll/questions/answers/new.html.erb b/app/views/admin/poll/questions/answers/new.html.erb index c3af00be6..863fdb56c 100644 --- a/app/views/admin/poll/questions/answers/new.html.erb +++ b/app/views/admin/poll/questions/answers/new.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= back_link_to admin_question_path(@question) %>
      • <%= @question.title %>
        • diff --git a/app/views/admin/poll/questions/answers/videos/edit.html.erb b/app/views/admin/poll/questions/answers/videos/edit.html.erb index 9e6f06a13..39d592ff5 100644 --- a/app/views/admin/poll/questions/answers/videos/edit.html.erb +++ b/app/views/admin/poll/questions/answers/videos/edit.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= back_link_to admin_answer_videos_path(@answer) %>

        <%= t("admin.answers.videos.edit.title") %> diff --git a/app/views/admin/poll/questions/edit.html.erb b/app/views/admin/poll/questions/edit.html.erb index 52c37badd..6c16a0ddf 100644 --- a/app/views/admin/poll/questions/edit.html.erb +++ b/app/views/admin/poll/questions/edit.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= back_link_to admin_poll_path(@question.poll) %>

        <%= t("admin.questions.edit.title") %>

        diff --git a/app/views/admin/poll/questions/show.html.erb b/app/views/admin/poll/questions/show.html.erb index f4e80e14d..954767747 100644 --- a/app/views/admin/poll/questions/show.html.erb +++ b/app/views/admin/poll/questions/show.html.erb @@ -1,4 +1,4 @@ -<%= back_link_to %> +<%= 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" %> diff --git a/spec/system/admin/poll/polls_spec.rb b/spec/system/admin/poll/polls_spec.rb index a677ca0cc..ede4bd5f3 100644 --- a/spec/system/admin/poll/polls_spec.rb +++ b/spec/system/admin/poll/polls_spec.rb @@ -61,6 +61,7 @@ describe "Admin polls", :admin do expect(page).not_to have_css("#poll_results_enabled") expect(page).not_to have_css("#poll_stats_enabled") + expect(page).to have_link "Go back", href: admin_polls_path click_button "Create poll" @@ -83,6 +84,7 @@ describe "Admin polls", :admin do end_date = 1.year.from_now.to_date expect(page).to have_css("img[alt='#{poll.image.title}']") + expect(page).to have_link "Go back", href: admin_polls_path fill_in "Name", with: "Next Poll" fill_in "poll_ends_at", with: end_date diff --git a/spec/system/admin/poll/questions/answers/answers_spec.rb b/spec/system/admin/poll/questions/answers/answers_spec.rb index 1b9722e69..3b12ce770 100644 --- a/spec/system/admin/poll/questions/answers/answers_spec.rb +++ b/spec/system/admin/poll/questions/answers/answers_spec.rb @@ -7,6 +7,8 @@ describe "Answers", :admin do visit admin_question_path(question) click_link "Add answer" + expect(page).to have_link "Go back", href: admin_question_path(question) + fill_in "Answer", with: "The answer is always 42" fill_in_ckeditor "Description", with: "The Hitchhiker's Guide To The Universe" @@ -39,6 +41,8 @@ describe "Answers", :admin do visit admin_question_path(question) within("tr", text: "Answer title") { click_link "Edit" } + expect(page).to have_link "Go back", href: admin_question_path(question) + fill_in "Answer", with: "New title" click_button "Save" diff --git a/spec/system/admin/poll/questions/answers/documents/documents_spec.rb b/spec/system/admin/poll/questions/answers/documents/documents_spec.rb index 5ab6f11c0..1ab90af53 100644 --- a/spec/system/admin/poll/questions/answers/documents/documents_spec.rb +++ b/spec/system/admin/poll/questions/answers/documents/documents_spec.rb @@ -9,6 +9,7 @@ describe "Documents", :admin do visit admin_answer_documents_path(answer) expect(page).not_to have_content(document.title) + expect(page).to have_link "Go back", href: admin_question_path(answer.question) end scenario "Answer with documents" do diff --git a/spec/system/admin/poll/questions/answers/videos/videos_spec.rb b/spec/system/admin/poll/questions/answers/videos/videos_spec.rb index 8735e2529..b4d2cdfab 100644 --- a/spec/system/admin/poll/questions/answers/videos/videos_spec.rb +++ b/spec/system/admin/poll/questions/answers/videos/videos_spec.rb @@ -28,6 +28,8 @@ describe "Videos", :admin do visit edit_admin_answer_video_path(answer, video) + expect(page).to have_link "Go back", href: admin_answer_videos_path(answer) + fill_in "Title", with: title fill_in "External video", with: url diff --git a/spec/system/admin/poll/questions_spec.rb b/spec/system/admin/poll/questions_spec.rb index bbf0eb77e..14a622e37 100644 --- a/spec/system/admin/poll/questions_spec.rb +++ b/spec/system/admin/poll/questions_spec.rb @@ -50,8 +50,9 @@ describe "Admin poll questions", :admin do visit admin_poll_path(poll) click_link "Edit answers" - expect(page).to have_content(question.title) - expect(page).to have_content(question.author.name) + expect(page).to have_link "Go back", href: admin_poll_path(poll) + expect(page).to have_content question.title + expect(page).to have_content question.author.name end scenario "Create" do @@ -123,6 +124,7 @@ describe "Admin poll questions", :admin 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
    <%= t("admin.questions.show.answers.document_title") %> <%= t("admin.questions.show.answers.document_actions") %>
    <%= link_to document.title, document.attachment %> diff --git a/app/components/admin/poll/questions/answers/documents/index_component.rb b/app/components/admin/poll/questions/answers/documents/index_component.rb new file mode 100644 index 000000000..ca72d4f98 --- /dev/null +++ b/app/components/admin/poll/questions/answers/documents/index_component.rb @@ -0,0 +1,7 @@ +class Admin::Poll::Questions::Answers::Documents::IndexComponent < ApplicationComponent + attr_reader :answer + + def initialize(answer) + @answer = answer + end +end diff --git a/app/controllers/admin/poll/questions/answers/documents_controller.rb b/app/controllers/admin/poll/questions/answers/documents_controller.rb new file mode 100644 index 000000000..3210896d6 --- /dev/null +++ b/app/controllers/admin/poll/questions/answers/documents_controller.rb @@ -0,0 +1,29 @@ +class Admin::Poll::Questions::Answers::DocumentsController < Admin::Poll::BaseController + include DocumentAttributes + + load_and_authorize_resource :answer, class: "::Poll::Question::Answer" + + def index + end + + def create + @answer.attributes = documents_params + + if @answer.save + redirect_to admin_answer_documents_path(@answer), + notice: t("admin.documents.create.success_notice") + else + render :new + end + end + + private + + def documents_params + params.require(:poll_question_answer).permit(allowed_params) + end + + def allowed_params + [documents_attributes: document_attributes] + end +end diff --git a/app/controllers/admin/poll/questions/answers_controller.rb b/app/controllers/admin/poll/questions/answers_controller.rb index 77e48e46e..9dbeeab1a 100644 --- a/app/controllers/admin/poll/questions/answers_controller.rb +++ b/app/controllers/admin/poll/questions/answers_controller.rb @@ -1,12 +1,10 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController include Translatable - include DocumentAttributes load_and_authorize_resource :question, class: "::Poll::Question" load_and_authorize_resource class: "::Poll::Question::Answer", through: :question, - through_association: :question_answers, - except: :documents + through_association: :question_answers def new end @@ -35,13 +33,6 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController end end - def documents - @answer = ::Poll::Question::Answer.find(params[:answer_id]) - @documents = @answer.documents - - render "admin/poll/questions/answers/documents" - end - def order_answers ::Poll::Question::Answer.order_answers(params[:ordered_list]) head :ok @@ -54,7 +45,7 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController end def allowed_params - attributes = [:title, :description, :given_order, documents_attributes: document_attributes] + attributes = [:title, :description, :given_order] [*attributes, translation_params(Poll::Question::Answer)] end diff --git a/app/views/admin/poll/questions/answers/documents/index.html.erb b/app/views/admin/poll/questions/answers/documents/index.html.erb new file mode 100644 index 000000000..037ab68f2 --- /dev/null +++ b/app/views/admin/poll/questions/answers/documents/index.html.erb @@ -0,0 +1 @@ +<%= render Admin::Poll::Questions::Answers::Documents::IndexComponent.new(@answer) %> diff --git a/config/routes/admin.rb b/config/routes/admin.rb index dde11dbc9..f61ab41a8 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -173,7 +173,7 @@ namespace :admin do resources :answers, only: [], controller: "questions/answers" do resources :images, controller: "questions/answers/images" resources :videos, controller: "questions/answers/videos" - get :documents, to: "questions/answers#documents" + resources :documents, only: [:index, :create], controller: "questions/answers/documents" end post "/answers/order_answers", to: "questions/answers#order_answers" end diff --git a/spec/system/admin/poll/questions/answers/documents/documents_spec.rb b/spec/system/admin/poll/questions/answers/documents/documents_spec.rb index c64b52bb4..5ab6f11c0 100644 --- a/spec/system/admin/poll/questions/answers/documents/documents_spec.rb +++ b/spec/system/admin/poll/questions/answers/documents/documents_spec.rb @@ -21,6 +21,18 @@ describe "Documents", :admin do end end + scenario "Create document for answer" do + answer = create(:poll_question_answer) + + visit admin_answer_documents_path(answer) + + documentable_attach_new_file(Rails.root.join("spec/fixtures/files/clippy.pdf")) + click_button "Save" + + expect(page).to have_content "Document uploaded succesfully" + expect(page).to have_link "clippy.pdf" + end + scenario "Remove document from answer" do answer = create(:poll_question_answer) document = create(:document, documentable: answer) From 78ed5a8bfc6003e8d67d9c065d30c2c64f739442 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 13 Sep 2022 20:47:13 +0200 Subject: [PATCH 10/19] Use caption instead of tr to label the answers table We might change the style of this caption or remove it completely in the future. In the meantime, we use the right HTML tag for the task to give information regarding what the table is about, and that tag is `
    `. --- app/views/admin/poll/questions/show.html.erb | 23 +++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/app/views/admin/poll/questions/show.html.erb b/app/views/admin/poll/questions/show.html.erb index 0c45f0234..33992f421 100644 --- a/app/views/admin/poll/questions/show.html.erb +++ b/app/views/admin/poll/questions/show.html.erb @@ -35,19 +35,16 @@ - - - - - - - - - - - + + + + + + + + + + <% @question.question_answers.each do |answer| %> From 24276a201e24be0c84e762e2a97d5caf1bd1d8bc Mon Sep 17 00:00:00 2001 From: Julian Herrero Date: Sat, 26 Feb 2022 17:41:37 +0700 Subject: [PATCH 11/19] Remove useless answers show view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now, in order to edit an answer, we had to click on its title on the table and then on the "Edit answer" link. That was tedious and different from what we usually do in the admin section. Furthermore, the code for the answers table was written twice and when we modified it we forgot to update the one in the `show` action, meaning the table here provided less information than the information present in the answers tables. Co-Authored-By: Javi Martín --- .../poll/questions/answers_controller.rb | 3 -- .../poll/questions/answers/show.html.erb | 31 ------------------- app/views/admin/poll/questions/show.html.erb | 6 +++- config/initializers/routes_hierarchy.rb | 2 +- config/locales/en/admin.yml | 6 ---- config/locales/es/admin.yml | 6 ---- config/routes/admin.rb | 6 +++- .../poll/questions/answers/answers_spec.rb | 8 ++--- 8 files changed, 14 insertions(+), 54 deletions(-) delete mode 100644 app/views/admin/poll/questions/answers/show.html.erb diff --git a/app/controllers/admin/poll/questions/answers_controller.rb b/app/controllers/admin/poll/questions/answers_controller.rb index 9dbeeab1a..6d6b8a759 100644 --- a/app/controllers/admin/poll/questions/answers_controller.rb +++ b/app/controllers/admin/poll/questions/answers_controller.rb @@ -18,9 +18,6 @@ class Admin::Poll::Questions::AnswersController < Admin::Poll::BaseController end end - def show - end - def edit end diff --git a/app/views/admin/poll/questions/answers/show.html.erb b/app/views/admin/poll/questions/answers/show.html.erb deleted file mode 100644 index 00bc72b02..000000000 --- a/app/views/admin/poll/questions/answers/show.html.erb +++ /dev/null @@ -1,31 +0,0 @@ -<%= back_link_to %> - -<%= link_to t("admin.answers.show.edit"), edit_admin_question_answer_path(@answer.question, @answer), - class: "button hollow float-right" %> - -
      -
    • <%= @answer.question.title %>
      • -
    • <%= @answer.title %>
      • -
    - -
    - -
    - <%= t("admin.questions.show.valid_answers") %> -
    <%= t("admin.questions.show.answers.title") %><%= t("admin.questions.show.answers.description") %><%= t("admin.questions.show.answers.images") %><%= t("admin.questions.show.answers.documents") %><%= t("admin.questions.show.answers.videos") %>
    <%= t("admin.questions.show.valid_answers") %>
    <%= t("admin.questions.show.answers.title") %><%= t("admin.questions.show.answers.description") %><%= t("admin.questions.show.answers.images") %><%= t("admin.questions.show.answers.documents") %><%= t("admin.questions.show.answers.videos") %>
    - - - - - - - - - - - - - - -
    <%= t("admin.answers.show.title") %><%= t("admin.answers.show.description") %><%= t("admin.answers.show.images") %>
    <%= @answer.title %><%= wysiwyg(@answer.description) %> - (<%= @answer.images.count %>)
    - <%= link_to t("admin.answers.show.images_list"), admin_answer_images_path(@answer) %> -
    diff --git a/app/views/admin/poll/questions/show.html.erb b/app/views/admin/poll/questions/show.html.erb index 33992f421..f4e80e14d 100644 --- a/app/views/admin/poll/questions/show.html.erb +++ b/app/views/admin/poll/questions/show.html.erb @@ -43,13 +43,14 @@
    <%= t("admin.questions.show.answers.images") %> <%= t("admin.questions.show.answers.documents") %> <%= t("admin.questions.show.answers.videos") %><%= t("admin.actions.actions") %>
    <%= link_to answer.title, admin_question_answer_path(@question, answer) %><%= answer.title %> <%= wysiwyg(answer.description) %> (<%= answer.images.count %>) @@ -69,6 +70,9 @@ <%= link_to t("admin.questions.show.answers.video_list"), admin_answer_videos_path(answer) %> + <%= render Admin::TableActionsComponent.new(answer, actions: [:edit]) %> +
    <%= video.title %> <%= link_to video.url, video.url %>