diff options
25 files changed, 249 insertions, 92 deletions
diff --git a/.gitignore b/.gitignore index ad7595dc7f2..246392bf175 100644 --- a/.gitignore +++ b/.gitignore @@ -42,6 +42,7 @@ eslint-report.html /config/initializers/relative_url.rb /config/resque.yml /config/redis.*.yml +/config/redis.yml /config/unicorn.rb /config/puma.rb /config/secrets.yml diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 46c397d5c74..b0426e76599 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -505,7 +505,7 @@ lib/gitlab/checks/** @proglottis @toon @zj-gitlab /doc/administration/sidekiq/ @axil /doc/administration/sidekiq/sidekiq_memory_killer.md @jglassman1 /doc/administration/smime_signing_email.md @axil -/doc/administration/snippets/ @ashrafkhamis +/doc/administration/snippets/ @aqualls /doc/administration/static_objects_external_storage.md @ashrafkhamis /doc/administration/system_hooks.md @ashrafkhamis /doc/administration/terraform_state.md @phillipwells @@ -626,7 +626,7 @@ lib/gitlab/checks/** @proglottis @toon @zj-gitlab /doc/api/project_level_variables.md @marcel.amirault /doc/api/project_relations_export.md @eread /doc/api/project_repository_storage_moves.md @eread -/doc/api/project_snippets.md @ashrafkhamis +/doc/api/project_snippets.md @aqualls /doc/api/project_statistics.md @aqualls /doc/api/project_templates.md @aqualls /doc/api/project_vulnerabilities.md @aqualls @@ -652,8 +652,8 @@ lib/gitlab/checks/** @proglottis @toon @zj-gitlab /doc/api/secure_files.md @marcel.amirault /doc/api/settings.md @jglassman1 /doc/api/sidekiq_metrics.md @axil -/doc/api/snippet_repository_storage_moves.md @ashrafkhamis -/doc/api/snippets.md @ashrafkhamis +/doc/api/snippet_repository_storage_moves.md @aqualls +/doc/api/snippets.md @aqualls /doc/api/statistics.md @jglassman1 /doc/api/status_checks.md @eread /doc/api/suggestions.md @aqualls @@ -1064,7 +1064,7 @@ lib/gitlab/checks/** @proglottis @toon @zj-gitlab /doc/user/reserved_names.md @lciutacu /doc/user/search/ @ashrafkhamis /doc/user/shortcuts.md @ashrafkhamis -/doc/user/snippets.md @ashrafkhamis +/doc/user/snippets.md @aqualls /doc/user/ssh.md @jglassman1 /doc/user/tasks.md @msedlakjakubowski /doc/user/todos.md @msedlakjakubowski diff --git a/app/graphql/types/notes/noteable_interface.rb b/app/graphql/types/notes/noteable_interface.rb index bd22f12d6f0..537084dff62 100644 --- a/app/graphql/types/notes/noteable_interface.rb +++ b/app/graphql/types/notes/noteable_interface.rb @@ -7,6 +7,7 @@ module Types field :notes, Types::Notes::NoteType.connection_type, null: false, description: "All notes on this noteable." field :discussions, Types::Notes::DiscussionType.connection_type, null: false, description: "All discussions on this noteable." + field :commenters, Types::UserType.connection_type, null: false, description: "All commenters on this noteable." def self.resolve_type(object, context) case object @@ -24,6 +25,10 @@ module Types raise "Unknown GraphQL type for #{object}" end end + + def commenters + object.commenters(user: current_user) + end end end end diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb index 492d55c74e2..36b134fdb65 100644 --- a/app/models/concerns/noteable.rb +++ b/app/models/concerns/noteable.rb @@ -205,6 +205,14 @@ module Noteable model_name.singular end + def commenters(user: nil) + eligable_notes = notes.user + + eligable_notes = eligable_notes.not_internal unless user&.can?(:read_internal_note, self) + + User.where(id: eligable_notes.select(:author_id).distinct) + end + private # Synthetic system notes don't have discussion IDs because these are generated dynamically diff --git a/app/models/note.rb b/app/models/note.rb index 052df6142c5..e8c35ba33c7 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -125,6 +125,7 @@ class Note < ApplicationRecord scope :for_commit_id, ->(commit_id) { where(noteable_type: "Commit", commit_id: commit_id) } scope :system, -> { where(system: true) } scope :user, -> { where(system: false) } + scope :not_internal, -> { where(internal: false) } scope :common, -> { where(noteable_type: ["", nil]) } scope :fresh, -> { order_created_asc.with_order_id_asc } scope :updated_after, ->(time) { where('updated_at > ?', time) } diff --git a/doc/administration/geo/index.md b/doc/administration/geo/index.md index 68fd0c63e37..dec6939176c 100644 --- a/doc/administration/geo/index.md +++ b/doc/administration/geo/index.md @@ -224,6 +224,8 @@ An [epic exists](https://gitlab.com/groups/gitlab-org/-/epics/4623) to fix this The only way to view designs replication data for a particular secondary site is to visit that secondary site directly. For example, `https://<IP of your secondary site>/admin/geo/replication/designs`. An [epic exists](https://gitlab.com/groups/gitlab-org/-/epics/4624) to fix this limitation. +Keep in mind that mentioned URLs don't work when [Admin Mode](../../user/admin_area/settings/sign_in_restrictions.md#admin-mode) is enabled. + ## Setup instructions For setup instructions, see [Setting up Geo](setup/index.md). diff --git a/doc/administration/snippets/index.md b/doc/administration/snippets/index.md index 4bd03aeb8c8..613d161a64c 100644 --- a/doc/administration/snippets/index.md +++ b/doc/administration/snippets/index.md @@ -1,7 +1,7 @@ --- type: reference, howto stage: Create -group: Editor +group: Source Code info: "To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments" --- diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 0e05044f2c1..2c97a54697f 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -10276,6 +10276,7 @@ Describes an alert from the project's Alert Management. | Name | Type | Description | | ---- | ---- | ----------- | | <a id="alertmanagementalertassignees"></a>`assignees` | [`UserCoreConnection`](#usercoreconnection) | Assignees of the alert. (see [Connections](#connections)) | +| <a id="alertmanagementalertcommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="alertmanagementalertcreatedat"></a>`createdAt` | [`Time`](#time) | Timestamp the alert was created. | | <a id="alertmanagementalertdescription"></a>`description` | [`String`](#string) | Description of the alert. | | <a id="alertmanagementalertdetails"></a>`details` | [`JSON`](#json) | Alert details. | @@ -10615,6 +10616,7 @@ Represents an epic on an issue board. | <a id="boardepicblockingcount"></a>`blockingCount` | [`Int`](#int) | Count of epics that this epic is blocking. | | <a id="boardepicclosedat"></a>`closedAt` | [`Time`](#time) | Timestamp of when the epic was closed. | | <a id="boardepiccolor"></a>`color` | [`String`](#string) | Color of the epic. Returns `null` if `epic_color_highlight` feature flag is disabled. | +| <a id="boardepiccommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="boardepicconfidential"></a>`confidential` | [`Boolean`](#boolean) | Indicates if the epic is confidential. | | <a id="boardepiccreatedat"></a>`createdAt` | [`Time`](#time) | Timestamp of when the epic was created. | | <a id="boardepicdefaultprojectforissuecreation"></a>`defaultProjectForIssueCreation` | [`Project`](#project) | Default Project for issue creation. Based on the project the user created the last issue in. | @@ -12116,6 +12118,7 @@ A single design. | Name | Type | Description | | ---- | ---- | ----------- | +| <a id="designcommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="designdiffrefs"></a>`diffRefs` | [`DiffRefs!`](#diffrefs) | Diff refs for this design. | | <a id="designdiscussions"></a>`discussions` | [`DiscussionConnection!`](#discussionconnection) | All discussions on this noteable. (see [Connections](#connections)) | | <a id="designevent"></a>`event` | [`DesignVersionEvent!`](#designversionevent) | How this design was changed in the current version. | @@ -12617,6 +12620,7 @@ Represents an epic. | <a id="epicblockingcount"></a>`blockingCount` | [`Int`](#int) | Count of epics that this epic is blocking. | | <a id="epicclosedat"></a>`closedAt` | [`Time`](#time) | Timestamp of when the epic was closed. | | <a id="epiccolor"></a>`color` | [`String`](#string) | Color of the epic. Returns `null` if `epic_color_highlight` feature flag is disabled. | +| <a id="epiccommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="epicconfidential"></a>`confidential` | [`Boolean`](#boolean) | Indicates if the epic is confidential. | | <a id="epiccreatedat"></a>`createdAt` | [`Time`](#time) | Timestamp of when the epic was created. | | <a id="epicdefaultprojectforissuecreation"></a>`defaultProjectForIssueCreation` | [`Project`](#project) | Default Project for issue creation. Based on the project the user created the last issue in. | @@ -12858,6 +12862,7 @@ Relationship between an epic and an issue. | <a id="epicissueblockingcount"></a>`blockingCount` | [`Int!`](#int) | Count of issues this issue is blocking. | | <a id="epicissueclosedasduplicateof"></a>`closedAsDuplicateOf` | [`Issue`](#issue) | Issue this issue was closed as a duplicate of. | | <a id="epicissueclosedat"></a>`closedAt` | [`Time`](#time) | Timestamp of when the issue was closed. | +| <a id="epicissuecommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="epicissueconfidential"></a>`confidential` | [`Boolean!`](#boolean) | Indicates the issue is confidential. | | <a id="epicissuecreatenoteemail"></a>`createNoteEmail` | [`String`](#string) | User specific email address for the issue. | | <a id="epicissuecreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp of when the issue was created. | @@ -14558,6 +14563,7 @@ Describes an issuable resource link for incident issues. | <a id="issueblockingcount"></a>`blockingCount` | [`Int!`](#int) | Count of issues this issue is blocking. | | <a id="issueclosedasduplicateof"></a>`closedAsDuplicateOf` | [`Issue`](#issue) | Issue this issue was closed as a duplicate of. | | <a id="issueclosedat"></a>`closedAt` | [`Time`](#time) | Timestamp of when the issue was closed. | +| <a id="issuecommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="issueconfidential"></a>`confidential` | [`Boolean!`](#boolean) | Indicates the issue is confidential. | | <a id="issuecreatenoteemail"></a>`createNoteEmail` | [`String`](#string) | User specific email address for the issue. | | <a id="issuecreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp of when the issue was created. | @@ -14986,6 +14992,7 @@ Defines which user roles, users, or groups can merge into a protected branch. | <a id="mergerequestautomergeenabled"></a>`autoMergeEnabled` | [`Boolean!`](#boolean) | Indicates if auto merge is enabled for the merge request. | | <a id="mergerequestautomergestrategy"></a>`autoMergeStrategy` | [`String`](#string) | Selected auto merge strategy. | | <a id="mergerequestavailableautomergestrategies"></a>`availableAutoMergeStrategies` | [`[String!]`](#string) | Array of available auto merge strategies. | +| <a id="mergerequestcommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="mergerequestcommitcount"></a>`commitCount` | [`Int`](#int) | Number of commits in the merge request. | | <a id="mergerequestcommits"></a>`commits` | [`CommitConnection`](#commitconnection) | Merge request commits. (see [Connections](#connections)) | | <a id="mergerequestcommitswithoutmergecommits"></a>`commitsWithoutMergeCommits` | [`CommitConnection`](#commitconnection) | Merge request commits excluding merge commits. (see [Connections](#connections)) | @@ -19412,6 +19419,7 @@ Represents a snippet entry. | Name | Type | Description | | ---- | ---- | ----------- | | <a id="snippetauthor"></a>`author` | [`UserCore`](#usercore) | Owner of the snippet. | +| <a id="snippetcommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="snippetcreatedat"></a>`createdAt` | [`Time!`](#time) | Timestamp this snippet was created. | | <a id="snippetdescription"></a>`description` | [`String`](#string) | Description of the snippet. | | <a id="snippetdescriptionhtml"></a>`descriptionHtml` | [`String`](#string) | The GitLab Flavored Markdown rendering of `description`. | @@ -20282,6 +20290,7 @@ Represents a vulnerability. | Name | Type | Description | | ---- | ---- | ----------- | +| <a id="vulnerabilitycommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="vulnerabilityconfirmedat"></a>`confirmedAt` | [`Time`](#time) | Timestamp of when the vulnerability state was changed to confirmed. | | <a id="vulnerabilityconfirmedby"></a>`confirmedBy` | [`UserCore`](#usercore) | User that confirmed the vulnerability. | | <a id="vulnerabilitydescription"></a>`description` | [`String`](#string) | Description of the vulnerability. | @@ -24173,6 +24182,7 @@ Implementations: | Name | Type | Description | | ---- | ---- | ----------- | +| <a id="noteableinterfacecommenters"></a>`commenters` | [`UserCoreConnection!`](#usercoreconnection) | All commenters on this noteable. (see [Connections](#connections)) | | <a id="noteableinterfacediscussions"></a>`discussions` | [`DiscussionConnection!`](#discussionconnection) | All discussions on this noteable. (see [Connections](#connections)) | | <a id="noteableinterfacenotes"></a>`notes` | [`NoteConnection!`](#noteconnection) | All notes on this noteable. (see [Connections](#connections)) | diff --git a/doc/api/project_snippets.md b/doc/api/project_snippets.md index afb7519d5f3..9c74df5639b 100644 --- a/doc/api/project_snippets.md +++ b/doc/api/project_snippets.md @@ -1,6 +1,6 @@ --- stage: Create -group: Editor +group: Source Code info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- diff --git a/doc/api/snippet_repository_storage_moves.md b/doc/api/snippet_repository_storage_moves.md index 1ef5299668d..acf9b50704d 100644 --- a/doc/api/snippet_repository_storage_moves.md +++ b/doc/api/snippet_repository_storage_moves.md @@ -1,6 +1,6 @@ --- stage: Create -group: Editor +group: Source Code info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments type: reference --- diff --git a/doc/api/snippets.md b/doc/api/snippets.md index c312642a450..2a52c989cc6 100644 --- a/doc/api/snippets.md +++ b/doc/api/snippets.md @@ -1,6 +1,6 @@ --- stage: Create -group: Editor +group: Source Code info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- diff --git a/doc/user/admin_area/settings/sign_in_restrictions.md b/doc/user/admin_area/settings/sign_in_restrictions.md index 6ba4d3423aa..d663238a88c 100644 --- a/doc/user/admin_area/settings/sign_in_restrictions.md +++ b/doc/user/admin_area/settings/sign_in_restrictions.md @@ -115,6 +115,9 @@ authentication steps. We may address these limitations in the future. For more information see the following epic: [Admin Mode for GitLab Administrators](https://gitlab.com/groups/gitlab-org/-/epics/2158). +Also, when GitLab Geo is enabled, you can't view the replication status of projects and designs while +on a secondary node. A fix is proposed when projects ([issue 367926](https://gitlab.com/gitlab-org/gitlab/-/issues/367926)) and designs ([issue 355660](https://gitlab.com/gitlab-org/gitlab/-/issues/355660)) move to the new Geo framework. + ### Troubleshooting Admin Mode If necessary, you can disable **Admin Mode** as an administrator by using one of these two methods: diff --git a/doc/user/snippets.md b/doc/user/snippets.md index ee84f8a4169..70669748cd8 100644 --- a/doc/user/snippets.md +++ b/doc/user/snippets.md @@ -1,6 +1,6 @@ --- stage: Create -group: Editor +group: Source Code info: "To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments" type: reference --- diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index de66ca7305f..e5f8a255f7d 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -78,7 +78,7 @@ module Gitlab def commit_deltas(commit) request = Gitaly::CommitDeltaRequest.new(diff_from_parent_request_params(commit)) response = gitaly_client_call(@repository.storage, :diff_service, :commit_delta, request, timeout: GitalyClient.fast_timeout) - response.flat_map { |msg| msg.deltas } + response.flat_map { |msg| msg.deltas.to_ary } end def tree_entry(ref, path, limit = nil) diff --git a/lib/gitlab/gitaly_client/ref_service.rb b/lib/gitlab/gitaly_client/ref_service.rb index da579276101..98b1d3dceef 100644 --- a/lib/gitlab/gitaly_client/ref_service.rb +++ b/lib/gitlab/gitaly_client/ref_service.rb @@ -235,7 +235,7 @@ module Gitlab end def consume_list_refs_response(response) - response.flat_map(&:references) + response.flat_map { |res| res.references.to_ary } end def sort_local_branches_by_param(sort_by) diff --git a/lib/gitlab/redis/wrapper.rb b/lib/gitlab/redis/wrapper.rb index 0e5389dc995..e5e1e1d4165 100644 --- a/lib/gitlab/redis/wrapper.rb +++ b/lib/gitlab/redis/wrapper.rb @@ -41,21 +41,6 @@ module Gitlab size end - def _raw_config - return @_raw_config if defined?(@_raw_config) - - @_raw_config = - begin - if filename = config_file_name - ERB.new(File.read(filename)).result.freeze - else - false - end - rescue Errno::ENOENT - false - end - end - def config_file_path(filename) path = File.join(rails_root, 'config', filename) return path if File.file?(path) @@ -67,10 +52,6 @@ module Gitlab File.expand_path('../../..', __dir__) end - def config_fallback? - config_file_name == config_fallback&.config_file_name - end - def config_file_name [ # Instance specific config sources: @@ -91,6 +72,10 @@ module Gitlab ].compact.first end + def redis_yml_path + File.join(rails_root, 'config/redis.yml') + end + def store_name name.demodulize end @@ -212,16 +197,20 @@ module Gitlab end def fetch_config - return false unless self.class._raw_config - - yaml = YAML.safe_load(self.class._raw_config, aliases: true) + redis_yml = read_yaml(self.class.redis_yml_path).fetch(@rails_env, {}) + instance_config_yml = read_yaml(self.class.config_file_name)[@rails_env] + + [ + redis_yml[self.class.store_name.underscore], + instance_config_yml, + self.class.config_fallback && redis_yml[self.class.config_fallback.store_name.underscore] + ].compact.first + end - # If the file has content but it's invalid YAML, `load` returns false - if yaml - yaml.fetch(@rails_env, false) - else - false - end + def read_yaml(path) + YAML.safe_load(ERB.new(File.read(path.to_s)).result, aliases: true) || {} + rescue Errno::ENOENT + {} end end end diff --git a/package.json b/package.json index f5c010e0d0b..2f3d73fa8c7 100644 --- a/package.json +++ b/package.json @@ -57,7 +57,7 @@ "@gitlab/favicon-overlay": "2.0.0", "@gitlab/fonts": "^1.0.1", "@gitlab/svgs": "3.14.0", - "@gitlab/ui": "52.6.1", + "@gitlab/ui": "52.7.0", "@gitlab/visual-review-tools": "1.7.3", "@gitlab/web-ide": "0.0.1-dev-20221217175648", "@rails/actioncable": "6.1.4-7", diff --git a/spec/frontend/design_management/components/upload/__snapshots__/design_version_dropdown_spec.js.snap b/spec/frontend/design_management/components/upload/__snapshots__/design_version_dropdown_spec.js.snap index 1acbf14db88..812d5029ef4 100644 --- a/spec/frontend/design_management/components/upload/__snapshots__/design_version_dropdown_spec.js.snap +++ b/spec/frontend/design_management/components/upload/__snapshots__/design_version_dropdown_spec.js.snap @@ -107,10 +107,14 @@ exports[`Design management design version dropdown component renders design vers </span> </span> </gl-listbox-item-stub> + + <!----> </ul> <!----> + <!----> + </gl-base-dropdown-stub> `; @@ -221,9 +225,13 @@ exports[`Design management design version dropdown component renders design vers </span> </span> </gl-listbox-item-stub> + + <!----> </ul> <!----> + <!----> + </gl-base-dropdown-stub> `; diff --git a/spec/frontend/pages/projects/graphs/__snapshots__/code_coverage_spec.js.snap b/spec/frontend/pages/projects/graphs/__snapshots__/code_coverage_spec.js.snap index e7c7ec0d336..aaef5a59a0f 100644 --- a/spec/frontend/pages/projects/graphs/__snapshots__/code_coverage_spec.js.snap +++ b/spec/frontend/pages/projects/graphs/__snapshots__/code_coverage_spec.js.snap @@ -73,10 +73,14 @@ exports[`Code Coverage when fetching data is successful matches the snapshot 1`] karma </gl-listbox-item-stub> + + <!----> </ul> <!----> + <!----> + </gl-base-dropdown-stub> </div> diff --git a/spec/graphql/types/notes/noteable_interface_spec.rb b/spec/graphql/types/notes/noteable_interface_spec.rb index be2c30aac72..e11dece60b8 100644 --- a/spec/graphql/types/notes/noteable_interface_spec.rb +++ b/spec/graphql/types/notes/noteable_interface_spec.rb @@ -7,6 +7,7 @@ RSpec.describe Types::Notes::NoteableInterface do expected_fields = %i[ discussions notes + commenters ] expect(described_class).to have_graphql_fields(*expected_fields) diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb index 82aca13c929..383ed68816e 100644 --- a/spec/models/concerns/noteable_spec.rb +++ b/spec/models/concerns/noteable_spec.rb @@ -63,6 +63,82 @@ RSpec.describe Noteable do end end + # rubocop:disable RSpec/MultipleMemoizedHelpers + describe '#commenters' do + shared_examples 'commenters' do + it 'does not automatically include the noteable author' do + expect(commenters).not_to include(noteable.author) + end + + context 'with no user' do + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter) + end + end + + context 'with non project member' do + let(:current_user) { create(:user) } + + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter) + end + + it 'does not include a commenter from another noteable' do + expect(commenters).not_to include(other_noteable_commenter) + end + end + end + + let_it_be(:commenter) { create(:user) } + let_it_be(:another_commenter) { create(:user) } + let_it_be(:internal_commenter) { create(:user) } + let_it_be(:other_noteable_commenter) { create(:user) } + + let(:current_user) {} + let(:commenters) { noteable.commenters(user: current_user) } + + let!(:comments) { create_list(:note, 2, author: commenter, noteable: noteable, project: noteable.project) } + let!(:more_comments) { create_list(:note, 2, author: another_commenter, noteable: noteable, project: noteable.project) } + + context 'when noteable is an issue' do + let(:noteable) { create(:issue) } + + let!(:internal_comments) { create_list(:note, 2, author: internal_commenter, noteable: noteable, project: noteable.project, internal: true) } + let!(:other_noteable_comments) { create_list(:note, 2, author: other_noteable_commenter, noteable: create(:issue, project: noteable.project), project: noteable.project) } + + it_behaves_like 'commenters' + + context 'with reporter' do + let(:current_user) { create(:user) } + + before do + noteable.project.add_reporter(current_user) + end + + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter, internal_commenter) + end + + context 'with noteable author' do + let(:current_user) { noteable.author } + + it 'contains a distinct list of non-internal note authors' do + expect(commenters).to contain_exactly(commenter, another_commenter, internal_commenter) + end + end + end + end + + context 'when noteable is a merge request' do + let(:noteable) { create(:merge_request) } + + let!(:other_noteable_comments) { create_list(:note, 2, author: other_noteable_commenter, noteable: create(:merge_request, source_project: noteable.project, source_branch: 'feat123'), project: noteable.project) } + + it_behaves_like 'commenters' + end + end + # rubocop:enable RSpec/MultipleMemoizedHelpers + describe '#discussion_ids_relation' do it 'returns ordered discussion_ids' do discussion_ids = subject.discussion_ids_relation.pluck(:discussion_id) diff --git a/spec/support/redis/redis_new_instance_shared_examples.rb b/spec/support/redis/redis_new_instance_shared_examples.rb index 943fe0f11ba..7f83d91bd3a 100644 --- a/spec/support/redis/redis_new_instance_shared_examples.rb +++ b/spec/support/redis/redis_new_instance_shared_examples.rb @@ -3,9 +3,12 @@ require 'spec_helper' RSpec.shared_examples "redis_new_instance_shared_examples" do |name, fallback_class| + include TmpdirHelper + let(:instance_specific_config_file) { "config/redis.#{name}.yml" } let(:environment_config_file_name) { "GITLAB_REDIS_#{name.upcase}_CONFIG_FILE" } let(:fallback_config_file) { nil } + let(:rails_root) { mktmpdir } before do redis_clear_raw_config!(fallback_class) @@ -22,8 +25,6 @@ RSpec.shared_examples "redis_new_instance_shared_examples" do |name, fallback_cl describe '.config_file_name' do subject { described_class.config_file_name } - let(:rails_root) { Dir.mktmpdir('redis_shared_examples') } - before do # Undo top-level stub of config_file_name because we are testing that method now. allow(described_class).to receive(:config_file_name).and_call_original @@ -32,10 +33,6 @@ RSpec.shared_examples "redis_new_instance_shared_examples" do |name, fallback_cl FileUtils.mkdir_p(File.join(rails_root, 'config')) end - after do - FileUtils.rm_rf(rails_root) - end - context 'when there is only a resque.yml' do before do FileUtils.touch(File.join(rails_root, 'config/resque.yml')) @@ -58,4 +55,49 @@ RSpec.shared_examples "redis_new_instance_shared_examples" do |name, fallback_cl end end end + + describe '#fetch_config' do + context 'when redis.yml exists' do + subject { described_class.new('test').send(:fetch_config) } + + before do + allow(described_class).to receive(:config_file_name).and_call_original + allow(described_class).to receive(:redis_yml_path).and_call_original + allow(described_class).to receive(:rails_root).and_return(rails_root) + FileUtils.mkdir_p(File.join(rails_root, 'config')) + end + + context 'when the fallback has a redis.yml entry' do + before do + File.write(File.join(rails_root, 'config/redis.yml'), { + 'test' => { + described_class.config_fallback.store_name.underscore => { 'fallback redis.yml' => 123 } + } + }.to_json) + end + + it { expect(subject).to eq({ 'fallback redis.yml' => 123 }) } + + context 'and an instance config file exists' do + before do + File.write(File.join(rails_root, instance_specific_config_file), { + 'test' => { 'instance specific file' => 456 } + }.to_json) + end + + it { expect(subject).to eq({ 'instance specific file' => 456 }) } + + context 'and the instance has a redis.yml entry' do + before do + File.write(File.join(rails_root, 'config/redis.yml'), { + 'test' => { name => { 'instance redis.yml' => 789 } } + }.to_json) + end + + it { expect(subject).to eq({ 'instance redis.yml' => 789 }) } + end + end + end + end + end end diff --git a/spec/support/redis/redis_shared_examples.rb b/spec/support/redis/redis_shared_examples.rb index 0368fd63357..2d5d8a185b2 100644 --- a/spec/support/redis/redis_shared_examples.rb +++ b/spec/support/redis/redis_shared_examples.rb @@ -2,6 +2,7 @@ RSpec.shared_examples "redis_shared_examples" do include StubENV + include TmpdirHelper let(:test_redis_url) { "redis://redishost:#{redis_port}" } let(:test_cluster_config) { { cluster: [{ host: "redis://redishost", port: redis_port }] } } @@ -18,10 +19,11 @@ RSpec.shared_examples "redis_shared_examples" do let(:sentinel_port) { 26379 } let(:config_with_environment_variable_inside) { "spec/fixtures/config/redis_config_with_env.yml" } let(:config_env_variable_url) { "TEST_GITLAB_REDIS_URL" } - let(:rails_root) { Dir.mktmpdir('redis_shared_examples') } + let(:rails_root) { mktmpdir } before do allow(described_class).to receive(:config_file_name).and_return(Rails.root.join(config_file_name).to_s) + allow(described_class).to receive(:redis_yml_path).and_return('/dev/null') redis_clear_raw_config!(described_class) end @@ -40,10 +42,6 @@ RSpec.shared_examples "redis_shared_examples" do FileUtils.mkdir_p(File.join(rails_root, 'config')) end - after do - FileUtils.rm_rf(rails_root) - end - context 'when there is no config file anywhere' do it { expect(subject).to be_nil } @@ -250,26 +248,6 @@ RSpec.shared_examples "redis_shared_examples" do end end - describe '._raw_config' do - subject { described_class._raw_config } - - let(:config_file_name) { '/var/empty/doesnotexist' } - - it 'is frozen' do - expect(subject).to be_frozen - end - - it 'returns false when the file does not exist' do - expect(subject).to eq(false) - end - - it "returns false when the filename can't be determined" do - expect(described_class).to receive(:config_file_name).and_return(nil) - - expect(subject).to eq(false) - end - end - describe '.with' do let(:config_file_name) { config_old_format_socket } @@ -313,10 +291,6 @@ RSpec.shared_examples "redis_shared_examples" do allow(described_class).to receive(:rails_root).and_return(rails_root) end - after do - FileUtils.rm_rf(rails_root) - end - it 'can run an empty block' do expect { described_class.with { nil } }.not_to raise_error end @@ -408,9 +382,7 @@ RSpec.shared_examples "redis_shared_examples" do context 'when sentinels are not defined' do let(:config_file_name) { config_old_format_host } - it 'returns false' do - is_expected.to be_falsey - end + it { expect(subject).to eq(nil) } end context 'when cluster is defined' do @@ -435,22 +407,39 @@ RSpec.shared_examples "redis_shared_examples" do end describe '#fetch_config' do - it 'returns false when no config file is present' do - allow(described_class).to receive(:_raw_config) { false } + it 'raises an exception when the config file contains invalid yaml' do + Tempfile.open('bad.yml') do |file| + file.write('{"not":"yaml"') + file.flush + allow(described_class).to receive(:config_file_name) { file.path } - expect(subject.send(:fetch_config)).to eq false + expect { subject.send(:fetch_config) }.to raise_error(Psych::SyntaxError) + end end - it 'returns false when config file is present but has invalid YAML' do - allow(described_class).to receive(:_raw_config) { "# development: true" } + it 'has a value for the legacy default URL' do + allow(subject).to receive(:fetch_config) { nil } - expect(subject.send(:fetch_config)).to eq false + expect(subject.send(:raw_config_hash)).to include(url: a_string_matching(%r{\Aredis://localhost:638[012]\Z})) end - it 'has a value for the legacy default URL' do - allow(subject).to receive(:fetch_config) { false } + context 'when redis.yml exists' do + subject { described_class.new('test').send(:fetch_config) } - expect(subject.send(:raw_config_hash)).to include(url: a_string_matching(%r{\Aredis://localhost:638[012]\Z})) + before do + allow(described_class).to receive(:config_file_name).and_call_original + allow(described_class).to receive(:redis_yml_path).and_call_original + allow(described_class).to receive(:rails_root).and_return(rails_root) + FileUtils.mkdir_p(File.join(rails_root, 'config')) + end + + it 'uses config/redis.yml' do + File.write(File.join(rails_root, 'config/redis.yml'), { + 'test' => { described_class.store_name.underscore => { 'foobar' => 123 } } + }.to_json) + + expect(subject).to eq({ 'foobar' => 123 }) + end end end diff --git a/spec/support/tmpdir.rb b/spec/support/tmpdir.rb new file mode 100644 index 00000000000..ea8e26d2878 --- /dev/null +++ b/spec/support/tmpdir.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module TmpdirHelper + def mktmpdir + @tmpdir_helper_dirs ||= [] + @tmpdir_helper_dirs << Dir.mktmpdir + @tmpdir_helper_dirs.last + end + + def self.included(base) + base.after do + if @tmpdir_helper_dirs + FileUtils.rm_rf(@tmpdir_helper_dirs) + @tmpdir_helper_dirs = nil + end + end + end +end diff --git a/yarn.lock b/yarn.lock index 0184cfbdfc8..88ecf2ede7e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1136,10 +1136,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-3.14.0.tgz#b32a673f08bbd5ba6d406bcf3abb6e7276271b6c" integrity sha512-mQYtW9eGHY7cF6elsWd76hUF7F3NznyzrJJy5eXBHjvRdYBtyHmwkVmh1Cwr3S/2Sl8fPC+qk41a+Nm6n+1mRQ== -"@gitlab/ui@52.6.1": - version "52.6.1" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-52.6.1.tgz#542c32802c63d071a7bcfa737d984fa66c53ea47" - integrity sha512-k0R7wLHiI3UoEEMpGK/CFhZNBwsvyxk6OiALYa0yZ5PAiYokEDQE96G3VshG3qc9wTzfIw6KSh6I20SU60tVsQ== +"@gitlab/ui@52.7.0": + version "52.7.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-52.7.0.tgz#431502f0b23324d57fa2f683925f17eb4c550f99" + integrity sha512-ttTCUt/amTMG9YhHJypR3FIFVSBjYDGh56izAhjMts7r216saaD8hQ8m4Yzts+fqmR42bQzUdjdThwj3Dthtsw== dependencies: "@popperjs/core" "^2.11.2" bootstrap-vue "2.20.1" |