hyrax icon indicating copy to clipboard operation
hyrax copied to clipboard

Sirenia docker image testing - unsolicited prompt to apply changes to contents when no changes to Visibility have been made

Open eporter23 opened this issue 1 year ago • 3 comments

Descriptive summary

While testing the steps in RA_1.15 from the Hyrax testing worksheet, an Admin user making a minor metadata-only edit to a work they did not deposit can save their changes, but then gets a prompt to "Apply changes to contents?" even though visibility/access has not been changed.

Steps to reproduce the behavior in User Interface (UI)

  1. Login as an Admin user
  2. Go to Dashboard > Works > All Works
  3. Edit a work deposited by a different user OR a work you deposited
  4. Make a minor metadata-only edit somewhere in the Description tab (do not adjust Visibility)
  5. Save the changes
  6. See the prompt to "Apply changes to contents? You have changed the access level..."

Actual behavior (include screenshots if available)

Include what version of Hyrax relates to this issue (3.x, 4.x, main branch, etc.) if appropriate, and any relevant error messages/tracebacks if you're reporting a bug.

Sirenia/Fedora 6 docker image - Hyrax 5 rc2 Screen Shot 2023-12-12 at 1 22 15 PM

Acceptance Criteria/Expected Behavior

  • [ ] If an admin user edits a work they did not deposit and they do not adjust Visibility, this prompt should not appear
  • [ ] The changes to the work should save and the Admin user should be returned to the view work page

A successful metadata-only edit should be confirmed such as shown in dev.nurax: Screen Shot 2023-12-12 at 1 29 02 PM

Rationale (for feature request only)

Provide the rationale or user story that describes "why" this issue should be addressed. Especially if this is a new feature or significant change to the existing implementation.

If no edits have been made to Visibility, users should not see this prompt.

Related work

Link to related issues or prior related work here.

eporter23 avatar Dec 12 '23 18:12 eporter23

@eporter23 @rjkati the underlying problem is in this method in app/controllers/concerns/hyrax/works_controller_behavior.rb:

def permissions_changed?
      @saved_permissions !=
        case curation_concern
        when ActiveFedora::Base
          curation_concern.permissions.map(&:to_hash)
        else
          Hyrax::AccessControl.for(resource: curation_concern).permissions
        end
end

when using Valkyrie, the array comparison between @saved_permissions and Hyrax::AccessControl.for(resource: curation_concern).permissions is always returning false even when permissions are exactly the same, because order in the two arrays is not always the same.

This is the fix I added for the application we are currently developing for reference:

def permissions_changed?
      # Hyrax 5.0.1 override
      # Sometimes the order of permissions is not the same between saved permissions and new permissions for Valkyrie 
      # I am replacing array comparison (which relies on order) with comparing array sizes and checking for existence of elements in both arrays
      
      case curation_concern
      when ActiveFedora::Base
        @saved_permissions != curation_concern.permissions.map(&:to_hash)
      else 
        new_permissions = Hyrax::AccessControl.for(resource: curation_concern).permissions
        saved_permissions_set = @saved_permissions.to_set
        new_permissions.size != @saved_permissions.size || new_permissions.any? { |e| !saved_permissions_set.include? e }
      end
end

abelemlih avatar May 17 '24 20:05 abelemlih

@eporter23 @rjkati I also added this PR: https://github.com/samvera/hyrax/pull/6800

abelemlih avatar May 17 '24 20:05 abelemlih