From 869b03ab4099bc0cd8b9f16b657d303d32b1de22 Mon Sep 17 00:00:00 2001 From: Rhian Moraes Date: Tue, 15 Jun 2021 09:36:52 -0300 Subject: [PATCH 1/9] Add filterrific gem --- Gemfile | 1 + Gemfile.lock | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Gemfile b/Gemfile index 25c4b51e1a..8a0de6f21e 100644 --- a/Gemfile +++ b/Gemfile @@ -24,6 +24,7 @@ gem "skylight" # automated performance testing https://www.skylight.io/ gem "webpacker", "~> 5.4" # Transpile app-like JavaScript. Read more: https://github.com/rails/webpacker gem "image_processing", "~> 1.12" # Set of higher-level helper methods for image processing. gem "lograge" # log less so heroku papertrail quits rate limiting our logs +gem "filterrific", "~> 5.2.1" # filtering and sorting of models gem "bootsnap", ">= 1.4.2", require: false # Reduces boot times through caching; required in config/boot.rb gem "bugsnag" # tracking errors in prod diff --git a/Gemfile.lock b/Gemfile.lock index 1f9f6c9315..1557cff51b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -173,6 +173,7 @@ GEM ffi-compiler (1.0.1) ffi (>= 1.0.0) rake + filterrific (5.2.1) globalid (0.4.2) activesupport (>= 4.2.0) html_tokenizer (0.0.7) @@ -450,6 +451,7 @@ DEPENDENCIES erb_lint factory_bot_rails faker + filterrific (~> 5.2.1) image_processing (~> 1.12) jbuilder (~> 2.11) letter_opener From 07d91c23db07a38c9aaf03028f1718a5e1c70691 Mon Sep 17 00:00:00 2001 From: Rhian Moraes Date: Tue, 15 Jun 2021 11:06:17 -0300 Subject: [PATCH 2/9] Update CaseContact model scopes and add support for filterrific --- app/controllers/case_contacts_controller.rb | 20 ++++- app/models/case_contact.rb | 38 ++++++++- app/presenters/case_contact_presenter.rb | 29 +++---- app/views/case_contacts/index.html.erb | 89 +++++++++++++++++++++ config/locales/views.en.yml | 10 +++ 5 files changed, 166 insertions(+), 20 deletions(-) diff --git a/app/controllers/case_contacts_controller.rb b/app/controllers/case_contacts_controller.rb index cb58d71600..abd8896d80 100644 --- a/app/controllers/case_contacts_controller.rb +++ b/app/controllers/case_contacts_controller.rb @@ -6,7 +6,25 @@ class CaseContactsController < ApplicationController def index authorize CaseContact - @presenter = CaseContactPresenter.new + + @current_organization_groups = current_organization.contact_type_groups + .joins(:contact_types) + .where(contact_types: {active: true}) + .uniq + + all_contacts = policy_scope( + current_organization.case_contacts.grab_all(current_user) + .includes(:creator, contact_types: :contact_type_group) + ) + + @filterrific = initialize_filterrific( + all_contacts, + params[:filterrific] + ) || return + + case_contacts = @filterrific.find.group_by(&:casa_case_id) + + @presenter = CaseContactPresenter.new(case_contacts) end def new diff --git a/app/models/case_contact.rb b/app/models/case_contact.rb index 742f67f39c..097a8a603f 100644 --- a/app/models/case_contact.rb +++ b/app/models/case_contact.rb @@ -40,18 +40,35 @@ class CaseContact < ApplicationRecord scope :occurred_between, ->(start_date = nil, end_date = nil) { where("occurred_at BETWEEN ? AND ?", start_date, end_date) if start_date.present? && end_date.present? } + scope :occurred_starting_at, ->(start_date = nil) { + where("occurred_at >= ?", start_date) if start_date.present? + } + scope :occurred_ending_at, ->(end_date = nil) { + where("occurred_at <= ?", end_date) if end_date.present? + } scope :contact_made, ->(contact_made = nil) { - where(contact_made: contact_made) if contact_made == true || contact_made == false + where(contact_made: contact_made) if contact_made.to_s.match(/true|false/) } scope :has_transitioned, ->(has_transitioned = nil) { - joins(:casa_case).where(casa_cases: {transition_aged_youth: has_transitioned}) if has_transitioned == true || has_transitioned == false + if has_transitioned.to_s.match(/true|false/) + joins(:casa_case).where(casa_cases: {transition_aged_youth: has_transitioned}) + end } scope :want_driving_reimbursement, ->(want_driving_reimbursement = nil) { - where(want_driving_reimbursement: want_driving_reimbursement) if want_driving_reimbursement == true || want_driving_reimbursement == false + if want_driving_reimbursement.to_s.match(/true|false/) + where(want_driving_reimbursement: want_driving_reimbursement) + end } scope :contact_type, ->(contact_type_ids = nil) { includes(:contact_types).where("contact_types.id": [contact_type_ids]) if contact_type_ids.present? } + scope :contact_types, ->(contact_type_id_list = nil) { + contact_type_id_list.reject! { |id| id.blank? } + + return if contact_type_id_list.blank? + + includes(:contact_types).where("contact_types.id": contact_type_id_list) + } scope :contact_type_groups, ->(contact_type_group_ids = nil) { # to handle case when passing ids == [''] && ids == nil if contact_type_group_ids&.join&.length&.positive? @@ -64,6 +81,21 @@ class CaseContact < ApplicationRecord with_deleted if current_user.is_a?(CasaAdmin) } + scope :contact_medium, -> (medium_type) { + where(medium_type: medium_type) + } + + filterrific( + available_filters: [ + :occurred_starting_at, + :occurred_ending_at, + :contact_types, + :contact_made, + :contact_medium, + :want_driving_reimbursement + ] + ) + IN_PERSON = "in-person".freeze TEXT_EMAIL = "text/email".freeze VIDEO = "video".freeze diff --git a/app/presenters/case_contact_presenter.rb b/app/presenters/case_contact_presenter.rb index e24e04b03d..0394b29db7 100644 --- a/app/presenters/case_contact_presenter.rb +++ b/app/presenters/case_contact_presenter.rb @@ -1,30 +1,27 @@ class CaseContactPresenter < BasePresenter - def casa_cases - @casa_cases ||= policy_scope(org_cases).group_by(&:id).transform_values(&:first) + attr_accessor :case_contacts + + def initialize(case_contacts) + @case_contacts = case_contacts end - def case_contacts - @case_contacts ||= case_contact_with_deleted.sort_by do |contact| - [contact.casa_case_id, Time.current - contact.occurred_at] - end.group_by(&:casa_case_id) + def casa_cases + @casa_cases ||= policy_scope(org_cases).group_by(&:id).transform_values(&:first) end def display_case_number(casa_case_id) "#{casa_cases[casa_case_id].decorate.transition_aged_youth_icon} #{casa_cases[casa_case_id].case_number}" end - private - - def case_contact_with_deleted - policy_scope( - current_organization.case_contacts - .grab_all(current_user) - .includes(:creator, contact_types: :contact_type_group) - ) - .order("contact_types.id asc") - .decorate + def boolean_select_options + [ + [I18n.t("common.yes_text"), true], + [I18n.t("common.no_text"), false] + ] end + private + def org_cases CasaOrg.includes(:casa_cases) .references(:casa_cases) diff --git a/app/views/case_contacts/index.html.erb b/app/views/case_contacts/index.html.erb index 55243b5a88..ef9d3dcd72 100644 --- a/app/views/case_contacts/index.html.erb +++ b/app/views/case_contacts/index.html.erb @@ -5,6 +5,95 @@ +<%= form_for_filterrific @filterrific do |f| %> +
+
+

