diff options
88 files changed, 702 insertions, 682 deletions
diff --git a/app/assets/javascripts/ci/catalog/components/list/ci_resources_list_item.vue b/app/assets/javascripts/ci/catalog/components/list/ci_resources_list_item.vue index acfe14051aa..080955b4322 100644 --- a/app/assets/javascripts/ci/catalog/components/list/ci_resources_list_item.vue +++ b/app/assets/javascripts/ci/catalog/components/list/ci_resources_list_item.vue @@ -111,7 +111,9 @@ export default { </span> </div> </div> - <div class="gl-display-flex gl-sm-flex-direction-column gl-justify-content-space-between"> + <div + class="gl-display-flex gl-flex-direction-column gl-md-flex-direction-row gl-justify-content-space-between" + > <span class="gl-display-flex gl-flex-basis-two-thirds gl-font-sm">{{ resource.description }}</span> diff --git a/app/assets/javascripts/ci/pipeline_editor/components/editor/ci_editor_header.vue b/app/assets/javascripts/ci/pipeline_editor/components/editor/ci_editor_header.vue index 8f4d566e7e6..204eaf20664 100644 --- a/app/assets/javascripts/ci/pipeline_editor/components/editor/ci_editor_header.vue +++ b/app/assets/javascripts/ci/pipeline_editor/components/editor/ci_editor_header.vue @@ -80,7 +80,7 @@ export default { <template> <div - class="gl-display-flex gl-p-3 gl-gap-3 gl-border-solid gl-border-gray-100 gl-border-1 gl-sm-flex-direction-column" + class="gl-display-flex gl-p-3 gl-gap-3 gl-border-solid gl-border-gray-100 gl-border-1 gl-flex-direction-column gl-md-flex-direction-row" > <slot></slot> <gl-button diff --git a/app/assets/javascripts/ci/pipeline_editor/pipeline_editor_home.vue b/app/assets/javascripts/ci/pipeline_editor/pipeline_editor_home.vue index 41e5199e204..09ba6292e13 100644 --- a/app/assets/javascripts/ci/pipeline_editor/pipeline_editor_home.vue +++ b/app/assets/javascripts/ci/pipeline_editor/pipeline_editor_home.vue @@ -168,7 +168,7 @@ export default { @toggle-file-tree="toggleFileTree" v-on="$listeners" /> - <div class="gl-display-flex gl-w-full gl-sm-flex-direction-column"> + <div class="gl-display-flex gl-w-full gl-flex-direction-column gl-md-flex-direction-row"> <pipeline-editor-file-tree v-if="showFileTree" class="gl-flex-shrink-0" diff --git a/app/assets/javascripts/organizations/mock_data.js b/app/assets/javascripts/organizations/mock_data.js index 6b6a9e711d3..725b6ac1ad8 100644 --- a/app/assets/javascripts/organizations/mock_data.js +++ b/app/assets/javascripts/organizations/mock_data.js @@ -281,12 +281,25 @@ export const organizationGroups = { ], }; -export const createOrganizationResponse = { - organization: { - name: 'Default', - path: '/-/organizations/default', +export const organizationCreateResponse = { + data: { + organizationCreate: { + organization: { + id: 'gid://gitlab/Organizations::Organization/1', + webUrl: 'http://127.0.0.1:3000/-/organizations/default', + }, + errors: [], + }, + }, +}; + +export const organizationCreateResponseWithErrors = { + data: { + organizationCreate: { + organization: null, + errors: ['Path is too short (minimum is 2 characters)'], + }, }, - errors: [], }; export const updateOrganizationResponse = { diff --git a/app/assets/javascripts/organizations/new/components/app.vue b/app/assets/javascripts/organizations/new/components/app.vue index 8f71fdfe68b..f7f7b79d52b 100644 --- a/app/assets/javascripts/organizations/new/components/app.vue +++ b/app/assets/javascripts/organizations/new/components/app.vue @@ -4,12 +4,13 @@ import { s__ } from '~/locale'; import { visitUrlWithAlerts } from '~/lib/utils/url_utility'; import { createAlert } from '~/alert'; import { helpPagePath } from '~/helpers/help_page_helper'; -import createOrganizationMutation from '../graphql/mutations/create_organization.mutation.graphql'; +import FormErrorsAlert from '~/vue_shared/components/form/errors_alert.vue'; +import organizationCreateMutation from '../graphql/mutations/organization_create.mutation.graphql'; import NewEditForm from '../../shared/components/new_edit_form.vue'; export default { name: 'OrganizationNewApp', - components: { NewEditForm, GlSprintf, GlLink }, + components: { NewEditForm, GlSprintf, GlLink, FormErrorsAlert }, i18n: { pageTitle: s__('Organization|New organization'), pageDescription: s__( @@ -22,6 +23,7 @@ export default { data() { return { loading: false, + errors: [], }; }, computed: { @@ -35,21 +37,22 @@ export default { try { const { data: { - createOrganization: { organization, errors }, + organizationCreate: { organization, errors }, }, } = await this.$apollo.mutate({ - mutation: createOrganizationMutation, + mutation: organizationCreateMutation, variables: { - ...formValues, + input: { name: formValues.name, path: formValues.path }, }, }); if (errors.length) { - // TODO: handle errors when using real API after https://gitlab.com/gitlab-org/gitlab/-/issues/417891 is complete. + this.errors = errors; + return; } - visitUrlWithAlerts(organization.path, [ + visitUrlWithAlerts(organization.webUrl, [ { id: 'organization-successfully-created', title: this.$options.i18n.successAlertTitle, @@ -69,6 +72,7 @@ export default { <template> <div class="gl-py-6"> + <form-errors-alert v-model="errors" /> <h1 class="gl-mt-0 gl-font-size-h-display">{{ $options.i18n.pageTitle }}</h1> <p> <gl-sprintf :message="$options.i18n.pageDescription"> diff --git a/app/assets/javascripts/organizations/new/graphql/mutations/create_organization.mutation.graphql b/app/assets/javascripts/organizations/new/graphql/mutations/create_organization.mutation.graphql deleted file mode 100644 index 766c7e96d14..00000000000 --- a/app/assets/javascripts/organizations/new/graphql/mutations/create_organization.mutation.graphql +++ /dev/null @@ -1,9 +0,0 @@ -mutation createOrganization($input: LocalCreateOrganizationInput!) { - createOrganization(input: $input) @client { - organization { - name - path - } - errors - } -} diff --git a/app/assets/javascripts/organizations/new/graphql/mutations/organization_create.mutation.graphql b/app/assets/javascripts/organizations/new/graphql/mutations/organization_create.mutation.graphql new file mode 100644 index 00000000000..81fbfddd1e4 --- /dev/null +++ b/app/assets/javascripts/organizations/new/graphql/mutations/organization_create.mutation.graphql @@ -0,0 +1,9 @@ +mutation organizationCreate($input: OrganizationCreateInput!) { + organizationCreate(input: $input) { + organization { + id + webUrl + } + errors + } +} diff --git a/app/assets/javascripts/organizations/new/graphql/typedefs.graphql b/app/assets/javascripts/organizations/new/graphql/typedefs.graphql deleted file mode 100644 index f708c4ad162..00000000000 --- a/app/assets/javascripts/organizations/new/graphql/typedefs.graphql +++ /dev/null @@ -1,5 +0,0 @@ -# TODO: Use real input type when https://gitlab.com/gitlab-org/gitlab/-/issues/417891 is complete. -input LocalCreateOrganizationInput { - name: String - path: String -} diff --git a/app/assets/javascripts/organizations/new/index.js b/app/assets/javascripts/organizations/new/index.js index a65603227f6..9c7e5344800 100644 --- a/app/assets/javascripts/organizations/new/index.js +++ b/app/assets/javascripts/organizations/new/index.js @@ -3,7 +3,6 @@ import VueApollo from 'vue-apollo'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import createDefaultClient from '~/lib/graphql'; -import resolvers from '../shared/graphql/resolvers'; import App from './components/app.vue'; export const initOrganizationsNew = () => { @@ -17,7 +16,7 @@ export const initOrganizationsNew = () => { const { organizationsPath, rootUrl } = convertObjectPropsToCamelCase(JSON.parse(appData)); const apolloProvider = new VueApollo({ - defaultClient: createDefaultClient(resolvers), + defaultClient: createDefaultClient(), }); return new Vue({ diff --git a/app/assets/javascripts/organizations/shared/components/new_edit_form.vue b/app/assets/javascripts/organizations/shared/components/new_edit_form.vue index 388dd704a70..8aaa680036f 100644 --- a/app/assets/javascripts/organizations/shared/components/new_edit_form.vue +++ b/app/assets/javascripts/organizations/shared/components/new_edit_form.vue @@ -103,7 +103,13 @@ export default { }, [FORM_FIELD_PATH]: { label: s__('Organization|Organization URL'), - validators: [formValidators.required(s__('Organization|Organization URL is required.'))], + validators: [ + formValidators.required(s__('Organization|Organization URL is required.')), + formValidators.factory( + s__('Organization|Organization URL must be a minimum of two characters.'), + (val) => val.length >= 2, + ), + ], groupAttrs: { class: 'gl-w-full', }, diff --git a/app/assets/javascripts/organizations/shared/graphql/resolvers.js b/app/assets/javascripts/organizations/shared/graphql/resolvers.js index 00c07e1b1e9..9ed1be62352 100644 --- a/app/assets/javascripts/organizations/shared/graphql/resolvers.js +++ b/app/assets/javascripts/organizations/shared/graphql/resolvers.js @@ -2,7 +2,6 @@ import { organizations, organizationProjects, organizationGroups, - createOrganizationResponse, updateOrganizationResponse, } from '../../mock_data'; @@ -35,12 +34,6 @@ export default { }, }, Mutation: { - createOrganization: async () => { - // Simulate API loading - await simulateLoading(); - - return createOrganizationResponse; - }, updateOrganization: async () => { // Simulate API loading await simulateLoading(); diff --git a/app/assets/javascripts/token_access/components/inbound_token_access.vue b/app/assets/javascripts/token_access/components/inbound_token_access.vue index 7e55f56279e..345db1752f6 100644 --- a/app/assets/javascripts/token_access/components/inbound_token_access.vue +++ b/app/assets/javascripts/token_access/components/inbound_token_access.vue @@ -46,12 +46,6 @@ export default { columnClass: 'gl-w-40p', }, { - key: 'namespace', - label: __('Namespace'), - thClass: 'gl-border-t-none!', - columnClass: 'gl-w-40p', - }, - { key: 'actions', label: '', tdClass: 'gl-text-right', diff --git a/app/assets/javascripts/token_access/components/outbound_token_access.vue b/app/assets/javascripts/token_access/components/outbound_token_access.vue index 43aa9b94b3a..846b0d1791f 100644 --- a/app/assets/javascripts/token_access/components/outbound_token_access.vue +++ b/app/assets/javascripts/token_access/components/outbound_token_access.vue @@ -54,12 +54,6 @@ export default { columnClass: 'gl-w-40p', }, { - key: 'namespace', - label: __('Namespace'), - thClass: 'gl-border-t-none!', - columnClass: 'gl-w-40p', - }, - { key: 'actions', label: '', tdClass: 'gl-text-right', diff --git a/app/assets/javascripts/token_access/components/token_projects_table.vue b/app/assets/javascripts/token_access/components/token_projects_table.vue index ee88b4ec339..4245b39dec1 100644 --- a/app/assets/javascripts/token_access/components/token_projects_table.vue +++ b/app/assets/javascripts/token_access/components/token_projects_table.vue @@ -29,9 +29,6 @@ export default { removeProject(project) { this.$emit('removeProject', project); }, - namespaceFallback(namespace) { - return namespace?.fullPath || ''; - }, }, }; </script> @@ -50,13 +47,7 @@ export default { </template> <template #cell(project)="{ item }"> - <span data-testid="token-access-project-name">{{ item.name }}</span> - </template> - - <template #cell(namespace)="{ item }"> - <span data-testid="token-access-project-namespace"> - {{ namespaceFallback(item.namespace) }} - </span> + <span data-testid="token-access-project-name">{{ item.fullPath }}</span> </template> <template #cell(actions)="{ item }"> diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue index 2a4bda51888..3c2d8efaffc 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue @@ -560,7 +560,7 @@ export default { <div class="mr-widget-body-controls gl-display-flex gl-align-items-center gl-flex-wrap"> <template v-if="shouldShowMergeControls"> <div - class="gl-display-flex gl-sm-flex-direction-column gl-md-align-items-center gl-flex-wrap gl-w-full" + class="gl-display-flex gl-flex-direction-column gl-md-flex-direction-row gl-md-align-items-center gl-flex-wrap gl-w-full" > <gl-form-checkbox v-if="canRemoveSourceBranch" diff --git a/app/assets/javascripts/vue_shared/components/form/errors_alert.stories.js b/app/assets/javascripts/vue_shared/components/form/errors_alert.stories.js new file mode 100644 index 00000000000..7c32e38a299 --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/form/errors_alert.stories.js @@ -0,0 +1,21 @@ +import ErrorsAlert from './errors_alert.vue'; + +export default { + component: ErrorsAlert, + title: 'vue_shared/form/errors_alert', +}; + +const defaultProps = { + errors: ['Name must be at least 5 characters.', 'Name cannot contain special characters.'], +}; + +const Template = (args) => ({ + components: { ErrorsAlert }, + data() { + return { errors: args.errors }; + }, + template: `<errors-alert v-model="errors" />`, +}); + +export const Default = Template.bind({}); +Default.args = defaultProps; diff --git a/app/assets/javascripts/vue_shared/components/form/errors_alert.vue b/app/assets/javascripts/vue_shared/components/form/errors_alert.vue new file mode 100644 index 00000000000..3e33168781b --- /dev/null +++ b/app/assets/javascripts/vue_shared/components/form/errors_alert.vue @@ -0,0 +1,42 @@ +<script> +import { GlAlert } from '@gitlab/ui'; +import { n__ } from '~/locale'; + +export default { + components: { GlAlert }, + model: { + prop: 'errors', + }, + props: { + errors: { + type: Array, + required: true, + }, + }, + computed: { + title() { + return n__( + 'The form contains the following error:', + 'The form contains the following errors:', + this.errors.length, + ); + }, + }, +}; +</script> + +<template> + <gl-alert + v-if="errors.length" + class="gl-mb-5" + :title="title" + variant="danger" + @dismiss="$emit('input', [])" + > + <ul class="gl-pl-5 gl-mb-0"> + <li v-for="error in errors" :key="error"> + {{ error }} + </li> + </ul> + </gl-alert> +</template> diff --git a/app/graphql/types/organizations/organization_type.rb b/app/graphql/types/organizations/organization_type.rb index cae0ef2232e..e7ba8de527c 100644 --- a/app/graphql/types/organizations/organization_type.rb +++ b/app/graphql/types/organizations/organization_type.rb @@ -33,6 +33,10 @@ module Types null: false, description: 'Path of the organization.', alpha: { milestone: '16.4' } + field :web_url, GraphQL::Types::String, + null: false, + description: 'Web URL of the organization.', + alpha: { milestone: '16.6' } end end end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index db40a908bd7..cd54ac1b24a 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -636,8 +636,7 @@ class Namespace < ApplicationRecord :route, :project_setting, :project_feature, - pages_metadatum: :pages_deployment - ) + :active_pages_deployments) end private diff --git a/app/models/organizations/organization.rb b/app/models/organizations/organization.rb index 893b08d7872..157b851e009 100644 --- a/app/models/organizations/organization.rb +++ b/app/models/organizations/organization.rb @@ -42,6 +42,10 @@ module Organizations organization_users.exists?(user: user) end + def web_url(only_path: nil) + Gitlab::UrlBuilder.build(self, only_path: only_path) + end + private def check_if_default_organization diff --git a/app/models/pages/lookup_path.rb b/app/models/pages/lookup_path.rb index 8a02415aef4..e5e23c3bb84 100644 --- a/app/models/pages/lookup_path.rb +++ b/app/models/pages/lookup_path.rb @@ -4,8 +4,6 @@ module Pages class LookupPath include Gitlab::Utils::StrongMemoize - LegacyStorageDisabledError = Class.new(::StandardError) - def initialize(project, trim_prefix: nil, domain: nil) @project = project @domain = domain @@ -15,6 +13,7 @@ module Pages def project_id project.id end + strong_memoize_attr :project_id def access_control project.private_pages? @@ -76,8 +75,15 @@ module Pages attr_reader :project, :trim_prefix, :domain + # project.active_pages_deployments is already loaded from the database, + # so selecting from the array to avoid N+1 + # this will change with when serving multiple versions on + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/133261 def deployment - project.pages_metadatum.pages_deployment + project + .active_pages_deployments + .to_a + .find { |deployment| deployment.path_prefix.blank? } end strong_memoize_attr :deployment diff --git a/app/models/pages_deployment.rb b/app/models/pages_deployment.rb index f05ed2aac6e..2aa36a94171 100644 --- a/app/models/pages_deployment.rb +++ b/app/models/pages_deployment.rb @@ -17,7 +17,8 @@ class PagesDeployment < ApplicationRecord scope :with_files_stored_locally, -> { where(file_store: ::ObjectStorage::Store::LOCAL) } scope :with_files_stored_remotely, -> { where(file_store: ::ObjectStorage::Store::REMOTE) } scope :project_id_in, ->(ids) { where(project_id: ids) } - scope :active, -> { where(deleted_at: nil) } + scope :with_path_prefix, ->(prefix) { where("COALESCE(path_prefix, '') = ?", prefix.to_s) } + scope :active, -> { where(deleted_at: nil).order(created_at: :desc) } scope :deactivated, -> { where('deleted_at < ?', Time.now.utc) } validates :file, presence: true @@ -33,11 +34,23 @@ class PagesDeployment < ApplicationRecord skip_callback :save, :after, :store_file! after_commit :store_file_after_commit!, on: [:create, :update] + def self.latest_pipeline_id + Ci::Build.id_in(pluck(:ci_build_id)).maximum(:commit_id) + end + + def self.deactivate_all(project) + now = Time.now.utc + active + .project_id_in(project.id) + .update_all(updated_at: now, deleted_at: now) + end + def self.deactivate_deployments_older_than(deployment, time: nil) now = Time.now.utc active .older_than(deployment.id) - .where(project_id: deployment.project_id, path_prefix: deployment.path_prefix) + .project_id_in(deployment.project_id) + .with_path_prefix(deployment.path_prefix) .update_all(updated_at: now, deleted_at: time || now) end diff --git a/app/models/pages_domain.rb b/app/models/pages_domain.rb index abc83fd5cee..cabd3924fd6 100644 --- a/app/models/pages_domain.rb +++ b/app/models/pages_domain.rb @@ -240,9 +240,7 @@ class PagesDomain < ApplicationRecord end def pages_deployed? - return false unless project - - project.pages_metadatum&.deployed? + project&.pages_deployed? end private diff --git a/app/models/project.rb b/app/models/project.rb index ba8180853e0..0d103094aec 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -464,8 +464,10 @@ class Project < ApplicationRecord # GitLab Pages has_many :pages_domains has_one :pages_metadatum, class_name: 'ProjectPagesMetadatum', inverse_of: :project - # we need to clean up files, not only remove records - has_many :pages_deployments, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + # rubocop:disable Cop/ActiveRecordDependent -- we need to clean up files, not only remove records + has_many :pages_deployments, dependent: :destroy, inverse_of: :project + # rubocop:enable Cop/ActiveRecordDependent + has_many :active_pages_deployments, -> { active }, class_name: 'PagesDeployment', inverse_of: :project # Can be too many records. We need to implement delete_all in batches. # Issue https://gitlab.com/gitlab-org/gitlab/-/issues/228637 @@ -740,7 +742,7 @@ class Project < ApplicationRecord end scope :with_pages_deployed, -> do - joins(:pages_metadatum).merge(ProjectPagesMetadatum.deployed) + where_exists(PagesDeployment.active.where('pages_deployments.project_id = projects.id')) end scope :pages_metadata_not_migrated, -> do @@ -2205,11 +2207,11 @@ class Project < ApplicationRecord end def pages_deployed? - pages_metadatum&.deployed? + active_pages_deployments.exists? end def pages_show_onboarding? - !(pages_metadatum&.onboarding_complete || pages_metadatum&.deployed) + !(pages_metadatum&.onboarding_complete || pages_deployed?) end def remove_private_deploy_keys @@ -2231,27 +2233,6 @@ class Project < ApplicationRecord ensure_pages_metadatum.update!(onboarding_complete: true) end - def mark_pages_as_deployed - ensure_pages_metadatum.update!(deployed: true) - end - - def mark_pages_as_not_deployed - ensure_pages_metadatum.update!(deployed: false) - end - - def update_pages_deployment!(deployment) - ensure_pages_metadatum.update!(pages_deployment: deployment) - end - - def set_first_pages_deployment!(deployment) - ensure_pages_metadatum - - # where().update_all to perform update in the single transaction with check for null - ProjectPagesMetadatum - .where(project_id: id, pages_deployment_id: nil) - .update_all(deployed: deployment.present?, pages_deployment_id: deployment&.id) - end - def set_full_path(gl_full_path: full_path) # We'd need to keep track of project full path otherwise directory tree # created with hashed storage enabled cannot be usefully imported using diff --git a/app/models/project_pages_metadatum.rb b/app/models/project_pages_metadatum.rb index eca2e5a740e..87cff4f2715 100644 --- a/app/models/project_pages_metadatum.rb +++ b/app/models/project_pages_metadatum.rb @@ -10,7 +10,4 @@ class ProjectPagesMetadatum < ApplicationRecord belongs_to :project, inverse_of: :pages_metadatum belongs_to :pages_deployment - - scope :deployed, -> { where(deployed: true) } - scope :with_project_route_and_deployment, -> { preload(:pages_deployment, project: [:namespace, :route]) } end diff --git a/app/models/users/phone_number_validation.rb b/app/models/users/phone_number_validation.rb index 27f13845102..2256eb8ddc4 100644 --- a/app/models/users/phone_number_validation.rb +++ b/app/models/users/phone_number_validation.rb @@ -33,7 +33,6 @@ module Users validates :telesign_reference_xid, length: { maximum: 255 } scope :for_user, -> (user_id) { where(user_id: user_id) } - scope :by_reference_id, ->(reference_id) { find_by(telesign_reference_xid: reference_id) } def self.related_to_banned_user?(international_dial_code, phone_number) joins(:banned_user).where( @@ -42,6 +41,10 @@ module Users ).exists? end + def self.by_reference_id(ref_id) + find_by(telesign_reference_xid: ref_id) + end + def validated? validated_at.present? end diff --git a/app/services/packages/nuget/check_duplicates_service.rb b/app/services/packages/nuget/check_duplicates_service.rb index 7ad9038d7c1..33a66c2bce1 100644 --- a/app/services/packages/nuget/check_duplicates_service.rb +++ b/app/services/packages/nuget/check_duplicates_service.rb @@ -49,40 +49,30 @@ module Packages strong_memoize_attr :existing_package def metadata - if remote_package_file? - ExtractMetadataContentService + if params[:remote_url].present? + ::Packages::Nuget::ExtractMetadataContentService .new(nuspec_file_content) .execute .payload else # to cover the case when package file is on disk not in object storage - MetadataExtractionService - .new(mock_package_file) - .execute - .payload + Zip::InputStream.open(params[:file]) do |zip| + ::Packages::Nuget::MetadataExtractionService + .new(zip) + .execute + .payload + end end end strong_memoize_attr :metadata - def remote_package_file? - params[:remote_url].present? - end - def nuspec_file_content - ExtractRemoteMetadataFileService + ::Packages::Nuget::ExtractRemoteMetadataFileService .new(params[:remote_url]) .execute .payload - rescue ExtractRemoteMetadataFileService::ExtractionError => e + rescue ::Packages::Nuget::ExtractRemoteMetadataFileService::ExtractionError => e raise ExtractionError, e.message end - - def mock_package_file - ::Packages::PackageFile.new( - params - .slice(:file, :file_name) - .merge(package: ::Packages::Package.nuget.build) - ) - end end end end diff --git a/app/services/packages/nuget/extract_metadata_file_service.rb b/app/services/packages/nuget/extract_metadata_file_service.rb index fd4f9b5d1c1..1daf0aba8d6 100644 --- a/app/services/packages/nuget/extract_metadata_file_service.rb +++ b/app/services/packages/nuget/extract_metadata_file_service.rb @@ -20,7 +20,7 @@ module Packages attr_reader :package_zip_file def nuspec_file_content - entry = package_zip_file.glob('*.nuspec').first + entry = extract_nuspec_file raise ExtractionError, 'nuspec file not found' unless entry raise ExtractionError, 'nuspec file too big' if MAX_FILE_SIZE < entry.size @@ -32,6 +32,16 @@ module Packages rescue Zip::EntrySizeError => e raise ExtractionError, "nuspec file has the wrong entry size: #{e.message}" end + + def extract_nuspec_file + if package_zip_file.is_a?(Zip::InputStream) + while (entry = package_zip_file.get_next_entry) # rubocop:disable Lint/AssignmentInCondition -- Following https://github.com/rubyzip/rubyzip#notes-on-zipinputstream and that's why the disable rubocop rule + break entry if entry.name.end_with?('.nuspec') + end + else + package_zip_file.glob('*.nuspec').first + end + end end end end diff --git a/app/services/packages/nuget/metadata_extraction_service.rb b/app/services/packages/nuget/metadata_extraction_service.rb index 53189063c85..813cb8e0979 100644 --- a/app/services/packages/nuget/metadata_extraction_service.rb +++ b/app/services/packages/nuget/metadata_extraction_service.rb @@ -3,8 +3,8 @@ module Packages module Nuget class MetadataExtractionService - def initialize(package_file) - @package_file = package_file + def initialize(package_zip_file) + @package_zip_file = package_zip_file end def execute @@ -13,19 +13,20 @@ module Packages private - attr_reader :package_file + attr_reader :package_zip_file def metadata - ExtractMetadataContentService + ::Packages::Nuget::ExtractMetadataContentService .new(nuspec_file_content) .execute .payload end def nuspec_file_content - ProcessPackageFileService - .new(package_file) - .execute[:nuspec_file_content] + ::Packages::Nuget::ExtractMetadataFileService + .new(package_zip_file) + .execute + .payload end end end diff --git a/app/services/packages/nuget/process_package_file_service.rb b/app/services/packages/nuget/process_package_file_service.rb index fa7a84ee3d6..99b59bd3322 100644 --- a/app/services/packages/nuget/process_package_file_service.rb +++ b/app/services/packages/nuget/process_package_file_service.rb @@ -4,7 +4,6 @@ module Packages module Nuget class ProcessPackageFileService ExtractionError = Class.new(StandardError) - NUGET_SYMBOL_FILE_EXTENSION = '.snupkg' def initialize(package_file) @package_file = package_file @@ -13,14 +12,9 @@ module Packages def execute raise ExtractionError, 'invalid package file' unless valid_package_file? - nuspec_content = nil - with_zip_file do |zip_file| - nuspec_content = nuspec_file_content(zip_file) - create_symbol_files(zip_file) if symbol_package_file? + ::Packages::Nuget::UpdatePackageFromMetadataService.new(package_file, zip_file).execute end - - ServiceResponse.success(payload: { nuspec_file_content: nuspec_content }) end private @@ -38,23 +32,6 @@ module Packages Zip::File.open(open_file.file_path, &block) # rubocop: disable Performance/Rubyzip end end - - def nuspec_file_content(zip_file) - ::Packages::Nuget::ExtractMetadataFileService - .new(zip_file) - .execute - .payload - end - - def create_symbol_files(zip_file) - ::Packages::Nuget::Symbols::CreateSymbolFilesService - .new(package_file.package, zip_file) - .execute - end - - def symbol_package_file? - package_file.file_name.end_with?(NUGET_SYMBOL_FILE_EXTENSION) - end end end end diff --git a/app/services/packages/nuget/update_package_from_metadata_service.rb b/app/services/packages/nuget/update_package_from_metadata_service.rb index 4cec4ed2fae..b7411d5f8a8 100644 --- a/app/services/packages/nuget/update_package_from_metadata_service.rb +++ b/app/services/packages/nuget/update_package_from_metadata_service.rb @@ -13,11 +13,11 @@ module Packages INVALID_METADATA_ERROR_SYMBOL_MESSAGE = 'package name, version and/or description not found in metadata' MISSING_MATCHING_PACKAGE_ERROR_MESSAGE = 'symbol package is invalid, matching package does not exist' - InvalidMetadataError = Class.new(StandardError) - ZipError = Class.new(StandardError) + InvalidMetadataError = ZipError = Class.new(StandardError) - def initialize(package_file) + def initialize(package_file, package_zip_file) @package_file = package_file + @package_zip_file = package_zip_file end def execute @@ -57,7 +57,7 @@ module Packages build_infos = package_to_destroy&.build_infos || [] update_package(target_package, build_infos) - update_symbol_files(target_package, package_to_destroy) if symbol_package? + create_symbol_files ::Packages::UpdatePackageFileService.new(@package_file, package_id: target_package.id, file_name: package_filename) .execute package_to_destroy&.destroy! @@ -79,8 +79,12 @@ module Packages raise InvalidMetadataError, e.message end - def update_symbol_files(package, package_to_destroy) - package_to_destroy.nuget_symbols.update_all(package_id: package.id) + def create_symbol_files + return unless symbol_package? + + ::Packages::Nuget::Symbols::CreateSymbolFilesService + .new(existing_package, @package_zip_file) + .execute end def valid_metadata? @@ -145,9 +149,10 @@ module Packages def symbol_package? package_types.include?(SYMBOL_PACKAGE_IDENTIFIER) end + strong_memoize_attr :symbol_package? def metadata - ::Packages::Nuget::MetadataExtractionService.new(@package_file).execute.payload + ::Packages::Nuget::MetadataExtractionService.new(@package_zip_file).execute.payload end strong_memoize_attr :metadata diff --git a/app/services/pages/delete_service.rb b/app/services/pages/delete_service.rb index dcee4c5b665..96b451aeba4 100644 --- a/app/services/pages/delete_service.rb +++ b/app/services/pages/delete_service.rb @@ -3,7 +3,7 @@ module Pages class DeleteService < BaseService def execute - project.mark_pages_as_not_deployed + PagesDeployment.deactivate_all(project) # project.pages_domains.delete_all will just nullify project_id: # > If no :dependent option is given, then it will follow the default diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index ab38efff7c9..83b28840d39 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -33,7 +33,7 @@ module Projects break error('The uploaded artifact size does not match the expected value') unless deployment break error(deployment_update.errors.first.full_message) unless deployment_update.valid? - update_project_pages_deployment(deployment) + deactive_old_deployments(deployment) success end rescue StandardError => e @@ -45,7 +45,6 @@ module Projects def success commit_status.success - @project.mark_pages_as_deployed publish_deployed_event super end @@ -84,11 +83,11 @@ module Projects def create_pages_deployment(artifacts_path, build) File.open(artifacts_path) do |file| attributes = pages_deployment_attributes(file, build) - deployment = project.pages_deployments.create!(**attributes) + deployment = project.pages_deployments.build(**attributes) - break if deployment.size != file.size || deployment.file.size != file.size + break if deployment.file.size != file.size - deployment + deployment.tap(&:save!) end end @@ -103,9 +102,7 @@ module Projects } end - def update_project_pages_deployment(deployment) - project.update_pages_deployment!(deployment) - + def deactive_old_deployments(deployment) PagesDeployment.deactivate_deployments_older_than( deployment, time: OLD_DEPLOYMENTS_DESTRUCTION_DELAY.from_now) diff --git a/app/views/admin/background_migrations/index.html.haml b/app/views/admin/background_migrations/index.html.haml index 9550ea2884e..e7212f00e5b 100644 --- a/app/views/admin/background_migrations/index.html.haml +++ b/app/views/admin/background_migrations/index.html.haml @@ -1,7 +1,7 @@ - page_title s_('BackgroundMigrations|Background Migrations') - @breadcrumb_link = admin_background_migrations_path(database: params[:database]) -.gl-display-flex.gl-sm-flex-direction-column.gl-sm-align-items-flex-end.gl-pb-5.gl-border-b-1.gl-border-b-solid.gl-border-b-gray-100 +.gl-display-flex.gl-flex-direction-column.gl-md-flex-direction-row.gl-sm-align-items-flex-end.gl-pb-5.gl-border-b-1.gl-border-b-solid.gl-border-b-gray-100 .gl-flex-grow-1 %h3= s_('BackgroundMigrations|Background Migrations') %p.light.gl-mb-0 diff --git a/app/views/groups/_home_panel.html.haml b/app/views/groups/_home_panel.html.haml index 544acd5ae56..269a7309ec2 100644 --- a/app/views/groups/_home_panel.html.haml +++ b/app/views/groups/_home_panel.html.haml @@ -3,7 +3,7 @@ - emails_disabled = @group.emails_disabled? .group-home-panel - .gl-display-flex.gl-justify-content-space-between.gl-flex-wrap.gl-sm-flex-direction-column.gl-gap-3.gl-my-5 + .gl-display-flex.gl-justify-content-space-between.gl-flex-wrap.gl-flex-direction-column.gl-md-flex-direction-row.gl-gap-3.gl-my-5 .home-panel-title-row.gl-display-flex.gl-align-items-center .avatar-container.rect-avatar.s64.home-panel-avatar.gl-flex-shrink-0.float-none{ class: 'gl-mr-3!' } = group_icon(@group, class: 'avatar avatar-tile s64', width: 64, height: 64, itemprop: 'logo') diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml index e8b78298eb4..e41a0d3d262 100644 --- a/app/views/projects/_home_panel.html.haml +++ b/app/views/projects/_home_panel.html.haml @@ -3,7 +3,7 @@ - emails_disabled = @project.emails_disabled? .project-home-panel.js-show-on-project-root.gl-mt-4.gl-mb-5{ class: [("empty-project" if empty_repo)] } - .gl-display-flex.gl-justify-content-space-between.gl-flex-wrap.gl-sm-flex-direction-column.gl-mb-3.gl-gap-5 + .gl-display-flex.gl-justify-content-space-between.gl-flex-wrap.gl-flex-direction-column.gl-md-flex-direction-row.gl-mb-3.gl-gap-5 .home-panel-title-row.gl-display-flex.gl-align-items-center %div{ class: 'avatar-container rect-avatar s64 home-panel-avatar gl-flex-shrink-0 gl-w-11 gl-h-11 gl-mr-3! float-none' } = project_icon(@project, alt: @project.name, class: 'avatar avatar-tile s64', width: 64, height: 64, itemprop: 'image') diff --git a/app/views/projects/forks/index.html.haml b/app/views/projects/forks/index.html.haml index 49047749b71..fe7d2c9d198 100644 --- a/app/views/projects/forks/index.html.haml +++ b/app/views/projects/forks/index.html.haml @@ -8,7 +8,7 @@ - full_count_title = "#{@public_forks_count} public, #{@internal_forks_count} internal, and #{@private_forks_count} private" #{pluralize(@total_forks_count, 'fork')}: #{full_count_title} - .gl-display-flex.gl-sm-flex-direction-column.gl-md-align-items-center + .gl-display-flex.gl-flex-direction-column.gl-md-flex-direction-row.gl-md-align-items-center = form_tag request.original_url, method: :get, class: 'project-filter-form gl-display-flex gl-mt-3 gl-md-mt-0', id: 'project-filter-form' do |f| = search_field_tag :filter_projects, nil, placeholder: _('Search forks'), class: 'projects-list-filter project-filter-form-field form-control input-short gl-flex-grow-1', spellcheck: false, data: { 'filter-selector' => 'span.namespace-name' } diff --git a/app/workers/packages/nuget/extraction_worker.rb b/app/workers/packages/nuget/extraction_worker.rb index 55aca0beb03..33fc98cf95b 100644 --- a/app/workers/packages/nuget/extraction_worker.rb +++ b/app/workers/packages/nuget/extraction_worker.rb @@ -18,7 +18,7 @@ module Packages return unless package_file - ::Packages::Nuget::UpdatePackageFromMetadataService.new(package_file).execute + ::Packages::Nuget::ProcessPackageFileService.new(package_file).execute rescue StandardError => exception process_package_file_error( package_file: package_file, diff --git a/data/deprecations/15-9-database-single-database-connection-conf.yml b/data/deprecations/15-9-database-single-database-connection-conf.yml index de4ae51d615..c7d59e860ca 100644 --- a/data/deprecations/15-9-database-single-database-connection-conf.yml +++ b/data/deprecations/15-9-database-single-database-connection-conf.yml @@ -6,8 +6,6 @@ stage: Enablement issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/387898 body: | - This deprecation is now superseded by another [deprecation notice](#running-a-single-database-is-deprecated). - Previously, [GitLab's database](https://docs.gitlab.com/omnibus/settings/database.html) configuration had a single `main:` section. This is being deprecated. The new configuration has both a `main:` and a `ci:` section. diff --git a/db/docs/namespaces.yml b/db/docs/namespaces.yml index 4010858ce5a..8fa7c2a3d31 100644 --- a/db/docs/namespaces.yml +++ b/db/docs/namespaces.yml @@ -15,3 +15,6 @@ schema_inconsistencies: - type: missing_indexes object_name: index_namespaces_on_created_at introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/134948 +- type: missing_indexes + object_name: index_namespaces_on_ldap_sync_last_successful_update_at + introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/135040 diff --git a/db/migrate/20231109133153_drop_idx_namespaces_on_ldap_sync_last_successful_update_at_for_gitlab.rb b/db/migrate/20231109133153_drop_idx_namespaces_on_ldap_sync_last_successful_update_at_for_gitlab.rb new file mode 100644 index 00000000000..1d171e3285c --- /dev/null +++ b/db/migrate/20231109133153_drop_idx_namespaces_on_ldap_sync_last_successful_update_at_for_gitlab.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +class DropIdxNamespacesOnLdapSyncLastSuccessfulUpdateAtForGitlab < Gitlab::Database::Migration[2.2] + milestone '16.6' + + disable_ddl_transaction! + + TABLE_NAME = :namespaces + INDEX_NAME = :index_namespaces_on_ldap_sync_last_successful_update_at + + def up + return unless should_run? + + remove_concurrent_index_by_name TABLE_NAME, INDEX_NAME + end + + def down + return unless should_run? + + add_concurrent_index TABLE_NAME, :ldap_sync_last_successful_update_at, name: INDEX_NAME + end + + private + + def should_run? + Gitlab.com_except_jh? + end +end diff --git a/db/schema_migrations/20231109133153 b/db/schema_migrations/20231109133153 new file mode 100644 index 00000000000..c9cfd53a77d --- /dev/null +++ b/db/schema_migrations/20231109133153 @@ -0,0 +1 @@ +fb17684ac5976811bd08e4a2edb3b3c45baaf293ee3c04e986ae3c197c59c54a
\ No newline at end of file diff --git a/doc/administration/settings/slack_app.md b/doc/administration/settings/slack_app.md index ef756dfeff7..de11da281e4 100644 --- a/doc/administration/settings/slack_app.md +++ b/doc/administration/settings/slack_app.md @@ -105,9 +105,13 @@ To enable the GitLab for Slack app functionality, your network must allow inboun ## Troubleshooting -### Slash commands return `/gitlab failed with the error "dispatch_failed"` in Slack +When administering the GitLab for Slack app for self-managed instances, you might encounter the following issues. + +For GitLab.com, see [GitLab for Slack app](../../user/project/integrations/gitlab_slack_application.md#troubleshooting). + +### Slash commands return an error in Slack Slash commands might return `/gitlab failed with the error "dispatch_failed"` in Slack. To resolve this issue, ensure: -- The GitLab for Slack app is properly [configured](#configure-the-settings), and the **Enable GitLab for Slack app** checkbox is selected. +- The GitLab for Slack app is properly [configured](#configure-the-settings) and the **Enable GitLab for Slack app** checkbox is selected. - Your GitLab instance [allows requests to and from Slack](#connectivity-requirements). diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index c7921115e1a..4a1b536fd40 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -22200,6 +22200,7 @@ Active period time range for on-call rotation. | <a id="organizationname"></a>`name` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Name of the organization. | | <a id="organizationorganizationusers"></a>`organizationUsers` **{warning-solid}** | [`OrganizationUserConnection!`](#organizationuserconnection) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Users with access to the organization. | | <a id="organizationpath"></a>`path` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.4. This feature is an Experiment. It can be changed or removed at any time. Path of the organization. | +| <a id="organizationweburl"></a>`webUrl` **{warning-solid}** | [`String!`](#string) | **Introduced** in 16.6. This feature is an Experiment. It can be changed or removed at any time. Web URL of the organization. | #### Fields with arguments diff --git a/doc/api/settings.md b/doc/api/settings.md index 07472d99945..9c0a1e8e4a8 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -20,6 +20,7 @@ For information on how to control the application settings cache for an instance > - `always_perform_delayed_deletion` feature flag [enabled](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113332) in GitLab 15.11. > - `delayed_project_deletion` and `delayed_group_deletion` attributes removed in GitLab 16.0. > - `in_product_marketing_emails_enabled` attribute [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/418137) in GitLab 16.6. +> - `repository_storages` attribute [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/429675) in GitLab 16.6. List the current [application settings](#list-of-settings-that-can-be-accessed-via-api-calls) of the GitLab instance. diff --git a/doc/ci/components/index.md b/doc/ci/components/index.md index 0ebfb974afb..3d46ec5bbd5 100644 --- a/doc/ci/components/index.md +++ b/doc/ci/components/index.md @@ -204,6 +204,13 @@ providing users with information about official releases. Components [can be used](#use-a-component-in-a-cicd-configuration) without being released, by using the commit SHA or ref. However, the `~latest` version keyword can only be used with released tags. +NOTE: +The `~latest` keyword always returns the most recent release, not the release with +the latest semantic version. For example, if you first release `v2.0.0`, and later release +a patch fix like `v1.5.1`, then `~latest` returns the `v1.5.1` release. +[Issue #427286](https://gitlab.com/gitlab-org/gitlab/-/issues/427286) proposes to +change this behavior. + ## Use a component in a CI/CD configuration You can add a component to a CI/CD configuration with the `include: component` keyword. diff --git a/doc/development/database/clickhouse/clickhouse_within_gitlab.md b/doc/development/database/clickhouse/clickhouse_within_gitlab.md index 297776429d7..2f7a3c4dfe0 100644 --- a/doc/development/database/clickhouse/clickhouse_within_gitlab.md +++ b/doc/development/database/clickhouse/clickhouse_within_gitlab.md @@ -45,22 +45,39 @@ ClickHouse::Client.select('SELECT 1', :main) ## Database schema and migrations -For the ClickHouse database there are no established schema migration procedures yet. We have very basic tooling to build up the database schema in the test environment from scratch using timestamp-prefixed SQL files. - -You can create a table by placing a new SQL file in the `db/click_house/main` folder: - -```sql -// 20230811124511_create_issues.sql -CREATE TABLE issues -( - id UInt64 DEFAULT 0, - title String DEFAULT '' -) -ENGINE = MergeTree -PRIMARY KEY (id) +There are `bundle exec rake gitlab:clickhouse:migrate` and `bundle exec rake gitlab:clickhouse:rollback` tasks +(introduced in [!136103](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/136103)). + +You can create a migration by creating a Ruby migration file in `db/click_house/migrate` folder. It should be prefixed with a timestamp in the format `YYYYMMDDHHMMSS_description_of_migration.rb` + +```ruby +# 20230811124511_create_issues.rb +# frozen_string_literal: true + +class CreateIssues < ClickHouse::Migration + def up + execute <<~SQL + CREATE TABLE issues + ( + id UInt64 DEFAULT 0, + title String DEFAULT '' + ) + ENGINE = MergeTree + PRIMARY KEY (id) + SQL + end + + def down + execute <<~SQL + DROP TABLE sync_cursors + SQL + end +end ``` -When you're working locally in your development environment, you can create or re-create your table schema by executing the respective `CREATE TABLE` statement. Alternatively, you can use the following snippet in the Rails console: +When you're working locally in your development environment, you can create or re-create your table schema by +executing `rake gitlab:clickhouse:rollback` and `rake gitlab:clickhouse:migrate`. +Alternatively, you can use the following snippet in the Rails console: ```ruby require_relative 'spec/support/database/click_house/hooks.rb' diff --git a/doc/update/deprecations.md b/doc/update/deprecations.md index dba4a328fb9..333dad86086 100644 --- a/doc/update/deprecations.md +++ b/doc/update/deprecations.md @@ -1007,8 +1007,6 @@ For updates and details about this deprecation, follow [this epic](https://gitla - To discuss this change or learn more, see the [deprecation issue](https://gitlab.com/gitlab-org/gitlab/-/issues/387898). </div> -This deprecation is now superseded by another [deprecation notice](#running-a-single-database-is-deprecated). - Previously, [GitLab's database](https://docs.gitlab.com/omnibus/settings/database.html) configuration had a single `main:` section. This is being deprecated. The new configuration has both a `main:` and a `ci:` section. diff --git a/doc/user/project/integrations/gitlab_slack_application.md b/doc/user/project/integrations/gitlab_slack_application.md index 67a6916c3b6..abfd4243e07 100644 --- a/doc/user/project/integrations/gitlab_slack_application.md +++ b/doc/user/project/integrations/gitlab_slack_application.md @@ -74,7 +74,8 @@ You can use slash commands to run common GitLab operations. Replace `<project>` - You must authorize your Slack user on GitLab.com when you run your first slash command. - You can [create a shorter project alias](#create-a-project-alias-for-slash-commands) for slash commands. -**For [Slack slash commands](slack_slash_commands.md) on self-managed GitLab and [Mattermost slash commands](mattermost_slash_commands.md), replace `/gitlab` with the slash command trigger name configured for your integration. +**For [Slack slash commands](slack_slash_commands.md) on self-managed GitLab and [Mattermost slash commands](mattermost_slash_commands.md)**, +replace `/gitlab` with the slash command trigger name configured for your integration. The following slash commands are available: @@ -172,7 +173,11 @@ The following events are available for Slack notifications: ## Troubleshooting -### GitLab for Slack app does not appear in the list of integrations +When configuring the GitLab for Slack app on GitLab.com, you might encounter the following issues. + +For self-managed GitLab, see [GitLab for Slack app administration](../../../administration/settings/slack_app.md#troubleshooting). + +### The app does not appear in the list of integrations The GitLab for Slack app might not appear in the list of integrations. To have the GitLab for Slack app on your self-managed instance, an administrator must [enable the integration](../../../administration/settings/slack_app.md). On GitLab.com, the GitLab for Slack app is available by default. @@ -193,9 +198,10 @@ As a workaround, ensure: - If using a [project alias](#create-a-project-alias-for-slash-commands), the alias is correct. - The GitLab for Slack app is [enabled for the project](#from-project-integration-settings). -### Slash commands return `/gitlab failed with the error "dispatch_failed"` in Slack +### Slash commands return an error in Slack -Slash commands might return `/gitlab failed with the error "dispatch_failed"` in Slack. To resolve this issue, ensure an administrator has properly configured the [GitLab for Slack app settings](../../../administration/settings/slack_app.md) on your self-managed instance. +Slash commands might return `/gitlab failed with the error "dispatch_failed"` in Slack. +To resolve this issue, ensure an administrator has properly configured the [GitLab for Slack app settings](../../../administration/settings/slack_app.md) on your self-managed instance. ### Notifications are not received to a channel diff --git a/doc/user/search/index.md b/doc/user/search/index.md index 15d3b2e41e1..79782b1c880 100644 --- a/doc/user/search/index.md +++ b/doc/user/search/index.md @@ -103,13 +103,13 @@ For example: ## Include archived projects in search results -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121981) in GitLab 16.1 [with a flag](../../administration/feature_flags.md) named `search_projects_hide_archived` for the project scope. Disabled by default. -> - [Generally available](https://gitlab.com/groups/gitlab-org/-/epics/10957) in GitLab 16.6 for all scopes. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/121981) in GitLab 16.1 [with a flag](../../administration/feature_flags.md) named `search_projects_hide_archived` for project search. Disabled by default. +> - [Generally available](https://gitlab.com/groups/gitlab-org/-/epics/10957) in GitLab 16.6 for all search scopes. By default, archived projects are excluded from search results. -To include the search result from archived projects: +To include archived projects in search results: -1. On the search result page, on the left sidebar, select the **Include archived** checkbox. +1. On the search page, on the left sidebar, select the **Include archived** checkbox. 1. On the left sidebar, select **Apply**. ## Search for code diff --git a/doc/user/workspace/index.md b/doc/user/workspace/index.md index e946a14e9a1..21905381577 100644 --- a/doc/user/workspace/index.md +++ b/doc/user/workspace/index.md @@ -95,9 +95,13 @@ Only these properties are relevant to the GitLab implementation of the `containe | `endpoints` | Port mappings to expose from the container. | | `volumeMounts` | Storage volume to mount in the container. | -### Using variables in a Devfile +### Using variables in a devfile -You can also define variables to be used in your devfile. The `variables` object is a map of name-value pairs used for string replacement in the devfile. The [Devfile 2.2.0 documentation](https://devfile.io/docs/2.2.0/defining-variables) lists some limits on how and where you can use variables. In addition, you cannot define any variables whose names are prefixed with `gl-`/`gl_`/`GL-`/`GL_`. +You can define variables to use in your devfile. +The `variables` object is a map of name-value pairs that you can use for string replacement in the devfile. + +Variables cannot have names that start with `gl-`, `gl_`, `GL-`, or `GL_`. +For more information about how and where to use variables, see the [devfile documentation](https://devfile.io/docs/2.2.0/defining-variables). ### Example configurations diff --git a/lib/gitlab/pages/deployment_update.rb b/lib/gitlab/pages/deployment_update.rb index 6845f5d88ec..bf6ac3a056d 100644 --- a/lib/gitlab/pages/deployment_update.rb +++ b/lib/gitlab/pages/deployment_update.rb @@ -89,14 +89,10 @@ module Gitlab project.actual_limits.pages_file_entries end + # If a newer pipeline already build a PagesDeployment def validate_outdated_sha return if latest? - - # use pipeline_id in case the build is retried - last_deployed_pipeline_id = project.pages_metadatum&.pages_deployment&.ci_build&.pipeline_id - - return unless last_deployed_pipeline_id - return if last_deployed_pipeline_id <= build.pipeline_id + return if latest_pipeline_id <= build.pipeline_id errors.add(:base, 'build SHA is outdated for this ref') end @@ -111,6 +107,13 @@ module Gitlab def sha build.sha end + + def latest_pipeline_id + project + .active_pages_deployments + .with_path_prefix(build.pages&.dig(:path_prefix)) + .latest_pipeline_id + end end end end diff --git a/lib/gitlab/url_builder.rb b/lib/gitlab/url_builder.rb index 1b7dcaa5cf4..a9b8dc313d0 100644 --- a/lib/gitlab/url_builder.rb +++ b/lib/gitlab/url_builder.rb @@ -40,6 +40,8 @@ module Gitlab note_url(object, **options) when Release instance.release_url(object, **options) + when Organizations::Organization + instance.organization_url(object, **options) when Project instance.project_url(object, **options) when Snippet diff --git a/lib/tasks/gitlab/click_house/migration.rake b/lib/tasks/gitlab/click_house/migration.rake index 2c4bce65d80..ddac81ec98f 100644 --- a/lib/tasks/gitlab/click_house/migration.rake +++ b/lib/tasks/gitlab/click_house/migration.rake @@ -34,10 +34,6 @@ namespace :gitlab do ENV['VERSION'].to_i if ENV['VERSION'] && !ENV['VERSION'].empty? end - def verbose - ENV['VERBOSE'] ? ENV['VERBOSE'] != 'false' : true - end - def migrate(direction) require_relative '../../../../lib/click_house/migration_support/schema_migration' require_relative '../../../../lib/click_house/migration_support/migration_context' @@ -47,7 +43,7 @@ namespace :gitlab do scope = ENV['SCOPE'] verbose_was = ClickHouse::Migration.verbose - ClickHouse::Migration.verbose = verbose + ClickHouse::Migration.verbose = ENV['VERBOSE'] ? ENV['VERBOSE'] != 'false' : true migrations_paths = ClickHouse::MigrationSupport::Migrator.migrations_paths schema_migration = ClickHouse::MigrationSupport::SchemaMigration diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 32017de48a0..f95890d9ccf 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -33515,6 +33515,9 @@ msgstr "" msgid "Organization|Organization URL is required." msgstr "" +msgid "Organization|Organization URL must be a minimum of two characters." +msgstr "" + msgid "Organization|Organization name" msgstr "" diff --git a/spec/click_house/migration_support/migration_context_spec.rb b/spec/click_house/migration_support/migration_context_spec.rb index 9df8391270d..48ad9d9e3fa 100644 --- a/spec/click_house/migration_support/migration_context_spec.rb +++ b/spec/click_house/migration_support/migration_context_spec.rb @@ -6,7 +6,7 @@ require_relative '../../../lib/click_house/migration_support/migration_error' RSpec.describe ClickHouse::MigrationSupport::MigrationContext, click_house: :without_migrations, feature_category: :database do - include ClickHouseHelpers + include ClickHouseTestHelpers # We don't need to delete data since we don't modify Postgres data self.use_transactional_tests = false @@ -96,7 +96,7 @@ RSpec.describe ClickHouse::MigrationSupport::MigrationContext, end around do |example| - clear_db(configuration: config) + clear_db(config) previous_config = ClickHouse::Migration.client_configuration ClickHouse::Migration.client_configuration = config diff --git a/spec/controllers/projects/pages_controller_spec.rb b/spec/controllers/projects/pages_controller_spec.rb index 34ec8d8d575..d94150a37d0 100644 --- a/spec/controllers/projects/pages_controller_spec.rb +++ b/spec/controllers/projects/pages_controller_spec.rb @@ -48,7 +48,6 @@ RSpec.describe Projects::PagesController, feature_category: :pages do context 'when the project does not have onboarding complete' do before do - project.pages_metadatum.update_attribute(:deployed, false) project.pages_metadatum.update_attribute(:onboarding_complete, false) end @@ -76,6 +75,17 @@ RSpec.describe Projects::PagesController, feature_category: :pages do end end + context 'when the project has a deployed pages app' do + before do + project.pages_metadatum.update_attribute(:onboarding_complete, false) + create(:pages_deployment, project: project) + end + + it 'does not redirect to #new' do + expect(subject).not_to redirect_to(action: 'new') + end + end + context 'when pages is disabled' do let(:project) { create(:project, :pages_disabled) } diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 0962c251382..1e3ade779af 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -508,7 +508,7 @@ FactoryBot.define do trait :pages_published do after(:create) do |project| project.mark_pages_onboarding_complete - project.mark_pages_as_deployed + create(:pages_deployment, project: project) # rubocop: disable RSpec/FactoryBot/StrategyInCallback end end diff --git a/spec/features/projects/pages/user_edits_settings_spec.rb b/spec/features/projects/pages/user_edits_settings_spec.rb index 255ea43758f..4ad729a29e1 100644 --- a/spec/features/projects/pages/user_edits_settings_spec.rb +++ b/spec/features/projects/pages/user_edits_settings_spec.rb @@ -22,7 +22,7 @@ RSpec.describe 'Pages edits pages settings', :js, feature_category: :pages do context 'when pages deployed' do before do - project.mark_pages_as_deployed + create(:pages_deployment, project: project) end it 'renders Access pages' do @@ -125,7 +125,7 @@ RSpec.describe 'Pages edits pages settings', :js, feature_category: :pages do before do project.namespace.update!(owner: user) - project.mark_pages_as_deployed + create(:pages_deployment, project: project) end it 'tries to change the setting' do @@ -187,7 +187,7 @@ RSpec.describe 'Pages edits pages settings', :js, feature_category: :pages do describe 'Remove page' do context 'when pages are deployed' do before do - project.mark_pages_as_deployed + create(:pages_deployment, project: project) end it 'removes the pages', :sidekiq_inline do diff --git a/spec/frontend/organizations/new/components/app_spec.js b/spec/frontend/organizations/new/components/app_spec.js index 06d30ad6b12..4f31baedbf6 100644 --- a/spec/frontend/organizations/new/components/app_spec.js +++ b/spec/frontend/organizations/new/components/app_spec.js @@ -3,16 +3,19 @@ import Vue, { nextTick } from 'vue'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import App from '~/organizations/new/components/app.vue'; -import resolvers from '~/organizations/shared/graphql/resolvers'; +import organizationCreateMutation from '~/organizations/new/graphql/mutations/organization_create.mutation.graphql'; import NewEditForm from '~/organizations/shared/components/new_edit_form.vue'; import { visitUrlWithAlerts } from '~/lib/utils/url_utility'; -import { createOrganizationResponse } from '~/organizations/mock_data'; +import FormErrorsAlert from '~/vue_shared/components/form/errors_alert.vue'; +import { + organizationCreateResponse, + organizationCreateResponseWithErrors, +} from '~/organizations/mock_data'; import { createAlert } from '~/alert'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; Vue.use(VueApollo); -jest.useFakeTimers(); jest.mock('~/lib/utils/url_utility'); jest.mock('~/alert'); @@ -21,8 +24,12 @@ describe('OrganizationNewApp', () => { let wrapper; let mockApollo; - const createComponent = ({ mockResolvers = resolvers } = {}) => { - mockApollo = createMockApollo([], mockResolvers); + const createComponent = ({ + handlers = [ + [organizationCreateMutation, jest.fn().mockResolvedValue(organizationCreateResponse)], + ], + } = {}) => { + mockApollo = createMockApollo(handlers); wrapper = shallowMountExtended(App, { apolloProvider: mockApollo }); }; @@ -46,13 +53,11 @@ describe('OrganizationNewApp', () => { describe('when form is submitted', () => { describe('when API is loading', () => { beforeEach(async () => { - const mockResolvers = { - Mutation: { - createOrganization: jest.fn().mockReturnValueOnce(new Promise(() => {})), - }, - }; - - createComponent({ mockResolvers }); + createComponent({ + handlers: [ + [organizationCreateMutation, jest.fn().mockReturnValueOnce(new Promise(() => {}))], + ], + }); await submitForm(); }); @@ -66,13 +71,12 @@ describe('OrganizationNewApp', () => { beforeEach(async () => { createComponent(); await submitForm(); - jest.runAllTimers(); await waitForPromises(); }); - it('redirects user to organization path', () => { + it('redirects user to organization web url', () => { expect(visitUrlWithAlerts).toHaveBeenCalledWith( - createOrganizationResponse.organization.path, + organizationCreateResponse.data.organizationCreate.organization.webUrl, [ { id: 'organization-successfully-created', @@ -86,26 +90,44 @@ describe('OrganizationNewApp', () => { }); describe('when API request is not successful', () => { - const error = new Error(); - - beforeEach(async () => { - const mockResolvers = { - Mutation: { - createOrganization: jest.fn().mockRejectedValueOnce(error), - }, - }; + describe('when there is a network error', () => { + const error = new Error(); + + beforeEach(async () => { + createComponent({ + handlers: [[organizationCreateMutation, jest.fn().mockRejectedValue(error)]], + }); + await submitForm(); + await waitForPromises(); + }); - createComponent({ mockResolvers }); - await submitForm(); - jest.runAllTimers(); - await waitForPromises(); + it('displays error alert', () => { + expect(createAlert).toHaveBeenCalledWith({ + message: 'An error occurred creating an organization. Please try again.', + error, + captureError: true, + }); + }); }); - it('displays error alert', () => { - expect(createAlert).toHaveBeenCalledWith({ - message: 'An error occurred creating an organization. Please try again.', - error, - captureError: true, + describe('when there are GraphQL errors', () => { + beforeEach(async () => { + createComponent({ + handlers: [ + [ + organizationCreateMutation, + jest.fn().mockResolvedValue(organizationCreateResponseWithErrors), + ], + ], + }); + await submitForm(); + await waitForPromises(); + }); + + it('displays form errors alert', () => { + expect(wrapper.findComponent(FormErrorsAlert).props('errors')).toEqual( + organizationCreateResponseWithErrors.data.organizationCreate.errors, + ); }); }); }); diff --git a/spec/frontend/organizations/shared/components/new_edit_form_spec.js b/spec/frontend/organizations/shared/components/new_edit_form_spec.js index 42a6f20962d..93f022a3259 100644 --- a/spec/frontend/organizations/shared/components/new_edit_form_spec.js +++ b/spec/frontend/organizations/shared/components/new_edit_form_spec.js @@ -49,6 +49,17 @@ describe('NewEditForm', () => { expect(findUrlField().exists()).toBe(true); }); + it('requires `Organization URL` field to be a minimum of two characters', async () => { + createComponent(); + + await findUrlField().setValue('f'); + await submitForm(); + + expect( + wrapper.findByText('Organization URL must be a minimum of two characters.').exists(), + ).toBe(true); + }); + describe('when `fieldsToRender` prop is set', () => { beforeEach(() => { createComponent({ propsData: { fieldsToRender: [FORM_FIELD_ID] } }); diff --git a/spec/frontend/token_access/token_projects_table_spec.js b/spec/frontend/token_access/token_projects_table_spec.js index 7654aa09b0a..7a78befa0d7 100644 --- a/spec/frontend/token_access/token_projects_table_spec.js +++ b/spec/frontend/token_access/token_projects_table_spec.js @@ -28,7 +28,6 @@ describe('Token projects table', () => { const findAllDeleteProjectBtn = () => wrapper.findAllComponents(GlButton); const findAllTableRows = () => wrapper.findAllByTestId('projects-token-table-row'); const findProjectNameCell = () => wrapper.findByTestId('token-access-project-name'); - const findProjectNamespaceCell = () => wrapper.findByTestId('token-access-project-namespace'); it('displays a table', () => { createComponent(); @@ -57,25 +56,9 @@ describe('Token projects table', () => { expect(findAllDeleteProjectBtn()).toHaveLength(1); }); - it('displays project and namespace cells', () => { + it('displays project fullpath', () => { createComponent(); - expect(findProjectNameCell().text()).toBe('merge-train-stuff'); - expect(findProjectNamespaceCell().text()).toBe('root'); - }); - - it('displays empty string for namespace when namespace is null', () => { - const nullNamespace = { - id: '1', - name: 'merge-train-stuff', - namespace: null, - fullPath: 'root/merge-train-stuff', - isLocked: false, - __typename: 'Project', - }; - - createComponent({ projects: [nullNamespace] }); - - expect(findProjectNamespaceCell().text()).toBe(''); + expect(findProjectNameCell().text()).toBe('root/merge-train-stuff'); }); }); diff --git a/spec/frontend/vue_shared/components/form/errors_alert_spec.js b/spec/frontend/vue_shared/components/form/errors_alert_spec.js new file mode 100644 index 00000000000..b7efe19378d --- /dev/null +++ b/spec/frontend/vue_shared/components/form/errors_alert_spec.js @@ -0,0 +1,60 @@ +import { GlAlert } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import FormErrorsAlert from '~/vue_shared/components/form/errors_alert.vue'; + +describe('FormErrorsAlert', () => { + let wrapper; + + const defaultPropsData = { + errors: ['Foo', 'Bar', 'Baz'], + }; + + function createComponent({ propsData = {} } = {}) { + wrapper = shallowMount(FormErrorsAlert, { + propsData: { + ...defaultPropsData, + ...propsData, + }, + }); + } + + const findAlert = () => wrapper.findComponent(GlAlert); + + describe('when there are no errors', () => { + it('renders nothing', () => { + createComponent({ propsData: { errors: [] } }); + + expect(wrapper.html()).toBe(''); + }); + }); + + describe('when there is one error', () => { + it('renders correct title and message', () => { + createComponent({ propsData: { errors: ['Foo'] } }); + + expect(findAlert().props('title')).toBe('The form contains the following error:'); + expect(findAlert().text()).toContain('Foo'); + }); + }); + + describe('when there are multiple errors', () => { + it('renders correct title and message', () => { + createComponent(); + + expect(findAlert().props('title')).toBe('The form contains the following errors:'); + expect(findAlert().text()).toContain('Foo'); + expect(findAlert().text()).toContain('Bar'); + expect(findAlert().text()).toContain('Baz'); + }); + }); + + describe('when alert is dismissed', () => { + it('emits input event with empty array as payload', () => { + createComponent(); + + findAlert().vm.$emit('dismiss'); + + expect(wrapper.emitted('input')).toEqual([[[]]]); + }); + }); +}); diff --git a/spec/graphql/types/organizations/organization_type_spec.rb b/spec/graphql/types/organizations/organization_type_spec.rb index 26d7c10a715..62787ad220d 100644 --- a/spec/graphql/types/organizations/organization_type_spec.rb +++ b/spec/graphql/types/organizations/organization_type_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe GitlabSchema.types['Organization'], feature_category: :cell do - let(:expected_fields) { %w[groups id name organization_users path] } + let(:expected_fields) { %w[groups id name organization_users path web_url] } specify { expect(described_class.graphql_name).to eq('Organization') } specify { expect(described_class).to require_graphql_authorizations(:read_organization) } diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index f1e1ea3d936..722b47ac9b8 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -660,6 +660,7 @@ project: - pages_domains - pages_metadatum - pages_deployments +- active_pages_deployments - authorized_users - project_authorizations - remote_mirrors diff --git a/spec/lib/gitlab/pages/deployment_update_spec.rb b/spec/lib/gitlab/pages/deployment_update_spec.rb index cf109248f36..9a7564ddd59 100644 --- a/spec/lib/gitlab/pages/deployment_update_spec.rb +++ b/spec/lib/gitlab/pages/deployment_update_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Pages::DeploymentUpdate do +RSpec.describe Gitlab::Pages::DeploymentUpdate, feature_category: :pages do let_it_be(:project, refind: true) { create(:project, :repository) } let_it_be(:old_pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) } diff --git a/spec/lib/gitlab/pages/virtual_host_finder_spec.rb b/spec/lib/gitlab/pages/virtual_host_finder_spec.rb index 8c34968bbfc..bc3f9d89b5f 100644 --- a/spec/lib/gitlab/pages/virtual_host_finder_spec.rb +++ b/spec/lib/gitlab/pages/virtual_host_finder_spec.rb @@ -5,10 +5,6 @@ require 'spec_helper' RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do let_it_be(:project) { create(:project) } - before_all do - project.update_pages_deployment!(create(:pages_deployment, project: project)) - end - before do stub_pages_setting(host: 'example.com') end @@ -24,10 +20,6 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do subject(:virtual_domain) { described_class.new(pages_domain.domain).execute } context 'when there are no pages deployed for the project' do - before_all do - project.mark_pages_as_not_deployed - end - it 'returns nil' do expect(virtual_domain).to be_nil end @@ -35,7 +27,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do context 'when there are pages deployed for the project' do before_all do - project.mark_pages_as_deployed + create(:pages_deployment, project: project) end it 'returns the virual domain when there are pages deployed for the project' do @@ -48,10 +40,6 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do context 'when host is a namespace domain' do context 'when there are no pages deployed for the project' do - before_all do - project.mark_pages_as_not_deployed - end - it 'returns no result if the provided host is not subdomain of the Pages host' do virtual_domain = described_class.new("#{project.namespace.path}.something.io").execute @@ -68,7 +56,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do context 'when there are pages deployed for the project' do before_all do - project.mark_pages_as_deployed + create(:pages_deployment, project: project) project.namespace.update!(path: 'topNAMEspace') end @@ -109,10 +97,6 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end context 'when there are no pages deployed for the project' do - before_all do - project.mark_pages_as_not_deployed - end - it 'returns nil' do expect(virtual_domain).to be_nil end @@ -120,7 +104,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do context 'when there are pages deployed for the project' do before_all do - project.mark_pages_as_deployed + create(:pages_deployment, project: project) end it 'returns the virual domain when there are pages deployed for the project' do @@ -133,9 +117,10 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do it 'prioritizes the unique domain project' do group = create(:group, path: 'unique-domain') other_project = build(:project, path: 'unique-domain.example.com', group: group) - other_project.save!(validate: false) - other_project.update_pages_deployment!(create(:pages_deployment, project: other_project)) - other_project.mark_pages_as_deployed + .tap { |project| project.save!(validate: false) } + + create(:pages_deployment, project: project) + create(:pages_deployment, project: other_project) expect(virtual_domain).to be_an_instance_of(Pages::VirtualDomain) expect(virtual_domain.lookup_paths.first.project_id).to eq(project.id) @@ -150,10 +135,6 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do end context 'when there are no pages deployed for the project' do - before_all do - project.mark_pages_as_not_deployed - end - it 'returns nil' do expect(virtual_domain).to be_nil end @@ -161,7 +142,7 @@ RSpec.describe Gitlab::Pages::VirtualHostFinder, feature_category: :pages do context 'when there are pages deployed for the project' do before_all do - project.mark_pages_as_deployed + create(:pages_deployment, project: project) end it 'returns nil' do diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb index 68eb38a1335..81b70f85c3a 100644 --- a/spec/lib/gitlab/url_builder_spec.rb +++ b/spec/lib/gitlab/url_builder_spec.rb @@ -30,6 +30,7 @@ RSpec.describe Gitlab::UrlBuilder do :project_snippet | ->(snippet) { "/#{snippet.project.full_path}/-/snippets/#{snippet.id}" } :project_wiki | ->(wiki) { "/#{wiki.container.full_path}/-/wikis/home" } :release | ->(release) { "/#{release.project.full_path}/-/releases/#{release.tag}" } + :organization | ->(organization) { "/-/organizations/#{organization.path}" } :ci_build | ->(build) { "/#{build.project.full_path}/-/jobs/#{build.id}" } :design | ->(design) { "/#{design.project.full_path}/-/design_management/designs/#{design.id}/raw_image" } diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 590abebb764..85569a68252 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1973,7 +1973,7 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do parent_2 = create(:group) # No projects create(:project, group: child_1_1).tap do |project| - project.pages_metadatum.update!(deployed: true) + create(:pages_deployment, project: project) end create(:project, group: child_1_1) diff --git a/spec/models/organizations/organization_spec.rb b/spec/models/organizations/organization_spec.rb index 2f9f04fd3e6..0670002135c 100644 --- a/spec/models/organizations/organization_spec.rb +++ b/spec/models/organizations/organization_spec.rb @@ -181,4 +181,13 @@ RSpec.describe Organizations::Organization, type: :model, feature_category: :cel it { is_expected.to eq false } end end + + describe '#web_url' do + it 'returns web url from `Gitlab::UrlBuilder`' do + web_url = 'http://127.0.0.1:3000/-/organizations/default' + + expect(Gitlab::UrlBuilder).to receive(:build).with(organization, only_path: nil).and_return(web_url) + expect(organization.web_url).to eq(web_url) + end + end end diff --git a/spec/models/pages/lookup_path_spec.rb b/spec/models/pages/lookup_path_spec.rb index 08ba823f8fa..570c369016b 100644 --- a/spec/models/pages/lookup_path_spec.rb +++ b/spec/models/pages/lookup_path_spec.rb @@ -55,12 +55,7 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end context 'when there is pages deployment' do - let(:deployment) { create(:pages_deployment, project: project) } - - before do - project.mark_pages_as_deployed - project.pages_metadatum.update!(pages_deployment: deployment) - end + let!(:deployment) { create(:pages_deployment, project: project) } it 'uses deployment from object storage' do freeze_time do @@ -75,12 +70,6 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end end - it 'does not recreate source hash' do - expect(deployment.file).to receive(:url_or_file_path).once - - 2.times { lookup_path.source } - end - context 'when deployment is in the local storage' do before do deployment.file.migrate!(::ObjectStorage::Store::LOCAL) @@ -159,12 +148,7 @@ RSpec.describe Pages::LookupPath, feature_category: :pages do end context 'when there is a deployment' do - let(:deployment) { create(:pages_deployment, project: project, root_directory: 'foo') } - - before do - project.mark_pages_as_deployed - project.pages_metadatum.update!(pages_deployment: deployment) - end + let!(:deployment) { create(:pages_deployment, project: project, root_directory: 'foo') } it 'returns the deployment\'s root_directory' do expect(lookup_path.root_directory).to eq('foo') diff --git a/spec/models/pages_deployment_spec.rb b/spec/models/pages_deployment_spec.rb index e74c7ee8612..1e6f8b33a86 100644 --- a/spec/models/pages_deployment_spec.rb +++ b/spec/models/pages_deployment_spec.rb @@ -71,54 +71,62 @@ RSpec.describe PagesDeployment, feature_category: :pages do end end - describe '.deactivate_deployments_older_than', :freeze_time do - let!(:other_project_deployment) do - create(:pages_deployment) - end + describe '.deactivate_all', :freeze_time do + let!(:deployment) { create(:pages_deployment, project: project, updated_at: 5.minutes.ago) } + let!(:nil_path_prefix_deployment) { create(:pages_deployment, project: project, path_prefix: nil) } + let!(:empty_path_prefix_deployment) { create(:pages_deployment, project: project, path_prefix: '') } - let!(:other_path_prefix_deployment) do - create(:pages_deployment, project: project, path_prefix: 'other') - end + let!(:other_project_deployment) { create(:pages_deployment) } + let!(:deactivated_deployment) { create(:pages_deployment, project: project, deleted_at: 5.minutes.ago) } - let!(:deactivated_deployment) do - create(:pages_deployment, project: project, deleted_at: 5.minutes.ago) + it 'updates only older deployments for the same project and path prefix' do + expect { described_class.deactivate_all(project) } + .to change { deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and change { deployment.reload.updated_at }.to(Time.zone.now) + .and change { nil_path_prefix_deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and change { empty_path_prefix_deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and not_change { other_project_deployment.reload.deleted_at } + .and not_change { deactivated_deployment.reload.deleted_at } end + end + + describe '.deactivate_deployments_older_than', :freeze_time do + let!(:nil_path_prefix_deployment) { create(:pages_deployment, project: project, path_prefix: nil) } + let!(:empty_path_prefix_deployment) { create(:pages_deployment, project: project, path_prefix: '') } + let!(:older_deployment) { create(:pages_deployment, project: project, updated_at: 5.minutes.ago) } + let!(:reference_deployment) { create(:pages_deployment, project: project, updated_at: 5.minutes.ago) } + let!(:newer_deployment) { create(:pages_deployment, project: project, updated_at: 5.minutes.ago) } + + let!(:other_project_deployment) { create(:pages_deployment) } + let!(:other_path_prefix_deployment) { create(:pages_deployment, project: project, path_prefix: 'other') } + let!(:deactivated_deployment) { create(:pages_deployment, project: project, deleted_at: 5.minutes.ago) } it 'updates only older deployments for the same project and path prefix' do - deployment1 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) - deployment2 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) - deployment3 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) - - expect { described_class.deactivate_deployments_older_than(deployment2) } - .to change { deployment1.reload.deleted_at } - .from(nil).to(Time.zone.now) - .and change { deployment1.reload.updated_at } - .to(Time.zone.now) - - expect(deployment2.reload.deleted_at).to be_nil - expect(deployment3.reload.deleted_at).to be_nil - expect(other_project_deployment.deleted_at).to be_nil - expect(other_path_prefix_deployment.reload.deleted_at).to be_nil - expect(deactivated_deployment.reload.deleted_at).to eq(5.minutes.ago) + expect { described_class.deactivate_deployments_older_than(reference_deployment) } + .to change { older_deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and change { older_deployment.reload.updated_at }.to(Time.zone.now) + .and change { nil_path_prefix_deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and change { empty_path_prefix_deployment.reload.deleted_at }.from(nil).to(Time.zone.now) + .and not_change { reference_deployment.reload.deleted_at } + .and not_change { newer_deployment.reload.deleted_at } + .and not_change { other_project_deployment.reload.deleted_at } + .and not_change { other_path_prefix_deployment.reload.deleted_at } + .and not_change { deactivated_deployment.reload.deleted_at } end it 'updates only older deployments for the same project with the given time' do - deployment1 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) - deployment2 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) - deployment3 = create(:pages_deployment, project: project, updated_at: 5.minutes.ago) time = 30.minutes.from_now - expect { described_class.deactivate_deployments_older_than(deployment2, time: time) } - .to change { deployment1.reload.deleted_at } - .from(nil).to(time) - .and change { deployment1.reload.updated_at } - .to(Time.zone.now) - - expect(deployment2.reload.deleted_at).to be_nil - expect(deployment3.reload.deleted_at).to be_nil - expect(other_project_deployment.deleted_at).to be_nil - expect(other_path_prefix_deployment.reload.deleted_at).to be_nil - expect(deactivated_deployment.reload.deleted_at).to eq(5.minutes.ago) + expect { described_class.deactivate_deployments_older_than(reference_deployment, time: time) } + .to change { older_deployment.reload.deleted_at }.from(nil).to(time) + .and change { older_deployment.reload.updated_at }.to(Time.zone.now) + .and change { nil_path_prefix_deployment.reload.deleted_at }.from(nil).to(time) + .and change { empty_path_prefix_deployment.reload.deleted_at }.from(nil).to(time) + .and not_change { reference_deployment.reload.deleted_at } + .and not_change { newer_deployment.reload.deleted_at } + .and not_change { other_project_deployment.reload.deleted_at } + .and not_change { other_path_prefix_deployment.reload.deleted_at } + .and not_change { deactivated_deployment.reload.deleted_at } end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 096edb40511..3ea5f6ea0ae 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2820,7 +2820,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr context "will return false if pages is deployed even if onboarding_complete is false" do before do project.pages_metadatum.update_column(:onboarding_complete, false) - project.pages_metadatum.update_column(:deployed, true) + create(:pages_deployment, project: project) end it { is_expected.to be_falsey } @@ -2834,7 +2834,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr context 'if pages are deployed' do before do - project.pages_metadatum.update_column(:deployed, true) + create(:pages_deployment, project: project) end it { is_expected.to be_truthy } @@ -7198,126 +7198,6 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr end end - describe '#mark_pages_as_deployed' do - let(:project) { create(:project) } - - it "works when artifacts_archive is missing" do - project.mark_pages_as_deployed - - expect(project.pages_metadatum.reload.deployed).to eq(true) - end - - it "creates new record and sets deployed to true if none exists yet" do - project.pages_metadatum.destroy! - project.reload - - project.mark_pages_as_deployed - - expect(project.pages_metadatum.reload.deployed).to eq(true) - end - - it "updates the existing record and sets deployed to true and records artifact archive" do - pages_metadatum = project.pages_metadatum - pages_metadatum.update!(deployed: false) - - expect do - project.mark_pages_as_deployed - end.to change { pages_metadatum.reload.deployed }.from(false).to(true) - end - end - - describe '#mark_pages_as_not_deployed' do - let(:project) { create(:project) } - - it "creates new record and sets deployed to false if none exists yet" do - project.pages_metadatum.destroy! - project.reload - - project.mark_pages_as_not_deployed - - expect(project.pages_metadatum.reload.deployed).to eq(false) - end - - it "updates the existing record and sets deployed to false and clears artifacts_archive" do - pages_metadatum = project.pages_metadatum - pages_metadatum.update!(deployed: true) - - expect do - project.mark_pages_as_not_deployed - end.to change { pages_metadatum.reload.deployed }.from(true).to(false) - end - end - - describe '#update_pages_deployment!' do - let(:project) { create(:project) } - let(:deployment) { create(:pages_deployment, project: project) } - - it "creates new metadata record if none exists yet and sets deployment" do - project.pages_metadatum.destroy! - project.reload - - project.update_pages_deployment!(deployment) - - expect(project.pages_metadatum.pages_deployment).to eq(deployment) - end - - it "updates the existing metadara record with deployment" do - expect do - project.update_pages_deployment!(deployment) - end.to change { project.pages_metadatum.reload.pages_deployment }.from(nil).to(deployment) - end - end - - describe '#set_first_pages_deployment!' do - let(:project) { create(:project) } - let(:deployment) { create(:pages_deployment, project: project) } - - it "creates new metadata record if none exists yet and sets deployment" do - project.pages_metadatum.destroy! - project.reload - - project.set_first_pages_deployment!(deployment) - - expect(project.pages_metadatum.reload.pages_deployment).to eq(deployment) - expect(project.pages_metadatum.reload.deployed).to eq(true) - end - - it "updates the existing metadara record with deployment" do - expect do - project.set_first_pages_deployment!(deployment) - end.to change { project.pages_metadatum.reload.pages_deployment }.from(nil).to(deployment) - - expect(project.pages_metadatum.reload.deployed).to eq(true) - end - - it 'only updates metadata for this project' do - other_project = create(:project) - - expect do - project.set_first_pages_deployment!(deployment) - end.not_to change { other_project.pages_metadatum.reload.pages_deployment }.from(nil) - - expect(other_project.pages_metadatum.reload.deployed).to eq(false) - end - - it 'does nothing if metadata already references some deployment' do - existing_deployment = create(:pages_deployment, project: project) - project.set_first_pages_deployment!(existing_deployment) - - expect do - project.set_first_pages_deployment!(deployment) - end.not_to change { project.pages_metadatum.reload.pages_deployment }.from(existing_deployment) - end - - it 'marks project as not deployed if deployment is nil' do - project.mark_pages_as_deployed - - expect do - project.set_first_pages_deployment!(nil) - end.to change { project.pages_metadatum.reload.deployed }.from(true).to(false) - end - end - describe '#has_pool_repository?' do it 'returns false when it does not have a pool repository' do subject = create(:project, :repository) @@ -7381,7 +7261,7 @@ RSpec.describe Project, factory_default: :keep, feature_category: :groups_and_pr it 'returns only projects that have pages deployed' do _project_without_pages = create(:project) project_with_pages = create(:project) - project_with_pages.mark_pages_as_deployed + create(:pages_deployment, project: project_with_pages) expect(described_class.with_pages_deployed).to contain_exactly(project_with_pages) end diff --git a/spec/models/users/phone_number_validation_spec.rb b/spec/models/users/phone_number_validation_spec.rb index 890109e99e5..e41719d8ca3 100644 --- a/spec/models/users/phone_number_validation_spec.rb +++ b/spec/models/users/phone_number_validation_spec.rb @@ -95,12 +95,6 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie end end end - - describe '#by_reference_id' do - it 'returns the correct phone number record for the given reference_id' do - expect(described_class.by_reference_id('target')).to eq phone_number_record_1 - end - end end describe '#validated?' do @@ -122,4 +116,20 @@ RSpec.describe Users::PhoneNumberValidation, feature_category: :instance_resilie end end end + + describe '.by_reference_id' do + let_it_be(:phone_number_record) { create(:phone_number_validation) } + + let(:ref_id) { phone_number_record.telesign_reference_xid } + + subject { described_class.by_reference_id(ref_id) } + + it { is_expected.to eq phone_number_record } + + context 'when there is no matching record' do + let(:ref_id) { 'does-not-exist' } + + it { is_expected.to be_nil } + end + end end diff --git a/spec/requests/api/internal/pages_spec.rb b/spec/requests/api/internal/pages_spec.rb index 1eeb3404157..7e2a778f433 100644 --- a/spec/requests/api/internal/pages_spec.rb +++ b/spec/requests/api/internal/pages_spec.rb @@ -46,17 +46,7 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do end end - context 'when authenticated' do - before do - project.update_pages_deployment!(create(:pages_deployment, project: project)) - end - - around do |example| - freeze_time do - example.run - end - end - + context 'when authenticated', :freeze_time do context 'when domain does not exist' do it 'responds with 204 no content' do get api('/internal/pages'), headers: auth_header, params: { host: 'any-domain.gitlab.io' } @@ -79,10 +69,6 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do end context 'when there are no pages deployed for the related project' do - before do - project.mark_pages_as_not_deployed - end - it 'responds with 204 No Content' do get api('/internal/pages'), headers: auth_header, params: { host: 'pages.io' } @@ -91,9 +77,7 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do end context 'when there are pages deployed for the related project' do - before do - project.mark_pages_as_deployed - end + let!(:deployment) { create(:pages_deployment, project: project) } it 'domain lookup is case insensitive' do get api('/internal/pages'), headers: auth_header, params: { host: 'Pages.IO' } @@ -110,7 +94,6 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do expect(json_response['certificate']).to eq(pages_domain.certificate) expect(json_response['key']).to eq(pages_domain.key) - deployment = project.pages_metadatum.pages_deployment expect(json_response['lookup_paths']).to eq( [ { @@ -144,10 +127,6 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do end context 'when there are no pages deployed for the related project' do - before do - project.mark_pages_as_not_deployed - end - it 'responds with 204 No Content' do get api('/internal/pages'), headers: auth_header, params: { host: 'unique-domain.example.com' } @@ -156,9 +135,7 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do end context 'when there are pages deployed for the related project' do - before do - project.mark_pages_as_deployed - end + let!(:deployment) { create(:pages_deployment, project: project) } context 'when the unique domain is disabled' do before do @@ -186,7 +163,6 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('internal/pages/virtual_domain') - deployment = project.pages_metadatum.pages_deployment expect(json_response['lookup_paths']).to eq( [ { @@ -218,10 +194,6 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do end context 'when there are no pages deployed for the related project' do - before do - project.mark_pages_as_not_deployed - end - it 'responds with 204 No Content' do get api('/internal/pages'), headers: auth_header, params: { host: "#{group.path}.gitlab-pages.io" } @@ -232,9 +204,7 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do end context 'when there are pages deployed for the related project' do - before do - project.mark_pages_as_deployed - end + let!(:deployment) { create(:pages_deployment, project: project) } context 'with a regular project' do it 'responds with the correct domain configuration' do @@ -243,7 +213,6 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('internal/pages/virtual_domain') - deployment = project.pages_metadatum.pages_deployment expect(json_response['lookup_paths']).to eq( [ { @@ -274,7 +243,7 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do 3.times do project = create(:project, group: group) - project.mark_pages_as_deployed + create(:pages_deployment, project: project) end expect { get api('/internal/pages'), headers: auth_header, params: { host: "#{group.path}.gitlab-pages.io" } } @@ -292,7 +261,6 @@ RSpec.describe API::Internal::Pages, feature_category: :pages do expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('internal/pages/virtual_domain') - deployment = project.pages_metadatum.pages_deployment expect(json_response['lookup_paths']).to eq( [ { diff --git a/spec/requests/api/pages/pages_spec.rb b/spec/requests/api/pages/pages_spec.rb index aa1869eaa84..74dd1ed4f2b 100644 --- a/spec/requests/api/pages/pages_spec.rb +++ b/spec/requests/api/pages/pages_spec.rb @@ -9,7 +9,6 @@ RSpec.describe API::Pages, feature_category: :pages do before do project.add_maintainer(user) - project.mark_pages_as_deployed end describe 'DELETE /projects/:id/pages' do @@ -17,7 +16,7 @@ RSpec.describe API::Pages, feature_category: :pages do it_behaves_like 'DELETE request permissions for admin mode' do before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + stub_pages_setting(enabled: true) end let(:succes_status_code) { :no_content } @@ -25,7 +24,7 @@ RSpec.describe API::Pages, feature_category: :pages do context 'when Pages is disabled' do before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(false) + stub_pages_setting(enabled: false) end it_behaves_like '404 response' do @@ -35,7 +34,7 @@ RSpec.describe API::Pages, feature_category: :pages do context 'when Pages is enabled' do before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + stub_pages_setting(enabled: true) end context 'when Pages are deployed' do @@ -48,15 +47,11 @@ RSpec.describe API::Pages, feature_category: :pages do it 'removes the pages' do delete api(path, admin, admin_mode: true) - expect(project.reload.pages_metadatum.deployed?).to be(false) + expect(project.reload.pages_deployed?).to be(false) end end context 'when pages are not deployed' do - before do - project.mark_pages_as_not_deployed - end - it 'returns 204' do delete api(path, admin, admin_mode: true) diff --git a/spec/services/packages/nuget/check_duplicates_service_spec.rb b/spec/services/packages/nuget/check_duplicates_service_spec.rb index 9675aa5f5e2..6274036800a 100644 --- a/spec/services/packages/nuget/check_duplicates_service_spec.rb +++ b/spec/services/packages/nuget/check_duplicates_service_spec.rb @@ -3,8 +3,6 @@ require 'spec_helper' RSpec.describe Packages::Nuget::CheckDuplicatesService, feature_category: :package_registry do - include PackagesManagerApiSpecHelpers - let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user) } let_it_be(:file_name) { 'package.nupkg' } @@ -12,7 +10,7 @@ RSpec.describe Packages::Nuget::CheckDuplicatesService, feature_category: :packa let(:params) do { file_name: file_name, - file: temp_file(file_name) + file: File.open(expand_fixture_path('packages/nuget/package.nupkg')) } end diff --git a/spec/services/packages/nuget/extract_metadata_file_service_spec.rb b/spec/services/packages/nuget/extract_metadata_file_service_spec.rb index 4c761826b53..ac5749c2dc4 100644 --- a/spec/services/packages/nuget/extract_metadata_file_service_spec.rb +++ b/spec/services/packages/nuget/extract_metadata_file_service_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Packages::Nuget::ExtractMetadataFileService, feature_category: :package_registry do + include PackagesManagerApiSpecHelpers + let_it_be(:package_file) { build(:package_file, :nuget) } let_it_be(:package_zip_file) { Zip::File.new(package_file.file) } @@ -38,6 +40,18 @@ RSpec.describe Packages::Nuget::ExtractMetadataFileService, feature_category: :p it 'returns the nuspec file content' do expect(subject.payload.squish).to include(expected_metadata) end + + context 'with InputStream zip' do + let(:package_zip_file) do + Zip::InputStream.open( + temp_file('package.nupkg', content: File.open(package_file.file.path)) + ) + end + + it 'returns the nuspec file content' do + expect(subject.payload.squish).to include(expected_metadata) + end + end end context 'without the nuspec file' do diff --git a/spec/services/packages/nuget/metadata_extraction_service_spec.rb b/spec/services/packages/nuget/metadata_extraction_service_spec.rb index 81a4e4a430b..46d5449d52b 100644 --- a/spec/services/packages/nuget/metadata_extraction_service_spec.rb +++ b/spec/services/packages/nuget/metadata_extraction_service_spec.rb @@ -4,7 +4,8 @@ require 'spec_helper' RSpec.describe Packages::Nuget::MetadataExtractionService, feature_category: :package_registry do let_it_be(:package_file) { build(:package_file, :nuget) } - let(:service) { described_class.new(package_file) } + let(:package_zip_file) { Zip::File.new(package_file.file) } + let(:service) { described_class.new(package_zip_file) } describe '#execute' do subject { service.execute } @@ -50,8 +51,8 @@ RSpec.describe Packages::Nuget::MetadataExtractionService, feature_category: :pa end it 'calls the necessary services and executes the metadata extraction' do - expect_next_instance_of(Packages::Nuget::ProcessPackageFileService, package_file) do |service| - expect(service).to receive(:execute).and_return(ServiceResponse.success(payload: { nuspec_file_content: nuspec_file_content })) + expect_next_instance_of(Packages::Nuget::ExtractMetadataFileService, package_zip_file) do |service| + expect(service).to receive(:execute).and_return(ServiceResponse.success(payload: nuspec_file_content)) end expect_next_instance_of(Packages::Nuget::ExtractMetadataContentService, nuspec_file_content) do |service| diff --git a/spec/services/packages/nuget/process_package_file_service_spec.rb b/spec/services/packages/nuget/process_package_file_service_spec.rb index cdeb5b32737..70e8bcb8c5c 100644 --- a/spec/services/packages/nuget/process_package_file_service_spec.rb +++ b/spec/services/packages/nuget/process_package_file_service_spec.rb @@ -14,25 +14,14 @@ RSpec.describe Packages::Nuget::ProcessPackageFileService, feature_category: :pa it { expect { subject }.to raise_error(described_class::ExtractionError, error_message) } end - shared_examples 'not creating a symbol file' do - it 'does not call the CreateSymbolFilesService' do - expect(Packages::Nuget::Symbols::CreateSymbolFilesService).not_to receive(:new) - - expect(subject).to be_success - end - end - context 'with valid package file' do - it 'calls the ExtractMetadataFileService' do - expect_next_instance_of(Packages::Nuget::ExtractMetadataFileService, instance_of(Zip::File)) do |service| - expect(service).to receive(:execute) do - instance_double(ServiceResponse).tap do |response| - expect(response).to receive(:payload).and_return(instance_of(String)) - end - end + it 'calls the UpdatePackageFromMetadataService' do + expect_next_instance_of(Packages::Nuget::UpdatePackageFromMetadataService, package_file, + instance_of(Zip::File)) do |service| + expect(service).to receive(:execute) end - expect(subject).to be_success + subject end end @@ -59,25 +48,5 @@ RSpec.describe Packages::Nuget::ProcessPackageFileService, feature_category: :pa it_behaves_like 'raises an error', 'invalid package file' end - - context 'with a symbol package file' do - let(:package_file) { build(:package_file, :snupkg) } - - it 'calls the CreateSymbolFilesService' do - expect_next_instance_of( - Packages::Nuget::Symbols::CreateSymbolFilesService, package_file.package, instance_of(Zip::File) - ) do |service| - expect(service).to receive(:execute) - end - - expect(subject).to be_success - end - end - - context 'with a non symbol package file' do - let(:package_file) { build(:package_file, :nuget) } - - it_behaves_like 'not creating a symbol file' - end end end diff --git a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb index c199afcf2ca..0e19f2ac3f9 100644 --- a/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb +++ b/spec/services/packages/nuget/update_package_from_metadata_service_spec.rb @@ -7,7 +7,8 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_ let!(:package) { create(:nuget_package, :processing, :with_symbol_package, :with_build) } let(:package_file) { package.package_files.first } - let(:service) { described_class.new(package_file) } + let(:package_zip_file) { Zip::File.new(package_file.file) } + let(:service) { described_class.new(package_file, package_zip_file) } let(:package_name) { 'DummyProject.DummyPackage' } let(:package_version) { '1.0.0' } let(:package_file_name) { 'dummyproject.dummypackage.1.0.0.nupkg' } @@ -247,11 +248,20 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_ context 'with existing package' do let!(:existing_package) { create(:nuget_package, project: package.project, name: package_name, version: package_version) } let(:package_id) { existing_package.id } + let(:package_zip_file) do + Zip::File.open(package_file.file.path) do |zipfile| + zipfile.add('package.pdb', expand_fixture_path('packages/nuget/symbol/package.pdb')) + zipfile + end + end it 'link existing package and updates package file', :aggregate_failures do expect(service).to receive(:try_obtain_lease).and_call_original expect(::Packages::Nuget::SyncMetadatumService).not_to receive(:new) expect(::Packages::UpdateTagsService).not_to receive(:new) + expect_next_instance_of(Packages::Nuget::Symbols::CreateSymbolFilesService, existing_package, package_zip_file) do |service| + expect(service).to receive(:execute).and_call_original + end expect { subject } .to change { ::Packages::Package.count }.by(-1) @@ -259,20 +269,11 @@ RSpec.describe Packages::Nuget::UpdatePackageFromMetadataService, :clean_gitlab_ .and change { Packages::DependencyLink.count }.by(0) .and change { Packages::Nuget::DependencyLinkMetadatum.count }.by(0) .and change { ::Packages::Nuget::Metadatum.count }.by(0) + .and change { existing_package.nuget_symbols.count }.by(1) expect(package_file.reload.file_name).to eq(package_file_name) expect(package_file.package).to eq(existing_package) end - context 'with packages_nuget_symbols records' do - before do - create_list(:nuget_symbol, 2, package: package) - end - - it 'links the symbol records to the existing package' do - expect { subject }.to change { existing_package.nuget_symbols.count }.by(2) - end - end - it_behaves_like 'taking the lease' it_behaves_like 'not updating the package if the lease is taken' diff --git a/spec/services/pages/delete_service_spec.rb b/spec/services/pages/delete_service_spec.rb index 488f29f6b7e..86b1bd5bde2 100644 --- a/spec/services/pages/delete_service_spec.rb +++ b/spec/services/pages/delete_service_spec.rb @@ -8,14 +8,12 @@ RSpec.describe Pages::DeleteService, feature_category: :pages do let(:project) { create(:project, path: "my.project") } let(:service) { described_class.new(project, admin) } - before do - project.mark_pages_as_deployed - end - it 'marks pages as not deployed' do - expect do - service.execute - end.to change { project.reload.pages_deployed? }.from(true).to(false) + create(:pages_deployment, project: project) + + expect { service.execute } + .to change { project.reload.pages_deployed? } + .from(true).to(false) end it 'deletes all domains' do @@ -29,9 +27,8 @@ RSpec.describe Pages::DeleteService, feature_category: :pages do end it 'schedules a destruction of pages deployments' do - expect(DestroyPagesDeploymentsWorker).to( - receive(:perform_async).with(project.id) - ) + expect(DestroyPagesDeploymentsWorker) + .to(receive(:perform_async).with(project.id)) service.execute end @@ -39,9 +36,8 @@ RSpec.describe Pages::DeleteService, feature_category: :pages do it 'removes pages deployments', :sidekiq_inline do create(:pages_deployment, project: project) - expect do - service.execute - end.to change { PagesDeployment.count }.by(-1) + expect { service.execute } + .to change { PagesDeployment.count }.by(-1) end it 'publishes a ProjectDeleted event with project id and namespace id' do diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index 0ad7693a047..b5d1276988f 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -48,7 +48,7 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do deploy_status = GenericCommitStatus.last expect(deploy_status.description).not_to be_present - expect(project.pages_metadatum).to be_deployed + expect(project.pages_deployed?).to eq(true) end it_behaves_like 'old deployments' @@ -116,15 +116,14 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do it "doesn't delete artifacts after deploying" do expect(service.execute[:status]).to eq(:success) - expect(project.pages_metadatum).to be_deployed + expect(project.pages_deployed?).to eq(true) expect(build.artifacts?).to eq(true) end it 'succeeds' do - expect(project.pages_deployed?).to be_falsey - expect(service.execute[:status]).to eq(:success) - expect(project.pages_metadatum).to be_deployed - expect(project.pages_deployed?).to be_truthy + expect { expect(service.execute[:status]).to eq(:success) } + .to change { project.pages_deployed? } + .from(false).to(true) end it 'publishes a PageDeployedEvent event with project id and namespace id' do @@ -137,10 +136,10 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do expect { service.execute }.to publish_event(Pages::PageDeployedEvent).with(expected_data) end - it 'creates pages_deployment and saves it in the metadata' do - expect do - expect(service.execute[:status]).to eq(:success) - end.to change { project.pages_deployments.count }.by(1) + it 'creates pages_deployment' do + expect { expect(service.execute[:status]).to eq(:success) } + .to change { project.pages_deployments.count } + .by(1) deployment = project.pages_deployments.last @@ -148,7 +147,6 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do expect(deployment.file).to be_present expect(deployment.file_count).to eq(3) expect(deployment.file_sha256).to eq(artifacts_archive.file_sha256) - expect(project.pages_metadatum.reload.pages_deployment_id).to eq(deployment.id) expect(deployment.ci_build_id).to eq(build.id) expect(deployment.root_directory).to be_nil end @@ -157,11 +155,9 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do project.pages_metadatum.destroy! project.reload - expect do - expect(service.execute[:status]).to eq(:success) - end.to change { project.pages_deployments.count }.by(1) - - expect(project.pages_metadatum.reload.pages_deployment).to eq(project.pages_deployments.last) + expect { expect(service.execute[:status]).to eq(:success) } + .to change { project.pages_deployments.count } + .by(1) end context 'when archive does not have pages directory' do @@ -171,7 +167,10 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do it 'returns an error' do expect(service.execute[:status]).not_to eq(:success) - expect(GenericCommitStatus.last.description).to eq("Error: You need to either include a `public/` folder in your artifacts, or specify which one to use for Pages using `publish` in `.gitlab-ci.yml`") + expect(GenericCommitStatus.last.description) + .to eq( + "Error: You need to either include a `public/` folder in your artifacts, " \ + "or specify which one to use for Pages using `publish` in `.gitlab-ci.yml`") end end @@ -196,7 +195,10 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do it 'returns an error' do expect(service.execute[:status]).not_to eq(:success) - expect(GenericCommitStatus.last.description).to eq("Error: You need to either include a `public/` folder in your artifacts, or specify which one to use for Pages using `publish` in `.gitlab-ci.yml`") + expect(GenericCommitStatus.last.description) + .to eq( + "Error: You need to either include a `public/` folder in your artifacts, " \ + "or specify which one to use for Pages using `publish` in `.gitlab-ci.yml`") end end @@ -208,7 +210,10 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do it 'returns an error' do expect(service.execute[:status]).not_to eq(:success) - expect(GenericCommitStatus.last.description).to eq("Error: You need to either include a `public/` folder in your artifacts, or specify which one to use for Pages using `publish` in `.gitlab-ci.yml`") + expect(GenericCommitStatus.last.description) + .to eq( + "Error: You need to either include a `public/` folder in your artifacts, " \ + "or specify which one to use for Pages using `publish` in `.gitlab-ci.yml`") end end end @@ -223,7 +228,8 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do expect(service.execute[:status]).not_to eq(:success) - expect(GenericCommitStatus.last.description).to eq("pages site contains 3 file entries, while limit is set to 2") + expect(GenericCommitStatus.last.description) + .to eq("pages site contains 3 file entries, while limit is set to 2") end context 'when timeout happens by DNS error' do @@ -240,13 +246,13 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do deploy_status = GenericCommitStatus.last expect(deploy_status).to be_failed - expect(project.pages_metadatum).not_to be_deployed + expect(project.pages_deployed?).to eq(false) end end context 'when missing artifacts metadata' do before do - expect(build).to receive(:artifacts_metadata?).and_return(false) + allow(build).to receive(:artifacts_metadata?).and_return(false) end it 'does not raise an error as failed job' do @@ -256,7 +262,7 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do deploy_status = GenericCommitStatus.last expect(deploy_status).to be_failed - expect(project.pages_metadatum).not_to be_deployed + expect(project.pages_deployed?).to eq(false) end end @@ -275,10 +281,9 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do end end - it 'creates a new pages deployment and mark it as deployed' do - expect do - expect(service.execute[:status]).to eq(:success) - end.to change { project.pages_deployments.count }.by(1) + it 'creates a new pages deployment' do + expect { expect(service.execute[:status]).to eq(:success) } + .to change { project.pages_deployments.count }.by(1) deployment = project.pages_deployments.last expect(deployment.ci_build_id).to eq(build.id) @@ -287,16 +292,12 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do it_behaves_like 'old deployments' context 'when newer deployment present' do - before do + it 'fails with outdated reference message' do new_pipeline = create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) new_build = create(:ci_build, name: 'pages', pipeline: new_pipeline, ref: 'HEAD') - new_deployment = create(:pages_deployment, ci_build: new_build, project: project) - project.update_pages_deployment!(new_deployment) - end + create(:pages_deployment, project: project, ci_build: new_build) - it 'fails with outdated reference message' do expect(service.execute[:status]).to eq(:error) - expect(project.reload.pages_metadatum).not_to be_deployed deploy_status = GenericCommitStatus.last expect(deploy_status).to be_failed @@ -308,16 +309,14 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do it 'fails when uploaded deployment size is wrong' do allow_next_instance_of(PagesDeployment) do |deployment| allow(deployment) - .to receive(:size) - .and_return(file.size + 1) + .to receive(:file) + .and_return(instance_double(Pages::DeploymentUploader, size: file.size + 1)) end expect(service.execute[:status]).not_to eq(:success) - expect(GenericCommitStatus.last.description).to eq('The uploaded artifact size does not match the expected value') - project.pages_metadatum.reload - expect(project.pages_metadatum).not_to be_deployed - expect(project.pages_metadatum.pages_deployment).to be_nil + expect(GenericCommitStatus.last.description) + .to eq('The uploaded artifact size does not match the expected value') end end end @@ -335,9 +334,8 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do end it 'fails with exception raised' do - expect do - service.execute - end.to raise_error("Validation failed: File sha256 can't be blank") + expect { service.execute } + .to raise_error("Validation failed: File sha256 can't be blank") end end diff --git a/spec/support/database/click_house/hooks.rb b/spec/support/database/click_house/hooks.rb index c13778f9c36..77b33b7aaa3 100644 --- a/spec/support/database/click_house/hooks.rb +++ b/spec/support/database/click_house/hooks.rb @@ -2,6 +2,8 @@ # rubocop: disable Gitlab/NamespacedClass class ClickHouseTestRunner + include ClickHouseTestHelpers + def truncate_tables ClickHouse::Client.configuration.databases.each_key do |db| # Select tables with at least one row @@ -18,17 +20,6 @@ class ClickHouseTestRunner end end - def clear_db(configuration = ClickHouse::Client.configuration) - configuration.databases.each_key do |db| - # drop all tables - lookup_tables(db, configuration).each do |table| - ClickHouse::Client.execute("DROP TABLE IF EXISTS #{table}", db, configuration) - end - - ClickHouse::MigrationSupport::SchemaMigration.create_table(db, configuration) - end - end - def ensure_schema return if @ensure_schema @@ -38,7 +29,7 @@ class ClickHouseTestRunner migrations_paths = ClickHouse::MigrationSupport::Migrator.migrations_paths schema_migration = ClickHouse::MigrationSupport::SchemaMigration migration_context = ClickHouse::MigrationSupport::MigrationContext.new(migrations_paths, schema_migration) - migration_context.up + migrate(nil, migration_context) @ensure_schema = true end @@ -49,10 +40,6 @@ class ClickHouseTestRunner @tables ||= {} @tables[db] ||= lookup_tables(db) - [ClickHouse::MigrationSupport::SchemaMigration.table_name] end - - def lookup_tables(db, configuration = ClickHouse::Client.configuration) - ClickHouse::Client.select('SHOW TABLES', db, configuration).pluck('name') - end end # rubocop: enable Gitlab/NamespacedClass @@ -61,9 +48,6 @@ RSpec.configure do |config| config.around(:each, :click_house) do |example| with_net_connect_allowed do - was_verbose = ClickHouse::Migration.verbose - ClickHouse::Migration.verbose = false - if example.example.metadata[:click_house] == :without_migrations click_house_test_runner.clear_db else @@ -72,8 +56,6 @@ RSpec.configure do |config| end example.run - ensure - ClickHouse::Migration.verbose = was_verbose end end end diff --git a/spec/support/helpers/click_house_helpers.rb b/spec/support/helpers/click_house_test_helpers.rb index 93d5b1d6e01..24f81a3ec01 100644 --- a/spec/support/helpers/click_house_helpers.rb +++ b/spec/support/helpers/click_house_test_helpers.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true -module ClickHouseHelpers - private - +module ClickHouseTestHelpers def migrate(target_version, migration_context) quietly { migration_context.up(target_version) } end @@ -36,8 +34,15 @@ module ClickHouseHelpers .map(&:symbolize_keys) end - def clear_db(configuration: ClickHouse::Client.configuration) - ClickHouseTestRunner.new.clear_db(configuration) + def clear_db(configuration = ClickHouse::Client.configuration) + configuration.databases.each_key do |db| + # drop all tables + lookup_tables(db, configuration).each do |table| + ClickHouse::Client.execute("DROP TABLE IF EXISTS #{table}", db, configuration) + end + + ClickHouse::MigrationSupport::SchemaMigration.create_table(db, configuration) + end end def register_database(config, database_identifier, db_config) @@ -51,16 +56,10 @@ module ClickHouseHelpers ) end - def clear_consts(fixtures_path) - $LOADED_FEATURES.select { |file| file.include? fixtures_path }.each do |file| - const = File.basename(file) - .scan(ClickHouse::Migration::MIGRATION_FILENAME_REGEXP)[0][1] - .camelcase - .safe_constantize + private - Object.send(:remove_const, const.to_s) if const - $LOADED_FEATURES.delete(file) - end + def lookup_tables(db, configuration = ClickHouse::Client.configuration) + ClickHouse::Client.select('SHOW TABLES', db, configuration).pluck('name') end def quietly(&_block) @@ -71,4 +70,16 @@ module ClickHouseHelpers ensure ClickHouse::Migration.verbose = was_verbose end + + def clear_consts(fixtures_path) + $LOADED_FEATURES.select { |file| file.include? fixtures_path }.each do |file| + const = File.basename(file) + .scan(ClickHouse::Migration::MIGRATION_FILENAME_REGEXP)[0][1] + .camelcase + .safe_constantize + + Object.send(:remove_const, const.to_s) if const + $LOADED_FEATURES.delete(file) + end + end end diff --git a/spec/support/helpers/packages_manager_api_spec_helpers.rb b/spec/support/helpers/packages_manager_api_spec_helpers.rb index 1c9fce183e9..c81c9b5982e 100644 --- a/spec/support/helpers/packages_manager_api_spec_helpers.rb +++ b/spec/support/helpers/packages_manager_api_spec_helpers.rb @@ -22,12 +22,12 @@ module PackagesManagerApiSpecHelpers end end - def temp_file(package_tmp) + def temp_file(package_tmp, content: nil) upload_path = ::Packages::PackageFileUploader.workhorse_local_upload_path file_path = "#{upload_path}/#{package_tmp}" FileUtils.mkdir_p(upload_path) - File.write(file_path, 'test') + content ? FileUtils.cp(content, file_path) : File.write(file_path, 'test') UploadedFile.new(file_path, filename: File.basename(file_path)) end diff --git a/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb index a3f9da082f7..c23d514abfc 100644 --- a/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb @@ -699,6 +699,7 @@ RSpec.shared_examples 'nuget upload endpoint' do |symbol_package: false| end context 'when package duplicates are not allowed' do + let(:params) { { package: temp_file(file_name, content: File.open(expand_fixture_path('packages/nuget/package.nupkg'))) } } let(:headers) { basic_auth_header(deploy_token.username, deploy_token.token).merge(workhorse_headers) } let_it_be(:existing_package) { create(:nuget_package, project: project) } let_it_be(:metadata) { { package_name: existing_package.name, package_version: existing_package.version } } diff --git a/spec/tasks/gitlab/click_house/migration_rake_spec.rb b/spec/tasks/gitlab/click_house/migration_rake_spec.rb index 6b834d52e9a..75a1c1a1856 100644 --- a/spec/tasks/gitlab/click_house/migration_rake_spec.rb +++ b/spec/tasks/gitlab/click_house/migration_rake_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe 'gitlab:clickhouse', click_house: :without_migrations, feature_category: :database do - include ClickHouseHelpers + include ClickHouseTestHelpers # We don't need to delete data since we don't modify Postgres data self.use_transactional_tests = false @@ -11,13 +11,14 @@ RSpec.describe 'gitlab:clickhouse', click_house: :without_migrations, feature_ca let(:migrations_base_dir) { 'click_house/migrations' } let(:migrations_dirname) { '' } let(:migrations_dir) { expand_fixture_path("#{migrations_base_dir}/#{migrations_dirname}") } + let(:verbose) { nil } before(:all) do Rake.application.rake_require 'tasks/gitlab/click_house/migration' end before do - stub_env('VERBOSE', 'false') + stub_env('VERBOSE', verbose) if verbose end describe 'migrate' do @@ -42,12 +43,26 @@ RSpec.describe 'gitlab:clickhouse', click_house: :without_migrations, feature_ca it 'creates a table' do expect { migration }.to change { active_schema_migrations_count }.from(0).to(1) + .and output.to_stdout expect(describe_table('some')).to match({ id: a_hash_including(type: 'UInt64'), date: a_hash_including(type: 'Date') }) end + + context 'when VERBOSE is false' do + let(:verbose) { 'false' } + + it 'does not write to stdout' do + expect { migration }.not_to output.to_stdout + + expect(describe_table('some')).to match({ + id: a_hash_including(type: 'UInt64'), + date: a_hash_including(type: 'Date') + }) + end + end end describe 'when dropping a table' do |