wth - RMID Functionalities - Part 1 #858

wtholt · 2017-01-04T15:56:06Z

Add Research Master ID field and validate field to be number only and
for presence if the Human Subjects checkbox is checked. [#136737011]

jayhardee9 · 2017-01-05T12:56:13Z

app/models/protocol.rb

@@ -260,6 +266,10 @@ def active?
    study_type_question_group.nil? ? false : study_type_question_group.active
  end

+  def has_human_subject_info?


ActiveRecord provides #human_subject_info?, so I think this method is a little redundant.

Had to add validation on the fly here in the form of a hidden field. If a user checks the Human Subject Info checkbox, it sets the has_human_subject_info hidden field to true which will then require the presence of a RMID.

This is just a way of doing it. I agree that naming conventions here aren't the best, open to any suggestions.

kayla-glick · 2017-01-05T14:44:30Z

app/assets/stylesheets/protocol_form.sass

@@ -46,3 +46,7 @@ textarea

 .gray_note 
  color: gray
+
+label.required-field:after


I actually like what you did here. I wasn't aware of this functionality. There's already a class for required fields, ".required", but it didn't have this css being used on it, and instead used a function in global.js.coffee to add the *s. Could you change this class to .required and perhaps remove the javascript function and calls to it?

kayla-glick · 2017-01-05T14:45:08Z

app/models/protocol.rb

+  attr_accessible :research_master_id
+  attr_accessible :has_human_subject_info
+
+  validates :research_master_id, numericality: true, allow_blank: true


Would this allow non-integers to be entered? Shouldn't we use numericality: { only_integer: true }

kayla-glick · 2017-01-05T14:46:33Z

app/views/protocols/form/study_form_sections/_research_involving.html.haml

@@ -29,7 +29,7 @@
      .form-group.row.human_subjects
        = rt_form.label :human_subjects, t(:protocols)[:studies][:research_involving][:humans][:header], class: 'col-lg-2 control-label'
        .col-lg-1
-          = rt_form.check_box :human_subjects, class: 'form-control'
+          = rt_form.check_box :human_subjects, class: 'form-control human-subjects'


Instead of adding the class, you can select using $("#protocol_research_types_info_attributes_human_subjects")

I think adding a class is cleaner. No real difference here.

kayla-glick · 2017-01-05T14:46:48Z

app/assets/javascripts/protocol_form.js.coffee

@@ -19,6 +19,15 @@
 # TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

 $(document).ready ->
+
+  $(document).on 'click', '.human-subjects', ->


Select using $("#protocol_research_types_info_attributes_human_subjects")

kayla-glick · 2017-01-05T14:46:59Z

app/assets/javascripts/protocol_form.js.coffee

@@ -19,6 +19,15 @@
 # TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

 $(document).ready ->
+
+  $(document).on 'click', '.human-subjects', ->
+    if $('.rm-id').hasClass('required-field')


See comments about ".required"

kayla-glick · 2017-01-05T14:48:08Z

app/views/protocols/form/study_form_sections/_research_involving.html.haml

@@ -29,7 +29,7 @@
      .form-group.row.human_subjects
        = rt_form.label :human_subjects, t(:protocols)[:studies][:research_involving][:humans][:header], class: 'col-lg-2 control-label'
        .col-lg-1
-          = rt_form.check_box :human_subjects, class: 'form-control'
+          = rt_form.check_box :human_subjects, class: 'form-control human-subjects'


Needs to be replicated in "app/views/dashboard/protocols/form/study_form_sections/_research_involving.html.haml"

kayla-glick · 2017-01-05T14:48:30Z

app/views/protocols/form/study_form_sections/_study_information.html.haml

@@ -27,6 +27,13 @@
      = t(:protocols)[:studies][:information][:subtext]
  .panel-body.container-fluid
    .form-group.row
+      = form.label :research_master_id, class: 'col-lg-2 control-label rm-id' do


Needs to be replicated in "app/views/dashboard/protocols/form/study_form_sections/_study_information.html.haml"

kayla-glick · 2017-01-05T14:49:22Z

app/views/protocols/form/study_form_sections/_study_information.html.haml

+        = link_to t(:protocols)[:studies][:information][:research_master_id], RESEARCH_MASTER_LINK
+      .col-lg-10
+        = form.text_field :research_master_id, class: 'form-control'
+        = form.hidden_field :has_human_subject_info, value: false, class: 'has-human-subject-info'


What is the purpose of this hidden field?

Have to add validation to a RMID on the fly. This is a hidden field set to default to false. If a user checks the Human Subjects Info checkbox, sets this field to true which then will require presence validation on the RMID attribute.

This was just my idea of doing it, probably other ways

kayla-glick · 2017-01-05T14:50:09Z

db/migrate/20170103155625_add_research_master_id_to_protocols.rb

@@ -0,0 +1,5 @@
+class AddResearchMasterIdToProtocols < ActiveRecord::Migration
+  def change
+    add_column :protocols, :research_master_id, :integer


Should there also be an index added here?

No association

kayla-glick · 2017-01-05T14:52:48Z

db/migrate/20170103201850_add_has_hs_info_to_protocols.rb

@@ -0,0 +1,5 @@
+class AddHasHsInfoToProtocols < ActiveRecord::Migration
+  def change
+    add_column :protocols, :has_human_subject_info, :boolean


This is unnecessary coupling and could lead to data inconsistencies. We already have access to either protocol.research_types_info.human_subjects? or protocol.human_subjects.present?

Add Research Master ID field and validate field to be number only and for presence if the Human Subjects checkbox is checked. [#136737011]

jayhardee9 approved these changes Jan 5, 2017

View reviewed changes

kayla-glick suggested changes Jan 5, 2017

View reviewed changes

wtholt force-pushed the wth-sparc-rmid-part-1 branch 3 times, most recently from 756f853 to 75af6be Compare January 9, 2017 15:21

jleonardw9 approved these changes Jan 9, 2017

View reviewed changes

wtholt force-pushed the wth-sparc-rmid-part-1 branch from 75af6be to 8a3838a Compare January 9, 2017 16:09

jleonardw9 approved these changes Jan 9, 2017

View reviewed changes

jwiel86 approved these changes Jan 9, 2017

View reviewed changes

wth - RMID Functionalities - Part 1

51e922e

Add Research Master ID field and validate field to be number only and for presence if the Human Subjects checkbox is checked. [#136737011]

wtholt force-pushed the wth-sparc-rmid-part-1 branch from 8a3838a to 51e922e Compare January 10, 2017 15:03

jleonardw9 merged commit a133f5c into master Jan 10, 2017

jleonardw9 deleted the wth-sparc-rmid-part-1 branch January 10, 2017 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

wth - RMID Functionalities - Part 1 #858

wth - RMID Functionalities - Part 1 #858

wtholt commented Jan 4, 2017

jayhardee9 Jan 5, 2017

wtholt Jan 5, 2017

kayla-glick Jan 5, 2017

kayla-glick Jan 5, 2017

kayla-glick Jan 5, 2017

wtholt Jan 5, 2017

kayla-glick Jan 5, 2017

kayla-glick Jan 5, 2017

kayla-glick Jan 5, 2017

kayla-glick Jan 5, 2017

kayla-glick Jan 5, 2017

wtholt Jan 5, 2017 •

edited

kayla-glick Jan 5, 2017

wtholt Jan 5, 2017

kayla-glick Jan 5, 2017

wth - RMID Functionalities - Part 1 #858

wth - RMID Functionalities - Part 1 #858

Conversation

wtholt commented Jan 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wtholt Jan 5, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wtholt Jan 5, 2017 •

edited