From e1aff00e5bfd45f4f2364dd20b28d80a2b300c8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Thu, 25 Feb 2016 16:06:26 +0100 Subject: [PATCH 1/6] =?UTF-8?q?favors=20join=20vs=20*=20=F0=9F=98=AD=20we?= =?UTF-8?q?=20are=20already=20using=20join=20everywhere?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/views/admin/spending_proposals/index.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/spending_proposals/index.html.erb b/app/views/admin/spending_proposals/index.html.erb index 295fb5e76..cddd63b76 100644 --- a/app/views/admin/spending_proposals/index.html.erb +++ b/app/views/admin/spending_proposals/index.html.erb @@ -25,7 +25,7 @@ <%= spending_proposal.valuators.first.name %> <% else %> - + <%= t('admin.spending_proposals.index.valuators_assigned', count: spending_proposal.valuators.size) %> <% end %> From 0e1e0ce37178a6ef916da029bc4a58b03497f45f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Thu, 25 Feb 2016 18:18:40 +0100 Subject: [PATCH 2/6] adds filters: by geozone and without_admin --- .../admin/spending_proposals_controller.rb | 17 ++++++++++- app/models/proposal.rb | 2 +- app/models/spending_proposal.rb | 2 ++ .../admin/spending_proposals/index.html.erb | 11 +++++++ config/locales/admin.en.yml | 7 ++--- config/locales/admin.es.yml | 7 ++--- .../features/admin/spending_proposals_spec.rb | 30 +++++++++++++++++++ spec/models/spending_proposal_spec.rb | 15 ++++++++++ 8 files changed, 81 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/spending_proposals_controller.rb b/app/controllers/admin/spending_proposals_controller.rb index ebe466355..2140d6f37 100644 --- a/app/controllers/admin/spending_proposals_controller.rb +++ b/app/controllers/admin/spending_proposals_controller.rb @@ -2,10 +2,12 @@ class Admin::SpendingProposalsController < Admin::BaseController include FeatureFlags feature_flag :spending_proposals + has_filters %w{all without_admin}, only: :index + load_and_authorize_resource def index - @spending_proposals = @spending_proposals.includes(:geozone, administrator: :user, valuators: :user).order(created_at: :desc).page(params[:page]) + @spending_proposals = geozone_filter(params[:geozone_id].presence).includes(:geozone, administrator: :user, valuators: :user).send(@current_filter).order(created_at: :desc).page(params[:page]) end def show @@ -24,4 +26,17 @@ class Admin::SpendingProposalsController < Admin::BaseController @spending_proposal.update(params.require(:spending_proposal).permit(valuator_ids: [])) end + private + + def geozone_filter(geozone) + case geozone + when nil + @spending_proposals + when 'all' + @spending_proposals.where(geozone_id: nil) + else + @spending_proposals.where(geozone_id: params[:geozone_id].presence) + end + end + end diff --git a/app/models/proposal.rb b/app/models/proposal.rb index 41ff57d10..c507f059c 100644 --- a/app/models/proposal.rb +++ b/app/models/proposal.rb @@ -39,7 +39,7 @@ class Proposal < ActiveRecord::Base scope :sort_by_created_at, -> { reorder(created_at: :desc) } scope :sort_by_most_commented, -> { reorder(comments_count: :desc) } scope :sort_by_random, -> { reorder("RANDOM()") } - scope :sort_by_relevance , -> { all } + scope :sort_by_relevance, -> { all } scope :sort_by_flags, -> { order(flags_count: :desc, updated_at: :desc) } scope :last_week, -> { where("proposals.created_at >= ?", 7.days.ago)} diff --git a/app/models/spending_proposal.rb b/app/models/spending_proposal.rb index 6339fd545..8b4a9f0ef 100644 --- a/app/models/spending_proposal.rb +++ b/app/models/spending_proposal.rb @@ -18,6 +18,8 @@ class SpendingProposal < ActiveRecord::Base validates :description, length: { maximum: SpendingProposal.description_max_length } validates :terms_of_service, acceptance: { allow_nil: false }, on: :create + scope :without_admin, -> { where(administrator_id: nil) } + def description super.try :html_safe end diff --git a/app/views/admin/spending_proposals/index.html.erb b/app/views/admin/spending_proposals/index.html.erb index cddd63b76..26f216876 100644 --- a/app/views/admin/spending_proposals/index.html.erb +++ b/app/views/admin/spending_proposals/index.html.erb @@ -1,5 +1,16 @@

<%= t("admin.spending_proposals.index.title") %>

+
+ <%= form_tag current_path_with_query_params(page: 1), method: :get, enforce_utf8: false do %> + <%= select_tag :geozone_id, options_for_select(geozone_select_options.unshift([t("geozones.none"), "all"]), params[:geozone_id]), + { prompt: t("admin.spending_proposals.index.geozone_filter_all"), + label: false, + class: "js-submit-on-change"} %> + <% end %> +
+ +<%= render 'shared/filter_subnav', i18n_namespace: "admin.spending_proposals.index" %> +

<%= page_entries_info @spending_proposals %>

diff --git a/config/locales/admin.en.yml b/config/locales/admin.en.yml index b777a2c26..7d5cada0a 100755 --- a/config/locales/admin.en.yml +++ b/config/locales/admin.en.yml @@ -138,11 +138,10 @@ en: placeholder: Search user by name or email' spending_proposals: index: - filter: Filter + geozone_filter_all: All zones filters: - accepted: Accepted - rejected: Rejected - unresolved: Unresolved + all: All + without_admin: Without assigned admin title: Investment projects for participatory budgeting admin_assigned: Assigned administrator no_admin_assigned: No admin assigned diff --git a/config/locales/admin.es.yml b/config/locales/admin.es.yml index ef7eaec37..bc545bb59 100644 --- a/config/locales/admin.es.yml +++ b/config/locales/admin.es.yml @@ -138,11 +138,10 @@ es: placeholder: Buscar usuario por nombre o email spending_proposals: index: - filter: Filtro + geozone_filter_all: Todos los ámbitos de actuación filters: - accepted: Aceptadas - rejected: Rechazadas - unresolved: Sin resolver + all: Todas + without_admin: Sin administrador asignado title: Propuestas de inversión para presupuestos participativos admin_assigned: Administrador asignado no_admin_assigned: Sin admin asignado diff --git a/spec/features/admin/spending_proposals_spec.rb b/spec/features/admin/spending_proposals_spec.rb index e1a58d378..9b0d21636 100644 --- a/spec/features/admin/spending_proposals_spec.rb +++ b/spec/features/admin/spending_proposals_spec.rb @@ -50,6 +50,36 @@ feature 'Admin spending proposals' do end end + scenario "Current filter is properly highlighted" do + visit admin_spending_proposals_path + + expect(page).to_not have_link('All') + expect(page).to have_link('Without assigned admin') + + visit admin_spending_proposals_path(filter: 'without_admin') + expect(page).to_not have_link('Without assigned admin') + expect(page).to have_link('All') + + visit admin_spending_proposals_path(filter: 'all') + expect(page).to_not have_link('All') + expect(page).to have_link('Without assigned admin') + end + + scenario "Filtering proposals" do + create(:spending_proposal, title: "New idea") + assigned = create(:spending_proposal, title: "Assigned idea", administrator: create(:administrator)) + + visit admin_spending_proposals_path(filter: 'all') + + expect(page).to have_content("New idea") + expect(page).to have_content("Assigned idea") + + visit admin_spending_proposals_path(filter: 'without_admin') + + expect(page).to have_content("New idea") + expect(page).to_not have_content("Assigned idea") + end + scenario 'Show' do administrator = create(:administrator, user: create(:user, username: 'Ana', email: 'ana@admins.org')) valuator = create(:valuator, user: create(:user, username: 'Rachel', email: 'rachel@valuators.org')) diff --git a/spec/models/spending_proposal_spec.rb b/spec/models/spending_proposal_spec.rb index d0b6973a4..ab5956bc5 100644 --- a/spec/models/spending_proposal_spec.rb +++ b/spec/models/spending_proposal_spec.rb @@ -61,4 +61,19 @@ describe SpendingProposal do end end + + describe "scopes" do + describe "without_admin" do + it "should return all spending proposals without assigned admin" do + spending_proposal1 = create(:spending_proposal) + spending_proposal2 = create(:spending_proposal, administrator: create(:administrator)) + + without_admin = SpendingProposal.without_admin + + expect(without_admin.size).to eq(1) + expect(without_admin.first).to eq(spending_proposal1) + end + end + end + end From 96e5be60a3a6490f0133bd81a647e93c85409c7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Thu, 25 Feb 2016 18:23:17 +0100 Subject: [PATCH 3/6] adds counter cache for assignments --- app/models/valuation_assignment.rb | 2 +- ...add_assignments_counter_cache_to_spending_proposal.rb | 5 +++++ db/schema.rb | 9 +++++---- 3 files changed, 11 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20160225171916_add_assignments_counter_cache_to_spending_proposal.rb diff --git a/app/models/valuation_assignment.rb b/app/models/valuation_assignment.rb index c26b7087f..8f50d9082 100644 --- a/app/models/valuation_assignment.rb +++ b/app/models/valuation_assignment.rb @@ -1,4 +1,4 @@ class ValuationAssignment < ActiveRecord::Base belongs_to :valuator - belongs_to :spending_proposal + belongs_to :spending_proposal, counter_cache: true end diff --git a/db/migrate/20160225171916_add_assignments_counter_cache_to_spending_proposal.rb b/db/migrate/20160225171916_add_assignments_counter_cache_to_spending_proposal.rb new file mode 100644 index 000000000..89a3ac37b --- /dev/null +++ b/db/migrate/20160225171916_add_assignments_counter_cache_to_spending_proposal.rb @@ -0,0 +1,5 @@ +class AddAssignmentsCounterCacheToSpendingProposal < ActiveRecord::Migration + def change + add_column :spending_proposals, :valuation_assignments_count, :integer, default: 0 + end +end diff --git a/db/schema.rb b/db/schema.rb index fd1e0e484..bd85c43a6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160224123110) do +ActiveRecord::Schema.define(version: 20160225171916) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -296,8 +296,8 @@ ActiveRecord::Schema.define(version: 20160224123110) do t.text "description" t.integer "author_id" t.string "external_url" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.integer "geozone_id" t.float "price" t.boolean "feasible" @@ -305,9 +305,10 @@ ActiveRecord::Schema.define(version: 20160224123110) do t.text "price_explanation" t.text "feasible_explanation" t.text "internal_comments" - t.boolean "valuation_finished", default: false + t.boolean "valuation_finished", default: false t.text "explanations_log" t.integer "administrator_id" + t.integer "valuation_assignments_count", default: 0 end add_index "spending_proposals", ["author_id"], name: "index_spending_proposals_on_author_id", using: :btree From 43635f41b099a24eb9aa073e228f76aed49997c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Thu, 25 Feb 2016 18:43:35 +0100 Subject: [PATCH 4/6] adds without_validators filter --- .../admin/spending_proposals_controller.rb | 2 +- app/models/spending_proposal.rb | 1 + .../admin/spending_proposals/index.html.erb | 2 +- config/locales/admin.en.yml | 1 + config/locales/admin.es.yml | 1 + .../features/admin/spending_proposals_spec.rb | 20 ++++++++++++++++--- spec/models/spending_proposal_spec.rb | 13 ++++++++++++ 7 files changed, 35 insertions(+), 5 deletions(-) diff --git a/app/controllers/admin/spending_proposals_controller.rb b/app/controllers/admin/spending_proposals_controller.rb index 2140d6f37..f01bcfa6d 100644 --- a/app/controllers/admin/spending_proposals_controller.rb +++ b/app/controllers/admin/spending_proposals_controller.rb @@ -2,7 +2,7 @@ class Admin::SpendingProposalsController < Admin::BaseController include FeatureFlags feature_flag :spending_proposals - has_filters %w{all without_admin}, only: :index + has_filters %w{all without_admin without_valuators}, only: :index load_and_authorize_resource diff --git a/app/models/spending_proposal.rb b/app/models/spending_proposal.rb index 8b4a9f0ef..961a5e72f 100644 --- a/app/models/spending_proposal.rb +++ b/app/models/spending_proposal.rb @@ -19,6 +19,7 @@ class SpendingProposal < ActiveRecord::Base validates :terms_of_service, acceptance: { allow_nil: false }, on: :create scope :without_admin, -> { where(administrator_id: nil) } + scope :without_valuators, -> { where(valuation_assignments_count: 0) } def description super.try :html_safe diff --git a/app/views/admin/spending_proposals/index.html.erb b/app/views/admin/spending_proposals/index.html.erb index 26f216876..dbbefebfb 100644 --- a/app/views/admin/spending_proposals/index.html.erb +++ b/app/views/admin/spending_proposals/index.html.erb @@ -1,7 +1,7 @@

<%= t("admin.spending_proposals.index.title") %>

- <%= form_tag current_path_with_query_params(page: 1), method: :get, enforce_utf8: false do %> + <%= form_tag admin_spending_proposals_path, method: :get, enforce_utf8: false do %> <%= select_tag :geozone_id, options_for_select(geozone_select_options.unshift([t("geozones.none"), "all"]), params[:geozone_id]), { prompt: t("admin.spending_proposals.index.geozone_filter_all"), label: false, diff --git a/config/locales/admin.en.yml b/config/locales/admin.en.yml index 7d5cada0a..54c10e44b 100755 --- a/config/locales/admin.en.yml +++ b/config/locales/admin.en.yml @@ -142,6 +142,7 @@ en: filters: all: All without_admin: Without assigned admin + without_valuators: Without valuator title: Investment projects for participatory budgeting admin_assigned: Assigned administrator no_admin_assigned: No admin assigned diff --git a/config/locales/admin.es.yml b/config/locales/admin.es.yml index bc545bb59..56c204bec 100644 --- a/config/locales/admin.es.yml +++ b/config/locales/admin.es.yml @@ -142,6 +142,7 @@ es: filters: all: Todas without_admin: Sin administrador asignado + without_valuators: Sin evaluador title: Propuestas de inversión para presupuestos participativos admin_assigned: Administrador asignado no_admin_assigned: Sin admin asignado diff --git a/spec/features/admin/spending_proposals_spec.rb b/spec/features/admin/spending_proposals_spec.rb index 9b0d21636..48d75d9bd 100644 --- a/spec/features/admin/spending_proposals_spec.rb +++ b/spec/features/admin/spending_proposals_spec.rb @@ -55,29 +55,43 @@ feature 'Admin spending proposals' do expect(page).to_not have_link('All') expect(page).to have_link('Without assigned admin') + expect(page).to have_link('Without valuator') visit admin_spending_proposals_path(filter: 'without_admin') expect(page).to_not have_link('Without assigned admin') expect(page).to have_link('All') + expect(page).to have_link('Without valuator') + + visit admin_spending_proposals_path(filter: 'without_valuators') + expect(page).to_not have_link('Without valuator') + expect(page).to have_link('Without assigned admin') + expect(page).to have_link('All') visit admin_spending_proposals_path(filter: 'all') expect(page).to_not have_link('All') expect(page).to have_link('Without assigned admin') + expect(page).to have_link('Without valuator') end scenario "Filtering proposals" do - create(:spending_proposal, title: "New idea") assigned = create(:spending_proposal, title: "Assigned idea", administrator: create(:administrator)) + valuating = create(:spending_proposal, title: "Evaluating...") + valuating.valuators << create(:valuator) visit admin_spending_proposals_path(filter: 'all') - expect(page).to have_content("New idea") expect(page).to have_content("Assigned idea") + expect(page).to have_content("Evaluating...") visit admin_spending_proposals_path(filter: 'without_admin') - expect(page).to have_content("New idea") + expect(page).to have_content("Evaluating...") expect(page).to_not have_content("Assigned idea") + + visit admin_spending_proposals_path(filter: 'without_valuators') + + expect(page).to have_content("Assigned idea") + expect(page).to_not have_content("Evaluating...") end scenario 'Show' do diff --git a/spec/models/spending_proposal_spec.rb b/spec/models/spending_proposal_spec.rb index ab5956bc5..5c7fcec4a 100644 --- a/spec/models/spending_proposal_spec.rb +++ b/spec/models/spending_proposal_spec.rb @@ -74,6 +74,19 @@ describe SpendingProposal do expect(without_admin.first).to eq(spending_proposal1) end end + + describe "without_valuators" do + it "should return all spending proposals without assigned valuators" do + spending_proposal1 = create(:spending_proposal) + spending_proposal2 = create(:spending_proposal) + spending_proposal1.valuators << create(:valuator) + + without_admin = SpendingProposal.without_valuators + + expect(without_admin.size).to eq(1) + expect(without_admin.first).to eq(spending_proposal2) + end + end end end From 0d48d4295c8998f9120e5b0798f91874f46ffa81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Thu, 25 Feb 2016 19:17:41 +0100 Subject: [PATCH 5/6] adds spec for geozone filtering --- .../features/admin/spending_proposals_spec.rb | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/features/admin/spending_proposals_spec.rb b/spec/features/admin/spending_proposals_spec.rb index 48d75d9bd..e5f77808c 100644 --- a/spec/features/admin/spending_proposals_spec.rb +++ b/spec/features/admin/spending_proposals_spec.rb @@ -50,6 +50,30 @@ feature 'Admin spending proposals' do end end + scenario "Index filtering by geozone", :js do + geozone = create(:geozone, name: "District 9") + create(:spending_proposal, title: "Realocate visitors", geozone: geozone) + create(:spending_proposal, title: "Destroy the city") + + visit admin_spending_proposals_path + expect(page).to have_link("Realocate visitors") + expect(page).to have_link("Destroy the city") + + select "District 9", from: "geozone_id" + + expect(page).to have_link("Realocate visitors") + expect(page).to_not have_link("Destroy the city") + + select "All city", from: "geozone_id" + + expect(page).to have_link("Destroy the city") + expect(page).to_not have_link("Realocate visitors") + + select "All zones", from: "geozone_id" + expect(page).to have_link("Realocate visitors") + expect(page).to have_link("Destroy the city") + end + scenario "Current filter is properly highlighted" do visit admin_spending_proposals_path From fa33859c580e8f9f61df95f5dae2ebca2457eeeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baza=CC=81n?= Date: Fri, 26 Feb 2016 11:01:35 +0100 Subject: [PATCH 6/6] fights a flaky test --- .../admin/spending_proposals_controller.rb | 16 ++++++++-------- spec/features/admin/spending_proposals_spec.rb | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/controllers/admin/spending_proposals_controller.rb b/app/controllers/admin/spending_proposals_controller.rb index f01bcfa6d..f2dc2da18 100644 --- a/app/controllers/admin/spending_proposals_controller.rb +++ b/app/controllers/admin/spending_proposals_controller.rb @@ -29,14 +29,14 @@ class Admin::SpendingProposalsController < Admin::BaseController private def geozone_filter(geozone) - case geozone - when nil - @spending_proposals - when 'all' - @spending_proposals.where(geozone_id: nil) - else - @spending_proposals.where(geozone_id: params[:geozone_id].presence) - end + case geozone + when nil + @spending_proposals + when 'all' + @spending_proposals.where(geozone_id: nil) + else + @spending_proposals.where(geozone_id: params[:geozone_id].presence) + end end end diff --git a/spec/features/admin/spending_proposals_spec.rb b/spec/features/admin/spending_proposals_spec.rb index e5f77808c..8e1c19c0e 100644 --- a/spec/features/admin/spending_proposals_spec.rb +++ b/spec/features/admin/spending_proposals_spec.rb @@ -158,9 +158,9 @@ feature 'Admin spending proposals' do expect(page).to have_select('spending_proposal[administrator_id]', selected: 'Undefined') select 'Ana (ana@admins.org)', from: 'spending_proposal[administrator_id]' - expect(page).to have_select('spending_proposal[administrator_id]', selected: 'Ana (ana@admins.org)') - visit admin_spending_proposal_path(spending_proposal) + visit admin_spending_proposals_path + click_link spending_proposal.title expect(page).to have_select('spending_proposal[administrator_id]', selected: 'Ana (ana@admins.org)') end