Add groups step to budget creation

Note we're keeping this section's original design (which had one button
to add a new group which after being pressed was replaced by a button to
cancel) but we aren't using Foundation's `data-toggle` because there
were a couple of usability and accessibility issues.

First, using `data-toggle` multiple times and applying it to multiple
elements led to the "cancel" button not being available after submitting
a form with errors. Fixing it made the code more complicated.

Second, the "Add new group" button always had the `aria-expanded`
attribute set to "true", so my screen reader was announcing the button
as expanded even when it wasn't. I didn't manage to fix it using
`data-toggle`.

Finally, after pressing either the "Add new group" and "Cancel" buttons,
the keyboard focus was lost since the elements disappeared.

So we're simplifying the HTML and adding some custom JavaScript to be
able to handle the focus and manually setting the `aria-expanded`
attribute.

Co-Authored-By: Javi Martín <javim@elretirao.net>
Co-Authored-By: Julian Herrero <microweb10@gmail.com>

app/assets/javascripts/admin/budgets_wizard/creation_step.js

@@ -0,0 +1,20 @@
+(function() {
+  "use strict";
+  App.AdminBudgetsWizardCreationStep = {
+    initialize: function() {
+      var element, add_button, cancel_button;
+
+      element = $(".admin .budget-creation-step");
+      add_button = element.find(".add");
+      cancel_button = element.find(".delete");
+
+      add_button.click(function() {
+        $(this).attr("aria-expanded", true).parent().find(":input:visible:first").focus();
+      });
+
+      cancel_button.click(function() {
+        add_button.attr("aria-expanded", false).focus();
+      });
+    }
+  };
+}).call(this);

app/assets/javascripts/application.js

@@ -113,6 +113,7 @@
 //= require columns_selector
 //= require budget_edit_associations
 //= require datepicker
+//= require_tree ./admin
 //= require_tree ./sdg
 //= require_tree ./sdg_management
 
