diff --git a/app/assets/stylesheets/admin/budgets/actions.scss b/app/assets/stylesheets/admin/budgets/actions.scss index 8bebcf5b3..03b82fd6b 100644 --- a/app/assets/stylesheets/admin/budgets/actions.scss +++ b/app/assets/stylesheets/admin/budgets/actions.scss @@ -1,20 +1,62 @@ .admin .budgets-actions { - $gap: $line-height; - @include flex-with-gap($gap); - align-items: baseline; - flex-wrap: wrap; - margin-top: -$gap; - > * { - margin-top: $gap; + $gap: $line-height; + $vertical-gap: $line-height / 2; + @include flex-with-gap($gap); + align-items: center; + flex-wrap: wrap; + margin-top: -$vertical-gap; + + > * { + margin-top: $vertical-gap; + } + + dt { + flex-basis: 12em; + } + + &:only-child dt { + flex-basis: auto; + } + + dd { + flex-basis: 20em; + flex-grow: 1; + max-width: $global-width * 3 / 4; + } + + + * { + border-top: $admin-border; + clip-path: inset(0 0 0 $gap); + margin-top: $line-height * 1.5; + padding-top: $line-height * 1.5; + } } button { cursor: pointer; + width: 100%; } .calculate-winners-link { @include hollow-button; + } + + .destroy-link { + @include regular-button($alert-color); + + &:hover, + &:focus { + background-color: $color-alert; + } + + &[disabled] { + opacity: 0.4; + } + } + + .calculate-winners-link, + .destroy-link { margin-bottom: 0; } } diff --git a/app/components/admin/action_component.rb b/app/components/admin/action_component.rb index 77783528c..2235bb682 100644 --- a/app/components/admin/action_component.rb +++ b/app/components/admin/action_component.rb @@ -32,18 +32,27 @@ class Admin::ActionComponent < ApplicationComponent { class: html_class, id: (dom_id(record, action) if record.respond_to?(:to_key)), + "aria-describedby": describedby, "aria-label": label, data: { confirm: confirmation_text, disable_with: (text if button?) } - }.merge(options.except(:"aria-label", :class, :confirm, :path, :text)) + }.merge(options.except(:"aria-describedby", :"aria-label", :class, :confirm, :path, :text)) end def html_class "admin-action #{options[:class] || "#{action.to_s.gsub("_", "-")}-link"}".strip end + def describedby + if options[:"aria-describedby"] == true + "#{dom_id(record, action)}_descriptor" + else + options[:"aria-describedby"] + end + end + def label if options[:"aria-label"] == true t("admin.actions.label", action: text, name: record_name) diff --git a/app/components/admin/budgets/actions_component.html.erb b/app/components/admin/budgets/actions_component.html.erb index dab442b38..1068e9ac3 100644 --- a/app/components/admin/budgets/actions_component.html.erb +++ b/app/components/admin/budgets/actions_component.html.erb @@ -1,8 +1,8 @@ -
- <%= render Admin::Budgets::CalculateWinnersButtonComponent.new(budget) %> - - <%= action(:destroy, - text: t("admin.budgets.edit.delete"), - method: :delete, - class: "delete") %> -
+
+ <% actions.each do |action_name, button| %> +
+
<%= button[:html] %>
+
<%= sanitize(button[:hint]) %>
+
+ <% end %> +
diff --git a/app/components/admin/budgets/actions_component.rb b/app/components/admin/budgets/actions_component.rb index b16604f17..0728f4a93 100644 --- a/app/components/admin/budgets/actions_component.rb +++ b/app/components/admin/budgets/actions_component.rb @@ -8,6 +8,54 @@ class Admin::Budgets::ActionsComponent < ApplicationComponent private def action(action_name, **options) - render Admin::ActionComponent.new(action_name, budget, **options) + render Admin::ActionComponent.new( + action_name, + budget, + "aria-describedby": true, + **options + ) + end + + def actions + @actions ||= { + calculate_winners: { + hint: winners_hint, + html: winners_action + }, + destroy: { + hint: destroy_hint, + html: destroy_action + } + }.select { |_, button| button[:html].present? } + end + + def winners_action + render Admin::Budgets::CalculateWinnersButtonComponent.new(budget) + end + + def winners_hint + t("admin.budgets.actions.descriptions.calculate_winners", phase: t("budgets.phase.finished")) + end + + def destroy_action + action(:destroy, + text: t("admin.budgets.edit.delete"), + method: :delete, + confirm: t("admin.budgets.actions.confirm.destroy"), + disabled: budget.investments.any? || budget.poll) + end + + def destroy_hint + if budget.investments.any? + t("admin.budgets.destroy.unable_notice") + elsif budget.poll + t("admin.budgets.destroy.unable_notice_polls") + else + t("admin.budgets.actions.descriptions.destroy") + end + end + + def descriptor_id(action_name) + "#{dom_id(budget, action_name)}_descriptor" end end diff --git a/app/components/admin/budgets/calculate_winners_button_component.html.erb b/app/components/admin/budgets/calculate_winners_button_component.html.erb index 49448c00e..62b26541c 100644 --- a/app/components/admin/budgets/calculate_winners_button_component.html.erb +++ b/app/components/admin/budgets/calculate_winners_button_component.html.erb @@ -2,6 +2,7 @@ <%= render Admin::ActionComponent.new( :calculate_winners, budget, + "aria-describedby": !from_investments || nil, text: text, method: :put, class: html_class ) %> <% elsif from_investments %> diff --git a/app/components/admin/budgets/table_actions_component.html.erb b/app/components/admin/budgets/table_actions_component.html.erb index e17d988a4..8a250ecef 100644 --- a/app/components/admin/budgets/table_actions_component.html.erb +++ b/app/components/admin/budgets/table_actions_component.html.erb @@ -1,4 +1,4 @@ -<%= render Admin::TableActionsComponent.new(budget, edit_path: admin_budget_path(budget)) do |actions| %> +<%= render actions_component do |actions| %> <%= actions.action(:investments, text: t("admin.budgets.index.budget_investments"), path: admin_budget_budget_investments_path(budget_id: budget.id)) %> diff --git a/app/components/admin/budgets/table_actions_component.rb b/app/components/admin/budgets/table_actions_component.rb index 0e0dfde97..92277c152 100644 --- a/app/components/admin/budgets/table_actions_component.rb +++ b/app/components/admin/budgets/table_actions_component.rb @@ -17,4 +17,12 @@ class Admin::Budgets::TableActionsComponent < ApplicationComponent ends_at: balloting_phase.ends_at }) end + + def actions_component + Admin::TableActionsComponent.new( + budget, + edit_path: admin_budget_path(budget), + actions: [:edit] + ) + end end diff --git a/app/controllers/admin/budgets_controller.rb b/app/controllers/admin/budgets_controller.rb index 240051ecd..ab2050d46 100644 --- a/app/controllers/admin/budgets_controller.rb +++ b/app/controllers/admin/budgets_controller.rb @@ -43,9 +43,9 @@ class Admin::BudgetsController < Admin::BaseController def destroy if @budget.investments.any? - redirect_to admin_budgets_path, alert: t("admin.budgets.destroy.unable_notice") + redirect_to admin_budget_path(@budget), alert: t("admin.budgets.destroy.unable_notice") elsif @budget.poll.present? - redirect_to admin_budgets_path, alert: t("admin.budgets.destroy.unable_notice_polls") + redirect_to admin_budget_path(@budget), alert: t("admin.budgets.destroy.unable_notice_polls") else @budget.destroy! redirect_to admin_budgets_path, notice: t("admin.budgets.destroy.success_notice") diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index 5a1a5e52d..c834727f9 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -69,6 +69,11 @@ en: no_activity: There are no moderators activity. budgets: actions: + confirm: + destroy: "Are you sure? This will delete the budget and all its associated groups and headings. This action cannot be undone." + descriptions: + calculate_winners: 'After calculating the results, only administrators will be able to access them. They will be public when the project reaches the "%{phase}" phase if the option "Show results" is enabled when editing the budget.' + destroy: "This will delete the budget and all its associated groups and headers. This action cannot be undone." edit: "Edit budget" preview: "Preview" preview_results: "Preview results" diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index f4af55f64..d9122a158 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -69,6 +69,11 @@ es: no_activity: No hay actividad de moderadores. budgets: actions: + confirm: + destroy: "¿Seguro? Esta acción borrará el presupuesto y todos sus grupos y partidas. Esta acción no se puede deshacer." + descriptions: + calculate_winners: 'Tras calcular los resultados, solamente los administradores tendrán acceso a los mismos. Se harán públicos cuando el presupuesto llegue a la fase "%{phase}" si se habilita la opción "Mostrar resultados" al editar el presupuesto.' + destroy: "Esta acción borrará el presupuesto y todos sus grupos y partidas. Esta acción no se puede deshacer." edit: "Editar presupuesto" preview: "Previsualizar" preview_results: "Previsualizar resultados" diff --git a/spec/components/admin/action_component_spec.rb b/spec/components/admin/action_component_spec.rb index 5712b4479..068692471 100644 --- a/spec/components/admin/action_component_spec.rb +++ b/spec/components/admin/action_component_spec.rb @@ -41,6 +41,31 @@ describe Admin::ActionComponent do end end + describe "aria-describedby attribute" do + it "is not rendered by default" do + render_inline Admin::ActionComponent.new(:edit, double, path: "/") + + expect(page).to have_link count: 1 + expect(page).not_to have_css "[aria-describedby]" + end + + it "renders with the given value" do + render_inline Admin::ActionComponent.new(:edit, double, path: "/", "aria-describedby": "my_descriptor") + + expect(page).to have_link count: 1 + expect(page).to have_css "[aria-describedby='my_descriptor']" + end + + it "renders a default value when aria-describedby is true" do + record = double(model_name: double(param_key: "book"), to_key: [23]) + + render_inline Admin::ActionComponent.new(:edit, record, path: "/", "aria-describedby": true) + + expect(page).to have_link count: 1 + expect(page).to have_css "[aria-describedby='edit_book_23_descriptor']" + end + end + describe "aria-label attribute" do it "is not rendered by default" do record = double(human_name: "Stay home") diff --git a/spec/components/admin/budgets/table_actions_component_spec.rb b/spec/components/admin/budgets/table_actions_component_spec.rb index ac91530b5..7f320da9e 100644 --- a/spec/components/admin/budgets/table_actions_component_spec.rb +++ b/spec/components/admin/budgets/table_actions_component_spec.rb @@ -4,7 +4,7 @@ describe Admin::Budgets::TableActionsComponent, controller: Admin::BaseControlle let(:budget) { create(:budget) } let(:component) { Admin::Budgets::TableActionsComponent.new(budget) } - it "renders actions to edit and delete budget, manage investments and manage ballots" do + it "renders actions to edit budget, manage investments and manage ballots" do render_inline component expect(page).to have_link count: 3 @@ -12,8 +12,7 @@ describe Admin::Budgets::TableActionsComponent, controller: Admin::BaseControlle expect(page).to have_link "Edit", href: /#{budget.id}\Z/ expect(page).to have_link "Preview", href: /budgets/ - expect(page).to have_button count: 2 - expect(page).to have_css "form[action*='budgets']", exact_text: "Delete" + expect(page).to have_button count: 1 expect(page).to have_button "Ballots" end diff --git a/spec/controllers/admin/budgets_controller_spec.rb b/spec/controllers/admin/budgets_controller_spec.rb index cb31b0acf..7c881975b 100644 --- a/spec/controllers/admin/budgets_controller_spec.rb +++ b/spec/controllers/admin/budgets_controller_spec.rb @@ -22,4 +22,41 @@ describe Admin::BudgetsController, :admin do end.to raise_error ActiveRecord::RecordNotFound end end + + describe "DELETE destroy" do + let(:budget) { create(:budget) } + + it "allows destroying budgets without investments but with administrators and valuators" do + budget.administrators << Administrator.first + budget.valuators << create(:valuator) + + delete :destroy, params: { id: budget } + + expect(response).to redirect_to admin_budgets_path + expect(flash[:notice]).to eq "Budget deleted successfully" + expect(Budget.count).to eq 0 + expect(BudgetAdministrator.count).to eq 0 + expect(BudgetValuator.count).to eq 0 + end + + it "does not destroy budgets with investments" do + create(:budget_investment, budget: budget) + + delete :destroy, params: { id: budget } + + expect(response).to redirect_to admin_budget_path(budget) + expect(flash[:alert]).to eq "You cannot delete a budget that has associated investments" + expect(Budget.all).to eq [budget] + end + + it "does not destroy budgets with a poll" do + create(:poll, budget: budget) + + delete :destroy, params: { id: budget } + + expect(response).to redirect_to admin_budget_path(budget) + expect(flash[:alert]).to eq "You cannot delete a budget that has an associated poll" + expect(Budget.all).to eq [budget] + end + end end diff --git a/spec/system/admin/budgets_spec.rb b/spec/system/admin/budgets_spec.rb index fd37d0262..db39f419f 100644 --- a/spec/system/admin/budgets_spec.rb +++ b/spec/system/admin/budgets_spec.rb @@ -92,22 +92,6 @@ describe "Admin budgets", :admin do end end end - - scenario "Delete budget from index" do - create(:budget, name: "To be deleted") - - visit admin_budgets_path - - within "tr", text: "To be deleted" do - message = "Are you sure? This action will delete \"To be deleted\" and can't be undone." - - accept_confirm(message) { click_button "Delete" } - end - - expect(page).to have_content("Budget deleted successfully") - expect(page).to have_content("There are no budgets.") - expect(page).not_to have_content "To be deleted" - end end context "Publish" do @@ -141,18 +125,10 @@ describe "Admin budgets", :admin do scenario "Destroy a budget without investments" do visit admin_budget_path(budget) - click_button "Delete budget" - expect(page).to have_content("Budget deleted successfully") - expect(page).to have_content("There are no budgets.") - end + message = "Are you sure? This will delete the budget and all its associated groups and headings. This action cannot be undone." - scenario "Destroy a budget without investments but with administrators and valuators" do - budget.administrators << Administrator.first - budget.valuators << create(:valuator) - - visit admin_budget_path(budget) - click_button "Delete budget" + accept_confirm(message) { click_button "Delete budget" } expect(page).to have_content "Budget deleted successfully" expect(page).to have_content "There are no budgets." @@ -162,20 +138,18 @@ describe "Admin budgets", :admin do create(:budget_investment, heading: heading) visit admin_budget_path(budget) - click_button "Delete budget" - expect(page).to have_content("You cannot delete a budget that has associated investments") - expect(page).to have_content("There is 1 budget") + expect(page).to have_button "Delete budget", disabled: true + expect(page).to have_content "You cannot delete a budget that has associated investments" end scenario "Try to destroy a budget with polls" do create(:poll, budget: budget) visit admin_budget_path(budget) - click_button "Delete budget" - expect(page).to have_content("You cannot delete a budget that has an associated poll") - expect(page).to have_content("There is 1 budget") + expect(page).to have_button "Delete budget", disabled: true + expect(page).to have_content "You cannot delete a budget that has an associated poll" end end