From 5a1c8b8042778e3d4fc45ddbb5aaeb20b065e356 Mon Sep 17 00:00:00 2001 From: momo3404 Date: Wed, 21 Jan 2026 14:30:58 -0700 Subject: [PATCH 01/10] Apply ROR changes to funder - Remove org_id setting since it was incorrectly setting org_id --- app/controllers/plans_controller.rb | 13 +++++++------ app/views/plans/_project_details.html.erb | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index 8e6e80ba7b..6260177cd6 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -239,7 +239,7 @@ def edit # PUT /plans/1 # rubocop:disable Metrics/MethodLength, Metrics/PerceivedComplexity - def update + def update # rubocop:disable Metrics/CyclomaticComplexity @plan = Plan.find(params[:id]) authorize @plan attrs = plan_params @@ -258,15 +258,14 @@ def update # TODO: For some reason the `fields_for` isn't adding the # appropriate namespace, so org_id represents our funder funder_attrs = plan_params[:funder] - funder_attrs[:org_id] = plan_params[:funder][:id] funder = org_from_params(params_in: funder_attrs, allow_create: true) - @plan.funder_id = funder&.id + @plan.funder_id = funder&.id if funder @plan.grant = plan_params[:grant] attrs.delete(:funder) attrs.delete(:grant) attrs = remove_org_selection_params(params_in: attrs) - if @plan.update(attrs) # _attributes(attrs) + if @plan.update(attrs) && funder.present? # _attributes(attrs) format.html do redirect_to plan_path(@plan), notice: success_message(@plan, _('saved')) @@ -275,13 +274,15 @@ def update render json: { code: 1, msg: success_message(@plan, _('saved')) } end else + failure_msg = failure_message(@plan, _('save')) + failure_msg += _(' Invalid funder.') if funder.nil? format.html do # TODO: Should do a `render :show` here instead but show defines too many # instance variables in the controller - redirect_to plan_path(@plan).to_s, alert: failure_message(@plan, _('save')) + redirect_to plan_path(@plan).to_s, alert: failure_msg end format.json do - render json: { code: 0, msg: failure_message(@plan, _('save')) } + render json: { code: 0, msg: failure_message } end end rescue StandardError => e diff --git a/app/views/plans/_project_details.html.erb b/app/views/plans/_project_details.html.erb index 7709a19948..17cb8268ba 100644 --- a/app/views/plans/_project_details.html.erb +++ b/app/views/plans/_project_details.html.erb @@ -150,7 +150,7 @@ ethics_report_tooltip = _("Link to a protocol from a meeting with an ethics comm
<%= form.fields_for :funder, Org.new do |funder_fields| %> - <%= render partial: "shared/org_selectors/text_only", + <%= render partial: "shared/org_selectors/combined", locals: { form: funder_fields, id_field: :id, From effd562fff24a39378063ec8a78ceb42b3ff063e Mon Sep 17 00:00:00 2001 From: momo3404 Date: Wed, 21 Jan 2026 15:52:29 -0700 Subject: [PATCH 02/10] Fix funder error message --- app/controllers/plans_controller.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index 6260177cd6..0430dfc79f 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -265,7 +265,9 @@ def update # rubocop:disable Metrics/CyclomaticComplexity attrs.delete(:grant) attrs = remove_org_selection_params(params_in: attrs) - if @plan.update(attrs) && funder.present? # _attributes(attrs) + invalid_funder = invalid_funder?(funder_attrs) + + if @plan.update(attrs) && !invalid_funder # _attributes(attrs) format.html do redirect_to plan_path(@plan), notice: success_message(@plan, _('saved')) @@ -275,14 +277,14 @@ def update # rubocop:disable Metrics/CyclomaticComplexity end else failure_msg = failure_message(@plan, _('save')) - failure_msg += _(' Invalid funder.') if funder.nil? + failure_msg += _(' Invalid funder.') if invalid_funder format.html do # TODO: Should do a `render :show` here instead but show defines too many # instance variables in the controller redirect_to plan_path(@plan).to_s, alert: failure_msg end format.json do - render json: { code: 0, msg: failure_message } + render json: { code: 0, msg: failure_msg } end end rescue StandardError => e @@ -560,5 +562,11 @@ def setup_local_orgs Org.includes(identifiers: :identifier_scheme).default_orgs) @orgs = @orgs.flatten.uniq.sort_by(&:name) end + + def invalid_funder?(funder_attrs) + funder_json = funder_attrs[:org_id].present? ? (JSON.parse(funder_attrs[:org_id]) rescue {}) : {} # rubocop:disable Style/RescueModifier + # Only returns true when name is present but ror is not, which only occurs when user enters invalid funder + funder_json['name'].present? && funder_json['ror'].blank? + end end # rubocop:enable Metrics/ClassLength From 2ffbe25b15ef0e91bfe0aefc8cfe789dc4a94f6a Mon Sep 17 00:00:00 2001 From: momo3404 Date: Fri, 23 Jan 2026 13:33:32 -0700 Subject: [PATCH 03/10] Properly fix validation error --- app/controllers/plans_controller.rb | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index 0430dfc79f..e50f2fe904 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -255,18 +255,23 @@ def update # rubocop:disable Metrics/CyclomaticComplexity end @plan.guidance_groups = GuidanceGroup.where(id: guidance_group_ids) - # TODO: For some reason the `fields_for` isn't adding the - # appropriate namespace, so org_id represents our funder funder_attrs = plan_params[:funder] - funder = org_from_params(params_in: funder_attrs, allow_create: true) - @plan.funder_id = funder&.id if funder + + funder = + if funder_attrs[:org_name].blank? + nil # user cleared funder — valid + else + org_from_params(params_in: funder_attrs, allow_create: true) + end + + invalid_funder = funder_attrs[:org_name].present? && funder.nil? + + @plan.funder_id = funder&.id @plan.grant = plan_params[:grant] attrs.delete(:funder) attrs.delete(:grant) attrs = remove_org_selection_params(params_in: attrs) - invalid_funder = invalid_funder?(funder_attrs) - if @plan.update(attrs) && !invalid_funder # _attributes(attrs) format.html do redirect_to plan_path(@plan), @@ -562,11 +567,5 @@ def setup_local_orgs Org.includes(identifiers: :identifier_scheme).default_orgs) @orgs = @orgs.flatten.uniq.sort_by(&:name) end - - def invalid_funder?(funder_attrs) - funder_json = funder_attrs[:org_id].present? ? (JSON.parse(funder_attrs[:org_id]) rescue {}) : {} # rubocop:disable Style/RescueModifier - # Only returns true when name is present but ror is not, which only occurs when user enters invalid funder - funder_json['name'].present? && funder_json['ror'].blank? - end end # rubocop:enable Metrics/ClassLength From 4d256eeeae128c5f3f1cf9d903c1ae8ecc872db2 Mon Sep 17 00:00:00 2001 From: momo3404 Date: Wed, 28 Jan 2026 14:31:43 -0700 Subject: [PATCH 04/10] Fix funder clearing and error message --- app/controllers/plans_controller.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index e50f2fe904..0d018c4b19 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -266,7 +266,9 @@ def update # rubocop:disable Metrics/CyclomaticComplexity invalid_funder = funder_attrs[:org_name].present? && funder.nil? - @plan.funder_id = funder&.id + # Only change funder_id when it is valid OR explicitly cleared + @plan.funder_id = funder&.id unless invalid_funder + @plan.grant = plan_params[:grant] attrs.delete(:funder) attrs.delete(:grant) @@ -282,7 +284,7 @@ def update # rubocop:disable Metrics/CyclomaticComplexity end else failure_msg = failure_message(@plan, _('save')) - failure_msg += _(' Invalid funder.') if invalid_funder + failure_msg = _(' Invalid funder.') if invalid_funder format.html do # TODO: Should do a `render :show` here instead but show defines too many # instance variables in the controller From 4d550654114addcafdec3a7330b64c74c8570216 Mon Sep 17 00:00:00 2001 From: momo3404 Date: Thu, 29 Jan 2026 13:48:37 -0700 Subject: [PATCH 05/10] Add helper function for funder logic --- app/controllers/plans_controller.rb | 32 +++++++++++++++++------------ 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index 0d018c4b19..54b2ed88e4 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -255,19 +255,7 @@ def update # rubocop:disable Metrics/CyclomaticComplexity end @plan.guidance_groups = GuidanceGroup.where(id: guidance_group_ids) - funder_attrs = plan_params[:funder] - - funder = - if funder_attrs[:org_name].blank? - nil # user cleared funder — valid - else - org_from_params(params_in: funder_attrs, allow_create: true) - end - - invalid_funder = funder_attrs[:org_name].present? && funder.nil? - - # Only change funder_id when it is valid OR explicitly cleared - @plan.funder_id = funder&.id unless invalid_funder + invalid_funder = assign_funder(plan_params) @plan.grant = plan_params[:grant] attrs.delete(:funder) @@ -569,5 +557,23 @@ def setup_local_orgs Org.includes(identifiers: :identifier_scheme).default_orgs) @orgs = @orgs.flatten.uniq.sort_by(&:name) end + + def assign_funder(plan_params) + funder_attrs = plan_params[:funder] + + funder = + if funder_attrs[:org_name].blank? + nil # user cleared funder — valid + else + org_from_params(params_in: funder_attrs, allow_create: true) + end + + invalid_funder = funder_attrs[:org_name].present? && funder.nil? + + # Only change funder_id when it is valid OR explicitly cleared + @plan.funder_id = funder&.id unless invalid_funder + + invalid_funder + end end # rubocop:enable Metrics/ClassLength From 02464462727b7dc5199564feffcef4b1342a2b52 Mon Sep 17 00:00:00 2001 From: momo3404 Date: Tue, 3 Feb 2026 15:16:40 -0700 Subject: [PATCH 06/10] Update error handling in plans_controller - Change handling to check for if plan updated first, then append invalid funder accordingly - Simplify some of the code for failure messages and alerts --- app/controllers/plans_controller.rb | 30 +++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index 54b2ed88e4..ca0ffcb2e4 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -262,34 +262,48 @@ def update # rubocop:disable Metrics/CyclomaticComplexity attrs.delete(:grant) attrs = remove_org_selection_params(params_in: attrs) - if @plan.update(attrs) && !invalid_funder # _attributes(attrs) + saved = @plan.update(attrs) + + if saved + notice = success_message(@plan, _('saved')) + alert = + (_('The plan was saved, but the funder was not updated because it is invalid.') if invalid_funder) + format.html do redirect_to plan_path(@plan), - notice: success_message(@plan, _('saved')) + notice: notice, + alert: alert end + format.json do - render json: { code: 1, msg: success_message(@plan, _('saved')) } + render json: { + code: 1, + msg: notice, + warning: invalid_funder ? _('Invalid funder.') : nil + } end else failure_msg = failure_message(@plan, _('save')) - failure_msg = _(' Invalid funder.') if invalid_funder format.html do # TODO: Should do a `render :show` here instead but show defines too many # instance variables in the controller redirect_to plan_path(@plan).to_s, alert: failure_msg end + format.json do render json: { code: 0, msg: failure_msg } end end rescue StandardError => e - flash[:alert] = failure_message(@plan, _('save')) + Rails.logger.error "Unable to save plan #{@plan&.id} - #{e.message}" + alert = failure_message(@plan, _('save')) + format.html do - Rails.logger.error "Unable to save plan #{@plan&.id} - #{e.message}" - redirect_to plan_path(@plan).to_s, alert: failure_message(@plan, _('save')) + redirect_to plan_path(@plan).to_s, alert: alert end + format.json do - render json: { code: 0, msg: flash[:alert] } + render json: { code: 0, msg: alert } end end # rubocop:enable Metrics/BlockLength From c40b0e23ba71de15c29ca3b670820966101a6d52 Mon Sep 17 00:00:00 2001 From: momo3404 Date: Tue, 3 Feb 2026 15:36:54 -0700 Subject: [PATCH 07/10] Update function name --- app/controllers/plans_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index ca0ffcb2e4..a6d61c8a27 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -255,7 +255,7 @@ def update # rubocop:disable Metrics/CyclomaticComplexity end @plan.guidance_groups = GuidanceGroup.where(id: guidance_group_ids) - invalid_funder = assign_funder(plan_params) + invalid_funder = invalid_funder?(plan_params) @plan.grant = plan_params[:grant] attrs.delete(:funder) @@ -572,7 +572,7 @@ def setup_local_orgs @orgs = @orgs.flatten.uniq.sort_by(&:name) end - def assign_funder(plan_params) + def invalid_funder?(plan_params) funder_attrs = plan_params[:funder] funder = From 91f57fba7f19c2db1f814b3a8e3bfe6ac47e10dc Mon Sep 17 00:00:00 2001 From: momo3404 Date: Wed, 4 Feb 2026 13:19:49 -0700 Subject: [PATCH 08/10] Change function name --- app/controllers/plans_controller.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index a6d61c8a27..2757b24c2a 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -255,7 +255,7 @@ def update # rubocop:disable Metrics/CyclomaticComplexity end @plan.guidance_groups = GuidanceGroup.where(id: guidance_group_ids) - invalid_funder = invalid_funder?(plan_params) + invalid_funder = validate_and_set_funder(plan_params[:funder]) @plan.grant = plan_params[:grant] attrs.delete(:funder) @@ -572,9 +572,7 @@ def setup_local_orgs @orgs = @orgs.flatten.uniq.sort_by(&:name) end - def invalid_funder?(plan_params) - funder_attrs = plan_params[:funder] - + def validate_and_set_funder(funder_attrs) funder = if funder_attrs[:org_name].blank? nil # user cleared funder — valid From 59f0fc4c93e9143218d234f417b39c1aa538c4f9 Mon Sep 17 00:00:00 2001 From: momo3404 Date: Wed, 4 Feb 2026 14:29:15 -0700 Subject: [PATCH 09/10] change to valid funder --- app/controllers/plans_controller.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index 2757b24c2a..be5dbba051 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -255,7 +255,7 @@ def update # rubocop:disable Metrics/CyclomaticComplexity end @plan.guidance_groups = GuidanceGroup.where(id: guidance_group_ids) - invalid_funder = validate_and_set_funder(plan_params[:funder]) + valid_funder = validate_and_set_funder(plan_params[:funder]) @plan.grant = plan_params[:grant] attrs.delete(:funder) @@ -267,7 +267,7 @@ def update # rubocop:disable Metrics/CyclomaticComplexity if saved notice = success_message(@plan, _('saved')) alert = - (_('The plan was saved, but the funder was not updated because it is invalid.') if invalid_funder) + (_('The plan was saved, but the funder was not updated because it is invalid.') unless valid_funder) format.html do redirect_to plan_path(@plan), @@ -279,7 +279,7 @@ def update # rubocop:disable Metrics/CyclomaticComplexity render json: { code: 1, msg: notice, - warning: invalid_funder ? _('Invalid funder.') : nil + warning: valid_funder ? nil : _('Invalid funder.') } end else @@ -580,12 +580,12 @@ def validate_and_set_funder(funder_attrs) org_from_params(params_in: funder_attrs, allow_create: true) end - invalid_funder = funder_attrs[:org_name].present? && funder.nil? + valid_funder = funder_attrs[:org_name].blank? || funder.present? # Only change funder_id when it is valid OR explicitly cleared - @plan.funder_id = funder&.id unless invalid_funder + @plan.funder_id = funder&.id if valid_funder - invalid_funder + valid_funder end end # rubocop:enable Metrics/ClassLength From 7b8a4244876ae26da42730df45274708a3e5acee Mon Sep 17 00:00:00 2001 From: momo3404 Date: Thu, 5 Feb 2026 13:22:14 -0700 Subject: [PATCH 10/10] remove save variable --- app/controllers/plans_controller.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/plans_controller.rb b/app/controllers/plans_controller.rb index be5dbba051..cf97101135 100644 --- a/app/controllers/plans_controller.rb +++ b/app/controllers/plans_controller.rb @@ -262,9 +262,7 @@ def update # rubocop:disable Metrics/CyclomaticComplexity attrs.delete(:grant) attrs = remove_org_selection_params(params_in: attrs) - saved = @plan.update(attrs) - - if saved + if @plan.update(attrs) notice = success_message(@plan, _('saved')) alert = (_('The plan was saved, but the funder was not updated because it is invalid.') unless valid_funder)