Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Changelog

- Feature for Org admins can show/hide the ethical_issues field on plans [#3555](https://github.com/DMPRoadmap/roadmap/pull/3555).

## v5.0.2
- Bump Ruby to v3.1.4 and use `.ruby-version` in CI
- [#3566](https://github.com/DMPRoadmap/roadmap/pull/3566)
Expand Down
1 change: 1 addition & 0 deletions app/controllers/orgs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ def org_params
.permit(:name, :abbreviation, :logo, :contact_email, :contact_name,
:remove_logo, :managed, :feedback_enabled, :org_links,
:funder, :institution, :organisation,
:add_ro_ethical_concerns,
:feedback_msg, :org_id, :org_name, :org_crosswalk,
:helpdesk_email,
identifiers_attributes: %i[identifier_scheme_id value],
Expand Down
1 change: 1 addition & 0 deletions app/models/org.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
# region_id :integer
# managed :boolean default(false), not null
# helpdesk_email :string
# add_ro_ethical_concerns :boolean default(true), not null
#
# Foreign Keys
#
Expand Down
30 changes: 30 additions & 0 deletions app/views/orgs/_profile_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,37 @@
</div>
<% end %>
</div>
<hr>
<%# Organisation Permissions block where super admin can add:
"Project details" tab in a plan
%>

<% org_perms_header_block = capture do %>
<h3>
<%= _('Organisation Permissions') %>
</h3>
<% end %>

<% add_ro_ethical_concerns_block = capture do %>
<div class="checkbox">
<%= f.label :add_ro_ethical_concerns do %>
<%= f.check_box :add_ro_ethical_concerns, { class: 'org_types', value: org.add_ro_ethical_concerns? }, "true", "false" %>
<%= _('Add "Research outputs may have ethical concerns" field to "Project details" tab in plans') %>
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if these labels are a bit too literal and developer-y. What do we think of something more along the following lines?:
h3 title: "Organisation Customisations"
Checkbox caption: "Allow users to describe potential ethical issues in plans"

<% end %>
</div>
<% end %>

<div class="row">
<% if current_user.can_super_admin? %>
<fieldset class="col-sm-8">
<%= org_perms_header_block %>
<%= add_ro_ethical_concerns_block %>
</fieldset>
<% elsif current_user.can_org_admin? %>
<%= org_perms_header_block %>
<%= add_ro_ethical_concerns_block %>
<% end %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why one uses <fieldset> tags but the other doesn't? If not, could we just refactor to the following?

    <% if current_user.can_org_admin? %>
      <fieldset class="col-sm-8">
        <%= org_perms_header_block %>
        <%= add_ro_ethical_concerns_block %>
      </fieldset>
    <% end %>

NOTE: We could omit current_user.can_super_admin? in the following case because can_org_admin? is implemented as follows:

  def can_org_admin?
    return true if can_super_admin?

    # Automatically false if the user has no Org or the Org is not managed
    return false unless org.present? && org.managed?

    can_grant_permissions? || can_modify_guidance? ||
      can_modify_templates? || can_modify_org_details? ||
      can_review_plans?
  end
  # rubo

</div>
<hr>

<% if !org.new_record? %>
Expand Down
12 changes: 11 additions & 1 deletion app/views/plans/_project_details.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ ethics_report_tooltip = _("Link to a protocol from a meeting with an ethics comm
</div>
<% end %>

<% if Rails.configuration.x.madmp.enable_ethical_issues %>
<% ethical_issues_block = capture do %>
<conditional>
<div class="form-control mb-3">
<div class="col-lg-8">
Expand Down Expand Up @@ -143,6 +143,16 @@ ethics_report_tooltip = _("Link to a protocol from a meeting with an ethics comm
</conditional>
<% end %>

<% if Rails.configuration.x.madmp.enable_ethical_issues %>
Copy link
Contributor

@aaronskiba aaronskiba Oct 27, 2025

Choose a reason for hiding this comment

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

What do you think of the following refactor?

<% if Rails.configuration.x.madmp.enable_ethical_issues && (plan.ethical_issues? || current_user.org.add_ro_ethical_concerns?) %>
  <%= ethical_issues_block %>
<% end %>

<% if plan.ethical_issues == true %>
<%= ethical_issues_block %>
<% else %>
<% if current_user.org.add_ro_ethical_concerns? %>
<%= ethical_issues_block %>
<% end %>
<% end %>
<% end %>

<conditional>
<div id="funder-org-controls" class="form-control mb-3">
<div class="col-lg-8">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddAddRoEthicalConcernsToOrgs < ActiveRecord::Migration[6.1]
def change
add_column :orgs, :add_ro_ethical_concerns, :boolean, default: true, null: false
Org.update_all(add_ro_ethical_concerns: true)
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 2025_01_15_102816) do
ActiveRecord::Schema[7.1].define(version: 2025_10_16_134521) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"

Expand Down Expand Up @@ -276,6 +276,7 @@
t.string "api_create_plan_email_subject"
t.text "api_create_plan_email_body"
t.string "helpdesk_email"
t.boolean "add_ro_ethical_concerns", default: true, null: false
t.index ["language_id"], name: "fk_rails_5640112cab"
t.index ["region_id"], name: "fk_rails_5a6adf6bab"
end
Expand Down