Search filters for overdue patients #907
Conversation
This adds https://github.com/guard/guard-rspec, which wire up guard and guard-rspec to get continual, fast testing while working locally.
This is all generated by the guard-rspec setup, we can modify it as we see fit to match our project and conventions.
Allows running just specific specs by changing "it" to "fit" in rspec
Need this for the new sidebar with search filters
Removing unused filters; adjusting copy
This is tricky because they are in different forms, so we need to use hidden fields to keep the state. We should probably just merge the forms if at all possible here.
Ugh this was gnarly. Forms that are GETs ignore the query string in their action, so we *have* to pass around the state via hidden field tags.
This all should be another object, but not until we know where else these filters are going to be used
| @@ -38,6 +41,37 @@ def update | |||
|
|
|||
| private | |||
|
|
|||
| def apply_search_filters | |||
There was a problem hiding this comment.
Here is the query logic - eventually I'd like to have this in a PatientSummarySearch object or something, but want to know where else we are going to be searching patients like this w/ other dashboards.
| @@ -0,0 +1,99 @@ | |||
| <% content_for(:content) do %> | |||
There was a problem hiding this comment.
This new layout was mostly modeled after the my facilities layout, which has a similar sidebar.
spec/spec_helper.rb
Outdated
| @@ -11,6 +11,9 @@ | |||
| RSpec.configure do |config| | |||
| SimpleCov.start | |||
|
|
|||
| config.filter_run focus: true | |||
| config.run_all_when_everything_filtered = true | |||
There was a problem hiding this comment.
Just making rspec smarter so it respects focused specs. Makes guard much nicer to use.
rsanheim
requested review from
harimohanraj89,
timcheadle and
kitallis
and removed request for
kitallis
| where(status: 'scheduled') | ||
| .where(arel_table[:scheduled_date].lt(Date.current)) | ||
| .where(arel_table[:remind_on].eq(nil).or(arel_table[:remind_on].lteq(Date.current))) | ||
| end |
There was a problem hiding this comment.
Now that we've got this in here, I'd love to update the overdue spec to use this one.
There was a problem hiding this comment.
The other option related to this conversation is that we introduce a lost_to_followup scope, and we union or or the scopes based on checkbox selections.
There was a problem hiding this comment.
@rsanheim, <bump>, should we update the overdue scope to use this method?
And simplify the default search filter logic in appointments controller
|
Didn’t know about that. Thanks!
… On May 6, 2020, at 03:26, Prabhanshu Gupta ***@***.***> wrote:
@prabhanshuguptagit commented on this pull request.
In app/views/layouts/overdue.html.erb:
> + </div>
+ </div>
+ </div>
+ </div>
+
+ <!---------------------
+ DESKTOP NAV
+ --------------------->
+ <div class="row">
+ <div class="col-lg-2 pl-lg-0 desktop">
+ <section class="secondary-nav-filters">
+ <h4>Patients</h4>
+
+ <%= form_tag appointments_path, method: :get do %>
+ <%= hidden_field_tag :facility_id, @facility_id %>
+ <%= hidden_field_tag :per_page, @per_page %>
Could setting these inputs as <input form="sidebar-form-id"> solve this? It allows you to have an input field belonging to a form outside the form.
This is being done in My Facilities > BP control period selection, for example: There are "two forms with shared state" but effectively they're the same form thanks to the form attribute.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
This overrides the facility / pagination select forms so that they post to the search filters form when on the Overdue listing. This means everything goes through the same form for this page, which makes managing the state much easier, and we don't need hidden fields leaking into both forms. This also fixes up the filters to work more better in the mobile view.
these cancel each other out, effectively
There was a problem hiding this comment.
Only one outstanding comment from me. Pre-emptive approval for unblockage 🚀
These should really be renamed soon if we agree on the "lost to follow up" terminology
Ah yup, took care of that. 👍 |
Story card: https://www.pivotaltracker.com/n/projects/2184102/stories/172537522/
Because
We want more filtering abilities on the overdue list.
This addresses
This adds search filters to the overdue listing. I ended up extracting to a small query class to keep things a bit cleaner.
Because there are two different filters w/ different forms on the page, we needed to add hidden field tags to maintain state. See the inline code comments for details. It isn't ideal, but combining the forms was beyond my bootstrap / layout abilities.
TODO
formattribute to resolve two forms problem