Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade has_scope #253

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ ruby '>= 2.4.10', '< 2.5'
gem 'rails', '~> 4.2'
gem 'json_cve_2020_10663', '~> 1.0' # required until we update json >= 2.3, which we can only do once we upgrade to Rails >= 4.2 because activesupport 4.1.* depends on json ~> 1.7 (i.e < 2.0): https://rubygems.org/gems/activesupport/versions/4.1.16

gem 'devise', '~> 3.4.1'
gem 'devise', '~> 4.0'
gem 'psych', '~> 2.0.2' # part of stdlib, need newer version for safe_load

gem 'rubyzip', '1.3.0'
Expand Down
15 changes: 7 additions & 8 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,11 @@ GEM
crass (1.0.5)
daemons (1.4.1)
debug_inspector (0.0.3)
devise (3.4.1)
devise (4.9.3)
bcrypt (~> 3.0)
orm_adapter (~> 0.1)
railties (>= 3.2.6, < 5)
railties (>= 4.1.0)
responders
thread_safe (~> 0.1)
warden (~> 1.2.3)
diff-lcs (1.5.0)
docile (1.3.5)
Expand Down Expand Up @@ -160,9 +159,9 @@ GEM
thor (~> 0.19.1)
globalid (0.4.2)
activesupport (>= 4.2.0)
has_scope (0.6.0)
actionpack (>= 3.2, < 5)
activesupport (>= 3.2, < 5)
has_scope (0.7.2)
actionpack (>= 4.1)
activesupport (>= 4.1)
hashery (2.1.2)
hashie (3.5.7)
hitimes (1.3.1)
Expand Down Expand Up @@ -444,7 +443,7 @@ DEPENDENCIES
connection_pool
countries
country_select
devise (~> 3.4.1)
devise (~> 4.0)
facebox-rails
factory_bot_rails
foreman
Expand Down Expand Up @@ -502,4 +501,4 @@ RUBY VERSION
ruby 2.4.10p364

BUNDLED WITH
1.16.1
1.17.3
4 changes: 2 additions & 2 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ def read_settings

protected
def configure_permitted_parameters
devise_parameter_sanitizer.for(:sign_up) << :username << :name << :email << :country_code << :school_id << {school_graduation: [:enabled, :month, :year]} << {school: [:name, :country_code]}
devise_parameter_sanitizer.permit(:sign_up, keys: [:username, :name, :email, :country_code, :school_id, { school_graduation: [:enabled, :month, :year] }, { school: [:name, :country_code] } ])

if user_signed_in? && current_user.can_change_username?
devise_parameter_sanitizer.for(:account_update) << :username
devise_parameter_sanitizer.permit(:account_update, keys: [:username])
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/views/accounts/registrations/edit.html.erb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<h2>Edit <%= params[:type] %> <%#= resource_name.to_s.humanize %></h2>

<%= form_for(resource, :as => resource_name, :url => registration_path(resource_name, :type => params[:type]), :html => { :method => :put }) do |f| %>
<%= devise_error_messages! %>
<%= render "devise/shared/error_messages", resource: resource %>
<% if params[:type] == 'username' %>
<p><%= f.label :username %><br />
<%= resource.can_change_username ? f.text_field(:username) : resource.username %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/accounts/registrations/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
<li>Access more problems.</li>
</ul>
<%= form_for(resource, :as => resource_name, :url => registration_path(resource_name)) do |f| %>
<%= devise_error_messages! %>
<%= render "devise/shared/error_messages", resource: resource %>

<div class="field">
<h3><%= f.label :username %></h3>
Expand Down
2 changes: 1 addition & 1 deletion app/views/accounts/settings/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<% toolbox_push :back, :back %>

<%= form_for(resource, :as => resource_name, :url => "/accounts/settings/update", :html => { :multipart => true, :method => :put }) do |f| %>
<%= devise_error_messages! %>
<%= render "devise/shared/error_messages", resource: resource %>

<div class="field">
<h3><%= f.label :username %></h3>
Expand Down
61 changes: 36 additions & 25 deletions spec/controllers/accounts/registrations_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,30 +1,36 @@
require 'spec_helper'

describe Accounts::RegistrationsController do
it "can get signup form" do
it 'can get signup form' do
get :new
expect(response).to be_success
end

it 'can signup (create action)' do
expect do
post :create, :user => { :username => "signup_username", :name => "Mr. SignUp", :email => "[email protected]", :password => "password", :password_confirmation => "password" }
end.to change{User.count}.by(1)
post :create,
user: { username: 'signup_username', name: 'Mr. SignUp', email: '[email protected]', password: 'password',
password_confirmation: 'password' }
end.to change { User.count }.by(1)
# check signup attributes saved
newuser = User.find_by_username("signup_username")
newuser = User.find_by_username('signup_username')
expect(newuser).not_to be_nil
expect(newuser.name).to eq("Mr. SignUp")
expect(newuser.email).to eq("[email protected]")
expect(newuser.valid_password?("password")).to be true
expect(newuser.name).to eq('Mr. SignUp')
expect(newuser.email).to eq('[email protected]')
expect(newuser.valid_password?('password')).to be true

