don't fall through to 204 No Content when unsure of model type #5221
Conversation
`PermissionTemplateAccessesController` decides where to redirect based on the SolrDocument's model type, but falls through to a 204 No Content if the SolrDocument isn't sure what it is. this is probably okay for successes, even if it's a bit user unfriendly, but is a serious problem for failures where we don't want to be returning a Successful response status code! even for successes, it's hard to see the downside of redirecting to the collection edit path by default. can't be worse than redirecting to a blank page.
| @@ -35,7 +35,7 @@ def after_destroy_success | |||
| redirect_to hyrax.edit_admin_admin_set_path(source_id, | |||
| anchor: 'participants'), | |||
| notice: translate('participants', scope: 'hyrax.admin.admin_sets.form.permission_update_notices') | |||
| elsif source.collection? | |||
| else | |||
There was a problem hiding this comment.
- Under what circumstances wouldn't the source
respond_to? :collection?? Is it an error that it doesn't? - Is it ever possible that the source wouldn't be an admin set or collection? What happens if the source isn't an admin set or a collection?
There was a problem hiding this comment.
Under what circumstances wouldn't the source respond_to? :collection?? Is it an error that it doesn't?
yes. this PR is about improving what happens when the SolrDocument behaves unexpectedly.
Is it ever possible that the source wouldn't be an admin set or collection? What happens if the source isn't an admin set or a collection?
anything is possible! more seriously, this is coming in because SolrDocument isn't handling the valkyrie case correctly. that's fixed in #5214.
currently, if neither branch returns true, we report a success with 204 No Content, regardless of whether we succeeded or failed to delete the PermissionTemplateAccess. after this, we'll try to route as a collection and therefore raise an error in the case that the path can't be resolved.
PermissionTemplateAccessesControllerdecides where to redirect based on theSolrDocument's model type, but falls through to a 204 No Content if the
SolrDocument isn't sure what it is.
this is probably okay for successes, even if it's a bit user unfriendly, but is
a serious problem for failures where we don't want to be returning a Successful
response status code!
even for successes, it's hard to see the downside of redirecting to the
collection edit path by default. can't be worse than redirecting to a blank
page.
@samvera/hyrax-code-reviewers