diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-07 18:07:51 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-07 18:07:51 +0000 |
commit | 6cae2159b8ce1e84fad48f3dbd5368995cbd87b1 (patch) | |
tree | 46d6b7cbd78d72fe5dcb5d6b1ed973b30deadc71 | |
parent | 84f9f0cb8137637708a41152347e7754c1e9fb83 (diff) | |
download | gitlab-ce-6cae2159b8ce1e84fad48f3dbd5368995cbd87b1.tar.gz |
Add latest changes from gitlab-org/gitlab@master
70 files changed, 1698 insertions, 580 deletions
diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index c38fc19d762..3386eb650f3 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -7019,7 +7019,6 @@ RSpec/MissingFeatureCategory: - 'spec/rubocop/cop/safe_params_spec.rb' - 'spec/rubocop/cop/scalability/bulk_perform_with_context_spec.rb' - 'spec/rubocop/cop/scalability/cron_worker_context_spec.rb' - - 'spec/rubocop/cop/scalability/file_uploads_spec.rb' - 'spec/rubocop/cop/scalability/idempotent_worker_spec.rb' - 'spec/rubocop/cop/sidekiq_api_usage_spec.rb' - 'spec/rubocop/cop/sidekiq_load_balancing/worker_data_consistency_spec.rb' diff --git a/.rubocop_todo/style/arguments_forwarding.yml b/.rubocop_todo/style/arguments_forwarding.yml deleted file mode 100644 index 5a2467f2e98..00000000000 --- a/.rubocop_todo/style/arguments_forwarding.yml +++ /dev/null @@ -1,15 +0,0 @@ ---- -# Cop supports --autocorrect. -Style/ArgumentsForwarding: - Details: grace period - Exclude: - - 'ee/app/models/concerns/geo/repository_replicator_strategy.rb' - - 'ee/app/serializers/security/vulnerability_report_data_entity.rb' - - 'ee/app/services/status_page/publish_base_service.rb' - - 'ee/app/workers/ee/project_cache_worker.rb' - - 'ee/lib/audit/details.rb' - - 'ee/lib/ee/api/geo.rb' - - 'ee/lib/ee/gitlab/auth/ldap/adapter.rb' - - 'ee/lib/ee/gitlab/background_migration/migrate_approver_to_approval_rules.rb' - - 'ee/spec/features/projects/mirror_spec.rb' - - 'ee/spec/lib/gitlab/status_page/storage/s3_multipart_upload_spec.rb' diff --git a/.rubocop_todo/style/empty_method.yml b/.rubocop_todo/style/empty_method.yml index 81b9544f574..f4d1d4e2cc4 100644 --- a/.rubocop_todo/style/empty_method.yml +++ b/.rubocop_todo/style/empty_method.yml @@ -106,7 +106,6 @@ Style/EmptyMethod: - 'ee/app/controllers/projects/security/sast_configuration_controller.rb' - 'ee/app/controllers/projects/settings/slacks_controller.rb' - 'ee/app/controllers/registrations/company_controller.rb' - - 'ee/app/controllers/registrations/verification_controller.rb' - 'ee/app/controllers/subscriptions/groups_controller.rb' - 'ee/app/controllers/trials_controller.rb' - 'ee/app/controllers/users/identity_verification_controller.rb' diff --git a/.rubocop_todo/style/mutable_constant.yml b/.rubocop_todo/style/mutable_constant.yml index 960a8ee6082..2409d4f3521 100644 --- a/.rubocop_todo/style/mutable_constant.yml +++ b/.rubocop_todo/style/mutable_constant.yml @@ -52,6 +52,7 @@ Style/MutableConstant: - 'rubocop/cop/scalability/idempotent_worker.rb' - 'rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb' - 'scripts/lib/glfm/constants.rb' + - 'scripts/lint-docs-blueprints.rb' - 'scripts/perf/gc/collect_gc_stats.rb' - 'spec/support/helpers/jira_integration_helpers.rb' - 'tooling/danger/stable_branch.rb' diff --git a/.rubocop_todo/style/redundant_freeze.yml b/.rubocop_todo/style/redundant_freeze.yml index ef3ce1cda95..f38a33042e5 100644 --- a/.rubocop_todo/style/redundant_freeze.yml +++ b/.rubocop_todo/style/redundant_freeze.yml @@ -227,8 +227,8 @@ Style/RedundantFreeze: - 'rubocop/cop/inject_enterprise_edition_module.rb' - 'rubocop/cop/project_path_helper.rb' - 'rubocop/cop/qa/selector_usage.rb' + - 'scripts/lint-docs-blueprints.rb' - 'scripts/qa/testcases-check' - - 'scripts/review_apps/automated_cleanup.rb' - 'scripts/validate_migration_timestamps' - 'spec/contracts/provider/helpers/contract_source_helper.rb' - 'spec/initializers/secret_token_spec.rb' @@ -240,4 +240,5 @@ Style/RedundantFreeze: - 'tooling/danger/datateam.rb' - 'tooling/danger/specs.rb' - 'tooling/danger/stable_branch.rb' + - 'tooling/lib/tooling/kubernetes_client.rb' - 'tooling/lib/tooling/mappings/view_to_js_mappings.rb' diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 4c65c966d10..c3ee1a551a8 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -fe20fe12a1c82a668585e0181593f896b187ea36 +10880feb242899156d4a1d5224cb445f1ff6680c diff --git a/GITLAB_ELASTICSEARCH_INDEXER_VERSION b/GITLAB_ELASTICSEARCH_INDEXER_VERSION index fcdb2e109f6..ee74734aa22 100644 --- a/GITLAB_ELASTICSEARCH_INDEXER_VERSION +++ b/GITLAB_ELASTICSEARCH_INDEXER_VERSION @@ -1 +1 @@ -4.0.0 +4.1.0 diff --git a/app/assets/javascripts/invite_members/components/project_select.vue b/app/assets/javascripts/invite_members/components/project_select.vue index b7a3918813b..c1114c240b9 100644 --- a/app/assets/javascripts/invite_members/components/project_select.vue +++ b/app/assets/javascripts/invite_members/components/project_select.vue @@ -1,28 +1,23 @@ <script> -import { - GlAvatarLabeled, - GlDropdown, - GlDropdownItem, - GlDropdownText, - GlSearchBoxByType, -} from '@gitlab/ui'; +import { GlAvatarLabeled, GlCollapsibleListbox } from '@gitlab/ui'; import { debounce } from 'lodash'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { s__ } from '~/locale'; import { getProjects } from '~/rest_api'; import { SEARCH_DELAY, GROUP_FILTERS } from '../constants'; +// We can have GlCollapsibleListbox dropdown panel with full +// width once we implement +// https://gitlab.com/gitlab-org/gitlab-ui/-/issues/2133 +// https://gitlab.com/gitlab-org/gitlab/-/issues/390411 export default { name: 'ProjectSelect', components: { GlAvatarLabeled, - GlDropdown, - GlDropdownItem, - GlDropdownText, - GlSearchBoxByType, + GlCollapsibleListbox, }, model: { - prop: 'selectedProject', + prop: 'selectedProjectId', }, props: { groupsFilter: { @@ -41,18 +36,21 @@ export default { return { isFetching: false, projects: [], - selectedProject: {}, + selectedProjectId: '', searchTerm: '', errorMessage: '', }; }, computed: { selectedProjectName() { - return this.selectedProject.name || this.$options.i18n.dropdownText; + return this.selectedProject.nameWithNamespace || this.$options.i18n.dropdownText; }, isFetchResultEmpty() { return this.projects.length === 0 && !this.isFetching; }, + selectedProject() { + return this.projects.find((prj) => prj.id === this.selectedProjectId) || {}; + }, }, watch: { searchTerm() { @@ -70,10 +68,14 @@ export default { .then((response) => { this.projects = response.data.map((project) => ({ ...convertObjectPropsToCamelCase(project), - name: project.name_with_namespace, + text: project.name_with_namespace, + value: project.id, })); }) .catch(() => { + // To be displayed in GlCollapsibleListbox once we implement + // https://gitlab.com/gitlab-org/gitlab-ui/-/issues/2132 + // https://gitlab.com/gitlab-org/gitlab/-/issues/389974 this.errorMessage = this.$options.i18n.errorFetchingProjects; }) .finally(() => { @@ -83,9 +85,7 @@ export default { fetchProjects() { return getProjects(this.searchTerm, this.$options.defaultFetchOptions); }, - selectProject(project) { - this.selectedProject = project; - + selectProject() { this.$emit('input', this.selectedProject); }, }, @@ -104,40 +104,28 @@ export default { }; </script> <template> - <div> - <gl-dropdown - data-testid="project-select-dropdown" - :text="selectedProjectName" - toggle-class="gl-mb-2" - block - menu-class="gl-w-full!" - > - <gl-search-box-by-type - v-model="searchTerm" - :is-loading="isFetching" - :placeholder="$options.i18n.searchPlaceholder" - data-qa-selector="project_select_dropdown_search_field" + <gl-collapsible-listbox + v-model="selectedProjectId" + searchable + :items="projects" + :searching="isFetching" + :toggle-text="selectedProjectName" + :search-placeholder="$options.i18n.searchPlaceholder" + :no-results-text="$options.i18n.emptySearchResult" + data-testid="project-select-dropdown" + data-qa-selector="project_select_dropdown" + class="gl-collapsible-listbox-w-full" + @search="searchTerm = $event" + @select="selectProject" + > + <template #list-item="{ item }"> + <gl-avatar-labeled + :label="item.text" + :src="item.avatarUrl" + :entity-id="item.id" + :entity-name="item.name" + :size="32" /> - <gl-dropdown-item - v-for="project in projects" - :key="project.id" - :name="project.name" - @click="selectProject(project)" - > - <gl-avatar-labeled - :label="project.name" - :src="project.avatarUrl" - :entity-id="project.id" - :entity-name="project.name" - :size="32" - /> - </gl-dropdown-item> - <gl-dropdown-text v-if="errorMessage" data-testid="error-message"> - <span class="gl-text-gray-500">{{ errorMessage }}</span> - </gl-dropdown-text> - <gl-dropdown-text v-else-if="isFetchResultEmpty" data-testid="empty-result-message"> - <span class="gl-text-gray-500">{{ $options.i18n.emptySearchResult }}</span> - </gl-dropdown-text> - </gl-dropdown> - </div> + </template> + </gl-collapsible-listbox> </template> diff --git a/app/assets/javascripts/work_items/graphql/work_item_discussion_note.fragment.graphql b/app/assets/javascripts/work_items/graphql/work_item_discussion_note.fragment.graphql new file mode 100644 index 00000000000..e5c45ff9751 --- /dev/null +++ b/app/assets/javascripts/work_items/graphql/work_item_discussion_note.fragment.graphql @@ -0,0 +1,25 @@ +#import "~/graphql_shared/fragments/user.fragment.graphql" +#import "~/work_items/graphql/work_item_note.fragment.graphql" + +fragment WorkItemDiscussionNote on Note { + id + bodyHtml + system + internal + systemNoteIconName + createdAt + author { + ...User + } + userPermissions { + adminNote + } + discussion { + id + notes { + nodes { + ...WorkItemNote + } + } + } +} diff --git a/app/assets/javascripts/work_items/graphql/work_item_note_created.subscription.graphql b/app/assets/javascripts/work_items/graphql/work_item_note_created.subscription.graphql new file mode 100644 index 00000000000..5591f11823c --- /dev/null +++ b/app/assets/javascripts/work_items/graphql/work_item_note_created.subscription.graphql @@ -0,0 +1,7 @@ +#import "~/work_items/graphql/work_item_discussion_note.fragment.graphql" + +subscription workItemNoteCreated($noteableId: NoteableID) { + workItemNoteCreated(noteableId: $noteableId) { + ...WorkItemDiscussionNote + } +} diff --git a/app/assets/javascripts/work_items/graphql/work_item_note_deleted.subscription.graphql b/app/assets/javascripts/work_items/graphql/work_item_note_deleted.subscription.graphql new file mode 100644 index 00000000000..6a59becdb99 --- /dev/null +++ b/app/assets/javascripts/work_items/graphql/work_item_note_deleted.subscription.graphql @@ -0,0 +1,7 @@ +subscription workItemNoteDeleted($noteableId: NoteableID) { + workItemNoteDeleted(noteableId: $noteableId) { + id + discussionId + lastDiscussionNote + } +} diff --git a/app/assets/javascripts/work_items/graphql/work_item_note_updated.subscription.graphql b/app/assets/javascripts/work_items/graphql/work_item_note_updated.subscription.graphql new file mode 100644 index 00000000000..4a04b311dde --- /dev/null +++ b/app/assets/javascripts/work_items/graphql/work_item_note_updated.subscription.graphql @@ -0,0 +1,7 @@ +#import "~/work_items/graphql/work_item_note.fragment.graphql" + +subscription workItemNoteUpdated($noteableId: NoteableID) { + workItemNoteUpdated(noteableId: $noteableId) { + ...WorkItemNote + } +} diff --git a/app/controllers/concerns/membership_actions.rb b/app/controllers/concerns/membership_actions.rb index 7c6e449b509..773e4c15d6e 100644 --- a/app/controllers/concerns/membership_actions.rb +++ b/app/controllers/concerns/membership_actions.rb @@ -6,7 +6,7 @@ module MembershipActions def update update_params = params.require(root_params_key).permit(:access_level, :expires_at) - member = membershipable.members_and_requesters.find(params[:id]) + member = members_and_requesters.find(params[:id]) result = Members::UpdateService .new(current_user, update_params) .execute(member) @@ -30,7 +30,7 @@ module MembershipActions end def destroy - member = membershipable.members_and_requesters.find(params[:id]) + member = members_and_requesters.find(params[:id]) skip_subresources = !ActiveRecord::Type::Boolean.new.cast(params.delete(:remove_sub_memberships)) # !! is used in case unassign_issuables contains empty string which would result in nil unassign_issuables = !!ActiveRecord::Type::Boolean.new.cast(params.delete(:unassign_issuables)) @@ -71,7 +71,7 @@ module MembershipActions end def approve_access_request - access_requester = membershipable.requesters.find(params[:id]) + access_requester = requesters.find(params[:id]) Members::ApproveAccessRequestService .new(current_user, params) .execute(access_requester) @@ -81,7 +81,7 @@ module MembershipActions # rubocop: disable CodeReuse/ActiveRecord def leave - member = membershipable.members_and_requesters.find_by!(user_id: current_user.id) + member = members_and_requesters.find_by!(user_id: current_user.id) Members::DestroyService.new(current_user).execute(member) notice = @@ -140,6 +140,14 @@ module MembershipActions raise NotImplementedError end + def members_and_requesters + membershipable.members_and_requesters + end + + def requesters + membershipable.requesters + end + def requested_relations(inherited_permissions = :with_inherited_permissions) case params[inherited_permissions].presence when 'exclude' diff --git a/app/controllers/import/github_controller.rb b/app/controllers/import/github_controller.rb index 9a8f6a74653..76772a72865 100644 --- a/app/controllers/import/github_controller.rb +++ b/app/controllers/import/github_controller.rb @@ -79,13 +79,7 @@ class Import::GithubController < Import::BaseController def realtime_changes Gitlab::PollingInterval.set_header(response, interval: 3_000) - render json: already_added_projects.map { |project| - { - id: project.id, - import_status: project.import_status, - stats: ::Gitlab::GithubImport::ObjectCounter.summary(project) - } - } + render json: Import::GithubRealtimeRepoSerializer.new.represent(already_added_projects) end def cancel diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb index cd9c6efb106..543ffa637e1 100644 --- a/app/controllers/projects/project_members_controller.rb +++ b/app/controllers/projects/project_members_controller.rb @@ -47,7 +47,7 @@ class Projects::ProjectMembersController < Projects::ApplicationController end def membershipable_members - project.members + query_members_via_project_namespace_enabled? ? project.namespace_members : project.members end def plain_source_type @@ -65,6 +65,18 @@ class Projects::ProjectMembersController < Projects::ApplicationController def root_params_key :project_member end + + def members_and_requesters + query_members_via_project_namespace_enabled? ? project.namespace_members_and_requesters : super + end + + def requesters + query_members_via_project_namespace_enabled? ? project.namespace_requesters : super + end + + def query_members_via_project_namespace_enabled? + Feature.enabled?(:project_members_index_by_project_namespace, project) + end end Projects::ProjectMembersController.prepend_mod_with('Projects::ProjectMembersController') diff --git a/app/graphql/graphql_triggers.rb b/app/graphql/graphql_triggers.rb index 7f83b62a2ff..89656f1e018 100644 --- a/app/graphql/graphql_triggers.rb +++ b/app/graphql/graphql_triggers.rb @@ -29,27 +29,33 @@ module GraphqlTriggers GitlabSchema.subscriptions.trigger('issuableMilestoneUpdated', { issuable_id: issuable.to_gid }, issuable) end + def self.work_item_note_created(work_item_gid, note_data) + GitlabSchema.subscriptions.trigger('workItemNoteCreated', { noteable_id: work_item_gid }, note_data) + end + + def self.work_item_note_deleted(work_item_gid, note_data) + GitlabSchema.subscriptions.trigger('workItemNoteDeleted', { noteable_id: work_item_gid }, note_data) + end + + def self.work_item_note_updated(work_item_gid, note_data) + GitlabSchema.subscriptions.trigger('workItemNoteUpdated', { noteable_id: work_item_gid }, note_data) + end + def self.merge_request_reviewers_updated(merge_request) GitlabSchema.subscriptions.trigger( - 'mergeRequestReviewersUpdated', - { issuable_id: merge_request.to_gid }, - merge_request + 'mergeRequestReviewersUpdated', { issuable_id: merge_request.to_gid }, merge_request ) end def self.merge_request_merge_status_updated(merge_request) GitlabSchema.subscriptions.trigger( - 'mergeRequestMergeStatusUpdated', - { issuable_id: merge_request.to_gid }, - merge_request + 'mergeRequestMergeStatusUpdated', { issuable_id: merge_request.to_gid }, merge_request ) end def self.merge_request_approval_state_updated(merge_request) GitlabSchema.subscriptions.trigger( - 'mergeRequestApprovalStateUpdated', - { issuable_id: merge_request.to_gid }, - merge_request + 'mergeRequestApprovalStateUpdated', { issuable_id: merge_request.to_gid }, merge_request ) end end diff --git a/app/graphql/subscriptions/notes/base.rb b/app/graphql/subscriptions/notes/base.rb new file mode 100644 index 00000000000..3653c01e0e2 --- /dev/null +++ b/app/graphql/subscriptions/notes/base.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Subscriptions + module Notes + class Base < ::Subscriptions::BaseSubscription + include Gitlab::Graphql::Laziness + + argument :noteable_id, ::Types::GlobalIDType[::Noteable], + required: false, + description: 'ID of the noteable.' + + def subscribe(*args) + nil + end + + def authorized?(noteable_id:) + noteable = force(GitlabSchema.find_by_gid(noteable_id)) + + # unsubscribe if user cannot read the noteable anymore for any reason, e.g. issue was set confidential, + # in the meantime the read note permissions is checked within its corresponding returned type, i.e. NoteType + unauthorized! unless noteable && Ability.allowed?(current_user, :"read_#{noteable.to_ability_name}", noteable) + + true + end + end + end +end diff --git a/app/graphql/subscriptions/notes/created.rb b/app/graphql/subscriptions/notes/created.rb new file mode 100644 index 00000000000..873280286f7 --- /dev/null +++ b/app/graphql/subscriptions/notes/created.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Subscriptions + module Notes + class Created < Base + payload_type ::Types::Notes::NoteType + end + end +end diff --git a/app/graphql/subscriptions/notes/deleted.rb b/app/graphql/subscriptions/notes/deleted.rb new file mode 100644 index 00000000000..d931ef00d0d --- /dev/null +++ b/app/graphql/subscriptions/notes/deleted.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Subscriptions + module Notes + class Deleted < Base + payload_type ::Types::Notes::DeletedNoteType + + DeletedNote = Struct.new(:model_id, :model_name, :discussion_model_id, :last_discussion_note) do + def to_global_id + ::Gitlab::GlobalId.as_global_id(model_id, model_name: model_name) + end + + def discussion_id + ::Gitlab::GlobalId.as_global_id(discussion_model_id, model_name: Discussion.name) + end + end + + def update(*args) + DeletedNote.new(object[:id], object[:model_name], object[:discussion_id], object[:last_discussion_note]) + end + end + end +end diff --git a/app/graphql/subscriptions/notes/updated.rb b/app/graphql/subscriptions/notes/updated.rb new file mode 100644 index 00000000000..a4748a3361e --- /dev/null +++ b/app/graphql/subscriptions/notes/updated.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Subscriptions + module Notes + class Updated < Base + payload_type Types::Notes::NoteType + end + end +end diff --git a/app/graphql/types/notes/deleted_note_type.rb b/app/graphql/types/notes/deleted_note_type.rb new file mode 100644 index 00000000000..f799fc01f6e --- /dev/null +++ b/app/graphql/types/notes/deleted_note_type.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module Types + module Notes + # rubocop: disable Graphql/AuthorizeTypes + class DeletedNoteType < BaseObject + graphql_name 'DeletedNote' + + field :id, ::Types::GlobalIDType[::Note], + null: false, + description: 'ID of the deleted note.' + + field :discussion_id, ::Types::GlobalIDType[::Discussion], + null: true, + description: 'ID of the discussion for the deleted note.' + + field :last_discussion_note, GraphQL::Types::Boolean, + null: true, + description: 'Whether deleted note is the last note in the discussion.' + end + # rubocop: enable Graphql/AuthorizeTypes + end +end diff --git a/app/graphql/types/subscription_type.rb b/app/graphql/types/subscription_type.rb index f7f26ba4c5a..33fc0cbe20e 100644 --- a/app/graphql/types/subscription_type.rb +++ b/app/graphql/types/subscription_type.rb @@ -4,41 +4,60 @@ module Types class SubscriptionType < ::Types::BaseObject graphql_name 'Subscription' - field :issuable_assignees_updated, subscription: Subscriptions::IssuableUpdated, null: true, - description: 'Triggered when the assignees of an issuable are updated.' + field :issuable_assignees_updated, + subscription: Subscriptions::IssuableUpdated, null: true, + description: 'Triggered when the assignees of an issuable are updated.' - field :issue_crm_contacts_updated, subscription: Subscriptions::IssuableUpdated, null: true, - description: 'Triggered when the crm contacts of an issuable are updated.' + field :issue_crm_contacts_updated, + subscription: Subscriptions::IssuableUpdated, null: true, + description: 'Triggered when the crm contacts of an issuable are updated.' - field :issuable_title_updated, subscription: Subscriptions::IssuableUpdated, null: true, - description: 'Triggered when the title of an issuable is updated.' + field :issuable_title_updated, + subscription: Subscriptions::IssuableUpdated, null: true, + description: 'Triggered when the title of an issuable is updated.' - field :issuable_description_updated, subscription: Subscriptions::IssuableUpdated, null: true, - description: 'Triggered when the description of an issuable is updated.' + field :issuable_description_updated, + subscription: Subscriptions::IssuableUpdated, null: true, + description: 'Triggered when the description of an issuable is updated.' - field :issuable_labels_updated, subscription: Subscriptions::IssuableUpdated, null: true, - description: 'Triggered when the labels of an issuable are updated.' + field :issuable_labels_updated, + subscription: Subscriptions::IssuableUpdated, null: true, + description: 'Triggered when the labels of an issuable are updated.' - field :issuable_dates_updated, subscription: Subscriptions::IssuableUpdated, null: true, - description: 'Triggered when the due date or start date of an issuable is updated.' + field :issuable_dates_updated, + subscription: Subscriptions::IssuableUpdated, null: true, + description: 'Triggered when the due date or start date of an issuable is updated.' - field :issuable_milestone_updated, subscription: Subscriptions::IssuableUpdated, null: true, - description: 'Triggered when the milestone of an issuable is updated.' + field :issuable_milestone_updated, + subscription: Subscriptions::IssuableUpdated, null: true, + description: 'Triggered when the milestone of an issuable is updated.' + + field :work_item_note_created, + subscription: ::Subscriptions::Notes::Created, null: true, + description: 'Triggered when a note is created.', + alpha: { milestone: '15.9' } + + field :work_item_note_deleted, + subscription: ::Subscriptions::Notes::Deleted, null: true, + description: 'Triggered when a note is deleted.', + alpha: { milestone: '15.9' } + + field :work_item_note_updated, + subscription: ::Subscriptions::Notes::Updated, null: true, + description: 'Triggered when a note is updated.', + alpha: { milestone: '15.9' } field :merge_request_reviewers_updated, - subscription: Subscriptions::IssuableUpdated, - null: true, - description: 'Triggered when the reviewers of a merge request are updated.' + subscription: Subscriptions::IssuableUpdated, null: true, + description: 'Triggered when the reviewers of a merge request are updated.' field :merge_request_merge_status_updated, - subscription: Subscriptions::IssuableUpdated, - null: true, - description: 'Triggered when the merge status of a merge request is updated.' + subscription: Subscriptions::IssuableUpdated, null: true, + description: 'Triggered when the merge status of a merge request is updated.' field :merge_request_approval_state_updated, - subscription: Subscriptions::IssuableUpdated, - null: true, - description: 'Triggered when approval state of a merge request is updated.' + subscription: Subscriptions::IssuableUpdated, null: true, + description: 'Triggered when approval state of a merge request is updated.' end end diff --git a/app/models/discussion.rb b/app/models/discussion.rb index 0a968be747a..83c85f30178 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -10,7 +10,8 @@ class Discussion # Bump this if we need to refresh the cached versions of discussions CACHE_VERSION = 1 - attr_reader :notes, :context_noteable + attr_reader :context_noteable + attr_accessor :notes delegate :created_at, :project, diff --git a/app/models/group.rb b/app/models/group.rb index 67800306606..3455b4d8507 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -44,7 +44,10 @@ class Group < Namespace has_many :requesters, -> { where.not(requested_at: nil) }, dependent: :destroy, as: :source, class_name: 'GroupMember' # rubocop:disable Cop/ActiveRecordDependent has_many :namespace_requesters, -> { where.not(requested_at: nil).unscope(where: %i[source_id source_type]) }, foreign_key: :member_namespace_id, inverse_of: :group, class_name: 'GroupMember' + has_many :members_and_requesters, as: :source, class_name: 'GroupMember' + has_many :namespace_members_and_requesters, -> { unscope(where: %i[source_id source_type]) }, + foreign_key: :member_namespace_id, inverse_of: :group, class_name: 'GroupMember' has_many :milestones has_many :integrations diff --git a/app/models/issue.rb b/app/models/issue.rb index b39e37f611b..6e0d228ab8c 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -678,6 +678,12 @@ class Issue < ApplicationRecord true end + # we want to have subscriptions working on work items only, legacy issues do not support graphql subscriptions, yet so + # we need sometimes GID of an issue instance to be represented as WorkItem GID. E.g. notes subscriptions. + def to_work_item_global_id + ::Gitlab::GlobalId.as_global_id(id, model_name: WorkItem.name) + end + private def due_date_after_start_date diff --git a/app/models/note.rb b/app/models/note.rb index 57a1ae1ac45..1bc664936ab 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -182,6 +182,39 @@ class Note < ApplicationRecord after_commit :notify_after_create, on: :create after_commit :notify_after_destroy, on: :destroy + after_commit :trigger_note_subscription_create, on: :create + after_commit :trigger_note_subscription_update, on: :update + after_commit :trigger_note_subscription_destroy, on: :destroy + + def trigger_note_subscription_create + return unless trigger_note_subscription? + + GraphqlTriggers.work_item_note_created(noteable.to_work_item_global_id, self) + end + + def trigger_note_subscription_update + return unless trigger_note_subscription? + + GraphqlTriggers.work_item_note_updated(noteable.to_work_item_global_id, self) + end + + def trigger_note_subscription_destroy + return unless trigger_note_subscription? + + # when deleting a note, we cannot pass it on as a Note instance, as GitlabSchema.object_from_id + # would try to resolve the given Note and fetch it from DB which would raise NotFound exception. + # So instead we just pass over the string representations of the note and discussion IDs, + # so that the subscriber can identify the discussion and the note. + deleted_note_data = { + id: self.id, + model_name: self.class.name, + discussion_id: self.discussion_id, + last_discussion_note: discussion.notes == [self] + } + + GraphqlTriggers.work_item_note_deleted(noteable.to_work_item_global_id, deleted_note_data) + end + class << self extend Gitlab::Utils::Override @@ -718,6 +751,10 @@ class Note < ApplicationRecord private + def trigger_note_subscription? + for_issue? && noteable + end + def system_note_viewable_by?(user) return true unless system_note_metadata diff --git a/app/models/project.rb b/app/models/project.rb index 04cbb582da8..8cb17632170 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -307,6 +307,9 @@ class Project < ApplicationRecord primary_key: :project_namespace_id, foreign_key: :member_namespace_id, inverse_of: :project, class_name: 'ProjectMember' has_many :members_and_requesters, as: :source, class_name: 'ProjectMember' + has_many :namespace_members_and_requesters, -> { unscope(where: %i[source_id source_type]) }, + primary_key: :project_namespace_id, foreign_key: :member_namespace_id, inverse_of: :project, + class_name: 'ProjectMember' has_many :users, through: :project_members diff --git a/app/models/release_highlight.rb b/app/models/release_highlight.rb index c2d498ecb13..7cead8a42cd 100644 --- a/app/models/release_highlight.rb +++ b/app/models/release_highlight.rb @@ -2,7 +2,6 @@ class ReleaseHighlight CACHE_DURATION = 1.hour - FILES_PATH = Rails.root.join('data', 'whats_new', '*.yml') FREE_PACKAGE = 'Free' PREMIUM_PACKAGE = 'Premium' @@ -48,13 +47,17 @@ class ReleaseHighlight nil end + def self.whats_new_path + Rails.root.join('data/whats_new/*.yml') + end + def self.file_paths @file_paths ||= self.relative_file_paths.map { |path| path.prepend(Rails.root.to_s) } end def self.relative_file_paths Rails.cache.fetch(self.cache_key('file_paths'), expires_in: CACHE_DURATION) do - Dir.glob(FILES_PATH).sort.reverse.map { |path| path.delete_prefix(Rails.root.to_s) } + Dir.glob(whats_new_path).sort.reverse.map { |path| path.delete_prefix(Rails.root.to_s) } end end @@ -119,3 +122,5 @@ class ReleaseHighlight item['available_in']&.include?(current_package) end end + +ReleaseHighlight.prepend_mod diff --git a/app/serializers/import/github_realtime_repo_entity.rb b/app/serializers/import/github_realtime_repo_entity.rb new file mode 100644 index 00000000000..c26ae5f668d --- /dev/null +++ b/app/serializers/import/github_realtime_repo_entity.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Import + class GithubRealtimeRepoEntity < Grape::Entity + expose :id, documentation: { type: 'integer', example: 1 } + expose :import_status, documentation: { type: 'string', example: 'importing' } + expose :stats, + documentation: { + type: 'object', example: '{"fetched":{"label":10},"imported":{"label":10}}' + } do |project| + ::Gitlab::GithubImport::ObjectCounter.summary(project) + end + + expose :import_error, if: ->(project) { project.import_state&.failed? } do |project| + project.import_failures.last&.exception_message + end + end +end diff --git a/app/serializers/import/github_realtime_repo_serializer.rb b/app/serializers/import/github_realtime_repo_serializer.rb new file mode 100644 index 00000000000..8d03f5ce002 --- /dev/null +++ b/app/serializers/import/github_realtime_repo_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module Import + class GithubRealtimeRepoSerializer < BaseSerializer + entity ::Import::GithubRealtimeRepoEntity + end +end diff --git a/app/serializers/project_import_entity.rb b/app/serializers/project_import_entity.rb index a3dbff3dc0b..58360321f7c 100644 --- a/app/serializers/project_import_entity.rb +++ b/app/serializers/project_import_entity.rb @@ -12,4 +12,8 @@ class ProjectImportEntity < ProjectEntity expose :provider_link, documentation: { type: 'string', example: '/source/source-repo' } do |project, options| provider_project_link_url(options[:provider_url], project[:import_source]) end + + expose :import_error, if: ->(project) { project.import_state&.failed? } do |project| + project.import_failures.last&.exception_message + end end diff --git a/config/feature_flags/ops/advanced_user_index.yml b/config/feature_flags/ops/advanced_user_index.yml new file mode 100644 index 00000000000..2aa33aa265a --- /dev/null +++ b/config/feature_flags/ops/advanced_user_index.yml @@ -0,0 +1,8 @@ +--- +name: advanced_user_index +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/110946 +rollout_issue_url: +milestone: '15.9' +type: ops +group: group::global_search +default_enabled: true diff --git a/db/migrate/20220722150231_create_function_gitlab_schema_prevent_write.rb b/db/migrate/20220722150231_create_function_gitlab_schema_prevent_write.rb index d25923923f2..29c6d9ce87d 100644 --- a/db/migrate/20220722150231_create_function_gitlab_schema_prevent_write.rb +++ b/db/migrate/20220722150231_create_function_gitlab_schema_prevent_write.rb @@ -26,7 +26,7 @@ class CreateFunctionGitlabSchemaPreventWrite < Gitlab::Database::Migration[2.0] return if Gitlab.com? execute(<<~SQL) - DROP FUNCTION #{TRIGGER_FUNCTION_NAME} + DROP FUNCTION #{TRIGGER_FUNCTION_NAME} CASCADE SQL end end diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 5c7a53c3089..d9e07d45355 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -12227,6 +12227,16 @@ The response from the AdminSidekiqQueuesDeleteJobs mutation. | <a id="deletejobsresponsedeletedjobs"></a>`deletedJobs` | [`Int`](#int) | Number of matching jobs deleted. | | <a id="deletejobsresponsequeuesize"></a>`queueSize` | [`Int`](#int) | Queue size after processing. | +### `DeletedNote` + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| <a id="deletednotediscussionid"></a>`discussionId` | [`DiscussionID`](#discussionid) | ID of the discussion for the deleted note. | +| <a id="deletednoteid"></a>`id` | [`NoteID!`](#noteid) | ID of the deleted note. | +| <a id="deletednotelastdiscussionnote"></a>`lastDiscussionNote` | [`Boolean`](#boolean) | Whether deleted note is the last note in the discussion. | + ### `DependencyProxyBlob` Dependency proxy blob. diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index e27d4911158..8672480e993 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -432,6 +432,12 @@ results are available, and not just the first failure. You must [set feature category metadata for each RSpec example](../feature_categorization/index.md#rspec-examples). +### Tests depending on EE license + +You can use `if: Gitlab.ee?` or `unless: Gitlab.ee?` on context/spec blocks to execute tests depending on whether running with `FOSS_ONLY=1`. + +Example: [SchemaValidator reads a different path depending on the license](https://gitlab.com/gitlab-org/gitlab/-/blob/7cdcf9819cfa02c701d6fa9f18c1e7a8972884ed/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb#L571) + ### Coverage [`simplecov`](https://github.com/colszowka/simplecov) is used to generate code test coverage reports. diff --git a/doc/user/group/compliance_frameworks.md b/doc/user/group/compliance_frameworks.md index 55bcb4ea1fb..a926acf5b08 100644 --- a/doc/user/group/compliance_frameworks.md +++ b/doc/user/group/compliance_frameworks.md @@ -26,7 +26,7 @@ Group owners can create, edit, and delete compliance frameworks: > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/375036) in GitLab 15.6. Group owners can set a default compliance framework. The default framework is applied to all the new and imported -projects that are created within that group. It does not affect the framework applied to the existing projects. The +projects that are created in that group. It does not affect the framework applied to the existing projects. The default framework cannot be deleted. A compliance framework that is set to default has a **default** label. @@ -202,7 +202,8 @@ include: # Execute individual project's configuration (if project contains .git When an MR originates in a fork, the branch to be merged usually only exists in the fork. When creating such an MR against a project with CF pipelines, the above snippet fails with a `Project <project-name> reference <branch-name> does not exist!` error message. -This is because in the context of the target project, `$CI_COMMIT_REF_NAME` evaluates to a non-existing branch name. +This error occurs because in the context of the target project, `$CI_COMMIT_REF_NAME` evaluates to a non-existing +branch name. To get the correct context, use `$CI_MERGE_REQUEST_SOURCE_PROJECT_PATH` instead of `$CI_PROJECT_PATH`. This variable is only available in @@ -241,7 +242,7 @@ Generally, if a value in a compliance job: Either might be wanted or not depending on your use case. -There are a few best practices for ensuring that these jobs are always run exactly +The following are a few best practices for ensuring that these jobs are always run exactly as you define them and that downstream, project-level pipeline configurations cannot change them: @@ -266,7 +267,7 @@ compatibility for combining compliance pipelines, and parent and child pipelines Compliance pipelines start on the run of _every_ pipeline in a labeled project. This means that if a pipeline in the labeled project triggers a child pipeline, the compliance pipeline runs first. This can trigger the parent pipeline, instead of the child pipeline. -Therefore, in projects with compliance frameworks, we recommend replacing +Therefore, in projects with compliance frameworks, you should replace [parent-child pipelines](../../ci/pipelines/downstream_pipelines.md#parent-child-pipelines) with the following: - Direct [`include`](../../ci/yaml/index.md#include) statements that provide the parent pipeline with child pipeline configuration. @@ -279,8 +280,9 @@ This alternative ensures the compliance pipeline does not re-start the parent pi ### Cannot remove compliance framework from a project -If you move a project, the compliance framework can become orphaned and can't be removed. To manually remove a compliance framework from a project, run the following GraphQL -mutation with your project's ID: +Because of a [known issue](https://gitlab.com/gitlab-org/gitlab/-/issues/390626), if you move a project, its compliance +framework becomes orphaned and can't be removed. To manually remove a compliance framework from a project, run the +following GraphQL mutation with your project's ID: ```graphql mutation { diff --git a/doc/user/group/import/index.md b/doc/user/group/import/index.md index 4eeb2a55189..e23458249f2 100644 --- a/doc/user/group/import/index.md +++ b/doc/user/group/import/index.md @@ -122,7 +122,8 @@ Create the group you want to import to and connect the source: > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/385689) in GitLab 15.8, option to import groups with or without projects. After you have authorized access to the source GitLab instance, you are redirected to the GitLab group -importer page. The top-level groups on the connected source instance you have the Owner role for are listed. +importer page. Here you can see a list of the top-level groups on the connected source instance where you have the Owner +role. 1. By default, the proposed group namespaces match the names as they exist in source instance, but based on your permissions, you can choose to edit these names before you proceed to import any of them. 1. Next to the groups you want to import, select either: diff --git a/doc/user/project/import/index.md b/doc/user/project/import/index.md index 9ea78631ee1..24748b2e89c 100644 --- a/doc/user/project/import/index.md +++ b/doc/user/project/import/index.md @@ -17,20 +17,22 @@ If you want to bring existing projects to GitLab or copy GitLab projects to a di Prerequisite: -- At least the Maintainer role on the destination group to import to. Using the Developer role for this purpose was [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/387891) in GitLab 15.8 and will be removed in GitLab 16.0. +- At least the Maintainer role on the destination group to import to. Using the Developer role for this purpose was + [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/387891) in GitLab 15.8 and will be removed in GitLab 16.0. For any type of source and target, you can migrate GitLab projects: -- When [migrating groups by direct transfer](../../group/import/index.md#migrate-groups-by-direct-transfer-recommended), which allows you to migrate all - projects in a group at once. Migrating projects by direct transfer is in [Beta](../../../policy/alpha-beta-support.md#beta-features). The feature is not ready - for production use. +- When [migrating groups by direct transfer](../../group/import/index.md#migrate-groups-by-direct-transfer-recommended), + which allows you to migrate all projects in a group simultaneously. Migrating projects by direct transfer is in + [Beta](../../../policy/alpha-beta-support.md#beta-features). The feature is not ready for production use. - Using [file exports](../settings/import_export.md). With this method you can migrate projects one by one. No network connection between instances is required. If you only need to migrate Git repositories, you can [import each project by URL](repo_by_url.md). However, you can't import issues and merge requests this way. To retain metadata like issues and merge requests, either: -- [Migrate projects with groups by direct transfer](../../group/import/index.md#migrate-groups-by-direct-transfer-recommended). This feature is in [Beta](../../../policy/alpha-beta-support.md#beta-features). It is not ready for production use. +- [Migrate projects with groups by direct transfer](../../group/import/index.md#migrate-groups-by-direct-transfer-recommended). + This feature is in [Beta](../../../policy/alpha-beta-support.md#beta-features). It is not ready for production use. - Use [file exports](../settings/import_export.md) to import projects. Keep in mind the limitations of [migrating using file exports](../settings/import_export.md#items-that-are-exported). diff --git a/doc/user/project/settings/import_export.md b/doc/user/project/settings/import_export.md index 2b5f5dfa21d..3715eef08e0 100644 --- a/doc/user/project/settings/import_export.md +++ b/doc/user/project/settings/import_export.md @@ -14,7 +14,7 @@ then imported into a new GitLab instance. You can also: GitLab maps user contributions correctly when an admin access token is used to perform the import. -As a result, migrating projects using file exports does not map user contributions correctly when you are importing +Consequently, migrating projects using file exports does not map user contributions correctly when you are importing projects from a self-managed instance to GitLab.com. Instead, all GitLab user associations (such as comment author) are changed to the user importing the project. For more diff --git a/lib/api/appearance.rb b/lib/api/appearance.rb index 2023c43b463..f8e4f5d2ab2 100644 --- a/lib/api/appearance.rb +++ b/lib/api/appearance.rb @@ -31,10 +31,10 @@ module API optional :pwa_short_name, type: String, desc: 'Optional, short name for Progressive Web App' optional :pwa_description, type: String, desc: 'An explanation of what the Progressive Web App does' # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 - optional :logo, type: File, desc: 'Instance image used on the sign in / sign up page' # rubocop:disable Scalability/FileUploads - optional :pwa_icon, type: File, desc: 'Icon used for Progressive Web App' # rubocop:disable Scalability/FileUploads - optional :header_logo, type: File, desc: 'Instance image used for the main navigation bar' # rubocop:disable Scalability/FileUploads - optional :favicon, type: File, desc: 'Instance favicon in .ico/.png format' # rubocop:disable Scalability/FileUploads + optional :logo, type: File, desc: 'Instance image used on the sign in / sign up page' # rubocop:todo Scalability/FileUploads + optional :pwa_icon, type: File, desc: 'Icon used for Progressive Web App' # rubocop:todo Scalability/FileUploads + optional :header_logo, type: File, desc: 'Instance image used for the main navigation bar' # rubocop:todo Scalability/FileUploads + optional :favicon, type: File, desc: 'Instance favicon in .ico/.png format' # rubocop:todo Scalability/FileUploads optional :new_project_guidelines, type: String, desc: 'Markdown text shown on the new project page' optional :profile_image_guidelines, type: String, desc: 'Markdown text shown on the profile page below Public Avatar' optional :header_message, type: String, desc: 'Message within the system header bar' diff --git a/lib/api/pages_domains.rb b/lib/api/pages_domains.rb index 15c1a78839f..db156186458 100644 --- a/lib/api/pages_domains.rb +++ b/lib/api/pages_domains.rb @@ -94,7 +94,7 @@ module API end params do requires :domain, type: String, desc: 'The domain' - # rubocop:disable Scalability/FileUploads + # rubocop:todo Scalability/FileUploads # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 optional :certificate, types: [File, String], desc: 'The certificate', as: :user_provided_certificate optional :key, types: [File, String], desc: 'The key', as: :user_provided_key @@ -122,7 +122,7 @@ module API desc 'Updates a pages domain' params do requires :domain, type: String, desc: 'The domain' - # rubocop:disable Scalability/FileUploads + # rubocop:todo Scalability/FileUploads # TODO: remove rubocop disable - https://gitlab.com/gitlab-org/gitlab/issues/14960 optional :certificate, types: [File, String], desc: 'The certificate', as: :user_provided_certificate optional :key, types: [File, String], desc: 'The key', as: :user_provided_key diff --git a/lib/gitlab/ci/artifacts/logger.rb b/lib/gitlab/ci/artifacts/logger.rb index 73f0409f8b1..63064118232 100644 --- a/lib/gitlab/ci/artifacts/logger.rb +++ b/lib/gitlab/ci/artifacts/logger.rb @@ -35,7 +35,7 @@ module Gitlab message: 'Artifact created', job_artifact_id: artifact.id, size: artifact.size, - type: artifact.file_type, + file_type: artifact.file_type, build_id: artifact.job_id, project_id: artifact.project_id ) @@ -51,7 +51,7 @@ module Gitlab job_artifact_id: artifact.id, expire_at: artifact.expire_at, size: artifact.size, - type: artifact.file_type, + file_type: artifact.file_type, build_id: artifact.job_id, project_id: artifact.project_id, method: method diff --git a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb index e6a2e5c3b33..bef4b147359 100644 --- a/lib/gitlab/ci/parsers/security/validators/schema_validator.rb +++ b/lib/gitlab/ci/parsers/security/validators/schema_validator.rb @@ -65,7 +65,7 @@ module Gitlab if latest_vendored_patch_version latest_vendored_patch_version_file = File.join(root_path, latest_vendored_patch_version, file_name) - return latest_vendored_patch_version_file if File.file?(latest_vendored_patch_version) + return latest_vendored_patch_version_file if File.file?(latest_vendored_patch_version_file) end earliest_supported_version = SUPPORTED_VERSIONS[report_type].min diff --git a/lib/tasks/gitlab/db/lock_writes.rake b/lib/tasks/gitlab/db/lock_writes.rake index 7f3b806b53d..59478d47d12 100644 --- a/lib/tasks/gitlab/db/lock_writes.rake +++ b/lib/tasks/gitlab/db/lock_writes.rake @@ -4,16 +4,20 @@ namespace :gitlab do namespace :db do desc "GitLab | DB | Install prevent write triggers on all databases" task lock_writes: [:environment, 'gitlab:db:validate_config'] do + logger = Logger.new($stdout) + logger.level = Gitlab::Utils.to_boolean(ENV['VERBOSE']) ? Logger::INFO : Logger::WARN Gitlab::Database::TablesLocker.new( - logger: Logger.new($stdout), + logger: logger, dry_run: Gitlab::Utils.to_boolean(ENV['DRY_RUN'], default: false) ).lock_writes end desc "GitLab | DB | Remove all triggers that prevents writes from all databases" task unlock_writes: :environment do + logger = Logger.new($stdout) + logger.level = Gitlab::Utils.to_boolean(ENV['VERBOSE']) ? Logger::INFO : Logger::WARN Gitlab::Database::TablesLocker.new( - logger: Logger.new($stdout), + logger: logger, dry_run: Gitlab::Utils.to_boolean(ENV['DRY_RUN'], default: false) ).unlock_writes end diff --git a/rubocop/cop/scalability/file_uploads.rb b/rubocop/cop/scalability/file_uploads.rb index 3ccb9110e79..fc52444c551 100644 --- a/rubocop/cop/scalability/file_uploads.rb +++ b/rubocop/cop/scalability/file_uploads.rb @@ -26,34 +26,25 @@ module RuboCop # end # class FileUploads < RuboCop::Cop::Base - MSG = 'Do not upload files without workhorse acceleration. Please refer to https://docs.gitlab.com/ee/development/uploads.html' - - def_node_search :file_type_params?, <<~PATTERN - (send nil? {:requires :optional} (sym _) (hash <(pair (sym :type)(const nil? :File)) ...>)) - PATTERN - - def_node_search :file_types_params?, <<~PATTERN - (send nil? {:requires :optional} (sym _) (hash <(pair (sym :types)(array <(const nil? :File) ...>)) ...>)) + MSG = 'Do not upload files without workhorse acceleration. ' \ + 'Please refer to https://docs.gitlab.com/ee/development/uploads.html' + + def_node_matcher :file_in_type, <<~PATTERN + (send nil? {:requires :optional} + (sym _) + (hash + { + <(pair (sym :types) (array <$(const nil? :File) ...>)) ...> + <(pair (sym :type) $(const nil? :File)) ...> + } + ) + ) PATTERN - def be_file_param_usage?(node) - file_type_params?(node) || file_types_params?(node) - end - def on_send(node) - return unless be_file_param_usage?(node) - - add_offense(find_file_param(node)) - end - - private - - def find_file_param(node) - node.each_descendant.find { |children| file_node_pattern.match(children) } - end - - def file_node_pattern - @file_node_pattern ||= RuboCop::NodePattern.new("(const nil? :File)") + file_in_type(node) do |file_node| + add_offense(file_node) + end end end end diff --git a/rubocop/rubocop-ruby27.yml b/rubocop/rubocop-ruby27.yml index 5c1b71f81e2..29957defeb0 100644 --- a/rubocop/rubocop-ruby27.yml +++ b/rubocop/rubocop-ruby27.yml @@ -2,7 +2,7 @@ # Ruby 3.0. This configuration should be removed after the transition has been # completed. -# These cops are disabled in Ruby 3.0 (rubocop-30.yml). +# These cops are enabled in Ruby 3.0 (rubocop-30.yml). Style/MutableConstant: Enabled: false Style/RedundantFreeze: diff --git a/scripts/utils.sh b/scripts/utils.sh index 6fee1909bb7..55005d0abff 100644 --- a/scripts/utils.sh +++ b/scripts/utils.sh @@ -112,7 +112,7 @@ function setup_db_praefect() { function setup_db() { run_timed_command "setup_db_user_only" - run_timed_command_with_metric "bundle exec rake db:drop db:create db:schema:load db:migrate" "setup_db" + run_timed_command_with_metric "bundle exec rake db:drop db:create db:schema:load db:migrate gitlab:db:lock_writes" "setup_db" run_timed_command "setup_db_praefect" } diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index fb27fe58cd9..ab33195eb83 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -8,183 +8,168 @@ RSpec.describe Projects::ProjectMembersController do let_it_be(:sub_group) { create(:group, parent: group) } let_it_be(:project, reload: true) { create(:project, :public) } - before do - travel_to DateTime.new(2019, 4, 1) - end + shared_examples_for 'controller actions' do + before do + travel_to DateTime.new(2019, 4, 1) + end - after do - travel_back - end + after do + travel_back + end - describe 'GET index' do - it 'has the project_members address with a 200 status code' do - get :index, params: { namespace_id: project.namespace, project_id: project } + describe 'GET index' do + it 'has the project_members address with a 200 status code' do + get :index, params: { namespace_id: project.namespace, project_id: project } - expect(response).to have_gitlab_http_status(:ok) - end + expect(response).to have_gitlab_http_status(:ok) + end - context 'project members' do - context 'when project belongs to group' do - let_it_be(:user_in_group) { create(:user) } - let_it_be(:project_in_group) { create(:project, :public, group: group) } + context 'project members' do + context 'when project belongs to group' do + let_it_be(:user_in_group) { create(:user) } + let_it_be(:project_in_group) { create(:project, :public, group: group) } - before do - group.add_owner(user_in_group) - project_in_group.add_maintainer(user) - sign_in(user) - end + before do + group.add_owner(user_in_group) + project_in_group.add_maintainer(user) + sign_in(user) + end - it 'lists inherited project members by default' do - get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group } + it 'lists inherited project members by default' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group } - expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id, user_in_group.id) - end + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id, user_in_group.id) + end - it 'lists direct project members only' do - get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'exclude' } + it 'lists direct project members only' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'exclude' } - expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id) - end + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id) + end - it 'lists inherited project members only' do - get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'only' } + it 'lists inherited project members only' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'only' } - expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user_in_group.id) + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user_in_group.id) + end end - end - context 'when project belongs to a sub-group' do - let_it_be(:user_in_group) { create(:user) } - let_it_be(:project_in_group) { create(:project, :public, group: sub_group) } + context 'when project belongs to a sub-group' do + let_it_be(:user_in_group) { create(:user) } + let_it_be(:project_in_group) { create(:project, :public, group: sub_group) } - before do - group.add_owner(user_in_group) - project_in_group.add_maintainer(user) - sign_in(user) - end + before do + group.add_owner(user_in_group) + project_in_group.add_maintainer(user) + sign_in(user) + end - it 'lists inherited project members by default' do - get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group } + it 'lists inherited project members by default' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group } - expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id, user_in_group.id) - end + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id, user_in_group.id) + end - it 'lists direct project members only' do - get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'exclude' } + it 'lists direct project members only' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'exclude' } - expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id) - end + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user.id) + end - it 'lists inherited project members only' do - get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'only' } + it 'lists inherited project members only' do + get :index, params: { namespace_id: project_in_group.namespace, project_id: project_in_group, with_inherited_permissions: 'only' } - expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user_in_group.id) + expect(assigns(:project_members).map(&:user_id)).to contain_exactly(user_in_group.id) + end end - end - context 'when invited project members are present' do - let!(:invited_member) { create(:project_member, :invited, project: project) } + context 'when invited project members are present' do + let!(:invited_member) { create(:project_member, :invited, project: project) } - before do - project.add_maintainer(user) - sign_in(user) - end + before do + project.add_maintainer(user) + sign_in(user) + end - it 'excludes the invited members from project members list' do - get :index, params: { namespace_id: project.namespace, project_id: project } + it 'excludes the invited members from project members list' do + get :index, params: { namespace_id: project.namespace, project_id: project } - expect(assigns(:project_members).map(&:invite_email)).not_to contain_exactly(invited_member.invite_email) + expect(assigns(:project_members).map(&:invite_email)).not_to contain_exactly(invited_member.invite_email) + end end end - end - - context 'invited members' do - let_it_be(:invited_member) { create(:project_member, :invited, project: project) } - before do - sign_in(user) - end + context 'invited members' do + let_it_be(:invited_member) { create(:project_member, :invited, project: project) } - context 'when user has `admin_project_member` permissions' do before do - project.add_maintainer(user) + sign_in(user) end - it 'lists invited members' do - get :index, params: { namespace_id: project.namespace, project_id: project } + context 'when user has `admin_project_member` permissions' do + before do + project.add_maintainer(user) + end + + it 'lists invited members' do + get :index, params: { namespace_id: project.namespace, project_id: project } - expect(assigns(:invited_members).map(&:invite_email)).to contain_exactly(invited_member.invite_email) + expect(assigns(:invited_members).map(&:invite_email)).to contain_exactly(invited_member.invite_email) + end end - end - context 'when user does not have `admin_project_member` permissions' do - it 'does not list invited members' do - get :index, params: { namespace_id: project.namespace, project_id: project } + context 'when user does not have `admin_project_member` permissions' do + it 'does not list invited members' do + get :index, params: { namespace_id: project.namespace, project_id: project } - expect(assigns(:invited_members)).to be_nil + expect(assigns(:invited_members)).to be_nil + end end end - end - context 'access requests' do - let_it_be(:access_requester_user) { create(:user) } - - before do - project.request_access(access_requester_user) - sign_in(user) - end + context 'access requests' do + let_it_be(:access_requester_user) { create(:user) } - context 'when user has `admin_project_member` permissions' do before do - project.add_maintainer(user) + project.request_access(access_requester_user) + sign_in(user) end - it 'lists access requests' do - get :index, params: { namespace_id: project.namespace, project_id: project } + context 'when user has `admin_project_member` permissions' do + before do + project.add_maintainer(user) + end + + it 'lists access requests' do + get :index, params: { namespace_id: project.namespace, project_id: project } - expect(assigns(:requesters).map(&:user_id)).to contain_exactly(access_requester_user.id) + expect(assigns(:requesters).map(&:user_id)).to contain_exactly(access_requester_user.id) + end end - end - context 'when user does not have `admin_project_member` permissions' do - it 'does not list access requests' do - get :index, params: { namespace_id: project.namespace, project_id: project } + context 'when user does not have `admin_project_member` permissions' do + it 'does not list access requests' do + get :index, params: { namespace_id: project.namespace, project_id: project } - expect(assigns(:requesters)).to be_nil + expect(assigns(:requesters)).to be_nil + end end end end - end - - describe 'PUT update' do - let_it_be(:requester) { create(:project_member, :access_request, project: project) } - - before do - project.add_maintainer(user) - sign_in(user) - end - context 'access level' do - Gitlab::Access.options.each do |label, value| - it "can change the access level to #{label}" do - params = { - project_member: { access_level: value }, - namespace_id: project.namespace, - project_id: project, - id: requester - } + describe 'PUT update' do + let_it_be(:requester) { create(:project_member, :access_request, project: project) } - put :update, params: params, xhr: true - - expect(requester.reload.human_access).to eq(label) - end + before do + project.add_maintainer(user) + sign_in(user) end - describe 'managing project direct owners' do - context 'when a Maintainer tries to elevate another user to OWNER' do - it 'does not allow the operation' do + context 'access level' do + Gitlab::Access.options.each do |label, value| + it "can change the access level to #{label}" do params = { - project_member: { access_level: Gitlab::Access::OWNER }, + project_member: { access_level: value }, namespace_id: project.namespace, project_id: project, id: requester @@ -192,368 +177,395 @@ RSpec.describe Projects::ProjectMembersController do put :update, params: params, xhr: true - expect(response).to have_gitlab_http_status(:forbidden) + expect(requester.reload.human_access).to eq(label) end end - context 'when a user with OWNER access tries to elevate another user to OWNER' do - # inherited owner role via personal project association - let(:user) { project.first_owner } + describe 'managing project direct owners' do + context 'when a Maintainer tries to elevate another user to OWNER' do + it 'does not allow the operation' do + params = { + project_member: { access_level: Gitlab::Access::OWNER }, + namespace_id: project.namespace, + project_id: project, + id: requester + } - before do - sign_in(user) + put :update, params: params, xhr: true + + expect(response).to have_gitlab_http_status(:forbidden) + end end - it 'returns success' do - params = { - project_member: { access_level: Gitlab::Access::OWNER }, - namespace_id: project.namespace, - project_id: project, - id: requester - } + context 'when a user with OWNER access tries to elevate another user to OWNER' do + # inherited owner role via personal project association + let(:user) { project.first_owner } - put :update, params: params, xhr: true + before do + sign_in(user) + end + + it 'returns success' do + params = { + project_member: { access_level: Gitlab::Access::OWNER }, + namespace_id: project.namespace, + project_id: project, + id: requester + } + + put :update, params: params, xhr: true - expect(response).to have_gitlab_http_status(:ok) - expect(requester.reload.access_level).to eq(Gitlab::Access::OWNER) + expect(response).to have_gitlab_http_status(:ok) + expect(requester.reload.access_level).to eq(Gitlab::Access::OWNER) + end end end end - end - context 'access expiry date' do - subject do - put :update, xhr: true, params: { - project_member: { - expires_at: expires_at - }, - namespace_id: project.namespace, - project_id: project, - id: requester - } - end + context 'access expiry date' do + subject do + put :update, xhr: true, params: { + project_member: { + expires_at: expires_at + }, + namespace_id: project.namespace, + project_id: project, + id: requester + } + end - context 'when set to a date in the past' do - let(:expires_at) { 2.days.ago } + context 'when set to a date in the past' do + let(:expires_at) { 2.days.ago } - it 'does not update the member' do - subject + it 'does not update the member' do + subject - expect(requester.reload.expires_at).not_to eq(expires_at.to_date) - end + expect(requester.reload.expires_at).not_to eq(expires_at.to_date) + end - it 'returns error status' do - subject + it 'returns error status' do + subject - expect(response).to have_gitlab_http_status(:unprocessable_entity) - end + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end - it 'returns error message' do - subject + it 'returns error message' do + subject - expect(json_response).to eq({ 'message' => 'Expires at cannot be a date in the past' }) + expect(json_response).to eq({ 'message' => 'Expires at cannot be a date in the past' }) + end end - end - context 'when set to a date in the future' do - let(:expires_at) { 5.days.from_now } + context 'when set to a date in the future' do + let(:expires_at) { 5.days.from_now } - it 'updates the member' do - subject + it 'updates the member' do + subject - expect(requester.reload.expires_at).to eq(expires_at.to_date) + expect(requester.reload.expires_at).to eq(expires_at.to_date) + end end end - end - context 'expiration date' do - let(:expiry_date) { 1.month.from_now.to_date } + context 'expiration date' do + let(:expiry_date) { 1.month.from_now.to_date } - before do - travel_to Time.now.utc.beginning_of_day - - put( - :update, - params: { - project_member: { expires_at: expiry_date }, - namespace_id: project.namespace, - project_id: project, - id: requester - }, - format: :json - ) - end + before do + travel_to Time.now.utc.beginning_of_day + + put( + :update, + params: { + project_member: { expires_at: expiry_date }, + namespace_id: project.namespace, + project_id: project, + id: requester + }, + format: :json + ) + end - context 'when `expires_at` is set' do - it 'returns correct json response' do - expect(json_response).to eq({ - "expires_soon" => false, - "expires_at_formatted" => expiry_date.to_time.in_time_zone.to_s(:medium) - }) + context 'when `expires_at` is set' do + it 'returns correct json response' do + expect(json_response).to eq({ + "expires_soon" => false, + "expires_at_formatted" => expiry_date.to_time.in_time_zone.to_s(:medium) + }) + end end - end - context 'when `expires_at` is not set' do - let(:expiry_date) { nil } + context 'when `expires_at` is not set' do + let(:expiry_date) { nil } - it 'returns empty json response' do - expect(json_response).to be_empty + it 'returns empty json response' do + expect(json_response).to be_empty + end end end end - end - describe 'DELETE destroy' do - let_it_be(:member) { create(:project_member, :developer, project: project) } + describe 'DELETE destroy' do + let_it_be(:member) { create(:project_member, :developer, project: project) } - before do - sign_in(user) - end + before do + sign_in(user) + end - context 'when member is not found' do - it 'returns 404' do - delete :destroy, params: { - namespace_id: project.namespace, - project_id: project, - id: 42 - } + context 'when member is not found' do + it 'returns 404' do + delete :destroy, params: { + namespace_id: project.namespace, + project_id: project, + id: 42 + } - expect(response).to have_gitlab_http_status(:not_found) + expect(response).to have_gitlab_http_status(:not_found) + end end - end - context 'when member is found' do - context 'when user does not have enough rights' do - context 'when user does not have rights to manage other members' do - before do - project.add_developer(user) + context 'when member is found' do + context 'when user does not have enough rights' do + context 'when user does not have rights to manage other members' do + before do + project.add_developer(user) + end + + it 'returns 404', :aggregate_failures do + delete :destroy, params: { + namespace_id: project.namespace, + project_id: project, + id: member + } + + expect(response).to have_gitlab_http_status(:not_found) + expect(project.members).to include member + end end - it 'returns 404', :aggregate_failures do - delete :destroy, params: { - namespace_id: project.namespace, - project_id: project, - id: member - } + context 'when user does not have rights to manage Owner members' do + let_it_be(:member) { create(:project_member, project: project, access_level: Gitlab::Access::OWNER) } - expect(response).to have_gitlab_http_status(:not_found) - expect(project.members).to include member + before do + project.add_maintainer(user) + end + + it 'returns 403', :aggregate_failures do + delete :destroy, params: { + namespace_id: project.namespace, + project_id: project, + id: member + } + + expect(response).to have_gitlab_http_status(:forbidden) + expect(project.members).to include member + end end end - context 'when user does not have rights to manage Owner members' do - let_it_be(:member) { create(:project_member, project: project, access_level: Gitlab::Access::OWNER) } - + context 'when user has enough rights' do before do project.add_maintainer(user) end - it 'returns 403', :aggregate_failures do + it '[HTML] removes user from members', :aggregate_failures do + delete :destroy, params: { + namespace_id: project.namespace, + project_id: project, + id: member + } + + expect(response).to redirect_to( + project_project_members_path(project) + ) + expect(project.members).not_to include member + end + + it '[JS] removes user from members', :aggregate_failures do delete :destroy, params: { namespace_id: project.namespace, project_id: project, id: member - } + }, xhr: true - expect(response).to have_gitlab_http_status(:forbidden) - expect(project.members).to include member + expect(response).to be_successful + expect(project.members).not_to include member end end end - - context 'when user has enough rights' do - before do - project.add_maintainer(user) - end - - it '[HTML] removes user from members', :aggregate_failures do - delete :destroy, params: { - namespace_id: project.namespace, - project_id: project, - id: member - } - - expect(response).to redirect_to( - project_project_members_path(project) - ) - expect(project.members).not_to include member - end - - it '[JS] removes user from members', :aggregate_failures do - delete :destroy, params: { - namespace_id: project.namespace, - project_id: project, - id: member - }, xhr: true - - expect(response).to be_successful - expect(project.members).not_to include member - end - end - end - end - - describe 'DELETE leave' do - before do - sign_in(user) end - context 'when member is not found' do - it 'returns 404' do - delete :leave, params: { - namespace_id: project.namespace, - project_id: project - } - - expect(response).to have_gitlab_http_status(:not_found) + describe 'DELETE leave' do + before do + sign_in(user) end - end - - context 'when member is found' do - context 'and is not an owner' do - before do - project.add_developer(user) - end - it 'removes user from members', :aggregate_failures do + context 'when member is not found' do + it 'returns 404' do delete :leave, params: { namespace_id: project.namespace, project_id: project } - expect(controller).to set_flash.to "You left the \"#{project.human_name}\" project." - expect(response).to redirect_to(dashboard_projects_path) - expect(project.users).not_to include user + expect(response).to have_gitlab_http_status(:not_found) end end - context 'and is an owner' do - let(:project) { create(:project, namespace: user.namespace) } + context 'when member is found' do + context 'and is not an owner' do + before do + project.add_developer(user) + end + + it 'removes user from members', :aggregate_failures do + delete :leave, params: { + namespace_id: project.namespace, + project_id: project + } - before do - project.add_maintainer(user) + expect(controller).to set_flash.to "You left the \"#{project.human_name}\" project." + expect(response).to redirect_to(dashboard_projects_path) + expect(project.users).not_to include user + end end - it 'cannot remove themselves from the project' do - delete :leave, params: { - namespace_id: project.namespace, - project_id: project - } + context 'and is an owner' do + let(:project) { create(:project, namespace: user.namespace) } - expect(response).to have_gitlab_http_status(:forbidden) - end - end + before do + project.add_maintainer(user) + end - context 'and is a requester' do - before do - project.request_access(user) + it 'cannot remove themselves from the project' do + delete :leave, params: { + namespace_id: project.namespace, + project_id: project + } + + expect(response).to have_gitlab_http_status(:forbidden) + end end - it 'removes user from members', :aggregate_failures do - delete :leave, params: { - namespace_id: project.namespace, - project_id: project - } + context 'and is a requester' do + before do + project.request_access(user) + end + + it 'removes user from members', :aggregate_failures do + delete :leave, params: { + namespace_id: project.namespace, + project_id: project + } - expect(controller).to set_flash.to 'Your access request to the project has been withdrawn.' - expect(response).to redirect_to(project_path(project)) - expect(project.requesters).to be_empty - expect(project.users).not_to include user + expect(controller).to set_flash.to 'Your access request to the project has been withdrawn.' + expect(response).to redirect_to(project_path(project)) + expect(project.requesters).to be_empty + expect(project.users).not_to include user + end end end end - end - - describe 'POST request_access' do - before do - sign_in(user) - end - it 'creates a new ProjectMember that is not a team member', :aggregate_failures do - post :request_access, params: { - namespace_id: project.namespace, - project_id: project - } - - expect(controller).to set_flash.to 'Your request for access has been queued for review.' - expect(response).to redirect_to( - project_path(project) - ) - expect(project.requesters.exists?(user_id: user)).to be_truthy - expect(project.users).not_to include user - end - end + describe 'POST request_access' do + before do + sign_in(user) + end - describe 'POST approve' do - let_it_be(:member) { create(:project_member, :access_request, project: project) } + it 'creates a new ProjectMember that is not a team member', :aggregate_failures do + post :request_access, params: { + namespace_id: project.namespace, + project_id: project + } - before do - sign_in(user) + expect(controller).to set_flash.to 'Your request for access has been queued for review.' + expect(response).to redirect_to( + project_path(project) + ) + expect(project.requesters.exists?(user_id: user)).to be_truthy + expect(project.users).not_to include user + end end - context 'when member is not found' do - it 'returns 404' do - post :approve_access_request, params: { - namespace_id: project.namespace, - project_id: project, - id: 42 - } + describe 'POST approve' do + let_it_be(:member) { create(:project_member, :access_request, project: project) } - expect(response).to have_gitlab_http_status(:not_found) + before do + sign_in(user) end - end - - context 'when member is found' do - context 'when user does not have rights to manage other members' do - before do - project.add_developer(user) - end - it 'returns 404', :aggregate_failures do + context 'when member is not found' do + it 'returns 404' do post :approve_access_request, params: { namespace_id: project.namespace, project_id: project, - id: member + id: 42 } expect(response).to have_gitlab_http_status(:not_found) - expect(project.members).not_to include member end end - context 'when user has enough rights' do - before do - project.add_maintainer(user) + context 'when member is found' do + context 'when user does not have rights to manage other members' do + before do + project.add_developer(user) + end + + it 'returns 404', :aggregate_failures do + post :approve_access_request, params: { + namespace_id: project.namespace, + project_id: project, + id: member + } + + expect(response).to have_gitlab_http_status(:not_found) + expect(project.members).not_to include member + end end - it 'adds user to members', :aggregate_failures do - post :approve_access_request, params: { - namespace_id: project.namespace, - project_id: project, - id: member - } + context 'when user has enough rights' do + before do + project.add_maintainer(user) + end - expect(response).to redirect_to( - project_project_members_path(project) - ) - expect(project.members).to include member + it 'adds user to members', :aggregate_failures do + post :approve_access_request, params: { + namespace_id: project.namespace, + project_id: project, + id: member + } + + expect(response).to redirect_to( + project_project_members_path(project) + ) + expect(project.members).to include member + end end end end - end - describe 'POST resend_invite' do - let_it_be(:member) { create(:project_member, project: project) } + describe 'POST resend_invite' do + let_it_be(:member) { create(:project_member, project: project) } - before do - project.add_maintainer(user) - sign_in(user) + before do + project.add_maintainer(user) + sign_in(user) + end + + it 'is successful' do + post :resend_invite, params: { namespace_id: project.namespace, project_id: project, id: member } + + expect(response).to have_gitlab_http_status(:found) + end end + end - it 'is successful' do - post :resend_invite, params: { namespace_id: project.namespace, project_id: project, id: member } + it_behaves_like 'controller actions' - expect(response).to have_gitlab_http_status(:found) + context 'when project_members_index_by_project_namespace feature flag is disabled' do + before do + stub_feature_flags(project_members_index_by_project_namespace: false) end + + it_behaves_like 'controller actions' end end diff --git a/spec/features/projects/settings/user_manages_project_members_spec.rb b/spec/features/projects/settings/user_manages_project_members_spec.rb index fac4d5a99a5..159a83a261d 100644 --- a/spec/features/projects/settings/user_manages_project_members_spec.rb +++ b/spec/features/projects/settings/user_manages_project_members_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe 'Projects > Settings > User manages project members', feature_category: :projects do include Spec::Support::Helpers::Features::MembersHelpers include Spec::Support::Helpers::ModalHelpers + include ListboxHelpers let(:group) { create(:group, name: 'OpenSource') } let(:project) { create(:project, :with_namespace_settings) } @@ -46,7 +47,7 @@ RSpec.describe 'Projects > Settings > User manages project members', feature_cat click_on 'Select a project' wait_for_requests - click_button project2.name + select_listbox_item(project2.name_with_namespace) click_button 'Import project members' wait_for_requests diff --git a/spec/frontend/invite_members/components/project_select_spec.js b/spec/frontend/invite_members/components/project_select_spec.js index acc062b5fff..6fbf95362fa 100644 --- a/spec/frontend/invite_members/components/project_select_spec.js +++ b/spec/frontend/invite_members/components/project_select_spec.js @@ -1,4 +1,4 @@ -import { GlSearchBoxByType, GlAvatarLabeled, GlDropdownItem } from '@gitlab/ui'; +import { GlAvatarLabeled, GlCollapsibleListbox } from '@gitlab/ui'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import * as projectsApi from '~/api/projects_api'; @@ -9,7 +9,12 @@ describe('ProjectSelect', () => { let wrapper; const createComponent = () => { - wrapper = shallowMountExtended(ProjectSelect, {}); + wrapper = shallowMountExtended(ProjectSelect, { + stubs: { + GlCollapsibleListbox, + GlAvatarLabeled, + }, + }); }; beforeEach(() => { @@ -22,16 +27,24 @@ describe('ProjectSelect', () => { wrapper.destroy(); }); - const findSearchBoxByType = () => wrapper.findComponent(GlSearchBoxByType); - const findDropdownItem = (index) => wrapper.findAllComponents(GlDropdownItem).at(index); - const findAvatarLabeled = (index) => findDropdownItem(index).findComponent(GlAvatarLabeled); - const findEmptyResultMessage = () => wrapper.findByTestId('empty-result-message'); - const findErrorMessage = () => wrapper.findByTestId('error-message'); - - it('renders GlSearchBoxByType with default attributes', () => { - expect(findSearchBoxByType().exists()).toBe(true); - expect(findSearchBoxByType().vm.$attrs).toMatchObject({ - placeholder: 'Search projects', + const findGlCollapsibleListbox = () => wrapper.findComponent(GlCollapsibleListbox); + const findAvatarLabeled = (index) => wrapper.findAllComponents(GlAvatarLabeled).at(index); + + it('renders GlCollapsibleListbox with default props', () => { + expect(findGlCollapsibleListbox().exists()).toBe(true); + expect(findGlCollapsibleListbox().props()).toMatchObject({ + items: [], + loading: false, + multiple: false, + noResultsText: 'No matching results', + placement: 'left', + searchPlaceholder: 'Search projects', + searchable: true, + searching: false, + size: 'medium', + toggleText: 'Select a project', + totalItems: null, + variant: 'default', }); }); @@ -48,7 +61,7 @@ describe('ProjectSelect', () => { }), ); - findSearchBoxByType().vm.$emit('input', project1.name); + findGlCollapsibleListbox().vm.$emit('search', project1.name); }); it('calls the API', () => { @@ -61,14 +74,12 @@ describe('ProjectSelect', () => { }); it('displays loading icon while waiting for API call to resolve and then sets loading false', async () => { - expect(findSearchBoxByType().props('isLoading')).toBe(true); + expect(findGlCollapsibleListbox().props('searching')).toBe(true); resolveApiRequest({ data: allProjects }); await waitForPromises(); - expect(findSearchBoxByType().props('isLoading')).toBe(false); - expect(findEmptyResultMessage().exists()).toBe(false); - expect(findErrorMessage().exists()).toBe(false); + expect(findGlCollapsibleListbox().props('searching')).toBe(false); }); it('displays a dropdown item and avatar for each project fetched', async () => { @@ -76,11 +87,11 @@ describe('ProjectSelect', () => { await waitForPromises(); allProjects.forEach((project, index) => { - expect(findDropdownItem(index).attributes('name')).toBe(project.name_with_namespace); expect(findAvatarLabeled(index).attributes()).toMatchObject({ src: project.avatar_url, 'entity-id': String(project.id), 'entity-name': project.name_with_namespace, + size: '32', }); expect(findAvatarLabeled(index).props('label')).toBe(project.name_with_namespace); }); @@ -90,16 +101,17 @@ describe('ProjectSelect', () => { resolveApiRequest({ data: [] }); await waitForPromises(); - expect(findEmptyResultMessage().text()).toBe('No matching results'); + expect(findGlCollapsibleListbox().text()).toBe('No matching results'); }); it('displays the error message when the fetch fails', async () => { rejectApiRequest(); await waitForPromises(); - expect(findErrorMessage().text()).toBe( - 'There was an error fetching the projects. Please try again.', - ); + // To be displayed in GlCollapsibleListbox once we implement + // https://gitlab.com/gitlab-org/gitlab-ui/-/issues/2132 + // https://gitlab.com/gitlab-org/gitlab/-/issues/389974 + expect(findGlCollapsibleListbox().text()).toBe('No matching results'); }); }); }); diff --git a/spec/frontend/invite_members/mock_data/api_response_data.js b/spec/frontend/invite_members/mock_data/api_response_data.js index 9509422b603..4ab9026c531 100644 --- a/spec/frontend/invite_members/mock_data/api_response_data.js +++ b/spec/frontend/invite_members/mock_data/api_response_data.js @@ -6,7 +6,7 @@ export const project1 = { }; export const project2 = { id: 2, - name: 'Project One', + name: 'Project Two', name_with_namespace: 'Project Two', avatar_url: 'test2', }; diff --git a/spec/graphql/types/notes/deleted_note_type_spec.rb b/spec/graphql/types/notes/deleted_note_type_spec.rb new file mode 100644 index 00000000000..70985484e75 --- /dev/null +++ b/spec/graphql/types/notes/deleted_note_type_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GitlabSchema.types['DeletedNote'], feature_category: :team_planning do + it 'exposes the expected fields' do + expected_fields = %i[ + id + discussion_id + last_discussion_note + ] + + expect(described_class).to have_graphql_fields(*expected_fields).only + end +end diff --git a/spec/lib/gitlab/ci/artifacts/logger_spec.rb b/spec/lib/gitlab/ci/artifacts/logger_spec.rb index 09d8a549640..7a2f8b6ea37 100644 --- a/spec/lib/gitlab/ci/artifacts/logger_spec.rb +++ b/spec/lib/gitlab/ci/artifacts/logger_spec.rb @@ -19,7 +19,7 @@ RSpec.describe Gitlab::Ci::Artifacts::Logger do message: 'Artifact created', job_artifact_id: artifact.id, size: artifact.size, - type: artifact.file_type, + file_type: artifact.file_type, build_id: artifact.job_id, project_id: artifact.project_id, 'correlation_id' => an_instance_of(String), @@ -47,7 +47,7 @@ RSpec.describe Gitlab::Ci::Artifacts::Logger do job_artifact_id: artifact.id, expire_at: artifact.expire_at, size: artifact.size, - type: artifact.file_type, + file_type: artifact.file_type, build_id: artifact.job_id, project_id: artifact.project_id, method: method, diff --git a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb index 12886c79d7d..5fbaae58a73 100644 --- a/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb +++ b/spec/lib/gitlab/ci/parsers/security/validators/schema_validator_spec.rb @@ -567,6 +567,28 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Validators::SchemaValidator, featu end it { is_expected.to match_array([message]) } + + context 'without license', unless: Gitlab.ee? do + let(:schema_path) { Rails.root.join(*%w[lib gitlab ci parsers security validators schemas]) } + + it 'tries to validate against the latest patch version available' do + expect(File).to receive(:file?).with("#{schema_path}/#{report_version}/#{report_type}-report-format.json") + expect(File).to receive(:file?).with("#{schema_path}/#{latest_patch_version}/#{report_type}-report-format.json") + + subject + end + end + + context 'with license', if: Gitlab.ee? do + let(:schema_path) { Rails.root.join(*%w[ee lib ee gitlab ci parsers security validators schemas]) } + + it 'tries to validate against the latest patch version available' do + expect(File).to receive(:file?).with("#{schema_path}/#{report_version}/#{report_type}-report-format.json") + expect(File).to receive(:file?).with("#{schema_path}/#{latest_patch_version}/#{report_type}-report-format.json") + + subject + end + end end context 'and the report is invalid' do diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 32746356803..0c2c3ffc664 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -487,6 +487,7 @@ project: - requesters - namespace_members - namespace_requesters +- namespace_members_and_requesters - deploy_keys_projects - deploy_keys - users_star_projects diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 384d3d5c82e..c51e098556c 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -17,6 +17,7 @@ RSpec.describe Group, feature_category: :subgroups do it { is_expected.to have_many(:requesters).dependent(:destroy) } it { is_expected.to have_many(:namespace_requesters) } it { is_expected.to have_many(:members_and_requesters) } + it { is_expected.to have_many(:namespace_members_and_requesters) } it { is_expected.to have_many(:project_group_links).dependent(:destroy) } it { is_expected.to have_many(:shared_projects).through(:project_group_links) } it { is_expected.to have_many(:notification_settings).dependent(:destroy) } @@ -93,6 +94,34 @@ RSpec.describe Group, feature_category: :subgroups do end end + describe '#namespace_members_and_requesters' do + let_it_be(:group) { create(:group) } + let_it_be(:requester) { create(:user) } + let_it_be(:developer) { create(:user) } + let_it_be(:invited_member) { create(:group_member, :invited, :owner, group: group) } + + before do + group.request_access(requester) + group.add_developer(developer) + end + + it 'includes the correct users' do + expect(group.namespace_members_and_requesters).to include( + Member.find_by(user: requester), + Member.find_by(user: developer), + Member.find(invited_member.id) + ) + end + + it 'is equivalent to #members_and_requesters' do + expect(group.namespace_members_and_requesters).to match_array group.members_and_requesters + end + + it_behaves_like 'query without source filters' do + subject { group.namespace_members_and_requesters } + end + end + shared_examples 'polymorphic membership relationship' do it do expect(membership.attributes).to include( @@ -139,6 +168,24 @@ RSpec.describe Group, feature_category: :subgroups do it_behaves_like 'member_namespace membership relationship' end + describe '#namespace_members_and_requesters setters' do + let(:requested_at) { Time.current } + let(:user) { create(:user) } + let(:membership) do + group.namespace_members_and_requesters.create!( + user: user, requested_at: requested_at, access_level: Gitlab::Access::DEVELOPER + ) + end + + it { expect(membership).to be_instance_of(GroupMember) } + it { expect(membership.user).to eq user } + it { expect(membership.group).to eq group } + it { expect(membership.requested_at).to eq requested_at } + + it_behaves_like 'polymorphic membership relationship' + it_behaves_like 'member_namespace membership relationship' + end + describe '#members & #requesters' do let_it_be(:requester) { create(:user) } let_it_be(:developer) { create(:user) } diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index aa284f34c2f..013070f7be5 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1482,6 +1482,7 @@ RSpec.describe Note do end it "expires cache for note's issue when note is destroyed" do + note.save! expect_expiration(note.noteable) note.destroy! diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1c71c3d209a..4988ba3b70b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -112,6 +112,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :projects do it { is_expected.to have_many(:uploads) } it { is_expected.to have_many(:pipeline_schedules) } it { is_expected.to have_many(:members_and_requesters) } + it { is_expected.to have_many(:namespace_members_and_requesters) } it { is_expected.to have_many(:clusters) } it { is_expected.to have_many(:management_clusters).class_name('Clusters::Cluster') } it { is_expected.to have_many(:kubernetes_namespaces) } @@ -402,6 +403,34 @@ RSpec.describe Project, factory_default: :keep, feature_category: :projects do end end + describe '#namespace_members_and_requesters' do + let_it_be(:project) { create(:project, :public) } + let_it_be(:requester) { create(:user) } + let_it_be(:developer) { create(:user) } + let_it_be(:invited_member) { create(:project_member, :invited, :owner, project: project) } + + before_all do + project.request_access(requester) + project.add_developer(developer) + end + + it 'includes the correct users' do + expect(project.namespace_members_and_requesters).to include( + Member.find_by(user: requester), + Member.find_by(user: developer), + Member.find(invited_member.id) + ) + end + + it 'is equivalent to #project_members' do + expect(project.namespace_members_and_requesters).to match_array(project.members_and_requesters) + end + + it_behaves_like 'query without source filters' do + subject { project.namespace_members_and_requesters } + end + end + shared_examples 'polymorphic membership relationship' do it do expect(membership.attributes).to include( @@ -450,6 +479,25 @@ RSpec.describe Project, factory_default: :keep, feature_category: :projects do it_behaves_like 'member_namespace membership relationship' end + describe '#namespace_members_and_requesters setters' do + let_it_be(:requested_at) { Time.current } + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be(:membership) do + project.namespace_members_and_requesters.create!( + user: user, requested_at: requested_at, access_level: Gitlab::Access::DEVELOPER + ) + end + + it { expect(membership).to be_instance_of(ProjectMember) } + it { expect(membership.user).to eq user } + it { expect(membership.project).to eq project } + it { expect(membership.requested_at).to eq requested_at } + + it_behaves_like 'polymorphic membership relationship' + it_behaves_like 'member_namespace membership relationship' + end + describe '#members & #requesters' do let_it_be(:project) { create(:project, :public) } let_it_be(:requester) { create(:user) } diff --git a/spec/requests/api/graphql/subscriptions/notes/created_spec.rb b/spec/requests/api/graphql/subscriptions/notes/created_spec.rb new file mode 100644 index 00000000000..7161b17d0a8 --- /dev/null +++ b/spec/requests/api/graphql/subscriptions/notes/created_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe 'Subscriptions::Notes::Created', feature_category: :team_planning do + include GraphqlHelpers + include Graphql::Subscriptions::Notes::Helper + + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:task) { create(:work_item, :task, project: project) } + + let(:current_user) { nil } + let(:subscribe) { notes_subscription('workItemNoteCreated', task, current_user) } + let(:response_note) { graphql_dig_at(graphql_data(response[:result]), :workItemNoteCreated) } + let(:discussion) { graphql_dig_at(response_note, :discussion) } + let(:discussion_notes) { graphql_dig_at(discussion, :notes, :nodes) } + + before do + stub_const('GitlabSchema', Graphql::Subscriptions::ActionCable::MockGitlabSchema) + Graphql::Subscriptions::ActionCable::MockActionCable.clear_mocks + project.add_guest(guest) + project.add_reporter(reporter) + end + + subject(:response) do + subscription_response do + # this creates note defined with let lazily and triggers the subscription event + new_note + end + end + + context 'when user is unauthorized' do + let(:new_note) { create(:note, noteable: task, project: project, type: 'DiscussionNote') } + + it 'does not receive any data' do + expect(response).to be_nil + end + end + + context 'when user is authorized' do + let(:current_user) { guest } + let(:new_note) { create(:note, noteable: task, project: project, type: 'DiscussionNote') } + + it 'receives created note' do + response + note = Note.find(new_note.id) + + expect(response_note['id']).to eq(note.to_gid.to_s) + expect(discussion['id']).to eq(note.discussion.to_gid.to_s) + expect(discussion_notes.pluck('id')).to eq([note.to_gid.to_s]) + end + + context 'when a new note is created as a reply' do + let_it_be(:note, refind: true) { create(:note, noteable: task, project: project, type: 'DiscussionNote') } + + let(:new_note) do + create(:note, noteable: task, project: project, in_reply_to: note, discussion_id: note.discussion_id) + end + + it 'receives created note' do + response + reply = Note.find(new_note.id) + + expect(response_note['id']).to eq(reply.to_gid.to_s) + expect(discussion['id']).to eq(reply.discussion.to_gid.to_s) + expect(discussion_notes.pluck('id')).to eq([note.to_gid.to_s, reply.to_gid.to_s]) + end + end + + context 'when note is confidential' do + let(:current_user) { reporter } + let(:new_note) { create(:note, :confidential, noteable: task, project: project, type: 'DiscussionNote') } + + context 'and user has permission to read confidential notes' do + it 'receives created note' do + response + confidential_note = Note.find(new_note.id) + + expect(response_note['id']).to eq(confidential_note.to_gid.to_s) + expect(discussion['id']).to eq(confidential_note.discussion.to_gid.to_s) + expect(discussion_notes.pluck('id')).to eq([confidential_note.to_gid.to_s]) + end + + context 'and replying' do + let_it_be(:note, refind: true) do + create(:note, :confidential, noteable: task, project: project, type: 'DiscussionNote') + end + + let(:new_note) do + create(:note, :confidential, + noteable: task, project: project, in_reply_to: note, discussion_id: note.discussion_id) + end + + it 'receives created note' do + response + reply = Note.find(new_note.id) + + expect(response_note['id']).to eq(reply.to_gid.to_s) + expect(discussion['id']).to eq(reply.discussion.to_gid.to_s) + expect(discussion_notes.pluck('id')).to eq([note.to_gid.to_s, reply.to_gid.to_s]) + end + end + end + + context 'and user does not have permission to read confidential notes' do + let(:current_user) { guest } + let(:new_note) { create(:note, :confidential, noteable: task, project: project, type: 'DiscussionNote') } + + it 'does not receive note data' do + response + expect(response_note).to be_nil + end + end + end + end +end diff --git a/spec/requests/api/graphql/subscriptions/notes/deleted_spec.rb b/spec/requests/api/graphql/subscriptions/notes/deleted_spec.rb new file mode 100644 index 00000000000..d98f1cfe77e --- /dev/null +++ b/spec/requests/api/graphql/subscriptions/notes/deleted_spec.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe 'Subscriptions::Notes::Deleted', feature_category: :team_planning do + include GraphqlHelpers + include Graphql::Subscriptions::Notes::Helper + + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:task) { create(:work_item, :task, project: project) } + let_it_be(:note, refind: true) { create(:note, noteable: task, project: project, type: 'DiscussionNote') } + let_it_be(:reply_note, refind: true) do + create(:note, noteable: task, project: project, in_reply_to: note, discussion_id: note.discussion_id) + end + + let(:current_user) { nil } + let(:subscribe) { notes_subscription('workItemNoteDeleted', task, current_user) } + let(:deleted_note) { graphql_dig_at(graphql_data(response[:result]), :workItemNoteDeleted) } + + before do + stub_const('GitlabSchema', Graphql::Subscriptions::ActionCable::MockGitlabSchema) + Graphql::Subscriptions::ActionCable::MockActionCable.clear_mocks + project.add_guest(guest) + project.add_reporter(reporter) + end + + subject(:response) do + subscription_response do + note.destroy! + end + end + + context 'when user is unauthorized' do + it 'does not receive any data' do + expect(response).to be_nil + end + end + + context 'when user is authorized' do + let(:current_user) { guest } + + it 'receives note id that is removed' do + expect(deleted_note['id']).to eq(note.to_gid.to_s) + expect(deleted_note['discussionId']).to eq(note.discussion.to_gid.to_s) + expect(deleted_note['lastDiscussionNote']).to be false + end + + context 'when last discussion note is deleted' do + let_it_be(:note, refind: true) { create(:note, noteable: task, project: project, type: 'DiscussionNote') } + + it 'receives note id that is removed' do + expect(deleted_note['id']).to eq(note.to_gid.to_s) + expect(deleted_note['discussionId']).to eq(note.discussion.to_gid.to_s) + expect(deleted_note['lastDiscussionNote']).to be true + end + end + + context 'when note is confidential' do + let_it_be(:note, refind: true) do + create(:note, :confidential, noteable: task, project: project, type: 'DiscussionNote') + end + + it 'receives note id that is removed' do + expect(deleted_note['id']).to eq(note.to_gid.to_s) + expect(deleted_note['discussionId']).to eq(note.discussion.to_gid.to_s) + expect(deleted_note['lastDiscussionNote']).to be true + end + end + end +end diff --git a/spec/requests/api/graphql/subscriptions/notes/updated_spec.rb b/spec/requests/api/graphql/subscriptions/notes/updated_spec.rb new file mode 100644 index 00000000000..25c0a79e7aa --- /dev/null +++ b/spec/requests/api/graphql/subscriptions/notes/updated_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe 'Subscriptions::Notes::Updated', feature_category: :team_planning do + include GraphqlHelpers + include Graphql::Subscriptions::Notes::Helper + + let_it_be(:guest) { create(:user) } + let_it_be(:reporter) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:task) { create(:work_item, :task, project: project) } + let_it_be(:note, refind: true) { create(:note, noteable: task, project: task.project, type: 'DiscussionNote') } + + let(:current_user) { nil } + let(:subscribe) { note_subscription('workItemNoteUpdated', task, current_user) } + let(:updated_note) { graphql_dig_at(graphql_data(response[:result]), :workItemNoteUpdated) } + + before do + stub_const('GitlabSchema', Graphql::Subscriptions::ActionCable::MockGitlabSchema) + Graphql::Subscriptions::ActionCable::MockActionCable.clear_mocks + project.add_guest(guest) + project.add_reporter(reporter) + end + + subject(:response) do + subscription_response do + note.update!(note: 'changing the note body') + end + end + + context 'when user is unauthorized' do + it 'does not receive any data' do + expect(response).to be_nil + end + end + + context 'when user is authorized' do + let(:current_user) { reporter } + + it 'receives updated note data' do + expect(updated_note['id']).to eq(note.to_gid.to_s) + expect(updated_note['body']).to eq('changing the note body') + end + + context 'when note is confidential' do + let_it_be(:note, refind: true) do + create(:note, :confidential, noteable: task, project: task.project, type: 'DiscussionNote') + end + + context 'and user has permission to read confidential notes' do + it 'receives updated note data' do + expect(updated_note['id']).to eq(note.to_gid.to_s) + expect(updated_note['body']).to eq('changing the note body') + end + end + + context 'and user does not have permission to read confidential notes' do + let(:current_user) { guest } + + it 'does not receive updated note data' do + expect(updated_note).to be_nil + end + end + end + end +end diff --git a/spec/serializers/import/github_realtime_repo_entity_spec.rb b/spec/serializers/import/github_realtime_repo_entity_spec.rb new file mode 100644 index 00000000000..7f137366be2 --- /dev/null +++ b/spec/serializers/import/github_realtime_repo_entity_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Import::GithubRealtimeRepoEntity, feature_category: :importers do + subject(:entity) { described_class.new(project) } + + let(:import_state) { instance_double(ProjectImportState, failed?: false, in_progress?: true) } + let(:import_failures) { [instance_double(ImportFailure, exception_message: 'test error')] } + let(:project) do + instance_double( + Project, + id: 100500, + import_status: 'importing', + import_state: import_state, + import_failures: import_failures, + import_checksums: {} + ) + end + + it 'exposes correct attributes' do + data = entity.as_json + + expect(data.keys).to contain_exactly(:id, :import_status, :stats) + expect(data[:id]).to eq project.id + expect(data[:import_status]).to eq project.import_status + end + + context 'when import stats is failed' do + let(:import_state) { instance_double(ProjectImportState, failed?: true, in_progress?: false) } + + it 'includes import_error' do + data = entity.as_json + + expect(data.keys).to contain_exactly(:id, :import_status, :stats, :import_error) + expect(data[:import_error]).to eq 'test error' + end + end +end diff --git a/spec/serializers/import/github_realtime_repo_serializer_spec.rb b/spec/serializers/import/github_realtime_repo_serializer_spec.rb new file mode 100644 index 00000000000..b656132e332 --- /dev/null +++ b/spec/serializers/import/github_realtime_repo_serializer_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Import::GithubRealtimeRepoSerializer, feature_category: :importers do + subject(:serializer) { described_class.new } + + it '.entity_class' do + expect(described_class.entity_class).to eq(Import::GithubRealtimeRepoEntity) + end + + describe '#represent' do + let(:import_state) { instance_double(ProjectImportState, failed?: false, in_progress?: true) } + let(:project) do + instance_double( + Project, + id: 100500, + import_status: 'importing', + import_state: import_state + ) + end + + let(:expected_data) do + { + id: project.id, + import_status: 'importing', + stats: { fetched: {}, imported: {} } + }.deep_stringify_keys + end + + context 'when a single object is being serialized' do + let(:resource) { project } + + it 'serializes organization object' do + expect(serializer.represent(resource).as_json).to eq expected_data + end + end + + context 'when multiple objects are being serialized' do + let(:count) { 3 } + let(:resource) { Array.new(count, project) } + + it 'serializes array of organizations' do + expect(serializer.represent(resource).as_json).to all(eq(expected_data)) + end + end + end +end diff --git a/spec/serializers/project_import_entity_spec.rb b/spec/serializers/project_import_entity_spec.rb index 94af9f1cbd8..6d292d18ae7 100644 --- a/spec/serializers/project_import_entity_spec.rb +++ b/spec/serializers/project_import_entity_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ProjectImportEntity do +RSpec.describe ProjectImportEntity, feature_category: :importers do include ImportHelper let_it_be(:project) { create(:project, import_status: :started, import_source: 'namespace/project') } @@ -10,6 +10,10 @@ RSpec.describe ProjectImportEntity do let(:provider_url) { 'https://provider.com' } let(:entity) { described_class.represent(project, provider_url: provider_url) } + before do + create(:import_failure, project: project) + end + describe '#as_json' do subject { entity.as_json } @@ -18,6 +22,19 @@ RSpec.describe ProjectImportEntity do expect(subject[:import_status]).to eq(project.import_status) expect(subject[:human_import_status_name]).to eq(project.human_import_status_name) expect(subject[:provider_link]).to eq(provider_project_link_url(provider_url, project[:import_source])) + expect(subject[:import_error]).to eq(nil) + end + + context 'when import is failed' do + let!(:last_import_failure) { create(:import_failure, project: project, exception_message: 'LAST ERROR') } + + before do + project.import_state.fail_op! + end + + it 'includes only the last import failure' do + expect(subject[:import_error]).to eq(last_import_failure.exception_message) + end end end end diff --git a/spec/support/graphql/subscriptions/action_cable/mock_action_cable.rb b/spec/support/graphql/subscriptions/action_cable/mock_action_cable.rb new file mode 100644 index 00000000000..5467564a79e --- /dev/null +++ b/spec/support/graphql/subscriptions/action_cable/mock_action_cable.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +# A stub implementation of ActionCable. +# Any methods to support the mock backend have `mock` in the name. +module Graphql + module Subscriptions + module ActionCable + class MockActionCable + class MockChannel + def initialize + @mock_broadcasted_messages = [] + end + + attr_reader :mock_broadcasted_messages + + def stream_from(stream_name, coder: nil, &block) + # Rails uses `coder`, we don't + block ||= ->(msg) { @mock_broadcasted_messages << msg } + MockActionCable.mock_stream_for(stream_name).add_mock_channel(self, block) + end + end + + class MockStream + def initialize + @mock_channels = {} + end + + def add_mock_channel(channel, handler) + @mock_channels[channel] = handler + end + + def mock_broadcast(message) + @mock_channels.each do |channel, handler| + handler && handler.call(message) + end + end + end + + class << self + def clear_mocks + @mock_streams = {} + end + + def server + self + end + + def broadcast(stream_name, message) + stream = @mock_streams[stream_name] + stream && stream.mock_broadcast(message) + end + + def mock_stream_for(stream_name) + @mock_streams[stream_name] ||= MockStream.new + end + + def get_mock_channel + MockChannel.new + end + + def mock_stream_names + @mock_streams.keys + end + end + end + + class MockSchema < GraphQL::Schema + class << self + def find_by_gid(gid) + return unless gid + + if gid.model_class < ApplicationRecord + Gitlab::Graphql::Loaders::BatchModelLoader.new(gid.model_class, gid.model_id).find + elsif gid.model_class.respond_to?(:lazy_find) + gid.model_class.lazy_find(gid.model_id) + else + gid.find + end + end + + def id_from_object(object, _type = nil, _ctx = nil) + unless object.respond_to?(:to_global_id) + # This is an error in our schema and needs to be solved. So raise a + # more meaningful error message + raise "#{object} does not implement `to_global_id`. " \ + "Include `GlobalID::Identification` into `#{object.class}" + end + + object.to_global_id + end + end + + query(::Types::QueryType) + subscription(::Types::SubscriptionType) + + use GraphQL::Subscriptions::ActionCableSubscriptions, action_cable: MockActionCable, action_cable_coder: JSON + end + end + end +end diff --git a/spec/support/graphql/subscriptions/action_cable/mock_gitlab_schema.rb b/spec/support/graphql/subscriptions/action_cable/mock_gitlab_schema.rb new file mode 100644 index 00000000000..cd5d78cc78b --- /dev/null +++ b/spec/support/graphql/subscriptions/action_cable/mock_gitlab_schema.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +# A stub implementation of ActionCable. +# Any methods to support the mock backend have `mock` in the name. +module Graphql + module Subscriptions + module ActionCable + class MockGitlabSchema < GraphQL::Schema + class << self + def find_by_gid(gid) + return unless gid + + if gid.model_class < ApplicationRecord + Gitlab::Graphql::Loaders::BatchModelLoader.new(gid.model_class, gid.model_id).find + elsif gid.model_class.respond_to?(:lazy_find) + gid.model_class.lazy_find(gid.model_id) + else + gid.find + end + end + + def id_from_object(object, _type = nil, _ctx = nil) + unless object.respond_to?(:to_global_id) + # This is an error in our schema and needs to be solved. So raise a + # more meaningful error message + raise "#{object} does not implement `to_global_id`. " \ + "Include `GlobalID::Identification` into `#{object.class}" + end + + object.to_global_id + end + end + + query(::Types::QueryType) + subscription(::Types::SubscriptionType) + + use GraphQL::Subscriptions::ActionCableSubscriptions, action_cable: MockActionCable, action_cable_coder: JSON + end + end + end +end diff --git a/spec/support/graphql/subscriptions/notes/helper.rb b/spec/support/graphql/subscriptions/notes/helper.rb new file mode 100644 index 00000000000..9a552f9879e --- /dev/null +++ b/spec/support/graphql/subscriptions/notes/helper.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module Graphql + module Subscriptions + module Notes + module Helper + def subscription_response + subscription_channel = subscribe + yield + subscription_channel.mock_broadcasted_messages.first + end + + def notes_subscription(name, noteable, current_user) + mock_channel = Graphql::Subscriptions::ActionCable::MockActionCable.get_mock_channel + + query = case name + when 'workItemNoteDeleted' + note_deleted_subscription_query(name, noteable) + when 'workItemNoteUpdated' + note_updated_subscription_query(name, noteable) + when 'workItemNoteCreated' + note_created_subscription_query(name, noteable) + else + raise "Subscription query unknown: #{name}" + end + + GitlabSchema.execute(query, context: { current_user: current_user, channel: mock_channel }) + + mock_channel + end + + def note_subscription(name, noteable, current_user) + mock_channel = Graphql::Subscriptions::ActionCable::MockActionCable.get_mock_channel + + query = <<~SUBSCRIPTION + subscription { + #{name}(noteableId: \"#{noteable.to_gid}\") { + id + body + } + } + SUBSCRIPTION + + GitlabSchema.execute(query, context: { current_user: current_user, channel: mock_channel }) + + mock_channel + end + + private + + def note_deleted_subscription_query(name, noteable) + <<~SUBSCRIPTION + subscription { + #{name}(noteableId: \"#{noteable.to_gid}\") { + id + discussionId + lastDiscussionNote + } + } + SUBSCRIPTION + end + + def note_created_subscription_query(name, noteable) + <<~SUBSCRIPTION + subscription { + #{name}(noteableId: \"#{noteable.to_gid}\") { + id + discussion { + id + notes { + nodes { + id + } + } + } + } + } + SUBSCRIPTION + end + + def note_updated_subscription_query(name, noteable) + <<~SUBSCRIPTION + subscription { + #{name}(noteableId: \"#{noteable.to_gid}\") { + id + body + } + } + SUBSCRIPTION + end + end + end + end +end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 240f9cac103..c43dd88e3fe 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -165,7 +165,6 @@ - './ee/spec/controllers/projects/vulnerability_feedback_controller_spec.rb' - './ee/spec/controllers/registrations/company_controller_spec.rb' - './ee/spec/controllers/registrations/groups_projects_controller_spec.rb' -- './ee/spec/controllers/registrations/verification_controller_spec.rb' - './ee/spec/controllers/repositories/git_http_controller_spec.rb' - './ee/spec/controllers/security/dashboard_controller_spec.rb' - './ee/spec/controllers/security/projects_controller_spec.rb' diff --git a/spec/support/shared_examples/graphql/subscriptions/notes/notes_subscription_shared_examples.rb b/spec/support/shared_examples/graphql/subscriptions/notes/notes_subscription_shared_examples.rb new file mode 100644 index 00000000000..949eb4fb643 --- /dev/null +++ b/spec/support/shared_examples/graphql/subscriptions/notes/notes_subscription_shared_examples.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'graphql notes subscriptions' do + describe '#resolve' do + let_it_be(:unauthorized_user) { create(:user) } + let_it_be(:work_item) { create(:work_item, :task) } + let_it_be(:note) { create(:note, noteable: work_item, project: work_item.project) } + let_it_be(:current_user) { work_item.author } + let_it_be(:noteable_id) { work_item.to_gid } + + subject { resolver.resolve_with_support(noteable_id: noteable_id) } + + context 'on initial subscription' do + let(:resolver) do + resolver_instance(described_class, ctx: { current_user: current_user }, subscription_update: false) + end + + it 'returns nil' do + expect(subject).to eq(nil) + end + + context 'when user is unauthorized' do + let(:current_user) { unauthorized_user } + + it 'raises an exception' do + expect { subject }.to raise_error(GraphQL::ExecutionError) + end + end + + context 'when work_item does not exist' do + let(:noteable_id) { GlobalID.parse("gid://gitlab/WorkItem/#{non_existing_record_id}") } + + it 'raises an exception' do + expect { subject }.to raise_error(GraphQL::ExecutionError) + end + end + end + + context 'on subscription updates' do + let(:resolver) do + resolver_instance(described_class, obj: note, ctx: { current_user: current_user }, subscription_update: true) + end + + it 'returns the resolved object' do + expect(subject).to eq(note) + end + + context 'when user is unauthorized' do + let(:current_user) { unauthorized_user } + + it 'unsubscribes the user' do + # GraphQL::Execution::Execute::Skip is returned when unsubscribed + expect(subject).to be_an(GraphQL::Execution::Execute::Skip) + end + end + end + end +end diff --git a/spec/tasks/gitlab/db/lock_writes_rake_spec.rb b/spec/tasks/gitlab/db/lock_writes_rake_spec.rb index 24a2c0a48f7..9d54241aa7f 100644 --- a/spec/tasks/gitlab/db/lock_writes_rake_spec.rb +++ b/spec/tasks/gitlab/db/lock_writes_rake_spec.rb @@ -14,7 +14,9 @@ RSpec.describe 'gitlab:db:lock_writes', :reestablished_active_record_base, featu end let(:table_locker) { instance_double(Gitlab::Database::TablesLocker) } - let(:logger) { instance_double(Logger) } + let(:logger) { instance_double(Logger, level: nil) } + let(:dry_run) { false } + let(:verbose) { false } before do allow(Logger).to receive(:new).with($stdout).and_return(logger) @@ -24,7 +26,10 @@ RSpec.describe 'gitlab:db:lock_writes', :reestablished_active_record_base, featu end shared_examples "call table locker" do |method| + let(:log_level) { verbose ? Logger::INFO : Logger::WARN } + it "creates TablesLocker with dry run set and calls #{method}" do + expect(logger).to receive(:level=).with(log_level) expect(table_locker).to receive(method) run_rake_task("gitlab:db:#{method}") @@ -57,6 +62,30 @@ RSpec.describe 'gitlab:db:lock_writes', :reestablished_active_record_base, featu include_examples "call table locker", :lock_writes end + + context 'when environment sets VERBOSE to true' do + let(:verbose) { true } + + before do + stub_env('VERBOSE', 'true') + end + + include_examples "call table locker", :lock_writes + end + + context 'when environment sets VERBOSE to false' do + let(:verbose) { false } + + before do + stub_env('VERBOSE', 'false') + end + + include_examples "call table locker", :lock_writes + end + + context 'when environment does not define VERBOSE' do + include_examples "call table locker", :lock_writes + end end describe 'unlock_writes' do @@ -71,8 +100,6 @@ RSpec.describe 'gitlab:db:lock_writes', :reestablished_active_record_base, featu end context 'when environment sets DRY_RUN to false' do - let(:dry_run) { false } - before do stub_env('DRY_RUN', 'false') end @@ -81,9 +108,31 @@ RSpec.describe 'gitlab:db:lock_writes', :reestablished_active_record_base, featu end context 'when environment does not define DRY_RUN' do - let(:dry_run) { false } - include_examples "call table locker", :unlock_writes end + + context 'when environment sets VERBOSE to true' do + let(:verbose) { true } + + before do + stub_env('VERBOSE', 'true') + end + + include_examples "call table locker", :lock_writes + end + + context 'when environment sets VERBOSE to false' do + let(:verbose) { false } + + before do + stub_env('VERBOSE', 'false') + end + + include_examples "call table locker", :lock_writes + end + + context 'when environment does not define VERBOSE' do + include_examples "call table locker", :lock_writes + end end end |