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

Merged
merged 18 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5bc22bf
feat(studies): Updates new/edit study questions
BenTopping Dec 3, 2024
9f6cbcd
feat(studies): corrects undefined cucumber tests in data release timing
BenTopping Dec 4, 2024
12d0368
feat(study): removes redundant old data release prevention reasons
BenTopping Dec 4, 2024
ea5deb3
tests(studies): updates study metadata test data to reflect data rele…
BenTopping Dec 4, 2024
5ffaa68
fix(studies): adds blank values to changed study data release values …
BenTopping Dec 9, 2024
6966d2a
tests(studies): updates cucumber studies data release test after remo…
BenTopping Dec 9, 2024
380d161
feat(study-metadata): removes data_release_delay_approval for new/edi…
BenTopping Dec 16, 2024
bac3e8b
tests(study-metadata): removes test references to deprecated release …
BenTopping Dec 16, 2024
b5e5c4a
feat(studies): updates data release other option
BenTopping Dec 17, 2024
fdc2e1a
feat(study): updates study edit dropdown options to display old optio…
BenTopping Jan 8, 2025
c400f4e
tests(study): adds tests for dropdown option generation functions
BenTopping Jan 8, 2025
1082201
fix(study): fixes old other option not firing mandatory validation
BenTopping Jan 8, 2025
dacc290
Merge branch 'develop' of https://github.com/sanger/sequencescape int…
BenTopping Jan 8, 2025
65a2638
Merge branch 'develop' of https://github.com/sanger/sequencescape int…
BenTopping Jan 13, 2025
ec24473
feat(study): adds data_release_prevention_other_comment to study meta…
BenTopping Jan 14, 2025
befe5d4
tests(cucumber): fixes test looking for removed text
BenTopping Jan 14, 2025
7b18b96
tests(cucumber): fixes study xml api response
BenTopping Jan 14, 2025
b37c5a0
misc(study): remove helper text for study description
BenTopping Jan 15, 2025
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
86 changes: 70 additions & 16 deletions app/models/study.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,28 @@ 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
OLD_DATA_RELEASE_PREVENTION_REASONS = ['data validity', 'legal', 'replication of data subset'].freeze
DATA_RELEASE_PREVENTION_REASON_OTHER = 'Other (please specify)'
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',
DATA_RELEASE_PREVENTION_REASON_OTHER
].freeze

OLD_DATA_RELEASE_DELAY_FOR_OTHER = 'other'
DATA_RELEASE_DELAY_FOR_OTHER = 'Other (please specify below)'
OLD_DATA_RELEASE_DELAY_REASONS = ['other', 'phd study'].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'].freeze

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

Expand Down Expand Up @@ -214,14 +231,15 @@ class Study < ApplicationRecord # rubocop:todo Metrics/ClassLength
custom_attribute(
:data_release_delay_reason,
required: true,
in: DATA_RELEASE_DELAY_REASONS_ASSAY,
in: [*DATA_RELEASE_DELAY_REASONS_STANDARD, *DATA_RELEASE_DELAY_REASONS_ASSAY, *OLD_DATA_RELEASE_DELAY_REASONS],
if: :delayed_release?
)

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

with_options(required: true, if: :delayed_for_other_reasons?) do
custom_attribute(:data_release_delay_other_comment)
with_options(if: :delayed_for_other_reasons?) do
custom_attribute(:data_release_delay_other_comment, required: true)
custom_attribute(:data_release_delay_reason_comment)
end

Expand All @@ -231,14 +249,19 @@ 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, *OLD_DATA_RELEASE_PREVENTION_REASONS],
required: true
)
custom_attribute(
:data_release_prevention_other_comment,
required: true,
if: :data_release_prevention_reason_other?
)
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 Expand Up @@ -567,6 +590,33 @@ def poly_metadatum_by_key(key)
poly_metadata.find { |pm| pm.key == key.to_s }
end

