From 51de3dc3a3d81685e233f5089c75083c07c8b31d Mon Sep 17 00:00:00 2001 From: Danielle Metzner Date: Mon, 15 Oct 2018 17:08:55 -0700 Subject: [PATCH 1/7] Updated Works_controller tests --- app/controllers/works_controller.rb | 2 +- db/schema.rb | 28 ++-- test/controllers/works_controller_test.rb | 153 +++++++++++++++++++++- 3 files changed, 166 insertions(+), 17 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/db/schema.rb b/db/schema.rb index 6bc8ba5c..d15e3378 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,35 +10,35 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170407164321) do +ActiveRecord::Schema.define(version: 2017_04_07_164321) 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 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" diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 0945ca47..b81bd3b5 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -1,18 +1,31 @@ require 'test_helper' describe WorksController do - describe "root" do + let(:poodr) { works(:poodr) } + describe "root" do #Check in the fixture that there is one of each 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 + delete work_path(poodr.id) + get root_path + + must_respond_with :success end it "succeeds with no media" do + works.each do |work| + delete work_path(work.id) + end + + get root_path + must_respond_with :success end end @@ -22,76 +35,212 @@ 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 + # works.each do |work| + # delete work_path(work.id) + # end + Work.all.destroy_all + + 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 + let (:work_hash) do + { + work: { + title: 'Harry Potter', + creator: 'JK Rowling', + description: 'Hogwarts things', + category: 'book' + } + } + end it "creates a work with valid data for a real category" do + 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.category).must_equal work_hash[:work][:category] end it "renders bad_request and does not update the DB for bogus data" do + work_hash[:work][:title] = nil + # Act-Assert + expect { + post works_path, params: work_hash + }.wont_change 'Work.count' + + must_respond_with :bad_request end it "renders 400 bad_request for bogus categories" do + INVALID_CATEGORIES.each do |category| + work_hash[:work][:category] = category + + # Act-Assert + expect { + post works_path, params: work_hash + }.wont_change 'Work.count' + must_respond_with :bad_request + end end end describe "show" do it "succeeds for an extant work ID" do + id = works(:poodr).id + + # Act + get work_path(id) + # Assert + must_respond_with :success end it "renders 404 not_found for a bogus work ID" do + id = -1 + # Act + get work_path(id) + + # Assert + must_respond_with :not_found + #expect(flash[:danger]).must_equal "Cannot find the work -1" end end describe "edit" do it "succeeds for an extant work ID" do + # Arrange + id = works(:poodr).id + + # Act + get edit_work_path(id) + # Assert + must_respond_with :success end it "renders 404 not_found for a bogus work ID" do + id = -1 + # Act + get edit_work_path(id) + + # Assert + expect(response).must_be :not_found? + must_respond_with :not_found end end describe "update" do + let (:work_hash) do + { + work: { + title: 'Harry Potter', + creator: 'JK Rowling', + description: 'Hogwarts things', + category: 'book' + } + } + end it "succeeds for valid data and an extant work ID" do + id = works(:poodr).id + expect { + patch work_path(id), params: work_hash + }.wont_change 'Work.count' + + must_respond_with :redirect + must_redirect_to work_path(id) + new_work = Work.find_by(id: id) + + expect(new_work.title).must_equal work_hash[:work][:title] + expect(new_work.creator).must_equal work_hash[:work][:creator] + expect(new_work.description).must_equal work_hash[:work][:description] + expect(new_work.category).must_equal work_hash[:work][:category] end it "renders bad_request for bogus data" do + work_hash[:work][:title] = nil + id = works(:poodr).id + old_poodr = works(:poodr) + + expect { + patch work_path(id), params: work_hash + }.wont_change 'Work.count' + new_poodr = Work.find(id) + must_respond_with :bad_request + + expect(old_poodr.title).must_equal new_poodr.title + expect(old_poodr.creator).must_equal new_poodr.creator + expect(old_poodr.description).must_equal new_poodr.description + expect(old_poodr.category).must_equal new_poodr.category end it "renders 404 not_found for a bogus work ID" do + 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 - + # Arrange + id = works(:poodr).id + title = works(:poodr).title + + # Act - Assert + expect { + delete work_path(id) + }.must_change 'Work.count', -1 + + must_respond_with :redirect + must_redirect_to root_path + #expect(flash[:success]).must_equal "#{title} deleted" + expect(Work.find_by(id: id)).must_equal 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 From d3d6e94aa5864d3efbc474829dc8b64c815af61a Mon Sep 17 00:00:00 2001 From: Danielle Metzner Date: Tue, 16 Oct 2018 16:02:33 -0700 Subject: [PATCH 2/7] Added Oauth through github for login and logout --- .gitignore | 1 + Gemfile | 3 ++ Gemfile.lock | 28 +++++++++++ app/controllers/sessions_controller.rb | 61 +++++++++++++++++++---- app/models/user.rb | 11 ++++ app/views/layouts/application.html.erb | 4 +- config/initializers/omniauth.rb | 3 ++ config/routes.rb | 9 ++-- db/migrate/20181016212659_userfields.rb | 7 +++ db/schema.rb | 5 +- test/controllers/works_controller_test.rb | 7 +++ 11 files changed, 122 insertions(+), 17 deletions(-) create mode 100644 config/initializers/omniauth.rb create mode 100644 db/migrate/20181016212659_userfields.rb diff --git a/.gitignore b/.gitignore index 48fb168f..c73dd41f 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ /tmp/* !/log/.keep !/tmp/.keep +.env # Ignore Byebug command history file. .byebug_history diff --git a/Gemfile b/Gemfile index 42f4bb2c..4315e5bc 100644 --- a/Gemfile +++ b/Gemfile @@ -30,6 +30,8 @@ gem 'jbuilder', '~> 2.5' # gem 'redis', '~> 3.0' # Use ActiveModel has_secure_password # gem 'bcrypt', '~> 3.1.7' +gem 'omniauth' +gem 'omniauth-github' # Use Capistrano for deployment # gem 'capistrano-rails', group: :development @@ -62,6 +64,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 5b407e7e..f66ae74f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -70,11 +70,18 @@ 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) + 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 +91,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 +121,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) @@ -207,6 +232,7 @@ DEPENDENCIES bootstrap (~> 4.1.3) byebug coffee-rails (~> 4.2) + dotenv-rails jbuilder (~> 2.5) jquery-rails listen (~> 3.0.5) @@ -214,6 +240,8 @@ DEPENDENCIES minitest-reporters minitest-skip minitest-spec-rails + omniauth + omniauth-github pg (~> 0.18) pry-rails puma (~> 3.0) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 5bce99e6..f8fa8fc4 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -2,33 +2,72 @@ class SessionsController < ApplicationController def login_form end - def login - username = params[:username] - if username and user = User.find_by(username: username) + # 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 + + def create + auth_hash = request.env['omniauth.auth'] + + user = User.find_by(uid: auth_hash[:uid], provider: 'github') + + if user + # User was found in the database 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) + 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}" + 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 + flash[:status] = :failure + flash[:result_text] = "Could not log in" + flash[:messages] = user.errors.messages + redirect_to root_path return 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 end diff --git a/app/models/user.rb b/app/models/user.rb index 4cac8fe0..009a5540 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) + new_user = User.new + + new_user.uid = auth_hash[:uid] + new_user.provider = 'github' + new_user.username = auth_hash[:info][:nickname] + new_user.email = auth_hash[:info][:email] + + return new_user + end end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index e7b07ce4..226b29f5 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -39,13 +39,13 @@ <%= link_to "Logged in as #{@login_user.username}", user_path(@login_user), class: "btn btn-primary" %> <% else %> <% 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..e5a995ef 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,9 +1,12 @@ 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' + # 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/20181016212659_userfields.rb b/db/migrate/20181016212659_userfields.rb new file mode 100644 index 00000000..d34af6fe --- /dev/null +++ b/db/migrate/20181016212659_userfields.rb @@ -0,0 +1,7 @@ +class Userfields < ActiveRecord::Migration[5.2] + def change + add_column :users, :email, :string + add_column :users, :uid, :integer, null: false + add_column :users, :provider, :string, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index d15e3378..4a1ac3b4 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: 2017_04_07_164321) do +ActiveRecord::Schema.define(version: 2018_10_16_212659) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -19,6 +19,9 @@ t.string "username" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "email" + t.integer "uid", null: false + t.string "provider", null: false end create_table "votes", force: :cascade do |t| diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index b81bd3b5..54076ccf 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -245,6 +245,13 @@ end describe "upvote" do + #Can also put this in the test helper.... + #We have to tests this and call the sessions controller... but it's okay + # since this is a dependency + # post login_path, params: { user: {name: 'Ada'}} + # expect(session[:user_id]).wont_be_nil + # delete logout_path + # expect (session[:user_id]).to_equal nil it "redirects to the work page if no user is logged in" do From d00824afc90581639e6bd5ec4044cd3af9370df4 Mon Sep 17 00:00:00 2001 From: Danielle Metzner Date: Mon, 29 Oct 2018 20:23:36 -0700 Subject: [PATCH 3/7] Added session controller tests --- app/controllers/application_controller.rb | 11 ++++- app/controllers/sessions_controller.rb | 3 +- app/models/user.rb | 2 + config/routes.rb | 2 +- test/controllers/sessions_controller_test.rb | 47 ++++++++++++++++++++ test/controllers/users_controller_test.rb | 2 +- test/fixtures/users.yml | 20 +++++++-- test/models/user_test.rb | 15 ++++--- test/test_helper.rb | 22 +++++++++ 9 files changed, 109 insertions(+), 15 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c12c7c17..2cf875d0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,13 +1,22 @@ class ApplicationController < ActionController::Base protect_from_forgery with: :exception - + before_action :require_login, except: [:root,:find_user] before_action :find_user + def render_404 # DPR: this will actually render a 404 page in production raise ActionController::RoutingError.new('Not Found') end + def require_login + if session[:user_id].nil? + flash[:status] = :failure + flash[:result_text] = "You need to login to review this page" + redirect 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 f8fa8fc4..261f85d8 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,4 +1,5 @@ class SessionsController < ApplicationController + skip_before_action :require_login, except: :destroy def login_form end @@ -42,7 +43,6 @@ def create 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) @@ -50,7 +50,6 @@ def create session[:user_id] = user.id flash[:status] = :success flash[:result_text] = "Successfully created new user #{user.username} with ID #{user.id}" - else flash[:status] = :failure flash[:result_text] = "Could not log in" diff --git a/app/models/user.rb b/app/models/user.rb index 009a5540..465a6673 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,6 +3,8 @@ class User < ApplicationRecord 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) new_user = User.new diff --git a/config/routes.rb b/config/routes.rb index e5a995ef..c8fad6a3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -5,7 +5,7 @@ # post '/login', to: 'sessions#login' # post '/logout', to: 'sessions#logout', as: 'logout' - get "/auth/:provider/callback", to: "sessions#create" + get "/auth/:provider/callback", to: "sessions#create", as: "auth_callback" delete "/logout", to: "sessions#destroy", as: "logout" resources :works diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index f641d15c..99de41d0 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -1,5 +1,52 @@ require "test_helper" describe SessionsController do + describe "auth_callback" do + it "logs in an existing user and redirects to the root route" do + start_count = User.count + + user = users(:grace) + + perform_login(user) + + must_redirect_to root_path + + session[:user_id].must_equal user.id + + # Should *not* have created a new user + User.count.must_equal start_count + end + + it "creates an account for a new user and redirects to the root route" do + user = users(:ada) + user.destroy + + perform_login(user) + must_redirect_to root_path + + # Should have created a new user + expect do + get auth_callback_path(:github).must_change('User.count' +1) + end + + # The new user's ID should be set in the session + session[:user_id].must_equal User.last.id + end + + it "redirects to the login route if given invalid user data" do + puts users.count + user = users(:ada) + user.uid = nil + user.provider = nil + + perform_login(user) + must_redirect_to root_path + + expect do + get auth_callback_path(:github).wont_change('User.count') + end + puts users.count + end + end end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index d2c5cfbb..d8503324 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -1,5 +1,5 @@ require 'test_helper' describe UsersController do - + end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index e2968d78..17e778cf 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -1,7 +1,19 @@ # Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html +# +# dan: +# username: dan +# +# kari: +# username: kari -dan: - username: dan +ada: + provider: github + uid: 12345 + email: ada@adadevelopersacademy.org + username: countess_ada -kari: - username: kari +grace: + provider: github + uid: 13371337 + email: grace@hooper.net + username: graceful_hopps diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 793ce7e6..47c7eb72 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -1,19 +1,22 @@ require 'test_helper' describe User do + before do + perform_login(users(:grace) + end describe "relations" do it "has a list of votes" do - dan = users(:dan) - dan.must_respond_to :votes - dan.votes.each do |vote| + grace = users(:grace) + grace.must_respond_to :votes + grace.votes.each do |vote| vote.must_be_kind_of Vote end end it "has a list of ranked works" do - dan = users(:dan) - dan.must_respond_to :ranked_works - dan.ranked_works.each do |work| + grace = users(:grace) + grace.must_respond_to :ranked_works + grace.ranked_works.each do |work| work.must_be_kind_of Work end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 5b4fb667..6228317c 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -23,4 +23,26 @@ 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... + 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 + + def mock_auth_hash(user) + return { + provider: user.provider, + uid: user.uid, + info: { + email: user.email, + 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 0f84577b0491fe94090020f9f653ac8f5a2c0233 Mon Sep 17 00:00:00 2001 From: Danielle Metzner Date: Mon, 29 Oct 2018 21:07:33 -0700 Subject: [PATCH 4/7] Fixed before action for require_login in the session --- app/controllers/application_controller.rb | 2 +- app/controllers/sessions_controller.rb | 32 +---------------------- 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2cf875d0..aa816e8a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -13,7 +13,7 @@ def require_login if session[:user_id].nil? flash[:status] = :failure flash[:result_text] = "You need to login to review this page" - redirect root_path + redirect_to root_path end end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 261f85d8..2caef065 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,38 +1,8 @@ class SessionsController < ApplicationController - skip_before_action :require_login, except: :destroy + skip_before_action :require_login, only: [:create] 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 - def create auth_hash = request.env['omniauth.auth'] From 47d4122b73ea996f2d5cde66de49eff8ac70c00e Mon Sep 17 00:00:00 2001 From: Danielle Metzner Date: Mon, 29 Oct 2018 22:48:14 -0700 Subject: [PATCH 5/7] Added tests for logged in users to works --- app/controllers/application_controller.rb | 3 +- config/routes.rb | 3 - test/controllers/sessions_controller_test.rb | 51 +++--- test/controllers/works_controller_test.rb | 156 +++++++++++++++++-- 4 files changed, 173 insertions(+), 40 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index aa816e8a..ca7d0146 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,7 +1,8 @@ class ApplicationController < ActionController::Base protect_from_forgery with: :exception - before_action :require_login, except: [:root,:find_user] + before_action :find_user + before_action :require_login, except: [:root,:find_user] def render_404 diff --git a/config/routes.rb b/config/routes.rb index c8fad6a3..a57e456a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,9 +1,6 @@ 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' get "/auth/:provider/callback", to: "sessions#create", as: "auth_callback" delete "/logout", to: "sessions#destroy", as: "logout" diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 99de41d0..60710790 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -1,14 +1,16 @@ require "test_helper" describe SessionsController do - describe "auth_callback" do - it "logs in an existing user and redirects to the root route" do + describe "Login" do + it "logs in existing user and redirects to roote path" do start_count = User.count user = users(:grace) - perform_login(user) + expect { + perform_login(user) + }.wont_change 'User.count' must_redirect_to root_path @@ -18,35 +20,46 @@ User.count.must_equal start_count end - it "creates an account for a new user and redirects to the root route" do + it "creates a new user and redirects to root path" do user = users(:ada) user.destroy - perform_login(user) - must_redirect_to root_path - # Should have created a new user - expect do - get auth_callback_path(:github).must_change('User.count' +1) - end - + expect { + perform_login(user) + }.must_change 'User.count', 1 + must_redirect_to root_path # The new user's ID should be set in the session session[:user_id].must_equal User.last.id end - it "redirects to the login route if given invalid user data" do - puts users.count + it "if given invalid user data redirects root path " do + user = users(:ada) user.uid = nil - user.provider = nil + #user.provider = nil + + expect { + perform_login(user) + }.wont_change 'User.count' - perform_login(user) must_redirect_to root_path + end + end + + describe 'Logout' do + it 'should logout user and redirect to root path' do + user = users(:ada) - expect do - get auth_callback_path(:github).wont_change('User.count') - end - puts users.count + perform_login(user) + login_id = session[:user_id] + delete logout_path + logout_id = session[:user_id] + + expect(login_id).must_equal users(:ada).id + expect(logout_id).must_be_nil + + must_redirect_to root_path end end end diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 54076ccf..ca620a49 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -2,6 +2,7 @@ describe WorksController do let(:poodr) { works(:poodr) } + let(:user) {users(:ada)} describe "root" do #Check in the fixture that there is one of each it "succeeds with all media types" do # Precondition: there is at least one media of each category @@ -12,7 +13,11 @@ it "succeeds with one media type absent" do # Precondition: there is at least one media in two of the categories - delete work_path(poodr.id) + poodr.destroy + + + expect(Work.find_by(category: "book")).must_be_nil + get root_path must_respond_with :success @@ -21,7 +26,7 @@ it "succeeds with no media" do works.each do |work| - delete work_path(work.id) + work.destroy end get root_path @@ -34,14 +39,15 @@ INVALID_CATEGORIES = ["nope", "42", "", " ", "albumstrailingtext"] describe "index" do - it "succeeds when there are works" do + it "succeeds when there are works (logged in)" do + perform_login(user) get works_path must_respond_with :success end - it "succeeds when there are no works" do - + it "succeeds when there are no works (logged in)" do + perform_login(user) # works.each do |work| # delete work_path(work.id) # end @@ -50,14 +56,30 @@ get works_path must_respond_with :success end + + it 'redirect to root path when user is logged out' do + perform_login(user) + delete logout_path + get works_path + + must_redirect_to root_path + end end describe "new" do - it "succeeds" do + it "succeeds when logged in" do + perform_login(user) get new_work_path must_respond_with :success end + it "redirect to root path when logged out" do + perform_login(user) + delete logout_path + get new_work_path + + must_redirect_to root_path + end end describe "create" do @@ -71,7 +93,8 @@ } } end - it "creates a work with valid data for a real category" do + it "creates a work with valid data for a real category (logged in)" do + perform_login(user) expect { post works_path, params: work_hash }.must_change 'Work.count', 1 @@ -85,7 +108,8 @@ expect(Work.last.category).must_equal work_hash[:work][:category] end - it "renders bad_request and does not update the DB for bogus data" do + it "renders bad_request and does not update the DB for bogus data (logged in)" do + perform_login(user) work_hash[:work][:title] = nil # Act-Assert @@ -97,6 +121,7 @@ end it "renders 400 bad_request for bogus categories" do + perform_login(user) INVALID_CATEGORIES.each do |category| work_hash[:work][:category] = category @@ -109,10 +134,21 @@ end end + it "redirect to root path when user is not logged in" do + delete logout_path + expect { + post works_path, params: work_hash + }.wont_change 'Work.count' + + must_respond_with :redirect + must_redirect_to root_path + end + end describe "show" do - it "succeeds for an extant work ID" do + it "succeeds for an extant work ID (logged in)" do + perform_login(user) id = works(:poodr).id # Act @@ -122,7 +158,8 @@ must_respond_with :success end - it "renders 404 not_found for a bogus work ID" do + it "renders 404 not_found for a bogus work ID (logged in)" do + perform_login(user) id = -1 # Act @@ -132,10 +169,22 @@ must_respond_with :not_found #expect(flash[:danger]).must_equal "Cannot find the work -1" end + + it "redirect to the root path when user is not logged in" do + delete logout_path + id = works(:poodr).id + # Act + get work_path(id) + + # Assert + + must_redirect_to root_path + end end describe "edit" do - it "succeeds for an extant work ID" do + it "succeeds for an extant work ID (logged in)" do + perform_login(user) # Arrange id = works(:poodr).id @@ -146,7 +195,8 @@ must_respond_with :success end - it "renders 404 not_found for a bogus work ID" do + it "renders 404 not_found for a bogus work ID (logged in)" do + perform_login(user) id = -1 # Act @@ -156,6 +206,18 @@ expect(response).must_be :not_found? must_respond_with :not_found end + + it "redirects to root path if user is not logged in" do + delete logout_path + # Arrange + id = works(:poodr).id + + # Act + get edit_work_path(id) + + # Assert + must_redirect_to root_path + end end describe "update" do @@ -169,7 +231,8 @@ } } end - it "succeeds for valid data and an extant work ID" do + it "succeeds for valid data and an extant work ID (logged in)" do + perform_login(user) id = works(:poodr).id expect { patch work_path(id), params: work_hash @@ -186,7 +249,8 @@ expect(new_work.category).must_equal work_hash[:work][:category] end - it "renders bad_request for bogus data" do + it "renders bad_request for bogus data (logged in)" do + perform_login(user) work_hash[:work][:title] = nil id = works(:poodr).id old_poodr = works(:poodr) @@ -204,7 +268,8 @@ expect(old_poodr.category).must_equal new_poodr.category end - it "renders 404 not_found for a bogus work ID" do + it "renders 404 not_found for a bogus work ID (logged in)" do + perform_login(user) id = -1 expect { @@ -214,10 +279,22 @@ must_respond_with :not_found end + + it 'redirects to root path is the user is not logged in' do + delete logout_path + id = works(:poodr).id + expect { + patch work_path(id), params: work_hash + }.wont_change 'Work.count' + + must_respond_with :redirect + must_redirect_to root_path + end end describe "destroy" do - it "succeeds for an extant work ID" do + it "succeeds for an extant work ID (logged in)" do + perform_login(user) # Arrange id = works(:poodr).id title = works(:poodr).title @@ -233,7 +310,8 @@ expect(Work.find_by(id: id)).must_equal nil end - it "renders 404 not_found and does not update the DB for a bogus work ID" do + it "renders 404 not_found and does not update the DB for a bogus work ID (logged in)" do + perform_login(user) id = -1 expect { @@ -242,6 +320,22 @@ must_respond_with :not_found end + + it "redirects to root path if user is logged out" do + delete logout_path + # Arrange + id = works(:poodr).id + title = works(:poodr).title + + # Act - Assert + expect { + delete work_path(id) + }.wont_change 'Work.count' + + must_respond_with :redirect + must_redirect_to root_path + + end end describe "upvote" do @@ -254,19 +348,47 @@ # expect (session[:user_id]).to_equal nil it "redirects to the work page if no user is logged in" do + delete logout_path + expect (session[:user_id]).must_be_nil + post upvote_path(poodr.id) + + must_respond_with :redirect + must_redirect_to root_path end it "redirects to the work page after the user has logged out" do + perform_login(user) + expect (session[:user_id]).wont_be_nil + + delete logout_path + expect (session[:user_id]).must_be_nil + 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(user) + expect (session[:user_id]).wont_be_nil + expect { + post upvote_path(poodr.id) + }.must_change 'poodr.votes.count', +1 + + must_redirect_to work_path(poodr.id) end it "redirects to the work page if the user has already voted for that work" do + perform_login(user) + expect (session[:user_id]).wont_be_nil + + post upvote_path(poodr.id) + expect { + post upvote_path(poodr.id) + }.wont_change 'poodr.votes.count', 0 + must_redirect_to work_path(poodr.id) end end end From ceaa573ac003811a4b4ec6a6f0bde7fb8cb15a59 Mon Sep 17 00:00:00 2001 From: Danielle Metzner Date: Mon, 29 Oct 2018 23:14:59 -0700 Subject: [PATCH 6/7] Fixed tests for user and vote models --- test/fixtures/votes.yml | 6 +++--- test/models/user_test.rb | 10 +++------- test/models/vote_test.rb | 6 ++++-- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/test/fixtures/votes.yml b/test/fixtures/votes.yml index 151043ba..2a43aeb8 100644 --- a/test/fixtures/votes.yml +++ b/test/fixtures/votes.yml @@ -5,13 +5,13 @@ # below each fixture, per the syntax in the comments below # one: - user: kari + user: ada work: album two: - user: dan + user: grace work: another_album three: - user: dan + user: grace work: album diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 47c7eb72..efc83551 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -1,12 +1,9 @@ require 'test_helper' describe User do - before do - perform_login(users(:grace) - end + let(:grace) {users(:grace)} describe "relations" do it "has a list of votes" do - grace = users(:grace) grace.must_respond_to :votes grace.votes.each do |vote| vote.must_be_kind_of Vote @@ -14,7 +11,6 @@ end it "has a list of ranked works" do - grace = users(:grace) grace.must_respond_to :ranked_works grace.ranked_works.each do |work| work.must_be_kind_of Work @@ -31,12 +27,12 @@ it "requires a unique username" do username = "test username" - user1 = User.new(username: username) + user1 = User.new(username: username, uid:1, 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..a6013cff 100644 --- a/test/models/vote_test.rb +++ b/test/models/vote_test.rb @@ -1,6 +1,8 @@ require 'test_helper' describe Vote do + let(:grace) {users(:grace)} + let(:ada) {users(:ada)} describe "relations" do it "has a user" do v = votes(:one) @@ -16,8 +18,8 @@ end describe "validations" do - let (:user1) { User.new(username: 'chris') } - let (:user2) { User.new(username: 'chris') } + let (:user1) { User.new(username: 'chris',uid:1, 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') } From b193b8f0d6f61e10f316645fd6d367a574b2dd3d Mon Sep 17 00:00:00 2001 From: Danielle Metzner Date: Tue, 30 Oct 2018 09:21:56 -0700 Subject: [PATCH 7/7] Added Users Controller tests --- test/controllers/users_controller_test.rb | 40 ++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index d8503324..fbd52666 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -1,5 +1,43 @@ require 'test_helper' describe UsersController do - + let(:user) {users(:ada)} + describe "Index" do + it "logged in user can see a list of all users" do + perform_login(user) + + get users_path + + must_respond_with :success + + end + + it 'redirects if a logged out user tries to view all users' do + delete logout_path + expect (session[:user_id]).must_be_nil + + get users_path + + must_redirect_to root_path + end + end + describe "Show" do + it "logged in User can view page" do + perform_login(user) + + expect (session[:user_id]).wont_be_nil + + get user_path(user.id) + + must_respond_with :success + end + it "redirects if the user is not logged in" do + delete logout_path + expect (session[:user_id]).must_be_nil + + get user_path(user.id) + + must_redirect_to root_path + end + end end