https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422014-06-09T14:46:20ZArvadosTapestry - Task #2553: Remove hard-coded internal references to "US" and "Boston"https://dev.arvados.org/issues/2553?journal_id=112202014-06-09T14:46:20ZPhil Hodgsonphil@curoverse.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> Tapestry - Task #2553: Remove hard-coded internal references to "US" and "Boston"https://dev.arvados.org/issues/2553?journal_id=112252014-06-09T18:18:23ZPhil Hodgsonphil@curoverse.com
<ul></ul><p>I decided just to first get rid of the internal references, column names and variable names, favouring the name <code>citizen_or_resident</code> over <code>us_citizen_or_resident</code>. It was painless refactoring and seemed a good first step. Then I got to the part where the validations themselves become configurable. I got stopped up there because the validation is done in a two-step process (first save the answers then go to the next enrollment step which is reviewing the answers). So my initial instinct to use the new <code>SiteSpecific::Validations</code> override mechanism couldn't work without some modifications.</p>
<p>I see a few choices here:</p>
<ol>
<li> Leave it as is and let different sites use a fork of the code and actually modify the model itself.</li>
<li> Develop some kind of mechanism akin to the <code>SiteSpecific::Validations</code> business but which overrides model instance methods such as <code>ScreeningSurveyResponse#passed?</code> so that site-specific changes remain outside of the main source code tree as much as possible. I am wary of this. It would be powerful, but perhaps too subject to abuse. The <code>SiteSpecific::Validations</code> is more contained in how it is meant to work, which is why we feel as if it is safer to unleash on the site programmers.</li>
<li> Change the way <code>ScreeningSurveyResponse</code> is validated so that it can use a real <code>ActiveRecord</code> model validation that can be overridden with the existing <code>SiteSpecific::Validations</code> mechanism. I might suggest adding a new column to <code>ScreeningSurveyResponse</code> called <code>passed</code>, and the way to find out if something has passed is to check if that column is set to true. If it is not then the way to test is a screening survey response has passed would be to attempt to set the <code>passed</code> column to true, triggering a validation failure if it cannot be set to true because of the conditions that are considered to be a "pass". This would fit into our existing framework.</li>
</ol>
<p>I also came across some code about being able to export lists of people who failed the screening survey based on a previous version of the survey, or something. It uses an alternate scope in <code>ScreeningSurveyResponse</code> called <code>failed</code> but which isn't just the opposite of the <code>passed</code> scope but rather a whole string of reversed scopes. It's weird and really overcomplicates trying to customize what "passing" means, mostly because it's so unDRY...</p> Tapestry - Task #2553: Remove hard-coded internal references to "US" and "Boston"https://dev.arvados.org/issues/2553?journal_id=112262014-06-09T18:29:30ZPhil Hodgsonphil@curoverse.com
<ul></ul><p>There's also, though I don't know if I should suggest it, the idea of not having a "review" step in the enrollment process and doing a more straightforward validation of the responses, moving all of the various enrollment steps towards a kind of basic, customizable form with a standard kind of validation...</p>
<p>On the other hand, we may want that standard kind of validation to have the ability to be part of a two-step process, because other stages of enrollment use a "review" step too... I guess I'd push for something like solution #3 above then.</p>
<p>Either way, it seems to me that every conceivable enrollment step <em>should</em> work with the same set of rules and mechanisms. These should be flexible enough at least to handle the way all of the current enrollment steps work. The idea would be to have <em>one</em> controller in total rather than <em>one controller for every step</em>, and needing to add a new controller for every new, custom step. I see this as less powerful but easier for different sites to deal with because it is more formalized.</p> Tapestry - Task #2553: Remove hard-coded internal references to "US" and "Boston"https://dev.arvados.org/issues/2553?journal_id=112272014-06-09T18:35:46ZPhil Hodgsonphil@curoverse.com
<ul></ul><p>I've just realized that where this has led me is essentially Redmine issue <a class="issue tracker-6 status-1 priority-4 priority-default" title="Idea: Configurable mini-consent form and validations (New)" href="https://dev.arvados.org/issues/2552">#2552</a></p> Tapestry - Task #2553: Remove hard-coded internal references to "US" and "Boston"https://dev.arvados.org/issues/2553?journal_id=112322014-06-10T10:20:57ZPhil Hodgsonphil@curoverse.com
<ul><li><strong>Subject</strong> changed from <i>Configurable "us residency" validations</i> to <i>Remove hard-coded internal references to "US" and "Boston"</i></li><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li><li><strong>Remaining (hours)</strong> set to <i>0.0</i></li></ul><p>Rescoping this issue and moving the configurability to <a class="issue tracker-6 status-1 priority-4 priority-default" title="Idea: Configurable mini-consent form and validations (New)" href="https://dev.arvados.org/issues/2552">#2552</a></p> Tapestry - Task #2553: Remove hard-coded internal references to "US" and "Boston"https://dev.arvados.org/issues/2553?journal_id=112362014-06-10T10:24:53ZPhil Hodgsonphil@curoverse.com
<ul><li><strong>Estimated time</strong> set to <i>0.00 h</i></li></ul><p>Please direct comments and answers to above comments to <a class="issue tracker-6 status-1 priority-4 priority-default" title="Idea: Configurable mini-consent form and validations (New)" href="https://dev.arvados.org/issues/2552">#2552</a></p>