Skip to content
Snippets Groups Projects
Commit 5a5a0ad9 authored by Doug Stull's avatar Doug Stull :two: Committed by Mayra Cabrera
Browse files

Persist group invite banner dismissal in database

parent 6247da83
No related branches found
No related tags found
1 merge request!68785Persist group invite banner dismissal in database
Showing
with 235 additions and 38 deletions
...@@ -315,7 +315,6 @@ Performance/MethodObjectAsBlock: ...@@ -315,7 +315,6 @@ Performance/MethodObjectAsBlock:
# Configuration parameters: AutoCorrect. # Configuration parameters: AutoCorrect.
Performance/StringInclude: Performance/StringInclude:
Exclude: Exclude:
- 'app/helpers/groups_helper.rb'
- 'app/models/snippet_repository.rb' - 'app/models/snippet_repository.rb'
- 'config/initializers/macos.rb' - 'config/initializers/macos.rb'
- 'config/spring.rb' - 'config/spring.rb'
......
<script> <script>
import { GlBanner } from '@gitlab/ui'; import { GlBanner } from '@gitlab/ui';
import eventHub from '~/invite_members/event_hub'; import eventHub from '~/invite_members/event_hub';
import { parseBoolean, setCookie, getCookie } from '~/lib/utils/common_utils'; import axios from '~/lib/utils/axios_utils';
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import Tracking from '~/tracking'; import Tracking from '~/tracking';
...@@ -12,10 +12,10 @@ export default { ...@@ -12,10 +12,10 @@ export default {
GlBanner, GlBanner,
}, },
mixins: [trackingMixin], mixins: [trackingMixin],
inject: ['svgPath', 'isDismissedKey', 'trackLabel'], inject: ['svgPath', 'trackLabel', 'calloutsPath', 'calloutsFeatureId', 'groupId'],
data() { data() {
return { return {
isDismissed: parseBoolean(getCookie(this.isDismissedKey)), isDismissed: false,
tracking: { tracking: {
label: this.trackLabel, label: this.trackLabel,
}, },
...@@ -26,7 +26,16 @@ export default { ...@@ -26,7 +26,16 @@ export default {
}, },
methods: { methods: {
handleClose() { handleClose() {
setCookie(this.isDismissedKey, true); axios
.post(this.calloutsPath, {
feature_name: this.calloutsFeatureId,
group_id: this.groupId,
})
.catch((e) => {
// eslint-disable-next-line @gitlab/require-i18n-strings, no-console
console.error('Failed to dismiss banner.', e);
});
this.isDismissed = true; this.isDismissed = true;
this.track(this.$options.dismissEvent); this.track(this.$options.dismissEvent);
}, },
...@@ -61,6 +70,7 @@ export default { ...@@ -61,6 +70,7 @@ export default {
<gl-banner <gl-banner
v-if="!isDismissed" v-if="!isDismissed"
ref="banner" ref="banner"
data-testid="invite-members-banner"
:title="$options.i18n.title" :title="$options.i18n.title"
:button-text="$options.i18n.button_text" :button-text="$options.i18n.button_text"
:svg-path="svgPath" :svg-path="svgPath"
......
...@@ -8,15 +8,24 @@ export default function initInviteMembersBanner() { ...@@ -8,15 +8,24 @@ export default function initInviteMembersBanner() {
return false; return false;
} }
const { svgPath, inviteMembersPath, isDismissedKey, trackLabel } = el.dataset; const {
svgPath,
inviteMembersPath,
trackLabel,
calloutsPath,
calloutsFeatureId,
groupId,
} = el.dataset;
return new Vue({ return new Vue({
el, el,
provide: { provide: {
svgPath, svgPath,
inviteMembersPath, inviteMembersPath,
isDismissedKey,
trackLabel, trackLabel,
calloutsPath,
calloutsFeatureId,
groupId,
}, },
render: (createElement) => createElement(InviteMembersBanner), render: (createElement) => createElement(InviteMembersBanner),
}); });
......
...@@ -4,10 +4,6 @@ class UserCalloutsController < ApplicationController ...@@ -4,10 +4,6 @@ class UserCalloutsController < ApplicationController
feature_category :navigation feature_category :navigation
def create def create
callout = Users::DismissUserCalloutService.new(
container: nil, current_user: current_user, params: { feature_name: feature_name }
).execute
if callout.persisted? if callout.persisted?
respond_to do |format| respond_to do |format|
format.json { head :ok } format.json { head :ok }
...@@ -21,6 +17,12 @@ def create ...@@ -21,6 +17,12 @@ def create
private private
def callout
Users::DismissUserCalloutService.new(
container: nil, current_user: current_user, params: { feature_name: feature_name }
).execute
end
def feature_name def feature_name
params.require(:feature_name) params.require(:feature_name)
end end
......
# frozen_string_literal: true
module Users
class GroupCalloutsController < UserCalloutsController
private
def callout
Users::DismissGroupCalloutService.new(
container: nil, current_user: current_user, params: callout_params
).execute
end
def callout_params
params.permit(:group_id).merge(feature_name: feature_name)
end
end
end
...@@ -122,12 +122,6 @@ def parent_group_options(current_group) ...@@ -122,12 +122,6 @@ def parent_group_options(current_group)
groups.to_json groups.to_json
end end
def show_invite_banner?(group)
can?(current_user, :admin_group, group) &&
!just_created? &&
!multiple_members?(group)
end
def render_setting_to_allow_project_access_token_creation?(group) def render_setting_to_allow_project_access_token_creation?(group)
group.root? && current_user.can?(:admin_setting_to_allow_project_access_token_creation, group) group.root? && current_user.can?(:admin_setting_to_allow_project_access_token_creation, group)
end end
...@@ -142,14 +136,6 @@ def project_list_sort_by ...@@ -142,14 +136,6 @@ def project_list_sort_by
private private
def just_created?
flash[:notice] =~ /successfully created/
end
def multiple_members?(group)
group.member_count > 1 || group.members_with_parents.count > 1
end
def group_title_link(group, hidable: false, show_avatar: false, for_dropdown: false) def group_title_link(group, hidable: false, show_avatar: false, for_dropdown: false)
link_to(group_path(group), class: "group-path #{'breadcrumb-item-text' unless for_dropdown} js-breadcrumb-item-text #{'hidable' if hidable}") do link_to(group_path(group), class: "group-path #{'breadcrumb-item-text' unless for_dropdown} js-breadcrumb-item-text #{'hidable' if hidable}") do
icon = group_icon(group, class: "avatar-tile", width: 15, height: 15) if (group.try(:avatar_url) || show_avatar) && !Rails.env.test? icon = group_icon(group, class: "avatar-tile", width: 15, height: 15) if (group.try(:avatar_url) || show_avatar) && !Rails.env.test?
......
...@@ -9,6 +9,7 @@ module UserCalloutsHelper ...@@ -9,6 +9,7 @@ module UserCalloutsHelper
FEATURE_FLAGS_NEW_VERSION = 'feature_flags_new_version' FEATURE_FLAGS_NEW_VERSION = 'feature_flags_new_version'
REGISTRATION_ENABLED_CALLOUT = 'registration_enabled_callout' REGISTRATION_ENABLED_CALLOUT = 'registration_enabled_callout'
UNFINISHED_TAG_CLEANUP_CALLOUT = 'unfinished_tag_cleanup_callout' UNFINISHED_TAG_CLEANUP_CALLOUT = 'unfinished_tag_cleanup_callout'
INVITE_MEMBERS_BANNER = 'invite_members_banner'
def show_gke_cluster_integration_callout?(project) def show_gke_cluster_integration_callout?(project)
active_nav_link?(controller: sidebar_operations_paths) && active_nav_link?(controller: sidebar_operations_paths) &&
...@@ -56,6 +57,13 @@ def show_registration_enabled_user_callout? ...@@ -56,6 +57,13 @@ def show_registration_enabled_user_callout?
def dismiss_two_factor_auth_recovery_settings_check def dismiss_two_factor_auth_recovery_settings_check
end end
def show_invite_banner?(group)
Ability.allowed?(current_user, :admin_group, group) &&
!just_created? &&
!user_dismissed_for_group(INVITE_MEMBERS_BANNER, group) &&
!multiple_members?(group)
end
private private
def user_dismissed?(feature_name, ignore_dismissal_earlier_than = nil) def user_dismissed?(feature_name, ignore_dismissal_earlier_than = nil)
...@@ -63,6 +71,43 @@ def user_dismissed?(feature_name, ignore_dismissal_earlier_than = nil) ...@@ -63,6 +71,43 @@ def user_dismissed?(feature_name, ignore_dismissal_earlier_than = nil)
current_user.dismissed_callout?(feature_name: feature_name, ignore_dismissal_earlier_than: ignore_dismissal_earlier_than) current_user.dismissed_callout?(feature_name: feature_name, ignore_dismissal_earlier_than: ignore_dismissal_earlier_than)
end end
def user_dismissed_for_group(feature_name, group, ignore_dismissal_earlier_than = nil)
return false unless current_user
set_dismissed_from_cookie(group)
current_user.dismissed_callout_for_group?(feature_name: feature_name,
group: group,
ignore_dismissal_earlier_than: ignore_dismissal_earlier_than)
end
def set_dismissed_from_cookie(group)
# bridge function for one milestone to try and not annoy users who might have already dismissed this alert
# remove in 14.4 or 14.5? https://gitlab.com/gitlab-org/gitlab/-/issues/340322
dismissed_key = "invite_#{group.id}_#{current_user.id}"
if cookies[dismissed_key].present?
params = {
feature_name: INVITE_MEMBERS_BANNER,
group_id: group.id
}
Users::DismissGroupCalloutService.new(
container: nil, current_user: current_user, params: params
).execute
cookies.delete dismissed_key
end
end
def just_created?
flash[:notice]&.include?('successfully created')
end
def multiple_members?(group)
group.member_count > 1 || group.members_with_parents.count > 1
end
end end
UserCalloutsHelper.prepend_mod UserCalloutsHelper.prepend_mod
# frozen_string_literal: true
module Calloutable
extend ActiveSupport::Concern
included do
belongs_to :user
validates :user, presence: true
end
def dismissed_after?(dismissed_after)
dismissed_at > dismissed_after
end
end
...@@ -85,6 +85,8 @@ def self.sti_name ...@@ -85,6 +85,8 @@ def self.sti_name
# debian_distributions and associated component_files must be destroyed by ruby code in order to properly remove carrierwave uploads # debian_distributions and associated component_files must be destroyed by ruby code in order to properly remove carrierwave uploads
has_many :debian_distributions, class_name: 'Packages::Debian::GroupDistribution', dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :debian_distributions, class_name: 'Packages::Debian::GroupDistribution', dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :group_callouts, class_name: 'Users::GroupCallout', foreign_key: :group_id
delegate :prevent_sharing_groups_outside_hierarchy, :new_user_signups_cap, :setup_for_company, :jobs_to_be_done, to: :namespace_settings delegate :prevent_sharing_groups_outside_hierarchy, :new_user_signups_cap, :setup_for_company, :jobs_to_be_done, to: :namespace_settings
accepts_nested_attributes_for :variables, allow_destroy: true accepts_nested_attributes_for :variables, allow_destroy: true
......
...@@ -200,6 +200,7 @@ def update_tracked_fields!(request) ...@@ -200,6 +200,7 @@ def update_tracked_fields!(request)
has_many :custom_attributes, class_name: 'UserCustomAttribute' has_many :custom_attributes, class_name: 'UserCustomAttribute'
has_many :callouts, class_name: 'UserCallout' has_many :callouts, class_name: 'UserCallout'
has_many :group_callouts, class_name: 'Users::GroupCallout'
has_many :term_agreements has_many :term_agreements
belongs_to :accepted_term, class_name: 'ApplicationSetting::Term' belongs_to :accepted_term, class_name: 'ApplicationSetting::Term'
...@@ -1928,10 +1929,14 @@ def set_role_required! ...@@ -1928,10 +1929,14 @@ def set_role_required!
def dismissed_callout?(feature_name:, ignore_dismissal_earlier_than: nil) def dismissed_callout?(feature_name:, ignore_dismissal_earlier_than: nil)
callout = callouts_by_feature_name[feature_name] callout = callouts_by_feature_name[feature_name]
return false unless callout callout_dismissed?(callout, ignore_dismissal_earlier_than)
return callout.dismissed_after?(ignore_dismissal_earlier_than) if ignore_dismissal_earlier_than end
true def dismissed_callout_for_group?(feature_name:, group:, ignore_dismissal_earlier_than: nil)
source_feature_name = "#{feature_name}_#{group.id}"
callout = group_callouts_by_feature_name[source_feature_name]
callout_dismissed?(callout, ignore_dismissal_earlier_than)
end end
# Load the current highest access by looking directly at the user's memberships # Load the current highest access by looking directly at the user's memberships
...@@ -1955,6 +1960,11 @@ def find_or_initialize_callout(feature_name) ...@@ -1955,6 +1960,11 @@ def find_or_initialize_callout(feature_name)
callouts.find_or_initialize_by(feature_name: ::UserCallout.feature_names[feature_name]) callouts.find_or_initialize_by(feature_name: ::UserCallout.feature_names[feature_name])
end end
def find_or_initialize_group_callout(feature_name, group_id)
group_callouts
.find_or_initialize_by(feature_name: ::Users::GroupCallout.feature_names[feature_name], group_id: group_id)
end
def can_trigger_notifications? def can_trigger_notifications?
confirmed? && !blocked? && !ghost? confirmed? && !blocked? && !ghost?
end end
...@@ -2026,10 +2036,21 @@ def commit_email_verified ...@@ -2026,10 +2036,21 @@ def commit_email_verified
errors.add(:commit_email, _("must be an email you have verified")) unless verified_emails.include?(commit_email) errors.add(:commit_email, _("must be an email you have verified")) unless verified_emails.include?(commit_email)
end end
def callout_dismissed?(callout, ignore_dismissal_earlier_than)
return false unless callout
return callout.dismissed_after?(ignore_dismissal_earlier_than) if ignore_dismissal_earlier_than
true
end
def callouts_by_feature_name def callouts_by_feature_name
@callouts_by_feature_name ||= callouts.index_by(&:feature_name) @callouts_by_feature_name ||= callouts.index_by(&:feature_name)
end end
def group_callouts_by_feature_name
@group_callouts_by_feature_name ||= group_callouts.index_by(&:source_feature_name)
end
def authorized_groups_without_shared_membership def authorized_groups_without_shared_membership
Group.from_union([ Group.from_union([
groups.select(Namespace.arel_table[Arel.star]), groups.select(Namespace.arel_table[Arel.star]),
......
# frozen_string_literal: true # frozen_string_literal: true
class UserCallout < ApplicationRecord class UserCallout < ApplicationRecord
belongs_to :user include Calloutable
enum feature_name: { enum feature_name: {
gke_cluster_integration: 1, gke_cluster_integration: 1,
...@@ -39,13 +39,8 @@ class UserCallout < ApplicationRecord ...@@ -39,13 +39,8 @@ class UserCallout < ApplicationRecord
terraform_notification_dismissed: 38 terraform_notification_dismissed: 38
} }
validates :user, presence: true
validates :feature_name, validates :feature_name,
presence: true, presence: true,
uniqueness: { scope: :user_id }, uniqueness: { scope: :user_id },
inclusion: { in: UserCallout.feature_names.keys } inclusion: { in: UserCallout.feature_names.keys }
def dismissed_after?(dismissed_after)
dismissed_at > dismissed_after
end
end end
# frozen_string_literal: true
module Users
class GroupCallout < ApplicationRecord
include Calloutable
self.table_name = 'user_group_callouts'
belongs_to :group
enum feature_name: {
invite_members_banner: 1
}
validates :group, presence: true
validates :feature_name,
presence: true,
uniqueness: { scope: [:user_id, :group_id] },
inclusion: { in: GroupCallout.feature_names.keys }
def source_feature_name
"#{feature_name}_#{group_id}"
end
end
end
# frozen_string_literal: true
module Users
class DismissGroupCalloutService < DismissUserCalloutService
private
def callout
current_user.find_or_initialize_group_callout(params[:feature_name], params[:group_id])
end
end
end
...@@ -3,9 +3,15 @@ ...@@ -3,9 +3,15 @@
module Users module Users
class DismissUserCalloutService < BaseContainerService class DismissUserCalloutService < BaseContainerService
def execute def execute
current_user.find_or_initialize_callout(params[:feature_name]).tap do |callout| callout.tap do |record|
callout.update(dismissed_at: Time.current) if callout.valid? record.update(dismissed_at: Time.current) if record.valid?
end end
end end
private
def callout
current_user.find_or_initialize_callout(params[:feature_name])
end
end end
end end
...@@ -12,9 +12,11 @@ ...@@ -12,9 +12,11 @@
= content_for :group_invite_members_banner do = content_for :group_invite_members_banner do
.container-fluid.container-limited{ class: "gl-pb-2! gl-pt-6! #{@content_class}" } .container-fluid.container-limited{ class: "gl-pb-2! gl-pt-6! #{@content_class}" }
.js-group-invite-members-banner{ data: { svg_path: image_path('illustrations/merge_requests.svg'), .js-group-invite-members-banner{ data: { svg_path: image_path('illustrations/merge_requests.svg'),
is_dismissed_key: "invite_#{@group.id}_#{current_user.id}",
track_label: 'invite_members_banner', track_label: 'invite_members_banner',
invite_members_path: group_group_members_path(@group) } } invite_members_path: group_group_members_path(@group),
callouts_path: group_callouts_path,
callouts_feature_id: UserCalloutsHelper::INVITE_MEMBERS_BANNER,
group_id: @group.id } }
= render 'groups/invite_members_modal', group: @group = render 'groups/invite_members_modal', group: @group
= content_for :meta_tags do = content_for :meta_tags do
......
...@@ -36,6 +36,8 @@ def override_omniauth(provider, controller, path_prefix = '/users/auth') ...@@ -36,6 +36,8 @@ def override_omniauth(provider, controller, path_prefix = '/users/auth')
post :accept, on: :member post :accept, on: :member
post :decline, on: :member post :decline, on: :member
end end
resources :group_callouts, only: [:create]
end end
scope(constraints: { username: Gitlab::PathRegex.root_namespace_route_regex }) do scope(constraints: { username: Gitlab::PathRegex.root_namespace_route_regex }) do
......
# frozen_string_literal: true
class CreateUserGroupCallout < ActiveRecord::Migration[6.1]
def up
create_table :user_group_callouts do |t|
t.bigint :user_id, null: false
t.bigint :group_id, null: false
t.integer :feature_name, limit: 2, null: false
t.datetime_with_timezone :dismissed_at
t.index :group_id
t.index [:user_id, :feature_name, :group_id], unique: true, name: 'index_group_user_callouts_feature'
end
end
def down
drop_table :user_group_callouts
end
end
# frozen_string_literal: true
class AddGroupIdFkeyForUserGroupCallout < Gitlab::Database::Migration[1.0]
disable_ddl_transaction!
def up
add_concurrent_foreign_key :user_group_callouts, :namespaces, column: :group_id, on_delete: :cascade
end
def down
with_lock_retries do
remove_foreign_key :user_group_callouts, column: :group_id
end
end
end
# frozen_string_literal: true
class AddUserIdFkeyForUserGroupCallout < Gitlab::Database::Migration[1.0]
disable_ddl_transaction!
def up
add_concurrent_foreign_key :user_group_callouts, :users, column: :user_id, on_delete: :cascade
end
def down
with_lock_retries do
remove_foreign_key :user_group_callouts, column: :user_id
end
end
end
e6570f8ee366431b17b34051b9d0dcf2aff6216f8d65b3b6eec5be5666fed229
\ No newline at end of file
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment