From 5f4d643165631ea503b8aa1e9abbecc8341c39c6 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 17 Mar 2021 09:09:27 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .gitlab-ci.yml | 2 +- .gitlab/ci/global.gitlab-ci.yml | 8 +-- .../pages/dashboard/todos/index/todos.js | 35 +++++++--- app/assets/stylesheets/pages/tree.scss | 2 +- app/finders/ci/variables_finder.rb | 10 +-- .../resolvers/ci/runner_platforms_resolver.rb | 3 +- .../resolvers/ci/runner_setup_resolver.rb | 1 + .../resolvers/concerns/resolves_snippets.rb | 2 +- .../user_starred_projects_resolver.rb | 2 +- app/graphql/types/user_type.rb | 1 + app/helpers/todos_helper.rb | 18 ----- app/models/issue.rb | 4 +- app/views/ci/group_variables/_index.html.haml | 3 + .../_variable_header.html.haml | 3 + app/views/dashboard/todos/index.html.haml | 4 +- ...-and-update-buttons-on-projects-commit.yml | 5 ++ .../ph-todosProjectGroupDropdownApiCall.yml | 5 ++ doc/.vale/gitlab/ReferenceLinks.yml | 2 +- doc/api/graphql/reference/index.md | 2 + doc/development/contributing/style_guides.md | 1 + ...curity_dashboard_status_change_v13_10.png} | Bin .../vulnerability_report/index.md | 4 +- lefthook.yml | 4 ++ lib/api/group_variables.rb | 36 ++++++---- lib/api/helpers/variables_helpers.rb | 27 ++++++++ lib/api/variables.rb | 30 ++------ locale/gitlab.pot | 2 +- spec/finders/ci/variables_finder_spec.rb | 65 +++++++++++------- .../ci/runner_platforms_resolver_spec.rb | 2 +- spec/helpers/todos_helper_spec.rb | 20 ------ .../lib/api/helpers/variables_helpers_spec.rb | 43 ++++++++++++ spec/requests/api/group_variables_spec.rb | 3 + spec/tooling/danger/project_helper_spec.rb | 3 + tooling/danger/project_helper.rb | 2 + 34 files changed, 223 insertions(+), 131 deletions(-) create mode 100644 changelogs/unreleased/273300-fy21q4-foundations-kr2-audit-and-update-buttons-on-projects-commit.yml create mode 100644 changelogs/unreleased/ph-todosProjectGroupDropdownApiCall.yml rename doc/user/application_security/vulnerability_report/img/{project_security_dashboard_status_change_v13_9.png => project_security_dashboard_status_change_v13_10.png} (100%) create mode 100644 lib/api/helpers/variables_helpers.rb create mode 100644 spec/lib/api/helpers/variables_helpers_spec.rb diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8bb09825e67..38115beec69 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -17,7 +17,7 @@ stages: # in cases where jobs require Docker-in-Docker, the job # definition must be extended with `.use-docker-in-docker` default: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-89-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.36" tags: - gitlab-org # All jobs are interruptible by default diff --git a/.gitlab/ci/global.gitlab-ci.yml b/.gitlab/ci/global.gitlab-ci.yml index de4b609098d..7b722c60cd1 100644 --- a/.gitlab/ci/global.gitlab-ci.yml +++ b/.gitlab/ci/global.gitlab-ci.yml @@ -87,7 +87,7 @@ policy: pull .use-pg11: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-89-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.36" services: - name: postgres:11.6 command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"] @@ -96,7 +96,7 @@ POSTGRES_HOST_AUTH_METHOD: trust .use-pg12: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-12-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-89-node-14.15-yarn-1.22-postgresql-12-graphicsmagick-1.3.36" services: - name: postgres:12 command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"] @@ -105,7 +105,7 @@ POSTGRES_HOST_AUTH_METHOD: trust .use-pg11-ee: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-89-node-14.15-yarn-1.22-postgresql-11-graphicsmagick-1.3.36" services: - name: postgres:11.6 command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"] @@ -116,7 +116,7 @@ POSTGRES_HOST_AUTH_METHOD: trust .use-pg12-ee: - image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-87-node-14.15-yarn-1.22-postgresql-12-graphicsmagick-1.3.34" + image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.7.2.patched-golang-1.14-git-2.29-lfs-2.9-chrome-89-node-14.15-yarn-1.22-postgresql-12-graphicsmagick-1.3.36" services: - name: postgres:12 command: ["postgres", "-c", "fsync=off", "-c", "synchronous_commit=off", "-c", "full_page_writes=off"] diff --git a/app/assets/javascripts/pages/dashboard/todos/index/todos.js b/app/assets/javascripts/pages/dashboard/todos/index/todos.js index d53cd405504..42341436b55 100644 --- a/app/assets/javascripts/pages/dashboard/todos/index/todos.js +++ b/app/assets/javascripts/pages/dashboard/todos/index/todos.js @@ -1,6 +1,8 @@ /* eslint-disable class-methods-use-this, no-unneeded-ternary */ import $ from 'jquery'; +import { getGroups } from '~/api/groups_api'; +import { getProjects } from '~/api/projects_api'; import initDeprecatedJQueryDropdown from '~/deprecated_jquery_dropdown'; import { deprecatedCreateFlash as flash } from '~/flash'; import axios from '~/lib/utils/axios_utils'; @@ -41,14 +43,37 @@ export default class Todos { } initFilters() { - this.initFilterDropdown($('.js-group-search'), 'group_id', ['text']); - this.initFilterDropdown($('.js-project-search'), 'project_id', ['text']); + this.initAjaxFilterDropdown(getGroups, $('.js-group-search'), 'group_id'); + this.initAjaxFilterDropdown(getProjects, $('.js-project-search'), 'project_id'); this.initFilterDropdown($('.js-type-search'), 'type'); this.initFilterDropdown($('.js-action-search'), 'action_id'); return new UsersSelect(); } + initAjaxFilterDropdown(apiMethod, $dropdown, fieldName) { + initDeprecatedJQueryDropdown($dropdown, { + fieldName, + selectable: true, + filterable: true, + filterRemote: true, + data(search, callback) { + return apiMethod(search, {}, (data) => { + callback( + data.map((d) => ({ + id: d.id, + text: d.full_name || d.name_with_namespace, + })), + ); + }); + }, + clicked: () => { + const $formEl = $dropdown.closest('form.filter-form'); + $formEl.submit(); + }, + }); + } + initFilterDropdown($dropdown, fieldName, searchFields) { initDeprecatedJQueryDropdown($dropdown, { fieldName, @@ -58,12 +83,6 @@ export default class Todos { data: $dropdown.data('data'), clicked: () => { const $formEl = $dropdown.closest('form.filter-form'); - const mutexDropdowns = { - group_id: 'project_id', - project_id: 'group_id', - }; - - $formEl.find(`input[name="${mutexDropdowns[fieldName]}"]`).remove(); $formEl.submit(); }, }); diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index fa008a05e11..a371aa37e07 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -22,7 +22,7 @@ .control { float: left; - margin-left: 10px; + margin-left: 8px; } } diff --git a/app/finders/ci/variables_finder.rb b/app/finders/ci/variables_finder.rb index d933643ffb2..3e61127ff5b 100644 --- a/app/finders/ci/variables_finder.rb +++ b/app/finders/ci/variables_finder.rb @@ -2,16 +2,14 @@ module Ci class VariablesFinder - attr_reader :project, :params - - def initialize(project, params) - @project, @params = project, params + def initialize(resource, params) + @resource, @params = resource, params raise ArgumentError, 'Please provide params[:key]' if params[:key].blank? end def execute - variables = project.variables + variables = resource.variables variables = by_key(variables) variables = by_environment_scope(variables) variables @@ -19,6 +17,8 @@ module Ci private + attr_reader :resource, :params + def by_key(variables) variables.by_key(params[:key]) end diff --git a/app/graphql/resolvers/ci/runner_platforms_resolver.rb b/app/graphql/resolvers/ci/runner_platforms_resolver.rb index 9677c5139b4..f120e94b67b 100644 --- a/app/graphql/resolvers/ci/runner_platforms_resolver.rb +++ b/app/graphql/resolvers/ci/runner_platforms_resolver.rb @@ -3,7 +3,8 @@ module Resolvers module Ci class RunnerPlatformsResolver < BaseResolver - type Types::Ci::RunnerPlatformType, null: false + type Types::Ci::RunnerPlatformType.connection_type, null: true + description 'Supported runner platforms.' def resolve(**args) runner_instructions.map do |platform, data| diff --git a/app/graphql/resolvers/ci/runner_setup_resolver.rb b/app/graphql/resolvers/ci/runner_setup_resolver.rb index ac2a56b89a7..7a7663ea63d 100644 --- a/app/graphql/resolvers/ci/runner_setup_resolver.rb +++ b/app/graphql/resolvers/ci/runner_setup_resolver.rb @@ -4,6 +4,7 @@ module Resolvers module Ci class RunnerSetupResolver < BaseResolver type Types::Ci::RunnerSetupType, null: true + description 'Runner setup instructions.' argument :platform, GraphQL::STRING_TYPE, required: true, diff --git a/app/graphql/resolvers/concerns/resolves_snippets.rb b/app/graphql/resolvers/concerns/resolves_snippets.rb index 445f3567b1d..8de85c074ec 100644 --- a/app/graphql/resolvers/concerns/resolves_snippets.rb +++ b/app/graphql/resolvers/concerns/resolves_snippets.rb @@ -4,7 +4,7 @@ module ResolvesSnippets extend ActiveSupport::Concern included do - type Types::SnippetType.connection_type, null: false + type Types::SnippetType.connection_type, null: true argument :ids, [::Types::GlobalIDType[::Snippet]], required: false, diff --git a/app/graphql/resolvers/user_starred_projects_resolver.rb b/app/graphql/resolvers/user_starred_projects_resolver.rb index db420b3d116..ff5e4ca82e5 100644 --- a/app/graphql/resolvers/user_starred_projects_resolver.rb +++ b/app/graphql/resolvers/user_starred_projects_resolver.rb @@ -2,7 +2,7 @@ module Resolvers class UserStarredProjectsResolver < BaseResolver - type Types::ProjectType, null: true + type Types::ProjectType.connection_type, null: true argument :search, GraphQL::STRING_TYPE, required: false, diff --git a/app/graphql/types/user_type.rb b/app/graphql/types/user_type.rb index 2cc7d379240..abaf823c305 100644 --- a/app/graphql/types/user_type.rb +++ b/app/graphql/types/user_type.rb @@ -3,6 +3,7 @@ module Types class UserType < BaseObject graphql_name 'User' + description 'Representation of a GitLab user.' authorize :read_user diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index 7b0e0df8998..1fc751eae2c 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -157,16 +157,6 @@ module TodosHelper ] end - def todo_projects_options - projects = current_user.authorized_projects.sorted_by_activity.non_archived.with_route - - projects = projects.map do |project| - { id: project.id, text: project.full_name } - end - - projects.unshift({ id: '', text: 'Any Project' }).to_json - end - def todo_types_options [ { id: '', text: 'Any Type' }, @@ -240,14 +230,6 @@ module TodosHelper false end end - - def todo_group_options - groups = current_user.authorized_groups.with_route.map do |group| - { id: group.id, text: group.full_name } - end - - groups.unshift({ id: '', text: 'Any Group' }).to_json - end end TodosHelper.prepend_if_ee('EE::TodosHelper') diff --git a/app/models/issue.rb b/app/models/issue.rb index 2f2d24cbe93..4c46f779893 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -88,7 +88,6 @@ class Issue < ApplicationRecord test_case: 2 ## EE-only } - alias_attribute :parent_ids, :project_id alias_method :issuing_parent, :project alias_attribute :external_author, :service_desk_reply_to @@ -191,7 +190,8 @@ class Issue < ApplicationRecord end def self.relative_positioning_query_base(issue) - in_projects(issue.parent_ids) + projects = issue.project.group&.root_ancestor&.all_projects || issue.project + in_projects(projects) end def self.relative_positioning_parent_column diff --git a/app/views/ci/group_variables/_index.html.haml b/app/views/ci/group_variables/_index.html.haml index a74dbe793a6..3a7d1f4dead 100644 --- a/app/views/ci/group_variables/_index.html.haml +++ b/app/views/ci/group_variables/_index.html.haml @@ -7,6 +7,9 @@ %tr %td.gl-text-truncate = variable.key + - if Feature.enabled?(:scoped_group_variables, default_enabled: :yaml) + %td.gl-text-truncate + = variable.environment_scope %td.gl-text-truncate %a.group-origin-link{ href: group_settings_ci_cd_path(variable.group) } = variable.group.name diff --git a/app/views/ci/group_variables/_variable_header.html.haml b/app/views/ci/group_variables/_variable_header.html.haml index ec512ab37e7..691231ab8ad 100644 --- a/app/views/ci/group_variables/_variable_header.html.haml +++ b/app/views/ci/group_variables/_variable_header.html.haml @@ -1,5 +1,8 @@ %tr %th = s_('Key') + - if Feature.enabled?(:scoped_group_variables, default_enabled: :yaml) + %th + = s_('Environments') %th = s_('Group') diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index d78059b6aed..a0016417f0c 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -42,12 +42,12 @@ - if params[:group_id].present? = hidden_field_tag(:group_id, params[:group_id]) = dropdown_tag(group_dropdown_label(params[:group_id], 'Group'), options: { toggle_class: 'js-group-search js-filter-submit', title: 'Filter by group', filter: true, filterInput: 'input#group-search', dropdown_class: 'dropdown-menu-selectable dropdown-menu-group js-filter-submit', - placeholder: 'Search groups', data: { data: todo_group_options, default_label: 'Group', display: 'static' } }) + placeholder: 'Search groups', data: { default_label: 'Group', display: 'static' } }) .filter-item.inline - if params[:project_id].present? = hidden_field_tag(:project_id, params[:project_id]) = dropdown_tag(project_dropdown_label(params[:project_id], 'Project'), options: { toggle_class: 'js-project-search js-filter-submit', title: 'Filter by project', filter: true, filterInput: 'input#project-search', dropdown_class: 'dropdown-menu-selectable dropdown-menu-project js-filter-submit', - placeholder: 'Search projects', data: { data: todo_projects_options, default_label: 'Project', display: 'static' } }) + placeholder: 'Search projects', data: { default_label: 'Project', display: 'static' } }) .filter-item.inline - if params[:author_id].present? = hidden_field_tag(:author_id, params[:author_id]) diff --git a/changelogs/unreleased/273300-fy21q4-foundations-kr2-audit-and-update-buttons-on-projects-commit.yml b/changelogs/unreleased/273300-fy21q4-foundations-kr2-audit-and-update-buttons-on-projects-commit.yml new file mode 100644 index 00000000000..31e7cb1bef6 --- /dev/null +++ b/changelogs/unreleased/273300-fy21q4-foundations-kr2-audit-and-update-buttons-on-projects-commit.yml @@ -0,0 +1,5 @@ +--- +title: Decrease spacing between controls on the Commit page header +merge_request: 56129 +author: +type: other diff --git a/changelogs/unreleased/ph-todosProjectGroupDropdownApiCall.yml b/changelogs/unreleased/ph-todosProjectGroupDropdownApiCall.yml new file mode 100644 index 00000000000..7c94af3e8a2 --- /dev/null +++ b/changelogs/unreleased/ph-todosProjectGroupDropdownApiCall.yml @@ -0,0 +1,5 @@ +--- +title: Move fetching projects and groups on todos page to API call +merge_request: 56507 +author: +type: performance diff --git a/doc/.vale/gitlab/ReferenceLinks.yml b/doc/.vale/gitlab/ReferenceLinks.yml index 160ed2e8e14..ca9948844f8 100644 --- a/doc/.vale/gitlab/ReferenceLinks.yml +++ b/doc/.vale/gitlab/ReferenceLinks.yml @@ -10,4 +10,4 @@ link: https://docs.gitlab.com/ee/development/documentation/styleguide/index.html level: error scope: raw raw: - - '\n\[.*\]: .*' + - '\n\[[^\]]*\]: .*' diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index b17017a2271..7039ee9fab0 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -6444,6 +6444,8 @@ An edge in a connection. ### `User` +Representation of a GitLab user. + | Field | Type | Description | | ----- | ---- | ----------- | | `assignedMergeRequests` | [`MergeRequestConnection`](#mergerequestconnection) | Merge Requests assigned to the user. | diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md index 444a067a780..0f4b69d3daf 100644 --- a/doc/development/contributing/style_guides.md +++ b/doc/development/contributing/style_guides.md @@ -62,6 +62,7 @@ Before you push your changes, Lefthook automatically runs the following checks: - SCSS lint: Run `yarn lint:stylelint` checks (with the [`.stylelintrc`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.stylelintrc) configuration) on the modified `*.scss{,.css}` files. Tags: `stylesheet`, `css`, `style`. - RuboCop: Run `bundle exec rubocop` checks (with the [`.rubocop.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.rubocop.yml) configuration) on the modified `*.rb` files. Tags: `backend`, `style`. - Vale: Run `vale` checks (with the [`.vale.ini`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.vale.ini) configuration) on the modified `*.md` files. Tags: `documentation`, `style`. +- Documentation metadata: Run checks for the absence of [documentation metadata](../documentation/index.md#metadata). In addition to the default configuration, you can define a [local configuration](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#local-config). diff --git a/doc/user/application_security/vulnerability_report/img/project_security_dashboard_status_change_v13_9.png b/doc/user/application_security/vulnerability_report/img/project_security_dashboard_status_change_v13_10.png similarity index 100% rename from doc/user/application_security/vulnerability_report/img/project_security_dashboard_status_change_v13_9.png rename to doc/user/application_security/vulnerability_report/img/project_security_dashboard_status_change_v13_10.png diff --git a/doc/user/application_security/vulnerability_report/index.md b/doc/user/application_security/vulnerability_report/index.md index 77b531f68ec..cbc5907a0c0 100644 --- a/doc/user/application_security/vulnerability_report/index.md +++ b/doc/user/application_security/vulnerability_report/index.md @@ -119,12 +119,14 @@ Hover over an **Activity** entry and select a link go to that issue. ## Change status of vulnerabilities +> The option to select a status other than Dismissed was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/292636) in GitLab 13.10. + To change the status of vulnerabilities in the table: 1. Select the checkbox for each vulnerability you want to update the status of. 1. In the dropdown that appears select the desired status, then select **Change status**. -![Project Vulnerability Report](img/project_security_dashboard_status_change_v13_9.png) +![Project Vulnerability Report](img/project_security_dashboard_status_change_v13_10.png) ## Export vulnerability details diff --git a/lefthook.yml b/lefthook.yml index 503ed8f8bb6..29a7cfafd83 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -38,3 +38,7 @@ pre-push: files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD glob: 'doc/*.md' run: if command -v vale 2> /dev/null; then vale --config .vale.ini --minAlertLevel error {files}; else echo "Vale not found. Install Vale"; fi + docs-metadata: # https://docs.gitlab.com/ee/development/documentation/#metadata + tags: documentation style + files: git diff --name-only $(git merge-base origin/master HEAD)..HEAD | grep '.md' + run: if [[ $(head -n1 {files} | grep -v --count -- '---') -ge 1 ]]; then echo "$(tput setaf 1)Documentation metadata missing in changed Markdown files.$(tput sgr0) For more information, see https://docs.gitlab.com/ee/development/documentation/#metadata."; false; else echo "$(tput sgr0)Metadata found in {files}."; fi; diff --git a/lib/api/group_variables.rb b/lib/api/group_variables.rb index 09744fbeda2..8d52a0a5b4e 100644 --- a/lib/api/group_variables.rb +++ b/lib/api/group_variables.rb @@ -8,6 +8,8 @@ module API before { authorize! :admin_group, user_group } feature_category :continuous_integration + helpers Helpers::VariablesHelpers + params do requires :id, type: String, desc: 'The ID of a group' end @@ -30,16 +32,13 @@ module API params do requires :key, type: String, desc: 'The key of the variable' end - # rubocop: disable CodeReuse/ActiveRecord get ':id/variables/:key' do - key = params[:key] - variable = user_group.variables.find_by(key: key) + variable = find_variable(user_group, params) break not_found!('GroupVariable') unless variable present variable, with: Entities::Ci::Variable end - # rubocop: enable CodeReuse/ActiveRecord desc 'Create a new variable in a group' do success Entities::Ci::Variable @@ -50,12 +49,19 @@ module API optional :protected, type: String, desc: 'Whether the variable is protected' optional :masked, type: String, desc: 'Whether the variable is masked' optional :variable_type, type: String, values: ::Ci::GroupVariable.variable_types.keys, desc: 'The type of variable, must be one of env_var or file. Defaults to env_var' + + use :optional_group_variable_params_ee end post ':id/variables' do + filtered_params = filter_variable_parameters( + user_group, + declared_params(include_missing: false) + ) + variable = ::Ci::ChangeVariableService.new( container: user_group, current_user: current_user, - params: { action: :create, variable_params: declared_params(include_missing: false) } + params: { action: :create, variable_params: filtered_params } ).execute if variable.valid? @@ -74,13 +80,19 @@ module API optional :protected, type: String, desc: 'Whether the variable is protected' optional :masked, type: String, desc: 'Whether the variable is masked' optional :variable_type, type: String, values: ::Ci::GroupVariable.variable_types.keys, desc: 'The type of variable, must be one of env_var or file' + + use :optional_group_variable_params_ee end - # rubocop: disable CodeReuse/ActiveRecord put ':id/variables/:key' do + filtered_params = filter_variable_parameters( + user_group, + declared_params(include_missing: false) + ) + variable = ::Ci::ChangeVariableService.new( container: user_group, current_user: current_user, - params: { action: :update, variable_params: declared_params(include_missing: false) } + params: { action: :update, variable_params: filtered_params } ).execute if variable.valid? @@ -91,7 +103,6 @@ module API rescue ::ActiveRecord::RecordNotFound not_found!('GroupVariable') end - # rubocop: enable CodeReuse/ActiveRecord desc 'Delete an existing variable from a group' do success Entities::Ci::Variable @@ -99,21 +110,18 @@ module API params do requires :key, type: String, desc: 'The key of the variable' end - # rubocop: disable CodeReuse/ActiveRecord delete ':id/variables/:key' do - variable = user_group.variables.find_by!(key: params[:key]) + variable = find_variable(user_group, params) + break not_found!('GroupVariable') unless variable destroy_conditionally!(variable) do |target_variable| ::Ci::ChangeVariableService.new( container: user_group, current_user: current_user, - params: { action: :destroy, variable_params: declared_params(include_missing: false) } + params: { action: :destroy, variable: variable } ).execute end - rescue ::ActiveRecord::RecordNotFound - not_found!('GroupVariable') end - # rubocop: enable CodeReuse/ActiveRecord end end end diff --git a/lib/api/helpers/variables_helpers.rb b/lib/api/helpers/variables_helpers.rb new file mode 100644 index 00000000000..e2b3372fc33 --- /dev/null +++ b/lib/api/helpers/variables_helpers.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module API + module Helpers + module VariablesHelpers + extend ActiveSupport::Concern + extend Grape::API::Helpers + + params :optional_group_variable_params_ee do + end + + def filter_variable_parameters(_, params) + params # Overridden in EE + end + + def find_variable(owner, params) + variables = ::Ci::VariablesFinder.new(owner, params).execute.to_a + + return variables.first unless variables.many? # rubocop: disable CodeReuse/ActiveRecord + + conflict!("There are multiple variables with provided parameters. Please use 'filter[environment_scope]'") + end + end + end +end + +API::Helpers::VariablesHelpers.prepend_if_ee('EE::API::Helpers::VariablesHelpers') diff --git a/lib/api/variables.rb b/lib/api/variables.rb index 94fa98b7a14..8b0745c6b5b 100644 --- a/lib/api/variables.rb +++ b/lib/api/variables.rb @@ -9,21 +9,7 @@ module API feature_category :continuous_integration - helpers do - def filter_variable_parameters(params) - # This method exists so that EE can more easily filter out certain - # parameters, without having to modify the source code directly. - params - end - - def find_variable(params) - variables = ::Ci::VariablesFinder.new(user_project, params).execute.to_a - - return variables.first unless variables.many? # rubocop: disable CodeReuse/ActiveRecord - - conflict!("There are multiple variables with provided parameters. Please use 'filter[environment_scope]'") - end - end + helpers Helpers::VariablesHelpers params do requires :id, type: String, desc: 'The ID of a project' @@ -49,7 +35,7 @@ module API end # rubocop: disable CodeReuse/ActiveRecord get ':id/variables/:key' do - variable = find_variable(params) + variable = find_variable(user_project, params) not_found!('Variable') unless variable present variable, with: Entities::Ci::Variable @@ -71,7 +57,7 @@ module API variable = ::Ci::ChangeVariableService.new( container: user_project, current_user: current_user, - params: { action: :create, variable_params: filter_variable_parameters(declared_params(include_missing: false)) } + params: { action: :create, variable_params: declared_params(include_missing: false) } ).execute if variable.valid? @@ -95,17 +81,13 @@ module API end # rubocop: disable CodeReuse/ActiveRecord put ':id/variables/:key' do - variable = find_variable(params) + variable = find_variable(user_project, params) not_found!('Variable') unless variable - variable_params = filter_variable_parameters( - declared_params(include_missing: false) - .except(:key, :filter) - ) variable = ::Ci::ChangeVariableService.new( container: user_project, current_user: current_user, - params: { action: :update, variable: variable, variable_params: variable_params } + params: { action: :update, variable: variable, variable_params: declared_params(include_missing: false).except(:key, :filter) } ).execute if variable.valid? @@ -125,7 +107,7 @@ module API end # rubocop: disable CodeReuse/ActiveRecord delete ':id/variables/:key' do - variable = find_variable(params) + variable = find_variable(user_project, params) not_found!('Variable') unless variable ::Ci::ChangeVariableService.new( diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 7e0d2eb2f6f..0453d2033ae 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -31923,7 +31923,7 @@ msgstr "" msgid "Trials|You won't get a free trial right now but you can always resume this process by clicking on your avatar and choosing 'Start a free trial'" msgstr "" -msgid "Trials|Your trial ends on %{boldStart}%{trialEndDate}%{boldEnd}. We hope you are enjoying GitLab %{planName}. To continue using GitLab %{planName} after your trial ends, you will need to buy a subscription. You can also choose GitLab Premium if its features are sufficient for your needs." +msgid "Trials|Your trial ends on %{boldStart}%{trialEndDate}%{boldEnd}. We hope you’re enjoying the features of GitLab %{planName}. To keep those features after your trial ends, you’ll need to buy a subscription. (You can also choose GitLab Premium if it meets your needs.)" msgstr "" msgid "Trial|Company name" diff --git a/spec/finders/ci/variables_finder_spec.rb b/spec/finders/ci/variables_finder_spec.rb index cd5f950ca8e..683788452cc 100644 --- a/spec/finders/ci/variables_finder_spec.rb +++ b/spec/finders/ci/variables_finder_spec.rb @@ -3,42 +3,57 @@ require 'spec_helper' RSpec.describe Ci::VariablesFinder do - let!(:project) { create(:project) } - let!(:params) { {} } + shared_examples 'scoped variables' do + describe '#initialize' do + subject { described_class.new(owner, params) } - let!(:var1) { create(:ci_variable, project: project, key: 'key1', environment_scope: 'staging') } - let!(:var2) { create(:ci_variable, project: project, key: 'key2', environment_scope: 'staging') } - let!(:var3) { create(:ci_variable, project: project, key: 'key2', environment_scope: 'production') } + context 'without key filter' do + let!(:params) { {} } - describe '#initialize' do - subject { described_class.new(project, params) } + it 'raises an error' do + expect { subject }.to raise_error(ArgumentError, 'Please provide params[:key]') + end + end + end - context 'without key filter' do - let!(:params) { {} } + describe '#execute' do + subject { described_class.new(owner.reload, params).execute } - it 'raises an error' do - expect { subject }.to raise_error(ArgumentError, 'Please provide params[:key]') + context 'with key filter' do + let!(:params) { { key: 'key1' } } + + it 'returns var1' do + expect(subject).to contain_exactly(var1) + end + end + + context 'with key and environment_scope filter' do + let!(:params) { { key: 'key2', filter: { environment_scope: 'staging' } } } + + it 'returns var2' do + expect(subject).to contain_exactly(var2) + end end end end - describe '#execute' do - subject { described_class.new(project.reload, params).execute } + context 'for a project' do + let(:owner) { create(:project) } - context 'with key filter' do - let!(:params) { { key: 'key1' } } + let!(:var1) { create(:ci_variable, project: owner, key: 'key1', environment_scope: 'staging') } + let!(:var2) { create(:ci_variable, project: owner, key: 'key2', environment_scope: 'staging') } + let!(:var3) { create(:ci_variable, project: owner, key: 'key2', environment_scope: 'production') } - it 'returns var1' do - expect(subject).to contain_exactly(var1) - end - end + include_examples 'scoped variables' + end - context 'with key and environment_scope filter' do - let!(:params) { { key: 'key2', filter: { environment_scope: 'staging' } } } + context 'for a group' do + let(:owner) { create(:group) } - it 'returns var2' do - expect(subject).to contain_exactly(var2) - end - end + let!(:var1) { create(:ci_group_variable, group: owner, key: 'key1', environment_scope: 'staging') } + let!(:var2) { create(:ci_group_variable, group: owner, key: 'key2', environment_scope: 'staging') } + let!(:var3) { create(:ci_group_variable, group: owner, key: 'key2', environment_scope: 'production') } + + include_examples 'scoped variables' end end diff --git a/spec/graphql/resolvers/ci/runner_platforms_resolver_spec.rb b/spec/graphql/resolvers/ci/runner_platforms_resolver_spec.rb index 1eb6f363d5b..3cb6e94e81e 100644 --- a/spec/graphql/resolvers/ci/runner_platforms_resolver_spec.rb +++ b/spec/graphql/resolvers/ci/runner_platforms_resolver_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Resolvers::Ci::RunnerPlatformsResolver do subject(:resolve_subject) { resolve(described_class) } it 'returns all possible runner platforms' do - expect(resolve_subject).to include( + expect(resolve_subject).to contain_exactly( hash_including(name: :linux), hash_including(name: :osx), hash_including(name: :windows), hash_including(name: :docker), hash_including(name: :kubernetes) diff --git a/spec/helpers/todos_helper_spec.rb b/spec/helpers/todos_helper_spec.rb index 9481d756c16..3787864e144 100644 --- a/spec/helpers/todos_helper_spec.rb +++ b/spec/helpers/todos_helper_spec.rb @@ -40,26 +40,6 @@ RSpec.describe TodosHelper do end end - describe '#todo_projects_options' do - let(:projects) { create_list(:project, 3) } - let(:user) { create(:user) } - - it 'returns users authorised projects in json format' do - projects.first.add_developer(user) - projects.second.add_developer(user) - - allow(helper).to receive(:current_user).and_return(user) - - expected_results = [ - { 'id' => '', 'text' => 'Any Project' }, - { 'id' => projects.second.id, 'text' => projects.second.full_name }, - { 'id' => projects.first.id, 'text' => projects.first.full_name } - ] - - expect(Gitlab::Json.parse(helper.todo_projects_options)).to match_array(expected_results) - end - end - describe '#todo_target_link' do context 'when given a design' do let(:todo) { design_todo } diff --git a/spec/lib/api/helpers/variables_helpers_spec.rb b/spec/lib/api/helpers/variables_helpers_spec.rb new file mode 100644 index 00000000000..de6bebaa827 --- /dev/null +++ b/spec/lib/api/helpers/variables_helpers_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Helpers::VariablesHelpers do + let(:helper) { Class.new.include(described_class).new } + + describe '#filter_variable_parameters' do + let(:project) { double } + let(:params) { double } + + subject { helper.filter_variable_parameters(project, params) } + + it 'returns unmodified params (overridden in EE)' do + expect(subject).to eq(params) + end + end + + describe '#find_variable' do + let(:owner) { double } + let(:params) { double } + let(:variables) { [double] } + + subject { helper.find_variable(owner, params) } + + before do + expect(Ci::VariablesFinder).to receive(:new).with(owner, params) + .and_return(double(execute: variables)) + end + + it { is_expected.to eq(variables.first) } + + context 'there are multiple variables with the supplied key' do + let(:variables) { [double, double] } + + it 'raises a conflict!' do + expect(helper).to receive(:conflict!).with(/There are multiple variables with provided parameters/) + + subject + end + end + end +end diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 0b6bf65ca44..6d5676bbe35 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -55,6 +55,7 @@ RSpec.describe API::GroupVariables do expect(json_response['value']).to eq(variable.value) expect(json_response['protected']).to eq(variable.protected?) expect(json_response['variable_type']).to eq(variable.variable_type) + expect(json_response['environment_scope']).to eq(variable.environment_scope) end it 'responds with 404 Not Found if requesting non-existing variable' do @@ -98,6 +99,7 @@ RSpec.describe API::GroupVariables do expect(json_response['protected']).to be_truthy expect(json_response['masked']).to be_truthy expect(json_response['variable_type']).to eq('env_var') + expect(json_response['environment_scope']).to eq('*') end it 'creates variable with optional attributes' do @@ -111,6 +113,7 @@ RSpec.describe API::GroupVariables do expect(json_response['protected']).to be_falsey expect(json_response['masked']).to be_falsey expect(json_response['variable_type']).to eq('file') + expect(json_response['environment_scope']).to eq('*') end it 'does not allow to duplicate variable key' do diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb index a8fda901b4a..c61ca159224 100644 --- a/spec/tooling/danger/project_helper_spec.rb +++ b/spec/tooling/danger/project_helper_spec.rb @@ -157,6 +157,9 @@ RSpec.describe Tooling::Danger::ProjectHelper do 'qa/foo' | [:qa] 'ee/qa/foo' | [:qa] + 'workhorse/main.go' | [:workhorse] + 'workhorse/internal/upload/upload.go' | [:workhorse] + 'changelogs/foo' | [:none] 'ee/changelogs/foo' | [:none] 'locale/gitlab.pot' | [:none] diff --git a/tooling/danger/project_helper.rb b/tooling/danger/project_helper.rb index 4458000261f..dd32835aba1 100644 --- a/tooling/danger/project_helper.rb +++ b/tooling/danger/project_helper.rb @@ -106,6 +106,8 @@ module Tooling %r{\A(ee/)?qa/} => :qa, + %r{\Aworkhorse/.*} => :workhorse, + # Files that don't fit into any category are marked with :none %r{\A(ee/)?changelogs/} => :none, %r{\Alocale/gitlab\.pot\z} => :none,