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-052 [BUG] Dropdown list dependencies #4076

Closed
KatyTaylor opened this issue Apr 8, 2024 · 3 comments · Fixed by #4078
Closed

Y24-052 [BUG] Dropdown list dependencies #4076

KatyTaylor opened this issue Apr 8, 2024 · 3 comments · Fixed by #4078
Assignees
Labels
Bug Bug in code

Comments

@KatyTaylor
Copy link
Contributor

Describe the bug
Sequencescape has two 'edit' pages for Studies - the standard one, and one for admins. The standard one has logic that restricts the available values in one field based on the value selected in a different list. The admin one does not obey this same logic, which can lead to undesired data combinations.

To Reproduce
Steps to reproduce the behaviour:

  1. Go to tab 'Studies'
  2. Click on link (right hand side) 'Create Study'
  3. Fill out the required fields
  4. For "What is the data release strategy for this study?", and "How is the data release to be timed?", play with the fields and notice how the value of the first influences the options for the second.
    • "Open", "Managed" --> "standard", "immediate", "delayed"
    • "Not applicable" --> "never"
  5. Save the study.
  6. Click on link 'Study details'
  7. Click on link 'Edit'
  8. For "What is the data release strategy for this study?", and "How is the data release to be timed?", play with the fields as before and notice that the behaviour is the same as when creating the study.
  9. Go to tab 'Admin'
  10. Click link 'Study management'
  11. Select the study you just created
  12. For "What is the data release strategy for this study?", and "How is the data release to be timed?", play with the fields as before.
  13. Notice that the value of the first does not influence the options for the second - this is the bug.

Expected behaviour
The value of "What is the data release strategy for this study?" should control the possible options for the field "How is the data release to be timed?", consistently in all creation and edit screens, so as to prevent undesired combinations of these two fields.

Screenshots
Screenshot showing incorrect behaviour:

Screenshot 2024-04-08 at 11 30 10

Additional Context
This was found while researching #4024, and is believed to be the root cause of the issue.

@KatyTaylor
Copy link
Contributor Author

KatyTaylor commented Apr 9, 2024

Thinking about adding validation server-side to enforce the relationship between data_release_strategy and data_relese_timing. There are many existing studies in the database that would not satisfy the new validation:

SELECT s.id, s.enforce_accessioning, s.enforce_data_release, sm.data_release_strategy, sm.data_release_timing, s.state, s.created_at
FROM studies s
JOIN study_metadata sm ON s.id = sm.study_id
WHERE sm.data_release_strategy <> 'not applicable' AND sm.data_release_timing = 'never'
ORDER BY s.created_at;
-- 955 rows, mix of fields combinations, and active / inactive, latest 2014

SELECT s.id, s.enforce_accessioning, s.enforce_data_release, sm.data_release_strategy, sm.data_release_timing, s.state, s.created_at
FROM studies s
JOIN study_metadata sm ON s.id = sm.study_id
WHERE sm.data_release_strategy = 'not applicable' AND sm.data_release_timing <> 'never'
ORDER BY s.created_at;
-- 15 rows, mix of field combinations, mostly active, latest 2023

SELECT Count(*) FROM studies;
-- 7,467 rows total

Could be worth checking the validation assumptions are correct with customers before implementing, and then discussing what to do with these existing studies.

@KatyTaylor
Copy link
Contributor Author

Created #4084 to track the investigation into the dodgy Studies data, and the turning on of the server-side validation feature flag.

@KatyTaylor
Copy link
Contributor Author

KatyTaylor commented Apr 18, 2024

Fuller query to produce report for Liz C:

SELECT s.id AS 'study_id'
	   , s.name AS 'study_name'
	   , u.login AS 'user_login'
	   , GROUP_CONCAT(u_owner.login) AS 'owners'
	   , GROUP_CONCAT(u_owner.login) AS 'managers'
	   , fs.name AS 'faculty_sponsor'
	   , s.enforce_accessioning
	   , s.enforce_data_release
	   , sm.data_release_strategy
	   , sm.data_release_timing
	   , s.state
	   , s.created_at
FROM studies s
JOIN study_metadata sm ON s.id = sm.study_id

LEFT JOIN users u ON s.user_id = u.id

LEFT JOIN roles role_owner ON role_owner.authorizable_id = s.id AND role_owner.authorizable_type = 'Study' AND role_owner.name = 'owner'
LEFT JOIN roles_users ru_owner ON ru_owner.role_id = role_owner.id
LEFT JOIN users u_owner ON u_owner.id = ru_owner.user_id

LEFT JOIN roles role_manager ON role_manager.authorizable_id = s.id AND role_manager.authorizable_type = 'Study' AND role_manager.name = 'manager'
LEFT JOIN roles_users ru_manager ON ru_manager.role_id = role_manager.id
LEFT JOIN users u_manager ON u_manager.id = ru_manager.user_id

LEFT JOIN faculty_sponsors fs ON sm.faculty_sponsor_id = fs.id

WHERE (sm.data_release_strategy <> 'not applicable' AND sm.data_release_timing = 'never') 
   OR (sm.data_release_strategy = 'not applicable' AND sm.data_release_timing <> 'never')
   
GROUP BY s.id
ORDER BY s.created_at;
-- 970 rows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug in code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant