From 275bd0520bf5ce0a6e5b7ecef857f4dcd4e2679b Mon Sep 17 00:00:00 2001 From: lmeckley Date: Tue, 29 Nov 2022 16:31:23 -0700 Subject: [PATCH 1/7] Add guest overage modal --- .../components/table/role_dropdown.vue | 19 ++- .../members/guest_overage_confirm_action.js | 3 + db/structure.sql | 80 ++++++------ .../javascripts/invite_members/constants.js | 8 ++ .../members/guest_overage_confirm_action.js | 35 ++++++ .../ee/projects/project_members_controller.rb | 6 + .../show_overage_on_role_promotion.yml | 8 ++ .../features/projects/audit_events_spec.rb | 115 ++++++++++-------- 8 files changed, 178 insertions(+), 96 deletions(-) create mode 100644 app/assets/javascripts/members/guest_overage_confirm_action.js create mode 100644 ee/app/assets/javascripts/members/guest_overage_confirm_action.js create mode 100644 ee/config/feature_flags/development/show_overage_on_role_promotion.yml diff --git a/app/assets/javascripts/members/components/table/role_dropdown.vue b/app/assets/javascripts/members/components/table/role_dropdown.vue index 6cd8bf57313ece..b9efc6d1ea4ed0 100644 --- a/app/assets/javascripts/members/components/table/role_dropdown.vue +++ b/app/assets/javascripts/members/components/table/role_dropdown.vue @@ -3,6 +3,10 @@ import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import { mapActions } from 'vuex'; import { s__ } from '~/locale'; +import { + userOverageConfirmAction, + guestOverageConfirmAction, +} from 'ee_else_ce/members/guest_overage_confirm_action'; export default { name: 'RoleDropdown', @@ -50,22 +54,31 @@ export default { return dispatch(`${this.namespace}/updateMemberRole`, payload); }, }), - handleSelect(value, name) { + async handleSelect(value, name) { if (value === this.member.accessLevel.integerValue) { return; } this.busy = true; + const confirmed = await guestOverageConfirmAction({ + dropdownIntValue: value, + subscriptionSeats: 0, + totalUsers: 0, + groupName: __('a name'), + }); + if (!confirmed) { + return; + } + this.updateMemberRole({ memberId: this.member.id, accessLevel: { integerValue: value, stringValue: name }, }) .then(() => { this.$toast.show(s__('Members|Role updated successfully.')); - this.busy = false; }) - .catch(() => { + .finally(() => { this.busy = false; }); }, diff --git a/app/assets/javascripts/members/guest_overage_confirm_action.js b/app/assets/javascripts/members/guest_overage_confirm_action.js new file mode 100644 index 00000000000000..2205c3ad7920f8 --- /dev/null +++ b/app/assets/javascripts/members/guest_overage_confirm_action.js @@ -0,0 +1,3 @@ +export const guestOverageConfirmAction = () => { + return true; +}; diff --git a/db/structure.sql b/db/structure.sql index 72d9c94be491c6..ecf4f7f949ce4e 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -417,6 +417,26 @@ CREATE TABLE batched_background_migration_job_transition_logs ( ) PARTITION BY RANGE (created_at); +CREATE TABLE p_ci_builds_metadata ( + project_id integer NOT NULL, + timeout integer, + timeout_source integer DEFAULT 1 NOT NULL, + interruptible boolean, + config_options jsonb, + config_variables jsonb, + has_exposed_artifacts boolean, + environment_auto_stop_in character varying(255), + expanded_environment_name character varying(255), + secrets jsonb DEFAULT '{}'::jsonb NOT NULL, + build_id bigint NOT NULL, + id bigint NOT NULL, + runtime_runner_features jsonb DEFAULT '{}'::jsonb NOT NULL, + id_tokens jsonb DEFAULT '{}'::jsonb NOT NULL, + partition_id bigint DEFAULT 100 NOT NULL, + debug_trace_enabled boolean DEFAULT false NOT NULL +) +PARTITION BY LIST (partition_id); + CREATE TABLE incident_management_pending_alert_escalations ( id bigint NOT NULL, rule_id bigint NOT NULL, @@ -11473,10 +11493,6 @@ CREATE TABLE application_settings ( database_grafana_api_url text, database_grafana_tag text, public_runner_releases_url text DEFAULT 'https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-runner/releases'::text NOT NULL, - password_uppercase_required boolean DEFAULT false NOT NULL, - password_lowercase_required boolean DEFAULT false NOT NULL, - password_number_required boolean DEFAULT false NOT NULL, - password_symbol_required boolean DEFAULT false NOT NULL, encrypted_arkose_labs_public_api_key bytea, encrypted_arkose_labs_public_api_key_iv bytea, encrypted_arkose_labs_private_api_key bytea, @@ -11487,7 +11503,6 @@ CREATE TABLE application_settings ( inactive_projects_min_size_mb integer DEFAULT 0 NOT NULL, inactive_projects_send_warning_email_after_months integer DEFAULT 1 NOT NULL, delayed_group_deletion boolean DEFAULT true NOT NULL, - maven_package_requests_forwarding boolean DEFAULT true NOT NULL, arkose_labs_namespace text DEFAULT 'client'::text NOT NULL, max_export_size integer DEFAULT 0, encrypted_slack_app_signing_secret bytea, @@ -11502,8 +11517,12 @@ CREATE TABLE application_settings ( encrypted_dingtalk_app_key_iv bytea, encrypted_dingtalk_app_secret bytea, encrypted_dingtalk_app_secret_iv bytea, - jira_connect_application_key text, globally_allowed_ips text DEFAULT ''::text NOT NULL, + password_uppercase_required boolean DEFAULT false NOT NULL, + password_lowercase_required boolean DEFAULT false NOT NULL, + password_number_required boolean DEFAULT false NOT NULL, + password_symbol_required boolean DEFAULT false NOT NULL, + jira_connect_application_key text, container_registry_pre_import_tags_rate numeric(6,2) DEFAULT 0.5 NOT NULL, license_usage_data_exported boolean DEFAULT false NOT NULL, phone_verification_code_enabled boolean DEFAULT false NOT NULL, @@ -11514,38 +11533,39 @@ CREATE TABLE application_settings ( encrypted_feishu_app_key_iv bytea, encrypted_feishu_app_secret bytea, encrypted_feishu_app_secret_iv bytea, + git_rate_limit_users_allowlist text[] DEFAULT '{}'::text[] NOT NULL, error_tracking_enabled boolean DEFAULT false NOT NULL, error_tracking_api_url text, - git_rate_limit_users_allowlist text[] DEFAULT '{}'::text[] NOT NULL, error_tracking_access_token_encrypted text, - invitation_flow_enforcement boolean DEFAULT false NOT NULL, package_registry_cleanup_policies_worker_capacity integer DEFAULT 2 NOT NULL, deactivate_dormant_users_period integer DEFAULT 90 NOT NULL, auto_ban_user_on_excessive_projects_download boolean DEFAULT false NOT NULL, + invitation_flow_enforcement boolean DEFAULT false NOT NULL, + maven_package_requests_forwarding boolean DEFAULT true NOT NULL, max_pages_custom_domains_per_project integer DEFAULT 0 NOT NULL, cube_api_base_url text, encrypted_cube_api_key bytea, encrypted_cube_api_key_iv bytea, + dashboard_limit_enabled boolean DEFAULT false NOT NULL, + dashboard_limit integer DEFAULT 0 NOT NULL, + dashboard_notification_limit integer DEFAULT 0 NOT NULL, + dashboard_enforcement_limit integer DEFAULT 0 NOT NULL, + dashboard_limit_new_namespace_creation_enforcement_date date, jitsu_host text, jitsu_project_xid text, clickhouse_connection_string text, jitsu_administrator_email text, encrypted_jitsu_administrator_password bytea, encrypted_jitsu_administrator_password_iv bytea, - dashboard_limit_enabled boolean DEFAULT false NOT NULL, - dashboard_limit integer DEFAULT 0 NOT NULL, - dashboard_notification_limit integer DEFAULT 0 NOT NULL, - dashboard_enforcement_limit integer DEFAULT 0 NOT NULL, - dashboard_limit_new_namespace_creation_enforcement_date date, can_create_group boolean DEFAULT true NOT NULL, lock_maven_package_requests_forwarding boolean DEFAULT false NOT NULL, lock_pypi_package_requests_forwarding boolean DEFAULT false NOT NULL, lock_npm_package_requests_forwarding boolean DEFAULT false NOT NULL, - jira_connect_proxy_url text, password_expiration_enabled boolean DEFAULT false NOT NULL, password_expires_in_days integer DEFAULT 90 NOT NULL, password_expires_notice_before_days integer DEFAULT 7 NOT NULL, product_analytics_enabled boolean DEFAULT false NOT NULL, + jira_connect_proxy_url text, email_confirmation_setting smallint DEFAULT 0, disable_admin_oauth_scopes boolean DEFAULT false NOT NULL, default_preferred_language text DEFAULT 'en'::text NOT NULL, @@ -12741,26 +12761,6 @@ CREATE SEQUENCE ci_builds_id_seq ALTER SEQUENCE ci_builds_id_seq OWNED BY ci_builds.id; -CREATE TABLE p_ci_builds_metadata ( - project_id integer NOT NULL, - timeout integer, - timeout_source integer DEFAULT 1 NOT NULL, - interruptible boolean, - config_options jsonb, - config_variables jsonb, - has_exposed_artifacts boolean, - environment_auto_stop_in character varying(255), - expanded_environment_name character varying(255), - secrets jsonb DEFAULT '{}'::jsonb NOT NULL, - build_id bigint NOT NULL, - id bigint NOT NULL, - runtime_runner_features jsonb DEFAULT '{}'::jsonb NOT NULL, - id_tokens jsonb DEFAULT '{}'::jsonb NOT NULL, - partition_id bigint DEFAULT 100 NOT NULL, - debug_trace_enabled boolean DEFAULT false NOT NULL -) -PARTITION BY LIST (partition_id); - CREATE SEQUENCE ci_builds_metadata_id_seq START WITH 1 INCREMENT BY 1 @@ -18202,13 +18202,13 @@ CREATE TABLE namespace_settings ( runner_token_expiration_interval integer, subgroup_runner_token_expiration_interval integer, project_runner_token_expiration_interval integer, - show_diff_preview_in_email boolean DEFAULT true NOT NULL, enabled_git_access_protocol smallint DEFAULT 0 NOT NULL, unique_project_download_limit smallint DEFAULT 0 NOT NULL, unique_project_download_limit_interval_in_seconds integer DEFAULT 0 NOT NULL, project_import_level smallint DEFAULT 50 NOT NULL, unique_project_download_limit_allowlist text[] DEFAULT '{}'::text[] NOT NULL, auto_ban_user_on_excessive_projects_download boolean DEFAULT false NOT NULL, + show_diff_preview_in_email boolean DEFAULT true NOT NULL, only_allow_merge_if_pipeline_succeeds boolean DEFAULT false NOT NULL, allow_merge_on_skipped_pipeline boolean DEFAULT false NOT NULL, only_allow_merge_if_all_discussions_are_resolved boolean DEFAULT false NOT NULL, @@ -20440,11 +20440,11 @@ CREATE TABLE project_settings ( target_platforms character varying[] DEFAULT '{}'::character varying[] NOT NULL, enforce_auth_checks_on_uploads boolean DEFAULT true NOT NULL, selective_code_owner_removals boolean DEFAULT false NOT NULL, - issue_branch_template text, show_diff_preview_in_email boolean DEFAULT true NOT NULL, - jitsu_key text, suggested_reviewers_enabled boolean DEFAULT false NOT NULL, + jitsu_key text, only_allow_merge_if_all_status_checks_passed boolean DEFAULT false NOT NULL, + issue_branch_template text, mirror_branch_regex text, allow_pipeline_trigger_approve_deployment boolean DEFAULT false NOT NULL, CONSTRAINT check_2981f15877 CHECK ((char_length(jitsu_key) <= 100)), @@ -28661,10 +28661,6 @@ CREATE UNIQUE INDEX p_ci_builds_metadata_build_id_partition_id_idx ON ONLY p_ci_ CREATE UNIQUE INDEX index_ci_builds_metadata_on_build_id_partition_id_unique ON ci_builds_metadata USING btree (build_id, partition_id); -CREATE UNIQUE INDEX p_ci_builds_metadata_id_partition_id_idx ON ONLY p_ci_builds_metadata USING btree (id, partition_id); - -CREATE UNIQUE INDEX index_ci_builds_metadata_on_id_partition_id_unique ON ci_builds_metadata USING btree (id, partition_id); - CREATE INDEX p_ci_builds_metadata_project_id_idx ON ONLY p_ci_builds_metadata USING btree (project_id); CREATE INDEX index_ci_builds_metadata_on_project_id ON ci_builds_metadata USING btree (project_id); @@ -32981,8 +32977,6 @@ ALTER INDEX p_ci_builds_metadata_build_id_id_idx ATTACH PARTITION index_ci_build ALTER INDEX p_ci_builds_metadata_build_id_partition_id_idx ATTACH PARTITION index_ci_builds_metadata_on_build_id_partition_id_unique; -ALTER INDEX p_ci_builds_metadata_id_partition_id_idx ATTACH PARTITION index_ci_builds_metadata_on_id_partition_id_unique; - ALTER INDEX p_ci_builds_metadata_project_id_idx ATTACH PARTITION index_ci_builds_metadata_on_project_id; CREATE TRIGGER chat_names_loose_fk_trigger AFTER DELETE ON chat_names REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records(); diff --git a/ee/app/assets/javascripts/invite_members/constants.js b/ee/app/assets/javascripts/invite_members/constants.js index 1abd12ef7bcf66..d1f75f5cf91ab9 100644 --- a/ee/app/assets/javascripts/invite_members/constants.js +++ b/ee/app/assets/javascripts/invite_members/constants.js @@ -1,6 +1,14 @@ import { __, s__, n__, sprintf } from '~/locale'; import { helpPagePath } from '~/helpers/help_page_helper'; +export const overageModalFields = Object.freeze({ + OVERAGE_MODAL_LINK: helpPagePath('subscriptions/quarterly_reconciliation'), + OVERAGE_MODAL_TITLE: s__('MembersOverage|You are about to incur additional charges'), + OVERAGE_MODAL_BACK_BUTTON: __('Back'), + OVERAGE_MODAL_CONTINUE_BUTTON: __('Continue'), + OVERAGE_MODAL_LINK_TEXT: __('Learn more.'), +}); + export const OVERAGE_MODAL_LINK = helpPagePath('subscriptions/quarterly_reconciliation'); export const OVERAGE_MODAL_TITLE = s__('MembersOverage|You are about to incur additional charges'); diff --git a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js new file mode 100644 index 00000000000000..d2156ffefe1ac7 --- /dev/null +++ b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js @@ -0,0 +1,35 @@ +import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; +import { + overageModalFields, + overageModalInfoText, + overageModalInfoWarning, +} from '../invite_members/constants'; + +const shouldShowOverageModal = (dropdownIntValue) => { + return this.glFeatures.showOverageOnRolePromotion && dropdownIntValue === 10; +}; + +const getConfirmContent = (subscriptionSeats, totalUsers, groupName) => { + const infoText = overageModalInfoText(subscriptionSeats); + const infoWarning = overageModalInfoWarning(totalUsers, groupName); + + return `${infoText} ${infoWarning}`; +}; + +export const guestOverageConfirmAction = async ({ + dropdownIntValue, + subscriptionSeats, + totalUsers, + groupName, +}) => { + if (!shouldShowOverageModal(dropdownIntValue)) { + return true; + } + + const confirmContent = getConfirmContent(subscriptionSeats, totalUsers, groupName); + + return confirmAction(confirmContent, { + primaryBtnText: overageModalFields.OVERAGE_MODAL_CONTINUE_BUTTON, + cancelBtnText: overageModalFields.OVERAGE_MODAL_BACK_BUTTON, + }); +}; diff --git a/ee/app/controllers/ee/projects/project_members_controller.rb b/ee/app/controllers/ee/projects/project_members_controller.rb index 1e90f446ba038a..a8fa5477e3f803 100644 --- a/ee/app/controllers/ee/projects/project_members_controller.rb +++ b/ee/app/controllers/ee/projects/project_members_controller.rb @@ -8,6 +8,12 @@ module ProjectMembersController private + prepended do + before_action do + push_frontend_feature_flag(:show_overage_on_role_promotion) + end + end + override :invited_members def invited_members super.or(members.awaiting.with_invited_user_state) diff --git a/ee/config/feature_flags/development/show_overage_on_role_promotion.yml b/ee/config/feature_flags/development/show_overage_on_role_promotion.yml new file mode 100644 index 00000000000000..2af0c48712c0de --- /dev/null +++ b/ee/config/feature_flags/development/show_overage_on_role_promotion.yml @@ -0,0 +1,8 @@ +--- +name: show_overage_on_role_promotion +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89180/ +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/199893 +milestone: '15.2' +type: development +group: group::billing and subscription management +default_enabled: false diff --git a/ee/spec/features/projects/audit_events_spec.rb b/ee/spec/features/projects/audit_events_spec.rb index d7049ce4bbc62b..70710b83b08959 100644 --- a/ee/spec/features/projects/audit_events_spec.rb +++ b/ee/spec/features/projects/audit_events_spec.rb @@ -5,9 +5,9 @@ RSpec.describe 'Projects > Audit Events', :js, feature_category: :audit_events do include Spec::Support::Helpers::Features::MembersHelpers - let_it_be(:user) { create(:user) } - let_it_be(:pete) { create(:user, name: 'Pete') } - let_it_be_with_reload(:project) { create(:project, :repository, namespace: user.namespace) } + let(:user) { create(:user) } + let(:pete) { create(:user, name: 'Pete') } + let(:project) { create(:project, :repository, namespace: user.namespace) } before do project.add_maintainer(user) @@ -110,27 +110,60 @@ end describe 'changing a user access level' do - before do - project.add_developer(pete) - end + describe 'without show_overage_on_role_promotion feature flag' do + before do + stub_feature_flags(show_overage_on_role_promotion: false) + project.add_developer(pete) + end - it "appears in the project's audit events" do - visit project_project_members_path(project) + it "appears in the project's audit events" do + visit project_project_members_path(project) + + page.within find_member_row(pete) do + click_button 'Developer' + click_button 'Maintainer' + end - page.within find_member_row(pete) do - click_button 'Developer' - click_button 'Maintainer' + page.within('.sidebar-top-level-items') do + find(:link, text: 'Security & Compliance').click + click_link 'Audit events' + end + + page.within('.audit-log-table') do + expect(page).to have_content 'Changed access level from Developer to Maintainer' + expect(page).to have_content(project.first_owner.name) + expect(page).to have_content('Pete') + end end + end - page.within('.sidebar-top-level-items') do - find(:link, text: 'Security & Compliance').click - click_link 'Audit events' + describe 'with show_overage_on_role_promotion feature flag' do + before do + project.add_developer(pete) end - page.within('.audit-log-table') do - expect(page).to have_content 'Changed access level from Developer to Maintainer' - expect(page).to have_content(project.first_owner.name) - expect(page).to have_content('Pete') + it "appears in the project's audit events" do + visit project_project_members_path(project) + + page.within find_member_row(pete) do + click_button 'Developer' + click_button 'Maintainer' + end + + page.within('.modal') do + click_button 'Back' + end + + page.within('.sidebar-top-level-items') do + find(:link, text: 'Security & Compliance').click + click_link 'Audit events' + end + + page.within('.audit-log-table') do + expect(page).to have_content 'Changed access level from Developer to Maintainer' + expect(page).to have_content(project.first_owner.name) + expect(page).to have_content('Pete') + end end end end @@ -142,7 +175,7 @@ end it "appears in the project's audit events", :js do - visit project_settings_merge_requests_path(project) + visit edit_project_path(project) page.within('[data-testid="merge-request-approval-settings"]') do find('[data-testid="prevent-author-approval"] > input').set(false) @@ -171,10 +204,20 @@ end end + describe 'filter by date' do + let!(:audit_event_1) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: 5.days.ago) } + let!(:audit_event_2) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: 3.days.ago) } + let!(:audit_event_3) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: Date.current) } + let!(:events_path) { :project_audit_events_path } + let!(:entity) { project } + + it_behaves_like 'audit events date filter' + end + describe 'combined list of authenticated and unauthenticated users' do - let_it_be(:audit_event_1) { create(:project_audit_event, :unauthenticated, entity_type: 'Project', entity_id: project.id) } - let_it_be(:audit_event_2) { create(:project_audit_event, author_id: non_existing_record_id, entity_type: 'Project', entity_id: project.id) } - let_it_be(:audit_event_3) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id) } + let!(:audit_event_1) { create(:project_audit_event, :unauthenticated, entity_type: 'Project', entity_id: project.id) } + let!(:audit_event_2) { create(:project_audit_event, author_id: non_existing_record_id, entity_type: 'Project', entity_id: project.id) } + let!(:audit_event_3) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id) } it 'displays the correct authors names' do visit project_audit_events_path(project) @@ -188,32 +231,4 @@ end end end - - describe 'audit event filter' do - let_it_be(:events_path) { :project_audit_events_path } - let_it_be(:entity) { project } - - describe 'filter by date' do - let_it_be(:audit_event_1) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: 5.days.ago) } - let_it_be(:audit_event_2) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: 3.days.ago) } - let_it_be(:audit_event_3) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: Date.current) } - - it_behaves_like 'audit events date filter' - end - - context 'signed in as a developer' do - before do - project.add_developer(pete) - sign_in(pete) - end - - describe 'filter by author' do - let_it_be(:audit_event_1) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: Date.today, ip_address: '1.1.1.1', author_id: pete.id) } - let_it_be(:audit_event_2) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: Date.today, ip_address: '0.0.0.0', author_id: user.id) } - let_it_be(:author) { user } - - it_behaves_like 'audit events author filtering without entity admin permission' - end - end - end end -- GitLab From 46058d993414d024deafb1008b2d2cfc813eda3f Mon Sep 17 00:00:00 2001 From: lmeckley Date: Wed, 30 Nov 2022 21:16:21 -0700 Subject: [PATCH 2/7] Add link to confirm modal Use modalHtmlMessage for confirmAction --- .../components/table/role_dropdown.vue | 24 ++-- .../javascripts/invite_members/constants.js | 2 +- .../members/guest_overage_confirm_action.js | 25 +++- .../features/projects/audit_events_spec.rb | 115 ++++++++---------- 4 files changed, 87 insertions(+), 79 deletions(-) diff --git a/app/assets/javascripts/members/components/table/role_dropdown.vue b/app/assets/javascripts/members/components/table/role_dropdown.vue index b9efc6d1ea4ed0..4ed6a71e243dac 100644 --- a/app/assets/javascripts/members/components/table/role_dropdown.vue +++ b/app/assets/javascripts/members/components/table/role_dropdown.vue @@ -4,7 +4,6 @@ import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import { mapActions } from 'vuex'; import { s__ } from '~/locale'; import { - userOverageConfirmAction, guestOverageConfirmAction, } from 'ee_else_ce/members/guest_overage_confirm_action'; @@ -54,20 +53,26 @@ export default { return dispatch(`${this.namespace}/updateMemberRole`, payload); }, }), + async handleOverageConfirm(currentAccessLevel, value) { + return guestOverageConfirmAction({ + currentAccessIntValue: currentAccessLevel, + dropdownIntValue: value, + subscriptionSeats: 0, + totalUsers: 0, + groupName: 'a name', + }); + }, async handleSelect(value, name) { - if (value === this.member.accessLevel.integerValue) { + const currentAccessLevel = this.member.accessLevel.integerValue; + if (value === currentAccessLevel) { return; } this.busy = true; - const confirmed = await guestOverageConfirmAction({ - dropdownIntValue: value, - subscriptionSeats: 0, - totalUsers: 0, - groupName: __('a name'), - }); + const confirmed = await this.handleOverageConfirm(currentAccessLevel, value); if (!confirmed) { + this.busy = false; return; } @@ -78,6 +83,9 @@ export default { .then(() => { this.$toast.show(s__('Members|Role updated successfully.')); }) + .catch(() => { + //log? + }) .finally(() => { this.busy = false; }); diff --git a/ee/app/assets/javascripts/invite_members/constants.js b/ee/app/assets/javascripts/invite_members/constants.js index d1f75f5cf91ab9..f417a22d6b6727 100644 --- a/ee/app/assets/javascripts/invite_members/constants.js +++ b/ee/app/assets/javascripts/invite_members/constants.js @@ -6,7 +6,7 @@ export const overageModalFields = Object.freeze({ OVERAGE_MODAL_TITLE: s__('MembersOverage|You are about to incur additional charges'), OVERAGE_MODAL_BACK_BUTTON: __('Back'), OVERAGE_MODAL_CONTINUE_BUTTON: __('Continue'), - OVERAGE_MODAL_LINK_TEXT: __('Learn more.'), + OVERAGE_MODAL_LINK_TEXT: __('%{linkStart} Learn more%{linkEnd}.'), }); export const OVERAGE_MODAL_LINK = helpPagePath('subscriptions/quarterly_reconciliation'); diff --git a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js index d2156ffefe1ac7..686566527ac29e 100644 --- a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js +++ b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js @@ -1,3 +1,4 @@ +import { sprintf } from '~/locale'; import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; import { overageModalFields, @@ -5,30 +6,44 @@ import { overageModalInfoWarning, } from '../invite_members/constants'; -const shouldShowOverageModal = (dropdownIntValue) => { - return this.glFeatures.showOverageOnRolePromotion && dropdownIntValue === 10; +const shouldShowOverageModal = (currentAccessIntValue) => { + if (!gon.features.showOverageOnRolePromotion || currentAccessIntValue !== 10) { + return false; + } + + return true; }; const getConfirmContent = (subscriptionSeats, totalUsers, groupName) => { const infoText = overageModalInfoText(subscriptionSeats); const infoWarning = overageModalInfoWarning(totalUsers, groupName); + const link = sprintf( + overageModalFields.OVERAGE_MODAL_LINK_TEXT, + { + linkStart: ``, + linkEnd: '', + }, + false, + ); - return `${infoText} ${infoWarning}`; + return `${infoText} ${infoWarning} ${link}`; }; export const guestOverageConfirmAction = async ({ + currentAccessIntValue, dropdownIntValue, subscriptionSeats, totalUsers, groupName, }) => { - if (!shouldShowOverageModal(dropdownIntValue)) { + if (!shouldShowOverageModal(currentAccessIntValue, dropdownIntValue)) { return true; } const confirmContent = getConfirmContent(subscriptionSeats, totalUsers, groupName); - return confirmAction(confirmContent, { + return confirmAction('', { + modalHtmlMessage: confirmContent, primaryBtnText: overageModalFields.OVERAGE_MODAL_CONTINUE_BUTTON, cancelBtnText: overageModalFields.OVERAGE_MODAL_BACK_BUTTON, }); diff --git a/ee/spec/features/projects/audit_events_spec.rb b/ee/spec/features/projects/audit_events_spec.rb index 70710b83b08959..d7049ce4bbc62b 100644 --- a/ee/spec/features/projects/audit_events_spec.rb +++ b/ee/spec/features/projects/audit_events_spec.rb @@ -5,9 +5,9 @@ RSpec.describe 'Projects > Audit Events', :js, feature_category: :audit_events do include Spec::Support::Helpers::Features::MembersHelpers - let(:user) { create(:user) } - let(:pete) { create(:user, name: 'Pete') } - let(:project) { create(:project, :repository, namespace: user.namespace) } + let_it_be(:user) { create(:user) } + let_it_be(:pete) { create(:user, name: 'Pete') } + let_it_be_with_reload(:project) { create(:project, :repository, namespace: user.namespace) } before do project.add_maintainer(user) @@ -110,60 +110,27 @@ end describe 'changing a user access level' do - describe 'without show_overage_on_role_promotion feature flag' do - before do - stub_feature_flags(show_overage_on_role_promotion: false) - project.add_developer(pete) - end - - it "appears in the project's audit events" do - visit project_project_members_path(project) - - page.within find_member_row(pete) do - click_button 'Developer' - click_button 'Maintainer' - end + before do + project.add_developer(pete) + end - page.within('.sidebar-top-level-items') do - find(:link, text: 'Security & Compliance').click - click_link 'Audit events' - end + it "appears in the project's audit events" do + visit project_project_members_path(project) - page.within('.audit-log-table') do - expect(page).to have_content 'Changed access level from Developer to Maintainer' - expect(page).to have_content(project.first_owner.name) - expect(page).to have_content('Pete') - end + page.within find_member_row(pete) do + click_button 'Developer' + click_button 'Maintainer' end - end - describe 'with show_overage_on_role_promotion feature flag' do - before do - project.add_developer(pete) + page.within('.sidebar-top-level-items') do + find(:link, text: 'Security & Compliance').click + click_link 'Audit events' end - it "appears in the project's audit events" do - visit project_project_members_path(project) - - page.within find_member_row(pete) do - click_button 'Developer' - click_button 'Maintainer' - end - - page.within('.modal') do - click_button 'Back' - end - - page.within('.sidebar-top-level-items') do - find(:link, text: 'Security & Compliance').click - click_link 'Audit events' - end - - page.within('.audit-log-table') do - expect(page).to have_content 'Changed access level from Developer to Maintainer' - expect(page).to have_content(project.first_owner.name) - expect(page).to have_content('Pete') - end + page.within('.audit-log-table') do + expect(page).to have_content 'Changed access level from Developer to Maintainer' + expect(page).to have_content(project.first_owner.name) + expect(page).to have_content('Pete') end end end @@ -175,7 +142,7 @@ end it "appears in the project's audit events", :js do - visit edit_project_path(project) + visit project_settings_merge_requests_path(project) page.within('[data-testid="merge-request-approval-settings"]') do find('[data-testid="prevent-author-approval"] > input').set(false) @@ -204,20 +171,10 @@ end end - describe 'filter by date' do - let!(:audit_event_1) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: 5.days.ago) } - let!(:audit_event_2) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: 3.days.ago) } - let!(:audit_event_3) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: Date.current) } - let!(:events_path) { :project_audit_events_path } - let!(:entity) { project } - - it_behaves_like 'audit events date filter' - end - describe 'combined list of authenticated and unauthenticated users' do - let!(:audit_event_1) { create(:project_audit_event, :unauthenticated, entity_type: 'Project', entity_id: project.id) } - let!(:audit_event_2) { create(:project_audit_event, author_id: non_existing_record_id, entity_type: 'Project', entity_id: project.id) } - let!(:audit_event_3) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id) } + let_it_be(:audit_event_1) { create(:project_audit_event, :unauthenticated, entity_type: 'Project', entity_id: project.id) } + let_it_be(:audit_event_2) { create(:project_audit_event, author_id: non_existing_record_id, entity_type: 'Project', entity_id: project.id) } + let_it_be(:audit_event_3) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id) } it 'displays the correct authors names' do visit project_audit_events_path(project) @@ -231,4 +188,32 @@ end end end + + describe 'audit event filter' do + let_it_be(:events_path) { :project_audit_events_path } + let_it_be(:entity) { project } + + describe 'filter by date' do + let_it_be(:audit_event_1) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: 5.days.ago) } + let_it_be(:audit_event_2) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: 3.days.ago) } + let_it_be(:audit_event_3) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: Date.current) } + + it_behaves_like 'audit events date filter' + end + + context 'signed in as a developer' do + before do + project.add_developer(pete) + sign_in(pete) + end + + describe 'filter by author' do + let_it_be(:audit_event_1) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: Date.today, ip_address: '1.1.1.1', author_id: pete.id) } + let_it_be(:audit_event_2) { create(:project_audit_event, entity_type: 'Project', entity_id: project.id, created_at: Date.today, ip_address: '0.0.0.0', author_id: user.id) } + let_it_be(:author) { user } + + it_behaves_like 'audit events author filtering without entity admin permission' + end + end + end end -- GitLab From 3a7f38a45249887a1abab22e644e0506722b43f1 Mon Sep 17 00:00:00 2001 From: lmeckley Date: Thu, 1 Dec 2022 22:15:20 -0700 Subject: [PATCH 3/7] Add tests for guest overage modal Add overage confirm action tests --- .../components/table/role_dropdown.vue | 11 +- db/structure.sql | 80 +++++++------- .../members/guest_overage_confirm_action.js | 26 +++-- .../show_overage_on_role_promotion.yml | 2 +- .../guest_overage_confirm_action_spec.js | 103 ++++++++++++++++++ locale/gitlab.pot | 3 + .../components/table/role_dropdown_spec.js | 58 +++++++++- .../guest_overage_confirm_action_spec.js | 7 ++ 8 files changed, 233 insertions(+), 57 deletions(-) create mode 100644 ee/spec/frontend/members/guest_overage_confirm_action_spec.js create mode 100644 spec/frontend/members/guest_overage_confirm_action_spec.js diff --git a/app/assets/javascripts/members/components/table/role_dropdown.vue b/app/assets/javascripts/members/components/table/role_dropdown.vue index 4ed6a71e243dac..0f790ef28dab0d 100644 --- a/app/assets/javascripts/members/components/table/role_dropdown.vue +++ b/app/assets/javascripts/members/components/table/role_dropdown.vue @@ -3,9 +3,7 @@ import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import { mapActions } from 'vuex'; import { s__ } from '~/locale'; -import { - guestOverageConfirmAction, -} from 'ee_else_ce/members/guest_overage_confirm_action'; +import { guestOverageConfirmAction } from 'ee_else_ce/members/guest_overage_confirm_action'; export default { name: 'RoleDropdown', @@ -54,12 +52,9 @@ export default { }, }), async handleOverageConfirm(currentAccessLevel, value) { - return guestOverageConfirmAction({ + return guestOverageConfirmAction({ currentAccessIntValue: currentAccessLevel, dropdownIntValue: value, - subscriptionSeats: 0, - totalUsers: 0, - groupName: 'a name', }); }, async handleSelect(value, name) { @@ -84,7 +79,7 @@ export default { this.$toast.show(s__('Members|Role updated successfully.')); }) .catch(() => { - //log? + // log? }) .finally(() => { this.busy = false; diff --git a/db/structure.sql b/db/structure.sql index ecf4f7f949ce4e..72d9c94be491c6 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -417,26 +417,6 @@ CREATE TABLE batched_background_migration_job_transition_logs ( ) PARTITION BY RANGE (created_at); -CREATE TABLE p_ci_builds_metadata ( - project_id integer NOT NULL, - timeout integer, - timeout_source integer DEFAULT 1 NOT NULL, - interruptible boolean, - config_options jsonb, - config_variables jsonb, - has_exposed_artifacts boolean, - environment_auto_stop_in character varying(255), - expanded_environment_name character varying(255), - secrets jsonb DEFAULT '{}'::jsonb NOT NULL, - build_id bigint NOT NULL, - id bigint NOT NULL, - runtime_runner_features jsonb DEFAULT '{}'::jsonb NOT NULL, - id_tokens jsonb DEFAULT '{}'::jsonb NOT NULL, - partition_id bigint DEFAULT 100 NOT NULL, - debug_trace_enabled boolean DEFAULT false NOT NULL -) -PARTITION BY LIST (partition_id); - CREATE TABLE incident_management_pending_alert_escalations ( id bigint NOT NULL, rule_id bigint NOT NULL, @@ -11493,6 +11473,10 @@ CREATE TABLE application_settings ( database_grafana_api_url text, database_grafana_tag text, public_runner_releases_url text DEFAULT 'https://gitlab.com/api/v4/projects/gitlab-org%2Fgitlab-runner/releases'::text NOT NULL, + password_uppercase_required boolean DEFAULT false NOT NULL, + password_lowercase_required boolean DEFAULT false NOT NULL, + password_number_required boolean DEFAULT false NOT NULL, + password_symbol_required boolean DEFAULT false NOT NULL, encrypted_arkose_labs_public_api_key bytea, encrypted_arkose_labs_public_api_key_iv bytea, encrypted_arkose_labs_private_api_key bytea, @@ -11503,6 +11487,7 @@ CREATE TABLE application_settings ( inactive_projects_min_size_mb integer DEFAULT 0 NOT NULL, inactive_projects_send_warning_email_after_months integer DEFAULT 1 NOT NULL, delayed_group_deletion boolean DEFAULT true NOT NULL, + maven_package_requests_forwarding boolean DEFAULT true NOT NULL, arkose_labs_namespace text DEFAULT 'client'::text NOT NULL, max_export_size integer DEFAULT 0, encrypted_slack_app_signing_secret bytea, @@ -11517,12 +11502,8 @@ CREATE TABLE application_settings ( encrypted_dingtalk_app_key_iv bytea, encrypted_dingtalk_app_secret bytea, encrypted_dingtalk_app_secret_iv bytea, - globally_allowed_ips text DEFAULT ''::text NOT NULL, - password_uppercase_required boolean DEFAULT false NOT NULL, - password_lowercase_required boolean DEFAULT false NOT NULL, - password_number_required boolean DEFAULT false NOT NULL, - password_symbol_required boolean DEFAULT false NOT NULL, jira_connect_application_key text, + globally_allowed_ips text DEFAULT ''::text NOT NULL, container_registry_pre_import_tags_rate numeric(6,2) DEFAULT 0.5 NOT NULL, license_usage_data_exported boolean DEFAULT false NOT NULL, phone_verification_code_enabled boolean DEFAULT false NOT NULL, @@ -11533,39 +11514,38 @@ CREATE TABLE application_settings ( encrypted_feishu_app_key_iv bytea, encrypted_feishu_app_secret bytea, encrypted_feishu_app_secret_iv bytea, - git_rate_limit_users_allowlist text[] DEFAULT '{}'::text[] NOT NULL, error_tracking_enabled boolean DEFAULT false NOT NULL, error_tracking_api_url text, + git_rate_limit_users_allowlist text[] DEFAULT '{}'::text[] NOT NULL, error_tracking_access_token_encrypted text, + invitation_flow_enforcement boolean DEFAULT false NOT NULL, package_registry_cleanup_policies_worker_capacity integer DEFAULT 2 NOT NULL, deactivate_dormant_users_period integer DEFAULT 90 NOT NULL, auto_ban_user_on_excessive_projects_download boolean DEFAULT false NOT NULL, - invitation_flow_enforcement boolean DEFAULT false NOT NULL, - maven_package_requests_forwarding boolean DEFAULT true NOT NULL, max_pages_custom_domains_per_project integer DEFAULT 0 NOT NULL, cube_api_base_url text, encrypted_cube_api_key bytea, encrypted_cube_api_key_iv bytea, - dashboard_limit_enabled boolean DEFAULT false NOT NULL, - dashboard_limit integer DEFAULT 0 NOT NULL, - dashboard_notification_limit integer DEFAULT 0 NOT NULL, - dashboard_enforcement_limit integer DEFAULT 0 NOT NULL, - dashboard_limit_new_namespace_creation_enforcement_date date, jitsu_host text, jitsu_project_xid text, clickhouse_connection_string text, jitsu_administrator_email text, encrypted_jitsu_administrator_password bytea, encrypted_jitsu_administrator_password_iv bytea, + dashboard_limit_enabled boolean DEFAULT false NOT NULL, + dashboard_limit integer DEFAULT 0 NOT NULL, + dashboard_notification_limit integer DEFAULT 0 NOT NULL, + dashboard_enforcement_limit integer DEFAULT 0 NOT NULL, + dashboard_limit_new_namespace_creation_enforcement_date date, can_create_group boolean DEFAULT true NOT NULL, lock_maven_package_requests_forwarding boolean DEFAULT false NOT NULL, lock_pypi_package_requests_forwarding boolean DEFAULT false NOT NULL, lock_npm_package_requests_forwarding boolean DEFAULT false NOT NULL, + jira_connect_proxy_url text, password_expiration_enabled boolean DEFAULT false NOT NULL, password_expires_in_days integer DEFAULT 90 NOT NULL, password_expires_notice_before_days integer DEFAULT 7 NOT NULL, product_analytics_enabled boolean DEFAULT false NOT NULL, - jira_connect_proxy_url text, email_confirmation_setting smallint DEFAULT 0, disable_admin_oauth_scopes boolean DEFAULT false NOT NULL, default_preferred_language text DEFAULT 'en'::text NOT NULL, @@ -12761,6 +12741,26 @@ CREATE SEQUENCE ci_builds_id_seq ALTER SEQUENCE ci_builds_id_seq OWNED BY ci_builds.id; +CREATE TABLE p_ci_builds_metadata ( + project_id integer NOT NULL, + timeout integer, + timeout_source integer DEFAULT 1 NOT NULL, + interruptible boolean, + config_options jsonb, + config_variables jsonb, + has_exposed_artifacts boolean, + environment_auto_stop_in character varying(255), + expanded_environment_name character varying(255), + secrets jsonb DEFAULT '{}'::jsonb NOT NULL, + build_id bigint NOT NULL, + id bigint NOT NULL, + runtime_runner_features jsonb DEFAULT '{}'::jsonb NOT NULL, + id_tokens jsonb DEFAULT '{}'::jsonb NOT NULL, + partition_id bigint DEFAULT 100 NOT NULL, + debug_trace_enabled boolean DEFAULT false NOT NULL +) +PARTITION BY LIST (partition_id); + CREATE SEQUENCE ci_builds_metadata_id_seq START WITH 1 INCREMENT BY 1 @@ -18202,13 +18202,13 @@ CREATE TABLE namespace_settings ( runner_token_expiration_interval integer, subgroup_runner_token_expiration_interval integer, project_runner_token_expiration_interval integer, + show_diff_preview_in_email boolean DEFAULT true NOT NULL, enabled_git_access_protocol smallint DEFAULT 0 NOT NULL, unique_project_download_limit smallint DEFAULT 0 NOT NULL, unique_project_download_limit_interval_in_seconds integer DEFAULT 0 NOT NULL, project_import_level smallint DEFAULT 50 NOT NULL, unique_project_download_limit_allowlist text[] DEFAULT '{}'::text[] NOT NULL, auto_ban_user_on_excessive_projects_download boolean DEFAULT false NOT NULL, - show_diff_preview_in_email boolean DEFAULT true NOT NULL, only_allow_merge_if_pipeline_succeeds boolean DEFAULT false NOT NULL, allow_merge_on_skipped_pipeline boolean DEFAULT false NOT NULL, only_allow_merge_if_all_discussions_are_resolved boolean DEFAULT false NOT NULL, @@ -20440,11 +20440,11 @@ CREATE TABLE project_settings ( target_platforms character varying[] DEFAULT '{}'::character varying[] NOT NULL, enforce_auth_checks_on_uploads boolean DEFAULT true NOT NULL, selective_code_owner_removals boolean DEFAULT false NOT NULL, + issue_branch_template text, show_diff_preview_in_email boolean DEFAULT true NOT NULL, - suggested_reviewers_enabled boolean DEFAULT false NOT NULL, jitsu_key text, + suggested_reviewers_enabled boolean DEFAULT false NOT NULL, only_allow_merge_if_all_status_checks_passed boolean DEFAULT false NOT NULL, - issue_branch_template text, mirror_branch_regex text, allow_pipeline_trigger_approve_deployment boolean DEFAULT false NOT NULL, CONSTRAINT check_2981f15877 CHECK ((char_length(jitsu_key) <= 100)), @@ -28661,6 +28661,10 @@ CREATE UNIQUE INDEX p_ci_builds_metadata_build_id_partition_id_idx ON ONLY p_ci_ CREATE UNIQUE INDEX index_ci_builds_metadata_on_build_id_partition_id_unique ON ci_builds_metadata USING btree (build_id, partition_id); +CREATE UNIQUE INDEX p_ci_builds_metadata_id_partition_id_idx ON ONLY p_ci_builds_metadata USING btree (id, partition_id); + +CREATE UNIQUE INDEX index_ci_builds_metadata_on_id_partition_id_unique ON ci_builds_metadata USING btree (id, partition_id); + CREATE INDEX p_ci_builds_metadata_project_id_idx ON ONLY p_ci_builds_metadata USING btree (project_id); CREATE INDEX index_ci_builds_metadata_on_project_id ON ci_builds_metadata USING btree (project_id); @@ -32977,6 +32981,8 @@ ALTER INDEX p_ci_builds_metadata_build_id_id_idx ATTACH PARTITION index_ci_build ALTER INDEX p_ci_builds_metadata_build_id_partition_id_idx ATTACH PARTITION index_ci_builds_metadata_on_build_id_partition_id_unique; +ALTER INDEX p_ci_builds_metadata_id_partition_id_idx ATTACH PARTITION index_ci_builds_metadata_on_id_partition_id_unique; + ALTER INDEX p_ci_builds_metadata_project_id_idx ATTACH PARTITION index_ci_builds_metadata_on_project_id; CREATE TRIGGER chat_names_loose_fk_trigger AFTER DELETE ON chat_names REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records(); diff --git a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js index 686566527ac29e..d55c1fe0775af5 100644 --- a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js +++ b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js @@ -6,15 +6,21 @@ import { overageModalInfoWarning, } from '../invite_members/constants'; -const shouldShowOverageModal = (currentAccessIntValue) => { - if (!gon.features.showOverageOnRolePromotion || currentAccessIntValue !== 10) { +const shouldShowOverageModal = (currentAccessIntValue, dropdownValue) => { + // When value is 10, the user is set to Guest + // When the value is > 10, the user perms are set to higher than a Guest + if ( + !gon.features.showOverageOnRolePromotion || + currentAccessIntValue !== 10 || + dropdownValue < 10 + ) { return false; } return true; }; -const getConfirmContent = (subscriptionSeats, totalUsers, groupName) => { +const getConfirmContent = ({ subscriptionSeats, totalUsers, groupName }) => { const infoText = overageModalInfoText(subscriptionSeats); const infoWarning = overageModalInfoWarning(totalUsers, groupName); const link = sprintf( @@ -30,17 +36,17 @@ const getConfirmContent = (subscriptionSeats, totalUsers, groupName) => { }; export const guestOverageConfirmAction = async ({ - currentAccessIntValue, - dropdownIntValue, - subscriptionSeats, - totalUsers, - groupName, -}) => { + currentAccessIntValue = 0, + dropdownIntValue = 0, + subscriptionSeats = 0, + totalUsers = 0, + groupName = '', +} = {}) => { if (!shouldShowOverageModal(currentAccessIntValue, dropdownIntValue)) { return true; } - const confirmContent = getConfirmContent(subscriptionSeats, totalUsers, groupName); + const confirmContent = getConfirmContent({ subscriptionSeats, totalUsers, groupName }); return confirmAction('', { modalHtmlMessage: confirmContent, diff --git a/ee/config/feature_flags/development/show_overage_on_role_promotion.yml b/ee/config/feature_flags/development/show_overage_on_role_promotion.yml index 2af0c48712c0de..2342c1551cb0be 100644 --- a/ee/config/feature_flags/development/show_overage_on_role_promotion.yml +++ b/ee/config/feature_flags/development/show_overage_on_role_promotion.yml @@ -2,7 +2,7 @@ name: show_overage_on_role_promotion introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89180/ rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/199893 -milestone: '15.2' +milestone: '15.7' type: development group: group::billing and subscription management default_enabled: false diff --git a/ee/spec/frontend/members/guest_overage_confirm_action_spec.js b/ee/spec/frontend/members/guest_overage_confirm_action_spec.js new file mode 100644 index 00000000000000..ed4d4b92adae01 --- /dev/null +++ b/ee/spec/frontend/members/guest_overage_confirm_action_spec.js @@ -0,0 +1,103 @@ +import { guestOverageConfirmAction } from 'ee/members/guest_overage_confirm_action'; +import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; + +jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'); + +describe('Guest Overage Confirm Action', () => { + let originalGon; + + beforeAll(() => { + originalGon = window.gon; + }); + + afterEach(() => { + window.gon = originalGon; + }); + + describe('when shouldShowOverageModal returns false', () => { + describe('with feature flag false', () => { + beforeEach(() => { + gon.features = { showOverageOnRolePromotion: false }; + }); + + it('returns confirmed', async () => { + const confirmReturn = await guestOverageConfirmAction(); + + expect(confirmReturn).toBe(true); + }); + }); + + describe('when Guest is not current the access level', () => { + it('returns confirmed', async () => { + const confirmReturn = await guestOverageConfirmAction({ currentAccessIntValue: 20 }); + + expect(confirmReturn).toBe(true); + }); + }); + + describe('when access is set to less than Guest', () => { + it('returns confirmed', async () => { + const confirmReturn = await guestOverageConfirmAction({ + currentAccessIntValue: 10, + dropdownIntValue: 5, + }); + + expect(confirmReturn).toBe(true); + }); + }); + }); + + describe('when the modal shows', () => { + let confirmReturn; + + beforeEach(() => { + gon.features = { showOverageOnRolePromotion: true }; + }); + + afterEach(() => { + confirmAction.mockReset(); + }); + + describe('calls underlying confirmAction', () => { + beforeEach(async () => { + confirmReturn = await guestOverageConfirmAction({ + currentAccessIntValue: 10, + dropdownIntValue: 30, + }); + }); + + it('calls confirmAction', () => { + expect(confirmAction).toHaveBeenCalled(); + }); + }); + + describe('when confirmAction returns true', () => { + beforeEach(async () => { + confirmAction.mockResolvedValue(true); + + confirmReturn = await guestOverageConfirmAction({ + currentAccessIntValue: 10, + dropdownIntValue: 30, + }); + }); + + it('guestOverageConfirmAction returns true', () => { + expect(confirmReturn).toBe(true); + }); + }); + + describe('when confirmAction returns false', () => {}); + beforeEach(async () => { + confirmAction.mockResolvedValue(false); + + confirmReturn = await guestOverageConfirmAction({ + currentAccessIntValue: 10, + dropdownIntValue: 30, + }); + }); + + it('guestOverageConfirmAction returns false', () => { + expect(confirmReturn).toBe(false); + }); + }); +}); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5b6dd92358b462..1c1a179c89c817 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -825,6 +825,9 @@ msgstr "" msgid "%{level_name} is not allowed since the fork source project has lower visibility." msgstr "" +msgid "%{linkStart} Learn more%{linkEnd}." +msgstr "" + msgid "%{link_start}Remove the %{draft_snippet} prefix%{link_end} from the title to allow this merge request to be merged when it's ready." msgstr "" diff --git a/spec/frontend/members/components/table/role_dropdown_spec.js b/spec/frontend/members/components/table/role_dropdown_spec.js index b254cce4d72119..b8ad6f8a77eef5 100644 --- a/spec/frontend/members/components/table/role_dropdown_spec.js +++ b/spec/frontend/members/components/table/role_dropdown_spec.js @@ -4,11 +4,14 @@ import { within } from '@testing-library/dom'; import { mount, createWrapper } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; import Vuex from 'vuex'; +import waitForPromises from 'helpers/wait_for_promises'; import RoleDropdown from '~/members/components/table/role_dropdown.vue'; import { MEMBER_TYPES } from '~/members/constants'; +import { guestOverageConfirmAction } from 'ee_else_ce/members/guest_overage_confirm_action'; import { member } from '../../mock_data'; Vue.use(Vuex); +jest.mock('ee_else_ce/members/guest_overage_confirm_action'); describe('RoleDropdown', () => { let wrapper; @@ -63,12 +66,21 @@ describe('RoleDropdown', () => { const findDropdownToggle = () => wrapper.find('button[aria-haspopup="true"]'); const findDropdown = () => wrapper.findComponent(GlDropdown); + let originalGon; + + beforeEach(() => { + originalGon = window.gon; + gon.features = { showOverageOnRolePromotion: true }; + }); + afterEach(() => { + window.gon = originalGon; wrapper.destroy(); }); describe('when dropdown is open', () => { beforeEach(() => { + guestOverageConfirmAction.mockReturnValue(true); createComponent(); return findDropdownToggle().trigger('click'); @@ -117,8 +129,12 @@ describe('RoleDropdown', () => { await getDropdownItemByText('Developer').trigger('click'); expect(findDropdown().props('disabled')).toBe(true); + }); - await nextTick(); + it('enables dropdown after `updateMemberRole` resolves', async () => { + await getDropdownItemByText('Developer').trigger('click'); + + await waitForPromises(); expect(findDropdown().props('disabled')).toBe(false); }); @@ -148,4 +164,44 @@ describe('RoleDropdown', () => { expect(findDropdown().props('right')).toBe(false); }); + + describe('when guestOverageConfirmAction', () => { + const mockConfirmAction = ({ confirmed }) => { + guestOverageConfirmAction.mockResolvedValueOnce(confirmed); + }; + + beforeEach(async () => { + await createComponent(); + + findDropdownToggle().trigger('click'); + }); + + afterEach(() => { + guestOverageConfirmAction.mockReset(); + }); + + describe('returns true', () => { + beforeEach(() => { + mockConfirmAction({ confirmed: true }); + + getDropdownItemByText('Reporter').trigger('click'); + }); + + it('allows updateMemberRole call', () => { + expect(actions.updateMemberRole).toHaveBeenCalled(); + }); + }); + + describe('returns false', () => { + beforeEach(() => { + mockConfirmAction({ confirmed: false }); + + getDropdownItemByText('Reporter').trigger('click'); + }); + + it('does not allow updateMemberRole call', () => { + expect(actions.updateMemberRole).not.toHaveBeenCalled(); + }); + }); + }); }); diff --git a/spec/frontend/members/guest_overage_confirm_action_spec.js b/spec/frontend/members/guest_overage_confirm_action_spec.js new file mode 100644 index 00000000000000..92b3bcd416754c --- /dev/null +++ b/spec/frontend/members/guest_overage_confirm_action_spec.js @@ -0,0 +1,7 @@ +import { guestOverageConfirmAction } from '~/members/guest_overage_confirm_action'; + +describe('Guest Overage Confirm Action', () => { + it('returns true', () => { + expect(guestOverageConfirmAction()).toBe(true); + }); +}); -- GitLab From a68c9dd9f6dc3009a5ad716897824dac60d14e45 Mon Sep 17 00:00:00 2001 From: snachnolkar Date: Tue, 13 Dec 2022 11:30:55 +0530 Subject: [PATCH 4/7] Update specs and minor changes Updated specs and added minor changes to move constants and update feature flag information. --- .../components/table/role_dropdown.vue | 4 +-- .../javascripts/invite_members/constants.js | 12 +++---- .../assets/javascripts/members/constants.js | 4 +++ .../members/guest_overage_confirm_action.js | 28 ++++++++-------- .../show_overage_on_role_promotion.yml | 4 +-- .../guest_overage_confirm_action_spec.js | 33 +++++++++++-------- .../components/table/role_dropdown_spec.js | 14 ++++---- .../guest_overage_confirm_action_spec.js | 2 +- 8 files changed, 55 insertions(+), 46 deletions(-) diff --git a/app/assets/javascripts/members/components/table/role_dropdown.vue b/app/assets/javascripts/members/components/table/role_dropdown.vue index 0f790ef28dab0d..a936937a8ca9cf 100644 --- a/app/assets/javascripts/members/components/table/role_dropdown.vue +++ b/app/assets/javascripts/members/components/table/role_dropdown.vue @@ -78,9 +78,7 @@ export default { .then(() => { this.$toast.show(s__('Members|Role updated successfully.')); }) - .catch(() => { - // log? - }) + .catch(() => {}) .finally(() => { this.busy = false; }); diff --git a/ee/app/assets/javascripts/invite_members/constants.js b/ee/app/assets/javascripts/invite_members/constants.js index f417a22d6b6727..833cb164283431 100644 --- a/ee/app/assets/javascripts/invite_members/constants.js +++ b/ee/app/assets/javascripts/invite_members/constants.js @@ -1,12 +1,12 @@ import { __, s__, n__, sprintf } from '~/locale'; import { helpPagePath } from '~/helpers/help_page_helper'; -export const overageModalFields = Object.freeze({ - OVERAGE_MODAL_LINK: helpPagePath('subscriptions/quarterly_reconciliation'), - OVERAGE_MODAL_TITLE: s__('MembersOverage|You are about to incur additional charges'), - OVERAGE_MODAL_BACK_BUTTON: __('Back'), - OVERAGE_MODAL_CONTINUE_BUTTON: __('Continue'), - OVERAGE_MODAL_LINK_TEXT: __('%{linkStart} Learn more%{linkEnd}.'), +export const GUEST_OVERAGE_MODAL_FIELDS = Object.freeze({ + TITLE: __('You are about to incur additional charges'), + LINK: helpPagePath('subscriptions/quarterly_reconciliation'), + BACK_BUTTON_LABEL: __('Back'), + CONTINUE_BUTTON_LABEL: __('Continue'), + LINK_TEXT: __('%{linkStart} Learn more%{linkEnd}.'), }); export const OVERAGE_MODAL_LINK = helpPagePath('subscriptions/quarterly_reconciliation'); diff --git a/ee/app/assets/javascripts/members/constants.js b/ee/app/assets/javascripts/members/constants.js index f146f631f77bc0..969f1832a36bb2 100644 --- a/ee/app/assets/javascripts/members/constants.js +++ b/ee/app/assets/javascripts/members/constants.js @@ -74,3 +74,7 @@ export const EE_APP_OPTIONS = uniqueProjectDownloadLimitEnabled }, } : {}; + +export const MEMBER_ACCESS_LEVELS = { + GUEST: 10, +}; diff --git a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js index d55c1fe0775af5..a9143144b0d83d 100644 --- a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js +++ b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js @@ -1,32 +1,31 @@ import { sprintf } from '~/locale'; import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; import { - overageModalFields, + GUEST_OVERAGE_MODAL_FIELDS, overageModalInfoText, overageModalInfoWarning, } from '../invite_members/constants'; +import { MEMBER_ACCESS_LEVELS } from './constants'; const shouldShowOverageModal = (currentAccessIntValue, dropdownValue) => { - // When value is 10, the user is set to Guest - // When the value is > 10, the user perms are set to higher than a Guest if ( - !gon.features.showOverageOnRolePromotion || - currentAccessIntValue !== 10 || - dropdownValue < 10 + gon.features.showOverageOnRolePromotion && + currentAccessIntValue === MEMBER_ACCESS_LEVELS.GUEST && + dropdownValue > MEMBER_ACCESS_LEVELS.GUEST ) { - return false; + return true; } - return true; + return false; }; const getConfirmContent = ({ subscriptionSeats, totalUsers, groupName }) => { const infoText = overageModalInfoText(subscriptionSeats); const infoWarning = overageModalInfoWarning(totalUsers, groupName); const link = sprintf( - overageModalFields.OVERAGE_MODAL_LINK_TEXT, + GUEST_OVERAGE_MODAL_FIELDS.LINK_TEXT, { - linkStart: ``, + linkStart: ``, linkEnd: '', }, false, @@ -36,8 +35,8 @@ const getConfirmContent = ({ subscriptionSeats, totalUsers, groupName }) => { }; export const guestOverageConfirmAction = async ({ - currentAccessIntValue = 0, - dropdownIntValue = 0, + currentAccessIntValue, + dropdownIntValue, subscriptionSeats = 0, totalUsers = 0, groupName = '', @@ -49,8 +48,9 @@ export const guestOverageConfirmAction = async ({ const confirmContent = getConfirmContent({ subscriptionSeats, totalUsers, groupName }); return confirmAction('', { + title: GUEST_OVERAGE_MODAL_FIELDS.TITLE, modalHtmlMessage: confirmContent, - primaryBtnText: overageModalFields.OVERAGE_MODAL_CONTINUE_BUTTON, - cancelBtnText: overageModalFields.OVERAGE_MODAL_BACK_BUTTON, + primaryBtnText: GUEST_OVERAGE_MODAL_FIELDS.CONTINUE_BUTTON_LABEL, + cancelBtnText: GUEST_OVERAGE_MODAL_FIELDS.BACK_BUTTON_LABEL, }); }; diff --git a/ee/config/feature_flags/development/show_overage_on_role_promotion.yml b/ee/config/feature_flags/development/show_overage_on_role_promotion.yml index 2342c1551cb0be..a9d81afd564e8c 100644 --- a/ee/config/feature_flags/development/show_overage_on_role_promotion.yml +++ b/ee/config/feature_flags/development/show_overage_on_role_promotion.yml @@ -1,7 +1,7 @@ --- name: show_overage_on_role_promotion -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/89180/ -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/199893 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/105613 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/385496 milestone: '15.7' type: development group: group::billing and subscription management diff --git a/ee/spec/frontend/members/guest_overage_confirm_action_spec.js b/ee/spec/frontend/members/guest_overage_confirm_action_spec.js index ed4d4b92adae01..0e734a27ae2f77 100644 --- a/ee/spec/frontend/members/guest_overage_confirm_action_spec.js +++ b/ee/spec/frontend/members/guest_overage_confirm_action_spec.js @@ -1,9 +1,10 @@ import { guestOverageConfirmAction } from 'ee/members/guest_overage_confirm_action'; import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; +import { MEMBER_ACCESS_LEVELS } from 'ee/members/constants'; jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'); -describe('Guest Overage Confirm Action', () => { +describe('guestOverageConfirmAction', () => { let originalGon; beforeAll(() => { @@ -15,30 +16,36 @@ describe('Guest Overage Confirm Action', () => { }); describe('when shouldShowOverageModal returns false', () => { - describe('with feature flag false', () => { + describe('when showOverageOnRolePromotion feature flag is set to false', () => { beforeEach(() => { gon.features = { showOverageOnRolePromotion: false }; }); - it('returns confirmed', async () => { - const confirmReturn = await guestOverageConfirmAction(); + it('returns true', async () => { + const confirmReturn = await guestOverageConfirmAction({ + currentAccessIntValue: 20, + dropdownIntValue: 30, + }); expect(confirmReturn).toBe(true); }); }); - describe('when Guest is not current the access level', () => { - it('returns confirmed', async () => { - const confirmReturn = await guestOverageConfirmAction({ currentAccessIntValue: 20 }); + describe('when current access level is not guest', () => { + it('returns true', async () => { + const confirmReturn = await guestOverageConfirmAction({ + currentAccessIntValue: 20, + dropdownIntValue: 30, + }); expect(confirmReturn).toBe(true); }); }); - describe('when access is set to less than Guest', () => { + describe('when access is set to less than guest', () => { it('returns confirmed', async () => { const confirmReturn = await guestOverageConfirmAction({ - currentAccessIntValue: 10, + currentAccessIntValue: MEMBER_ACCESS_LEVELS.GUEST, dropdownIntValue: 5, }); @@ -47,7 +54,7 @@ describe('Guest Overage Confirm Action', () => { }); }); - describe('when the modal shows', () => { + describe('when shouldShowOverageModal returns true', () => { let confirmReturn; beforeEach(() => { @@ -61,7 +68,7 @@ describe('Guest Overage Confirm Action', () => { describe('calls underlying confirmAction', () => { beforeEach(async () => { confirmReturn = await guestOverageConfirmAction({ - currentAccessIntValue: 10, + currentAccessIntValue: MEMBER_ACCESS_LEVELS.GUEST, dropdownIntValue: 30, }); }); @@ -76,7 +83,7 @@ describe('Guest Overage Confirm Action', () => { confirmAction.mockResolvedValue(true); confirmReturn = await guestOverageConfirmAction({ - currentAccessIntValue: 10, + currentAccessIntValue: MEMBER_ACCESS_LEVELS.GUEST, dropdownIntValue: 30, }); }); @@ -91,7 +98,7 @@ describe('Guest Overage Confirm Action', () => { confirmAction.mockResolvedValue(false); confirmReturn = await guestOverageConfirmAction({ - currentAccessIntValue: 10, + currentAccessIntValue: MEMBER_ACCESS_LEVELS.GUEST, dropdownIntValue: 30, }); }); diff --git a/spec/frontend/members/components/table/role_dropdown_spec.js b/spec/frontend/members/components/table/role_dropdown_spec.js index b8ad6f8a77eef5..3815064b3f6ae8 100644 --- a/spec/frontend/members/components/table/role_dropdown_spec.js +++ b/spec/frontend/members/components/table/role_dropdown_spec.js @@ -165,13 +165,13 @@ describe('RoleDropdown', () => { expect(findDropdown().props('right')).toBe(false); }); - describe('when guestOverageConfirmAction', () => { + describe('guestOverageConfirmAction', () => { const mockConfirmAction = ({ confirmed }) => { guestOverageConfirmAction.mockResolvedValueOnce(confirmed); }; - beforeEach(async () => { - await createComponent(); + beforeEach(() => { + createComponent(); findDropdownToggle().trigger('click'); }); @@ -180,26 +180,26 @@ describe('RoleDropdown', () => { guestOverageConfirmAction.mockReset(); }); - describe('returns true', () => { + describe('when guestOverageConfirmAction returns true', () => { beforeEach(() => { mockConfirmAction({ confirmed: true }); getDropdownItemByText('Reporter').trigger('click'); }); - it('allows updateMemberRole call', () => { + it('calls updateMemberRole', () => { expect(actions.updateMemberRole).toHaveBeenCalled(); }); }); - describe('returns false', () => { + describe('when guestOverageConfirmAction returns false', () => { beforeEach(() => { mockConfirmAction({ confirmed: false }); getDropdownItemByText('Reporter').trigger('click'); }); - it('does not allow updateMemberRole call', () => { + it('does not call updateMemberRole', () => { expect(actions.updateMemberRole).not.toHaveBeenCalled(); }); }); diff --git a/spec/frontend/members/guest_overage_confirm_action_spec.js b/spec/frontend/members/guest_overage_confirm_action_spec.js index 92b3bcd416754c..d7ab54fa13b2a6 100644 --- a/spec/frontend/members/guest_overage_confirm_action_spec.js +++ b/spec/frontend/members/guest_overage_confirm_action_spec.js @@ -1,6 +1,6 @@ import { guestOverageConfirmAction } from '~/members/guest_overage_confirm_action'; -describe('Guest Overage Confirm Action', () => { +describe('guestOverageConfirmAction', () => { it('returns true', () => { expect(guestOverageConfirmAction()).toBe(true); }); -- GitLab From c61f238b57ed5131a2524e70833300634bd2d931 Mon Sep 17 00:00:00 2001 From: snachnolkar Date: Wed, 14 Dec 2022 11:58:51 +0530 Subject: [PATCH 5/7] FE review changes Updating specs as per suggested review changes. --- .../javascripts/invite_members/constants.js | 8 --- .../assets/javascripts/members/constants.js | 31 +++++++++- .../members/guest_overage_confirm_action.js | 8 ++- .../guest_overage_confirm_action_spec.js | 59 ++++--------------- locale/gitlab.pot | 3 + 5 files changed, 50 insertions(+), 59 deletions(-) diff --git a/ee/app/assets/javascripts/invite_members/constants.js b/ee/app/assets/javascripts/invite_members/constants.js index 833cb164283431..1abd12ef7bcf66 100644 --- a/ee/app/assets/javascripts/invite_members/constants.js +++ b/ee/app/assets/javascripts/invite_members/constants.js @@ -1,14 +1,6 @@ import { __, s__, n__, sprintf } from '~/locale'; import { helpPagePath } from '~/helpers/help_page_helper'; -export const GUEST_OVERAGE_MODAL_FIELDS = Object.freeze({ - TITLE: __('You are about to incur additional charges'), - LINK: helpPagePath('subscriptions/quarterly_reconciliation'), - BACK_BUTTON_LABEL: __('Back'), - CONTINUE_BUTTON_LABEL: __('Continue'), - LINK_TEXT: __('%{linkStart} Learn more%{linkEnd}.'), -}); - export const OVERAGE_MODAL_LINK = helpPagePath('subscriptions/quarterly_reconciliation'); export const OVERAGE_MODAL_TITLE = s__('MembersOverage|You are about to incur additional charges'); diff --git a/ee/app/assets/javascripts/members/constants.js b/ee/app/assets/javascripts/members/constants.js index 969f1832a36bb2..0ecf2603cdc937 100644 --- a/ee/app/assets/javascripts/members/constants.js +++ b/ee/app/assets/javascripts/members/constants.js @@ -1,13 +1,14 @@ import { GlFilteredSearchToken } from '@gitlab/ui'; import { groupMemberRequestFormatter } from '~/groups/members/utils'; -import { __ } from '~/locale'; +import { __, n__, sprintf } from '~/locale'; import { OPERATORS_IS } from '~/vue_shared/components/filtered_search_bar/constants'; import { AVAILABLE_FILTERED_SEARCH_TOKENS as AVAILABLE_FILTERED_SEARCH_TOKENS_CE, MEMBER_TYPES as MEMBER_TYPES_CE, TAB_QUERY_PARAM_VALUES as CE_TAB_QUERY_PARAM_VALUES, } from '~/members/constants'; +import { helpPagePath } from '~/helpers/help_page_helper'; // eslint-disable-next-line import/export export * from '~/members/constants'; @@ -75,6 +76,34 @@ export const EE_APP_OPTIONS = uniqueProjectDownloadLimitEnabled } : {}; +export const GUEST_OVERAGE_MODAL_FIELDS = Object.freeze({ + TITLE: __('You are about to incur additional charges'), + LINK: helpPagePath('subscriptions/quarterly_reconciliation'), + BACK_BUTTON_LABEL: __('Back'), + CONTINUE_BUTTON_LABEL: __('Continue'), + LINK_TEXT: __('%{linkStart} Learn more%{linkEnd}.'), +}); + +export const overageModalInfoText = (quantity) => + n__( + 'MembersOverage|Your subscription includes %d seat.', + 'MembersOverage|Your subscription includes %d seats.', + quantity, + ); + +export const overageModalInfoWarning = (quantity, groupName) => + sprintf( + n__( + 'MembersOverage|If you continue, the %{groupName} group will have %{quantity} seat in use and will be billed for the overage.', + 'MembersOverage|If you continue, the %{groupName} group will have %{quantity} seats in use and will be billed for the overage.', + quantity, + ), + { + groupName, + quantity, + }, + ); + export const MEMBER_ACCESS_LEVELS = { GUEST: 10, }; diff --git a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js index a9143144b0d83d..a2d3dabc29efa4 100644 --- a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js +++ b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js @@ -2,14 +2,13 @@ import { sprintf } from '~/locale'; import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; import { GUEST_OVERAGE_MODAL_FIELDS, + MEMBER_ACCESS_LEVELS, overageModalInfoText, overageModalInfoWarning, -} from '../invite_members/constants'; -import { MEMBER_ACCESS_LEVELS } from './constants'; +} from './constants'; const shouldShowOverageModal = (currentAccessIntValue, dropdownValue) => { if ( - gon.features.showOverageOnRolePromotion && currentAccessIntValue === MEMBER_ACCESS_LEVELS.GUEST && dropdownValue > MEMBER_ACCESS_LEVELS.GUEST ) { @@ -41,6 +40,9 @@ export const guestOverageConfirmAction = async ({ totalUsers = 0, groupName = '', } = {}) => { + if (!gon.features.showOverageOnRolePromotion) { + return true; + } if (!shouldShowOverageModal(currentAccessIntValue, dropdownIntValue)) { return true; } diff --git a/ee/spec/frontend/members/guest_overage_confirm_action_spec.js b/ee/spec/frontend/members/guest_overage_confirm_action_spec.js index 0e734a27ae2f77..ba28b299eeedba 100644 --- a/ee/spec/frontend/members/guest_overage_confirm_action_spec.js +++ b/ee/spec/frontend/members/guest_overage_confirm_action_spec.js @@ -1,6 +1,6 @@ import { guestOverageConfirmAction } from 'ee/members/guest_overage_confirm_action'; import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; -import { MEMBER_ACCESS_LEVELS } from 'ee/members/constants'; +import { MEMBER_ACCESS_LEVELS, GUEST_OVERAGE_MODAL_FIELDS } from 'ee/members/constants'; jest.mock('~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'); @@ -15,7 +15,7 @@ describe('guestOverageConfirmAction', () => { window.gon = originalGon; }); - describe('when shouldShowOverageModal returns false', () => { + describe('when overage modal should not be shown', () => { describe('when showOverageOnRolePromotion feature flag is set to false', () => { beforeEach(() => { gon.features = { showOverageOnRolePromotion: false }; @@ -43,7 +43,7 @@ describe('guestOverageConfirmAction', () => { }); describe('when access is set to less than guest', () => { - it('returns confirmed', async () => { + it('returns true', async () => { const confirmReturn = await guestOverageConfirmAction({ currentAccessIntValue: MEMBER_ACCESS_LEVELS.GUEST, dropdownIntValue: 5, @@ -54,57 +54,22 @@ describe('guestOverageConfirmAction', () => { }); }); - describe('when shouldShowOverageModal returns true', () => { - let confirmReturn; - + describe('when overage modal should be shown', () => { beforeEach(() => { gon.features = { showOverageOnRolePromotion: true }; - }); - - afterEach(() => { - confirmAction.mockReset(); - }); - - describe('calls underlying confirmAction', () => { - beforeEach(async () => { - confirmReturn = await guestOverageConfirmAction({ - currentAccessIntValue: MEMBER_ACCESS_LEVELS.GUEST, - dropdownIntValue: 30, - }); - }); - - it('calls confirmAction', () => { - expect(confirmAction).toHaveBeenCalled(); - }); - }); - - describe('when confirmAction returns true', () => { - beforeEach(async () => { - confirmAction.mockResolvedValue(true); - - confirmReturn = await guestOverageConfirmAction({ - currentAccessIntValue: MEMBER_ACCESS_LEVELS.GUEST, - dropdownIntValue: 30, - }); - }); - - it('guestOverageConfirmAction returns true', () => { - expect(confirmReturn).toBe(true); - }); - }); - - describe('when confirmAction returns false', () => {}); - beforeEach(async () => { - confirmAction.mockResolvedValue(false); - - confirmReturn = await guestOverageConfirmAction({ + guestOverageConfirmAction({ currentAccessIntValue: MEMBER_ACCESS_LEVELS.GUEST, dropdownIntValue: 30, }); }); - it('guestOverageConfirmAction returns false', () => { - expect(confirmReturn).toBe(false); + it('calls confirmAction with expected arguments', () => { + expect(confirmAction).toHaveBeenCalledWith('', { + title: GUEST_OVERAGE_MODAL_FIELDS.TITLE, + modalHtmlMessage: expect.any(String), + primaryBtnText: GUEST_OVERAGE_MODAL_FIELDS.CONTINUE_BUTTON_LABEL, + cancelBtnText: GUEST_OVERAGE_MODAL_FIELDS.BACK_BUTTON_LABEL, + }); }); }); }); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1c1a179c89c817..94922eeb72f91d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -47331,6 +47331,9 @@ msgstr "" msgid "You are about to delete this project containing:" msgstr "" +msgid "You are about to incur additional charges" +msgstr "" + msgid "You are about to transfer the control of your account to %{group_name} group. This action is NOT reversible, you won't be able to access any of your groups and projects outside of %{group_name} once this transfer is complete." msgstr "" -- GitLab From 258dd95384feeb8c6a7a666236698c84b07eec53 Mon Sep 17 00:00:00 2001 From: snachnolkar Date: Wed, 14 Dec 2022 19:30:14 +0530 Subject: [PATCH 6/7] UX review changes --- ee/app/assets/javascripts/members/constants.js | 4 ++-- locale/gitlab.pot | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/ee/app/assets/javascripts/members/constants.js b/ee/app/assets/javascripts/members/constants.js index 0ecf2603cdc937..2dfb7f4ec5fe66 100644 --- a/ee/app/assets/javascripts/members/constants.js +++ b/ee/app/assets/javascripts/members/constants.js @@ -79,8 +79,8 @@ export const EE_APP_OPTIONS = uniqueProjectDownloadLimitEnabled export const GUEST_OVERAGE_MODAL_FIELDS = Object.freeze({ TITLE: __('You are about to incur additional charges'), LINK: helpPagePath('subscriptions/quarterly_reconciliation'), - BACK_BUTTON_LABEL: __('Back'), - CONTINUE_BUTTON_LABEL: __('Continue'), + BACK_BUTTON_LABEL: __('Cancel'), + CONTINUE_BUTTON_LABEL: __('Continue with overages'), LINK_TEXT: __('%{linkStart} Learn more%{linkEnd}.'), }); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 94922eeb72f91d..5823a4aa9729e2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -10965,6 +10965,9 @@ msgstr "" msgid "Continue to the next step" msgstr "" +msgid "Continue with overages" +msgstr "" + msgid "Continuous Integration and Deployment" msgstr "" -- GitLab From 821aad2812cbe0229a262eafc8f9ec01ad907a05 Mon Sep 17 00:00:00 2001 From: snachnolkar Date: Tue, 20 Dec 2022 15:20:31 +0530 Subject: [PATCH 7/7] FE review changes --- .../members/components/table/role_dropdown.vue | 5 ++++- .../javascripts/members/guest_overage_confirm_action.js | 8 ++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/members/components/table/role_dropdown.vue b/app/assets/javascripts/members/components/table/role_dropdown.vue index a936937a8ca9cf..daf5e95e6efcb9 100644 --- a/app/assets/javascripts/members/components/table/role_dropdown.vue +++ b/app/assets/javascripts/members/components/table/role_dropdown.vue @@ -2,6 +2,7 @@ import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import { mapActions } from 'vuex'; +import * as Sentry from '@sentry/browser'; import { s__ } from '~/locale'; import { guestOverageConfirmAction } from 'ee_else_ce/members/guest_overage_confirm_action'; @@ -78,7 +79,9 @@ export default { .then(() => { this.$toast.show(s__('Members|Role updated successfully.')); }) - .catch(() => {}) + .catch((error) => { + Sentry.captureException(error); + }) .finally(() => { this.busy = false; }); diff --git a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js index a2d3dabc29efa4..1fa97ec0b933a8 100644 --- a/ee/app/assets/javascripts/members/guest_overage_confirm_action.js +++ b/ee/app/assets/javascripts/members/guest_overage_confirm_action.js @@ -40,10 +40,10 @@ export const guestOverageConfirmAction = async ({ totalUsers = 0, groupName = '', } = {}) => { - if (!gon.features.showOverageOnRolePromotion) { - return true; - } - if (!shouldShowOverageModal(currentAccessIntValue, dropdownIntValue)) { + if ( + !gon.features.showOverageOnRolePromotion || + !shouldShowOverageModal(currentAccessIntValue, dropdownIntValue) + ) { return true; } -- GitLab