Add information about budget actions

Both the calculate winners and delete actions benefit from some kind of
hint.

The "calculate winners" hint informs administrators that results won't
be publicly available unless the "show results" option is enabled.

The delete action was redirecting with an error message when the budget
couldn't be deleted; IMHO it's better to disable it and inform
administrators why it's disabled. Alternatively we could remove the
button completely; however, users might be looking for a way to delete a
budget and wouldn't find any hint about it.

We're now removing the "Delete" action from the budgets index table,
since most of the time it isn't possible to delete a budget and so the
action takes up space and we get little gain in return. We could keep
the "Delete" icon just for budgets which can be deleted; however, the
alignment of the table rows would suffer, making it harder to find the
intended action.

app/assets/stylesheets/admin/budgets/actions.scss

@@ -1,20 +1,62 @@
 .admin .budgets-actions {
+  > * {
     $gap: $line-height;
+    $vertical-gap: $line-height / 2;
     @include flex-with-gap($gap);
-  align-items: baseline;
+    align-items: center;
     flex-wrap: wrap;
-  margin-top: -$gap;
+    margin-top: -$vertical-gap;
 
     > * {
-    margin-top: $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;
   }
 }

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)

app/components/admin/budgets/actions_component.html.erb

@@ -1,8 +1,8 @@
-<div class="budgets-actions">
+<dl class="budgets-actions">
-  <%= render Admin::Budgets::CalculateWinnersButtonComponent.new(budget) %>
+  <% actions.each do |action_name, button| %>
-
+    <div>
-  <%= action(:destroy,
+      <dt><%= button[:html] %></dt>
-             text: t("admin.budgets.edit.delete"),
+      <dd id="<%= descriptor_id(action_name) %>"><%= sanitize(button[:hint]) %></dd>
-             method: :delete,
-             class: "delete") %>
     </div>
+  <% end %>
+</dl>

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

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 %>

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)) %>

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

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")

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 <strong>if the option "Show results" is enabled</strong> 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"

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}" <strong>si se habilita la opción "Mostrar resultados"</strong> 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"

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")

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_button count: 1
-    expect(page).to have_css "form[action*='budgets']", exact_text: "Delete"
     expect(page).to have_button "Ballots"
   end
 

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

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")
+      message = "Are you sure? This will delete the budget and all its associated groups and headings. This action cannot be undone."
-      expect(page).to have_content("There are no budgets.")
-    end
 
-    scenario "Destroy a budget without investments but with administrators and valuators" do
+      accept_confirm(message) { click_button "Delete budget" }
-      budget.administrators << Administrator.first
-      budget.valuators << create(:valuator)
-
-      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."
@@ -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_button "Delete budget", disabled: true
-      expect(page).to have_content("There is 1 budget")
+      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_button "Delete budget", disabled: true
-      expect(page).to have_content("There is 1 budget")
+      expect(page).to have_content "You cannot delete a budget that has an associated poll"
     end
   end