Valkyrize inherit_permissions_job #4227
Conversation
bkeese
force-pushed
the
inherit_permissions_job
branch
5 times, most recently
from
183ae0f
to
d91927c
Compare
bkeese
force-pushed
the
inherit_permissions_job
branch
from
d91927c
to
81ac924
Compare
| context "when read groups change" do | ||
| let(:name) { 'my_read_group' } | ||
| let(:type) { 'group' } | ||
| let(:access) { 'read' } | ||
|
|
||
| it 'copies permissions to its contained files' do | ||
| # files have the depositor as the edit user to begin with |
There was a problem hiding this comment.
This comment does not appear to match described line below.
| let(:access) { 'read' } | ||
|
|
||
| it 'copies permissions to its contained files' do | ||
| # files have the depositor as the edit user to begin with |
There was a problem hiding this comment.
This comment does not appear to match described line below.
| expect(file.read_groups).to match_array ["my_read_group"] | ||
| expect(file.edit_users).to match_array [user.to_s] | ||
| # files have both edit users from parent resource | ||
| Hyrax.query_service.custom_queries.find_child_filesets(resource: resource).each do |file| |
There was a problem hiding this comment.
In reading this spec, it would be possible that we have an empty array, which would mean the spec is not testing anything.
It may be helpful to capture the Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) as a variable and say that you expect it to have a specific count/size.
|
|
||
| described_class.perform_now(work) | ||
| described_class.perform_now(work, use_valkyrie: false) | ||
| work.reload.file_sets.each do |file| |
There was a problem hiding this comment.
In reading this spec, it would be possible that we have an empty array, which would mean the spec is not testing anything.
It may be helpful to capture the Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) as a variable and say that you expect it to have a specific count/size.
| expect(file.read_users).to match_array ["abc@123.com"] | ||
| expect(file.edit_users).to match_array [user.to_s] | ||
| described_class.perform_now(work, use_valkyrie: false) | ||
| work.reload.file_sets.each do |file| |
There was a problem hiding this comment.
In reading this spec, it would be possible that we have an empty array, which would mean the spec is not testing anything.
It may be helpful to capture the Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) as a variable and say that you expect it to have a specific count/size.
| described_class.perform_now(resource, use_valkyrie: true) | ||
|
|
||
| # files have single edit user from parent resource | ||
| Hyrax.query_service.custom_queries.find_child_filesets(resource: resource).each do |file| |
There was a problem hiding this comment.
In reading this spec, it would be possible that we have an empty array, which would mean the spec is not testing anything.
It may be helpful to capture the Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) as a variable and say that you expect it to have a specific count/size.
| described_class.perform_now(resource, use_valkyrie: true) | ||
|
|
||
| # files have the extra user as the read user | ||
| Hyrax.query_service.custom_queries.find_child_filesets(resource: resource).each do |file| |
There was a problem hiding this comment.
In reading this spec, it would be possible that we have an empty array, which would mean the spec is not testing anything.
It may be helpful to capture the Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) as a variable and say that you expect it to have a specific count/size.
| expect(resource.read_groups).to match_array ["my_read_group"] | ||
|
|
||
| described_class.perform_now(resource, use_valkyrie: true) | ||
| Hyrax.query_service.custom_queries.find_child_filesets(resource: resource).each do |file| |
There was a problem hiding this comment.
In reading this spec, it would be possible that we have an empty array, which would mean the spec is not testing anything.
It may be helpful to capture the Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) as a variable and say that you expect it to have a specific count/size.
| it 'copies permissions to its contained files' do | ||
| resource.permission_manager.acl.grant(:edit).to(group).save | ||
|
|
||
| # files have the depositor as the edit user to begin with |
There was a problem hiding this comment.
This comment does not appear to match described line below.
| expect(file.edit_groups).to match_array ["my_edit_group"] | ||
| expect(file.edit_users).to match_array [user.to_s] | ||
| described_class.perform_now(resource, use_valkyrie: true) | ||
| Hyrax.query_service.custom_queries.find_child_filesets(resource: resource).each do |file| |
There was a problem hiding this comment.
In reading this spec, it would be possible that we have an empty array, which would mean the spec is not testing anything.
It may be helpful to capture the Hyrax.query_service.custom_queries.find_child_filesets(resource: resource) as a variable and say that you expect it to have a specific count/size.
|
I have a lot of "copy/paste" comments, but I also am very thankful for your contribution and PR. Good stuff to move us ever towards Valkyrie. |
bkeese
force-pushed
the
inherit_permissions_job
branch
from
81ac924
to
22adf52
Compare
|
Thank you @jeremyf. Good points. Fixed. Well, not quite fixed. I missed the ones in the AF part. |
bkeese
force-pushed
the
inherit_permissions_job
branch
from
22adf52
to
8bf7896
Compare
|
OK. Now they're all fixed. |
|
Woot! Now…why is CircleCI failing. |
bkeese
force-pushed
the
inherit_permissions_job
branch
from
8bf7896
to
c90215c
Compare
Fixes #4200
Allows inherit_permissions_job to receive either an AF object or a Valkyrie resource as the Work
@samvera/hyrax-code-reviewers