-
Notifications
You must be signed in to change notification settings - Fork 2
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
Continued authz #81
Continued authz #81
Conversation
1f031b1
to
06c401f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A great start, I'm looking forward to building on top of this!
also fixes test
There is a lot of duplicated code here. The plan is to defer refactoring for now and take a look at a better error handling system.
a4b733d
to
5e4e6db
Compare
5e4e6db
to
501d1d6
Compare
.route("/notes", get(note_controller::index)) | ||
.route_layer(from_fn_with_state(app_state.clone(), protect::notes::index)) | ||
.merge( | ||
// GET /notes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding these inline for readability as there are a few nestings of functions/methods
@calebbourg This looks great, feel free to merge whenever ready. The |
Description
Adds Authorization plumbing and functions for
index
actions.There's a good amount of duplicated code. I think I'm at a point where I'd like to audit and re-think our error handling a bit. Probably use an existing crate. My thought is that it will be much easier to refactor some of this authorization stuff to remove the duplication if I take a step back and improve on how errors are propagated through our abstraction layers.
The plan would be something like this:
Testing Strategy
valid coaching_relationship_id