Add Practical Support Mode! #3050
Conversation
There was a problem hiding this comment.
this is fabulous stuff, thank you @elimbaum ! Very clean effective rails and you've hit the vast majority of the spots where we'd need to do this. I think we'll also want to add this to exports and the accounting panel but both of those can be done in separate PRs if you think this one is too busy. I'll pull down and acid test more extensively once tests are in place if that's cool with you, let me know whether there's anything in particular you'd like my opinion on.
| @@ -308,7 +315,7 @@ def validate_time_zone | |||
|
|
|||
| def validate_yes_or_no | |||
| # allow yes or no, to be nice (technically only yes is considered) | |||
| options.last =~ /\A(yes|no)\z/i | |||
| validate_singleton and options.last =~ /\A(yes|no)\z/i | |||
| @@ -22,7 +22,8 @@ class Config < ApplicationRecord | |||
| hide_budget_bar: 'Enter "yes" to hide the budget bar display.', | |||
| aggregate_statistics: 'Enter "yes" to show aggregate statistics on the budget bar.', | |||
| hide_standard_dropdown_values: 'Enter "yes" to hide standard dropdown values. Only custom options (specified on this page) will be used.', | |||
| time_zone: "Time zone to use for displaying dates. Default is Eastern. Valid options are Eastern, Central, Mountain, Pacific, Alaska, Hawaii, Arizona, Indiana (East), or Puerto Rico." | |||
| time_zone: "Time zone to use for displaying dates. Default is Eastern. Valid options are Eastern, Central, Mountain, Pacific, Alaska, Hawaii, Arizona, Indiana (East), or Puerto Rico.", | |||
| practical_support_mode: 'Enter "yes" to enable practical-support-only mode. Pledges will be disabled.' | |||
There was a problem hiding this comment.
| practical_support_mode: 'Enter "yes" to enable practical-support-only mode. Pledges will be disabled.' | |
| practical_support_mode: 'Enter "yes" to enable practical-support-only mode. Pledges will be disabled, in in favor of marking people complete/incomplete.' |
|
Thanks for review @colinxfleming! am moving this week (and starting school...) but once I get a dev environment spun back up I'll close this out! |
| def mark_complete_button | ||
| content_tag :span, class: 'btn btn-primary btn-lg submit-btn btn-block', | ||
| aria: { hidden: true }, | ||
| id: 'submit-pledge-button' do | ||
| t('patient.menu.mark_complete') | ||
| end | ||
| end | ||
|
|
||
| def mark_incomplete_button | ||
| content_tag :span, class: 'btn btn-warning btn-lg cancel-btn btn-block', | ||
| aria: { hidden: true }, | ||
| id: 'submit-pledge-button' do | ||
| t('patient.menu.mark_incomplete') | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
These two functions look extremely similar to the submit pledge button pair (above). Thoughts on condensing, or fine to keep separate?
There was a problem hiding this comment.
I'd say keep separate - reasonable for them to diverge a bit here
| help_text: I18n.t('patient.status.help.resolved')} | ||
| help_text: I18n.t('patient.status.help.resolved')}, | ||
| completed: { key: I18n.t('patient.status.key.completed'), | ||
| help_text: I18n.t('patient.status.help.completed') }, |
There was a problem hiding this comment.
What do we use help_text for? I wasn't able to find a reference
There was a problem hiding this comment.
These show up in the Status tooltip on patient records!
|
|
||
| def validate_only_one_practical_support | ||
| # TODO - Should not be able to use both practical support configs at the | ||
| # same time. | ||
| true | ||
| end |
There was a problem hiding this comment.
After this is done, I'm thinking about opening a separate issue to combine "practical support mode" and "hide practical supports" into a single option — practical supports only, on, off. in the interim, should we have some validation? Weird things will happen if we turn on both options.
How would that best work? We can't compare the actual saved values because on validation, the submitted value has not yet been committed. We could have a conditional check based on the option key?
There was a problem hiding this comment.
Let me say a bit more about the thinking here, and let's see if that makes sense with how you're thinking about it. I think there are three possible states:
- fund is not doing practical support at all (in which case we'd want to hide these options) - unusual but it happens
- fund is doing pledges AND practical support - most of our clientele
- fund is doing zero pledges, ONLY practical support - unusual but it happens
So I think you're def right that 'hide practical support on' and 'practical support only on' doesn't make sense, but I think the other cases (hide off, practical only on; hide on, practical only off; hide off, practical only off) are cases I think we'd like to support. Does that match your understanding?
There was a problem hiding this comment.
what do you think about this modal? i don't know if we need any confirmation steps, so maybe we don't even need the modal - just the button?
There was a problem hiding this comment.
I think yes to the modal -- an extra UX step to signify 'this is a significant action' is helpful (and helps prevent problems from misclicks)
| <% if Config.practical_support_mode? %> | ||
| <% if not patient.marked_complete? %> | ||
| <%= link_to mark_complete_button, | ||
| mark_complete_path(patient), | ||
| class: 'submit-pledge-button', # reuse classes from submit pledge button | ||
| aria: { label: t('patient.menu.mark_complete') }, | ||
| remote: true %> | ||
| <% else %> | ||
| <%= link_to mark_incomplete_button, | ||
| mark_complete_path(patient), | ||
| class: 'submit-pledge-button', | ||
| aria: { label: t('patient.menu.mark_incomplete') }, | ||
| remote: true %> | ||
| <% end %> | ||
| <% else %> |
There was a problem hiding this comment.
same question here as with the button controller function - very similar code between in/complete and submit/cancel.
There was a problem hiding this comment.
I think reasonable to collapse this one if you like. Up to you!
There was a problem hiding this comment.
ergh i hate this but is it ok?
There was a problem hiding this comment.
this reads as ok to me, what's the issue?
I rule and have completed some work on Case Manager that's ready for review!
This pull request makes the following changes:
marked_completefield in the Patient model.Submit Pledgebutton toMark Complete, and updates the confirmation modals:Currently, there is no valid for marking a patient complete, as there is for fulifilling a pledge; this could be changed.
DRAFT until tests are added.
It relates to the following issue #s:
For reviewer:
featureif it contains a feature, fix, or similar. This is anything that contains a user-facing fix in some way, such as frontend changes, alterations to backend behavior, or bug fixes.dependenciesif it contains library upgrades or similar. This is anything that upgrades any dependency, such as a Gemfile update or npm package upgrade.