-
Notifications
You must be signed in to change notification settings - Fork 242
Add Practical Support Mode! #3050
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,10 +20,13 @@ module Statusable | |
| dropoff: { key: I18n.t('patient.status.key.dropoff'), | ||
| help_text: I18n.t('patient.status.help.dropoff')}, | ||
| resolved: { key: I18n.t('patient.status.key.resolved', fund: ActsAsTenant.current_tenant&.name), | ||
| 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') }, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do we use
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| }.freeze | ||
|
|
||
| def status | ||
| return STATUSES[:completed][:key] if practical_support_completed? | ||
| return STATUSES[:fulfilled][:key] if fulfillment.fulfilled? | ||
| return STATUSES[:resolved][:key] if resolved_without_fund? | ||
| return STATUSES[:pledge_unfulfilled][:key] if days_since_pledge_sent > 150 | ||
|
|
@@ -36,6 +39,10 @@ def status | |
|
|
||
| private | ||
|
|
||
| def practical_support_completed? | ||
| Config.practical_support_mode? && marked_complete? | ||
| end | ||
|
|
||
| def contact_made? | ||
| calls.each do |call| | ||
| return true if call.reached_patient? | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -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.' | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| }.freeze | ||||||
|
|
||||||
| enum config_key: { | ||||||
|
|
@@ -46,18 +47,18 @@ class Config < ApplicationRecord | |||||
| aggregate_statistics: 17, | ||||||
| hide_standard_dropdown_values: 18, | ||||||
| county: 19, | ||||||
| time_zone: 20 | ||||||
| time_zone: 20, | ||||||
| practical_support_mode: 21, | ||||||
| } | ||||||
|
|
||||||
| # which fields are URLs (run special validation only on those) | ||||||
|
|
||||||
| # symbols are required here because functions are not objects in rails :) | ||||||
| CLEAN_PRE_VALIDATION = { | ||||||
| start_of_week: [:fix_capitalization], | ||||||
| hide_practical_support: [:fix_capitalization], | ||||||
| language: [:fix_capitalization], | ||||||
| county: [:fix_capitalization], | ||||||
| time_zone: [:titleize_capitalization] | ||||||
| time_zone: [:titleize_capitalization], | ||||||
| practical_support_mode: [:fix_capitalization], | ||||||
| }.freeze | ||||||
|
|
||||||
| VALIDATIONS = { | ||||||
|
|
@@ -82,7 +83,7 @@ class Config < ApplicationRecord | |||||
| [:validate_singleton, :validate_time_zone], | ||||||
|
|
||||||
| hide_practical_support: | ||||||
| [:validate_singleton, :validate_yes_or_no], | ||||||
| [:validate_yes_or_no, :validate_only_one_practical_support], | ||||||
|
|
||||||
| budget_bar_max: | ||||||
| [:validate_singleton, :validate_number], | ||||||
|
|
@@ -102,11 +103,13 @@ class Config < ApplicationRecord | |||||
| [:validate_singleton, :validate_shared_reset], | ||||||
|
|
||||||
| hide_budget_bar: | ||||||
| [:validate_singleton, :validate_yes_or_no], | ||||||
| [:validate_yes_or_no], | ||||||
| aggregate_statistics: | ||||||
| [:validate_singleton, :validate_yes_or_no], | ||||||
| [:validate_yes_or_no], | ||||||
| hide_standard_dropdown_values: | ||||||
| [:validate_singleton, :validate_yes_or_no], | ||||||
| [:validate_yes_or_no], | ||||||
| practical_support_mode: | ||||||
| [:validate_yes_or_no, :validate_only_one_practical_support], | ||||||
| }.freeze | ||||||
|
|
||||||
| before_validation :clean_config_value | ||||||
|
|
@@ -194,6 +197,10 @@ def self.hide_standard_dropdown? | |||||
| config_to_bool('hide_standard_dropdown_values') | ||||||
| end | ||||||
|
|
||||||
| def self.practical_support_mode? | ||||||
| config_to_bool('practical_support_mode') | ||||||
| end | ||||||
|
|
||||||
| private | ||||||
| ### Generic Functions | ||||||
|
|
||||||
|
|
@@ -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 | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. great rails |
||||||
| end | ||||||
|
|
||||||
| ### Patient archive | ||||||
|
|
@@ -335,4 +342,10 @@ def validate_length | |||||
| end | ||||||
| true | ||||||
| end | ||||||
|
|
||||||
| def validate_only_one_practical_support | ||||||
| # TODO - Should not be able to use both practical support configs at the | ||||||
| # same time. | ||||||
| true | ||||||
| end | ||||||
|
Comment on lines
+345
to
+350
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
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? |
||||||
| end | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| <% if not patient.marked_complete %> | ||
| <div class="modal-header"> | ||
| <h4 class="modal-title"><%= t('patient.complete.title') %></h4> | ||
| </div> | ||
|
|
||
| <div class="modal-body"> | ||
| <div class="row"> | ||
| <%= t('patient.complete.body') %> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div class="modal-footer"> | ||
| <div class="col"> | ||
| <button type="button" class="btn btn-outline-secondary js-btn-step float-left" style="color:red;" data-dismiss="modal"><%= t('common.no') %></button> | ||
| </div> | ||
|
|
||
| <div class="col"> | ||
| <%= bootstrap_form_with model: @patient, | ||
| html: { id: 'submit-pledge-form' }, | ||
| local: false, | ||
| method: 'patch', | ||
| class: 'edit_patient' do |f| %> | ||
| <%= f.hidden_field :marked_complete, value: true %> | ||
| <%= f.submit t('common.yes'), class: "btn btn-success js-btn-step float-right", id: 'submit-pledge-submit'%> | ||
| <% end %> | ||
| </div> | ||
| </div> | ||
| <% else %> | ||
| <div class="modal-header"> | ||
| <h4 class="modal-title"><%= t('patient.complete.cancel.title') %></h4> | ||
| </div> | ||
|
|
||
| <div class="modal-body"> | ||
| <div class="row"> | ||
| <p><%= t('patient.complete.cancel.confirm') %></p> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div class="modal-footer"> | ||
| <div class="col"> | ||
| <button type="button" class="btn btn-outline-secondary js-btn-step float-left" style="color:red;" data-dismiss="modal"><%= t('common.no') %></button> | ||
| </div> | ||
|
|
||
| <div class="col"> | ||
| <%= bootstrap_form_with model: @patient, | ||
| html: { id: 'cancel-pledge-form' }, | ||
| local: false, | ||
| method: 'patch', | ||
| class: 'edit_patient' do |f| %> | ||
| <%= f.hidden_field :marked_complete, value: false %> | ||
| <%= f.submit t('common.yes'), class: "btn btn-success js-btn-step float-right", id: 'cancel-pledge-submit'%> | ||
| <% end %> | ||
| </div> | ||
| </div> | ||
| <% end %> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,29 @@ | ||
| <% if not patient.pledge_sent? %> | ||
| <%= link_to submit_pledge_button, | ||
| submit_pledge_path(patient), | ||
| class: 'submit-pledge-button', | ||
| aria: { label: t('patient.menu.submit_pledge') }, | ||
| remote: true %> | ||
| <% 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 %> | ||
|
Comment on lines
+1
to
15
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same question here as with the button controller function - very similar code between in/complete and submit/cancel.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think reasonable to collapse this one if you like. Up to you! |
||
| <%= link_to cancel_pledge_button, | ||
| submit_pledge_path(patient), | ||
| class: 'submit-pledge-button', | ||
| aria: { label: t('patient.menu.cancel_pledge') }, | ||
| remote: true %> | ||
| <% end %> | ||
| <% if not patient.pledge_sent? %> | ||
| <%= link_to submit_pledge_button, | ||
| submit_pledge_path(patient), | ||
| class: 'submit-pledge-button', | ||
| aria: { label: t('patient.menu.submit_pledge') }, | ||
| remote: true %> | ||
| <% else %> | ||
| <%= link_to cancel_pledge_button, | ||
| submit_pledge_path(patient), | ||
| class: 'submit-pledge-button', | ||
| aria: { label: t('patient.menu.cancel_pledge') }, | ||
| remote: true %> | ||
| <% end %> | ||
| <% end %> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ergh i hate this but is it ok?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this reads as ok to me, what's the issue? |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| $('.modal-content').html('') | ||
| $('.modal').attr('id', 'pledge-modal') | ||
| $('.modal-content').append( | ||
| '<%= escape_javascript(render partial: "patients/complete", locals: { patient: @patient, disable_next: disable_continue?(@patient) })%>' | ||
| ); | ||
|
|
||
| $('.modal').modal('toggle'); | ||
|
|
||
| <% if ([email protected]_complete) %> | ||
|
|
||
| $('#pledge-modal').modalSteps(); | ||
|
|
||
| <% else %> | ||
| $('#cancel-pledge-form').on('submit', function() { | ||
| $('.modal').modal('hide'); | ||
| }) | ||
| <% end %> |

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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say keep separate - reasonable for them to diverge a bit here