Conditional question fix for removing questions bug rails7
A Conditional Question (with action: "remove" and in a Phase with multiple sections) for this bug can be described structurally as follows:
Conditional Question: Q2. Will your data be open?,
Answer Options: "Yes", "No", "Don't Know"
If answer "Yes", then
[Action: "remove",
[Section 1, Q5. What data will be publicly shared?],
[Section 2, Q12. What type of organisations will you share your data?]
]
Else if answer "No"
[ Action: "remove",
[Section 1, "Q7. Under what licence will be the data/code released?"]
[Section 2, "Q25. Which persons/organisations will you share your data with?],
[ Section 3, "Q30. Will your data be placed in a research data repository?"]]
Else if answer "Don't know"
Do nothing.
Bug Description: Currently in the code if we answer several questions in a phase in various sections, then subsequently answer an option of a Conditional question that removes some of the questions answered. The removed answers are hidden in UI, but the answers to the hidden questions remain persisted in the database and are hidden in the UI, not cleared. As a result if we change our answer to the same Conditional Question the removed questions with the "stale/old" answers reappear.
Expected Behavior We would expect the removed questions answers to be deleted from the database and cleared from the UI. So if we change our answer to the same Conditional Question the removed questions should reappear without the "stale/old" answers. Further the removed questions could be in different sections in phase.
Testing Attached is a script for testing in your development environment and running the content in a Rails console. Test_script_for_setup_template_with_conditional_question.txt
We found convenient to use the seeds file to populate the database as follows:
➜ bin/rails db:environment:set RAILS_ENV=development ➜ rails db:schema:load
➜ rails db:seed
We then brought up a Rails console and ran the content of the test script by copying and pasting.
- We switch to development branch to view the bug by following test suggestions below.
- Next we switch fix-for-conditional-question-removed-answers-not-being-deleted branch to test if it has been resolved.
You can view the template we created as the Admin User you logged in as.
Here is the structure of the conditional question we are testing, namely question "Q2. Will your data be open? THE CONDITIONAL QUESTION"
Testing suggestions With each branch login as user with email: [email protected] and password: password123 .
Create a plan with organisation Conditional Tests Organisation and no funder, e.g.,
Then fill in answers to following sets of questions:
- Q5 (checkbox), Q9 (radiobuttons), Q14 (textarea), Q17 (text), Q18 (date), Q20 (dropdown) (which will be removed by condition option Yes)
- Q6 (dropdown), Q7 (textarea), Q10 (text), Q11 (date), Q16 (radiobuttons), Q19 (checkbox)) (which will be removed by condition option No)
You can use the same plan for both branches. You just need to fill in the values again to test each time.
Now answer question "Q2. Will your data be open? THE CONDITIONAL QUESTION" with Yes, No and Don't know
As mentioned above, the branch for issue, should show the expected behaviour:
We would expect the removed questions answers to be deleted from the database and cleared from the UI. So if we change our answer to the same Conditional Question the removed questions should reappear without the "stale/old" answers. Further the removed questions could be in different sections in phase.
| 1 Warning | |
|---|---|
| :warning: | This PR is too big! Consider breaking it down into smaller PRs. |
Generated by :no_entry_sign: Danger
I had mentioned this bug in the following prior Conditional Question PR: https://github.com/DMPRoadmap/roadmap/pull/3497#issuecomment-2770800820
The same bug seems to persist in this PR. I've created the following issue to track it: https://github.com/DMPRoadmap/roadmap/issues/3520
This a future suggestion. I like how the comments inform the user of the consequences of clicking the various buttons (e.g. "REMOVES: Q5 (checkbox)"). I know this was added explicitly to help with the testing. However, it could also improve user experience by helping them not delete answers by accident.
Additionally/alternatively, in the future a warning could be sent before deleting any existing answers (e.g. have a confirmation button with a message like "Selecting this option will delete the following answers from your plan...") so they don't accidentally delete answers from their plan.
https://github.com/DMPRoadmap/roadmap/actions/runs/14705500339/job/41750808748
There is one intermittent test failure regarding Validation failed: Title must be unique.
1) AnswersController AnswersController#create_or_update for action_type: remove POST /answers/create_or_update (where atleast one question has one or more conditional options) with conditional radiobuttons question handles single option (with condition) in option_list
Failure/Error: question.question_options = create_list(:question_option, evaluator.options)
ActiveRecord::RecordInvalid:
Validation failed: Title must be unique
# ./spec/factories/questions.rb:44:in `block (3 levels) in <main>'
# ./spec/controllers/answers_controller_with_conditional_questions_spec.rb:28:in `block (2 levels) in <main>'
https://github.com/DMPRoadmap/roadmap/actions/runs/14705500339/job/41750808748
There is one intermittent test failure regarding
Validation failed: Title must be unique.1) AnswersController AnswersController#create_or_update for action_type: remove POST /answers/create_or_update (where atleast one question has one or more conditional options) with conditional radiobuttons question handles single option (with condition) in option_list Failure/Error: question.question_options = create_list(:question_option, evaluator.options) ActiveRecord::RecordInvalid: Validation failed: Title must be unique # ./spec/factories/questions.rb:44:in `block (3 levels) in <main>' # ./spec/controllers/answers_controller_with_conditional_questions_spec.rb:28:in `block (2 levels) in <main>'
Not sure how to get round this. Any ideas. I have this issue occasionally too locally in past.