From c0e71104f6c1d7cf3457b033ecb4ee45beddc610 Mon Sep 17 00:00:00 2001 From: Marc Anguera Insa Date: Sun, 21 Feb 2021 20:27:22 +0100 Subject: [PATCH 1/3] keep org filter (scope) in public search (closes #508) --- Gemfile.lock | 162 ++++++++++----------- app/controllers/posts_controller.rb | 12 +- app/views/inquiries/index.html.erb | 40 +---- app/views/offers/index.html.erb | 39 +---- app/views/organizations/index.html.erb | 4 +- app/views/shared/_post_filters.html.erb | 40 +++++ app/views/users/_user_row.html.erb | 4 +- spec/controllers/offers_controller_spec.rb | 22 +-- 8 files changed, 144 insertions(+), 179 deletions(-) create mode 100644 app/views/shared/_post_filters.html.erb diff --git a/Gemfile.lock b/Gemfile.lock index b7a14315b..027b1a854 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,40 +1,40 @@ GEM remote: https://rubygems.org/ specs: - actioncable (6.1.1) - actionpack (= 6.1.1) - activesupport (= 6.1.1) + actioncable (6.1.3) + actionpack (= 6.1.3) + activesupport (= 6.1.3) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (6.1.1) - actionpack (= 6.1.1) - activejob (= 6.1.1) - activerecord (= 6.1.1) - activestorage (= 6.1.1) - activesupport (= 6.1.1) + actionmailbox (6.1.3) + actionpack (= 6.1.3) + activejob (= 6.1.3) + activerecord (= 6.1.3) + activestorage (= 6.1.3) + activesupport (= 6.1.3) mail (>= 2.7.1) - actionmailer (6.1.1) - actionpack (= 6.1.1) - actionview (= 6.1.1) - activejob (= 6.1.1) - activesupport (= 6.1.1) + actionmailer (6.1.3) + actionpack (= 6.1.3) + actionview (= 6.1.3) + activejob (= 6.1.3) + activesupport (= 6.1.3) mail (~> 2.5, >= 2.5.4) rails-dom-testing (~> 2.0) - actionpack (6.1.1) - actionview (= 6.1.1) - activesupport (= 6.1.1) + actionpack (6.1.3) + actionview (= 6.1.3) + activesupport (= 6.1.3) rack (~> 2.0, >= 2.0.9) rack-test (>= 0.6.3) rails-dom-testing (~> 2.0) rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (6.1.1) - actionpack (= 6.1.1) - activerecord (= 6.1.1) - activestorage (= 6.1.1) - activesupport (= 6.1.1) + actiontext (6.1.3) + actionpack (= 6.1.3) + activerecord (= 6.1.3) + activestorage (= 6.1.3) + activesupport (= 6.1.3) nokogiri (>= 1.8.5) - actionview (6.1.1) - activesupport (= 6.1.1) + actionview (6.1.3) + activesupport (= 6.1.3) builder (~> 3.1) erubi (~> 1.4) rails-dom-testing (~> 2.0) @@ -48,22 +48,22 @@ GEM kaminari (~> 1.0, >= 1.2.1) railties (>= 5.2, < 6.2) ransack (~> 2.1, >= 2.1.1) - activejob (6.1.1) - activesupport (= 6.1.1) + activejob (6.1.3) + activesupport (= 6.1.3) globalid (>= 0.3.6) - activemodel (6.1.1) - activesupport (= 6.1.1) - activerecord (6.1.1) - activemodel (= 6.1.1) - activesupport (= 6.1.1) - activestorage (6.1.1) - actionpack (= 6.1.1) - activejob (= 6.1.1) - activerecord (= 6.1.1) - activesupport (= 6.1.1) + activemodel (6.1.3) + activesupport (= 6.1.3) + activerecord (6.1.3) + activemodel (= 6.1.3) + activesupport (= 6.1.3) + activestorage (6.1.3) + actionpack (= 6.1.3) + activejob (= 6.1.3) + activerecord (= 6.1.3) + activesupport (= 6.1.3) marcel (~> 0.3.1) mimemagic (~> 0.3.2) - activesupport (6.1.1) + activesupport (6.1.3) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 1.6, < 2) minitest (>= 5.1) @@ -76,8 +76,8 @@ GEM arbre (1.4.0) activesupport (>= 3.0.0, < 6.2) ruby2_keywords (>= 0.0.2, < 1.0) - ast (2.4.1) - autoprefixer-rails (10.2.0.0) + ast (2.4.2) + autoprefixer-rails (10.2.4.0) execjs bcrypt (3.1.16) bindex (0.8.1) @@ -99,16 +99,16 @@ GEM capistrano-rbenv (2.2.0) capistrano (~> 3.1) sshkit (~> 1.3) - capybara (3.34.0) + capybara (3.35.3) addressable mini_mime (>= 0.1.3) nokogiri (~> 1.8) rack (>= 1.6.0) rack-test (>= 0.6.3) - regexp_parser (~> 1.5) + regexp_parser (>= 1.5, < 3.0) xpath (~> 3.2) childprocess (3.0.0) - concurrent-ruby (1.1.7) + concurrent-ruby (1.1.8) connection_pool (2.2.3) crass (1.0.6) database_cleaner (1.8.5) @@ -119,7 +119,7 @@ GEM responders warden (~> 1.2.3) diff-lcs (1.4.4) - docile (1.3.4) + docile (1.3.5) domain_name (0.5.20190701) unf (>= 0.0.5, < 1.0.0) dotenv (2.7.6) @@ -131,16 +131,16 @@ GEM tzinfo execjs (2.7.0) fabrication (2.21.1) - faker (2.15.1) + faker (2.16.0) i18n (>= 1.6, < 2) ffi (1.14.2) - formtastic (3.1.5) - actionpack (>= 3.2.13) + formtastic (4.0.0) + actionpack (>= 5.2.0) formtastic_i18n (0.6.0) - fugit (1.4.1) + fugit (1.4.2) et-orbi (~> 1.1, >= 1.1.8) raabro (~> 1.4) - gli (2.19.2) + gli (2.20.0) globalid (0.4.2) activesupport (>= 4.2.0) has_scope (0.7.2) @@ -150,7 +150,7 @@ GEM http-cookie (1.0.3) domain_name (~> 0.5) http_accept_language (2.1.1) - i18n (1.8.7) + i18n (1.8.9) concurrent-ruby (~> 1.0) inherited_resources (1.12.0) actionpack (>= 5.2, < 6.2) @@ -189,7 +189,7 @@ GEM i18n (>= 0.7, < 2) json (>= 1.7.7) rest-client (>= 1.8.0) - loofah (2.8.0) + loofah (2.9.0) crass (~> 1.0.2) nokogiri (>= 1.5.9) mail (2.7.1) @@ -199,7 +199,7 @@ GEM method_source (1.0.0) mime-types (3.3.1) mime-types-data (~> 3.2015) - mime-types-data (3.2020.1104) + mime-types-data (3.2021.0212) mimemagic (0.3.5) mini_mime (1.0.2) mini_portile2 (2.5.0) @@ -208,7 +208,7 @@ GEM net-ssh (>= 2.6.5, < 7.0.0) net-ssh (6.1.0) netrc (0.11.0) - nio4r (2.5.4) + nio4r (2.5.5) nokogiri (1.11.1) mini_portile2 (~> 2.5.0) racc (~> 1.4) @@ -234,20 +234,20 @@ GEM rack (2.2.3) rack-test (1.1.0) rack (>= 1.0, < 3) - rails (6.1.1) - actioncable (= 6.1.1) - actionmailbox (= 6.1.1) - actionmailer (= 6.1.1) - actionpack (= 6.1.1) - actiontext (= 6.1.1) - actionview (= 6.1.1) - activejob (= 6.1.1) - activemodel (= 6.1.1) - activerecord (= 6.1.1) - activestorage (= 6.1.1) - activesupport (= 6.1.1) + rails (6.1.3) + actioncable (= 6.1.3) + actionmailbox (= 6.1.3) + actionmailer (= 6.1.3) + actionpack (= 6.1.3) + actiontext (= 6.1.3) + actionview (= 6.1.3) + activejob (= 6.1.3) + activemodel (= 6.1.3) + activerecord (= 6.1.3) + activestorage (= 6.1.3) + activesupport (= 6.1.3) bundler (>= 1.15.0) - railties (= 6.1.1) + railties (= 6.1.3) sprockets-rails (>= 2.0.0) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) @@ -261,16 +261,16 @@ GEM rails-i18n (6.0.0) i18n (>= 0.7, < 2) railties (>= 6.0.0, < 7) - railties (6.1.1) - actionpack (= 6.1.1) - activesupport (= 6.1.1) + railties (6.1.3) + actionpack (= 6.1.3) + activesupport (= 6.1.3) method_source rake (>= 0.8.7) thor (~> 1.0) rainbow (3.0.0) raindrops (0.19.1) rake (13.0.3) - ransack (2.4.1) + ransack (2.4.2) activerecord (>= 5.2.4) activesupport (>= 5.2.4) i18n @@ -279,7 +279,7 @@ GEM ffi (~> 1.0) rdiscount (2.2.0.2) redis (4.2.5) - regexp_parser (1.8.2) + regexp_parser (2.0.3) responders (3.0.1) actionpack (>= 5.0) railties (>= 5.0) @@ -295,7 +295,7 @@ GEM rspec-expectations (3.10.1) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.10.0) - rspec-mocks (3.10.1) + rspec-mocks (3.10.2) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.10.0) rspec-rails (4.0.2) @@ -306,8 +306,8 @@ GEM rspec-expectations (~> 3.10) rspec-mocks (~> 3.10) rspec-support (~> 3.10) - rspec-support (3.10.1) - rubocop (1.8.0) + rspec-support (3.10.2) + rubocop (1.10.0) parallel (~> 1.10) parser (>= 3.0.0.0) rainbow (>= 2.2.2, < 4.0) @@ -316,14 +316,14 @@ GEM rubocop-ast (>= 1.2.0, < 2.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 3.0) - rubocop-ast (1.4.0) + rubocop-ast (1.4.1) parser (>= 2.7.1.5) rubocop-rails (2.9.1) activesupport (>= 4.2.0) rack (>= 1.1) rubocop (>= 0.90.0, < 2.0) ruby-progressbar (1.11.0) - ruby2_keywords (0.0.2) + ruby2_keywords (0.0.4) rubyzip (2.3.0) sassc (2.4.0) ffi (~> 1.9) @@ -337,9 +337,9 @@ GEM selenium-webdriver (3.142.7) childprocess (>= 0.5, < 4.0) rubyzip (>= 1.2.2) - shoulda-matchers (4.4.1) + shoulda-matchers (4.5.1) activesupport (>= 4.2.0) - sidekiq (6.1.2) + sidekiq (6.1.3) connection_pool (>= 2.2.2) rack (~> 2.0) redis (>= 4.2.0) @@ -349,7 +349,7 @@ GEM simple_form (5.0.3) actionpack (>= 5.0) activemodel (>= 5.0) - simplecov (0.21.1) + simplecov (0.21.2) docile (~> 1.1) simplecov-html (~> 0.11) simplecov_json_formatter (~> 0.1) @@ -366,10 +366,10 @@ GEM actionpack (>= 4.0) activesupport (>= 4.0) sprockets (>= 3.0.0) - sshkit (1.21.1) + sshkit (1.21.2) net-scp (>= 1.1.2) net-ssh (>= 2.8.0) - thor (1.0.1) + thor (1.1.0) tilt (2.0.10) ttfunk (1.7.0) tzinfo (2.0.4) @@ -390,7 +390,7 @@ GEM activemodel (>= 6.0.0) bindex (>= 0.4.0) railties (>= 6.0.0) - webdrivers (4.4.2) + webdrivers (4.5.0) nokogiri (~> 1.6) rubyzip (>= 1.3.0) selenium-webdriver (>= 3.0, < 4.0) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index d0c359a97..373c6cee8 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -5,20 +5,16 @@ class PostsController < ApplicationController def index context = model.active.of_active_members + if current_organization.present? context = context.where( organization_id: current_organization.id ) end - posts = if (query = params[:q]).present? - context. - search_by_query(query). - page(params[:page]). - per(25) - else - apply_scopes(context).page(params[:page]).per(25) - end + posts = apply_scopes(context) + posts = posts.search_by_query(params[:q]) if params[:q].present? + posts = posts.page(params[:page]).per(25) instance_variable_set("@#{resources}", posts) end diff --git a/app/views/inquiries/index.html.erb b/app/views/inquiries/index.html.erb index 3dabc4c83..95d440ac1 100644 --- a/app/views/inquiries/index.html.erb +++ b/app/views/inquiries/index.html.erb @@ -1,5 +1,3 @@ -<% @category = Category.where(id: params[:cat]).first %> -
@@ -13,43 +11,7 @@
-
- - - -
+ <%= render "shared/post_filters", base_path: inquiries_path %>
<% if current_user && current_organization && !params[:org] %> diff --git a/app/views/offers/index.html.erb b/app/views/offers/index.html.erb index 2953bb285..3b9552da8 100644 --- a/app/views/offers/index.html.erb +++ b/app/views/offers/index.html.erb @@ -2,7 +2,6 @@

- <% @category = Category.where(id: params[:cat]).first %> <%= Offer.model_name.human(count: :many) %> <%= render "shared/show_filter_hint" %>

@@ -12,43 +11,7 @@
-
- - - -
+ <%= render "shared/post_filters", base_path: offers_path %>
<% if current_user && current_organization && !params[:org] %> diff --git a/app/views/organizations/index.html.erb b/app/views/organizations/index.html.erb index 6847e1905..bc255c7ac 100644 --- a/app/views/organizations/index.html.erb +++ b/app/views/organizations/index.html.erb @@ -18,7 +18,7 @@ <%= Organization.model_name.human(count: :one) %> <%= t '.member_count' %> - <%= glyph :wrench %><%= t 'global.table.actions' %> + @@ -26,7 +26,7 @@ <%= link_to org.name, org %> <%= org.users.count %> - + <% if current_user&.admins?(org) %> <%= link_to edit_organization_path(org), class: 'action' do %> <%= glyph :pencil %> diff --git a/app/views/shared/_post_filters.html.erb b/app/views/shared/_post_filters.html.erb new file mode 100644 index 000000000..3bb46e0bb --- /dev/null +++ b/app/views/shared/_post_filters.html.erb @@ -0,0 +1,40 @@ +<% @category = Category.find_by(id: params[:cat]) %> + +
+ + + +
diff --git a/app/views/users/_user_row.html.erb b/app/views/users/_user_row.html.erb index 1e5233cf1..bbcab977b 100644 --- a/app/views/users/_user_row.html.erb +++ b/app/views/users/_user_row.html.erb @@ -9,7 +9,7 @@ <%= phone_to member.phone %> <%= member.account_balance %> <% if current_user.manages?(current_organization) %> - + <% if member.active? %> <%= render 'toggle_manager_link', member: member if can_toggle_manager?(member) %> <% else %> @@ -19,4 +19,4 @@ <%= render 'toggle_active_link', member: member if can_toggle_active?(member) %> <% end %> -<% end %> \ No newline at end of file +<% end %> diff --git a/spec/controllers/offers_controller_spec.rb b/spec/controllers/offers_controller_spec.rb index e9b0184c3..d8b1319e0 100644 --- a/spec/controllers/offers_controller_spec.rb +++ b/spec/controllers/offers_controller_spec.rb @@ -19,9 +19,9 @@ describe "GET #index" do context "with a logged user" do - it "populates an array of offers" do - login(another_member.user) + before { login(another_member.user) } + it "populates an array of offers" do get :index expect(assigns(:offers)).to eq([other_offer, offer]) @@ -34,8 +34,6 @@ end it "only returns active offers" do - login(another_member.user) - get :index expect(assigns(:offers)).to eq([offer]) @@ -49,14 +47,13 @@ end it "only returns offers from active users" do - login(another_member.user) - get :index expect(assigns(:offers)).to eq([other_offer]) end end end + context "with another organization" do it "skips the original org's offers" do login(yet_another_member.user) @@ -69,10 +66,11 @@ end describe "GET #index (search)" do - before { login(another_member.user) } before do - offer.title = "Queridos compañeros" - offer.save! + login(another_member.user) + + offer.title = "Queridos compañeros" + offer.save! end it "populates an array of offers" do @@ -87,6 +85,12 @@ expect(assigns(:offers)).to eq([offer]) end + it "applies organization filter if passed" do + get :index, params: { q: offer.title, org: 2 } + + expect(assigns(:offers)).to eq([]) + end + context "when one offer is not active" do before do other_offer.active = false From 45f305f86e4d464eff75c7e3d868fb1f1463c7b5 Mon Sep 17 00:00:00 2001 From: Marc Anguera Insa Date: Sun, 21 Feb 2021 20:57:43 +0100 Subject: [PATCH 2/3] refactor Reports CSV/PDF code --- app/services/report/csv.rb | 2 -- app/services/report/csv/base.rb | 17 +++++++++++++++++ app/services/report/csv/member.rb | 17 ++--------------- app/services/report/csv/post.rb | 18 ++---------------- app/services/report/pdf.rb | 2 -- app/services/report/pdf/base.rb | 17 +++++++++++++++++ app/services/report/pdf/member.rb | 17 ++--------------- app/services/report/pdf/post.rb | 18 ++---------------- 8 files changed, 42 insertions(+), 66 deletions(-) create mode 100644 app/services/report/csv/base.rb create mode 100644 app/services/report/pdf/base.rb diff --git a/app/services/report/csv.rb b/app/services/report/csv.rb index 768d87b59..bbfa4c963 100644 --- a/app/services/report/csv.rb +++ b/app/services/report/csv.rb @@ -2,8 +2,6 @@ module Report module Csv - MIME_TYPE = Mime[:csv] - def self.run(headers, rows) ::CSV.generate do |csv| csv << headers diff --git a/app/services/report/csv/base.rb b/app/services/report/csv/base.rb new file mode 100644 index 000000000..4e9bedd45 --- /dev/null +++ b/app/services/report/csv/base.rb @@ -0,0 +1,17 @@ +module Report + module Csv + class Base + def name + @decorator.name(:csv) + end + + def mime_type + Mime[:csv] + end + + def run + Report::Csv.run(@decorator.headers, @decorator.rows) + end + end + end +end diff --git a/app/services/report/csv/member.rb b/app/services/report/csv/member.rb index d0a0d3a6c..d1f823d7b 100644 --- a/app/services/report/csv/member.rb +++ b/app/services/report/csv/member.rb @@ -1,21 +1,8 @@ module Report module Csv - class Member + class Member < Base def initialize(org, collection) - @collection = collection - @decorator = MemberReportDecorator.new(org, @collection) - end - - def name - @decorator.name(:csv) - end - - def mime_type - Report::Csv::MIME_TYPE - end - - def run - Report::Csv.run(@decorator.headers, @decorator.rows) + @decorator = MemberReportDecorator.new(org, collection) end end end diff --git a/app/services/report/csv/post.rb b/app/services/report/csv/post.rb index 35ff79659..c7bda2483 100644 --- a/app/services/report/csv/post.rb +++ b/app/services/report/csv/post.rb @@ -1,22 +1,8 @@ module Report module Csv - class Post + class Post < Base def initialize(org, collection, type) - @collection = collection - @type = type - @decorator = PostReportDecorator.new(org, @collection, @type) - end - - def name - @decorator.name(:csv) - end - - def mime_type - Report::Csv::MIME_TYPE - end - - def run - Report::Csv.run(@decorator.headers, @decorator.rows) + @decorator = PostReportDecorator.new(org, collection, type) end end end diff --git a/app/services/report/pdf.rb b/app/services/report/pdf.rb index d4335d8a6..1bf8f88f3 100644 --- a/app/services/report/pdf.rb +++ b/app/services/report/pdf.rb @@ -1,7 +1,5 @@ module Report module Pdf - MIME_TYPE = Mime[:pdf] - def self.run(headers, rows) pdf = Prawn::Document.new(options) pdf.table [headers] + rows diff --git a/app/services/report/pdf/base.rb b/app/services/report/pdf/base.rb new file mode 100644 index 000000000..db4601e4b --- /dev/null +++ b/app/services/report/pdf/base.rb @@ -0,0 +1,17 @@ +module Report + module Pdf + class Base + def name + @decorator.name(:pdf) + end + + def mime_type + Mime[:pdf] + end + + def run + Report::Pdf.run(@decorator.headers, @decorator.rows) + end + end + end +end diff --git a/app/services/report/pdf/member.rb b/app/services/report/pdf/member.rb index be3f858e0..8f35969bc 100644 --- a/app/services/report/pdf/member.rb +++ b/app/services/report/pdf/member.rb @@ -1,21 +1,8 @@ module Report module Pdf - class Member + class Member < Base def initialize(org, collection) - @collection = collection - @decorator = MemberReportDecorator.new(org, @collection) - end - - def name - @decorator.name(:pdf) - end - - def mime_type - Report::Pdf::MIME_TYPE - end - - def run - Report::Pdf.run(@decorator.headers, @decorator.rows) + @decorator = MemberReportDecorator.new(org, collection) end end end diff --git a/app/services/report/pdf/post.rb b/app/services/report/pdf/post.rb index c04211d27..ed1f04fc9 100644 --- a/app/services/report/pdf/post.rb +++ b/app/services/report/pdf/post.rb @@ -1,22 +1,8 @@ module Report module Pdf - class Post + class Post < Base def initialize(org, collection, type) - @collection = collection - @type = type - @decorator = PostReportDecorator.new(org, @collection, @type) - end - - def name - @decorator.name(:pdf) - end - - def mime_type - Report::Pdf::MIME_TYPE - end - - def run - Report::Pdf.run(@decorator.headers, @decorator.rows) + @decorator = PostReportDecorator.new(org, collection, type) end end end From f7e479f78522f73afb854dd63e333291d0c265ad Mon Sep 17 00:00:00 2001 From: Marc Anguera Insa Date: Sun, 21 Feb 2021 21:45:51 +0100 Subject: [PATCH 3/3] specs: more tests for reports --- spec/controllers/reports_controller_spec.rb | 27 ++++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/spec/controllers/reports_controller_spec.rb b/spec/controllers/reports_controller_spec.rb index 078c0ef12..0ba68b839 100644 --- a/spec/controllers/reports_controller_spec.rb +++ b/spec/controllers/reports_controller_spec.rb @@ -23,12 +23,31 @@ end end - describe 'GET #post_list' do - context 'with a logged user' do + context 'with a logged user' do + before { login(member1.user) } + + describe 'GET #user_list' do + it 'downloads a csv' do + get :user_list, params: { format: 'csv' } + + report = Report::Csv::Member.new(test_organization, test_organization.members.active) + expect(response.body).to match(report.run) + expect(response.media_type).to eq("text/csv") + end + + it 'downloads a pdf' do + get :user_list, params: { format: 'pdf' } + + report = Report::Pdf::Member.new(test_organization, test_organization.members.active) + expect(response.body).to eq(report.run) + expect(response.media_type).to eq("application/pdf") + end + end + + describe 'GET #post_list' do it 'do NOT show the inactive members' do - login(member1.user) + get :post_list, params: { type: 'offer' } - get 'post_list', params: { type: 'offer' } posts = assigns(:posts)[0][1] expect(posts.size).to eq(active_organization_offers.size) expect(posts.map(&:id)).to match_array(active_organization_offers.map(&:id))