cpjk / canary

:hatching_chick: Elixir authorization and resource-loading library for Plug applications.
MIT License
474 stars 51 forks source link

skip non_id_actions in handle_not_found when opts[:persisted] == true #80

Closed messutied closed 4 years ago

messutied commented 5 years ago

So this PR is mostly to start a conversation, this fixes an issue (that affects me at least), which is when using load_and_authorize_resource with persisted: true in order to authorize a parent resource of the currently being targeted resource when in one of the non_id_actions it will not error, and if the controller action or template you assume that parent resource will be available then it will break, to me it makes sense to use the :not_found_handler.

This PR skips the non_id_actions when opts[:persisted] == true, if this problem is not obvious to everybody else perhaps it needs an extra parameter or perhaps it doesn't make sense in upstream at all.

cpjk commented 4 years ago

👋 Hey! I'm sorry for the lack of movement on this. I shouldn't have left this for so long without a reply.

I am not currently writing Elixir at all, and therefore, I think it would be irresponsible of me to review PRs on this project. I just kept putting off reviewing the issues/PRs on this repo until I started writing Elixir again, but never got around to it.

That is my mistake, and I'm really sorry about that 🙏. You put in the time to make this PR, and you deserve a response at the very least.

I've marked the project as "not being actively maintained" in the readme. Please feel free to fork it with a different name and put it up on hex with current fixes.