Issue #3555 - Added feature for Org admins can show/hide the

[johnpinto1]

Ethical_Issues fragment in plans project details page.
Co-authored-by: don-stuckey dstuckey@ed.ac.uk

This feature was contributed to DMPonline by @don-stuckey.

Changes:

• These changes only kick in if enable_ethical_issues is enable in the application config.
• Added to Org model attribute in a migration add_ro_ethical_concerns :boolean default(true), not null
• Added a checkbox 'Add "Research outputs may have ethical concerns" field to "Project details" tab in plans' to app/views/orgs/_profile_form.html.erb.
• Added code to the Plan Project Details view to show or hide the Ethical Issues fragment.
• Added :add_ro_ethical_concerns to the strong params org_params for the Org controller.

Here is the spec and tests compiled by @don-stuckey who contributed this feature to DMPonline:

Specification
For each organisation’s profile in DMPonline, the “Organisation Permission” section now includes the checkbox “Add "Research outputs may have ethical concerns" field to "Project details" tab in plans”, and the default value of this checkbox is true.

Test 1: Ensuring that the option to enable / disable the "Research outputs may have ethical concerns" checkbox for an organisation is available for super-admins

Test steps:

tester signs in as a super-admin and opens their organisation, e.g. “Organisation 1” (Org1) in edit mode 

Expected results:

tester sees the option to enable / disable the "Research outputs may have ethical concerns" checkbox for the organisation 

Test 2: Ensuring that the option to enable / disable the "Research outputs may have ethical concerns" checkbox for an organisation is available for org-admins for their organisation

Test steps:

tester signs in as a org-admin and opens their organisation, e.g. “Organisation 1” (Org1) in edit mode 

Expected results:

tester sees the option to enable / disable the "Research outputs may have ethical concerns" checkbox for their organisation 

Test 3: Disabling "Research outputs may have ethical concerns" checkbox for an organisation

Test steps:

tester signs in as a org-admin and opens their organisation, e.g. “Organisation 1” (Org1) in edit mode 

tester de-selects the checkbox “Organisation Permissions > “Add "Research outputs may have ethical concerns" field to "Project details" tab in plans” and selects “Save” 

tester signs in as a standard user associated with Org1 

tester creates a plan and saves a plan 

Expected results:

tester does not see the "Research outputs may have ethical concerns" field in the “Project details” page on creating a plan or on editing that plan they just created 

Test 4: Ensuring that existing plans that have the "Research outputs may have ethical concerns" checkbox checked still have that field as visible if the organisation disables the "Research outputs may have ethical concerns" checkbox

Test steps:

tester signs in as a org-admin and opens an organisation, e.g. “Organisation 1” (Org1) in edit mode, and ensures that  

the “Organisation Permissions > “Add "Research outputs may have ethical concerns" field to "Project details" tab in plans” field is checked 

tester signs in as a standard user associated with Org1 

tester creates a plan, checks the  "Research outputs may have ethical concerns" field, and saves the plan 

tester signs in as a org-admin and opens up Org1 in edit mode 

tester then deselects the “Organisation Permissions > “Add "Research outputs may have ethical concerns" field to "Project details" tab in plans” checkbox and saves the change 

tester signs in as a standard user  

Expected results:

tester should still see the "Research outputs may have ethical concerns" checkbox in the plan they created even though that field has been disabled for all new (not existing) plans going forward 

Test 5: Ensuring that existing plans that have the do not have the "Research outputs may have ethical concerns" checkbox checked subsequently do not show that field when the organisation disables the "Research outputs may have ethical concerns" checkbox

Test steps:

tester signs in as a org-admin and opens an organisation, e.g. “Organisation 1” (Org1) in edit mode, and ensures that  

the “Organisation Permissions > “Add "Research outputs may have ethical concerns" field to "Project details" tab in plans” checkbox is checked 

tester signs in as a standard user associated with Org1 

tester creates a plan, and does not check the "Research outputs may have ethical concerns" checkbox, and saves the plan 

tester signs in as a org-admin and opens up Org1 in edit mode 

tester then de-selects the “Organisation Permissions > “Add "Research outputs may have ethical concerns" field to "Project details" tab in plans” checkbox and saves the change 

tester signs in as a standard user and opens the plan they just created 

Expected results:

tester should not see the "Research outputs may have ethical concerns" checkbox in the plan they created 

[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

[gjacob24]

Looks good @johnpinto1. I used the same test cases that we used in DMPonline to test this branch.

[aaronskiba]

It may be best to recreate this migration file (i.e. rails generate migration AddAddRoEthicalConcernsToOrgs) so that it uses the version of Rails that we currently on (i.e. 7.1 rather than 6.1).

[johnpinto1]

The migration has been re-created for Rails version 7.1 as suggested.

[aaronskiba]

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 %>

[aaronskiba]

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"

[aaronskiba]

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

[aaronskiba]

I'm considering the research outputs ethical concerns handling we already have prior to this change. We have Rails.configuration.x.madmp.enable_ethical_issues to enable/disable the option globally. Users also have the "Research outputs may have ethical concerns" checkbox to enable/disable it on a per plan level. I wonder how necessary it is to also have the same enable/disable option at the org level.

There is also the issue of users from different orgs collaborating on the same plan. current_user.org.add_ro_ethical_concerns? could evaluate differently for different users working on the same plan. If we go with this approach, would we also need to address that scenario?

Also, I'm not sure if DMP Assistant would even want org admins to have the ability to customise the visibility of this field. Hopefully, this is a conversation we can have at the next all-team monthly meeting.

[johnpinto1]

I'm considering the research outputs ethical concerns handling we already have prior to this change. We have Rails.configuration.x.madmp.enable_ethical_issues to enable/disable the option globally. Users also have the "Research outputs may have ethical concerns" checkbox to enable/disable it on a per plan level. I wonder how necessary it is to also have the same enable/disable option at the org level.

There is also the issue of users from different orgs collaborating on the same plan. current_user.org.add_ro_ethical_concerns? could evaluate differently for different users working on the same plan. If we go with this approach, would we also need to address that scenario?

Also, I'm not sure if DMP Assistant would even want org admins to have the ability to customise the visibility of this field. Hopefully, this is a conversation we can have at the next all-team monthly meeting.

I think the way forward with this is you don't make this option available in the Org Admin view. In a DSpace based project I work we customise the base code to our requirements. Otherwise, we won't be able add features because it is unlikely that some features will be universally required. Many European and British users want this feature as they feel having the Ethical Questions in a DMP confuses researchers.