From 3e50b7ccaf5ab253e6c36614b9d374519931cfab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 8 Aug 2022 14:02:38 +0200 Subject: [PATCH 1/5] Order filters the same way in all hidden content We were doing it differently for investments. --- .../hidden_budget_investments_controller.rb | 2 +- .../admin/hidden_budget_investments_spec.rb | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/app/controllers/admin/hidden_budget_investments_controller.rb b/app/controllers/admin/hidden_budget_investments_controller.rb index 6b977cdd3..1b99963f1 100644 --- a/app/controllers/admin/hidden_budget_investments_controller.rb +++ b/app/controllers/admin/hidden_budget_investments_controller.rb @@ -1,7 +1,7 @@ class Admin::HiddenBudgetInvestmentsController < Admin::BaseController include FeatureFlags - has_filters %w[all with_confirmed_hide without_confirmed_hide], only: :index + has_filters %w[without_confirmed_hide all with_confirmed_hide], only: :index feature_flag :budgets diff --git a/spec/system/admin/hidden_budget_investments_spec.rb b/spec/system/admin/hidden_budget_investments_spec.rb index 29c8a0a72..9f3f20bfe 100644 --- a/spec/system/admin/hidden_budget_investments_spec.rb +++ b/spec/system/admin/hidden_budget_investments_spec.rb @@ -31,9 +31,6 @@ describe "Admin hidden budget investments", :admin do investment = create(:budget_investment, :hidden, heading: heading) visit admin_hidden_budget_investments_path - click_link "Pending" - - expect(page).not_to have_link "Pending" expect(page).to have_content(investment.title) click_button "Confirm moderation" @@ -46,18 +43,18 @@ describe "Admin hidden budget investments", :admin do scenario "Current filter is properly highlighted" do visit admin_hidden_budget_investments_path - expect(page).not_to have_link("All") - expect(page).to have_link("Pending") - expect(page).to have_link("Confirmed") - - visit admin_hidden_budget_investments_path(filter: "without_confirmed_hide") + expect(page).not_to have_link("Pending") expect(page).to have_link("All") expect(page).to have_link("Confirmed") - expect(page).not_to have_link("Pending") + + visit admin_hidden_budget_investments_path(filter: "all") + expect(page).to have_link("Pending") + expect(page).not_to have_link("All") + expect(page).to have_link("Confirmed") visit admin_hidden_budget_investments_path(filter: "with_confirmed_hide") - expect(page).to have_link("All") expect(page).to have_link("Pending") + expect(page).to have_link("All") expect(page).not_to have_link("Confirmed") end From f6fefde91d1b1cf17ea0c754125c30169fbf9afe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 8 Aug 2022 14:27:58 +0200 Subject: [PATCH 2/5] Extract concern to share hidden content code --- .../admin/hidden_budget_investments_controller.rb | 7 ++----- app/controllers/admin/hidden_comments_controller.rb | 5 ++--- app/controllers/admin/hidden_debates_controller.rb | 5 ++--- .../admin/hidden_proposal_notifications_controller.rb | 7 ++----- app/controllers/admin/hidden_proposals_controller.rb | 6 ++---- app/controllers/admin/hidden_users_controller.rb | 4 ++-- app/controllers/concerns/admin/hidden_content.rb | 11 +++++++++++ 7 files changed, 23 insertions(+), 22 deletions(-) create mode 100644 app/controllers/concerns/admin/hidden_content.rb diff --git a/app/controllers/admin/hidden_budget_investments_controller.rb b/app/controllers/admin/hidden_budget_investments_controller.rb index 1b99963f1..dc095bd1e 100644 --- a/app/controllers/admin/hidden_budget_investments_controller.rb +++ b/app/controllers/admin/hidden_budget_investments_controller.rb @@ -1,16 +1,13 @@ class Admin::HiddenBudgetInvestmentsController < Admin::BaseController include FeatureFlags - - has_filters %w[without_confirmed_hide all with_confirmed_hide], only: :index + include Admin::HiddenContent feature_flag :budgets before_action :load_investment, only: [:confirm_hide, :restore] def index - @investments = Budget::Investment.only_hidden.send(@current_filter) - .order(hidden_at: :desc) - .page(params[:page]) + @investments = hidden_content(Budget::Investment.all) end def confirm_hide diff --git a/app/controllers/admin/hidden_comments_controller.rb b/app/controllers/admin/hidden_comments_controller.rb index fc778a1a6..50270eb92 100644 --- a/app/controllers/admin/hidden_comments_controller.rb +++ b/app/controllers/admin/hidden_comments_controller.rb @@ -1,11 +1,10 @@ class Admin::HiddenCommentsController < Admin::BaseController - has_filters %w[without_confirmed_hide all with_confirmed_hide] + include Admin::HiddenContent before_action :load_comment, only: [:confirm_hide, :restore] def index - @comments = Comment.not_valuations.only_hidden.with_visible_author - .send(@current_filter).order(hidden_at: :desc).page(params[:page]) + @comments = hidden_content(Comment.not_valuations).with_visible_author end def confirm_hide diff --git a/app/controllers/admin/hidden_debates_controller.rb b/app/controllers/admin/hidden_debates_controller.rb index b5a4ea502..7c267c746 100644 --- a/app/controllers/admin/hidden_debates_controller.rb +++ b/app/controllers/admin/hidden_debates_controller.rb @@ -1,14 +1,13 @@ class Admin::HiddenDebatesController < Admin::BaseController include FeatureFlags + include Admin::HiddenContent feature_flag :debates - has_filters %w[without_confirmed_hide all with_confirmed_hide], only: :index - before_action :load_debate, only: [:confirm_hide, :restore] def index - @debates = Debate.only_hidden.send(@current_filter).order(hidden_at: :desc).page(params[:page]) + @debates = hidden_content(Debate.all) end def confirm_hide diff --git a/app/controllers/admin/hidden_proposal_notifications_controller.rb b/app/controllers/admin/hidden_proposal_notifications_controller.rb index 365471dd6..27f09b6b1 100644 --- a/app/controllers/admin/hidden_proposal_notifications_controller.rb +++ b/app/controllers/admin/hidden_proposal_notifications_controller.rb @@ -1,13 +1,10 @@ class Admin::HiddenProposalNotificationsController < Admin::BaseController - has_filters %w[without_confirmed_hide all with_confirmed_hide], only: :index + include Admin::HiddenContent before_action :load_proposal, only: [:confirm_hide, :restore] def index - @proposal_notifications = ProposalNotification.only_hidden - .send(@current_filter) - .order(hidden_at: :desc) - .page(params[:page]) + @proposal_notifications = hidden_content(ProposalNotification.all) end def confirm_hide diff --git a/app/controllers/admin/hidden_proposals_controller.rb b/app/controllers/admin/hidden_proposals_controller.rb index 8ee137140..86be183a7 100644 --- a/app/controllers/admin/hidden_proposals_controller.rb +++ b/app/controllers/admin/hidden_proposals_controller.rb @@ -1,15 +1,13 @@ class Admin::HiddenProposalsController < Admin::BaseController include FeatureFlags - - has_filters %w[without_confirmed_hide all with_confirmed_hide], only: :index + include Admin::HiddenContent feature_flag :proposals before_action :load_proposal, only: [:confirm_hide, :restore] def index - @proposals = Proposal.only_hidden.send(@current_filter).order(hidden_at: :desc) - .page(params[:page]) + @proposals = hidden_content(Proposal.all) end def confirm_hide diff --git a/app/controllers/admin/hidden_users_controller.rb b/app/controllers/admin/hidden_users_controller.rb index 86801fcda..536d8d725 100644 --- a/app/controllers/admin/hidden_users_controller.rb +++ b/app/controllers/admin/hidden_users_controller.rb @@ -1,10 +1,10 @@ class Admin::HiddenUsersController < Admin::BaseController - has_filters %w[without_confirmed_hide all with_confirmed_hide], only: :index + include Admin::HiddenContent before_action :load_user, only: [:confirm_hide, :restore] def index - @users = User.only_hidden.send(@current_filter).order(hidden_at: :desc).page(params[:page]) + @users = hidden_content(User.all) end def show diff --git a/app/controllers/concerns/admin/hidden_content.rb b/app/controllers/concerns/admin/hidden_content.rb new file mode 100644 index 000000000..4555a4835 --- /dev/null +++ b/app/controllers/concerns/admin/hidden_content.rb @@ -0,0 +1,11 @@ +module Admin::HiddenContent + extend ActiveSupport::Concern + + included do + has_filters %w[without_confirmed_hide all with_confirmed_hide], only: :index + end + + def hidden_content(relation) + relation.only_hidden.send(@current_filter).order(hidden_at: :desc).page(params[:page]) + end +end From e66b9687a2b78a9961f975e799bc47b8d521517c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 9 Aug 2022 13:33:45 +0200 Subject: [PATCH 3/5] Fix calculating tsvector on hidden records We introduced this bug in commit 55d339572, since we didn't take hidden records into consideration. I've tried to use `update_column` to simplify the code, but got a syntax error `unnamed portal parameter` and didn't find how to fix it. --- app/models/concerns/search_cache.rb | 2 +- spec/models/search_cache_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 spec/models/search_cache_spec.rb diff --git a/app/models/concerns/search_cache.rb b/app/models/concerns/search_cache.rb index ad25b07de..478f9a0e1 100644 --- a/app/models/concerns/search_cache.rb +++ b/app/models/concerns/search_cache.rb @@ -6,7 +6,7 @@ module SearchCache end def calculate_tsvector - self.class.where(id: id).update_all("tsv = (#{searchable_values_sql})") + self.class.with_hidden.where(id: id).update_all("tsv = (#{searchable_values_sql})") end private diff --git a/spec/models/search_cache_spec.rb b/spec/models/search_cache_spec.rb new file mode 100644 index 000000000..03dc09368 --- /dev/null +++ b/spec/models/search_cache_spec.rb @@ -0,0 +1,27 @@ +require "rails_helper" + +describe SearchCache do + describe "#calculate_tsvector" do + it "calculates the tsv column of a record" do + debate = create(:debate) + debate.update_column(:tsv, nil) + + expect(debate.reload.tsv).to be_nil + + debate.calculate_tsvector + + expect(debate.reload.tsv).not_to be_nil + end + + it "calculates the tsv column of a hidden record" do + debate = create(:debate, :hidden) + debate.update_column(:tsv, nil) + + expect(debate.reload.tsv).to be_nil + + debate.calculate_tsvector + + expect(debate.reload.tsv).not_to be_nil + end + end +end From 2af7e324157375426d1462569c4494f7c9625d0f Mon Sep 17 00:00:00 2001 From: Jacek Skrzypacz Date: Wed, 20 Mar 2019 11:35:41 +0100 Subject: [PATCH 4/5] Add search form for hidden content Added search for comments and proposal_notifications, added tsv column for search and rake tasks to update/create tsv vector. --- .../concerns/admin/hidden_content.rb | 6 +- app/models/comment.rb | 12 ++ app/models/proposal_notification.rb | 12 ++ .../hidden_budget_investments/index.html.erb | 1 + .../admin/hidden_comments/index.html.erb | 1 + app/views/admin/hidden_debates/index.html.erb | 1 + .../index.html.erb | 1 + .../admin/hidden_proposals/index.html.erb | 1 + app/views/admin/hidden_users/index.html.erb | 1 + config/locales/en/admin.yml | 2 + config/locales/es/admin.yml | 2 + ..._to_comments_and_proposal_notifications.rb | 9 ++ db/schema.rb | 4 + lib/tasks/consul.rake | 7 +- lib/tasks/db.rake | 11 ++ spec/lib/tasks/db_spec.rb | 39 +++++++ spec/models/comment_spec.rb | 21 ++++ spec/models/proposal_notification_spec.rb | 20 ++++ .../admin/hidden_content_search_spec.rb | 105 ++++++++++++++++++ 19 files changed, 254 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20190307113726_add_tsvector_to_comments_and_proposal_notifications.rb create mode 100644 spec/lib/tasks/db_spec.rb create mode 100644 spec/system/admin/hidden_content_search_spec.rb diff --git a/app/controllers/concerns/admin/hidden_content.rb b/app/controllers/concerns/admin/hidden_content.rb index 4555a4835..7269539a8 100644 --- a/app/controllers/concerns/admin/hidden_content.rb +++ b/app/controllers/concerns/admin/hidden_content.rb @@ -1,11 +1,15 @@ module Admin::HiddenContent extend ActiveSupport::Concern + include Search included do has_filters %w[without_confirmed_hide all with_confirmed_hide], only: :index end def hidden_content(relation) - relation.only_hidden.send(@current_filter).order(hidden_at: :desc).page(params[:page]) + records = relation.only_hidden + records = records.search(@search_terms) if @search_terms.present? + + records.send(@current_filter).order(hidden_at: :desc).page(params[:page]) end end diff --git a/app/models/comment.rb b/app/models/comment.rb index 8e7c886a5..e1a348bfa 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -3,6 +3,7 @@ class Comment < ApplicationRecord include HasPublicAuthor include Graphqlable include Notifiable + include Searchable COMMENTABLE_TYPES = %w[Debate Proposal Budget::Investment Poll Topic Legislation::Question Legislation::Annotation @@ -131,6 +132,17 @@ class Comment < ApplicationRecord cached_votes_up - cached_votes_down end + def searchable_values + { + body => "A", + commentable&.title => "B" + } + end + + def self.search(terms) + pg_search(terms) + end + private def validate_body_length diff --git a/app/models/proposal_notification.rb b/app/models/proposal_notification.rb index a49dfe9a2..8e24bca40 100644 --- a/app/models/proposal_notification.rb +++ b/app/models/proposal_notification.rb @@ -1,6 +1,7 @@ class ProposalNotification < ApplicationRecord include Graphqlable include Notifiable + include Searchable belongs_to :author, class_name: "User" belongs_to :proposal @@ -55,6 +56,17 @@ class ProposalNotification < ApplicationRecord update(moderated: false) end + def searchable_values + { + title => "A", + body => "B" + } + end + + def self.search(terms) + pg_search(terms) + end + private def set_author diff --git a/app/views/admin/hidden_budget_investments/index.html.erb b/app/views/admin/hidden_budget_investments/index.html.erb index 0fa2879ac..34c3201ea 100644 --- a/app/views/admin/hidden_budget_investments/index.html.erb +++ b/app/views/admin/hidden_budget_investments/index.html.erb @@ -1,4 +1,5 @@

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

