samvera-deprecated / sufia

[DEPRECATED] Sufia: a fully featured, flexible Samvera repository front-end.
http://sufia.io/
Other
111 stars 78 forks source link

Batch Edit Permissions Tab is not working #2450

Closed carolyncole closed 7 years ago

carolyncole commented 8 years ago

Descriptive summary

In SUfia 7 when I select a group of works for batch editing, and I change the permissions nothing happens and the page spins forever.

In the log I see

[CANCAN] Checking edit permissions for user: blah@psu.edu with groups: ["public", "registered"]
Completed 404 Not Found in 11ms (Views: 0.2ms | ActiveRecord: 0.1ms)

This happens when I try to change the visibility of add an edit user.

Expected behavior

The spinning wheels should stop and we should get changes saved on the ui. screen shot 2016-08-10 at 12 32 26 pm

Actual behavior

The wheels spin forever and the works do not get updated screen shot 2016-08-10 at 12 33 14 pm

Steps to reproduce the behavior

When I have multiple works in the system And I go to the My Works listing And I check the box next to two or more works And I click on Batch Edit And I Click on Permissions And I change the visibility And I click on "Save Changes" Then I should see the spinning wheels And I the wheels should stop (which they do not) And "Changes Saved" should appear next to the "Save Changes" button

Related work

2330 Batch Edit Redesign

nrm149 commented 8 years ago

PSUSprint01

mjgiarlo commented 7 years ago

Fixed in scholarsphere by https://github.com/psu-stewardship/scholarsphere/commit/aba077d5b9e9895b5811c761c3e4c5755db2bd1f Consider adapting this work in Sufia.

hackartisan commented 7 years ago

Still broken.

mjgiarlo commented 7 years ago

@hackmastera in the same way? Darn, sorry!

jcoyne commented 7 years ago

😢 I'm seeing this in the logs which seems wrong:

"batch_document_ids"=>["mc391vv8943,ck321sr7565", "mc391vv8943", "ck321sr7565#permissions_display"]
jcoyne commented 7 years ago

Ah-ha! There is javascript afoot. The .updates-batches selector triggers javascript from curation_concerns, which adds parameters to the form.

jcoyne commented 7 years ago

Additionally, this line doesn't seem to work anymore: https://github.com/projecthydra/curation_concerns/blob/b1d34b15f233da1df1ed07421fffc5a8959eb18f/app/assets/javascripts/curation_concerns/batch_select.js#L18

The parameter is batch_document_ids[] rather than batch_document_ids%5B%5D

hackartisan commented 7 years ago

I've just tested with pr #3047 and it does now update visibility and permissions.

However, @awead's fix for PSU propagates these changes down to children, which makes this feature much more useful. Making a bunch of works publicly visible doesn't do much if you still have to go into each work to make its files visible.

jcoyne commented 7 years ago

@hackmastera I've had use cases at DCE where we didn't want to propagate changes like that. In fact it would be harmful to do so. For example you have a dissertation that has the text (public) and some attached data (private or institution only) which is under certain publication restrictions. This is why I chose not to copy @awead's approach. cc @dunn @mark-dce

hackartisan commented 7 years ago

@jcoyne That makes sense. I feel like that is more likely to be the exception, though. Batch edit is for quickly making a lot of changes.

I would prefer: In the case where you don't want it to propagate, make the change on the individual work. In the case where you do want them to propagate, use batch update.

mjgiarlo commented 7 years ago

@hackmastera @jcoyne We already have some machinery for handling conditional permission percolation, don't we? When you change the visibility of a work, you are taken to an interstitial page at which point you can choose, or not, to copy permissions down to member filesets (using the InheritPermissionsJob). What if we expanded that to also ask about copying permissions down to member works? (As an enhancement in Hyrax, I mean.)

hackartisan commented 7 years ago

@mjgiarlo The interstitial page only exists on a single-work edit. it would be great to pull it into the batch edit.

hackartisan commented 7 years ago

sorry I clicked the wrong button I guess? I didn't intend to reopen if @mjgiarlo judges this sufficient.

mjgiarlo commented 7 years ago

@hackmastera Ah, yes. I'll write up a ticket in Hyrax for this and tag you for review. :clap:

mjgiarlo commented 7 years ago

@hackmastera https://github.com/projecthydra-labs/hyrax/issues/351