diff --git a/.gitignore b/.gitignore index 48fb168f..4fdeab3b 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,6 @@ # Ignore Byebug command history file. .byebug_history + +# Ignore .env files, so that it is never put on Git +.env diff --git a/Gemfile b/Gemfile index c029c6da..b9a695ad 100644 --- a/Gemfile +++ b/Gemfile @@ -36,6 +36,9 @@ gem 'jbuilder', '~> 2.5' # Use Capistrano for deployment # gem 'capistrano-rails', group: :development +gem "omniauth" +gem "omniauth-github" + # Use the Foundation CSS framework gem 'foundation-rails' gem 'autoprefixer-rails' @@ -65,6 +68,8 @@ 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' + # Use for .env file + 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 f03db854..2e180b49 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -63,9 +63,15 @@ GEM coffee-script-source (1.12.2) concurrent-ruby (1.0.5) crass (1.0.3) + dotenv (2.2.2) + dotenv-rails (2.2.2) + dotenv (= 2.2.2) + railties (>= 3.2, < 6.0) erubi (1.7.1) erubis (2.7.0) execjs (2.7.0) + faraday (0.12.2) + multipart-post (>= 1.2, < 3) ffi (1.9.23) foundation-rails (6.4.3.0) railties (>= 3.1.0) @@ -73,6 +79,7 @@ GEM sprockets-es6 (>= 0.9.0) globalid (0.4.1) activesupport (>= 4.2.0) + hashie (3.5.7) i18n (1.0.0) concurrent-ruby (~> 1.0) jbuilder (2.7.0) @@ -82,6 +89,7 @@ GEM rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) + jwt (1.5.6) listen (3.0.8) rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) @@ -108,9 +116,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.0) nokogiri (1.8.2) mini_portile2 (~> 2.3.0) + oauth2 (1.4.0) + faraday (>= 0.8, < 0.13) + jwt (~> 1.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) pry (0.11.3) coderay (~> 1.1.0) @@ -199,6 +224,7 @@ DEPENDENCIES better_errors byebug coffee-rails (~> 4.2) + dotenv-rails foundation-rails jbuilder (~> 2.5) jquery-rails @@ -207,6 +233,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..ab3a5a12 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -2,33 +2,48 @@ 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}" - 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}" + def index + @user = User.find(session[:user_id]) # < recalls the value set in a previous request + end + + def new + @user = User.new + end + + def create + auth_hash = request.env['omniauth.auth'] + + if auth_hash['uid'] + @user = User.find_by(uid: auth_hash[:uid], provider: 'github') + if @user.nil? + # User doesn't match anything in the DB + # Attempt to create a new user + @user = User.new( name: auth_hash['info']['name'], email: auth_hash['info']['email'], uid: auth_hash['uid'], provider: auth_hash['provider']) + if @user.save + # saved successfully + session[:user_id] = @user.id + flash[:success] = "Logged in successfully" + redirect_to root_path + else + # not saved successfully + flash[:error] = "Could not log in" + redirect_to root_path + end 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 + session[:user_id] = @user.id + flash[:success] = "Logged in successfully" + redirect_to root_path end + else + flash[:error] = "Could not log in" + redirect_to root_path end - 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/controllers/works_controller.rb b/app/controllers/works_controller.rb index 2020bee4..bec9b790 100644 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -1,3 +1,4 @@ +require 'pry' class WorksController < ApplicationController # We should always be able to tell what category # of work we're dealing with @@ -50,7 +51,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/app/models/user.rb b/app/models/user.rb index 4cac8fe0..379658a5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,6 +1,2 @@ class User < ApplicationRecord - has_many :votes - has_many :ranked_works, through: :votes, source: :work - - validates :username, uniqueness: true, presence: true end diff --git a/app/models/work.rb b/app/models/work.rb index c61da303..67d9e672 100644 --- a/app/models/work.rb +++ b/app/models/work.rb @@ -23,7 +23,7 @@ def self.to_category_hash end def self.by_category(category) - category = category.singularize.downcase + category = category.to_s.singularize.downcase self.where(category: category).order(vote_count: :desc) end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 82ca0fdc..cc6e2f5a 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -25,29 +25,28 @@ <%= link_to "View all users", users_path, class: "button" %>
- <% if @login_user %> - <%= link_to "Logged in as #{@login_user.username}", user_path(@login_user), class: "button" %> - <%= link_to "Log Out", logout_path, method: :post, class: "button" %> + <% if session[:user_id] %> + <%= link_to "Logout", logout_path, method: :delete, class: "hollow button" %> <% else %> - <%= link_to "Log In", login_path, class: "button" %> + <%= link_to "Login", "/auth/github", class: "hollow button" %> <% end %>
<% if flash[:result_text] or flash[:messages] %> -
-

