diff options
54 files changed, 1269 insertions, 312 deletions
diff --git a/.eslintrc b/.eslintrc index a86f8a841c3..3f187db0c07 100644 --- a/.eslintrc +++ b/.eslintrc @@ -35,15 +35,22 @@ "import/no-commonjs": "error", "no-multiple-empty-lines": ["error", { "max": 1 }], "promise/catch-or-return": "error", - "no-underscore-dangle": ["error", { "allow": ["__", "_links"]}], - "vue/html-self-closing": ["error", { - "html": { - "void": "always", - "normal": "never", - "component": "always" - }, - "svg": "always", - "math": "always" - }] + "no-underscore-dangle": ["error", { "allow": ["__", "_links"] }], + "no-mixed-operators": 0, + "space-before-function-paren": 0, + "curly": 0, + "arrow-parens": 0, + "vue/html-self-closing": [ + "error", + { + "html": { + "void": "always", + "normal": "never", + "component": "always" + }, + "svg": "always", + "math": "always" + } + ] } } diff --git a/.gitignore b/.gitignore index 90c6bd99d30..447fb71bd64 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ eslint-report.html /.gitlab_shell_secret .idea +/.vscode/* /.rbenv-version .rbx/ /.ruby-gemset diff --git a/.prettierrc b/.prettierrc new file mode 100644 index 00000000000..a20502b7f06 --- /dev/null +++ b/.prettierrc @@ -0,0 +1,4 @@ +{ + "singleQuote": true, + "trailingComma": "all" +} @@ -34,7 +34,7 @@ gem 'omniauth-gitlab', '~> 1.0.2' gem 'omniauth-google-oauth2', '~> 0.5.2' gem 'omniauth-kerberos', '~> 0.3.0', group: :kerberos gem 'omniauth-oauth2-generic', '~> 0.2.2' -gem 'omniauth-saml', '~> 1.7.0' +gem 'omniauth-saml', '~> 1.10.0' gem 'omniauth-shibboleth', '~> 1.2.0' gem 'omniauth-twitter', '~> 1.2.0' gem 'omniauth_crowd', '~> 2.2.0' @@ -104,7 +104,7 @@ gem 'carrierwave', '~> 1.2' gem 'dropzonejs-rails', '~> 0.7.1' # for backups -gem 'fog-aws', '~> 1.4' +gem 'fog-aws', '~> 2.0' gem 'fog-core', '~> 1.44' gem 'fog-google', '~> 0.5' gem 'fog-local', '~> 0.3' @@ -152,7 +152,7 @@ end gem 'state_machines-activerecord', '~> 0.4.0' # Issue tags -gem 'acts-as-taggable-on', '~> 4.0' +gem 'acts-as-taggable-on', '~> 5.0' # Background jobs gem 'sidekiq', '~> 5.0' @@ -164,7 +164,7 @@ gem 'sidekiq-limit_fetch', '~> 3.4', require: false gem 'rufus-scheduler', '~> 3.4' # HTTP requests -gem 'httparty', '~> 0.13.3' +gem 'httparty', '~> 0.15.6' # Colored output to console gem 'rainbow', '~> 2.2' @@ -264,7 +264,7 @@ gem 'virtus', '~> 1.0.1' gem 'base32', '~> 0.3.0' # Sentry integration -gem 'sentry-raven', '~> 2.5.3' +gem 'sentry-raven', '~> 2.7' gem 'premailer-rails', '~> 1.9.7' @@ -381,14 +381,14 @@ group :test do gem 'test-prof', '~> 0.2.5' end -gem 'octokit', '~> 4.6.2' +gem 'octokit', '~> 4.8' gem 'mail_room', '~> 0.9.1' gem 'email_reply_trimmer', '~> 0.1' gem 'html2text' -gem 'ruby-prof', '~> 0.16.2' +gem 'ruby-prof', '~> 0.17.0' # OAuth gem 'oauth2', '~> 1.4' @@ -401,7 +401,7 @@ gem 'vmstat', '~> 2.3.0' gem 'sys-filesystem', '~> 1.1.6' # SSH host key support -gem 'net-ssh', '~> 4.1.0' +gem 'net-ssh', '~> 4.2.0' gem 'sshkey', '~> 1.9.0' # Required for ED25519 SSH host key support diff --git a/Gemfile.lock b/Gemfile.lock index 86b4e5301d8..dbaf6a93ff8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -40,8 +40,8 @@ GEM minitest (~> 5.1) thread_safe (~> 0.3, >= 0.3.4) tzinfo (~> 1.1) - acts-as-taggable-on (4.0.0) - activerecord (>= 4.0) + acts-as-taggable-on (5.0.0) + activerecord (>= 4.2.8) adamantium (0.2.0) ice_nine (~> 0.11.0) memoizable (~> 0.4.0) @@ -194,7 +194,7 @@ GEM et-orbi (1.0.3) tzinfo eventmachine (1.0.8) - excon (0.57.1) + excon (0.60.0) execjs (2.6.0) expression_parser (0.9.0) factory_bot (4.8.2) @@ -233,14 +233,14 @@ GEM fog-json (~> 1.0) ipaddress (~> 0.8) xml-simple (~> 1.1) - fog-aws (1.4.0) + fog-aws (2.0.1) fog-core (~> 1.38) fog-json (~> 1.0) fog-xml (~> 0.1) ipaddress (~> 0.8) - fog-core (1.44.3) + fog-core (1.45.0) builder - excon (~> 0.49) + excon (~> 0.58) formatador (~> 0.2) fog-google (0.5.3) fog-core @@ -388,7 +388,7 @@ GEM thor tilt hashdiff (0.3.4) - hashie (3.5.6) + hashie (3.5.7) hashie-forbidden_attributes (0.1.1) hashie (>= 3.0) health_check (2.6.0) @@ -411,11 +411,10 @@ GEM domain_name (~> 0.5) http-form_data (1.0.1) http_parser.rb (0.6.0) - httparty (0.13.7) - json (~> 1.8) + httparty (0.15.7) multi_xml (>= 0.5.2) httpclient (2.8.2) - i18n (0.9.1) + i18n (0.9.5) concurrent-ruby (~> 1.0) ice_nine (0.11.2) influxdb (0.2.3) @@ -513,7 +512,7 @@ GEM mustermann (~> 1.0.0) mysql2 (0.4.10) net-ldap (0.16.0) - net-ssh (4.1.0) + net-ssh (4.2.0) netrc (0.11.0) nokogiri (1.8.2) mini_portile2 (~> 2.3.0) @@ -525,12 +524,12 @@ GEM multi_json (~> 1.3) multi_xml (~> 0.5) rack (>= 1.2, < 3) - octokit (4.6.2) + octokit (4.8.0) sawyer (~> 0.8.0, >= 0.5.3) oj (2.17.5) - omniauth (1.4.2) + omniauth (1.4.3) hashie (>= 1.2, < 4) - rack (>= 1.0, < 3) + rack (>= 1.6.2, < 3) omniauth-auth0 (1.4.1) omniauth-oauth2 (~> 1.1) omniauth-authentiq (0.3.1) @@ -569,9 +568,9 @@ GEM omniauth (~> 1.2) omniauth-oauth2-generic (0.2.2) omniauth-oauth2 (~> 1.0) - omniauth-saml (1.7.0) - omniauth (~> 1.3) - ruby-saml (~> 1.4) + omniauth-saml (1.10.0) + omniauth (~> 1.3, >= 1.3.2) + ruby-saml (~> 1.7) omniauth-shibboleth (1.2.1) omniauth (>= 1.0.0) omniauth-twitter (1.2.1) @@ -650,7 +649,7 @@ GEM pry (>= 0.9.10) public_suffix (3.0.2) pyu-ruby-sasl (0.0.3.3) - rack (1.6.8) + rack (1.6.9) rack-accept (0.4.5) rack (>= 0.4) rack-attack (4.4.1) @@ -803,9 +802,9 @@ GEM i18n ruby-fogbugz (0.2.1) crack (~> 0.4) - ruby-prof (0.16.2) + ruby-prof (0.17.0) ruby-progressbar (1.9.0) - ruby-saml (1.4.1) + ruby-saml (1.7.2) nokogiri (>= 1.5.10) ruby_parser (3.9.0) sexp_processor (~> 4.1) @@ -844,7 +843,7 @@ GEM selenium-webdriver (3.5.0) childprocess (~> 0.5) rubyzip (~> 1.0) - sentry-raven (2.5.3) + sentry-raven (2.7.2) faraday (>= 0.7.6, < 1.0) settingslogic (2.0.9) sexp_processor (4.9.0) @@ -933,7 +932,7 @@ GEM truncato (0.7.10) htmlentities (~> 4.3.1) nokogiri (~> 1.8.0, >= 1.7.0) - tzinfo (1.2.4) + tzinfo (1.2.5) thread_safe (~> 0.1) u2f (0.2.1) uber (0.1.0) @@ -994,7 +993,7 @@ DEPENDENCIES RedCloth (~> 4.3.2) ace-rails-ap (~> 4.1.0) activerecord_sane_schema_dumper (= 0.2) - acts-as-taggable-on (~> 4.0) + acts-as-taggable-on (~> 5.0) addressable (~> 2.5.2) akismet (~> 2.0) allocations (~> 1.0) @@ -1048,7 +1047,7 @@ DEPENDENCIES flipper-active_record (~> 0.11.0) flipper-active_support_cache_store (~> 0.11.0) fog-aliyun (~> 0.2.0) - fog-aws (~> 1.4) + fog-aws (~> 2.0) fog-core (~> 1.44) fog-google (~> 0.5) fog-local (~> 0.3) @@ -1086,7 +1085,7 @@ DEPENDENCIES hipchat (~> 1.5.0) html-pipeline (~> 1.11.0) html2text - httparty (~> 0.13.3) + httparty (~> 0.15.6) influxdb (~> 0.2) jira-ruby (~> 1.4) jquery-atwho-rails (~> 1.3.2) @@ -1107,10 +1106,10 @@ DEPENDENCIES mousetrap-rails (~> 1.4.6) mysql2 (~> 0.4.10) net-ldap - net-ssh (~> 4.1.0) + net-ssh (~> 4.2.0) nokogiri (~> 1.8.2) oauth2 (~> 1.4) - octokit (~> 4.6.2) + octokit (~> 4.8) oj (~> 2.17.4) omniauth (~> 1.4.2) omniauth-auth0 (~> 1.4.1) @@ -1123,7 +1122,7 @@ DEPENDENCIES omniauth-google-oauth2 (~> 0.5.2) omniauth-kerberos (~> 0.3.0) omniauth-oauth2-generic (~> 0.2.2) - omniauth-saml (~> 1.7.0) + omniauth-saml (~> 1.10.0) omniauth-shibboleth (~> 1.2.0) omniauth-twitter (~> 1.2.0) omniauth_crowd (~> 2.2.0) @@ -1173,7 +1172,7 @@ DEPENDENCIES rubocop (~> 0.52.1) rubocop-rspec (~> 1.22.1) ruby-fogbugz (~> 0.2.1) - ruby-prof (~> 0.16.2) + ruby-prof (~> 0.17.0) ruby_parser (~> 3.8) rufus-scheduler (~> 3.4) rugged (~> 0.26.0) @@ -1183,7 +1182,7 @@ DEPENDENCIES seed-fu (~> 2.3.7) select2-rails (~> 3.5.9) selenium-webdriver (~> 3.5) - sentry-raven (~> 2.5.3) + sentry-raven (~> 2.7) settingslogic (~> 2.0.9) sham_rack (~> 1.3.6) shoulda-matchers (~> 3.1.2) diff --git a/app/assets/javascripts/pages/ci/lints/create/index.js b/app/assets/javascripts/pages/ci/lints/new/index.js index 8e8a843da0b..8e8a843da0b 100644 --- a/app/assets/javascripts/pages/ci/lints/create/index.js +++ b/app/assets/javascripts/pages/ci/lints/new/index.js diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index b230b7f47ef..f8a3600e863 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -252,23 +252,23 @@ module Ci # All variables, including those dependent on environment, which could # contain unexpanded variables. def variables(environment: persisted_environment) - variables = predefined_variables - variables += project.predefined_variables - variables += pipeline.predefined_variables - variables += runner.predefined_variables if runner - variables += project.container_registry_variables - variables += project.deployment_variables if has_environment? - variables += project.auto_devops_variables - variables += yaml_variables - variables += user_variables - variables += project.group.secret_variables_for(ref, project).map(&:to_runner_variable) if project.group - variables += secret_variables(environment: environment) - variables += trigger_request.user_variables if trigger_request - variables += pipeline.variables.map(&:to_runner_variable) - variables += pipeline.pipeline_schedule.job_variables if pipeline.pipeline_schedule - variables += persisted_environment_variables if environment - - variables + collection = Gitlab::Ci::Variables::Collection.new.tap do |variables| + variables.concat(predefined_variables) + variables.concat(project.predefined_variables) + variables.concat(pipeline.predefined_variables) + variables.concat(runner.predefined_variables) if runner + variables.concat(project.deployment_variables(environment: environment)) if has_environment? + variables.concat(yaml_variables) + variables.concat(user_variables) + variables.concat(project.group.secret_variables_for(ref, project)) if project.group + variables.concat(secret_variables(environment: environment)) + variables.concat(trigger_request.user_variables) if trigger_request + variables.concat(pipeline.variables) + variables.concat(pipeline.pipeline_schedule.job_variables) if pipeline.pipeline_schedule + variables.concat(persisted_environment_variables) if environment + end + + collection.to_runner_variables end def features @@ -430,14 +430,14 @@ module Ci end def user_variables - return [] if user.blank? + Gitlab::Ci::Variables::Collection.new.tap do |variables| + return variables if user.blank? - [ - { key: 'GITLAB_USER_ID', value: user.id.to_s, public: true }, - { key: 'GITLAB_USER_EMAIL', value: user.email, public: true }, - { key: 'GITLAB_USER_LOGIN', value: user.username, public: true }, - { key: 'GITLAB_USER_NAME', value: user.name, public: true } - ] + variables.append(key: 'GITLAB_USER_ID', value: user.id.to_s) + variables.append(key: 'GITLAB_USER_EMAIL', value: user.email) + variables.append(key: 'GITLAB_USER_LOGIN', value: user.username) + variables.append(key: 'GITLAB_USER_NAME', value: user.name) + end end def secret_variables(environment: persisted_environment) @@ -540,60 +540,57 @@ module Ci CI_REGISTRY_USER = 'gitlab-ci-token'.freeze def predefined_variables - variables = [ - { key: 'CI', value: 'true', public: true }, - { key: 'GITLAB_CI', value: 'true', public: true }, - { key: 'GITLAB_FEATURES', value: project.namespace.features.join(','), public: true }, - { key: 'CI_SERVER_NAME', value: 'GitLab', public: true }, - { key: 'CI_SERVER_VERSION', value: Gitlab::VERSION, public: true }, - { key: 'CI_SERVER_REVISION', value: Gitlab::REVISION, public: true }, - { key: 'CI_JOB_ID', value: id.to_s, public: true }, - { key: 'CI_JOB_NAME', value: name, public: true }, - { key: 'CI_JOB_STAGE', value: stage, public: true }, - { key: 'CI_JOB_TOKEN', value: token, public: false }, - { key: 'CI_COMMIT_SHA', value: sha, public: true }, - { key: 'CI_COMMIT_REF_NAME', value: ref, public: true }, - { key: 'CI_COMMIT_REF_SLUG', value: ref_slug, public: true }, - { key: 'CI_REGISTRY_USER', value: CI_REGISTRY_USER, public: true }, - { key: 'CI_REGISTRY_PASSWORD', value: token, public: false }, - { key: 'CI_REPOSITORY_URL', value: repo_url, public: false } - ] - - variables << { key: "CI_COMMIT_TAG", value: ref, public: true } if tag? - variables << { key: "CI_PIPELINE_TRIGGERED", value: 'true', public: true } if trigger_request - variables << { key: "CI_JOB_MANUAL", value: 'true', public: true } if action? - variables.concat(legacy_variables) + Gitlab::Ci::Variables::Collection.new.tap do |variables| + variables.append(key: 'CI', value: 'true') + variables.append(key: 'GITLAB_CI', value: 'true') + variables.append(key: 'GITLAB_FEATURES', value: project.namespace.features.join(',')) + variables.append(key: 'CI_SERVER_NAME', value: 'GitLab') + variables.append(key: 'CI_SERVER_VERSION', value: Gitlab::VERSION) + variables.append(key: 'CI_SERVER_REVISION', value: Gitlab::REVISION) + variables.append(key: 'CI_JOB_ID', value: id.to_s) + variables.append(key: 'CI_JOB_NAME', value: name) + variables.append(key: 'CI_JOB_STAGE', value: stage) + variables.append(key: 'CI_JOB_TOKEN', value: token, public: false) + variables.append(key: 'CI_COMMIT_SHA', value: sha) + variables.append(key: 'CI_COMMIT_REF_NAME', value: ref) + variables.append(key: 'CI_COMMIT_REF_SLUG', value: ref_slug) + variables.append(key: 'CI_REGISTRY_USER', value: CI_REGISTRY_USER) + variables.append(key: 'CI_REGISTRY_PASSWORD', value: token, public: false) + variables.append(key: 'CI_REPOSITORY_URL', value: repo_url, public: false) + variables.append(key: "CI_COMMIT_TAG", value: ref) if tag? + variables.append(key: "CI_PIPELINE_TRIGGERED", value: 'true') if trigger_request + variables.append(key: "CI_JOB_MANUAL", value: 'true') if action? + variables.concat(legacy_variables) + end end def persisted_environment_variables - return [] unless persisted_environment + Gitlab::Ci::Variables::Collection.new.tap do |variables| + return variables unless persisted_environment - variables = persisted_environment.predefined_variables + variables.concat(persisted_environment.predefined_variables) - # Here we're passing unexpanded environment_url for runner to expand, - # and we need to make sure that CI_ENVIRONMENT_NAME and - # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded. - variables << { key: 'CI_ENVIRONMENT_URL', value: environment_url, public: true } if environment_url - - variables + # Here we're passing unexpanded environment_url for runner to expand, + # and we need to make sure that CI_ENVIRONMENT_NAME and + # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded. + variables.append(key: 'CI_ENVIRONMENT_URL', value: environment_url) if environment_url + end end def legacy_variables - variables = [ - { key: 'CI_BUILD_ID', value: id.to_s, public: true }, - { key: 'CI_BUILD_TOKEN', value: token, public: false }, - { key: 'CI_BUILD_REF', value: sha, public: true }, - { key: 'CI_BUILD_BEFORE_SHA', value: before_sha, public: true }, - { key: 'CI_BUILD_REF_NAME', value: ref, public: true }, - { key: 'CI_BUILD_REF_SLUG', value: ref_slug, public: true }, - { key: 'CI_BUILD_NAME', value: name, public: true }, - { key: 'CI_BUILD_STAGE', value: stage, public: true } - ] - - variables << { key: "CI_BUILD_TAG", value: ref, public: true } if tag? - variables << { key: "CI_BUILD_TRIGGERED", value: 'true', public: true } if trigger_request - variables << { key: "CI_BUILD_MANUAL", value: 'true', public: true } if action? - variables + Gitlab::Ci::Variables::Collection.new.tap do |variables| + variables.append(key: 'CI_BUILD_ID', value: id.to_s) + variables.append(key: 'CI_BUILD_TOKEN', value: token, public: false) + variables.append(key: 'CI_BUILD_REF', value: sha) + variables.append(key: 'CI_BUILD_BEFORE_SHA', value: before_sha) + variables.append(key: 'CI_BUILD_REF_NAME', value: ref) + variables.append(key: 'CI_BUILD_REF_SLUG', value: ref_slug) + variables.append(key: 'CI_BUILD_NAME', value: name) + variables.append(key: 'CI_BUILD_STAGE', value: stage) + variables.append(key: "CI_BUILD_TAG", value: ref) if tag? + variables.append(key: "CI_BUILD_TRIGGERED", value: 'true') if trigger_request + variables.append(key: "CI_BUILD_MANUAL", value: 'true') if action? + end end def environment_url diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index a72a815bfe8..4966ea62df9 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -473,11 +473,10 @@ module Ci end def predefined_variables - [ - { key: 'CI_PIPELINE_ID', value: id.to_s, public: true }, - { key: 'CI_CONFIG_PATH', value: ci_yaml_file_path, public: true }, - { key: 'CI_PIPELINE_SOURCE', value: source.to_s, public: true } - ] + Gitlab::Ci::Variables::Collection.new + .append(key: 'CI_PIPELINE_ID', value: id.to_s) + .append(key: 'CI_CONFIG_PATH', value: ci_yaml_file_path) + .append(key: 'CI_PIPELINE_SOURCE', value: source.to_s) end def queued_duration diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 609620a62bb..7173f88f1c7 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -132,11 +132,10 @@ module Ci end def predefined_variables - [ - { key: 'CI_RUNNER_ID', value: id.to_s, public: true }, - { key: 'CI_RUNNER_DESCRIPTION', value: description, public: true }, - { key: 'CI_RUNNER_TAGS', value: tag_list.to_s, public: true } - ] + Gitlab::Ci::Variables::Collection.new + .append(key: 'CI_RUNNER_ID', value: id.to_s) + .append(key: 'CI_RUNNER_DESCRIPTION', value: description) + .append(key: 'CI_RUNNER_TAGS', value: tag_list.to_s) end def tick_runner_queue diff --git a/app/models/clusters/platforms/kubernetes.rb b/app/models/clusters/platforms/kubernetes.rb index 7ce8befeeeb..015c8e9bcf8 100644 --- a/app/models/clusters/platforms/kubernetes.rb +++ b/app/models/clusters/platforms/kubernetes.rb @@ -56,19 +56,19 @@ module Clusters def predefined_variables config = YAML.dump(kubeconfig) - variables = [ - { key: 'KUBE_URL', value: api_url, public: true }, - { key: 'KUBE_TOKEN', value: token, public: false }, - { key: 'KUBE_NAMESPACE', value: actual_namespace, public: true }, - { key: 'KUBECONFIG', value: config, public: false, file: true } - ] - - if ca_pem.present? - variables << { key: 'KUBE_CA_PEM', value: ca_pem, public: true } - variables << { key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true } + Gitlab::Ci::Variables::Collection.new.tap do |variables| + variables + .append(key: 'KUBE_URL', value: api_url) + .append(key: 'KUBE_TOKEN', value: token, public: false) + .append(key: 'KUBE_NAMESPACE', value: actual_namespace) + .append(key: 'KUBECONFIG', value: config, public: false, file: true) + + if ca_pem.present? + variables + .append(key: 'KUBE_CA_PEM', value: ca_pem) + .append(key: 'KUBE_CA_PEM_FILE', value: ca_pem, file: true) + end end - - variables end # Constructs a list of terminals from the reactive cache diff --git a/app/models/concerns/storage/legacy_namespace.rb b/app/models/concerns/storage/legacy_namespace.rb index 67a988addbe..f05e606995d 100644 --- a/app/models/concerns/storage/legacy_namespace.rb +++ b/app/models/concerns/storage/legacy_namespace.rb @@ -7,29 +7,24 @@ module Storage raise Gitlab::UpdatePathError.new('Namespace cannot be moved, because at least one project has tags in container registry') end - expires_full_path_cache - - # Move the namespace directory in all storage paths used by member projects - repository_storage_paths.each do |repository_storage_path| - # Ensure old directory exists before moving it - gitlab_shell.add_namespace(repository_storage_path, full_path_was) - - # Ensure new directory exists before moving it (if there's a parent) - gitlab_shell.add_namespace(repository_storage_path, parent.full_path) if parent + parent_was = if parent_changed? && parent_id_was.present? + Namespace.find(parent_id_was) # raise NotFound early if needed + end - unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path) + expires_full_path_cache - Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}" + move_repositories - # if we cannot move namespace directory we should rollback - # db changes in order to prevent out of sync between db and fs - raise Gitlab::UpdatePathError.new('namespace directory cannot be moved') - end + if parent_changed? + former_parent_full_path = parent_was&.full_path + parent_full_path = parent&.full_path + Gitlab::UploadsTransfer.new.move_namespace(path, former_parent_full_path, parent_full_path) + Gitlab::PagesTransfer.new.move_namespace(path, former_parent_full_path, parent_full_path) + else + Gitlab::UploadsTransfer.new.rename_namespace(full_path_was, full_path) + Gitlab::PagesTransfer.new.rename_namespace(full_path_was, full_path) end - Gitlab::UploadsTransfer.new.rename_namespace(full_path_was, full_path) - Gitlab::PagesTransfer.new.rename_namespace(full_path_was, full_path) - remove_exports! # If repositories moved successfully we need to @@ -57,6 +52,26 @@ module Storage private + def move_repositories + # Move the namespace directory in all storage paths used by member projects + repository_storage_paths.each do |repository_storage_path| + # Ensure old directory exists before moving it + gitlab_shell.add_namespace(repository_storage_path, full_path_was) + + # Ensure new directory exists before moving it (if there's a parent) + gitlab_shell.add_namespace(repository_storage_path, parent.full_path) if parent + + unless gitlab_shell.mv_namespace(repository_storage_path, full_path_was, full_path) + + Rails.logger.error "Exception moving path #{repository_storage_path} from #{full_path_was} to #{full_path}" + + # if we cannot move namespace directory we should rollback + # db changes in order to prevent out of sync between db and fs + raise Gitlab::UpdatePathError.new('namespace directory cannot be moved') + end + end + end + def old_repository_storage_paths @old_repository_storage_paths ||= repository_storage_paths end diff --git a/app/models/environment.rb b/app/models/environment.rb index 2b0a88ac5b4..9517723d9d9 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -65,10 +65,9 @@ class Environment < ActiveRecord::Base end def predefined_variables - [ - { key: 'CI_ENVIRONMENT_NAME', value: name, public: true }, - { key: 'CI_ENVIRONMENT_SLUG', value: slug, public: true } - ] + Gitlab::Ci::Variables::Collection.new + .append(key: 'CI_ENVIRONMENT_NAME', value: name) + .append(key: 'CI_ENVIRONMENT_SLUG', value: slug) end def recently_updated_on_branch?(ref) diff --git a/app/models/member.rb b/app/models/member.rb index 36090676051..ec8156bbb01 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -55,7 +55,7 @@ class Member < ActiveRecord::Base scope :active_without_invites, -> do left_join_users .where(users: { state: 'active' }) - .where(requested_at: nil) + .non_request .reorder(nil) end diff --git a/app/models/project.rb b/app/models/project.rb index 0183e3d0a38..a291ad7eed5 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1572,29 +1572,30 @@ class Project < ActiveRecord::Base end def predefined_variables - [ - { key: 'CI_PROJECT_ID', value: id.to_s, public: true }, - { key: 'CI_PROJECT_NAME', value: path, public: true }, - { key: 'CI_PROJECT_PATH', value: full_path, public: true }, - { key: 'CI_PROJECT_PATH_SLUG', value: full_path_slug, public: true }, - { key: 'CI_PROJECT_NAMESPACE', value: namespace.full_path, public: true }, - { key: 'CI_PROJECT_URL', value: web_url, public: true }, - { key: 'CI_PROJECT_VISIBILITY', value: Gitlab::VisibilityLevel.string_level(visibility_level), public: true } - ] + visibility = Gitlab::VisibilityLevel.string_level(visibility_level) + + Gitlab::Ci::Variables::Collection.new + .append(key: 'CI_PROJECT_ID', value: id.to_s) + .append(key: 'CI_PROJECT_NAME', value: path) + .append(key: 'CI_PROJECT_PATH', value: full_path) + .append(key: 'CI_PROJECT_PATH_SLUG', value: full_path_slug) + .append(key: 'CI_PROJECT_NAMESPACE', value: namespace.full_path) + .append(key: 'CI_PROJECT_URL', value: web_url) + .append(key: 'CI_PROJECT_VISIBILITY', value: visibility) + .concat(container_registry_variables) + .concat(auto_devops_variables) end def container_registry_variables - return [] unless Gitlab.config.registry.enabled + Gitlab::Ci::Variables::Collection.new.tap do |variables| + return variables unless Gitlab.config.registry.enabled - variables = [ - { key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port, public: true } - ] + variables.append(key: 'CI_REGISTRY', value: Gitlab.config.registry.host_port) - if container_registry_enabled? - variables << { key: 'CI_REGISTRY_IMAGE', value: container_registry_url, public: true } + if container_registry_enabled? + variables.append(key: 'CI_REGISTRY_IMAGE', value: container_registry_url) + end end - - variables end def secret_variables_for(ref:, environment: nil) @@ -1614,16 +1615,14 @@ class Project < ActiveRecord::Base end end - def deployment_variables - return [] unless deployment_platform - - deployment_platform.predefined_variables + def deployment_variables(environment: nil) + deployment_platform(environment: environment)&.predefined_variables || [] end def auto_devops_variables return [] unless auto_devops_enabled? - (auto_devops || build_auto_devops)&.variables + (auto_devops || build_auto_devops)&.predefined_variables end def append_or_update_attribute(name, value) diff --git a/app/models/project_auto_devops.rb b/app/models/project_auto_devops.rb index 112ed7ed434..ed6c1eddbc1 100644 --- a/app/models/project_auto_devops.rb +++ b/app/models/project_auto_devops.rb @@ -14,9 +14,12 @@ class ProjectAutoDevops < ActiveRecord::Base domain.present? || instance_domain.present? end - def variables - variables = [] - variables << { key: 'AUTO_DEVOPS_DOMAIN', value: domain.presence || instance_domain, public: true } if has_domain? - variables + def predefined_variables + Gitlab::Ci::Variables::Collection.new.tap do |variables| + if has_domain? + variables.append(key: 'AUTO_DEVOPS_DOMAIN', + value: domain.presence || instance_domain) + end + end end end diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb index ad4ad7903ad..b4cd1c1a155 100644 --- a/app/models/project_services/kubernetes_service.rb +++ b/app/models/project_services/kubernetes_service.rb @@ -105,19 +105,19 @@ class KubernetesService < DeploymentService def predefined_variables config = YAML.dump(kubeconfig) - variables = [ - { key: 'KUBE_URL', value: api_url, public: true }, - { key: 'KUBE_TOKEN', value: token, public: false }, - { key: 'KUBE_NAMESPACE', value: actual_namespace, public: true }, - { key: 'KUBECONFIG', value: config, public: false, file: true } - ] - - if ca_pem.present? - variables << { key: 'KUBE_CA_PEM', value: ca_pem, public: true } - variables << { key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true } + Gitlab::Ci::Variables::Collection.new.tap do |variables| + variables + .append(key: 'KUBE_URL', value: api_url) + .append(key: 'KUBE_TOKEN', value: token, public: false) + .append(key: 'KUBE_NAMESPACE', value: actual_namespace) + .append(key: 'KUBECONFIG', value: config, public: false, file: true) + + if ca_pem.present? + variables + .append(key: 'KUBE_CA_PEM', value: ca_pem) + .append(key: 'KUBE_CA_PEM_FILE', value: ca_pem, file: true) + end end - - variables end # Constructs a list of terminals from the reactive cache diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index e07ecda27b5..ab94db2c1e5 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -208,9 +208,9 @@ class NotificationService def new_access_request(member) return true unless member.notifiable?(:subscription) - recipients = member.source.members.owners_and_masters + recipients = member.source.members.active_without_invites.owners_and_masters if fallback_to_group_owners_masters?(recipients, member) - recipients = member.source.group.members.owners_and_masters + recipients = member.source.group.members.active_without_invites.owners_and_masters end recipients.each { |recipient| deliver_access_request_email(recipient, member) } diff --git a/changelogs/unreleased/43806-update-ruby-saml-to-1-7-2.yml b/changelogs/unreleased/43806-update-ruby-saml-to-1-7-2.yml new file mode 100644 index 00000000000..7335d313510 --- /dev/null +++ b/changelogs/unreleased/43806-update-ruby-saml-to-1-7-2.yml @@ -0,0 +1,5 @@ +--- +title: Update ruby-saml to 1.7.2 and omniauth-saml to 1.10.0 +merge_request: 17734 +author: Takuya Noguchi +type: security diff --git a/changelogs/unreleased/adamco-gitlab-ce-move-issue-command.yml b/changelogs/unreleased/adamco-gitlab-ce-move-issue-command.yml new file mode 100644 index 00000000000..3b057373e7d --- /dev/null +++ b/changelogs/unreleased/adamco-gitlab-ce-move-issue-command.yml @@ -0,0 +1,5 @@ +--- +title: Add slash command for moving issues +merge_request: +author: Adam Pahlevi +type: added diff --git a/changelogs/unreleased/fix-42459---in-branch.yml b/changelogs/unreleased/fix-42459---in-branch.yml new file mode 100644 index 00000000000..26cc2046206 --- /dev/null +++ b/changelogs/unreleased/fix-42459---in-branch.yml @@ -0,0 +1,5 @@ +--- +title: Fix relative uri when "#" is in branch name +merge_request: +author: Jan +type: fixed diff --git a/changelogs/unreleased/mk-fix-move-upload-files-on-group-transfer.yml b/changelogs/unreleased/mk-fix-move-upload-files-on-group-transfer.yml new file mode 100644 index 00000000000..ba366b81600 --- /dev/null +++ b/changelogs/unreleased/mk-fix-move-upload-files-on-group-transfer.yml @@ -0,0 +1,5 @@ +--- +title: Fix missing uploads after group transfer +merge_request: 17658 +author: +type: fixed diff --git a/db/migrate/20180223120443_create_user_interacted_projects_table.rb b/db/migrate/20180223120443_create_user_interacted_projects_table.rb index 20749940b1e..8da8cf68088 100644 --- a/db/migrate/20180223120443_create_user_interacted_projects_table.rb +++ b/db/migrate/20180223120443_create_user_interacted_projects_table.rb @@ -3,13 +3,15 @@ class CreateUserInteractedProjectsTable < ActiveRecord::Migration DOWNTIME = false - disable_ddl_transaction! + INDEX_NAME = 'user_interacted_projects_non_unique_index' def up create_table :user_interacted_projects, id: false do |t| t.references :user, null: false t.references :project, null: false end + + add_index :user_interacted_projects, [:project_id, :user_id], name: INDEX_NAME end def down diff --git a/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb b/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb index 5e729b1aa53..d1a29a5c71b 100644 --- a/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb +++ b/db/post_migrate/20180223124427_build_user_interacted_projects_table.rb @@ -1,9 +1,14 @@ +require_relative '../migrate/20180223120443_create_user_interacted_projects_table.rb' +# rubocop:disable AddIndex +# rubocop:disable AddConcurrentForeignKey class BuildUserInteractedProjectsTable < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers # Set this constant to true if this migration requires downtime. DOWNTIME = false + UNIQUE_INDEX_NAME = 'index_user_interacted_projects_on_project_id_and_user_id' + disable_ddl_transaction! def up @@ -13,16 +18,8 @@ class BuildUserInteractedProjectsTable < ActiveRecord::Migration MysqlStrategy.new end.up - unless index_exists?(:user_interacted_projects, [:project_id, :user_id]) - add_concurrent_index :user_interacted_projects, [:project_id, :user_id], unique: true - end - - unless foreign_key_exists?(:user_interacted_projects, :user_id) - add_concurrent_foreign_key :user_interacted_projects, :users, column: :user_id, on_delete: :cascade - end - - unless foreign_key_exists?(:user_interacted_projects, :project_id) - add_concurrent_foreign_key :user_interacted_projects, :projects, column: :project_id, on_delete: :cascade + if index_exists_by_name?(:user_interacted_projects, CreateUserInteractedProjectsTable::INDEX_NAME) + remove_concurrent_index_by_name :user_interacted_projects, CreateUserInteractedProjectsTable::INDEX_NAME end end @@ -37,31 +34,16 @@ class BuildUserInteractedProjectsTable < ActiveRecord::Migration remove_foreign_key :user_interacted_projects, :projects end - if index_exists_by_name?(:user_interacted_projects, 'index_user_interacted_projects_on_project_id_and_user_id') - remove_concurrent_index_by_name :user_interacted_projects, 'index_user_interacted_projects_on_project_id_and_user_id' + if index_exists_by_name?(:user_interacted_projects, UNIQUE_INDEX_NAME) + remove_concurrent_index_by_name :user_interacted_projects, UNIQUE_INDEX_NAME end - end - private - - # Rails' index_exists? doesn't work when you only give it a table and index - # name. As such we have to use some extra code to check if an index exists for - # a given name. - def index_exists_by_name?(table, index) - indexes_for_table[table].include?(index) - end - - def indexes_for_table - @indexes_for_table ||= Hash.new do |hash, table_name| - hash[table_name] = indexes(table_name).map(&:name) + unless index_exists_by_name?(:user_interacted_projects, CreateUserInteractedProjectsTable::INDEX_NAME) + add_concurrent_index :user_interacted_projects, [:project_id, :user_id], name: CreateUserInteractedProjectsTable::INDEX_NAME end end - def foreign_key_exists?(table, column) - foreign_keys(table).any? do |key| - key.options[:column] == column.to_s - end - end + private class PostgresStrategy < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers @@ -71,33 +53,86 @@ class BuildUserInteractedProjectsTable < ActiveRecord::Migration def up with_index(:events, [:author_id, :project_id], name: 'events_user_interactions_temp', where: 'project_id IS NOT NULL') do - iteration = 0 - records = 0 - begin - Rails.logger.info "Building user_interacted_projects table, batch ##{iteration}" - result = execute <<~SQL + insert_missing_records + + # Do this once without lock to speed up the second invocation + remove_duplicates + with_table_lock(:user_interacted_projects) do + remove_duplicates + create_unique_index + end + + remove_without_project + with_table_lock(:user_interacted_projects, :projects) do + remove_without_project + create_fk :user_interacted_projects, :projects, :project_id + end + + remove_without_user + with_table_lock(:user_interacted_projects, :users) do + remove_without_user + create_fk :user_interacted_projects, :users, :user_id + end + end + + execute "ANALYZE user_interacted_projects" + end + + private + def insert_missing_records + iteration = 0 + records = 0 + begin + Rails.logger.info "Building user_interacted_projects table, batch ##{iteration}" + result = execute <<~SQL INSERT INTO user_interacted_projects (user_id, project_id) SELECT e.user_id, e.project_id FROM (SELECT DISTINCT author_id AS user_id, project_id FROM events WHERE project_id IS NOT NULL) AS e LEFT JOIN user_interacted_projects ucp USING (user_id, project_id) WHERE ucp.user_id IS NULL LIMIT #{BATCH_SIZE} - SQL - iteration += 1 - records += result.cmd_tuples - Rails.logger.info "Building user_interacted_projects table, batch ##{iteration} complete, created #{records} overall" - Kernel.sleep(SLEEP_TIME) if result.cmd_tuples > 0 - rescue ActiveRecord::InvalidForeignKey => e - Rails.logger.info "Retry on InvalidForeignKey: #{e}" - retry - end while result.cmd_tuples > 0 - end + SQL + iteration += 1 + records += result.cmd_tuples + Rails.logger.info "Building user_interacted_projects table, batch ##{iteration} complete, created #{records} overall" + Kernel.sleep(SLEEP_TIME) if result.cmd_tuples > 0 + end while result.cmd_tuples > 0 + end - execute "ANALYZE user_interacted_projects" + def remove_duplicates + execute <<~SQL + WITH numbered AS (select ctid, ROW_NUMBER() OVER (PARTITION BY (user_id, project_id)) as row_number, user_id, project_id from user_interacted_projects) + DELETE FROM user_interacted_projects WHERE ctid IN (SELECT ctid FROM numbered WHERE row_number > 1); + SQL + end + def remove_without_project + execute "DELETE FROM user_interacted_projects WHERE NOT EXISTS (SELECT 1 FROM projects WHERE id = user_interacted_projects.project_id)" end - private + def remove_without_user + execute "DELETE FROM user_interacted_projects WHERE NOT EXISTS (SELECT 1 FROM users WHERE id = user_interacted_projects.user_id)" + end + + def create_fk(table, target, column) + return if foreign_key_exists?(table, column) + + add_foreign_key table, target, column: column, on_delete: :cascade + end + + def create_unique_index + return if index_exists_by_name?(:user_interacted_projects, UNIQUE_INDEX_NAME) + + add_index :user_interacted_projects, [:project_id, :user_id], unique: true, name: UNIQUE_INDEX_NAME + end + + # Protect table against concurrent data changes while still allowing reads + def with_table_lock(*tables) + ActiveRecord::Base.connection.transaction do + execute "LOCK TABLE #{tables.join(", ")} IN SHARE MODE" + yield + end + end def with_index(*args) add_concurrent_index(*args) unless index_exists?(*args) @@ -118,7 +153,18 @@ class BuildUserInteractedProjectsTable < ActiveRecord::Migration LEFT JOIN user_interacted_projects ucp USING (user_id, project_id) WHERE ucp.user_id IS NULL SQL + + unless index_exists?(:user_interacted_projects, [:project_id, :user_id]) + add_concurrent_index :user_interacted_projects, [:project_id, :user_id], unique: true, name: UNIQUE_INDEX_NAME + end + + unless foreign_key_exists?(:user_interacted_projects, :user_id) + add_concurrent_foreign_key :user_interacted_projects, :users, column: :user_id, on_delete: :cascade + end + + unless foreign_key_exists?(:user_interacted_projects, :project_id) + add_concurrent_foreign_key :user_interacted_projects, :projects, column: :project_id, on_delete: :cascade + end end end - end diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index ae200f9b6e2..accf6340398 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -966,7 +966,7 @@ tag including only the files that are untracked by Git: ```yaml job: artifacts: - name: "${CI_JOB_NAME}_${CI_COMMIT_REF_NAME}" + name: "$CI_JOB_NAME-$CI_COMMIT_REF_NAME" untracked: true ``` @@ -975,7 +975,7 @@ To create an archive with a name of the current [stage](#stages) and branch name ```yaml job: artifacts: - name: "${CI_JOB_STAGE}_${CI_COMMIT_REF_NAME}" + name: "$CI_JOB_STAGE-$CI_COMMIT_REF_NAME" untracked: true ``` @@ -987,7 +987,7 @@ If you use **Windows Batch** to run your shell scripts you need to replace ```yaml job: artifacts: - name: "%CI_JOB_STAGE%_%CI_COMMIT_REF_NAME%" + name: "%CI_JOB_STAGE%-%CI_COMMIT_REF_NAME%" untracked: true ``` @@ -997,7 +997,7 @@ If you use **Windows PowerShell** to run your shell scripts you need to replace ```yaml job: artifacts: - name: "$env:CI_JOB_STAGE_$env:CI_COMMIT_REF_NAME" + name: "$env:CI_JOB_STAGE-$env:CI_COMMIT_REF_NAME" untracked: true ``` diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index b732cc65b73..960eabd5538 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -17,10 +17,13 @@ are very appreciative of the work done by translators and proofreaders! - French - Rรฉmy Coutable - [GitLab](https://gitlab.com/rymai), [Crowdin](https://crowdin.com/profile/rymai) - German +- Indonesian + - Ahmad Naufal Mukhtar - [GitLab](https://gitlab.com/anaufalm), [Crowdin](https://crowdin.com/profile/anaufalm) - Italian - Paolo Falomo - [GitLab](https://gitlab.com/paolofalomo), [Crowdin](https://crowdin.com/profile/paolo.falomo) - Japanese - Korean + - Chang-Ho Cha - [GitLab](https://gitlab.com/changho-cha), [Crowdin](https://crowdin.com/profile/zzazang) - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve) - Polish - Filip Mech - [GitLab](https://gitlab.com/mehenz), [Crowdin](https://crowdin.com/profile/mehenz) diff --git a/doc/development/new_fe_guide/principles.md b/doc/development/new_fe_guide/principles.md index 2126d202a7e..0af5f506a91 100644 --- a/doc/development/new_fe_guide/principles.md +++ b/doc/development/new_fe_guide/principles.md @@ -1,3 +1,35 @@ # Principles -> TODO: Add principles +These principles will ensure that your frontend contribution starts off in the right direction. + +## Discuss architecture before implementation + +Discuss your architecture design in an issue before writing code. This helps decrease the review time and also provides good practice for writing and thinking about system design. + +## Be consistent + +There are multiple ways of writing code to accomplish the same results. We should be as consistent as possible in how we write code across our codebases. This will make it more easier us to maintain our code across GitLab. + +## Enhance progressively + +Whenever you see with existing code that does not follow our current style guide, update it proactively. Refrain from changing everything but each merge request should progressively enhance our codebase and reduce technical debt. + +## When to use Vue + +- Use Vue for feature that make use of heavy DOM manipulation +- Use Vue for reusable components + +## When to use jQuery + +- Use jQuery to interact with Bootstrap JavaScript components +- Avoid jQuery when a better alternative exists. We are slowly moving away from it [#43559][jquery-future] + +## Mixing Vue and jQuery + +- Mixing Vue and jQuery is not recommended. +- If you need to use a specific jQuery plugin in Vue, [create a wrapper around it][select2]. +- It is acceptable for Vue to listen to existing jQuery events using jQuery event listeners. +- It is not recommended to add new jQuery events for Vue to interact with jQuery. + +[jquery-future]: https://gitlab.com/gitlab-org/gitlab-ce/issues/43559 +[select2]: https://vuejs.org/v2/examples/select2.html diff --git a/doc/development/new_fe_guide/style/html.md b/doc/development/new_fe_guide/style/html.md index 5489def5d6e..2d5b7d048ab 100644 --- a/doc/development/new_fe_guide/style/html.md +++ b/doc/development/new_fe_guide/style/html.md @@ -1,3 +1,53 @@ # HTML style guide -> TODO: Add content +## Buttons + +<a name="button-type"></a><a name="1.1"></a> +- [1.1](#button-type) **Use button type** Button tags requires a `type` attribute according to the [W3C HTML specification][button-type-spec]. + +``` +// bad +<button></button> + +// good +<button type="button"></button> +``` + +<a name="button-role"></a><a name="1.2"></a> +- [1.2](#button-role) **Use button role for non buttons** If an HTML element has an onClick handler but is not a button, it should have `role="button"`. This is more [accessible][button-role-accessible]. + +``` +// bad +<div onClick="doSomething"></div> + +// good +<div role="button" onClick="doSomething"></div> +``` + +## Links + +<a name="blank-links"></a><a name="2.1"></a> +- [2.1](#blank-links) **Use rel for target blank** Use `rel="noopener noreferrer"` whenever your links open in a new window i.e. `target="_blank"`. This prevents [the following][jitbit-target-blank] security vulnerability documented by JitBit + +``` +// bad +<a href="url" target="_blank"></a> + +// good +<a href="url" target="_blank" rel="noopener noreferrer"></a> +``` + +<a name="fake-links"></a><a name="2.2"></a> +- [2.2](#fake-links) **Do not use fake links** Use a button tag if a link only invokes JavaScript click event handlers. This is more semantic. + +``` +// bad +<a class="js-do-something" href="#"></a> + +// good +<button class="js-do-something" type="button"></button> +``` + +[button-type-spec]: https://www.w3.org/TR/2011/WD-html5-20110525/the-button-element.html#dom-button-type +[button-role-accessible]: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_button_role +[jitbit-target-blank]: https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/ diff --git a/doc/development/new_fe_guide/style/index.md b/doc/development/new_fe_guide/style/index.md index d2d576b3b46..ebee57bebbf 100644 --- a/doc/development/new_fe_guide/style/index.md +++ b/doc/development/new_fe_guide/style/index.md @@ -7,3 +7,9 @@ ## [JavaScript style guide](javascript.md) ## [Vue style guide](vue.md) + +# Tooling + +## [Prettier](prettier.md) + +Our code is automatically formatted with [Prettier](https://prettier.io) to follow our guidelines. diff --git a/doc/development/new_fe_guide/style/prettier.md b/doc/development/new_fe_guide/style/prettier.md new file mode 100644 index 00000000000..eb18189282b --- /dev/null +++ b/doc/development/new_fe_guide/style/prettier.md @@ -0,0 +1,45 @@ +# Formatting with Prettier + +Our code is automatically formatted with [Prettier](https://prettier.io) to follow our style guides. Prettier is taking care of formatting .js, .vue, and .scss files based on the standard prettier rules. You can find all settings for Prettier in `.prettierrc`. + +## Editor + +The easiest way to include prettier in your workflow is by setting up your preferred editor (all major editors are supported) accordingly. We suggest setting up prettier to run automatically when each file is saved. Find [here](https://prettier.io/docs/en/editors.html) the best way to set it up in your preferred editor. + +Please take care that you only let Prettier format the same file types as the global Yarn script does (.js, .vue, and .scss). In VSCode by example you can easily exclude file formats in your settings file: + +``` + "prettier.disableLanguages": [ + "json", + "markdown" + ], +``` + +## Yarn Script + +The following yarn scripts are available to do global formatting: + +``` +yarn prettier-staged-save +``` + +Updates all currently staged files (based on `git diff`) with Prettier and saves the needed changes. + +``` +yarn prettier-staged +``` +Checks all currently staged files (based on `git diff`) with Prettier and log which files would need manual updating to the console. + +``` +yarn prettier-all +``` + +Checks all files with Prettier and logs which files need manual updating to the console. + +``` +yarn prettier-all-save +``` + +Formats all files in the repository with Prettier. (This should only be used to test global rule updates otherwise you would end up with huge MR's). + +The source of these Yarn scripts can be found in `/scripts/frontend/prettier.js`. diff --git a/doc/integration/slash_commands.md b/doc/integration/slash_commands.md index 36a8844e953..7d73026a6c6 100644 --- a/doc/integration/slash_commands.md +++ b/doc/integration/slash_commands.md @@ -15,9 +15,10 @@ Taking the trigger term as `project-name`, the commands are: | `/project-name issue new <title> <shift+return> <description>` | Creates a new issue with title `<title>` and description `<description>` | | `/project-name issue show <id>` | Shows the issue with id `<id>` | | `/project-name issue search <query>` | Shows up to 5 issues matching `<query>` | +| `/project-name issue move <id> to <project>` | Moves issue ID `<id>` to `<project>` | | `/project-name deploy <from> to <to>` | Deploy from the `<from>` environment to the `<to>` environment | -Note that if you are using the [GitLab Slack application](https://docs.gitlab.com/ee/user/project/integrations/gitlab_slack_application.html) for +Note that if you are using the [GitLab Slack application](https://docs.gitlab.com/ee/user/project/integrations/gitlab_slack_application.html) for your GitLab.com projects, you need to [add the `gitlab` keyword at the beginning of the command](https://docs.gitlab.com/ee/user/project/integrations/gitlab_slack_application.html#usage). ## Issue commands diff --git a/lib/banzai/filter/relative_link_filter.rb b/lib/banzai/filter/relative_link_filter.rb index 9bdedeb6615..262458a872a 100644 --- a/lib/banzai/filter/relative_link_filter.rb +++ b/lib/banzai/filter/relative_link_filter.rb @@ -84,7 +84,7 @@ module Banzai relative_url_root, project.full_path, uri_type(file_path), - Addressable::URI.escape(ref), + Addressable::URI.escape(ref).gsub('#', '%23'), Addressable::URI.escape(file_path) ].compact.join('/').squeeze('/').chomp('/') diff --git a/lib/gitlab/ci/variables/collection.rb b/lib/gitlab/ci/variables/collection.rb new file mode 100644 index 00000000000..0deca55fe8f --- /dev/null +++ b/lib/gitlab/ci/variables/collection.rb @@ -0,0 +1,38 @@ +module Gitlab + module Ci + module Variables + class Collection + include Enumerable + + def initialize(variables = []) + @variables = [] + + variables.each { |variable| self.append(variable) } + end + + def append(resource) + tap { @variables.append(Collection::Item.fabricate(resource)) } + end + + def concat(resources) + tap { resources.each { |variable| self.append(variable) } } + end + + def each + @variables.each { |variable| yield variable } + end + + def +(other) + self.class.new.tap do |collection| + self.each { |variable| collection.append(variable) } + other.each { |variable| collection.append(variable) } + end + end + + def to_runner_variables + self.map(&:to_hash) + end + end + end + end +end diff --git a/lib/gitlab/ci/variables/collection/item.rb b/lib/gitlab/ci/variables/collection/item.rb new file mode 100644 index 00000000000..939912981e6 --- /dev/null +++ b/lib/gitlab/ci/variables/collection/item.rb @@ -0,0 +1,50 @@ +module Gitlab + module Ci + module Variables + class Collection + class Item + def initialize(**options) + @variable = { + key: options.fetch(:key), + value: options.fetch(:value), + public: options.fetch(:public, true), + file: options.fetch(:files, false) + } + end + + def [](key) + @variable.fetch(key) + end + + def ==(other) + to_hash == self.class.fabricate(other).to_hash + end + + ## + # If `file: true` has been provided we expose it, otherwise we + # don't expose `file` attribute at all (stems from what the runner + # expects). + # + def to_hash + @variable.reject do |hash_key, hash_value| + hash_key == :file && hash_value == false + end + end + + def self.fabricate(resource) + case resource + when Hash + self.new(resource) + when ::HasVariable + self.new(resource.to_runner_variable) + when self + resource.dup + else + raise ArgumentError, "Unknown `#{resource.class}` variable resource!" + end + end + end + end + end + end +end diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index dbe6259fce7..21287a8efd0 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -859,6 +859,19 @@ into similar problems in the future (e.g. when new tables are created). BackgroundMigrationWorker.perform_in(delay_interval * index, job_class_name, [start_id, end_id]) end end + + def foreign_key_exists?(table, column) + foreign_keys(table).any? do |key| + key.options[:column] == column.to_s + end + end + + # Rails' index_exists? doesn't work when you only give it a table and index + # name. As such we have to use some extra code to check if an index exists for + # a given name. + def index_exists_by_name?(table, index) + indexes(table).map(&:name).include?(index) + end end end end diff --git a/lib/gitlab/project_transfer.rb b/lib/gitlab/project_transfer.rb index 1bba0b78e2f..690c38737c0 100644 --- a/lib/gitlab/project_transfer.rb +++ b/lib/gitlab/project_transfer.rb @@ -1,13 +1,19 @@ module Gitlab + # This class is used to move local, unhashed files owned by projects to their new location class ProjectTransfer - def move_project(project_path, namespace_path_was, namespace_path) - new_namespace_folder = File.join(root_dir, namespace_path) - FileUtils.mkdir_p(new_namespace_folder) unless Dir.exist?(new_namespace_folder) - from = File.join(root_dir, namespace_path_was, project_path) - to = File.join(root_dir, namespace_path, project_path) + # nil parent_path (or parent_path_was) represents a root namespace + def move_namespace(path, parent_path_was, parent_path) + parent_path_was ||= '' + parent_path ||= '' + new_parent_folder = File.join(root_dir, parent_path) + FileUtils.mkdir_p(new_parent_folder) + from = File.join(root_dir, parent_path_was, path) + to = File.join(root_dir, parent_path, path) move(from, to, "") end + alias_method :move_project, :move_namespace + def rename_project(path_was, path, namespace_path) base_dir = File.join(root_dir, namespace_path) move(path_was, path, base_dir) diff --git a/lib/gitlab/slash_commands/command.rb b/lib/gitlab/slash_commands/command.rb index 85aaa6b0eba..bb778f37096 100644 --- a/lib/gitlab/slash_commands/command.rb +++ b/lib/gitlab/slash_commands/command.rb @@ -5,6 +5,7 @@ module Gitlab Gitlab::SlashCommands::IssueShow, Gitlab::SlashCommands::IssueNew, Gitlab::SlashCommands::IssueSearch, + Gitlab::SlashCommands::IssueMove, Gitlab::SlashCommands::Deploy ].freeze diff --git a/lib/gitlab/slash_commands/issue_move.rb b/lib/gitlab/slash_commands/issue_move.rb new file mode 100644 index 00000000000..3985e635983 --- /dev/null +++ b/lib/gitlab/slash_commands/issue_move.rb @@ -0,0 +1,45 @@ +module Gitlab + module SlashCommands + class IssueMove < IssueCommand + def self.match(text) + %r{ + \A # the beginning of a string + issue\s+move\s+ # the command + \#?(?<iid>\d+)\s+ # the issue id, may preceded by hash sign + (to\s+)? # aid the command to be much more human-ly + (?<project_path>[^\s]+) # named group for id of dest. project + }x.match(text) + end + + def self.help_message + 'issue move <issue_id> (to)? <project_path>' + end + + def self.allowed?(project, user) + can?(user, :admin_issue, project) + end + + def execute(match) + old_issue = find_by_iid(match[:iid]) + target_project = Project.find_by_full_path(match[:project_path]) + + unless current_user.can?(:read_project, target_project) && old_issue + return Gitlab::SlashCommands::Presenters::Access.new.not_found + end + + new_issue = Issues::MoveService.new(project, current_user) + .execute(old_issue, target_project) + + presenter(new_issue).present(old_issue) + rescue Issues::MoveService::MoveError => e + presenter(old_issue).display_move_error(e.message) + end + + private + + def presenter(issue) + Gitlab::SlashCommands::Presenters::IssueMove.new(issue) + end + end + end +end diff --git a/lib/gitlab/slash_commands/presenters/issue_move.rb b/lib/gitlab/slash_commands/presenters/issue_move.rb new file mode 100644 index 00000000000..03921729941 --- /dev/null +++ b/lib/gitlab/slash_commands/presenters/issue_move.rb @@ -0,0 +1,53 @@ +# coding: utf-8 +module Gitlab + module SlashCommands + module Presenters + class IssueMove < Presenters::Base + include Presenters::IssueBase + + def present(old_issue) + in_channel_response(moved_issue(old_issue)) + end + + def display_move_error(error) + message = header_with_list("The action was not successful, because:", [error]) + + ephemeral_response(text: message) + end + + private + + def moved_issue(old_issue) + { + attachments: [ + { + title: "#{@resource.title} ยท #{@resource.to_reference}", + title_link: resource_url, + author_name: author.name, + author_icon: author.avatar_url, + fallback: "Issue #{@resource.to_reference}: #{@resource.title}", + pretext: pretext(old_issue), + color: color(@resource), + fields: fields, + mrkdwn_in: [ + :title, + :pretext, + :text, + :fields + ] + } + ] + } + end + + def pretext(old_issue) + "Moved issue *#{issue_link(old_issue)}* to *#{issue_link(@resource)}*" + end + + def issue_link(issue) + "[#{issue.to_reference}](#{project_issue_url(issue.project, issue)})" + end + end + end + end +end diff --git a/package.json b/package.json index 472bdbebda8..deee668ae3b 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,10 @@ "karma": "karma start config/karma.config.js --single-run", "karma-coverage": "BABEL_ENV=coverage karma start config/karma.config.js --single-run", "karma-start": "karma start config/karma.config.js", + "prettier-staged": "node ./scripts/frontend/prettier.js", + "prettier-staged-save": "node ./scripts/frontend/prettier.js save", + "prettier-all": "node ./scripts/frontend/prettier.js check-all", + "prettier-all-save": "node ./scripts/frontend/prettier.js save-all", "webpack": "webpack --config config/webpack.config.js", "webpack-prod": "NODE_ENV=production webpack --config config/webpack.config.js" }, @@ -114,7 +118,7 @@ "karma-sourcemap-loader": "^0.3.7", "karma-webpack": "2.0.7", "nodemon": "^1.15.1", - "prettier": "1.9.2", + "prettier": "1.11.1", "webpack-dev-server": "^2.11.2" } } diff --git a/scripts/frontend/frontend_script_utils.js b/scripts/frontend/frontend_script_utils.js new file mode 100644 index 00000000000..2c06747255c --- /dev/null +++ b/scripts/frontend/frontend_script_utils.js @@ -0,0 +1,30 @@ +/* eslint import/no-commonjs: "off" */ +const execFileSync = require('child_process').execFileSync; + +const exec = (command, args) => { + const options = { + cwd: process.cwd(), + env: process.env, + encoding: 'utf-8', + }; + return execFileSync(command, args, options); +}; + +const execGitCmd = args => + exec('git', args) + .trim() + .toString() + .split('\n'); + +module.exports = { + getStagedFiles: fileExtensionFilter => { + const gitOptions = [ + 'diff', + '--name-only', + '--cached', + '--diff-filter=ACMRTUB', + ]; + if (fileExtensionFilter) gitOptions.push(...fileExtensionFilter); + return execGitCmd(gitOptions); + }, +}; diff --git a/scripts/frontend/prettier.js b/scripts/frontend/prettier.js new file mode 100644 index 00000000000..863572bf64d --- /dev/null +++ b/scripts/frontend/prettier.js @@ -0,0 +1,103 @@ +/* eslint import/no-commonjs: "off", import/no-extraneous-dependencies: "off", no-console: "off" */ +const glob = require('glob'); +const prettier = require('prettier'); +const fs = require('fs'); + +const getStagedFiles = require('./frontend_script_utils').getStagedFiles; + +const mode = process.argv[2] || 'check'; +const shouldSave = mode === 'save' || mode === 'save-all'; +const allFiles = mode === 'check-all' || mode === 'save-all'; + +const config = { + patterns: ['**/*.js', '**/*.vue', '**/*.scss'], + ignore: ['**/node_modules/**', '**/vendor/**', '**/public/**'], + parsers: { + js: 'babylon', + vue: 'vue', + scss: 'css', + }, +}; +const availableExtensions = Object.keys(config.parsers); + +console.log(`Loading ${allFiles ? 'All' : 'Staged'} Files ...`); + +const stagedFiles = allFiles + ? null + : getStagedFiles(availableExtensions.map(ext => `*.${ext}`)); + +if (stagedFiles) { + if (!stagedFiles.length || (stagedFiles.length === 1 && !stagedFiles[0])) { + console.log('No matching staged files.'); + return; + } + console.log(`Matching staged Files : ${stagedFiles.length}`); +} + +let didWarn = false; +let didError = false; + +let files; +if (allFiles) { + const ignore = config.ignore; + const patterns = config.patterns; + const globPattern = + patterns.length > 1 ? `{${patterns.join(',')}}` : `${patterns.join(',')}`; + files = glob + .sync(globPattern, { ignore }) + .filter(f => allFiles || stagedFiles.includes(f)); +} else { + files = stagedFiles.filter(f => + availableExtensions.includes(f.split('.').pop()), + ); +} + +if (!files.length) { + console.log('No Files found to process with Prettier'); + return; +} + +console.log(`${shouldSave ? 'Updating' : 'Checking'} ${files.length} file(s)`); + +prettier + .resolveConfig('.') + .then(options => { + console.log('Found options : ', options); + files.forEach(file => { + try { + const fileExtension = file.split('.').pop(); + Object.assign(options, { + parser: config.parsers[fileExtension], + }); + + const input = fs.readFileSync(file, 'utf8'); + + if (shouldSave) { + const output = prettier.format(input, options); + if (output !== input) { + fs.writeFileSync(file, output, 'utf8'); + console.log(`Prettified : ${file}`); + } + } else if (!prettier.check(input, options)) { + if (!didWarn) { + console.log( + '\n===============================\nGitLab uses Prettier to format all JavaScript code.\nPlease format each file listed below or run "yarn prettier-staged-save"\n===============================\n', + ); + didWarn = true; + } + console.log(`Prettify Manually : ${file}`); + } + } catch (error) { + didError = true; + console.log(`\n\nError with ${file}: ${error.message}`); + } + }); + + if (didWarn || didError) { + process.exit(1); + } + }) + .catch(e => { + console.log(`Error on loading the Config File: ${e.message}`); + process.exit(1); + }); diff --git a/spec/features/ci_lint_spec.rb b/spec/features/ci_lint_spec.rb index b1dceec9da8..220b934154e 100644 --- a/spec/features/ci_lint_spec.rb +++ b/spec/features/ci_lint_spec.rb @@ -39,6 +39,7 @@ describe 'CI Lint', :js do it 'displays information about an error' do expect(page).to have_content('Status: syntax is incorrect') + expect(page).to have_selector('.ace_content', text: yaml_content) end end diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb index 3ca4652f7cc..ba8dc68ceda 100644 --- a/spec/lib/banzai/filter/relative_link_filter_spec.rb +++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb @@ -217,6 +217,23 @@ describe Banzai::Filter::RelativeLinkFilter do end end + context 'when ref name contains special chars' do + let(:ref) {'mark#\'@],+;-._/#@!$&()+down'} + + it 'correctly escapes the ref' do + # Adressable won't escape the '#', so we do this manually + ref_escaped = 'mark%23\'@%5D,+;-._/%23@!$&()+down' + + # Stub this method so the branch doesn't actually need to be in the repo + allow_any_instance_of(described_class).to receive(:uri_type).and_return(:raw) + + doc = filter(link('files/images/logo-black.png')) + + expect(doc.at_css('a')['href']) + .to eq "/#{project_path}/raw/#{ref_escaped}/files/images/logo-black.png" + end + end + context 'when requested path is a directory with space in the repo' do let(:ref) { 'master' } let(:commit) { project.commit('38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e') } diff --git a/spec/lib/gitlab/ci/variables/collection/item_spec.rb b/spec/lib/gitlab/ci/variables/collection/item_spec.rb new file mode 100644 index 00000000000..cc1257484d2 --- /dev/null +++ b/spec/lib/gitlab/ci/variables/collection/item_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +describe Gitlab::Ci::Variables::Collection::Item do + let(:variable) do + { key: 'VAR', value: 'something', public: true } + end + + describe '.fabricate' do + it 'supports using a hash' do + resource = described_class.fabricate(variable) + + expect(resource).to be_a(described_class) + expect(resource).to eq variable + end + + it 'supports using an active record resource' do + variable = create(:ci_variable, key: 'CI_VAR', value: '123') + resource = described_class.fabricate(variable) + + expect(resource).to be_a(described_class) + expect(resource).to eq(key: 'CI_VAR', value: '123', public: false) + end + + it 'supports using another collection item' do + item = described_class.new(**variable) + + resource = described_class.fabricate(item) + + expect(resource).to be_a(described_class) + expect(resource).to eq variable + expect(resource.object_id).not_to eq item.object_id + end + end + + describe '#==' do + it 'compares a hash representation of a variable' do + expect(described_class.new(**variable) == variable).to be true + end + end + + describe '#[]' do + it 'behaves like a hash accessor' do + item = described_class.new(**variable) + + expect(item[:key]).to eq 'VAR' + end + end + + describe '#to_hash' do + it 'returns a hash representation of a collection item' do + expect(described_class.new(**variable).to_hash).to eq variable + end + end +end diff --git a/spec/lib/gitlab/ci/variables/collection_spec.rb b/spec/lib/gitlab/ci/variables/collection_spec.rb new file mode 100644 index 00000000000..90b6e178242 --- /dev/null +++ b/spec/lib/gitlab/ci/variables/collection_spec.rb @@ -0,0 +1,99 @@ +require 'spec_helper' + +describe Gitlab::Ci::Variables::Collection do + describe '.new' do + it 'can be initialized with an array' do + variable = { key: 'VAR', value: 'value', public: true } + + collection = described_class.new([variable]) + + expect(collection.first.to_hash).to eq variable + end + + it 'can be initialized without an argument' do + expect(subject).to be_none + end + end + + describe '#append' do + it 'appends a hash' do + subject.append(key: 'VARIABLE', value: 'something') + + expect(subject).to be_one + end + + it 'appends a Ci::Variable' do + subject.append(build(:ci_variable)) + + expect(subject).to be_one + end + + it 'appends an internal resource' do + collection = described_class.new([{ key: 'TEST', value: 1 }]) + + subject.append(collection.first) + + expect(subject).to be_one + end + + it 'returns self' do + expect(subject.append(key: 'VAR', value: 'test')) + .to eq subject + end + end + + describe '#concat' do + it 'appends all elements from an array' do + collection = described_class.new([{ key: 'VAR_1', value: '1' }]) + variables = [{ key: 'VAR_2', value: '2' }, { key: 'VAR_3', value: '3' }] + + collection.concat(variables) + + expect(collection).to include(key: 'VAR_1', value: '1', public: true) + expect(collection).to include(key: 'VAR_2', value: '2', public: true) + expect(collection).to include(key: 'VAR_3', value: '3', public: true) + end + + it 'appends all elements from other collection' do + collection = described_class.new([{ key: 'VAR_1', value: '1' }]) + additional = described_class.new([{ key: 'VAR_2', value: '2' }, + { key: 'VAR_3', value: '3' }]) + + collection.concat(additional) + + expect(collection).to include(key: 'VAR_1', value: '1', public: true) + expect(collection).to include(key: 'VAR_2', value: '2', public: true) + expect(collection).to include(key: 'VAR_3', value: '3', public: true) + end + + it 'returns self' do + expect(subject.concat([key: 'VAR', value: 'test'])) + .to eq subject + end + end + + describe '#+' do + it 'makes it possible to combine with an array' do + collection = described_class.new([{ key: 'TEST', value: 1 }]) + variables = [{ key: 'TEST', value: 'something' }] + + expect((collection + variables).count).to eq 2 + end + + it 'makes it possible to combine with another collection' do + collection = described_class.new([{ key: 'TEST', value: 1 }]) + other = described_class.new([{ key: 'TEST', value: 2 }]) + + expect((collection + other).count).to eq 2 + end + end + + describe '#to_runner_variables' do + it 'creates an array of hashes in a runner-compatible format' do + collection = described_class.new([{ key: 'TEST', value: 1 }]) + + expect(collection.to_runner_variables) + .to eq [{ key: 'TEST', value: 1, public: true }] + end + end +end diff --git a/spec/lib/gitlab/project_transfer_spec.rb b/spec/lib/gitlab/project_transfer_spec.rb index 10c5fb148cd..0b9b1f537b5 100644 --- a/spec/lib/gitlab/project_transfer_spec.rb +++ b/spec/lib/gitlab/project_transfer_spec.rb @@ -21,30 +21,77 @@ describe Gitlab::ProjectTransfer do describe '#move_project' do it "moves project upload to another namespace" do - FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path)) + path_to_be_moved = File.join(@root_dir, @namespace_path_was, @project_path) + expected_path = File.join(@root_dir, @namespace_path, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + @project_transfer.move_project(@project_path, @namespace_path_was, @namespace_path) - expected_path = File.join(@root_dir, @namespace_path, @project_path) expect(Dir.exist?(expected_path)).to be_truthy end end + describe '#move_namespace' do + context 'when moving namespace from root into another namespace' do + it "moves namespace projects' upload" do + child_namespace = 'test_child_namespace' + path_to_be_moved = File.join(@root_dir, child_namespace, @project_path) + expected_path = File.join(@root_dir, @namespace_path, child_namespace, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + + @project_transfer.move_namespace(child_namespace, nil, @namespace_path) + + expect(Dir.exist?(expected_path)).to be_truthy + end + end + + context 'when moving namespace from one parent to another' do + it "moves namespace projects' upload" do + child_namespace = 'test_child_namespace' + path_to_be_moved = File.join(@root_dir, @namespace_path_was, child_namespace, @project_path) + expected_path = File.join(@root_dir, @namespace_path, child_namespace, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + + @project_transfer.move_namespace(child_namespace, @namespace_path_was, @namespace_path) + + expect(Dir.exist?(expected_path)).to be_truthy + end + end + + context 'when moving namespace from having a parent to root' do + it "moves namespace projects' upload" do + child_namespace = 'test_child_namespace' + path_to_be_moved = File.join(@root_dir, @namespace_path_was, child_namespace, @project_path) + expected_path = File.join(@root_dir, child_namespace, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + + @project_transfer.move_namespace(child_namespace, @namespace_path_was, nil) + + expect(Dir.exist?(expected_path)).to be_truthy + end + end + end + describe '#rename_project' do it "renames project" do - FileUtils.mkdir_p(File.join(@root_dir, @namespace_path, @project_path_was)) + path_to_be_moved = File.join(@root_dir, @namespace_path, @project_path_was) + expected_path = File.join(@root_dir, @namespace_path, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + @project_transfer.rename_project(@project_path_was, @project_path, @namespace_path) - expected_path = File.join(@root_dir, @namespace_path, @project_path) expect(Dir.exist?(expected_path)).to be_truthy end end describe '#rename_namespace' do it "renames namespace" do - FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path)) + path_to_be_moved = File.join(@root_dir, @namespace_path_was, @project_path) + expected_path = File.join(@root_dir, @namespace_path, @project_path) + FileUtils.mkdir_p(path_to_be_moved) + @project_transfer.rename_namespace(@namespace_path_was, @namespace_path) - expected_path = File.join(@root_dir, @namespace_path, @project_path) expect(Dir.exist?(expected_path)).to be_truthy end end diff --git a/spec/lib/gitlab/slash_commands/command_spec.rb b/spec/lib/gitlab/slash_commands/command_spec.rb index e3447d974aa..194cae8c645 100644 --- a/spec/lib/gitlab/slash_commands/command_spec.rb +++ b/spec/lib/gitlab/slash_commands/command_spec.rb @@ -108,5 +108,10 @@ describe Gitlab::SlashCommands::Command do it { is_expected.to eq(Gitlab::SlashCommands::IssueSearch) } end + + context 'IssueMove is triggered' do + let(:params) { { text: 'issue move #78291 to gitlab/gitlab-ci' } } + it { is_expected.to eq(Gitlab::SlashCommands::IssueMove) } + end end end diff --git a/spec/lib/gitlab/slash_commands/issue_move_spec.rb b/spec/lib/gitlab/slash_commands/issue_move_spec.rb new file mode 100644 index 00000000000..d41441c9472 --- /dev/null +++ b/spec/lib/gitlab/slash_commands/issue_move_spec.rb @@ -0,0 +1,117 @@ +require 'spec_helper' + +describe Gitlab::SlashCommands::IssueMove, service: true do + describe '#match' do + shared_examples_for 'move command' do |text_command| + it 'can be parsed to extract the needed fields' do + match_data = described_class.match(text_command) + + expect(match_data['iid']).to eq('123456') + expect(match_data['project_path']).to eq('gitlab/gitlab-ci') + end + end + + it_behaves_like 'move command', 'issue move #123456 to gitlab/gitlab-ci' + it_behaves_like 'move command', 'issue move #123456 gitlab/gitlab-ci' + it_behaves_like 'move command', 'issue move #123456 gitlab/gitlab-ci ' + it_behaves_like 'move command', 'issue move 123456 to gitlab/gitlab-ci' + it_behaves_like 'move command', 'issue move 123456 gitlab/gitlab-ci' + it_behaves_like 'move command', 'issue move 123456 gitlab/gitlab-ci ' + end + + describe '#execute' do + set(:user) { create(:user) } + set(:issue) { create(:issue) } + set(:chat_name) { create(:chat_name, user: user) } + set(:project) { issue.project } + set(:other_project) { create(:project, namespace: project.namespace) } + + before do + [project, other_project].each { |prj| prj.add_master(user) } + end + + subject { described_class.new(project, chat_name) } + + def process_message(message) + subject.execute(described_class.match(message)) + end + + context 'when the user can move the issue' do + context 'when the move fails' do + it 'returns the error message' do + message = "issue move #{issue.iid} #{project.full_path}" + + expect(process_message(message)).to include(response_type: :ephemeral, + text: a_string_matching('Cannot move issue')) + end + end + + context 'when the move succeeds' do + let(:message) { "issue move #{issue.iid} #{other_project.full_path}" } + + it 'moves the issue to the new destination' do + expect { process_message(message) }.to change { Issue.count }.by(1) + + new_issue = issue.reload.moved_to + + expect(new_issue.state).to eq('opened') + expect(new_issue.project_id).to eq(other_project.id) + expect(new_issue.author_id).to eq(issue.author_id) + + expect(issue.state).to eq('closed') + expect(issue.project_id).to eq(project.id) + end + + it 'returns the new issue' do + expect(process_message(message)) + .to include(response_type: :in_channel, + attachments: [a_hash_including(title_link: a_string_including(other_project.full_path))]) + end + + it 'mentions the old issue' do + expect(process_message(message)) + .to include(attachments: [a_hash_including(pretext: a_string_including(project.full_path))]) + end + end + end + + context 'when the issue does not exist' do + it 'returns not found' do + message = "issue move #{issue.iid.succ} #{other_project.full_path}" + + expect(process_message(message)).to include(response_type: :ephemeral, + text: a_string_matching('not found')) + end + end + + context 'when the target project does not exist' do + it 'returns not found' do + message = "issue move #{issue.iid} #{other_project.full_path}/foo" + + expect(process_message(message)).to include(response_type: :ephemeral, + text: a_string_matching('not found')) + end + end + + context 'when the user cannot see the target project' do + it 'returns not found' do + message = "issue move #{issue.iid} #{other_project.full_path}" + other_project.team.truncate + + expect(process_message(message)).to include(response_type: :ephemeral, + text: a_string_matching('not found')) + end + end + + context 'when the user does not have the required permissions on the target project' do + it 'returns the error message' do + message = "issue move #{issue.iid} #{other_project.full_path}" + other_project.team.truncate + other_project.team.add_guest(user) + + expect(process_message(message)).to include(response_type: :ephemeral, + text: a_string_matching('Cannot move issue')) + end + end + end +end diff --git a/spec/lib/gitlab/slash_commands/presenters/issue_move_spec.rb b/spec/lib/gitlab/slash_commands/presenters/issue_move_spec.rb new file mode 100644 index 00000000000..58c341a284e --- /dev/null +++ b/spec/lib/gitlab/slash_commands/presenters/issue_move_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Gitlab::SlashCommands::Presenters::IssueMove do + set(:admin) { create(:admin) } + set(:project) { create(:project) } + set(:other_project) { create(:project) } + set(:old_issue) { create(:issue, project: project) } + set(:new_issue) { Issues::MoveService.new(project, admin).execute(old_issue, other_project) } + let(:attachment) { subject[:attachments].first } + + subject { described_class.new(new_issue).present(old_issue) } + + it { is_expected.to be_a(Hash) } + + it 'shows the new issue' do + expect(subject[:response_type]).to be(:in_channel) + expect(subject).to have_key(:attachments) + expect(attachment[:title]).to start_with(new_issue.title) + expect(attachment[:title_link]).to include(other_project.full_path) + end + + it 'mentions the old issue and the new issue in the pretext' do + expect(attachment[:pretext]).to include(project.full_path) + expect(attachment[:pretext]).to include(other_project.full_path) + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 6e202de0db9..01203ff44c8 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -679,21 +679,21 @@ describe Ci::Build do describe '#erase' do before do - build.erase(erased_by: user) + build.erase(erased_by: erased_by) end context 'erased by user' do - let!(:user) { create(:user, username: 'eraser') } + let!(:erased_by) { create(:user, username: 'eraser') } include_examples 'erasable' it 'records user who erased a build' do - expect(build.erased_by).to eq user + expect(build.erased_by).to eq erased_by end end context 'erased by system' do - let(:user) { nil } + let(:erased_by) { nil } include_examples 'erasable' @@ -748,21 +748,21 @@ describe Ci::Build do describe '#erase' do before do - build.erase(erased_by: user) + build.erase(erased_by: erased_by) end context 'erased by user' do - let!(:user) { create(:user, username: 'eraser') } + let!(:erased_by) { create(:user, username: 'eraser') } include_examples 'erasable' it 'records user who erased a build' do - expect(build.erased_by).to eq user + expect(build.erased_by).to eq erased_by end end context 'erased by system' do - let(:user) { nil } + let(:erased_by) { nil } include_examples 'erasable' @@ -1885,10 +1885,10 @@ describe Ci::Build do describe 'variables ordering' do context 'when variables hierarchy is stubbed' do - let(:build_pre_var) { { key: 'build', value: 'value' } } - let(:project_pre_var) { { key: 'project', value: 'value' } } - let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } } - let(:build_yaml_var) { { key: 'yaml', value: 'value' } } + let(:build_pre_var) { { key: 'build', value: 'value', public: true } } + let(:project_pre_var) { { key: 'project', value: 'value', public: true } } + let(:pipeline_pre_var) { { key: 'pipeline', value: 'value', public: true } } + let(:build_yaml_var) { { key: 'yaml', value: 'value', public: true } } before do allow(build).to receive(:predefined_variables) { [build_pre_var] } @@ -1958,7 +1958,7 @@ describe Ci::Build do context 'when depended job has not been completed yet' do let!(:pre_stage_job) { create(:ci_build, :manual, pipeline: pipeline, name: 'test', stage_idx: 0) } - it { expect { job.run! }.not_to raise_error(Ci::Build::MissingDependenciesError) } + it { expect { job.run! }.not_to raise_error } end context 'when artifacts of depended job has been expired' do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 86bb2fefae1..4635f8cfe9d 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -170,10 +170,8 @@ describe Ci::Pipeline, :mailer do describe '#predefined_variables' do subject { pipeline.predefined_variables } - it { is_expected.to be_an(Array) } - it 'includes all predefined variables in a valid order' do - keys = subject.map { |variable| variable.fetch(:key) } + keys = subject.map { |variable| variable[:key] } expect(keys).to eq %w[CI_PIPELINE_ID CI_CONFIG_PATH CI_PIPELINE_SOURCE] end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index e626efd054d..ee142718f7e 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -204,43 +204,67 @@ describe Namespace do expect(gitlab_shell.exists?(project.repository_storage_path, "#{namespace.path}/#{project.path}.git")).to be_truthy end - context 'with subgroups' do + context 'with subgroups', :nested_groups do let(:parent) { create(:group, name: 'parent', path: 'parent') } + let(:new_parent) { create(:group, name: 'new_parent', path: 'new_parent') } let(:child) { create(:group, name: 'child', path: 'child', parent: parent) } let!(:project) { create(:project_empty_repo, :legacy_storage, path: 'the-project', namespace: child, skip_disk_validation: true) } let(:uploads_dir) { FileUploader.root } let(:pages_dir) { File.join(TestEnv.pages_path) } + def expect_project_directories_at(namespace_path) + expected_repository_path = File.join(TestEnv.repos_path, namespace_path, 'the-project.git') + expected_upload_path = File.join(uploads_dir, namespace_path, 'the-project') + expected_pages_path = File.join(pages_dir, namespace_path, 'the-project') + + expect(File.directory?(expected_repository_path)).to be_truthy + expect(File.directory?(expected_upload_path)).to be_truthy + expect(File.directory?(expected_pages_path)).to be_truthy + end + before do + FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project.full_path}.git")) FileUtils.mkdir_p(File.join(uploads_dir, project.full_path)) FileUtils.mkdir_p(File.join(pages_dir, project.full_path)) end context 'renaming child' do it 'correctly moves the repository, uploads and pages' do - expected_repository_path = File.join(TestEnv.repos_path, 'parent', 'renamed', 'the-project.git') - expected_upload_path = File.join(uploads_dir, 'parent', 'renamed', 'the-project') - expected_pages_path = File.join(pages_dir, 'parent', 'renamed', 'the-project') + child.update!(path: 'renamed') - child.update_attributes!(path: 'renamed') - - expect(File.directory?(expected_repository_path)).to be(true) - expect(File.directory?(expected_upload_path)).to be(true) - expect(File.directory?(expected_pages_path)).to be(true) + expect_project_directories_at('parent/renamed') end end context 'renaming parent' do it 'correctly moves the repository, uploads and pages' do - expected_repository_path = File.join(TestEnv.repos_path, 'renamed', 'child', 'the-project.git') - expected_upload_path = File.join(uploads_dir, 'renamed', 'child', 'the-project') - expected_pages_path = File.join(pages_dir, 'renamed', 'child', 'the-project') + parent.update!(path: 'renamed') + + expect_project_directories_at('renamed/child') + end + end + + context 'moving from one parent to another' do + it 'correctly moves the repository, uploads and pages' do + child.update!(parent: new_parent) - parent.update_attributes!(path: 'renamed') + expect_project_directories_at('new_parent/child') + end + end + + context 'moving from having a parent to root' do + it 'correctly moves the repository, uploads and pages' do + child.update!(parent: nil) + + expect_project_directories_at('child') + end + end + + context 'moving from root to having a parent' do + it 'correctly moves the repository, uploads and pages' do + parent.update!(parent: new_parent) - expect(File.directory?(expected_repository_path)).to be(true) - expect(File.directory?(expected_upload_path)).to be(true) - expect(File.directory?(expected_pages_path)).to be(true) + expect_project_directories_at('new_parent/parent/child') end end end @@ -525,7 +549,6 @@ describe Namespace do end end - # Note: Group transfers are not yet implemented context 'when a group is transferred into a root group' do context 'when the root group "Share with group lock" is enabled' do let(:root_group) { create(:group, share_with_group_lock: true) } diff --git a/spec/models/project_auto_devops_spec.rb b/spec/models/project_auto_devops_spec.rb index 296b91a771c..7545c0797e9 100644 --- a/spec/models/project_auto_devops_spec.rb +++ b/spec/models/project_auto_devops_spec.rb @@ -36,14 +36,14 @@ describe ProjectAutoDevops do end end - describe '#variables' do + describe '#predefined_variables' do let(:auto_devops) { build_stubbed(:project_auto_devops, project: project, domain: domain) } context 'when domain is defined' do let(:domain) { 'example.com' } it 'returns AUTO_DEVOPS_DOMAIN' do - expect(auto_devops.variables).to include(domain_variable) + expect(auto_devops.predefined_variables).to include(domain_variable) end end @@ -55,7 +55,7 @@ describe ProjectAutoDevops do allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return('example.com') end - it { expect(auto_devops.variables).to include(domain_variable) } + it { expect(auto_devops.predefined_variables).to include(domain_variable) } end context 'when there is no instance domain specified' do @@ -63,7 +63,7 @@ describe ProjectAutoDevops do allow(Gitlab::CurrentSettings).to receive(:auto_devops_domain).and_return(nil) end - it { expect(auto_devops.variables).not_to include(domain_variable) } + it { expect(auto_devops.predefined_variables).not_to include(domain_variable) } end end diff --git a/yarn.lock b/yarn.lock index adbb37bea72..3cc5445c402 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6685,9 +6685,9 @@ preserve@^0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/preserve/-/preserve-0.2.0.tgz#815ed1f6ebc65926f865b310c0713bcb3315ce4b" -prettier@1.9.2: - version "1.9.2" - resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.9.2.tgz#96bc2132f7a32338e6078aeb29727178c6335827" +prettier@1.11.1: + version "1.11.1" + resolved "https://registry.yarnpkg.com/prettier/-/prettier-1.11.1.tgz#61e43fc4cd44e68f2b0dfc2c38cd4bb0fccdcc75" prettier@^1.7.0: version "1.8.2" |