diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index c9be31ea62b..b4b09204773 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -838d4c8bc4d82ef2ad79437b37f0d8b43394c7f7 +d56698332e3e5380c4e6723828bb01ac32edd4c9 diff --git a/Gemfile b/Gemfile index e51a01ad1f6..d5879e7d47d 100644 --- a/Gemfile +++ b/Gemfile @@ -393,6 +393,8 @@ group :development, :test do gem 'parallel', '~> 1.19', require: false gem 'test_file_finder', '~> 0.1.3' + + gem 'sigdump', '~> 0.2.4', require: 'sigdump/setup' end group :development, :test, :danger do diff --git a/Gemfile.lock b/Gemfile.lock index 2dcfa78394b..5e98b5d2981 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1181,6 +1181,7 @@ GEM sidekiq-cron (1.2.0) fugit (~> 1.1) sidekiq (>= 4.2.1) + sigdump (0.2.4) signet (0.14.0) addressable (~> 2.3) faraday (>= 0.17.3, < 2.0) @@ -1622,6 +1623,7 @@ DEPENDENCIES shoulda-matchers (~> 4.0.1) sidekiq (~> 6.4) sidekiq-cron (~> 1.2) + sigdump (~> 0.2.4) simple_po_parser (~> 1.1.2) simplecov (~> 0.18.5) simplecov-cobertura (~> 1.3.1) diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 9387500e66f..e378fcb6129 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -377,10 +377,6 @@ span.idiff { color: $gl-text-color; } - .file-actions .ide-edit-button { - z-index: 2; - } - @include media-breakpoint-down(md) { .file-actions { margin-top: $gl-padding-8; diff --git a/app/finders/projects/members/effective_access_level_finder.rb b/app/finders/projects/members/effective_access_level_finder.rb index 4538fc4c855..d17609ff59f 100644 --- a/app/finders/projects/members/effective_access_level_finder.rb +++ b/app/finders/projects/members/effective_access_level_finder.rb @@ -40,7 +40,7 @@ module Projects avenues = [authorizable_project_members] avenues << if project.personal? - project_owner_acting_as_maintainer + project_owner else authorizable_group_members end @@ -85,9 +85,15 @@ module Projects Member.from_union(members) end - def project_owner_acting_as_maintainer + # workaround until we migrate Project#owners to have membership with + # OWNER access level + def project_owner user_id = project.namespace.owner.id - access_level = Gitlab::Access::MAINTAINER + access_level = if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml) + Gitlab::Access::OWNER + else + Gitlab::Access::MAINTAINER + end Member .from(generate_from_statement([[user_id, access_level]])) # rubocop: disable CodeReuse/ActiveRecord diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index f0e8ff7778e..fcf6a177984 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -65,40 +65,13 @@ module BlobHelper return unless blob = readable_blob(options, path, project, ref) common_classes = "btn gl-button btn-confirm js-edit-blob gl-ml-3 #{options[:extra_class]}" - data = { track_action: 'click_edit', track_label: 'edit' } - - if Feature.enabled?(:web_ide_primary_edit, project.group) - common_classes += " btn-inverted" - data[:track_property] = 'secondary' - end edit_button_tag(blob, common_classes, _('Edit'), edit_blob_path(project, ref, path, options), project, - ref, - data) - end - - def ide_edit_button(project = @project, ref = @ref, path = @path, blob:) - return unless blob - - common_classes = 'btn gl-button btn-confirm ide-edit-button gl-ml-3' - data = { track_action: 'click_edit_ide', track_label: 'web_ide' } - - unless Feature.enabled?(:web_ide_primary_edit, project.group) - common_classes += " btn-inverted" - data[:track_property] = 'secondary' - end - - edit_button_tag(blob, - common_classes, - _('Web IDE'), - ide_edit_path(project, ref, path), - project, - ref, - data) + ref) end def modify_file_button(project = @project, ref = @ref, path = @path, blob:, label:, action:, btn_class:, modal_type:) @@ -363,16 +336,16 @@ module BlobHelper content_tag(:span, button, class: 'has-tooltip', title: _('You can only edit files when you are on a branch'), data: { container: 'body' }) end - def edit_link_tag(link_text, edit_path, common_classes, data) - link_to link_text, edit_path, class: "#{common_classes}", data: data + def edit_link_tag(link_text, edit_path, common_classes) + link_to link_text, edit_path, class: "#{common_classes}" end - def edit_button_tag(blob, common_classes, text, edit_path, project, ref, data) + def edit_button_tag(blob, common_classes, text, edit_path, project, ref) if !on_top_of_branch?(project, ref) edit_disabled_button_tag(text, common_classes) # This condition only applies to users who are logged in elsif !current_user || (current_user && can_modify_blob?(blob, project, ref)) - edit_link_tag(text, edit_path, common_classes, data) + edit_link_tag(text, edit_path, common_classes) elsif can?(current_user, :fork_project, project) && can?(current_user, :create_merge_request_in, project) edit_fork_button_tag(common_classes, project, text, edit_blob_fork_params(edit_path)) end diff --git a/app/models/concerns/merge_request_reviewer_state.rb b/app/models/concerns/merge_request_reviewer_state.rb index 5859f43a70c..893d06b4da8 100644 --- a/app/models/concerns/merge_request_reviewer_state.rb +++ b/app/models/concerns/merge_request_reviewer_state.rb @@ -14,6 +14,14 @@ module MergeRequestReviewerState presence: true, inclusion: { in: self.states.keys } + belongs_to :updated_state_by, class_name: 'User', foreign_key: :updated_state_by_user_id + after_initialize :set_state, unless: :persisted? + + def attention_requested_by + return unless attention_requested? + + updated_state_by + end end end diff --git a/app/models/concerns/select_for_project_authorization.rb b/app/models/concerns/select_for_project_authorization.rb index 49342e30db6..e176e29f9d9 100644 --- a/app/models/concerns/select_for_project_authorization.rb +++ b/app/models/concerns/select_for_project_authorization.rb @@ -8,8 +8,14 @@ module SelectForProjectAuthorization select("projects.id AS project_id", "members.access_level") end - def select_as_maintainer_for_project_authorization - select(["projects.id AS project_id", "#{Gitlab::Access::MAINTAINER} AS access_level"]) + # workaround until we migrate Project#owners to have membership with + # OWNER access level + def select_project_owner_for_project_authorization + if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml) + select(["projects.id AS project_id", "#{Gitlab::Access::OWNER} AS access_level"]) + else + select(["projects.id AS project_id", "#{Gitlab::Access::MAINTAINER} AS access_level"]) + end end end end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 3a449055bc1..d75a25942c8 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -94,9 +94,16 @@ class ProjectMember < Member override :access_level_inclusion def access_level_inclusion - return if access_level.in?(Gitlab::Access.values) + allowed_values = if ::Feature.enabled?(:personal_project_owner_with_owner_access, + default_enabled: :yaml) + Gitlab::Access.all_values + else + Gitlab::Access.values + end - errors.add(:access_level, "is not included in the list") + unless access_level.in?(allowed_values) + errors.add(:access_level, "is not included in the list") + end end override :refresh_member_authorized_projects diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5baf286d860..29340fe472d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1936,10 +1936,18 @@ class MergeRequest < ApplicationRecord merge_request_assignees.find_by(user_id: user.id) end + def merge_request_assignees_with(user_ids) + merge_request_assignees.where(user_id: user_ids) + end + def find_reviewer(user) merge_request_reviewers.find_by(user_id: user.id) end + def merge_request_reviewers_with(user_ids) + merge_request_reviewers.where(user_id: user_ids) + end + def enabled_reports { sast: report_type_enabled?(:sast), diff --git a/app/models/project.rb b/app/models/project.rb index 57a5d0a0211..749d0d51392 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -459,7 +459,7 @@ class Project < ApplicationRecord delegate :name, to: :owner, allow_nil: true, prefix: true delegate :members, to: :team, prefix: true delegate :add_user, :add_users, to: :team - delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_role, to: :team + delegate :add_guest, :add_reporter, :add_developer, :add_maintainer, :add_owner, :add_role, to: :team delegate :group_runners_enabled, :group_runners_enabled=, to: :ci_cd_settings, allow_nil: true delegate :root_ancestor, to: :namespace, allow_nil: true delegate :last_pipeline, to: :commit, allow_nil: true diff --git a/app/models/project_team.rb b/app/models/project_team.rb index c3c7508df9f..ee5ecc2dd3c 100644 --- a/app/models/project_team.rb +++ b/app/models/project_team.rb @@ -23,6 +23,10 @@ class ProjectTeam add_user(user, :maintainer, current_user: current_user) end + def add_owner(user, current_user: nil) + add_user(user, :owner, current_user: current_user) + end + def add_role(user, role, current_user: nil) public_send(:"add_#{role}", user, current_user: current_user) # rubocop:disable GitlabSecurity/PublicSend end @@ -103,7 +107,9 @@ class ProjectTeam if group group.owners else - [project.owner] + # workaround until we migrate Project#owners to have membership with + # OWNER access level + Array.wrap(fetch_members(Gitlab::Access::OWNER)) | Array.wrap(project.owner) end end diff --git a/app/services/members/projects/creator_service.rb b/app/services/members/projects/creator_service.rb index 2e974177075..4dba81acf73 100644 --- a/app/services/members/projects/creator_service.rb +++ b/app/services/members/projects/creator_service.rb @@ -4,7 +4,7 @@ module Members module Projects class CreatorService < Members::CreatorService def self.access_levels - Gitlab::Access.sym_options + Gitlab::Access.sym_options_with_owner end private diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 3363fc90997..c4fc4b0f79d 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -61,6 +61,8 @@ module MergeRequests unless new_reviewers.include?(current_user) remove_attention_requested(merge_request, current_user) + + merge_request.merge_request_reviewers_with(new_reviewers).update_all(updated_state_by_user_id: current_user.id) end end diff --git a/app/services/merge_requests/handle_assignees_change_service.rb b/app/services/merge_requests/handle_assignees_change_service.rb index 97be9fe8d9f..ae76e103f4b 100644 --- a/app/services/merge_requests/handle_assignees_change_service.rb +++ b/app/services/merge_requests/handle_assignees_change_service.rb @@ -21,6 +21,8 @@ module MergeRequests merge_request_activity_counter.track_users_assigned_to_mr(users: new_assignees) merge_request_activity_counter.track_assignees_changed_action(user: current_user) + merge_request.merge_request_assignees_with(new_assignees).update_all(updated_state_by_user_id: current_user.id) + execute_assignees_hooks(merge_request, old_assignees) if options[:execute_hooks] unless new_assignees.include?(current_user) diff --git a/app/services/merge_requests/toggle_attention_requested_service.rb b/app/services/merge_requests/toggle_attention_requested_service.rb index d9f81ac310f..70932e5f7e0 100644 --- a/app/services/merge_requests/toggle_attention_requested_service.rb +++ b/app/services/merge_requests/toggle_attention_requested_service.rb @@ -59,7 +59,8 @@ module MergeRequests end def update_state(reviewer_or_assignee) - reviewer_or_assignee&.update(state: reviewer_or_assignee&.attention_requested? ? :reviewed : :attention_requested) + reviewer_or_assignee&.update(state: reviewer_or_assignee&.attention_requested? ? :reviewed : :attention_requested, + updated_state_by: current_user) end end end diff --git a/app/services/notification_recipients/builder/project_maintainers.rb b/app/services/notification_recipients/builder/project_maintainers.rb index e8f22c00a83..a295929a1a9 100644 --- a/app/services/notification_recipients/builder/project_maintainers.rb +++ b/app/services/notification_recipients/builder/project_maintainers.rb @@ -14,6 +14,7 @@ module NotificationRecipients return [] unless project add_recipients(project.team.maintainers, :mention, nil) + add_recipients(project.team.owners, :mention, nil) end def acting_user diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index c885369dfec..ecae90e576d 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -147,7 +147,11 @@ module Projects priority: UserProjectAccessChangedService::LOW_PRIORITY ) else - @project.add_maintainer(@project.namespace.owner, current_user: current_user) + if ::Feature.enabled?(:personal_project_owner_with_owner_access, default_enabled: :yaml) + @project.add_owner(@project.namespace.owner, current_user: current_user) + else + @project.add_maintainer(@project.namespace.owner, current_user: current_user) + end end end diff --git a/app/views/admin/dashboard/_security_newsletter_callout.html.haml b/app/views/admin/dashboard/_security_newsletter_callout.html.haml index 3aba91e8765..aced997bada 100644 --- a/app/views/admin/dashboard/_security_newsletter_callout.html.haml +++ b/app/views/admin/dashboard/_security_newsletter_callout.html.haml @@ -4,7 +4,6 @@ title: s_('AdminArea|Get security updates from GitLab and stay up to date'), variant: :tip, alert_class: 'js-security-newsletter-callout', - is_contained: true, alert_data: { feature_id: Users::CalloutsHelper::SECURITY_NEWSLETTER_CALLOUT, dismiss_endpoint: callouts_path, defer_links: 'true' }, close_button_data: { testid: 'close-security-newsletter-callout' } do .gl-alert-body diff --git a/app/views/clusters/clusters/_banner.html.haml b/app/views/clusters/clusters/_banner.html.haml index 1ca4f9c670e..6fb3f26ff4f 100644 --- a/app/views/clusters/clusters/_banner.html.haml +++ b/app/views/clusters/clusters/_banner.html.haml @@ -9,7 +9,6 @@ = render 'shared/global_alert', variant: :warning, alert_class: 'hidden js-cluster-api-unreachable', - is_contained: true, close_button_class: 'js-close' do .gl-alert-body = s_('ClusterIntegration|Your cluster API is unreachable. Please ensure your API URL is correct.') @@ -17,7 +16,6 @@ = render 'shared/global_alert', variant: :warning, alert_class: 'hidden js-cluster-authentication-failure js-cluster-api-unreachable', - is_contained: true, close_button_class: 'js-close' do .gl-alert-body = s_('ClusterIntegration|There was a problem authenticating with your cluster. Please ensure your CA Certificate and Token are valid.') diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 97056db6b74..fdcee3670b7 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -5,7 +5,6 @@ = render 'shared/global_alert', variant: :info, alert_class: 'gl-my-5', - is_contained: true, dismissible: false do .gl-alert-body = s_('Profiles|Some options are unavailable for LDAP accounts') @@ -14,7 +13,6 @@ = render 'shared/global_alert', variant: :success, alert_class: 'gl-my-5', - is_contained: true, close_button_class: 'js-close-2fa-enabled-success-alert' do .gl-alert-body = html_escape(_('You have set up 2FA for your account! If you lose access to your 2FA device, you can use your recovery codes to access your account. Alternatively, if you upload an SSH key, you can %{anchorOpen}use that key to generate additional recovery codes%{anchorClose}.')) % { anchorOpen: ''.html_safe % { href: help_page_path('user/profile/account/two_factor_authentication', anchor: 'generate-new-recovery-codes-using-ssh') }, anchorClose: ''.html_safe } diff --git a/app/views/projects/branches/new.html.haml b/app/views/projects/branches/new.html.haml index 8ee7910de4b..5cc83111b34 100644 --- a/app/views/projects/branches/new.html.haml +++ b/app/views/projects/branches/new.html.haml @@ -4,8 +4,7 @@ - if @error = render 'shared/global_alert', variant: :danger, - close_button_class: 'js-close', - is_contained: true do + close_button_class: 'js-close' do .gl-alert-body = @error %h3.page-title diff --git a/app/views/projects/diffs/_warning.html.haml b/app/views/projects/diffs/_warning.html.haml index 1d9b1b13d5c..3d31773694f 100644 --- a/app/views/projects/diffs/_warning.html.haml +++ b/app/views/projects/diffs/_warning.html.haml @@ -1,7 +1,6 @@ = render 'shared/global_alert', title: _('Too many changes to show.'), variant: :warning, - is_contained: true, alert_class: 'gl-mb-5' do .gl-alert-body = html_escape(_("To preserve performance only %{strong_open}%{display_size} of %{real_size}%{strong_close} files are displayed.")) % { display_size: diff_files.size, real_size: diff_files.real_size, strong_open: ''.html_safe, strong_close: ''.html_safe } diff --git a/app/views/projects/forks/error.html.haml b/app/views/projects/forks/error.html.haml index 30e2e9f19d9..7933e0e07b3 100644 --- a/app/views/projects/forks/error.html.haml +++ b/app/views/projects/forks/error.html.haml @@ -4,7 +4,6 @@ title: _('Fork Error!'), variant: :danger, alert_class: 'gl-mt-5', - is_contained: true, dismissible: false do .gl-alert-body %p diff --git a/app/views/projects/issues/_alert_moved_from_service_desk.html.haml b/app/views/projects/issues/_alert_moved_from_service_desk.html.haml index 662270fb8e1..26bd65fbe26 100644 --- a/app/views/projects/issues/_alert_moved_from_service_desk.html.haml +++ b/app/views/projects/issues/_alert_moved_from_service_desk.html.haml @@ -4,7 +4,6 @@ = render 'shared/global_alert', variant: :warning, - is_contained: true, close_button_class: 'js-close', alert_class: 'hide js-alert-moved-from-service-desk-warning gl-mt-5' do .gl-alert-body.gl-mr-3 diff --git a/app/views/projects/milestones/show.html.haml b/app/views/projects/milestones/show.html.haml index dbde3346b81..225f8c7dd66 100644 --- a/app/views/projects/milestones/show.html.haml +++ b/app/views/projects/milestones/show.html.haml @@ -15,7 +15,6 @@ = render 'shared/global_alert', variant: :info, dismissible: false, - is_contained: true, alert_data: { testid: 'no-issues-alert' }, alert_class: 'gl-mt-3 gl-mb-5' do .gl-alert-body diff --git a/app/views/shared/_global_alert.html.haml b/app/views/shared/_global_alert.html.haml index 1eaf21fc568..cb7ad32e474 100644 --- a/app/views/shared/_global_alert.html.haml +++ b/app/views/shared/_global_alert.html.haml @@ -8,16 +8,14 @@ - close_button_class = local_assigns.fetch(:close_button_class, nil) - close_button_data = local_assigns.fetch(:close_button_data, nil) - icon = icons[variant] -- alert_container_class = [container_class, @content_class] unless fluid_layout || local_assigns.fetch(:is_contained, false) %div{ role: 'alert', class: ['gl-alert', "gl-alert-#{variant}", alert_class], data: alert_data } - .gl-alert-container{ class: alert_container_class } - = sprite_icon(icon, size: 16, css_class: "gl-alert-icon#{' gl-alert-icon-no-title' if title.nil?}") - - if dismissible - %button.btn.gl-dismiss-btn.btn-default.btn-sm.gl-button.btn-default-tertiary.btn-icon.js-close{ type: 'button', aria: { label: _('Dismiss') }, class: close_button_class, data: close_button_data } - = sprite_icon('close', size: 16) - .gl-alert-content{ role: 'alert' } - - if title - %h4.gl-alert-title - = title - = yield + = sprite_icon(icon, css_class: "gl-alert-icon#{' gl-alert-icon-no-title' if title.nil?}") + - if dismissible + %button.btn.gl-dismiss-btn.btn-default.btn-sm.gl-button.btn-default-tertiary.btn-icon.js-close{ type: 'button', aria: { label: _('Dismiss') }, class: close_button_class, data: close_button_data } + = sprite_icon('close') + .gl-alert-content{ role: 'alert' } + - if title + %h4.gl-alert-title + = title + = yield diff --git a/app/views/shared/_service_ping_consent.html.haml b/app/views/shared/_service_ping_consent.html.haml index 821d92e9d7e..9cdff35ead2 100644 --- a/app/views/shared/_service_ping_consent.html.haml +++ b/app/views/shared/_service_ping_consent.html.haml @@ -1,7 +1,6 @@ - if session[:ask_for_usage_stats_consent] = render 'shared/global_alert', variant: :info, - is_contained: true, alert_class: 'service-ping-consent-message' do .gl-alert-body - docs_link = link_to _('collect usage information'), help_page_path('user/admin_area/settings/usage_statistics.md'), class: 'gl-link' diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index ae896b7348d..446d04d96b8 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -9,7 +9,6 @@ = render 'shared/global_alert', variant: :danger, dismissible: false, - is_contained: true, alert_class: 'gl-mb-5' do .gl-alert-body Someone edited the #{issuable.class.model_name.human.downcase} the same time you did. diff --git a/app/views/shared/milestones/_milestone_complete_alert.html.haml b/app/views/shared/milestones/_milestone_complete_alert.html.haml index 1c25fae747e..5b05fdb6019 100644 --- a/app/views/shared/milestones/_milestone_complete_alert.html.haml +++ b/app/views/shared/milestones/_milestone_complete_alert.html.haml @@ -3,7 +3,6 @@ - if milestone.complete? && milestone.active? = render 'shared/global_alert', variant: :success, - is_contained: true, alert_data: { testid: 'all-issues-closed-alert' }, dismissible: false do .gl-alert-body diff --git a/app/views/shared/web_hooks/_hook_errors.html.haml b/app/views/shared/web_hooks/_hook_errors.html.haml index 23010b8349c..03f373783f8 100644 --- a/app/views/shared/web_hooks/_hook_errors.html.haml +++ b/app/views/shared/web_hooks/_hook_errors.html.haml @@ -13,7 +13,6 @@ = render 'shared/global_alert', title: s_('Webhooks|Webhook was automatically disabled'), variant: :danger, - is_contained: true, close_button_class: 'js-close' do .gl-alert-body = s_('Webhooks|The webhook was triggered more than %{limit} times per minute and is now disabled. To re-enable this webhook, fix the problems shown in %{strong_start}Recent events%{strong_end}, then re-test your settings. %{support_link_start}Contact Support%{support_link_end} if you need help re-enabling your webhook.').html_safe % placeholders @@ -21,7 +20,6 @@ = render 'shared/global_alert', title: s_('Webhooks|Webhook failed to connect'), variant: :danger, - is_contained: true, close_button_class: 'js-close' do .gl-alert-body = s_('Webhooks|The webhook failed to connect, and is disabled. To re-enable it, check %{strong_start}Recent events%{strong_end} for error details, then test your settings below.').html_safe % { strong_start: strong_start, strong_end: strong_end } @@ -35,7 +33,6 @@ = render 'shared/global_alert', title: s_('Webhooks|Webhook fails to connect'), variant: :warning, - is_contained: true, close_button_class: 'js-close' do .gl-alert-body = s_('Webhooks|The webhook %{help_link_start}failed to connect%{help_link_end}, and will retry in %{retry_time}. To re-enable it, check %{strong_start}Recent events%{strong_end} for error details, then test your settings below.').html_safe % placeholders diff --git a/app/views/shared/wikis/pages.html.haml b/app/views/shared/wikis/pages.html.haml index 0a8ca309823..abe7753b9f1 100644 --- a/app/views/shared/wikis/pages.html.haml +++ b/app/views/shared/wikis/pages.html.haml @@ -1,8 +1,8 @@ - add_to_breadcrumbs _('Wiki'), wiki_path(@wiki) - breadcrumb_title s_("Wiki|Pages") - page_title s_("Wiki|Pages"), _("Wiki") -- sort_title = wiki_sort_title(params[:sort]) - add_page_specific_style 'page_bundles/wiki' +- wiki_sort_options = [{ text: s_("Wiki|Title"), value: 'title', href: wiki_path(@wiki, action: :pages, sort: Wiki::TITLE_ORDER)}, { text: s_("Wiki|Created date"), value: 'created_at', href: wiki_path(@wiki, action: :pages, sort: Wiki::CREATED_AT_ORDER) }] .wiki-page-header.top-area.flex-column.flex-lg-row %h3.page-title.gl-flex-grow-1 @@ -15,14 +15,7 @@ .dropdown.inline.wiki-sort-dropdown .btn-group{ role: 'group' } - .btn-group{ role: 'group' } - %button.dropdown-toggle{ type: 'button', data: { toggle: 'dropdown', display: 'static' }, class: 'btn gl-button btn-default' } - = sort_title - = sprite_icon('chevron-down') - %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable.dropdown-menu-sort - %li - = sortable_item(s_("Wiki|Title"), wiki_path(@wiki, action: :pages, sort: Wiki::TITLE_ORDER), sort_title) - = sortable_item(s_("Wiki|Created date"), wiki_path(@wiki, action: :pages, sort: Wiki::CREATED_AT_ORDER), sort_title) + = gl_redirect_listbox_tag wiki_sort_options, params[:sort], data: { right: true } = wiki_sort_controls(@wiki, params[:sort], params[:direction]) %ul.wiki-pages-list.content-list diff --git a/config/feature_flags/development/personal_project_owner_with_owner_access.yml b/config/feature_flags/development/personal_project_owner_with_owner_access.yml new file mode 100644 index 00000000000..a82521e88e5 --- /dev/null +++ b/config/feature_flags/development/personal_project_owner_with_owner_access.yml @@ -0,0 +1,8 @@ +--- +name: personal_project_owner_with_owner_access +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/78193 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351919 +milestone: '14.8' +type: development +group: group::workspace +default_enabled: false diff --git a/config/feature_flags/development/web_ide_primary_edit.yml b/config/feature_flags/development/web_ide_primary_edit.yml deleted file mode 100644 index 5a609ae1d88..00000000000 --- a/config/feature_flags/development/web_ide_primary_edit.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: web_ide_primary_edit -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/35957 -rollout_issue_url: -milestone: '13.3' -type: development -group: group::editor -default_enabled: false diff --git a/db/migrate/20211203160952_add_updated_state_by_user_id_to_merge_request_reviewers.rb b/db/migrate/20211203160952_add_updated_state_by_user_id_to_merge_request_reviewers.rb new file mode 100644 index 00000000000..dafd2108b43 --- /dev/null +++ b/db/migrate/20211203160952_add_updated_state_by_user_id_to_merge_request_reviewers.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddUpdatedStateByUserIdToMergeRequestReviewers < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + def change + add_column :merge_request_reviewers, :updated_state_by_user_id, :bigint + end +end diff --git a/db/migrate/20211203161149_add_index_to_merge_request_reviewers_updated_state_by_user_id.rb b/db/migrate/20211203161149_add_index_to_merge_request_reviewers_updated_state_by_user_id.rb new file mode 100644 index 00000000000..6f4ee079015 --- /dev/null +++ b/db/migrate/20211203161149_add_index_to_merge_request_reviewers_updated_state_by_user_id.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddIndexToMergeRequestReviewersUpdatedStateByUserId < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + INDEX_NAME = 'index_on_merge_request_reviewers_updated_state_by_user_id' + + def up + add_concurrent_index :merge_request_reviewers, :updated_state_by_user_id, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :merge_request_reviewers, INDEX_NAME + end +end diff --git a/db/migrate/20211203161840_add_updated_state_by_user_id_to_merge_request_assignees.rb b/db/migrate/20211203161840_add_updated_state_by_user_id_to_merge_request_assignees.rb new file mode 100644 index 00000000000..1c9e7193630 --- /dev/null +++ b/db/migrate/20211203161840_add_updated_state_by_user_id_to_merge_request_assignees.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddUpdatedStateByUserIdToMergeRequestAssignees < Gitlab::Database::Migration[1.0] + enable_lock_retries! + + def change + add_column :merge_request_assignees, :updated_state_by_user_id, :bigint + end +end diff --git a/db/migrate/20211203161942_add_index_to_merge_request_assignees_updated_state_by_user_id.rb b/db/migrate/20211203161942_add_index_to_merge_request_assignees_updated_state_by_user_id.rb new file mode 100644 index 00000000000..d052ffdf4d6 --- /dev/null +++ b/db/migrate/20211203161942_add_index_to_merge_request_assignees_updated_state_by_user_id.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddIndexToMergeRequestAssigneesUpdatedStateByUserId < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + INDEX_NAME = 'index_on_merge_request_assignees_updated_state_by_user_id' + + def up + add_concurrent_index :merge_request_assignees, :updated_state_by_user_id, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :merge_request_assignees, INDEX_NAME + end +end diff --git a/db/migrate/20220105152547_add_foreign_key_to_updated_state_by_user_id_to_merge_request_assignees.rb b/db/migrate/20220105152547_add_foreign_key_to_updated_state_by_user_id_to_merge_request_assignees.rb new file mode 100644 index 00000000000..58411c1dc0f --- /dev/null +++ b/db/migrate/20220105152547_add_foreign_key_to_updated_state_by_user_id_to_merge_request_assignees.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddForeignKeyToUpdatedStateByUserIdToMergeRequestAssignees < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_request_assignees, :users, column: :updated_state_by_user_id, on_delete: :nullify + end + + def down + with_lock_retries do + remove_foreign_key :merge_request_assignees, column: :updated_state_by_user_id + end + end +end diff --git a/db/migrate/20220105153149_add_foreign_key_to_updated_state_by_user_id_to_merge_request_reviewers.rb b/db/migrate/20220105153149_add_foreign_key_to_updated_state_by_user_id_to_merge_request_reviewers.rb new file mode 100644 index 00000000000..13e375a5b97 --- /dev/null +++ b/db/migrate/20220105153149_add_foreign_key_to_updated_state_by_user_id_to_merge_request_reviewers.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddForeignKeyToUpdatedStateByUserIdToMergeRequestReviewers < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + def up + add_concurrent_foreign_key :merge_request_reviewers, :users, column: :updated_state_by_user_id, on_delete: :nullify + end + + def down + with_lock_retries do + remove_foreign_key :merge_request_reviewers, column: :updated_state_by_user_id + end + end +end diff --git a/db/schema_migrations/20211203160952 b/db/schema_migrations/20211203160952 new file mode 100644 index 00000000000..10d033ef200 --- /dev/null +++ b/db/schema_migrations/20211203160952 @@ -0,0 +1 @@ +f25c65dfcb5b7b4a663cc4c792ffd985f6afd3156036485a5a43a791ee799e7b \ No newline at end of file diff --git a/db/schema_migrations/20211203161149 b/db/schema_migrations/20211203161149 new file mode 100644 index 00000000000..3f58490a89c --- /dev/null +++ b/db/schema_migrations/20211203161149 @@ -0,0 +1 @@ +01482a299a7dac9d3f786f0dbe4650c686911bf788467146d3e9a91eafd0fc32 \ No newline at end of file diff --git a/db/schema_migrations/20211203161840 b/db/schema_migrations/20211203161840 new file mode 100644 index 00000000000..347bd0f7691 --- /dev/null +++ b/db/schema_migrations/20211203161840 @@ -0,0 +1 @@ +1b895e979ba2f1696559179c46c000e349da2d1ab94c968dd95103f188425103 \ No newline at end of file diff --git a/db/schema_migrations/20211203161942 b/db/schema_migrations/20211203161942 new file mode 100644 index 00000000000..f43c3733392 --- /dev/null +++ b/db/schema_migrations/20211203161942 @@ -0,0 +1 @@ +62432b2679cafa381671c9555f503867c254a7b3734e10cf634b34998d5fb5a3 \ No newline at end of file diff --git a/db/schema_migrations/20220105152547 b/db/schema_migrations/20220105152547 new file mode 100644 index 00000000000..d8c425da736 --- /dev/null +++ b/db/schema_migrations/20220105152547 @@ -0,0 +1 @@ +0f1ea41fae57710e0e05c9b71a14800394c4c57e37a39e92be49c50120d7d2ee \ No newline at end of file diff --git a/db/schema_migrations/20220105153149 b/db/schema_migrations/20220105153149 new file mode 100644 index 00000000000..11b71946bc8 --- /dev/null +++ b/db/schema_migrations/20220105153149 @@ -0,0 +1 @@ +8194c695a809f2eb29e5033f089c1d20874f61731a4289026f2d550854e7097d \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 2abb40b4122..91f4a5874e2 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16236,7 +16236,8 @@ CREATE TABLE merge_request_assignees ( user_id integer NOT NULL, merge_request_id integer NOT NULL, created_at timestamp with time zone, - state smallint DEFAULT 0 NOT NULL + state smallint DEFAULT 0 NOT NULL, + updated_state_by_user_id bigint ); CREATE SEQUENCE merge_request_assignees_id_seq @@ -16462,7 +16463,8 @@ CREATE TABLE merge_request_reviewers ( user_id bigint NOT NULL, merge_request_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, - state smallint DEFAULT 0 NOT NULL + state smallint DEFAULT 0 NOT NULL, + updated_state_by_user_id bigint ); CREATE SEQUENCE merge_request_reviewers_id_seq @@ -27232,8 +27234,12 @@ CREATE INDEX index_on_label_links_all_columns ON label_links USING btree (target CREATE INDEX index_on_merge_request_assignees_state ON merge_request_assignees USING btree (state) WHERE (state = 2); +CREATE INDEX index_on_merge_request_assignees_updated_state_by_user_id ON merge_request_assignees USING btree (updated_state_by_user_id); + CREATE INDEX index_on_merge_request_reviewers_state ON merge_request_reviewers USING btree (state) WHERE (state = 2); +CREATE INDEX index_on_merge_request_reviewers_updated_state_by_user_id ON merge_request_reviewers USING btree (updated_state_by_user_id); + CREATE INDEX index_on_merge_requests_for_latest_diffs ON merge_requests USING btree (target_project_id) INCLUDE (id, latest_merge_request_diff_id); COMMENT ON INDEX index_on_merge_requests_for_latest_diffs IS 'Index used to efficiently obtain the oldest merge request for a commit SHA'; @@ -29678,6 +29684,9 @@ ALTER TABLE ONLY epics ALTER TABLE ONLY ci_pipelines ADD CONSTRAINT fk_3d34ab2e06 FOREIGN KEY (pipeline_schedule_id) REFERENCES ci_pipeline_schedules(id) ON DELETE SET NULL; +ALTER TABLE ONLY merge_request_reviewers + ADD CONSTRAINT fk_3d674b9f23 FOREIGN KEY (updated_state_by_user_id) REFERENCES users(id) ON DELETE SET NULL; + ALTER TABLE ONLY ci_pipeline_schedule_variables ADD CONSTRAINT fk_41c35fda51 FOREIGN KEY (pipeline_schedule_id) REFERENCES ci_pipeline_schedules(id) ON DELETE CASCADE; @@ -30002,6 +30011,9 @@ ALTER TABLE ONLY merge_request_metrics ALTER TABLE ONLY dast_profile_schedules ADD CONSTRAINT fk_aef03d62e5 FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; +ALTER TABLE ONLY merge_request_assignees + ADD CONSTRAINT fk_af036e3261 FOREIGN KEY (updated_state_by_user_id) REFERENCES users(id) ON DELETE SET NULL; + ALTER TABLE ONLY analytics_cycle_analytics_group_stages ADD CONSTRAINT fk_analytics_cycle_analytics_group_stages_group_value_stream_id FOREIGN KEY (group_value_stream_id) REFERENCES analytics_cycle_analytics_group_value_streams(id) ON DELETE CASCADE; diff --git a/doc/install/relative_url.md b/doc/install/relative_url.md index 43f2414e8f9..831e33870bd 100644 --- a/doc/install/relative_url.md +++ b/doc/install/relative_url.md @@ -6,7 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Install GitLab under a relative URL **(FREE SELF)** -While it is recommended to install GitLab on its own (sub)domain, sometimes +While we recommend to install GitLab on its own (sub)domain, sometimes this is not possible due to a variety of reasons. In that case, GitLab can also be installed under a relative URL, for example `https://example.com/gitlab`. @@ -19,8 +19,8 @@ first time. There is no limit to how deeply nested the relative URL can be. For example you could serve GitLab under `/foo/bar/gitlab/git` without any issues. -Note that by changing the URL on an existing GitLab installation, all remote -URLs will change, so you'll have to manually edit them in any local repository +Changing the URL on an existing GitLab installation, changes all remote +URLs, so you have to manually edit them in any local repository that points to your GitLab instance. The list of configuration files you must change to serve GitLab from a @@ -32,7 +32,7 @@ relative URL is: - `/home/git/gitlab-shell/config.yml` - `/etc/default/gitlab` -After all the changes you need to recompile the assets and [restart GitLab](../administration/restart_gitlab.md#installations-from-source). +After all the changes, you must recompile the assets and [restart GitLab](../administration/restart_gitlab.md#installations-from-source). ## Relative URL requirements diff --git a/doc/user/permissions.md b/doc/user/permissions.md index ccb45cfbb8d..3e22435d154 100644 --- a/doc/user/permissions.md +++ b/doc/user/permissions.md @@ -33,14 +33,27 @@ usernames. A GitLab administrator can configure the GitLab instance to ## Project members permissions +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/219299) in GitLab 14.8, personal namespace owners appear with Owner role in new projects in their namespace. Introduced [with a flag](../administration/feature_flags.md) named `personal_project_owner_with_owner_access`. Disabled by default. + +FLAG: +On self-managed GitLab, personal namespace owners appearing with the Owner role in new projects in their namespace is disabled. To make it available, +ask an administrator to [enable the feature flag](../administration/feature_flags.md) named `personal_project_owner_with_owner_access`. +The feature is not ready for production use. +On GitLab.com, this feature is not available. + A user's role determines what permissions they have on a project. The Owner role provides all permissions but is available only: - For group owners. The role is inherited for a group's projects. - For Administrators. -Personal namespace owners have the same permissions as an Owner, but are displayed with the Maintainer role on projects created in their personal namespace. -For more information, see [projects members documentation](project/members/index.md). +Personal [namespace](group/index.md#namespaces) owners: + +- Are displayed as having the Maintainer role on projects in the namespace, but have the same permissions as a user with the Owner role. +- (Disabled by default) In GitLab 14.8 and later, for new projects in the namespace, are displayed as having the Owner role. + +For more information about how to manage project members, see +[members of a project](project/members/index.md). The following table lists project permissions available for each role: diff --git a/lib/backup/files.rb b/lib/backup/files.rb index 880522bc0f5..db6278360a3 100644 --- a/lib/backup/files.rb +++ b/lib/backup/files.rb @@ -9,13 +9,11 @@ module Backup DEFAULT_EXCLUDE = 'lost+found' - attr_reader :name, :app_files_dir, :backup_tarball, :excludes, :files_parent_dir + attr_reader :name, :backup_tarball, :excludes def initialize(name, app_files_dir, excludes: []) @name = name - @app_files_dir = File.realpath(app_files_dir) - @files_parent_dir = File.realpath(File.join(@app_files_dir, '..')) - @backup_files_dir = File.join(Gitlab.config.backup.path, File.basename(@app_files_dir) ) + @app_files_dir = app_files_dir @backup_tarball = File.join(Gitlab.config.backup.path, name + '.tar.gz') @excludes = [DEFAULT_EXCLUDE].concat(excludes) end @@ -26,7 +24,7 @@ module Backup FileUtils.rm_f(backup_tarball) if ENV['STRATEGY'] == 'copy' - cmd = [%w[rsync -a --delete], exclude_dirs(:rsync), %W[#{app_files_dir} #{Gitlab.config.backup.path}]].flatten + cmd = [%w[rsync -a --delete], exclude_dirs(:rsync), %W[#{app_files_realpath} #{Gitlab.config.backup.path}]].flatten output, status = Gitlab::Popen.popen(cmd) # Retry if rsync source files vanish @@ -40,11 +38,11 @@ module Backup raise_custom_error end - tar_cmd = [tar, exclude_dirs(:tar), %W[-C #{@backup_files_dir} -cf - .]].flatten + tar_cmd = [tar, exclude_dirs(:tar), %W[-C #{backup_files_realpath} -cf - .]].flatten status_list, output = run_pipeline!([tar_cmd, gzip_cmd], out: [backup_tarball, 'w', 0600]) - FileUtils.rm_rf(@backup_files_dir) + FileUtils.rm_rf(backup_files_realpath) else - tar_cmd = [tar, exclude_dirs(:tar), %W[-C #{app_files_dir} -cf - .]].flatten + tar_cmd = [tar, exclude_dirs(:tar), %W[-C #{app_files_realpath} -cf - .]].flatten status_list, output = run_pipeline!([tar_cmd, gzip_cmd], out: [backup_tarball, 'w', 0600]) end @@ -56,7 +54,7 @@ module Backup def restore backup_existing_files_dir - cmd_list = [%w[gzip -cd], %W[#{tar} --unlink-first --recursive-unlink -C #{app_files_dir} -xf -]] + cmd_list = [%w[gzip -cd], %W[#{tar} --unlink-first --recursive-unlink -C #{app_files_realpath} -xf -]] status_list, output = run_pipeline!(cmd_list, in: backup_tarball) unless pipeline_succeeded?(gzip_status: status_list[0], tar_status: status_list[1], output: output) raise Backup::Error, "Restore operation failed: #{output}" @@ -78,17 +76,17 @@ module Backup def backup_existing_files_dir timestamped_files_path = File.join(Gitlab.config.backup.path, "tmp", "#{name}.#{Time.now.to_i}") - if File.exist?(app_files_dir) + if File.exist?(app_files_realpath) # Move all files in the existing repos directory except . and .. to # repositories.old. directory FileUtils.mkdir_p(timestamped_files_path, mode: 0700) - files = Dir.glob(File.join(app_files_dir, "*"), File::FNM_DOTMATCH) - [File.join(app_files_dir, "."), File.join(app_files_dir, "..")] + files = Dir.glob(File.join(app_files_realpath, "*"), File::FNM_DOTMATCH) - [File.join(app_files_realpath, "."), File.join(app_files_realpath, "..")] begin FileUtils.mv(files, timestamped_files_path) rescue Errno::EACCES - access_denied_error(app_files_dir) + access_denied_error(app_files_realpath) rescue Errno::EBUSY - resource_busy_error(app_files_dir) + resource_busy_error(app_files_realpath) end end end @@ -141,7 +139,7 @@ module Backup if s == DEFAULT_EXCLUDE '--exclude=' + s elsif fmt == :rsync - '--exclude=/' + File.join(File.basename(app_files_dir), s) + '--exclude=/' + File.join(File.basename(app_files_realpath), s) elsif fmt == :tar '--exclude=./' + s end @@ -149,7 +147,17 @@ module Backup end def raise_custom_error - raise FileBackupError.new(app_files_dir, backup_tarball) + raise FileBackupError.new(app_files_realpath, backup_tarball) + end + + private + + def app_files_realpath + @app_files_realpath ||= File.realpath(@app_files_dir) + end + + def backup_files_realpath + @backup_files_realpath ||= File.join(Gitlab.config.backup.path, File.basename(@app_files_dir) ) end end end diff --git a/lib/gitlab/project_authorizations.rb b/lib/gitlab/project_authorizations.rb index 121626ced56..1d7b179baf0 100644 --- a/lib/gitlab/project_authorizations.rb +++ b/lib/gitlab/project_authorizations.rb @@ -22,7 +22,7 @@ module Gitlab user.projects_with_active_memberships.select_for_project_authorization, # The personal projects of the user. - user.personal_projects.select_as_maintainer_for_project_authorization, + user.personal_projects.select_project_owner_for_project_authorization, # Projects that belong directly to any of the groups the user has # access to. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7b65eaa28b3..59ac1afde3c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -38560,10 +38560,8 @@ msgstr "" msgid "Trending" msgstr "" -msgid "Trials|%{planName} Trial %{enDash} %{num} day left" -msgid_plural "Trials|%{planName} Trial %{enDash} %{num} days left" -msgstr[0] "" -msgstr[1] "" +msgid "Trials|%{planName} Trial" +msgstr "" msgid "Trials|Compare all plans" msgstr "" @@ -38571,6 +38569,9 @@ msgstr "" msgid "Trials|Create a new group to start your GitLab Ultimate trial." msgstr "" +msgid "Trials|Day %{daysUsed}/%{duration}" +msgstr "" + msgid "Trials|Go back to GitLab" msgstr "" diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index d8ef95cf11a..d6af5976743 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -665,7 +665,7 @@ RSpec.describe Projects::ProjectMembersController do sign_in(user) end - it 'does not create a member' do + it 'creates a member' do expect do post :create, params: { user_ids: stranger.id, @@ -673,7 +673,9 @@ RSpec.describe Projects::ProjectMembersController do access_level: Member::OWNER, project_id: project } - end.to change { project.members.count }.by(0) + end.to change { project.members.count }.by(1) + + expect(project.team_members).to include(user) end end diff --git a/spec/features/projects/wikis_spec.rb b/spec/features/projects/wikis_spec.rb index 621f8c71b20..879ffd2932b 100644 --- a/spec/features/projects/wikis_spec.rb +++ b/spec/features/projects/wikis_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -RSpec.describe 'Project wikis' do +RSpec.describe 'Project wikis', :js do let_it_be(:user) { create(:user) } let(:wiki) { create(:project_wiki, user: user, project: project) } diff --git a/spec/finders/projects/members/effective_access_level_finder_spec.rb b/spec/finders/projects/members/effective_access_level_finder_spec.rb index 33fbb5aca30..446b0f8f9a2 100644 --- a/spec/finders/projects/members/effective_access_level_finder_spec.rb +++ b/spec/finders/projects/members/effective_access_level_finder_spec.rb @@ -11,21 +11,40 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do context 'for a personal project' do let_it_be(:project) { create(:project) } - shared_examples_for 'includes access level of the owner of the project as Maintainer' do - it 'includes access level of the owner of the project as Maintainer' do - expect(subject).to( - contain_exactly( - hash_including( - 'user_id' => project.namespace.owner.id, - 'access_level' => Gitlab::Access::MAINTAINER + shared_examples_for 'includes access level of the owner of the project' do + context 'when personal_project_owner_with_owner_access feature flag is enabled' do + it 'includes access level of the owner of the project as Owner' do + expect(subject).to( + contain_exactly( + hash_including( + 'user_id' => project.namespace.owner.id, + 'access_level' => Gitlab::Access::OWNER + ) ) ) - ) + end + end + + context 'when personal_project_owner_with_owner_access feature flag is disabled' do + before do + stub_feature_flags(personal_project_owner_with_owner_access: false) + end + + it 'includes access level of the owner of the project as Maintainer' do + expect(subject).to( + contain_exactly( + hash_including( + 'user_id' => project.namespace.owner.id, + 'access_level' => Gitlab::Access::MAINTAINER + ) + ) + ) + end end end context 'when the project owner is a member of the project' do - it_behaves_like 'includes access level of the owner of the project as Maintainer' + it_behaves_like 'includes access level of the owner of the project' end context 'when the project owner is not explicitly a member of the project' do @@ -33,7 +52,7 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do project.members.find_by(user_id: project.namespace.owner.id).destroy! end - it_behaves_like 'includes access level of the owner of the project as Maintainer' + it_behaves_like 'includes access level of the owner of the project' end end @@ -84,17 +103,32 @@ RSpec.describe Projects::Members::EffectiveAccessLevelFinder, '#execute' do context 'for a project within a group' do context 'project in a root group' do - it 'includes access levels of users who are direct members of the parent group' do - group_member = create(:group_member, :developer, source: group) + context 'includes access levels of users who are direct members of the parent group' do + it 'when access level is developer' do + group_member = create(:group_member, :developer, source: group) - expect(subject).to( - include( - hash_including( - 'user_id' => group_member.user.id, - 'access_level' => Gitlab::Access::DEVELOPER + expect(subject).to( + include( + hash_including( + 'user_id' => group_member.user.id, + 'access_level' => Gitlab::Access::DEVELOPER + ) ) ) - ) + end + + it 'when access level is owner' do + group_member = create(:group_member, :owner, source: group) + + expect(subject).to( + include( + hash_including( + 'user_id' => group_member.user.id, + 'access_level' => Gitlab::Access::OWNER + ) + ) + ) + end end end diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb index efcb8125f68..65e46b61882 100644 --- a/spec/helpers/blob_helper_spec.rb +++ b/spec/helpers/blob_helper_spec.rb @@ -54,42 +54,6 @@ RSpec.describe BlobHelper do expect(Capybara.string(link_with_mr).find_link('Edit')[:href]).to eq("/#{project.full_path}/-/edit/master/README.md?mr_id=10") end - - context 'when edit is the primary button' do - before do - stub_feature_flags(web_ide_primary_edit: false) - end - - it 'is rendered as primary' do - expect(link).not_to match(/btn-inverted/) - end - - it 'passes on primary tracking attributes' do - parsed_link = Capybara.string(link).find_link('Edit') - - expect(parsed_link[:'data-track-action']).to eq("click_edit") - expect(parsed_link[:'data-track-label']).to eq("edit") - expect(parsed_link[:'data-track-property']).to eq(nil) - end - end - - context 'when Web IDE is the primary button' do - before do - stub_feature_flags(web_ide_primary_edit: true) - end - - it 'is rendered as inverted' do - expect(link).to match(/btn-inverted/) - end - - it 'passes on secondary tracking attributes' do - parsed_link = Capybara.string(link).find_link('Edit') - - expect(parsed_link[:'data-track-action']).to eq("click_edit") - expect(parsed_link[:'data-track-label']).to eq("edit") - expect(parsed_link[:'data-track-property']).to eq("secondary") - end - end end describe "#relative_raw_path" do @@ -324,63 +288,6 @@ RSpec.describe BlobHelper do end end - describe `#ide_edit_button` do - let_it_be(:namespace) { create(:namespace, name: 'gitlab') } - let_it_be(:project) { create(:project, :repository, namespace: namespace) } - let_it_be(:current_user) { create(:user) } - - let(:can_push_code) { true } - let(:blob) { project.repository.blob_at('refs/heads/master', 'README.md') } - - subject(:link) { helper.ide_edit_button(project, 'master', 'README.md', blob: blob) } - - before do - allow(helper).to receive(:current_user).and_return(current_user) - allow(helper).to receive(:can?).with(current_user, :push_code, project).and_return(can_push_code) - allow(helper).to receive(:can_collaborate_with_project?).and_return(true) - end - - it 'returns a link with a Web IDE route' do - expect(Capybara.string(link).find_link('Web IDE')[:href]).to eq("/-/ide/project/#{project.full_path}/edit/master/-/README.md") - end - - context 'when edit is the primary button' do - before do - stub_feature_flags(web_ide_primary_edit: false) - end - - it 'is rendered as inverted' do - expect(link).to match(/btn-inverted/) - end - - it 'passes on secondary tracking attributes' do - parsed_link = Capybara.string(link).find_link('Web IDE') - - expect(parsed_link[:'data-track-action']).to eq("click_edit_ide") - expect(parsed_link[:'data-track-label']).to eq("web_ide") - expect(parsed_link[:'data-track-property']).to eq("secondary") - end - end - - context 'when Web IDE is the primary button' do - before do - stub_feature_flags(web_ide_primary_edit: true) - end - - it 'is rendered as primary' do - expect(link).not_to match(/btn-inverted/) - end - - it 'passes on primary tracking attributes' do - parsed_link = Capybara.string(link).find_link('Web IDE') - - expect(parsed_link[:'data-track-action']).to eq("click_edit_ide") - expect(parsed_link[:'data-track-label']).to eq("web_ide") - expect(parsed_link[:'data-track-property']).to eq(nil) - end - end - end - describe '#ide_edit_path' do let(:project) { create(:project) } let(:current_user) { create(:user) } diff --git a/spec/lib/backup/artifacts_spec.rb b/spec/lib/backup/artifacts_spec.rb index 102d787a5e1..e65dc79b65b 100644 --- a/spec/lib/backup/artifacts_spec.rb +++ b/spec/lib/backup/artifacts_spec.rb @@ -7,16 +7,6 @@ RSpec.describe Backup::Artifacts do subject(:backup) { described_class.new(progress) } - describe '#initialize' do - it 'uses the correct upload dir' do - Dir.mktmpdir do |tmpdir| - allow(JobArtifactUploader).to receive(:root) { "#{tmpdir}" } - - expect(backup.app_files_dir).to eq("#{File.realpath(tmpdir)}") - end - end - end - describe '#dump' do before do allow(File).to receive(:realpath).with('/var/gitlab-artifacts').and_return('/var/gitlab-artifacts') @@ -24,10 +14,6 @@ RSpec.describe Backup::Artifacts do allow(JobArtifactUploader).to receive(:root) { '/var/gitlab-artifacts' } end - it 'uses the correct artifact dir' do - expect(backup.app_files_dir).to eq('/var/gitlab-artifacts') - end - it 'excludes tmp from backup tar' do expect(backup).to receive(:tar).and_return('blabla-tar') expect(backup).to receive(:run_pipeline!).with([%w(blabla-tar --exclude=lost+found --exclude=./tmp -C /var/gitlab-artifacts -cf - .), 'gzip -c -1'], any_args).and_return([[true, true], '']) diff --git a/spec/lib/backup/lfs_spec.rb b/spec/lib/backup/lfs_spec.rb index fdc1c0c885d..6525019d9ac 100644 --- a/spec/lib/backup/lfs_spec.rb +++ b/spec/lib/backup/lfs_spec.rb @@ -16,7 +16,6 @@ RSpec.describe Backup::Lfs do end it 'uses the correct lfs dir in tar command', :aggregate_failures do - expect(backup.app_files_dir).to eq('/var/lfs-objects') expect(backup).to receive(:tar).and_return('blabla-tar') expect(backup).to receive(:run_pipeline!).with([%w(blabla-tar --exclude=lost+found -C /var/lfs-objects -cf - .), 'gzip -c -1'], any_args).and_return([[true, true], '']) expect(backup).to receive(:pipeline_succeeded?).and_return(true) diff --git a/spec/lib/backup/manager_spec.rb b/spec/lib/backup/manager_spec.rb index ac693ad8b98..9c186205067 100644 --- a/spec/lib/backup/manager_spec.rb +++ b/spec/lib/backup/manager_spec.rb @@ -12,11 +12,6 @@ RSpec.describe Backup::Manager do before do allow(progress).to receive(:puts) allow(progress).to receive(:print) - FileUtils.mkdir_p('tmp/tests/public/uploads') - end - - after do - FileUtils.rm_rf('tmp/tests/public/uploads', secure: true) end describe '#pack' do diff --git a/spec/lib/backup/object_backup_spec.rb b/spec/lib/backup/object_backup_spec.rb index 6192b5c3482..4d34dc0ade7 100644 --- a/spec/lib/backup/object_backup_spec.rb +++ b/spec/lib/backup/object_backup_spec.rb @@ -17,7 +17,6 @@ RSpec.shared_examples 'backup object' do |setting| end it 'uses the correct storage dir in tar command and excludes tmp', :aggregate_failures do - expect(backup.app_files_dir).to eq(backup_path) expect(backup).to receive(:tar).and_return('blabla-tar') expect(backup).to receive(:run_pipeline!).with([%W(blabla-tar --exclude=lost+found --exclude=./tmp -C #{backup_path} -cf - .), 'gzip -c -1'], any_args).and_return([[true, true], '']) expect(backup).to receive(:pipeline_succeeded?).and_return(true) diff --git a/spec/lib/backup/pages_spec.rb b/spec/lib/backup/pages_spec.rb index 551d2df8f30..f9ee4bbdc41 100644 --- a/spec/lib/backup/pages_spec.rb +++ b/spec/lib/backup/pages_spec.rb @@ -13,12 +13,6 @@ RSpec.describe Backup::Pages do end describe '#dump' do - it 'uses the correct pages dir' do - allow(Gitlab.config.pages).to receive(:path) { '/var/gitlab-pages' } - - expect(subject.app_files_dir).to eq('/var/gitlab-pages') - end - it 'excludes tmp from backup tar' do allow(Gitlab.config.pages).to receive(:path) { '/var/gitlab-pages' } diff --git a/spec/lib/backup/uploads_spec.rb b/spec/lib/backup/uploads_spec.rb index c173916fe91..25ad0c0d3f7 100644 --- a/spec/lib/backup/uploads_spec.rb +++ b/spec/lib/backup/uploads_spec.rb @@ -7,18 +7,6 @@ RSpec.describe Backup::Uploads do subject(:backup) { described_class.new(progress) } - describe '#initialize' do - it 'uses the correct upload dir' do - Dir.mktmpdir do |tmpdir| - FileUtils.mkdir_p("#{tmpdir}/uploads") - - allow(Gitlab.config.uploads).to receive(:storage_path) { tmpdir } - - expect(backup.app_files_dir).to eq("#{File.realpath(tmpdir)}/uploads") - end - end - end - describe '#dump' do before do allow(File).to receive(:realpath).and_call_original @@ -27,10 +15,6 @@ RSpec.describe Backup::Uploads do allow(Gitlab.config.uploads).to receive(:storage_path) { '/var' } end - it 'uses the correct upload dir' do - expect(backup.app_files_dir).to eq('/var/uploads') - end - it 'excludes tmp from backup tar' do expect(backup).to receive(:tar).and_return('blabla-tar') expect(backup).to receive(:run_pipeline!).with([%w(blabla-tar --exclude=lost+found --exclude=./tmp -C /var/uploads -cf - .), 'gzip -c -1'], any_args).and_return([[true, true], '']) diff --git a/spec/lib/gitlab/project_authorizations_spec.rb b/spec/lib/gitlab/project_authorizations_spec.rb index 7852470196b..8630762e06f 100644 --- a/spec/lib/gitlab/project_authorizations_spec.rb +++ b/spec/lib/gitlab/project_authorizations_spec.rb @@ -34,12 +34,28 @@ RSpec.describe Gitlab::ProjectAuthorizations do .to include(owned_project.id, other_project.id, group_project.id) end - it 'includes the correct access levels' do - mapping = map_access_levels(authorizations) + context 'when personal_project_owner_with_owner_access feature flag is enabled' do + it 'includes the correct access levels' do + mapping = map_access_levels(authorizations) - expect(mapping[owned_project.id]).to eq(Gitlab::Access::MAINTAINER) - expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER) - expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER) + expect(mapping[owned_project.id]).to eq(Gitlab::Access::OWNER) + expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER) + expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER) + end + end + + context 'when personal_project_owner_with_owner_access feature flag is disabled' do + before do + stub_feature_flags(personal_project_owner_with_owner_access: false) + end + + it 'includes the correct access levels' do + mapping = map_access_levels(authorizations) + + expect(mapping[owned_project.id]).to eq(Gitlab::Access::MAINTAINER) + expect(mapping[other_project.id]).to eq(Gitlab::Access::REPORTER) + expect(mapping[group_project.id]).to eq(Gitlab::Access::DEVELOPER) + end end end diff --git a/spec/models/merge_request_assignee_spec.rb b/spec/models/merge_request_assignee_spec.rb index 58b802de8e0..1591c517049 100644 --- a/spec/models/merge_request_assignee_spec.rb +++ b/spec/models/merge_request_assignee_spec.rb @@ -51,4 +51,24 @@ RSpec.describe MergeRequestAssignee do it { is_expected.to have_attributes(state: 'reviewed') } end + + describe '#attention_requested_by' do + let(:current_user) { create(:user) } + + before do + subject.update!(updated_state_by: current_user) + end + + context 'attention requested' do + it { expect(subject.attention_requested_by).to eq(current_user) } + end + + context 'attention requested' do + before do + subject.update!(state: :reviewed) + end + + it { expect(subject.attention_requested_by).to eq(nil) } + end + end end diff --git a/spec/models/merge_request_reviewer_spec.rb b/spec/models/merge_request_reviewer_spec.rb index d99fd4afb0f..dd00c4d8627 100644 --- a/spec/models/merge_request_reviewer_spec.rb +++ b/spec/models/merge_request_reviewer_spec.rb @@ -25,4 +25,24 @@ RSpec.describe MergeRequestReviewer do it { is_expected.to belong_to(:merge_request).class_name('MergeRequest') } it { is_expected.to belong_to(:reviewer).class_name('User').inverse_of(:merge_request_reviewers) } end + + describe '#attention_requested_by' do + let(:current_user) { create(:user) } + + before do + subject.update!(updated_state_by: current_user) + end + + context 'attention requested' do + it { expect(subject.attention_requested_by).to eq(current_user) } + end + + context 'attention requested' do + before do + subject.update!(state: :reviewed) + end + + it { expect(subject.attention_requested_by).to eq(nil) } + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index d5cbe1b16e6..b104a89aafd 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -5109,4 +5109,34 @@ RSpec.describe MergeRequest, factory_default: :keep do let!(:model) { create(:merge_request, head_pipeline: parent) } end end + + describe '#merge_request_reviewers_with' do + let_it_be(:reviewer1) { create(:user) } + let_it_be(:reviewer2) { create(:user) } + + before do + subject.update!(reviewers: [reviewer1, reviewer2]) + end + + it 'returns reviewers' do + reviewers = subject.merge_request_reviewers_with([reviewer1.id]) + + expect(reviewers).to match_array([subject.merge_request_reviewers[0]]) + end + end + + describe '#merge_request_assignees_with' do + let_it_be(:assignee1) { create(:user) } + let_it_be(:assignee2) { create(:user) } + + before do + subject.update!(assignees: [assignee1, assignee2]) + end + + it 'returns assignees' do + assignees = subject.merge_request_assignees_with([assignee1.id]) + + expect(assignees).to match_array([subject.merge_request_assignees[0]]) + end + end end diff --git a/spec/models/project_team_spec.rb b/spec/models/project_team_spec.rb index bfdebbc33df..5b11f9d828a 100644 --- a/spec/models/project_team_spec.rb +++ b/spec/models/project_team_spec.rb @@ -225,7 +225,7 @@ RSpec.describe ProjectTeam do let_it_be(:maintainer) { create(:user) } let_it_be(:developer) { create(:user) } let_it_be(:guest) { create(:user) } - let_it_be(:project) { create(:project, namespace: maintainer.namespace) } + let_it_be(:project) { create(:project, group: create(:group)) } let_it_be(:access_levels) { [Gitlab::Access::DEVELOPER, Gitlab::Access::MAINTAINER] } subject(:members_with_access_levels) { project.team.members_with_access_levels(access_levels) } diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e4f25c79e53..6e7cccfccf6 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -3717,7 +3717,7 @@ RSpec.describe User do context 'with min_access_level' do let!(:user) { create(:user) } - let!(:project) { create(:project, :private, namespace: user.namespace) } + let!(:project) { create(:project, :private, group: create(:group)) } before do project.add_developer(user) diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 6186a43f992..5c867b4580b 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -675,13 +675,30 @@ RSpec.describe API::Members do end context 'adding owner to project' do - it 'returns 403' do - expect do - post api("/projects/#{project.id}/members", maintainer), - params: { user_id: stranger.id, access_level: Member::OWNER } + context 'when personal_project_owner_with_owner_access feature flag is enabled' do + it 'returns created status' do + expect do + post api("/projects/#{project.id}/members", maintainer), + params: { user_id: stranger.id, access_level: Member::OWNER } - expect(response).to have_gitlab_http_status(:bad_request) - end.not_to change { project.members.count } + expect(response).to have_gitlab_http_status(:created) + end.to change { project.members.count }.by(1) + end + end + + context 'when personal_project_owner_with_owner_access feature flag is disabled' do + before do + stub_feature_flags(personal_project_owner_with_owner_access: false) + end + + it 'returns created status' do + expect do + post api("/projects/#{project.id}/members", maintainer), + params: { user_id: stranger.id, access_level: Member::OWNER } + + expect(response).to have_gitlab_http_status(:bad_request) + end.not_to change { project.members.count } + end end end diff --git a/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb b/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb index c6b184bd003..537d1986384 100644 --- a/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb +++ b/spec/services/authorized_project_update/find_records_due_for_refresh_service_spec.rb @@ -40,7 +40,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do it 'is called' do ProjectAuthorization.delete_all - expect(callback).to receive(:call).with(project.id, Gitlab::Access::MAINTAINER).once + expect(callback).to receive(:call).with(project.id, Gitlab::Access::OWNER).once service.execute end @@ -60,20 +60,20 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do to_be_removed = [project2.id] to_be_added = [ - { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER } + { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } ] expect(service.execute).to eq([to_be_removed, to_be_added]) end it 'finds duplicate entries that has to be removed' do - [Gitlab::Access::MAINTAINER, Gitlab::Access::REPORTER].each do |access_level| + [Gitlab::Access::OWNER, Gitlab::Access::REPORTER].each do |access_level| user.project_authorizations.create!(project: project, access_level: access_level) end to_be_removed = [project.id] to_be_added = [ - { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER } + { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } ] expect(service.execute).to eq([to_be_removed, to_be_added]) @@ -85,7 +85,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do to_be_removed = [project.id] to_be_added = [ - { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER } + { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } ] expect(service.execute).to eq([to_be_removed, to_be_added]) @@ -143,16 +143,16 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do end it 'sets the keys to the project IDs' do - expect(hash.keys).to eq([project.id]) + expect(hash.keys).to match_array([project.id]) end it 'sets the values to the access levels' do - expect(hash.values).to eq([Gitlab::Access::MAINTAINER]) + expect(hash.values).to match_array([Gitlab::Access::OWNER]) end context 'personal projects' do it 'includes the project with the right access level' do - expect(hash[project.id]).to eq(Gitlab::Access::MAINTAINER) + expect(hash[project.id]).to eq(Gitlab::Access::OWNER) end end @@ -242,7 +242,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do value = hash.values[0] expect(value.project_id).to eq(project.id) - expect(value.access_level).to eq(Gitlab::Access::MAINTAINER) + expect(value.access_level).to eq(Gitlab::Access::OWNER) end end @@ -267,7 +267,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do end it 'includes the access level for every row' do - expect(row.access_level).to eq(Gitlab::Access::MAINTAINER) + expect(row.access_level).to eq(Gitlab::Access::OWNER) end end end @@ -283,7 +283,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do rows = service.fresh_authorizations.to_a expect(rows.length).to eq(1) - expect(rows.first.access_level).to eq(Gitlab::Access::MAINTAINER) + expect(rows.first.access_level).to eq(Gitlab::Access::OWNER) end context 'every returned row' do @@ -294,7 +294,7 @@ RSpec.describe AuthorizedProjectUpdate::FindRecordsDueForRefreshService do end it 'includes the access level' do - expect(row.access_level).to eq(Gitlab::Access::MAINTAINER) + expect(row.access_level).to eq(Gitlab::Access::OWNER) end end end diff --git a/spec/services/members/projects/creator_service_spec.rb b/spec/services/members/projects/creator_service_spec.rb index c6917a21bcd..7ba183759bc 100644 --- a/spec/services/members/projects/creator_service_spec.rb +++ b/spec/services/members/projects/creator_service_spec.rb @@ -9,8 +9,8 @@ RSpec.describe Members::Projects::CreatorService do end describe '.access_levels' do - it 'returns Gitlab::Access.sym_options' do - expect(described_class.access_levels).to eq(Gitlab::Access.sym_options) + it 'returns Gitlab::Access.sym_options_with_owner' do + expect(described_class.access_levels).to eq(Gitlab::Access.sym_options_with_owner) end end end diff --git a/spec/services/merge_requests/handle_assignees_change_service_spec.rb b/spec/services/merge_requests/handle_assignees_change_service_spec.rb index fa3b1614e21..34c598e6cc0 100644 --- a/spec/services/merge_requests/handle_assignees_change_service_spec.rb +++ b/spec/services/merge_requests/handle_assignees_change_service_spec.rb @@ -95,6 +95,12 @@ RSpec.describe MergeRequests::HandleAssigneesChangeService do execute end + it 'updates attention requested by of assignee' do + execute + + expect(merge_request.find_assignee(assignee).updated_state_by).to eq(user) + end + it 'tracks users assigned event' do expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) .to receive(:track_users_assigned_to_mr).once.with(users: [assignee]) diff --git a/spec/services/merge_requests/toggle_attention_requested_service_spec.rb b/spec/services/merge_requests/toggle_attention_requested_service_spec.rb index 63fa61b8097..83e6750b2b9 100644 --- a/spec/services/merge_requests/toggle_attention_requested_service_spec.rb +++ b/spec/services/merge_requests/toggle_attention_requested_service_spec.rb @@ -59,6 +59,13 @@ RSpec.describe MergeRequests::ToggleAttentionRequestedService do expect(reviewer.state).to eq 'attention_requested' end + it 'adds who toggled attention' do + service.execute + reviewer.reload + + expect(reviewer.updated_state_by).to eq current_user + end + it 'creates a new todo for the reviewer' do expect(todo_service).to receive(:create_attention_requested_todo).with(merge_request, current_user, user) diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 48d9f019274..eb587797201 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -215,6 +215,14 @@ RSpec.describe MergeRequests::UpdateService, :mailer do MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) end + + it 'updates attention requested by of reviewer' do + opts[:reviewers] = [user2] + + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) + + expect(merge_request.find_reviewer(user2).updated_state_by).to eq(user) + end end context 'when reviewers did not change' do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 9cbc16f0c95..d12b70d403a 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -3312,7 +3312,7 @@ RSpec.describe NotificationService, :mailer do describe "##{sym}" do subject(:notify!) { notification.send(sym, domain) } - it 'emails current watching maintainers' do + it 'emails current watching maintainers and owners' do expect(Notify).to receive(:"#{sym}_email").at_least(:once).and_call_original notify! @@ -3410,7 +3410,7 @@ RSpec.describe NotificationService, :mailer do reset_delivered_emails! end - it 'emails current watching maintainers' do + it 'emails current watching maintainers and owners' do notification.remote_mirror_update_failed(remote_mirror) should_only_email(u_maintainer1, u_maintainer2, u_owner) diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 10f694827e1..0d4974a7a86 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -116,14 +116,34 @@ RSpec.describe Projects::CreateService, '#execute' do end context 'user namespace' do - it 'creates a project in user namespace' do - project = create_project(user, opts) + context 'when personal_project_owner_with_owner_access feature flag is enabled' do + it 'creates a project in user namespace' do + project = create_project(user, opts) - expect(project).to be_valid - expect(project.first_owner).to eq(user) - expect(project.team.maintainers).to include(user) - expect(project.namespace).to eq(user.namespace) - expect(project.project_namespace).to be_in_sync_with_project(project) + expect(project).to be_valid + expect(project.first_owner).to eq(user) + expect(project.team.maintainers).not_to include(user) + expect(project.team.owners).to contain_exactly(user) + expect(project.namespace).to eq(user.namespace) + expect(project.project_namespace).to be_in_sync_with_project(project) + end + end + + context 'when personal_project_owner_with_owner_access feature flag is disabled' do + before do + stub_feature_flags(personal_project_owner_with_owner_access: false) + end + + it 'creates a project in user namespace' do + project = create_project(user, opts) + + expect(project).to be_valid + expect(project.first_owner).to eq(user) + expect(project.team.maintainers).to contain_exactly(user) + expect(project.team.owners).to contain_exactly(user) + expect(project.namespace).to eq(user.namespace) + expect(project.project_namespace).to be_in_sync_with_project(project) + end end end @@ -162,7 +182,7 @@ RSpec.describe Projects::CreateService, '#execute' do expect(project).to be_persisted expect(project.owner).to eq(user) expect(project.first_owner).to eq(user) - expect(project.team.maintainers).to contain_exactly(user) + expect(project.team.owners).to contain_exactly(user) expect(project.namespace).to eq(user.namespace) expect(project.project_namespace).to be_in_sync_with_project(project) end diff --git a/spec/services/users/refresh_authorized_projects_service_spec.rb b/spec/services/users/refresh_authorized_projects_service_spec.rb index a31902c7f16..b8fd2455445 100644 --- a/spec/services/users/refresh_authorized_projects_service_spec.rb +++ b/spec/services/users/refresh_authorized_projects_service_spec.rb @@ -52,7 +52,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do it 'is called' do ProjectAuthorization.delete_all - expect(callback).to receive(:call).with(project.id, Gitlab::Access::MAINTAINER).once + expect(callback).to receive(:call).with(project.id, Gitlab::Access::OWNER).once service.execute end @@ -73,7 +73,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do to_be_removed = [project_authorization.project_id] to_be_added = [ - { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER } + { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } ] expect(service).to receive(:update_authorizations) @@ -83,14 +83,14 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do end it 'removes duplicate entries' do - [Gitlab::Access::MAINTAINER, Gitlab::Access::REPORTER].each do |access_level| + [Gitlab::Access::OWNER, Gitlab::Access::REPORTER].each do |access_level| user.project_authorizations.create!(project: project, access_level: access_level) end to_be_removed = [project.id] to_be_added = [ - { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER } + { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } ] expect(service).to( receive(:update_authorizations) @@ -103,7 +103,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do project_authorization = ProjectAuthorization.where( project_id: project.id, user_id: user.id, - access_level: Gitlab::Access::MAINTAINER) + access_level: Gitlab::Access::OWNER) expect(project_authorization).to exist end @@ -116,7 +116,7 @@ RSpec.describe Users::RefreshAuthorizedProjectsService do to_be_removed = [project_authorization.project_id] to_be_added = [ - { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::MAINTAINER } + { user_id: user.id, project_id: project.id, access_level: Gitlab::Access::OWNER } ] expect(service).to receive(:update_authorizations) diff --git a/spec/support/shared_examples/features/wiki/user_views_wiki_page_shared_examples.rb b/spec/support/shared_examples/features/wiki/user_views_wiki_page_shared_examples.rb index eec911f3b6f..39a42fa2d5c 100644 --- a/spec/support/shared_examples/features/wiki/user_views_wiki_page_shared_examples.rb +++ b/spec/support/shared_examples/features/wiki/user_views_wiki_page_shared_examples.rb @@ -93,13 +93,12 @@ RSpec.shared_examples 'User views a wiki page' do let(:path) { upload_file_to_wiki(wiki, user, 'dk.png') } it do - expect(page).to have_xpath("//img[@data-src='#{wiki.wiki_base_path}/#{path}']") + expect(page).to have_xpath("//img[@src='#{wiki.wiki_base_path}/#{path}']") expect(page).to have_link('image', href: "#{wiki.wiki_base_path}/#{path}") click_on('image') expect(current_path).to match("wikis/#{path}") - expect(page).not_to have_xpath('/html') # Page should render the image which means there is no html involved end end diff --git a/spec/support/shared_examples/features/wiki/user_views_wiki_pages_shared_examples.rb b/spec/support/shared_examples/features/wiki/user_views_wiki_pages_shared_examples.rb index 314c2074eee..32cb2b1d187 100644 --- a/spec/support/shared_examples/features/wiki/user_views_wiki_pages_shared_examples.rb +++ b/spec/support/shared_examples/features/wiki/user_views_wiki_pages_shared_examples.rb @@ -60,7 +60,7 @@ RSpec.shared_examples 'User views wiki pages' do before do page.within('.wiki-sort-dropdown') do click_button('Title') - click_link('Created date') + click_button('Created date') end end diff --git a/spec/views/shared/_global_alert.html.haml_spec.rb b/spec/views/shared/_global_alert.html.haml_spec.rb index 84198cbb75e..a400d5b39b0 100644 --- a/spec/views/shared/_global_alert.html.haml_spec.rb +++ b/spec/views/shared/_global_alert.html.haml_spec.rb @@ -43,33 +43,4 @@ RSpec.describe 'shared/_global_alert.html.haml' do expect(rendered).not_to have_selector('.gl-dismiss-btn') end end - - context 'fixed layout' do - before do - allow(view).to receive(:fluid_layout).and_return(false) - end - - it 'adds container classes' do - render - - expect(rendered).to have_selector('.container-fluid.container-limited') - end - - it 'does not add container classes if is_contained is true' do - render partial: 'shared/global_alert', locals: { is_contained: true } - - expect(rendered).not_to have_selector('.container-fluid.container-limited') - end - end - - context 'fluid layout' do - before do - allow(view).to receive(:fluid_layout).and_return(true) - render - end - - it 'does not add container classes' do - expect(rendered).not_to have_selector('.container-fluid.container-limited') - end - end end