+<%= render Admin::SearchComponent.new(label: t("admin.shared.search.label.budget_investments")) %>

<%= t("admin.shared.moderated_content") %>

<%= render "shared/filter_subnav", i18n_namespace: "admin.hidden_budget_investments.index" %> diff --git a/app/views/admin/hidden_comments/index.html.erb b/app/views/admin/hidden_comments/index.html.erb index dbf9c84a1..1cc84b84b 100644 --- a/app/views/admin/hidden_comments/index.html.erb +++ b/app/views/admin/hidden_comments/index.html.erb @@ -1,4 +1,5 @@

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

+<%= render Admin::SearchComponent.new(label: t("admin.shared.search.label.comments")) %>

<%= t("admin.shared.moderated_content") %>

<%= render "shared/filter_subnav", i18n_namespace: "admin.hidden_comments.index" %> diff --git a/app/views/admin/hidden_debates/index.html.erb b/app/views/admin/hidden_debates/index.html.erb index 3074ab646..8c50d90aa 100644 --- a/app/views/admin/hidden_debates/index.html.erb +++ b/app/views/admin/hidden_debates/index.html.erb @@ -1,4 +1,5 @@

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

+<%= render Admin::SearchComponent.new(label: t("admin.shared.search.label.debates")) %>

<%= t("admin.shared.moderated_content") %>

