diff options
23 files changed, 107 insertions, 270 deletions
diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index c8d7c1c83cc..8aa944ba30b 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -595,8 +595,6 @@ RSpec/EmptyLineAfterFinalLetItBe: - spec/features/file_uploads/user_avatar_spec.rb - spec/features/issues/user_sees_breadcrumb_links_spec.rb - spec/features/markdown/metrics_spec.rb - - spec/features/merge_request/user_creates_merge_request_spec.rb - - spec/features/merge_request/user_posts_notes_spec.rb - spec/features/operations_sidebar_link_spec.rb - spec/features/participants_autocomplete_spec.rb - spec/features/snippets/embedded_snippet_spec.rb diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 5e04576a889..eca95040562 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -d8154c73a22b2d1a08e4a348f9463aeb5144f74c +d0d2f4a763c2be059e4e2353f2e5affaff83305d @@ -267,7 +267,7 @@ gem 'kubeclient', '~> 4.9.1' # Sanitize user input gem 'sanitize', '~> 5.2.1' -gem 'babosa', '~> 1.0.2' +gem 'babosa', '~> 1.0.4' # Sanitizes SVG input gem 'loofah', '~> 2.2' diff --git a/Gemfile.lock b/Gemfile.lock index 4fac86a77e5..a305f280717 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -132,7 +132,7 @@ GEM faraday_middleware (~> 1.0.0.rc1) net-http-persistent (~> 4.0) nokogiri (~> 1.11.0.rc2) - babosa (1.0.2) + babosa (1.0.4) backport (1.1.2) base32 (0.3.2) batch-loader (2.0.1) @@ -1378,7 +1378,7 @@ DEPENDENCIES aws-sdk-cloudformation (~> 1) aws-sdk-core (~> 3) aws-sdk-s3 (~> 1) - babosa (~> 1.0.2) + babosa (~> 1.0.4) base32 (~> 0.3.0) batch-loader (~> 2.0.1) bcrypt (~> 3.1, >= 3.1.14) diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb index 7c57d321c80..71bc8bef53b 100644 --- a/app/controllers/concerns/service_params.rb +++ b/app/controllers/concerns/service_params.rb @@ -9,7 +9,6 @@ module ServiceParams :alert_events, :api_key, :api_url, - :api_version, :bamboo_url, :branches_to_be_notified, :labels_to_be_notified, @@ -52,7 +51,6 @@ module ServiceParams :mock_service_url, :namespace, :new_issue_url, - :notify, :notify_only_broken_pipelines, :password, :priority, diff --git a/app/models/project.rb b/app/models/project.rb index f03e5293b58..f18d41e3556 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -161,7 +161,6 @@ class Project < ApplicationRecord has_one :pipelines_email_service has_one :irker_service has_one :pivotaltracker_service - has_one :hipchat_service has_one :flowdock_service has_one :assembla_service has_one :asana_service diff --git a/app/models/project_services/hipchat_service.rb b/app/models/project_services/hipchat_service.rb index cd49c6d253d..93f3d93bd2b 100644 --- a/app/models/project_services/hipchat_service.rb +++ b/app/models/project_services/hipchat_service.rb @@ -1,54 +1,17 @@ # frozen_string_literal: true +# This service is scheduled for removal. All records must +# be deleted before the class can be removed. +# https://gitlab.com/gitlab-org/gitlab/-/issues/27954 class HipchatService < Service - include ActionView::Helpers::SanitizeHelper - - MAX_COMMITS = 3 - HIPCHAT_ALLOWED_TAGS = %w[ - a b i strong em br img pre code - table th tr td caption colgroup col thead tbody tfoot - ul ol li dl dt dd - ].freeze - - prop_accessor :token, :room, :server, :color, :api_version - boolean_accessor :notify_only_broken_pipelines, :notify - validates :token, presence: true, if: :activated? - - def initialize_properties - if properties.nil? - self.properties = {} - self.notify_only_broken_pipelines = true - end - end - - def title - 'HipChat' - end - - def description - 'Private group chat and IM' - end + before_save :prevent_save def self.to_param 'hipchat' end - def fields - [ - { type: 'text', name: 'token', placeholder: 'Room token', required: true }, - { type: 'text', name: 'room', placeholder: 'Room name or ID' }, - { type: 'checkbox', name: 'notify' }, - { type: 'select', name: 'color', choices: %w(yellow red green purple gray random) }, - { type: 'text', name: 'api_version', title: _('API version'), - placeholder: 'Leave blank for default (v2)' }, - { type: 'text', name: 'server', - placeholder: 'Leave blank for default. https://hipchat.example.com' }, - { type: 'checkbox', name: 'notify_only_broken_pipelines' } - ] - end - def self.supported_events - %w(push issue confidential_issue merge_request note confidential_note tag_push pipeline) + [] end def execute(data) @@ -56,96 +19,14 @@ class HipchatService < Service # HipChat is unusable anyway, so do nothing in this method end - def test(data) - begin - result = execute(data) - rescue StandardError => error - return { success: false, result: error } - end - - { success: true, result: result } - end - private - def message_options(data = nil) - { notify: notify.present? && Gitlab::Utils.to_boolean(notify), color: message_color(data) } - end - - def render_line(text) - markdown(text.lines.first.chomp, pipeline: :single_line) if text - end - - def markdown(text, options = {}) - return "" unless text - - context = { - project: project, - pipeline: :email - } - - Banzai.render(text, context) - - context.merge!(options) - - html = Banzai.render_and_post_process(text, context) - sanitized_html = sanitize(html, tags: HIPCHAT_ALLOWED_TAGS, attributes: %w[href title alt]) - - sanitized_html.truncate(200, separator: ' ', omission: '...') - end - - def format_title(title) - "<b>#{render_line(title)}</b>" - end - - def message_color(data) - pipeline_status_color(data) || color || 'yellow' - end - - def pipeline_status_color(data) - return unless data && data[:object_kind] == 'pipeline' - - case data[:object_attributes][:status] - when 'success' - 'green' - else - 'red' - end - end - - def project_name - project.full_name.gsub(/\s/, '') - end - - def project_url - project.web_url - end - - def project_link - "<a href=\"#{project_url}\">#{project_name}</a>" - end - - def update?(data) - data[:object_attributes][:action] == 'update' - end - - def humanized_status(status) - case status - when 'success' - 'passed' - else - status - end - end + def prevent_save + errors.add(:base, _('HipChat endpoint is deprecated and should not be created or modified.')) - def should_pipeline_be_notified?(data) - case data[:object_attributes][:status] - when 'success' - !notify_only_broken_pipelines? - when 'failed' - true - else - false - end + # Stops execution of callbacks and database operation while + # preserving expectations of #save (will not raise) & #save! (raises) + # https://guides.rubyonrails.org/active_record_callbacks.html#halting-execution + throw :abort # rubocop:disable Cop/BanCatchThrow end end diff --git a/app/views/devise/mailer/reset_password_instructions.text.erb b/app/views/devise/mailer/reset_password_instructions.text.erb index 116313ee11c..c8d86fe998e 100644 --- a/app/views/devise/mailer/reset_password_instructions.text.erb +++ b/app/views/devise/mailer/reset_password_instructions.text.erb @@ -1,10 +1,9 @@ -Hello, <%= @resource.name %>! +<%= _("Hello, %{name}!") % { name: @resource.name } %> -Someone, hopefully you, has requested to reset the password for your GitLab -account on <%= Gitlab.config.gitlab.url %> +<%= _("Someone, hopefully you, has requested to reset the password for your GitLab account on %{link_to_gitlab}.") % { link_to_gitlab: Gitlab.config.gitlab.url } %> -If you did not perform this request, you can safely ignore this email. +<%= _("If you did not perform this request, you can safely ignore this email.") %> -Otherwise, click the link below to complete the process: +<%= _("Otherwise, click the link below to complete the process:") %> <%= edit_password_url(@resource, reset_password_token: @token) %> diff --git a/changelogs/unreleased/27954-remove-hipchat-service.yml b/changelogs/unreleased/27954-remove-hipchat-service.yml new file mode 100644 index 00000000000..20a2fec3682 --- /dev/null +++ b/changelogs/unreleased/27954-remove-hipchat-service.yml @@ -0,0 +1,5 @@ +--- +title: Delete HipChat service database records +merge_request: 59769 +author: +type: removed diff --git a/changelogs/unreleased/Externalize-strings-in-reset_password_instructions-text-erb.yml b/changelogs/unreleased/Externalize-strings-in-reset_password_instructions-text-erb.yml new file mode 100644 index 00000000000..440ba3f821a --- /dev/null +++ b/changelogs/unreleased/Externalize-strings-in-reset_password_instructions-text-erb.yml @@ -0,0 +1,5 @@ +--- +title: Externalise strings in reset_password_instructions.text.erb +merge_request: 58226 +author: nuwe1 +type: other diff --git a/changelogs/unreleased/issue-325836-fix-empty-line-after-let-it-be-merge-request-module.yml b/changelogs/unreleased/issue-325836-fix-empty-line-after-let-it-be-merge-request-module.yml new file mode 100644 index 00000000000..b7d744d81e8 --- /dev/null +++ b/changelogs/unreleased/issue-325836-fix-empty-line-after-let-it-be-merge-request-module.yml @@ -0,0 +1,5 @@ +--- +title: Fix EmptyLineAfterFinalLetItBe offenses for merge request module +merge_request: 58185 +author: Huzaifa Iftikhar @huzaifaiftikhar +type: fixed diff --git a/db/post_migrate/20210420103955_remove_hipchat_service_records.rb b/db/post_migrate/20210420103955_remove_hipchat_service_records.rb new file mode 100644 index 00000000000..5ad34cfad22 --- /dev/null +++ b/db/post_migrate/20210420103955_remove_hipchat_service_records.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +class RemoveHipchatServiceRecords < ActiveRecord::Migration[6.0] + disable_ddl_transaction! + + class Service < ActiveRecord::Base + include EachBatch + + self.table_name = 'services' + end + + def up + Service.each_batch(of: 100_000, column: :id) do |relation| + relation.delete_by(type: 'HipchatService') + end + end + + def down + # no-op + end +end diff --git a/db/schema_migrations/20210420103955 b/db/schema_migrations/20210420103955 new file mode 100644 index 00000000000..001c89e1d10 --- /dev/null +++ b/db/schema_migrations/20210420103955 @@ -0,0 +1 @@ +1ee3df66a0e7d1802196740cc1c0a899724c1f5b3dd8be4316915b354446f238
\ No newline at end of file diff --git a/lib/api/helpers/services_helpers.rb b/lib/api/helpers/services_helpers.rb index 2f2ad88c942..84f03e5f2c2 100644 --- a/lib/api/helpers/services_helpers.rb +++ b/lib/api/helpers/services_helpers.rb @@ -420,44 +420,6 @@ module API }, chat_notification_events ].flatten, - 'hipchat' => [ - { - required: true, - name: :token, - type: String, - desc: 'The room token' - }, - { - required: false, - name: :room, - type: String, - desc: 'The room name or ID' - }, - { - required: false, - name: :color, - type: String, - desc: 'The room color' - }, - { - required: false, - name: :notify, - type: Boolean, - desc: 'Enable notifications' - }, - { - required: false, - name: :api_version, - type: String, - desc: 'Leave blank for default (v2)' - }, - { - required: false, - name: :server, - type: String, - desc: 'Leave blank for default. https://hipchat.example.com' - } - ], 'irker' => [ { required: true, @@ -828,7 +790,6 @@ module API ::ExternalWikiService, ::FlowdockService, ::HangoutsChatService, - ::HipchatService, ::IrkerService, ::JenkinsService, ::JiraService, diff --git a/lib/gitlab/auth/o_auth/auth_hash.rb b/lib/gitlab/auth/o_auth/auth_hash.rb index 46ff6b2ccab..519efd02ea4 100644 --- a/lib/gitlab/auth/o_auth/auth_hash.rb +++ b/lib/gitlab/auth/o_auth/auth_hash.rb @@ -81,7 +81,7 @@ module Gitlab # Get the first part of the email address (before @) # In addition in removes illegal characters def generate_username(email) - email.match(/^[^@]*/)[0].mb_chars.normalize(:kd).gsub(/[^\x00-\x7F]/, '').to_s + email.match(/^[^@]*/)[0].mb_chars.unicode_normalize(:nfkd).gsub(/[^\x00-\x7F]/, '').to_s end def generate_temporarily_email(username) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index a38babc24d6..34f315daa9c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1490,9 +1490,6 @@ msgstr "" msgid "API key" msgstr "" -msgid "API version" -msgstr "" - msgid "API?" msgstr "" @@ -16010,6 +16007,9 @@ msgstr "" msgid "HighlightBar|Time to SLA:" msgstr "" +msgid "HipChat endpoint is deprecated and should not be created or modified." +msgstr "" + msgid "History" msgstr "" @@ -22571,6 +22571,9 @@ msgstr "" msgid "Otherwise, click the link below to complete the process." msgstr "" +msgid "Otherwise, click the link below to complete the process:" +msgstr "" + msgid "Our documentation includes an example DevOps Score report." msgstr "" diff --git a/spec/factories/services.rb b/spec/factories/services.rb index 25ef75880bb..51d9898dbe4 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -159,12 +159,6 @@ FactoryBot.define do password { 'my-secret-password' } end - factory :hipchat_service do - project - type { 'HipchatService' } - token { 'test_token' } - end - factory :slack_service do project active { true } diff --git a/spec/features/merge_request/user_creates_merge_request_spec.rb b/spec/features/merge_request/user_creates_merge_request_spec.rb index 119cf31098c..617aceae54c 100644 --- a/spec/features/merge_request/user_creates_merge_request_spec.rb +++ b/spec/features/merge_request/user_creates_merge_request_spec.rb @@ -7,6 +7,7 @@ RSpec.describe "User creates a merge request", :js do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } + let(:title) { "Some feature" } before do diff --git a/spec/features/merge_request/user_posts_notes_spec.rb b/spec/features/merge_request/user_posts_notes_spec.rb index a6dfae72912..83d9388914b 100644 --- a/spec/features/merge_request/user_posts_notes_spec.rb +++ b/spec/features/merge_request/user_posts_notes_spec.rb @@ -6,6 +6,7 @@ RSpec.describe 'Merge request > User posts notes', :js do include NoteInteractionHelpers let_it_be(:project) { create(:project, :repository) } + let(:user) { project.creator } let(:merge_request) do create(:merge_request, source_project: project, target_project: project) diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 5d1e3c79474..817c3769b80 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -369,7 +369,6 @@ project: - packagist_service - pivotaltracker_service - prometheus_service -- hipchat_service - flowdock_service - assembla_service - asana_service diff --git a/spec/migrations/remove_hipchat_service_records_spec.rb b/spec/migrations/remove_hipchat_service_records_spec.rb new file mode 100644 index 00000000000..bc76d7933d8 --- /dev/null +++ b/spec/migrations/remove_hipchat_service_records_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20210420103955_remove_hipchat_service_records.rb') + +RSpec.describe RemoveHipchatServiceRecords do + let(:services) { table(:services) } + + before do + services.create!(type: 'HipchatService') + services.create!(type: 'SomeOtherType') + end + + it 'removes services records of type HipchatService' do + expect(services.count).to eq(2) + + migrate! + + expect(services.count).to eq(1) + expect(services.first.type).to eq('SomeOtherType') + expect(services.where(type: 'HipchatService')).to be_empty + end +end diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index 82a4cde752b..42368c31ba0 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -2,91 +2,35 @@ require 'spec_helper' +# HipchatService is partially removed and it will be remove completely +# after the deletion of all the database records. +# https://gitlab.com/gitlab-org/gitlab/-/issues/27954 RSpec.describe HipchatService do - describe "Associations" do - it { is_expected.to belong_to :project } - it { is_expected.to have_one :service_hook } - end + let_it_be(:project) { create(:project) } - describe 'Validations' do - context 'when service is active' do - before do - subject.active = true - end + subject(:service) { described_class.new(project: project) } - it { is_expected.to validate_presence_of(:token) } - end + it { is_expected.to be_valid } - context 'when service is inactive' do - before do - subject.active = false - end + describe '#to_param' do + subject { service.to_param } - it { is_expected.not_to validate_presence_of(:token) } - end + it { is_expected.to eq('hipchat') } end - describe "Execute" do - let(:hipchat) { described_class.new } - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:api_url) { 'https://hipchat.example.com/v2/room/123456/notification?auth_token=verySecret' } - let(:project_name) { project.full_name.gsub(/\s/, '') } - let(:token) { 'verySecret' } - let(:server_url) { 'https://hipchat.example.com'} - let(:push_sample_data) do - Gitlab::DataBuilder::Push.build_sample(project, user) - end - - before do - allow(hipchat).to receive_messages( - project_id: project.id, - project: project, - room: 123456, - server: server_url, - token: token - ) - WebMock.stub_request(:post, api_url) - end - - it 'does nothing' do - expect { hipchat.execute(push_sample_data) }.not_to raise_error - end + describe '#supported_events' do + subject { service.supported_events } - describe "#message_options" do - it "is set to the defaults" do - expect(hipchat.__send__(:message_options)).to eq({ notify: false, color: 'yellow' }) - end - - it "sets notify to true" do - allow(hipchat).to receive(:notify).and_return('1') - - expect(hipchat.__send__(:message_options)).to eq({ notify: true, color: 'yellow' }) - end - - it "sets the color" do - allow(hipchat).to receive(:color).and_return('red') - - expect(hipchat.__send__(:message_options)).to eq({ notify: false, color: 'red' }) - end - - context 'with a successful build' do - it 'uses the green color' do - data = { object_kind: 'pipeline', - object_attributes: { status: 'success' } } - - expect(hipchat.__send__(:message_options, data)).to eq({ notify: false, color: 'green' }) - end - end + it { is_expected.to be_empty } + end - context 'with a failed build' do - it 'uses the red color' do - data = { object_kind: 'pipeline', - object_attributes: { status: 'failed' } } + describe '#save' do + it 'prevents records from being created or updated' do + expect(service.save).to be_falsey - expect(hipchat.__send__(:message_options, data)).to eq({ notify: false, color: 'red' }) - end - end + expect(service.errors.full_messages).to include( + 'HipChat endpoint is deprecated and should not be created or modified.' + ) end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 12c17e699e3..8c21b227ad3 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -52,7 +52,6 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_one(:pipelines_email_service) } it { is_expected.to have_one(:irker_service) } it { is_expected.to have_one(:pivotaltracker_service) } - it { is_expected.to have_one(:hipchat_service) } it { is_expected.to have_one(:flowdock_service) } it { is_expected.to have_one(:assembla_service) } it { is_expected.to have_one(:slack_slash_commands_service) } |