<%= flash[:status] == :failure ? "A problem occurred: " : "" %><%= flash[:result_text] %>

- <% if flash[:messages] %> - +
+

<%= flash[:status] == :failure ? "A problem occurred: " : "" %><%= flash[:result_text] %>

+ <% if flash[:messages] %> +
+ <% end %> + + <% end %> +
<% end %>
diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb new file mode 100644 index 00000000..fd441612 --- /dev/null +++ b/config/initializers/omniauth.rb @@ -0,0 +1,3 @@ +Rails.application.config.middleware.use OmniAuth::Builder do + provider :github, ENV["GITHUB_CLIENT_ID"], ENV["GITHUB_CLIENT_SECRET"], scope: "user:email" +end diff --git a/config/routes.rb b/config/routes.rb index a7e8af1d..bb8f14ac 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,11 +1,16 @@ 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#new', as: 'login' + get "/auth/:provider/callback", to: "sessions#create" + # post '/login', to: 'sessions#login' + # 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' resources :users, only: [:index, :show] diff --git a/db/migrate/20180501072044_create_users.rb b/db/migrate/20180501072044_create_users.rb new file mode 100644 index 00000000..53c95fe6 --- /dev/null +++ b/db/migrate/20180501072044_create_users.rb @@ -0,0 +1,12 @@ +class CreateUsers < ActiveRecord::Migration[5.0] + def change + create_table :users do |t| + t.string :name + t.string :email + t.integer :uid, null: false # this is the identifier provided by GitHub + t.string :provider, null: false # this tells us who provided the identifier + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 6bc8ba5c..8c8e6d40 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,13 +10,16 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170407164321) do +ActiveRecord::Schema.define(version: 20180501072044) 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 "name" + t.string "email" + t.integer "uid", null: false + t.string "provider", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false end @@ -41,6 +44,5 @@ t.integer "publication_year" end - add_foreign_key "votes", "users" add_foreign_key "votes", "works" end diff --git a/test/controllers/sessions_controller_test.rb b/test/controllers/sessions_controller_test.rb index f641d15c..4387f50f 100644 --- a/test/controllers/sessions_controller_test.rb +++ b/test/controllers/sessions_controller_test.rb @@ -1,5 +1,99 @@ require "test_helper" describe SessionsController do + describe 'login' do + # USER EXSISTS ALREADY + it 'logs in an exsisting user' do + user = users(:dan) + post login_path, params: {username: user.username} + + # Now we should have access to a session. And can test that we are in it. + session.keys.must_include "user_id" + session["user_id"].must_equal user.id + + # Flash needs to be tested Now + flash.keys.must_include "status" + flash["status"].must_equal :success + flash.keys.must_include 'result_text' + + must_redirect_to root_path + end + + # USER DOES NOT EXSIST, WE CREATE NEW USER + it 'logs in a new User' do + username = 'test username' + + counting = User.count + + User.new(username: username).must_be :valid? + + post login_path, params: {username: username} + + # Now we should have access to a session. And can test that we are in it. + session.keys.must_include "user_id" + session["user_id"].must_equal User.last.id + + # Flash needs to be tested Now + flash.keys.must_include "status" + flash["status"].must_equal :success + flash.keys.must_include 'result_text' + + User.count.must_equal counting + 1 + must_redirect_to root_path + end + + # USER DOESN'T EXSIST AND CANNOT CREATE + it 'renders bad request for an invalid username' do + username = nil + + counting = User.count + + User.new(username: username).wont_be :valid? + + post login_path, params: {username: username} + + session.keys.wont_include "user_id" + + flash.keys.must_include "status" + flash["status"].must_equal :failure + flash.keys.must_include 'messages' + flash.keys.must_include 'result_text' + + User.count.must_equal counting + must_respond_with :bad_request + end + end + + describe 'logout' do + it 'logs out the logged in user' do + user = users(:dan) + + post login_path, params: {username: user.username} + + session.keys.must_include "user_id" + + post logout_path + + session[:user_id].must_be_nil + + flash.keys.must_include "status" + flash["status"].must_equal :success + flash.keys.must_include 'result_text' + + must_redirect_to root_path + end + + it 'does nothing when no user is currently logged in' do + post logout_path + + session[:user_id].must_be_nil + + flash.keys.must_include "status" + flash["status"].must_equal :success + flash.keys.must_include 'result_text' + + must_redirect_to root_path + end + end end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index d2c5cfbb..43486ab3 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -1,5 +1,34 @@ require 'test_helper' describe UsersController do + describe 'index' do + it 'works with lots of users' do + User.count.must_be :>,0 + get users_path + must_respond_with :success + end + + it 'works with no users' do + User.destroy_all + + get users_path + must_respond_with :success + end + end + + describe 'show' do + it 'works for a real exsisting user' do + get user_path(User.first) + must_respond_with :success + end + + it 'renders not found (404) for a non-exsistent user' do + user_id = User.last.id + 1 + + get user_path(user_id) + + must_respond_with :not_found + end + end end diff --git a/test/controllers/works_controller_test.rb b/test/controllers/works_controller_test.rb index 0945ca47..dc56c666 100644 --- a/test/controllers/works_controller_test.rb +++ b/test/controllers/works_controller_test.rb @@ -4,16 +4,31 @@ describe "root" do it "succeeds with all media types" do # Precondition: there is at least one media of each category + %w(albums books movies).each do |category| + Work.by_category(category).length.must_be :>,0 - end + get root_path + end + end it "succeeds with one media type absent" do # Precondition: there is at least one media in two of the categories + %w(albums movies).each do |category| + Work.by_category(category).length.must_be :>,0 + Work.by_category(:books).destroy_all + + get root_path + must_respond_with :success + end end it "succeeds with no media" do + Work.destroy_all + get root_path + + must_respond_with :success end end @@ -22,95 +37,251 @@ describe "index" do it "succeeds when there are works" do + # Arrange - for each category + CATEGORIES.each do |category| + Work.by_category(category).length.must_be :>,0 + + get works_path(category) + must_respond_with :success + end end it "succeeds when there are no works" do + # Arrange - for each category + CATEGORIES.each do |category| + Work.destroy_all + + get works_path + must_respond_with :success + end end end describe "new" do it "succeeds" do - + get new_work_path + must_respond_with :success end end describe "create" do it "creates a work with valid data for a real category" do + work_data = { + title: "fake title", + category: "album" + } + + counting = Work.count + + Work.new(work_data).must_be :valid? + post works_path, params: {work: work_data} + + must_respond_with :redirect + must_redirect_to work_path(Work.last.id) + + Work.count.must_equal counting + 1 + Work.last.title.must_equal work_data[:title] end it "renders bad_request and does not update the DB for bogus data" do + work_data = { + title: " ", + category: "movie" + } + + counting = Work.count + Work.new(work_data).wont_be :valid? + + post works_path, params: { work: work_data} + + must_respond_with :bad_request + Work.count.must_equal counting end it "renders 400 bad_request for bogus categories" do + work_data = { + title: "test title", + category: INVALID_CATEGORIES.sample + } - end + counting = Work.count + Work.new(work_data).wont_be :valid? + + post works_path, params: { work: work_data} + + must_respond_with :bad_request + Work.count.must_equal counting + end end describe "show" do it "succeeds for an extant work ID" do - + get work_path(Work.first) + must_respond_with :success end it "renders 404 not_found for a bogus work ID" do + work_id = Work.last.id + 1 + + get work_path(work_id) + must_respond_with :not_found end end describe "edit" do it "succeeds for an extant work ID" do - + get work_path(Work.first) + must_respond_with :success end it "renders 404 not_found for a bogus work ID" do + bogus_work_id = Work.last.id + 1 + get edit_work_path(bogus_work_id) + + must_respond_with :not_found end end describe "update" do it "succeeds for valid data and an extant work ID" do + work = works(:album) + work_data = work.attributes + work_data[:title] = "random new title" + + work.assign_attributes(work_data) + work.must_be :valid? + + patch work_path(work), params: { work: work_data } + must_redirect_to work_path(work) + + work.reload + work.title.must_equal work_data[:title] end it "renders bad_request for bogus data" do + work = Work.first + work_data = work.attributes + work_data[:title] = " " + + work.assign_attributes(work_data) + work.wont_be :valid? + + patch work_path(work), params: { work: work_data } + must_respond_with :bad_request + + work.reload + work.title.wont_equal work_data[:title] end it "renders 404 not_found for a bogus work ID" do + work_id = Work.last.id + 1 + + patch work_path(work_id) + must_respond_with :not_found end end describe "destroy" do it "succeeds for an extant work ID" do + # Arrange + work_id = Work.first.id + start_count = Work.count + + # Act + delete work_path(work_id) + # Assert + must_respond_with :redirect + must_redirect_to root_path + + Work.count.must_equal start_count - 1 + Work.find_by(id: work_id).must_be_nil end it "renders 404 not_found and does not update the DB for a bogus work ID" do + work_id = Work.last.id + 1 + start_count = Work.count + + delete work_path(work_id) + must_respond_with :not_found + Work.count.must_equal start_count end end describe "upvote" do it "redirects to the work page if no user is logged in" do + work_id = Work.first.id + post upvote_path(work_id) + must_respond_with :redirect end it "redirects to the work page after the user has logged out" do + work_id = Work.first.id + + post logout_path + must_respond_with :redirect end it "succeeds for a logged-in user and a fresh user-vote pair" do + test_work = Work.first + + start_vote_count = test_work.votes.count + + post login_path, params: { username: User.first.username} + must_respond_with :redirect + + post upvote_path(test_work.id) + must_respond_with :redirect + test_work.votes.count.must_equal start_vote_count + 1 + end + + it 'adds a vote to the user\'s votes list' do + test_work = Work.first + + test_user = User.first + + start_vote_count = test_user.votes.count + + post login_path, params: { username: test_user.username} + must_respond_with :redirect + + post upvote_path(test_work.id) + must_respond_with :redirect + + test_user.votes.count.must_equal start_vote_count + 1 end it "redirects to the work page if the user has already voted for that work" do + test_work = Work.last + + start_vote_count = Vote.count + + post login_path, params: { username: User.first.username} + must_respond_with :redirect + post upvote_path(test_work.id) + must_respond_with :redirect + + Vote.count.must_equal start_vote_count end end end + +# TODO: GO BACK AND UPDATE USING FIXTURES +# TODO: Wave 2, OAuth login code. +# TODO: Wave 3, restriction logic and code for limited user view and access diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index e2968d78..dc3ee79b 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -1,7 +1,11 @@ # Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html -dan: - username: dan - -kari: - username: kari +# This model initially had no columns defined. If you add columns to the +# model remove the "{}" from the fixture names and add the columns immediately +# below each fixture, per the syntax in the comments below +# +one: {} +# column: value +# +two: {} +# column: value diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 793ce7e6..cc862ac2 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -1,44 +1,9 @@ -require 'test_helper' +require "test_helper" describe User do - describe "relations" do - it "has a list of votes" do - dan = users(:dan) - dan.must_respond_to :votes - dan.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| - work.must_be_kind_of Work - end - end - end - - describe "validations" do - it "requires a username" do - user = User.new - user.valid?.must_equal false - user.errors.messages.must_include :username - end - - it "requires a unique username" do - username = "test username" - user1 = User.new(username: username) - - # This must go through, so we use create! - user1.save! - - user2 = User.new(username: username) - result = user2.save - result.must_equal false - user2.errors.messages.must_include :username - end - + let(:user) { User.new } + it "must be valid" do + value(user).must_be :valid? end end diff --git a/test/models/work_test.rb b/test/models/work_test.rb index d9c00073..184c9541 100644 --- a/test/models/work_test.rb +++ b/test/models/work_test.rb @@ -93,7 +93,6 @@ describe "top_ten" do before do - # TODO DPR: This runs pretty slow. Fixtures? # Create users to do the voting test_users = [] 20.times do |i|