diff --git a/.gitlab/ci/pages.gitlab-ci.yml b/.gitlab/ci/pages.gitlab-ci.yml index 218ec7043d9..d374f53a5e2 100644 --- a/.gitlab/ci/pages.gitlab-ci.yml +++ b/.gitlab/ci/pages.gitlab-ci.yml @@ -3,11 +3,16 @@ pages: - .default-retry - .pages:rules stage: pages - dependencies: ["rspec:coverage", "karma", "gitlab:assets:compile pull-cache"] + dependencies: + - rspec:coverage + - coverage-frontend + - karma + - gitlab:assets:compile pull-cache script: - mv public/ .public/ - mkdir public/ - mv coverage/ public/coverage-ruby/ || true + - mv coverage-frontend/ public/coverage-frontend/ || true - mv coverage-javascript/ public/coverage-javascript/ || true - mv webpack-report/ public/webpack-report/ || true - cp .public/assets/application-*.css public/application.css || true diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb index c9fe6bddb50..df3fb6b67c2 100644 --- a/app/controllers/groups/milestones_controller.rb +++ b/app/controllers/groups/milestones_controller.rb @@ -12,7 +12,7 @@ class Groups::MilestonesController < Groups::ApplicationController def index respond_to do |format| format.html do - @milestone_states = Milestone.states_count(group_projects_with_access, [group]) + @milestone_states = Milestone.states_count(group_projects_with_access.without_order, [group]) @milestones = milestones.page(params[:page]) end format.json do diff --git a/app/controllers/projects/group_links_controller.rb b/app/controllers/projects/group_links_controller.rb index d06e24ef39c..b1ecd439e5a 100644 --- a/app/controllers/projects/group_links_controller.rb +++ b/app/controllers/projects/group_links_controller.rb @@ -6,7 +6,7 @@ class Projects::GroupLinksController < Projects::ApplicationController before_action :authorize_admin_project_member!, only: [:update] def index - redirect_to namespace_project_settings_members_path + redirect_to namespace_project_project_members_path end def create diff --git a/app/finders/events_finder.rb b/app/finders/events_finder.rb index 9c56451fd44..52612f1f8aa 100644 --- a/app/finders/events_finder.rb +++ b/app/finders/events_finder.rb @@ -72,9 +72,10 @@ class EventsFinder # rubocop: disable CodeReuse/ActiveRecord def by_action(events) - return events unless Event::ACTIONS[params[:action]] + safe_action = Event.actions[params[:action]] + return events unless safe_action - events.where(action: Event::ACTIONS[params[:action]]) + events.where(action: safe_action) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/graphql/resolvers/user_resolver.rb b/app/graphql/resolvers/user_resolver.rb new file mode 100644 index 00000000000..9696988f3b4 --- /dev/null +++ b/app/graphql/resolvers/user_resolver.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +module Resolvers + class UserResolver < BaseResolver + description 'Retrieve a single user' + + argument :id, GraphQL::ID_TYPE, + required: false, + description: 'ID of the User' + + argument :username, GraphQL::STRING_TYPE, + required: false, + description: 'Username of the User' + + def resolve(id: nil, username: nil) + id_or_username = GitlabSchema.parse_gid(id, expected_type: ::User).model_id if id + id_or_username ||= username + + ::UserFinder.new(id_or_username).find_by_id_or_username + end + + def ready?(id: nil, username: nil) + unless id.present? ^ username.present? + raise Gitlab::Graphql::Errors::ArgumentError, 'Provide either a single username or id' + end + + super + end + end +end diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 70cdcb62bc6..bedb68d989b 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -47,6 +47,11 @@ module Types null: false, description: 'Fields related to design management' + field :user, Types::UserType, + null: true, + description: 'Find a user', + resolver: Resolvers::UserResolver + field :echo, GraphQL::STRING_TYPE, null: false, description: 'Text to echo back', resolver: Resolvers::EchoResolver diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 79546212bcc..1e3f964a709 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -677,7 +677,6 @@ module ProjectsHelper def sidebar_settings_paths %w[ projects#edit - project_members#index integrations#show services#edit hooks#index diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 0979d03f6e6..c7e4d64d3d5 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -5,6 +5,10 @@ class ApplicationRecord < ActiveRecord::Base alias_method :reset, :reload + def self.without_order + reorder(nil) + end + def self.id_in(ids) where(id: ids) end diff --git a/app/models/concerns/timebox.rb b/app/models/concerns/timebox.rb index 16b86401b50..4c8d4b1ea59 100644 --- a/app/models/concerns/timebox.rb +++ b/app/models/concerns/timebox.rb @@ -9,6 +9,7 @@ module Timebox include IidRoutes include Referable include StripAttribute + include FromUnion TimeboxStruct = Struct.new(:title, :name, :id) do # Ensure these models match the interface required for exporting @@ -65,7 +66,11 @@ module Timebox groups = groups.compact if groups.is_a? Array groups = [] if groups.nil? - where(project_id: projects).or(where(group_id: groups)) + if Feature.enabled?(:optimized_timebox_queries) + from_union([where(project_id: projects), where(group_id: groups)], remove_duplicates: false) + else + where(project_id: projects).or(where(group_id: groups)) + end end scope :within_timeframe, -> (start_date, end_date) do diff --git a/app/models/event.rb b/app/models/event.rb index 25d016291a7..9e93bf5a864 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -10,35 +10,24 @@ class Event < ApplicationRecord default_scope { reorder(nil) } - CREATED = 1 - UPDATED = 2 - CLOSED = 3 - REOPENED = 4 - PUSHED = 5 - COMMENTED = 6 - MERGED = 7 - JOINED = 8 # User joined project - LEFT = 9 # User left project - DESTROYED = 10 - EXPIRED = 11 # User left project due to expiry - APPROVED = 12 - ACTIONS = HashWithIndifferentAccess.new( - created: CREATED, - updated: UPDATED, - closed: CLOSED, - reopened: REOPENED, - pushed: PUSHED, - commented: COMMENTED, - merged: MERGED, - joined: JOINED, - left: LEFT, - destroyed: DESTROYED, - expired: EXPIRED, - approved: APPROVED + created: 1, + updated: 2, + closed: 3, + reopened: 4, + pushed: 5, + commented: 6, + merged: 7, + joined: 8, # User joined project + left: 9, # User left project + destroyed: 10, + expired: 11, # User left project due to expiry + approved: 12 ).freeze - WIKI_ACTIONS = [CREATED, UPDATED, DESTROYED].freeze + private_constant :ACTIONS + + WIKI_ACTIONS = [:created, :updated, :destroyed].freeze TARGET_TYPES = HashWithIndifferentAccess.new( issue: Issue, @@ -54,6 +43,8 @@ class Event < ApplicationRecord RESET_PROJECT_ACTIVITY_INTERVAL = 1.hour REPOSITORY_UPDATED_AT_INTERVAL = 5.minutes + enum action: ACTIONS, _suffix: true + delegate :name, :email, :public_email, :username, to: :author, prefix: true, allow_nil: true delegate :title, to: :issue, prefix: true, allow_nil: true delegate :title, to: :merge_request, prefix: true, allow_nil: true @@ -83,8 +74,6 @@ class Event < ApplicationRecord # Scopes scope :recent, -> { reorder(id: :desc) } - scope :code_push, -> { where(action: PUSHED) } - scope :merged, -> { where(action: MERGED) } scope :for_wiki_page, -> { where(target_type: 'WikiPage::Meta') } # Needed to implement feature flag: can be removed when feature flag is removed @@ -115,7 +104,7 @@ class Event < ApplicationRecord end def find_sti_class(action) - if action.to_i == PUSHED + if actions.fetch(action, action) == actions[:pushed] # action can be integer or symbol PushEvent else Event @@ -125,19 +114,15 @@ class Event < ApplicationRecord # Update Gitlab::ContributionsCalendar#activity_dates if this changes def contributions where("action = ? OR (target_type IN (?) AND action IN (?)) OR (target_type = ? AND action = ?)", - Event::PUSHED, - %w(MergeRequest Issue), [Event::CREATED, Event::CLOSED, Event::MERGED], - "Note", Event::COMMENTED) + actions[:pushed], + %w(MergeRequest Issue), [actions[:created], actions[:closed], actions[:merged]], + "Note", actions[:commented]) end def limit_recent(limit = 20, offset = nil) recent.limit(limit).offset(offset) end - def actions - ACTIONS.keys - end - def target_types TARGET_TYPES.keys end @@ -161,46 +146,10 @@ class Event < ApplicationRecord target.try(:title) end - def created_action? - action == CREATED - end - def push_action? false end - def merged_action? - action == MERGED - end - - def closed_action? - action == CLOSED - end - - def reopened_action? - action == REOPENED - end - - def joined_action? - action == JOINED - end - - def left_action? - action == LEFT - end - - def expired_action? - action == EXPIRED - end - - def destroyed_action? - action == DESTROYED - end - - def commented_action? - action == COMMENTED - end - def membership_changed? joined_action? || left_action? || expired_action? end @@ -210,11 +159,11 @@ class Event < ApplicationRecord end def created_wiki_page? - wiki_page? && action == CREATED + wiki_page? && created_action? end def updated_wiki_page? - wiki_page? && action == UPDATED + wiki_page? && updated_action? end def created_target? diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 3409c249826..0ba660aadbd 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -896,11 +896,11 @@ class MergeRequest < ApplicationRecord end def merge_event - @merge_event ||= target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::MERGED).last + @merge_event ||= target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: :merged).last end def closed_event - @closed_event ||= target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last + @closed_event ||= target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: :closed).last end def work_in_progress? diff --git a/app/models/project.rb b/app/models/project.rb index 0929757f8d4..8c8e3db3336 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -445,7 +445,7 @@ class Project < ApplicationRecord scope :archived, -> { where(archived: true) } scope :non_archived, -> { where(archived: false) } scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct } - scope :with_push, -> { joins(:events).where('events.action = ?', Event::PUSHED) } + scope :with_push, -> { joins(:events).merge(Event.pushed_action) } scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') } scope :inc_routes, -> { includes(:route, namespace: :route) } scope :with_statistics, -> { includes(:statistics) } diff --git a/app/models/push_event.rb b/app/models/push_event.rb index 5cab686f20b..0f626cb618e 100644 --- a/app/models/push_event.rb +++ b/app/models/push_event.rb @@ -68,7 +68,7 @@ class PushEvent < Event end def self.sti_name - PUSHED + actions[:pushed] end def push_action? @@ -111,7 +111,7 @@ class PushEvent < Event end def validate_push_action - return if action == PUSHED + return if pushed_action? errors.add(:action, "the action #{action.inspect} is not valid") end diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index c879c7432a9..1fb87ca2a98 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -13,79 +13,79 @@ class EventCreateService def open_issue(issue, current_user) create_resource_event(issue, current_user, :opened) - create_record_event(issue, current_user, Event::CREATED) + create_record_event(issue, current_user, :created) end def close_issue(issue, current_user) create_resource_event(issue, current_user, :closed) - create_record_event(issue, current_user, Event::CLOSED) + create_record_event(issue, current_user, :closed) end def reopen_issue(issue, current_user) create_resource_event(issue, current_user, :reopened) - create_record_event(issue, current_user, Event::REOPENED) + create_record_event(issue, current_user, :reopened) end def open_mr(merge_request, current_user) create_resource_event(merge_request, current_user, :opened) - create_record_event(merge_request, current_user, Event::CREATED) + create_record_event(merge_request, current_user, :created) end def close_mr(merge_request, current_user) create_resource_event(merge_request, current_user, :closed) - create_record_event(merge_request, current_user, Event::CLOSED) + create_record_event(merge_request, current_user, :closed) end def reopen_mr(merge_request, current_user) create_resource_event(merge_request, current_user, :reopened) - create_record_event(merge_request, current_user, Event::REOPENED) + create_record_event(merge_request, current_user, :reopened) end def merge_mr(merge_request, current_user) create_resource_event(merge_request, current_user, :merged) - create_record_event(merge_request, current_user, Event::MERGED) + create_record_event(merge_request, current_user, :merged) end def open_milestone(milestone, current_user) - create_record_event(milestone, current_user, Event::CREATED) + create_record_event(milestone, current_user, :created) end def close_milestone(milestone, current_user) - create_record_event(milestone, current_user, Event::CLOSED) + create_record_event(milestone, current_user, :closed) end def reopen_milestone(milestone, current_user) - create_record_event(milestone, current_user, Event::REOPENED) + create_record_event(milestone, current_user, :reopened) end def destroy_milestone(milestone, current_user) - create_record_event(milestone, current_user, Event::DESTROYED) + create_record_event(milestone, current_user, :destroyed) end def leave_note(note, current_user) - create_record_event(note, current_user, Event::COMMENTED) + create_record_event(note, current_user, :commented) end def join_project(project, current_user) - create_event(project, current_user, Event::JOINED) + create_event(project, current_user, :joined) end def leave_project(project, current_user) - create_event(project, current_user, Event::LEFT) + create_event(project, current_user, :left) end def expired_leave_project(project, current_user) - create_event(project, current_user, Event::EXPIRED) + create_event(project, current_user, :expired) end def create_project(project, current_user) - create_event(project, current_user, Event::CREATED) + create_event(project, current_user, :created) end def push(project, current_user, push_data) @@ -100,7 +100,7 @@ class EventCreateService # # @param [WikiPage::Meta] wiki_page_meta The event target # @param [User] author The event author - # @param [Integer] action One of the Event::WIKI_ACTIONS + # @param [Symbol] action One of the Event::WIKI_ACTIONS # # @return a tuple of event and either :found or :created def wiki_event(wiki_page_meta, author, action) @@ -114,7 +114,7 @@ class EventCreateService event = create_record_event(wiki_page_meta, author, action) # Ensure that the event is linked in time to the metadata, for non-deletes - unless action == Event::DESTROYED + unless event.destroyed_action? time_stamp = wiki_page_meta.updated_at event.update_columns(updated_at: time_stamp, created_at: time_stamp) end @@ -125,9 +125,9 @@ class EventCreateService private def existing_wiki_event(wiki_page_meta, action) - if action == Event::DESTROYED + if Event.actions.fetch(action) == Event.actions[:destroyed] most_recent = Event.for_wiki_meta(wiki_page_meta).recent.first - return most_recent if most_recent.present? && most_recent.action == action + return most_recent if most_recent.present? && Event.actions[most_recent.action] == Event.actions[action] else Event.for_wiki_meta(wiki_page_meta).created_at(wiki_page_meta.updated_at).first end @@ -142,7 +142,7 @@ class EventCreateService # when creating push payload data will result in the event creation being # rolled back as well. event = Event.transaction do - new_event = create_event(project, current_user, Event::PUSHED) + new_event = create_event(project, current_user, :pushed) service_class.new(new_event, push_data).execute diff --git a/app/services/git/wiki_push_service/change.rb b/app/services/git/wiki_push_service/change.rb index 8685850165a..14e622dd147 100644 --- a/app/services/git/wiki_push_service/change.rb +++ b/app/services/git/wiki_push_service/change.rb @@ -21,11 +21,11 @@ module Git def event_action case raw_change.operation when :added - Event::CREATED + :created when :deleted - Event::DESTROYED + :destroyed else - Event::UPDATED + :updated end end diff --git a/app/services/wiki_pages/create_service.rb b/app/services/wiki_pages/create_service.rb index 4ef19676d82..63107445782 100644 --- a/app/services/wiki_pages/create_service.rb +++ b/app/services/wiki_pages/create_service.rb @@ -22,7 +22,7 @@ module WikiPages end def event_action - Event::CREATED + :created end end end diff --git a/app/services/wiki_pages/destroy_service.rb b/app/services/wiki_pages/destroy_service.rb index eb162223723..d59c27bb92a 100644 --- a/app/services/wiki_pages/destroy_service.rb +++ b/app/services/wiki_pages/destroy_service.rb @@ -19,7 +19,7 @@ module WikiPages end def event_action - Event::DESTROYED + :destroyed end end end diff --git a/app/services/wiki_pages/update_service.rb b/app/services/wiki_pages/update_service.rb index 0a056f1ec33..5ac6902e0b0 100644 --- a/app/services/wiki_pages/update_service.rb +++ b/app/services/wiki_pages/update_service.rb @@ -22,7 +22,7 @@ module WikiPages end def event_action - Event::UPDATED + :updated end def slug_for_page(page) diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 409e602f306..d1a06851b5f 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -330,6 +330,18 @@ %strong.fly-out-top-item-name = _('Snippets') + = nav_link(controller: :project_members) do + = link_to project_project_members_path(@project), title: _('Members'), class: 'qa-members-link', id: 'js-onboarding-members-link' do + .nav-icon-container + = sprite_icon('users') + %span.nav-item-name + = _('Members') + %ul.sidebar-sub-level-items.is-fly-out-only + = nav_link(path: %w[members#show], html_options: { class: "fly-out-top-item" } ) do + = link_to project_project_members_path(@project) do + %strong.fly-out-top-item-name + = _('Members') + - if project_nav_tab? :settings = nav_link(path: sidebar_settings_paths) do = link_to edit_project_path(@project), class: 'shortcuts-tree' do @@ -350,10 +362,6 @@ = link_to edit_project_path(@project), title: _('General'), class: 'qa-general-settings-link' do %span = _('General') - = nav_link(controller: :project_members) do - = link_to project_project_members_path(@project), title: _('Members'), class: 'qa-link-members-settings', id: 'js-onboarding-settings-members-link' do - %span - = _('Members') - if can_edit = nav_link(controller: [:integrations, :services]) do = link_to project_settings_integrations_path(@project), title: _('Integrations'), data: { qa_selector: 'integrations_settings_link' } do @@ -389,19 +397,6 @@ = render_if_exists 'projects/sidebar/settings_audit_events' - - else - = nav_link(controller: :project_members) do - = link_to project_settings_members_path(@project), title: _('Members'), class: 'shortcuts-tree' do - .nav-icon-container - = sprite_icon('users') - %span.nav-item-name - = _('Members') - %ul.sidebar-sub-level-items.is-fly-out-only - = nav_link(path: %w[members#show], html_options: { class: "fly-out-top-item" } ) do - = link_to project_project_members_path(@project) do - %strong.fly-out-top-item-name - = _('Members') - = render 'shared/sidebar_toggle_button' -# Shortcut to Project > Activity diff --git a/changelogs/unreleased/214102-move-the-members-section-from-settings-to-the-side-nav-for-project.yml b/changelogs/unreleased/214102-move-the-members-section-from-settings-to-the-side-nav-for-project.yml new file mode 100644 index 00000000000..2cb3569a38e --- /dev/null +++ b/changelogs/unreleased/214102-move-the-members-section-from-settings-to-the-side-nav-for-project.yml @@ -0,0 +1,5 @@ +--- +title: Move the Members section from settings to the side nav for projects +merge_request: 32667 +author: +type: changed diff --git a/changelogs/unreleased/215658-add-users-graphql.yml b/changelogs/unreleased/215658-add-users-graphql.yml new file mode 100644 index 00000000000..a28ae9f1a12 --- /dev/null +++ b/changelogs/unreleased/215658-add-users-graphql.yml @@ -0,0 +1,5 @@ +--- +title: Add user root query to GraphQL API +merge_request: 33041 +author: +type: added diff --git a/changelogs/unreleased/optimize-milestones-page.yml b/changelogs/unreleased/optimize-milestones-page.yml new file mode 100644 index 00000000000..dd067c08faf --- /dev/null +++ b/changelogs/unreleased/optimize-milestones-page.yml @@ -0,0 +1,5 @@ +--- +title: Optimize SQL queries on Milestone index page +merge_request: 32953 +author: +type: performance diff --git a/config/routes/project.rb b/config/routes/project.rb index 219cf3a5ea6..9c979e642cc 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -71,8 +71,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do end namespace :settings do - get :members, to: redirect("%{namespace_id}/%{project_id}/-/project_members") - resource :ci_cd, only: [:show, :update], controller: 'ci_cd' do post :reset_cache put :reset_registration_token diff --git a/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md b/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md index 400122f3a4a..b851e6feb06 100644 --- a/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md +++ b/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md @@ -302,7 +302,7 @@ you will see two pushes with the same "from" SHA: ```ruby p = Project.find_with_namespace('u/p') -p.events.code_push.last(100).each do |e| +p.events.pushed_action.last(100).each do |e| printf "%-20.20s %8s...%8s (%s)\n", e.data[:ref], e.data[:before], e.data[:after], e.author.try(:username) end ``` @@ -311,7 +311,7 @@ GitLab 9.5 and above: ```ruby p = Project.find_by_full_path('u/p') -p.events.code_push.last(100).each do |e| +p.events.pushed_action.last(100).each do |e| printf "%-20.20s %8s...%8s (%s)\n", e.push_event_payload[:ref], e.push_event_payload[:commit_from], e.push_event_payload[:commit_to], e.author.try(:username) end ``` diff --git a/doc/api/graphql/index.md b/doc/api/graphql/index.md index 044c3500bf4..a216b4fe188 100644 --- a/doc/api/graphql/index.md +++ b/doc/api/graphql/index.md @@ -57,6 +57,7 @@ The GraphQL API includes the following queries at the root level: 1. `project` : Project information, with many of its associations such as issues and merge requests. 1. `group` : Basic group information and epics **(ULTIMATE)** are currently supported. +1. `user` : Information about a particular user. 1. `namespace` : Within a namespace it is also possible to fetch `projects`. 1. `currentUser`: Information about the currently logged in user. 1. `metaData`: Metadata about GitLab and the GraphQL API. diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 8e881a124a6..7a814b3a4f4 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -9142,6 +9142,21 @@ type Query { visibility: VisibilityScopesEnum ): SnippetConnection + """ + Find a user + """ + user( + """ + ID of the User + """ + id: ID + + """ + Username of the User + """ + username: String + ): User + """ Vulnerabilities reported on projects on the current user's instance security dashboard """ diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 6cebd565ae7..e34127de1e8 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -26833,6 +26833,39 @@ "isDeprecated": false, "deprecationReason": null }, + { + "name": "user", + "description": "Find a user", + "args": [ + { + "name": "id", + "description": "ID of the User", + "type": { + "kind": "SCALAR", + "name": "ID", + "ofType": null + }, + "defaultValue": null + }, + { + "name": "username", + "description": "Username of the User", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null + } + ], + "type": { + "kind": "OBJECT", + "name": "User", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "vulnerabilities", "description": "Vulnerabilities reported on projects on the current user's instance security dashboard", diff --git a/doc/development/testing_guide/end_to_end/style_guide.md b/doc/development/testing_guide/end_to_end/style_guide.md index 9c02af12d5d..32701a1fa29 100644 --- a/doc/development/testing_guide/end_to_end/style_guide.md +++ b/doc/development/testing_guide/end_to_end/style_guide.md @@ -123,7 +123,7 @@ Capybara DSL, potentially leading to confusion and bugs. **Good** ```ruby -Page::Project::Settings::Members.perform do |members| +Page::Project::Members.perform do |members| members.do_something end ``` @@ -149,7 +149,7 @@ end **Bad** ```ruby -Page::Project::Settings::Members.perform do |project_settings_members_page| +Page::Project::Members.perform do |project_settings_members_page| project_settings_members_page.do_something end ``` diff --git a/doc/user/project/clusters/index.md b/doc/user/project/clusters/index.md index 26521df35cc..961a9fda5ff 100644 --- a/doc/user/project/clusters/index.md +++ b/doc/user/project/clusters/index.md @@ -46,6 +46,7 @@ GitLab is committed to support at least two production-ready Kubernetes minor ve Currently, GitLab supports the following Kubernetes versions: +- 1.16 - 1.15 - 1.14 - 1.13 (deprecated, support ends on November 22, 2020) diff --git a/doc/user/project/code_owners.md b/doc/user/project/code_owners.md index 8b02dcb17f1..81bdf1dfb2f 100644 --- a/doc/user/project/code_owners.md +++ b/doc/user/project/code_owners.md @@ -103,7 +103,7 @@ Any of the following groups would be eligible to be specified as code owners: - `@group/sub-group` - `@group/sub-group/sub-subgroup` -In addition, any groups that have been invited to the project using the **Settings > Members** tool will also be recognized as eligible code owners. +In addition, any groups that have been invited to the project using the **Members** tool will also be recognized as eligible code owners. The order in which the paths are defined is significant: the last pattern that matches a given path will be used to find the code diff --git a/doc/user/project/members/index.md b/doc/user/project/members/index.md index c0575fc16aa..787a74b0065 100644 --- a/doc/user/project/members/index.md +++ b/doc/user/project/members/index.md @@ -8,7 +8,7 @@ You should have Maintainer or Owner [permissions](../../permissions.md) to add or import a new user to your project. To view, edit, add, and remove project's members, go to your -project's **Settings > Members**. +project's **Members**. ## Inherited membership diff --git a/doc/user/project/members/share_project_with_groups.md b/doc/user/project/members/share_project_with_groups.md index d0ceac3e1f3..033d69cbbfa 100644 --- a/doc/user/project/members/share_project_with_groups.md +++ b/doc/user/project/members/share_project_with_groups.md @@ -18,7 +18,7 @@ This is where the group sharing feature can be of use. To share 'Project Acme' with the 'Engineering' group: -1. For 'Project Acme' use the left navigation menu to go to **Settings > Members** +1. For 'Project Acme' use the left navigation menu to go to **Members** ![share project with groups](img/share_project_with_groups.png) diff --git a/doc/user/project/settings/project_access_tokens.md b/doc/user/project/settings/project_access_tokens.md index 460a5b6f88d..42ba2654b42 100644 --- a/doc/user/project/settings/project_access_tokens.md +++ b/doc/user/project/settings/project_access_tokens.md @@ -4,7 +4,7 @@ CAUTION: **Warning:** This is an [Alpha](https://about.gitlab.com/handbook/product/#alpha) feature, and it is subject to change at any time without prior notice. -> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2587) in GitLab 13.0. +> - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/2587) in GitLab 13.0. > - It's deployed behind a feature flag, disabled by default. > - It's disabled on GitLab.com. > - To use it in GitLab self-managed instances, ask a GitLab administrator to [enable it](#enable-or-disable-project-access-tokens). @@ -34,7 +34,7 @@ For each project access token created, a bot user will also be created and added ["Maintainer" level permissions](../../permissions.md#project-members-permissions). API calls made with a project access token will be associated to the corresponding bot user. -These users will appear in **{settings}** **Settings > Members** but can not be modified. +These users will appear in **Members** but can not be modified. Furthermore, the bot user can not be added to any other project. When the project access token is [revoked](#revoking-a-project-access-token) the bot user will be deleted and all diff --git a/lib/event_filter.rb b/lib/event_filter.rb index 8cb0b1441df..538727dc422 100644 --- a/lib/event_filter.rb +++ b/lib/event_filter.rb @@ -27,15 +27,15 @@ class EventFilter case filter when PUSH - events.where(action: Event::PUSHED) + events.pushed_action when MERGED - events.where(action: Event::MERGED) + events.merged_action when COMMENTS - events.where(action: Event::COMMENTED) + events.commented_action when TEAM - events.where(action: [Event::JOINED, Event::LEFT, Event::EXPIRED]) + events.where(action: [:joined, :left, :expired]) when ISSUE - events.where(action: [Event::CREATED, Event::UPDATED, Event::CLOSED, Event::REOPENED], target_type: 'Issue') + events.where(action: [:created, :updated, :closed, :reopened], target_type: 'Issue') when WIKI wiki_events(events) else diff --git a/lib/gitlab/contributions_calendar.rb b/lib/gitlab/contributions_calendar.rb index 5b0b91de5da..4e430d8937d 100644 --- a/lib/gitlab/contributions_calendar.rb +++ b/lib/gitlab/contributions_calendar.rb @@ -24,13 +24,13 @@ module Gitlab # project_features for the (currently) 3 different contribution types date_from = 1.year.ago repo_events = event_counts(date_from, :repository) - .having(action: Event::PUSHED) + .having(action: :pushed) issue_events = event_counts(date_from, :issues) - .having(action: [Event::CREATED, Event::CLOSED], target_type: "Issue") + .having(action: [:created, :closed], target_type: "Issue") mr_events = event_counts(date_from, :merge_requests) - .having(action: [Event::MERGED, Event::CREATED, Event::CLOSED], target_type: "MergeRequest") + .having(action: [:merged, :created, :closed], target_type: "MergeRequest") note_events = event_counts(date_from, :merge_requests) - .having(action: [Event::COMMENTED]) + .having(action: :commented) events = Event .from_union([repo_events, issue_events, mr_events, note_events]) diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index f1ca8900c30..6702c053fc7 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -24,6 +24,7 @@ module Gitlab deploy_key_upload: 'This deploy key does not have write access to this project.', no_repo: 'A repository for this project does not exist yet.', project_not_found: 'The project you were looking for could not be found.', + namespace_not_found: 'The namespace you were looking for could not be found.', command_not_allowed: "The command you're trying to execute is not allowed.", upload_pack_disabled_over_http: 'Pulling over HTTP is not allowed.', receive_pack_disabled_over_http: 'Pushing over HTTP is not allowed.', @@ -73,6 +74,7 @@ module Gitlab return custom_action if custom_action check_db_accessibility!(cmd) + check_namespace! check_project!(changes, cmd) check_repository_existence! @@ -111,7 +113,6 @@ module Gitlab private def check_project!(changes, cmd) - check_namespace! ensure_project_on_push!(cmd, changes) check_project_accessibility! add_project_moved_message! @@ -156,7 +157,7 @@ module Gitlab def check_namespace! return if namespace_path.present? - raise NotFoundError, ERROR_MESSAGES[:project_not_found] + raise NotFoundError, ERROR_MESSAGES[:namespace_not_found] end def check_active_user! diff --git a/lib/gitlab/git_access_snippet.rb b/lib/gitlab/git_access_snippet.rb index 70db4271469..ba9920caf6f 100644 --- a/lib/gitlab/git_access_snippet.rb +++ b/lib/gitlab/git_access_snippet.rb @@ -39,11 +39,17 @@ module Gitlab private + override :check_namespace! + def check_namespace! + return unless snippet.is_a?(ProjectSnippet) + + super + end + override :check_project! def check_project!(cmd, changes) return unless snippet.is_a?(ProjectSnippet) - check_namespace! check_project_accessibility! add_project_moved_message! end diff --git a/qa/qa.rb b/qa/qa.rb index c2bf003d18e..933ffe090ba 100644 --- a/qa/qa.rb +++ b/qa/qa.rb @@ -229,6 +229,7 @@ module QA autoload :Show, 'qa/page/project/show' autoload :Activity, 'qa/page/project/activity' autoload :Menu, 'qa/page/project/menu' + autoload :Members, 'qa/page/project/members' module Branches autoload :Show, 'qa/page/project/branches/show' @@ -265,7 +266,6 @@ module QA autoload :CiVariables, 'qa/page/project/settings/ci_variables' autoload :Runners, 'qa/page/project/settings/runners' autoload :MergeRequest, 'qa/page/project/settings/merge_request' - autoload :Members, 'qa/page/project/settings/members' autoload :MirroringRepositories, 'qa/page/project/settings/mirroring_repositories' autoload :VisibilityFeaturesPermissions, 'qa/page/project/settings/visibility_features_permissions' diff --git a/qa/qa/flow/project.rb b/qa/qa/flow/project.rb index 72b9357a604..8eddd0f30b2 100644 --- a/qa/qa/flow/project.rb +++ b/qa/qa/flow/project.rb @@ -8,9 +8,9 @@ module QA def add_member(project:, username:) project.visit! - Page::Project::Menu.perform(&:go_to_members_settings) + Page::Project::Menu.perform(&:click_members) - Page::Project::Settings::Members.perform do |member_settings| + Page::Project::Members.perform do |member_settings| member_settings.add_member(username) end end diff --git a/qa/qa/page/project/members.rb b/qa/qa/page/project/members.rb new file mode 100644 index 00000000000..88b05ceb1d1 --- /dev/null +++ b/qa/qa/page/project/members.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module QA + module Page + module Project + class Members < Page::Base + include QA::Page::Component::Select2 + + view 'app/views/shared/members/_invite_member.html.haml' do + element :member_select_field + element :invite_member_button + end + + view 'app/views/projects/project_members/_team.html.haml' do + element :members_list + end + + view 'app/views/projects/project_members/index.html.haml' do + element :invite_group_tab + end + + view 'app/views/shared/members/_invite_group.html.haml' do + element :group_select_field + element :invite_group_button + end + + view 'app/views/shared/members/_group.html.haml' do + element :group_row + element :delete_group_access_link + end + + def select_group(group_name) + click_element :group_select_field + search_and_select(group_name) + end + + def invite_group(group_name) + click_element :invite_group_tab + select_group(group_name) + click_element :invite_group_button + end + + def add_member(username) + click_element :member_select_field + search_and_select username + click_element :invite_member_button + end + + def remove_group(group_name) + click_element :invite_group_tab + page.accept_alert do + within_element(:group_row, text: group_name) do + click_element :delete_group_access_link + end + end + end + end + end + end +end diff --git a/qa/qa/page/project/menu.rb b/qa/qa/page/project/menu.rb index e2984bf72bd..3d4d0ff9d22 100644 --- a/qa/qa/page/project/menu.rb +++ b/qa/qa/page/project/menu.rb @@ -17,6 +17,7 @@ module QA element :merge_requests_link element :wiki_link element :snippets_link + element :members_link end def click_merge_requests @@ -42,6 +43,12 @@ module QA click_element(:snippets_link) end end + + def click_members + within_sidebar do + click_element(:members_link) + end + end end end end diff --git a/qa/qa/page/project/settings/members.rb b/qa/qa/page/project/settings/members.rb deleted file mode 100644 index 5dc873750b0..00000000000 --- a/qa/qa/page/project/settings/members.rb +++ /dev/null @@ -1,62 +0,0 @@ -# frozen_string_literal: true - -module QA - module Page - module Project - module Settings - class Members < Page::Base - include QA::Page::Component::Select2 - - view 'app/views/shared/members/_invite_member.html.haml' do - element :member_select_field - element :invite_member_button - end - - view 'app/views/projects/project_members/_team.html.haml' do - element :members_list - end - - view 'app/views/projects/project_members/index.html.haml' do - element :invite_group_tab - end - - view 'app/views/shared/members/_invite_group.html.haml' do - element :group_select_field - element :invite_group_button - end - - view 'app/views/shared/members/_group.html.haml' do - element :group_row - element :delete_group_access_link - end - - def select_group(group_name) - click_element :group_select_field - search_and_select(group_name) - end - - def invite_group(group_name) - click_element :invite_group_tab - select_group(group_name) - click_element :invite_group_button - end - - def add_member(username) - click_element :member_select_field - search_and_select username - click_element :invite_member_button - end - - def remove_group(group_name) - click_element :invite_group_tab - page.accept_alert do - within_element(:group_row, text: group_name) do - click_element :delete_group_access_link - end - end - end - end - end - end - end -end diff --git a/qa/qa/page/project/sub_menus/settings.rb b/qa/qa/page/project/sub_menus/settings.rb index 0dd4bd1817a..47274c8db54 100644 --- a/qa/qa/page/project/sub_menus/settings.rb +++ b/qa/qa/page/project/sub_menus/settings.rb @@ -15,7 +15,6 @@ module QA view 'app/views/layouts/nav/sidebar/_project.html.haml' do element :settings_item - element :link_members_settings element :general_settings_link element :integrations_settings_link element :operations_settings_link @@ -31,14 +30,6 @@ module QA end end - def go_to_members_settings - hover_settings do - within_submenu do - click_element :link_members_settings - end - end - end - def go_to_repository_settings hover_settings do within_submenu do diff --git a/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb index e30afbf8ae0..67055537567 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/project/add_project_member_spec.rb @@ -12,8 +12,8 @@ module QA project.name = 'add-member-project' end.visit! - Page::Project::Menu.perform(&:go_to_members_settings) - Page::Project::Settings::Members.perform do |members| + Page::Project::Menu.perform(&:click_members) + Page::Project::Members.perform do |members| members.add_member(user.username) end diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index ee61ef73b45..b6c91db8b4d 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -145,7 +145,7 @@ describe Projects::MilestonesController do delete :destroy, params: { namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid }, format: :js expect(response).to be_successful - expect(Event.recent.first.action).to eq(Event::DESTROYED) + expect(Event.recent.first).to be_destroyed_action expect { Milestone.find(milestone.id) }.to raise_exception(ActiveRecord::RecordNotFound) issue.reload diff --git a/spec/factories/events.rb b/spec/factories/events.rb index ed6cb3505f4..021d2070b87 100644 --- a/spec/factories/events.rb +++ b/spec/factories/events.rb @@ -4,27 +4,27 @@ FactoryBot.define do factory :event do project author(factory: :user) { project.creator } - action { Event::JOINED } + action { :joined } - trait(:created) { action { Event::CREATED } } - trait(:updated) { action { Event::UPDATED } } - trait(:closed) { action { Event::CLOSED } } - trait(:reopened) { action { Event::REOPENED } } - trait(:pushed) { action { Event::PUSHED } } - trait(:commented) { action { Event::COMMENTED } } - trait(:merged) { action { Event::MERGED } } - trait(:joined) { action { Event::JOINED } } - trait(:left) { action { Event::LEFT } } - trait(:destroyed) { action { Event::DESTROYED } } - trait(:expired) { action { Event::EXPIRED } } + trait(:created) { action { :created } } + trait(:updated) { action { :updated } } + trait(:closed) { action { :closed } } + trait(:reopened) { action { :reopened } } + trait(:pushed) { action { :pushed } } + trait(:commented) { action { :commented } } + trait(:merged) { action { :merged } } + trait(:joined) { action { :joined } } + trait(:left) { action { :left } } + trait(:destroyed) { action { :destroyed } } + trait(:expired) { action { :expired } } factory :closed_issue_event do - action { Event::CLOSED } + action { :closed } target factory: :closed_issue end factory :wiki_page_event do - action { Event::CREATED } + action { :created } project { @overrides[:wiki_page]&.container || create(:project, :wiki_repo) } target { create(:wiki_page_meta, :for_wiki_page, wiki_page: wiki_page) } @@ -39,7 +39,7 @@ FactoryBot.define do note { create(:note, author: author, project: project, noteable: design) } end - action { Event::COMMENTED } + action { :commented } target { note } end end @@ -47,7 +47,7 @@ FactoryBot.define do factory :push_event, class: 'PushEvent' do project factory: :project_empty_repo author(factory: :user) { project.creator } - action { Event::PUSHED } + action { :pushed } end factory :push_event_payload do diff --git a/spec/features/calendar_spec.rb b/spec/features/calendar_spec.rb index acdc38038aa..0d0912a9ed9 100644 --- a/spec/features/calendar_spec.rb +++ b/spec/features/calendar_spec.rb @@ -59,7 +59,7 @@ describe 'Contributions Calendar', :js do def note_comment_contribution note_comment_params = { project: contributed_project, - action: Event::COMMENTED, + action: :commented, target: issue_note, author_id: user.id } diff --git a/spec/features/dashboard/datetime_on_tooltips_spec.rb b/spec/features/dashboard/datetime_on_tooltips_spec.rb index 56b47b74626..42ec40f21da 100644 --- a/spec/features/dashboard/datetime_on_tooltips_spec.rb +++ b/spec/features/dashboard/datetime_on_tooltips_spec.rb @@ -12,7 +12,7 @@ describe 'Tooltips on .timeago dates', :js do before do project.add_maintainer(user) - Event.create( project: project, author_id: user.id, action: Event::JOINED, + Event.create( project: project, author_id: user.id, action: :joined, updated_at: created_date, created_at: created_date) sign_in user diff --git a/spec/features/dashboard/project_member_activity_index_spec.rb b/spec/features/dashboard/project_member_activity_index_spec.rb index 8e7a0b2a611..120c0357495 100644 --- a/spec/features/dashboard/project_member_activity_index_spec.rb +++ b/spec/features/dashboard/project_member_activity_index_spec.rb @@ -18,7 +18,7 @@ describe 'Project member activity', :js do context 'when a user joins the project' do before do - visit_activities_and_wait_with_event(Event::JOINED) + visit_activities_and_wait_with_event(:joined) end it "presents the correct message" do @@ -29,7 +29,7 @@ describe 'Project member activity', :js do context 'when a user leaves the project' do before do - visit_activities_and_wait_with_event(Event::LEFT) + visit_activities_and_wait_with_event(:left) end it "presents the correct message" do @@ -40,7 +40,7 @@ describe 'Project member activity', :js do context 'when a users membership expires for the project' do before do - visit_activities_and_wait_with_event(Event::EXPIRED) + visit_activities_and_wait_with_event(:expired) end it "presents the correct message" do diff --git a/spec/features/projects/members/group_members_spec.rb b/spec/features/projects/members/group_members_spec.rb index d37f912a2bc..0a0809206d2 100644 --- a/spec/features/projects/members/group_members_spec.rb +++ b/spec/features/projects/members/group_members_spec.rb @@ -21,7 +21,7 @@ describe 'Projects members' do context 'with a group invitee' do before do group_invitee - visit project_settings_members_path(project) + visit project_project_members_path(project) end it 'does not appear in the project members page' do @@ -70,7 +70,7 @@ describe 'Projects members' do before do group_invitee project_invitee - visit project_settings_members_path(project) + visit project_project_members_path(project) end it 'shows the project invitee, the project developer, and the group owner' do @@ -91,7 +91,7 @@ describe 'Projects members' do context 'with a group requester' do before do group.request_access(group_requester) - visit project_settings_members_path(project) + visit project_project_members_path(project) end it 'does not appear in the project members page' do @@ -105,7 +105,7 @@ describe 'Projects members' do before do group.request_access(group_requester) project.request_access(project_requester) - visit project_settings_members_path(project) + visit project_project_members_path(project) end it 'shows the project requester, the project developer, and the group owner' do @@ -129,7 +129,7 @@ describe 'Projects members' do it_behaves_like 'showing user status' do let(:user_with_status) { developer } - subject { visit project_settings_members_path(project) } + subject { visit project_project_members_path(project) } end end end diff --git a/spec/features/projects/members/groups_with_access_list_spec.rb b/spec/features/projects/members/groups_with_access_list_spec.rb index 6e8d1a945e1..90838eb1504 100644 --- a/spec/features/projects/members/groups_with_access_list_spec.rb +++ b/spec/features/projects/members/groups_with_access_list_spec.rb @@ -12,7 +12,7 @@ describe 'Projects > Members > Groups with access list', :js do @group_link = create(:project_group_link, project: project, group: group) sign_in(user) - visit project_settings_members_path(project) + visit project_project_members_path(project) end it 'updates group access level' do @@ -24,7 +24,7 @@ describe 'Projects > Members > Groups with access list', :js do wait_for_requests - visit project_settings_members_path(project) + visit project_project_members_path(project) expect(first('.group_member')).to have_content('Guest') end diff --git a/spec/features/projects/members/invite_group_spec.rb b/spec/features/projects/members/invite_group_spec.rb index e76637039c6..f6fc88d6403 100644 --- a/spec/features/projects/members/invite_group_spec.rb +++ b/spec/features/projects/members/invite_group_spec.rb @@ -11,14 +11,14 @@ describe 'Project > Members > Invite group', :js do describe 'Share with group lock' do shared_examples 'the project can be shared with groups' do it 'the "Invite group" tab exists' do - visit project_settings_members_path(project) + visit project_project_members_path(project) expect(page).to have_selector('#invite-group-tab') end end shared_examples 'the project cannot be shared with groups' do it 'the "Invite group" tab does not exist' do - visit project_settings_members_path(project) + visit project_project_members_path(project) expect(page).not_to have_selector('#invite-group-tab') end end @@ -37,7 +37,7 @@ describe 'Project > Members > Invite group', :js do it_behaves_like 'the project can be shared with groups' it 'the project can be shared with another group' do - visit project_settings_members_path(project) + visit project_project_members_path(project) click_on 'invite-group-tab' @@ -118,7 +118,7 @@ describe 'Project > Members > Invite group', :js do group.add_guest(maintainer) sign_in(maintainer) - visit project_settings_members_path(project) + visit project_project_members_path(project) click_on 'invite-group-tab' @@ -151,7 +151,7 @@ describe 'Project > Members > Invite group', :js do create(:group).add_owner(maintainer) create(:group).add_owner(maintainer) - visit project_settings_members_path(project) + visit project_project_members_path(project) click_link 'Invite group' @@ -184,7 +184,7 @@ describe 'Project > Members > Invite group', :js do end it 'the groups dropdown does not show ancestors' do - visit project_settings_members_path(project) + visit project_project_members_path(project) click_on 'invite-group-tab' click_link 'Search for a group' diff --git a/spec/features/projects/members/list_spec.rb b/spec/features/projects/members/list_spec.rb index f404699b2f6..99ab5e641ed 100644 --- a/spec/features/projects/members/list_spec.rb +++ b/spec/features/projects/members/list_spec.rb @@ -113,6 +113,6 @@ describe 'Project members list' do end def visit_members_page - visit project_settings_members_path(project) + visit project_project_members_path(project) end end diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index f29aa8de928..a6a3b427920 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -85,8 +85,8 @@ describe "Internal Project Access" do it { is_expected.to be_denied_for(:visitor) } end - describe "GET /:project_path/-/settings/members" do - subject { project_settings_members_path(project) } + describe "GET /:project_path/-/project_members" do + subject { project_project_members_path(project) } it { is_expected.to be_allowed_for(:admin) } it { is_expected.to be_allowed_for(:owner).of(project) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index ac8596d89bc..95c8b922bef 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -85,8 +85,8 @@ describe "Private Project Access" do it { is_expected.to be_denied_for(:visitor) } end - describe "GET /:project_path/-/settings/members" do - subject { project_settings_members_path(project) } + describe "GET /:project_path/-/project_members" do + subject { project_project_members_path(project) } it { is_expected.to be_allowed_for(:admin) } it { is_expected.to be_allowed_for(:owner).of(project) } diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index 11e9bff10a1..814fd83bf73 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -85,8 +85,8 @@ describe "Public Project Access" do it { is_expected.to be_allowed_for(:visitor) } end - describe "GET /:project_path/-/settings/members" do - subject { project_settings_members_path(project) } + describe "GET /:project_path/-/project_members" do + subject { project_project_members_path(project) } it { is_expected.to be_allowed_for(:admin) } it { is_expected.to be_allowed_for(:owner).of(project) } diff --git a/spec/finders/events_finder_spec.rb b/spec/finders/events_finder_spec.rb index 443e9ab4bc4..037377cf756 100644 --- a/spec/finders/events_finder_spec.rb +++ b/spec/finders/events_finder_spec.rb @@ -11,18 +11,18 @@ describe EventsFinder do let(:closed_issue) { create(:closed_issue, project: project1, author: user) } let(:opened_merge_request) { create(:merge_request, source_project: project2, author: user) } - let!(:closed_issue_event) { create(:event, project: project1, author: user, target: closed_issue, action: Event::CLOSED, created_at: Date.new(2016, 12, 30)) } - let!(:opened_merge_request_event) { create(:event, project: project2, author: user, target: opened_merge_request, action: Event::CREATED, created_at: Date.new(2017, 1, 31)) } + let!(:closed_issue_event) { create(:event, :closed, project: project1, author: user, target: closed_issue, created_at: Date.new(2016, 12, 30)) } + let!(:opened_merge_request_event) { create(:event, :created, project: project2, author: user, target: opened_merge_request, created_at: Date.new(2017, 1, 31)) } let(:closed_issue2) { create(:closed_issue, project: project1, author: user) } let(:opened_merge_request2) { create(:merge_request, source_project: project2, author: user) } - let!(:closed_issue_event2) { create(:event, project: project1, author: user, target: closed_issue, action: Event::CLOSED, created_at: Date.new(2016, 2, 2)) } - let!(:opened_merge_request_event2) { create(:event, project: project2, author: user, target: opened_merge_request, action: Event::CREATED, created_at: Date.new(2017, 2, 2)) } + let!(:closed_issue_event2) { create(:event, :closed, project: project1, author: user, target: closed_issue, created_at: Date.new(2016, 2, 2)) } + let!(:opened_merge_request_event2) { create(:event, :created, project: project2, author: user, target: opened_merge_request, created_at: Date.new(2017, 2, 2)) } let(:opened_merge_request3) { create(:merge_request, source_project: project1, author: other_user) } - let!(:other_developer_event) { create(:event, project: project1, author: other_user, target: opened_merge_request3, action: Event::CREATED) } + let!(:other_developer_event) { create(:event, :created, project: project1, author: other_user, target: opened_merge_request3 ) } let_it_be(:public_project) { create(:project, :public, creator_id: user.id, namespace: user.namespace) } let(:confidential_issue) { create(:closed_issue, confidential: true, project: public_project, author: user) } - let!(:confidential_event) { create(:event, project: public_project, author: user, target: confidential_issue, action: Event::CLOSED) } + let!(:confidential_event) { create(:event, :closed, project: public_project, author: user, target: confidential_issue) } context 'when targeting a user' do it 'returns events between specified dates filtered on action and type' do diff --git a/spec/graphql/resolvers/user_resolver_spec.rb b/spec/graphql/resolvers/user_resolver_spec.rb new file mode 100644 index 00000000000..1bfede41567 --- /dev/null +++ b/spec/graphql/resolvers/user_resolver_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Resolvers::UserResolver do + include GraphqlHelpers + + describe '#resolve' do + let_it_be(:user) { create(:user) } + + context 'when neither an ID or a username is provided' do + it 'raises an ArgumentError' do + expect { resolve_user } + .to raise_error(Gitlab::Graphql::Errors::ArgumentError) + end + end + + it 'raises an ArgumentError when both an ID and username are provided' do + expect { resolve_user(id: user.to_global_id, username: user.username) } + .to raise_error(Gitlab::Graphql::Errors::ArgumentError) + end + + context 'by username' do + it 'returns the correct user' do + expect( + resolve_user(username: user.username) + ).to eq(user) + end + end + + context 'by ID' do + it 'returns the correct user' do + expect( + resolve_user(id: user.to_global_id) + ).to eq(user) + end + end + end + + private + + def resolve_user(args = {}) + resolve(described_class, args: args) + end +end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb index 1f269a80d00..a23103b4e58 100644 --- a/spec/graphql/types/query_type_spec.rb +++ b/spec/graphql/types/query_type_spec.rb @@ -8,9 +8,24 @@ describe GitlabSchema.types['Query'] do end it 'has the expected fields' do - expected_fields = %i[project namespace group echo metadata current_user snippets design_management] + expected_fields = %i[ + current_user + design_management + geoNode + group + echo + instanceSecurityDashboard + metadata + namespace + project + projects + snippets + user + vulnerabilities + vulnerabilitiesCountByDayAndSeverity + ] - expect(described_class).to have_graphql_fields(*expected_fields).at_least + expect(described_class).to have_graphql_fields(*expected_fields) end describe 'namespace field' do diff --git a/spec/lib/gitlab/contributions_calendar_spec.rb b/spec/lib/gitlab/contributions_calendar_spec.rb index 1154f029a8d..97742a3e815 100644 --- a/spec/lib/gitlab/contributions_calendar_spec.rb +++ b/spec/lib/gitlab/contributions_calendar_spec.rb @@ -42,7 +42,7 @@ describe Gitlab::ContributionsCalendar do described_class.new(contributor, current_user) end - def create_event(project, day, hour = 0, action = Event::CREATED, target_symbol = :issue) + def create_event(project, day, hour = 0, action = :created, target_symbol = :issue) @targets ||= {} @targets[project] ||= create(target_symbol, project: project, author: contributor) @@ -77,14 +77,14 @@ describe Gitlab::ContributionsCalendar do end it "counts the diff notes on merge request" do - create_event(public_project, today, 0, Event::COMMENTED, :diff_note_on_merge_request) + create_event(public_project, today, 0, :commented, :diff_note_on_merge_request) expect(calendar(contributor).activity_dates[today]).to eq(1) end it "counts the discussions on merge requests and issues" do - create_event(public_project, today, 0, Event::COMMENTED, :discussion_note_on_merge_request) - create_event(public_project, today, 2, Event::COMMENTED, :discussion_note_on_issue) + create_event(public_project, today, 0, :commented, :discussion_note_on_merge_request) + create_event(public_project, today, 2, :commented, :discussion_note_on_issue) expect(calendar(contributor).activity_dates[today]).to eq(2) end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index a29c56c598f..55526241673 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -10,7 +10,7 @@ describe Gitlab::GitAccess do let(:actor) { user } let(:project) { create(:project, :repository) } - let(:project_path) { project.path } + let(:project_path) { project&.path } let(:namespace_path) { project&.namespace&.path } let(:protocol) { 'ssh' } let(:authentication_abilities) { %i[read_project download_code push_code] } @@ -89,13 +89,14 @@ describe Gitlab::GitAccess do end end - context 'when namespace does not exist' do + context 'when namespace and project are nil' do + let(:project) { nil } let(:namespace_path) { nil } it 'does not allow push and pull access' do aggregate_failures do - expect { push_access_check }.to raise_not_found - expect { pull_access_check }.to raise_not_found + expect { push_access_check }.to raise_namespace_not_found + expect { pull_access_check }.to raise_namespace_not_found end end end @@ -228,13 +229,6 @@ describe Gitlab::GitAccess do let(:project) { nil } let(:project_path) { "new-project" } - it 'blocks push and pull with "not found"' do - aggregate_failures do - expect { pull_access_check }.to raise_not_found - expect { push_access_check }.to raise_not_found - end - end - context 'when user is allowed to create project in namespace' do let(:namespace_path) { user.namespace.path } let(:access) do @@ -1219,6 +1213,10 @@ describe Gitlab::GitAccess do raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found]) end + def raise_namespace_not_found + raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:namespace_not_found]) + end + def build_authentication_abilities [ :read_project, diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 98d8de48dfb..fdb9457b211 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -46,12 +46,12 @@ describe ProjectMember do it "creates an expired event when left due to expiry" do expired = create(:project_member, project: project, expires_at: Time.current - 6.days) expired.destroy - expect(Event.recent.first.action).to eq(Event::EXPIRED) + expect(Event.recent.first).to be_expired_action end it "creates a left event when left due to leave" do maintainer.destroy - expect(Event.recent.first.action).to eq(Event::LEFT) + expect(Event.recent.first).to be_left_action end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 06061bcc1a1..33f84da27f6 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -225,70 +225,88 @@ describe Milestone do end end - describe '#for_projects_and_groups' do - let(:project) { create(:project) } - let(:project_other) { create(:project) } - let(:group) { create(:group) } - let(:group_other) { create(:group) } + shared_examples '#for_projects_and_groups' do + describe '#for_projects_and_groups' do + let_it_be(:project) { create(:project) } + let_it_be(:project_other) { create(:project) } + let_it_be(:group) { create(:group) } + let_it_be(:group_other) { create(:group) } + before(:all) do + create(:milestone, project: project) + create(:milestone, project: project_other) + create(:milestone, group: group) + create(:milestone, group: group_other) + end + + subject { described_class.for_projects_and_groups(projects, groups) } + + shared_examples 'filters by projects and groups' do + it 'returns milestones filtered by project' do + milestones = described_class.for_projects_and_groups(projects, []) + + expect(milestones.count).to eq(1) + expect(milestones.first.project_id).to eq(project.id) + end + + it 'returns milestones filtered by group' do + milestones = described_class.for_projects_and_groups([], groups) + + expect(milestones.count).to eq(1) + expect(milestones.first.group_id).to eq(group.id) + end + + it 'returns milestones filtered by both project and group' do + milestones = described_class.for_projects_and_groups(projects, groups) + + expect(milestones.count).to eq(2) + expect(milestones).to contain_exactly(project.milestones.first, group.milestones.first) + end + end + + context 'ids as params' do + let(:projects) { [project.id] } + let(:groups) { [group.id] } + + it_behaves_like 'filters by projects and groups' + end + + context 'relations as params' do + let(:projects) { Project.where(id: project.id).select(:id) } + let(:groups) { Group.where(id: group.id).select(:id) } + + it_behaves_like 'filters by projects and groups' + end + + context 'objects as params' do + let(:projects) { [project] } + let(:groups) { [group] } + + it_behaves_like 'filters by projects and groups' + end + + it 'returns no records if projects and groups are nil' do + milestones = described_class.for_projects_and_groups(nil, nil) + + expect(milestones).to be_empty + end + end + end + + context 'when `optimized_timebox_queries` feature flag is enabled' do before do - create(:milestone, project: project) - create(:milestone, project: project_other) - create(:milestone, group: group) - create(:milestone, group: group_other) + stub_feature_flags(optimized_timebox_queries: true) end - subject { described_class.for_projects_and_groups(projects, groups) } + it_behaves_like '#for_projects_and_groups' + end - shared_examples 'filters by projects and groups' do - it 'returns milestones filtered by project' do - milestones = described_class.for_projects_and_groups(projects, []) - - expect(milestones.count).to eq(1) - expect(milestones.first.project_id).to eq(project.id) - end - - it 'returns milestones filtered by group' do - milestones = described_class.for_projects_and_groups([], groups) - - expect(milestones.count).to eq(1) - expect(milestones.first.group_id).to eq(group.id) - end - - it 'returns milestones filtered by both project and group' do - milestones = described_class.for_projects_and_groups(projects, groups) - - expect(milestones.count).to eq(2) - expect(milestones).to contain_exactly(project.milestones.first, group.milestones.first) - end + context 'when `optimized_timebox_queries` feature flag is disabled' do + before do + stub_feature_flags(optimized_timebox_queries: false) end - context 'ids as params' do - let(:projects) { [project.id] } - let(:groups) { [group.id] } - - it_behaves_like 'filters by projects and groups' - end - - context 'relations as params' do - let(:projects) { Project.where(id: project.id).select(:id) } - let(:groups) { Group.where(id: group.id).select(:id) } - - it_behaves_like 'filters by projects and groups' - end - - context 'objects as params' do - let(:projects) { [project] } - let(:groups) { [group] } - - it_behaves_like 'filters by projects and groups' - end - - it 'returns no records if projects and groups are nil' do - milestones = described_class.for_projects_and_groups(nil, nil) - - expect(milestones).to be_empty - end + it_behaves_like '#for_projects_and_groups' end describe '.upcoming_ids' do diff --git a/spec/models/push_event_spec.rb b/spec/models/push_event_spec.rb index 8682e1c797b..5c1802669c1 100644 --- a/spec/models/push_event_spec.rb +++ b/spec/models/push_event_spec.rb @@ -118,8 +118,8 @@ describe PushEvent do end describe '.sti_name' do - it 'returns Event::PUSHED' do - expect(described_class.sti_name).to eq(Event::PUSHED) + it 'returns the integer representation of the :pushed event action' do + expect(described_class.sti_name).to eq(Event.actions[:pushed]) end end @@ -299,7 +299,7 @@ describe PushEvent do describe '#validate_push_action' do it 'adds an error when the action is not PUSHED' do - event.action = Event::CREATED + event.action = :created event.validate_push_action expect(event.errors.count).to eq(1) diff --git a/spec/models/user_interacted_project_spec.rb b/spec/models/user_interacted_project_spec.rb index e2c485343ae..75386e220f5 100644 --- a/spec/models/user_interacted_project_spec.rb +++ b/spec/models/user_interacted_project_spec.rb @@ -8,7 +8,7 @@ describe UserInteractedProject do let(:event) { build(:event) } - Event::ACTIONS.each do |action| + Event.actions.each_key do |action| context "for all actions (event types)" do let(:event) { build(:event, action: action) } diff --git a/spec/requests/api/events_spec.rb b/spec/requests/api/events_spec.rb index decdcc66327..0425e0791eb 100644 --- a/spec/requests/api/events_spec.rb +++ b/spec/requests/api/events_spec.rb @@ -7,9 +7,9 @@ describe API::Events do let(:non_member) { create(:user) } let(:private_project) { create(:project, :private, creator_id: user.id, namespace: user.namespace) } let(:closed_issue) { create(:closed_issue, project: private_project, author: user) } - let!(:closed_issue_event) { create(:event, project: private_project, author: user, target: closed_issue, action: Event::CLOSED, created_at: Date.new(2016, 12, 30)) } + let!(:closed_issue_event) { create(:event, :closed, project: private_project, author: user, target: closed_issue, created_at: Date.new(2016, 12, 30)) } let(:closed_issue2) { create(:closed_issue, project: private_project, author: non_member) } - let!(:closed_issue_event2) { create(:event, project: private_project, author: non_member, target: closed_issue2, action: Event::CLOSED, created_at: Date.new(2016, 12, 30)) } + let!(:closed_issue_event2) { create(:event, :closed, project: private_project, author: non_member, target: closed_issue2, created_at: Date.new(2016, 12, 30)) } describe 'GET /events' do context 'when unauthenticated' do @@ -117,7 +117,7 @@ describe API::Events do context 'when the list of events includes wiki page events' do it 'returns information about the wiki event', :aggregate_failures do page = create(:wiki_page, project: private_project) - [Event::CREATED, Event::UPDATED, Event::DESTROYED].each do |action| + [:created, :updated, :destroyed].each do |action| create(:wiki_page_event, wiki_page: page, action: action, author: user) end diff --git a/spec/requests/api/graphql/user_spec.rb b/spec/requests/api/graphql/user_spec.rb new file mode 100644 index 00000000000..097c75b3541 --- /dev/null +++ b/spec/requests/api/graphql/user_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'User' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + + shared_examples 'a working user query' do + it_behaves_like 'a working graphql query' do + before do + post_graphql(query, current_user: current_user) + end + end + + it 'includes the user' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['user']).not_to be_nil + end + + it 'returns no user when global restricted_visibility_levels includes PUBLIC' do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) + + post_graphql(query) + + expect(graphql_data['user']).to be_nil + end + end + + context 'when id parameter is used' do + let(:query) { graphql_query_for(:user, { id: current_user.to_global_id.to_s }) } + + it_behaves_like 'a working user query' + end + + context 'when username parameter is used' do + let(:query) { graphql_query_for(:user, { username: current_user.username.to_s }) } + + it_behaves_like 'a working user query' + end + + context 'when username and id parameter are used' do + let_it_be(:query) { graphql_query_for(:user, { id: current_user.to_global_id.to_s, username: current_user.username }, 'id') } + + it 'displays an error' do + post_graphql(query) + + expect(graphql_errors).to include( + a_hash_including('message' => a_string_matching(%r{Provide either a single username or id})) + ) + end + end +end diff --git a/spec/requests/api/project_events_spec.rb b/spec/requests/api/project_events_spec.rb index 489b83de434..f65c62f9402 100644 --- a/spec/requests/api/project_events_spec.rb +++ b/spec/requests/api/project_events_spec.rb @@ -7,7 +7,7 @@ describe API::ProjectEvents do let(:non_member) { create(:user) } let(:private_project) { create(:project, :private, creator_id: user.id, namespace: user.namespace) } let(:closed_issue) { create(:closed_issue, project: private_project, author: user) } - let!(:closed_issue_event) { create(:event, project: private_project, author: user, target: closed_issue, action: Event::CLOSED, created_at: Date.new(2016, 12, 30)) } + let!(:closed_issue_event) { create(:event, project: private_project, author: user, target: closed_issue, action: :closed, created_at: Date.new(2016, 12, 30)) } describe 'GET /projects/:id/events' do context 'when unauthenticated ' do @@ -29,9 +29,9 @@ describe API::ProjectEvents do context 'with inaccessible events' do let(:public_project) { create(:project, :public, creator_id: user.id, namespace: user.namespace) } let(:confidential_issue) { create(:closed_issue, confidential: true, project: public_project, author: user) } - let!(:confidential_event) { create(:event, project: public_project, author: user, target: confidential_issue, action: Event::CLOSED) } + let!(:confidential_event) { create(:event, project: public_project, author: user, target: confidential_issue, action: :closed) } let(:public_issue) { create(:closed_issue, project: public_project, author: user) } - let!(:public_event) { create(:event, project: public_project, author: user, target: public_issue, action: Event::CLOSED) } + let!(:public_event) { create(:event, project: public_project, author: user, target: public_issue, action: :closed) } it 'returns only accessible events' do get api("/projects/#{public_project.id}/events", non_member) @@ -53,19 +53,19 @@ describe API::ProjectEvents do before do create(:event, + :closed, project: public_project, target: create(:issue, project: public_project, title: 'Issue 1'), - action: Event::CLOSED, created_at: Date.parse('2018-12-10')) create(:event, + :closed, project: public_project, target: create(:issue, confidential: true, project: public_project, title: 'Confidential event'), - action: Event::CLOSED, created_at: Date.parse('2018-12-11')) create(:event, + :closed, project: public_project, target: create(:issue, project: public_project, title: 'Issue 2'), - action: Event::CLOSED, created_at: Date.parse('2018-12-12')) end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 05d2e4bb5a2..e28236b9600 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -3,21 +3,18 @@ require 'spec_helper' describe API::Users, :do_not_mock_admin_mode do - let(:user) { create(:user, username: 'user.with.dot') } - let(:admin) { create(:admin) } - let(:key) { create(:key, user: user) } - let(:gpg_key) { create(:gpg_key, user: user) } - let(:email) { create(:email, user: user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:user, reload: true) { create(:user, username: 'user.with.dot') } + let_it_be(:key) { create(:key, user: user) } + let_it_be(:gpg_key) { create(:gpg_key, user: user) } + let_it_be(:email) { create(:email, user: user) } let(:omniauth_user) { create(:omniauth_user) } - let(:ldap_user) { create(:omniauth_user, provider: 'ldapmain') } let(:ldap_blocked_user) { create(:omniauth_user, provider: 'ldapmain', state: 'ldap_blocked') } - let(:not_existing_user_id) { (User.maximum('id') || 0 ) + 10 } - let(:not_existing_pat_id) { (PersonalAccessToken.maximum('id') || 0 ) + 10 } let(:private_user) { create(:user, private_profile: true) } context 'admin notes' do - let(:admin) { create(:admin, note: '2019-10-06 | 2FA added | user requested | www.gitlab.com') } - let(:user) { create(:user, note: '2018-11-05 | 2FA removed | user requested | www.gitlab.com') } + let_it_be(:admin) { create(:admin, note: '2019-10-06 | 2FA added | user requested | www.gitlab.com') } + let_it_be(:user, reload: true) { create(:user, note: '2018-11-05 | 2FA removed | user requested | www.gitlab.com') } describe 'POST /users' do context 'when unauthenticated' do @@ -428,8 +425,9 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns the correct order when sorted by id' do - admin - user + # order of let_it_be definitions: + # - admin + # - user get api('/users', admin), params: { order_by: 'id', sort: 'asc' } @@ -440,8 +438,6 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns users with 2fa enabled' do - admin - user user_with_2fa = create(:user, :two_factor_via_otp) get api('/users', admin), params: { two_factor: 'enabled' } @@ -663,10 +659,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe "POST /users" do - before do - admin - end - it "creates user" do expect do post api("/users", admin), params: attributes_for(:user, projects_limit: 3) @@ -905,8 +897,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe "PUT /users/:id" do - let_it_be(:admin_user) { create(:admin) } - it "returns 200 OK on success" do put api("/users/#{user.id}", admin), params: { bio: 'new test bio' } @@ -923,8 +913,7 @@ describe API::Users, :do_not_mock_admin_mode do end it "updates user with empty bio" do - user.bio = 'previous bio' - user.save! + user.update!(bio: 'previous bio') put api("/users/#{user.id}", admin), params: { bio: '' } @@ -1041,7 +1030,7 @@ describe API::Users, :do_not_mock_admin_mode do end it "updates private profile to false when nil is given" do - user.update(private_profile: true) + user.update!(private_profile: true) put api("/users/#{user.id}", admin), params: { private_profile: nil } @@ -1050,7 +1039,7 @@ describe API::Users, :do_not_mock_admin_mode do end it "does not modify private profile when field is not provided" do - user.update(private_profile: true) + user.update!(private_profile: true) put api("/users/#{user.id}", admin), params: {} @@ -1062,7 +1051,7 @@ describe API::Users, :do_not_mock_admin_mode do theme = Gitlab::Themes.each.find { |t| t.id != Gitlab::Themes.default.id } scheme = Gitlab::ColorSchemes.each.find { |t| t.id != Gitlab::ColorSchemes.default.id } - user.update(theme_id: theme.id, color_scheme_id: scheme.id) + user.update!(theme_id: theme.id, color_scheme_id: scheme.id) put api("/users/#{user.id}", admin), params: {} @@ -1072,6 +1061,8 @@ describe API::Users, :do_not_mock_admin_mode do end it "does not update admin status" do + admin_user = create(:admin) + put api("/users/#{admin_user.id}", admin), params: { can_create_group: false } expect(response).to have_gitlab_http_status(:ok) @@ -1242,10 +1233,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe "POST /users/:id/keys" do - before do - admin - end - it "does not create invalid ssh key" do post api("/users/#{user.id}/keys", admin), params: { title: "invalid key" } @@ -1275,9 +1262,7 @@ describe API::Users, :do_not_mock_admin_mode do describe 'GET /user/:id/keys' do it 'returns 404 for non-existing user' do - user_id = not_existing_user_id - - get api("/users/#{user_id}/keys") + get api("/users/#{non_existing_record_id}/keys") expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') @@ -1285,7 +1270,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns array of ssh keys' do user.keys << key - user.save get api("/users/#{user.id}/keys") @@ -1298,7 +1282,7 @@ describe API::Users, :do_not_mock_admin_mode do describe 'GET /user/:user_id/keys' do it 'returns 404 for non-existing user' do - get api("/users/#{not_existing_user_id}/keys") + get api("/users/#{non_existing_record_id}/keys") expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') @@ -1306,7 +1290,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns array of ssh keys' do user.keys << key - user.save get api("/users/#{user.username}/keys") @@ -1318,13 +1301,9 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'DELETE /user/:id/keys/:key_id' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do - delete api("/users/#{user.id}/keys/42") + delete api("/users/#{user.id}/keys/#{non_existing_record_id}") expect(response).to have_gitlab_http_status(:unauthorized) end end @@ -1332,7 +1311,6 @@ describe API::Users, :do_not_mock_admin_mode do context 'when authenticated' do it 'deletes existing key' do user.keys << key - user.save expect do delete api("/users/#{user.id}/keys/#{key.id}", admin) @@ -1347,14 +1325,14 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 404 error if user not found' do user.keys << key - user.save + delete api("/users/0/keys/#{key.id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end it 'returns 404 error if key not foud' do - delete api("/users/#{user.id}/keys/42", admin) + delete api("/users/#{user.id}/keys/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Key Not Found') end @@ -1362,10 +1340,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'POST /users/:id/keys' do - before do - admin - end - it 'does not create invalid GPG key' do post api("/users/#{user.id}/gpg_keys", admin) @@ -1374,7 +1348,8 @@ describe API::Users, :do_not_mock_admin_mode do end it 'creates GPG key' do - key_attrs = attributes_for :gpg_key + key_attrs = attributes_for :gpg_key, key: GpgHelpers::User2.public_key + expect do post api("/users/#{user.id}/gpg_keys", admin), params: key_attrs @@ -1390,10 +1365,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'GET /user/:id/gpg_keys' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do get api("/users/#{user.id}/gpg_keys") @@ -1411,7 +1382,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 error if key not foud' do - delete api("/users/#{user.id}/gpg_keys/42", admin) + delete api("/users/#{user.id}/gpg_keys/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -1419,7 +1390,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns array of GPG keys' do user.gpg_keys << gpg_key - user.save get api("/users/#{user.id}/gpg_keys", admin) @@ -1432,13 +1402,9 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'DELETE /user/:id/gpg_keys/:key_id' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do - delete api("/users/#{user.id}/keys/42") + delete api("/users/#{user.id}/keys/#{non_existing_record_id}") expect(response).to have_gitlab_http_status(:unauthorized) end @@ -1447,7 +1413,6 @@ describe API::Users, :do_not_mock_admin_mode do context 'when authenticated' do it 'deletes existing key' do user.gpg_keys << gpg_key - user.save expect do delete api("/users/#{user.id}/gpg_keys/#{gpg_key.id}", admin) @@ -1458,7 +1423,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 404 error if user not found' do user.keys << key - user.save delete api("/users/0/gpg_keys/#{gpg_key.id}", admin) @@ -1467,7 +1431,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 error if key not foud' do - delete api("/users/#{user.id}/gpg_keys/42", admin) + delete api("/users/#{user.id}/gpg_keys/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -1476,13 +1440,9 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'POST /user/:id/gpg_keys/:key_id/revoke' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do - post api("/users/#{user.id}/gpg_keys/42/revoke") + post api("/users/#{user.id}/gpg_keys/#{non_existing_record_id}/revoke") expect(response).to have_gitlab_http_status(:unauthorized) end @@ -1491,7 +1451,6 @@ describe API::Users, :do_not_mock_admin_mode do context 'when authenticated' do it 'revokes existing key' do user.gpg_keys << gpg_key - user.save expect do post api("/users/#{user.id}/gpg_keys/#{gpg_key.id}/revoke", admin) @@ -1502,7 +1461,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 404 error if user not found' do user.gpg_keys << gpg_key - user.save post api("/users/0/gpg_keys/#{gpg_key.id}/revoke", admin) @@ -1511,7 +1469,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 error if key not foud' do - post api("/users/#{user.id}/gpg_keys/42/revoke", admin) + post api("/users/#{user.id}/gpg_keys/#{non_existing_record_id}/revoke", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -1520,10 +1478,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe "POST /users/:id/emails" do - before do - admin - end - it "does not create invalid email" do post api("/users/#{user.id}/emails", admin), params: {} @@ -1561,10 +1515,6 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'GET /user/:id/emails' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do get api("/users/#{user.id}/emails") @@ -1581,7 +1531,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns array of emails' do user.emails << email - user.save get api("/users/#{user.id}/emails", admin) @@ -1600,13 +1549,9 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'DELETE /user/:id/emails/:email_id' do - before do - admin - end - context 'when unauthenticated' do it 'returns authentication error' do - delete api("/users/#{user.id}/emails/42") + delete api("/users/#{user.id}/emails/#{non_existing_record_id}") expect(response).to have_gitlab_http_status(:unauthorized) end end @@ -1614,7 +1559,6 @@ describe API::Users, :do_not_mock_admin_mode do context 'when authenticated' do it 'deletes existing email' do user.emails << email - user.save expect do delete api("/users/#{user.id}/emails/#{email.id}", admin) @@ -1629,14 +1573,14 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 404 error if user not found' do user.emails << email - user.save + delete api("/users/0/emails/#{email.id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end it 'returns 404 error if email not foud' do - delete api("/users/#{user.id}/emails/42", admin) + delete api("/users/#{user.id}/emails/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Email Not Found') end @@ -1650,19 +1594,16 @@ describe API::Users, :do_not_mock_admin_mode do end describe "DELETE /users/:id" do - let!(:namespace) { user.namespace } - let!(:issue) { create(:issue, author: user) } - - before do - admin - end + let_it_be(:issue) { create(:issue, author: user) } it "deletes user", :sidekiq_might_not_need_inline do + namespace_id = user.namespace.id + perform_enqueued_jobs { delete api("/users/#{user.id}", admin) } expect(response).to have_gitlab_http_status(:no_content) expect { User.find(user.id) }.to raise_error ActiveRecord::RecordNotFound - expect { Namespace.find(namespace.id) }.to raise_error ActiveRecord::RecordNotFound + expect { Namespace.find(namespace_id) }.to raise_error ActiveRecord::RecordNotFound end context "sole owner of a group" do @@ -1731,11 +1672,11 @@ describe API::Users, :do_not_mock_admin_mode do end describe "GET /user" do - let(:personal_access_token) { create(:personal_access_token, user: user).token } - shared_examples 'get user info' do |version| context 'with regular user' do context 'with personal access token' do + let(:personal_access_token) { create(:personal_access_token, user: user).token } + it 'returns 403 without private token when sudo is defined' do get api("/user?private_token=#{personal_access_token}&sudo=123", version: version) @@ -1803,7 +1744,6 @@ describe API::Users, :do_not_mock_admin_mode do context "when authenticated" do it "returns array of ssh keys" do user.keys << key - user.save get api("/user/keys", user) @@ -1825,14 +1765,14 @@ describe API::Users, :do_not_mock_admin_mode do describe "GET /user/keys/:key_id" do it "returns single key" do user.keys << key - user.save + get api("/user/keys/#{key.id}", user) expect(response).to have_gitlab_http_status(:ok) expect(json_response["title"]).to eq(key.title) end it "returns 404 Not Found within invalid ID" do - get api("/user/keys/42", user) + get api("/user/keys/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Key Not Found') @@ -1840,8 +1780,8 @@ describe API::Users, :do_not_mock_admin_mode do it "returns 404 error if admin accesses user's ssh key" do user.keys << key - user.save admin + get api("/user/keys/#{key.id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Key Not Found') @@ -1898,7 +1838,6 @@ describe API::Users, :do_not_mock_admin_mode do describe "DELETE /user/keys/:key_id" do it "deletes existed key" do user.keys << key - user.save expect do delete api("/user/keys/#{key.id}", user) @@ -1912,7 +1851,7 @@ describe API::Users, :do_not_mock_admin_mode do end it "returns 404 if key ID not found" do - delete api("/user/keys/42", user) + delete api("/user/keys/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Key Not Found') @@ -1920,7 +1859,7 @@ describe API::Users, :do_not_mock_admin_mode do it "returns 401 error if unauthorized" do user.keys << key - user.save + delete api("/user/keys/#{key.id}") expect(response).to have_gitlab_http_status(:unauthorized) end @@ -1944,7 +1883,6 @@ describe API::Users, :do_not_mock_admin_mode do context 'when authenticated' do it 'returns array of GPG keys' do user.gpg_keys << gpg_key - user.save get api('/user/gpg_keys', user) @@ -1966,7 +1904,6 @@ describe API::Users, :do_not_mock_admin_mode do describe 'GET /user/gpg_keys/:key_id' do it 'returns a single key' do user.gpg_keys << gpg_key - user.save get api("/user/gpg_keys/#{gpg_key.id}", user) @@ -1975,7 +1912,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 Not Found within invalid ID' do - get api('/user/gpg_keys/42', user) + get api("/user/gpg_keys/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -1983,7 +1920,6 @@ describe API::Users, :do_not_mock_admin_mode do it "returns 404 error if admin accesses user's GPG key" do user.gpg_keys << gpg_key - user.save get api("/user/gpg_keys/#{gpg_key.id}", admin) @@ -2007,7 +1943,8 @@ describe API::Users, :do_not_mock_admin_mode do describe 'POST /user/gpg_keys' do it 'creates a GPG key' do - key_attrs = attributes_for :gpg_key + key_attrs = attributes_for :gpg_key, key: GpgHelpers::User2.public_key + expect do post api('/user/gpg_keys', user), params: key_attrs @@ -2032,7 +1969,6 @@ describe API::Users, :do_not_mock_admin_mode do describe 'POST /user/gpg_keys/:key_id/revoke' do it 'revokes existing GPG key' do user.gpg_keys << gpg_key - user.save expect do post api("/user/gpg_keys/#{gpg_key.id}/revoke", user) @@ -2042,7 +1978,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 if key ID not found' do - post api('/user/gpg_keys/42/revoke', user) + post api("/user/gpg_keys/#{non_existing_record_id}/revoke", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -2050,7 +1986,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 401 error if unauthorized' do user.gpg_keys << gpg_key - user.save post api("/user/gpg_keys/#{gpg_key.id}/revoke") @@ -2067,7 +2002,6 @@ describe API::Users, :do_not_mock_admin_mode do describe 'DELETE /user/gpg_keys/:key_id' do it 'deletes existing GPG key' do user.gpg_keys << gpg_key - user.save expect do delete api("/user/gpg_keys/#{gpg_key.id}", user) @@ -2077,7 +2011,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns 404 if key ID not found' do - delete api('/user/gpg_keys/42', user) + delete api("/user/gpg_keys/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 GPG Key Not Found') @@ -2085,7 +2019,6 @@ describe API::Users, :do_not_mock_admin_mode do it 'returns 401 error if unauthorized' do user.gpg_keys << gpg_key - user.save delete api("/user/gpg_keys/#{gpg_key.id}") @@ -2110,7 +2043,6 @@ describe API::Users, :do_not_mock_admin_mode do context "when authenticated" do it "returns array of emails" do user.emails << email - user.save get api("/user/emails", user) @@ -2132,22 +2064,22 @@ describe API::Users, :do_not_mock_admin_mode do describe "GET /user/emails/:email_id" do it "returns single email" do user.emails << email - user.save + get api("/user/emails/#{email.id}", user) expect(response).to have_gitlab_http_status(:ok) expect(json_response["email"]).to eq(email.email) end it "returns 404 Not Found within invalid ID" do - get api("/user/emails/42", user) + get api("/user/emails/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Email Not Found') end it "returns 404 error if admin accesses user's email" do user.emails << email - user.save admin + get api("/user/emails/#{email.id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Email Not Found') @@ -2192,7 +2124,6 @@ describe API::Users, :do_not_mock_admin_mode do describe "DELETE /user/emails/:email_id" do it "deletes existed email" do user.emails << email - user.save expect do delete api("/user/emails/#{email.id}", user) @@ -2206,7 +2137,7 @@ describe API::Users, :do_not_mock_admin_mode do end it "returns 404 if email ID not found" do - delete api("/user/emails/42", user) + delete api("/user/emails/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Email Not Found') @@ -2214,7 +2145,7 @@ describe API::Users, :do_not_mock_admin_mode do it "returns 401 error if unauthorized" do user.emails << email - user.save + delete api("/user/emails/#{email.id}") expect(response).to have_gitlab_http_status(:unauthorized) end @@ -2320,7 +2251,7 @@ describe API::Users, :do_not_mock_admin_mode do context 'performed by an admin user' do context 'for an active user' do let(:activity) { {} } - let(:user) { create(:user, username: 'user.with.dot', **activity) } + let(:user) { create(:user, **activity) } context 'with no recent activity' do let(:activity) { { last_activity_on: ::User::MINIMUM_INACTIVE_DAYS.next.days.ago } } @@ -2405,10 +2336,6 @@ describe API::Users, :do_not_mock_admin_mode do describe 'POST /users/:id/block' do let(:blocked_user) { create(:user, state: 'blocked') } - before do - admin - end - it 'blocks existing user' do post api("/users/#{user.id}/block", admin) @@ -2451,10 +2378,6 @@ describe API::Users, :do_not_mock_admin_mode do let(:blocked_user) { create(:user, state: 'blocked') } let(:deactivated_user) { create(:user, state: 'deactivated') } - before do - admin - end - it 'unblocks existing user' do post api("/users/#{user.id}/unblock", admin) expect(response).to have_gitlab_http_status(:created) @@ -2648,21 +2571,21 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'GET /users/:user_id/impersonation_tokens' do - let!(:active_personal_access_token) { create(:personal_access_token, user: user) } - let!(:revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user) } - let!(:expired_personal_access_token) { create(:personal_access_token, :expired, user: user) } - let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } - let!(:revoked_impersonation_token) { create(:personal_access_token, :impersonation, :revoked, user: user) } + let_it_be(:active_personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:revoked_personal_access_token) { create(:personal_access_token, :revoked, user: user) } + let_it_be(:expired_personal_access_token) { create(:personal_access_token, :expired, user: user) } + let_it_be(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + let_it_be(:revoked_impersonation_token) { create(:personal_access_token, :impersonation, :revoked, user: user) } it 'returns a 404 error if user not found' do - get api("/users/#{not_existing_user_id}/impersonation_tokens", admin) + get api("/users/#{non_existing_record_id}/impersonation_tokens", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end it 'returns a 403 error when authenticated as normal user' do - get api("/users/#{not_existing_user_id}/impersonation_tokens", user) + get api("/users/#{non_existing_record_id}/impersonation_tokens", user) expect(response).to have_gitlab_http_status(:forbidden) expect(json_response['message']).to eq('403 Forbidden') @@ -2711,7 +2634,7 @@ describe API::Users, :do_not_mock_admin_mode do end it 'returns a 404 error if user not found' do - post api("/users/#{not_existing_user_id}/impersonation_tokens", admin), + post api("/users/#{non_existing_record_id}/impersonation_tokens", admin), params: { name: name, expires_at: expires_at @@ -2755,18 +2678,18 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'GET /users/:user_id/impersonation_tokens/:impersonation_token_id' do - let!(:personal_access_token) { create(:personal_access_token, user: user) } - let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } it 'returns 404 error if user not found' do - get api("/users/#{not_existing_user_id}/impersonation_tokens/1", admin) + get api("/users/#{non_existing_record_id}/impersonation_tokens/1", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end it 'returns a 404 error if impersonation token not found' do - get api("/users/#{user.id}/impersonation_tokens/#{not_existing_pat_id}", admin) + get api("/users/#{user.id}/impersonation_tokens/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Impersonation Token Not Found') @@ -2796,18 +2719,18 @@ describe API::Users, :do_not_mock_admin_mode do end describe 'DELETE /users/:user_id/impersonation_tokens/:impersonation_token_id' do - let!(:personal_access_token) { create(:personal_access_token, user: user) } - let!(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:impersonation_token) { create(:personal_access_token, :impersonation, user: user) } it 'returns a 404 error if user not found' do - delete api("/users/#{not_existing_user_id}/impersonation_tokens/1", admin) + delete api("/users/#{non_existing_record_id}/impersonation_tokens/1", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 User Not Found') end it 'returns a 404 error if impersonation token not found' do - delete api("/users/#{user.id}/impersonation_tokens/#{not_existing_pat_id}", admin) + delete api("/users/#{user.id}/impersonation_tokens/#{non_existing_record_id}", admin) expect(response).to have_gitlab_http_status(:not_found) expect(json_response['message']).to eq('404 Impersonation Token Not Found') diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index f8e1ccac912..6150a9637c7 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -482,8 +482,6 @@ describe 'project routing' do let(:controller) { 'project_members' } let(:controller_path) { '/-/project_members' } end - - it_behaves_like 'redirecting a legacy project path', "/gitlab/gitlabhq/-/settings/members", "/gitlab/gitlabhq/-/project_members" end # project_milestones GET /:project_id/milestones(.:format) milestones#index diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index 282f2485f3e..a43678626f7 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -174,7 +174,7 @@ describe EventCreateService do wiki_page?: true, valid?: true, persisted?: true, - action: action, + action: action.to_s, wiki_page: wiki_page, author: user ) @@ -200,7 +200,7 @@ describe EventCreateService do end end - (Event::ACTIONS.values - Event::WIKI_ACTIONS).each do |bad_action| + (Event.actions.keys - Event::WIKI_ACTIONS).each do |bad_action| context "The action is #{bad_action}" do it 'raises an error' do expect { service.wiki_event(meta, user, bad_action) }.to raise_error(described_class::IllegalActionError) diff --git a/spec/services/git/branch_hooks_service_spec.rb b/spec/services/git/branch_hooks_service_spec.rb index ae0506ad442..908b9772c40 100644 --- a/spec/services/git/branch_hooks_service_spec.rb +++ b/spec/services/git/branch_hooks_service_spec.rb @@ -91,7 +91,7 @@ describe Git::BranchHooksService do end describe 'Push Event' do - let(:event) { Event.find_by_action(Event::PUSHED) } + let(:event) { Event.pushed_action.first } before do service.execute @@ -101,7 +101,7 @@ describe Git::BranchHooksService do it 'generates a push event with one commit' do expect(event).to be_an_instance_of(PushEvent) expect(event.project).to eq(project) - expect(event.action).to eq(Event::PUSHED) + expect(event).to be_pushed_action expect(event.push_event_payload).to be_an_instance_of(PushEventPayload) expect(event.push_event_payload.commit_from).to eq(oldrev) expect(event.push_event_payload.commit_to).to eq(newrev) @@ -117,7 +117,7 @@ describe Git::BranchHooksService do it 'generates a push event with more than one commit' do expect(event).to be_an_instance_of(PushEvent) expect(event.project).to eq(project) - expect(event.action).to eq(Event::PUSHED) + expect(event).to be_pushed_action expect(event.push_event_payload).to be_an_instance_of(PushEventPayload) expect(event.push_event_payload.commit_from).to be_nil expect(event.push_event_payload.commit_to).to eq(newrev) @@ -133,7 +133,7 @@ describe Git::BranchHooksService do it 'generates a push event with no commits' do expect(event).to be_an_instance_of(PushEvent) expect(event.project).to eq(project) - expect(event.action).to eq(Event::PUSHED) + expect(event).to be_pushed_action expect(event.push_event_payload).to be_an_instance_of(PushEventPayload) expect(event.push_event_payload.commit_from).to eq(oldrev) expect(event.push_event_payload.commit_to).to be_nil diff --git a/spec/services/git/wiki_push_service/change_spec.rb b/spec/services/git/wiki_push_service/change_spec.rb index 547874270ab..4da3f0fc738 100644 --- a/spec/services/git/wiki_push_service/change_spec.rb +++ b/spec/services/git/wiki_push_service/change_spec.rb @@ -89,20 +89,20 @@ describe Git::WikiPushService::Change do context 'the page is deleted' do let(:operation) { :deleted } - it { is_expected.to have_attributes(event_action: Event::DESTROYED) } + it { is_expected.to have_attributes(event_action: :destroyed) } end context 'the page is added' do let(:operation) { :added } - it { is_expected.to have_attributes(event_action: Event::CREATED) } + it { is_expected.to have_attributes(event_action: :created) } end %i[renamed modified].each do |op| context "the page is #{op}" do let(:operation) { op } - it { is_expected.to have_attributes(event_action: Event::UPDATED) } + it { is_expected.to have_attributes(event_action: :updated) } end end end diff --git a/spec/services/git/wiki_push_service_spec.rb b/spec/services/git/wiki_push_service_spec.rb index cdb1dc5a435..b2234c81c24 100644 --- a/spec/services/git/wiki_push_service_spec.rb +++ b/spec/services/git/wiki_push_service_spec.rb @@ -54,7 +54,7 @@ describe Git::WikiPushService, services: true do it 'handles all known actions' do run_service - expect(Event.last(count).pluck(:action)).to match_array(Event::WIKI_ACTIONS) + expect(Event.last(count).pluck(:action)).to match_array(Event::WIKI_ACTIONS.map(&:to_s)) end end @@ -77,7 +77,7 @@ describe Git::WikiPushService, services: true do it 'creates appropriate events' do run_service - expect(Event.last(2)).to all(have_attributes(wiki_page?: true, action: Event::CREATED)) + expect(Event.last(2)).to all(have_attributes(wiki_page?: true, action: 'created')) end end @@ -100,7 +100,7 @@ describe Git::WikiPushService, services: true do it 'creates a wiki page creation event' do expect { run_service }.to change(Event, :count).by(1) - expect(Event.last).to have_attributes(wiki_page?: true, action: Event::CREATED) + expect(Event.last).to have_attributes(wiki_page?: true, action: 'created') end it 'creates one metadata record' do @@ -129,7 +129,7 @@ describe Git::WikiPushService, services: true do expect(Event.last).to have_attributes( wiki_page?: true, - action: Event::CREATED + action: 'created' ) end end @@ -158,7 +158,7 @@ describe Git::WikiPushService, services: true do expect(Event.last).to have_attributes( wiki_page?: true, - action: Event::UPDATED + action: 'updated' ) end end @@ -182,7 +182,7 @@ describe Git::WikiPushService, services: true do expect(Event.last).to have_attributes( wiki_page?: true, - action: Event::UPDATED + action: 'updated' ) end end @@ -206,7 +206,7 @@ describe Git::WikiPushService, services: true do expect(Event.last).to have_attributes( wiki_page?: true, - action: Event::DESTROYED + action: 'destroyed' ) end end @@ -218,7 +218,7 @@ describe Git::WikiPushService, services: true do message = 'something went very very wrong' allow_next_instance_of(WikiPages::EventCreateService, current_user) do |service| allow(service).to receive(:execute) - .with(String, WikiPage, Integer) + .with(String, WikiPage, Symbol) .and_return(ServiceResponse.error(message: message)) end diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 9155db16d17..6655728d939 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -59,7 +59,7 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do it 'creates exactly 1 create MR event', :sidekiq_might_not_need_inline do attributes = { - action: Event::CREATED, + action: :created, target_id: merge_request.id, target_type: merge_request.class.name } diff --git a/spec/services/wiki_pages/event_create_service_spec.rb b/spec/services/wiki_pages/event_create_service_spec.rb index cf971b0a02c..c725c67d7a7 100644 --- a/spec/services/wiki_pages/event_create_service_spec.rb +++ b/spec/services/wiki_pages/event_create_service_spec.rb @@ -11,7 +11,7 @@ describe WikiPages::EventCreateService do describe '#execute' do let_it_be(:page) { create(:wiki_page, project: project) } let(:slug) { generate(:sluggified_title) } - let(:action) { Event::CREATED } + let(:action) { :created } let(:response) { subject.execute(slug, page, action) } context 'feature flag is not enabled' do @@ -38,7 +38,7 @@ describe WikiPages::EventCreateService do end context 'the action is illegal' do - let(:action) { Event::WIKI_ACTIONS.max + 1 } + let(:action) { :illegal_action } it 'returns an error' do expect(response).to be_error @@ -58,7 +58,7 @@ describe WikiPages::EventCreateService do end context 'the action is a deletion' do - let(:action) { Event::DESTROYED } + let(:action) { :destroyed } it 'does not synchronize the wiki metadata timestamps with the git commit' do expect_next_instance_of(WikiPage::Meta) do |instance| @@ -74,7 +74,7 @@ describe WikiPages::EventCreateService do end it 'returns an event in the payload' do - expect(response.payload).to include(event: have_attributes(author: user, wiki_page?: true, action: action)) + expect(response.payload).to include(event: have_attributes(author: user, wiki_page?: true, action: 'created')) end it 'records the slug for the page' do diff --git a/spec/support/shared_contexts/navbar_structure_context.rb b/spec/support/shared_contexts/navbar_structure_context.rb index fe3c32ec0c5..79b5ff44d4f 100644 --- a/spec/support/shared_contexts/navbar_structure_context.rb +++ b/spec/support/shared_contexts/navbar_structure_context.rb @@ -79,11 +79,14 @@ RSpec.shared_context 'project navbar structure' do nav_item: _('Snippets'), nav_sub_items: [] }, + { + nav_item: _('Members'), + nav_sub_items: [] + }, { nav_item: _('Settings'), nav_sub_items: [ _('General'), - _('Members'), _('Integrations'), _('Webhooks'), _('Access Tokens'), diff --git a/spec/support/shared_examples/services/wiki_pages/create_service_shared_examples.rb b/spec/support/shared_examples/services/wiki_pages/create_service_shared_examples.rb index 71bdd46572f..efcb83a34af 100644 --- a/spec/support/shared_examples/services/wiki_pages/create_service_shared_examples.rb +++ b/spec/support/shared_examples/services/wiki_pages/create_service_shared_examples.rb @@ -45,7 +45,7 @@ RSpec.shared_examples 'WikiPages::CreateService#execute' do |container_type| expect { service.execute }.to change { Event.count }.by 1 expect(Event.recent.first).to have_attributes( - action: Event::CREATED, + action: 'created', target: have_attributes(canonical_slug: page_title) ) end diff --git a/spec/support/shared_examples/services/wiki_pages/destroy_service_shared_examples.rb b/spec/support/shared_examples/services/wiki_pages/destroy_service_shared_examples.rb index 62541eb3da9..1231c012c31 100644 --- a/spec/support/shared_examples/services/wiki_pages/destroy_service_shared_examples.rb +++ b/spec/support/shared_examples/services/wiki_pages/destroy_service_shared_examples.rb @@ -27,7 +27,7 @@ RSpec.shared_examples 'WikiPages::DestroyService#execute' do |container_type| expect { service.execute(page) }.to change { Event.count }.by 1 expect(Event.recent.first).to have_attributes( - action: Event::DESTROYED, + action: 'destroyed', target: have_attributes(canonical_slug: page.slug) ) end diff --git a/spec/support/shared_examples/services/wiki_pages/update_service_shared_examples.rb b/spec/support/shared_examples/services/wiki_pages/update_service_shared_examples.rb index 0dfc99d043b..77354fec069 100644 --- a/spec/support/shared_examples/services/wiki_pages/update_service_shared_examples.rb +++ b/spec/support/shared_examples/services/wiki_pages/update_service_shared_examples.rb @@ -48,7 +48,7 @@ RSpec.shared_examples 'WikiPages::UpdateService#execute' do |container_type| expect { service.execute(page) }.to change { Event.count }.by 1 expect(Event.recent.first).to have_attributes( - action: Event::UPDATED, + action: 'updated', wiki_page: page, target_title: page.title ) diff --git a/spec/workers/repository_check/single_repository_worker_spec.rb b/spec/workers/repository_check/single_repository_worker_spec.rb index 6870e15424f..1c8aea3131d 100644 --- a/spec/workers/repository_check/single_repository_worker_spec.rb +++ b/spec/workers/repository_check/single_repository_worker_spec.rb @@ -86,7 +86,7 @@ describe RepositoryCheck::SingleRepositoryWorker do end def create_push_event(project) - project.events.create(action: Event::PUSHED, author_id: create(:user).id) + project.events.create(action: :pushed, author_id: create(:user).id) end def break_wiki(project)