# Helper method for edit dropdowns to support backwards compatibility with old options.
#
# @return [Array<String>] the list of options for the data release prevention reason dropdown
def data_release_prevention_options
additional_options = []
if OLD_DATA_RELEASE_PREVENTION_REASONS.include? study_metadata.data_release_prevention_reason
additional_options << study_metadata.data_release_prevention_reason
end

DATA_RELEASE_PREVENTION_REASONS + additional_options
end

# Helper method for edit dropdowns to support backwards compatibility with old options.
#
# @param [Boolean] assay_option - whether to include assay-specific options
# @return [Array<String>] the list of options for the data release delay reason dropdown
def data_release_delay_options(assay_option: false)
# If the current value is an old one, then we need to include it in the list of options
additional_options = []
if OLD_DATA_RELEASE_DELAY_REASONS.include? study_metadata.data_release_delay_reason
additional_options << study_metadata.data_release_delay_reason
end

additional_options.concat(DATA_RELEASE_DELAY_REASONS_ASSAY) if assay_option
DATA_RELEASE_DELAY_REASONS_STANDARD + additional_options
end

private

def valid_ethically_approved?
Expand Down Expand Up @@ -601,7 +651,11 @@ def never_release?
end

def delayed_for_other_reasons?
data_release_delay_reason == DATA_RELEASE_DELAY_FOR_OTHER
[DATA_RELEASE_DELAY_FOR_OTHER, OLD_DATA_RELEASE_DELAY_FOR_OTHER].include?(data_release_delay_reason)
end

def data_release_prevention_reason_other?
data_release_prevention_reason == DATA_RELEASE_PREVENTION_REASON_OTHER
end

def delayed_for_long_time?
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
21 changes: 9 additions & 12 deletions app/views/shared/metadata/edit/_study.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -59,29 +59,26 @@
%>

<% 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_options) %>
<% metadata_fields.related_fields(to: :data_release_prevention_reason, in: [Study::DATA_RELEASE_PREVENTION_REASON_OTHER]) do %>
<%= group.text_area(:data_release_prevention_other_comment) %>
<% end %>
<%= 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_options) %>
<%
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,
DataReleaseStudyType.non_assay_types.map(&:id) => [ '' ] + Study::DATA_RELEASE_DELAY_REASONS_STANDARD
DataReleaseStudyType.assay_types.map(&:id) => [ '' ] + study.data_release_delay_options(assay_option: true),
yoldas marked this conversation as resolved.
Show resolved Hide resolved
DataReleaseStudyType.non_assay_types.map(&:id) => [ '' ] + study.data_release_delay_options
yoldas marked this conversation as resolved.
Show resolved Hide resolved
})
%>

