DPL-1105 [BUG] A study set to never release should not require accession numbers for the samples

[yoldas]

Describe the bug I (ec20) have a study set to never release, therefore it shouldn’t require accession numbers for the samples. However, when I try to generate the sequencing request, I get this message:

If I do then try to accession the study, I get the following message, confirming that I don’t need to accession the study:

My sequencing request is urgent, so I will change the study release strategy in order to get the accession and the sequencing requested, however I believe this needs to be fixed.

RT Ticket Number N/A

To Reproduce Steps to reproduce the behaviour: TBD

Expected behaviour Validation should not fail.

Screenshots See description.

Desktop (please complete the following information): N/A

Additional context Related bug-fix DPL-944 JS banner not being displayed

[KatyTaylor]

It's difficult to be 100% certain about this, because the study data has been changed since the errors were experienced, but here's my best guess.

TL;DR The study was not set correctly to never release data - the data release strategy was 'Not applicable', and the data release timing was 'standard' (should have been 'never'), which sent conflicting messages and resulted in the errors shown. It shouldn't be possible to get the data into this state - I've created a new bug https://github.com/sanger/sequencescape/issues/4076 to deal with this.

Details of technical investigation

Validation code is in check_data_release_and_accession_for_submission, in Submission::AccessionBehaviour module, included in Order.

Based on the error message in the first screenshot, we can see it's returning true for both study.valid_data_release_properties? and study.accession_required?. These are defined in DataRelease module. In order to return true for accession_required?, the following must be true:

• The enforce_accessioning flag, on by default and only modifiable by admins, must have been left alone.
• Either:
  • There must be an 'invalid' combination of values for the accessioning-related fields - we know this isn't true because it was already checked above, in valid_data_release_properties?
  • data_release_timing field on StudyMetadata must NOT be set to never - although the user said the study was set to never release, I think this must have been a misunderstanding.

I couldn't find a bug in the validation logic, so I believe that the issue was caused by an unexpected combination of values in the fields. When I checked the values of the relevant fields for the mentioned Study, I got the following:

Both flags being 1 (true) is expected - these are the default values. The combination of data strategy 'Not applicable' and data timing 'standard' is not expected, and not possible to set through the normal UI. We know the data strategy was initially 'Not applicable', as it is shown in the second screenshot (unless the error message is incorrect, which I don't think is the case). The user then said they changed the data release strategy to allow accessioning, however they must have then changed it back, because the value seen in the field is 'Not applicable'. There's no mention of changing the data release timing field, so I will assume it was also 'standard' at the time the error occurred.

When I test the validation logic with the combination of data strategy 'Not applicable' and data timing 'standard', I can reproduce the logic that would lead to the error messages shown in the description:

1. Create a study in the UI (start with 'Studies' tab), filling out all the required fields.
2. Go to 'Admin' tab > 'Study management' > select your study and edit.
3. Change the data release strategy to 'Not applicable' and the data release timing to 'never'
4. Start the Rails console
5. Run the following code:

   s = Study.find(your_study_id)
   s.accession_required?
   => true

Although it's not possible to get the data into this state using the standard UI, I found it is possible using the admin UI. I created a new bug to solve this - https://github.com/sanger/sequencescape/issues/4076

[KatyTaylor]

Communication sent to Liz C (requestor):

I’m working on an issue that it looks like you raised - DPL-1105 [BUG] A study set to never release should not require accession numbers for the samples

Here’s a summary of what I’ve found:

It's difficult to be 100% certain about this, because the study data has been changed since the errors were experienced, but here's my best guess.

I couldn’t find a bug around the study validation. I believe it acted as it did because the data was in an unexpected state – with the data release strategy set to ‘Not applicable’, and the data release timing set to ‘standard’ (when it should have been ‘never’).

With the data in this state, the first error (saying accessioning is required) is because it’s checking the data release timing field, and that’s NOT set to ‘never’. The second error (saying accessioning is not allowed) is because it’s checking the data release strategy field, and that’s set to ‘Not applicable’.

If you use the standard Study creation and edit screens, it shouldn’t be possible to get the data into this state. However, I’ve found that the Study Edit page reached through the Admin tab does not enforce the relationships between these fields (this is a bug, I’ve logged it here). Therefore I think it must have been edited by an admin into this unexpected state.

Does this sound plausible to you, from what you remember of the history here? Have you experienced this issue before? If you have any other Studies where this is a problem, please let me know. Otherwise I think I’ll close off this issue and ask for the new bug in the Admin Study Edit page to be prioritised.

[KatyTaylor]

Reply:

Thanks for following up on this – that makes sense because I actually generated a “request additional sequencing” request on a never release study at the end of last week and SS allowed me to do this. I thought that the bug has already been picked up and fixed (I have a note to remind myself to say thank you to psd!!)

I honestly can’t remember the details of setting up and/or changing that study, but it’s certainly plausible. It would be great to have that new bug fix logged instead and I’ll flag it to the SSR team in case anyone else has this issue in the meantime.

I have added Y24-052 to the bug backlog, and advised Liz that the study in this ticket should start working again if she changes the data release timing field to 'never'.