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

Why is the ||= removed? current_user method #93

Open
E36lewis opened this issue Apr 15, 2023 · 2 comments
Open

Why is the ||= removed? current_user method #93

E36lewis opened this issue Apr 15, 2023 · 2 comments

Comments

@E36lewis
Copy link

I thought the whole point of having this operator was to have the user memoized? With it removed yes the current user is cached into the db, but does that not add to response time? Are there pitfalls in keeping the Current.user ||= instead of the updated: Current.user =.

Looking for clarification, more detail on this. Thanks.

@stevepolitodesign
Copy link
Owner

Excellent question! I think the issue was that I was getting failing tests when I introduced af768c2, and just assumed memoization was the culprit. However, that assumption might be incorrect. I think I'll need to investigate if the current_user test helper needs to be updated.

def current_user
if session[:current_active_session_id].present?
ActiveSession.find_by(id: session[:current_active_session_id])&.user
elsif cookies[:remember_token]
ActiveSession.find_by(remember_token: cookies[:remember_token])&.user
end
end

@passbe
Copy link

passbe commented Mar 12, 2024

I believe the tests are failing because using ||= inside current_user will not re-query the ActiveSession table. It will use the cached value inside Current.user. Therefore if a user deletes their current session current_user still returns a valid user indicating the session is still valid.

def destroy
@active_session = current_user.active_sessions.find(params[:id])
@active_session.destroy
if current_user
redirect_to account_path, notice: "Session deleted."
else
forget_active_session
reset_session
redirect_to root_path, notice: "Signed out."
end
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants