diff --git a/app/models/chat_name.rb b/app/models/chat_name.rb index ff3f2663b730da359125dffb4a8d531cb9456c27..60370c525d5b60f21457ab39c5c8d55762d0a414 100644 --- a/app/models/chat_name.rb +++ b/app/models/chat_name.rb @@ -3,7 +3,7 @@ class ChatName < ApplicationRecord LAST_USED_AT_INTERVAL = 1.hour - belongs_to :integration, foreign_key: :service_id + belongs_to :integration belongs_to :user validates :user, presence: true @@ -11,8 +11,8 @@ class ChatName < ApplicationRecord validates :team_id, presence: true validates :chat_id, presence: true - validates :user_id, uniqueness: { scope: [:service_id] } - validates :chat_id, uniqueness: { scope: [:service_id, :team_id] } + validates :user_id, uniqueness: { scope: [:integration_id] } + validates :chat_id, uniqueness: { scope: [:integration_id, :team_id] } # Updates the "last_used_timestamp" but only if it wasn't already updated # recently. diff --git a/app/models/integrations/base_slash_commands.rb b/app/models/integrations/base_slash_commands.rb index a0ac54748935016465d1ca26f848ea115694a591..e51d748b56238a7afda85f77d47262748b08c060 100644 --- a/app/models/integrations/base_slash_commands.rb +++ b/app/models/integrations/base_slash_commands.rb @@ -8,7 +8,7 @@ class BaseSlashCommands < Integration prop_accessor :token - has_many :chat_names, foreign_key: :service_id, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :chat_names, foreign_key: :integration_id, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent def valid_token?(token) self.respond_to?(:token) && diff --git a/app/services/chat_names/authorize_user_service.rb b/app/services/chat_names/authorize_user_service.rb index 8da76c809aca50e8a0cf68d3b1d12f02949eaf8c..6c28a1cea7e467279b37c4fbeeefcef34539261b 100644 --- a/app/services/chat_names/authorize_user_service.rb +++ b/app/services/chat_names/authorize_user_service.rb @@ -4,8 +4,8 @@ module ChatNames class AuthorizeUserService include Gitlab::Routing - def initialize(service, params) - @service = service + def initialize(integration, params) + @integration = integration @params = params end @@ -29,7 +29,7 @@ def chat_name_token def chat_name_params { - service_id: @service.id, + integration_id: @integration.id, team_id: @params[:team_id], team_domain: @params[:team_domain], chat_id: @params[:user_id], diff --git a/db/migrate/20220707105335_rename_chat_name_service_id_to_integration_id.rb b/db/migrate/20220707105335_rename_chat_name_service_id_to_integration_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..a6625d030a426674f2c69ae76b02f8556798dab9 --- /dev/null +++ b/db/migrate/20220707105335_rename_chat_name_service_id_to_integration_id.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class RenameChatNameServiceIdToIntegrationId < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + def up + rename_column_concurrently :chat_names, :service_id, :integration_id + end + + def down + undo_rename_column_concurrently :chat_names, :service_id, :integration_id + end +end diff --git a/db/post_migrate/20220707105529_cleanup_chat_name_service_id.rb b/db/post_migrate/20220707105529_cleanup_chat_name_service_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..aa3af4acf3110fa6ca38c3b74f13ddd73de5041e --- /dev/null +++ b/db/post_migrate/20220707105529_cleanup_chat_name_service_id.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class CleanupChatNameServiceId < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + def up + cleanup_concurrent_column_rename :chat_names, :service_id, :integration_id + end + + def down + undo_cleanup_concurrent_column_rename :chat_names, :service_id, :integration_id + end +end diff --git a/db/schema_migrations/20220707105335 b/db/schema_migrations/20220707105335 new file mode 100644 index 0000000000000000000000000000000000000000..1f1a4d8b41b444b63d09dde159036b4ad25a33ee --- /dev/null +++ b/db/schema_migrations/20220707105335 @@ -0,0 +1 @@ +2f3dc1952c43a6786f8a66713ac89ca24f828f683a57f7373c91d5e629242909 \ No newline at end of file diff --git a/db/schema_migrations/20220707105529 b/db/schema_migrations/20220707105529 new file mode 100644 index 0000000000000000000000000000000000000000..df01b63f89c05bd8f4344a970e4967da3baceb5a --- /dev/null +++ b/db/schema_migrations/20220707105529 @@ -0,0 +1 @@ +82504ed0c287565d9b9eadf929badaa893beaac36224c7c2c7b4e14a663fa9e5 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 4eeba13e90f4dcf5632e7973069c12c9bc795f00..dc5f55a86b0882342b13eefaa4192eac4e67a917 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -12423,14 +12423,15 @@ ALTER SEQUENCE bulk_imports_id_seq OWNED BY bulk_imports.id; CREATE TABLE chat_names ( id integer NOT NULL, user_id integer NOT NULL, - service_id integer NOT NULL, team_id character varying NOT NULL, team_domain character varying, chat_id character varying NOT NULL, chat_name character varying, last_used_at timestamp without time zone, created_at timestamp without time zone NOT NULL, - updated_at timestamp without time zone NOT NULL + updated_at timestamp without time zone NOT NULL, + integration_id integer, + CONSTRAINT check_2b0a0d0f0f CHECK ((integration_id IS NOT NULL)) ); CREATE SEQUENCE chat_names_id_seq @@ -27659,9 +27660,9 @@ CREATE INDEX index_bulk_import_failures_on_correlation_id_value ON bulk_import_f CREATE INDEX index_bulk_imports_on_user_id ON bulk_imports USING btree (user_id); -CREATE UNIQUE INDEX index_chat_names_on_service_id_and_team_id_and_chat_id ON chat_names USING btree (service_id, team_id, chat_id); +CREATE UNIQUE INDEX index_chat_names_on_integration_id_and_team_id_and_chat_id ON chat_names USING btree (integration_id, team_id, chat_id); -CREATE UNIQUE INDEX index_chat_names_on_user_id_and_service_id ON chat_names USING btree (user_id, service_id); +CREATE UNIQUE INDEX index_chat_names_on_user_id_and_integration_id ON chat_names USING btree (user_id, integration_id); CREATE UNIQUE INDEX index_chat_teams_on_namespace_id ON chat_teams USING btree (namespace_id); @@ -31891,9 +31892,6 @@ CREATE TRIGGER trigger_update_vulnerability_reads_on_vulnerability_update AFTER CREATE TRIGGER users_loose_fk_trigger AFTER DELETE ON users REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records(); -ALTER TABLE ONLY chat_names - ADD CONSTRAINT fk_00797a2bf9 FOREIGN KEY (service_id) REFERENCES integrations(id) ON DELETE CASCADE; - ALTER TABLE ONLY deployments ADD CONSTRAINT fk_009fd21147 FOREIGN KEY (environment_id) REFERENCES environments(id) ON DELETE CASCADE NOT VALID; @@ -32398,6 +32396,9 @@ ALTER TABLE ONLY vulnerability_occurrences ALTER TABLE ONLY protected_branch_merge_access_levels ADD CONSTRAINT fk_98f3d044fe FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; +ALTER TABLE ONLY chat_names + ADD CONSTRAINT fk_99a1348daf FOREIGN KEY (integration_id) REFERENCES integrations(id) ON DELETE CASCADE; + ALTER TABLE ONLY notes ADD CONSTRAINT fk_99e097b079 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/lib/gitlab/chat_name_token.rb b/lib/gitlab/chat_name_token.rb index 9b4cb9d0134abff20d9f988b2209d574f8e0f45e..76f2a4ae38c49c2b90af8346f83e822d28cd9d69 100644 --- a/lib/gitlab/chat_name_token.rb +++ b/lib/gitlab/chat_name_token.rb @@ -16,7 +16,9 @@ def initialize(token = new_token) def get Gitlab::Redis::SharedState.with do |redis| data = redis.get(redis_shared_state_key) - Gitlab::Json.parse(data, symbolize_names: true) if data + params = Gitlab::Json.parse(data, symbolize_names: true) if data + params[:integration_id] ||= params.delete(:service_id) if params && params[:service_id] + params end end diff --git a/spec/models/chat_name_spec.rb b/spec/models/chat_name_spec.rb index 1d2ad8b4dcee75cf50e5a43899b9d4afe82c797e..02c38479d1ad1e117795b0c168d0f33a9db41da5 100644 --- a/spec/models/chat_name_spec.rb +++ b/spec/models/chat_name_spec.rb @@ -15,8 +15,8 @@ it { is_expected.to validate_presence_of(:team_id) } it { is_expected.to validate_presence_of(:chat_id) } - it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(:service_id) } - it { is_expected.to validate_uniqueness_of(:chat_id).scoped_to(:service_id, :team_id) } + it { is_expected.to validate_uniqueness_of(:user_id).scoped_to(:integration_id) } + it { is_expected.to validate_uniqueness_of(:chat_id).scoped_to(:integration_id, :team_id) } it 'is removed when the project is deleted' do expect { subject.reload.integration.project.delete }.to change { ChatName.count }.by(-1)