Skip to content
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

Y24-481 - Study setup page changes #4541

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions app/models/study.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,24 @@ class Study < ApplicationRecord # rubocop:todo Metrics/ClassLength
DATA_RELEASE_TIMING_IMMEDIATE,
DATA_RELEASE_TIMING_DELAYED
].freeze
DATA_RELEASE_PREVENTION_REASONS = ['data validity', 'legal', 'replication of data subset'].freeze

DATA_RELEASE_PREVENTION_REASONS = [
'Pilot or validation studies - DAC approval not required',
'Collaborators will share data in a research repository - DAC approval not required',
'Prevent harm (e.g sensitive studies or biosecurity) - DAC approval required',
'Protecting IP - DAC approval required',
'Other (please specify)'
yoldas marked this conversation as resolved.
Show resolved Hide resolved
].freeze

DATA_RELEASE_DELAY_FOR_OTHER = 'other'
DATA_RELEASE_DELAY_REASONS_STANDARD = ['phd study', DATA_RELEASE_DELAY_FOR_OTHER].freeze
DATA_RELEASE_DELAY_REASONS_ASSAY = ['phd study', 'assay of no other use', DATA_RELEASE_DELAY_FOR_OTHER].freeze
DATA_RELEASE_DELAY_REASONS_STANDARD = [
'PhD study',
'Capacity building',
'Intellectual property protection',
'Additional time to make data FAIR',
DATA_RELEASE_DELAY_FOR_OTHER
BenTopping marked this conversation as resolved.
Show resolved Hide resolved
].freeze
DATA_RELEASE_DELAY_REASONS_ASSAY = ['assay of no other use', *DATA_RELEASE_DELAY_REASONS_STANDARD].freeze

DATA_RELEASE_DELAY_PERIODS = ['3 months', '6 months', '9 months', '12 months', '18 months'].freeze

Expand Down Expand Up @@ -217,6 +230,7 @@ class Study < ApplicationRecord # rubocop:todo Metrics/ClassLength
in: DATA_RELEASE_DELAY_REASONS_ASSAY,
if: :delayed_release?
)

custom_attribute(:data_release_delay_period, required: true, in: DATA_RELEASE_DELAY_PERIODS, if: :delayed_release?)
custom_attribute(:bam, default: true)

Expand All @@ -235,10 +249,10 @@ class Study < ApplicationRecord # rubocop:todo Metrics/ClassLength
custom_attribute(:data_release_delay_approval, in: YES_OR_NO, default: NO)
end

with_options(if: :never_release?, required: true) do
custom_attribute(:data_release_prevention_reason, in: DATA_RELEASE_PREVENTION_REASONS)
custom_attribute(:data_release_prevention_approval, in: YES_OR_NO)
custom_attribute(:data_release_prevention_reason_comment)
with_options(if: :never_release?) do
custom_attribute(:data_release_prevention_reason, in: DATA_RELEASE_PREVENTION_REASONS, required: true)
yoldas marked this conversation as resolved.
Show resolved Hide resolved
custom_attribute(:data_release_prevention_reason_comment, required: true)
custom_attribute(:data_release_prevention_approval)
end

# NOTE: Additional validation in Study::Metadata Class to validate_presence_of :data_access_group, if: :managed
Expand Down
6 changes: 3 additions & 3 deletions app/views/shared/metadata/edit/_study.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@
%>

<% metadata_fields.related_fields(to: :data_release_strategy, when: Study::DATA_RELEASE_STRATEGY_NOT_APPLICABLE) do %>
<%= group.select(:data_release_prevention_reason, Study::DATA_RELEASE_PREVENTION_REASONS) %>
<%= group.radio_select(:data_release_prevention_approval, Study::YES_OR_NO) %>
<%= group.select(:data_release_prevention_reason, [''] + Study::DATA_RELEASE_PREVENTION_REASONS) %>
yoldas marked this conversation as resolved.
Show resolved Hide resolved
<%= group.text_area(:data_release_prevention_approval) %>
<%= group.text_area(:data_release_prevention_reason_comment) %>
<% end %>
<% metadata_fields.related_fields(to: :data_release_strategy, in: Study::DATA_RELEASE_STRATEGIES, not: Study::DATA_RELEASE_STRATEGY_NOT_APPLICABLE) do %>
<% metadata_fields.related_fields(to: :data_release_timing, when: Study::DATA_RELEASE_TIMING_DELAYED) do %>
<%= group.select(:data_release_delay_reason, Study::DATA_RELEASE_DELAY_REASONS_STANDARD) %>
<%= group.select(:data_release_delay_reason, [''] + Study::DATA_RELEASE_DELAY_REASONS_STANDARD) %>
yoldas marked this conversation as resolved.
Show resolved Hide resolved
yoldas marked this conversation as resolved.
Show resolved Hide resolved
<%
group.change_select_options_for(:data_release_delay_reason, when: :data_release_study_type_id, values: {
DataReleaseStudyType.assay_types.map(&:id) => [ '' ] + Study::DATA_RELEASE_DELAY_REASONS_ASSAY,
Expand Down
7 changes: 3 additions & 4 deletions config/locales/metadata/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ en:
values:
open: "Open (ENA)"
managed: "Managed (EGA)"
not_applicable: "Not Applicable (Contact Datasharing)"
not_applicable: "Not Applicable"

data_release_standard_agreement:
label: "Will you be using WTSI's standard access agreement?"
Expand All @@ -432,21 +432,20 @@ en:

data_release_timing:
label: How is the data release to be timed?
help: "Choose from:<p><strong>Immediate:</strong> To be released as soon as possible.</p><p><strong>Standard:</strong> To be released:<ul><li>For managed (EGA) studies: 6 months</li><li>For open (ENA) studies: 12 months</li><li>Transcriptomics studies: on request only</li></ul></p><p><strong>Delayed:</strong><ul><li>For managed (EGA) studies: 6 months plus delay for period</li><li>For open (ENA) studies: 12 months plus delay for period.</li></ul></p><p><strong>Never:</strong> This option is only available if the data release strategy is set to 'Not applicable.'</p>"
help: "Choose from:<p><strong>Immediate:</strong> To be released as soon as possible.</p><p><strong>Standard:</strong> To be released:<ul><li>For managed (EGA) studies: 12 months</li><li>For open (ENA) studies: 12 months</li><li>Transcriptomics studies: 12 months</li></ul></p><p><strong>Delayed:</strong><ul><li>For managed (EGA) studies: 12 months plus delay for period</li><li>For open (ENA) studies: 12 months plus delay for period.</li></ul></p><p><strong>Never:</strong> This option is only available if the data release strategy is set to 'Not applicable.'</p>"
BenTopping marked this conversation as resolved.
Show resolved Hide resolved
BenTopping marked this conversation as resolved.
Show resolved Hide resolved

data_release_prevention_reason:
label: What is the reason for preventing data release?

data_release_prevention_approval:
label: Has this been approved?
label: If reason for exemption requires DAC approval, what is the approval number?
help: "If this is for data validity reasons: approval from the sponsor is required<br />If this is for legal reasons: approval from the Data Sharing Committee is required (please contact sd4)<br />"

data_release_prevention_reason_comment:
label: Comment regarding prevention of data release and approval

data_release_delay_reason:
label: Reason for delaying release
help: 'To apply for a delay, please contact <a title="<%= configatron.data_sharing_contact.name %>" href="mailto:<%= configatron.data_sharing_contact.email %>"><%= configatron.data_sharing_contact.email %></a>'
BenTopping marked this conversation as resolved.
Show resolved Hide resolved

data_release_delay_other_comment:
label: Please explain the reason for delaying release
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ Feature: The XML for the sequencescape API
<value>Not specified</value>
</descriptor>
<descriptor>
<name>Has this been approved?</name>
<name>If reason for exemption requires DAC approval, what is the approval number?</name>
<value></value>
</descriptor>
<descriptor>
Expand Down
9 changes: 3 additions & 6 deletions features/studies/8447221_data_release_help_text.feature
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,17 @@ Feature: Update the data release fields for creating a study
When I choose "<release strategy>" from "What is the data release strategy for this study?"
When I select "delayed" from "How is the data release to be timed?"
When I select "other" from "Reason for delaying release"
Then the help text for "Reason for delaying release" should contain:
"""
To apply for a delay, please contact [email protected]
"""
Then I should exactly see "Reason for delaying release"

Examples:
| release strategy |
| Managed (EGA) |
| Open (ENA) |

Scenario: Add help text to has this been approved for never release (4044343)
When I choose "Not Applicable (Contact Datasharing)" from "What is the data release strategy for this study?"
When I choose "Not Applicable" from "What is the data release strategy for this study?"
When I select "never" from "How is the data release to be timed?"
Then the help text for "Has this been approved?" should contain:
Then the help text for "If reason for exemption requires DAC approval, what is the approval number?" should contain:
"""
If this is for data validity reasons: approval from the sponsor is required
If this is for legal reasons: approval from the Data Sharing Committee is required (please contact sd4)
Expand Down
12 changes: 7 additions & 5 deletions features/studies/data_release_timings.feature
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Feature: Studies have timings for release of their data

Scenario: When the data release is delayed for PhD study
Given I select "delayed" from "How is the data release to be timed?"
And I select "phd study" from "Reason for delaying release"
And I select "PhD study" from "Reason for delaying release"
Then the "Comment regarding data release timing and approval" field is hidden
When I select "6 months" from "Delay for"
And I press "Create"
Expand Down Expand Up @@ -70,19 +70,21 @@ Feature: Studies have timings for release of their data
| 12 months |

Scenario: When the data release is never but the comment is not supplied
When I choose "Not Applicable (Contact Datasharing)" from "What is the data release strategy for this study?"
When I choose "Not Applicable" from "What is the data release strategy for this study?"
And I select "never" from "How is the data release to be timed?"
And I choose "Yes" from "Has this been approved?"
And I select "Protecting IP - DAC approval required" from "What is the reason for preventing data release?"
And I fill in "If reason for exemption requires DAC approval, what is the approval number?" with "12345"
When I press "Create"
Then I should be on the studies page
# Again, ideally without study metadata
And I should see "Study metadata data release prevention reason comment can't be blank"

Scenario: When the data release is never and the comment is supplied
When I choose "Not Applicable (Contact Datasharing)" from "What is the data release strategy for this study?"
When I choose "Not Applicable" from "What is the data release strategy for this study?"
And I select "never" from "How is the data release to be timed?"
And I select "Protecting IP - DAC approval required" from "What is the reason for preventing data release?"
And I fill in "Comment regarding prevention of data release and approval" with "Some reason"
And I choose "Yes" from "Has this been approved?"
And I fill in "If reason for exemption requires DAC approval, what is the approval number?" with "12345"
When I press "Create"
Then I should be on the study information page for "Testing data release strategies"
And I should see "Your study has been created"
2 changes: 1 addition & 1 deletion spec/factories/study_factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
new_field_values = {
data_release_strategy: 'not applicable',
data_release_timing: 'never',
data_release_prevention_reason: 'data validity',
data_release_prevention_reason: 'Protecting IP - DAC approval required',
data_release_prevention_approval: 'Yes',
data_release_prevention_reason_comment: 'This is the reason context'
}
Expand Down
6 changes: 3 additions & 3 deletions spec/features/studies/create_study_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
within_fieldset('What is the data release strategy for this study?') do
expect(page).to have_field('Open (ENA)', type: :radio)
expect(page).to have_field('Managed (EGA)', type: :radio)
expect(page).to have_field('Not Applicable (Contact Datasharing)', type: :radio)
expect(page).to have_field('Not Applicable', type: :radio)
end

within_fieldset('Study Visibility') do
Expand Down Expand Up @@ -132,8 +132,8 @@
expect(page).to have_field('HuMFre approval number', type: :text)
end

it 'displays HuMFre approval number when Not Applicable (Contact Datasharing) is clicked' do
choose('Not Applicable (Contact Datasharing)', allow_label_click: true)
it 'displays HuMFre approval number when Not Applicable is clicked' do
choose('Not Applicable', allow_label_click: true)
expect(page).to have_field('HuMFre approval number', type: :text)
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/features/studies/edit_study_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@
expect(page).to have_field('HuMFre approval number', type: :text)
end

it 'displays HuMFre approval number when Not Applicable (Contact Datasharing) is clicked' do
choose('Not Applicable (Contact Datasharing)', allow_label_click: true)
it 'displays HuMFre approval number when Not Applicable is clicked' do
choose('Not Applicable', allow_label_click: true)
expect(page).to have_field('HuMFre approval number', type: :text)
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/features/studies/manage_study_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
expect(page).to have_field('HuMFre approval number', type: :text)
end

it 'displays HuMFre approval number when Not Applicable (Contact Datasharing) is clicked' do
choose('Not Applicable (Contact Datasharing)', allow_label_click: true)
it 'displays HuMFre approval number when Not Applicable is clicked' do
choose('Not Applicable', allow_label_click: true)
expect(page).to have_field('HuMFre approval number', type: :text)
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/features/studies/view_study_properties_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
expect(page).to have_content('HuMFre approval number: 12345')
end

it 'displays HuMFre approval number for Not Applicable (Contact Datasharing) data release strategy' do
it 'displays HuMFre approval number for Not Applicable data release strategy' do
study.study_metadata.data_release_strategy = Study::DATA_RELEASE_STRATEGY_NOT_APPLICABLE
study.study_metadata.data_release_timing = Study::DATA_RELEASE_TIMING_NEVER
study.study_metadata.data_release_prevention_reason = Study::DATA_RELEASE_PREVENTION_REASONS[0]
Expand Down
4 changes: 2 additions & 2 deletions spec/models/study_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@
data_release_strategy: 'open',
data_release_standard_agreement: 'Yes',
data_release_timing: 'standard',
data_release_delay_reason: 'phd study',
data_release_delay_reason: 'PhD study',
data_release_delay_period: '3 months',
bam: true,
data_release_delay_other_comment: 'Data Release delay other comment',
Expand All @@ -456,7 +456,7 @@
ega_policy_accession_number: 'POL123456',
array_express_accession_number: 'ARR123456',
data_release_delay_approval: 'Yes',
data_release_prevention_reason: 'data validity',
data_release_prevention_reason: 'Protecting IP - DAC approval required',
data_release_prevention_approval: 'Yes',
data_release_prevention_reason_comment: 'Data Release prevention reason comment',
data_access_group: 'something',
Expand Down
4 changes: 2 additions & 2 deletions test/unit/data_release_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class DataReleaseTest < ActiveSupport::TestCase
setup do
@study.study_metadata.data_release_strategy = 'never'
@study.study_metadata.data_release_timing = 'never'
@study.study_metadata.data_release_prevention_reason = 'legal'
@study.study_metadata.data_release_prevention_reason = 'Protecting IP - DAC approval required'
@study.study_metadata.data_release_prevention_approval = 'Yes'
@study.study_metadata.data_release_prevention_reason_comment = 'It just is'
end
Expand All @@ -106,7 +106,7 @@ class DataReleaseTest < ActiveSupport::TestCase
context 'and release timing is delayed' do
setup do
@study.study_metadata.data_release_timing = 'delayed'
@study.study_metadata.data_release_delay_reason = 'phd study'
@study.study_metadata.data_release_delay_reason = 'PhD study'
end

data_release_strategies.each do |strategy|
Expand Down
Loading