-
Notifications
You must be signed in to change notification settings - Fork 202
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
Add support for API clients #407
Conversation
@matthewbennink Give this one another look. I think I was actually able to solve those good points that you raised (ivars and redirect). Is this a good overall solution now? |
Yea, I think this makes sense overall. We can request information and generally get everything that's needed for the HTML view, and if we mutate anything, we'll be given a redirect URL that will give us the information we may need, or another redirect perhaps. Thinking through creating messages, if we send a POST to I did a quick search for where instance variables are set and, apart from the For More importantly, the
TL;DR - |
Oh - the only other thing I forgot to note that we'll need to deal with is skipping |
ff43c86
to
104292f
Compare
I'm loving the thoroughness. Great catch on @user. I just removed those two unnecessary setting of them. However, it really seems wrong that we should need to be concerned with a user seeing the @user object. The fact that we're even having to ask ourselves this suggests that the I think the right fix is: we no longer hack the openai_key attribute. Instead, on the user model we add a method on the model like:
I don't quite get this because every controller action already has a before_filter concerning authentication. There's a default app-wide in application_controller and some controllers override that for various actions. If we also add an authentication check in the redirect_to, which is at the bottom, then we could end up with a discrepancy between the auth check at the start of an action and the end. So that doesn't seem right. @matthewbennink Is there a situation I'm not thinking of where we need redirect_to to check it again?
I think it's just |
I think I had the unauthenticated routes in mind at the time, such as signing in. But we're no longer setting instance variables for any of those now that
Yep |
A request will be authenticated if there is a proper Bearer token in the request and JSON should be returned for all endpoints.
Automatically returning a response for actions that end with redirect_to was a little tricky. The way this was implemented was to assign a special significance to
redirect_to
with astatus: :see_other
. When this status is present, we take this as a sign of success and we render a simple success JSON response.