Conversation
| private | ||
| def current_user | ||
| @current_user ||= User.find_by_id(session[:user_id]) | ||
| if (user_id = session[:user_id]) |
There was a problem hiding this comment.
Its assignment, not comparison
| @current_user ||= User.find_by_id(session[:user_id]) | ||
| if (user_id = session[:user_id]) | ||
| @current_user ||= User.find_by(id: user_id) | ||
| elsif (user_id = cookies.signed[:user_id]) |
There was a problem hiding this comment.
Its assignment, not comparison
| extend ActiveSupport::Concern | ||
|
|
||
| def update_support_issue | ||
| byebug |
|
|
||
| def update_support_issue | ||
| byebug | ||
| issue = Issue.all.find(params[:issue_id]) |
app/controllers/issues_controller.rb
Outdated
| before_action :logged_in_user, only: [:create, :destroy] | ||
| before_action :correct_user, only: :destroy | ||
|
|
||
| before_action :verify_user_has_logged_in, only: [:new, :create, :destroy] |
There was a problem hiding this comment.
@kotlav This filter should be there for every action. Move this filter to base controller.
There was a problem hiding this comment.
It is like that, 'verify_user_has_logged_in' method is defined in the base class 'ApplicationController'
app/controllers/issues_controller.rb
Outdated
|
|
||
| def issue_params | ||
| params.require(:issue).permit(:content, :imageurl, :user_id, :title) | ||
| params.require(:issue).permit(:content, :imageurl, :user_id, :title, :impact, :cost, :socialinterest_ids) |
There was a problem hiding this comment.
Don't allow user_id and you shouldn't populate user_id field in issue creation form.
app/controllers/users_controller.rb
Outdated
|
|
||
| include UsersHelper | ||
| before_action :redirect_if_logged_in, only: [:new, :create] | ||
| before_action :verify_user_has_logged_in, only: [:show] |
There was a problem hiding this comment.
'verify_user_has_logged_in' is defined in base controller only.
app/helpers/sessions_helper.rb
Outdated
| # Returns true if the user is logged in, false otherwise. | ||
| def logged_in? | ||
| !current_user.nil? | ||
| !@current_user.nil? |
app/models/social_interest.rb
Outdated
| has_and_belongs_to_many :users | ||
| has_and_belongs_to_many :issues | ||
|
|
||
| # def initialize(hash) |
app/models/user.rb
Outdated
| class User < ApplicationRecord | ||
| has_many :issues, dependent: :destroy | ||
| has_many :issues, dependent: :destroy # created issues | ||
| has_and_belongs_to_many :social_interests, class_name: "SocialInterest" |
There was a problem hiding this comment.
@kotlav I think there is no need of giving class_name.
There was a problem hiding this comment.
I thought of giving another name for member var name, that's why put a class there. For my reference, I keep it for now.
kotlav
left a comment
kotlav
left a comment
There was a problem hiding this comment.
I have done the changes according to the feedback. Please review it again.
3 participants
No description provided.