samvera / hyrax

Hyrax is a Ruby on Rails Engine built by the Samvera community. Hyrax provides a foundation for creating many different digital repository applications.
http://hyrax.samvera.org/
Apache License 2.0
182 stars 122 forks source link

Fix visibility prompt bug #6800

Open abelemlih opened 1 month ago

abelemlih commented 1 month ago

Fixes

Fixes #6537

Summary

When saving a work in Sirenia, users consistently receive a prompt to verify changes to visibility, even when they did not make any edits to visibility. This is due to an underlying bug in comparing saved permissions with new permissions.

Guidance for testing, such as acceptance criteria or new user interface behaviors:

  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. Verify that you cannot see the prompt to "Apply changes to contents? You have changed the access level..."

Type of change (for release notes)

Detailed Description

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.

Changes proposed in this pull request:

@samvera/hyrax-code-reviewers

github-actions[bot] commented 1 month ago

Test Results

    17 files  ±0      17 suites  ±0   2h 16m 56s :stopwatch: - 2m 35s  6 704 tests ±0   6 407 :white_check_mark: +1  297 :zzz: ±0  0 :x:  - 1  13 175 runs  ±0  12 780 :white_check_mark: +1  395 :zzz: ±0  0 :x:  - 1 

Results for commit b6ce4dd5. ± Comparison against base commit 52af985f.

This pull request removes 267 and adds 267 tests. Note that renamed tests count towards both. ``` spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create # spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create # spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create # spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create # spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy AdminSet: 4a8c8ac7-ff85-4afd-81a1-8a5c47c94249 spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy Hyrax::AdministrativeSet: 1946627f-8fdb-4dd7-93dc-cb7dff3176f4 spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit AdminSet: 9ea91db6-37b3-488f-8659-94e064100920 spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit Hyrax::AdministrativeSet: 16a18629-53fa-413e-9ca7-4ff9a8dd64f7 spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update AdminSet: a555b3ee-d4c3-4f79-b2ed-7afc9c518953 spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update Hyrax::AdministrativeSet: d214f880-a833-48ca-9005-99a1a070cf9d … ``` ``` spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create # spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create # spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create # spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create # spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy AdminSet: cbfd39dc-0f5c-495f-94af-ff230cf6c082 spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy Hyrax::AdministrativeSet: 935af733-4bc1-4f64-8567-662eb889788a spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit AdminSet: 92ebb31a-8295-4ef1-b928-04a22c107898 spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit Hyrax::AdministrativeSet: 4943b486-1795-4137-8997-13d66580bf2a spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update AdminSet: 571b5c52-c728-4fca-b159-69a674e648a3 spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update Hyrax::AdministrativeSet: 8c17cf9b-7b0c-4c40-a0d1-11419db2c58f … ```

:recycle: This comment has been updated with latest results.