Skip to content

Remove unused stored_location analytics parameter #10105

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.

Sign up for GitHub

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

Merged
aduth merged 2 commits into from
Feb 20, 2024
Merged

Remove unused stored_location analytics parameter #10105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fda66c6
Remove unused stored_location analytics parameter
aduth Feb 16, 2024
1ad6a02
Remove lingering references to stored_location
aduth Feb 20, 2024
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
6 changes: 1 addition & 5 deletions app/controllers/users/sessions_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@ def new
@issuer_forced_reauthentication = issuer_forced_reauthentication?(
issuer: decorated_sp_session.sp_issuer,
)
analytics.sign_in_page_visit(
flash: flash[:alert],
stored_location: session['user_return_to'],
)
analytics.sign_in_page_visit(flash: flash[:alert])
super
end

Expand Down Expand Up @@ -134,7 +131,6 @@ def track_authentication_attempt(email)
user_id: user.uuid,
user_locked_out: user_locked_out?(user),
bad_password_count: session[:bad_password_count].to_i,
stored_location: session['user_return_to'],
sp_request_url_present: sp_session[:request_url].present?,
remember_device: remember_device_cookie.present?,
)
Expand Down
13 changes: 2 additions & 11 deletions app/services/analytics_events.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,6 @@ def edit_password_visit
# @param [String] user_id
# @param [Boolean] user_locked_out if the user is currently locked out of their second factor
# @param [String] bad_password_count represents number of prior login failures
# @param [String] stored_location the URL to return to after signing in
# @param [Boolean] sp_request_url_present if was an SP request URL in the session
# @param [Boolean] remember_device if the remember device cookie was present
# Tracks authentication attempts at the email/password screen
Expand All @@ -362,7 +361,6 @@ def email_and_password_auth(
user_id:,
user_locked_out:,
bad_password_count:,
stored_location:,
sp_request_url_present:,
remember_device:,
**extra
Expand All @@ -373,7 +371,6 @@ def email_and_password_auth(
user_id: user_id,
user_locked_out: user_locked_out,
bad_password_count: bad_password_count,
stored_location: stored_location,
sp_request_url_present: sp_request_url_present,
remember_device: remember_device,
**extra,
Expand Down Expand Up @@ -4365,15 +4362,9 @@ def session_total_duration_timeout
end

# @param [String] flash
# @param [String] stored_location
# tracks when a user visits the sign in page
def sign_in_page_visit(flash:, stored_location:, **extra)
track_event(
'Sign in page visited',
flash: flash,
stored_location: stored_location,
**extra,
)
def sign_in_page_visit(flash:, **extra)
track_event('Sign in page visited', flash:, **extra)
end

# @param [Boolean] success
Expand Down
12 changes: 0 additions & 12 deletions spec/controllers/users/sessions_controller_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@

it 'tracks the successful authentication for existing user' do
user = create(:user, :fully_registered)
subject.session['user_return_to'] = mock_valid_site

stub_analytics
stub_attempts_tracker
Expand All @@ -52,7 +51,6 @@
user_id: user.uuid,
user_locked_out: false,
bad_password_count: 0,
stored_location: mock_valid_site,
sp_request_url_present: false,
remember_device: false,
}
Expand All @@ -76,7 +74,6 @@
user_id: user.uuid,
user_locked_out: false,
bad_password_count: 1,
stored_location: nil,
sp_request_url_present: false,
remember_device: false,
}
Expand All @@ -96,7 +93,6 @@
user_id: 'anonymous-uuid',
user_locked_out: false,
bad_password_count: 1,
stored_location: nil,
sp_request_url_present: false,
remember_device: false,
}
Expand Down Expand Up @@ -135,7 +131,6 @@
user_id: user.uuid,
user_locked_out: true,
bad_password_count: 0,
stored_location: nil,
sp_request_url_present: false,
remember_device: false,
}
Expand All @@ -159,7 +154,6 @@
user_id: user.uuid,
user_locked_out: false,
bad_password_count: 2,
stored_location: nil,
sp_request_url_present: false,
remember_device: false,
}
Expand All @@ -178,7 +172,6 @@
user_id: 'anonymous-uuid',
user_locked_out: false,
bad_password_count: 1,
stored_location: nil,
sp_request_url_present: true,
remember_device: false,
}
Expand Down Expand Up @@ -253,7 +246,6 @@
user_id: user.uuid,
user_locked_out: false,
bad_password_count: 0,
stored_location: nil,
sp_request_url_present: false,
remember_device: false,
}
Expand Down Expand Up @@ -380,7 +372,6 @@
user_id: user.uuid,
user_locked_out: false,
bad_password_count: 0,
stored_location: nil,
sp_request_url_present: false,
remember_device: true,
}
Expand All @@ -406,7 +397,6 @@
user_id: user.uuid,
user_locked_out: false,
bad_password_count: 0,
stored_location: nil,
sp_request_url_present: false,
remember_device: true,
}
Expand Down Expand Up @@ -513,12 +503,10 @@
it 'tracks page visit, any alert flashes, and the Devise stored location' do
stub_analytics
allow(controller).to receive(:flash).and_return(alert: 'hello')
subject.session['user_return_to'] = mock_valid_site

expect(@analytics).to receive(:track_event).with(
'Sign in page visited',
flash: 'hello',
stored_location: mock_valid_site,
)

get :new
Expand Down