From 1a2906f6c54f4141fd63f70aaecafbb89d2babe0 Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 15 Oct 2018 16:25:44 -0700 Subject: [PATCH 01/20] Added controller tests for all actions in Users and Works controllers except upvote --- app/controllers/works_controller.rb | 2 +- test/controllers/users_controller_test.rb | 39 +++++ test/controllers/works_controller_test.rb | 188 +++++++++++++++++++++- 3 files changed, 222 insertions(+), 7 deletions(-) diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 2020bee4..6084055c 100644 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -50,7 +50,7 @@ def update flash.now[:status] = :failure flash.now[:result_text] = "Could not update #{@media_category.singularize}" flash.now[:messages] = @work.errors.messages - render :edit, status: :not_found + render :edit, status: :bad_request end end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index d2c5cfbb..cfd547c3 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -1,5 +1,44 @@ require 'test_helper' describe UsersController do + let (:dan) { users(:dan) } + describe "index" do + it "succeeds when there are works" do + get users_path + + must_respond_with :success + end + + it "succeeds when there are no users" do + User.all.each do |user| + user.votes.each do |vote| + vote.destroy + end + user.destroy + end + + expect(User.count).must_equal 0 + + get users_path + + must_respond_with :success + end + end + + describe "show" do + it "succeeds for an extant work ID" do + get user_path(dan.id) + + must_respond_with :success + end + + it "renders 404 not_found for a bogus work ID" do + id = -1 + + get user_path(id) + + must_respond_with :not_found + end + end end diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 0945ca47..ac4c2c1b 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -1,19 +1,37 @@ require 'test_helper' describe WorksController do + let (:poodr) { works(:poodr) } + describe "root" do it "succeeds with all media types" do # Precondition: there is at least one media of each category + get root_path + must_respond_with :success end it "succeeds with one media type absent" do # Precondition: there is at least one media in two of the categories + expect { + delete work_path(movie.id) + }.must_change 'Work.count', -1 + + get root_path + must_respond_with :success end it "succeeds with no media" do + Work.all.each do |work| + work.destroy + end + + expect(Work.count).must_equal 0 + + get root_path + must_respond_with :success end end @@ -22,83 +40,241 @@ describe "index" do it "succeeds when there are works" do + get works_path + must_respond_with :success end it "succeeds when there are no works" do + Work.all.each do |work| + work.destroy + end + expect(Work.count).must_equal 0 + + get works_path + + must_respond_with :success end end describe "new" do it "succeeds" do + get new_work_path + must_respond_with :success end end describe "create" do it "creates a work with valid data for a real category" do - + work_hash = { + work: { + title: "Return of the King", + creator: "Tolkien", + description: "Lord of the Rings", + publication_year: 1955, + category: "book" + } + } + + expect { + post works_path, params: work_hash + }.must_change 'Work.count', 1 + + must_respond_with :redirect + must_redirect_to work_path(Work.last.id) + + expect(Work.last.title).must_equal work_hash[:work][:title] + expect(Work.last.creator).must_equal work_hash[:work][:creator] + expect(Work.last.description).must_equal work_hash[:work][:description] + expect(Work.last.publication_year).must_equal work_hash[:work][:publication_year] + expect(Work.last.category).must_equal work_hash[:work][:category] + + expect(flash[:result_text]).must_equal "Successfully created #{Work.last.category} #{Work.last.id}" end it "renders bad_request and does not update the DB for bogus data" do - + work_hash = { + work: { + title: nil, + creator: "Tolkien", + description: "Lord of the Rings", + publication_year: 1955, + category: "book" + } + } + + expect { + post works_path, params: work_hash + }.wont_change 'Work.count' + + must_respond_with :bad_request + expect(flash[:result_text]).must_equal "Could not create #{work_hash[:work][:category]}" end it "renders 400 bad_request for bogus categories" do - + work_hash = { + work: { + title: "Return of the King", + creator: "Tolkien", + description: "Lord of the Rings", + publication_year: 1955, + category: "spoken word" + } + } + + expect { + post works_path, params: work_hash + }.wont_change 'Work.count' + + must_respond_with :bad_request + expect(flash[:result_text]).must_equal "Could not create #{work_hash[:work][:category]}" end end describe "show" do it "succeeds for an extant work ID" do + get work_path(poodr.id) + must_respond_with :success end it "renders 404 not_found for a bogus work ID" do + id = -1 + + get work_path(id) + must_respond_with :not_found end end describe "edit" do it "succeeds for an extant work ID" do + get edit_work_path(poodr.id) + must_respond_with :success end it "renders 404 not_found for a bogus work ID" do + id = -1 + get work_path(id) + + # Arrange + must_respond_with :not_found end end describe "update" do it "succeeds for valid data and an extant work ID" do - + work_hash = { + work: { + title: poodr.title, + creator: poodr.creator, + description: "a new description", + publication_year: poodr.publication_year, + category: poodr.category + } + } + + expect { + patch work_path(poodr.id), params: work_hash + }.wont_change 'Work.count' + + work = Work.find_by(id: poodr.id) + + must_respond_with :redirect + must_redirect_to work_path(work.id) + + expect(work.title).must_equal work_hash[:work][:title] + expect(work.creator).must_equal work_hash[:work][:creator] + expect(work.description).must_equal work_hash[:work][:description] + expect(work.publication_year).must_equal work_hash[:work][:publication_year] + expect(work.category).must_equal work_hash[:work][:category] + + expect(flash[:result_text]).must_equal "Successfully updated #{work.category} #{work.id}" end it "renders bad_request for bogus data" do - + work_hash = { + work: { + title: nil, + creator: poodr.creator, + description: poodr.description, + publication_year: poodr.publication_year, + category: poodr.category + } + } + + work = Work.find_by(id: poodr.id) + + expect { + patch work_path(work.id), params: work_hash + }.wont_change 'Work.count' + + must_respond_with :bad_request + expect(flash[:result_text]).must_equal "Could not update #{work_hash[:work][:category]}" + + # expect no change + expect(work.title).must_equal poodr.title + expect(work.creator).must_equal poodr.creator + expect(work.description).must_equal poodr.description + expect(work.publication_year).must_equal poodr.publication_year + expect(work.category).must_equal poodr.category end it "renders 404 not_found for a bogus work ID" do - + work_hash = { + work: { + title: poodr.title, + creator: poodr.creator, + description: "won't work", + publication_year: poodr.publication_year, + category: poodr.category + } + } + + id = -1 + + expect { + patch work_path(id), params: work_hash + }.wont_change 'Work.count' + + must_respond_with :not_found end end describe "destroy" do it "succeeds for an extant work ID" do + expect { + delete work_path(poodr.id) + }.must_change 'Work.count', -1 + must_respond_with :redirect + expect(flash[:result_text]).must_equal "Successfully destroyed #{poodr.category} #{poodr.id}" + expect(Work.find_by(id: poodr.id)).must_be_nil end it "renders 404 not_found and does not update the DB for a bogus work ID" do + id = -1 + + expect { + delete work_path(id) + }.wont_change 'Work.count' + must_respond_with :not_found end end describe "upvote" do it "redirects to the work page if no user is logged in" do + post upvote_path(poodr.id) + must_respond_with :redirect + must_redirect_to work_path(poodr.id) end it "redirects to the work page after the user has logged out" do From cd5663cf9ea70aa48ae8828091cf97fcf15b4415 Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 15 Oct 2018 16:29:01 -0700 Subject: [PATCH 02/20] Apologies for not branching with the previous commit... --- test/controllers/works_controller_test.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index ac4c2c1b..d20fab7d 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -6,6 +6,7 @@ describe "root" do it "succeeds with all media types" do # Precondition: there is at least one media of each category + get root_path must_respond_with :success @@ -13,6 +14,7 @@ it "succeeds with one media type absent" do # Precondition: there is at least one media in two of the categories + expect { delete work_path(movie.id) }.must_change 'Work.count', -1 From a3b169059f77afcd604ea66695e1f4635ad1a52d Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 15 Oct 2018 17:07:18 -0700 Subject: [PATCH 03/20] Added upvote tests --- test/controllers/works_controller_test.rb | 45 ++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index d20fab7d..405cba56 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -2,6 +2,8 @@ describe WorksController do let (:poodr) { works(:poodr) } + let (:movie) { works(:movie) } + let (:dan) { users(:dan) } describe "root" do it "succeeds with all media types" do @@ -14,7 +16,7 @@ it "succeeds with one media type absent" do # Precondition: there is at least one media in two of the categories - + expect { delete work_path(movie.id) }.must_change 'Work.count', -1 @@ -280,15 +282,56 @@ end it "redirects to the work page after the user has logged out" do + user_hash = { + username: "dan" + } + + post login_path, params: user_hash + + expect(session[:user_id]).must_equal dan.id + + post logout_path + expect(session[:user_id]).must_be_nil + + expect{ + post upvote_path(poodr.id) + }.wont_change 'dan.votes.count' + + must_respond_with :redirect + must_redirect_to work_path(poodr.id) end it "succeeds for a logged-in user and a fresh user-vote pair" do + user_hash = { + username: "jackie" + } + post login_path, params: user_hash + + user = User.find_by(username: "jackie") + + expect{ + post upvote_path(poodr.id) + }.must_change 'user.votes.count', 1 + + must_respond_with :redirect + must_redirect_to work_path(poodr.id) end it "redirects to the work page if the user has already voted for that work" do + user_hash = { + username: "dan" + } + + post login_path, params: user_hash + expect{ + post upvote_path(works(:album).id) + }.wont_change 'dan.votes.count' + + must_respond_with :redirect + must_redirect_to work_path(works(:album).id) end end end From daf44da547660d3faf62d585f6a1fb5e8a07e99c Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 15 Oct 2018 21:22:25 -0700 Subject: [PATCH 04/20] Completed sessions controller tests --- test/controllers/sessions_controller_test.rb | 68 ++++++++++++++++++++ test/controllers/works_controller_test.rb | 4 ++ 2 files changed, 72 insertions(+) diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index f641d15c..af7e1b6f 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -1,5 +1,73 @@ require "test_helper" describe SessionsController do + let (:dan) { users(:dan) } + it "login form" do + get login_path + + must_respond_with :success + end + + describe "login action" do + it "can create a new user" do + user_hash = { + username: 'jackie' + } + + expect { + post login_path, params: user_hash + }.must_change 'User.count', 1 + + must_respond_with :redirect + must_redirect_to root_path + + new_user = User.find_by(username: user_hash[:username]) + + expect(new_user).wont_be_nil + expect(session[:user_id]).must_equal new_user.id + + expect(flash[:result_text]).must_equal "Successfully created new user #{new_user.username} with ID #{new_user.id}" + end + + it "should log in an existing user without changing anything" do + user_hash = { + username: 'dan' + } + + expect { + post login_path, params: user_hash + }.wont_change 'User.count' + + must_respond_with :redirect + must_redirect_to root_path + + user = User.find_by(username: user_hash[:username]) + + expect(user).wont_be_nil + expect(session[:user_id]).must_equal user.id + expect(user.id).must_equal dan.id + + expect(flash[:result_text]).must_equal "Successfully logged in as existing user #{dan.username}" + end + + it "should give a bad_request for an invalid username" do + user_hash = { + username: nil + } + + expect { + post login_path, params: user_hash + }.wont_change 'User.count' + + must_respond_with :bad_request + + new_user = User.find_by(username: user_hash[:username]) + + expect(new_user).must_be_nil + expect(session[:user_id]).must_be_nil + + expect(flash[:result_text]).must_equal "Could not log in" + end + end end diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 405cba56..87f32b2d 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -277,6 +277,10 @@ it "redirects to the work page if no user is logged in" do post upvote_path(poodr.id) + expect{ + post upvote_path(poodr.id) + }.wont_change 'Vote.count' + must_respond_with :redirect must_redirect_to work_path(poodr.id) end From b425bd8c325f79137c1801d6462ef0cbad44a3bf Mon Sep 17 00:00:00 2001 From: jackie Date: Tue, 16 Oct 2018 14:24:17 -0700 Subject: [PATCH 05/20] OAuth gem and routes setup --- .gitignore | 13 +++++++++++ Gemfile | 3 +++ Gemfile.lock | 23 +++++++++++++++++++ app/models/user.rb | 11 +++++++++ config/initializers/omniauth.rb | 3 +++ config/routes.rb | 11 ++++++--- .../20181016211249_add_oauth_to_user.rb | 7 ++++++ 7 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 config/initializers/omniauth.rb create mode 100644 db/migrate/20181016211249_add_oauth_to_user.rb diff --git a/.gitignore b/.gitignore index 48fb168f..3fb9bf3f 100644 --- a/.gitignore +++ b/.gitignore @@ -13,5 +13,18 @@ !/log/.keep !/tmp/.keep +# Ignore uploaded files in development +/storage/* +!/storage/.keep + +/node_modules +/yarn-error.log + +/public/assets +.byebug_history +/coverage +.DS_Store +.env + # Ignore Byebug command history file. .byebug_history diff --git a/Gemfile b/Gemfile index 42f4bb2c..c85147e4 100644 --- a/Gemfile +++ b/Gemfile @@ -20,6 +20,9 @@ gem 'coffee-rails', '~> 4.2' # See https://github.com/rails/execjs#readme for more supported runtimes # gem 'therubyracer', platforms: :ruby +gem 'omniauth' +gem 'omniauth-github' + # Use jquery as the JavaScript library gem 'jquery-rails' # Turbolinks makes navigating your web application faster. Read more: https://github.com/turbolinks/turbolinks diff --git a/Gemfile.lock b/Gemfile.lock index 5b407e7e..b4b8ef23 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -72,9 +72,12 @@ GEM debug_inspector (0.0.3) erubi (1.7.1) execjs (2.7.0) + faraday (0.15.3) + multipart-post (>= 1.2, < 3) ffi (1.9.25) globalid (0.4.1) activesupport (>= 4.2.0) + hashie (3.5.7) i18n (1.1.0) concurrent-ruby (~> 1.0) jbuilder (2.7.0) @@ -84,6 +87,7 @@ GEM rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) + jwt (2.1.0) listen (3.0.8) rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) @@ -113,9 +117,26 @@ GEM minitest (~> 5.0) rails (>= 4.1) multi_json (1.13.1) + multi_xml (0.6.0) + multipart-post (2.0.0) nio4r (2.3.1) nokogiri (1.8.4) mini_portile2 (~> 2.3.0) + oauth2 (1.4.1) + faraday (>= 0.8, < 0.16.0) + jwt (>= 1.0, < 3.0) + multi_json (~> 1.3) + multi_xml (~> 0.5) + rack (>= 1.2, < 3) + omniauth (1.8.1) + hashie (>= 3.4.6, < 3.6.0) + rack (>= 1.6.2, < 3) + omniauth-github (1.3.0) + omniauth (~> 1.5) + omniauth-oauth2 (>= 1.4.0, < 2.0) + omniauth-oauth2 (1.5.0) + oauth2 (~> 1.1) + omniauth (~> 1.2) pg (0.21.0) popper_js (1.14.3) pry (0.11.3) @@ -214,6 +235,8 @@ DEPENDENCIES minitest-reporters minitest-skip minitest-spec-rails + omniauth + omniauth-github pg (~> 0.18) pry-rails puma (~> 3.0) diff --git a/app/models/user.rb b/app/models/user.rb index 4cac8fe0..5411d47e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,4 +3,15 @@ class User < ApplicationRecord has_many :ranked_works, through: :votes, source: :work validates :username, uniqueness: true, presence: true + + def self.build_from_github(auth_hash) + user = User.new + + user.provider = 'github' + user.uid = auth_hash[:uid] + user.username = auth_hash[:info][:nickname] + user.name = auth_hash[:info][:name] + + return user + end end diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb new file mode 100644 index 00000000..fd441612 --- /dev/null +++ b/config/initializers/omniauth.rb @@ -0,0 +1,3 @@ +Rails.application.config.middleware.use OmniAuth::Builder do + provider :github, ENV["GITHUB_CLIENT_ID"], ENV["GITHUB_CLIENT_SECRET"], scope: "user:email" +end diff --git a/config/routes.rb b/config/routes.rb index a7e8af1d..8755567b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,9 +1,14 @@ Rails.application.routes.draw do # For details on the DSL available within this file, see http://guides.rubyonrails.org/routing.html root 'works#root' - get '/login', to: 'sessions#login_form', as: 'login' - post '/login', to: 'sessions#login' - post '/logout', to: 'sessions#logout', as: 'logout' + + # ROUTES PRIOR TO OAUTH USE: + # get '/login', to: 'sessions#login_form', as: 'login' + # post '/login', to: 'sessions#login' + # post '/logout', to: 'sessions#logout', as: 'logout' + + get "/auth/:provider/callback", to: "sessions#create" + delete "/logout", to: "sessions#destroy", as: "logout" resources :works post '/works/:id/upvote', to: 'works#upvote', as: 'upvote' diff --git a/db/migrate/20181016211249_add_oauth_to_user.rb b/db/migrate/20181016211249_add_oauth_to_user.rb new file mode 100644 index 00000000..016ee0ef --- /dev/null +++ b/db/migrate/20181016211249_add_oauth_to_user.rb @@ -0,0 +1,7 @@ +class AddOauthToUser < ActiveRecord::Migration[5.2] + def change + add_column :users, :uid, :integer, null: false + add_column :users, :provider, :string, null: false + add_column :users, :name, :string + end +end From 9499881b15ed05c893f99cb8f78adfaef029d529 Mon Sep 17 00:00:00 2001 From: jackie Date: Tue, 16 Oct 2018 14:25:06 -0700 Subject: [PATCH 06/20] Modified sessions controller create and destroy methods for OAuth --- app/controllers/sessions_controller.rb | 69 ++++++++++++++++++-------- 1 file changed, 49 insertions(+), 20 deletions(-) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 5bce99e6..f870af58 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,34 +1,63 @@ class SessionsController < ApplicationController - def login_form - end + def create + auth_hash = request.env['omniauth.auth'] - def login - username = params[:username] - if username and user = User.find_by(username: username) - session[:user_id] = user.id - flash[:status] = :success - flash[:result_text] = "Successfully logged in as existing user #{user.username}" + user = User.find_by(uid: auth_hash[:uid], provider: 'github') + if user + flash[:success] = "Logged in as returning user #{user.name}" else - user = User.new(username: username) + user = User.build_from_github(auth_hash) + if user.save - session[:user_id] = user.id - flash[:status] = :success - flash[:result_text] = "Successfully created new user #{user.username} with ID #{user.id}" + flash[:success] = "Logged in as new user #{user.name}" else - flash.now[:status] = :failure - flash.now[:result_text] = "Could not log in" - flash.now[:messages] = user.errors.messages - render "login_form", status: :bad_request - return + flash[:error] = "Could not create new user account: #{user.errors.messages}" + redirect_to root_path end end + + session[:user_id] = user.id redirect_to root_path end - def logout + def destroy session[:user_id] = nil - flash[:status] = :success - flash[:result_text] = "Successfully logged out" + flash[:success] = "Successfully logged out!" + redirect_to root_path end + + # PRIOR TO OAUTH: + # def login_form + # end + # + # def login + # username = params[:username] + # if username and user = User.find_by(username: username) + # session[:user_id] = user.id + # flash[:status] = :success + # flash[:result_text] = "Successfully logged in as existing user #{user.username}" + # else + # user = User.new(username: username) + # if user.save + # session[:user_id] = user.id + # flash[:status] = :success + # flash[:result_text] = "Successfully created new user #{user.username} with ID #{user.id}" + # else + # flash.now[:status] = :failure + # flash.now[:result_text] = "Could not log in" + # flash.now[:messages] = user.errors.messages + # render "login_form", status: :bad_request + # return + # end + # end + # redirect_to root_path + # end + # + # def logout + # session[:user_id] = nil + # flash[:status] = :success + # flash[:result_text] = "Successfully logged out" + # redirect_to root_path + # end end From a8f428b065bb0f548671dda3b4c3592e92698e60 Mon Sep 17 00:00:00 2001 From: jackie Date: Tue, 16 Oct 2018 14:49:34 -0700 Subject: [PATCH 07/20] Ran migration, modified login button and flash messages, added missing development gem to Gemfile --- Gemfile | 1 + Gemfile.lock | 5 +++ app/controllers/sessions_controller.rb | 51 ++++++-------------------- app/views/layouts/application.html.erb | 6 +-- db/schema.rb | 31 +++++++++------- 5 files changed, 38 insertions(+), 56 deletions(-) diff --git a/Gemfile b/Gemfile index c85147e4..de76c524 100644 --- a/Gemfile +++ b/Gemfile @@ -65,6 +65,7 @@ group :development do # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring gem 'spring' gem 'spring-watcher-listen', '~> 2.0.0' + gem 'dotenv-rails' end # Windows does not include zoneinfo files, so bundle the tzinfo-data gem diff --git a/Gemfile.lock b/Gemfile.lock index b4b8ef23..f66ae74f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -70,6 +70,10 @@ GEM concurrent-ruby (1.0.5) crass (1.0.4) debug_inspector (0.0.3) + dotenv (2.5.0) + dotenv-rails (2.5.0) + dotenv (= 2.5.0) + railties (>= 3.2, < 6.0) erubi (1.7.1) execjs (2.7.0) faraday (0.15.3) @@ -228,6 +232,7 @@ DEPENDENCIES bootstrap (~> 4.1.3) byebug coffee-rails (~> 4.2) + dotenv-rails jbuilder (~> 2.5) jquery-rails listen (~> 3.0.5) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index f870af58..228affba 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -4,15 +4,21 @@ def create user = User.find_by(uid: auth_hash[:uid], provider: 'github') if user - flash[:success] = "Logged in as returning user #{user.name}" + session[:user_id] = user.id + flash[:status] = :success + flash[:result_text] = "Successfully logged in as existing user #{user.username}" else user = User.build_from_github(auth_hash) if user.save - flash[:success] = "Logged in as new user #{user.name}" + session[:user_id] = user.id + flash[:status] = :success + flash[:result_text] = "Successfully created new user #{user.username}" else - flash[:error] = "Could not create new user account: #{user.errors.messages}" - redirect_to root_path + flash.now[:status] = :failure + flash.now[:result_text] = "Could not log in" + flash.now[:messages] = user.errors.messages + render "login_form", status: :bad_request end end @@ -22,42 +28,9 @@ def create def destroy session[:user_id] = nil - flash[:success] = "Successfully logged out!" + flash[:status] = :success + flash[:result_text] = "Successfully logged out" redirect_to root_path end - - # PRIOR TO OAUTH: - # def login_form - # end - # - # def login - # username = params[:username] - # if username and user = User.find_by(username: username) - # session[:user_id] = user.id - # flash[:status] = :success - # flash[:result_text] = "Successfully logged in as existing user #{user.username}" - # else - # user = User.new(username: username) - # if user.save - # session[:user_id] = user.id - # flash[:status] = :success - # flash[:result_text] = "Successfully created new user #{user.username} with ID #{user.id}" - # else - # flash.now[:status] = :failure - # flash.now[:result_text] = "Could not log in" - # flash.now[:messages] = user.errors.messages - # render "login_form", status: :bad_request - # return - # end - # end - # redirect_to root_path - # end - # - # def logout - # session[:user_id] = nil - # flash[:status] = :success - # flash[:result_text] = "Successfully logged out" - # redirect_to root_path - # end end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index e7b07ce4..4d3ef694 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -36,16 +36,16 @@ <% if @login_user %>
  • - <%= link_to "Logged in as #{@login_user.username}", user_path(@login_user), class: "btn btn-primary" %> + <%= link_to "View profile", user_path(@login_user), class: "btn btn-primary" %>
  • - <%= link_to "Log Out", logout_path, method: :post, class: "btn btn-primary" %> + <%= link_to "Log out", logout_path, method: "delete", class: "btn btn-primary" %>
    • <% else %>
  • - <%= link_to "Log In", login_path, class: "btn btn-primary" %> + <%= link_to "Login with Github", "/auth/github", class: "btn btn-primary" %>
    • <% end %> diff --git a/db/schema.rb b/db/schema.rb index 6bc8ba5c..a9380f82 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,35 +10,38 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170407164321) do +ActiveRecord::Schema.define(version: 2018_10_16_211249) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" create_table "users", force: :cascade do |t| - t.string "username" + t.string "username" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "uid", null: false + t.string "provider", null: false + t.string "name" end create_table "votes", force: :cascade do |t| - t.integer "user_id" - t.integer "work_id" + t.integer "user_id" + t.integer "work_id" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["user_id"], name: "index_votes_on_user_id", using: :btree - t.index ["work_id"], name: "index_votes_on_work_id", using: :btree + t.index ["user_id"], name: "index_votes_on_user_id" + t.index ["work_id"], name: "index_votes_on_work_id" end create_table "works", force: :cascade do |t| - t.string "title" - t.string "creator" - t.string "description" - t.string "category" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.integer "vote_count", default: 0 - t.integer "publication_year" + t.string "title" + t.string "creator" + t.string "description" + t.string "category" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.integer "vote_count", default: 0 + t.integer "publication_year" end add_foreign_key "votes", "users" From c0f49092877344a60c80adef48f85c62bb226b7f Mon Sep 17 00:00:00 2001 From: jackie Date: Tue, 16 Oct 2018 15:57:50 -0700 Subject: [PATCH 08/20] Modified controller tests to account for OAuth --- config/routes.rb | 4 +- test/controllers/sessions_controller_test.rb | 140 ++++++++++--------- test/controllers/users_controller_test.rb | 49 +++++++ test/controllers/works_controller_test.rb | 23 +-- test/fixtures/users.yml | 6 + test/test_helper.rb | 26 ++++ 6 files changed, 160 insertions(+), 88 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 8755567b..c994ea09 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -7,8 +7,8 @@ # post '/login', to: 'sessions#login' # post '/logout', to: 'sessions#logout', as: 'logout' - get "/auth/:provider/callback", to: "sessions#create" - delete "/logout", to: "sessions#destroy", as: "logout" + get '/auth/:provider/callback', to: 'sessions#create', as: 'auth_callback' + delete '/logout', to: 'sessions#destroy', as: 'logout' resources :works post '/works/:id/upvote', to: 'works#upvote', as: 'upvote' diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index af7e1b6f..f17b208d 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -1,73 +1,75 @@ require "test_helper" describe SessionsController do - let (:dan) { users(:dan) } - - it "login form" do - get login_path - - must_respond_with :success - end - - describe "login action" do - it "can create a new user" do - user_hash = { - username: 'jackie' - } - - expect { - post login_path, params: user_hash - }.must_change 'User.count', 1 - - must_respond_with :redirect - must_redirect_to root_path - - new_user = User.find_by(username: user_hash[:username]) - - expect(new_user).wont_be_nil - expect(session[:user_id]).must_equal new_user.id - - expect(flash[:result_text]).must_equal "Successfully created new user #{new_user.username} with ID #{new_user.id}" - end - - it "should log in an existing user without changing anything" do - user_hash = { - username: 'dan' - } - - expect { - post login_path, params: user_hash - }.wont_change 'User.count' - - must_respond_with :redirect - must_redirect_to root_path - - user = User.find_by(username: user_hash[:username]) - - expect(user).wont_be_nil - expect(session[:user_id]).must_equal user.id - expect(user.id).must_equal dan.id - - expect(flash[:result_text]).must_equal "Successfully logged in as existing user #{dan.username}" - end - - it "should give a bad_request for an invalid username" do - user_hash = { - username: nil - } - - expect { - post login_path, params: user_hash - }.wont_change 'User.count' - - must_respond_with :bad_request - - new_user = User.find_by(username: user_hash[:username]) - - expect(new_user).must_be_nil - expect(session[:user_id]).must_be_nil - - expect(flash[:result_text]).must_equal "Could not log in" - end - end + # THESE TESTS NO LONGER NEEDED AFTER OAUTH + + # let (:dan) { users(:dan) } + # + # it "login form" do + # get login_path + # + # must_respond_with :success + # end + # + # describe "login action" do + # it "can create a new user" do + # user_hash = { + # username: 'jackie' + # } + # + # expect { + # post login_path, params: user_hash + # }.must_change 'User.count', 1 + # + # must_respond_with :redirect + # must_redirect_to root_path + # + # new_user = User.find_by(username: user_hash[:username]) + # + # expect(new_user).wont_be_nil + # expect(session[:user_id]).must_equal new_user.id + # + # expect(flash[:result_text]).must_equal "Successfully created new user #{new_user.username} with ID #{new_user.id}" + # end + # + # it "should log in an existing user without changing anything" do + # user_hash = { + # username: 'dan' + # } + # + # expect { + # post login_path, params: user_hash + # }.wont_change 'User.count' + # + # must_respond_with :redirect + # must_redirect_to root_path + # + # user = User.find_by(username: user_hash[:username]) + # + # expect(user).wont_be_nil + # expect(session[:user_id]).must_equal user.id + # expect(user.id).must_equal dan.id + # + # expect(flash[:result_text]).must_equal "Successfully logged in as existing user #{dan.username}" + # end + # + # it "should give a bad_request for an invalid username" do + # user_hash = { + # username: nil + # } + # + # expect { + # post login_path, params: user_hash + # }.wont_change 'User.count' + # + # must_respond_with :bad_request + # + # new_user = User.find_by(username: user_hash[:username]) + # + # expect(new_user).must_be_nil + # expect(session[:user_id]).must_be_nil + # + # expect(flash[:result_text]).must_equal "Could not log in" + # end + # end end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index cfd547c3..866fbd34 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -41,4 +41,53 @@ must_respond_with :not_found end end + + describe "auth_callback" do + it "logs in an existing user and redirects to the root route" do + expect { + perform_login(dan) + }.wont_change 'User.count' + + expect(session[:user_id]).must_equal dan.id + + must_redirect_to root_path + end + + it "creates an account for a new user and redirects to the root route" do + user = User.new(provider: "github", uid: 99999, username: "test_user", name: "Test Person") + + expect { + perform_login(user) + }.must_change 'User.count', 1 + + must_redirect_to root_path + + new_user = User.find_by(username: user.username) + + expect(new_user).wont_be_nil + expect(session[:user_id]).must_equal new_user.id + + must_redirect_to root_path + + expect(flash[:result_text]).must_equal "Successfully created new user #{new_user.username}" + end + + it "redirects to the login route if given invalid user data" do + user = User.new(provider: "github", uid: 99999, username: nil, name: "Test Person") + + expect { + perform_login(user) + }.wont_change 'User.count' + + must_respond_with :bad_request + # Possibly getting 500 here because of username is nil and cant have null value in server + + invalid_user = User.find_by(username: user.username) + + expect(invalid_user).must_be_nil + expect(session[:user_id]).must_be_nil + + expect(flash[:result_text]).must_equal "Could not log in" + end + end end diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 87f32b2d..3a8ff30f 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -1,4 +1,5 @@ require 'test_helper' +require 'pry' describe WorksController do let (:poodr) { works(:poodr) } @@ -286,15 +287,11 @@ end it "redirects to the work page after the user has logged out" do - user_hash = { - username: "dan" - } - - post login_path, params: user_hash + perform_login(dan) expect(session[:user_id]).must_equal dan.id - post logout_path + delete logout_path expect(session[:user_id]).must_be_nil @@ -307,13 +304,9 @@ end it "succeeds for a logged-in user and a fresh user-vote pair" do - user_hash = { - username: "jackie" - } + perform_login(dan) - post login_path, params: user_hash - - user = User.find_by(username: "jackie") + user = User.find_by(username: "dan") expect{ post upvote_path(poodr.id) @@ -324,11 +317,7 @@ end it "redirects to the work page if the user has already voted for that work" do - user_hash = { - username: "dan" - } - - post login_path, params: user_hash + perform_login(dan) expect{ post upvote_path(works(:album).id) diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index e2968d78..cca1be1f 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -2,6 +2,12 @@ dan: username: dan + name: Dan + uid: 123123 + provider: github kari: username: kari + name: Kari + uid: 456456 + provider: github diff --git a/test/test_helper.rb b/test/test_helper.rb index 5b4fb667..2e54c340 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -23,4 +23,30 @@ class ActiveSupport::TestCase # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order. fixtures :all # Add more helper methods to be used by all tests here... + + # OK this is what you're adding + def setup + # Once you have enabled test mode, all requests + # to OmniAuth will be short circuited to use the mock authentication hash. + # A request to /auth/provider will redirect immediately to /auth/provider/callback. + OmniAuth.config.test_mode = true + end + + # Test helper method to generate a mock auth hash + # for fixture data + def mock_auth_hash(user) + return { + provider: user.provider, + uid: user.uid, + info: { + name: user.name, + nickname: user.username + } + } + end + + def perform_login(user) + OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new(mock_auth_hash(user)) + get auth_callback_path(:github) + end end From 6938478b4c462f0b9ddec3d0c555cd9a0bd5dbda Mon Sep 17 00:00:00 2001 From: jackie Date: Tue, 16 Oct 2018 16:02:44 -0700 Subject: [PATCH 09/20] Modified model tests to account for new OAuth columns --- test/models/user_test.rb | 4 ++-- test/models/vote_test.rb | 4 ++-- test/models/work_test.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 793ce7e6..de92b83f 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -28,12 +28,12 @@ it "requires a unique username" do username = "test username" - user1 = User.new(username: username) + user1 = User.new(username: username, uid: 123, provider: 'github') # This must go through, so we use create! user1.save! - user2 = User.new(username: username) + user2 = User.new(username: username, uid: 123, provider: 'github') result = user2.save result.must_equal false user2.errors.messages.must_include :username diff --git a/test/models/vote_test.rb b/test/models/vote_test.rb index f2615aa1..f336fa86 100644 --- a/test/models/vote_test.rb +++ b/test/models/vote_test.rb @@ -16,8 +16,8 @@ end describe "validations" do - let (:user1) { User.new(username: 'chris') } - let (:user2) { User.new(username: 'chris') } + let (:user1) { User.new(username: 'chris', uid: 123, provider: 'github') } + let (:user2) { User.new(username: 'chris', uid: 123, provider: 'github') } let (:work1) { Work.new(category: 'book', title: 'House of Leaves') } let (:work2) { Work.new(category: 'book', title: 'For Whom the Bell Tolls') } diff --git a/test/models/work_test.rb b/test/models/work_test.rb index d9c00073..78243b13 100644 --- a/test/models/work_test.rb +++ b/test/models/work_test.rb @@ -83,7 +83,7 @@ it "tracks the number of votes" do work = Work.create!(title: "test title", category: "movie") 4.times do |i| - user = User.create!(username: "user#{i}") + user = User.create!(username: "user#{i}", uid: i, provider: 'github') Vote.create!(user: user, work: work) end work.vote_count.must_equal 4 @@ -97,7 +97,7 @@ # Create users to do the voting test_users = [] 20.times do |i| - test_users << User.create!(username: "user#{i}") + test_users << User.create!(username: "user#{i}", uid: i, provider: 'github') end # Create media to vote upon From b79720ae028b37cc4d7cc7c6b59a0680fe86c164 Mon Sep 17 00:00:00 2001 From: jackie Date: Tue, 16 Oct 2018 16:58:57 -0700 Subject: [PATCH 10/20] Added authorization to prevent non-logged in users from going anywhere but homepage --- app/controllers/application_controller.rb | 10 +++++++++- app/controllers/sessions_controller.rb | 2 ++ app/controllers/works_controller.rb | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c12c7c17..2289b2c0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,13 +1,21 @@ class ApplicationController < ActionController::Base protect_from_forgery with: :exception - before_action :find_user + before_action :find_user, :require_login def render_404 # DPR: this will actually render a 404 page in production raise ActionController::RoutingError.new('Not Found') end + def require_login + unless find_user + flash[:status] = :failure + flash[:result_text] = "You must be logged in to view this section" + redirect_to root_path + end + end + private def find_user if session[:user_id] diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 228affba..03e99032 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,4 +1,6 @@ class SessionsController < ApplicationController + skip_before_action :require_login, only: [:create] + def create auth_hash = request.env['omniauth.auth'] diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 6084055c..fffaeedf 100644 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -2,6 +2,7 @@ class WorksController < ApplicationController # We should always be able to tell what category # of work we're dealing with before_action :category_from_work, except: [:root, :index, :new, :create] + skip_before_action :require_login, only: [:root] def root @albums = Work.best_albums From c88db1005d674d4d11782edd86c2d2bbcdb45469 Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 29 Oct 2018 11:29:25 -0700 Subject: [PATCH 11/20] github login now passing controller tests --- app/controllers/sessions_controller.rb | 3 +- app/models/user.rb | 2 +- test/controllers/sessions_controller_test.rb | 50 +++++++++++++++++++- test/controllers/users_controller_test.rb | 49 ------------------- test/test_helper.rb | 1 + 5 files changed, 52 insertions(+), 53 deletions(-) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 228affba..e7d63361 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,3 +1,4 @@ +require 'pry' class SessionsController < ApplicationController def create auth_hash = request.env['omniauth.auth'] @@ -18,11 +19,9 @@ def create flash.now[:status] = :failure flash.now[:result_text] = "Could not log in" flash.now[:messages] = user.errors.messages - render "login_form", status: :bad_request end end - session[:user_id] = user.id redirect_to root_path end diff --git a/app/models/user.rb b/app/models/user.rb index 5411d47e..151a57d1 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,5 +1,5 @@ class User < ApplicationRecord - has_many :votes + has_many :votes, dependent: :destroy has_many :ranked_works, through: :votes, source: :work validates :username, uniqueness: true, presence: true diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index f17b208d..1b3b6c9e 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -1,8 +1,56 @@ require "test_helper" describe SessionsController do + let (:dan) { users(:dan) } + + describe "auth_callback" do + it "logs in an existing user and redirects to the root route" do + expect { + perform_login(dan) + }.wont_change 'User.count' + + expect(session[:user_id]).must_equal dan.id + + must_redirect_to root_path + end + + it "creates an account for a new user and redirects to the root route" do + user = User.new(provider: "github", uid: 99999, username: "test_user", name: "Test Person") + + expect { + perform_login(user) + }.must_change 'User.count', 1 + + new_user = User.find_by(username: user.username) + + expect(new_user).wont_be_nil + expect(session[:user_id]).must_equal new_user.id + + must_redirect_to root_path + + expect(flash[:result_text]).must_equal "Successfully created new user #{new_user.username}" + end + + it "redirects to the login route if given invalid user data" do + user = User.new(provider: "github", uid: 99999, username: nil, name: "Test Person") + + expect { + perform_login(user) + }.wont_change 'User.count' + + invalid_user = User.find_by(username: user.username) + + expect(invalid_user).must_be_nil + expect(session[:user_id]).must_be_nil + + expect(flash[:result_text]).must_equal "Could not log in" + + must_redirect_to root_path + end + end + # THESE TESTS NO LONGER NEEDED AFTER OAUTH - + # let (:dan) { users(:dan) } # # it "login form" do diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 866fbd34..cfd547c3 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -41,53 +41,4 @@ must_respond_with :not_found end end - - describe "auth_callback" do - it "logs in an existing user and redirects to the root route" do - expect { - perform_login(dan) - }.wont_change 'User.count' - - expect(session[:user_id]).must_equal dan.id - - must_redirect_to root_path - end - - it "creates an account for a new user and redirects to the root route" do - user = User.new(provider: "github", uid: 99999, username: "test_user", name: "Test Person") - - expect { - perform_login(user) - }.must_change 'User.count', 1 - - must_redirect_to root_path - - new_user = User.find_by(username: user.username) - - expect(new_user).wont_be_nil - expect(session[:user_id]).must_equal new_user.id - - must_redirect_to root_path - - expect(flash[:result_text]).must_equal "Successfully created new user #{new_user.username}" - end - - it "redirects to the login route if given invalid user data" do - user = User.new(provider: "github", uid: 99999, username: nil, name: "Test Person") - - expect { - perform_login(user) - }.wont_change 'User.count' - - must_respond_with :bad_request - # Possibly getting 500 here because of username is nil and cant have null value in server - - invalid_user = User.find_by(username: user.username) - - expect(invalid_user).must_be_nil - expect(session[:user_id]).must_be_nil - - expect(flash[:result_text]).must_equal "Could not log in" - end - end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 2e54c340..7aaf75bd 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -47,6 +47,7 @@ def mock_auth_hash(user) def perform_login(user) OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new(mock_auth_hash(user)) + get auth_callback_path(:github) end end From a21155941b03205adbfaba20521d7a9b8264ae12 Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 29 Oct 2018 14:51:19 -0700 Subject: [PATCH 12/20] Completed testing for authorization in controllers --- app/controllers/sessions_controller.rb | 1 - app/controllers/works_controller.rb | 1 + test/controllers/users_controller_test.rb | 70 +++- test/controllers/works_controller_test.rb | 488 ++++++++++++---------- 4 files changed, 326 insertions(+), 234 deletions(-) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 948b58f3..19305d52 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,4 +1,3 @@ -require 'pry' class SessionsController < ApplicationController skip_before_action :require_login, only: [:create] diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index fffaeedf..33c8729d 100644 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -75,6 +75,7 @@ def upvote end else flash[:result_text] = "You must log in to do that" + redirect_to root_path end # Refresh the page to show either the updated vote count diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index cfd547c3..d8ceed9e 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -3,42 +3,68 @@ describe UsersController do let (:dan) { users(:dan) } - describe "index" do - it "succeeds when there are works" do - get users_path + describe "logged in users" do + describe "index" do + it "succeeds when there are users" do + perform_login(dan) + get users_path - must_respond_with :success - end + must_respond_with :success + end - it "succeeds when there are no users" do - User.all.each do |user| - user.votes.each do |vote| - vote.destroy + it "succeeds when there are no users" do + User.all.each do |user| + user.votes.each do |vote| + vote.destroy + end + user.destroy end - user.destroy + + expect(User.count).must_equal 0 + + user = User.new(provider: "github", uid: 99999, username: "test_user", name: "Test Person") + + perform_login(user) + get users_path + + must_respond_with :success end + end - expect(User.count).must_equal 0 + describe "show" do + it "succeeds for an extant user ID" do + perform_login(dan) + get user_path(users(:kari).id) - get users_path + must_respond_with :success + end + + it "renders 404 not_found for a bogus user ID" do + perform_login(dan) + get user_path(-1) - must_respond_with :success + must_respond_with :not_found + end end end - describe "show" do - it "succeeds for an extant work ID" do - get user_path(dan.id) + describe "guest users" do + describe "index" do + it "cannot access user index" do + get users_path - must_respond_with :success + must_respond_with :redirect + must_redirect_to root_path + end end - it "renders 404 not_found for a bogus work ID" do - id = -1 - - get user_path(id) + describe "show" do + it "cannot access user show" do + get user_path(dan.id) - must_respond_with :not_found + must_respond_with :redirect + must_redirect_to root_path + end end end end diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 3a8ff30f..9543910e 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -1,15 +1,29 @@ require 'test_helper' -require 'pry' describe WorksController do let (:poodr) { works(:poodr) } let (:movie) { works(:movie) } let (:dan) { users(:dan) } + let (:work_hash) { + { + work: { + title: "Return of the King", + creator: "Tolkien", + description: "Lord of the Rings", + publication_year: 1955, + category: "book" + } + } + } + + CATEGORIES = %w(albums books movies) + INVALID_CATEGORIES = ["nope", "42", "", " ", "albumstrailingtext"] describe "root" do it "succeeds with all media types" do # Precondition: there is at least one media of each category + get root_path must_respond_with :success @@ -19,6 +33,7 @@ # Precondition: there is at least one media in two of the categories expect { + perform_login(dan) delete work_path(movie.id) }.must_change 'Work.count', -1 @@ -40,291 +55,342 @@ end end - CATEGORIES = %w(albums books movies) - INVALID_CATEGORIES = ["nope", "42", "", " ", "albumstrailingtext"] - - describe "index" do - it "succeeds when there are works" do - get works_path - - must_respond_with :success - end + describe "logged in users" do + describe "index" do + it "succeeds when there are works" do + perform_login(dan) + get works_path - it "succeeds when there are no works" do - Work.all.each do |work| - work.destroy + must_respond_with :success end - expect(Work.count).must_equal 0 + it "succeeds when there are no works" do + Work.all.each do |work| + work.destroy + end - get works_path + expect(Work.count).must_equal 0 - must_respond_with :success + perform_login(dan) + get works_path + + must_respond_with :success + end end - end - describe "new" do - it "succeeds" do - get new_work_path + describe "new" do + it "succeeds" do + perform_login(dan) + get new_work_path - must_respond_with :success + must_respond_with :success + end end - end - describe "create" do - it "creates a work with valid data for a real category" do - work_hash = { - work: { - title: "Return of the King", - creator: "Tolkien", - description: "Lord of the Rings", - publication_year: 1955, - category: "book" - } - } + describe "create" do + it "creates a work with valid data for a real category" do + expect { + perform_login(dan) + post works_path, params: work_hash + }.must_change 'Work.count', 1 - expect { - post works_path, params: work_hash - }.must_change 'Work.count', 1 + must_respond_with :redirect + must_redirect_to work_path(Work.last.id) - must_respond_with :redirect - must_redirect_to work_path(Work.last.id) + expect(Work.last.title).must_equal work_hash[:work][:title] + expect(Work.last.creator).must_equal work_hash[:work][:creator] + expect(Work.last.description).must_equal work_hash[:work][:description] + expect(Work.last.publication_year).must_equal work_hash[:work][:publication_year] + expect(Work.last.category).must_equal work_hash[:work][:category] - expect(Work.last.title).must_equal work_hash[:work][:title] - expect(Work.last.creator).must_equal work_hash[:work][:creator] - expect(Work.last.description).must_equal work_hash[:work][:description] - expect(Work.last.publication_year).must_equal work_hash[:work][:publication_year] - expect(Work.last.category).must_equal work_hash[:work][:category] + expect(flash[:result_text]).must_equal "Successfully created #{Work.last.category} #{Work.last.id}" + end - expect(flash[:result_text]).must_equal "Successfully created #{Work.last.category} #{Work.last.id}" - end + it "renders bad_request and does not update the DB for bogus data" do + work_hash[:work][:title] = nil - it "renders bad_request and does not update the DB for bogus data" do - work_hash = { - work: { - title: nil, - creator: "Tolkien", - description: "Lord of the Rings", - publication_year: 1955, - category: "book" - } - } + expect { + perform_login(dan) + post works_path, params: work_hash + }.wont_change 'Work.count' - expect { - post works_path, params: work_hash - }.wont_change 'Work.count' + must_respond_with :bad_request + expect(flash[:result_text]).must_equal "Could not create #{work_hash[:work][:category]}" + end - must_respond_with :bad_request - expect(flash[:result_text]).must_equal "Could not create #{work_hash[:work][:category]}" - end + it "renders 400 bad_request for bogus categories" do + work_hash[:work][:category] = "spoken word" - it "renders 400 bad_request for bogus categories" do - work_hash = { - work: { - title: "Return of the King", - creator: "Tolkien", - description: "Lord of the Rings", - publication_year: 1955, - category: "spoken word" - } - } + expect { + perform_login(dan) + post works_path, params: work_hash + }.wont_change 'Work.count' - expect { - post works_path, params: work_hash - }.wont_change 'Work.count' + must_respond_with :bad_request + expect(flash[:result_text]).must_equal "Could not create #{work_hash[:work][:category]}" + end - must_respond_with :bad_request - expect(flash[:result_text]).must_equal "Could not create #{work_hash[:work][:category]}" end - end + describe "show" do + it "succeeds for an extant work ID" do + perform_login(dan) + get work_path(poodr.id) - describe "show" do - it "succeeds for an extant work ID" do - get work_path(poodr.id) + must_respond_with :success + end - must_respond_with :success + it "renders 404 not_found for a bogus work ID" do + id = -1 + + perform_login(dan) + get work_path(id) + + must_respond_with :not_found + end end - it "renders 404 not_found for a bogus work ID" do - id = -1 + describe "edit" do + it "succeeds for an extant work ID" do + perform_login(dan) + get edit_work_path(poodr.id) - get work_path(id) + must_respond_with :success + end - must_respond_with :not_found - end - end + it "renders 404 not_found for a bogus work ID" do + id = -1 - describe "edit" do - it "succeeds for an extant work ID" do - get edit_work_path(poodr.id) + perform_login(dan) + get work_path(id) - must_respond_with :success + # Arrange + must_respond_with :not_found + end end - it "renders 404 not_found for a bogus work ID" do - id = -1 + describe "update" do + it "succeeds for valid data and an extant work ID" do + work_hash[:work][:description] = "a new description" - get work_path(id) + expect { + perform_login(dan) + patch work_path(poodr.id), params: work_hash + }.wont_change 'Work.count' - # Arrange - must_respond_with :not_found - end - end + work = Work.find_by(id: poodr.id) - describe "update" do - it "succeeds for valid data and an extant work ID" do - work_hash = { - work: { - title: poodr.title, - creator: poodr.creator, - description: "a new description", - publication_year: poodr.publication_year, - category: poodr.category - } - } + must_respond_with :redirect + must_redirect_to work_path(work.id) - expect { - patch work_path(poodr.id), params: work_hash - }.wont_change 'Work.count' + expect(work.title).must_equal work_hash[:work][:title] + expect(work.creator).must_equal work_hash[:work][:creator] + expect(work.description).must_equal work_hash[:work][:description] + expect(work.publication_year).must_equal work_hash[:work][:publication_year] + expect(work.category).must_equal work_hash[:work][:category] + + expect(flash[:result_text]).must_equal "Successfully updated #{work.category} #{work.id}" + end - work = Work.find_by(id: poodr.id) + it "renders bad_request for bogus data" do + work_hash[:work][:title] = nil - must_respond_with :redirect - must_redirect_to work_path(work.id) + work = Work.find_by(id: poodr.id) - expect(work.title).must_equal work_hash[:work][:title] - expect(work.creator).must_equal work_hash[:work][:creator] - expect(work.description).must_equal work_hash[:work][:description] - expect(work.publication_year).must_equal work_hash[:work][:publication_year] - expect(work.category).must_equal work_hash[:work][:category] + expect { + perform_login(dan) + patch work_path(work.id), params: work_hash + }.wont_change 'Work.count' - expect(flash[:result_text]).must_equal "Successfully updated #{work.category} #{work.id}" - end + must_respond_with :bad_request + expect(flash[:result_text]).must_equal "Could not update #{work_hash[:work][:category]}" - it "renders bad_request for bogus data" do - work_hash = { - work: { - title: nil, - creator: poodr.creator, - description: poodr.description, - publication_year: poodr.publication_year, - category: poodr.category - } - } + # expect no change + expect(work.title).must_equal poodr.title + expect(work.creator).must_equal poodr.creator + expect(work.description).must_equal poodr.description + expect(work.publication_year).must_equal poodr.publication_year + expect(work.category).must_equal poodr.category + end - work = Work.find_by(id: poodr.id) + it "renders 404 not_found for a bogus work ID" do + work_hash[:work][:description] = "won't work" - expect { - patch work_path(work.id), params: work_hash - }.wont_change 'Work.count' - - must_respond_with :bad_request - expect(flash[:result_text]).must_equal "Could not update #{work_hash[:work][:category]}" - - # expect no change - expect(work.title).must_equal poodr.title - expect(work.creator).must_equal poodr.creator - expect(work.description).must_equal poodr.description - expect(work.publication_year).must_equal poodr.publication_year - expect(work.category).must_equal poodr.category + id = -1 + + expect { + perform_login(dan) + patch work_path(id), params: work_hash + }.wont_change 'Work.count' + + must_respond_with :not_found + end end - it "renders 404 not_found for a bogus work ID" do - work_hash = { - work: { - title: poodr.title, - creator: poodr.creator, - description: "won't work", - publication_year: poodr.publication_year, - category: poodr.category - } - } + describe "destroy" do + it "succeeds for an extant work ID" do + expect { + perform_login(dan) + delete work_path(poodr.id) + }.must_change 'Work.count', -1 - id = -1 + must_respond_with :redirect + expect(flash[:result_text]).must_equal "Successfully destroyed #{poodr.category} #{poodr.id}" + expect(Work.find_by(id: poodr.id)).must_be_nil + end - expect { - patch work_path(id), params: work_hash - }.wont_change 'Work.count' + it "renders 404 not_found and does not update the DB for a bogus work ID" do + id = -1 + + expect { + perform_login(dan) + delete work_path(id) + }.wont_change 'Work.count' - must_respond_with :not_found + must_respond_with :not_found + end end - end - describe "destroy" do - it "succeeds for an extant work ID" do - expect { - delete work_path(poodr.id) - }.must_change 'Work.count', -1 + describe "upvote" do + it "redirects to the homepage after the user has logged out" do + perform_login(dan) - must_respond_with :redirect - expect(flash[:result_text]).must_equal "Successfully destroyed #{poodr.category} #{poodr.id}" - expect(Work.find_by(id: poodr.id)).must_be_nil - end + expect(session[:user_id]).must_equal dan.id - it "renders 404 not_found and does not update the DB for a bogus work ID" do - id = -1 + delete logout_path - expect { - delete work_path(id) - }.wont_change 'Work.count' + expect(session[:user_id]).must_be_nil + + expect{ + post upvote_path(poodr.id) + }.wont_change 'dan.votes.count' + + must_respond_with :redirect + must_redirect_to root_path + end + + it "succeeds for a logged-in user and a fresh user-vote pair" do + perform_login(dan) + + user = User.find_by(username: "dan") + + expect{ + post upvote_path(poodr.id) + }.must_change 'user.votes.count', 1 + + must_respond_with :redirect + must_redirect_to work_path(poodr.id) + end - must_respond_with :not_found + it "redirects to the work page if the user has already voted for that work" do + perform_login(dan) + + expect{ + post upvote_path(works(:album).id) + }.wont_change 'dan.votes.count' + + must_respond_with :redirect + must_redirect_to work_path(works(:album).id) + end end end - describe "upvote" do + describe "guest users" do + describe "index" do + it "cannot access index" do + get works_path - it "redirects to the work page if no user is logged in" do - post upvote_path(poodr.id) + must_respond_with :redirect + must_redirect_to root_path + expect(flash[:result_text]).must_equal "You must be logged in to view this section" + end + end - expect{ - post upvote_path(poodr.id) - }.wont_change 'Vote.count' + describe "show" do + it "cannot access show" do + get work_path(Work.first.id) - must_respond_with :redirect - must_redirect_to work_path(poodr.id) + must_respond_with :redirect + must_redirect_to root_path + expect(flash[:result_text]).must_equal "You must be logged in to view this section" + end end - it "redirects to the work page after the user has logged out" do - perform_login(dan) + describe "new" do + it "cannot access new" do + get new_work_path(poodr.id) - expect(session[:user_id]).must_equal dan.id + must_respond_with :redirect + must_redirect_to root_path + expect(flash[:result_text]).must_equal "You must be logged in to view this section" + end + end - delete logout_path + describe "create" do + it "cannot access create" do + expect{ + post works_path, params: work_hash + }.wont_change 'Work.count' - expect(session[:user_id]).must_be_nil + must_respond_with :redirect + must_redirect_to root_path + expect(flash[:result_text]).must_equal "You must be logged in to view this section" + end + end - expect{ - post upvote_path(poodr.id) - }.wont_change 'dan.votes.count' + describe "edit" do + it "cannot access edit" do + get edit_work_path(poodr.id) - must_respond_with :redirect - must_redirect_to work_path(poodr.id) + must_respond_with :redirect + must_redirect_to root_path + expect(flash[:result_text]).must_equal "You must be logged in to view this section" + end end - it "succeeds for a logged-in user and a fresh user-vote pair" do - perform_login(dan) - - user = User.find_by(username: "dan") + describe "update" do + it "cannot access update" do + patch work_path(poodr.id), params: work_hash - expect{ - post upvote_path(poodr.id) - }.must_change 'user.votes.count', 1 + must_respond_with :redirect + must_redirect_to root_path + expect(flash[:result_text]).must_equal "You must be logged in to view this section" - must_respond_with :redirect - must_redirect_to work_path(poodr.id) + expect(poodr.title).must_equal "Practical Object Oriented Design in Ruby" + expect(poodr.creator).must_equal "Sandi Metz" + expect(poodr.description).must_equal "programming!" + expect(poodr.publication_year).must_equal 2012 + expect(poodr.category).must_equal "book" + end end - it "redirects to the work page if the user has already voted for that work" do - perform_login(dan) + describe "destroy" do + it "cannot access destroy" do + expect{ + delete work_path(poodr.id) + }.wont_change 'Work.count' + + must_respond_with :redirect + must_redirect_to root_path + expect(flash[:result_text]).must_equal "You must be logged in to view this section" + + expect(poodr.title).must_equal "Practical Object Oriented Design in Ruby" + expect(poodr.creator).must_equal "Sandi Metz" + expect(poodr.description).must_equal "programming!" + expect(poodr.publication_year).must_equal 2012 + expect(poodr.category).must_equal "book" + end + end - expect{ - post upvote_path(works(:album).id) - }.wont_change 'dan.votes.count' + describe "upvote" do + it "redirects to the root path if no user is logged in" do + expect{ + post upvote_path(poodr.id) + }.wont_change 'Vote.count' - must_respond_with :redirect - must_redirect_to work_path(works(:album).id) + must_respond_with :redirect + must_redirect_to root_path + end end end end From fb293f07a6bb280d452d418918a0468140328fd5 Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 29 Oct 2018 15:08:31 -0700 Subject: [PATCH 13/20] Added controller tests for adv authorization of edit, update, delete in Works controller --- test/controllers/sessions_controller_test.rb | 8 +- test/controllers/works_controller_test.rb | 77 ++++++++++++++++---- test/fixtures/works.yml | 4 + 3 files changed, 73 insertions(+), 16 deletions(-) diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 1b3b6c9e..d89c5737 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -3,7 +3,7 @@ describe SessionsController do let (:dan) { users(:dan) } - describe "auth_callback" do + describe "create with auth_callback" do it "logs in an existing user and redirects to the root route" do expect { perform_login(dan) @@ -49,6 +49,12 @@ end end + describe "destory" do + it "logs a user out if they are logged in" do + skip + end + end + # THESE TESTS NO LONGER NEEDED AFTER OAUTH # let (:dan) { users(:dan) } diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 9543910e..132496bc 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -23,7 +23,6 @@ it "succeeds with all media types" do # Precondition: there is at least one media of each category - get root_path must_respond_with :success @@ -102,6 +101,7 @@ expect(Work.last.description).must_equal work_hash[:work][:description] expect(Work.last.publication_year).must_equal work_hash[:work][:publication_year] expect(Work.last.category).must_equal work_hash[:work][:category] + expect(Work.last.user).must_equal dan expect(flash[:result_text]).must_equal "Successfully created #{Work.last.category} #{Work.last.id}" end @@ -118,7 +118,7 @@ expect(flash[:result_text]).must_equal "Could not create #{work_hash[:work][:category]}" end - it "renders 400 bad_request for bogus categories" do + it "renders bad_request for bogus categories" do work_hash[:work][:category] = "spoken word" expect { @@ -129,7 +129,6 @@ must_respond_with :bad_request expect(flash[:result_text]).must_equal "Could not create #{work_hash[:work][:category]}" end - end describe "show" do @@ -151,7 +150,7 @@ end describe "edit" do - it "succeeds for an extant work ID" do + it "succeeds for an extant work ID and a work the user created" do perform_login(dan) get edit_work_path(poodr.id) @@ -167,10 +166,20 @@ # Arrange must_respond_with :not_found end + + it "does not permit access if logged in user is not the user the work belongs to" do + perform_login(dan) + get work_path(movie.id) + + # Arrange + must_respond_with :redirect + must_redirect_to works_path + expect(flash[:result_text]).must_equal "You can only edit works you added to the site" + end end describe "update" do - it "succeeds for valid data and an extant work ID" do + it "succeeds for valid data and an extant work ID and a work the user created" do work_hash[:work][:description] = "a new description" expect { @@ -225,6 +234,24 @@ must_respond_with :not_found end + + it "does not update work if logged in user is not the user the work belongs to" do + perform_login(dan) + patch work_path(movie.id), params: work_hash + + must_respond_with :redirect + must_redirect_to works_path + expect(flash[:result_text]).must_equal "You can only edit works you added to the site" + + # expect no change + work = Work.find_by(id: movie.id) + + expect(work.title).must_equal movie.title + expect(work.creator).must_equal movie.creator + expect(work.description).must_equal movie.description + expect(work.publication_year).must_equal movie.publication_year + expect(work.category).must_equal movie.category + end end describe "destroy" do @@ -249,6 +276,24 @@ must_respond_with :not_found end + + it "does not delete work if logged in user is not the user the work belongs to" do + expect { + perform_login(dan) + delete work_path(movie.id), params: work_hash + }.wont_change 'Work.count' + + must_respond_with :redirect + must_redirect_to works_path + expect(flash[:result_text]).must_equal "You can only delete works you added to the site" + + # expect no change + expect(Work.find_by(id: movie.id).title).must_equal movie.title + expect(Work.find_by(id: movie.id).creator).must_equal movie.creator + expect(Work.find_by(id: movie.id).description).must_equal movie.description + expect(Work.find_by(id: movie.id).publication_year).must_equal movie.publication_year + expect(Work.find_by(id: movie.id).category).must_equal movie.category + end end describe "upvote" do @@ -356,11 +401,13 @@ must_redirect_to root_path expect(flash[:result_text]).must_equal "You must be logged in to view this section" - expect(poodr.title).must_equal "Practical Object Oriented Design in Ruby" - expect(poodr.creator).must_equal "Sandi Metz" - expect(poodr.description).must_equal "programming!" - expect(poodr.publication_year).must_equal 2012 - expect(poodr.category).must_equal "book" + work = Work.find_by(id: poodr.id) + + expect(work.title).must_equal "Practical Object Oriented Design in Ruby" + expect(work.creator).must_equal "Sandi Metz" + expect(work.description).must_equal "programming!" + expect(work.publication_year).must_equal 2012 + expect(work.category).must_equal "book" end end @@ -374,11 +421,11 @@ must_redirect_to root_path expect(flash[:result_text]).must_equal "You must be logged in to view this section" - expect(poodr.title).must_equal "Practical Object Oriented Design in Ruby" - expect(poodr.creator).must_equal "Sandi Metz" - expect(poodr.description).must_equal "programming!" - expect(poodr.publication_year).must_equal 2012 - expect(poodr.category).must_equal "book" + expect(Work.find_by(id: poodr.id).title).must_equal "Practical Object Oriented Design in Ruby" + expect(Work.find_by(id: poodr.id).creator).must_equal "Sandi Metz" + expect(Work.find_by(id: poodr.id).description).must_equal "programming!" + expect(Work.find_by(id: poodr.id).publication_year).must_equal 2012 + expect(Work.find_by(id: poodr.id).category).must_equal "book" end end diff --git a/test/fixtures/works.yml b/test/fixtures/works.yml index a321c1e9..f2a76589 100644 --- a/test/fixtures/works.yml +++ b/test/fixtures/works.yml @@ -6,6 +6,7 @@ album: description: This is an older album publication_year: 1955-11-01 category: album + user: kari another_album: title: New Title @@ -13,6 +14,7 @@ another_album: description: This is a newer album publication_year: 2016-04-08 category: album + user: kari poodr: title: Practical Object Oriented Design in Ruby @@ -20,7 +22,9 @@ poodr: description: programming! publication_year: 2012 category: book + user: dan movie: title: test movie - has only required fields category: movie + user: kari From 0dac23971fdf3f31fd218d14e6061f39bff40a7a Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 29 Oct 2018 15:12:05 -0700 Subject: [PATCH 14/20] Added session controller logout tests --- test/controllers/sessions_controller_test.rb | 25 ++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index d89c5737..7554a3e6 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -49,9 +49,30 @@ end end - describe "destory" do + describe "destroy" do it "logs a user out if they are logged in" do - skip + perform_login(dan) + expect(session[:user_id]).must_equal dan.id + + expect { + delete logout_path + }.wont_change 'User.count' + + expect(session[:user_id]).must_be_nil + + must_respond_with :redirect + must_redirect_to root_path + end + + it "redirects to the homepage even if no one is logged in" do + expect { + delete logout_path + }.wont_change 'User.count' + + expect(session[:user_id]).must_be_nil + + must_respond_with :redirect + must_redirect_to root_path end end From cd53228e7887173fe1af9ec604a469561d0bab5c Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 29 Oct 2018 15:26:42 -0700 Subject: [PATCH 15/20] Added relationship bwtn users and works with migratio and model adjustments, created model tests and controller tests for this --- app/models/user.rb | 3 +++ app/models/work.rb | 1 + ...0181029221607_add_user_reference_to_work.rb | 5 +++++ db/schema.rb | 5 ++++- test/controllers/users_controller_test.rb | 3 +++ test/models/user_test.rb | 18 ++++++++++++++++++ 6 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20181029221607_add_user_reference_to_work.rb diff --git a/app/models/user.rb b/app/models/user.rb index 151a57d1..dfdf8305 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,8 +1,11 @@ class User < ApplicationRecord has_many :votes, dependent: :destroy + has_many :works, dependent: :destroy has_many :ranked_works, through: :votes, source: :work validates :username, uniqueness: true, presence: true + validates :uid, presence: true + validates :provider, presence: true def self.build_from_github(auth_hash) user = User.new diff --git a/app/models/work.rb b/app/models/work.rb index c61da303..04780ea0 100644 --- a/app/models/work.rb +++ b/app/models/work.rb @@ -1,5 +1,6 @@ class Work < ApplicationRecord CATEGORIES = %w(album book movie) + belongs_to :user has_many :votes, dependent: :destroy has_many :ranking_users, through: :votes, source: :user diff --git a/db/migrate/20181029221607_add_user_reference_to_work.rb b/db/migrate/20181029221607_add_user_reference_to_work.rb new file mode 100644 index 00000000..52b7ba5b --- /dev/null +++ b/db/migrate/20181029221607_add_user_reference_to_work.rb @@ -0,0 +1,5 @@ +class AddUserReferenceToWork < ActiveRecord::Migration[5.2] + def change + add_reference :works, :user, foreign_key: true + end +end diff --git a/db/schema.rb b/db/schema.rb index a9380f82..5170a956 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_10_16_211249) do +ActiveRecord::Schema.define(version: 2018_10_29_221607) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -42,8 +42,11 @@ t.datetime "updated_at", null: false t.integer "vote_count", default: 0 t.integer "publication_year" + t.bigint "user_id" + t.index ["user_id"], name: "index_works_on_user_id" end add_foreign_key "votes", "users" add_foreign_key "votes", "works" + add_foreign_key "works", "users" end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index d8ceed9e..a271a653 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -17,6 +17,9 @@ user.votes.each do |vote| vote.destroy end + user.works.each do |work| + work.destroy + end user.destroy end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index de92b83f..b2f5ec56 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -10,6 +10,14 @@ end end + it "has a list of works the user added to the site" do + dan = users(:dan) + dan.must_respond_to :works + dan.works.each do |work| + work.must_be_kind_of Work + end + end + it "has a list of ranked works" do dan = users(:dan) dan.must_respond_to :ranked_works @@ -39,6 +47,16 @@ user2.errors.messages.must_include :username end + it "requires a uid" do + user = User.new + user.valid?.must_equal false + user.errors.messages.must_include :uid + end + it "requires a provider" do + user = User.new + user.valid?.must_equal false + user.errors.messages.must_include :uid + end end end From 03fc9a2369e4c58e3138b6810821d10fe39ff9a3 Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 29 Oct 2018 15:37:41 -0700 Subject: [PATCH 16/20] Passing all adv auth controller and model tests --- app/controllers/works_controller.rb | 11 ++++++++++- app/views/works/_form.html.erb | 2 ++ test/controllers/works_controller_test.rb | 11 ++++++----- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 33c8729d..b6685bb7 100644 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -2,6 +2,7 @@ class WorksController < ApplicationController # We should always be able to tell what category # of work we're dealing with before_action :category_from_work, except: [:root, :index, :new, :create] + before_action :check_owner, only: [:edit, :update, :destroy] skip_before_action :require_login, only: [:root] def root @@ -85,7 +86,7 @@ def upvote private def media_params - params.require(:work).permit(:title, :category, :creator, :description, :publication_year) + params.require(:work).permit(:title, :category, :creator, :description, :publication_year, :user_id) end def category_from_work @@ -93,4 +94,12 @@ def category_from_work render_404 unless @work @media_category = @work.category.downcase.pluralize end + + def check_owner + unless @work.user.id == @login_user.id + flash[:status] = :failure + flash[:result_text] = "You can only edit works you added to the site" + redirect_to works_path + end + end end diff --git a/app/views/works/_form.html.erb b/app/views/works/_form.html.erb index 8debadb9..a39c5b9a 100644 --- a/app/views/works/_form.html.erb +++ b/app/views/works/_form.html.erb @@ -25,6 +25,8 @@ <%= f.text_area :description, class: "form-control" %> + <%= f.hidden_field :user_id, value: @login_user.id %> +
    <%= f.submit class: "btn btn-primary" %>
    diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 132496bc..de3f9b4d 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -11,7 +11,8 @@ creator: "Tolkien", description: "Lord of the Rings", publication_year: 1955, - category: "book" + category: "book", + user_id: dan.id } } } @@ -32,7 +33,7 @@ # Precondition: there is at least one media in two of the categories expect { - perform_login(dan) + perform_login(users(:kari)) delete work_path(movie.id) }.must_change 'Work.count', -1 @@ -101,7 +102,7 @@ expect(Work.last.description).must_equal work_hash[:work][:description] expect(Work.last.publication_year).must_equal work_hash[:work][:publication_year] expect(Work.last.category).must_equal work_hash[:work][:category] - expect(Work.last.user).must_equal dan + expect(Work.last.user_id).must_equal dan.id expect(flash[:result_text]).must_equal "Successfully created #{Work.last.category} #{Work.last.id}" end @@ -169,7 +170,7 @@ it "does not permit access if logged in user is not the user the work belongs to" do perform_login(dan) - get work_path(movie.id) + get edit_work_path(movie.id) # Arrange must_respond_with :redirect @@ -285,7 +286,7 @@ must_respond_with :redirect must_redirect_to works_path - expect(flash[:result_text]).must_equal "You can only delete works you added to the site" + expect(flash[:result_text]).must_equal "You can only edit works you added to the site" # expect no change expect(Work.find_by(id: movie.id).title).must_equal movie.title From 9c762c5b6552488d71945df75960895d8a7c548b Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 29 Oct 2018 15:52:25 -0700 Subject: [PATCH 17/20] Modified seed file to account for work ownership by user --- db/media_seeds.csv | 50 +++++++++++++++++++++++----------------------- db/schema.rb | 6 +++--- db/seeds.rb | 2 ++ 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/db/media_seeds.csv b/db/media_seeds.csv index 5f5b252a..104ae273 100644 --- a/db/media_seeds.csv +++ b/db/media_seeds.csv @@ -1,25 +1,25 @@ -category,title,creator,publication_year,description -album,Blue Breaker,Dr. Sarai Langosh,1949,Et et expedita non aut quo. -book,Joe Treat,Blaise Lesch,1968,Voluptatem adipisci qui velit. -album,Kreb-Full-o Been,Ms. Trevion Buckridge,2016,Vero consectetur delectus consequatur id aut accusantium unde excepturi. -album,Wake-up Pie,Timmy Streich I,1919,Voluptatem consequatur qui consectetur nisi officiis culpa. -album,Major Cup,Jayde Bartoletti,1944,Quis recusandae cum est facere consequatur minima magni et. -book,Summer Select,Ms. Gwendolyn Ortiz,1946,Et molestiae eos nam odit aut sed. -album,Holiday Choice,Alexandria Lehner,1940,Excepturi voluptas ut voluptatum. -book,Postmodern Blend,Meredith Brekke,1970,Dolorem fugit accusantium qui. -book,Green Forrester,Raquel Hirthe,1933,Omnis qui quia odio. -album,Winter Mug,Tia Weissnat II,1990,Laboriosam autem iusto quae sed voluptate et. -book,Red Pie,Davon Kub,1961,Id dolorem qui laborum quia. -album,Major Equinox,Queen Satterfield,1997,Fugit perferendis est quam sunt porro vel rerum. -book,Melty Breaker,Montana Dickinson Sr.,1991,Perferendis harum fuga corporis. -book,Winter Pie,Mr. Syble Kuhn,1970,Incidunt molestias deserunt laudantium. -album,Goodbye Utopia,Orion Spencer,1962,Praesentium enim pariatur voluptatem sed quod dolorum. -album,Green Select,Berneice Jenkins,1957,Hic repudiandae molestiae id nulla aliquid maiores necessitatibus. -book,Blacktop Enlightenment,Seamus D'Amore,1928,Ea id cumque et pariatur magni nemo dolorem. -album,Express Extract,Dorothy Jast I,1969,Dolores dolorum aut ea aperiam et voluptatem. -album,Winter Been,Mackenzie Wilkinson,1932,Culpa repudiandae et at sint et amet fugiat et. -book,Heart Mug,Orpha Douglas,2009,Qui voluptas alias quia. -album,Blue Treat,Eliseo Gorczany,1979,Sit est quis veniam saepe. -book,Hello Town,Laury Walter,2005,Est sed ut asperiores sed fugiat. -album,Blacktop Choice,Casey Feil,2008,Temporibus ex maxime labore quam et natus quia ipsum. -book,Huggy Star,Nigel Lesch DVM,1962,Voluptatem ea aspernatur nesciunt ipsa quis error corporis placeat. +category,title,creator,publication_year,description,user_id +album,Blue Breaker,Dr. Sarai Langosh,1949,Et et expedita non aut quo.,1 +book,Joe Treat,Blaise Lesch,1968,Voluptatem adipisci qui velit.,1 +album,Kreb-Full-o Been,Ms. Trevion Buckridge,2016,Vero consectetur delectus consequatur id aut accusantium unde excepturi.,1 +album,Wake-up Pie,Timmy Streich I,1919,Voluptatem consequatur qui consectetur nisi officiis culpa.,1 +album,Major Cup,Jayde Bartoletti,1944,Quis recusandae cum est facere consequatur minima magni et.,1 +book,Summer Select,Ms. Gwendolyn Ortiz,1946,Et molestiae eos nam odit aut sed.,1 +album,Holiday Choice,Alexandria Lehner,1940,Excepturi voluptas ut voluptatum.,1 +book,Postmodern Blend,Meredith Brekke,1970,Dolorem fugit accusantium qui.,1 +book,Green Forrester,Raquel Hirthe,1933,Omnis qui quia odio.,1 +album,Winter Mug,Tia Weissnat II,1990,Laboriosam autem iusto quae sed voluptate et.,1 +book,Red Pie,Davon Kub,1961,Id dolorem qui laborum quia.,1 +album,Major Equinox,Queen Satterfield,1997,Fugit perferendis est quam sunt porro vel rerum.,1 +book,Melty Breaker,Montana Dickinson Sr.,1991,Perferendis harum fuga corporis.,1 +book,Winter Pie,Mr. Syble Kuhn,1970,Incidunt molestias deserunt laudantium.,1 +album,Goodbye Utopia,Orion Spencer,1962,Praesentium enim pariatur voluptatem sed quod dolorum.,1 +album,Green Select,Berneice Jenkins,1957,Hic repudiandae molestiae id nulla aliquid maiores necessitatibus.,1 +book,Blacktop Enlightenment,Seamus D'Amore,1928,Ea id cumque et pariatur magni nemo dolorem.,1 +album,Express Extract,Dorothy Jast I,1969,Dolores dolorum aut ea aperiam et voluptatem.,1 +album,Winter Been,Mackenzie Wilkinson,1932,Culpa repudiandae et at sint et amet fugiat et.,1 +book,Heart Mug,Orpha Douglas,2009,Qui voluptas alias quia.,1 +album,Blue Treat,Eliseo Gorczany,1979,Sit est quis veniam saepe.,1 +book,Hello Town,Laury Walter,2005,Est sed ut asperiores sed fugiat.,1 +album,Blacktop Choice,Casey Feil,2008,Temporibus ex maxime labore quam et natus quia ipsum.,1 +book,Huggy Star,Nigel Lesch DVM,1962,Voluptatem ea aspernatur nesciunt ipsa quis error corporis placeat.,1 diff --git a/db/schema.rb b/db/schema.rb index 5170a956..0f2d2d12 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -15,7 +15,7 @@ # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" - create_table "users", force: :cascade do |t| + create_table "users", id: :serial, force: :cascade do |t| t.string "username" t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -24,7 +24,7 @@ t.string "name" end - create_table "votes", force: :cascade do |t| + create_table "votes", id: :serial, force: :cascade do |t| t.integer "user_id" t.integer "work_id" t.datetime "created_at", null: false @@ -33,7 +33,7 @@ t.index ["work_id"], name: "index_votes_on_work_id" end - create_table "works", force: :cascade do |t| + create_table "works", id: :serial, force: :cascade do |t| t.string "title" t.string "creator" t.string "description" diff --git a/db/seeds.rb b/db/seeds.rb index 75cb6480..0385518e 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -1,6 +1,8 @@ require 'csv' media_file = Rails.root.join('db', 'media_seeds.csv') +User.create(provider: "github", uid: 99999, username: "seed_user", name: "Seed Person") + CSV.foreach(media_file, headers: true, header_converters: :symbol, converters: :all) do |row| data = Hash[row.headers.zip(row.fields)] puts data From 6abb74a151e1e90ea949a7671a852b64d2ae886a Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 29 Oct 2018 22:07:19 -0700 Subject: [PATCH 18/20] Added Google OAuth login functionality --- Gemfile | 1 + Gemfile.lock | 5 +++++ app/assets/stylesheets/application.scss | 2 +- app/controllers/sessions_controller.rb | 8 +++++--- app/models/user.rb | 13 ++++++++++++- app/views/layouts/application.html.erb | 5 ++++- config/initializers/omniauth.rb | 4 ++++ db/migrate/20181030044147_change_uid_to_bigint.rb | 5 +++++ .../20181030044631_change_integer_limit_for_uid.rb | 5 +++++ db/migrate/20181030045718_change_uid_to_string.rb | 5 +++++ db/schema.rb | 4 ++-- 11 files changed, 49 insertions(+), 8 deletions(-) create mode 100644 db/migrate/20181030044147_change_uid_to_bigint.rb create mode 100644 db/migrate/20181030044631_change_integer_limit_for_uid.rb create mode 100644 db/migrate/20181030045718_change_uid_to_string.rb diff --git a/Gemfile b/Gemfile index de76c524..13a71345 100644 --- a/Gemfile +++ b/Gemfile @@ -22,6 +22,7 @@ gem 'coffee-rails', '~> 4.2' gem 'omniauth' gem 'omniauth-github' +gem 'omniauth-google-oauth2' # Use jquery as the JavaScript library gem 'jquery-rails' diff --git a/Gemfile.lock b/Gemfile.lock index f66ae74f..9018588e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -138,6 +138,10 @@ GEM omniauth-github (1.3.0) omniauth (~> 1.5) omniauth-oauth2 (>= 1.4.0, < 2.0) + omniauth-google-oauth2 (0.5.3) + jwt (>= 1.5) + omniauth (>= 1.1.1) + omniauth-oauth2 (>= 1.5) omniauth-oauth2 (1.5.0) oauth2 (~> 1.1) omniauth (~> 1.2) @@ -242,6 +246,7 @@ DEPENDENCIES minitest-spec-rails omniauth omniauth-github + omniauth-google-oauth2 pg (~> 0.18) pry-rails puma (~> 3.0) diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss index 5f08f884..f841c842 100644 --- a/app/assets/stylesheets/application.scss +++ b/app/assets/stylesheets/application.scss @@ -85,7 +85,7 @@ a:hover { } .app-header__user-nav-container .nav-item { - margin-left: 2rem; + margin-left: 1rem; } .list-group-item { diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 19305d52..344975af 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -4,14 +4,16 @@ class SessionsController < ApplicationController def create auth_hash = request.env['omniauth.auth'] - user = User.find_by(uid: auth_hash[:uid], provider: 'github') + user = User.find_by(uid: auth_hash[:uid], provider: auth_hash[:provider]) + if user session[:user_id] = user.id flash[:status] = :success flash[:result_text] = "Successfully logged in as existing user #{user.username}" else - user = User.build_from_github(auth_hash) - + user = User.build_from_github(auth_hash) if auth_hash[:provider] == 'github' + user = User.build_from_google(auth_hash) if auth_hash[:provider] == 'google_oauth2' + if user.save session[:user_id] = user.id flash[:status] = :success diff --git a/app/models/user.rb b/app/models/user.rb index dfdf8305..0134e920 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -12,7 +12,18 @@ def self.build_from_github(auth_hash) user.provider = 'github' user.uid = auth_hash[:uid] - user.username = auth_hash[:info][:nickname] + user.username = auth_hash[:info][:email] + user.name = auth_hash[:info][:name] + + return user + end + + def self.build_from_google(auth_hash) + user = User.new + + user.provider = 'google_oauth2' + user.uid = auth_hash[:uid] + user.username = auth_hash[:info][:email] user.name = auth_hash[:info][:name] return user diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 4d3ef694..b6477f0c 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -45,7 +45,10 @@ <% else %>
  • - <%= link_to "Login with Github", "/auth/github", class: "btn btn-primary" %> + <%= link_to "Sign in with Github", "/auth/github", class: "btn btn-primary" %> +
    • +
  • + <%= link_to "Sign in with Google", "/auth/google_oauth2", class: "btn btn-primary" %>
    • <% end %> diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index fd441612..252a89b4 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -1,3 +1,7 @@ Rails.application.config.middleware.use OmniAuth::Builder do provider :github, ENV["GITHUB_CLIENT_ID"], ENV["GITHUB_CLIENT_SECRET"], scope: "user:email" end + +Rails.application.config.middleware.use OmniAuth::Builder do + provider :google_oauth2, ENV['GOOGLE_CLIENT_ID'], ENV['GOOGLE_CLIENT_SECRET'], { scope: 'userinfo.email, userinfo.profile', redirect_uri: 'http://localhost:3000/auth/google_oauth2/callback' } +end diff --git a/db/migrate/20181030044147_change_uid_to_bigint.rb b/db/migrate/20181030044147_change_uid_to_bigint.rb new file mode 100644 index 00000000..60e88f1a --- /dev/null +++ b/db/migrate/20181030044147_change_uid_to_bigint.rb @@ -0,0 +1,5 @@ +class ChangeUidToBigint < ActiveRecord::Migration[5.2] + def change + change_column :users, :uid, :bigint + end +end diff --git a/db/migrate/20181030044631_change_integer_limit_for_uid.rb b/db/migrate/20181030044631_change_integer_limit_for_uid.rb new file mode 100644 index 00000000..1098cfdb --- /dev/null +++ b/db/migrate/20181030044631_change_integer_limit_for_uid.rb @@ -0,0 +1,5 @@ +class ChangeIntegerLimitForUid < ActiveRecord::Migration[5.2] + def change + change_column :users, :uid, :integer, limit: 8 + end +end diff --git a/db/migrate/20181030045718_change_uid_to_string.rb b/db/migrate/20181030045718_change_uid_to_string.rb new file mode 100644 index 00000000..87cac40e --- /dev/null +++ b/db/migrate/20181030045718_change_uid_to_string.rb @@ -0,0 +1,5 @@ +class ChangeUidToString < ActiveRecord::Migration[5.2] + def change + change_column :users, :uid, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 0f2d2d12..c9fb6d62 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_10_29_221607) do +ActiveRecord::Schema.define(version: 2018_10_30_045718) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -19,7 +19,7 @@ t.string "username" t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.integer "uid", null: false + t.string "uid", null: false t.string "provider", null: false t.string "name" end From 04d763ad35645a5b7d2e84b7f77661c0ae975caf Mon Sep 17 00:00:00 2001 From: jackie Date: Mon, 29 Oct 2018 22:47:14 -0700 Subject: [PATCH 19/20] Added test for google oauth --- app/controllers/sessions_controller.rb | 2 +- test/controllers/sessions_controller_test.rb | 14 ++++++++++++-- test/controllers/users_controller_test.rb | 2 +- test/fixtures/users.yml | 10 ++++++++-- test/test_helper.rb | 19 ++++++++++++++++++- 5 files changed, 40 insertions(+), 7 deletions(-) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 344975af..9a9b73da 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -13,7 +13,7 @@ def create else user = User.build_from_github(auth_hash) if auth_hash[:provider] == 'github' user = User.build_from_google(auth_hash) if auth_hash[:provider] == 'google_oauth2' - + if user.save session[:user_id] = user.id flash[:status] = :success diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 7554a3e6..54113731 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -14,8 +14,18 @@ must_redirect_to root_path end + it "logs in an existing google user and redirects to the root route" do + expect { + perform_google_login(users(:google_user)) + }.wont_change 'User.count' + + expect(session[:user_id]).must_equal users(:google_user).id + + must_redirect_to root_path + end + it "creates an account for a new user and redirects to the root route" do - user = User.new(provider: "github", uid: 99999, username: "test_user", name: "Test Person") + user = User.new(provider: "github", uid: "400400", username: "test_user", name: "Test Person") expect { perform_login(user) @@ -32,7 +42,7 @@ end it "redirects to the login route if given invalid user data" do - user = User.new(provider: "github", uid: 99999, username: nil, name: "Test Person") + user = User.new(provider: "github", uid: "99999", username: nil, name: "Test Person") expect { perform_login(user) diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index a271a653..344ef4ef 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -25,7 +25,7 @@ expect(User.count).must_equal 0 - user = User.new(provider: "github", uid: 99999, username: "test_user", name: "Test Person") + user = User.new(provider: "github", uid: "400400", username: "test_user", name: "Test Person") perform_login(user) get users_path diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index cca1be1f..5db3d074 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -3,11 +3,17 @@ dan: username: dan name: Dan - uid: 123123 + uid: "123123" provider: github kari: username: kari name: Kari - uid: 456456 + uid: "456456" provider: github + +google_user: + username: goog + name: Goog + uid: "789789" + provider: google_oauth2 diff --git a/test/test_helper.rb b/test/test_helper.rb index 7aaf75bd..0035b69c 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -45,9 +45,26 @@ def mock_auth_hash(user) } end + def mock_google_auth_hash(user) + return { + provider: user.provider, + uid: user.uid, + info: { + name: user.name, + email: user.username + } + } + end + def perform_login(user) OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new(mock_auth_hash(user)) - + get auth_callback_path(:github) end + + def perform_google_login(user) + OmniAuth.config.mock_auth[:google_oauth2] = OmniAuth::AuthHash.new(mock_google_auth_hash(user)) + + get auth_callback_path(:google_oauth2) + end end From 757c1c9ad9c22e6f7833336b8ed7a8949037af6b Mon Sep 17 00:00:00 2001 From: jackie Date: Tue, 30 Oct 2018 09:20:53 -0700 Subject: [PATCH 20/20] Fixed bug that was causing 2 login tests not to pass --- app/models/user.rb | 2 +- test/controllers/sessions_controller_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 0134e920..4172bba5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -12,7 +12,7 @@ def self.build_from_github(auth_hash) user.provider = 'github' user.uid = auth_hash[:uid] - user.username = auth_hash[:info][:email] + user.username = auth_hash[:info][:nickname] user.name = auth_hash[:info][:name] return user diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 54113731..1016fdac 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -25,7 +25,7 @@ end it "creates an account for a new user and redirects to the root route" do - user = User.new(provider: "github", uid: "400400", username: "test_user", name: "Test Person") + user = User.new(provider: "github", uid: 400400, username: "test_user", name: "Test Person") expect { perform_login(user)