diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index fac358d65435bdb3ef4f90cd74e46850bcd8b0cf..299d17cb0dbec1b55ed6669850f3f4ca6095fce7 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -4744,7 +4744,6 @@ RSpec/FeatureCategory: - 'spec/models/users/credit_card_validation_spec.rb' - 'spec/models/users/ghost_user_migration_spec.rb' - 'spec/models/users/group_callout_spec.rb' - - 'spec/models/users/in_product_marketing_email_spec.rb' - 'spec/models/users/merge_request_interaction_spec.rb' - 'spec/models/users/phone_number_validation_spec.rb' - 'spec/models/users/project_callout_spec.rb' diff --git a/.rubocop_todo/style/block_delimiters.yml b/.rubocop_todo/style/block_delimiters.yml index 8d7d98ec0144a1dd76b233f4f5f7c2a51afbf7c6..9a6ecf1e955a5a8634b8d0f5f286c5b4b74a4443 100644 --- a/.rubocop_todo/style/block_delimiters.yml +++ b/.rubocop_todo/style/block_delimiters.yml @@ -59,7 +59,6 @@ Style/BlockDelimiters: - 'spec/models/packages/npm/metadatum_spec.rb' - 'spec/models/packages/protection/rule_spec.rb' - 'spec/models/packages/pypi/metadatum_spec.rb' - - 'spec/models/users/in_product_marketing_email_spec.rb' - 'spec/models/users/phone_number_validation_spec.rb' - 'spec/models/users/project_callout_spec.rb' - 'spec/presenters/tree_entry_presenter_spec.rb' diff --git a/app/models/user.rb b/app/models/user.rb index 7e6eb3529d9c2ebc08028a7966ff67c52a767842..8ae89d60b0b54dd28ddfaa9b78cc8bb2c1acb396 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -284,8 +284,6 @@ def update_tracked_fields!(request) has_many :reviews, foreign_key: :author_id, inverse_of: :author - has_many :in_product_marketing_emails, class_name: '::Users::InProductMarketingEmail' - has_many :timelogs has_many :resource_label_events, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent diff --git a/app/models/users/in_product_marketing_email.rb b/app/models/users/in_product_marketing_email.rb deleted file mode 100644 index 5362a726ff5e97cc677ba0048f1225aa45f705d6..0000000000000000000000000000000000000000 --- a/app/models/users/in_product_marketing_email.rb +++ /dev/null @@ -1,75 +0,0 @@ -# frozen_string_literal: true - -module Users - class InProductMarketingEmail < ApplicationRecord - include BulkInsertSafe - - belongs_to :user - - validates :user, presence: true - validates :track, presence: true - validates :series, presence: true - - validates :user_id, uniqueness: { - scope: [:track, :series], - message: 'track series email has already been sent' - }, if: -> { track.present? } - - enum track: { - create: 0, - verify: 1, - trial: 2, - team: 3, - experience: 4, - team_short: 5, - trial_short: 6, - admin_verify: 7, - invite_team: 8 - }, _suffix: true - - # Tracks we don't send emails for (e.g. unsuccessful experiment). These - # are kept since we already have DB records that use the enum value. - INACTIVE_TRACK_NAMES = %w[invite_team experience].freeze - ACTIVE_TRACKS = tracks.except(*INACTIVE_TRACK_NAMES) - - scope :for_user_with_track_and_series, ->(user, track, series) do - where(user: user, track: track, series: series) - end - - scope :without_track_and_series, ->(track, series) do - join_condition = for_user.and(for_track_and_series(track, series)) - users_without_records(join_condition) - end - - def self.users_table - User.arel_table - end - - def self.distinct_users_sql - name = users_table.name - Arel.sql("DISTINCT ON(#{name}.id) #{name}.*") - end - - def self.users_without_records(condition) - arel_join = users_table.join(arel_table, Arel::Nodes::OuterJoin).on(condition) - joins(arel_join.join_sources) - .where(in_product_marketing_emails: { id: nil }) - .select(distinct_users_sql) - end - - def self.for_user - arel_table[:user_id].eq(users_table[:id]) - end - - def self.for_track_and_series(track, series) - arel_table[:track].eq(ACTIVE_TRACKS[track]) - .and(arel_table[:series]).eq(series) - end - - def self.save_cta_click(user, track, series) - email = for_user_with_track_and_series(user, track, series).take - - email.update(cta_clicked_at: Time.zone.now) if email && email.cta_clicked_at.blank? - end - end -end diff --git a/db/docs/in_product_marketing_emails.yml b/db/docs/deleted_tables/in_product_marketing_emails.yml similarity index 71% rename from db/docs/in_product_marketing_emails.yml rename to db/docs/deleted_tables/in_product_marketing_emails.yml index 8021f4a2634f0cd7bd5a9c5bd8bd5db93a081a8c..e36a4546ff4feace1a2f3caad016ea38382f7dfd 100644 --- a/db/docs/in_product_marketing_emails.yml +++ b/db/docs/deleted_tables/in_product_marketing_emails.yml @@ -7,4 +7,6 @@ feature_categories: description: TODO introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55840 milestone: '13.10' +removed_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/138835 +removed_in_milestone: '16.8' gitlab_schema: gitlab_main diff --git a/db/post_migrate/20231205200847_rollback_user_foreign_key_from_in_product_marketing_emails.rb b/db/post_migrate/20231205200847_rollback_user_foreign_key_from_in_product_marketing_emails.rb new file mode 100644 index 0000000000000000000000000000000000000000..fe4509204db457a695881cd0a160d6a6fa5bc5e7 --- /dev/null +++ b/db/post_migrate/20231205200847_rollback_user_foreign_key_from_in_product_marketing_emails.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RollbackUserForeignKeyFromInProductMarketingEmails < Gitlab::Database::Migration[2.2] + milestone '16.8' + disable_ddl_transaction! + + def up + with_lock_retries do + remove_foreign_key_if_exists :in_product_marketing_emails, :users, name: 'fk_35c9101b63' + end + end + + def down + add_concurrent_foreign_key :in_product_marketing_emails, :users, column: :user_id, name: 'fk_35c9101b63', + on_delete: :cascade + end +end diff --git a/db/post_migrate/20231205200925_drop_in_product_marketing_emails.rb b/db/post_migrate/20231205200925_drop_in_product_marketing_emails.rb new file mode 100644 index 0000000000000000000000000000000000000000..5a1217933330c5f8635344ce82200efbc3acb98b --- /dev/null +++ b/db/post_migrate/20231205200925_drop_in_product_marketing_emails.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class DropInProductMarketingEmails < Gitlab::Database::Migration[2.2] + milestone '16.8' + + def up + drop_table :in_product_marketing_emails + end + + def down + create_table :in_product_marketing_emails do |t| + t.bigint :user_id, null: false + t.datetime_with_timezone :cta_clicked_at + t.integer :track, null: false, limit: 2 + t.integer :series, null: false, limit: 2 + + t.timestamps_with_timezone + end + + add_index :in_product_marketing_emails, :user_id + add_index :in_product_marketing_emails, %i[user_id track series], unique: true, + name: 'index_in_product_marketing_emails_on_user_track_series' + add_index :in_product_marketing_emails, %i[track series id cta_clicked_at], + name: 'index_in_product_marketing_emails_on_track_series_id_clicked' + end +end diff --git a/db/schema_migrations/20231205200847 b/db/schema_migrations/20231205200847 new file mode 100644 index 0000000000000000000000000000000000000000..ee54e218e09ea30a0c9a8f6b5ac13b6ac3960f7c --- /dev/null +++ b/db/schema_migrations/20231205200847 @@ -0,0 +1 @@ +cac5543864045ad7e9ad386aebcf7f29ee5c6351fcadd81f4c7b5d29b2fad16b \ No newline at end of file diff --git a/db/schema_migrations/20231205200925 b/db/schema_migrations/20231205200925 new file mode 100644 index 0000000000000000000000000000000000000000..92cee99ca027a256f63310d12bf7dbd69cfd160d --- /dev/null +++ b/db/schema_migrations/20231205200925 @@ -0,0 +1 @@ +9318699a34d585059d68a62cfe74e5f5d9a8669d0f1acb2b0c9d98545c7a3a26 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 16ca18337fdcd53bf5d15afec7315441544de04a..db86925ed8edfdefc31247e6e79ea5e0a29fe391 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17590,27 +17590,6 @@ CREATE SEQUENCE import_failures_id_seq ALTER SEQUENCE import_failures_id_seq OWNED BY import_failures.id; -CREATE TABLE in_product_marketing_emails ( - id bigint NOT NULL, - user_id bigint NOT NULL, - cta_clicked_at timestamp with time zone, - track smallint, - series smallint, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, - CONSTRAINT check_c9bb978e4b CHECK ((track IS NOT NULL)), - CONSTRAINT check_ee6c42a107 CHECK ((series IS NOT NULL)) -); - -CREATE SEQUENCE in_product_marketing_emails_id_seq - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE in_product_marketing_emails_id_seq OWNED BY in_product_marketing_emails.id; - CREATE TABLE incident_management_escalation_policies ( id bigint NOT NULL, project_id bigint NOT NULL, @@ -26814,8 +26793,6 @@ ALTER TABLE ONLY import_export_uploads ALTER COLUMN id SET DEFAULT nextval('impo ALTER TABLE ONLY import_failures ALTER COLUMN id SET DEFAULT nextval('import_failures_id_seq'::regclass); -ALTER TABLE ONLY in_product_marketing_emails ALTER COLUMN id SET DEFAULT nextval('in_product_marketing_emails_id_seq'::regclass); - ALTER TABLE ONLY incident_management_escalation_policies ALTER COLUMN id SET DEFAULT nextval('incident_management_escalation_policies_id_seq'::regclass); ALTER TABLE ONLY incident_management_escalation_rules ALTER COLUMN id SET DEFAULT nextval('incident_management_escalation_rules_id_seq'::regclass); @@ -29014,9 +28991,6 @@ ALTER TABLE ONLY import_export_uploads ALTER TABLE ONLY import_failures ADD CONSTRAINT import_failures_pkey PRIMARY KEY (id); -ALTER TABLE ONLY in_product_marketing_emails - ADD CONSTRAINT in_product_marketing_emails_pkey PRIMARY KEY (id); - ALTER TABLE ONLY incident_management_oncall_shifts ADD CONSTRAINT inc_mgmnt_no_overlapping_oncall_shifts EXCLUDE USING gist (rotation_id WITH =, tstzrange(starts_at, ends_at, '[)'::text) WITH &&); @@ -33200,12 +33174,6 @@ CREATE INDEX index_imported_projects_on_import_type_creator_id_created_at ON pro CREATE INDEX index_imported_projects_on_import_type_id ON projects USING btree (import_type, id) WHERE (import_type IS NOT NULL); -CREATE INDEX index_in_product_marketing_emails_on_track_series_id_clicked ON in_product_marketing_emails USING btree (track, series, id, cta_clicked_at); - -CREATE INDEX index_in_product_marketing_emails_on_user_id ON in_product_marketing_emails USING btree (user_id); - -CREATE UNIQUE INDEX index_in_product_marketing_emails_on_user_track_series ON in_product_marketing_emails USING btree (user_id, track, series); - CREATE INDEX index_inc_mgmnt_oncall_participants_on_oncall_user_id ON incident_management_oncall_participants USING btree (user_id); CREATE UNIQUE INDEX index_inc_mgmnt_oncall_participants_on_user_id_and_rotation_id ON incident_management_oncall_participants USING btree (user_id, oncall_rotation_id); @@ -37501,9 +37469,6 @@ ALTER TABLE ONLY project_topics ALTER TABLE ONLY saml_providers ADD CONSTRAINT fk_351dde3a84 FOREIGN KEY (member_role_id) REFERENCES member_roles(id) ON DELETE SET NULL; -ALTER TABLE ONLY in_product_marketing_emails - ADD CONSTRAINT fk_35c9101b63 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; - ALTER TABLE ONLY epics ADD CONSTRAINT fk_3654b61b03 FOREIGN KEY (author_id) REFERENCES users(id) ON DELETE CASCADE; diff --git a/spec/factories/users/in_product_marketing_email.rb b/spec/factories/users/in_product_marketing_email.rb deleted file mode 100644 index c86c469ff31bd90757de5b64e132badf64cd5e82..0000000000000000000000000000000000000000 --- a/spec/factories/users/in_product_marketing_email.rb +++ /dev/null @@ -1,10 +0,0 @@ -# frozen_string_literal: true - -FactoryBot.define do - factory :in_product_marketing_email, class: 'Users::InProductMarketingEmail' do - user - - track { 'create' } - series { 0 } - end -end diff --git a/spec/lib/gitlab/usage/service_ping_report_spec.rb b/spec/lib/gitlab/usage/service_ping_report_spec.rb index a848c286fa9f74dbf07f489a2562d1947af187ca..098661986395f25d8928a24545e474c826ef4ac9 100644 --- a/spec/lib/gitlab/usage/service_ping_report_spec.rb +++ b/spec/lib/gitlab/usage/service_ping_report_spec.rb @@ -168,11 +168,6 @@ def type_cast_to_defined_type(value, metric_definition) memoized_constatns += Gitlab::UsageData::EE_MEMOIZED_VALUES if defined? Gitlab::UsageData::EE_MEMOIZED_VALUES memoized_constatns.each { |v| Gitlab::UsageData.clear_memoization(v) } stub_database_flavor_check('Cloud SQL for PostgreSQL') - - # in_product_marketing_email metrics values are extracted from a single group by query - # to check if the queries for individual metrics return the same value as group by when the value is non-zero - create(:in_product_marketing_email, track: :create, series: 0, cta_clicked_at: Time.current) - create(:in_product_marketing_email, track: :verify, series: 0) end let(:service_ping_payload) { described_class.for(output: :all_metrics_values) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d098f0b4c4ad4d178f8b3e954600559655df021f..ddf5a9a7221eded3f925f73070bd0c68b6806927 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -187,7 +187,6 @@ it { is_expected.to have_many(:merge_request_assignees).inverse_of(:assignee) } it { is_expected.to have_many(:merge_request_reviewers).inverse_of(:reviewer) } 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(:timelogs) } it { is_expected.to have_many(:callouts).class_name('Users::Callout') } it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout') } diff --git a/spec/models/users/in_product_marketing_email_spec.rb b/spec/models/users/in_product_marketing_email_spec.rb deleted file mode 100644 index b1642383e4227830a841223d7e3f1edb727537e5..0000000000000000000000000000000000000000 --- a/spec/models/users/in_product_marketing_email_spec.rb +++ /dev/null @@ -1,137 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Users::InProductMarketingEmail, type: :model, feature_category: :onboarding do - let(:track) { :create } - let(:series) { 0 } - - describe 'associations' do - it { is_expected.to belong_to(:user) } - end - - describe 'validations' do - subject { build(:in_product_marketing_email) } - - it { is_expected.to validate_presence_of(:user) } - - context 'when track+series email' do - it { is_expected.to validate_presence_of(:track) } - it { is_expected.to validate_presence_of(:series) } - - it { - is_expected.to validate_uniqueness_of(:user_id) - .scoped_to([:track, :series]).with_message('track series email has already been sent') - } - end - end - - describe '.without_track_and_series' do - let_it_be(:user) { create(:user) } - - subject(:without_track_and_series) { User.merge(described_class.without_track_and_series(track, series)) } - - before do - create(:in_product_marketing_email, track: :create, series: 0, user: user) - create(:in_product_marketing_email, track: :create, series: 1, user: user) - create(:in_product_marketing_email, track: :verify, series: 0, user: user) - end - - context 'when given track and series already exists' do - it { expect(without_track_and_series).to be_empty } - end - - context 'when track does not exist' do - let(:track) { :trial } - - it { expect(without_track_and_series).to eq [user] } - end - - context 'when series does not exist' do - let(:series) { 2 } - - it { expect(without_track_and_series).to eq [user] } - end - - context 'when no track or series for a user exists' do - let(:track) { :create } - let(:series) { 0 } - let(:other_user) { create(:user) } - - it { expect(without_track_and_series).to eq [other_user] } - end - end - - describe '.for_user_with_track_and_series' do - let_it_be(:user) { create(:user) } - let_it_be(:in_product_marketing_email) { create(:in_product_marketing_email, series: 0, track: 0, user: user) } - - subject(:for_user_with_track_and_series) do - described_class.for_user_with_track_and_series(user, track, series).first - end - - context 'when record for user with given track and series exists' do - it { is_expected.to eq(in_product_marketing_email) } - end - - context 'when user is different' do - let(:user) { build_stubbed(:user) } - - it { is_expected.to be_nil } - end - - context 'when track is different' do - let(:track) { 1 } - - it { is_expected.to be_nil } - end - - context 'when series is different' do - let(:series) { 1 } - - it { is_expected.to be_nil } - end - end - - describe '.save_cta_click' do - let(:user) { create(:user) } - - subject(:save_cta_click) { described_class.save_cta_click(user, track, series) } - - context 'when there is no record' do - it 'does not error' do - expect { save_cta_click }.not_to raise_error - end - end - - context 'when there is no record for the track and series' do - it 'does not perform an update' do - other_email = create(:in_product_marketing_email, user: user, track: :verify, series: 2, cta_clicked_at: nil) - - expect { save_cta_click }.not_to change { other_email.reload } - end - end - - context 'when there is a record for the track and series' do - it 'saves the cta click date' do - email = create(:in_product_marketing_email, user: user, track: track, series: series, cta_clicked_at: nil) - - freeze_time do - expect { save_cta_click }.to change { email.reload.cta_clicked_at }.from(nil).to(Time.zone.now) - end - end - - context 'when cta_clicked_at is already set' do - it 'does not update' do - create(:in_product_marketing_email, user: user, track: track, series: series, cta_clicked_at: Time.zone.now) - - expect_next_found_instance_of(described_class) do |record| - expect(record).not_to receive(:update) - end - - save_cta_click - end - end - end - end -end diff --git a/spec/support/helpers/database/duplicate_indexes.yml b/spec/support/helpers/database/duplicate_indexes.yml index cbb0b0be45712074633151b9d0fc80f44b00ce6b..87a1e0c2c50abaf3326e4b23aa7f83710c73a78d 100644 --- a/spec/support/helpers/database/duplicate_indexes.yml +++ b/spec/support/helpers/database/duplicate_indexes.yml @@ -69,9 +69,6 @@ error_tracking_errors: geo_node_namespace_links: index_geo_node_namespace_links_on_geo_node_id_and_namespace_id: - index_geo_node_namespace_links_on_geo_node_id -in_product_marketing_emails: - index_in_product_marketing_emails_on_user_track_series: - - index_in_product_marketing_emails_on_user_id incident_management_oncall_participants: index_inc_mgmnt_oncall_participants_on_user_id_and_rotation_id: - index_inc_mgmnt_oncall_participants_on_oncall_user_id