<%= t(".filter.title") %>

+
+
+
+
+

+
+
+ <%= f.label t(".filter.start_date"), for: :occurred_starting_at %> + <%= f.text_field(:occurred_starting_at, data: {provide: "datepicker", date_format: "yyyy/mm/dd"}, + class: "form-control") %> +
+
+ <%= f.label t(".filter.end_date"), for: :occurred_until %> + <%= f.text_field(:occurred_ending_at, data: {provide: "datepicker", date_format: "yyyy/mm/dd"}, + class: "form-control") %> +
+
+ +
+
+

+ +
+
+ <% @current_organization_groups.each do |group| %> +
+
<%= group.name %>
+ <% group.contact_types.each do |contact_type| %> +
+ <%= + f.check_box :contact_types, + {multiple: true, class: "form-check-input case-contact-contact-type"}, + contact_type.id, + nil + %> + +
+ <% end %> +
+ <% end %> +
+
+
+
+ +
+
+

+
+ +
+ <%= f.label :contact_medium %> + <%= f.select(:contact_medium, options_from_collection_for_select(contact_mediums, "value", "label"), + {include_blank: t(".filter.display_all")}, + {class: "form-control"}) %> +
+
+ <%= f.label :want_driving_reimbursement %> + <%= f.select(:want_driving_reimbursement, @presenter.boolean_select_options, + {include_blank: t(".filter.display_all")}, + {class: "form-control"}) %> +
+ +
+ <%= f.label :contact_made %> + <%= f.select(:contact_made, @presenter.boolean_select_options, + {include_blank: t(".filter.display_all")}, + {class: "form-control"}) %> +
+
+ +
+ <%= f.submit(t(".filter.submit"), class: "btn btn-primary") %> + <%= link_to( + t(".filter.reset"), + reset_filterrific_url, + class: "btn btn-outline-primary" + ) %> +
+
+
+<% end %> + <% @presenter.case_contacts.each do |casa_case_id, data| %>
diff --git a/config/locales/views.en.yml b/config/locales/views.en.yml index f9304ff32b..c64fbadca3 100644 --- a/config/locales/views.en.yml +++ b/config/locales/views.en.yml @@ -3,6 +3,16 @@ en: index: title: Case Contacts new: New Case Contact + filter: + title: Filters + submit: Filter + reset: Reset filters + display_all: Display all + contact_types: Contact types + other: Other filters + contact_date: Date of contact + start_date: Starting from + end_date: Ending at edit: title: Editing Case Contact new: From a94b94e1c937ebb1bb2fea00f8eac61e324ee588 Mon Sep 17 00:00:00 2001 From: Rhian Moraes Date: Wed, 16 Jun 2021 10:57:29 -0300 Subject: [PATCH 3/9] Update 'no contacts found' message in contacts list --- app/views/case_contacts/index.html.erb | 4 ++-- config/locales/views.en.yml | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/case_contacts/index.html.erb b/app/views/case_contacts/index.html.erb index ef9d3dcd72..f3a2170307 100644 --- a/app/views/case_contacts/index.html.erb +++ b/app/views/case_contacts/index.html.erb @@ -102,11 +102,11 @@
<% end %> + <% unless @presenter.case_contacts.any? %>
- You have no case contacts for this case. Please click New Case Contact button above to create a case contact for - your youth! + <%= params[:filterrific] ? t('.no_contacts_found') : t('.no_contacts_present') %>
<% end %> diff --git a/config/locales/views.en.yml b/config/locales/views.en.yml index c64fbadca3..c18578f726 100644 --- a/config/locales/views.en.yml +++ b/config/locales/views.en.yml @@ -13,6 +13,8 @@ en: contact_date: Date of contact start_date: Starting from end_date: Ending at + no_contacts_found: No case contacts have been found. + no_contacts_present: You have no case contacts for this case. Please click New Case Contact button above to create a case contact for your youth! edit: title: Editing Case Contact new: From 03046d0437a57c8ad1c8f682de78f2291e80f67b Mon Sep 17 00:00:00 2001 From: Rhian Moraes Date: Wed, 16 Jun 2021 11:10:47 -0300 Subject: [PATCH 4/9] Allow collapsing and expanding case contacts filters card --- app/views/case_contacts/index.html.erb | 17 ++++++++++++++--- config/locales/views.en.yml | 1 + 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/views/case_contacts/index.html.erb b/app/views/case_contacts/index.html.erb index f3a2170307..9f3cff5dc5 100644 --- a/app/views/case_contacts/index.html.erb +++ b/app/views/case_contacts/index.html.erb @@ -7,10 +7,21 @@ <%= form_for_filterrific @filterrific do |f| %>
-
-