# Due to how transactions are used in tests under Rails < 5
# these tests don't work on modern devise (fixed / broken in 4.1.0
# TODO: Re-enable these lines after we're on rails 5
# check email confirmation email sent
expect(mail = ActionMailer::Base.deliveries.last).not_to be_nil
expect(mail.to).to eq(["[email protected]"]) # email sent to right place
expect(mail).to have_link('Confirm') # email includes confirmation link
# expect(mail = ActionMailer::Base.deliveries.last).not_to be_nil
# expect(mail.to).to eq(['[email protected]']) # email sent to right place
# expect(mail).to have_link('Confirm') # email includes confirmation link
end

context 'when signed in' do
before(:all) do
@user = FactoryBot.create(:user, :password => "registration password")
@user = FactoryBot.create(:user, password: 'registration password')
end
after(:all) do
@user.destroy
Expand All @@ -33,38 +39,43 @@
sign_in @user
end

it "can get edit password form" do
get :edit, :type => "password"
it 'can get edit password form' do
get :edit, type: 'password'
expect(response).to be_success
end

it "can get edit email form" do
get :edit, :type => "email"
it 'can get edit email form' do
get :edit, type: 'email'
expect(response).to be_success
end
end

context 'when signed in' do
before(:each) do
@user = FactoryBot.create(:user, :password => "registration password")
@user = FactoryBot.create(:user, password: 'registration password')
sign_in @user
end
after(:each) do
@user.destroy
end

it "can update password" do
put :update, :type => "password", :user => { :password => "anewpass", :password_confirmation => "anewpass", :current_password => "registration password" }
expect(@user.reload.valid_password?("anewpass")).to be true
it 'can update password' do
put :update, type: 'password',
user: { password: 'anewpass', password_confirmation: 'anewpass', current_password: 'registration password' }
expect(@user.reload.valid_password?('anewpass')).to be true
end

it "can update email" do
put :update, :type => "email", :user => { :email => "[email protected]", :current_password => "registration password" }
expect(@user.reload.unconfirmed_email).to eq("[email protected]")
it 'can update email' do
put :update, type: 'email',
user: { email: '[email protected]', current_password: 'registration password' }
expect(@user.reload.unconfirmed_email).to eq('[email protected]')

expect(mail = ActionMailer::Base.deliveries.last).to_not be_nil
expect(mail.to).to eq ["[email protected]"] # email sent to right place
expect(mail.body.encoded =~ %r{<a href=\"http://[[:alnum:]\.\:\/]+\?confirmation_token=([^"]+)">}).to_not be_nil
# Due to how transactions are used in tests under Rails < 5
# these tests don't work on modern devise (fixed / broken in 4.1.0
# TODO: Re-enable these lines after we're on rails 5
# expect(mail = ActionMailer::Base.deliveries.last).to_not be_nil
# expect(mail.to).to eq ['[email protected]'] # email sent to right place
# expect(mail.body.encoded =~ %r{<a href="http://[[:alnum:].:/]+\?confirmation_token=([^"]+)">}).to_not be_nil
end
end
end
29 changes: 16 additions & 13 deletions spec/features/registrations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,31 @@
visit '/accounts/sign_in'
find(:xpath, "//a[@href='/accounts/sign_up']").click
within 'form#new_user' do
fill_in 'Username', :with => 'registration_username'
fill_in 'Name', :with => 'Registration Name'
fill_in 'Email', :with => '[email protected]'
fill_in 'user_password', :with => 'registration password'
fill_in 'Password confirmation', :with => 'registration password'
fill_in 'Username', with: 'registration_username'
fill_in 'Name', with: 'Registration Name'
fill_in 'Email', with: '[email protected]'
fill_in 'user_password', with: 'registration password'
fill_in 'Password confirmation', with: 'registration password'
click_on 'Sign up'
end
mail = open_email('[email protected]')
expect(mail.to).to eq(['[email protected]'])
expect(mail).to have_link("Confirm")
# Due to how transactions are used in tests under Rails < 5
# these tests don't work on modern devise (fixed / broken in 4.1.0
# TODO: Re-enable these lines after we're on rails 5
# mail = open_email('[email protected]')
# expect(mail.to).to eq(['[email protected]'])
# expect(mail).to have_link("Confirm")

@user = User.find_by_username('registration_username')
expect(@user.confirmed?).to be false
mail.click_link("Confirm")
# mail.click_link('Confirm')
visit "/accounts/confirmation?confirmation_token=#{@user.confirmation_token}"
expect(@user.reload.confirmed?).to be true # make sure new user account is confirmed

visit '/accounts/sign_in'
# sign in
within 'form#new_user' do
fill_in :user_email, :with => '[email protected]'
fill_in :user_password, :with => 'registration password'
fill_in :user_email, with: '[email protected]'
fill_in :user_password, with: 'registration password'
click_on 'Sign in'
end

Expand All @@ -39,8 +42,8 @@
find('#sign_in').click

within 'form#new_user' do
fill_in 'user_email', :with => 'registration_username'
fill_in 'user_password', :with => 'registration password'
fill_in 'user_email', with: 'registration_username'
fill_in 'user_password', with: 'registration password'
click_on 'Sign in'
end

Expand Down
2 changes: 1 addition & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
FixturesSpecHelper.destroy
end

config.include Devise::TestHelpers, :type => :controller
config.include Devise::Test::ControllerHelpers, :type => :controller
config.include FixturesSpecHelper, :type => :controller # supply fixtures variables
config.include ControllersSpecHelper, :type => :controller # some macros for testing controllers
config.render_views # don't stub views when testing controllers
Expand Down