Fix visibility prompt bug

[abelemlih]

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)

• notes-bugfix Bug Fixes

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:

• Replace comparing arrays with comparing array sizes and checking that each permission in the new permissions array exists in the saved permissions set

@samvera/hyrax-code-reviewers

[github-actions]

Test Results

    17 files  ±0      17 suites  ±0   2h 17m 46s ⏱️ - 3m 40s
 6 705 tests ±0   6 408 ✅ +1  297 💤 ±0  0 ❌  - 1 
13 178 runs  ±0  12 783 ✅ +1  395 💤 ±0  0 ❌  - 1 

Results for commit 0b0297c. ± Comparison against base commit 59a42ad.

This pull request removes 274 and adds 274 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 #<Hyrax::PermissionTemplate:0x00007f14897bd708>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007fba83ec5c80>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007f148a194600>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007fba83ffc450>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy AdminSet: 133a9822-768b-48ad-a346-396b70d63b8b
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy Hyrax::AdministrativeSet: d2a1f5ab-99e3-4f8a-af33-0f3fdb07e5a2
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit AdminSet: c5cdc4e2-8185-4228-a98c-b012fbea7e7f
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit Hyrax::AdministrativeSet: 67871b8d-cc2d-4fee-8a21-c9ae70f921ec
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update AdminSet: 84ed5efd-d6ce-4bf0-9187-6e1f28380843
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update Hyrax::AdministrativeSet: 7f6fdad7-4891-48e5-b492-1da2b3ab04db
…

spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007f25ffcd5010>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplate:0x00007fd0eac8b550>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007f25fb76a7a0>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to create #<Hyrax::PermissionTemplateAccess:0x00007fd0ea812320>
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy AdminSet: 9383c2f1-6531-4023-9726-c4f76f0c69ff
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to destroy Hyrax::AdministrativeSet: 387551d3-b7a9-4776-be01-d64894413315
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit AdminSet: b0e7d4ad-31de-4ea7-aea7-86e13b2fb854
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to edit Hyrax::AdministrativeSet: c0ebbc04-f6b9-4c8c-9abf-0bbcb109b87e
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update AdminSet: 4edd576b-350b-4f4a-b93a-925d30e16087
spec.abilities.ability_spec ‑ Hyrax::Ability AdminSets and PermissionTemplates a user without edit access is expected not to be able to update Hyrax::AdministrativeSet: 33ca9cae-0913-4aa2-88d6-63b46b8b6ea8
…

♻️ This comment has been updated with latest results.

[cjcolvar]

I think it would be good to add tests to ensure that all cases are being covered as expected. Checking size and inclusion of all items from new permissions in old permissions could miss items in old permissions which have been removed. I see that you're casting @saved_permissions into a set so would set comparison work (@saved_permissions.to_set == new_permissions.to_set) ?