-
-
Notifications
You must be signed in to change notification settings - Fork 476
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
[#1071] Add filters for case contacts #2166
Conversation
There was a problem hiding this 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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
||= is overkill after initialize
… presenter attributes more consistent
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
I did not add a contact type group filter since the contact type filter already allows the user to choose by group.
Acceptance criteria
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
Screenshots please :)
Overview (Desktop)
Filter section (Desktop)
Overview (Mobile)
Filter section (Mobile)