From 40977d34ed7bb0f49e04984642db219895e1bc5c Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Mon, 15 Oct 2018 14:32:56 -0700 Subject: [PATCH 01/15] write test for create --- db/schema.rb | 34 +++++++++++------------ test/controllers/works_controller_test.rb | 21 ++++++++++++++ 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index 6bc8ba5c..a35df558 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" + create_table "users", id: :serial, force: :cascade do |t| + 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" + 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/works_controller_test.rb b/test/controllers/works_controller_test.rb index 0945ca47..ec143d50 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -39,8 +39,29 @@ describe "create" do it "creates a work with valid data for a real category" do + # Arrange + work_data = { + work: { + title: "new test book", + category: "album" + } + } + + # Assumptions + test_work = Work.new(work_data[:work]) + test_work.must_be :valid?, "Work data was invalid. Please come fix this test" + + # Act + expect { + post works_path, params: work_data + }.must_change('Work.count', +1) + + # Assert + must_redirect_to work_path(Work.last) end + + it "renders bad_request and does not update the DB for bogus data" do end From 94e5afaf05eed9020bea9e9cb4233b23d3e2abad Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Mon, 15 Oct 2018 14:49:39 -0700 Subject: [PATCH 02/15] add tests for new in works controller --- test/controllers/works_controller_test.rb | 37 ++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index ec143d50..fcf74b49 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -63,13 +63,48 @@ it "renders bad_request and does not update the DB for bogus data" do + # Arrange + work_data = { + work: { + title: Work.first, + category: "novel" + } + } + + # Assumptions + Work.new(work_data[:work]).wont_be :valid?, "Work data wasn't invalid. Please come fix this test" + + # Act + expect { + post works_path, params: work_data + }.wont_change('Work.count') + + # Assert + must_respond_with :bad_request end it "renders 400 bad_request for bogus categories" do - end + # Arrange + work_data = { + work: { + title: "new test book", + category: "novel" + } + } + + # Assumptions + Work.new(work_data[:work]).wont_be :valid?, "Work data wasn't invalid. Please come fix this test" + # Act + expect { + post works_path, params: work_data + }.wont_change('Work.count') + + # Assert + must_respond_with :bad_request + end end describe "show" do From cee1a9a04536b3f365258b009838a0012527f681 Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Mon, 15 Oct 2018 15:45:26 -0700 Subject: [PATCH 03/15] add test for root --- app/controllers/works_controller.rb | 2 +- test/controllers/works_controller_test.rb | 59 ++++++++++++++++++++++- 2 files changed, 59 insertions(+), 2 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/works_controller_test.rb b/test/controllers/works_controller_test.rb index fcf74b49..577cd371 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -4,15 +4,52 @@ 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 + # Arrange + movie = works(:movie) + id = movie.id + + get work_path(id) + must_respond_with :success + + movie.destroy + get root_path + must_respond_with :success end it "succeeds with no media" do + movie = works(:movie) + id = movie.id + get work_path(id) + must_respond_with :success + movie.destroy + + album = works(:album) + id = album.id + get work_path(id) + must_respond_with :success + album.destroy + + another_album = works(:another_album) + id = another_album.id + get work_path(id) + must_respond_with :success + another_album.destroy + + book = works(:poodr) + id = book.id + get work_path(id) + must_respond_with :success + book.destroy + + get root_path + must_respond_with :success end end @@ -109,11 +146,31 @@ describe "show" do it "succeeds for an extant work ID" do + # Arrange + existing_work = works(:poodr) + + # Act + get work_path(existing_work.id) + + # Assert + must_respond_with :success end it "renders 404 not_found for a bogus work ID" do + # Arrange + album = works(:album) + id = album.id + get work_path(id) + must_respond_with :success + + album.destroy + + # Act + get work_path(id) + # Assert + must_respond_with :not_found end end From aa6c512f6ab068a8075b92b5c9fc5c92a0b50202 Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Tue, 16 Oct 2018 10:56:23 -0700 Subject: [PATCH 04/15] add omniauth --- Gemfile | 3 + test/controllers/works_controller_test.rb | 83 +++++++++++++++++++++-- 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/Gemfile b/Gemfile index 42f4bb2c..365a794b 100644 --- a/Gemfile +++ b/Gemfile @@ -66,3 +66,6 @@ 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/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 577cd371..0464566d 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -27,25 +27,25 @@ movie = works(:movie) id = movie.id get work_path(id) - must_respond_with :success + #must_respond_with :success movie.destroy album = works(:album) id = album.id get work_path(id) - must_respond_with :success + #must_respond_with :success album.destroy another_album = works(:another_album) id = another_album.id get work_path(id) - must_respond_with :success + #must_respond_with :success another_album.destroy book = works(:poodr) id = book.id get work_path(id) - must_respond_with :success + #must_respond_with :success book.destroy get root_path @@ -59,16 +59,41 @@ describe "index" do it "succeeds when there are works" do + movie = works(:movie) + id = movie.id + get work_path(id) + #must_respond_with :success + movie.destroy - end + album = works(:album) + id = album.id + get work_path(id) + #must_respond_with :success + album.destroy - it "succeeds when there are no works" do + another_album = works(:another_album) + id = another_album.id + get work_path(id) + #must_respond_with :success + another_album.destroy + book = works(:poodr) + id = book.id + get work_path(id) + #must_respond_with :success + book.destroy + + 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 @@ -176,17 +201,63 @@ describe "edit" do it "succeeds for an extant work ID" do + get edit_work_path(Work.first) + must_respond_with :success end it "renders 404 not_found for a bogus work ID" do + # Arrange + album = works(:album) + id = album.id + get work_path(id) + must_respond_with :success + + album.destroy + + # Act + get work_path(id) + # Assert + must_respond_with :not_found end end describe "update" do + + let (:work_hash) { + { + work: { + title: "new test book", + category: "book" + } + } + } + + # work_data = { + # work: { + # title: "new test book", + # category: "novel" + # } + # } + it "succeeds for valid data and an extant work ID" do + id = works(:poodr).id + + expect { + patch work_path(id), params: work_hash + must_respond_with :success + }.wont_change 'Work.count' + + must_respond_with :redirect + + book = Work.find_by(id: id) + + must_respond_with :success + # expect(work.title).must_equal work_hash[:work][:title] + # + # expect(work.description).must_equal work_hash[:work][:category] end it "renders bad_request for bogus data" do From 313c88a4659d9c522be7e1f3849ae959e7849175 Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Tue, 16 Oct 2018 13:43:16 -0700 Subject: [PATCH 05/15] add columns for uid, provider --- .gitignore | 2 + Gemfile | 4 + Gemfile.lock | 30 +++++++- app/controllers/sessions_controller.rb | 73 ++++++++++++++----- app/models/user.rb | 11 +++ app/views/layouts/application.html.erb | 5 +- config/initializers/omniauth.rb | 3 + config/routes.rb | 9 ++- ...20181016184106_change_user_name_to_name.rb | 5 ++ .../20181016184450_add_email_uid_provider.rb | 7 ++ db/migrate/20181016190313_add_user_name.rb | 5 ++ db/schema.rb | 38 +++++----- 12 files changed, 152 insertions(+), 40 deletions(-) create mode 100644 config/initializers/omniauth.rb create mode 100644 db/migrate/20181016184106_change_user_name_to_name.rb create mode 100644 db/migrate/20181016184450_add_email_uid_provider.rb create mode 100644 db/migrate/20181016190313_add_user_name.rb 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..c61be7ad 100644 --- a/Gemfile +++ b/Gemfile @@ -59,6 +59,7 @@ group :development do # Access an IRB console on exception pages or by using <%= console %> anywhere in the code. gem 'web-console', '>= 3.3.0' gem 'listen', '~> 3.0.5' + gem 'dotenv-rails' # 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' @@ -66,3 +67,6 @@ 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..e1aa6665 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) @@ -227,4 +255,4 @@ DEPENDENCIES web-console (>= 3.3.0) BUNDLED WITH - 1.16.5 + 1.16.6 diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 5bce99e6..146dc519 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) - session[:user_id] = user.id - flash[:status] = :success - flash[:result_text] = "Successfully logged in as existing user #{user.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 + flash[:success] = "Logged in as returning user #{user.name}" else - user = User.new(username: username) + # User doesn't match anything in the DB + # TODO: 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 + + 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..c27e1253 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.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..07ef2a22 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -38,14 +38,15 @@
  • <%= link_to "Logged in as #{@login_user.username}", 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/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..1cbdb2d5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,12 +1,15 @@ 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' resources :works post '/works/:id/upvote', to: 'works#upvote', as: 'upvote' + get "/auth/:provider/callback", to: "sessions#create" + delete "/logout", to: "sessions#destroy", as: "logout" resources :users, only: [:index, :show] + end diff --git a/db/migrate/20181016184106_change_user_name_to_name.rb b/db/migrate/20181016184106_change_user_name_to_name.rb new file mode 100644 index 00000000..2bf8c56e --- /dev/null +++ b/db/migrate/20181016184106_change_user_name_to_name.rb @@ -0,0 +1,5 @@ +class ChangeUserNameToName < ActiveRecord::Migration[5.2] + def change + rename_column(:users, :username, :name) + end +end diff --git a/db/migrate/20181016184450_add_email_uid_provider.rb b/db/migrate/20181016184450_add_email_uid_provider.rb new file mode 100644 index 00000000..14446243 --- /dev/null +++ b/db/migrate/20181016184450_add_email_uid_provider.rb @@ -0,0 +1,7 @@ +class AddEmailUidProvider < 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/migrate/20181016190313_add_user_name.rb b/db/migrate/20181016190313_add_user_name.rb new file mode 100644 index 00000000..a5f6d368 --- /dev/null +++ b/db/migrate/20181016190313_add_user_name.rb @@ -0,0 +1,5 @@ +class AddUserName < ActiveRecord::Migration[5.2] + def change + add_column(:users, :username, :string) + end +end diff --git a/db/schema.rb b/db/schema.rb index 6bc8ba5c..af979153 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_16_190313) 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 "name" 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 + t.string "username" 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" From aa8b507475792124b8ea4f107d57452165c9697f Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Tue, 16 Oct 2018 14:02:25 -0700 Subject: [PATCH 06/15] add username --- app/controllers/sessions_controller.rb | 1 - app/models/user.rb | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 146dc519..fc6dead3 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -35,7 +35,6 @@ def login_form 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 diff --git a/app/models/user.rb b/app/models/user.rb index c27e1253..65d9bf57 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -10,6 +10,7 @@ def self.build_from_github(auth_hash) user.provider = 'github' user.name = auth_hash['info']['name'] user.email = auth_hash['info']['email'] + user.username = auth_hash['info']['nickname'] # Note that the user has not been saved return user From c1a3f877a3b9230f51fce6cfff733899d469fd19 Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Tue, 16 Oct 2018 14:14:35 -0700 Subject: [PATCH 07/15] add update tests in session controller --- .env | 2 ++ Gemfile.lock | 23 +++++++++++++++++++++++ test/controllers/works_controller_test.rb | 20 ++++++-------------- 3 files changed, 31 insertions(+), 14 deletions(-) create mode 100644 .env diff --git a/.env b/.env new file mode 100644 index 00000000..12b1fdf1 --- /dev/null +++ b/.env @@ -0,0 +1,2 @@ +GITHUB_CLIENT_ID: b36d6a8355150bead721 +GITHUB_CLIENT_SECRET: 345618b701298840aec16d0cfc8b6db2fdc4ea0a \ No newline at end of file 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/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 0464566d..6ada7641 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -87,7 +87,7 @@ must_respond_with :success end - + end describe "new" do @@ -229,35 +229,27 @@ { work: { title: "new test book", - category: "book" + category: "book", + description: "new test description" } } } - # work_data = { - # work: { - # title: "new test book", - # category: "novel" - # } - # } - it "succeeds for valid data and an extant work ID" do id = works(:poodr).id expect { patch work_path(id), params: work_hash - must_respond_with :success }.wont_change 'Work.count' must_respond_with :redirect - book = Work.find_by(id: id) + work = Work.find_by(id: id) - must_respond_with :success - # expect(work.title).must_equal work_hash[:work][:title] + expect(work.title).must_equal "new test book" # - # expect(work.description).must_equal work_hash[:work][:category] + expect(work.description).must_equal "new test description" end it "renders bad_request for bogus data" do From 12feec506757b6151a51c4ca4fa009fbec1eccf2 Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Tue, 16 Oct 2018 14:49:55 -0700 Subject: [PATCH 08/15] saved merge conflict --- .env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.env b/.env index 12b1fdf1..d7551109 100644 --- a/.env +++ b/.env @@ -1,2 +1,2 @@ GITHUB_CLIENT_ID: b36d6a8355150bead721 -GITHUB_CLIENT_SECRET: 345618b701298840aec16d0cfc8b6db2fdc4ea0a \ No newline at end of file +GITHUB_CLIENT_SECRET: 345618b701298840aec16d0cfc8b6db2fdc4ea0a From 3562fe4208253866d02635ef679cd42581825872 Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Tue, 16 Oct 2018 14:55:37 -0700 Subject: [PATCH 09/15] save initializer and migrate files --- config/initializers/omniauth.rb | 3 +++ db/migrate/20181016184450_add_email_uid_provider.rb | 7 +++++++ db/migrate/20181016190313_add_user_name.rb | 5 +++++ 3 files changed, 15 insertions(+) create mode 100644 config/initializers/omniauth.rb create mode 100644 db/migrate/20181016184450_add_email_uid_provider.rb create mode 100644 db/migrate/20181016190313_add_user_name.rb 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/db/migrate/20181016184450_add_email_uid_provider.rb b/db/migrate/20181016184450_add_email_uid_provider.rb new file mode 100644 index 00000000..14446243 --- /dev/null +++ b/db/migrate/20181016184450_add_email_uid_provider.rb @@ -0,0 +1,7 @@ +class AddEmailUidProvider < 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/migrate/20181016190313_add_user_name.rb b/db/migrate/20181016190313_add_user_name.rb new file mode 100644 index 00000000..a5f6d368 --- /dev/null +++ b/db/migrate/20181016190313_add_user_name.rb @@ -0,0 +1,5 @@ +class AddUserName < ActiveRecord::Migration[5.2] + def change + add_column(:users, :username, :string) + end +end From bbc380728b965524723237307a1c4cba61a921f5 Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Tue, 16 Oct 2018 15:20:43 -0700 Subject: [PATCH 10/15] add uid and provider to user tests --- test/fixtures/users.yml | 4 ++++ test/models/user_test.rb | 4 ++-- test/models/vote_test.rb | 4 ++-- test/models/work_test.rb | 4 ++-- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index e2968d78..6715d745 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -2,6 +2,10 @@ dan: username: dan + uid: 11 + provider: github kari: username: kari + uid: 11 + provider: github diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 793ce7e6..066c5356 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: 88, provider: 'github') # This must go through, so we use create! user1.save! - user2 = User.new(username: username) + user2 = User.new(username: username, uid: 99, 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..a25843bb 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: 44, provider: 'github') } + let (:user2) { User.new(username: 'chris', uid: 55, 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..2b1f9938 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: 100, 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: 100, provider: 'github') end # Create media to vote upon From 80fce92b76aaadb70b73f4db534c966c3a7e3efe Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Tue, 16 Oct 2018 18:28:50 -0700 Subject: [PATCH 11/15] add test for destroy method --- test/controllers/works_controller_test.rb | 39 +++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 6ada7641..089952f1 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -207,6 +207,7 @@ end it "renders 404 not_found for a bogus work ID" do + # Arrange album = works(:album) id = album.id @@ -217,7 +218,7 @@ album.destroy # Act - get work_path(id) + get work_path(id) # Assert must_respond_with :not_found end @@ -248,26 +249,60 @@ work = Work.find_by(id: id) expect(work.title).must_equal "new test book" - # + expect(work.category).must_equal "book" expect(work.description).must_equal "new test description" end it "renders bad_request for bogus data" do + # Arrange + id = " " + + patch work_path(id) + + # Assert + must_respond_with :not_found end it "renders 404 not_found for a bogus work ID" do + # Arrange + id = " " + patch work_path(id), params: work_hash + + # Assert + must_respond_with :not_found end end describe "destroy" do it "succeeds for an extant work ID" do + # arrange + id = works(:album).id + #act + expect{ + delete work_path(id) + }.must_change 'Work.count', -1 + + # assert + must_respond_with :redirect + 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 + # arrange + id = " " + # act + expect{ + delete work_path(id) + }.wont_change 'Work.count' + + #assert + must_respond_with :not_found + #expect(Work.find_by_id(id)).must_equal nil + end end From 13983cd1a4d99b1cd7a2a9d51a5068713d6b264d Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Sun, 28 Oct 2018 18:47:29 -0700 Subject: [PATCH 12/15] add tests for users controller --- test/controllers/users_controller_test.rb | 27 +++++++++++++++++++++++ test/controllers/works_controller_test.rb | 23 +++++++++++++++++-- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index d2c5cfbb..949aea4e 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -2,4 +2,31 @@ describe UsersController do + describe "index" do + it "it should get index" do + + get users_path + must_respond_with :success + + end + end + + describe "show" do + it "should respond with success for showing an existing user" do + + existing_user = users(:dan) + get user_path(existing_user.id) + must_respond_with :success + end + + it "should respond with not found for showing a non-existing user" do + + get user_path(22) + + 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 089952f1..1cd320d6 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -116,7 +116,7 @@ # Act expect { post works_path, params: work_data - }.must_change('Work.count', +1) + }.must_change('Work.count', + 1) # Assert must_redirect_to work_path(Work.last) @@ -301,7 +301,7 @@ #assert must_respond_with :not_found - #expect(Work.find_by_id(id)).must_equal nil + expect(Work.find_by_id(id)).must_equal nil end end @@ -310,18 +310,37 @@ it "redirects to the work page if no user is logged in" do + # arrange + id = works(:movie).id + # act + post upvote_path(id) + + # assert + must_respond_with :redirect end it "redirects to the work page after the user has logged out" do + # arrange + + # act + # assert end it "succeeds for a logged-in user and a fresh user-vote pair" do + # arrange + # act + + # assert end it "redirects to the work page if the user has already voted for that work" do + # arrange + + # act + # assert end end end From fae43d75847d9958c47f47be20ea5b87133c5027 Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Mon, 29 Oct 2018 12:49:14 -0700 Subject: [PATCH 13/15] work on session controller test --- config/routes.rb | 4 +- test/controllers/sessions_controller_test.rb | 40 ++++++++++++++++++++ test/test_helper.rb | 29 ++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 1cbdb2d5..e23158f5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,8 +8,8 @@ resources :works post '/works/:id/upvote', to: 'works#upvote', as: 'upvote' - 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 :users, only: [:index, :show] - + end diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index f641d15c..da039a22 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -2,4 +2,44 @@ describe SessionsController do + it "can successfully log in with github as an existing user" do + # Arrange + # make sure that for some existing user, everything is configured! + + dan = users(:dan) + + OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) + + # Act + # Simulating logging in with github being successful (given the OmniAuth hash made above) + get auth_callback_path(:github) + + # Assert + + must_respond_with :success + expect(session[:user_id]).must_equal dan.id + expect(flash[:success]).must_equal "Logged in as returning user #{dan.name}" + + end + + +end + +describe "destroy" do + it "can logout a user" do + + dan = users(:dan) + + OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) + + get auth_callback_path(:github) + + expect ( session[:user_id] ).must_equal users(:dan.id) + + delete logout_path + + must_respond_with :redirect + must_redirect_to root_path + end + end diff --git a/test/test_helper.rb b/test/test_helper.rb index 5b4fb667..705d1ee6 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -23,4 +23,33 @@ 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 { + uid: user.id, + provider: user.provider, + info: { + nickname: user.username, + email: user.email + } + } + end + + def login_test_user + dan = users(:dan) + OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) + get auth_callback_path(:github) + end + + def logout_test_user + OmniAuth.config.mock_auth[:github] = nil + get logout_path + end + end From aea06f21358a918d376ae390d2739e212055c379 Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Tue, 30 Oct 2018 08:35:52 -0700 Subject: [PATCH 14/15] add user and works controller tests --- test/controllers/sessions_controller_test.rb | 78 ++++++++++---------- test/controllers/users_controller_test.rb | 13 ++++ test/controllers/works_controller_test.rb | 38 +++++++++- test/test_helper.rb | 6 +- 4 files changed, 90 insertions(+), 45 deletions(-) diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index da039a22..081beb6b 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -2,44 +2,46 @@ describe SessionsController do - it "can successfully log in with github as an existing user" do - # Arrange - # make sure that for some existing user, everything is configured! - dan = users(:dan) - - OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) - - # Act - # Simulating logging in with github being successful (given the OmniAuth hash made above) - get auth_callback_path(:github) - - # Assert - - must_respond_with :success - expect(session[:user_id]).must_equal dan.id - expect(flash[:success]).must_equal "Logged in as returning user #{dan.name}" - - end - - -end - -describe "destroy" do - it "can logout a user" do - - dan = users(:dan) - - OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) - - get auth_callback_path(:github) - - expect ( session[:user_id] ).must_equal users(:dan.id) - - delete logout_path - - must_respond_with :redirect - must_redirect_to root_path - end +# it "can successfully log in with github as an existing user" do +# # Arrange +# # make sure that for some existing user, everything is configured! +# +# dan = users(:dan) +# +# OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) +# +# # Act +# # Simulating logging in with github being successful (given the OmniAuth hash made above) +# get auth_callback_path(:github) +# +# # Assert +# +# must_respond_with :success +# expect(session[:user_id]).must_equal dan.id +# expect(flash[:success]).must_equal "Logged in as returning user #{dan.name}" +# +# end +# +# +# end + +# describe "destroy" do +# it "can logout a user" do +# +# dan = users(:dan) +# +# OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) +# +# get auth_callback_path(:github) +# +# expect ( session[:user_id] ).must_equal users(:dan.id) +# +# delete logout_path +# +# must_respond_with :redirect +# must_redirect_to root_path +# end +# end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 949aea4e..f8b1386d 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -2,6 +2,19 @@ describe UsersController do + it "logs in an existing user" do + start_count = User.count + user = users(:dan) + + 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 + + describe "index" do it "it should get index" do diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 1cd320d6..c92ea05d 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -1,6 +1,12 @@ require 'test_helper' describe WorksController do + + + let(:dan) { users(:dan) } + + + describe "root" do it "succeeds with all media types" do # Precondition: there is at least one media of each category @@ -308,6 +314,11 @@ describe "upvote" do + before do + perform_login(users(:dan)) + end + + it "redirects to the work page if no user is logged in" do # arrange @@ -321,18 +332,20 @@ it "redirects to the work page after the user has logged out" do # arrange - + id = works(:movie).id # act - + delete logout_path(id) # assert + must_respond_with :redirect end it "succeeds for a logged-in user and a fresh user-vote pair" do # arrange - + vote = works(:movie).id # act - + post upvote_path(vote) # assert + must_respond_with :success end it "redirects to the work page if the user has already voted for that work" do @@ -341,6 +354,23 @@ # act # assert + must_respond_with :redirect + end + end + + describe "Guest users" do + it "can access the index" do + get works_path + must_respond_with :success + end + + it "cannot access show" do + existing_work = works(:poodr) + + get work_path(existing_work.id) + + must_redirect_to root_path + flash[:message].must_equal "You must be logged in to see that page!" end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 705d1ee6..7cc7c5c8 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -41,9 +41,9 @@ def mock_auth_hash(user) } end - def login_test_user - dan = users(:dan) - OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) + def perform_login(user) + + OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash(user) ) get auth_callback_path(:github) end From 6a64367127189c9c96c3c42bff7f50d1c32351fb Mon Sep 17 00:00:00 2001 From: Melissa O'Hearn Date: Tue, 30 Oct 2018 08:41:05 -0700 Subject: [PATCH 15/15] uncommented tests --- test/controllers/sessions_controller_test.rb | 80 ++++++++++---------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index 081beb6b..f0bfd8b3 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -4,44 +4,44 @@ -# it "can successfully log in with github as an existing user" do -# # Arrange -# # make sure that for some existing user, everything is configured! -# -# dan = users(:dan) -# -# OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) -# -# # Act -# # Simulating logging in with github being successful (given the OmniAuth hash made above) -# get auth_callback_path(:github) -# -# # Assert -# -# must_respond_with :success -# expect(session[:user_id]).must_equal dan.id -# expect(flash[:success]).must_equal "Logged in as returning user #{dan.name}" -# -# end -# -# -# end - -# describe "destroy" do -# it "can logout a user" do -# -# dan = users(:dan) -# -# OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) -# -# get auth_callback_path(:github) -# -# expect ( session[:user_id] ).must_equal users(:dan.id) -# -# delete logout_path -# -# must_respond_with :redirect -# must_redirect_to root_path -# end -# + it "can successfully log in with github as an existing user" do + # Arrange + # make sure that for some existing user, everything is configured! + + dan = users(:dan) + + OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) + + # Act + # Simulating logging in with github being successful (given the OmniAuth hash made above) + get auth_callback_path(:github) + + # Assert + + must_respond_with :success + expect(session[:user_id]).must_equal dan.id + expect(flash[:success]).must_equal "Logged in as returning user #{dan.name}" + + end + + +end + +describe "destroy" do + it "can logout a user" do + + dan = users(:dan) + + OmniAuth.config.mock_auth[:github] = OmniAuth::AuthHash.new( mock_auth_hash( dan ) ) + + get auth_callback_path(:github) + + expect ( session[:user_id] ).must_equal users(:dan.id) + + delete logout_path + + must_respond_with :redirect + must_redirect_to root_path + end + end