Commit 3531ea09 authored by Toon Claes's avatar Toon Claes

Devise can assign trackable fields, but only allow writes once/hour

Not assigning the trackable fields seems to cause strange side-effects.
parent 6a915d6f
...@@ -40,14 +40,15 @@ class User < ActiveRecord::Base ...@@ -40,14 +40,15 @@ class User < ActiveRecord::Base
devise :lockable, :recoverable, :rememberable, :trackable, devise :lockable, :recoverable, :rememberable, :trackable,
:validatable, :omniauthable, :confirmable, :registerable :validatable, :omniauthable, :confirmable, :registerable
# Limit trackable fields to update at most once every hour # Override Devise::Models::Trackable#update_tracked_fields!
alias_method :devise_update_tracked_fields!, :update_tracked_fields! # to limit database writes to at most once every hour
def update_tracked_fields!(request) def update_tracked_fields!(request)
update_tracked_fields(request)
lease = Gitlab::ExclusiveLease.new("user_update_tracked_fields:#{id}", timeout: 1.hour.to_i) lease = Gitlab::ExclusiveLease.new("user_update_tracked_fields:#{id}", timeout: 1.hour.to_i)
return unless lease.try_obtain return unless lease.try_obtain
devise_update_tracked_fields!(request) save(validate: false)
end end
attr_accessor :force_random_password attr_accessor :force_random_password
......
...@@ -68,7 +68,7 @@ feature 'Groups > Members > Sorting', feature: true do ...@@ -68,7 +68,7 @@ feature 'Groups > Members > Sorting', feature: true do
expect(page).to have_css('.member-sort-dropdown .dropdown-toggle-text', text: 'Name, descending') expect(page).to have_css('.member-sort-dropdown .dropdown-toggle-text', text: 'Name, descending')
end end
scenario 'sorts by recent sign in' do scenario 'sorts by recent sign in', :redis do
visit_members_list(sort: :recent_sign_in) visit_members_list(sort: :recent_sign_in)
expect(first_member).to include(owner.name) expect(first_member).to include(owner.name)
...@@ -76,7 +76,7 @@ feature 'Groups > Members > Sorting', feature: true do ...@@ -76,7 +76,7 @@ feature 'Groups > Members > Sorting', feature: true do
expect(page).to have_css('.member-sort-dropdown .dropdown-toggle-text', text: 'Recent sign in') expect(page).to have_css('.member-sort-dropdown .dropdown-toggle-text', text: 'Recent sign in')
end end
scenario 'sorts by oldest sign in' do scenario 'sorts by oldest sign in', :redis do
visit_members_list(sort: :oldest_sign_in) visit_members_list(sort: :oldest_sign_in)
expect(first_member).to include(developer.name) expect(first_member).to include(developer.name)
......
...@@ -359,7 +359,17 @@ describe User, models: true do ...@@ -359,7 +359,17 @@ describe User, models: true do
expect do expect do
user.update_tracked_fields!(request) user.update_tracked_fields!(request)
end.not_to change { user.current_sign_in_at } end.not_to change { user.reload.current_sign_in_at }
end
it 'writes trackable attributes for a different user' do
user2 = create(:user)
user.update_tracked_fields!(request)
expect do
user2.update_tracked_fields!(request)
end.to change { user2.reload.current_sign_in_at }
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment