Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#1071] Add filters for case contacts #2166

Merged
merged 9 commits into from
Jun 23, 2021

Conversation

rhian-cs
Copy link
Contributor

What github issue is this PR for, if any?

Resolves #1071

What changed, and why?

Volunteers can now filter and sort the case contacts for each of their cases.

Volunteers should be able to filter by

  • date ranges
  • contact_type_group
  • case_contact_contact_type
  • contact_made (yes/no)
  • contact_medium
  • want_driving_reimbursement (yes/no)
  • Volunteers can sort all of the above by ascending and descending order.

I did not add a contact type group filter since the contact type filter already allows the user to choose by group.

Acceptance criteria

  • A button labeled "Filtered by" displays in the new case_contacts table header viewable by volunteers (see new header for case contacts #696)
  • Clicking the button displays all "Filter by" options
  • Volunteer can filter by any combination of conditions above
  • It is easy for the volunteer to close the "Filter by" menu after making their selections

I didn't strictly follow the suggested design, because the new case contacts layout is not yet implemented, but I can always change the view if something is undesirable.

How will this affect user permissions?

No permissions have been affected.

How is this tested? (please write tests!) 💖💪

Automated Testing

I've added specs for the new scopes and some basic system specs to check if a filter is working.

Manual Testing

  • Go to the case contacts page
  • At the right side of the "Filter by" card, click on Show/Hide.
  • Select the desired filters and click on "Filter".
  • After clicking on the "Filter" button, the "Filter by" card should remain expanded
  • Check if the filters and the sorting work as expected

Screenshots please :)

Overview (Desktop)

01-overview

Filter section (Desktop)

02-filters-expanded

Overview (Mobile)

03-overview-mobile

Filter section (Mobile)

04-full-filter-mobile

@github-actions github-actions bot added dependencies Pull requests that update a dependency file erb ruby Pull requests that update Ruby code Tests! 🎉💖👏 labels Jun 18, 2021
Copy link
Collaborator

@compwron compwron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay tests :)

Gemfile Outdated
@@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't specifi gem version unless there is an error with a future version (and if so, add a comment about why it's pinned) because pinning prevents automatic upgrades

this gem was last updated in 2019 so I am not super enthusiastic about using it but it does seem stable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I'll remove the version specification.

I thought it was good practice to always pin the versions to make it more stable haha

}
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 /true|false/.match?(has_transitioned.to_s)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regex rather than boolean? why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used regex because the filter coming from the request will always be a string (and the previous if didn't work with it)

That was the best solution I could think of at the moment

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this case stability ~= fragility, we have been instead choosing flexibility and trusting the tests... so we need lots of tests :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, very interesting.

@@ -64,6 +81,41 @@ class CaseContact < ApplicationRecord
with_deleted if current_user.is_a?(CasaAdmin)
}

scope :contact_medium, ->(medium_type) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice scope :)

end
end

private_class_method def self.sorted_by_params
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

[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)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

||= is overkill after initialize

@compwron compwron merged commit 697edc4 into rubyforgood:main Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file erb ruby Pull requests that update Ruby code Tests! 🎉💖👏
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Volunteers can filter case_contact view
2 participants