<%= 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 %>
<% metadata_fields.related_fields(to: :data_release_delay_reason, in: [Study::DATA_RELEASE_DELAY_FOR_OTHER, Study::OLD_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 %>
<%= group.text_area(:data_release_delay_reason_comment) %>
<% end %>
<% end %>
<% end %>
<% end %>
Expand Down
1 change: 1 addition & 0 deletions app/views/shared/metadata/show/_study.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
<%= group.plain_value(:data_release_prevention_reason) %>
<%= group.plain_value(:data_release_prevention_approval) %>
<%= group.plain_value(:data_release_prevention_reason_comment) %>
<%= group.plain_value(:data_release_prevention_other_comment) %>
<% end %>
<%= metadata_fields.plain_value(:dac_policy_title) %>
<%= metadata_fields.plain_value(:dac_policy) %>
Expand Down
12 changes: 6 additions & 6 deletions config/locales/metadata/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,6 @@ en:
study_description:
label: Study description
edit_info: "ENA requirement"
help: Please choose one of the following 2 standard statements to be included with your data submissions (one or the other, depending on the study). <b>If you use the second statement, replace [doi or ref] by a reference or doi for your publication:</b><br /><br />This data is part of a pre-publication release. For information on the proper use of pre-publication data shared by the Wellcome Trust Sanger Institute (including details of any publication moratoria), please see http://www.sanger.ac.uk/datasharing/<br /><br />OR<br /><br />This data has been described in the following article [doi or ref] and its further analysis can be freely submitted for publication. For information on the proper use of data shared by the Wellcome Trust Sanger Institute (including information on acknowledgement), please see http://www.sanger.ac.uk/datasharing/<br/><p>If applicable, include a brief description of any restrictions on data usage, e.g. 'For AIDS-related research only'</p>

contaminated_human_dna:
label: Does this study contain samples that are contaminated with human DNA which must be removed prior to analysis?
Expand Down Expand Up @@ -424,7 +423,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 @@ -434,21 +433,22 @@ 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?
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 />"
label: If reason for exemption requires DAC approval, what is the approval number?

data_release_prevention_other_comment:
label: Please explain the reason for preventing data release

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
@@ -0,0 +1,6 @@
# frozen_string_literal: true
class AddDataReleasePreventionOtherCommentToStudyMetadata < ActiveRecord::Migration[7.0]
def change
add_column :study_metadata, :data_release_prevention_other_comment, :string, default: nil
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2024_12_11_143636) do
ActiveRecord::Schema[7.0].define(version: 2025_01_14_135342) do
create_table "aliquot_indices", id: :integer, charset: "utf8mb4", collation: "utf8mb4_unicode_ci", options: "ENGINE=InnoDB ROW_FORMAT=DYNAMIC", force: :cascade do |t|
t.integer "aliquot_id", null: false
t.integer "lane_id", null: false
Expand Down Expand Up @@ -1548,6 +1548,7 @@
t.string "s3_email_list"
t.string "data_deletion_period"
t.string "contaminated_human_data_access_group"
t.string "data_release_prevention_other_comment"
t.index ["faculty_sponsor_id"], name: "index_study_metadata_on_faculty_sponsor_id"
t.index ["study_id"], name: "index_study_metadata_on_study_id"
end
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 All @@ -178,6 +174,10 @@ Feature: The XML for the sequencescape API
<name>Please explain the reason for delaying release</name>
<value></value>
</descriptor>
<descriptor>
<name>Please explain the reason for preventing data release</name>
<value></value>
</descriptor>
<descriptor>
<name>SNP parent study ID</name>
<value></value>
Expand Down
34 changes: 3 additions & 31 deletions features/studies/8447221_data_release_help_text.feature
Original file line number Diff line number Diff line change
Expand Up @@ -6,54 +6,26 @@ Feature: Update the data release fields for creating a study
Given a faculty sponsor called "Jack Sponsor" exists
And I am on the new study page

Scenario: Add help text to study description (8348119)
Then the help text for "Study description" should contain:
"""
Please choose one of the following 2 standard statements to be included with your data submissions (one or the other, depending on the study). If you use the second statement, replace [doi or ref] by a reference or doi for your publication:

This data is part of a pre-publication release. For information on the proper use of pre-publication data shared by the Wellcome Trust Sanger Institute (including details of any publication moratoria), please see http://www.sanger.ac.uk/datasharing/

OR

This data has been described in the following article [doi or ref] and its further analysis can be freely submitted for publication. For information on the proper use of data shared by the Wellcome Trust Sanger Institute (including information on acknowledgement), please see http://www.sanger.ac.uk/datasharing/
If applicable, include a brief description of any restrictions on data usage, e.g. 'For AIDS-related research only'
"""

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 (please specify below)" 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 select "never" from "How is the data release to be timed?"
Then the help text for "Has this been approved?" 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 (please specify below)" 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:
| Study name | new study |
| Study description | writing cukes |
| Please explain the reason for delaying release | some comment |
| Comment regarding data release timing and approval | another comment |

And I select "Jack Sponsor" from "Faculty Sponsor"
And I choose "Yes" from "Do any of the samples in this study contain human DNA?"
Expand Down
Loading
Loading