summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-07-01 23:35:36 -0700
committerStan Hu <stanhu@gmail.com>2019-07-02 00:23:35 -0700
commit7db32c40800d678208bb3cc9cdb4fa76e57cbb11 (patch)
tree4fa3ea9ca9a1cebc38534b35272316ec30110490
parent045ab84e0b54dd2b8c03d281a8d4f9c15ae26a6e (diff)
downloadgitlab-ce-7db32c40800d678208bb3cc9cdb4fa76e57cbb11.tar.gz
Use an uncached application setting for usage ping checks
The introduction of the in-memory cache for application settings had a side effect of making it harder to invalidate changes when the settings occur. We now bypass the cache because it's possible the admin enabled the usage ping, and we don't want to annoy the user again if they already set the value. To avoid causing significant load on the system, we add an extra check to ensure the user is an admin. and we don't want to annoy the user again if they already set the value. This is a bit of hack, but the alternative would be to put in a more complex cache invalidation step. Since this call only gets called in the uncommon situation where the user is an admin and the only user in the instance, this shouldn't cause too much load on the system.
-rw-r--r--app/models/user.rb11
-rw-r--r--spec/models/user_spec.rb2
2 files changed, 10 insertions, 3 deletions
diff --git a/app/models/user.rb b/app/models/user.rb
index 38cb4d1a6e8..26be197209a 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1460,7 +1460,7 @@ class User < ApplicationRecord
end
def requires_usage_stats_consent?
- !consented_usage_stats? && 7.days.ago > self.created_at && !has_current_license? && User.single_user?
+ self.admin? && 7.days.ago > self.created_at && !has_current_license? && User.single_user? && !consented_usage_stats?
end
# Avoid migrations only building user preference object when needed.
@@ -1495,7 +1495,14 @@ class User < ApplicationRecord
end
def consented_usage_stats?
- Gitlab::CurrentSettings.usage_stats_set_by_user_id == self.id
+ # Bypass the cache here because it's possible the admin enabled the
+ # usage ping, and we don't want to annoy the user again if they
+ # already set the value. This is a bit of hack, but the alternative
+ # would be to put in a more complex cache invalidation step. Since
+ # this call only gets called in the uncommon situation where the
+ # user is an admin and the only user in the instance, this shouldn't
+ # cause too much load on the system.
+ ApplicationSetting.current_without_cache&.usage_stats_set_by_user_id == self.id
end
# Added according to https://github.com/plataformatec/devise/blob/7df57d5081f9884849ca15e4fde179ef164a575f/README.md#activejob-integration
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index b098fe3c9f4..11401b116d3 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -3354,7 +3354,7 @@ describe User do
end
describe '#requires_usage_stats_consent?' do
- let(:user) { create(:user, created_at: 8.days.ago) }
+ let(:user) { create(:user, :admin, created_at: 8.days.ago) }
before do
allow(user).to receive(:has_current_license?).and_return false