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.

Sign up for GitHub

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

Y24-481 - Study setup page changes #4541

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 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
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
39 changes: 36 additions & 3 deletions app/models/study.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
DATA_RELEASE_TIMING_DELAYED
].freeze

OLD_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',
Expand All @@ -91,6 +92,7 @@
'Other (please specify)'
yoldas marked this conversation as resolved.
Show resolved Hide resolved
].freeze

OLD_DATA_RELEASE_DELAY_REASONS = ['other', 'phd study'].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',
Expand All @@ -99,7 +101,7 @@
'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_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 @@ -227,7 +229,7 @@
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?
)

Expand All @@ -246,7 +248,11 @@
custom_attribute(:array_express_accession_number)

with_options(if: :never_release?) do
custom_attribute(:data_release_prevention_reason, in: DATA_RELEASE_PREVENTION_REASONS, required: true)
custom_attribute(
:data_release_prevention_reason,
in: [*DATA_RELEASE_PREVENTION_REASONS, *OLD_DATA_RELEASE_PREVENTION_REASONS],
required: true
)
custom_attribute(:data_release_prevention_reason_comment, required: true)
custom_attribute(:data_release_prevention_approval)
end
Expand Down Expand Up @@ -577,6 +583,33 @@
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

Check warning on line 592 in app/models/study.rb

View check run for this annotation

Codecov / codecov/patch

app/models/study.rb#L592 

Added line #L592 was not covered by tests
end

additional_options + DATA_RELEASE_PREVENTION_REASONS
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

Check warning on line 606 in app/models/study.rb

View check run for this annotation

Codecov / codecov/patch

app/models/study.rb#L606 

Added line #L606 was not covered by tests
end

additional_options << DATA_RELEASE_DELAY_REASONS_ASSAY if assay_option
additional_options + DATA_RELEASE_DELAY_REASONS_STANDARD
end

private

def valid_ethically_approved?
Expand Down
10 changes: 5 additions & 5 deletions app/views/shared/metadata/edit/_study.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,22 +59,22 @@
%>

<% 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.select(:data_release_prevention_reason, study.data_release_prevention_options) %>
<%= 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),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to keep the empty option?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm not sure. These empty options were already here, I guess its for a similar reason I added them elsewhere - to force a user selection.

DataReleaseStudyType.non_assay_types.map(&:id) => [ '' ] + study.data_release_delay_options
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need to keep the empty option?

})
%>

<%= group.select(:data_release_delay_period, Study::DATA_RELEASE_DELAY_PERIODS) %>
<% 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, 'other']) do %>
BenTopping marked this conversation as resolved.
Show resolved Hide resolved
<%= 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) %>
Expand Down
Loading