<%= t(".filter.title") %>

+
+

<%= t(".filter.title") %>

+
-
+ + <% collapse_class = params[:filterrific] ? "" : "collapse" %> +

diff --git a/config/locales/views.en.yml b/config/locales/views.en.yml index c18578f726..9c33d631f1 100644 --- a/config/locales/views.en.yml +++ b/config/locales/views.en.yml @@ -5,6 +5,7 @@ en: new: New Case Contact filter: title: Filters + show_hide: Show / Hide submit: Filter reset: Reset filters display_all: Display all From eef6b6b946750e0dbe236f5ed2746f8663abafd7 Mon Sep 17 00:00:00 2001 From: Rhian Moraes Date: Thu, 17 Jun 2021 10:30:46 -0300 Subject: [PATCH 5/9] Add filters to allow sorting case contacts --- app/controllers/case_contacts_controller.rb | 5 ++- app/models/case_contact.rb | 43 +++++++++++++++++++++ app/views/case_contacts/index.html.erb | 9 ++++- config/locales/models.en.yml | 14 +++++++ 4 files changed, 69 insertions(+), 2 deletions(-) create mode 100644 config/locales/models.en.yml diff --git a/app/controllers/case_contacts_controller.rb b/app/controllers/case_contacts_controller.rb index abd8896d80..28b4e81c2a 100644 --- a/app/controllers/case_contacts_controller.rb +++ b/app/controllers/case_contacts_controller.rb @@ -19,7 +19,10 @@ def index @filterrific = initialize_filterrific( all_contacts, - params[:filterrific] + params[:filterrific], + select_options: { + sorted_by: CaseContact.options_for_sorted_by + } ) || return case_contacts = @filterrific.find.group_by(&:casa_case_id) diff --git a/app/models/case_contact.rb b/app/models/case_contact.rb index 097a8a603f..a62713f6f3 100644 --- a/app/models/case_contact.rb +++ b/app/models/case_contact.rb @@ -85,8 +85,28 @@ class CaseContact < ApplicationRecord where(medium_type: medium_type) } + scope :sorted_by, -> (sort_option) { + direction = /desc$/.match?(sort_option) ? "desc" : "asc" + + case sort_option.to_s + when /^occurred_at/ + order(occurred_at: direction) + when /^contact_type/ + joins(:contact_types).order(name: direction) + when /^medium_type/ + order(medium_type: direction) + when /^want_driving_reimbursement/ + order(want_driving_reimbursement: direction) + when /^contact_made/ + order(contact_made: direction) + else + raise(ArgumentError, "Invalid sort option: #{sort_option.inspect}") + end + } + filterrific( available_filters: [ + :sorted_by, :occurred_starting_at, :occurred_ending_at, :contact_types, @@ -157,6 +177,29 @@ def contact_groups_with_types def requested_followup followups.requested.first end + + def self.options_for_sorted_by + sorted_by_params.map do |option| + [I18n.t("models.case_contact.options_for_sorted_by.#{option}"), option] + end + end + + private + + def self.sorted_by_params + %i[ + occurred_at_asc + occurred_at_desc + contact_type_asc + contact_type_desc + medium_type_asc + medium_type_desc + want_driving_reimbursement_asc + want_driving_reimbursement_desc + contact_made_asc + contact_made_desc + ] + end end # == Schema Information diff --git a/app/views/case_contacts/index.html.erb b/app/views/case_contacts/index.html.erb index 9f3cff5dc5..61133aed75 100644 --- a/app/views/case_contacts/index.html.erb +++ b/app/views/case_contacts/index.html.erb @@ -67,7 +67,7 @@
-
+

@@ -93,6 +93,13 @@
+
+
+ <%= f.label :sorted_by %> + <%= f.select(:sorted_by, @filterrific.select_options[:sorted_by], {}, {class: "form-control"}) %> +
+
+
<%= f.submit(t(".filter.submit"), class: "btn btn-primary") %> <%= link_to( diff --git a/config/locales/models.en.yml b/config/locales/models.en.yml new file mode 100644 index 0000000000..0d6ae5e7ba --- /dev/null +++ b/config/locales/models.en.yml @@ -0,0 +1,14 @@ +en: + models: + case_contact: + options_for_sorted_by: + occurred_at_asc: Date of contact (oldest first) + occurred_at_desc: Date of contact (newest first) + contact_type_asc: Contact type (A-z) + contact_type_desc: Contact type (z-A) + medium_type_asc: Contact medium (A-z) + medium_type_desc: Contact medium (z-A) + want_driving_reimbursement_asc: Want driving reimbursement ('no' first) + want_driving_reimbursement_desc: Want driving reimbursement ('yes' first) + contact_made_asc: Contact made ('no' first) + contact_made_desc: Contact made ('yes' first) \ No newline at end of file From 06730df4f10a66fc9a448302a26cc9def5093ca8 Mon Sep 17 00:00:00 2001 From: Rhian Moraes Date: Thu, 17 Jun 2021 11:37:44 -0300 Subject: [PATCH 6/9] Update specs and add specs for case contacts filters --- app/models/case_contact.rb | 4 +- app/views/case_contacts/index.html.erb | 4 +- spec/models/case_contact_spec.rb | 228 +++++++++++++++++- spec/system/case_contacts/index_spec.rb | 50 +++- .../case_contacts/index.html.erb_spec.rb | 25 +- 5 files changed, 287 insertions(+), 24 deletions(-) diff --git a/app/models/case_contact.rb b/app/models/case_contact.rb index a62713f6f3..97e90590da 100644 --- a/app/models/case_contact.rb +++ b/app/models/case_contact.rb @@ -82,7 +82,7 @@ class CaseContact < ApplicationRecord } scope :contact_medium, -> (medium_type) { - where(medium_type: medium_type) + where(medium_type: medium_type) if medium_type.present? } scope :sorted_by, -> (sort_option) { @@ -109,7 +109,7 @@ class CaseContact < ApplicationRecord :sorted_by, :occurred_starting_at, :occurred_ending_at, - :contact_types, + :contact_type, :contact_made, :contact_medium, :want_driving_reimbursement diff --git a/app/views/case_contacts/index.html.erb b/app/views/case_contacts/index.html.erb index 61133aed75..8beacad9e5 100644 --- a/app/views/case_contacts/index.html.erb +++ b/app/views/case_contacts/index.html.erb @@ -50,12 +50,12 @@ <% group.contact_types.each do |contact_type| %>
<%= - f.check_box :contact_types, + f.check_box :contact_type, {multiple: true, class: "form-check-input case-contact-contact-type"}, contact_type.id, nil %> -
diff --git a/spec/models/case_contact_spec.rb b/spec/models/case_contact_spec.rb index 2d5b3be340..160c0e7e42 100644 --- a/spec/models/case_contact_spec.rb +++ b/spec/models/case_contact_spec.rb @@ -106,22 +106,65 @@ end describe "scopes" do + describe "date related scopes" do + let!(:case_contacts) do + [ + create(:case_contact, occurred_at: Time.zone.yesterday - 1), + create(:case_contact, occurred_at: Time.zone.yesterday), + create(:case_contact, occurred_at: Time.zone.today) + ] + end + + let(:date) { Time.zone.yesterday } + + describe ".occurred_starting_at" do + subject(:occurred_starting_at) { described_class.occurred_starting_at(date) } + + context "with specified date" do + it { is_expected.to eq [case_contacts.second, case_contacts.third] } + end + + context "with no specified date" do + let(:date) { nil } + + it { is_expected.to eq case_contacts } + end + end + + describe ".occurred_ending_at" do + subject(:occurred_ending_at) { described_class.occurred_ending_at(date) } + + context "with specified date" do + it { is_expected.to eq [case_contacts.first, case_contacts.second] } + end + + context "with no specified date" do + let(:date) { nil } + + it { is_expected.to eq case_contacts } + end + end + end + describe ".contact_type" do - it "returns case contacts filtered by contact type id" do - group = create(:contact_type_group) - youth_type = create(:contact_type, name: "Youth", contact_type_group: group) - supervisor_type = create(:contact_type, name: "Supervisor", contact_type_group: group) - parent_type = create(:contact_type, name: "Parent", contact_type_group: group) + subject(:contact_type) { described_class.contact_type([youth_type.id, supervisor_type.id]) } + + let(:group) { create(:contact_type_group) } + let(:youth_type) { create(:contact_type, name: "Youth", contact_type_group: group) } + let(:supervisor_type) { create(:contact_type, name: "Supervisor", contact_type_group: group) } + let(:parent_type) { create(:contact_type, name: "Parent", contact_type_group: group) } - case_contacts_to_match = [ + let!(:case_contacts_to_match) do + [ create(:case_contact, contact_types: [youth_type, supervisor_type]), create(:case_contact, contact_types: [supervisor_type]), create(:case_contact, contact_types: [youth_type, parent_type]) ] - create(:case_contact, contact_types: [parent_type]) - - expect(CaseContact.contact_type([youth_type.id, supervisor_type.id])).to match_array(case_contacts_to_match) end + + let!(:other_case_contact) { create(:case_contact, contact_types: [parent_type]) } + + it { is_expected.to match_array(case_contacts_to_match) } end describe ".contact_made" do @@ -213,6 +256,173 @@ end end end + + describe ".contact_medium" do + subject(:contact_medium) { described_class.contact_medium(medium_type) } + + let!(:case_contacts) do + [ + create(:case_contact, medium_type: "in-person"), + create(:case_contact, medium_type: "letter") + ] + end + + describe "with specified medium parameter" do + let(:medium_type) { "in-person" } + + it { is_expected.to contain_exactly case_contacts.first } + end + + describe "without specified medium parameter" do + let(:medium_type) { nil } + + it { is_expected.to eq case_contacts } + end + end + + describe ".sorted_by" do + subject(:sorted_by) { described_class.sorted_by(sort_option) } + + context "without sort option" do + let(:sort_option) { nil } + + it { expect { sorted_by }.to raise_error(ArgumentError, "Invalid sort option: nil") } + end + + context "with invalid sort option" do + let(:sort_option) { "1254645" } + + it { expect { sorted_by }.to raise_error(ArgumentError, "Invalid sort option: \"1254645\"") } + end + + context "with valid sort option" do + context "with occurred_at option" do + let(:sort_option) { "occurred_at_#{direction}" } + + let!(:case_contacts) do + [ + create(:case_contact, occurred_at: Time.zone.today - 3), + create(:case_contact, occurred_at: Time.zone.today - 1), + create(:case_contact, occurred_at: Time.zone.today - 2) + ] + end + + context "when sorting by ascending order" do + let(:direction) { "asc" } + + it { is_expected.to match_array [case_contacts[0], case_contacts[2], case_contacts[1]] } + end + + context "when sorting by descending order" do + let(:direction) { "desc" } + + it { is_expected.to match_array [case_contacts[1], case_contacts[2], case_contacts[0]] } + end + end + + context "with contact_type option" do + let(:sort_option) { "contact_type_#{direction}" } + + let(:group) { create(:contact_type_group) } + + let(:contact_types) do + [ + create(:contact_type, name: "Supervisor", contact_type_group: group), + create(:contact_type, name: "Parent", contact_type_group: group), + create(:contact_type, name: "Youth", contact_type_group: group) + ] + end + + let!(:case_contacts) do + contact_types.map do |contact_type| + create(:case_contact, contact_types: [contact_type]) + end + end + + context "when sorting by ascending order" do + let(:direction) { "asc" } + + it { is_expected.to match_array [case_contacts[1], case_contacts[0], case_contacts[2]] } + end + + context "when sorting by descending order" do + let(:direction) { "desc" } + + it { is_expected.to match_array [case_contacts[2], case_contacts[0], case_contacts[1]] } + end + end + + context "with medium_type option" do + let(:sort_option) { "contact_type_#{direction}" } + + let!(:case_contacts) do + [ + create(:case_contact, medium_type: "in-person"), + create(:case_contact, medium_type: "text/email"), + create(:case_contact, medium_type: "letter") + ] + end + + context "when sorting by ascending order" do + let(:direction) { "asc" } + + it { is_expected.to match_array [case_contacts[0], case_contacts[2], case_contacts[1]] } + end + + context "when sorting by descending order" do + let(:direction) { "desc" } + + it { is_expected.to match_array [case_contacts[1], case_contacts[2], case_contacts[0]] } + end + end + + context "with want_driving_reimbursement option" do + let(:sort_option) { "want_driving_reimbursement_#{direction}" } + + let!(:case_contacts) do + [ + create(:case_contact, miles_driven: 1, want_driving_reimbursement: true), + create(:case_contact, miles_driven: 1, want_driving_reimbursement: false) + ] + end + + context "when sorting by ascending order" do + let(:direction) { "asc" } + + it { is_expected.to match_array [case_contacts[0], case_contacts[1]] } + end + + context "when sorting by descending order" do + let(:direction) { "desc" } + + it { is_expected.to match_array [case_contacts[1], case_contacts[0]] } + end + end + + context "with contact_made option" do + let(:sort_option) { "contact_made_#{direction}" } + + let!(:case_contacts) do + [ + create(:case_contact, contact_made: true), + create(:case_contact, contact_made: false), + ] + end + + context "when sorting by ascending order" do + let(:direction) { "asc" } + + it { is_expected.to match_array [case_contacts[1], case_contacts[0]] } + end + + context "when sorting by descending order" do + let(:direction) { "desc" } + + it { is_expected.to match_array [case_contacts[0], case_contacts[1]] } + end + end + end + end end describe "#contact_groups_with_types" do diff --git a/spec/system/case_contacts/index_spec.rb b/spec/system/case_contacts/index_spec.rb index ffd02ff303..e63f421c4d 100644 --- a/spec/system/case_contacts/index_spec.rb +++ b/spec/system/case_contacts/index_spec.rb @@ -1,30 +1,60 @@ require "rails_helper" -RSpec.describe "case_contacts/index", :disable_bullet, type: :system do +RSpec.describe "case_contacts/index", :disable_bullet, js: true, type: :system do let(:volunteer) { create(:volunteer, display_name: "Bob Loblaw", casa_org: organization) } let(:organization) { create(:casa_org) } context "with case contacts" do let(:casa_case) { create(:casa_case, casa_org: organization, case_number: "CINA-1") } let!(:case_assignment) { create(:case_assignment, volunteer: volunteer, casa_case: casa_case) } - let!(:case_contact) { create(:case_contact, creator: volunteer, casa_case: casa_case) } before(:each) do + case_contacts + sign_in volunteer visit case_contacts_path end - it "can see case creator in card" do - expect(page).to have_text("Bob Loblaw") - end + context "without filter" do + let(:case_contacts) { [create(:case_contact, creator: volunteer, casa_case: casa_case)] } + + it "can see case creator in card" do + expect(page).to have_text("Bob Loblaw") + end + + it "can navigate to edit volunteer page" do + expect(page).to have_no_link("Bob Loblaw") + end - it "can navigate to edit volunteer page" do - expect(page).to have_no_link("Bob Loblaw") + it "displays the contact type groups" do + within(".card-title") do + expect(page).to have_text(case_contacts[0].contact_groups_with_types.keys.first) + end + end end - it "displays the contact type groups" do - within(".card-title") do - expect(page).to have_text(case_contact.contact_groups_with_types.keys.first) + describe "filtering case contacts" do + describe "by date of contact" do + let(:case_contacts) do + [ + create(:case_contact, creator: volunteer, casa_case: casa_case, occurred_at: Time.zone.yesterday - 1), + create(:case_contact, creator: volunteer, casa_case: casa_case, occurred_at: Time.zone.yesterday), + create(:case_contact, creator: volunteer, casa_case: casa_case, occurred_at: Time.zone.today) + ] + end + + it "only shows the contacts with the correct date" do + click_button "Show / Hide" + + fill_in "filterrific_occurred_starting_at", with: Time.zone.yesterday.to_s + fill_in "filterrific_occurred_ending_at", with: Time.zone.tomorrow.to_s + + click_button "Filter" + + expect(page).not_to have_content I18n.l(case_contacts[0].occurred_at.to_date, format: :long) + expect(page).to have_content I18n.l(case_contacts[1].occurred_at.to_date, format: :long) + expect(page).to have_content I18n.l(case_contacts[2].occurred_at.to_date, format: :long) + end end end end diff --git a/spec/views/case_contacts/index.html.erb_spec.rb b/spec/views/case_contacts/index.html.erb_spec.rb index c7cf9a5490..f01f1dd28a 100644 --- a/spec/views/case_contacts/index.html.erb_spec.rb +++ b/spec/views/case_contacts/index.html.erb_spec.rb @@ -2,12 +2,35 @@ RSpec.describe "case_contacts/index", :disable_bullet, type: :view do let(:user) { build_stubbed(:volunteer) } + let(:case_contacts) { CaseContact.all } + + let(:filterrific_param_set) do + param_set = Filterrific::ParamSet.new(case_contacts, {}) + param_set.select_options = { sorted_by: CaseContact.options_for_sorted_by } + + param_set + end + + let(:groups) do + user.casa_org.contact_type_groups + .joins(:contact_types) + .where(contact_types: {active: true}) + .uniq + end before do enable_pundit(view, user) + + # Allow filterrific to fetch the correct controller name + allow_any_instance_of(ActionView::TestCase::TestController).to receive(:controller_name).and_return('case_contacts') + allow(RequestStore).to receive(:read).with(:current_user).and_return(user) allow(RequestStore).to receive(:read).with(:current_organization).and_return(user.casa_org) - assign(:presenter, CaseContactPresenter.new) + + assign(:current_organization_groups, groups) + assign(:filterrific, filterrific_param_set) + assign(:presenter, CaseContactPresenter.new(case_contacts)) + render template: "case_contacts/index" end From eacfb0305cad9bf0c6d2e3d654324ef9259e1a76 Mon Sep 17 00:00:00 2001 From: Rhian Moraes Date: Fri, 18 Jun 2021 11:14:22 -0300 Subject: [PATCH 7/9] Fix linter issues --- app/controllers/case_contacts_controller.rb | 6 ++-- app/models/case_contact.rb | 36 +++++++++---------- config/locales/models.en.yml | 2 +- config/locales/views.en.yml | 2 +- spec/models/case_contact_spec.rb | 2 +- .../case_contacts/index.html.erb_spec.rb | 4 +-- 6 files changed, 25 insertions(+), 27 deletions(-) diff --git a/app/controllers/case_contacts_controller.rb b/app/controllers/case_contacts_controller.rb index 28b4e81c2a..62c9ea4dde 100644 --- a/app/controllers/case_contacts_controller.rb +++ b/app/controllers/case_contacts_controller.rb @@ -8,9 +8,9 @@ def index authorize CaseContact @current_organization_groups = current_organization.contact_type_groups - .joins(:contact_types) - .where(contact_types: {active: true}) - .uniq + .joins(:contact_types) + .where(contact_types: {active: true}) + .uniq all_contacts = policy_scope( current_organization.case_contacts.grab_all(current_user) diff --git a/app/models/case_contact.rb b/app/models/case_contact.rb index 97e90590da..751cf97975 100644 --- a/app/models/case_contact.rb +++ b/app/models/case_contact.rb @@ -47,15 +47,15 @@ class CaseContact < ApplicationRecord where("occurred_at <= ?", end_date) if end_date.present? } scope :contact_made, ->(contact_made = nil) { - where(contact_made: contact_made) if contact_made.to_s.match(/true|false/) + where(contact_made: contact_made) if /true|false/.match?(contact_made.to_s) } scope :has_transitioned, ->(has_transitioned = nil) { - if has_transitioned.to_s.match(/true|false/) + if /true|false/.match?(has_transitioned.to_s) joins(:casa_case).where(casa_cases: {transition_aged_youth: has_transitioned}) end } scope :want_driving_reimbursement, ->(want_driving_reimbursement = nil) { - if want_driving_reimbursement.to_s.match(/true|false/) + if /true|false/.match?(want_driving_reimbursement.to_s) where(want_driving_reimbursement: want_driving_reimbursement) end } @@ -81,11 +81,11 @@ class CaseContact < ApplicationRecord with_deleted if current_user.is_a?(CasaAdmin) } - scope :contact_medium, -> (medium_type) { + scope :contact_medium, ->(medium_type) { where(medium_type: medium_type) if medium_type.present? } - scope :sorted_by, -> (sort_option) { + scope :sorted_by, ->(sort_option) { direction = /desc$/.match?(sort_option) ? "desc" : "asc" case sort_option.to_s @@ -184,21 +184,19 @@ def self.options_for_sorted_by end end - private - - def self.sorted_by_params + private_class_method def self.sorted_by_params %i[ - occurred_at_asc - occurred_at_desc - contact_type_asc - contact_type_desc - medium_type_asc - medium_type_desc - want_driving_reimbursement_asc - want_driving_reimbursement_desc - contact_made_asc - contact_made_desc - ] + occurred_at_asc + occurred_at_desc + contact_type_asc + contact_type_desc + medium_type_asc + medium_type_desc + want_driving_reimbursement_asc + want_driving_reimbursement_desc + contact_made_asc + contact_made_desc + ] end end diff --git a/config/locales/models.en.yml b/config/locales/models.en.yml index 0d6ae5e7ba..3c1fe34725 100644 --- a/config/locales/models.en.yml +++ b/config/locales/models.en.yml @@ -11,4 +11,4 @@ en: want_driving_reimbursement_asc: Want driving reimbursement ('no' first) want_driving_reimbursement_desc: Want driving reimbursement ('yes' first) contact_made_asc: Contact made ('no' first) - contact_made_desc: Contact made ('yes' first) \ No newline at end of file + contact_made_desc: Contact made ('yes' first) diff --git a/config/locales/views.en.yml b/config/locales/views.en.yml index 9c33d631f1..4457d5b505 100644 --- a/config/locales/views.en.yml +++ b/config/locales/views.en.yml @@ -4,7 +4,7 @@ en: title: Case Contacts new: New Case Contact filter: - title: Filters + title: Filter by show_hide: Show / Hide submit: Filter reset: Reset filters diff --git a/spec/models/case_contact_spec.rb b/spec/models/case_contact_spec.rb index 160c0e7e42..2c96c3f561 100644 --- a/spec/models/case_contact_spec.rb +++ b/spec/models/case_contact_spec.rb @@ -405,7 +405,7 @@ let!(:case_contacts) do [ create(:case_contact, contact_made: true), - create(:case_contact, contact_made: false), + create(:case_contact, contact_made: false) ] end diff --git a/spec/views/case_contacts/index.html.erb_spec.rb b/spec/views/case_contacts/index.html.erb_spec.rb index f01f1dd28a..efdefc5915 100644 --- a/spec/views/case_contacts/index.html.erb_spec.rb +++ b/spec/views/case_contacts/index.html.erb_spec.rb @@ -6,7 +6,7 @@ let(:filterrific_param_set) do param_set = Filterrific::ParamSet.new(case_contacts, {}) - param_set.select_options = { sorted_by: CaseContact.options_for_sorted_by } + param_set.select_options = {sorted_by: CaseContact.options_for_sorted_by} param_set end @@ -22,7 +22,7 @@ enable_pundit(view, user) # Allow filterrific to fetch the correct controller name - allow_any_instance_of(ActionView::TestCase::TestController).to receive(:controller_name).and_return('case_contacts') + allow_any_instance_of(ActionView::TestCase::TestController).to receive(:controller_name).and_return("case_contacts") allow(RequestStore).to receive(:read).with(:current_user).and_return(user) allow(RequestStore).to receive(:read).with(:current_organization).and_return(user.casa_org) From 7b80c9fd427a54c6aa155ba8f393aad6c77403cc Mon Sep 17 00:00:00 2001 From: Rhian Moraes Date: Fri, 18 Jun 2021 12:10:30 -0300 Subject: [PATCH 8/9] Improve case contacts controller index action --- app/controllers/case_contacts_controller.rb | 26 +++++++++++++-------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/app/controllers/case_contacts_controller.rb b/app/controllers/case_contacts_controller.rb index 62c9ea4dde..85c341a801 100644 --- a/app/controllers/case_contacts_controller.rb +++ b/app/controllers/case_contacts_controller.rb @@ -7,18 +7,10 @@ class CaseContactsController < ApplicationController def index authorize CaseContact - @current_organization_groups = current_organization.contact_type_groups - .joins(:contact_types) - .where(contact_types: {active: true}) - .uniq - - all_contacts = policy_scope( - current_organization.case_contacts.grab_all(current_user) - .includes(:creator, contact_types: :contact_type_group) - ) + @current_organization_groups = current_organization_groups @filterrific = initialize_filterrific( - all_contacts, + all_case_contacts, params[:filterrific], select_options: { sorted_by: CaseContact.options_for_sorted_by @@ -158,4 +150,18 @@ def update_case_contact_params .new(params) .with_converted_duration_minutes(params[:case_contact][:duration_hours].to_i) end + + def current_organization_groups + current_organization.contact_type_groups + .joins(:contact_types) + .where(contact_types: {active: true}) + .uniq + end + + def all_case_contacts + policy_scope( + current_organization.case_contacts.grab_all(current_user) + .includes(:creator, contact_types: :contact_type_group) + ) + end end From aa5a8b653f725cd15dac4b10d477174e8a861c28 Mon Sep 17 00:00:00 2001 From: Rhian Moraes Date: Mon, 21 Jun 2021 11:26:15 -0300 Subject: [PATCH 9/9] Remove filterrific gem version specification and making case contacts presenter attributes more consistent --- Gemfile | 2 +- Gemfile.lock | 2 +- app/presenters/case_contact_presenter.rb | 8 +++----- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/Gemfile b/Gemfile index 8a0de6f21e..484c369db2 100644 --- a/Gemfile +++ b/Gemfile @@ -24,7 +24,7 @@ gem "skylight" # automated performance testing https://www.skylight.io/ gem "webpacker", "~> 5.4" # Transpile app-like JavaScript. Read more: https://github.com/rails/webpacker gem "image_processing", "~> 1.12" # Set of higher-level helper methods for image processing. gem "lograge" # log less so heroku papertrail quits rate limiting our logs -gem "filterrific", "~> 5.2.1" # filtering and sorting of models +gem "filterrific" # filtering and sorting of models gem "bootsnap", ">= 1.4.2", require: false # Reduces boot times through caching; required in config/boot.rb gem "bugsnag" # tracking errors in prod diff --git a/Gemfile.lock b/Gemfile.lock index 1557cff51b..a844aef242 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -451,7 +451,7 @@ DEPENDENCIES erb_lint factory_bot_rails faker - filterrific (~> 5.2.1) + filterrific image_processing (~> 1.12) jbuilder (~> 2.11) letter_opener diff --git a/app/presenters/case_contact_presenter.rb b/app/presenters/case_contact_presenter.rb index 0394b29db7..46b0cf3cc0 100644 --- a/app/presenters/case_contact_presenter.rb +++ b/app/presenters/case_contact_presenter.rb @@ -1,12 +1,10 @@ class CaseContactPresenter < BasePresenter - attr_accessor :case_contacts + attr_reader :case_contacts + attr_reader :casa_cases def initialize(case_contacts) @case_contacts = case_contacts - end - - def casa_cases - @casa_cases ||= policy_scope(org_cases).group_by(&:id).transform_values(&:first) + @casa_cases = policy_scope(org_cases).group_by(&:id).transform_values(&:first) end def display_case_number(casa_case_id)