diff --git a/.gitignore b/.gitignore index 48fb168f..5a111e4e 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,5 @@ # Ignore Byebug command history file. .byebug_history + +.env diff --git a/Gemfile b/Gemfile index 42f4bb2c..94d1d206 100644 --- a/Gemfile +++ b/Gemfile @@ -62,7 +62,10 @@ 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 gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] +gem "omniauth" +gem "omniauth-github" 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..c441736c 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,34 +1,59 @@ class SessionsController < ApplicationController - def login_form - 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 + flash[:success] = "Logged in as returning user #{user.name}" - 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) + # User doesn't match anything in the DB + # Attempt to create a new user + 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 + # Couldn't save the user for some reason. If we + # hit this it probably means there's a bug with the + # way we've configured GitHub. Our strategy will + # be to display error messages to make future + # debugging easier. + flash[:error] = "Could not create new user account: #{user.errors.messages}" + redirect_to root_path return end end - redirect_to root_path - 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 + # def logout + # session[:user_id] = nil + # flash[:status] = :success + # flash[:result_text] = "Successfully logged out" + # redirect_to root_path + # end + end end -end diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 2020bee4..dde9d38b 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 @@ -63,6 +63,7 @@ def destroy def upvote flash[:status] = :failure + if @login_user vote = Vote.new(user: @login_user, work: @work) if vote.save @@ -79,6 +80,7 @@ def upvote # Refresh the page to show either the updated vote count # or the error message redirect_back fallback_location: work_path(@work) + end private diff --git a/app/models/user.rb b/app/models/user.rb index 4cac8fe0..7280c813 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -3,4 +3,16 @@ 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.uid = auth_hash[:uid] + user.provider = 'github' + user.name = auth_hash['info']['name'] + user.email = auth_hash['info']['email'] + + # Note that the user has not been saved + return user + end + end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index e7b07ce4..066be8a8 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -32,22 +32,11 @@ <%= link_to "View all users", users_path, class: "nav-link" %> - diff --git a/config/initializers/edit.html.erb b/config/initializers/edit.html.erb new file mode 100644 index 00000000..b8521da3 --- /dev/null +++ b/config/initializers/edit.html.erb @@ -0,0 +1 @@ +<%= render partial: 'form', locals: {action_name: 'Edit a passenger', button_title: 'Update'} %> 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..26d62907 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,12 +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 "/auth/:provider/callback", to: "sessions#create", as: 'auth_callback' get '/login', to: 'sessions#login_form', as: 'login' post '/login', to: 'sessions#login' - post '/logout', to: 'sessions#logout', as: 'logout' - + # post '/logout', to: 'sessions#logout', as: 'logout' + delete "/logout", to: "sessions#destroy", as: "logout" resources :works post '/works/:id/upvote', to: 'works#upvote', as: 'upvote' - + get '/votes/new', to: 'votes#new' + post '/votes', to: 'votes#create' resources :users, only: [:index, :show] end diff --git a/db/migrate/20181016214201_change_column_name_to_nickname.rb b/db/migrate/20181016214201_change_column_name_to_nickname.rb new file mode 100644 index 00000000..1c231e3f --- /dev/null +++ b/db/migrate/20181016214201_change_column_name_to_nickname.rb @@ -0,0 +1,5 @@ +class ChangeColumnNameToNickname < ActiveRecord::Migration[5.2] + def change + rename_column :users, :username, :nickname + end +end diff --git a/db/migrate/20181016215622_change_column_name_back_to_users.rb b/db/migrate/20181016215622_change_column_name_back_to_users.rb new file mode 100644 index 00000000..90d82572 --- /dev/null +++ b/db/migrate/20181016215622_change_column_name_back_to_users.rb @@ -0,0 +1,5 @@ +class ChangeColumnNameBackToUsers < ActiveRecord::Migration[5.2] + def change + rename_column :users, :nickname, :username + end +end diff --git a/db/migrate/20181017012927_add_columnto_users.rb b/db/migrate/20181017012927_add_columnto_users.rb new file mode 100644 index 00000000..6e2a2731 --- /dev/null +++ b/db/migrate/20181017012927_add_columnto_users.rb @@ -0,0 +1,5 @@ +class AddColumntoUsers < ActiveRecord::Migration[5.2] + def change + add_column :users, :uid, :integer + end +end diff --git a/db/migrate/20181017035709_add_provider_column_to_users.rb b/db/migrate/20181017035709_add_provider_column_to_users.rb new file mode 100644 index 00000000..38383353 --- /dev/null +++ b/db/migrate/20181017035709_add_provider_column_to_users.rb @@ -0,0 +1,5 @@ +class AddProviderColumnToUsers < ActiveRecord::Migration[5.2] + def change + add_column :users, :provider, :string + end +end diff --git a/db/migrate/20181017040520_add_pname_column_to_users.rb b/db/migrate/20181017040520_add_pname_column_to_users.rb new file mode 100644 index 00000000..5855a9ae --- /dev/null +++ b/db/migrate/20181017040520_add_pname_column_to_users.rb @@ -0,0 +1,5 @@ +class AddPnameColumnToUsers < ActiveRecord::Migration[5.2] + def change + add_column :users, :name, :string + end +end diff --git a/db/migrate/20181017040740_add_email_to_users.rb b/db/migrate/20181017040740_add_email_to_users.rb new file mode 100644 index 00000000..cd7fc310 --- /dev/null +++ b/db/migrate/20181017040740_add_email_to_users.rb @@ -0,0 +1,5 @@ +class AddEmailToUsers < ActiveRecord::Migration[5.2] + def change + add_column :users, :email, :string + end +end diff --git a/db/migrate/edit.html.erb b/db/migrate/edit.html.erb new file mode 100644 index 00000000..b8521da3 --- /dev/null +++ b/db/migrate/edit.html.erb @@ -0,0 +1 @@ +<%= render partial: 'form', locals: {action_name: 'Edit a passenger', button_title: 'Update'} %> diff --git a/db/schema.rb b/db/schema.rb index 6bc8ba5c..83fb8536 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,35 +10,39 @@ # # 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_17_040740) 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" + create_table "users", id: :serial, force: :cascade do |t| + t.string "username" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "uid" + t.string "provider" + t.string "name" + t.string "email" end - create_table "votes", force: :cascade do |t| - t.integer "user_id" - t.integer "work_id" + create_table "votes", id: :serial, force: :cascade do |t| + 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" + create_table "works", id: :serial, 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" end add_foreign_key "votes", "users" diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index f641d15c..465d8790 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -1,5 +1,40 @@ require "test_helper" describe SessionsController do + def create + auth_hash = request.env['omniauth.auth'] + user = User.find_by(nickname: auth_hash[:nickname], provider: 'github') + if user + # User was found in the database + flash[:success] = "Logged in as returning user #{user.name}" + else + # User doesn't match anything in the DB + # Attempt to create a new user + user = User.build_from_github(auth_hash) + if user.save + flash[:success] = "Logged in as new user #{user.name}" + + else + # Couldn't save the user for some reason. If we + # hit this it probably means there's a bug with the + # way we've configured GitHub. Our strategy will + # be to display error messages to make future + # debugging easier. + flash[:error] = "Could not create new user account: #{user.errors.messages}" + redirect_to root_path + return + end + end + + # If we get here, we have a valid user instance + session[:user_id] = user.id + redirect_to root_path + end + def destroy + session[:user_id] = nil + flash[:success] = "Successfully logged out!" + + redirect_to root_path +end end diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 0945ca47..6591f365 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -4,15 +4,26 @@ 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 + work = works(:poodr) + work,category = 'movie' + get root_path + must_respond_with :success end it "succeeds with no media" do + works = Work.all + works = nil + get root_path + + must_respond_with :success end end @@ -22,95 +33,254 @@ describe "index" do it "succeeds when there are works" do + works = Work.all + get works_path + + must_respond_with :success end it "succeeds when there are no works" do + Work.destroy_all + + get works_path + must_respond_with :success + expect(Work.all.count).must_equal 0 end end describe "new" do it "succeeds" do + get new_work_path + # Assert + must_respond_with :success end end describe "create" do + let (:work_hash) do + { + work: { + title: 'Eternal Sunshine of the spotless mind', + category: 'movie' + } + } + end it "creates a work with valid data for a real category" do - + # Act-Assert + 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.description).must_equal work_hash[:work][:description] end it "renders bad_request and does not update the DB for bogus data" do + # Arranges + 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 + # Arranges + work_hash[:work][:category] = 'magazine' + + # Act-Assert + expect { + post works_path, params: work_hash + }.wont_change 'Work.count' + must_respond_with :bad_request end end describe "show" do - it "succeeds for an extant work ID" do + it "succeeds for an existing work ID" do + # Arrange + 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 + # Arrange - invalid id + id = -1 + + # Act + get work_path(id) + # Assert + must_respond_with :not_found end end describe "edit" do - it "succeeds for an extant work ID" do + it "succeeds for an existing work ID" do + 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 + # Arrange - invalid id + id = -1 + + # Act + get work_path(id) + # Assert + must_respond_with :not_found end end describe "update" do - it "succeeds for valid data and an extant work ID" do + let (:work_hash) do + { + work: { + title: 'Eat Pray Love', + creator: "Elizabeth Gilbert", + description:"beautiful life memoir", + publication_year: 2004, + category: "book" + } + } + end + it "succeeds for valid data and an existing 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] 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 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 + it "succeeds for an existing work ID" do + 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(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) + # }.must_change 'Book.count', 0 + }.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 + id = works(:poodr).id - end + expect{ + post upvote_path(id)}.wont_change 'Vote.count' - it "redirects to the work page after the user has logged out" do + must_redirect_to work_path(id) + # expect(flash[:warning]).must_equal "You must be logged in to vote for a work." end - it "succeeds for a logged-in user and a fresh user-vote pair" do - end + it "redirects to the work page after the user has logged out" do + end - it "redirects to the work page if the user has already voted for that work" do + it "succeeds for a logged-in user and a fresh user-vote pair" do + user = users(:grace) + id = works(:poodr).id + vote = Vote.new(user: @login_user, work: @work) - end + perform_login(user) + + post upvote_path(id) + + must_redirect_to work_path(id) + + end + + it "redirects to the work page if the user has already voted for that work" do + user = users(:grace) + id = works(:poodr).id + vote = votes(:one) + + perform_login(user) + + post upvote_path(id) + expect{ post upvote_path(id)}.wont_change 'Vote.count' + + # expect(flash[:result_text]).must_equal "Could not upvote" + # must_redirect_to work_path(@work) + end + end end -end diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index e2968d78..84dd9caf 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -5,3 +5,15 @@ dan: kari: username: kari + +ada: + provider: github + uid: 12345 + email: ada@adadevelopersacademy.org + username: countess_ada + +grace: + provider: github + uid: 13371337 + email: grace@hooper.net + username: graceful_hopps diff --git a/test/test_helper.rb b/test/test_helper.rb index 5b4fb667..df748d42 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -22,5 +22,25 @@ 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 + OmniAuth.config.test_mode = true + end + + def mock_auth_hash(user) + return { + provider: user.provider, + uid: user.uid, + info: { + username: user.username, + 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