@@ -166,6 +167,7 @@ var initialize_modules = function() {
   if ($("#js-columns-selector").length) {
     App.ColumnsSelector.initialize();
   }
+  App.AdminBudgetsWizardCreationStep.initialize();
   App.BudgetEditAssociations.initialize();
   App.Datepicker.initialize();
   App.SDGRelatedListSelector.initialize();

app/assets/stylesheets/admin/budgets_wizard/creation_step.scss

@@ -0,0 +1,50 @@
+.budget-creation-step {
+
+  .add {
+    @include has-fa-icon(plus-square, solid);
+    @include regular-button;
+
+    font-weight: bold;
+    padding-left: rem-calc(10);
+
+    &::before {
+      margin-right: rem-calc(12);
+    }
+
+    &[aria-expanded="false"] {
+      ~ :not(.next-step) {
+        display: none;
+      }
+    }
+
+    &[aria-expanded="true"] {
+      display: none;
+
+      ~ .next-step {
+        display: none;
+      }
+    }
+  }
+
+  .cancel {
+    display: block;
+    margin-bottom: $line-height;
+  }
+
+  .next-step {
+    @include regular-button;
+  }
+
+  a {
+    &.next-step {
+      @include button-style($success-color, auto, auto);
+    }
+  }
+
+  p {
+    &.next-step {
+      @include button-style($secondary-color, auto, auto);
+      @include button-disabled;
+    }
+  }
+}

app/assets/stylesheets/admin/budgets_wizard/creation_timeline.scss

@@ -5,7 +5,7 @@
   position: relative;
 
   li {
-    border-top: 4px solid $admin-border-color;
+    border-top: 4px solid $brand;
     display: inline-block;
     font-size: $small-font-size;
     font-weight: bold;
@@ -13,7 +13,7 @@
     text-transform: uppercase;
 
     &::before {
-      background: $admin-border-color;
+      background: $brand;
       border-radius: 50%;
       content: "";
       height: 20px;
@@ -23,11 +23,11 @@
       width: 20px;
     }
 
-    &[aria-current] {
+    &[aria-current] ~ * {
-      border-color: $brand;
+      border-color: $admin-border-color;
 
       &::before {
-        background: $brand;
+        background: $admin-border-color;
       }
     }
   }

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

@@ -52,7 +52,11 @@
 
   <div class="small-12 column">
     <div class="clear small-12 medium-4 large-3 inline-block">
+      <% if budget.persisted? %>
         <%= f.submit nil, class: "button success" %>
+      <% else %>
+        <%= f.submit t("admin.budgets_wizard.budgets.continue"), class: "button success expanded" %>
+      <% end %>
     </div>
 
     <div class="float-right">

app/components/admin/budgets/form_component.rb

@@ -1,6 +1,7 @@
 class Admin::Budgets::FormComponent < ApplicationComponent
   include TranslatableFormHelper
   include GlobalizeHelper
+  include Admin::Namespace
 
   attr_reader :budget
   delegate :display_calculate_winners_button?,
@@ -12,14 +13,6 @@ class Admin::Budgets::FormComponent < ApplicationComponent
     @budget = budget
   end
 
-  def namespace
-    if controller.class.name.starts_with?("Admin::BudgetsWizard")
-      :admin_budgets_wizard
-    else
-      helpers.namespace.to_sym
-    end
-  end
-
   def voting_styles_select_options
     Budget::VOTING_STYLES.map do |style|
       [Budget.human_attribute_name("voting_style_#{style}"), style]

app/components/admin/budgets_wizard/budgets/new_component.html.erb

@@ -2,5 +2,5 @@
 
 <%= header %>
 
-<%= render Admin::BudgetsWizard::CreationTimelineComponent.new %>
+<%= render Admin::BudgetsWizard::CreationTimelineComponent.new("budget") %>
 <%= render Admin::Budgets::FormComponent.new(budget) %>

app/components/admin/budgets_wizard/creation_timeline_component.html.erb

@@ -1,5 +1,8 @@
 <ol class="creation-timeline">
-  <li aria-current="step">
+  <li <%= "aria-current='step'" unless step_groups? %>>
     <%= t("admin.budgets_wizard.creation_timeline.budget") %>
   </li>
+  <li <%= "aria-current='step'" if step_groups? %>>
+    <%= t("admin.budgets_wizard.creation_timeline.groups") %>
+  </li>
 </ol>

app/components/admin/budgets_wizard/creation_timeline_component.rb

@@ -1,2 +1,13 @@
 class Admin::BudgetsWizard::CreationTimelineComponent < ApplicationComponent
+  attr_reader :step
+
+  def initialize(step)
+    @step = step
+  end
+
+  private
+
+    def step_groups?
+      step == "groups"
+    end
 end

app/components/admin/budgets_wizard/groups/creation_step_component.html.erb

@@ -0,0 +1,19 @@
+<div class="budget-creation-step">
+  <button type="button" class="add" aria-expanded="<%= show_form? %>">
+    <%= t("admin.budget_groups.index.new_button") %>
+  </button>
+
+  <%= render "/admin/budget_groups/form", group: group, path: form_path, action: "create" %>
+
+  <button type="button" class="cancel delete"><%= t("links.form.cancel_button") %></button>
+
+  <% if next_step_path %>
+    <%= link_to t("admin.budgets_wizard.groups.continue"),
+                next_step_path,
+                class: "next-step" %>
+  <% else %>
+    <p class="next-step">
+      <%= t("admin.budgets_wizard.groups.continue") %>
+    </p>
+  <% end %>
+</div>

app/components/admin/budgets_wizard/groups/creation_step_component.rb

@@ -0,0 +1,30 @@
+class Admin::BudgetsWizard::Groups::CreationStepComponent < ApplicationComponent
+  attr_reader :group, :next_step_group
+
+  def initialize(group, next_step_group)
+    @group = group
+    @next_step_group = next_step_group
+  end
+
+  private
+
+    def budget
+      group.budget
+    end
+
+    def show_form?
+      group.errors.any?
+    end
+
+    def form_path
+      admin_budgets_wizard_budget_groups_path(budget)
+    end
+
+    def next_step_path
+      admin_budget_group_headings_path(budget, next_step_group) if next_step_enabled?
+    end
+
+    def next_step_enabled?
+      next_step_group.present?
+    end
+end

app/components/admin/budgets_wizard/groups/edit_component.html.erb

@@ -0,0 +1,7 @@
+<%= back_link_to admin_budgets_wizard_budget_groups_path(budget) %>
+
+<%= header %>
+
+<%= render Admin::BudgetsWizard::CreationTimelineComponent.new("groups") %>
+
+<%= render "/admin/budget_groups/form", group: group, path: form_path, action: "submit" %>

app/components/admin/budgets_wizard/groups/edit_component.rb

@@ -0,0 +1,22 @@
+class Admin::BudgetsWizard::Groups::EditComponent < ApplicationComponent
+  include Header
+  attr_reader :group
+
+  def initialize(group)
+    @group = group
+  end
+
+  def budget
+    group.budget
+  end
+
+  def title
+    budget.name
+  end
+
+  private
+
+    def form_path
+      admin_budgets_wizard_budget_group_path(budget, group)
+    end
+end

app/components/admin/budgets_wizard/groups/index_component.html.erb

@@ -0,0 +1,9 @@
+<%= back_link_to admin_budget_path(budget), t("admin.budget_groups.index.back") %>
+
+<%= header %>
+
+<%= render Admin::Budgets::HelpComponent.new("budget_groups") %>
+<%= render Admin::BudgetsWizard::CreationTimelineComponent.new("groups") %>
+
+<%= render Admin::BudgetGroups::GroupsComponent.new(groups) %>
+<%= render Admin::BudgetsWizard::Groups::CreationStepComponent.new(new_group, groups.first) %>

app/components/admin/budgets_wizard/groups/index_component.rb

@@ -0,0 +1,17 @@
+class Admin::BudgetsWizard::Groups::IndexComponent < ApplicationComponent
+  include Header
+  attr_reader :groups, :new_group
+
+  def initialize(groups, new_group)
+    @groups = groups
+    @new_group = new_group
+  end
+
+  def budget
+    @new_group.budget
+  end
+
+  def title
+    budget.name
+  end
+end

app/components/admin/menu_component.rb

@@ -13,7 +13,7 @@ class Admin::MenuComponent < ApplicationComponent
     end
 
     def budgets?
-      controller_name.starts_with?("budget")
+      controller_name.starts_with?("budget") || controller_path =~ /budgets_wizard/
     end
 
     def polls?

app/components/admin/table_actions_component.rb

@@ -1,7 +1,7 @@
 class Admin::TableActionsComponent < ApplicationComponent
   include TableActionLink
+  include Admin::Namespace
   attr_reader :record, :options
-  delegate :namespace, to: :helpers
 
   def initialize(record = nil, **options)
     @record = record

app/components/concerns/admin/namespace.rb

@@ -0,0 +1,9 @@
+module Admin::Namespace
+  def namespace
+    if controller.class.name.starts_with?("Admin::BudgetsWizard")
+      :admin_budgets_wizard
+    else
+      helpers.namespace.to_sym
+    end
+  end
+end

app/controllers/admin/budget_groups_controller.rb

@@ -1,64 +1,22 @@
 class Admin::BudgetGroupsController < Admin::BaseController
-  include Translatable
+  include Admin::BudgetGroupsActions
-  include FeatureFlags
-  feature_flag :budgets
 
-  before_action :load_budget
+  before_action :load_groups, only: :index
-  before_action :load_group, only: [:edit, :update, :destroy]
 
   def index
-    @groups = @budget.groups.order(:id)
   end
 
   def new
     @group = @budget.groups.new
   end
 
-  def edit
-  end
-
-  def create
-    @group = @budget.groups.new(budget_group_params)
-    if @group.save
-      redirect_to groups_index, notice: t("admin.budget_groups.create.notice")
-    else
-      render :new
-    end
-  end
-
-  def update
-    if @group.update(budget_group_params)
-      redirect_to groups_index, notice: t("admin.budget_groups.update.notice")
-    else
-      render :edit
-    end
-  end
-
-  def destroy
-    if @group.headings.any?
-      redirect_to groups_index, alert: t("admin.budget_groups.destroy.unable_notice")
-    else
-      @group.destroy!
-      redirect_to groups_index, notice: t("admin.budget_groups.destroy.success_notice")
-    end
-  end
-
   private
 
-    def load_budget
-      @budget = Budget.find_by_slug_or_id! params[:budget_id]
-    end
-
-    def load_group
-      @group = @budget.groups.find_by_slug_or_id! params[:id]
-    end
-
     def groups_index
       admin_budget_groups_path(@budget)
     end
 
-    def budget_group_params
+    def new_action
-      valid_attributes = [:max_votable_headings]
+      :new
-      params.require(:budget_group).permit(*valid_attributes, translation_params(Budget::Group))
     end
 end

app/controllers/admin/budgets_wizard/budgets_controller.rb

@@ -12,7 +12,7 @@ class Admin::BudgetsWizard::BudgetsController < Admin::BaseController
     @budget.published = false
 
     if @budget.save
-      redirect_to edit_admin_budget_path(@budget), notice: t("admin.budgets.create.notice")
+      redirect_to admin_budgets_wizard_budget_groups_path(@budget), notice: t("admin.budgets.create.notice")
     else
       render :new
     end

app/controllers/admin/budgets_wizard/groups_controller.rb

@@ -0,0 +1,19 @@
+class Admin::BudgetsWizard::GroupsController < Admin::BaseController
+  include Admin::BudgetGroupsActions
+
+  before_action :load_groups, only: [:index, :create]
+
+  def index
+    @group = @budget.groups.new
+  end
+
+  private
+
+    def groups_index
+      admin_budgets_wizard_budget_groups_path(@budget)
+    end
+
+    def new_action
+      :index
+    end
+end

app/controllers/concerns/admin/budget_groups_actions.rb

@@ -0,0 +1,60 @@
+module Admin::BudgetGroupsActions
+  extend ActiveSupport::Concern
+
+  included do
+    include Translatable
+    include FeatureFlags
+    feature_flag :budgets
+
+    before_action :load_budget
+    before_action :load_group, only: [:edit, :update, :destroy]
+  end
+
+  def edit
+  end
+
+  def create
+    @group = @budget.groups.new(budget_group_params)
+    if @group.save
+      redirect_to groups_index, notice: t("admin.budget_groups.create.notice")
+    else
+      render new_action
+    end
+  end
+
+  def update
+    if @group.update(budget_group_params)
+      redirect_to groups_index, notice: t("admin.budget_groups.update.notice")
+    else
+      render :edit
+    end
+  end
+
+  def destroy
+    if @group.headings.any?
+      redirect_to groups_index, alert: t("admin.budget_groups.destroy.unable_notice")
+    else
+      @group.destroy!
+      redirect_to groups_index, notice: t("admin.budget_groups.destroy.success_notice")
+    end
+  end
+
+  private
+
+    def load_budget
+      @budget = Budget.find_by_slug_or_id! params[:budget_id]
+    end
+
+    def load_groups
+      @groups = @budget.groups.order(:id)
+    end
+
+    def load_group
+      @group = @budget.groups.find_by_slug_or_id! params[:id]
+    end
+
+    def budget_group_params
+      valid_attributes = [:max_votable_headings]
+      params.require(:budget_group).permit(*valid_attributes, translation_params(Budget::Group))
+    end
+end

app/views/admin/budgets_wizard/groups/edit.html.erb

@@ -0,0 +1 @@
+<%= render Admin::BudgetsWizard::Groups::EditComponent.new(@group) %>

app/views/admin/budgets_wizard/groups/index.html.erb

@@ -0,0 +1 @@
+<%= render Admin::BudgetsWizard::Groups::IndexComponent.new(@groups, @group) %>

config/i18n-tasks.yml

@@ -147,6 +147,7 @@ ignore_unused:
   - "admin.hidden_proposal_notifications.index.filter*"
   - "admin.budgets.index.filter*"
   - "admin.budgets.edit.(administrators|valuators).*"
+  - "admin.budget_groups.index.*.help_block"
   - "admin.budget_investments.index.filter*"
   - "admin.organizations.index.filter*"
   - "admin.hidden_users.index.filter*"

config/locales/en/admin.yml

@@ -156,6 +156,7 @@ en:
       index:
         back: "Go back to budgets"
         help: "Groups are meant to organize headings. After a group is created and it contais headings, it's possible to determine in how many headings a user can vote per group."
+        new_button: "Add new group"
     budget_headings:
       no_headings: "There are no headings."
       amount:
@@ -287,6 +288,11 @@ en:
     budgets_wizard:
       creation_timeline:
         budget: Budget
+        groups: Groups
+      budgets:
+        continue: "Continue to groups"
+      groups:
+        continue: "Continue to headings"
     milestones:
       index:
         table_id: "ID"

config/locales/es/admin.yml

@@ -156,6 +156,7 @@ es:
       index:
         back: "Volver a presupuestos"
         help: "Los grupos sirven para organizar las partidas del presupuesto. Después de que un grupo sea creado y éste contenga partidas, es posible determinar el número de partidas a las que un usuario puede votar por grupo."
+        new_button: "Añadir un grupo nuevo"
     budget_headings:
       no_headings: "No hay partidas."
       amount:
@@ -287,6 +288,11 @@ es:
     budgets_wizard:
       creation_timeline:
         budget: Presupuesto
+        groups: Grupos
+      budgets:
+        continue: "Continuar a grupos"
+      groups:
+        continue: "Continuar a partidas"
     milestones:
       index:
         table_id: "ID"

config/routes/admin.rb

@@ -73,7 +73,9 @@ namespace :admin do
   end
 
   namespace :budgets_wizard do
-    resources :budgets, only: [:create, :new]
+    resources :budgets, only: [:create, :new] do
+      resources :groups, only: [:index, :create, :edit, :update, :destroy]
+    end
   end
 
   resources :milestone_statuses, only: [:index, :new, :create, :update, :edit, :destroy]

spec/system/admin/budgets_wizard/budgets_spec.rb

@@ -7,9 +7,12 @@ describe "Budgets wizard, first step", :admin do
       click_link "Create new budget"
 
       fill_in "Name", with: "M30 - Summer campaign"
-      click_button "Create Budget"
+      click_button "Continue to groups"
 
       expect(page).to have_content "New participatory budget created successfully!"
+
+      click_link "Go back to budgets"
+
       expect(page).to have_field "Name", with: "M30 - Summer campaign"
       expect(page).to have_select "Final voting style", selected: "Knapsack"
     end
@@ -22,9 +25,12 @@ describe "Budgets wizard, first step", :admin do
 
       fill_in "Name", with: "M30 - Summer campaign"
       select "Approval", from: "Final voting style"
-      click_button "Create Budget"
+      click_button "Continue to groups"
 
       expect(page).to have_content "New participatory budget created successfully!"
+
+      click_link "Go back to budgets"
+
       expect(page).to have_field "Name", with: "M30 - Summer campaign"
       expect(page).to have_select "Final voting style", selected: "Approval"
 
@@ -35,7 +41,7 @@ describe "Budgets wizard, first step", :admin do
 
     scenario "Submit the form with errors" do
       visit new_admin_budgets_wizard_budget_path
-      click_button "Create Budget"
+      click_button "Continue to groups"
 
       expect(page).not_to have_content "New participatory budget created successfully!"
       expect(page).to have_css ".is-invalid-label", text: "Name"
@@ -47,7 +53,7 @@ describe "Budgets wizard, first step", :admin do
 
       visit new_admin_budgets_wizard_budget_path
       fill_in "Name", with: "Existing Name"
-      click_button "Create Budget"
+      click_button "Continue to groups"
 
       expect(page).not_to have_content "New participatory budget created successfully!"
       expect(page).to have_css(".is-invalid-label", text: "Name")
@@ -71,9 +77,12 @@ describe "Budgets wizard, first step", :admin do
 
       fill_in "Name", with: "M30 - Summer campaign"
 
-      click_button "Create Budget"
+      click_button "Continue to groups"
 
       expect(page).to have_content "New participatory budget created successfully!"
+
+      click_link "Go back to budgets"
+
       expect(page).to have_content "This participatory budget is in draft mode"
       expect(page).to have_link "Preview budget"
       expect(page).to have_link "Publish budget"

spec/system/admin/budgets_wizard/groups_spec.rb

@@ -0,0 +1,127 @@
+require "rails_helper"
+
+describe "Budgets wizard, groups step", :admin do
+  let(:budget) { create(:budget, :drafting) }
+
+  describe "New" do
+    scenario "create group" do
+      visit admin_budgets_wizard_budget_groups_path(budget)
+
+      within "#side_menu" do
+        expect(page).to have_css ".is-active", exact_text: "Participatory budgets"
+      end
+
+      expect(page).to have_content "Continue to headings"
+      expect(page).not_to have_link "Continue to headings"
+
+      click_button "Add new group"
+
+      fill_in "Group name", with: "All City"
+
+      click_button "Create new group"
+
+      expect(page).to have_content "Group created successfully!"
+      expect(page).to have_content "All City"
+      expect(page).to have_button "Add new group"
+      expect(page).to have_link "Continue to headings"
+    end
+
+    scenario "cancel creating a group" do
+      visit admin_budgets_wizard_budget_groups_path(budget)
+
+      expect(page).not_to have_field "Group name"
+      expect(page).not_to have_button "Cancel"
+      expect(page).to have_content "Continue to headings"
+
+      click_button "Add new group"
+
+      expect(page).to have_field "Group name"
+      expect(page).not_to have_button "Add new group"
+      expect(page).not_to have_content "Continue to headings"
+
+      click_button "Cancel"
+
+      expect(page).to have_button "Add new group"
+      expect(page).not_to have_field "Group name"
+      expect(page).not_to have_button "Cancel"
+      expect(page).to have_content "Continue to headings"
+    end
+
+    scenario "submit the form with errors" do
+      visit admin_budgets_wizard_budget_groups_path(budget)
+      click_button "Add new group"
+
+      click_button "Create new group"
+
+      expect(page).not_to have_content "Group created successfully!"
+      expect(page).to have_css ".is-invalid-label", text: "Group name"
+      expect(page).to have_css ".creation-timeline"
+      expect(page).to have_content "can't be blank"
+      expect(page).to have_button "Create new group"
+      expect(page).to have_button "Cancel"
+      expect(page).not_to have_button "Add new group"
+      expect(page).not_to have_content "Continue to headings"
+    end
+  end
+
+  describe "Edit" do
+    scenario "update group" do
+      create(:budget_group, budget: budget, name: "Group wiht a typo")
+
+      visit admin_budgets_wizard_budget_groups_path(budget)
+
+      expect(page).to have_css ".creation-timeline"
+
+      within("tr", text: "Group wiht a typo") { click_link "Edit" }
+      fill_in "Group name", with: "Group without typos"
+      click_button "Save group"
+
+      expect(page).to have_content "Group updated successfully"
+      expect(page).to have_css ".creation-timeline"
+      expect(page).to have_css "td", exact_text: "Group without typos"
+    end
+
+    scenario "submit the form with errors and then without errors" do
+      group = create(:budget_group, budget: budget, name: "Group wiht a typo")
+
+      visit edit_admin_budgets_wizard_budget_group_path(budget, group)
+      fill_in "Group name", with: ""
+      click_button "Save group"
+
+      expect(page).to have_css "#error_explanation"
+
+      fill_in "Group name", with: "Group without typos"
+      click_button "Save group"
+
+      expect(page).to have_content "Group updated successfully"
+      expect(page).to have_css ".creation-timeline"
+      expect(page).to have_css "td", exact_text: "Group without typos"
+    end
+  end
+
+  describe "Destroy" do
+    scenario "delete a group without headings" do
+      create(:budget_group, budget: budget, name: "Delete me!")
+
+      visit admin_budgets_wizard_budget_groups_path(budget)
+      within("tr", text: "Delete me!") { accept_confirm { click_link "Delete" } }
+
+      expect(page).to have_content "Group deleted successfully"
+      expect(page).not_to have_content "Delete me!"
+      expect(page).to have_css ".creation-timeline"
+    end
+
+    scenario "try to delete a group with headings" do
+      group = create(:budget_group, budget: budget, name: "Don't delete me!")
+      create(:budget_heading, group: group)
+
+      visit admin_budgets_wizard_budget_groups_path(budget)
+
+      within("tr", text: "Don't delete me!") { accept_confirm { click_link "Delete" } }
+
+      expect(page).to have_content "You cannot delete a Group that has associated headings"
+      expect(page).to have_content "Don't delete me!"
+      expect(page).to have_css ".creation-timeline"
+    end
+  end
+end