Fix issues with Conditional question serialization #667

briri · 2024-12-03T16:25:34Z

Fixes #666 <--- how ominous!

Changes proposed in this PR:

Run SQL script to convert existing records in the conditions table so that they are JSON Arrays (see query below)
Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller.
Removed all JSON.parse calls in the app/helpers/conditions.rb helper

Made the following changes to simplify this patch and to make it a little more user friendly:

The "Add Conditions" button will now say "Edit Conditions" if there are any.
Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent.
Conditions can be added or removed (not updated anymore)
Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc.

Needed to update underlying data within the table so that they are JSON arrays using the following query:

UPDATE conditions
SET option_list = CONCAT(CONCAT('[', REPLACE(REPLACE(REPLACE(option_list, '---\n- \'', '"'), '\'\n- \'', '","'), '\'\n', '"')), ']'),
  remove_data = CONCAT(CONCAT('[', REPLACE(REPLACE(REPLACE(remove_data, '---\n- \'', '"'), '\'\n- \'', '","'), '\'\n', '"')), ']')
where option_list like '---%';

github-actions · 2024-12-03T16:27:18Z

</tr>

	1 Error
🚫	Please include a CHANGELOG entry. `You can find it at [CHANGELOG.md](https://github.com/DMPRoadmap/roadmap/blob/main/CHANGELOG.md).`

	1 Warning
⚠️	There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in behavior. \n Ignore this warning if the PR ONLY contains translation.io synced updates.

Generated by 🚫 Danger

from DMPTool). Based on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): Run SQL script to convert existing records in the conditions table so that they are JSON Arrays (see query below) Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. Removed all JSON.parse calls in the app/helpers/conditions.rb helper Made the following changes to simplify this patch and to make it a little more user friendly: The "Add Conditions" button will now say "Edit Conditions" if there are any. Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. Conditions can be added or removed (not updated anymore) Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc. Needed to update underlying data within the table so that they are JSON arrays using MySQL or Postgres scripts in db/migrate-sql-scripts folder: 20241219_UpdateConditions-MySql.sql.txt 20241219_UpdateConditions-Postgres.sql.txt (Note. The scripts have .txt endings to be able to get round .sql in .gitignore.)

from DMPTool). Based on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): Run SQL script to convert existing records in the conditions table so that they are JSON Arrays (see query below) Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. Removed all JSON.parse calls in the app/helpers/conditions.rb helper Made the following changes to simplify this patch and to make it a little more user friendly: The "Add Conditions" button will now say "Edit Conditions" if there are any. Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. Conditions can be added or removed (not updated anymore) Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc. h

from DMPTool). Based on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): - Run SQL script to convert existing records in the conditions table so that they are JSON Arrays (see query below) - Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. - Removed all JSON.parse calls in the app/helpers/conditions.rb helper Made the following changes to simplify this patch and to make it a little more user friendly: - The "Add Conditions" button will now say "Edit Conditions" if there are any. - Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. - Conditions can be added or removed (not updated anymore) - Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc.

from DMPTool). Based on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): - There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. - Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. - Removed all JSON.parse calls in the app/helpers/conditions.rb helper Made the following changes to simplify this patch and to make it a little more user friendly: - The "Add Conditions" button will now say "Edit Conditions" if there are any. - Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. - Conditions can be added or removed (not updated anymore) - Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc. - We allow only one question option to be selected when adding a Condition unlike inthe DMPTool version because experience with multiple options chosen has been problematic and buggy when used by users in DMPOnline.

from DMPTool).nBased on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): - There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. - Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. - Removed all JSON.parse calls in the app/helpers/conditions.rb helper Made the following changes to simplify this patch and to make it a little more user friendly: - The "Add Conditions" button will now say "Edit Conditions" if there are any. - Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. - Conditions can be added or removed (not updated anymore) - Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc. - We allow only one question option to be selected when adding a Condition unlike inthe DMPTool version because experience with multiple options chosen has been problematic and buggy when used by users in DMPOnline.

from DMPTool).nBased on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): - There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. - Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. - Removed all JSON.parse calls in the app/helpers/conditions.rb helper Made the following changes to simplify this patch and to make it a little more user friendly: - The "Add Conditions" button will now say "Edit Conditions" if there are any conditions for a given question. - Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. - Conditions can be added or removed (not updated anymore) - Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc. - We allow only one question option to be selected when adding a Condition unlike inthe DMPTool version because experience with multiple options chosen has been problematic and buggy when used by users in DMPOnline.

