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 9 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
34 changes: 22 additions & 12 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_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_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 (with free text box)'
yoldas marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -231,14 +245,10 @@ class Study < ApplicationRecord # rubocop:todo Metrics/ClassLength
custom_attribute(:ega_policy_accession_number)
custom_attribute(:array_express_accession_number)

with_options(if: :delayed_for_long_time?, required: true) do
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
2 changes: 1 addition & 1 deletion app/views/shared/metadata/_related_fields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

var valueFrom = function(element) {
if (element === null) { return null }
var value = element.value.toLowerCase().replace(/[^a-z0-9]+/, '_');
var value = element.value.toLowerCase().replaceAll(/[^a-z0-9]+/g, '_');
yoldas marked this conversation as resolved.
Show resolved Hide resolved
return (value.length == 0) ? 'blank' : value;
};

Expand Down
9 changes: 3 additions & 6 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 All @@ -74,9 +74,6 @@
%>

<%= group.select(:data_release_delay_period, Study::DATA_RELEASE_DELAY_PERIODS) %>
<% metadata_fields.related_fields(to: :data_release_delay_period, in: Study::DATA_RELEASE_DELAY_PERIODS) do %>
<%= group.radio_select(:data_release_delay_approval, Study::YES_OR_NO) %>
<% end %>
<% metadata_fields.related_fields(to: :data_release_delay_reason, when: Study::DATA_RELEASE_DELAY_FOR_OTHER) do %>
<%= group.text_area(:data_release_delay_other_comment) %>
<% metadata_fields.related_fields(to: :data_release_delay_period, in: Study::DATA_RELEASE_DELAY_PERIODS) do %>
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 Expand Up @@ -150,10 +150,6 @@ Feature: The XML for the sequencescape API
<name>Are all the samples to be used in this study commercially available, unlinked anonymised cell-lines?</name>
<value>No</value>
</descriptor>
<descriptor>
<name>Has the delay period been approved by the data sharing committee for this project?</name>
<value></value>
</descriptor>
<descriptor>
<name>ENA Project ID</name>
<value></value>
Expand Down
14 changes: 5 additions & 9 deletions features/studies/8447221_data_release_help_text.feature
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,27 @@ Feature: Update the data release fields for creating a study
Scenario Outline: Add help text opposite delay drop down (4044305)
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]
"""
When I select "Other (with free text box)" from "Reason for delaying release"
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)
"""

Scenario Outline: Delaying for 3 months should have the same questions as all other delays (4044273)
When I select "delayed" from "How is the data release to be timed?"
And I select "other" from "Reason for delaying release"
And I select "Other (with free text box)" from "Reason for delaying release"
And I select "<delay_period>" from "Delay for"
Then I should exactly see "Has the delay period been approved by the data sharing committee for this project?"
And I should exactly see "Comment regarding data release timing and approval"

When I fill in the following:
Expand Down
16 changes: 9 additions & 7 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 All @@ -37,7 +37,7 @@ Feature: Studies have timings for release of their data

Scenario Outline: When the data release is delayed but no reasons are provided
Given I select "delayed" from "How is the data release to be timed?"
And I select "other" from "Reason for delaying release"
And I select "Other (with free text box)" from "Reason for delaying release"
And I fill in "Please explain the reason for delaying release" with "Some reason"
And I select "<period>" from "Delay for"
When I press "Create"
Expand All @@ -54,7 +54,7 @@ Feature: Studies have timings for release of their data

Scenario Outline: When the data release is delayed and the reasons are provided
Given I select "delayed" from "How is the data release to be timed?"
And I select "other" from "Reason for delaying release"
And I select "Other (with free text box)" from "Reason for delaying release"
And I fill in "Please explain the reason for delaying release" with "Some reason"
And I select "<period>" from "Delay for"
And I fill in "Comment regarding data release timing and approval" with "Because it is ok?"
Expand All @@ -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"
3 changes: 1 addition & 2 deletions features/support/step_definitions/study_steps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,9 @@ def given_study_metadata(attribute, regexp)
study.update!(
study_metadata_attributes: {
data_release_timing: 'delayed',
data_release_delay_reason: 'other',
data_release_delay_reason: 'Other (with free text box)',
data_release_delay_other_comment: reason,
data_release_delay_period: "#{period} months",
data_release_delay_approval: 'Yes',
data_release_delay_reason_comment: reason
}
)
Expand Down
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
26 changes: 6 additions & 20 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 Expand Up @@ -713,7 +713,10 @@
create(
:study,
study_metadata:
create(:study_metadata, metadata.merge(data_release_timing: 'delayed', data_release_delay_reason: 'other'))
create(
:study_metadata,
metadata.merge(data_release_timing: 'delayed', data_release_delay_reason: 'Other (with free text box)')
)
)
end

Expand All @@ -739,23 +742,6 @@
end
end

context 'delayed for long time' do
let(:study) do
create(
:study,
study_metadata:
create(
:study_metadata,
metadata.merge(data_release_timing: 'delayed', data_release_delay_period: '6 months')
)
)
end

it 'will have a data_release_delay_approval' do
expect(study.study_metadata.data_release_delay_approval).to eq(metadata[:data_release_delay_approval])
end
end

context 'never released' do
let(:never_release_fields) do
{
Expand Down
Loading
Loading