diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 3b064bc51c8..9a917b9dc9e 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -cc42cf8f28dc37bf808dabaac8a055a84b83a5db +8b95897d918a4fefdc0542a72121182cd1f27f8f diff --git a/app/assets/javascripts/projects/new/components/new_project_url_select.vue b/app/assets/javascripts/projects/new/components/new_project_url_select.vue index f4a21c6057c..506f1ec5ffd 100644 --- a/app/assets/javascripts/projects/new/components/new_project_url_select.vue +++ b/app/assets/javascripts/projects/new/components/new_project_url_select.vue @@ -13,6 +13,7 @@ import { MINIMUM_SEARCH_LENGTH } from '~/graphql_shared/constants'; import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import Tracking from '~/tracking'; import { DEBOUNCE_DELAY } from '~/vue_shared/components/filtered_search_bar/constants'; +import { s__ } from '~/locale'; import searchNamespacesWhereUserCanCreateProjectsQuery from '../queries/search_namespaces_where_user_can_create_projects.query.graphql'; import eventHub from '../event_hub'; @@ -43,14 +44,7 @@ export default { debounce: DEBOUNCE_DELAY, }, }, - inject: [ - 'namespaceFullPath', - 'namespaceId', - 'rootUrl', - 'trackLabel', - 'userNamespaceFullPath', - 'userNamespaceId', - ], + inject: ['namespaceFullPath', 'namespaceId', 'rootUrl', 'trackLabel', 'userNamespaceId'], data() { return { currentUser: {}, @@ -62,10 +56,11 @@ export default { fullPath: this.namespaceFullPath, } : { - id: this.userNamespaceId, - fullPath: this.userNamespaceFullPath, + id: undefined, + fullPath: s__('ProjectsNew|Pick a group or namespace'), }, shouldSkipQuery: true, + userNamespaceId: this.userNamespaceId, }; }, computed: { @@ -92,6 +87,9 @@ export default { hasNoMatches() { return !this.hasGroupMatches && !this.hasNamespaceMatches; }, + dropdownPlaceholderClass() { + return this.selectedNamespace.id ? '' : 'gl-text-gray-500!'; + }, }, created() { eventHub.$on('select-template', this.handleSelectTemplate); @@ -130,11 +128,18 @@ export default { diff --git a/app/assets/javascripts/projects/new/index.js b/app/assets/javascripts/projects/new/index.js index 4de9b8a6f47..a72172a4f5e 100644 --- a/app/assets/javascripts/projects/new/index.js +++ b/app/assets/javascripts/projects/new/index.js @@ -58,7 +58,6 @@ export function initNewProjectUrlSelect() { namespaceId: el.dataset.namespaceId, rootUrl: el.dataset.rootUrl, trackLabel: el.dataset.trackLabel, - userNamespaceFullPath: el.dataset.userNamespaceFullPath, userNamespaceId: el.dataset.userNamespaceId, }, render: (createElement) => createElement(NewProjectUrlSelect), diff --git a/app/assets/javascripts/projects/project_new.js b/app/assets/javascripts/projects/project_new.js index f1b7e3df7d6..fbef51b43b2 100644 --- a/app/assets/javascripts/projects/project_new.js +++ b/app/assets/javascripts/projects/project_new.js @@ -14,6 +14,7 @@ import { let hasUserDefinedProjectPath = false; let hasUserDefinedProjectName = false; const invalidInputClass = 'gl-field-error-outline'; +const invalidDropdownClass = 'gl-inset-border-1-red-400!'; const cancelSource = axios.CancelToken.source(); const endpoint = `${gon.relative_url_root}/import/url/validate`; @@ -50,6 +51,25 @@ const onProjectPathChange = ($projectNameInput, $projectPathInput, hasExistingPr } }; +const selectedNamespaceId = () => document.querySelector('[name="project[selected_namespace_id]"]'); +const dropdownButton = () => document.querySelector('.js-group-namespace-dropdown > button'); +const namespaceButton = () => document.querySelector('.js-group-namespace-button'); +const namespaceError = () => document.querySelector('.js-group-namespace-error'); + +const validateGroupNamespaceDropdown = (e) => { + if (selectedNamespaceId() && !selectedNamespaceId().attributes.value) { + document.querySelector('input[data-qa-selector="project_name"]').reportValidity(); + e.preventDefault(); + dropdownButton().classList.add(invalidDropdownClass); + namespaceButton().classList.add(invalidDropdownClass); + namespaceError().classList.remove('gl-display-none'); + } else { + dropdownButton().classList.remove(invalidDropdownClass); + namespaceButton().classList.remove(invalidDropdownClass); + namespaceError().classList.add('gl-display-none'); + } +}; + const setProjectNamePathHandlers = ($projectNameInput, $projectPathInput) => { const specialRepo = document.querySelector('.js-user-readme-repo'); @@ -70,6 +90,10 @@ const setProjectNamePathHandlers = ($projectNameInput, $projectPathInput) => { $projectPathInput.val() !== $projectPathInput.data('username'), ); }); + + document.querySelector('.js-create-project-button').addEventListener('click', (e) => { + validateGroupNamespaceDropdown(e); + }); }; const deriveProjectPathFromUrl = ($projectImportUrl) => { diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 6efd34efa37..25fdb5e6f8f 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -68,6 +68,13 @@ class ProjectsController < Projects::ApplicationController @namespace = Namespace.find_by(id: params[:namespace_id]) if params[:namespace_id] return access_denied! if @namespace && !can?(current_user, :create_projects, @namespace) + @current_user_group = + if current_user.manageable_groups(include_groups_with_developer_maintainer_access: true).count == 1 + current_user.manageable_groups(include_groups_with_developer_maintainer_access: true).first + else + nil + end + @project = Project.new(namespace_id: @namespace&.id) end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 803b9781ac4..c3ea0e14447 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -222,7 +222,7 @@ class WikiPage update_attributes(attrs) - if title.present? && title_changed? && wiki.find_page(title).present? + if title.present? && title_changed? && wiki.find_page(title, load_content: false).present? attributes[:title] = page.title raise PageRenameError, s_('WikiEdit|There is already a page with the same title in that path.') end diff --git a/app/views/projects/_new_project_fields.html.haml b/app/views/projects/_new_project_fields.html.haml index d30a7cc3172..83a00ce4ab7 100644 --- a/app/views/projects/_new_project_fields.html.haml +++ b/app/views/projects/_new_project_fields.html.haml @@ -9,27 +9,29 @@ = f.label :name, class: 'label-bold' do %span= _("Project name") = f.text_field :name, placeholder: "My awesome project", class: "form-control gl-form-input input-lg", data: { qa_selector: 'project_name', track_label: "#{track_label}", track_action: "activate_form_input", track_property: "project_name", track_value: "" }, required: true, aria: { required: true } - .form-group.project-path.col-sm-6 + .form-group.project-path.col-sm-6.gl-pr-0 = f.label :namespace_id, class: 'label-bold' do %span= _('Project URL') .input-group.gl-flex-nowrap - if current_user.can_select_namespace? - namespace_id = namespace_id_from(params) - .js-vue-new-project-url-select{ data: { namespace_full_path: GroupFinder.new(current_user).execute(id: namespace_id)&.full_path, - namespace_id: namespace_id, + .js-vue-new-project-url-select{ data: { namespace_full_path: GroupFinder.new(current_user).execute(id: namespace_id)&.full_path || @current_user_group&.full_path, + namespace_id: namespace_id || @current_user_group&.id, root_url: root_url, track_label: track_label, - user_namespace_full_path: current_user.namespace.full_path, user_namespace_id: current_user.namespace.id } } - else .input-group-prepend.static-namespace.flex-shrink-0.has-tooltip{ title: user_url(current_user.username) + '/' } .input-group-text.border-0 #{user_url(current_user.username)}/ = f.hidden_field :namespace_id, value: current_user.namespace_id + .gl-align-self-center.gl-pl-5 / .form-group.project-path.col-sm-6 = f.label :path, class: 'label-bold' do %span= _("Project slug") = f.text_field :path, placeholder: "my-awesome-project", class: "form-control gl-form-input", required: true, aria: { required: true }, data: { qa_selector: 'project_path', username: current_user.username } +.js-group-namespace-error.form-text.gl-text-red-500.gl-display-none + = s_('ProjectsNew|Pick a group or namespace where you want to create this project.') - if current_user.can_create_group? .form-text.text-muted - link_start_group_path = '' % { path: new_group_path } @@ -73,5 +75,5 @@ = s_('ProjectsNew|Analyze your source code for known security vulnerabilities.') = link_to _('Learn more.'), help_page_path('user/application_security/sast/index'), target: '_blank', rel: 'noopener noreferrer', data: { track_action: 'followed' } -= f.submit _('Create project'), class: "btn gl-button btn-confirm", data: { qa_selector: 'project_create_button', track_label: "#{track_label}", track_action: "click_button", track_property: "create_project", track_value: "" } += f.submit _('Create project'), class: "btn gl-button btn-confirm js-create-project-button", data: { qa_selector: 'project_create_button', track_label: "#{track_label}", track_action: "click_button", track_property: "create_project", track_value: "" } = link_to _('Cancel'), dashboard_projects_path, class: 'btn gl-button btn-default btn-cancel', data: { track_label: "#{track_label}", track_action: "click_button", track_property: "cancel", track_value: "" } diff --git a/danger/product_intelligence/Dangerfile b/danger/product_intelligence/Dangerfile index 8f782e3de65..c38d87604cc 100644 --- a/danger/product_intelligence/Dangerfile +++ b/danger/product_intelligence/Dangerfile @@ -1,22 +1,3 @@ # frozen_string_literal: true -# rubocop:disable Style/SignalException -CHANGED_FILES_MESSAGE = <<~MSG -For the following files, a review from the [Data team and Product Intelligence team](https://gitlab.com/groups/gitlab-org/growth/product-intelligence/engineers/-/group_members?with_inherited_permissions=exclude) is recommended -Please check the ~"product intelligence" [guide](https://docs.gitlab.com/ee/development/usage_ping.html). - -For MR review guidelines, see the [Service Ping review guidelines](https://docs.gitlab.com/ee/development/usage_ping/review_guidelines.html) or the [Snowplow review guidelines](https://docs.gitlab.com/ee/development/snowplow/review_guidelines.html). - -%s - -MSG - -# exit if not matching files or if no product intelligence labels -product_intelligence_paths_to_review = helper.changes_by_category[:product_intelligence] -labels_to_add = product_intelligence.missing_labels - -return if product_intelligence_paths_to_review.empty? || product_intelligence.skip_review? - -warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(product_intelligence_paths_to_review)) unless product_intelligence.has_approved_label? - -helper.labels_to_add.concat(labels_to_add) unless labels_to_add.empty? +product_intelligence.check! diff --git a/db/post_migrate/20220324081709_fix_and_backfill_project_namespaces_for_projects_with_duplicate_name.rb b/db/post_migrate/20220324081709_fix_and_backfill_project_namespaces_for_projects_with_duplicate_name.rb new file mode 100644 index 00000000000..f5927a2cc16 --- /dev/null +++ b/db/post_migrate/20220324081709_fix_and_backfill_project_namespaces_for_projects_with_duplicate_name.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class FixAndBackfillProjectNamespacesForProjectsWithDuplicateName < Gitlab::Database::Migration[1.0] + disable_ddl_transaction! + + MIGRATION = 'FixDuplicateProjectNameAndPath' + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 1000 + + class Project < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'projects' + + scope :without_project_namespace, -> { where(project_namespace_id: nil) } + end + + def up + queue_background_migration_jobs_by_range_at_intervals( + Project.without_project_namespace, MIGRATION, DELAY_INTERVAL, batch_size: BATCH_SIZE, track_jobs: true + ) + end + + def down + # no-op + end +end diff --git a/db/schema_migrations/20220324081709 b/db/schema_migrations/20220324081709 new file mode 100644 index 00000000000..f34f2b7aaaa --- /dev/null +++ b/db/schema_migrations/20220324081709 @@ -0,0 +1 @@ +6ce75a41607ed1e60ea4ecdfb36703d9f4192dc26ecaa774f2c30818027dd1d7 \ No newline at end of file diff --git a/doc/development/background_migrations.md b/doc/development/background_migrations.md index 9fffbd25518..7e246e1e7e1 100644 --- a/doc/development/background_migrations.md +++ b/doc/development/background_migrations.md @@ -1,8 +1,7 @@ --- -type: reference, dev -stage: none -group: Development -info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" +stage: Enablement +group: Database +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Background migrations diff --git a/doc/development/cached_queries.md b/doc/development/cached_queries.md index 492c8d13600..8c69981b27a 100644 --- a/doc/development/cached_queries.md +++ b/doc/development/cached_queries.md @@ -1,6 +1,6 @@ --- -stage: none -group: unassigned +stage: Enablement +group: Memory info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- diff --git a/doc/development/code_review.md b/doc/development/code_review.md index b0557a7ef55..bdfe8fefd30 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -396,6 +396,11 @@ When you are ready to have your merge request reviewed, you should [request an initial review](../user/project/merge_requests/getting_started.md#reviewer) by selecting a reviewer from your group or team. However, you can also assign it to any reviewer. The list of reviewers can be found on [Engineering projects](https://about.gitlab.com/handbook/engineering/projects/) page. +When a merge request has multiple areas for review, it is recommended you specify which area a reviewer should be reviewing, and at which stage (first or second). +This will help team members who qualify as a reviewer for multiple areas to know which area they're being requested to review. +For example, when a merge request has both `backend` and `frontend` concerns, you can mention the reviewer in this manner: +`@john_doe can you please review ~backend?` or `@jane_doe - could you please give this MR a ~frontend maintainer review?` + You can also use `workflow::ready for review` label. That means that your merge request is ready to be reviewed and any reviewer can pick it. It is recommended to use that label only if there isn't time pressure and make sure the merge request is assigned to a reviewer. When your merge request receives an approval from the first reviewer it can be passed to a maintainer. You should default to choosing a maintainer with [domain expertise](#domain-experts), and otherwise follow the Reviewer Roulette recommendation or use the label `ready for merge`. diff --git a/doc/development/database/client_side_connection_pool.md b/doc/development/database/client_side_connection_pool.md index 8316a75ac8d..60c8665df87 100644 --- a/doc/development/database/client_side_connection_pool.md +++ b/doc/development/database/client_side_connection_pool.md @@ -1,8 +1,7 @@ --- -type: dev, reference -stage: none -group: Development -info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" +stage: Enablement +group: Database +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Client-side connection-pool diff --git a/doc/development/deleting_migrations.md b/doc/development/deleting_migrations.md index 25ec1c08335..be9009f365d 100644 --- a/doc/development/deleting_migrations.md +++ b/doc/development/deleting_migrations.md @@ -1,6 +1,6 @@ --- -stage: none -group: unassigned +stage: Enablement +group: Database info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- diff --git a/doc/development/post_deployment_migrations.md b/doc/development/post_deployment_migrations.md index 6ab3620c197..799eefdb875 100644 --- a/doc/development/post_deployment_migrations.md +++ b/doc/development/post_deployment_migrations.md @@ -1,6 +1,6 @@ --- -stage: none -group: unassigned +stage: Enablement +group: Database info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- diff --git a/doc/development/query_recorder.md b/doc/development/query_recorder.md index 424c089f88e..17f2fecc1bc 100644 --- a/doc/development/query_recorder.md +++ b/doc/development/query_recorder.md @@ -1,6 +1,6 @@ --- -stage: none -group: unassigned +stage: Enablement +group: Database info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- diff --git a/doc/development/single_table_inheritance.md b/doc/development/single_table_inheritance.md index eb406b02a91..4acb9d76d79 100644 --- a/doc/development/single_table_inheritance.md +++ b/doc/development/single_table_inheritance.md @@ -1,6 +1,6 @@ --- -stage: none -group: unassigned +stage: Enablement +group: Database info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- diff --git a/doc/integration/jira/connect-app.md b/doc/integration/jira/connect-app.md index 59cdba93543..5c8f78a94b1 100644 --- a/doc/integration/jira/connect-app.md +++ b/doc/integration/jira/connect-app.md @@ -94,7 +94,7 @@ from outside the Marketplace, which allows you to install the application: 1. Place your Jira instance into [development mode](https://developer.atlassian.com/cloud/jira/platform/getting-started-with-connect/#step-2--enable-development-mode). 1. Sign in to your GitLab application as a user with administrator access. -1. Install the GitLab application from your self-managed GitLab instance, as +1. Install the GitLab application from your Jira instance, as described in the [Atlassian developer guides](https://developer.atlassian.com/cloud/jira/platform/getting-started-with-connect/#step-3--install-and-test-your-app): 1. In your Jira instance, go to **Apps > Manage Apps** and select **Upload app**: diff --git a/lib/gitlab/background_migration/fix_duplicate_project_name_and_path.rb b/lib/gitlab/background_migration/fix_duplicate_project_name_and_path.rb new file mode 100644 index 00000000000..defd9ea832b --- /dev/null +++ b/lib/gitlab/background_migration/fix_duplicate_project_name_and_path.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # Fix project name duplicates and backfill missing project namespace ids + class FixDuplicateProjectNameAndPath + SUB_BATCH_SIZE = 10 + # isolated project active record + class Project < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'projects' + + scope :without_project_namespace, -> { where(project_namespace_id: nil) } + scope :id_in, ->(ids) { where(id: ids) } + end + + def perform(start_id, end_id) + @project_ids = fetch_project_ids(start_id, end_id) + backfill_project_namespaces_service = init_backfill_service(project_ids) + backfill_project_namespaces_service.cleanup_gin_index('projects') + + project_ids.each_slice(SUB_BATCH_SIZE) do |ids| + ActiveRecord::Base.connection.execute(update_projects_name_and_path_sql(ids)) + end + + backfill_project_namespaces_service.backfill_project_namespaces + + mark_job_as_succeeded(start_id, end_id) + end + + private + + attr_accessor :project_ids + + def fetch_project_ids(start_id, end_id) + Project.without_project_namespace.where(id: start_id..end_id) + end + + def init_backfill_service(project_ids) + service = Gitlab::BackgroundMigration::ProjectNamespaces::BackfillProjectNamespaces.new + service.project_ids = project_ids + service.sub_batch_size = SUB_BATCH_SIZE + + service + end + + def update_projects_name_and_path_sql(project_ids) + <<~SQL + WITH cte (project_id, path_from_route ) AS ( + #{path_from_route_sql(project_ids).to_sql} + ) + UPDATE + projects + SET + name = concat(projects.name, '-', id), + path = CASE + WHEN projects.path <> cte.path_from_route THEN path_from_route + ELSE projects.path + END + FROM + cte + WHERE + projects.id = cte.project_id; + SQL + end + + def path_from_route_sql(project_ids) + Project.without_project_namespace.id_in(project_ids) + .joins("INNER JOIN routes ON routes.source_id = projects.id AND routes.source_type = 'Project'") + .select("projects.id, SUBSTRING(routes.path FROM '[^/]+(?=/$|$)') AS path_from_route") + end + + def mark_job_as_succeeded(*arguments) + ::Gitlab::Database::BackgroundMigrationJob.mark_all_as_succeeded( + 'FixDuplicateProjectNameAndPath', + arguments + ) + end + end + end +end diff --git a/lib/gitlab/background_migration/project_namespaces/backfill_project_namespaces.rb b/lib/gitlab/background_migration/project_namespaces/backfill_project_namespaces.rb index c34cc57ce60..bd7d7d02162 100644 --- a/lib/gitlab/background_migration/project_namespaces/backfill_project_namespaces.rb +++ b/lib/gitlab/background_migration/project_namespaces/backfill_project_namespaces.rb @@ -7,6 +7,8 @@ module Gitlab # # rubocop: disable Metrics/ClassLength class BackfillProjectNamespaces + attr_accessor :project_ids, :sub_batch_size + SUB_BATCH_SIZE = 25 PROJECT_NAMESPACE_STI_NAME = 'Project' @@ -18,7 +20,7 @@ module Gitlab case migration_type when 'up' - backfill_project_namespaces(namespace_id) + backfill_project_namespaces mark_job_as_succeeded(start_id, end_id, namespace_id, 'up') when 'down' cleanup_backfilled_project_namespaces(namespace_id) @@ -28,11 +30,7 @@ module Gitlab end end - private - - attr_accessor :project_ids, :sub_batch_size - - def backfill_project_namespaces(namespace_id) + def backfill_project_namespaces project_ids.each_slice(sub_batch_size) do |project_ids| # cleanup gin indexes on namespaces table cleanup_gin_index('namespaces') @@ -64,6 +62,8 @@ module Gitlab end end + private + def cleanup_backfilled_project_namespaces(namespace_id) project_ids.each_slice(sub_batch_size) do |project_ids| # IMPORTANT: first nullify project_namespace_id in projects table to avoid removing projects when records diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9d00d5e42a0..8e99fe0818c 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -29885,6 +29885,12 @@ msgstr "" msgid "ProjectsNew|No import options available" msgstr "" +msgid "ProjectsNew|Pick a group or namespace" +msgstr "" + +msgid "ProjectsNew|Pick a group or namespace where you want to create this project." +msgstr "" + msgid "ProjectsNew|Project Configuration" msgstr "" diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index 18a30ee0e49..7b91815a43e 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -90,7 +90,12 @@ module QA Page::Project::New.perform(&:click_blank_project_link) Page::Project::New.perform do |new_page| - new_page.choose_test_namespace unless @personal_namespace + if @personal_namespace + new_page.choose_namespace(@personal_namespace) + else + new_page.choose_test_namespace + end + new_page.choose_name(@name) new_page.add_description(@description) new_page.set_visibility(@visibility) diff --git a/scripts/gitlab_workhorse_component_helpers.sh b/scripts/gitlab_workhorse_component_helpers.sh index 06fe7b2ea51..ebd43a125b9 100644 --- a/scripts/gitlab_workhorse_component_helpers.sh +++ b/scripts/gitlab_workhorse_component_helpers.sh @@ -30,6 +30,7 @@ function create_gitlab_workhorse_package() { function extract_gitlab_workhorse_package() { local tar_working_folder="${TMP_TEST_FOLDER}" + mkdir -p "${tar_working_folder}" echoinfo "Extracting archive to ${tar_working_folder}" diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb index c57e39b6508..0046dfe436f 100644 --- a/spec/features/projects/new_project_spec.rb +++ b/spec/features/projects/new_project_spec.rb @@ -191,7 +191,8 @@ RSpec.describe 'New project', :js do click_link 'Create blank project' end - it 'selects the user namespace' do + it 'does not select the user namespace' do + click_on 'Pick a group or namespace' expect(page).to have_button user.username end end @@ -328,6 +329,14 @@ RSpec.describe 'New project', :js do click_on 'Create project' + expect(page).to have_content( + s_('ProjectsNew|Pick a group or namespace where you want to create this project.') + ) + + click_on 'Pick a group or namespace' + click_on user.username + click_on 'Create project' + expect(page).to have_css('#import-project-pane.active') expect(page).not_to have_css('.toggle-import-form.hide') end diff --git a/spec/features/projects/user_creates_project_spec.rb b/spec/features/projects/user_creates_project_spec.rb index 84977b6c962..b07f2d12660 100644 --- a/spec/features/projects/user_creates_project_spec.rb +++ b/spec/features/projects/user_creates_project_spec.rb @@ -70,7 +70,7 @@ RSpec.describe 'User creates a project', :js do fill_in :project_name, with: 'A Subgroup Project' fill_in :project_path, with: 'a-subgroup-project' - click_button user.username + click_on 'Pick a group or namespace' click_button subgroup.full_path click_button('Create project') @@ -97,9 +97,6 @@ RSpec.describe 'User creates a project', :js do fill_in :project_name, with: 'a-new-project' fill_in :project_path, with: 'a-new-project' - click_button user.username - click_button group.full_path - page.within('#content-body') do click_button('Create project') end diff --git a/spec/frontend/projects/new/components/new_project_url_select_spec.js b/spec/frontend/projects/new/components/new_project_url_select_spec.js index 921f5b74278..ba22622e1f7 100644 --- a/spec/frontend/projects/new/components/new_project_url_select_spec.js +++ b/spec/frontend/projects/new/components/new_project_url_select_spec.js @@ -15,6 +15,7 @@ import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import eventHub from '~/projects/new/event_hub'; import NewProjectUrlSelect from '~/projects/new/components/new_project_url_select.vue'; import searchQuery from '~/projects/new/queries/search_namespaces_where_user_can_create_projects.query.graphql'; +import { s__ } from '~/locale'; describe('NewProjectUrlSelect component', () => { let wrapper; @@ -61,7 +62,6 @@ describe('NewProjectUrlSelect component', () => { namespaceId: '28', rootUrl: 'https://gitlab.com/', trackLabel: 'blank_project', - userNamespaceFullPath: 'root', userNamespaceId: '1', }; @@ -91,7 +91,10 @@ describe('NewProjectUrlSelect component', () => { const findButtonLabel = () => wrapper.findComponent(GlButton); const findDropdown = () => wrapper.findComponent(GlDropdown); const findInput = () => wrapper.findComponent(GlSearchBoxByType); - const findHiddenInput = () => wrapper.find('[name="project[namespace_id]"]'); + const findHiddenNamespaceInput = () => wrapper.find('[name="project[namespace_id]"]'); + + const findHiddenSelectedNamespaceInput = () => + wrapper.find('[name="project[selected_namespace_id]"]'); const clickDropdownItem = async () => { wrapper.findComponent(GlDropdownItem).vm.$emit('click'); @@ -122,11 +125,20 @@ describe('NewProjectUrlSelect component', () => { }); it('renders a dropdown with the given namespace full path as the text', () => { - expect(findDropdown().props('text')).toBe(defaultProvide.namespaceFullPath); + const dropdownProps = findDropdown().props(); + + expect(dropdownProps.text).toBe(defaultProvide.namespaceFullPath); + expect(dropdownProps.toggleClass).not.toContain('gl-text-gray-500!'); }); - it('renders a dropdown with the given namespace id in the hidden input', () => { - expect(findHiddenInput().attributes('value')).toBe(defaultProvide.namespaceId); + it('renders a hidden input with the given namespace id', () => { + expect(findHiddenNamespaceInput().attributes('value')).toBe(defaultProvide.namespaceId); + }); + + it('renders a hidden input with the selected namespace id', () => { + expect(findHiddenSelectedNamespaceInput().attributes('value')).toBe( + defaultProvide.namespaceId, + ); }); }); @@ -142,11 +154,18 @@ describe('NewProjectUrlSelect component', () => { }); it("renders a dropdown with the user's namespace full path as the text", () => { - expect(findDropdown().props('text')).toBe(defaultProvide.userNamespaceFullPath); + const dropdownProps = findDropdown().props(); + + expect(dropdownProps.text).toBe(s__('ProjectsNew|Pick a group or namespace')); + expect(dropdownProps.toggleClass).toContain('gl-text-gray-500!'); }); - it("renders a dropdown with the user's namespace id in the hidden input", () => { - expect(findHiddenInput().attributes('value')).toBe(defaultProvide.userNamespaceId); + it("renders a hidden input with the user's namespace id", () => { + expect(findHiddenNamespaceInput().attributes('value')).toBe(defaultProvide.userNamespaceId); + }); + + it('renders a hidden input with the selected namespace id', () => { + expect(findHiddenSelectedNamespaceInput().attributes('value')).toBe(undefined); }); }); @@ -270,7 +289,7 @@ describe('NewProjectUrlSelect component', () => { await clickDropdownItem(); - expect(findHiddenInput().attributes('value')).toBe( + expect(findHiddenNamespaceInput().attributes('value')).toBe( getIdFromGraphQLId(data.currentUser.groups.nodes[0].id).toString(), ); }); diff --git a/spec/lib/gitlab/background_migration/fix_duplicate_project_name_and_path_spec.rb b/spec/lib/gitlab/background_migration/fix_duplicate_project_name_and_path_spec.rb new file mode 100644 index 00000000000..00d47d8ecf8 --- /dev/null +++ b/spec/lib/gitlab/background_migration/fix_duplicate_project_name_and_path_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::FixDuplicateProjectNameAndPath, :migration do + let(:migration) { described_class.new } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:routes) { table(:routes) } + + let(:namespace1) { namespaces.create!(name: 'batchtest1', type: 'Group', path: 'batch-test1') } + let(:namespace2) { namespaces.create!(name: 'batchtest2', type: 'Group', parent_id: namespace1.id, path: 'batch-test2') } + let(:namespace3) { namespaces.create!(name: 'batchtest3', type: 'Group', parent_id: namespace2.id, path: 'batch-test3') } + + let(:project_namespace2) { namespaces.create!(name: 'project2', path: 'project2', type: 'Project', parent_id: namespace2.id, visibility_level: 20) } + let(:project_namespace3) { namespaces.create!(name: 'project3', path: 'project3', type: 'Project', parent_id: namespace3.id, visibility_level: 20) } + + let(:project1) { projects.create!(name: 'project1', path: 'project1', namespace_id: namespace1.id, visibility_level: 20) } + let(:project2) { projects.create!(name: 'project2', path: 'project2', namespace_id: namespace2.id, project_namespace_id: project_namespace2.id, visibility_level: 20) } + let(:project2_dup) { projects.create!(name: 'project2', path: 'project2', namespace_id: namespace2.id, visibility_level: 20) } + let(:project3) { projects.create!(name: 'project3', path: 'project3', namespace_id: namespace3.id, project_namespace_id: project_namespace3.id, visibility_level: 20) } + let(:project3_dup) { projects.create!(name: 'project3', path: 'project3', namespace_id: namespace3.id, visibility_level: 20) } + + let!(:namespace_route1) { routes.create!(path: 'batch-test1', source_id: namespace1.id, source_type: 'Namespace') } + let!(:namespace_route2) { routes.create!(path: 'batch-test1/batch-test2', source_id: namespace2.id, source_type: 'Namespace') } + let!(:namespace_route3) { routes.create!(path: 'batch-test1/batch-test3', source_id: namespace3.id, source_type: 'Namespace') } + + let!(:proj_route1) { routes.create!(path: 'batch-test1/project1', source_id: project1.id, source_type: 'Project') } + let!(:proj_route2) { routes.create!(path: 'batch-test1/batch-test2/project2', source_id: project2.id, source_type: 'Project') } + let!(:proj_route2_dup) { routes.create!(path: "batch-test1/batch-test2/project2-route-#{project2_dup.id}", source_id: project2_dup.id, source_type: 'Project') } + let!(:proj_route3) { routes.create!(path: 'batch-test1/batch-test3/project3', source_id: project3.id, source_type: 'Project') } + let!(:proj_route3_dup) { routes.create!(path: "batch-test1/batch-test3/project3-route-#{project3_dup.id}", source_id: project3_dup.id, source_type: 'Project') } + + subject(:perform_migration) { migration.perform(projects.minimum(:id), projects.maximum(:id)) } + + describe '#up' do + it 'backfills namespace_id for the selected records', :aggregate_failures do + expect(namespaces.where(type: 'Project').count).to eq(2) + + perform_migration + + expect(namespaces.where(type: 'Project').count).to eq(5) + + expect(project1.reload.name).to eq("project1-#{project1.id}") + expect(project1.path).to eq('project1') + + expect(project2.reload.name).to eq('project2') + expect(project2.path).to eq('project2') + + expect(project2_dup.reload.name).to eq("project2-#{project2_dup.id}") + expect(project2_dup.path).to eq("project2-route-#{project2_dup.id}") + + expect(project3.reload.name).to eq("project3") + expect(project3.path).to eq("project3") + + expect(project3_dup.reload.name).to eq("project3-#{project3_dup.id}") + expect(project3_dup.path).to eq("project3-route-#{project3_dup.id}") + + projects.all.each do |pr| + project_namespace = namespaces.find(pr.project_namespace_id) + expect(project_namespace).to be_in_sync_with_project(pr) + end + end + end +end diff --git a/spec/migrations/fix_and_backfill_project_namespaces_for_projects_with_duplicate_name_spec.rb b/spec/migrations/fix_and_backfill_project_namespaces_for_projects_with_duplicate_name_spec.rb new file mode 100644 index 00000000000..44a2220b2ad --- /dev/null +++ b/spec/migrations/fix_and_backfill_project_namespaces_for_projects_with_duplicate_name_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe FixAndBackfillProjectNamespacesForProjectsWithDuplicateName, :migration do + let(:projects) { table(:projects) } + let(:namespaces) { table(:namespaces) } + + let!(:group) { namespaces.create!(name: 'group1', path: 'group1', type: 'Group') } + let!(:project_namespace) { namespaces.create!(name: 'project2', path: 'project2', type: 'Project') } + let!(:project1) { projects.create!(name: 'project1', path: 'project1', project_namespace_id: nil, namespace_id: group.id, visibility_level: 20) } + let!(:project2) { projects.create!(name: 'project2', path: 'project2', project_namespace_id: project_namespace.id, namespace_id: group.id, visibility_level: 20) } + let!(:project3) { projects.create!(name: 'project3', path: 'project3', project_namespace_id: nil, namespace_id: group.id, visibility_level: 20) } + let!(:project4) { projects.create!(name: 'project4', path: 'project4', project_namespace_id: nil, namespace_id: group.id, visibility_level: 20) } + + describe '#up' do + it 'schedules background migrations' do + Sidekiq::Testing.fake! do + freeze_time do + described_class.new.up + + migration = described_class::MIGRATION + + expect(migration).to be_scheduled_delayed_migration(2.minutes, project1.id, project4.id) + expect(BackgroundMigrationWorker.jobs.size).to eq 1 + end + end + end + + context 'in batches' do + before do + stub_const('FixAndBackfillProjectNamespacesForProjectsWithDuplicateName::BATCH_SIZE', 2) + end + + it 'schedules background migrations' do + Sidekiq::Testing.fake! do + freeze_time do + described_class.new.up + + migration = described_class::MIGRATION + + expect(migration).to be_scheduled_delayed_migration(2.minutes, project1.id, project3.id) + expect(migration).to be_scheduled_delayed_migration(4.minutes, project4.id, project4.id) + expect(BackgroundMigrationWorker.jobs.size).to eq 2 + end + end + end + end + end +end diff --git a/spec/services/ci/after_requeue_job_service_spec.rb b/spec/services/ci/after_requeue_job_service_spec.rb index 48573acb897..c9e01437d16 100644 --- a/spec/services/ci/after_requeue_job_service_spec.rb +++ b/spec/services/ci/after_requeue_job_service_spec.rb @@ -202,7 +202,7 @@ RSpec.describe Ci::AfterRequeueJobService, :sidekiq_inline do stub_feature_flags(ci_fix_order_of_subsequent_jobs: false) end - it 'does not mark b1 as processable' do + it 'does not mark b1 as processable', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/356571' do execute_after_requeue_service(a1) check_jobs_statuses( diff --git a/spec/support/matchers/project_namespace_matcher.rb b/spec/support/matchers/project_namespace_matcher.rb index 95aa5429679..8666a605276 100644 --- a/spec/support/matchers/project_namespace_matcher.rb +++ b/spec/support/matchers/project_namespace_matcher.rb @@ -10,7 +10,7 @@ RSpec::Matchers.define :be_in_sync_with_project do |project| project_namespace.present? && project.name == project_namespace.name && project.path == project_namespace.path && - project.namespace == project_namespace.parent && + project.namespace_id == project_namespace.parent_id && project.visibility_level == project_namespace.visibility_level && project.shared_runners_enabled == project_namespace.shared_runners_enabled end diff --git a/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb b/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb index 202606c6aa6..4751d91efde 100644 --- a/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb +++ b/spec/support/shared_examples/workers/concerns/git_garbage_collect_methods_shared_examples.rb @@ -230,76 +230,6 @@ RSpec.shared_examples 'can collect git garbage' do |update_statistics: true| stub_feature_flags(optimized_housekeeping: false) end - it 'incremental repack adds a new packfile' do - create_objects(resource) - before_packs = packs(resource) - - expect(before_packs.count).to be >= 1 - - subject.perform(resource.id, 'incremental_repack', lease_key, lease_uuid) - after_packs = packs(resource) - - # Exactly one new pack should have been created - expect(after_packs.count).to eq(before_packs.count + 1) - - # Previously existing packs are still around - expect(before_packs & after_packs).to eq(before_packs) - end - - it 'full repack consolidates into 1 packfile' do - create_objects(resource) - subject.perform(resource.id, 'incremental_repack', lease_key, lease_uuid) - before_packs = packs(resource) - - expect(before_packs.count).to be >= 2 - - subject.perform(resource.id, 'full_repack', lease_key, lease_uuid) - after_packs = packs(resource) - - expect(after_packs.count).to eq(1) - - # Previously existing packs should be gone now - expect(after_packs - before_packs).to eq(after_packs) - - expect(File.exist?(bitmap_path(after_packs.first))).to eq(bitmaps_enabled) - end - - it 'gc consolidates into 1 packfile and updates packed-refs' do - create_objects(resource) - before_packs = packs(resource) - before_packed_refs = packed_refs(resource) - - expect(before_packs.count).to be >= 1 - - # It's quite difficult to use `expect_next_instance_of` in this place - # because the RepositoryService is instantiated several times to do - # some repository calls like `exists?`, `create_repository`, ... . - # Therefore, since we're instantiating the object several times, - # RSpec has troubles figuring out which instance is the next and which - # one we want to mock. - # Besides, at this point, we actually want to perform the call to Gitaly, - # otherwise we would just use `instance_double` like in other parts of the - # spec file. - expect_any_instance_of(Gitlab::GitalyClient::RepositoryService) # rubocop:disable RSpec/AnyInstanceOf - .to receive(:garbage_collect) - .with(bitmaps_enabled, prune: false) - .and_call_original - - subject.perform(resource.id, 'gc', lease_key, lease_uuid) - after_packed_refs = packed_refs(resource) - after_packs = packs(resource) - - expect(after_packs.count).to eq(1) - - # Previously existing packs should be gone now - expect(after_packs - before_packs).to eq(after_packs) - - # The packed-refs file should have been updated during 'git gc' - expect(before_packed_refs).not_to eq(after_packed_refs) - - expect(File.exist?(bitmap_path(after_packs.first))).to eq(bitmaps_enabled) - end - it 'cleans up repository after finishing' do expect(resource).to receive(:cleanup).and_call_original diff --git a/spec/tooling/danger/product_intelligence_spec.rb b/spec/tooling/danger/product_intelligence_spec.rb index d0d4b8d4df4..ea08e3bc6db 100644 --- a/spec/tooling/danger/product_intelligence_spec.rb +++ b/spec/tooling/danger/product_intelligence_spec.rb @@ -20,62 +20,56 @@ RSpec.describe Tooling::Danger::ProductIntelligence do allow(fake_helper).to receive(:changed_lines).and_return(changed_lines) end - describe '#missing_labels' do - subject { product_intelligence.missing_labels } + describe '#check!' do + subject { product_intelligence.check! } + let(:markdown_formatted_list) { 'markdown formatted list' } + let(:review_pending_label) { 'product intelligence::review pending' } + let(:approved_label) { 'product intelligence::approved' } let(:ci_env) { true } + let(:previous_label_to_add) { 'label_to_add' } + let(:labels_to_add) { [previous_label_to_add] } + let(:has_product_intelligence_label) { true } before do - allow(fake_helper).to receive(:mr_has_labels?).and_return(false) + allow(fake_helper).to receive(:changes_by_category).and_return(product_intelligence: changed_files, database: ['other_files.yml']) allow(fake_helper).to receive(:ci?).and_return(ci_env) + allow(fake_helper).to receive(:mr_has_labels?).with('product intelligence').and_return(has_product_intelligence_label) + allow(fake_helper).to receive(:markdown_list).with(changed_files).and_return(markdown_formatted_list) + allow(fake_helper).to receive(:labels_to_add).and_return(labels_to_add) end - context 'with ci? false' do - let(:ci_env) { false } + shared_examples "doesn't add new labels" do + it "doesn't add new labels" do + subject - it { is_expected.to be_empty } - end - - context 'with ci? true' do - let(:expected_labels) { ['product intelligence', 'product intelligence::review pending'] } - - it { is_expected.to match_array(expected_labels) } - end - - context 'with product intelligence label' do - let(:expected_labels) { ['product intelligence::review pending'] } - let(:mr_labels) { [] } - - before do - allow(fake_helper).to receive(:mr_has_labels?).with('product intelligence').and_return(true) - allow(fake_helper).to receive(:mr_labels).and_return(mr_labels) - end - - it { is_expected.to match_array(expected_labels) } - - context 'with product intelligence::review pending' do - let(:mr_labels) { ['product intelligence::review pending'] } - - it { is_expected.to be_empty } - end - - context 'with product intelligence::approved' do - let(:mr_labels) { ['product intelligence::approved'] } - - it { is_expected.to be_empty } + expect(labels_to_add).to match_array [previous_label_to_add] end end - end - describe '#skip_review' do - subject { product_intelligence.skip_review? } + shared_examples "doesn't add new warnings" do + it "doesn't add new warnings" do + expect(product_intelligence).not_to receive(:warn) + + subject + end + end + + shared_examples 'adds new labels' do + it 'adds new labels' do + subject + + expect(labels_to_add).to match_array [previous_label_to_add, review_pending_label] + end + end context 'with growth experiment label' do before do allow(fake_helper).to receive(:mr_has_labels?).with('growth experiment').and_return(true) end - it { is_expected.to be true } + include_examples "doesn't add new labels" + include_examples "doesn't add new warnings" end context 'without growth experiment label' do @@ -83,7 +77,48 @@ RSpec.describe Tooling::Danger::ProductIntelligence do allow(fake_helper).to receive(:mr_has_labels?).with('growth experiment').and_return(false) end - it { is_expected.to be false } + context 'with approved label' do + let(:mr_labels) { [approved_label] } + + include_examples "doesn't add new labels" + include_examples "doesn't add new warnings" + end + + context 'without approved label' do + include_examples 'adds new labels' + + it 'warns with proper message' do + expect(product_intelligence).to receive(:warn).with(%r{#{markdown_formatted_list}}) + + subject + end + end + + context 'with product intelligence::review pending label' do + let(:mr_labels) { ['product intelligence::review pending'] } + + include_examples "doesn't add new labels" + end + + context 'with product intelligence::approved label' do + let(:mr_labels) { ['product intelligence::approved'] } + + include_examples "doesn't add new labels" + end + + context 'with the product intelligence label' do + let(:has_product_intelligence_label) { true } + + context 'with ci? false' do + let(:ci_env) { false } + + include_examples "doesn't add new labels" + end + + context 'with ci? true' do + include_examples 'adds new labels' + end + end end end end diff --git a/tooling/danger/product_intelligence.rb b/tooling/danger/product_intelligence.rb index 6185b2f0d08..dcac1099687 100644 --- a/tooling/danger/product_intelligence.rb +++ b/tooling/danger/product_intelligence.rb @@ -6,12 +6,35 @@ module Tooling module ProductIntelligence APPROVED_LABEL = 'product intelligence::approved' REVIEW_LABEL = 'product intelligence::review pending' + CHANGED_FILES_MESSAGE = <<~MSG + For the following files, a review from the [Data team and Product Intelligence team](https://gitlab.com/groups/gitlab-org/growth/product-intelligence/engineers/-/group_members?with_inherited_permissions=exclude) is recommended + Please check the ~"product intelligence" [guide](https://docs.gitlab.com/ee/development/usage_ping.html). + + For MR review guidelines, see the [Service Ping review guidelines](https://docs.gitlab.com/ee/development/usage_ping/review_guidelines.html) or the [Snowplow review guidelines](https://docs.gitlab.com/ee/development/snowplow/review_guidelines.html). + + %s + + MSG WORKFLOW_LABELS = [ APPROVED_LABEL, REVIEW_LABEL ].freeze + def check! + # exit if not matching files or if no product intelligence labels + product_intelligence_paths_to_review = helper.changes_by_category[:product_intelligence] + labels_to_add = missing_labels + + return if product_intelligence_paths_to_review.empty? || skip_review? + + warn format(CHANGED_FILES_MESSAGE, changed_files: helper.markdown_list(product_intelligence_paths_to_review)) unless has_approved_label? + + helper.labels_to_add.concat(labels_to_add) unless labels_to_add.empty? + end + + private + def missing_labels return [] unless helper.ci? @@ -30,8 +53,6 @@ module Tooling helper.mr_has_labels?('growth experiment') end - private - def has_workflow_labels? (WORKFLOW_LABELS & helper.mr_labels).any? end