diff options
25 files changed, 274 insertions, 110 deletions
diff --git a/app/assets/javascripts/gfm_auto_complete.js b/app/assets/javascripts/gfm_auto_complete.js index 14efc2acbac..36c586ddfd2 100644 --- a/app/assets/javascripts/gfm_auto_complete.js +++ b/app/assets/javascripts/gfm_auto_complete.js @@ -109,8 +109,10 @@ class GfmAutoComplete { tpl += ' <small class="params"><%- params.join(" ") %></small>'; } if (value.warning && value.icon && value.icon === 'confidential') { - tpl += - '<small class="description"><em><i class="fa fa-eye-slash" aria-hidden="true"/><%- warning %></em></small>'; + tpl += `<small class="description gl-display-flex gl-align-items-center">${spriteIcon( + 'eye-slash', + 's16 gl-mr-2', + )}<em><%- warning %></em></small>`; } else if (value.warning) { tpl += '<small class="description"><em><%- warning %></em></small>'; } else if (value.description !== '') { diff --git a/app/assets/javascripts/integrations/edit/components/jira_issues_fields.vue b/app/assets/javascripts/integrations/edit/components/jira_issues_fields.vue index 885b067c334..71ce9fe104a 100644 --- a/app/assets/javascripts/integrations/edit/components/jira_issues_fields.vue +++ b/app/assets/javascripts/integrations/edit/components/jira_issues_fields.vue @@ -119,10 +119,12 @@ export default { <template v-if="showJiraIssuesIntegration"> <gl-form-group :label="s__('JiraService|Jira project key')" + label-for="service_project_key" :invalid-feedback="__('This field is required.')" :state="validProjectKey" > <gl-form-input + id="service_project_key" v-model="projectKey" name="service[project_key]" :placeholder="s__('JiraService|e.g. AB')" diff --git a/app/assets/javascripts/issuables_list/components/issuable.vue b/app/assets/javascripts/issuables_list/components/issuable.vue index 18ff91e0c1f..ed89cfea741 100644 --- a/app/assets/javascripts/issuables_list/components/issuable.vue +++ b/app/assets/javascripts/issuables_list/components/issuable.vue @@ -230,13 +230,15 @@ export default { <div class="flex-grow-1"> <div class="title"> <span class="issue-title-text"> - <i + <gl-icon v-if="issuable.confidential" v-gl-tooltip - class="fa fa-eye-slash" + name="eye-slash" + class="gl-vertical-align-text-bottom" + :size="16" :title="$options.confidentialTooltipText" :aria-label="$options.confidentialTooltipText" - ></i> + /> <gl-link :href="issuable.web_url" :target="isJiraIssue ? '_blank' : null" diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 244b97c7196..495706f621a 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -41,7 +41,7 @@ module IssuesHelper end def confidential_icon(issue) - icon('eye-slash') if issue.confidential? + sprite_icon('eye-slash', size: 16, css_class: 'gl-vertical-align-text-bottom') if issue.confidential? end def award_user_list(awards, current_user, limit: 10) diff --git a/app/models/snippet_input_action.rb b/app/models/snippet_input_action.rb index 52c77be1f45..cc6373264cc 100644 --- a/app/models/snippet_input_action.rb +++ b/app/models/snippet_input_action.rb @@ -15,7 +15,7 @@ class SnippetInputAction validates :action, inclusion: { in: ACTIONS, message: "%{value} is not a valid action" } validates :previous_path, presence: true, if: :move_action? - validates :file_path, presence: true + validates :file_path, presence: true, unless: :create_action? validates :content, presence: true, if: -> (action) { action.create_action? || action.update_action? } validate :ensure_same_file_path_and_previous_path, if: :update_action? validate :ensure_different_file_path_and_previous_path, if: :move_action? diff --git a/app/views/layouts/nav/sidebar/_admin.html.haml b/app/views/layouts/nav/sidebar/_admin.html.haml index f40b0d74a1a..e72535b8824 100644 --- a/app/views/layouts/nav/sidebar/_admin.html.haml +++ b/app/views/layouts/nav/sidebar/_admin.html.haml @@ -8,7 +8,7 @@ = _('Admin Area') %ul.sidebar-top-level-items{ data: { qa_selector: 'admin_sidebar_overview_submenu_content' } } = nav_link(controller: %w(dashboard admin admin/projects users groups jobs runners gitaly_servers), html_options: {class: 'home'}) do - = link_to admin_root_path, class: 'shortcuts-tree' do + = link_to admin_root_path do .nav-icon-container = sprite_icon('overview') %span.nav-item-name diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 5fc6b0a17b6..9e7b2c6ef77 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -362,7 +362,7 @@ - if project_nav_tab? :settings = nav_link(path: sidebar_settings_paths) do - = link_to edit_project_path(@project), class: 'shortcuts-tree' do + = link_to edit_project_path(@project) do .nav-icon-container = sprite_icon('settings') %span.nav-item-name.qa-settings-item#js-onboarding-settings-link diff --git a/changelogs/unreleased/225938-replace-fa-eyes-slash-icons-with-gitlab-svg-eye-slash-icon.yml b/changelogs/unreleased/225938-replace-fa-eyes-slash-icons-with-gitlab-svg-eye-slash-icon.yml new file mode 100644 index 00000000000..e9d768ee271 --- /dev/null +++ b/changelogs/unreleased/225938-replace-fa-eyes-slash-icons-with-gitlab-svg-eye-slash-icon.yml @@ -0,0 +1,5 @@ +--- +title: Replace fa-eyes-slash icons with GitLab SVG eye-slash icon +merge_request: 36602 +author: +type: performance diff --git a/changelogs/unreleased/fj-228825-remove-file-path-validation-in-snippet-create-action.yml b/changelogs/unreleased/fj-228825-remove-file-path-validation-in-snippet-create-action.yml new file mode 100644 index 00000000000..8db8de05139 --- /dev/null +++ b/changelogs/unreleased/fj-228825-remove-file-path-validation-in-snippet-create-action.yml @@ -0,0 +1,5 @@ +--- +title: Remove file_path validation in snippet create action +merge_request: 36809 +author: +type: changed diff --git a/changelogs/unreleased/parameterize-pg-deprecation-notice.yml b/changelogs/unreleased/parameterize-pg-deprecation-notice.yml new file mode 100644 index 00000000000..80fa8bf0052 --- /dev/null +++ b/changelogs/unreleased/parameterize-pg-deprecation-notice.yml @@ -0,0 +1,5 @@ +--- +title: Parameterize PG deprecation notice +merge_request: 35271 +author: +type: changed diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 583e31bdf1e..62fd9110975 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -2831,7 +2831,12 @@ be available for download in the GitLab UI. Paths are relative to the project directory (`$CI_PROJECT_DIR`) and can't directly link outside it. Wildcards can be used that follow the [glob](https://en.wikipedia.org/wiki/Glob_(programming)) -patterns and [`filepath.Match`](https://golang.org/pkg/path/filepath/#Match). +patterns and: + +- In [GitLab Runner 13.0](https://gitlab.com/gitlab-org/gitlab-runner/-/issues/2620) and later, +[`doublestar.Glob`](https://pkg.go.dev/github.com/bmatcuk/doublestar@v1.2.2?tab=doc#Match). +- In GitLab Runner 12.10 and earlier, +[`filepath.Match`](https://pkg.go.dev/path/filepath/#Match). To restrict which jobs a specific job will fetch artifacts from, see [dependencies](#dependencies). diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md index e7be67b8da5..02387c15951 100644 --- a/doc/development/fe_guide/vuex.md +++ b/doc/development/fe_guide/vuex.md @@ -201,6 +201,72 @@ By following this pattern we guarantee: 1. All data in the application follows the same lifecycle pattern 1. Unit tests are easier +#### Updating complex state + +Sometimes, especially when the state is complex, is really hard to traverse the state to precisely update what the mutation needs to update. +Ideally a `vuex` state should be as normalized/decoupled as possible but this is not always the case. + +It's important to remember that the code is much easier to read and maintain when the `portion of the mutated state` is selected and mutated in the mutation itself. + +Given this state: + +```javascript + export default () => ({ + items: [ + { + id: 1, + name: 'my_issue', + closed: false, + }, + { + id: 2, + name: 'another_issue', + closed: false, + } + ] +}); +``` + +It may be tempting to write a mutation like so: + +```javascript +// Bad +export default { + [types.MARK_AS_CLOSED](state, item) { + Object.assign(item, {closed: true}) + } +} +``` + +While this approach works it has several dependencies: + +- Correct selection of `item` in the component/action. +- The `item` property is already declared in the `closed` state. + - A new `confidential` property would not be reactive. +- Noting that `item` is referenced by `items` + +A mutation written like this is harder to maintain and more error prone. We should rather write a mutation like this: + +```javascript +// Good +export default { + [types.MARK_AS_CLOSED](state, itemId) { + const item = state.items.find(i => i.id == itemId); + Vue.set(item, 'closed', true) + + state.items.splice(index, 1, item) + } +} +``` + +This approach is better because: + +- It selects and updates the state in the mutation, which is more maintainable. +- It has no external dependencies, if the correct `itemId` is passed the state is correctly updated. +- It does not have reactivity caveats, as we generate a new `item` to avoid coupling to the initial state. + +A mutation written like this is easier to maintain. In addition, we avoid errors due to the limitation of the reactivity system. + ### `getters.js` Sometimes we may need to get derived state based on store state, like filtering for a specific prop. diff --git a/doc/development/testing_guide/end_to_end/environment_selection.md b/doc/development/testing_guide/end_to_end/environment_selection.md index d0e93eb95b6..698d061ced2 100644 --- a/doc/development/testing_guide/end_to_end/environment_selection.md +++ b/doc/development/testing_guide/end_to_end/environment_selection.md @@ -12,7 +12,7 @@ what environments to run tests against using the `only` metadata. | `domain` | Set the domain matcher | `String` | | `production` | Match against production | `Static` | -WARNING: **Be advised:** +CAUTION: **Caution:** You cannot specify `:production` and `{ <switch>: 'value' }` simultaneously. These options are mutually exclusive. If you want to specify production, you can control the `tld` and `domain` independently. diff --git a/doc/operations/feature_flags.md b/doc/operations/feature_flags.md index 116dc0b6b96..cb21039d187 100644 --- a/doc/operations/feature_flags.md +++ b/doc/operations/feature_flags.md @@ -61,7 +61,7 @@ for. The rollout strategy will have no effect if the environment spec is disable It can be set to: - All users -- [Percent rollout (logged in users)](#percent-rollout-logged-in-users) +- [Percent of users](#percent-of-users) - Optionally, you can click the **Include additional user IDs** checkbox and add a list of specific users IDs to enable the feature for. - [User IDs](#user-ids) @@ -82,9 +82,9 @@ for granular feature flag controls. GitLab Feature Flags can have multiple strat and the supported strategies are: - [All users](#all-users) -- [Percent rollout (logged in users)](#percent-rollout-logged-in-users) +- [Percent of Users](#percent-of-users) - [User IDs](#user-ids) -- [List](#list) +- [User List](#user-list) Strategies can be added to feature flags when [creating a feature flag](#create-a-feature-flag), or by editing an existing feature flag after creation by navigating to **Operations > Feature Flags** @@ -95,7 +95,7 @@ and clicking **{pencil}** (edit). Enables the feature for all users. It uses the [`default`](https://unleash.github.io/docs/activation_strategy#default) Unleash activation strategy. -### Percent rollout (logged in users) +### Percent of Users Enables the feature for a percentage of authenticated users. It uses the [`gradualRolloutUserId`](https://unleash.github.io/docs/activation_strategy#gradualrolloutuserid) @@ -130,7 +130,7 @@ CAUTION: **Caution:** The Unleash client **must** be given a user ID for the feature to be enabled for target users. See the [Ruby example](#ruby-application-example) below. -### List +### User List > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/35930) in GitLab 13.1. @@ -165,6 +165,7 @@ to disable a feature flag for a specific environment: 1. Navigate to your project's **Operations > Feature Flags**. 1. For the feature flag you want to disable, click the Pencil icon. 1. To disable the flag: + - In GitLab 13.0 and earlier: Slide the Status toggle for the environment. Or, to delete the environment spec, on the right, click the **Remove (X)** icon. - In GitLab 13.1 and later: For each strategy it applies to, under **Environments**, delete the environment. diff --git a/doc/user/gitlab_com/index.md b/doc/user/gitlab_com/index.md index ae16e176bab..bcaba97cab7 100644 --- a/doc/user/gitlab_com/index.md +++ b/doc/user/gitlab_com/index.md @@ -584,9 +584,9 @@ is used to forward logs to an [Elastic cluster](https://gitlab.com/gitlab-com/ru You can view more information in our runbooks such as: -- A [detailed list of what we're logging](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#what-are-we-logging) -- Our [current log retention policies](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#retention) -- A [diagram of our logging infrastructure](https://gitlab.com/gitlab-com/runbooks/tree/master/logging/doc#logging-infrastructure-overview) +- A [detailed list of what we're logging](https://gitlab.com/gitlab-com/runbooks/-/tree/master/docs/logging#what-are-we-logging) +- Our [current log retention policies](https://gitlab.com/gitlab-com/runbooks/-/tree/master/docs/logging#retention) +- A [diagram of our logging infrastructure](https://gitlab.com/gitlab-com/runbooks/-/tree/master/docs/logging#logging-infrastructure-overview) ## GitLab.com at scale diff --git a/lib/gitlab/config_checker/external_database_checker.rb b/lib/gitlab/config_checker/external_database_checker.rb index 795082a10a0..c08dd0351f3 100644 --- a/lib/gitlab/config_checker/external_database_checker.rb +++ b/lib/gitlab/config_checker/external_database_checker.rb @@ -5,22 +5,43 @@ module Gitlab module ExternalDatabaseChecker extend self - # DB is considered deprecated if it is below version 11 - def db_version_deprecated? - Gitlab::Database.version.to_f < 11 - end - def check - return [] unless db_version_deprecated? + notices = [] + + unless Gitlab::Database.postgresql_minimum_supported_version? + notices << + { + type: 'warning', + message: _('You are using PostgreSQL %{pg_version_current}, but PostgreSQL ' \ + '%{pg_version_minimum} is required for this version of GitLab. ' \ + 'Please upgrade your environment to a supported PostgreSQL version, ' \ + 'see %{pg_requirements_url} for details.') % { + pg_version_current: Gitlab::Database.version, + pg_version_minimum: Gitlab::Database::MINIMUM_POSTGRES_VERSION, + pg_requirements_url: '<a href="https://docs.gitlab.com/ee/install/requirements.html#database">database requirements</a>' + } + } + end + + if Gitlab::Database.postgresql_upcoming_deprecation? + upcoming_deprecation = Gitlab::Database::UPCOMING_POSTGRES_VERSION_DETAILS + + notices << + { + type: 'warning', + message: _('Note that PostgreSQL %{pg_version_upcoming} will become the minimum required ' \ + 'version in GitLab %{gl_version_upcoming} (%{gl_version_upcoming_date}). Please ' \ + 'consider upgrading your environment to a supported PostgreSQL version soon, ' \ + 'see <a href="%{pg_version_upcoming_url}">the related epic</a> for details.') % { + pg_version_upcoming: upcoming_deprecation[:pg_version_minimum], + gl_version_upcoming: upcoming_deprecation[:gl_version], + gl_version_upcoming_date: upcoming_deprecation[:gl_version_date], + pg_version_upcoming_url: upcoming_deprecation[:url] + } + } + end - [ - { - type: 'warning', - message: _('Note that PostgreSQL 11 will become the minimum required PostgreSQL version in GitLab 13.0 (May 2020). '\ - 'PostgreSQL 9.6 and PostgreSQL 10 will no longer be supported in GitLab 13.0. '\ - 'Please consider upgrading your PostgreSQL version (%{db_version}) soon.') % { db_version: Gitlab::Database.version.to_s } - } - ] + notices end end end diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 45895c0b1e2..2bfb6c32886 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -4,8 +4,20 @@ module Gitlab module Database include Gitlab::Metrics::Methods + # Minimum PostgreSQL version requirement per documentation: + # https://docs.gitlab.com/ee/install/requirements.html#postgresql-requirements MINIMUM_POSTGRES_VERSION = 11 + # Upcoming PostgreSQL version requirements + # Allows a soft warning about an upcoming minimum version requirement + # so administrators can prepare to upgrade + UPCOMING_POSTGRES_VERSION_DETAILS = { + gl_version: '13.6.0', + gl_version_date: 'November 2020', + pg_version_minimum: 12, + url: 'https://gitlab.com/groups/gitlab-org/-/epics/2374' + }.freeze + # https://www.postgresql.org/docs/9.2/static/datatype-numeric.html MAX_INT_VALUE = 2147483647 @@ -103,6 +115,10 @@ module Gitlab version.to_f >= MINIMUM_POSTGRES_VERSION end + def self.postgresql_upcoming_deprecation? + version.to_f < UPCOMING_POSTGRES_VERSION_DETAILS[:pg_version_minimum] + end + def self.check_postgres_version_and_print_warning return if Gitlab::Database.postgresql_minimum_supported_version? return if Gitlab::Runtime.rails_runner? diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9ed4f23293b..cc506ebae86 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -15820,7 +15820,7 @@ msgstr "" msgid "Note parameters are invalid: %{errors}" msgstr "" -msgid "Note that PostgreSQL 11 will become the minimum required PostgreSQL version in GitLab 13.0 (May 2020). PostgreSQL 9.6 and PostgreSQL 10 will no longer be supported in GitLab 13.0. Please consider upgrading your PostgreSQL version (%{db_version}) soon." +msgid "Note that PostgreSQL %{pg_version_upcoming} will become the minimum required version in GitLab %{gl_version_upcoming} (%{gl_version_upcoming_date}). Please consider upgrading your environment to a supported PostgreSQL version soon, see <a href=\"%{pg_version_upcoming_url}\">the related epic</a> for details." msgstr "" msgid "Note that this invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}." @@ -16707,7 +16707,7 @@ msgstr "" msgid "People without permission will never get a notification." msgstr "" -msgid "Percent rollout (logged in users)" +msgid "Percent of users" msgstr "" msgid "Percentage" @@ -25502,6 +25502,9 @@ msgstr "" msgid "User IDs" msgstr "" +msgid "User List" +msgstr "" + msgid "User Lists can only be created and modified with %{linkStart}the API%{linkEnd}" msgstr "" @@ -26648,6 +26651,9 @@ msgstr "" msgid "You are trying to upload something other than an image. Please upload a .png, .jpg, .jpeg, .gif, .bmp, .tiff or .ico." msgstr "" +msgid "You are using PostgreSQL %{pg_version_current}, but PostgreSQL %{pg_version_minimum} is required for this version of GitLab. Please upgrade your environment to a supported PostgreSQL version, see %{pg_requirements_url} for details." +msgstr "" + msgid "You can %{linkStart}view the blob%{linkEnd} instead." msgstr "" diff --git a/qa/qa/page/project/operations/metrics/show.rb b/qa/qa/page/project/operations/metrics/show.rb index a1c15e72f44..e9e4923a0e2 100644 --- a/qa/qa/page/project/operations/metrics/show.rb +++ b/qa/qa/page/project/operations/metrics/show.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'securerandom' + module QA module Page module Project @@ -60,7 +62,7 @@ module QA def duplicate_dashboard(save_as = 'test_duplication.yml', commit_option = 'Commit to master branch') click_element :dashboards_filter_dropdown click_on 'Duplicate dashboard' - fill_element :duplicate_dashboard_filename_field, save_as + fill_element :duplicate_dashboard_filename_field, "#{SecureRandom.hex(8)}-#{save_as}" choose commit_option within('.modal-content') { click_button(class: 'btn-success') } end diff --git a/spec/frontend/issuables_list/components/issuable_spec.js b/spec/frontend/issuables_list/components/issuable_spec.js index 2af06f59a0f..78a38506059 100644 --- a/spec/frontend/issuables_list/components/issuable_spec.js +++ b/spec/frontend/issuables_list/components/issuable_spec.js @@ -1,5 +1,5 @@ import { shallowMount } from '@vue/test-utils'; -import { GlSprintf, GlLabel } from '@gitlab/ui'; +import { GlSprintf, GlLabel, GlIcon } from '@gitlab/ui'; import { TEST_HOST } from 'helpers/test_constants'; import { trimText } from 'helpers/text_helper'; import initUserPopovers from '~/user_popovers'; @@ -75,7 +75,9 @@ describe('Issuable component', () => { window.Date = DateOrig; }); - const findConfidentialIcon = () => wrapper.find('.fa-eye-slash'); + const checkExists = findFn => () => findFn().exists(); + const hasConfidentialIcon = () => + wrapper.findAll(GlIcon).wrappers.some(iconWrapper => iconWrapper.props('name') === 'eye-slash'); const findTaskStatus = () => wrapper.find('.task-status'); const findOpenedAgoContainer = () => wrapper.find('[data-testid="openedByMessage"]'); const findMilestone = () => wrapper.find('.js-milestone'); @@ -169,19 +171,19 @@ describe('Issuable component', () => { }); it.each` - desc | finder - ${'bulk editing checkbox'} | ${findBulkCheckbox} - ${'confidential icon'} | ${findConfidentialIcon} - ${'task status'} | ${findTaskStatus} - ${'milestone'} | ${findMilestone} - ${'due date'} | ${findDueDate} - ${'labels'} | ${findLabels} - ${'weight'} | ${findWeight} - ${'merge request count'} | ${findMergeRequestsCount} - ${'upvotes'} | ${findUpvotes} - ${'downvotes'} | ${findDownvotes} - `('does not render $desc', ({ finder }) => { - expect(finder().exists()).toBe(false); + desc | check + ${'bulk editing checkbox'} | ${checkExists(findBulkCheckbox)} + ${'confidential icon'} | ${hasConfidentialIcon} + ${'task status'} | ${checkExists(findTaskStatus)} + ${'milestone'} | ${checkExists(findMilestone)} + ${'due date'} | ${checkExists(findDueDate)} + ${'labels'} | ${checkExists(findLabels)} + ${'weight'} | ${checkExists(findWeight)} + ${'merge request count'} | ${checkExists(findMergeRequestsCount)} + ${'upvotes'} | ${checkExists(findUpvotes)} + ${'downvotes'} | ${checkExists(findDownvotes)} + `('does not render $desc', ({ check }) => { + expect(check()).toBe(false); }); it('show relative reference path', () => { @@ -215,7 +217,7 @@ describe('Issuable component', () => { }); it('renders the confidential icon', () => { - expect(findConfidentialIcon().exists()).toBe(true); + expect(hasConfidentialIcon()).toBe(true); }); }); diff --git a/spec/lib/gitlab/config_checker/external_database_checker_spec.rb b/spec/lib/gitlab/config_checker/external_database_checker_spec.rb index c651868bd1e..316696bc584 100644 --- a/spec/lib/gitlab/config_checker/external_database_checker_spec.rb +++ b/spec/lib/gitlab/config_checker/external_database_checker_spec.rb @@ -6,51 +6,54 @@ RSpec.describe Gitlab::ConfigChecker::ExternalDatabaseChecker do describe '#check' do subject { described_class.check } - context 'database version is not deprecated' do + let_it_be(:deprecation_warning) { "Please upgrade" } + let_it_be(:upcoming_deprecation_warning) { "Please consider upgrading" } + + context 'when database meets minimum version and there is no upcoming deprecation' do before do - allow(described_class).to receive(:db_version_deprecated?).and_return(false) + allow(Gitlab::Database).to receive(:postgresql_minimum_supported_version?).and_return(true) + allow(Gitlab::Database).to receive(:postgresql_upcoming_deprecation?).and_return(false) end it { is_expected.to be_empty } end - context 'database version is deprecated' do + context 'when database does not meet minimum version and there is no upcoming deprecation' do before do - allow(described_class).to receive(:db_version_deprecated?).and_return(true) - end - - let(:notice_deprecated_database) do - { - type: 'warning', - message: _('Note that PostgreSQL 11 will become the minimum required PostgreSQL version in GitLab 13.0 (May 2020). '\ - 'PostgreSQL 9.6 and PostgreSQL 10 will no longer be supported in GitLab 13.0. '\ - 'Please consider upgrading your PostgreSQL version (%{db_version}) soon.') % { db_version: Gitlab::Database.version.to_s } - } + allow(Gitlab::Database).to receive(:postgresql_minimum_supported_version?).and_return(false) + allow(Gitlab::Database).to receive(:postgresql_upcoming_deprecation?).and_return(false) end - it 'reports deprecated database notices' do - is_expected.to contain_exactly(notice_deprecated_database) + it 'only returns notice about deprecated database version' do + is_expected.to include(a_hash_including(message: include(deprecation_warning))) + is_expected.not_to include(a_hash_including(message: include(upcoming_deprecation_warning))) end end - end - describe '#db_version_deprecated' do - subject { described_class.db_version_deprecated? } - - context 'database version is not deprecated' do + context 'when database meets minimum version and there is an upcoming deprecation' do before do - allow(Gitlab::Database).to receive(:version).and_return(11) + allow(Gitlab::Database).to receive(:postgresql_minimum_supported_version?).and_return(true) + allow(Gitlab::Database).to receive(:postgresql_upcoming_deprecation?).and_return(true) end - it { is_expected.to be false } + it 'only returns notice about an upcoming deprecation' do + is_expected.to include(a_hash_including(message: include(upcoming_deprecation_warning))) + is_expected.not_to include(a_hash_including(message: include(deprecation_warning))) + end end - context 'database version is deprecated' do + context 'when database does not meet minimum version and there is an upcoming deprecation' do before do - allow(Gitlab::Database).to receive(:version).and_return(10) + allow(Gitlab::Database).to receive(:postgresql_minimum_supported_version?).and_return(false) + allow(Gitlab::Database).to receive(:postgresql_upcoming_deprecation?).and_return(true) end - it { is_expected.to be true } + it 'returns notice about deprecated database version and an upcoming deprecation' do + is_expected.to include( + a_hash_including(message: include(deprecation_warning)), + a_hash_including(message: include(upcoming_deprecation_warning)) + ) + end end end end diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index c9e87b46ac9..cd009f955af 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -90,28 +90,42 @@ RSpec.describe Gitlab::Database do end describe '.postgresql_minimum_supported_version?' do - it 'returns false when using PostgreSQL 9.5' do - allow(described_class).to receive(:version).and_return('9.5') + it 'returns false when using PostgreSQL 10' do + allow(described_class).to receive(:version).and_return('10') expect(described_class.postgresql_minimum_supported_version?).to eq(false) end - it 'returns false when using PostgreSQL 9.6' do - allow(described_class).to receive(:version).and_return('9.6') + it 'returns true when using PostgreSQL 11' do + allow(described_class).to receive(:version).and_return('11') - expect(described_class.postgresql_minimum_supported_version?).to eq(false) + expect(described_class.postgresql_minimum_supported_version?).to eq(true) end - it 'returns false when using PostgreSQL 10' do - allow(described_class).to receive(:version).and_return('10') + it 'returns true when using PostgreSQL 12' do + allow(described_class).to receive(:version).and_return('12') - expect(described_class.postgresql_minimum_supported_version?).to eq(false) + expect(described_class.postgresql_minimum_supported_version?).to eq(true) end + end - it 'returns true when using PostgreSQL 11 or newer' do - allow(described_class).to receive(:version).and_return('11.0') + describe '.postgresql_upcoming_deprecation?' do + it 'returns true when database version is lower than the upcoming minimum' do + allow(described_class).to receive(:version).and_return('11') - expect(described_class.postgresql_minimum_supported_version?).to eq(true) + expect(described_class.postgresql_upcoming_deprecation?).to eq(true) + end + + it 'returns false when database version equals the upcoming minimum' do + allow(described_class).to receive(:version).and_return('12') + + expect(described_class.postgresql_upcoming_deprecation?).to eq(false) + end + + it 'returns false when database version is greater the upcoming minimum' do + allow(described_class).to receive(:version).and_return('13') + + expect(described_class.postgresql_upcoming_deprecation?).to eq(false) end end diff --git a/spec/models/snippet_input_action_spec.rb b/spec/models/snippet_input_action_spec.rb index 43c2919735b..ca61b80df4c 100644 --- a/spec/models/snippet_input_action_spec.rb +++ b/spec/models/snippet_input_action_spec.rb @@ -29,8 +29,8 @@ RSpec.describe SnippetInputAction do :move | 'foobar' | '' | 'foo1' | nil | true | nil :create | 'foobar' | nil | 'foobar' | nil | false | :content :create | 'foobar' | '' | 'foobar' | nil | false | :content - :create | nil | 'foobar' | 'foobar' | nil | false | :file_path - :create | '' | 'foobar' | 'foobar' | nil | false | :file_path + :create | nil | 'foobar' | 'foobar' | nil | true | nil + :create | '' | 'foobar' | 'foobar' | nil | true | nil :update | 'foobar' | nil | 'foobar' | nil | false | :content :update | 'foobar' | '' | 'foobar' | nil | false | :content :update | 'other' | 'foobar' | 'foobar' | nil | false | :file_path diff --git a/spec/services/snippets/create_service_spec.rb b/spec/services/snippets/create_service_spec.rb index f8e69b4b4bb..d0b2dde6ee4 100644 --- a/spec/services/snippets/create_service_spec.rb +++ b/spec/services/snippets/create_service_spec.rb @@ -177,10 +177,8 @@ RSpec.describe Snippets::CreateService do end it 'returns a generic error' do - response = subject - - expect(response).to be_error - expect(response.payload[:snippet].errors[:repository]).to eq ['Error creating the snippet'] + expect(subject).to be_error + expect(snippet.errors[:repository]).to eq ['Error creating the snippet'] end end @@ -250,7 +248,7 @@ RSpec.describe Snippets::CreateService do end it 'commit the files to the repository' do - subject + expect(subject).to be_success blob = snippet.repository.blob_at('master', file_path) @@ -261,10 +259,7 @@ RSpec.describe Snippets::CreateService do let(:extra_opts) { { content: 'foo', file_name: 'path' } } it 'a validation error is raised' do - response = subject - snippet = response.payload[:snippet] - - expect(response).to be_error + expect(subject).to be_error expect(snippet.errors.full_messages_for(:content)).to eq ['Content and snippet files cannot be used together'] expect(snippet.errors.full_messages_for(:file_name)).to eq ['File name and snippet files cannot be used together'] expect(snippet.repository.exists?).to be_falsey @@ -275,10 +270,7 @@ RSpec.describe Snippets::CreateService do let(:snippet_files) { [{ action: 'invalid_action', file_path: 'snippet_file_path.rb', content: 'snippet_content' }] } it 'a validation error is raised' do - response = subject - snippet = response.payload[:snippet] - - expect(response).to be_error + expect(subject).to be_error expect(snippet.errors.full_messages_for(:snippet_files)).to eq ['Snippet files have invalid data'] expect(snippet.repository.exists?).to be_falsey end @@ -288,14 +280,21 @@ RSpec.describe Snippets::CreateService do let(:snippet_files) { [{ action: 'delete', file_path: 'snippet_file_path.rb' }] } it 'a validation error is raised' do - response = subject - snippet = response.payload[:snippet] - - expect(response).to be_error + expect(subject).to be_error expect(snippet.errors.full_messages_for(:snippet_files)).to eq ['Snippet files have invalid data'] expect(snippet.repository.exists?).to be_falsey end end + + context 'when "create" operation does not have file_path or is empty' do + let(:snippet_files) { [{ action: 'create', content: content }, { action: 'create', content: content, file_path: '' }] } + + it 'generates the file path for the files' do + expect(subject).to be_success + expect(snippet.repository.blob_at('master', 'snippetfile1.txt').data).to eq content + expect(snippet.repository.blob_at('master', 'snippetfile2.txt').data).to eq content + end + end end context 'when ProjectSnippet' do diff --git a/spec/services/snippets/update_service_spec.rb b/spec/services/snippets/update_service_spec.rb index b16c4d64af3..5587668e9ed 100644 --- a/spec/services/snippets/update_service_spec.rb +++ b/spec/services/snippets/update_service_spec.rb @@ -537,10 +537,18 @@ RSpec.describe Snippets::UpdateService do it_behaves_like 'returns an error', 'Snippet files have invalid data' end - context 'when file_path is not present' do - let(:snippet_files) { [{ action: :create, content: content }] } + context 'when file_path is not present or empty' do + let(:snippet_files) { [{ action: :create, content: content }, { action: :create, file_path: '', content: content }] } - it_behaves_like 'returns an error', 'Snippet files have invalid data' + it 'generates the file path for the files' do + expect(blob('snippetfile1.txt')).to be_nil + expect(blob('snippetfile2.txt')).to be_nil + + expect(subject).to be_success + + expect(blob('snippetfile1.txt').data).to eq content + expect(blob('snippetfile2.txt').data).to eq content + end end context 'when file_path already exists in the repository' do |