<%= render "shared/filter_subnav", i18n_namespace: "admin.hidden_debates.index" %> diff --git a/app/views/admin/hidden_proposal_notifications/index.html.erb b/app/views/admin/hidden_proposal_notifications/index.html.erb index 31aafc141..227a38215 100644 --- a/app/views/admin/hidden_proposal_notifications/index.html.erb +++ b/app/views/admin/hidden_proposal_notifications/index.html.erb @@ -1,4 +1,5 @@

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

+<%= render Admin::SearchComponent.new(label: t("admin.shared.search.label.proposal_notifications")) %>

<%= t("admin.shared.moderated_content") %>

<%= render "shared/filter_subnav", i18n_namespace: "admin.hidden_proposal_notifications.index" %> diff --git a/app/views/admin/hidden_proposals/index.html.erb b/app/views/admin/hidden_proposals/index.html.erb index c28f0f5d4..83a36e329 100644 --- a/app/views/admin/hidden_proposals/index.html.erb +++ b/app/views/admin/hidden_proposals/index.html.erb @@ -1,4 +1,5 @@

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

+<%= render Admin::SearchComponent.new(label: t("admin.shared.search.label.proposals")) %>

<%= t("admin.shared.moderated_content") %>

<%= render "shared/filter_subnav", i18n_namespace: "admin.hidden_proposals.index" %> diff --git a/app/views/admin/hidden_users/index.html.erb b/app/views/admin/hidden_users/index.html.erb index c4addaeda..9f31bdd7e 100644 --- a/app/views/admin/hidden_users/index.html.erb +++ b/app/views/admin/hidden_users/index.html.erb @@ -1,4 +1,5 @@

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

