From a52af3da8b6b44767493d3c0bcec874c6a87e1d4 Mon Sep 17 00:00:00 2001 From: "M. Bellucci" Date: Fri, 11 Jan 2019 14:00:09 -0300 Subject: [PATCH] Avoid using meta-programming for solving code duplication. (#1457) * Add feature spec for twitter login * Avoid using class eval for solving code duplication. This change make code easier to change in the future. * Fix codeclimate issues * Introduce explanatory methods --- .../omniauth_callbacks_controller.rb | 54 ++++++++++------- .../user_logs_in_with_twitter_spec.rb | 58 +++++++++++++++++++ 2 files changed, 90 insertions(+), 22 deletions(-) create mode 100644 spec/features/user_logs_in_with_twitter_spec.rb diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index e17309514..e4d587c1a 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -1,29 +1,39 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController # Don't need a policy for this since this is our sign up/in route include Devise::Controllers::Rememberable - def self.provides_callback_for(provider) - # raise ApplicationConfig["omniauth.auth"].to_yaml - class_eval %{ - def #{provider} - cta_variant = request.env["omniauth.params"]['state'].to_s - @user = AuthorizationService.new(request.env["omniauth.auth"], current_user, cta_variant).get_user - if @user.persisted? && @user.valid? - remember_me(@user) - sign_in_and_redirect @user, event: :authentication - set_flash_message(:notice, :success, kind: "#{provider}".capitalize) if is_navigational_format? - elsif @user.persisted? && @user.errors.full_messages.join(", ").include?("username has already been taken") - redirect_to "/settings?state=previous-registration" - else - session["devise.#{provider}_data"] = request.env["omniauth.auth"] - user_errors = @user.errors.full_messages - flash[:alert] = user_errors - redirect_to new_user_registration_url - end - end - } + + def twitter + callback_for("twitter") end - %i[twitter github].each do |provider| - provides_callback_for provider + def github + callback_for("github") + end + + private + + def callback_for(provider) + cta_variant = request.env["omniauth.params"]["state"].to_s + @user = AuthorizationService.new(request.env["omniauth.auth"], current_user, cta_variant).get_user + if persisted_and_valid? + remember_me(@user) + sign_in_and_redirect @user, event: :authentication + set_flash_message(:notice, :success, kind: provider.to_s.capitalize) if is_navigational_format? + elsif persisted_but_username_taken? + redirect_to "/settings?state=previous-registration" + else + session["devise.#{provider}_data"] = request.env["omniauth.auth"] + user_errors = @user.errors.full_messages + flash[:alert] = user_errors + redirect_to new_user_registration_url + end + end + + def persisted_and_valid? + @user.persisted? && @user.valid? + end + + def persisted_but_username_taken? + @user.persisted? && @user.errors.full_messages.join(", ").include?("username has already been taken") end end diff --git a/spec/features/user_logs_in_with_twitter_spec.rb b/spec/features/user_logs_in_with_twitter_spec.rb new file mode 100644 index 000000000..3785aed7c --- /dev/null +++ b/spec/features/user_logs_in_with_twitter_spec.rb @@ -0,0 +1,58 @@ +require 'rails_helper' + +def user_grants_authorization_on_twitter_popup(twitter_callback_hash) + OmniAuth.config.add_mock(:twitter, twitter_callback_hash) +end + +def user_do_not_grants_authorization_on_twitter_popup + OmniAuth.config.mock_auth[:twitter] = :invalid_credentials +end + +RSpec.feature "Authenticating with twitter" do + let(:twitter_callback_hash) do + { + :provider => 'twitter', + :uid => '111111', + :credentials => { + :token => "222222", + :secret => "333333" + }, + :extra => { + :access_token => "", + :raw_info => { + :name => "Bruce Wayne", + :created_at => "Thu Jul 4 00:00:00 +0000 2013", # This is mandatory + } + }, + :info => { + :nickname => "batman", + :name => "Bruce Wayne", + :email => "batman@batcave.com", + } + } + end + + context "when user is new on dev.to" do + scenario "logging in with twitter using valid credentials" do + user_grants_authorization_on_twitter_popup(twitter_callback_hash) + + visit root_path + click_link "SIGN IN VIA TWITTER" + + expect(page).to have_link("Write your first post now") + expect(page).to have_link("Welcome Thread") + end + + scenario "logging in with twitter using invalid credentials" do + user_do_not_grants_authorization_on_twitter_popup + + visit root_path + click_link "SIGN IN VIA TWITTER" + + expect(page).to have_link "Sign In/Up" + expect(page).to have_link "Via Twitter" + expect(page).to have_link "Via GitHub" + expect(page).to have_link "All about dev.to" + end + end +end