Skip to content
代码片段 群组 项目
未验证 提交 068f3bf8 编辑于 作者: Doug Stull's avatar Doug Stull
浏览文件

Move away from super.tap usage in user assignment creation

- currently we are relying on a cached check if the user
  is already assigned to be cached before the work is
  done to assign the user. Then after that user is added
  we have a check to see that the user isn't already assigned
  based on the cache only to retain that knowledge.
  It would be better to merely call this on success if
  the user is actually assigned during the service execute,
  which is what we are trying to measure here.
  Changing this could also better insulate us from future
  regressions as it is more declaritive about when and
  why the work happens.

- see https://gitlab.com/gitlab-org/gitlab/-/issues/508955
上级 c34d3cd8
No related branches found
No related tags found
无相关合并请求
...@@ -38,9 +38,11 @@ def execute ...@@ -38,9 +38,11 @@ def execute
Rails.cache.delete(user.duo_pro_cache_key_formatted) Rails.cache.delete(user.duo_pro_cache_key_formatted)
log_event('User AddOn assignment created') log_event('User AddOn assignment created')
ServiceResponse.success
end end
after_success_hook
ServiceResponse.success
rescue NoSeatsAvailableError => error rescue NoSeatsAvailableError => error
Gitlab::ErrorTracking.log_exception( Gitlab::ErrorTracking.log_exception(
error, base_log_params.merge({ message: 'User AddOn assignment creation failed' }) error, base_log_params.merge({ message: 'User AddOn assignment creation failed' })
...@@ -53,6 +55,10 @@ def execute ...@@ -53,6 +55,10 @@ def execute
attr_reader :add_on_purchase, :user attr_reader :add_on_purchase, :user
def after_success_hook
# overridden in inheriting classes.
end
def validate def validate
return ERROR_NO_SEATS_AVAILABLE unless seats_available? return ERROR_NO_SEATS_AVAILABLE unless seats_available?
......
...@@ -4,16 +4,15 @@ module GitlabSubscriptions ...@@ -4,16 +4,15 @@ module GitlabSubscriptions
module UserAddOnAssignments module UserAddOnAssignments
module Saas module Saas
class CreateService < ::GitlabSubscriptions::UserAddOnAssignments::Saas::CreateWithoutNotificationService class CreateService < ::GitlabSubscriptions::UserAddOnAssignments::Saas::CreateWithoutNotificationService
def execute extend ::Gitlab::Utils::Override
super.tap do |response|
create_iterable_trigger if should_trigger_duo_iterable? response
end
end
private private
def should_trigger_duo_iterable?(response) override :after_success_hook
response.success? && duo_pro_or_enterprise? && !user_already_assigned? def after_success_hook
super
create_iterable_trigger if duo_pro_or_enterprise?
end end
def duo_pro_or_enterprise? def duo_pro_or_enterprise?
......
...@@ -8,8 +8,6 @@ class CreateWithoutNotificationService < ::GitlabSubscriptions::UserAddOnAssignm ...@@ -8,8 +8,6 @@ class CreateWithoutNotificationService < ::GitlabSubscriptions::UserAddOnAssignm
private private
attr_reader :add_on_purchase, :user
def eligible_for_gitlab_duo_pro_seat? def eligible_for_gitlab_duo_pro_seat?
namespace.eligible_for_gitlab_duo_pro_seat?(user) namespace.eligible_for_gitlab_duo_pro_seat?(user)
end end
......
...@@ -5,16 +5,16 @@ module UserAddOnAssignments ...@@ -5,16 +5,16 @@ module UserAddOnAssignments
module SelfManaged module SelfManaged
class CreateService < ::GitlabSubscriptions::UserAddOnAssignments::BaseCreateService class CreateService < ::GitlabSubscriptions::UserAddOnAssignments::BaseCreateService
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
extend ::Gitlab::Utils::Override
def execute
super.tap do |response|
send_duo_seat_assignment_email if should_send_duo_seat_assignment_email? response
end
end
private private
attr_reader :add_on_purchase, :user override :after_success_hook
def after_success_hook
super
send_duo_seat_assignment_email if Feature.enabled?(:duo_seat_assignment_email_for_sm, :instance)
end
def eligible_for_gitlab_duo_pro_seat? def eligible_for_gitlab_duo_pro_seat?
user.eligible_for_self_managed_gitlab_duo_pro? user.eligible_for_self_managed_gitlab_duo_pro?
...@@ -25,12 +25,6 @@ def send_duo_seat_assignment_email ...@@ -25,12 +25,6 @@ def send_duo_seat_assignment_email
DuoSeatAssignmentMailer.duo_pro_email(user).deliver_later if add_on_purchase.add_on.code_suggestions? DuoSeatAssignmentMailer.duo_pro_email(user).deliver_later if add_on_purchase.add_on.code_suggestions?
DuoSeatAssignmentMailer.duo_enterprise_email(user).deliver_later if add_on_purchase.add_on.duo_enterprise? DuoSeatAssignmentMailer.duo_enterprise_email(user).deliver_later if add_on_purchase.add_on.duo_enterprise?
end end
def should_send_duo_seat_assignment_email?(response)
Feature.enabled?(:duo_seat_assignment_email_for_sm, :instance) &&
!user_already_assigned? &&
response.success?
end
end end
end end
end end
......
0% 加载中 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册