Skip to content

Commit

Permalink
finishes:
Browse files Browse the repository at this point in the history
185979216 UX for reset password link is confusing for patron who has never logged in before
185979213 "Act on behalf of" ribbon disappears when buying tickets
186436007 change superadmin-cannot-be-merged message wording
  • Loading branch information
armandofox committed Dec 29, 2023
1 parent 552af09 commit 5dc4bc4
Show file tree
Hide file tree
Showing 12 changed files with 62 additions and 41 deletions.
17 changes: 8 additions & 9 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,6 @@ def redirect_after_login(customer)
customer_path(customer))
end

def redirect_after_reset_token(customer)
redirect_to ((r = session.delete(:return_to)) ?
r.merge(:customer_id => customer) :
change_password_for_customer_path(customer))
flash[:alert] = I18n.t('login.change_password_now')
end

def find_cart
# if there is an existing order that HAS NOT BEEN finalized, return it....
if (o = Order.find_by(:id => session[:cart])) && o.sold_on.nil?
Expand Down Expand Up @@ -193,6 +186,11 @@ def create_session(u = nil, action = '')
# button. Uncomment if you understand the tradeoffs.
# reset_session
self.current_user = @user
# 185979216: explicitly update last_login so that if this customer has never logged
# in before, this counts as a 'Login' action and they will now see the action tabs.
# This update used to occur in SessionsController#create, but creating a session
# can also happen as the result of resetting a password.
@user.update_attribute(:last_login, Time.current)
session[:guest_checkout] = false
# 'remember me' checked?
new_cookie_flag = (params[:remember_me] == "1")
Expand All @@ -202,9 +200,10 @@ def create_session(u = nil, action = '')
reset_shopping unless @gOrderInProgress
session[:new_session] = true
if action == 'reset_token'
return redirect_after_reset_token(@user)
redirect_to change_password_for_customer_path(@user), :alert => I18n.t('login.change_password_now')
else
redirect_after_login(@user)
end
redirect_after_login(@user)
end
end

Expand Down
12 changes: 5 additions & 7 deletions app/controllers/customers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,15 +151,13 @@ def forgot_password

def reset_token
token = params[:token]
@customer = Customer.where(token: token).first
if @customer.nil? ||
@customer.token_created_at.nil? ||
(Time.zone.now).utc >= (@customer.token_created_at + 10.minutes).utc
return redirect_to(login_path, :alert => "The reset password link has expired")
@customer = Customer.find_by(token: token)
if @customer.try(:valid_reset_token?)
@customer.token_created_at = 10.minutes.ago
create_session(@customer, 'reset_token')
else
@customer.token_created_at = (Time.zone.now).utc - 10.minutes
redirect_to login_path, :alert => "The reset password link is invalid or has expired"
end
create_session(@customer, 'reset_token')
end

# Regular user creating new account
Expand Down
1 change: 0 additions & 1 deletion app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ def create
note_failed_signin(@email, u)
redirect_to new_session_path, :email => @email, :remember_me => @remember_me
else
u.update_attribute(:last_login,Time.current)
session.delete(:admin_disabled) # in case admin signin
end
u
Expand Down
11 changes: 8 additions & 3 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,15 @@ def link_icon
end

def display_customer_actions?
# when to display the 'staff ribbon' showing customer being acted on behalf of?
allowed_actions = {
:customers => [:show, :edit, :change_password_for, :change_secret_question],
:vouchers => [:index],
:orders => [:index],
:store => [:index, :subscribe, :donate, :donate_to_fund, :shipping_address, :checkout]
}
! @customer.try(:new_record?) &&
((controller.controller_name == 'customers' && action_name !~ /^index|list_duplicate/) ||
(controller.controller_name == 'vouchers' && action_name == 'index') ||
(controller.controller_name == 'orders' && action_name == 'index'))
(allowed_actions[controller.controller_name.to_sym] || {}).include?(action_name.to_sym)
end

def display_order_in_progress?
Expand Down
5 changes: 5 additions & 0 deletions app/models/customer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,11 @@ def login_message
msg
end

def valid_reset_token?
token_created_at &&
token_created_at >= 10.minutes.ago
end

