diff options
33 files changed, 302 insertions, 37 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index be1818391ca..be8b36128b2 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -16,6 +16,8 @@ stages: default: tags: - gitlab-org + # All jobs are interruptible by default + interruptible: true workflow: rules: diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index 6420e951a78..3dc09b9988e 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -16,6 +16,9 @@ .if-master-refs: &if-master-refs if: '$CI_COMMIT_REF_NAME == "master"' +.if-auto-deploy-branches: &if-auto-deploy-branches + if: '$CI_COMMIT_BRANCH =~ /^\d+-\d+-auto-deploy-\d+$/' + .if-master-or-tag: &if-master-or-tag if: '$CI_COMMIT_REF_NAME == "master" || $CI_COMMIT_TAG' @@ -509,6 +512,14 @@ changes: *code-backstage-qa-patterns when: on_success +.setup:rules:dont-interrupt-me: + rules: + - <<: *if-master-or-tag + when: on_success + - <<: *if-auto-deploy-branches + when: on_success + - when: manual + .setup:rules:gitlab_git_test: rules: - <<: *if-default-refs diff --git a/.gitlab/ci/setup.gitlab-ci.yml b/.gitlab/ci/setup.gitlab-ci.yml index b1918961f3e..60e52ffdf5e 100644 --- a/.gitlab/ci/setup.gitlab-ci.yml +++ b/.gitlab/ci/setup.gitlab-ci.yml @@ -23,6 +23,18 @@ cache gems: - .default-retry needs: [] +dont-interrupt-me: + extends: .setup:rules:dont-interrupt-me + stage: prepare + image: alpine:edge + interruptible: false + allow_failure: true + variables: + GIT_STRATEGY: none + dependencies: [] + script: + - echo "This jobs makes sure this pipeline won't be interrupted! See https://docs.gitlab.com/ee/ci/yaml/#interruptible." + gitlab_git_test: extends: - .minimal-job diff --git a/app/finders/keys_finder.rb b/app/finders/keys_finder.rb index 0263d809246..e7e78d71a58 100644 --- a/app/finders/keys_finder.rb +++ b/app/finders/keys_finder.rb @@ -46,7 +46,7 @@ class KeysFinder return keys unless params[:fingerprint].present? raise InvalidFingerprint unless valid_fingerprint_param? - keys.where(fingerprint_query).first # rubocop: disable CodeReuse/ActiveRecord + keys.find_by(fingerprint_query) # rubocop: disable CodeReuse/ActiveRecord end def valid_fingerprint_param? diff --git a/app/models/notification_recipient.rb b/app/models/notification_recipient.rb index 107d00d055a..36b7cd64c73 100644 --- a/app/models/notification_recipient.rb +++ b/app/models/notification_recipient.rb @@ -74,10 +74,12 @@ class NotificationRecipient end def unsubscribed? - return false unless @target - return false unless @target.respond_to?(:subscriptions) + subscribable_target = @target.is_a?(Note) ? @target.noteable : @target - subscription = @target.subscriptions.find { |subscription| subscription.user_id == @user.id } + return false unless subscribable_target + return false unless subscribable_target.respond_to?(:subscriptions) + + subscription = subscribable_target.subscriptions.find { |subscription| subscription.user_id == @user.id } subscription && !subscription.subscribed end diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb index 82ebf6ba6a5..2217aa1326c 100644 --- a/app/policies/note_policy.rb +++ b/app/policies/note_policy.rb @@ -19,7 +19,7 @@ class NotePolicy < BasePolicy condition(:confidential, scope: :subject) { @subject.confidential? } condition(:can_read_confidential) do - access_level >= Gitlab::Access::REPORTER || @subject.noteable_assignee_or_author?(@user) + access_level >= Gitlab::Access::REPORTER || @subject.noteable_assignee_or_author?(@user) || admin? end rule { ~editable }.prevent :admin_note diff --git a/app/services/notification_recipients/builder/base.rb b/app/services/notification_recipients/builder/base.rb index 3aa00c09ba2..81e6750a4ca 100644 --- a/app/services/notification_recipients/builder/base.rb +++ b/app/services/notification_recipients/builder/base.rb @@ -23,6 +23,11 @@ module NotificationRecipients raise 'abstract' end + # override if needed + def recipients_target + target + end + def project target.project end @@ -59,7 +64,7 @@ module NotificationRecipients project: project, group: group, custom_action: custom_action, - target: target, + target: recipients_target, acting_user: acting_user ) end diff --git a/app/services/notification_recipients/builder/new_note.rb b/app/services/notification_recipients/builder/new_note.rb index 27699a0d9cc..17e4728d352 100644 --- a/app/services/notification_recipients/builder/new_note.rb +++ b/app/services/notification_recipients/builder/new_note.rb @@ -12,6 +12,10 @@ module NotificationRecipients note.noteable end + def recipients_target + note + end + # NOTE: may be nil, in the case of a PersonalSnippet # # (this is okay because NotificationRecipient is written diff --git a/changelogs/unreleased/cleanup-migration-to-security-scans.yml b/changelogs/unreleased/cleanup-migration-to-security-scans.yml new file mode 100644 index 00000000000..d7c12ba566f --- /dev/null +++ b/changelogs/unreleased/cleanup-migration-to-security-scans.yml @@ -0,0 +1,5 @@ +--- +title: Complete the migration of Job Artifact to Security Scan +merge_request: 24244 +author: +type: other diff --git a/danger/telemetry/Dangerfile b/danger/telemetry/Dangerfile index 68a226ef11b..227621cc1dd 100644 --- a/danger/telemetry/Dangerfile +++ b/danger/telemetry/Dangerfile @@ -6,14 +6,28 @@ review from the Data team and Telemetry team is recommended. @gitlab-org/growth/telemetry group is mentioned in order to notify team members. MSG +USAGE_DATA_FILES_MESSAGE = <<~MSG +For the following files, a review from the [Data team and Telemetry team](https://gitlab.com/groups/gitlab-org/growth/telemetry/-/group_members?with_inherited_permissions=exclude) is recommended: +MSG + usage_data_changed_files = git.modified_files.grep(%r{usage_data}) +def has_label?(label) + gitlab.mr_labels.include?(label) +end + +def labels_for_merge_request(labels) + labels_list = labels.map { |label| %Q{~"#{label}"} }.join(' ') + "/label #{labels_list}" +end + if usage_data_changed_files.any? warn format(TELEMETRY_CHANGED_FILES_MESSAGE) - USAGE_DATA_FILES_MESSAGE = <<~MSG - For the following files, a review from the [Data team and Telemetry team](https://gitlab.com/groups/gitlab-org/growth/telemetry/-/group_members?with_inherited_permissions=exclude) is recommended: - MSG - markdown(USAGE_DATA_FILES_MESSAGE + helper.markdown_list(usage_data_changed_files)) + + telemetry_labels = ['telemetry'] + telemetry_labels << 'telemetry::review pending' unless has_label?('telemetry::reviewed') + + markdown(labels_for_merge_request(telemetry_labels)) end diff --git a/db/post_migrate/20200323011225_complete_migrate_security_scans.rb b/db/post_migrate/20200323011225_complete_migrate_security_scans.rb new file mode 100644 index 00000000000..39c9a78b1b6 --- /dev/null +++ b/db/post_migrate/20200323011225_complete_migrate_security_scans.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class CompleteMigrateSecurityScans < ActiveRecord::Migration[6.0] + disable_ddl_transaction! + + def up + Gitlab::BackgroundMigration.steal('MigrateSecurityScans') + end + + def down + # intentionally blank + end +end diff --git a/db/post_migrate/20200323011955_remove_index_used_for_scan_migration.rb b/db/post_migrate/20200323011955_remove_index_used_for_scan_migration.rb new file mode 100644 index 00000000000..0568cdb8483 --- /dev/null +++ b/db/post_migrate/20200323011955_remove_index_used_for_scan_migration.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class RemoveIndexUsedForScanMigration < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'job_artifacts_secure_reports_temp_index' + COLUMNS = [:id, :file_type, :job_id, :created_at, :updated_at] + + disable_ddl_transaction! + + def up + if index_exists?(:ci_job_artifacts, COLUMNS, name: INDEX_NAME) + remove_concurrent_index(:ci_job_artifacts, COLUMNS, name: INDEX_NAME) + end + end + + def down + add_concurrent_index(:ci_job_artifacts, + COLUMNS, + name: INDEX_NAME, + where: 'file_type BETWEEN 5 AND 8') + end +end diff --git a/db/structure.sql b/db/structure.sql index 8bd83517bb8..bb291590abc 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -10214,8 +10214,6 @@ CREATE UNIQUE INDEX issue_user_mentions_on_issue_id_and_note_id_index ON public. CREATE UNIQUE INDEX issue_user_mentions_on_issue_id_index ON public.issue_user_mentions USING btree (issue_id) WHERE (note_id IS NULL); -CREATE INDEX job_artifacts_secure_reports_temp_index ON public.ci_job_artifacts USING btree (id, file_type, job_id, created_at, updated_at) WHERE ((file_type >= 5) AND (file_type <= 8)); - CREATE UNIQUE INDEX kubernetes_namespaces_cluster_and_namespace ON public.clusters_kubernetes_namespaces USING btree (cluster_id, namespace); CREATE INDEX merge_request_mentions_temp_index ON public.merge_requests USING btree (id) WHERE ((description ~~ '%@%'::text) OR ((title)::text ~~ '%@%'::text)); @@ -12800,6 +12798,8 @@ COPY "schema_migrations" (version) FROM STDIN; 20200319203901 20200320112455 20200320123839 +20200323011225 +20200323011955 20200323071918 20200323074147 20200323075043 diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 9766fc39e2e..6c5de925fe9 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -6401,6 +6401,26 @@ type Project { Returns the last _n_ elements from the list. """ last: Int + + """ + Filter vulnerabilities by project + """ + projectId: [ID!] + + """ + Filter vulnerabilities by report type + """ + reportType: [VulnerabilityReportType!] + + """ + Filter vulnerabilities by severity + """ + severity: [VulnerabilitySeverity!] + + """ + Filter vulnerabilities by state + """ + state: [VulnerabilityState!] ): VulnerabilityConnection """ diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 7584e48b1e5..14d4f798f6e 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -19019,6 +19019,78 @@ "description": "Vulnerabilities reported on the project. Available only when feature flag `first_class_vulnerabilities` is enabled", "args": [ { + "name": "projectId", + "description": "Filter vulnerabilities by project", + "type": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "ID", + "ofType": null + } + } + }, + "defaultValue": null + }, + { + "name": "reportType", + "description": "Filter vulnerabilities by report type", + "type": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "ENUM", + "name": "VulnerabilityReportType", + "ofType": null + } + } + }, + "defaultValue": null + }, + { + "name": "severity", + "description": "Filter vulnerabilities by severity", + "type": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "ENUM", + "name": "VulnerabilitySeverity", + "ofType": null + } + } + }, + "defaultValue": null + }, + { + "name": "state", + "description": "Filter vulnerabilities by state", + "type": { + "kind": "LIST", + "name": null, + "ofType": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "ENUM", + "name": "VulnerabilityState", + "ofType": null + } + } + }, + "defaultValue": null + }, + { "name": "after", "description": "Returns the elements in the list that come after the specified cursor.", "type": { diff --git a/doc/development/pipelines.md b/doc/development/pipelines.md index 5e2b5b3c230..bfd184d7d5a 100644 --- a/doc/development/pipelines.md +++ b/doc/development/pipelines.md @@ -142,6 +142,15 @@ and included in `rules` definitions via [YAML anchors](../ci/yaml/README.md#anch | `code-qa-patterns` | Combination of `code-patterns` and `qa-patterns`. | | `code-backstage-qa-patterns` | Combination of `code-patterns`, `backstage-patterns`, and `qa-patterns`. | +## Interruptible jobs pipelines + +By default, all jobs are [interruptible](../ci/yaml/README.md#interruptible), except the +`dont-interrupt-me` job which runs automatically on `master`, and is `manual` +otherwise. + +If you want a running pipeline to finish even if you push new commits to a merge +request, be sure to start the `dont-interrupt-me` job before pushing. + ## Directed acyclic graph We're using the [`needs:`](../ci/yaml/README.md#needs) keyword to diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces.rb index cea25967801..6b9af51a6ab 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_namespaces.rb @@ -45,7 +45,7 @@ module Gitlab reverts_for_type('namespace') do |path_before_rename, current_path| matches_path = MigrationClasses::Route.arel_table[:path].matches(current_path) namespace = MigrationClasses::Namespace.joins(:route) - .where(matches_path).first&.becomes(MigrationClasses::Namespace) + .find_by(matches_path)&.becomes(MigrationClasses::Namespace) if namespace perform_rename(namespace, current_path, path_before_rename) diff --git a/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb b/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb index 4dc7a62797a..8b92b296408 100644 --- a/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb +++ b/lib/gitlab/database/rename_reserved_paths_migration/v1/rename_projects.rb @@ -37,7 +37,7 @@ module Gitlab reverts_for_type('project') do |path_before_rename, current_path| matches_path = MigrationClasses::Route.arel_table[:path].matches(current_path) project = MigrationClasses::Project.joins(:route) - .where(matches_path).first + .find_by(matches_path) if project perform_rename(project, current_path, path_before_rename) diff --git a/lib/gitlab/import_export/base/object_builder.rb b/lib/gitlab/import_export/base/object_builder.rb index 109d2e233a5..48836729ff6 100644 --- a/lib/gitlab/import_export/base/object_builder.rb +++ b/lib/gitlab/import_export/base/object_builder.rb @@ -62,7 +62,7 @@ module Gitlab end def find_object - klass.where(where_clause).first + klass.find_by(where_clause) end def where_clause diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index 5832b72ab46..263c49c509f 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -19,7 +19,7 @@ module Gitlab @exported_members.inject(missing_keys_tracking_hash) do |hash, member| if member['user'] old_user_id = member['user']['id'] - existing_user = User.where(find_user_query(member)).first + existing_user = User.find_by(find_user_query(member)) hash[old_user_id] = existing_user.id if existing_user && add_team_member(member, existing_user) else add_team_member(member) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6de9caf097a..7a718a39a4e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9144,6 +9144,9 @@ msgstr "" msgid "Geo Nodes" msgstr "" +msgid "Geo Nodes|Cannot remove a primary node if there is a secondary node" +msgstr "" + msgid "Geo Settings" msgstr "" diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb index 2957d0bed15..9daf35d0311 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -67,7 +67,7 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, schema: 201802081 it 'does not add hashed files to the untracked_files_for_uploads table' do described_class.new.perform - hashed_file_path = get_uploads(project2, 'Project').where(uploader: 'FileUploader').first.path + hashed_file_path = get_uploads(project2, 'Project').find_by(uploader: 'FileUploader').path expect(untracked_files_for_uploads.where("path like '%#{hashed_file_path}%'").exists?).to be_falsey end diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index e38ef75d085..229f45e9543 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -123,7 +123,7 @@ describe Gitlab::ImportExport::Project::TreeRestorer do end it 'preserves updated_at on issues' do - issue = Issue.where(description: 'Aliquam enim illo et possimus.').first + issue = Issue.find_by(description: 'Aliquam enim illo et possimus.') expect(issue.reload.updated_at.to_s).to eq('2016-06-14 15:02:47 UTC') end @@ -170,7 +170,7 @@ describe Gitlab::ImportExport::Project::TreeRestorer do end context 'event at forth level of the tree' do - let(:event) { Event.where(action: 6).first } + let(:event) { Event.find_by(action: 6) } it 'restores the event' do expect(event).not_to be_nil @@ -440,7 +440,7 @@ describe Gitlab::ImportExport::Project::TreeRestorer do end it 'restores external pull request for the restored pipeline' do - pipeline_with_external_pr = @project.ci_pipelines.where(source: 'external_pull_request_event').first + pipeline_with_external_pr = @project.ci_pipelines.find_by(source: 'external_pull_request_event') expect(pipeline_with_external_pr.external_pull_request).to be_persisted end diff --git a/spec/migrations/generate_missing_routes_spec.rb b/spec/migrations/generate_missing_routes_spec.rb index 3ff220aa8d3..bc7cd3bd55e 100644 --- a/spec/migrations/generate_missing_routes_spec.rb +++ b/spec/migrations/generate_missing_routes_spec.rb @@ -26,7 +26,7 @@ describe GenerateMissingRoutes do described_class.new.up - route = routes.where(source_type: 'Project').take + route = routes.find_by(source_type: 'Project') expect(route.source_id).to eq(project.id) expect(route.path).to eq("gitlab/gitlab-ce-#{project.id}") @@ -37,7 +37,7 @@ describe GenerateMissingRoutes do described_class.new.up - route = routes.where(source_type: 'Namespace').take + route = routes.find_by(source_type: 'Namespace') expect(route.source_id).to eq(namespace.id) expect(route.path).to eq("gitlab-#{namespace.id}") diff --git a/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb b/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb index 9188c19f76a..2fe6f3f62a9 100644 --- a/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb +++ b/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb @@ -89,11 +89,11 @@ describe MigrateAutoDevOpsDomainToClusterDomain do end def find_cluster_project(project_id) - cluster_projects_table.where(project_id: project_id).first + cluster_projects_table.find_by(project_id: project_id) end def find_cluster(cluster_id) - clusters_table.where(id: cluster_id).first + clusters_table.find_by(id: cluster_id) end def project_auto_devops_with_domain diff --git a/spec/migrations/nullify_users_role_spec.rb b/spec/migrations/nullify_users_role_spec.rb index 487d84e2a35..b11929ef76f 100644 --- a/spec/migrations/nullify_users_role_spec.rb +++ b/spec/migrations/nullify_users_role_spec.rb @@ -18,16 +18,16 @@ describe NullifyUsersRole do it 'nullifies the role of the user with updated_at < 2019-11-05 12:08:00 and a role of 0' do expect(users.where(role: nil).count).to eq(1) - expect(users.where(role: nil).first.email).to eq('1') + expect(users.find_by(role: nil).email).to eq('1') end it 'leaves the user with role of 1' do expect(users.where(role: 1).count).to eq(1) - expect(users.where(role: 1).first.email).to eq('2') + expect(users.find_by(role: 1).email).to eq('2') end it 'leaves the user with updated_at > 2019-11-05 12:08:00' do expect(users.where(role: 0).count).to eq(1) - expect(users.where(role: 0).first.email).to eq('3') + expect(users.find_by(role: 0).email).to eq('3') end end diff --git a/spec/migrations/schedule_to_archive_legacy_traces_spec.rb b/spec/migrations/schedule_to_archive_legacy_traces_spec.rb index e9158df01c4..69c4b15a74f 100644 --- a/spec/migrations/schedule_to_archive_legacy_traces_spec.rb +++ b/spec/migrations/schedule_to_archive_legacy_traces_spec.rb @@ -39,9 +39,9 @@ describe ScheduleToArchiveLegacyTraces do expect(File.exist?(legacy_trace_path(@build_failed))).to be_falsy expect(File.exist?(legacy_trace_path(@builds_canceled))).to be_falsy expect(File.exist?(legacy_trace_path(@build_running))).to be_truthy - expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @build_success.id).first))).to be_truthy - expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @build_failed.id).first))).to be_truthy - expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @builds_canceled.id).first))).to be_truthy + expect(File.exist?(archived_trace_path(job_artifacts.find_by(job_id: @build_success.id)))).to be_truthy + expect(File.exist?(archived_trace_path(job_artifacts.find_by(job_id: @build_failed.id)))).to be_truthy + expect(File.exist?(archived_trace_path(job_artifacts.find_by(job_id: @builds_canceled.id)))).to be_truthy expect(job_artifacts.where(job_id: @build_running.id)).not_to be_exist end end diff --git a/spec/policies/note_policy_spec.rb b/spec/policies/note_policy_spec.rb index 94e6a86025c..e9dd5ee1c51 100644 --- a/spec/policies/note_policy_spec.rb +++ b/spec/policies/note_policy_spec.rb @@ -263,6 +263,7 @@ describe NotePolicy do let(:non_member) { create(:user) } let(:author) { create(:user) } let(:assignee) { create(:user) } + let(:admin) { create(:admin) } before do project.add_reporter(reporter) @@ -294,6 +295,10 @@ describe NotePolicy do expect(permissions(maintainer, confidential_note)).to be_allowed(:read_note, :admin_note, :resolve_note, :award_emoji) end + it 'allows admins to read all notes and admin them' do + expect(permissions(admin, confidential_note)).to be_allowed(:read_note, :admin_note, :resolve_note, :award_emoji) + end + it 'allows noteable author to read and resolve all notes' do expect(permissions(author, confidential_note)).to be_allowed(:read_note, :resolve_note, :award_emoji) expect(permissions(author, confidential_note)).to be_disallowed(:admin_note) diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 03dfd13c25b..bd8aeb65d3f 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -23,9 +23,9 @@ describe API::Jobs do json_job['artifacts'].each do |artifact| expect(artifact).not_to be_nil file_type = Ci::JobArtifact.file_types[artifact['file_type']] - expect(artifact['size']).to eq(second_job.job_artifacts.where(file_type: file_type).first.size) - expect(artifact['filename']).to eq(second_job.job_artifacts.where(file_type: file_type).first.filename) - expect(artifact['file_format']).to eq(second_job.job_artifacts.where(file_type: file_type).first.file_format) + expect(artifact['size']).to eq(second_job.job_artifacts.find_by(file_type: file_type).size) + expect(artifact['filename']).to eq(second_job.job_artifacts.find_by(file_type: file_type).filename) + expect(artifact['file_format']).to eq(second_job.job_artifacts.find_by(file_type: file_type).file_format) end end end diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 87f93ec97c9..23c2f53dca0 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -16,7 +16,7 @@ describe Emails::CreateService do it 'creates an email with additional attributes' do expect { service.execute(confirmation_token: 'abc') }.to change { Email.count }.by(1) - expect(Email.where(opts).first.confirmation_token).to eq 'abc' + expect(Email.find_by(opts).confirmation_token).to eq 'abc' end it 'has the right user association' do diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index ccd4dd4231b..a449541f459 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -100,7 +100,7 @@ describe Issues::MoveService do context 'when issue has notes with mentions' do it 'saves user mentions with actual mentions for new issue' do - expect(new_issue.user_mentions.where(note_id: nil).first.mentioned_users_ids).to match_array([user.id]) + expect(new_issue.user_mentions.find_by(note_id: nil).mentioned_users_ids).to match_array([user.id]) expect(new_issue.user_mentions.where.not(note_id: nil).first.mentioned_users_ids).to match_array([user.id]) expect(new_issue.user_mentions.where.not(note_id: nil).count).to eq 1 expect(new_issue.user_mentions.count).to eq 2 diff --git a/spec/services/notification_recipients/builder/new_note_spec.rb b/spec/services/notification_recipients/builder/new_note_spec.rb new file mode 100644 index 00000000000..f88e8b2dfb0 --- /dev/null +++ b/spec/services/notification_recipients/builder/new_note_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe NotificationRecipients::Builder::NewNote do + describe '#notification_recipients' do + let_it_be(:group) { create(:group, :public) } + let_it_be(:project) { create(:project, :public, group: group) } + let_it_be(:issue) { create(:issue, project: project) } + + let_it_be(:other_user) { create(:user) } + let_it_be(:participant) { create(:user) } + let_it_be(:non_member_participant) { create(:user) } + let_it_be(:group_watcher) { create(:user) } + let_it_be(:project_watcher) { create(:user) } + let_it_be(:guest_project_watcher) { create(:user) } + let_it_be(:subscriber) { create(:user) } + let_it_be(:unsubscribed_user) { create(:user) } + let_it_be(:non_member_subscriber) { create(:user) } + + let_it_be(:notification_setting_project_w) { create(:notification_setting, source: project, user: project_watcher, level: 2) } + let_it_be(:notification_setting_guest_w) { create(:notification_setting, source: project, user: guest_project_watcher, level: 2) } + let_it_be(:notification_setting_group_w) { create(:notification_setting, source: group, user: group_watcher, level: 2) } + let_it_be(:subscriptions) do + [ + create(:subscription, project: project, user: subscriber, subscribable: issue, subscribed: true), + create(:subscription, project: project, user: unsubscribed_user, subscribable: issue, subscribed: false), + create(:subscription, project: project, user: non_member_subscriber, subscribable: issue, subscribed: true) + ] + end + + subject { described_class.new(note) } + + before do + project.add_developer(participant) + project.add_developer(project_watcher) + project.add_guest(guest_project_watcher) + project.add_developer(subscriber) + group.add_developer(group_watcher) + + expect(issue).to receive(:participants).and_return([participant, non_member_participant]) + end + + context 'for public notes' do + let_it_be(:note) { create(:note, noteable: issue, project: project) } + + it 'adds all participants, watchers and subscribers' do + expect(subject.notification_recipients.map(&:user)).to contain_exactly( + participant, non_member_participant, project_watcher, group_watcher, guest_project_watcher, subscriber, non_member_subscriber + ) + end + end + + context 'for confidential notes' do + let_it_be(:note) { create(:note, :confidential, noteable: issue, project: project) } + + it 'adds all participants, watchers and subscribers that are project memebrs' do + expect(subject.notification_recipients.map(&:user)).to contain_exactly( + participant, project_watcher, group_watcher, subscriber + ) + end + end + end +end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 55987c6fa0f..443e3dfddf1 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -321,9 +321,9 @@ describe Projects::ForkService do Projects::UpdateRepositoryStorageService.new(project).execute('test_second_storage') fork_after_move = fork_project(project) pool_repository_before_move = PoolRepository.joins(:shard) - .where(source_project: project, shards: { name: 'default' }).first + .find_by(source_project: project, shards: { name: 'default' }) pool_repository_after_move = PoolRepository.joins(:shard) - .where(source_project: project, shards: { name: 'test_second_storage' }).first + .find_by(source_project: project, shards: { name: 'test_second_storage' }) expect(fork_before_move.pool_repository).to eq(pool_repository_before_move) expect(fork_after_move.pool_repository).to eq(pool_repository_after_move) |