Fix issues with Conditional question serialization (offered by @briri from DMPTool)

[johnpinto1]

Fix issues with Conditional question serialization (offered by @briri from DMPTool) in PR 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 can no 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:
  -- If Action is 'remove', you need to select one or more choices in Target.
  -- 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.

Some screenshots:

[github-actions]

	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

[johnpinto1]

I plan to contribute a fix for a bug with conditional questions we discovered after this PR is accepted. That will include a comprehensive set of Conditional Question RSpec tests.

[johnpinto1]

Not sure who how to sort it.

The Tests - PostgreSQL / postgresql (pull_request) Failing after 1m because

Created database 'roadmap_test'
You have 1 pending migration:
20250115102816 UpdateConditionsJsonColumnsData

[briri]

@johnpinto1 it's likely that the timestamp of the db/schema.rb does not match the timestamp of that new migration. The Rails schema file's timestamp needs to match the most recent migration.

Since you cannot run the db migrations from scratch (due to old migrations that reference models that no longer exist) so it runs rails db:schema:load.

Try updating the file to: ActiveRecord::Schema.define(version: 2025_01_15_102816) do

[aaronskiba]

Not sure who how to sort it.

The Tests - PostgreSQL / postgresql (pull_request) Failing after 1m because

Created database 'roadmap_test'
You have 1 pending migration:
20250115102816 UpdateConditionsJsonColumnsData

Are you able to execute bin/rails db:migrate on the branch? If so, commit the changes to db/schema.rb. I imagine it would achieve the same change that @briri is recommending?

[aaronskiba]

Not sure who how to sort it.
The Tests - PostgreSQL / postgresql (pull_request) Failing after 1m because

Created database 'roadmap_test'
You have 1 pending migration:
20250115102816 UpdateConditionsJsonColumnsData

Are you able to execute bin/rails db:migrate on the branch? If so, commit the changes to db/schema.rb. I imagine it would achieve the same change that @briri is recommending?

I just tested it myself and it did in fact update db/schema.rb as @briri recommended. Fixed the workflow too, so I hope it's okay that I just pushed the change.

[johnpinto1]

Thanks @aaronskiba & @briri for sorting issue of migration.

[aaronskiba]

I see that this PR was made to resolve the following issue: CDLUC3#666. In particular, it resolves breaking changes with the upgrade to Rails 7.2. I know the current upcoming release will take us to Rails 7.1. Would it be best to review and merge this when we make the jump to 7.2?

[johnpinto1]

@

I see that this PR was made to resolve the following issue: CDLUC3#666. In particular, it resolves breaking changes with the upgrade to Rails 7.2. I know the current upcoming release will take us to Rails 7.1. Would it be best to review and merge this when we make the jump to 7.2?

@aaronskiba That seems like a good idea.

[johnpinto1]

Will create new PR to avoid merge conflicts.