def has_ever_logged_in?
last_login > Time.zone.parse('2007-04-07') # sentinel date should match what's in schema.rb
end
Expand Down
2 changes: 1 addition & 1 deletion app/views/components/_main_tabs.html.haml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
%ul.nav.nav-tabs.a1-nav
%ul.nav.nav-tabs.a1-nav#main-tabs
%li.nav-item#t_store_index= link_to 'Buy Tickets', store_path(@customer), :class => 'nav-link'
%li.nav-item#t_store_donate= link_to 'Donate', quick_donate_path(:customer_id => @customer), :class => 'nav-link'
%li.nav-item#t_store_subscribe= link_to 'Subscribe', store_subscribe_path(@customer), :class => 'nav-link'
Expand Down
2 changes: 1 addition & 1 deletion app/views/layouts/application.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
= render :partial => 'components/season_tabs'
#staff-nav
= render :partial => 'components/quick_search' if current_user && @gAdminDisplay
= render :partial => 'components/staff_buttons' if display_customer_actions? && @gAdminDisplay
= render :partial => 'components/acting_on_behalf_of' if display_customer_actions? && @gAdminDisplay
#content.a1-plain.p-3
- if controller.controller_name =~ /checkins|walkup_sales/
= render :partial => 'checkins/change_showdate'
Expand Down
4 changes: 2 additions & 2 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,8 @@ en:

errors:
merge:
admin: Special customers and super admins cannot be merged
special: Merges disallowed into all special customers except Anonymous Customer
admin: Customers with the role of Admin cannot be merged with other customers.
special: The customer Boxoffice Daemon cannot be merged with other customers.

confirm_delete: >
Selected customer(s) will be PERMANENTLY deleted, and their transactions will be linked
Expand Down
2 changes: 1 addition & 1 deletion features/login/change_password.feature
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Scenario: supply new valid password
When I fill in "New Password" with "syzygy"
And I fill in "Confirm New Password" with "syzygy"
And I press "Save Changes"
Then I should be able to login with username "[email protected]" and password "syzygy"
And I should be able to login with username "[email protected]" and password "syzygy"

Scenario: confirmation mismatch

Expand Down
13 changes: 10 additions & 3 deletions features/login/forgot_password.feature
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,16 @@ Scenario: magic link expires after 10 minutes
And customer "[email protected]" clicks on "http://www.example.com/customers/reset_token?token=test_token"
Then I should be on the login page
But customer "John Doe" should not be logged in




Scenario: user created by admin and who has never logged in resets password

Given customer with email "[email protected]" activates a valid forgot-password link
Then I should be on the change password page
When I fill in "New Password" with "syzygy"
And I fill in "Confirm New Password" with "syzygy"
And I press "Save Changes"
Then I should see "My Tickets" within "#main-tabs"




Expand Down
34 changes: 21 additions & 13 deletions features/step_definitions/login_steps.rb
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
module CustomerLoginHelper
def verify_successful_login
# if someone is currently logged in, log them out first
visit login_path
return if page.has_content?("Log Out #{@customer.full_name}") # correct person already logged in
visit logout_path if page.has_content?("Log Out") # logout whoever's logged in
visit login_path
fill_in 'email', :with => @customer.email
fill_in 'password', :with => @password
click_button 'Login'
expect(page).to have_content("Log Out #{@customer.full_name}")
expect(page).to have_css('.admin') if @is_admin
module ScenarioHelpers
module CustomerLoginHelper
def verify_successful_login
# if someone is currently logged in, log them out first
visit login_path
return if page.has_content?("Log Out #{@customer.full_name}") # correct person already logged in
visit logout_path if page.has_content?("Log Out") # logout whoever's logged in
visit login_path
fill_in 'email', :with => @customer.email
fill_in 'password', :with => @password
click_button 'Login'
expect(page).to have_content("Log Out #{@customer.full_name}")
expect(page).to have_css('.admin') if @is_admin
end
end
end
World(CustomerLoginHelper)
World(ScenarioHelpers::CustomerLoginHelper)

Given /^I am not logged in$/ do
visit logout_path
Expand Down Expand Up @@ -61,6 +63,12 @@ def verify_successful_login
end
end

Given /customer with email "(.*)" activates a valid forgot-password link/ do |email|
@customer = Customer.find_by!(:email => email)
@customer.update_attributes!(:token => 'TEST', :token_created_at => Time.current)
visit reset_token_customers_path(@customer, :token => 'TEST')
end

When /I ask to send a password reset email to "(.*)"/ do |email|
visit forgot_password_customers_path
fill_in 'email', :with => email
Expand Down

0 comments on commit 5dc4bc4

Please sign in to comment.