Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor pair_retro :create action to use Canary #56

Closed
mbramson opened this issue Jan 9, 2017 · 1 comment
Closed

Refactor pair_retro :create action to use Canary #56

mbramson opened this issue Jan 9, 2017 · 1 comment

Comments

@mbramson
Copy link
Contributor

mbramson commented Jan 9, 2017

The pair_retro create route is reimplementing a lot of logic that should in theory be handled by Canary. Evaluate whether this code can be improved through the use of Canary.

Some fiddling reveals that this is probably a bit more complicated than simply adding the :create action to the load_and_authorize_resource, as the Canada protocol doesn't seem to pick up the user_id of the retro, as it is passed in as a parameter nested in the pair_retro parameter. So conn.assigns.authorized will be true for admins, but not when the passed in nested user_id parameter matches the user id of the logged in user in .conn.assigns.current_user.

This might be as simple as figuring out what to match on in this case in the Canada protocol, or it may involve some more in depth logic. Could be a good opportunity for a PR to Canada or Canary.

It's also possible that the solution is actually less elegant than the currently existing code. In which case this would be closed with no code change.

@mbramson
Copy link
Contributor Author

Closing this for now, as after some further investigation, this is a pretty substantial change to canary, and the a lot of the complexity in this method will be removed once we move to Phoenix 1.3 and get FallBackControllers.

@mbramson mbramson moved this from Ready for work to Complete in Pairmotron Mar 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Pairmotron
Complete
Development

No branches or pull requests

1 participant