diff options
77 files changed, 1484 insertions, 708 deletions
diff --git a/app/assets/images/ext_snippet_icons/ext_snippet_icons.png b/app/assets/images/ext_snippet_icons/ext_snippet_icons.png Binary files differindex 20380adc4e5..c864e558bfd 100644 --- a/app/assets/images/ext_snippet_icons/ext_snippet_icons.png +++ b/app/assets/images/ext_snippet_icons/ext_snippet_icons.png diff --git a/app/assets/images/ext_snippet_icons/logo.png b/app/assets/images/ext_snippet_icons/logo.png Binary files differdeleted file mode 100644 index 794c9cc2dbc..00000000000 --- a/app/assets/images/ext_snippet_icons/logo.png +++ /dev/null diff --git a/app/assets/images/ext_snippet_icons/logo.svg b/app/assets/images/ext_snippet_icons/logo.svg new file mode 100644 index 00000000000..9cb3042213a --- /dev/null +++ b/app/assets/images/ext_snippet_icons/logo.svg @@ -0,0 +1 @@ +<svg width="100" height="32" xmlns="http://www.w3.org/2000/svg"><g fill-rule="nonzero" fill="none"><path fill="#8C929D" d="M67.67 8.11h-2.06l.009 15.364h8.348v-1.9H67.68l-.01-13.465zM81.913 20.778a3.517 3.517 0 01-2.553 1.078c-1.57 0-2.203-.775-2.203-1.787 0-1.522 1.059-2.25 3.309-2.25.487.002.974.04 1.456.113v2.846h-.01zm-2.137-9.313a6.826 6.826 0 00-4.387 1.579l.728 1.267c.841-.492 1.872-.983 3.356-.983 1.693 0 2.44.87 2.44 2.326v.747a9.4 9.4 0 00-1.428-.114c-3.612 0-5.446 1.267-5.446 3.914 0 2.374 1.456 3.565 3.659 3.565 1.484 0 2.912-.68 3.404-1.787l.378 1.503h1.456v-7.866c-.01-2.487-1.087-4.151-4.16-4.151zM90.587 21.926c-.776 0-1.456-.094-1.967-.33v-7.102c.7-.586 1.57-1.011 2.676-1.011 1.995 0 2.76 1.408 2.76 3.687 0 3.234-1.238 4.756-3.47 4.756m.87-10.457a3.775 3.775 0 00-2.836 1.257V10.74l-.01-2.629h-2.013l.01 14.987c1.01.425 2.391.652 3.895.652 3.848 0 5.701-2.458 5.701-6.704-.01-3.356-1.72-5.578-4.746-5.578M45.228 9.776c1.825 0 3.006.605 3.772 1.22l.889-1.541c-1.2-1.06-2.827-1.627-4.567-1.627-4.387 0-7.46 2.676-7.46 8.075 0 5.654 3.319 7.857 7.11 7.857a12.083 12.083 0 004.577-.888L49.5 16.83v-1.9h-5.63v1.9h3.594l.047 4.586c-.473.236-1.286.425-2.392.425-3.045 0-5.087-1.92-5.087-5.957-.01-4.113 2.1-6.108 5.19-6.108M59.744 8.107H57.73l.01 2.582v8.916c0 2.487 1.078 4.15 4.15 4.15.416.002.83-.036 1.24-.113v-1.806c-.31.047-.624.07-.937.066-1.692 0-2.44-.87-2.44-2.326v-6.145h3.376v-1.683h-3.373l-.009-3.64h-.003zM52.608 23.474h2.014V11.75h-2.014zM52.608 10.133h2.014V8.119h-2.014z"/><path d="M31.864 17.907l-1.788-5.496-3.538-10.9a.612.612 0 00-1.16 0L21.84 12.406H10.085L6.547 1.512a.612.612 0 00-1.16 0L1.855 12.405.066 17.907c-.162.5.015 1.05.44 1.36L15.963 30.5l15.456-11.233a1.22 1.22 0 00.446-1.36" fill="#FC6D26"/><path d="M15.966 30.49l5.875-18.086H10.09z" fill="#E24329"/><path d="M15.962 30.49l-5.877-18.086H1.859z" fill="#FC6D26"/><path d="M1.852 12.41L.063 17.906c-.162.5.015 1.05.441 1.36L15.959 30.5 1.852 12.41z" fill="#FCA326"/><path d="M1.854 12.41h8.237L6.546 1.517a.612.612 0 00-1.16 0L1.854 12.41z" fill="#E24329"/><path d="M15.966 30.49l5.875-18.086h8.236z" fill="#FC6D26"/><path d="M30.074 12.41l1.79 5.496a1.219 1.219 0 01-.44 1.36L15.966 30.49l14.107-18.08z" fill="#FCA326"/><path d="M30.079 12.41H21.84L25.38 1.517a.612.612 0 011.16 0l3.539 10.893z" fill="#E24329"/></g></svg>
\ No newline at end of file diff --git a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue index 0883b89d75b..6994f83bce0 100644 --- a/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue +++ b/app/assets/javascripts/pages/projects/shared/permissions/components/settings_panel.vue @@ -104,6 +104,7 @@ export default { visibilityLevel: visibilityOptions.PUBLIC, issuesAccessLevel: 20, repositoryAccessLevel: 20, + forkingAccessLevel: 20, mergeRequestsAccessLevel: 20, buildsAccessLevel: 20, wikiAccessLevel: 20, @@ -301,6 +302,19 @@ export default { /> </project-setting-row> <project-setting-row + :label="s__('ProjectSettings|Forks')" + :help-text=" + s__('ProjectSettings|Allow users to make copies of your repository to a new project') + " + > + <project-feature-setting + v-model="forkingAccessLevel" + :options="featureAccessLevelOptions" + :disabled-input="!repositoryEnabled" + name="project[project_feature_attributes][forking_access_level]" + /> + </project-setting-row> + <project-setting-row :label="s__('ProjectSettings|Pipelines')" :help-text="s__('ProjectSettings|Build, test, and deploy your changes')" > diff --git a/app/assets/stylesheets/framework/highlight.scss b/app/assets/stylesheets/framework/highlight.scss index 58516cbd1a9..ee6e53adaf7 100644 --- a/app/assets/stylesheets/framework/highlight.scss +++ b/app/assets/stylesheets/framework/highlight.scss @@ -8,7 +8,7 @@ pre { padding: 10px 0; border: 0; - border-radius: 0 0 $border-radius-default $border-radius-default; + border-radius: 0 0 $border-radius-default; font-family: $monospace-font; font-size: $code-font-size; line-height: 1.5; diff --git a/app/assets/stylesheets/snippets.scss b/app/assets/stylesheets/snippets.scss index dcdb195518c..0008a0e5c51 100644 --- a/app/assets/stylesheets/snippets.scss +++ b/app/assets/stylesheets/snippets.scss @@ -11,7 +11,7 @@ line-height: $code-line-height; color: $gl-text-color; margin: 20px; - font-weight: 200; + font-weight: $gl-font-weight-normal; .gl-snippet-icon { display: inline-block; @@ -34,7 +34,7 @@ .file-content.code { border: $border-style; - border-radius: 0 0 4px 4px; + border-radius: 0 0 $border-radius-default $border-radius-default; display: flex; box-shadow: none; margin: 0; @@ -45,6 +45,7 @@ overflow-x: auto; pre { + height: 100%; padding: 10px; border: 0; border-radius: 0; @@ -110,17 +111,13 @@ } } - .gitlab-logo { - display: inline-block; - padding-left: 5px; - text-decoration: none; - color: $gl-text-color-secondary; + .gitlab-logo-wrapper { + padding-left: $gl-padding-8; + position: relative; + top: 2px; - .logo-text { - background: image_url('ext_snippet_icons/logo.png') no-repeat left center; - background-size: 18px; - font-weight: $gl-font-weight-normal; - padding-left: 24px; + .gitlab-logo { + height: 18px; } } } @@ -128,7 +125,7 @@ img, .gl-snippet-icon { display: inline-block; - vertical-align: middle; + vertical-align: text-bottom; } } @@ -136,7 +133,7 @@ a.btn { background-color: $white-light; text-decoration: none; - padding: 7px 9px; + padding: 8px 9px; border: $border-style; border-right: 0; @@ -147,11 +144,11 @@ } &:first-child { - border-radius: 3px 0 0 3px; + border-radius: $border-radius-default 0 0 $border-radius-default; } &:last-child { - border-radius: 0 3px 3px 0; + border-radius: 0 $border-radius-default $border-radius-default 0; border-right: $border-style; } } diff --git a/app/controllers/projects/forks_controller.rb b/app/controllers/projects/forks_controller.rb index cb6d9c2ba18..9806b91c7e8 100644 --- a/app/controllers/projects/forks_controller.rb +++ b/app/controllers/projects/forks_controller.rb @@ -9,6 +9,7 @@ class Projects::ForksController < Projects::ApplicationController before_action :require_non_empty_project before_action :authorize_download_code! before_action :authenticate_user!, only: [:new, :create] + before_action :authorize_fork_project!, only: [:new, :create] # rubocop: disable CodeReuse/ActiveRecord def index @@ -61,6 +62,8 @@ class Projects::ForksController < Projects::ApplicationController end # rubocop: enable CodeReuse/ActiveRecord + private + def whitelist_query_limiting Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42335') end diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 76acca3b3bc..bf05defbc2e 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -391,6 +391,7 @@ class ProjectsController < Projects::ApplicationController project_feature_attributes: %i[ builds_access_level issues_access_level + forking_access_level merge_requests_access_level repository_access_level snippets_access_level diff --git a/app/graphql/types/group_type.rb b/app/graphql/types/group_type.rb index 386ae6ed4a3..393948fcede 100644 --- a/app/graphql/types/group_type.rb +++ b/app/graphql/types/group_type.rb @@ -17,6 +17,9 @@ module Types group.avatar_url(only_path: false) end + field :mentions_disabled, GraphQL::BOOLEAN_TYPE, null: true, + description: 'Indicates if a group is disabled from getting mentioned' + field :parent, GroupType, null: true, description: 'Parent group', resolve: -> (obj, _args, _ctx) { Gitlab::Graphql::Loaders::BatchModelLoader.new(Group, obj.parent_id).find } diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 7bd6c6670c1..63f1f24b611 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -563,6 +563,7 @@ module ProjectsHelper requestAccessEnabled: !!project.request_access_enabled, issuesAccessLevel: feature.issues_access_level, repositoryAccessLevel: feature.repository_access_level, + forkingAccessLevel: feature.forking_access_level, mergeRequestsAccessLevel: feature.merge_requests_access_level, buildsAccessLevel: feature.builds_access_level, wikiAccessLevel: feature.wiki_access_level, diff --git a/app/models/concerns/project_features_compatibility.rb b/app/models/concerns/project_features_compatibility.rb index 551a2e56ecf..eac676f30a5 100644 --- a/app/models/concerns/project_features_compatibility.rb +++ b/app/models/concerns/project_features_compatibility.rb @@ -50,6 +50,10 @@ module ProjectFeaturesCompatibility write_feature_attribute_string(:merge_requests_access_level, value) end + def forking_access_level=(value) + write_feature_attribute_string(:forking_access_level, value) + end + def issues_access_level=(value) write_feature_attribute_string(:issues_access_level, value) end diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 4b61eb02914..a904cf4ac46 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -128,7 +128,7 @@ module ErrorTracking # -> # http://HOST/ORG/PROJECT def self.extract_sentry_external_url(url) - url.sub('api/0/projects/', '') + url&.sub('api/0/projects/', '') end def api_host diff --git a/app/models/project.rb b/app/models/project.rb index 735c9ebda7f..c48360290c7 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -317,10 +317,12 @@ class Project < ApplicationRecord accepts_nested_attributes_for :metrics_setting, update_only: true, allow_destroy: true accepts_nested_attributes_for :grafana_integration, update_only: true, allow_destroy: true - delegate :feature_available?, :builds_enabled?, :wiki_enabled?, :merge_requests_enabled?, - :issues_enabled?, :pages_enabled?, :public_pages?, :private_pages?, - :merge_requests_access_level, :issues_access_level, :wiki_access_level, - :snippets_access_level, :builds_access_level, :repository_access_level, + delegate :feature_available?, :builds_enabled?, :wiki_enabled?, + :merge_requests_enabled?, :forking_enabled?, :issues_enabled?, + :pages_enabled?, :public_pages?, :private_pages?, + :merge_requests_access_level, :forking_access_level, :issues_access_level, + :wiki_access_level, :snippets_access_level, :builds_access_level, + :repository_access_level, to: :project_feature, allow_nil: true delegate :scheduled?, :started?, :in_progress?, :failed?, :finished?, prefix: :import, to: :import_state, allow_nil: true diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb index ec097844499..a9753c3c53a 100644 --- a/app/models/project_feature.rb +++ b/app/models/project_feature.rb @@ -22,7 +22,7 @@ class ProjectFeature < ApplicationRecord ENABLED = 20 PUBLIC = 30 - FEATURES = %i(issues merge_requests wiki snippets builds repository pages).freeze + FEATURES = %i(issues forking merge_requests wiki snippets builds repository pages).freeze PRIVATE_FEATURES_MIN_ACCESS_LEVEL = { merge_requests: Gitlab::Access::REPORTER }.freeze PRIVATE_FEATURES_MIN_ACCESS_LEVEL_FOR_PRIVATE_PROJECT = { repository: Gitlab::Access::REPORTER }.freeze STRING_OPTIONS = HashWithIndifferentAccess.new({ @@ -92,6 +92,7 @@ class ProjectFeature < ApplicationRecord default_value_for :builds_access_level, value: ENABLED, allows_nil: false default_value_for :issues_access_level, value: ENABLED, allows_nil: false + default_value_for :forking_access_level, value: ENABLED, allows_nil: false default_value_for :merge_requests_access_level, value: ENABLED, allows_nil: false default_value_for :snippets_access_level, value: ENABLED, allows_nil: false default_value_for :wiki_access_level, value: ENABLED, allows_nil: false @@ -132,6 +133,10 @@ class ProjectFeature < ApplicationRecord merge_requests_access_level > DISABLED end + def forking_enabled? + forking_access_level > DISABLED + end + def issues_enabled? issues_access_level > DISABLED end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 7b0297ea81b..ca193acb21c 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -83,6 +83,11 @@ class ProjectPolicy < BasePolicy project.merge_requests_allowing_push_to_user(user).any? end + with_scope :subject + condition(:forking_allowed) do + @subject.feature_available?(:forking, @user) + end + with_scope :global condition(:mirror_available, score: 0) do ::Gitlab::CurrentSettings.current_application_settings.mirror_available @@ -203,7 +208,6 @@ class ProjectPolicy < BasePolicy enable :download_code enable :read_statistics enable :download_wiki_code - enable :fork_project enable :create_project_snippet enable :update_issue enable :reopen_issue @@ -232,12 +236,15 @@ class ProjectPolicy < BasePolicy enable :public_access enable :guest_access - enable :fork_project enable :build_download_code enable :build_read_container_image enable :request_access end + rule { can?(:download_code) & forking_allowed }.policy do + enable :fork_project + end + rule { owner | admin | guest | group_member }.prevent :request_access rule { ~request_access_enabled }.prevent :request_access diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 25e3282d3fb..38e0a7d34ad 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -60,9 +60,7 @@ module SystemNoteService # # Returns the created Note object def change_due_date(noteable, project, author, due_date) - body = due_date ? "changed due date to #{due_date.to_s(:long)}" : 'removed due date' - - create_note(NoteSummary.new(noteable, project, author, body, action: 'due_date')) + ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_due_date(due_date) end # Called when the estimated time of a Noteable is changed @@ -80,14 +78,7 @@ module SystemNoteService # # Returns the created Note object def change_time_estimate(noteable, project, author) - parsed_time = Gitlab::TimeTrackingFormatter.output(noteable.time_estimate) - body = if noteable.time_estimate == 0 - "removed time estimate" - else - "changed time estimate to #{parsed_time}" - end - - create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking')) + ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_time_estimate end # Called when the spent time of a Noteable is changed @@ -105,21 +96,7 @@ module SystemNoteService # # Returns the created Note object def change_time_spent(noteable, project, author) - time_spent = noteable.time_spent - - if time_spent == :reset - body = "removed time spent" - else - spent_at = noteable.spent_at - parsed_time = Gitlab::TimeTrackingFormatter.output(time_spent.abs) - action = time_spent > 0 ? 'added' : 'subtracted' - - text_parts = ["#{action} #{parsed_time} of time spent"] - text_parts << "at #{spent_at}" if spent_at - body = text_parts.join(' ') - end - - create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking')) + ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_time_spent end def change_status(noteable, project, author, status, source = nil) diff --git a/app/services/system_notes/time_tracking_service.rb b/app/services/system_notes/time_tracking_service.rb new file mode 100644 index 00000000000..8de42bd3225 --- /dev/null +++ b/app/services/system_notes/time_tracking_service.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module SystemNotes + class TimeTrackingService < ::SystemNotes::BaseService + # Called when the due_date of a Noteable is changed + # + # due_date - Due date being assigned, or nil + # + # Example Note text: + # + # "removed due date" + # + # "changed due date to September 20, 2018" + # + # Returns the created Note object + def change_due_date(due_date) + body = due_date ? "changed due date to #{due_date.to_s(:long)}" : 'removed due date' + + create_note(NoteSummary.new(noteable, project, author, body, action: 'due_date')) + end + + # Called when the estimated time of a Noteable is changed + # + # time_estimate - Estimated time + # + # Example Note text: + # + # "removed time estimate" + # + # "changed time estimate to 3d 5h" + # + # Returns the created Note object + def change_time_estimate + parsed_time = Gitlab::TimeTrackingFormatter.output(noteable.time_estimate) + body = if noteable.time_estimate == 0 + "removed time estimate" + else + "changed time estimate to #{parsed_time}" + end + + create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking')) + end + + # Called when the spent time of a Noteable is changed + # + # time_spent - Spent time + # + # Example Note text: + # + # "removed time spent" + # + # "added 2h 30m of time spent" + # + # Returns the created Note object + def change_time_spent + time_spent = noteable.time_spent + + if time_spent == :reset + body = "removed time spent" + else + spent_at = noteable.spent_at + parsed_time = Gitlab::TimeTrackingFormatter.output(time_spent.abs) + action = time_spent > 0 ? 'added' : 'subtracted' + + text_parts = ["#{action} #{parsed_time} of time spent"] + text_parts << "at #{spent_at}" if spent_at + body = text_parts.join(' ') + end + + create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking')) + end + end +end diff --git a/app/views/shared/snippets/_embed.html.haml b/app/views/shared/snippets/_embed.html.haml index d2e35511b32..b401820daf6 100644 --- a/app/views/shared/snippets/_embed.html.haml +++ b/app/views/shared/snippets/_embed.html.haml @@ -10,10 +10,8 @@ %small = number_to_human_size(blob.raw_size) - %a.gitlab-logo{ href: url_for(only_path: false, overwrite_params: nil), title: 'view on gitlab' } - on - %span.logo-text - GitLab + %a.gitlab-logo-wrapper{ href: url_for(only_path: false, overwrite_params: nil), title: 'view on gitlab' } + %img.gitlab-logo{ src: image_url('ext_snippet_icons/logo.svg'), alt: "GitLab logo" } .file-actions.d-none.d-sm-block .btn-group{ role: "group" }< diff --git a/changelogs/unreleased/15082-restrict_project_forks.yml b/changelogs/unreleased/15082-restrict_project_forks.yml new file mode 100644 index 00000000000..a178e2108c2 --- /dev/null +++ b/changelogs/unreleased/15082-restrict_project_forks.yml @@ -0,0 +1,5 @@ +--- +title: Add an option to configure forking restriction +merge_request: 17988 +author: +type: added diff --git a/changelogs/unreleased/default-epic_new_issue-ff.yml b/changelogs/unreleased/default-epic_new_issue-ff.yml new file mode 100644 index 00000000000..ea6ed8e479a --- /dev/null +++ b/changelogs/unreleased/default-epic_new_issue-ff.yml @@ -0,0 +1,5 @@ +--- +title: Add ability to create an issue in an epic +merge_request: 22833 +author: +type: added diff --git a/changelogs/unreleased/embed-snippet-ui-polish.yml b/changelogs/unreleased/embed-snippet-ui-polish.yml new file mode 100644 index 00000000000..a0836f5f443 --- /dev/null +++ b/changelogs/unreleased/embed-snippet-ui-polish.yml @@ -0,0 +1,5 @@ +--- +title: Fix embedded snippets UI polish issues +merge_request: !22637 +author: +type: changed diff --git a/changelogs/unreleased/jej-prevent-enabling-group-managed-accounts-when-SSO-not-linked.yml b/changelogs/unreleased/jej-prevent-enabling-group-managed-accounts-when-SSO-not-linked.yml new file mode 100644 index 00000000000..e9ca185f90a --- /dev/null +++ b/changelogs/unreleased/jej-prevent-enabling-group-managed-accounts-when-SSO-not-linked.yml @@ -0,0 +1,5 @@ +--- +title: Require group owner to have linked SAML before enabling Group Managed Accounts +merge_request: 21721 +author: +type: fixed diff --git a/changelogs/unreleased/pl-nil-guard-extract-sentry-external-url.yml b/changelogs/unreleased/pl-nil-guard-extract-sentry-external-url.yml new file mode 100644 index 00000000000..01a0bbf8326 --- /dev/null +++ b/changelogs/unreleased/pl-nil-guard-extract-sentry-external-url.yml @@ -0,0 +1,5 @@ +--- +title: Fix extracting Sentry external URL when URL is nil +merge_request: 23162 +author: +type: fixed diff --git a/changelogs/unreleased/refactor-expose-missing-mention-disabled-group-api.yml b/changelogs/unreleased/refactor-expose-missing-mention-disabled-group-api.yml new file mode 100644 index 00000000000..f919ee3a00c --- /dev/null +++ b/changelogs/unreleased/refactor-expose-missing-mention-disabled-group-api.yml @@ -0,0 +1,5 @@ +--- +title: Expose mentions_disabled value via group API +merge_request: 23070 +author: Fabio Huser +type: added diff --git a/db/migrate/20200104113850_add_forking_access_level_to_project_feature.rb b/db/migrate/20200104113850_add_forking_access_level_to_project_feature.rb new file mode 100644 index 00000000000..2c5f764ebf3 --- /dev/null +++ b/db/migrate/20200104113850_add_forking_access_level_to_project_feature.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddForkingAccessLevelToProjectFeature < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def change + add_column :project_features, :forking_access_level, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index ebe75bf5abc..8c019f96f94 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -3167,6 +3167,7 @@ ActiveRecord::Schema.define(version: 2020_01_14_204949) do t.datetime "updated_at" t.integer "repository_access_level", default: 20, null: false t.integer "pages_access_level", null: false + t.integer "forking_access_level" t.index ["project_id"], name: "index_project_features_on_project_id", unique: true end diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 206590092a4..23ffae3b097 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -2595,6 +2595,11 @@ type Group { lfsEnabled: Boolean """ + Indicates if a group is disabled from getting mentioned + """ + mentionsDisabled: Boolean + + """ Name of the namespace """ name: String! diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 0240876c53d..6239a398c7e 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -3461,6 +3461,20 @@ "deprecationReason": null }, { + "name": "mentionsDisabled", + "description": "Indicates if a group is disabled from getting mentioned", + "args": [ + + ], + "type": { + "kind": "SCALAR", + "name": "Boolean", + "ofType": null + }, + "isDeprecated": false, + "deprecationReason": null + }, + { "name": "name", "description": "Name of the namespace", "args": [ diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 4a9cbed7dca..72fc82444ca 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -393,6 +393,7 @@ Autogenerated return type of EpicTreeReorder | `userPermissions` | GroupPermissions! | Permissions for the current user on the resource | | `webUrl` | String! | Web URL of the group | | `avatarUrl` | String | Avatar URL of the group | +| `mentionsDisabled` | Boolean | Indicates if a group is disabled from getting mentioned | | `parent` | Group | Parent group | | `epicsEnabled` | Boolean | Indicates if Epics are enabled for namespace | | `groupTimelogsEnabled` | Boolean | Indicates if Group timelogs are enabled for namespace | diff --git a/doc/api/groups.md b/doc/api/groups.md index 32e2a88f25b..f4dfefe3cb7 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -40,6 +40,7 @@ GET /groups "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, + "mentions_disabled": null, "lfs_enabled": true, "avatar_url": "http://localhost:3000/uploads/group/avatar/1/foo.jpg", "web_url": "http://localhost:3000/groups/foo-bar", @@ -73,6 +74,7 @@ GET /groups?statistics=true "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, + "mentions_disabled": null, "lfs_enabled": true, "avatar_url": "http://localhost:3000/uploads/group/avatar/1/foo.jpg", "web_url": "http://localhost:3000/groups/foo-bar", @@ -144,6 +146,7 @@ GET /groups/:id/subgroups "auto_devops_enabled": null, "subgroup_creation_level": "owner", "emails_disabled": null, + "mentions_disabled": null, "lfs_enabled": true, "avatar_url": "http://gitlab.example.com/uploads/group/avatar/1/foo.jpg", "web_url": "http://gitlab.example.com/groups/foo-bar", @@ -486,6 +489,7 @@ Parameters: | `auto_devops_enabled` | boolean | no | Default to Auto DevOps pipeline for all projects within this group. | | `subgroup_creation_level` | integer | no | Allowed to create subgroups. Can be `owner` (Owners), or `maintainer` (Maintainers). | | `emails_disabled` | boolean | no | Disable email notifications | +| `mentions_disabled` | boolean | no | Disable the capability of a group from getting mentioned | | `lfs_enabled` | boolean | no | Enable/disable Large File Storage (LFS) for the projects in this group. | | `request_access_enabled` | boolean | no | Allow users to request member access. | | `parent_id` | integer | no | The parent group ID for creating nested group. | @@ -531,6 +535,7 @@ PUT /groups/:id | `auto_devops_enabled` | boolean | no | Default to Auto DevOps pipeline for all projects within this group. | | `subgroup_creation_level` | integer | no | Allowed to create subgroups. Can be `owner` (Owners), or `maintainer` (Maintainers). | | `emails_disabled` | boolean | no | Disable email notifications | +| `mentions_disabled` | boolean | no | Disable the capability of a group from getting mentioned | | `lfs_enabled` (optional) | boolean | no | Enable/disable Large File Storage (LFS) for the projects in this group. | | `request_access_enabled` | boolean | no | Allow users to request member access. | | `file_template_project_id` | integer | no | **(PREMIUM)** The ID of a project to load custom file templates from. | diff --git a/doc/development/README.md b/doc/development/README.md index c5830bbded0..d551e6f471e 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -17,7 +17,7 @@ description: 'Learn how to contribute to GitLab.' - [GitLab core team & GitLab Inc. contribution process](https://gitlab.com/gitlab-org/gitlab/blob/master/PROCESS.md) - [Generate a changelog entry with `bin/changelog`](changelog.md) - [Code review guidelines](code_review.md) for reviewing code and having code reviewed -- [Database review guidelines](database_review.md) for reviewing database-related changes and complex SQL queries +- [Database review guidelines](database_review.md) for reviewing database-related changes and complex SQL queries, and having them reviewed - [Pipelines for the GitLab project](pipelines.md) - [Guidelines for implementing Enterprise Edition features](ee_features.md) - [Security process for developers](https://gitlab.com/gitlab-org/release/docs/blob/master/general/security/developer.md#security-releases-critical-non-critical-as-a-developer) diff --git a/doc/user/group/index.md b/doc/user/group/index.md index ad16aaa34ff..bf0a8c6bfbd 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -204,6 +204,25 @@ and give all group members access to the project at once. Alternatively, you can [lock the sharing with group feature](#share-with-group-lock). +## Sharing a group with another group + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/18328) in GitLab 12.7. + +Similarly to [sharing a project with a group](#sharing-a-project-with-a-group), +you can share a group with another group to give direct group members access +to the shared group. This is not valid for inherited members. + +To share a given group, for example, 'Frontend' with another group, for example, +'Engineering': + +1. Navigate to your 'Frontend' group page and use the left navigation menu to go + to your group **Members**. +1. Select the **Invite group** tab. +1. Add 'Engineering' with the maximum access level of your choice. +1. Click **Invite**. + +All the members of the 'Engineering' group will have been added to 'Frontend'. + ## Manage group memberships via LDAP In GitLab Enterprise Edition, it is possible to manage GitLab group memberships using LDAP groups. diff --git a/doc/user/project/settings/index.md b/doc/user/project/settings/index.md index 8fa005cc39c..4e55f55dd28 100644 --- a/doc/user/project/settings/index.md +++ b/doc/user/project/settings/index.md @@ -43,6 +43,7 @@ Use the switches to enable or disable the following features: | **Issues** | ✓ | Activates the GitLab issues tracker | | **Repository** | ✓ | Enables [repository](../repository/) functionality | | **Merge Requests** | ✓ | Enables [merge request](../merge_requests/) functionality; also see [Merge request settings](#merge-request-settings) | +| **Forks** | ✓ | Enables [forking](../index.md#fork-a-project) functionality | | **Pipelines** | ✓ | Enables [CI/CD](../../../ci/README.md) functionality | | **Container Registry** | | Activates a [registry](../../packages/container_registry/) for your docker images | | **Git Large File Storage** | | Enables the use of [large files](../../../administration/lfs/manage_large_binaries_with_git_lfs.md#git-lfs) | diff --git a/lib/api/entities.rb b/lib/api/entities.rb index a2813d0e063..dfd0e676586 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -414,6 +414,7 @@ module API expose :auto_devops_enabled expose :subgroup_creation_level_str, as: :subgroup_creation_level expose :emails_disabled + expose :mentions_disabled expose :lfs_enabled?, as: :lfs_enabled expose :avatar_url do |group, options| group.avatar_url(only_path: false) diff --git a/lib/api/helpers/groups_helpers.rb b/lib/api/helpers/groups_helpers.rb index 2cc18acb7ec..e0fea4c7c96 100644 --- a/lib/api/helpers/groups_helpers.rb +++ b/lib/api/helpers/groups_helpers.rb @@ -18,6 +18,7 @@ module API optional :auto_devops_enabled, type: Boolean, desc: 'Default to Auto DevOps pipeline for all projects within this group' optional :subgroup_creation_level, type: String, values: ::Gitlab::Access.subgroup_creation_string_values, desc: 'Allowed to create subgroups', as: :subgroup_creation_level_str optional :emails_disabled, type: Boolean, desc: 'Disable email notifications' + optional :mentions_disabled, type: Boolean, desc: 'Disable a group from getting mentioned' optional :lfs_enabled, type: Boolean, desc: 'Enable/disable LFS for the projects in this group' optional :request_access_enabled, type: Boolean, desc: 'Allow users to request member access' end diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 967db0c628c..124581c961f 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -35,6 +35,12 @@ module Gitlab message: 'key may not be used with `rules`' }, if: :has_rules? + validates :config, + disallowed_keys: { + in: %i[release], + message: 'release features are not enabled' + }, + unless: -> { Feature.enabled?(:ci_release_generation, default_enabled: false) } with_options allow_nil: true do validates :allow_failure, boolean: true diff --git a/lib/gitlab/import_export/attribute_cleaner.rb b/lib/gitlab/import_export/attribute_cleaner.rb index 00c4c41e6be..d1c20dff799 100644 --- a/lib/gitlab/import_export/attribute_cleaner.rb +++ b/lib/gitlab/import_export/attribute_cleaner.rb @@ -3,7 +3,14 @@ module Gitlab module ImportExport class AttributeCleaner - ALLOWED_REFERENCES = RelationFactory::PROJECT_REFERENCES + RelationFactory::USER_REFERENCES + %w[group_id commit_id discussion_id custom_attributes] + ALLOWED_REFERENCES = [ + *ProjectRelationFactory::PROJECT_REFERENCES, + *ProjectRelationFactory::USER_REFERENCES, + 'group_id', + 'commit_id', + 'discussion_id', + 'custom_attributes' + ].freeze PROHIBITED_REFERENCES = Regexp.union(/\Acached_markdown_version\Z/, /_id\Z/, /_ids\Z/, /_html\Z/, /attributes/).freeze def self.clean(*args) diff --git a/lib/gitlab/import_export/base_object_builder.rb b/lib/gitlab/import_export/base_object_builder.rb new file mode 100644 index 00000000000..ec66b7a7a4f --- /dev/null +++ b/lib/gitlab/import_export/base_object_builder.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +module Gitlab + module ImportExport + # Base class for Group & Project Object Builders. + # This class is not intended to be used on its own but + # rather inherited from. + # + # Cache keeps 1000 entries at most, 1000 is chosen based on: + # - one cache entry uses around 0.5K memory, 1000 items uses around 500K. + # (leave some buffer it should be less than 1M). It is afforable cost for project import. + # - for projects in Gitlab.com, it seems 1000 entries for labels/milestones is enough. + # For example, gitlab has ~970 labels and 26 milestones. + LRU_CACHE_SIZE = 1000 + + class BaseObjectBuilder + def self.build(*args) + new(*args).find + end + + def initialize(klass, attributes) + @klass = klass.ancestors.include?(Label) ? Label : klass + @attributes = attributes + + if Gitlab::SafeRequestStore.active? + @lru_cache = cache_from_request_store + @cache_key = [klass, attributes] + end + end + + def find + find_with_cache do + find_object || klass.create(prepare_attributes) + end + end + + protected + + def where_clauses + raise NotImplementedError + end + + # attributes wrapped in a method to be + # adjusted in sub-class if needed + def prepare_attributes + attributes + end + + private + + attr_reader :klass, :attributes, :lru_cache, :cache_key + + def find_with_cache + return yield unless lru_cache && cache_key + + lru_cache[cache_key] ||= yield + end + + def cache_from_request_store + Gitlab::SafeRequestStore[:lru_cache] ||= LruRedux::Cache.new(LRU_CACHE_SIZE) + end + + def find_object + klass.where(where_clause).first + end + + def where_clause + where_clauses.reduce(:and) + end + + def table + @table ||= klass.arel_table + end + + # Returns Arel clause: + # `"{table_name}"."{attrs.keys[0]}" = '{attrs.values[0]} AND {table_name}"."{attrs.keys[1]}" = '{attrs.values[1]}"` + # from the given Hash of attributes. + def attrs_to_arel(attrs) + attrs.map do |key, value| + table[key].eq(value) + end.reduce(:and) + end + + # Returns Arel clause `"{table_name}"."title" = '{attributes['title']}'` + # if attributes has 'title key, otherwise `nil`. + def where_clause_for_title + attrs_to_arel(attributes.slice('title')) + end + + # Returns Arel clause `"{table_name}"."description" = '{attributes['description']}'` + # if attributes has 'description key, otherwise `nil`. + def where_clause_for_description + attrs_to_arel(attributes.slice('description')) + end + + # Returns Arel clause `"{table_name}"."created_at" = '{attributes['created_at']}'` + # if attributes has 'created_at key, otherwise `nil`. + def where_clause_for_created_at + attrs_to_arel(attributes.slice('created_at')) + end + end + end +end diff --git a/lib/gitlab/import_export/base_relation_factory.rb b/lib/gitlab/import_export/base_relation_factory.rb new file mode 100644 index 00000000000..562b549f6a1 --- /dev/null +++ b/lib/gitlab/import_export/base_relation_factory.rb @@ -0,0 +1,306 @@ +# frozen_string_literal: true + +module Gitlab + module ImportExport + class BaseRelationFactory + include Gitlab::Utils::StrongMemoize + + IMPORTED_OBJECT_MAX_RETRIES = 5.freeze + + OVERRIDES = {}.freeze + EXISTING_OBJECT_RELATIONS = %i[].freeze + + # This represents all relations that have unique key on `project_id` or `group_id` + UNIQUE_RELATIONS = %i[].freeze + + USER_REFERENCES = %w[ + author_id + assignee_id + updated_by_id + merged_by_id + latest_closed_by_id + user_id + created_by_id + last_edited_by_id + merge_user_id + resolved_by_id + closed_by_id owner_id + ].freeze + + TOKEN_RESET_MODELS = %i[Project Namespace Group Ci::Trigger Ci::Build Ci::Runner ProjectHook].freeze + + def self.create(*args) + new(*args).create + end + + def self.relation_class(relation_name) + # There are scenarios where the model is pluralized (e.g. + # MergeRequest::Metrics), and we don't want to force it to singular + # with #classify. + relation_name.to_s.classify.constantize + rescue NameError + relation_name.to_s.constantize + end + + def initialize(relation_sym:, relation_hash:, members_mapper:, object_builder:, merge_requests_mapping: nil, user:, importable:, excluded_keys: []) + @relation_name = self.class.overrides[relation_sym]&.to_sym || relation_sym + @relation_hash = relation_hash.except('noteable_id') + @members_mapper = members_mapper + @object_builder = object_builder + @merge_requests_mapping = merge_requests_mapping + @user = user + @importable = importable + @imported_object_retries = 0 + @relation_hash[importable_column_name] = @importable.id + + # Remove excluded keys from relation_hash + # We don't do this in the parsed_relation_hash because of the 'transformed attributes' + # For example, MergeRequestDiffFiles exports its diff attribute as utf8_diff. Then, + # in the create method that attribute is renamed to diff. And because diff is an excluded key, + # if we clean the excluded keys in the parsed_relation_hash, it will be removed + # from the object attributes and the export will fail. + @relation_hash.except!(*excluded_keys) + end + + # Creates an object from an actual model with name "relation_sym" with params from + # the relation_hash, updating references with new object IDs, mapping users using + # the "members_mapper" object, also updating notes if required. + def create + return if invalid_relation? + + setup_base_models + setup_models + + generate_imported_object + end + + def self.overrides + self::OVERRIDES + end + + def self.existing_object_relations + self::EXISTING_OBJECT_RELATIONS + end + + private + + def invalid_relation? + false + end + + def setup_models + raise NotImplementedError + end + + def unique_relations + # define in sub-class if any + self.class::UNIQUE_RELATIONS + end + + def setup_base_models + update_user_references + remove_duplicate_assignees + reset_tokens! + remove_encrypted_attributes! + end + + def update_user_references + self.class::USER_REFERENCES.each do |reference| + if @relation_hash[reference] + @relation_hash[reference] = @members_mapper.map[@relation_hash[reference]] + end + end + end + + def remove_duplicate_assignees + return unless @relation_hash['issue_assignees'] + + # When an assignee did not exist in the members mapper, the importer is + # assigned. We only need to assign each user once. + @relation_hash['issue_assignees'].uniq!(&:user_id) + end + + def generate_imported_object + imported_object + end + + def reset_tokens! + return unless Gitlab::ImportExport.reset_tokens? && self.class::TOKEN_RESET_MODELS.include?(@relation_name) + + # If we import/export to the same instance, tokens will have to be reset. + # We also have to reset them to avoid issues when the gitlab secrets file cannot be copied across. + relation_class.attribute_names.select { |name| name.include?('token') }.each do |token| + @relation_hash[token] = nil + end + end + + def remove_encrypted_attributes! + return unless relation_class.respond_to?(:encrypted_attributes) && relation_class.encrypted_attributes.any? + + relation_class.encrypted_attributes.each_key do |key| + @relation_hash[key.to_s] = nil + end + end + + def relation_class + @relation_class ||= self.class.relation_class(@relation_name) + end + + def importable_column_name + importable_class_name.concat('_id') + end + + def importable_class_name + @importable.class.to_s.downcase + end + + def imported_object + if existing_or_new_object.respond_to?(:importing) + existing_or_new_object.importing = true + end + + existing_or_new_object + rescue ActiveRecord::RecordNotUnique + # as the operation is not atomic, retry in the unlikely scenario an INSERT is + # performed on the same object between the SELECT and the INSERT + @imported_object_retries += 1 + retry if @imported_object_retries < IMPORTED_OBJECT_MAX_RETRIES + end + + def parsed_relation_hash + @parsed_relation_hash ||= Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: @relation_hash, + relation_class: relation_class) + end + + def existing_or_new_object + # Only find existing records to avoid mapping tables such as milestones + # Otherwise always create the record, skipping the extra SELECT clause. + @existing_or_new_object ||= begin + if existing_object? + attribute_hash = attribute_hash_for(['events']) + + existing_object.assign_attributes(attribute_hash) if attribute_hash.any? + + existing_object + else + # Because of single-type inheritance, we need to be careful to use the `type` field + # See https://gitlab.com/gitlab-org/gitlab/issues/34860#note_235321497 + inheritance_column = relation_class.try(:inheritance_column) + inheritance_attributes = parsed_relation_hash.slice(inheritance_column) + object = relation_class.new(inheritance_attributes) + object.assign_attributes(parsed_relation_hash) + object + end + end + end + + def attribute_hash_for(attributes) + attributes.each_with_object({}) do |hash, value| + hash[value] = parsed_relation_hash.delete(value) if parsed_relation_hash[value] + hash + end + end + + def existing_object + @existing_object ||= find_or_create_object! + end + + def unique_relation_object + unique_relation_object = relation_class.find_or_create_by(importable_column_name => @importable.id) + unique_relation_object.assign_attributes(parsed_relation_hash) + unique_relation_object + end + + def find_or_create_object! + return unique_relation_object if unique_relation? + + # Can't use IDs as validation exists calling `group` or `project` attributes + finder_hash = parsed_relation_hash.tap do |hash| + if relation_class.attribute_method?('group_id') && @importable.is_a?(Project) + hash['group'] = @importable.group + end + + hash[importable_class_name] = @importable if relation_class.reflect_on_association(importable_class_name.to_sym) + hash.delete(importable_column_name) + end + + @object_builder.build(relation_class, finder_hash) + end + + def setup_note + set_note_author + # attachment is deprecated and note uploads are handled by Markdown uploader + @relation_hash['attachment'] = nil + end + + # Sets the author for a note. If the user importing the project + # has admin access, an actual mapping with new project members + # will be used. Otherwise, a note stating the original author name + # is left. + def set_note_author + old_author_id = @relation_hash['author_id'] + author = @relation_hash.delete('author') + + update_note_for_missing_author(author['name']) unless has_author?(old_author_id) + end + + def has_author?(old_author_id) + admin_user? && @members_mapper.include?(old_author_id) + end + + def missing_author_note(updated_at, author_name) + timestamp = updated_at.split('.').first + "\n\n *By #{author_name} on #{timestamp} (imported from GitLab project)*" + end + + def update_note_for_missing_author(author_name) + @relation_hash['note'] = '*Blank note*' if @relation_hash['note'].blank? + @relation_hash['note'] = "#{@relation_hash['note']}#{missing_author_note(@relation_hash['updated_at'], author_name)}" + end + + def admin_user? + @user.admin? + end + + def existing_object? + strong_memoize(:_existing_object) do + self.class.existing_object_relations.include?(@relation_name) || unique_relation? + end + end + + def unique_relation? + strong_memoize(:unique_relation) do + importable_foreign_key.present? && + (has_unique_index_on_importable_fk? || uses_importable_fk_as_primary_key?) + end + end + + def has_unique_index_on_importable_fk? + cache = cached_has_unique_index_on_importable_fk + table_name = relation_class.table_name + return cache[table_name] if cache.has_key?(table_name) + + index_exists = + ActiveRecord::Base.connection.index_exists?( + relation_class.table_name, + importable_foreign_key, + unique: true) + + cache[table_name] = index_exists + end + + # Avoid unnecessary DB requests + def cached_has_unique_index_on_importable_fk + Thread.current[:cached_has_unique_index_on_importable_fk] ||= {} + end + + def uses_importable_fk_as_primary_key? + relation_class.primary_key == importable_foreign_key + end + + def importable_foreign_key + relation_class.reflect_on_association(importable_class_name.to_sym)&.foreign_key + end + end + end +end diff --git a/lib/gitlab/import_export/group_project_object_builder.rb b/lib/gitlab/import_export/group_project_object_builder.rb index 78ba4894459..d6d780f165e 100644 --- a/lib/gitlab/import_export/group_project_object_builder.rb +++ b/lib/gitlab/import_export/group_project_object_builder.rb @@ -11,61 +11,29 @@ module Gitlab # finds or initializes a label with the given attributes. # # It also adds some logic around Group Labels/Milestones for edge cases. - class GroupProjectObjectBuilder - # Cache keeps 1000 entries at most, 1000 is chosen based on: - # - one cache entry uses around 0.5K memory, 1000 items uses around 500K. - # (leave some buffer it should be less than 1M). It is afforable cost for project import. - # - for projects in Gitlab.com, it seems 1000 entries for labels/milestones is enough. - # For example, gitlab has ~970 labels and 26 milestones. - LRU_CACHE_SIZE = 1000 - + class GroupProjectObjectBuilder < BaseObjectBuilder def self.build(*args) Project.transaction do - new(*args).find + super end end def initialize(klass, attributes) - @klass = klass < Label ? Label : klass - @attributes = attributes + super + @group = @attributes['group'] @project = @attributes['project'] - - if Gitlab::SafeRequestStore.active? - @lru_cache = cache_from_request_store - @cache_key = [klass, attributes] - end end def find return if epic? && group.nil? - find_with_cache do - find_object || klass.create(project_attributes) - end + super end private - attr_reader :klass, :attributes, :group, :project, :lru_cache, :cache_key - - def find_with_cache - return yield unless lru_cache && cache_key - - lru_cache[cache_key] ||= yield - end - - def cache_from_request_store - Gitlab::SafeRequestStore[:lru_cache] ||= LruRedux::Cache.new(LRU_CACHE_SIZE) - end - - def find_object - klass.where(where_clause).first - end - - def where_clause - where_clauses.reduce(:and) - end + attr_reader :group, :project def where_clauses [ @@ -86,26 +54,12 @@ module Gitlab end.reduce(:or) end - # Returns Arel clause `"{table_name}"."title" = '{attributes['title']}'` - # if attributes has 'title key, otherwise `nil`. - def where_clause_for_title - attrs_to_arel(attributes.slice('title')) - end - - # Returns Arel clause: - # `"{table_name}"."{attrs.keys[0]}" = '{attrs.values[0]} AND {table_name}"."{attrs.keys[1]}" = '{attrs.values[1]}"` - # from the given Hash of attributes. - def attrs_to_arel(attrs) - attrs.map do |key, value| - table[key].eq(value) - end.reduce(:and) - end - - def table - @table ||= klass.arel_table + # Returns Arel clause for a particular model or `nil`. + def where_clause_for_klass + attrs_to_arel(attributes.slice('iid')) if merge_request? end - def project_attributes + def prepare_attributes attributes.except('group').tap do |atts| if label? atts['type'] = 'ProjectLabel' # Always create project labels @@ -154,13 +108,6 @@ module Gitlab milestone.ensure_project_iid! milestone.save! end - - protected - - # Returns Arel clause for a particular model or `nil`. - def where_clause_for_klass - return attrs_to_arel(attributes.slice('iid')) if merge_request? - end end end end diff --git a/lib/gitlab/import_export/project_relation_factory.rb b/lib/gitlab/import_export/project_relation_factory.rb new file mode 100644 index 00000000000..e27bb9d3af1 --- /dev/null +++ b/lib/gitlab/import_export/project_relation_factory.rb @@ -0,0 +1,184 @@ +# frozen_string_literal: true + +module Gitlab + module ImportExport + class ProjectRelationFactory < BaseRelationFactory + prepend_if_ee('::EE::Gitlab::ImportExport::ProjectRelationFactory') # rubocop: disable Cop/InjectEnterpriseEditionModule + + OVERRIDES = { snippets: :project_snippets, + ci_pipelines: 'Ci::Pipeline', + pipelines: 'Ci::Pipeline', + stages: 'Ci::Stage', + statuses: 'commit_status', + triggers: 'Ci::Trigger', + pipeline_schedules: 'Ci::PipelineSchedule', + builds: 'Ci::Build', + runners: 'Ci::Runner', + hooks: 'ProjectHook', + merge_access_levels: 'ProtectedBranch::MergeAccessLevel', + push_access_levels: 'ProtectedBranch::PushAccessLevel', + create_access_levels: 'ProtectedTag::CreateAccessLevel', + labels: :project_labels, + priorities: :label_priorities, + auto_devops: :project_auto_devops, + label: :project_label, + custom_attributes: 'ProjectCustomAttribute', + project_badges: 'Badge', + metrics: 'MergeRequest::Metrics', + ci_cd_settings: 'ProjectCiCdSetting', + error_tracking_setting: 'ErrorTracking::ProjectErrorTrackingSetting', + links: 'Releases::Link', + metrics_setting: 'ProjectMetricsSetting' }.freeze + + BUILD_MODELS = %i[Ci::Build commit_status].freeze + + GROUP_REFERENCES = %w[group_id].freeze + + PROJECT_REFERENCES = %w[project_id source_project_id target_project_id].freeze + + EXISTING_OBJECT_RELATIONS = %i[ + milestone + milestones + label + labels + project_label + project_labels + group_label + group_labels + project_feature + merge_request + epic + ProjectCiCdSetting + container_expiration_policy + ].freeze + + def create + @object = super + + # We preload the project, user, and group to re-use objects + @object = preload_keys(@object, PROJECT_REFERENCES, @importable) + @object = preload_keys(@object, GROUP_REFERENCES, @importable.group) + @object = preload_keys(@object, USER_REFERENCES, @user) + end + + private + + def invalid_relation? + # Do not create relation if it is: + # - An unknown service + # - A legacy trigger + unknown_service? || + (!Feature.enabled?(:use_legacy_pipeline_triggers, @importable) && legacy_trigger?) + end + + def setup_models + case @relation_name + when :merge_request_diff_files then setup_diff + when :notes then setup_note + when :'Ci::Pipeline' then setup_pipeline + when *BUILD_MODELS then setup_build + end + + update_project_references + update_group_references + end + + def generate_imported_object + if @relation_name == :merge_requests + MergeRequestParser.new(@importable, @relation_hash.delete('diff_head_sha'), super, @relation_hash).parse! + else + super + end + end + + def update_project_references + # If source and target are the same, populate them with the new project ID. + if @relation_hash['source_project_id'] + @relation_hash['source_project_id'] = same_source_and_target? ? @relation_hash['project_id'] : MergeRequestParser::FORKED_PROJECT_ID + end + + @relation_hash['target_project_id'] = @relation_hash['project_id'] if @relation_hash['target_project_id'] + end + + def same_source_and_target? + @relation_hash['target_project_id'] && @relation_hash['target_project_id'] == @relation_hash['source_project_id'] + end + + def update_group_references + return unless existing_object? + return unless @relation_hash['group_id'] + + @relation_hash['group_id'] = @importable.namespace_id + end + + # This code is a workaround for broken project exports that don't + # export merge requests with CI pipelines (i.e. exports that were + # generated from + # https://gitlab.com/gitlab-org/gitlab/merge_requests/17844). + # This method can be removed in GitLab 12.6. + def update_merge_request_references + # If a merge request was properly created, we don't need to fix + # up this export. + return if @relation_hash['merge_request'] + + merge_request_id = @relation_hash['merge_request_id'] + + return unless merge_request_id + + new_merge_request_id = @merge_requests_mapping[merge_request_id] + + return unless new_merge_request_id + + @relation_hash['merge_request_id'] = new_merge_request_id + parsed_relation_hash['merge_request_id'] = new_merge_request_id + end + + def setup_build + @relation_hash.delete('trace') # old export files have trace + @relation_hash.delete('token') + @relation_hash.delete('commands') + @relation_hash.delete('artifacts_file_store') + @relation_hash.delete('artifacts_metadata_store') + @relation_hash.delete('artifacts_size') + end + + def setup_diff + @relation_hash['diff'] = @relation_hash.delete('utf8_diff') + end + + def setup_pipeline + update_merge_request_references + + @relation_hash.fetch('stages', []).each do |stage| + stage.statuses.each do |status| + status.pipeline = imported_object + end + end + end + + def unknown_service? + @relation_name == :services && parsed_relation_hash['type'] && + !Object.const_defined?(parsed_relation_hash['type']) + end + + def legacy_trigger? + @relation_name == :'Ci::Trigger' && @relation_hash['owner_id'].nil? + end + + def preload_keys(object, references, value) + return object unless value + + references.each do |key| + attribute = "#{key.delete_suffix('_id')}=".to_sym + next unless object.respond_to?(key) && object.respond_to?(attribute) + + if object.read_attribute(key) == value&.id + object.public_send(attribute, value) # rubocop:disable GitlabSecurity/PublicSend + end + end + + object + end + end + end +end diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb index f309f106ec9..e598cfc143e 100644 --- a/lib/gitlab/import_export/project_tree_restorer.rb +++ b/lib/gitlab/import_export/project_tree_restorer.rb @@ -48,6 +48,7 @@ module Gitlab shared: @shared, importable: @project, tree_hash: @tree_hash, + object_builder: object_builder, members_mapper: members_mapper, relation_factory: relation_factory, reader: reader @@ -60,8 +61,12 @@ module Gitlab importable: @project) end + def object_builder + Gitlab::ImportExport::GroupProjectObjectBuilder + end + def relation_factory - Gitlab::ImportExport::RelationFactory + Gitlab::ImportExport::ProjectRelationFactory end def reader diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb deleted file mode 100644 index 43f3b673614..00000000000 --- a/lib/gitlab/import_export/relation_factory.rb +++ /dev/null @@ -1,418 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module ImportExport - class RelationFactory - include Gitlab::Utils::StrongMemoize - - prepend_if_ee('::EE::Gitlab::ImportExport::RelationFactory') # rubocop: disable Cop/InjectEnterpriseEditionModule - - OVERRIDES = { snippets: :project_snippets, - ci_pipelines: 'Ci::Pipeline', - pipelines: 'Ci::Pipeline', - stages: 'Ci::Stage', - statuses: 'commit_status', - triggers: 'Ci::Trigger', - pipeline_schedules: 'Ci::PipelineSchedule', - builds: 'Ci::Build', - runners: 'Ci::Runner', - hooks: 'ProjectHook', - merge_access_levels: 'ProtectedBranch::MergeAccessLevel', - push_access_levels: 'ProtectedBranch::PushAccessLevel', - create_access_levels: 'ProtectedTag::CreateAccessLevel', - labels: :project_labels, - priorities: :label_priorities, - auto_devops: :project_auto_devops, - label: :project_label, - custom_attributes: 'ProjectCustomAttribute', - project_badges: 'Badge', - metrics: 'MergeRequest::Metrics', - ci_cd_settings: 'ProjectCiCdSetting', - error_tracking_setting: 'ErrorTracking::ProjectErrorTrackingSetting', - links: 'Releases::Link', - metrics_setting: 'ProjectMetricsSetting' }.freeze - - USER_REFERENCES = %w[author_id assignee_id updated_by_id merged_by_id latest_closed_by_id user_id created_by_id last_edited_by_id merge_user_id resolved_by_id closed_by_id owner_id].freeze - - PROJECT_REFERENCES = %w[project_id source_project_id target_project_id].freeze - - GROUP_REFERENCES = %w[group_id].freeze - - BUILD_MODELS = %i[Ci::Build commit_status].freeze - - IMPORTED_OBJECT_MAX_RETRIES = 5.freeze - - EXISTING_OBJECT_RELATIONS = %i[ - milestone - milestones - label - labels - project_label - project_labels - group_label - group_labels - project_feature - merge_request - epic - ProjectCiCdSetting - container_expiration_policy - ].freeze - - TOKEN_RESET_MODELS = %i[Project Namespace Ci::Trigger Ci::Build Ci::Runner ProjectHook].freeze - - def self.create(*args) - new(*args).create - end - - def self.relation_class(relation_name) - # There are scenarios where the model is pluralized (e.g. - # MergeRequest::Metrics), and we don't want to force it to singular - # with #classify. - relation_name.to_s.classify.constantize - rescue NameError - relation_name.to_s.constantize - end - - def initialize(relation_sym:, relation_hash:, members_mapper:, merge_requests_mapping:, user:, project:, excluded_keys: []) - @relation_name = self.class.overrides[relation_sym]&.to_sym || relation_sym - @relation_hash = relation_hash.except('noteable_id') - @members_mapper = members_mapper - @merge_requests_mapping = merge_requests_mapping - @user = user - @project = project - @imported_object_retries = 0 - - @relation_hash['project_id'] = @project.id - - # Remove excluded keys from relation_hash - # We don't do this in the parsed_relation_hash because of the 'transformed attributes' - # For example, MergeRequestDiffFiles exports its diff attribute as utf8_diff. Then, - # in the create method that attribute is renamed to diff. And because diff is an excluded key, - # if we clean the excluded keys in the parsed_relation_hash, it will be removed - # from the object attributes and the export will fail. - @relation_hash.except!(*excluded_keys) - end - - # Creates an object from an actual model with name "relation_sym" with params from - # the relation_hash, updating references with new object IDs, mapping users using - # the "members_mapper" object, also updating notes if required. - def create - return if unknown_service? - - setup_models - - object = generate_imported_object - - # We preload the project, user, and group to re-use objects - object = preload_keys(object, PROJECT_REFERENCES, @project) - object = preload_keys(object, GROUP_REFERENCES, @project.group) - object = preload_keys(object, USER_REFERENCES, @user) - object - end - - def self.overrides - OVERRIDES - end - - def self.existing_object_relations - EXISTING_OBJECT_RELATIONS - end - - private - - def existing_object? - strong_memoize(:_existing_object) do - self.class.existing_object_relations.include?(@relation_name) || unique_relation? - end - end - - def setup_models - case @relation_name - when :merge_request_diff_files then setup_diff - when :notes then setup_note - end - - update_user_references - update_project_references - update_group_references - remove_duplicate_assignees - - if @relation_name == :'Ci::Pipeline' - update_merge_request_references - setup_pipeline - end - - reset_tokens! - remove_encrypted_attributes! - end - - def preload_keys(object, references, value) - return object unless value - - references.each do |key| - attribute = "#{key.delete_suffix('_id')}=".to_sym - next unless object.respond_to?(key) && object.respond_to?(attribute) - - if object.read_attribute(key) == value&.id - object.public_send(attribute, value) # rubocop:disable GitlabSecurity/PublicSend - end - end - - object - end - - def update_user_references - USER_REFERENCES.each do |reference| - if @relation_hash[reference] - @relation_hash[reference] = @members_mapper.map[@relation_hash[reference]] - end - end - end - - def remove_duplicate_assignees - return unless @relation_hash['issue_assignees'] - - # When an assignee did not exist in the members mapper, the importer is - # assigned. We only need to assign each user once. - @relation_hash['issue_assignees'].uniq!(&:user_id) - end - - def setup_note - set_note_author - # attachment is deprecated and note uploads are handled by Markdown uploader - @relation_hash['attachment'] = nil - end - - # Sets the author for a note. If the user importing the project - # has admin access, an actual mapping with new project members - # will be used. Otherwise, a note stating the original author name - # is left. - def set_note_author - old_author_id = @relation_hash['author_id'] - author = @relation_hash.delete('author') - - update_note_for_missing_author(author['name']) unless has_author?(old_author_id) - end - - def has_author?(old_author_id) - admin_user? && @members_mapper.include?(old_author_id) - end - - def missing_author_note(updated_at, author_name) - timestamp = updated_at.split('.').first - "\n\n *By #{author_name} on #{timestamp} (imported from GitLab project)*" - end - - def generate_imported_object - if BUILD_MODELS.include?(@relation_name) - @relation_hash.delete('trace') # old export files have trace - @relation_hash.delete('token') - @relation_hash.delete('commands') - @relation_hash.delete('artifacts_file_store') - @relation_hash.delete('artifacts_metadata_store') - @relation_hash.delete('artifacts_size') - - imported_object - elsif @relation_name == :merge_requests - MergeRequestParser.new(@project, @relation_hash.delete('diff_head_sha'), imported_object, @relation_hash).parse! - else - imported_object - end - end - - def update_project_references - # If source and target are the same, populate them with the new project ID. - if @relation_hash['source_project_id'] - @relation_hash['source_project_id'] = same_source_and_target? ? @relation_hash['project_id'] : MergeRequestParser::FORKED_PROJECT_ID - end - - @relation_hash['target_project_id'] = @relation_hash['project_id'] if @relation_hash['target_project_id'] - end - - def same_source_and_target? - @relation_hash['target_project_id'] && @relation_hash['target_project_id'] == @relation_hash['source_project_id'] - end - - def update_group_references - return unless existing_object? - return unless @relation_hash['group_id'] - - @relation_hash['group_id'] = @project.namespace_id - end - - # This code is a workaround for broken project exports that don't - # export merge requests with CI pipelines (i.e. exports that were - # generated from - # https://gitlab.com/gitlab-org/gitlab/merge_requests/17844). - # This method can be removed in GitLab 12.6. - def update_merge_request_references - # If a merge request was properly created, we don't need to fix - # up this export. - return if @relation_hash['merge_request'] - - merge_request_id = @relation_hash['merge_request_id'] - - return unless merge_request_id - - new_merge_request_id = @merge_requests_mapping[merge_request_id] - - return unless new_merge_request_id - - @relation_hash['merge_request_id'] = new_merge_request_id - parsed_relation_hash['merge_request_id'] = new_merge_request_id - end - - def reset_tokens! - return unless Gitlab::ImportExport.reset_tokens? && TOKEN_RESET_MODELS.include?(@relation_name) - - # If we import/export a project to the same instance, tokens will have to be reset. - # We also have to reset them to avoid issues when the gitlab secrets file cannot be copied across. - relation_class.attribute_names.select { |name| name.include?('token') }.each do |token| - @relation_hash[token] = nil - end - end - - def remove_encrypted_attributes! - return unless relation_class.respond_to?(:encrypted_attributes) && relation_class.encrypted_attributes.any? - - relation_class.encrypted_attributes.each_key do |key| - @relation_hash[key.to_s] = nil - end - end - - def relation_class - @relation_class ||= self.class.relation_class(@relation_name) - end - - def imported_object - if existing_or_new_object.respond_to?(:importing) - existing_or_new_object.importing = true - end - - existing_or_new_object - rescue ActiveRecord::RecordNotUnique - # as the operation is not atomic, retry in the unlikely scenario an INSERT is - # performed on the same object between the SELECT and the INSERT - @imported_object_retries += 1 - retry if @imported_object_retries < IMPORTED_OBJECT_MAX_RETRIES - end - - def update_note_for_missing_author(author_name) - @relation_hash['note'] = '*Blank note*' if @relation_hash['note'].blank? - @relation_hash['note'] = "#{@relation_hash['note']}#{missing_author_note(@relation_hash['updated_at'], author_name)}" - end - - def admin_user? - @user.admin? - end - - def parsed_relation_hash - @parsed_relation_hash ||= Gitlab::ImportExport::AttributeCleaner.clean(relation_hash: @relation_hash, - relation_class: relation_class) - end - - def setup_diff - @relation_hash['diff'] = @relation_hash.delete('utf8_diff') - end - - def setup_pipeline - @relation_hash.fetch('stages', []).each do |stage| - stage.statuses.each do |status| - status.pipeline = imported_object - end - end - end - - def existing_or_new_object - # Only find existing records to avoid mapping tables such as milestones - # Otherwise always create the record, skipping the extra SELECT clause. - @existing_or_new_object ||= begin - if existing_object? - attribute_hash = attribute_hash_for(['events']) - - existing_object.assign_attributes(attribute_hash) if attribute_hash.any? - - existing_object - else - # Because of single-type inheritance, we need to be careful to use the `type` field - # See https://gitlab.com/gitlab-org/gitlab/issues/34860#note_235321497 - inheritance_column = relation_class.try(:inheritance_column) - inheritance_attributes = parsed_relation_hash.slice(inheritance_column) - object = relation_class.new(inheritance_attributes) - object.assign_attributes(parsed_relation_hash) - object - end - end - end - - def attribute_hash_for(attributes) - attributes.inject({}) do |hash, value| - hash[value] = parsed_relation_hash.delete(value) if parsed_relation_hash[value] - hash - end - end - - def existing_object - @existing_object ||= find_or_create_object! - end - - def unknown_service? - @relation_name == :services && parsed_relation_hash['type'] && - !Object.const_defined?(parsed_relation_hash['type']) - end - - def unique_relation? - strong_memoize(:unique_relation) do - project_foreign_key.present? && - (has_unique_index_on_project_fk? || uses_project_fk_as_primary_key?) - end - end - - def has_unique_index_on_project_fk? - cache = cached_has_unique_index_on_project_fk - table_name = relation_class.table_name - return cache[table_name] if cache.has_key?(table_name) - - index_exists = - ActiveRecord::Base.connection.index_exists?( - relation_class.table_name, - project_foreign_key, - unique: true) - - cache[table_name] = index_exists - end - - # Avoid unnecessary DB requests - def cached_has_unique_index_on_project_fk - Thread.current[:cached_has_unique_index_on_project_fk] ||= {} - end - - def uses_project_fk_as_primary_key? - relation_class.primary_key == project_foreign_key - end - - # Should be `:project_id` for most of the cases, but this is more general - def project_foreign_key - relation_class.reflect_on_association(:project)&.foreign_key - end - - def find_or_create_object! - if unique_relation? - unique_relation_object = relation_class.find_or_create_by(project_id: @project.id) - unique_relation_object.assign_attributes(parsed_relation_hash) - - return unique_relation_object - end - - # Can't use IDs as validation exists calling `group` or `project` attributes - finder_hash = parsed_relation_hash.tap do |hash| - hash['group'] = @project.group if relation_class.attribute_method?('group_id') - hash['project'] = @project if relation_class.reflect_on_association(:project) - hash.delete('project_id') - end - - GroupProjectObjectBuilder.build(relation_class, finder_hash) - end - end - end -end diff --git a/lib/gitlab/import_export/relation_tree_restorer.rb b/lib/gitlab/import_export/relation_tree_restorer.rb index 4f58406360e..03c70f17025 100644 --- a/lib/gitlab/import_export/relation_tree_restorer.rb +++ b/lib/gitlab/import_export/relation_tree_restorer.rb @@ -11,12 +11,13 @@ module Gitlab attr_reader :importable attr_reader :tree_hash - def initialize(user:, shared:, importable:, tree_hash:, members_mapper:, relation_factory:, reader:) + def initialize(user:, shared:, importable:, tree_hash:, members_mapper:, object_builder:, relation_factory:, reader:) @user = user @shared = shared @importable = importable @tree_hash = tree_hash @members_mapper = members_mapper + @object_builder = object_builder @relation_factory = relation_factory @reader = reader end @@ -221,15 +222,16 @@ module Gitlab def relation_factory_params(relation_key, data_hash) base_params = { - relation_sym: relation_key.to_sym, - relation_hash: data_hash, + relation_sym: relation_key.to_sym, + relation_hash: data_hash, + importable: @importable, members_mapper: @members_mapper, - user: @user, - excluded_keys: excluded_keys_for_relation(relation_key) + object_builder: @object_builder, + user: @user, + excluded_keys: excluded_keys_for_relation(relation_key) } base_params[:merge_requests_mapping] = merge_requests_mapping if importable_class == Project - base_params[importable_class_sym] = @importable base_params end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fc51ef694be..04ce92d64ec 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9235,6 +9235,9 @@ msgstr "" msgid "Group ID: %{group_id}" msgstr "" +msgid "Group Owner must have signed in with SAML before enabling Group Managed Accounts" +msgstr "" + msgid "Group Runners" msgstr "" @@ -14360,6 +14363,9 @@ msgstr "" msgid "ProjectSettings|All discussions must be resolved" msgstr "" +msgid "ProjectSettings|Allow users to make copies of your repository to a new project" +msgstr "" + msgid "ProjectSettings|Allow users to request access" msgstr "" @@ -14420,6 +14426,9 @@ msgstr "" msgid "ProjectSettings|Fast-forward merges only" msgstr "" +msgid "ProjectSettings|Forks" +msgstr "" + msgid "ProjectSettings|Git Large File Storage" msgstr "" diff --git a/qa/qa/runtime/browser.rb b/qa/qa/runtime/browser.rb index 69ba90702be..340f6dc0356 100644 --- a/qa/qa/runtime/browser.rb +++ b/qa/qa/runtime/browser.rb @@ -15,6 +15,10 @@ module QA CAPYBARA_MAX_WAIT_TIME = 10 + class << self + attr_accessor :rspec_configured, :capybara_configured + end + def initialize self.class.configure! end @@ -45,11 +49,40 @@ module QA end def self.configure! + configure_rspec! + configure_capybara! + end + + def self.configure_rspec! + # We don't want to enter this infinite loop: + # Runtime::Release.perform_before_hooks -> `QA::Runtime::Browser.visit` -> configure! -> configure_rspec! -> Runtime::Release.perform_before_hooks + # So we make sure this method is called only once. + return if self.rspec_configured + + browser = self + RSpec.configure do |config| config.define_derived_metadata(file_path: %r{/qa/specs/features/}) do |metadata| metadata[:type] = :feature end + + config.before do + unless browser.rspec_configured + browser.rspec_configured = true + + ## + # Perform before hooks, which are different for CE and EE + # + Runtime::Release.perform_before_hooks + end + end end + end + + def self.configure_capybara! + return if self.capybara_configured + + self.capybara_configured = true Capybara.server_port = 9887 + ENV['TEST_ENV_NUMBER'].to_i diff --git a/qa/qa/runtime/logger.rb b/qa/qa/runtime/logger.rb index 7f73f1bd01b..a70c8faf7d2 100644 --- a/qa/qa/runtime/logger.rb +++ b/qa/qa/runtime/logger.rb @@ -14,11 +14,9 @@ module QA attr_writer :logger def logger - return @logger if @logger - - @logger = ::Logger.new Runtime::Env.log_destination - @logger.level = Runtime::Env.debug? ? ::Logger::DEBUG : ::Logger::ERROR - @logger + @logger ||= ::Logger.new(Runtime::Env.log_destination).tap do |logger| + logger.level = Runtime::Env.debug? ? ::Logger::DEBUG : ::Logger::ERROR + end end end end diff --git a/qa/qa/scenario/template.rb b/qa/qa/scenario/template.rb index 74d4c8f8757..97373f7a059 100644 --- a/qa/qa/scenario/template.rb +++ b/qa/qa/scenario/template.rb @@ -23,11 +23,6 @@ module QA def perform(options, *args) extract_address(:gitlab_address, options, args) - ## - # Perform before hooks, which are different for CE and EE - # - Runtime::Release.perform_before_hooks - Runtime::Feature.enable(options[:enable_feature]) if options.key?(:enable_feature) Specs::Runner.perform do |specs| diff --git a/qa/qa/scenario/test/instance.rb b/qa/qa/scenario/test/instance.rb index b4098619e4e..79dad7f4619 100644 --- a/qa/qa/scenario/test/instance.rb +++ b/qa/qa/scenario/test/instance.rb @@ -20,11 +20,6 @@ module QA def self.do_perform(address, *rspec_options) Runtime::Scenario.define(:gitlab_address, address) - ## - # Perform before hooks, which are different for CE and EE - # - Runtime::Release.perform_before_hooks - Specs::Runner.perform do |specs| specs.tty = true specs.options = rspec_options if rspec_options.any? diff --git a/qa/qa/support/page/logging.rb b/qa/qa/support/page/logging.rb index 82a679b6d15..f59795e17c3 100644 --- a/qa/qa/support/page/logging.rb +++ b/qa/qa/support/page/logging.rb @@ -16,7 +16,7 @@ module QA super end - def wait_until(max_duration: 60, sleep_interval: 0.1, reload: true) + def wait_until(max_duration: 60, sleep_interval: 0.1, reload: true, raise_on_failure: false) log("next wait uses reload: #{reload}") # Logging of wait start/end/duration is handled by QA::Support::Waiter diff --git a/qa/spec/page/base_spec.rb b/qa/spec/page/base_spec.rb index 717112b12ce..2d13889d26d 100644 --- a/qa/spec/page/base_spec.rb +++ b/qa/spec/page/base_spec.rb @@ -62,7 +62,7 @@ describe QA::Page::Base do end end - describe '#wait' do + describe '#wait_until' do subject { Class.new(described_class).new } context 'when the condition is true' do diff --git a/qa/spec/spec_helper.rb b/qa/spec/spec_helper.rb index 3a26ed89e9c..1336bea16bc 100644 --- a/qa/spec/spec_helper.rb +++ b/qa/spec/spec_helper.rb @@ -12,9 +12,9 @@ QA::Runtime::Browser.configure! QA::Runtime::Scenario.from_env(QA::Runtime::Env.runtime_scenario_attributes) if QA::Runtime::Env.runtime_scenario_attributes -%w[helpers shared_examples].each do |d| - Dir[::File.join(__dir__, d, '**', '*.rb')].each { |f| require f } -end +Dir[::File.join(__dir__, "support/helpers/*.rb")].each { |f| require f } +Dir[::File.join(__dir__, "support/shared_contexts/*.rb")].each { |f| require f } +Dir[::File.join(__dir__, "support/shared_examples/*.rb")].each { |f| require f } RSpec.configure do |config| QA::Specs::Helpers::Quarantine.configure_rspec diff --git a/qa/spec/helpers/stub_env.rb b/qa/spec/support/helpers/stub_env.rb index 8ad864dbec8..8ad864dbec8 100644 --- a/qa/spec/helpers/stub_env.rb +++ b/qa/spec/support/helpers/stub_env.rb diff --git a/qa/spec/shared_examples/scenario_shared_examples.rb b/qa/spec/support/shared_examples/scenario_shared_examples.rb index 697e6cb39c8..17469ea470c 100644 --- a/qa/spec/shared_examples/scenario_shared_examples.rb +++ b/qa/spec/support/shared_examples/scenario_shared_examples.rb @@ -31,12 +31,6 @@ shared_examples 'a QA scenario class' do expect(attributes).to have_received(:define).with(:gitlab_address, 'http://gitlab_address').at_least(:once) end - it 'performs before hooks' do - subject.perform(args) - - expect(release).to have_received(:perform_before_hooks) - end - it 'sets tags on runner' do subject.perform(args) diff --git a/spec/controllers/projects/forks_controller_spec.rb b/spec/controllers/projects/forks_controller_spec.rb index 80b5eb9a7ee..e351fb2b1f6 100644 --- a/spec/controllers/projects/forks_controller_spec.rb +++ b/spec/controllers/projects/forks_controller_spec.rb @@ -12,6 +12,21 @@ describe Projects::ForksController do group.add_owner(user) end + shared_examples 'forking disabled' do + let(:project) { create(:project, :private, :repository, :forking_disabled) } + + before do + project.add_developer(user) + sign_in(user) + end + + it 'returns with 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + describe 'GET index' do def get_forks(search: nil) get :index, @@ -138,19 +153,19 @@ describe Projects::ForksController do end describe 'GET new' do - def get_new + subject do get :new, - params: { - namespace_id: project.namespace, - project_id: project - } + params: { + namespace_id: project.namespace, + project_id: project + } end context 'when user is signed in' do it 'responds with status 200' do sign_in(user) - get_new + subject expect(response).to have_gitlab_http_status(200) end @@ -160,21 +175,26 @@ describe Projects::ForksController do it 'redirects to the sign-in page' do sign_out(user) - get_new + subject expect(response).to redirect_to(new_user_session_path) end end + + it_behaves_like 'forking disabled' end describe 'POST create' do - def post_create(params = {}) - post :create, - params: { - namespace_id: project.namespace, - project_id: project, - namespace_key: user.namespace.id - }.merge(params) + let(:params) do + { + namespace_id: project.namespace, + project_id: project, + namespace_key: user.namespace.id + } + end + + subject do + post :create, params: params end context 'when user is signed in' do @@ -183,18 +203,34 @@ describe Projects::ForksController do end it 'responds with status 302' do - post_create + subject expect(response).to have_gitlab_http_status(302) expect(response).to redirect_to(namespace_project_import_path(user.namespace, project)) end - it 'passes continue params to the redirect' do - continue_params = { to: '/-/ide/project/path', notice: 'message' } - post_create continue: continue_params + context 'continue params' do + let(:params) do + { + namespace_id: project.namespace, + project_id: project, + namespace_key: user.namespace.id, + continue: continue_params + } + end + let(:continue_params) do + { + to: '/-/ide/project/path', + notice: 'message' + } + end - expect(response).to have_gitlab_http_status(302) - expect(response).to redirect_to(namespace_project_import_path(user.namespace, project, continue: continue_params)) + it 'passes continue params to the redirect' do + subject + + expect(response).to have_gitlab_http_status(302) + expect(response).to redirect_to(namespace_project_import_path(user.namespace, project, continue: continue_params)) + end end end @@ -202,10 +238,12 @@ describe Projects::ForksController do it 'redirects to the sign-in page' do sign_out(user) - post_create + subject expect(response).to redirect_to(new_user_session_path) end end + + it_behaves_like 'forking disabled' end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 46efc0111c0..490ae9e84e7 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -25,6 +25,7 @@ FactoryBot.define do builds_access_level { ProjectFeature::ENABLED } snippets_access_level { ProjectFeature::ENABLED } issues_access_level { ProjectFeature::ENABLED } + forking_access_level { ProjectFeature::ENABLED } merge_requests_access_level { ProjectFeature::ENABLED } repository_access_level { ProjectFeature::ENABLED } pages_access_level do @@ -48,6 +49,7 @@ FactoryBot.define do builds_access_level: builds_access_level, snippets_access_level: evaluator.snippets_access_level, issues_access_level: evaluator.issues_access_level, + forking_access_level: evaluator.forking_access_level, merge_requests_access_level: merge_requests_access_level, repository_access_level: evaluator.repository_access_level } @@ -264,6 +266,9 @@ FactoryBot.define do trait(:issues_disabled) { issues_access_level { ProjectFeature::DISABLED } } trait(:issues_enabled) { issues_access_level { ProjectFeature::ENABLED } } trait(:issues_private) { issues_access_level { ProjectFeature::PRIVATE } } + trait(:forking_disabled) { forking_access_level { ProjectFeature::DISABLED } } + trait(:forking_enabled) { forking_access_level { ProjectFeature::ENABLED } } + trait(:forking_private) { forking_access_level { ProjectFeature::PRIVATE } } trait(:merge_requests_enabled) { merge_requests_access_level { ProjectFeature::ENABLED } } trait(:merge_requests_disabled) { merge_requests_access_level { ProjectFeature::DISABLED } } trait(:merge_requests_private) { merge_requests_access_level { ProjectFeature::PRIVATE } } diff --git a/spec/features/projects/features_visibility_spec.rb b/spec/features/projects/features_visibility_spec.rb index a825911b01a..9854335a7ad 100644 --- a/spec/features/projects/features_visibility_spec.rb +++ b/spec/features/projects/features_visibility_spec.rb @@ -186,7 +186,7 @@ describe 'Edit Project Settings' do click_button "Save changes" end - expect(find(".sharing-permissions")).to have_selector(".project-feature-toggle.is-disabled", count: 2) + expect(find(".sharing-permissions")).to have_selector(".project-feature-toggle.is-disabled", count: 3) end it "shows empty features project homepage" do diff --git a/spec/features/projects/fork_spec.rb b/spec/features/projects/fork_spec.rb index 0f97032eefa..bfab4387688 100644 --- a/spec/features/projects/fork_spec.rb +++ b/spec/features/projects/fork_spec.rb @@ -27,6 +27,89 @@ describe 'Project fork' do expect(page).to have_css('a.disabled', text: 'Fork') end + context 'forking enabled / disabled in project settings' do + before do + project.project_feature.update_attribute( + :forking_access_level, forking_access_level) + end + + context 'forking is enabled' do + let(:forking_access_level) { ProjectFeature::ENABLED } + + it 'enables fork button' do + visit project_path(project) + + expect(page).to have_css('a', text: 'Fork') + expect(page).not_to have_css('a.disabled', text: 'Fork') + end + + it 'renders new project fork page' do + visit new_project_fork_path(project) + + expect(page.status_code).to eq(200) + expect(page).to have_text(' Select a namespace to fork the project ') + end + end + + context 'forking is disabled' do + let(:forking_access_level) { ProjectFeature::DISABLED } + + it 'does not render fork button' do + visit project_path(project) + + expect(page).not_to have_css('a', text: 'Fork') + end + + it 'does not render new project fork page' do + visit new_project_fork_path(project) + + expect(page.status_code).to eq(404) + end + end + + context 'forking is private' do + let(:forking_access_level) { ProjectFeature::PRIVATE } + + before do + project.update(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end + + context 'user is not a team member' do + it 'does not render fork button' do + visit project_path(project) + + expect(page).not_to have_css('a', text: 'Fork') + end + + it 'does not render new project fork page' do + visit new_project_fork_path(project) + + expect(page.status_code).to eq(404) + end + end + + context 'user is a team member' do + before do + project.add_developer(user) + end + + it 'enables fork button' do + visit project_path(project) + + expect(page).to have_css('a', text: 'Fork') + expect(page).not_to have_css('a.disabled', text: 'Fork') + end + + it 'renders new project fork page' do + visit new_project_fork_path(project) + + expect(page.status_code).to eq(200) + expect(page).to have_text(' Select a namespace to fork the project ') + end + end + end + end + it 'forks the project', :sidekiq_might_not_need_inline do visit project_path(project) diff --git a/spec/features/projects/settings/project_settings_spec.rb b/spec/features/projects/settings/project_settings_spec.rb index 7afddc0e712..b601866c96b 100644 --- a/spec/features/projects/settings/project_settings_spec.rb +++ b/spec/features/projects/settings/project_settings_spec.rb @@ -34,6 +34,26 @@ describe 'Projects settings' do expect_toggle_state(:expanded) end + context 'forking enabled', :js do + it 'toggles forking enabled / disabled' do + visit edit_project_path(project) + + forking_enabled_input = find('input[name="project[project_feature_attributes][forking_access_level]"]', visible: :hidden) + forking_enabled_button = find('input[name="project[project_feature_attributes][forking_access_level]"] + label > button') + + expect(forking_enabled_input.value).to eq('20') + + # disable by clicking toggle + forking_enabled_button.click + page.within('.sharing-permissions') do + find('input[value="Save changes"]').click + end + wait_for_requests + + expect(forking_enabled_input.value).to eq('0') + end + end + def expect_toggle_state(state) is_collapsed = state == :collapsed diff --git a/spec/frontend/sidebar/assignees_spec.js b/spec/frontend/sidebar/assignees_spec.js index fcd08dc2dc7..0cb182b2df4 100644 --- a/spec/frontend/sidebar/assignees_spec.js +++ b/spec/frontend/sidebar/assignees_spec.js @@ -15,7 +15,6 @@ describe('Assignee component', () => { const createWrapper = (propsData = getDefaultProps()) => { wrapper = mount(Assignee, { propsData, - attachToDocument: true, }); }; diff --git a/spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js b/spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js index 8d57fd94eb5..03d1ac3ab8d 100644 --- a/spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js +++ b/spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js @@ -23,7 +23,6 @@ describe('AssigneeAvatarLink component', () => { }; wrapper = shallowMount(AssigneeAvatarLink, { - attachToDocument: true, propsData, }); } diff --git a/spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js b/spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js index a303e5f493e..a1e19c1dd8e 100644 --- a/spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js +++ b/spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js @@ -16,7 +16,6 @@ describe('CollapsedAssigneeList component', () => { }; wrapper = shallowMount(CollapsedAssigneeList, { - attachToDocument: true, propsData, }); } diff --git a/spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js b/spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js index 6d8e62404e0..1cf0af48bef 100644 --- a/spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js +++ b/spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js @@ -18,7 +18,6 @@ describe('UncollapsedAssigneeList component', () => { }; wrapper = mount(UncollapsedAssigneeList, { - attachToDocument: true, propsData, }); } diff --git a/spec/graphql/types/group_type_spec.rb b/spec/graphql/types/group_type_spec.rb index 3dd5b602aa2..de11bad0723 100644 --- a/spec/graphql/types/group_type_spec.rb +++ b/spec/graphql/types/group_type_spec.rb @@ -8,4 +8,10 @@ describe GitlabSchema.types['Group'] do it { expect(described_class.graphql_name).to eq('Group') } it { expect(described_class).to require_graphql_authorizations(:read_group) } + + it 'has the expected fields' do + expected_fields = %w[web_url avatar_url mentions_disabled parent] + + is_expected.to include_graphql_fields(*expected_fields) + end end diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 058afddd73f..11168a969fc 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -1331,9 +1331,9 @@ module Gitlab stub_feature_flags(ci_release_generation: false) end - it "returns release info" do - expect(processor.stage_builds_attributes('release').first[:options].include?(config[:release])) - .to be false + it 'raises error' do + expect { processor }.to raise_error( + 'jobs:release config release features are not enabled: release') end end end diff --git a/spec/lib/gitlab/import_export/base_object_builder_spec.rb b/spec/lib/gitlab/import_export/base_object_builder_spec.rb new file mode 100644 index 00000000000..fbb3b08cf56 --- /dev/null +++ b/spec/lib/gitlab/import_export/base_object_builder_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::ImportExport::BaseObjectBuilder do + let(:project) do + create(:project, :repository, + :builds_disabled, + :issues_disabled, + name: 'project', + path: 'project') + end + let(:klass) { Milestone } + let(:attributes) { { 'title' => 'Test BaseObjectBuilder Milestone', 'project' => project } } + + subject { described_class.build(klass, attributes) } + + describe '#build' do + context 'when object exists' do + context 'when where_clauses are implemented' do + before do + allow_next_instance_of(described_class) do |object_builder| + allow(object_builder).to receive(:where_clauses).and_return([klass.arel_table['title'].eq(attributes['title'])]) + end + end + + let!(:milestone) { create(:milestone, title: attributes['title'], project: project) } + + it 'finds existing object instead of creating one' do + expect(subject).to eq(milestone) + end + end + + context 'when where_clauses are not implemented' do + it 'raises NotImplementedError' do + expect { subject }.to raise_error(NotImplementedError) + end + end + end + + context 'when object does not exist' do + before do + allow_next_instance_of(described_class) do |object_builder| + allow(object_builder).to receive(:find_object).and_return(nil) + end + end + + it 'creates new object' do + expect { subject }.to change { Milestone.count }.from(0).to(1) + end + end + end +end diff --git a/spec/lib/gitlab/import_export/base_relation_factory_spec.rb b/spec/lib/gitlab/import_export/base_relation_factory_spec.rb new file mode 100644 index 00000000000..def3e43de9b --- /dev/null +++ b/spec/lib/gitlab/import_export/base_relation_factory_spec.rb @@ -0,0 +1,145 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::ImportExport::BaseRelationFactory do + let(:user) { create(:admin) } + let(:project) { create(:project) } + let(:members_mapper) { double('members_mapper').as_null_object } + let(:relation_sym) { :project_snippets } + let(:merge_requests_mapping) { {} } + let(:relation_hash) { {} } + let(:excluded_keys) { [] } + + subject do + described_class.create(relation_sym: relation_sym, + relation_hash: relation_hash, + object_builder: Gitlab::ImportExport::GroupProjectObjectBuilder, + members_mapper: members_mapper, + merge_requests_mapping: merge_requests_mapping, + user: user, + importable: project, + excluded_keys: excluded_keys) + end + + describe '#create' do + context 'when relation is invalid' do + before do + expect_next_instance_of(described_class) do |relation_factory| + expect(relation_factory).to receive(:invalid_relation?).and_return(true) + end + end + + it 'returns without creating new relations' do + expect(subject).to be_nil + end + end + + context 'when #setup_models is not implemented' do + it 'raises NotImplementedError' do + expect { subject }.to raise_error(NotImplementedError) + end + end + + context 'when #setup_models is implemented' do + let(:relation_sym) { :notes } + let(:relation_hash) do + { + "id" => 4947, + "note" => "merged", + "noteable_type" => "MergeRequest", + "author_id" => 999, + "created_at" => "2016-11-18T09:29:42.634Z", + "updated_at" => "2016-11-18T09:29:42.634Z", + "project_id" => 1, + "attachment" => { + "url" => nil + }, + "noteable_id" => 377, + "system" => true, + "events" => [] + } + end + + before do + expect_next_instance_of(described_class) do |relation_factory| + expect(relation_factory).to receive(:setup_models).and_return(true) + end + end + + it 'creates imported object' do + expect(subject).to be_instance_of(Note) + end + + context 'when relation contains user references' do + let(:new_user) { create(:user) } + let(:exported_member) do + { + "id" => 111, + "access_level" => 30, + "source_id" => 1, + "source_type" => "Project", + "user_id" => 3, + "notification_level" => 3, + "created_at" => "2016-11-18T09:29:42.634Z", + "updated_at" => "2016-11-18T09:29:42.634Z", + "user" => { + "id" => 999, + "email" => new_user.email, + "username" => new_user.username + } + } + end + + let(:members_mapper) do + Gitlab::ImportExport::MembersMapper.new( + exported_members: [exported_member], + user: user, + importable: project) + end + + it 'maps the right author to the imported note' do + expect(subject.author).to eq(new_user) + end + end + + context 'when relation contains token attributes' do + let(:relation_sym) { 'ProjectHook' } + let(:relation_hash) { { token: 'secret' } } + + it 'removes token attributes' do + expect(subject.token).to be_nil + end + end + + context 'when relation contains encrypted attributes' do + let(:relation_sym) { 'Ci::Variable' } + let(:relation_hash) do + create(:ci_variable).as_json + end + + it 'removes encrypted attributes' do + expect(subject.value).to be_nil + end + end + end + end + + describe '.relation_class' do + context 'when relation name is pluralized' do + let(:relation_name) { 'MergeRequest::Metrics' } + + it 'returns constantized class' do + expect(described_class.relation_class(relation_name)).to eq(MergeRequest::Metrics) + end + end + + context 'when relation name is singularized' do + let(:relation_name) { 'Badge' } + + it 'returns constantized class' do + expect(described_class.relation_class(relation_name)).to eq(Badge) + end + end + end +end diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project_relation_factory_spec.rb index 5704f823b93..0ade7ac4fc7 100644 --- a/spec/lib/gitlab/import_export/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project_relation_factory_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::ImportExport::RelationFactory do +describe Gitlab::ImportExport::ProjectRelationFactory do let(:group) { create(:group) } let(:project) { create(:project, :repository, group: group) } let(:members_mapper) { double('members_mapper').as_null_object } @@ -12,10 +12,11 @@ describe Gitlab::ImportExport::RelationFactory do let(:created_object) do described_class.create(relation_sym: relation_sym, relation_hash: relation_hash, + object_builder: Gitlab::ImportExport::GroupProjectObjectBuilder, members_mapper: members_mapper, merge_requests_mapping: merge_requests_mapping, user: user, - project: project, + importable: project, excluded_keys: excluded_keys) end @@ -97,7 +98,7 @@ describe Gitlab::ImportExport::RelationFactory do end end - context 'merge_requset object' do + context 'merge_request object' do let(:relation_sym) { :merge_requests } let(:exported_member) do @@ -244,11 +245,11 @@ describe Gitlab::ImportExport::RelationFactory do context 'Project references' do let(:relation_sym) { :project_foo_model } let(:relation_hash) do - Gitlab::ImportExport::RelationFactory::PROJECT_REFERENCES.map { |ref| { ref => 99 } }.inject(:merge) + Gitlab::ImportExport::ProjectRelationFactory::PROJECT_REFERENCES.map { |ref| { ref => 99 } }.inject(:merge) end class ProjectFooModel < FooModel - attr_accessor(*Gitlab::ImportExport::RelationFactory::PROJECT_REFERENCES) + attr_accessor(*Gitlab::ImportExport::ProjectRelationFactory::PROJECT_REFERENCES) end before do diff --git a/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb index c761f9652ab..edb2c0a131a 100644 --- a/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb @@ -27,6 +27,7 @@ describe Gitlab::ImportExport::RelationTreeRestorer do shared: shared, tree_hash: tree_hash, importable: importable, + object_builder: object_builder, members_mapper: members_mapper, relation_factory: relation_factory, reader: reader @@ -38,7 +39,8 @@ describe Gitlab::ImportExport::RelationTreeRestorer do context 'when restoring a project' do let(:path) { 'spec/fixtures/lib/gitlab/import_export/complex/project.json' } let(:importable) { create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project') } - let(:relation_factory) { Gitlab::ImportExport::RelationFactory } + let(:object_builder) { Gitlab::ImportExport::GroupProjectObjectBuilder } + let(:relation_factory) { Gitlab::ImportExport::ProjectRelationFactory } let(:reader) { Gitlab::ImportExport::Reader.new(shared: shared) } let(:tree_hash) { importable_hash } diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 01beb5ba33c..ad363233bfe 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -545,6 +545,7 @@ ProjectFeature: - id - project_id - merge_requests_access_level +- forking_access_level - issues_access_level - wiki_access_level - snippets_access_level diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 6babc39cdc3..5b402e572c3 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -64,6 +64,22 @@ describe ErrorTracking::ProjectErrorTrackingSetting do end end + describe '.extract_sentry_external_url' do + subject { described_class.extract_sentry_external_url(sentry_url) } + + describe 'when passing a URL' do + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + + it { is_expected.to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project') } + end + + describe 'when passing nil' do + let(:sentry_url) { nil } + + it { is_expected.to be_nil } + end + end + describe '#sentry_external_url' do let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index a4f68df928f..35b77832c73 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -358,6 +358,7 @@ describe API::Groups do expect(json_response['two_factor_grace_period']).to eq(group1.two_factor_grace_period) expect(json_response['auto_devops_enabled']).to eq(group1.auto_devops_enabled) expect(json_response['emails_disabled']).to eq(group1.emails_disabled) + expect(json_response['mentions_disabled']).to eq(group1.mentions_disabled) expect(json_response['project_creation_level']).to eq('maintainer') expect(json_response['subgroup_creation_level']).to eq('maintainer') expect(json_response['web_url']).to eq(group1.web_url) @@ -556,6 +557,7 @@ describe API::Groups do expect(json_response['two_factor_grace_period']).to eq(48) expect(json_response['auto_devops_enabled']).to eq(nil) expect(json_response['emails_disabled']).to eq(nil) + expect(json_response['mentions_disabled']).to eq(nil) expect(json_response['project_creation_level']).to eq("noone") expect(json_response['subgroup_creation_level']).to eq("maintainer") expect(json_response['request_access_enabled']).to eq(true) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index e1297c035b5..fce49d0248c 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -2858,6 +2858,20 @@ describe API::Projects do expect(json_response['message']).to eq('401 Unauthorized') end end + + context 'forking disabled' do + before do + project.project_feature.update_attribute( + :forking_access_level, ProjectFeature::DISABLED) + end + + it 'denies project to be forked' do + post api("/projects/#{project.id}/fork", admin) + + expect(response).to have_gitlab_http_status(409) + expect(json_response['message']['forked_from_project_id']).to eq(['is forbidden']) + end + end end describe 'POST /projects/:id/housekeeping' do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index fc01c93b5cf..e7b904fcd60 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -224,6 +224,19 @@ describe Projects::ForkService do end end end + + context 'when forking is disabled' do + before do + @from_project.project_feature.update_attribute( + :forking_access_level, ProjectFeature::DISABLED) + end + + it 'fails' do + to_project = fork_project(@from_project, @to_user, namespace: @to_user.namespace) + + expect(to_project.errors[:forked_from_project_id]).to eq(['is forbidden']) + end + end end describe 'fork to namespace' do diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index fa7b1003b8d..4ba22af85f0 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -76,28 +76,14 @@ describe SystemNoteService do end describe '.change_due_date' do - subject { described_class.change_due_date(noteable, project, author, due_date) } + let(:due_date) { double } - let(:due_date) { Date.today } - - it_behaves_like 'a note with overridable created_at' - - it_behaves_like 'a system note' do - let(:action) { 'due_date' } - end - - context 'when due date added' do - it 'sets the note text' do - expect(subject.note).to eq "changed due date to #{Date.today.to_s(:long)}" + it 'calls TimeTrackingService' do + expect_next_instance_of(::SystemNotes::TimeTrackingService) do |service| + expect(service).to receive(:change_due_date).with(due_date) end - end - context 'when due date removed' do - let(:due_date) { nil } - - it 'sets the note text' do - expect(subject.note).to eq 'removed due date' - end + described_class.change_due_date(noteable, project, author, due_date) end end @@ -488,36 +474,12 @@ describe SystemNoteService do end describe '.change_time_estimate' do - subject { described_class.change_time_estimate(noteable, project, author) } - - it_behaves_like 'a system note' do - let(:action) { 'time_tracking' } - end - - context 'with a time estimate' do - it 'sets the note text' do - noteable.update_attribute(:time_estimate, 277200) - - expect(subject.note).to eq "changed time estimate to 1w 4d 5h" - end - - context 'when time_tracking_limit_to_hours setting is true' do - before do - stub_application_setting(time_tracking_limit_to_hours: true) - end - - it 'sets the note text' do - noteable.update_attribute(:time_estimate, 277200) - - expect(subject.note).to eq "changed time estimate to 77h" - end + it 'calls TimeTrackingService' do + expect_next_instance_of(::SystemNotes::TimeTrackingService) do |service| + expect(service).to receive(:change_time_estimate) end - end - context 'without a time estimate' do - it 'sets the note text' do - expect(subject.note).to eq "removed time estimate" - end + described_class.change_time_estimate(noteable, project, author) end end @@ -548,61 +510,12 @@ describe SystemNoteService do end describe '.change_time_spent' do - # We need a custom noteable in order to the shared examples to be green. - let(:noteable) do - mr = create(:merge_request, source_project: project) - mr.spend_time(duration: 360000, user_id: author.id) - mr.save! - mr - end - - subject do - described_class.change_time_spent(noteable, project, author) - end - - it_behaves_like 'a system note' do - let(:action) { 'time_tracking' } - end - - context 'when time was added' do - it 'sets the note text' do - spend_time!(277200) - - expect(subject.note).to eq "added 1w 4d 5h of time spent" + it 'calls TimeTrackingService' do + expect_next_instance_of(::SystemNotes::TimeTrackingService) do |service| + expect(service).to receive(:change_time_spent) end - end - - context 'when time was subtracted' do - it 'sets the note text' do - spend_time!(-277200) - - expect(subject.note).to eq "subtracted 1w 4d 5h of time spent" - end - end - - context 'when time was removed' do - it 'sets the note text' do - spend_time!(:reset) - expect(subject.note).to eq "removed time spent" - end - end - - context 'when time_tracking_limit_to_hours setting is true' do - before do - stub_application_setting(time_tracking_limit_to_hours: true) - end - - it 'sets the note text' do - spend_time!(277200) - - expect(subject.note).to eq "added 77h of time spent" - end - end - - def spend_time!(seconds) - noteable.spend_time(duration: seconds, user_id: author.id) - noteable.save! + described_class.change_time_spent(noteable, project, author) end end diff --git a/spec/services/system_notes/time_tracking_service_spec.rb b/spec/services/system_notes/time_tracking_service_spec.rb new file mode 100644 index 00000000000..7e3e6a75cdf --- /dev/null +++ b/spec/services/system_notes/time_tracking_service_spec.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ::SystemNotes::TimeTrackingService do + let_it_be(:author) { create(:user) } + let_it_be(:project) { create(:project, :repository) } + + let(:noteable) { create(:issue, project: project) } + + describe '#change_due_date' do + subject { described_class.new(noteable: noteable, project: project, author: author).change_due_date(due_date) } + + let(:due_date) { Date.today } + + it_behaves_like 'a note with overridable created_at' + + it_behaves_like 'a system note' do + let(:action) { 'due_date' } + end + + context 'when due date added' do + it 'sets the note text' do + expect(subject.note).to eq "changed due date to #{due_date.to_s(:long)}" + end + end + + context 'when due date removed' do + let(:due_date) { nil } + + it 'sets the note text' do + expect(subject.note).to eq 'removed due date' + end + end + end + + describe '.change_time_estimate' do + subject { described_class.new(noteable: noteable, project: project, author: author).change_time_estimate } + + it_behaves_like 'a system note' do + let(:action) { 'time_tracking' } + end + + context 'with a time estimate' do + it 'sets the note text' do + noteable.update_attribute(:time_estimate, 277200) + + expect(subject.note).to eq "changed time estimate to 1w 4d 5h" + end + + context 'when time_tracking_limit_to_hours setting is true' do + before do + stub_application_setting(time_tracking_limit_to_hours: true) + end + + it 'sets the note text' do + noteable.update_attribute(:time_estimate, 277200) + + expect(subject.note).to eq "changed time estimate to 77h" + end + end + end + + context 'without a time estimate' do + it 'sets the note text' do + expect(subject.note).to eq "removed time estimate" + end + end + end + + describe '.change_time_spent' do + # We need a custom noteable in order to the shared examples to be green. + let(:noteable) do + mr = create(:merge_request, source_project: project) + mr.spend_time(duration: 360000, user_id: author.id) + mr.save! + mr + end + + subject do + described_class.new(noteable: noteable, project: project, author: author).change_time_spent + end + + it_behaves_like 'a system note' do + let(:action) { 'time_tracking' } + end + + context 'when time was added' do + it 'sets the note text' do + spend_time!(277200) + + expect(subject.note).to eq "added 1w 4d 5h of time spent" + end + end + + context 'when time was subtracted' do + it 'sets the note text' do + spend_time!(-277200) + + expect(subject.note).to eq "subtracted 1w 4d 5h of time spent" + end + end + + context 'when time was removed' do + it 'sets the note text' do + spend_time!(:reset) + + expect(subject.note).to eq "removed time spent" + end + end + + context 'when time_tracking_limit_to_hours setting is true' do + before do + stub_application_setting(time_tracking_limit_to_hours: true) + end + + it 'sets the note text' do + spend_time!(277200) + + expect(subject.note).to eq "added 77h of time spent" + end + end + + def spend_time!(seconds) + noteable.spend_time(duration: seconds, user_id: author.id) + noteable.save! + end + end +end diff --git a/spec/support/import_export/configuration_helper.rb b/spec/support/import_export/configuration_helper.rb index 2e5a99bb8b2..27819b5201a 100644 --- a/spec/support/import_export/configuration_helper.rb +++ b/spec/support/import_export/configuration_helper.rb @@ -36,8 +36,8 @@ module ConfigurationHelper end def relation_class_for_name(relation_name) - relation_name = Gitlab::ImportExport::RelationFactory.overrides[relation_name.to_sym] || relation_name - Gitlab::ImportExport::RelationFactory.relation_class(relation_name) + relation_name = Gitlab::ImportExport::ProjectRelationFactory.overrides[relation_name.to_sym] || relation_name + Gitlab::ImportExport::ProjectRelationFactory.relation_class(relation_name) end def parsed_attributes(relation_name, attributes, config: Gitlab::ImportExport.config_file) |