+<%= render "admin/shared/user_search", url: admin_hidden_users_path %>

<%= t("admin.shared.moderated_content") %>

<%= render "shared/filter_subnav", i18n_namespace: "admin.hidden_users.index" %> diff --git a/config/locales/en/admin.yml b/config/locales/en/admin.yml index c6bedf583..f615458c3 100644 --- a/config/locales/en/admin.yml +++ b/config/locales/en/admin.yml @@ -1346,6 +1346,7 @@ en: label: booths: "Search booth by name or location" budget_investments: "Search investments by title, description or heading" + comments: "Search comments" debates: "Search debates by title or description" legislation_processes: "Search processes by title or description" legislation_proposals: "Search proposals by title or description" @@ -1355,6 +1356,7 @@ en: poll_questions: "Search poll questions" polls: "Search polls by name or description" proposals: "Search proposals by title, code, description or question" + proposal_notifications: "Search notifications by title or description" users: "Search user by name or email" search: "Search" search_results: "Search results" diff --git a/config/locales/es/admin.yml b/config/locales/es/admin.yml index 42d446409..aa861ab09 100644 --- a/config/locales/es/admin.yml +++ b/config/locales/es/admin.yml @@ -1345,6 +1345,7 @@ es: label: booths: "Buscar urna por nombre" budget_investments: "Buscar proyectos por título, descripción o partida" + comments: "Buscar comentarios" debates: "Buscar debates por título o descripción" legislation_processes: "Buscar procesos por título o descripción" legislation_proposals: "Buscar propuestas por título o descripción" @@ -1354,6 +1355,7 @@ es: poll_questions: "Buscar preguntas" polls: "Buscar votaciones por nombre o descripción" proposals: "Buscar propuestas por título, código, descripción o pregunta" + proposal_notifications: "Buscar notificaciones por título o descripción" users: "Buscar usuario por nombre o email" search: "Buscar" search_results: "Resultados de la búsqueda" diff --git a/db/migrate/20190307113726_add_tsvector_to_comments_and_proposal_notifications.rb b/db/migrate/20190307113726_add_tsvector_to_comments_and_proposal_notifications.rb new file mode 100644 index 000000000..16f7389f8 --- /dev/null +++ b/db/migrate/20190307113726_add_tsvector_to_comments_and_proposal_notifications.rb @@ -0,0 +1,9 @@ +class AddTsvectorToCommentsAndProposalNotifications < ActiveRecord::Migration[5.2] + def change + add_column :comments, :tsv, :tsvector + add_index :comments, :tsv, using: "gin" + + add_column :proposal_notifications, :tsv, :tsvector + add_index :proposal_notifications, :tsv, using: "gin" + end +end diff --git a/db/schema.rb b/db/schema.rb index 9b9355562..8d87806d6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -448,6 +448,7 @@ ActiveRecord::Schema.define(version: 2021_11_03_112944) do t.string "ancestry" t.integer "confidence_score", default: 0, null: false t.boolean "valuation", default: false + t.tsvector "tsv" t.index ["ancestry"], name: "index_comments_on_ancestry" t.index ["cached_votes_down"], name: "index_comments_on_cached_votes_down" t.index ["cached_votes_total"], name: "index_comments_on_cached_votes_total" @@ -455,6 +456,7 @@ ActiveRecord::Schema.define(version: 2021_11_03_112944) do t.index ["commentable_id", "commentable_type"], name: "index_comments_on_commentable_id_and_commentable_type" t.index ["confidence_score"], name: "index_comments_on_confidence_score" t.index ["hidden_at"], name: "index_comments_on_hidden_at" + t.index ["tsv"], name: "index_comments_on_tsv", using: :gin t.index ["user_id"], name: "index_comments_on_user_id" t.index ["valuation"], name: "index_comments_on_valuation" end @@ -1268,6 +1270,8 @@ ActiveRecord::Schema.define(version: 2021_11_03_112944) do t.datetime "hidden_at" t.datetime "ignored_at" t.datetime "confirmed_hide_at" + t.tsvector "tsv" + t.index ["tsv"], name: "index_proposal_notifications_on_tsv", using: :gin end create_table "proposal_translations", id: :serial, force: :cascade do |t| diff --git a/lib/tasks/consul.rake b/lib/tasks/consul.rake index 6bd19134b..f1074d23e 100644 --- a/lib/tasks/consul.rake +++ b/lib/tasks/consul.rake @@ -2,10 +2,15 @@ namespace :consul do desc "Runs tasks needed to upgrade to the latest version" task execute_release_tasks: ["settings:rename_setting_keys", "settings:add_new_settings", - "execute_release_1.5.0_tasks"] + "execute_release_1.6.0_tasks"] desc "Runs tasks needed to upgrade from 1.4.0 to 1.5.0" task "execute_release_1.5.0_tasks": [ "active_storage:remove_paperclip_compatibility_in_existing_attachments" ] + + desc "Runs tasks needed to upgrade from 1.5.0 to 1.6.0" + task "execute_release_1.6.0_tasks": [ + "db:calculate_tsv" + ] end diff --git a/lib/tasks/db.rake b/lib/tasks/db.rake index 7896eacdf..cb6b20f92 100644 --- a/lib/tasks/db.rake +++ b/lib/tasks/db.rake @@ -5,4 +5,15 @@ namespace :db do I18n.enforce_available_locales = false load(Rails.root.join("db", "dev_seeds.rb")) end + + desc "Calculates the TSV column for all comments and proposal notifications" + task calculate_tsv: :environment do + logger = ApplicationLogger.new + + logger.info "Calculating tsvector for comments" + Comment.with_hidden.find_each(&:calculate_tsvector) + + logger.info "Calculating tsvector for proposal notifications" + ProposalNotification.with_hidden.find_each(&:calculate_tsvector) + end end diff --git a/spec/lib/tasks/db_spec.rb b/spec/lib/tasks/db_spec.rb new file mode 100644 index 000000000..118d9d00c --- /dev/null +++ b/spec/lib/tasks/db_spec.rb @@ -0,0 +1,39 @@ +require "rails_helper" + +describe "rake db:calculate_tsv" do + before { Rake::Task["db:calculate_tsv"].reenable } + + let :run_rake_task do + Rake.application.invoke_task("db:calculate_tsv") + end + + it "calculates the tsvector for comments, including hidden ones" do + comment = create(:comment) + hidden = create(:comment, :hidden) + comment.update_column(:tsv, nil) + hidden.update_column(:tsv, nil) + + expect(comment.reload.tsv).to be nil + expect(hidden.reload.tsv).to be nil + + run_rake_task + + expect(comment.reload.tsv).not_to be nil + expect(hidden.reload.tsv).not_to be nil + end + + it "calculates the tsvector for proposal notifications, including hidden ones" do + notification = create(:proposal_notification) + hidden = create(:proposal_notification, :hidden) + notification.update_column(:tsv, nil) + hidden.update_column(:tsv, nil) + + expect(notification.reload.tsv).to be nil + expect(hidden.reload.tsv).to be nil + + run_rake_task + + expect(notification.reload.tsv).not_to be nil + expect(hidden.reload.tsv).not_to be nil + end +end diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb index 7123c9605..f98c86172 100644 --- a/spec/models/comment_spec.rb +++ b/spec/models/comment_spec.rb @@ -193,4 +193,25 @@ describe Comment do expect(Comment.public_for_api).to be_empty end end + + describe ".search" do + it "searches by body" do + comment = create(:comment, body: "I agree") + + expect(Comment.search("agree")).to eq([comment]) + end + + it "searches by commentable title" do + proposal = create(:proposal, title: "More wood!") + comment = create(:comment, body: "I agree", commentable: proposal) + + expect(Comment.search("wood")).to eq([comment]) + end + + it "does not return non-matching records" do + create(:comment, body: "I agree") + + expect(Comment.search("disagree")).to be_empty + end + end end diff --git a/spec/models/proposal_notification_spec.rb b/spec/models/proposal_notification_spec.rb index 4bd337c36..682ec9ed4 100644 --- a/spec/models/proposal_notification_spec.rb +++ b/spec/models/proposal_notification_spec.rb @@ -42,6 +42,26 @@ describe ProposalNotification do end end + describe ".search" do + it "searches by title" do + notification = create(:proposal_notification, title: "Check this!", body: "It's awesome!") + + expect(ProposalNotification.search("Check")).to eq([notification]) + end + + it "searches by body" do + notification = create(:proposal_notification, title: "Check this!", body: "It's awesome!") + + expect(ProposalNotification.search("awesome")).to eq([notification]) + end + + it "does not return non-matching records" do + create(:proposal_notification, title: "Check this!", body: "It's awesome!") + + expect(ProposalNotification.search("terrible")).to be_empty + end + end + describe "minimum interval between notifications" do before do Setting[:proposal_notification_minimum_interval_in_days] = 3 diff --git a/spec/system/admin/hidden_content_search_spec.rb b/spec/system/admin/hidden_content_search_spec.rb new file mode 100644 index 000000000..5dd9e5b01 --- /dev/null +++ b/spec/system/admin/hidden_content_search_spec.rb @@ -0,0 +1,105 @@ +require "rails_helper" + +describe "Hidden content search", :admin do + scenario "finds matching records" do + create(:budget_investment, :hidden, title: "New football field") + create(:budget_investment, :hidden, title: "New basketball field") + create(:budget_investment, :hidden, title: "New sports center") + + visit admin_hidden_budget_investments_path + fill_in "search", with: "field" + click_button "Search" + + expect(page).not_to have_content "New sports center" + expect(page).to have_content "New football field" + expect(page).to have_content "New basketball field" + end + + scenario "returns no results if no records match the term" do + create(:comment, :hidden, body: "I like this feature") + + visit admin_hidden_comments_path + fill_in "search", with: "love" + click_button "Search" + + expect(page).to have_content "There are no hidden comments" + expect(page).not_to have_content "I like this feature" + expect(page).not_to have_content "I hate this feature" + end + + scenario "returns all records when the search term is empty" do + create(:debate, :hidden, title: "Can we make it better?") + create(:debate, :hidden, title: "Can we make it worse?") + + visit admin_hidden_debates_path(search: "worse") + + expect(page).not_to have_content "Can we make it better?" + expect(page).to have_content "Can we make it worse?" + + fill_in "search", with: " " + click_button "Search" + + expect(page).to have_content "Can we make it better?" + expect(page).to have_content "Can we make it worse?" + expect(page).not_to have_content "There are no hidden debates" + end + + scenario "keeps search parameters after restoring a record" do + create(:proposal_notification, :hidden, title: "Someone is telling you something") + create(:proposal_notification, :hidden, title: "Someone else says whatever") + create(:proposal_notification, :hidden, title: "Nobody is saying anything") + + visit admin_hidden_proposal_notifications_path(search: "Someone") + + expect(page).to have_content "Someone is telling you something" + expect(page).to have_content "Someone else says whatever" + expect(page).not_to have_content "Nobody is saying anything" + + within "tr", text: "Someone is telling you something" do + accept_confirm("Are you sure? Restore") { click_button "Restore" } + end + + expect(page).not_to have_content "Someone is telling you something" + expect(page).to have_content "Someone else says whatever" + expect(page).not_to have_content "Nobody is saying anything" + end + + scenario "keeps search parameters after confirming moderation" do + create(:proposal, :hidden, title: "Reduce the incoming traffic") + create(:proposal, :hidden, title: "Reduce pollution") + create(:proposal, :hidden, title: "Increment pollution") + + visit admin_hidden_proposals_path(search: "Reduce") + + expect(page).to have_content "Reduce the incoming traffic" + expect(page).to have_content "Reduce pollution" + expect(page).not_to have_content "Increment pollution" + + within("tr", text: "Reduce the incoming traffic") { click_button "Confirm moderation" } + + expect(page).not_to have_content "Reduce the incoming traffic" + expect(page).to have_content "Reduce pollution" + expect(page).not_to have_content "Increment pollution" + end + + scenario "keeps search parameters while browsing through filters" do + create(:user, :hidden, username: "person1") + create(:user, :hidden, username: "alien1") + create(:user, :hidden, :with_confirmed_hide, username: "person2") + create(:user, :hidden, :with_confirmed_hide, username: "alien2") + + visit admin_hidden_users_path(search: "person") + + expect(page).to have_content "person1" + expect(page).not_to have_content "person2" + expect(page).not_to have_content "alien1" + expect(page).not_to have_content "alien2" + + click_link "Confirmed" + + expect(page).not_to have_content "person1" + expect(page).to have_content "person2" + expect(page).not_to have_content "alien1" + expect(page).not_to have_content "alien2" + end +end From 92941d3304d0dbd01192dd737ce5481fde4a5760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 22 Aug 2022 19:59:03 +0200 Subject: [PATCH 5/5] Keep filters in admin search While this bug was already present in the general admin search, the combination of both search and filters was very uncommon. I've only found this combinations in the users section, where you've got the "erased" filter, but in this case searching for erased users doesn't really make sense since their username and email have been deleted and so there's nothing to find. So the hidden content seemed to be the only affected section. However, we're adding the field to every section so we don't have to make sure we add it when we need it (like we did in the SDGManagement section). --- .../admin/search_component.html.erb | 1 + app/components/admin/search_component.rb | 10 ++++++ .../relations/index_component.html.erb | 2 +- .../relations/index_component.rb | 2 -- .../relations/search_component.html.erb | 1 - .../relations/search_component.rb | 5 ++- .../components/admin/search_component_spec.rb | 31 +++++++++++++++++++ .../admin/hidden_content_search_spec.rb | 20 ++++++++++++ 8 files changed, 65 insertions(+), 7 deletions(-) create mode 100644 spec/components/admin/search_component_spec.rb diff --git a/app/components/admin/search_component.html.erb b/app/components/admin/search_component.html.erb index 0286dcb48..4658f2971 100644 --- a/app/components/admin/search_component.html.erb +++ b/app/components/admin/search_component.html.erb @@ -1,4 +1,5 @@ <%= form_tag(url, options) do |f| %> + <%= hidden_current_filter_tag %> <%= text_field_tag :search, search_terms.to_s, placeholder: label, "aria-label": label %> <%= content %> <%= submit_tag t("admin.shared.search.search") %> diff --git a/app/components/admin/search_component.rb b/app/components/admin/search_component.rb index e7c4a3e45..6a75b029a 100644 --- a/app/components/admin/search_component.rb +++ b/app/components/admin/search_component.rb @@ -20,4 +20,14 @@ class Admin::SearchComponent < ApplicationComponent def options { method: :get, role: "search" }.merge(form_options) end + + def hidden_current_filter_tag + hidden_field_tag(:filter, current_filter) if current_filter + end + + def current_filter + if helpers.respond_to?(:current_filter) + helpers.current_filter + end + end end diff --git a/app/components/sdg_management/relations/index_component.html.erb b/app/components/sdg_management/relations/index_component.html.erb index 4dfd0cc5a..57127a51f 100644 --- a/app/components/sdg_management/relations/index_component.html.erb +++ b/app/components/sdg_management/relations/index_component.html.erb @@ -1,6 +1,6 @@ <%= header %> -<%= render SDGManagement::Relations::SearchComponent.new(label: search_label, current_filter: current_filter) %> +<%= render SDGManagement::Relations::SearchComponent.new(label: search_label) %> <%= render "shared/filter_subnav", i18n_namespace: "sdg_management.relations.index" %> diff --git a/app/components/sdg_management/relations/index_component.rb b/app/components/sdg_management/relations/index_component.rb index 0db934e13..55fdde345 100644 --- a/app/components/sdg_management/relations/index_component.rb +++ b/app/components/sdg_management/relations/index_component.rb @@ -1,7 +1,5 @@ class SDGManagement::Relations::IndexComponent < ApplicationComponent include Header - delegate :valid_filters, :current_filter, to: :helpers - attr_reader :records def initialize(records) diff --git a/app/components/sdg_management/relations/search_component.html.erb b/app/components/sdg_management/relations/search_component.html.erb index d77f21c8f..31282e747 100644 --- a/app/components/sdg_management/relations/search_component.html.erb +++ b/app/components/sdg_management/relations/search_component.html.erb @@ -5,5 +5,4 @@ <%= component.select_tag :target_code, target_options, include_blank: target_blank_option, "aria-label": target_label %> - <%= component.hidden_field_tag :filter, current_filter %> <% end %> diff --git a/app/components/sdg_management/relations/search_component.rb b/app/components/sdg_management/relations/search_component.rb index fdbe9c833..c48504424 100644 --- a/app/components/sdg_management/relations/search_component.rb +++ b/app/components/sdg_management/relations/search_component.rb @@ -1,10 +1,9 @@ class SDGManagement::Relations::SearchComponent < ApplicationComponent include SDG::OptionsForSelect - attr_reader :label, :current_filter + attr_reader :label - def initialize(label:, current_filter: nil) + def initialize(label:) @label = label - @current_filter = current_filter end private diff --git a/spec/components/admin/search_component_spec.rb b/spec/components/admin/search_component_spec.rb new file mode 100644 index 000000000..196735d52 --- /dev/null +++ b/spec/components/admin/search_component_spec.rb @@ -0,0 +1,31 @@ +require "rails_helper" + +describe Admin::SearchComponent do + describe "#hidden_current_filter_tag" do + context "controller responds to current_filter", controller: ApplicationController do + it "is present when the controller has a current filter" do + allow(controller).to receive(:current_filter).and_return("all") + + render_inline Admin::SearchComponent.new(label: "Search") + + expect(page).to have_field "filter", type: :hidden, with: "all" + end + + it "is not present when the controller has no current filter" do + render_inline Admin::SearchComponent.new(label: "Search") + + expect(page).not_to have_field "filter", type: :hidden + expect(page).not_to have_field "filter" + end + end + + context "controller does not respond to current_filter", controller: ActionController::Base do + it "is not present" do + render_inline Admin::SearchComponent.new(label: "Search") + + expect(page).not_to have_field "filter", type: :hidden + expect(page).not_to have_field "filter" + end + end + end +end diff --git a/spec/system/admin/hidden_content_search_spec.rb b/spec/system/admin/hidden_content_search_spec.rb index 5dd9e5b01..183f72ac8 100644 --- a/spec/system/admin/hidden_content_search_spec.rb +++ b/spec/system/admin/hidden_content_search_spec.rb @@ -102,4 +102,24 @@ describe "Hidden content search", :admin do expect(page).not_to have_content "alien1" expect(page).not_to have_content "alien2" end + + scenario "keeps filter parameters after searching" do + create(:user, :hidden, :with_confirmed_hide, username: "person1") + create(:user, :hidden, :with_confirmed_hide, username: "alien1") + create(:user, :hidden, username: "person2") + create(:user, :hidden, username: "alien2") + + visit admin_hidden_users_path(filter: "with_confirmed_hide") + + fill_in "search", with: "alien" + click_button "Search" + + expect(page).not_to have_content "person1" + expect(page).to have_content "alien1" + expect(page).not_to have_content "person2" + expect(page).not_to have_content "alien2" + + expect(page).to have_content "Confirmed" + expect(page).not_to have_link "Confirmed" + end end