Commit f0aa3f87 authored by Mayra Cabrera's avatar Mayra Cabrera

Merge branch...

Merge branch '334726-investigate-confirm-the-dismiss-for-the-invite-your-teammates-banner-works-as-intended' into 'master'

Persist group invite banner dismissal in database

See merge request gitlab-org/gitlab!68785
parents 87c9ce3a 5a5a0ad9
...@@ -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 @@ class UserCalloutsController < ApplicationController ...@@ -21,6 +17,12 @@ class UserCalloutsController < ApplicationController
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 @@ module GroupsHelper ...@@ -122,12 +122,6 @@ module GroupsHelper
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 @@ module GroupsHelper ...@@ -142,14 +136,6 @@ module GroupsHelper
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 @@ module UserCalloutsHelper ...@@ -56,6 +57,13 @@ module UserCalloutsHelper
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 @@ module UserCalloutsHelper ...@@ -63,6 +71,43 @@ module UserCalloutsHelper
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 @@ class Group < Namespace ...@@ -85,6 +85,8 @@ class Group < Namespace
# 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 @@ class User < ApplicationRecord ...@@ -200,6 +200,7 @@ class User < ApplicationRecord
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 @@ class User < ApplicationRecord ...@@ -1928,10 +1929,14 @@ class User < ApplicationRecord
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 @@ class User < ApplicationRecord ...@@ -1955,6 +1960,11 @@ class User < ApplicationRecord
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 @@ class User < ApplicationRecord ...@@ -2026,10 +2036,21 @@ class User < ApplicationRecord
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
......
...@@ -60,6 +60,8 @@ scope '-/users', module: :users do ...@@ -60,6 +60,8 @@ scope '-/users', module: :users do
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
ad564a1fda815473b09f1eda469e67cdd8f532b9b481f7e8ae3ddb8f2df6ee40
\ No newline at end of file
da57784c8c7f8bcb3c8c61089b5a695efdb31b209cb1616af68240380c734669
\ No newline at end of file
...@@ -19787,6 +19787,23 @@ CREATE TABLE user_follow_users ( ...@@ -19787,6 +19787,23 @@ CREATE TABLE user_follow_users (
followee_id integer NOT NULL followee_id integer NOT NULL
); );
CREATE TABLE user_group_callouts (
id bigint NOT NULL,
user_id bigint NOT NULL,
group_id bigint NOT NULL,
feature_name smallint NOT NULL,
dismissed_at timestamp with time zone
);
CREATE SEQUENCE user_group_callouts_id_seq
START WITH 1
INCREMENT BY 1
NO MINVALUE
NO MAXVALUE
CACHE 1;
ALTER SEQUENCE user_group_callouts_id_seq OWNED BY user_group_callouts.id;
CREATE TABLE user_highest_roles ( CREATE TABLE user_highest_roles (
user_id bigint NOT NULL, user_id bigint NOT NULL,
updated_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL,
...@@ -21697,6 +21714,8 @@ ALTER TABLE ONLY user_custom_attributes ALTER COLUMN id SET DEFAULT nextval('use ...@@ -21697,6 +21714,8 @@ ALTER TABLE ONLY user_custom_attributes ALTER COLUMN id SET DEFAULT nextval('use
ALTER TABLE ONLY user_details ALTER COLUMN user_id SET DEFAULT nextval('user_details_user_id_seq'::regclass); ALTER TABLE ONLY user_details ALTER COLUMN user_id SET DEFAULT nextval('user_details_user_id_seq'::regclass);
ALTER TABLE ONLY user_group_callouts ALTER COLUMN id SET DEFAULT nextval('user_group_callouts_id_seq'::regclass);
ALTER TABLE ONLY user_permission_export_uploads ALTER COLUMN id SET DEFAULT nextval('user_permission_export_uploads_id_seq'::regclass); ALTER TABLE ONLY user_permission_export_uploads ALTER COLUMN id SET DEFAULT nextval('user_permission_export_uploads_id_seq'::regclass);
ALTER TABLE ONLY user_preferences ALTER COLUMN id SET DEFAULT nextval('user_preferences_id_seq'::regclass); ALTER TABLE ONLY user_preferences ALTER COLUMN id SET DEFAULT nextval('user_preferences_id_seq'::regclass);
...@@ -23642,6 +23661,9 @@ ALTER TABLE ONLY user_details ...@@ -23642,6 +23661,9 @@ ALTER TABLE ONLY user_details
ALTER TABLE ONLY user_follow_users ALTER TABLE ONLY user_follow_users
ADD CONSTRAINT user_follow_users_pkey PRIMARY KEY (follower_id, followee_id); ADD CONSTRAINT user_follow_users_pkey PRIMARY KEY (follower_id, followee_id);
ALTER TABLE ONLY user_group_callouts
ADD CONSTRAINT user_group_callouts_pkey PRIMARY KEY (id);
ALTER TABLE ONLY user_highest_roles ALTER TABLE ONLY user_highest_roles
ADD CONSTRAINT user_highest_roles_pkey PRIMARY KEY (user_id); ADD CONSTRAINT user_highest_roles_pkey PRIMARY KEY (user_id);
...@@ -25197,6 +25219,8 @@ CREATE UNIQUE INDEX index_group_stages_on_group_id_group_value_stream_id_and_nam ...@@ -25197,6 +25219,8 @@ CREATE UNIQUE INDEX index_group_stages_on_group_id_group_value_stream_id_and_nam
CREATE INDEX index_group_stages_on_stage_event_hash_id ON analytics_cycle_analytics_group_stages USING btree (stage_event_hash_id); CREATE INDEX index_group_stages_on_stage_event_hash_id ON analytics_cycle_analytics_group_stages USING btree (stage_event_hash_id);
CREATE UNIQUE INDEX index_group_user_callouts_feature ON user_group_callouts USING btree (user_id, feature_name, group_id);
CREATE UNIQUE INDEX index_group_wiki_repositories_on_disk_path ON group_wiki_repositories USING btree (disk_path); CREATE UNIQUE INDEX index_group_wiki_repositories_on_disk_path ON group_wiki_repositories USING btree (disk_path);
CREATE INDEX index_group_wiki_repositories_on_shard_id ON group_wiki_repositories USING btree (shard_id); CREATE INDEX index_group_wiki_repositories_on_shard_id ON group_wiki_repositories USING btree (shard_id);
...@@ -26591,6 +26615,8 @@ CREATE INDEX index_user_details_on_provisioned_by_group_id ON user_details USING ...@@ -26591,6 +26615,8 @@ CREATE INDEX index_user_details_on_provisioned_by_group_id ON user_details USING
CREATE UNIQUE INDEX index_user_details_on_user_id ON user_details USING btree (user_id); CREATE UNIQUE INDEX index_user_details_on_user_id ON user_details USING btree (user_id);
CREATE INDEX index_user_group_callouts_on_group_id ON user_group_callouts USING btree (group_id);
CREATE INDEX index_user_highest_roles_on_user_id_and_highest_access_level ON user_highest_roles USING btree (user_id, highest_access_level); CREATE INDEX index_user_highest_roles_on_user_id_and_highest_access_level ON user_highest_roles USING btree (user_id, highest_access_level);
CREATE INDEX index_user_interacted_projects_on_user_id ON user_interacted_projects USING btree (user_id); CREATE INDEX index_user_interacted_projects_on_user_id ON user_interacted_projects USING btree (user_id);
...@@ -27783,6 +27809,9 @@ ALTER TABLE ONLY issues ...@@ -27783,6 +27809,9 @@ ALTER TABLE ONLY issues
ALTER TABLE ONLY epics ALTER TABLE ONLY epics
ADD CONSTRAINT fk_9d480c64b2 FOREIGN KEY (start_date_sourcing_epic_id) REFERENCES epics(id) ON DELETE SET NULL; ADD CONSTRAINT fk_9d480c64b2 FOREIGN KEY (start_date_sourcing_epic_id) REFERENCES epics(id) ON DELETE SET NULL;
ALTER TABLE ONLY user_group_callouts
ADD CONSTRAINT fk_9dc8b9d4b2 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
ALTER TABLE ONLY protected_environments ALTER TABLE ONLY protected_environments
ADD CONSTRAINT fk_9e112565b7 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; ADD CONSTRAINT fk_9e112565b7 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
...@@ -27927,6 +27956,9 @@ ALTER TABLE ONLY geo_event_log ...@@ -27927,6 +27956,9 @@ ALTER TABLE ONLY geo_event_log
ALTER TABLE ONLY analytics_cycle_analytics_project_stages ALTER TABLE ONLY analytics_cycle_analytics_project_stages
ADD CONSTRAINT fk_c3339bdfc9 FOREIGN KEY (stage_event_hash_id) REFERENCES analytics_cycle_analytics_stage_event_hashes(id) ON DELETE CASCADE; ADD CONSTRAINT fk_c3339bdfc9 FOREIGN KEY (stage_event_hash_id) REFERENCES analytics_cycle_analytics_stage_event_hashes(id) ON DELETE CASCADE;
ALTER TABLE ONLY user_group_callouts
ADD CONSTRAINT fk_c366e12ec3 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE;
ALTER TABLE ONLY vulnerability_exports ALTER TABLE ONLY vulnerability_exports
ADD CONSTRAINT fk_c3d3cb5d0f FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; ADD CONSTRAINT fk_c3d3cb5d0f FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE;
...@@ -3,14 +3,16 @@ ...@@ -3,14 +3,16 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe UserCalloutsController do RSpec.describe UserCalloutsController do
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
before do before do
sign_in(user) sign_in(user)
end end
describe "POST #create" do describe "POST #create" do
subject { post :create, params: { feature_name: feature_name }, format: :json } let(:params) { { feature_name: feature_name } }
subject { post :create, params: params, format: :json }
context 'with valid feature name' do context 'with valid feature name' do
let(:feature_name) { UserCallout.feature_names.each_key.first } let(:feature_name) { UserCallout.feature_names.each_key.first }
...@@ -30,9 +32,8 @@ RSpec.describe UserCalloutsController do ...@@ -30,9 +32,8 @@ RSpec.describe UserCalloutsController do
context 'when callout entry already exists' do context 'when callout entry already exists' do
let!(:callout) { create(:user_callout, feature_name: UserCallout.feature_names.each_key.first, user: user) } let!(:callout) { create(:user_callout, feature_name: UserCallout.feature_names.each_key.first, user: user) }
it 'returns success' do it 'returns success', :aggregate_failures do
subject expect { subject }.not_to change { UserCallout.count }
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
end end
end end
......
# frozen_string_literal: true
FactoryBot.define do
factory :group_callout, class: 'Users::GroupCallout' do
feature_name { :invite_members_banner }
user
group
end
end
...@@ -3,15 +3,15 @@ ...@@ -3,15 +3,15 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Group show page' do RSpec.describe 'Group show page' do
let(:group) { create(:group) } let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let(:path) { group_path(group) } let(:path) { group_path(group) }
context 'when signed in' do context 'when signed in' do
let(:user) do context 'with non-admin group concerns' do
create(:group_member, :developer, user: create(:user), group: group ).user
end
before do before do
group.add_developer(user)
sign_in(user) sign_in(user)
visit path visit path
end end
...@@ -25,6 +25,55 @@ RSpec.describe 'Group show page' do ...@@ -25,6 +25,55 @@ RSpec.describe 'Group show page' do
end end
end end
context 'when user is an owner' do
before do
group.add_owner(user)
sign_in(user)
end
it 'shows the invite banner and persists dismissal', :js do
visit path
expect(page).to have_content('Collaborate with your team')
page.within(find('[data-testid="invite-members-banner"]')) do
find('[data-testid="close-icon"]').click
end
expect(page).not_to have_content('Collaborate with your team')
visit path
expect(page).not_to have_content('Collaborate with your team')
end
context 'when group has a project with emoji in description', :js do
let!(:project) { create(:project, description: ':smile:', namespace: group) }
it 'shows the project info', :aggregate_failures do
visit path
expect(page).to have_content(project.title)
expect(page).to have_emoji('smile')
end
end
context 'when group has projects' do
it 'allows users to sorts projects by most stars', :js do
project1 = create(:project, namespace: group, star_count: 2)
project2 = create(:project, namespace: group, star_count: 3)
project3 = create(:project, namespace: group, star_count: 0)
visit group_path(group, sort: :stars_desc)
expect(find('.group-row:nth-child(1) .namespace-title > a')).to have_content(project2.title)
expect(find('.group-row:nth-child(2) .namespace-title > a')).to have_content(project1.title)
expect(find('.group-row:nth-child(3) .namespace-title > a')).to have_content(project3.title)
end
end
end
end
context 'when signed out' do context 'when signed out' do
describe 'RSS' do describe 'RSS' do
before do before do
...@@ -37,7 +86,7 @@ RSpec.describe 'Group show page' do ...@@ -37,7 +86,7 @@ RSpec.describe 'Group show page' do
context 'when group has a public project', :js do context 'when group has a public project', :js do
let!(:project) { create(:project, :public, namespace: group) } let!(:project) { create(:project, :public, namespace: group) }
it 'renders public project' do it 'renders public project', :aggregate_failures do
visit path visit path
expect(page).to have_link group.name expect(page).to have_link group.name
...@@ -48,7 +97,7 @@ RSpec.describe 'Group show page' do ...@@ -48,7 +97,7 @@ RSpec.describe 'Group show page' do
context 'when group has a private project', :js do context 'when group has a private project', :js do
let!(:project) { create(:project, :private, namespace: group) } let!(:project) { create(:project, :private, namespace: group) }
it 'does not render private project' do it 'does not render private project', :aggregate_failures do
visit path visit path
expect(page).to have_link group.name expect(page).to have_link group.name
...@@ -58,28 +107,19 @@ RSpec.describe 'Group show page' do ...@@ -58,28 +107,19 @@ RSpec.describe 'Group show page' do
end end
context 'subgroup support' do context 'subgroup support' do
let(:restricted_group) do let_it_be(:restricted_group) do
create(:group, subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS) create(:group, subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS)
end end
let(:relaxed_group) do
create(:group, subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS)
end
let(:owner) { create(:user) }
let(:maintainer) { create(:user) }
context 'for owners' do context 'for owners' do
let(:path) { group_path(restricted_group) }
before do before do
restricted_group.add_owner(owner) restricted_group.add_owner(user)
sign_in(owner) sign_in(user)
end end
context 'when subgroups are supported' do context 'when subgroups are supported' do
it 'allows creating subgroups' do it 'allows creating subgroups' do
visit path visit group_path(restricted_group)
expect(page).to have_link('New subgroup') expect(page).to have_link('New subgroup')
end end
...@@ -88,18 +128,21 @@ RSpec.describe 'Group show page' do ...@@ -88,18 +128,21 @@ RSpec.describe 'Group show page' do
context 'for maintainers' do context 'for maintainers' do
before do before do
sign_in(maintainer) sign_in(user)
end end
context 'when subgroups are supported' do context 'when subgroups are supported' do
context 'when subgroup_creation_level is set to maintainers' do context 'when subgroup_creation_level is set to maintainers' do
let(:relaxed_group) do
create(:group, subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS)
end
before do before do
relaxed_group.add_maintainer(maintainer) relaxed_group.add_maintainer(user)
end end
it 'allows creating subgroups' do it 'allows creating subgroups' do
path = group_path(relaxed_group) visit group_path(relaxed_group)
visit path
expect(page).to have_link('New subgroup') expect(page).to have_link('New subgroup')
end end
...@@ -107,12 +150,11 @@ RSpec.describe 'Group show page' do ...@@ -107,12 +150,11 @@ RSpec.describe 'Group show page' do
context 'when subgroup_creation_level is set to owners' do context 'when subgroup_creation_level is set to owners' do
before do before do
restricted_group.add_maintainer(maintainer) restricted_group.add_maintainer(user)
end end
it 'does not allow creating subgroups' do it 'does not allow creating subgroups' do
path = group_path(restricted_group) visit group_path(restricted_group)
visit path
expect(page).not_to have_link('New subgroup') expect(page).not_to have_link('New subgroup')
end end
...@@ -121,50 +163,10 @@ RSpec.describe 'Group show page' do ...@@ -121,50 +163,10 @@ RSpec.describe 'Group show page' do
end end
end end
context 'group has a project with emoji in description', :js do
let(:user) { create(:user) }
let!(:project) { create(:project, description: ':smile:', namespace: group) }
before do
group.add_owner(user)
sign_in(user)
visit path
end
it 'shows the project info' do
expect(page).to have_content(project.title)
expect(page).to have_emoji('smile')
end
end
context 'where group has projects' do
let(:user) { create(:user) }
before do
group.add_owner(user)
sign_in(user)
end
it 'allows users to sorts projects by most stars', :js do
project1 = create(:project, namespace: group, star_count: 2)
project2 = create(:project, namespace: group, star_count: 3)
project3 = create(:project, namespace: group, star_count: 0)
visit group_path(group, sort: :stars_desc)
expect(find('.group-row:nth-child(1) .namespace-title > a')).to have_content(project2.title)
expect(find('.group-row:nth-child(2) .namespace-title > a')).to have_content(project1.title)
expect(find('.group-row:nth-child(3) .namespace-title > a')).to have_content(project3.title)
end
end
context 'notification button', :js do context 'notification button', :js do
let(:maintainer) { create(:user) }
let!(:project) { create(:project, namespace: group) }
before do before do
group.add_maintainer(maintainer) group.add_maintainer(user)
sign_in(maintainer) sign_in(user)
end end
it 'is enabled by default' do it 'is enabled by default' do
...@@ -174,7 +176,8 @@ RSpec.describe 'Group show page' do ...@@ -174,7 +176,8 @@ RSpec.describe 'Group show page' do
end end
it 'is disabled if emails are disabled' do it 'is disabled if emails are disabled' do
group.update_attribute(:emails_disabled, true) group.update!(emails_disabled: true)
visit path visit path
expect(page).to have_selector('[data-testid="notification-dropdown"] .disabled') expect(page).to have_selector('[data-testid="notification-dropdown"] .disabled')
...@@ -182,12 +185,10 @@ RSpec.describe 'Group show page' do ...@@ -182,12 +185,10 @@ RSpec.describe 'Group show page' do
end end
context 'page og:description' do context 'page og:description' do
let(:group) { create(:group, description: '**Lorem** _ipsum_ dolor sit [amet](https://example.com)') }
let(:maintainer) { create(:user) }
before do before do
group.add_maintainer(maintainer) group.update!(description: '**Lorem** _ipsum_ dolor sit [amet](https://example.com)')
sign_in(maintainer) group.add_maintainer(user)
sign_in(user)
visit path visit path
end end
...@@ -237,7 +238,7 @@ RSpec.describe 'Group show page' do ...@@ -237,7 +238,7 @@ RSpec.describe 'Group show page' do
end end
end end
it 'does not include structured markup in shared projects tab', :js do it 'does not include structured markup in shared projects tab', :aggregate_failures, :js do
other_project = create(:project, :public) other_project = create(:project, :public)
other_project.project_group_links.create!(group: group) other_project.project_group_links.create!(group: group)
...@@ -248,7 +249,7 @@ RSpec.describe 'Group show page' do ...@@ -248,7 +249,7 @@ RSpec.describe 'Group show page' do
expect(page).not_to have_selector('[itemprop="owns"][itemtype="https://schema.org/SoftwareSourceCode"]') expect(page).not_to have_selector('[itemprop="owns"][itemtype="https://schema.org/SoftwareSourceCode"]')
end end
it 'does not include structured markup in archived projects tab', :js do it 'does not include structured markup in archived projects tab', :aggregate_failures, :js do
project.update!(archived: true) project.update!(archived: true)
visit group_archived_path(group) visit group_archived_path(group)
......
import { GlBanner, GlButton } from '@gitlab/ui'; import { GlBanner } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils'; import { shallowMount } from '@vue/test-utils';
import MockAdapter from 'axios-mock-adapter';
import { mockTracking, unmockTracking } from 'helpers/tracking_helper'; import { mockTracking, unmockTracking } from 'helpers/tracking_helper';
import InviteMembersBanner from '~/groups/components/invite_members_banner.vue'; import InviteMembersBanner from '~/groups/components/invite_members_banner.vue';
import eventHub from '~/invite_members/event_hub'; import eventHub from '~/invite_members/event_hub';
import { setCookie, parseBoolean } from '~/lib/utils/common_utils'; import axios from '~/lib/utils/axios_utils';
jest.mock('~/lib/utils/common_utils'); jest.mock('~/lib/utils/common_utils');
const isDismissedKey = 'invite_99_1';
const title = 'Collaborate with your team'; const title = 'Collaborate with your team';
const body = const body =
"We noticed that you haven't invited anyone to this group. Invite your colleagues so you can discuss issues, collaborate on merge requests, and share your knowledge"; "We noticed that you haven't invited anyone to this group. Invite your colleagues so you can discuss issues, collaborate on merge requests, and share your knowledge";
const svgPath = '/illustrations/background';
const inviteMembersPath = 'groups/members';
const buttonText = 'Invite your colleagues'; const buttonText = 'Invite your colleagues';
const trackLabel = 'invite_members_banner'; const provide = {
svgPath: '/illustrations/background',
inviteMembersPath: 'groups/members',
trackLabel: 'invite_members_banner',
calloutsPath: 'call/out/path',
calloutsFeatureId: 'some-feature-id',
groupId: '1',
};
const createComponent = (stubs = {}) => { const createComponent = (stubs = {}) => {
return shallowMount(InviteMembersBanner, { return shallowMount(InviteMembersBanner, {
provide: { provide,
svgPath,
inviteMembersPath,
isDismissedKey,
trackLabel,
},
stubs, stubs,
}); });
}; };
...@@ -31,8 +31,10 @@ const createComponent = (stubs = {}) => { ...@@ -31,8 +31,10 @@ const createComponent = (stubs = {}) => {
describe('InviteMembersBanner', () => { describe('InviteMembersBanner', () => {
let wrapper; let wrapper;
let trackingSpy; let trackingSpy;
let mockAxios;
beforeEach(() => { beforeEach(() => {
mockAxios = new MockAdapter(axios);
document.body.dataset.page = 'any:page'; document.body.dataset.page = 'any:page';
trackingSpy = mockTracking('_category_', undefined, jest.spyOn); trackingSpy = mockTracking('_category_', undefined, jest.spyOn);
}); });
...@@ -40,22 +42,28 @@ describe('InviteMembersBanner', () => { ...@@ -40,22 +42,28 @@ describe('InviteMembersBanner', () => {
afterEach(() => { afterEach(() => {
wrapper.destroy(); wrapper.destroy();
wrapper = null; wrapper = null;
mockAxios.restore();
unmockTracking(); unmockTracking();
}); });
describe('tracking', () => { describe('tracking', () => {
const mockTrackingOnWrapper = () => {
unmockTracking();
trackingSpy = mockTracking('_category_', wrapper.element, jest.spyOn);
};
beforeEach(() => { beforeEach(() => {
wrapper = createComponent({ GlBanner }); wrapper = createComponent({ GlBanner });
}); });
const trackCategory = undefined; const trackCategory = undefined;
const displayEvent = 'invite_members_banner_displayed';
const buttonClickEvent = 'invite_members_banner_button_clicked'; const buttonClickEvent = 'invite_members_banner_button_clicked';
const dismissEvent = 'invite_members_banner_dismissed';
it('sends the displayEvent when the banner is displayed', () => { it('sends the displayEvent when the banner is displayed', () => {
const displayEvent = 'invite_members_banner_displayed';
expect(trackingSpy).toHaveBeenCalledWith(trackCategory, displayEvent, { expect(trackingSpy).toHaveBeenCalledWith(trackCategory, displayEvent, {
label: trackLabel, label: provide.trackLabel,
}); });
}); });
...@@ -74,16 +82,20 @@ describe('InviteMembersBanner', () => { ...@@ -74,16 +82,20 @@ describe('InviteMembersBanner', () => {
it('sends the buttonClickEvent with correct trackCategory and trackLabel', () => { it('sends the buttonClickEvent with correct trackCategory and trackLabel', () => {
expect(trackingSpy).toHaveBeenCalledWith(trackCategory, buttonClickEvent, { expect(trackingSpy).toHaveBeenCalledWith(trackCategory, buttonClickEvent, {
label: trackLabel, label: provide.trackLabel,
}); });
}); });
}); });
it('sends the dismissEvent when the banner is dismissed', () => { it('sends the dismissEvent when the banner is dismissed', () => {
mockTrackingOnWrapper();
mockAxios.onPost(provide.calloutsPath).replyOnce(200);
const dismissEvent = 'invite_members_banner_dismissed';
wrapper.find(GlBanner).vm.$emit('close'); wrapper.find(GlBanner).vm.$emit('close');
expect(trackingSpy).toHaveBeenCalledWith(trackCategory, dismissEvent, { expect(trackingSpy).toHaveBeenCalledWith(trackCategory, dismissEvent, {
label: trackLabel, label: provide.trackLabel,
}); });
}); });
}); });
...@@ -98,7 +110,7 @@ describe('InviteMembersBanner', () => { ...@@ -98,7 +110,7 @@ describe('InviteMembersBanner', () => {
}); });
it('uses the svgPath for the banner svgpath', () => { it('uses the svgPath for the banner svgpath', () => {
expect(findBanner().attributes('svgpath')).toBe(svgPath); expect(findBanner().attributes('svgpath')).toBe(provide.svgPath);
}); });
it('uses the title from options for title', () => { it('uses the title from options for title', () => {
...@@ -115,35 +127,20 @@ describe('InviteMembersBanner', () => { ...@@ -115,35 +127,20 @@ describe('InviteMembersBanner', () => {
}); });
describe('dismissing', () => { describe('dismissing', () => {
const findButton = () => wrapper.findAll(GlButton).at(1);
beforeEach(() => { beforeEach(() => {
wrapper = createComponent({ GlBanner }); wrapper = createComponent({ GlBanner });
findButton().vm.$emit('click');
});
it('sets iDismissed to true', () => {
expect(wrapper.vm.isDismissed).toBe(true);
}); });
it('sets the cookie with the isDismissedKey', () => { it('should render the banner when not dismissed', () => {
expect(setCookie).toHaveBeenCalledWith(isDismissedKey, true); expect(wrapper.find(GlBanner).exists()).toBe(true);
}); });
});
describe('when a dismiss cookie exists', () => {
beforeEach(() => {
parseBoolean.mockReturnValue(true);
wrapper = createComponent({ GlBanner }); it('should close the banner when dismiss is clicked', async () => {
}); mockAxios.onPost(provide.calloutsPath).replyOnce(200);
expect(wrapper.find(GlBanner).exists()).toBe(true);
it('sets isDismissed to true', () => { wrapper.find(GlBanner).vm.$emit('close');
expect(wrapper.vm.isDismissed).toBe(true);
});
it('does not render the banner', () => { await wrapper.vm.$nextTick();
expect(wrapper.find(GlBanner).exists()).toBe(false); expect(wrapper.find(GlBanner).exists()).toBe(false);
}); });
}); });
......
...@@ -375,67 +375,6 @@ RSpec.describe GroupsHelper do ...@@ -375,67 +375,6 @@ RSpec.describe GroupsHelper do
end end
end end
describe '#show_invite_banner?' do
let_it_be(:current_user) { create(:user) }
let_it_be_with_refind(:group) { create(:group) }
let_it_be(:subgroup) { create(:group, parent: group) }
let_it_be(:users) { [current_user, create(:user)] }
before do
allow(helper).to receive(:current_user) { current_user }
allow(helper).to receive(:can?).with(current_user, :admin_group, group).and_return(can_admin_group)
allow(helper).to receive(:can?).with(current_user, :admin_group, subgroup).and_return(can_admin_group)
users.take(group_members_count).each { |user| group.add_guest(user) }
end
using RSpec::Parameterized::TableSyntax
where(:can_admin_group, :group_members_count, :expected_result) do
true | 1 | true
false | 1 | false
true | 2 | false
false | 2 | false
end
with_them do
context 'for a parent group' do
subject { helper.show_invite_banner?(group) }
context 'when the group was just created' do
before do
flash[:notice] = "Group #{group.name} was successfully created"
end
it { is_expected.to be_falsey }
end
context 'when no flash message' do
it 'returns the expected result' do
expect(subject).to eq(expected_result)
end
end
end
context 'for a subgroup' do
subject { helper.show_invite_banner?(subgroup) }
context 'when the subgroup was just created' do
before do
flash[:notice] = "Group #{subgroup.name} was successfully created"
end
it { is_expected.to be_falsey }
end
context 'when no flash message' do
it 'returns the expected result' do
expect(subject).to eq(expected_result)
end
end
end
end
end
describe '#render_setting_to_allow_project_access_token_creation?' do describe '#render_setting_to_allow_project_access_token_creation?' do
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
let_it_be(:parent) { create(:group) } let_it_be(:parent) { create(:group) }
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require "spec_helper" require "spec_helper"
RSpec.describe UserCalloutsHelper do RSpec.describe UserCalloutsHelper do
let_it_be(:user) { create(:user) } let_it_be(:user, refind: true) { create(:user) }
before do before do
allow(helper).to receive(:current_user).and_return(user) allow(helper).to receive(:current_user).and_return(user)
...@@ -202,4 +202,95 @@ RSpec.describe UserCalloutsHelper do ...@@ -202,4 +202,95 @@ RSpec.describe UserCalloutsHelper do
it { is_expected.to be false } it { is_expected.to be false }
end end
end end
describe '.show_invite_banner?' do
let_it_be(:group) { create(:group) }
subject { helper.show_invite_banner?(group) }
context 'when user has the admin ability for the group' do
before do
group.add_owner(user)
end
context 'when the invite_members_banner has not been dismissed' do
it { is_expected.to eq(true) }
context 'when a user has dismissed this banner via cookies already' do
before do
helper.request.cookies["invite_#{group.id}_#{user.id}"] = 'true'
end
it { is_expected.to eq(false) }
it 'creates the callout from cookie', :aggregate_failures do
expect { subject }.to change { Users::GroupCallout.count }.by(1)
expect(Users::GroupCallout.last).to have_attributes(group_id: group.id,
feature_name: described_class::INVITE_MEMBERS_BANNER)
end
end
context 'when the group was just created' do
before do
flash[:notice] = "Group #{group.name} was successfully created"
end
it { is_expected.to eq(false) }
end
context 'with concerning multiple members' do
let_it_be(:user_2) { create(:user) }
context 'on current group' do
before do
group.add_guest(user_2)
end
it { is_expected.to eq(false) }
end
context 'on current group that is a subgroup' do
let_it_be(:subgroup) { create(:group, parent: group) }
subject { helper.show_invite_banner?(subgroup) }
context 'with only one user on parent and this group' do
it { is_expected.to eq(true) }
end
context 'when another user is on this group' do
before do
subgroup.add_guest(user_2)
end
it { is_expected.to eq(false) }
end
context 'when another user is on the parent group' do
before do
group.add_guest(user_2)
end
it { is_expected.to eq(false) }
end
end
end
end
context 'when the invite_members_banner has been dismissed' do
before do
create(:group_callout,
user: user,
group: group,
feature_name: described_class::INVITE_MEMBERS_BANNER)
end
it { is_expected.to eq(false) }
end
end
context 'when user does not have admin ability for the group' do
it { is_expected.to eq(false) }
end
end
end end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Calloutable do
subject { build(:user_callout) }
describe "Associations" do
it { is_expected.to belong_to(:user) }
end
describe 'validations' do
it { is_expected.to validate_presence_of(:user) }
end
describe '#dismissed_after?' do
let(:some_feature_name) { UserCallout.feature_names.keys.second }
let(:callout_dismissed_month_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.month.ago )}
let(:callout_dismissed_day_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.day.ago )}
it 'returns whether a callout dismissed after specified date' do
expect(callout_dismissed_month_ago.dismissed_after?(15.days.ago)).to eq(false)
expect(callout_dismissed_day_ago.dismissed_after?(15.days.ago)).to eq(true)
end
end
end
...@@ -35,6 +35,7 @@ RSpec.describe Group do ...@@ -35,6 +35,7 @@ RSpec.describe Group do
it { is_expected.to have_many(:dependency_proxy_manifests) } it { is_expected.to have_many(:dependency_proxy_manifests) }
it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::GroupDistribution').dependent(:destroy) } it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::GroupDistribution').dependent(:destroy) }
it { is_expected.to have_many(:daily_build_group_report_results).class_name('Ci::DailyBuildGroupReportResult') } it { is_expected.to have_many(:daily_build_group_report_results).class_name('Ci::DailyBuildGroupReportResult') }
it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout').with_foreign_key(:group_id) }
describe '#members & #requesters' do describe '#members & #requesters' do
let(:requester) { create(:user) } let(:requester) { create(:user) }
......
...@@ -3,29 +3,12 @@ ...@@ -3,29 +3,12 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe UserCallout do RSpec.describe UserCallout do
let!(:callout) { create(:user_callout) } let_it_be(:callout) { create(:user_callout) }
it_behaves_like 'having unique enum values' it_behaves_like 'having unique enum values'
describe 'relationships' do
it { is_expected.to belong_to(:user) }
end
describe 'validations' do describe 'validations' do
it { is_expected.to validate_presence_of(:user) }
it { is_expected.to validate_presence_of(:feature_name) } it { is_expected.to validate_presence_of(:feature_name) }
it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id).ignoring_case_sensitivity } it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id).ignoring_case_sensitivity }
end end
describe '#dismissed_after?' do
let(:some_feature_name) { described_class.feature_names.keys.second }
let(:callout_dismissed_month_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.month.ago )}
let(:callout_dismissed_day_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.day.ago )}
it 'returns whether a callout dismissed after specified date' do
expect(callout_dismissed_month_ago.dismissed_after?(15.days.ago)).to eq(false)
expect(callout_dismissed_day_ago.dismissed_after?(15.days.ago)).to eq(true)
end
end
end end
...@@ -120,6 +120,8 @@ RSpec.describe User do ...@@ -120,6 +120,8 @@ RSpec.describe User do
it { is_expected.to have_many(:created_custom_emoji).inverse_of(:creator) } it { is_expected.to have_many(:created_custom_emoji).inverse_of(:creator) }
it { is_expected.to have_many(:in_product_marketing_emails) } it { is_expected.to have_many(:in_product_marketing_emails) }
it { is_expected.to have_many(:timelogs) } it { is_expected.to have_many(:timelogs) }
it { is_expected.to have_many(:callouts).class_name('UserCallout') }
it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout') }
describe "#user_detail" do describe "#user_detail" do
it 'does not persist `user_detail` by default' do it 'does not persist `user_detail` by default' do
...@@ -5542,22 +5544,17 @@ RSpec.describe User do ...@@ -5542,22 +5544,17 @@ RSpec.describe User do
end end
describe '#dismissed_callout?' do describe '#dismissed_callout?' do
subject(:user) { create(:user) } let_it_be(:user, refind: true) { create(:user) }
let_it_be(:feature_name) { UserCallout.feature_names.each_key.first }
let(:feature_name) { UserCallout.feature_names.each_key.first }
context 'when no callout dismissal record exists' do context 'when no callout dismissal record exists' do
it 'returns false when no ignore_dismissal_earlier_than provided' do it 'returns false when no ignore_dismissal_earlier_than provided' do
expect(user.dismissed_callout?(feature_name: feature_name)).to eq false expect(user.dismissed_callout?(feature_name: feature_name)).to eq false
end end
it 'returns false when ignore_dismissal_earlier_than provided' do
expect(user.dismissed_callout?(feature_name: feature_name, ignore_dismissal_earlier_than: 3.months.ago)).to eq false
end
end end
context 'when dismissed callout exists' do context 'when dismissed callout exists' do
before do before_all do
create(:user_callout, user: user, feature_name: feature_name, dismissed_at: 4.months.ago) create(:user_callout, user: user, feature_name: feature_name, dismissed_at: 4.months.ago)
end end
...@@ -5575,6 +5572,123 @@ RSpec.describe User do ...@@ -5575,6 +5572,123 @@ RSpec.describe User do
end end
end end
describe '#find_or_initialize_callout' do
let_it_be(:user, refind: true) { create(:user) }
let_it_be(:feature_name) { UserCallout.feature_names.each_key.first }
subject(:find_or_initialize_callout) { user.find_or_initialize_callout(feature_name) }
context 'when callout exists' do
let!(:callout) { create(:user_callout, user: user, feature_name: feature_name) }
it 'returns existing callout' do
expect(find_or_initialize_callout).to eq(callout)
end
end
context 'when callout does not exist' do
context 'when feature name is valid' do
it 'initializes a new callout' do
expect(find_or_initialize_callout).to be_a_new(UserCallout)
end
it 'is valid' do
expect(find_or_initialize_callout).to be_valid
end
end
context 'when feature name is not valid' do
let(:feature_name) { 'notvalid' }
it 'initializes a new callout' do
expect(find_or_initialize_callout).to be_a_new(UserCallout)
end
it 'is not valid' do
expect(find_or_initialize_callout).not_to be_valid
end
end
end
end
describe '#dismissed_callout_for_group?' do
let_it_be(:user, refind: true) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:feature_name) { Users::GroupCallout.feature_names.each_key.first }
context 'when no callout dismissal record exists' do
it 'returns false when no ignore_dismissal_earlier_than provided' do
expect(user.dismissed_callout_for_group?(feature_name: feature_name, group: group)).to eq false
end
end
context 'when dismissed callout exists' do
before_all do
create(:group_callout,
user: user,
group_id: group.id,
feature_name: feature_name,
dismissed_at: 4.months.ago)
end
it 'returns true when no ignore_dismissal_earlier_than provided' do
expect(user.dismissed_callout_for_group?(feature_name: feature_name, group: group)).to eq true
end
it 'returns true when ignore_dismissal_earlier_than is earlier than dismissed_at' do
expect(user.dismissed_callout_for_group?(feature_name: feature_name, group: group, ignore_dismissal_earlier_than: 6.months.ago)).to eq true
end
it 'returns false when ignore_dismissal_earlier_than is later than dismissed_at' do
expect(user.dismissed_callout_for_group?(feature_name: feature_name, group: group, ignore_dismissal_earlier_than: 3.months.ago)).to eq false
end
end
end
describe '#find_or_initialize_group_callout' do
let_it_be(:user, refind: true) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:feature_name) { Users::GroupCallout.feature_names.each_key.first }
subject(:callout_with_source) do
user.find_or_initialize_group_callout(feature_name, group.id)
end
context 'when callout exists' do
let!(:callout) do
create(:group_callout, user: user, feature_name: feature_name, group_id: group.id)
end
it 'returns existing callout' do
expect(callout_with_source).to eq(callout)
end
end
context 'when callout does not exist' do
context 'when feature name is valid' do
it 'initializes a new callout' do
expect(callout_with_source).to be_a_new(Users::GroupCallout)
end
it 'is valid' do
expect(callout_with_source).to be_valid
end
end
context 'when feature name is not valid' do
let(:feature_name) { 'notvalid' }
it 'initializes a new callout' do
expect(callout_with_source).to be_a_new(Users::GroupCallout)
end
it 'is not valid' do
expect(callout_with_source).not_to be_valid
end
end
end
end
describe '#hook_attrs' do describe '#hook_attrs' do
it 'includes id, name, username, avatar_url, and email' do it 'includes id, name, username, avatar_url, and email' do
user = create(:user) user = create(:user)
...@@ -5937,45 +6051,6 @@ RSpec.describe User do ...@@ -5937,45 +6051,6 @@ RSpec.describe User do
end end
end end
describe '#find_or_initialize_callout' do
subject(:find_or_initialize_callout) { user.find_or_initialize_callout(feature_name) }
let(:user) { create(:user) }
let(:feature_name) { UserCallout.feature_names.each_key.first }
context 'when callout exists' do
let!(:callout) { create(:user_callout, user: user, feature_name: feature_name) }
it 'returns existing callout' do
expect(find_or_initialize_callout).to eq(callout)
end
end
context 'when callout does not exist' do
context 'when feature name is valid' do
it 'initializes a new callout' do
expect(find_or_initialize_callout).to be_a_new(UserCallout)
end
it 'is valid' do
expect(find_or_initialize_callout).to be_valid
end
end
context 'when feature name is not valid' do
let(:feature_name) { 'notvalid' }
it 'initializes a new callout' do
expect(find_or_initialize_callout).to be_a_new(UserCallout)
end
it 'is not valid' do
expect(find_or_initialize_callout).not_to be_valid
end
end
end
end
describe '#default_dashboard?' do describe '#default_dashboard?' do
it 'is the default dashboard' do it 'is the default dashboard' do
user = build(:user) user = build(:user)
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Users::GroupCallout do
let_it_be(:user) { create_default(:user) }
let_it_be(:group) { create_default(:group) }
let_it_be(:callout) { create(:group_callout) }
it_behaves_like 'having unique enum values'
describe 'relationships' do
it { is_expected.to belong_to(:group) }
end
describe 'validations' do
it { is_expected.to validate_presence_of(:group) }
it { is_expected.to validate_presence_of(:feature_name) }
it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id, :group_id).ignoring_case_sensitivity }
end
describe '#source_feature_name' do
it 'provides string based off source and feature' do
expect(callout.source_feature_name).to eq "#{callout.feature_name}_#{callout.group_id}"
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Group callouts' do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
before do
sign_in(user)
end
describe 'POST /-/users/group_callouts' do
let(:params) { { feature_name: feature_name, group_id: group.id } }
subject { post group_callouts_path, params: params, headers: { 'ACCEPT' => 'application/json' } }
context 'with valid feature name and group' do
let(:feature_name) { Users::GroupCallout.feature_names.each_key.first }
context 'when callout entry does not exist' do
it 'creates a callout entry with dismissed state' do
expect { subject }.to change { Users::GroupCallout.count }.by(1)
end
it 'returns success' do
subject
expect(response).to have_gitlab_http_status(:ok)
end
end
context 'when callout entry already exists' do
let!(:callout) do
create(:group_callout,
feature_name: Users::GroupCallout.feature_names.each_key.first,
user: user,
group: group)
end
it 'returns success', :aggregate_failures do
expect { subject }.not_to change { Users::GroupCallout.count }
expect(response).to have_gitlab_http_status(:ok)
end
end
end
context 'with invalid feature name' do
let(:feature_name) { 'bogus_feature_name' }
it 'returns bad request' do
subject
expect(response).to have_gitlab_http_status(:bad_request)
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Users::DismissGroupCalloutService do
describe '#execute' do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let(:params) { { feature_name: feature_name, group_id: group.id } }
let(:feature_name) { Users::GroupCallout.feature_names.each_key.first }
subject(:execute) do
described_class.new(
container: nil, current_user: user, params: params
).execute
end
it_behaves_like 'dismissing user callout', Users::GroupCallout
it 'sets the group_id' do
expect(execute.group_id).to eq(group.id)
end
end
end
...@@ -3,25 +3,18 @@ ...@@ -3,25 +3,18 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Users::DismissUserCalloutService do RSpec.describe Users::DismissUserCalloutService do
let(:user) { create(:user) }
let(:service) do
described_class.new(
container: nil, current_user: user, params: { feature_name: UserCallout.feature_names.each_key.first }
)
end
describe '#execute' do describe '#execute' do
subject(:execute) { service.execute } let_it_be(:user) { create(:user) }
it 'returns a user callout' do let(:params) { { feature_name: feature_name } }
expect(execute).to be_an_instance_of(UserCallout) let(:feature_name) { UserCallout.feature_names.each_key.first }
end
it 'sets the dismisse_at attribute to current time' do subject(:execute) do
freeze_time do described_class.new(
expect(execute).to have_attributes(dismissed_at: Time.current) container: nil, current_user: user, params: params
end ).execute
end end
it_behaves_like 'dismissing user callout', UserCallout
end end
end end
# frozen_string_literal: true
RSpec.shared_examples_for 'dismissing user callout' do |model|
it 'creates a new user callout' do
expect { execute }.to change { model.count }.by(1)
end
it 'returns a user callout' do
expect(execute).to be_an_instance_of(model)
end
it 'sets the dismissed_at attribute to current time' do
freeze_time do
expect(execute).to have_attributes(dismissed_at: Time.current)
end
end
it 'updates an existing callout dismissed_at time' do
freeze_time do
old_time = 1.day.ago
new_time = Time.current
attributes = params.merge(dismissed_at: old_time, user: user)
existing_callout = create("#{model.name.split('::').last.underscore}".to_sym, attributes)
expect { execute }.to change { existing_callout.reload.dismissed_at }.from(old_time).to(new_time)
end
end
it 'does not update an invalid record with dismissed_at time', :aggregate_failures do
callout = described_class.new(
container: nil, current_user: user, params: { feature_name: nil }
).execute
expect(callout.dismissed_at).to be_nil
expect(callout).to be_invalid
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