samvera / hydra-head

Samvera Repository Rails Engine
Other
98 stars 41 forks source link

Don't create ACLs agressively when accessing permissions #469

Closed no-reply closed 5 years ago

no-reply commented 6 years ago

ACLs are real Fedora objects. Saving them involves a round trip, and other complications. It turns out we don't actually need to do this, and can just build a missing ACL.

Fixes #453.

@samvera/hydra-head

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 62.46% when pulling 4b0e3aaf32b6ab9305f01062703bdffcbcf4c556 on no-extra-acls into e6baf59a6c856d1bc64e5b2933af4e6b0b831772 on master.

no-reply commented 6 years ago

Trying a Hyrax build on https://github.com/samvera/hyrax/compare/hydra-head-469

cjcolvar commented 6 years ago

Four of the hyrax tests are failing on

let!(:col_none) { build(:typeless_collection, id: 'col_none', user: user, edit_users: [user.user_key], do_save: true) }

I'm able to build this collection if I attempt the save twice:

(byebug) c = FactoryBot.build(:typeless_collection, id: 'col_none', user: user, edit_users: [user.user_key], do_save: false)
#<Collection id: "col_none", depositor: "user2@example.com", title: ["Typeless Collection Title 2"], date_uploaded: nil, date_modified: nil, head: [], tail: [], collection_type_gid: nil, label: nil, relative_path: nil, import_url: nil, resource_type: [], creator: [], contributor: [], description: [], keyword: [], license: [], rights_statement: [], publisher: [], date_created: [], subject: [], language: [], identifier: [], based_near: [], related_url: [], bibliographic_citation: [], source: [], access_control_id: nil, representative_id: nil, thumbnail_id: nil>
(byebug) c.save(validate: false)
*** Ldp::BadRequest Exception: javax.jcr.PathNotFoundException: No node exists at path '/test/co/l_/no/ne/col_none' in workspace "default"

nil
(byebug) c.save(validate: false)
true
cjcolvar commented 5 years ago

@jrgriffiniii and @laritakr reviewed the failing Hyrax tests with me yesterday and I think they are purely in the test suite and not actual issues. The Avalon tests also pass with this branch so I'm inclined to merge it and cut a minor release once a PR is in place for the failing Hyrax tests.