Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 15 additions & 6 deletions app/controllers/plans_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ def edit

# PUT /plans/1
# rubocop:disable Metrics/MethodLength, Metrics/PerceivedComplexity
def update
def update # rubocop:disable Metrics/CyclomaticComplexity
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This update action is already a bit of a monster. Maybe a private helper for the new funder assignment logic will at least prevent the new CyclomaticComplexity offense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a helper for the funder assignment logic, but that doesn't seem to remove the error. It looks like that was added due to adding !invalid_funder to this line if @plan.update(attrs) && !invalid_funder.

@plan = Plan.find(params[:id])
authorize @plan
attrs = plan_params
Expand All @@ -258,15 +258,16 @@ 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)
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'))
Expand All @@ -275,13 +276,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 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_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_msg }
end
end
rescue StandardError => e
Expand Down Expand Up @@ -559,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
2 changes: 1 addition & 1 deletion app/views/plans/_project_details.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ ethics_report_tooltip = _("Link to a protocol from a meeting with an ethics comm
<div id="funder-org-controls" class="form-group">
<div class="col-md-8">
<%= 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,
Expand Down