-
Notifications
You must be signed in to change notification settings - Fork 24
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
All Features and Pages Complete - No Testing #15
base: master
Are you sure you want to change the base?
Changes from all commits
9ef022d
bba87ea
c868958
d37dc0b
16190a5
d98e525
a968ded
ec7ed40
ddebf58
6324e95
04e3654
a9b0318
18cc8ab
d4c3447
f98cd01
fd84e9e
de0ba2d
e824a61
b3d1b05
868ee96
5350af3
92d2db6
670d66c
bf152dc
8cddc20
69f285a
aaf4398
4233533
40ec870
b6c832b
86954cb
621b941
d708f29
99a761d
dccb1a1
c573fdb
7e7b83f
bd63de3
4bbddf8
a9fba31
9dcc9ec
b29a369
c7b9249
a37dd56
b9704f3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# Place all the behaviors and hooks related to the matching controller here. | ||
# All this logic will automatically be available in application.js. | ||
# You can use CoffeeScript in this file: http://coffeescript.org/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Best practice is to delete these files you never use. They're auto-generated by rails. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# Place all the behaviors and hooks related to the matching controller here. | ||
# All this logic will automatically be available in application.js. | ||
# You can use CoffeeScript in this file: http://coffeescript.org/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
// Place all the styles related to the Sessions controller here. | ||
// They will automatically be included in application.css. | ||
// You can use Sass (SCSS) here: http://sass-lang.com/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
// Place all the styles related to the Suggestions controller here. | ||
// They will automatically be included in application.css. | ||
// You can use Sass (SCSS) here: http://sass-lang.com/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,4 +2,70 @@ class ApplicationController < ActionController::Base | |
# Prevent CSRF attacks by raising an exception. | ||
# For APIs, you may want to use :null_session instead. | ||
protect_from_forgery with: :exception | ||
helper_method :current_user | ||
|
||
before_action :require_login | ||
|
||
def current_user | ||
@current_user ||= User.find_by(id: session[:user_id]) | ||
end | ||
|
||
def require_login | ||
if current_user.nil? | ||
flash[:error] = "You must be logged in to view this section" | ||
redirect_to root_path | ||
end | ||
end | ||
|
||
def extract_suggestions(suggestions) | ||
if suggestions.class != Array | ||
suggestions = pull_from_id(suggestions.suggestions) | ||
suggestions.each do |suggestion_hash| | ||
food = suggestion_hash.suggestion["food_id"] | ||
suggestion_hash.suggestion["food_suggestion"]= Food.retrieve(food) | ||
suggestion_hash.suggestion["music_suggestion"] = Music.retrieve(suggestion_hash.suggestion["music_id"], suggestion_hash.suggestion["music_type"]) | ||
end | ||
return suggestions | ||
end | ||
suggestions.each do |suggestion_hash| | ||
food = suggestion_hash["food_id"] | ||
suggestion_hash["food_suggestion"]= Food.retrieve(food) | ||
suggestion_hash["music_suggestion"] = Music.retrieve(suggestion_hash["music_id"], suggestion_hash["music_type"]) | ||
end | ||
return suggestions | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic is confusing to me. What is the class of suggestions you're expecting? Is it a string? Why not do:
|
||
|
||
def pull_from_id(suggestions) | ||
hashes = [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider better naming for this variable. It's an array, but it's called "hashes." |
||
suggestions.each do |id| | ||
hashes << TunesTakeoutWrapper.specific_suggestion(id) | ||
end | ||
return hashes | ||
end | ||
|
||
def get_food(suggestions) | ||
if suggestions.count == 1 | ||
food = Food.find(id) | ||
end | ||
food_array = [] | ||
suggestions.each do |hash| | ||
hash["food_id"] = id | ||
food = Food.find(id) | ||
food_array << food | ||
end | ||
return food_array | ||
end | ||
|
||
def get_music(suggestions) | ||
if suggestions.count == 1 | ||
music = Music.find(id) | ||
end | ||
music_array = [] | ||
suggestions.each do |hash| | ||
hash["music_id"] = id | ||
music = Music.find(id) | ||
music_array << music | ||
end | ||
return music_array | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On first glance, I feel like these methods belong in models, not in the application_controller. Remember that all your controllers have access to these methods, which is bad separation of concerns. (current_user and require_login totally do make sense in the application_controller) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
class SessionsController < ApplicationController | ||
skip_before_action :require_login, only: [:new, :create] | ||
|
||
def new | ||
|
||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A nice way to write this would be |
||
|
||
def create | ||
auth_hash = request.env['omniauth.auth'] | ||
if auth_hash["info"]["id"] | ||
@user = User.find_or_create_from_omniauth(auth_hash) | ||
if @user | ||
session[:user_id] = @user.id | ||
redirect_to root_path | ||
else | ||
redirect_to root_path, notice: "Failed to save the user" | ||
end | ||
else | ||
redirect_to root_path, notice: "Failed to save the user" | ||
end | ||
end | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yayyyy authentication! |
||
|
||
|
||
def destroy | ||
session.delete :user_id | ||
redirect_to root_path | ||
end | ||
|
||
end |
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.
good job not checking your secrets in to github!