from DMPTool).nBased on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): - There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. - Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. - Removed all JSON.parse calls in the app/helpers/conditions.rb helper - The user canno longer edit a condition. They need to remove it and create a new condition. This applies to the email for 'add notifications' too. Made the following changes to simplify this patch and to make it a little more user friendly: - The "Add Conditions" button will now say "Edit Conditions" if there are any conditions for a given question. - Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. - Conditions can be added or removed (not updated anymore) - Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc. - We allow only one question option to be selected when adding a Condition unlike inthe DMPTool version because experience with multiple options chosen has been problematic and buggy when used by users in DMPOnline.

from DMPTool).nBased on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): - There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. - Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. - Removed all JSON.parse calls in the app/helpers/conditions.rb helper - The user canno longer edit a condition. They need to remove it and create a new condition. This applies to the email for 'add notifications' too. Made the following changes to simplify this patch and to make it a little more user friendly: - The "Add Conditions" button will now say "Edit Conditions" if there are any conditions for a given question. - Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. - Conditions of action type 'remove' can be added or removed (not updated anymore). - Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc. - We allow only one question option to be selected when adding a Condition unlike inthe DMPTool version because experience with multiple options chosen has been problematic and buggy when used by users in DMPOnline.

from DMPTool).nBased on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): - There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. - Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. - Removed all JSON.parse calls in the app/helpers/conditions.rb helper - The user canno longer edit a condition. They need to remove it and create a new condition. This applies to the email for 'add notifications' too. Made the following changes to simplify this patch and to make it a little more user friendly: - The "Add Conditions" button will now say "Edit Conditions" if there are any conditions for a given question. - Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. - Conditions of any action type can be added or removed (not updated anymore). - Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc. The email content can no longer be edited once saved. It will need to be removed and re-created. - We allow only one question option to be selected when adding a Condition unlike inthe DMPTool version because experience with multiple options chosen has been problematic and buggy when used by users in DMPOnline.

from DMPTool).nBased on DMPtool commit CDLUC3#667. Changes proposed in this PR (text from cited PR): - There is a migration file with code for MySQL and Postgres to update the Conditions table to convert JSON Arrays in string format records in the conditions table so that they are JSON Arrays. - Updated the form partials in app/views/org_admin/conditions/_form.erb.rb to properly send condition data to the controller. - Removed all JSON.parse calls in the app/helpers/conditions.rb helper - The user canno longer edit a condition. They need to remove it and create a new condition. This applies to the email for 'add notifications' too. Made the following changes to simplify this patch and to make it a little more user friendly: - The "Add Conditions" button will now say "Edit Conditions" if there are any conditions for a given question. - Updated the column heading over the "thing that happens when the condition is met" from "Remove" to "Target" since the content of the column can either be questions being removed or an email notification being sent. - Conditions of any action type can be added or removed (not updated anymore). - Hovering over the email of an existing condition displays a tooltip that shows the email message, subject, etc. The email content can no longer be edited once saved. It will need to be removed and re-created. - We allow only one question option to be selected when adding a Condition unlike inthe DMPTool version because experience with multiple options chosen has been problematic and buggy when used by users in DMPOnline. - To add a condition you must have selected an Option and Action together with: o if Action is 'remove', you need to select one or more choices in Target. o if Action is 'add notification', you need to fill in all the fields in the 'Send email' popup. Otherwise, the condition will not be saved.

briri added 2 commits December 2, 2024 18:40

fixes for broken conditional logic after rails 7.2 upgrade

b8b4fce

cleanup of debug

c0e5809

briri merged commit d562e95 into v5 Dec 3, 2024
2 of 9 checks passed

briri deleted the bug/BR-conditions branch December 3, 2024 17:41

johnpinto1 mentioned this pull request Jan 30, 2025

Fix issues with Conditional question serialization (offered by @briri from DMPTool) DMPRoadmap/roadmap#3476

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issues with Conditional question serialization #667

Fix issues with Conditional question serialization #667

briri commented Dec 3, 2024

github-actions bot commented Dec 3, 2024

Fix issues with Conditional question serialization #667

Fix issues with Conditional question serialization #667

Conversation

briri commented Dec 3, 2024

github-actions bot commented Dec 3, 2024