diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2018-01-25 18:33:50 +0000 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2018-01-25 18:33:50 +0000 |
commit | 1f10fa8cee8ffb5587ad037d8d8d7c5371264cf4 (patch) | |
tree | eeb1844c4c7f192ed281efe1f13fea9271623ef1 | |
parent | 6b24028b0d0240ad55790b9d1f1493b6e56afc94 (diff) | |
parent | eaa37a4445b303de3665625c6541c914fb1f118b (diff) | |
download | gitlab-ce-1f10fa8cee8ffb5587ad037d8d8d7c5371264cf4.tar.gz |
Merge branch 'master' into fl-mr-widget-components
* master:
Open links in a new tab on the user page
Improve empty project overview
Make Gitaly RepositoryExists opt-out
Fix .batch_lfs_pointers accepting a lazy enumerator
Look for rugged with static analysis
Look at notes created just before merge when deciding if an MR can be reverted
Cache rubocop cache for CI
Update gitlab-styles and re-enable the RSpec/SingleLineHook cop again
Make Gitlab::Git::Repository#run_git private
Default to HTTPS for all Gravatar URLs
Handle special characters on API request of issuable templates
Update secret_values to support dynamic elements within parent
Add note within ux documentation that further changes should be made within the design.gitlab project
Disable throwOnError in KaTeX to reveal user where is the problem
56 files changed, 451 insertions, 272 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 349ea49fe8f..1b409cca95c 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -388,7 +388,6 @@ spinach-mysql 2 3: *spinach-metadata-mysql # Static analysis jobs .ruby-static-analysis: &ruby-static-analysis - <<: *pull-cache variables: SIMPLECOV: "false" SETUP_DB: "false" @@ -409,6 +408,12 @@ static-analysis: stage: test script: - scripts/static-analysis + cache: + key: "ruby-2.3.6-with-yarn-and-rubocop" + paths: + - vendor/ruby + - .yarn-cache/ + - tmp/rubocop_cache # Documentation checks: # - Check validity of relative links diff --git a/.rubocop.yml b/.rubocop.yml index 9adc2fae7a8..563a00db6c0 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -17,6 +17,7 @@ AllCops: - 'bin/**/*' - 'generator_templates/**/*' - 'builds/**/*' + CacheRootDirectory: tmp # Gitlab ################################################################### diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 8d2276f71be..442d61bcf4f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -342,10 +342,6 @@ RSpec/SharedContext: Exclude: - 'spec/features/admin/admin_groups_spec.rb' -# Offense count: 90 -RSpec/SingleLineHook: - Enabled: false - # Offense count: 5 RSpec/VoidExpect: Exclude: diff --git a/Gemfile.lock b/Gemfile.lock index 1cbeab8d6b5..c8570cbb897 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -304,7 +304,7 @@ GEM mime-types (>= 1.16) posix-spawn (~> 0.3) gitlab-markup (1.6.3) - gitlab-styles (2.3.0) + gitlab-styles (2.3.1) rubocop (~> 0.51) rubocop-gitlab-security (~> 0.1.0) rubocop-rspec (~> 1.19) diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index 38c67b5f04e..7cb81bf4d5b 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -178,7 +178,7 @@ const Api = { issueTemplate(namespacePath, projectPath, key, type, callback) { const url = Api.buildUrl(Api.issuableTemplatePath) - .replace(':key', key) + .replace(':key', encodeURIComponent(key)) .replace(':type', type) .replace(':project_path', projectPath) .replace(':namespace_path', namespacePath); diff --git a/app/assets/javascripts/behaviors/secret_values.js b/app/assets/javascripts/behaviors/secret_values.js index 1cf0b960eb0..7f70fce913a 100644 --- a/app/assets/javascripts/behaviors/secret_values.js +++ b/app/assets/javascripts/behaviors/secret_values.js @@ -2,18 +2,19 @@ import { n__ } from '../locale'; import { convertPermissionToBoolean } from '../lib/utils/common_utils'; export default class SecretValues { - constructor(container) { + constructor({ + container, + valueSelector = '.js-secret-value', + placeholderSelector = '.js-secret-value-placeholder', + }) { this.container = container; + this.valueSelector = valueSelector; + this.placeholderSelector = placeholderSelector; } init() { - this.values = this.container.querySelectorAll('.js-secret-value'); - this.placeholders = this.container.querySelectorAll('.js-secret-value-placeholder'); this.revealButton = this.container.querySelector('.js-secret-value-reveal-button'); - this.revealText = n__('Reveal value', 'Reveal values', this.values.length); - this.hideText = n__('Hide value', 'Hide values', this.values.length); - const isRevealed = convertPermissionToBoolean(this.revealButton.dataset.secretRevealStatus); this.updateDom(isRevealed); @@ -28,15 +29,17 @@ export default class SecretValues { } updateDom(isRevealed) { - this.values.forEach((value) => { + const values = this.container.querySelectorAll(this.valueSelector); + values.forEach((value) => { value.classList.toggle('hide', !isRevealed); }); - this.placeholders.forEach((placeholder) => { + const placeholders = this.container.querySelectorAll(this.placeholderSelector); + placeholders.forEach((placeholder) => { placeholder.classList.toggle('hide', isRevealed); }); - this.revealButton.textContent = isRevealed ? this.hideText : this.revealText; + this.revealButton.textContent = isRevealed ? n__('Hide value', 'Hide values', values.length) : n__('Reveal value', 'Reveal values', values.length); this.revealButton.dataset.secretRevealStatus = isRevealed; } } diff --git a/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js b/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js index c4691cd367c..f26c7360fbe 100644 --- a/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js +++ b/app/assets/javascripts/pages/groups/settings/ci_cd/show/index.js @@ -3,7 +3,9 @@ import SecretValues from '~/behaviors/secret_values'; export default () => { const secretVariableTable = document.querySelector('.js-secret-variable-table'); if (secretVariableTable) { - const secretVariableTableValues = new SecretValues(secretVariableTable); + const secretVariableTableValues = new SecretValues({ + container: secretVariableTable, + }); secretVariableTableValues.init(); } }; diff --git a/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js b/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js index 94b927a1548..18dc1dc03a5 100644 --- a/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js +++ b/app/assets/javascripts/pages/projects/settings/ci_cd/show/index.js @@ -6,13 +6,17 @@ export default function () { initSettingsPanels(); const runnerToken = document.querySelector('.js-secret-runner-token'); if (runnerToken) { - const runnerTokenSecretValue = new SecretValues(runnerToken); + const runnerTokenSecretValue = new SecretValues({ + container: runnerToken, + }); runnerTokenSecretValue.init(); } const secretVariableTable = document.querySelector('.js-secret-variable-table'); if (secretVariableTable) { - const secretVariableTableValues = new SecretValues(secretVariableTable); + const secretVariableTableValues = new SecretValues({ + container: secretVariableTable, + }); secretVariableTableValues.init(); } } diff --git a/app/assets/javascripts/render_math.js b/app/assets/javascripts/render_math.js index a759992cd54..15205d8a4e2 100644 --- a/app/assets/javascripts/render_math.js +++ b/app/assets/javascripts/render_math.js @@ -18,7 +18,7 @@ function renderWithKaTeX(elements) { const display = $this.attr('data-math-style') === 'display'; try { - katex.render($this.text(), mathNode.get(0), { displayMode: display }); + katex.render($this.text(), mathNode.get(0), { displayMode: display, throwOnError: false }); mathNode.insertAfter($this); $this.remove(); } catch (err) { diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index d7153d7b816..f84bf132854 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -524,7 +524,7 @@ module Ci return unless sha project.repository.gitlab_ci_yml_for(sha, ci_yaml_file_path) - rescue GRPC::NotFound, Rugged::ReferenceError, GRPC::Internal + rescue GRPC::NotFound, GRPC::Internal nil end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 7bcded5b5e1..3aed071dd49 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -45,14 +45,7 @@ class Deployment < ActiveRecord::Base def includes_commit?(commit) return false unless commit - # Before 8.10, deployments didn't have keep-around refs. Any deployment - # created before then could have a `sha` referring to a commit that no - # longer exists in the repository, so just ignore those. - begin - project.repository.ancestor?(commit.id, sha) - rescue Rugged::OdbError - false - end + project.repository.ancestor?(commit.id, sha) end def update_merge_request_metrics! diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 8028ff3875b..4accb08eaf9 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -989,8 +989,14 @@ class MergeRequest < ActiveRecord::Base merged_at = metrics&.merged_at notes_association = notes_with_associations + # It is not guaranteed that Note#created_at will be strictly later than + # MergeRequestMetric#merged_at. Nanoseconds on MySQL may break this + # comparison, as will a HA environment if clocks are not *precisely* + # synchronized. Add a minute's leeway to compensate for both possibilities + cutoff = merged_at - 1.minute + if merged_at - notes_association = notes_association.where('created_at > ?', merged_at) + notes_association = notes_association.where('created_at >= ?', cutoff) end !merge_commit.has_been_reverted?(current_user, notes_association) diff --git a/app/models/repository.rb b/app/models/repository.rb index 824e18bec78..5101c087a50 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -166,16 +166,10 @@ class Repository return [] end - raw_repository.gitaly_migrate(:commits_by_message) do |is_enabled| - commits = - if is_enabled - find_commits_by_message_by_gitaly(query, ref, path, limit, offset) - else - find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) - end - - CommitCollection.new(project, commits, ref) + commits = raw_repository.find_commits_by_message(query, ref, path, limit, offset).map do |c| + commit(c) end + CommitCollection.new(project, commits, ref) end def find_branch(name, fresh_repo: true) @@ -740,23 +734,6 @@ class Repository Commit.order_by(collection: commits, order_by: order_by, sort: sort) end - def refs_contains_sha(ref_type, sha) - args = %W(#{ref_type} --contains #{sha}) - names = run_git(args).first - - if names.respond_to?(:split) - names = names.split("\n").map(&:strip) - - names.each do |name| - name.slice! '* ' - end - - names - else - [] - end - end - def branch_names_contains(sha) refs_contains_sha('branch', sha) end @@ -921,25 +898,6 @@ class Repository end end - def search_files_by_content(query, ref) - return [] if empty? || query.blank? - - offset = 2 - args = %W(grep -i -I -n -z --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref}) - - run_git(args).first.scrub.split(/^--$/) - end - - def search_files_by_name(query, ref) - safe_query = Regexp.escape(query.sub(/^\/*/, "")) - - return [] if empty? || safe_query.blank? - - args = %W(ls-tree --full-tree -r #{ref || root_ref} --name-status | #{safe_query}) - - run_git(args).first.lines.map(&:strip) - end - def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil) unless remote_name remote_name = "tmp-#{SecureRandom.hex}" @@ -973,6 +931,18 @@ class Repository raw_repository.ls_files(actual_ref) end + def search_files_by_content(query, ref) + return [] if empty? || query.blank? + + raw_repository.search_files_by_content(query, ref) + end + + def search_files_by_name(query, ref) + return [] if empty? + + raw_repository.search_files_by_name(query, ref) + end + def copy_gitattributes(ref) actual_ref = ref || root_ref begin @@ -1133,25 +1103,4 @@ class Repository def rugged_can_be_merged?(their_commit, our_commit) !rugged.merge_commits(our_commit, their_commit).conflicts? end - - def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) - ref ||= root_ref - - args = %W( - log #{ref} --pretty=%H --skip #{offset} - --max-count #{limit} --grep=#{query} --regexp-ignore-case - ) - args = args.concat(%W(-- #{path})) if path.present? - - git_log_results = run_git(args).first.lines - - git_log_results.map { |c| commit(c.chomp) }.compact - end - - def find_commits_by_message_by_gitaly(query, ref, path, limit, offset) - raw_repository - .gitaly_commit_client - .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset) - .map { |c| commit(c) } - end end diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 0f773933ac2..5c76d2d8f51 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -13,11 +13,11 @@ - if @user.avatar? You can change your avatar here - if gravatar_enabled? - or remove the current avatar to revert to #{link_to Gitlab.config.gravatar.host, 'http://' + Gitlab.config.gravatar.host} + or remove the current avatar to revert to #{link_to Gitlab.config.gravatar.host, 'https://' + Gitlab.config.gravatar.host} - else You can upload an avatar here - if gravatar_enabled? - or change it at #{link_to Gitlab.config.gravatar.host, 'http://' + Gitlab.config.gravatar.host} + or change it at #{link_to Gitlab.config.gravatar.host, 'https://' + Gitlab.config.gravatar.host} .col-lg-8 .clearfix.avatar-image.append-bottom-default = link_to avatar_icon(@user, 400), target: '_blank', rel: 'noopener noreferrer' do diff --git a/app/views/projects/_readme.html.haml b/app/views/projects/_readme.html.haml index 32901d30b96..aebdfbc8218 100644 --- a/app/views/projects/_readme.html.haml +++ b/app/views/projects/_readme.html.haml @@ -9,15 +9,15 @@ - else .row-content-block.second-block.center - %h3.page-title + %h4 This project does not have a README yet + - if can?(current_user, :push_code, @project) %p A %code README file contains information about other files in a repository and is commonly distributed with computer software, forming part of its documentation. + GitLab will render it here instead of this message. %p - We recommend you to - = link_to "add a README", add_special_file_path(@project, file_name: 'README.md') - file to the repository and GitLab will render it here instead of this message. + = link_to "Add Readme", add_special_file_path(@project, file_name: 'README.md'), class: 'btn btn-new' diff --git a/app/views/projects/buttons/_dropdown.html.haml b/app/views/projects/buttons/_dropdown.html.haml index dab94d10bb1..18e948ce35a 100644 --- a/app/views/projects/buttons/_dropdown.html.haml +++ b/app/views/projects/buttons/_dropdown.html.haml @@ -6,7 +6,10 @@ %ul.dropdown-menu.dropdown-menu-align-right.project-home-dropdown - can_create_issue = can?(current_user, :create_issue, @project) - merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project)) - - can_create_snippet = can?(current_user, :create_snippet, @project) + - can_create_project_snippet = can?(current_user, :create_project_snippet, @project) + + - if can_create_issue || merge_project || can_create_project_snippet + %li.dropdown-header= _('This project') - if can_create_issue %li= link_to _('New issue'), new_project_issue_path(@project) @@ -14,11 +17,11 @@ - if merge_project %li= link_to _('New merge request'), project_new_merge_request_path(merge_project) - - if can_create_snippet + - if can_create_project_snippet %li= link_to _('New snippet'), new_project_snippet_path(@project) - - if can_create_issue || merge_project || can_create_snippet - %li.divider + - if can?(current_user, :push_code, @project) + %li.dropdown-header= _('This repository') - if can?(current_user, :push_code, @project) %li= link_to _('New file'), project_new_blob_path(@project, @project.default_branch || 'master') @@ -31,5 +34,5 @@ - continue_params = { to: project_new_blob_path(@project, @project.default_branch || 'master'), notice: edit_in_new_fork_notice, notice_now: edit_in_new_fork_notice_now } - - fork_path = project_forks_path(@project, namespace_key: current_user.namespace.id, continue: continue_params) + - fork_path = project_forks_path(@project, namespace_key: current_user.namespace.id, continue: continue_params) %li= link_to _('New file'), fork_path, method: :post diff --git a/app/views/projects/empty.html.haml b/app/views/projects/empty.html.haml index 58e89a481a9..ab225796b12 100644 --- a/app/views/projects/empty.html.haml +++ b/app/views/projects/empty.html.haml @@ -6,8 +6,9 @@ = render "home_panel" .row-content-block.second-block.center - %h3.page-title + %h4 The repository for this project is empty + - if can?(current_user, :push_code, @project) %p If you already have files you can push them using command line instructions below. @@ -28,8 +29,8 @@ %p - link = link_to(s_('AutoDevOps|Auto DevOps (Beta)'), project_settings_ci_cd_path(@project, anchor: 'js-general-pipeline-settings')) = s_('AutoDevOps|You can activate %{link_to_settings} for this project.').html_safe % { link_to_settings: link } - %p - = s_('AutoDevOps|It will automatically build, test, and deploy your application based on a predefined CI/CD configuration.') + %p= s_('AutoDevOps|It will automatically build, test, and deploy your application based on a predefined CI/CD configuration.') + %p= link_to _('New file'), project_new_blob_path(@project, @project.default_branch || 'master'), class: 'btn btn-new' - if can?(current_user, :push_code, @project) %div{ class: container_class } @@ -79,4 +80,4 @@ - if can? current_user, :remove_project, @project .prepend-top-20 - = link_to 'Remove project', [@project.namespace.becomes(Namespace), @project], data: { confirm: remove_project_message(@project)}, method: :delete, class: "btn btn-remove pull-right" + = link_to 'Remove project', [@project.namespace.becomes(Namespace), @project], data: { confirm: remove_project_message(@project)}, method: :delete, class: "btn btn-inverted btn-remove pull-right" diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 4f4e81c705f..90aa1be30ac 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -58,15 +58,15 @@ = icon('skype') - unless @user.linkedin.blank? .profile-link-holder.middle-dot-divider - = link_to linkedin_url(@user), title: "LinkedIn" do + = link_to linkedin_url(@user), title: "LinkedIn", target: '_blank', rel: 'noopener noreferrer nofollow' do = icon('linkedin-square') - unless @user.twitter.blank? .profile-link-holder.middle-dot-divider - = link_to twitter_url(@user), title: "Twitter" do + = link_to twitter_url(@user), title: "Twitter", target: '_blank', rel: 'noopener noreferrer nofollow' do = icon('twitter-square') - unless @user.website_url.blank? .profile-link-holder.middle-dot-divider - = link_to @user.short_website_url, @user.full_website_url, class: 'text-link' + = link_to @user.short_website_url, @user.full_website_url, class: 'text-link', target: '_blank', rel: 'noopener noreferrer nofollow' - unless @user.location.blank? .profile-link-holder.middle-dot-divider = icon('map-marker') diff --git a/app/workers/repository_check/single_repository_worker.rb b/app/workers/repository_check/single_repository_worker.rb index 4e3c691e8da..116bc185b38 100644 --- a/app/workers/repository_check/single_repository_worker.rb +++ b/app/workers/repository_check/single_repository_worker.rb @@ -20,10 +20,7 @@ module RepositoryCheck # Historically some projects never had their wiki repos initialized; # this happens on project creation now. Let's initialize an empty repo # if it is not already there. - begin - project.create_wiki - rescue Rugged::RepositoryError - end + project.create_wiki git_fsck(project.wiki.repository) else diff --git a/changelogs/unreleased/40028-special-characters-on-issuable-templates.yml b/changelogs/unreleased/40028-special-characters-on-issuable-templates.yml new file mode 100644 index 00000000000..ffab28acbd5 --- /dev/null +++ b/changelogs/unreleased/40028-special-characters-on-issuable-templates.yml @@ -0,0 +1,5 @@ +--- +title: Handle special characters on API request of issuable templates +merge_request: 15323 +author: Takuya Noguchi +type: fixed diff --git a/changelogs/unreleased/default-to-https-for-gravatar-urls.yml b/changelogs/unreleased/default-to-https-for-gravatar-urls.yml new file mode 100644 index 00000000000..544c34fe31d --- /dev/null +++ b/changelogs/unreleased/default-to-https-for-gravatar-urls.yml @@ -0,0 +1,5 @@ +--- +title: Default to HTTPS for all Gravatar URLs +merge_request: 16666 +author: +type: fixed diff --git a/changelogs/unreleased/disable-throwOnError-in-katex.yml b/changelogs/unreleased/disable-throwOnError-in-katex.yml new file mode 100644 index 00000000000..0cd17bb29fe --- /dev/null +++ b/changelogs/unreleased/disable-throwOnError-in-katex.yml @@ -0,0 +1,5 @@ +--- +title: Disable throwOnError in KaTeX to reveal user where is the problem +merge_request: 16684 +author: Jakub Jirutka +type: other diff --git a/changelogs/unreleased/feat-add-section-headers-to-project-repo-buttons.yml b/changelogs/unreleased/feat-add-section-headers-to-project-repo-buttons.yml new file mode 100644 index 00000000000..8f3459a7381 --- /dev/null +++ b/changelogs/unreleased/feat-add-section-headers-to-project-repo-buttons.yml @@ -0,0 +1,5 @@ +--- +title: Improve empty project overview +merge_request: 16617 +author: George Tsiolis +type: added diff --git a/changelogs/unreleased/gitaly-repo-exists.yml b/changelogs/unreleased/gitaly-repo-exists.yml new file mode 100644 index 00000000000..a9eb42a2038 --- /dev/null +++ b/changelogs/unreleased/gitaly-repo-exists.yml @@ -0,0 +1,5 @@ +--- +title: Make Gitaly RepositoryExists opt-out +merge_request: 16680 +author: +type: other diff --git a/changelogs/unreleased/ux-guide-deprecation.yml b/changelogs/unreleased/ux-guide-deprecation.yml new file mode 100644 index 00000000000..16477f59abf --- /dev/null +++ b/changelogs/unreleased/ux-guide-deprecation.yml @@ -0,0 +1,6 @@ +--- +title: Add note within ux documentation that further changes should be made within + the design.gitlab project +merge_request: +author: +type: deprecated diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index f2f05b3eeb2..238e1583770 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -175,10 +175,12 @@ production: &base host: 'https://mattermost.example.com' ## Gravatar - ## For Libravatar see: http://doc.gitlab.com/ce/customization/libravatar.html + ## If using gravatar.com, there's nothing to change here. For Libravatar + ## you'll need to provide the custom URLs. For more information, + ## see: https://docs.gitlab.com/ee/customization/libravatar.html gravatar: - # gravatar urls: possible placeholders: %{hash} %{size} %{email} %{username} - # plain_url: "http://..." # default: http://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon + # Gravatar/Libravatar URLs: possible placeholders: %{hash} %{size} %{email} %{username} + # plain_url: "http://..." # default: https://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon # ssl_url: "https://..." # default: https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon ## Auxiliary jobs diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index abc992e49dc..899e612ffbd 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -350,7 +350,7 @@ Settings.mattermost['host'] = nil unless Settings.mattermost.enabled # Settings['gravatar'] ||= Settingslogic.new({}) Settings.gravatar['enabled'] = true if Settings.gravatar['enabled'].nil? -Settings.gravatar['plain_url'] ||= 'http://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon' +Settings.gravatar['plain_url'] ||= 'https://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon' Settings.gravatar['ssl_url'] ||= 'https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon' Settings.gravatar['host'] = Settings.host_without_www(Settings.gravatar['plain_url']) diff --git a/config/initializers/rugged_use_gitlab_git_attributes.rb b/config/initializers/rugged_use_gitlab_git_attributes.rb deleted file mode 100644 index c0d45caec42..00000000000 --- a/config/initializers/rugged_use_gitlab_git_attributes.rb +++ /dev/null @@ -1,28 +0,0 @@ -# We don't want to ever call Rugged::Repository#fetch_attributes, because it has -# a lot of I/O overhead: -# <https://gitlab.com/gitlab-org/gitlab_git/commit/340e111e040ae847b614d35b4d3173ec48329015> -# -# While we don't do this from within the GitLab source itself, the Linguist gem -# has a dependency on Rugged and uses the gitattributes file when calculating -# repository-wide language statistics: -# <https://github.com/github/linguist/blob/v4.7.0/lib/linguist/lazy_blob.rb#L33-L36> -# -# The options passed by Linguist are those assumed by Gitlab::Git::InfoAttributes -# anyway, and there is no great efficiency gain from just fetching the listed -# attributes with our implementation, so we ignore the additional arguments. -# -module Rugged - class Repository - module UseGitlabGitAttributes - def fetch_attributes(name, *) - attributes.attributes(name) - end - - def attributes - @attributes ||= Gitlab::Git::InfoAttributes.new(path) - end - end - - prepend UseGitlabGitAttributes - end -end diff --git a/config/routes/project.rb b/config/routes/project.rb index 43ada9ba145..0496bd85b4e 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -40,7 +40,7 @@ constraints(ProjectUrlConstrainer.new) do # # Templates # - get '/templates/:template_type/:key' => 'templates#show', as: :template + get '/templates/:template_type/:key' => 'templates#show', as: :template, constraints: { key: /[^\/]+/ } resource :avatar, only: [:show, :destroy] resources :commit, only: [:show], constraints: { id: /\h{7,40}/ } do diff --git a/doc/development/ux_guide/index.md b/doc/development/ux_guide/index.md index 42bcf234e12..c59e7b72a1a 100644 --- a/doc/development/ux_guide/index.md +++ b/doc/development/ux_guide/index.md @@ -1,3 +1,5 @@ +> We are in the process of transferring UX documentation to the [design.gitlab.com](https://gitlab.com/gitlab-org/design.gitlab.com) project. Any updates to these docs should be made in that project. If documentation does not yet exist within [design.gitlab.com](https://gitlab.com/gitlab-org/design.gitlab.com), [create an issue](https://gitlab.com/gitlab-org/design.gitlab.com/issues) and merge request to add your new changes. + # GitLab UX Guide The goal of this guide is to provide standards, principles and in-depth information to design beautiful and effective GitLab features. This will be a living document, and we welcome contributions, feedback and suggestions. diff --git a/lib/gitlab/bare_repository_import/repository.rb b/lib/gitlab/bare_repository_import/repository.rb index c0c666dfb7b..fe267248275 100644 --- a/lib/gitlab/bare_repository_import/repository.rb +++ b/lib/gitlab/bare_repository_import/repository.rb @@ -1,3 +1,5 @@ +# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/953 +# module Gitlab module BareRepositoryImport class Repository diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index 81e46028752..13120120223 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -70,11 +70,9 @@ module Gitlab # Returns array of Gitlab::Git::Blob # Does not guarantee blob data will be set def batch_lfs_pointers(repository, blob_ids) - return [] if blob_ids.empty? - repository.gitaly_migrate(:batch_lfs_pointers) do |is_enabled| if is_enabled - repository.gitaly_blob_client.batch_lfs_pointers(blob_ids) + repository.gitaly_blob_client.batch_lfs_pointers(blob_ids.to_a) else blob_ids.lazy .select { |sha| possible_lfs_blob?(repository, sha) } diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index d6c0980255f..5333766c338 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -133,7 +133,7 @@ module Gitlab end def exists? - Gitlab::GitalyClient.migrate(:repository_exists) do |enabled| + Gitlab::GitalyClient.migrate(:repository_exists, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |enabled| if enabled gitaly_repository_client.exists? else @@ -563,6 +563,8 @@ module Gitlab return false if ancestor_id.nil? || descendant_id.nil? merge_base_commit(ancestor_id, descendant_id) == ancestor_id + rescue Rugged::OdbError + false end # Returns true is +from+ is direct ancestor to +to+, otherwise false @@ -1126,23 +1128,6 @@ module Gitlab end # Refactoring aid; allows us to copy code from app/models/repository.rb - def run_git(args, chdir: path, env: {}, nice: false, &block) - cmd = [Gitlab.config.git.bin_path, *args] - cmd.unshift("nice") if nice - circuit_breaker.perform do - popen(cmd, chdir, env, &block) - end - end - - def run_git!(args, chdir: path, env: {}, nice: false, &block) - output, status = run_git(args, chdir: chdir, env: env, nice: nice, &block) - - raise GitError, output unless status.zero? - - output - end - - # Refactoring aid; allows us to copy code from app/models/repository.rb def run_git_with_timeout(args, timeout, env: {}) circuit_breaker.perform do popen_with_timeout([Gitlab.config.git.bin_path, *args], timeout, path, env) @@ -1365,6 +1350,52 @@ module Gitlab raise CommandError.new(e) end + def refs_contains_sha(ref_type, sha) + args = %W(#{ref_type} --contains #{sha}) + names = run_git(args).first + + if names.respond_to?(:split) + names = names.split("\n").map(&:strip) + + names.each do |name| + name.slice! '* ' + end + + names + else + [] + end + end + + def search_files_by_content(query, ref) + return [] if empty? || query.blank? + + offset = 2 + args = %W(grep -i -I -n -z --before-context #{offset} --after-context #{offset} -E -e #{Regexp.escape(query)} #{ref || root_ref}) + + run_git(args).first.scrub.split(/^--$/) + end + + def search_files_by_name(query, ref) + safe_query = Regexp.escape(query.sub(/^\/*/, "")) + + return [] if empty? || safe_query.blank? + + args = %W(ls-tree --full-tree -r #{ref || root_ref} --name-status | #{safe_query}) + + run_git(args).first.lines.map(&:strip) + end + + def find_commits_by_message(query, ref, path, limit, offset) + gitaly_migrate(:commits_by_message) do |is_enabled| + if is_enabled + find_commits_by_message_by_gitaly(query, ref, path, limit, offset) + else + find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) + end + end + end + private def shell_write_ref(ref_path, ref, old_ref) @@ -1386,6 +1417,22 @@ module Gitlab Rails.logger.error "Unable to create #{ref_path} reference for repository #{path}: #{ex}" end + def run_git(args, chdir: path, env: {}, nice: false, &block) + cmd = [Gitlab.config.git.bin_path, *args] + cmd.unshift("nice") if nice + circuit_breaker.perform do + popen(cmd, chdir, env, &block) + end + end + + def run_git!(args, chdir: path, env: {}, nice: false, &block) + output, status = run_git(args, chdir: chdir, env: env, nice: nice, &block) + + raise GitError, output unless status.zero? + + output + end + def fresh_worktree?(path) File.exist?(path) && !clean_stuck_worktree(path) end @@ -2161,6 +2208,26 @@ module Gitlab def gitlab_projects_error raise CommandError, @gitlab_projects.output end + + def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset) + ref ||= root_ref + + args = %W( + log #{ref} --pretty=%H --skip #{offset} + --max-count #{limit} --grep=#{query} --regexp-ignore-case + ) + args = args.concat(%W(-- #{path})) if path.present? + + git_log_results = run_git(args).first.lines + + git_log_results.map { |c| commit(c.chomp) }.compact + end + + def find_commits_by_message_by_gitaly(query, ref, path, limit, offset) + gitaly_commit_client + .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset) + .map { |c| commit(c) } + end end end end diff --git a/lib/gitlab/gitaly_client/blob_service.rb b/lib/gitlab/gitaly_client/blob_service.rb index ee36684197b..d70a1a7665e 100644 --- a/lib/gitlab/gitaly_client/blob_service.rb +++ b/lib/gitlab/gitaly_client/blob_service.rb @@ -34,6 +34,8 @@ module Gitlab end def batch_lfs_pointers(blob_ids) + return [] if blob_ids.empty? + request = Gitaly::GetLFSPointersRequest.new( repository: @gitaly_repo, blob_ids: blob_ids diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb index 33a8d3e5612..cadc7149301 100644 --- a/lib/gitlab/gitaly_client/commit_service.rb +++ b/lib/gitlab/gitaly_client/commit_service.rb @@ -38,19 +38,27 @@ module Gitlab from_id = case from when NilClass EMPTY_TREE_ID - when Rugged::Commit - from.oid else - from + if from.respond_to?(:oid) + # This is meant to match a Rugged::Commit. This should be impossible in + # the future. + from.oid + else + from + end end to_id = case to when NilClass EMPTY_TREE_ID - when Rugged::Commit - to.oid else - to + if to.respond_to?(:oid) + # This is meant to match a Rugged::Commit. This should be impossible in + # the future. + to.oid + else + to + end end request_params = diff_between_commits_request_params(from_id, to_id, options) diff --git a/lib/gitlab/github_import/importer/pull_requests_importer.rb b/lib/gitlab/github_import/importer/pull_requests_importer.rb index 5437e32e9f1..e70361c163b 100644 --- a/lib/gitlab/github_import/importer/pull_requests_importer.rb +++ b/lib/gitlab/github_import/importer/pull_requests_importer.rb @@ -57,10 +57,7 @@ module Gitlab end def commit_exists?(sha) - project.repository.lookup(sha) - true - rescue Rugged::Error - false + project.repository.commit(sha).present? end def collection_method diff --git a/lib/tasks/gitlab/cleanup.rake b/lib/tasks/gitlab/cleanup.rake index 04d56509ac6..ab601b0d66b 100644 --- a/lib/tasks/gitlab/cleanup.rake +++ b/lib/tasks/gitlab/cleanup.rake @@ -1,3 +1,5 @@ +# Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/954 +# namespace :gitlab do namespace :cleanup do HASHED_REPOSITORY_NAME = '@hashed'.freeze diff --git a/scripts/lint-rugged b/scripts/lint-rugged new file mode 100755 index 00000000000..3f8fcb558e3 --- /dev/null +++ b/scripts/lint-rugged @@ -0,0 +1,36 @@ +#!/usr/bin/env ruby + +ALLOWED = [ + # Can be deleted (?) once rugged is no longer used in production. Doesn't make Rugged calls. + 'config/initializers/8_metrics.rb', + + # Can be deleted once wiki's are fully (mandatory) migrated + 'config/initializers/gollum.rb', + + # Needs to be migrated, https://gitlab.com/gitlab-org/gitaly/issues/953 + 'lib/gitlab/bare_repository_import/repository.rb', + + # Needs to be migrated, https://gitlab.com/gitlab-org/gitaly/issues/954 + 'lib/tasks/gitlab/cleanup.rake', + + # https://gitlab.com/gitlab-org/gitaly/issues/961 + 'app/models/repository.rb', + + # The only place where Rugged code is still allowed in production + 'lib/gitlab/git/' +].freeze + +rugged_lines = IO.popen(%w[git grep -i -n rugged -- app config lib], &:read).lines +rugged_lines = rugged_lines.reject { |l| l.start_with?(*ALLOWED) } +rugged_lines = rugged_lines.reject do |line| + code, _comment = line.split('# ', 2) + code !~ /rugged/i +end + +exit if rugged_lines.empty? + +puts "Using Rugged is only allowed in test and #{ALLOWED}\n\n" + +puts rugged_lines + +exit(false) diff --git a/scripts/static-analysis b/scripts/static-analysis index 9690b42c788..96d08287ded 100755 --- a/scripts/static-analysis +++ b/scripts/static-analysis @@ -13,7 +13,8 @@ tasks = [ %w[bundle exec rake gettext:lint], %w[bundle exec rake lint:static_verification], %w[scripts/lint-changelog-yaml], - %w[scripts/lint-conflicts.sh] + %w[scripts/lint-conflicts.sh], + %w[scripts/lint-rugged] ] failed_tasks = tasks.reduce({}) do |failures, task| diff --git a/spec/fixtures/emails/attachment.eml b/spec/fixtures/emails/attachment.eml index f25c3d1a449..b3a30b3221b 100644 --- a/spec/fixtures/emails/attachment.eml +++ b/spec/fixtures/emails/attachment.eml @@ -91,7 +91,7 @@ x #ccc solid;padding-left:1ex"><div> adding=3D"0" border=3D"0"><tbody> <tr> <td style=3D"vertical-align:top;width:55px"> - <img src=3D"http://www.gravatar.com/avatar/42776c4982dff1fa45ee8248= + <img src=3D"https://www.gravatar.com/avatar/42776c4982dff1fa45ee8248= 532f8ad0.png?s=3D45&r=3Dpg&d=3Didenticon" title=3D"Neil" style=3D"m= ax-width:694px" width=3D"45" height=3D"45"> </td> @@ -121,7 +121,7 @@ nk">@eviltrout</a> Any idea why it showed up in suggested topics? </p> <div style=3D"color:#666"> <p>To respond, reply to this email or visit <a href=3D"http://meta.disc= ourse.org/t/spam-post-pops-back-up-in-suggested-topics/11005/5" style=3D"co= -lor:#666" target=3D"_blank">http://meta.discourse.org/t/spam-post-pops-back= +lor:#666" target=3D"_blank">https://meta.discourse.org/t/spam-post-pops-back= -up-in-suggested-topics/11005/5</a> in your browser.</p> </div> @@ -132,12 +132,12 @@ lor:#666" target=3D"_blank">http://meta.discourse.org/t/spam-post-pops-back= lpadding=3D"0" border=3D"0"><tbody> <tr> <td style=3D"vertical-align:top;width:55px"> - <img src=3D"http://www.gravatar.com/avatar/42776c4982dff1fa45ee8248= + <img src=3D"https://www.gravatar.com/avatar/42776c4982dff1fa45ee8248= 532f8ad0.png?s=3D45&r=3Dpg&d=3Didenticon" title=3D"Neil" style=3D"m= ax-width:694px" width=3D"45" height=3D"45"> </td> <td> - <a href=3D"http://meta.discourse.org/users/neil" style=3D"font-size= + <a href=3D"https://meta.discourse.org/users/neil" style=3D"font-size= :13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;c= olor:#3b5998;text-decoration:none;font-weight:bold" target=3D"_blank">Neil<= /a><br> @@ -155,12 +155,12 @@ vember 19</span> adding=3D"0" border=3D"0"><tbody> <tr> <td style=3D"vertical-align:top;width:55px"> - <img src=3D"http://www.gravatar.com/avatar/5120fc4e345db0d1a9648882= + <img src=3D"https://www.gravatar.com/avatar/5120fc4e345db0d1a9648882= 72073819.png?s=3D45&r=3Dpg&d=3Didenticon" title=3D"riking" style=3D= "max-width:694px" width=3D"45" height=3D"45"> </td> <td> - <a href=3D"http://meta.discourse.org/users/riking" style=3D"font-si= + <a href=3D"https://meta.discourse.org/users/riking" style=3D"font-si= ze:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif= ;color:#3b5998;text-decoration:none;font-weight:bold" target=3D"_blank">rik= ing</a><br> @@ -173,7 +173,7 @@ vember 19</span> <td style=3D"padding-top:5px" colspan=3D"2"> <p style=3D"margin-top:0"><u></u></p><div> <div></div> -<img width=3D"20" height=3D"20" src=3D"http://www.gravatar.com/avatar/51d62= +<img width=3D"20" height=3D"20" src=3D"https://www.gravatar.com/avatar/51d62= 3f33f8b83095db84ff35e15dbe8.png?s=3D40&r=3Dpg&d=3Didenticon" style= =3D"max-width:694px">codinghorror:</div> <blockquote><p style=3D"margin-top:0">I can't even find that topic by n= @@ -193,12 +193,12 @@ uld be invisible to me, and not showing up in Suggested Topics.</p> adding=3D"0" border=3D"0"><tbody> <tr> <td style=3D"vertical-align:top;width:55px"> - <img src=3D"http://www.gravatar.com/avatar/51d623f33f8b83095db84ff3= + <img src=3D"https://www.gravatar.com/avatar/51d623f33f8b83095db84ff3= 5e15dbe8.png?s=3D45&r=3Dpg&d=3Didenticon" title=3D"codinghorror" st= yle=3D"max-width:694px" width=3D"45" height=3D"45"> </td> <td> - <a href=3D"http://meta.discourse.org/users/codinghorror" style=3D"f= + <a href=3D"https://meta.discourse.org/users/codinghorror" style=3D"f= ont-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans= -serif;color:#3b5998;text-decoration:none;font-weight:bold" target=3D"_blan= k">codinghorror</a><br> @@ -219,12 +219,12 @@ rout" target=3D"_blank">@eviltrout</a>? I can't even find that topic by= adding=3D"0" border=3D"0"><tbody> <tr> <td style=3D"vertical-align:top;width:55px"> - <img src=3D"http://www.gravatar.com/avatar/5120fc4e345db0d1a9648882= + <img src=3D"https://www.gravatar.com/avatar/5120fc4e345db0d1a9648882= 72073819.png?s=3D45&r=3Dpg&d=3Didenticon" title=3D"riking" style=3D= "max-width:694px" width=3D"45" height=3D"45"> </td> <td> - <a href=3D"http://meta.discourse.org/users/riking" style=3D"font-si= + <a href=3D"https://meta.discourse.org/users/riking" style=3D"font-si= ze:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif= ;color:#3b5998;text-decoration:none;font-weight:bold" target=3D"_blank">rik= ing</a><br> @@ -241,7 +241,7 @@ lar spam post, and it was promptly deleted/hidden, but it just popped up in= <p style=3D"margin-top:0"></p> <div><a href=3D"//cdn.discourse.org/uploads/meta_discourse/2158/50b8b49557c= -b249e.png" target=3D"_blank"><img src=3D"http://cdn.discourse.org/uploads/m= +b249e.png" target=3D"_blank"><img src=3D"https://cdn.discourse.org/uploads/m= eta_discourse/_optimized/ab1/c92/acd2c33402_584x134.png" width=3D"584" heig= ht=3D"134" style=3D"max-width:694px"><div> @@ -257,12 +257,12 @@ ht=3D"134" style=3D"max-width:694px"><div> <div style=3D"color:#666"> <p>To respond, reply to this email or visit <a href=3D"http://meta.discours= e.org/t/spam-post-pops-back-up-in-suggested-topics/11005/5" style=3D"color:= -#666" target=3D"_blank">http://meta.discourse.org/t/spam-post-pops-back-up-= +#666" target=3D"_blank">https://meta.discourse.org/t/spam-post-pops-back-up-= in-suggested-topics/11005/5</a> in your browser.</p> </div> <div style=3D"color:#666"> -<p>To unsubscribe from these emails, visit your <a href=3D"http://meta.disc= +<p>To unsubscribe from these emails, visit your <a href=3D"https://meta.disc= ourse.org/user_preferences" style=3D"color:#666" target=3D"_blank">user pre= ferences</a>.</p> </div> diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index 5c5d53877a6..da0343588ef 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -117,7 +117,7 @@ describe ApplicationHelper do stub_config_setting(https: false) expect(helper.gravatar_icon(user_email)) - .to match('http://www.gravatar.com/avatar/b58c6f14d292556214bd64909bcdb118') + .to match('https://www.gravatar.com/avatar/b58c6f14d292556214bd64909bcdb118') end it 'uses HTTPs when configured' do diff --git a/spec/initializers/settings_spec.rb b/spec/initializers/settings_spec.rb index a11824d0ac5..838ca9fabef 100644 --- a/spec/initializers/settings_spec.rb +++ b/spec/initializers/settings_spec.rb @@ -24,7 +24,7 @@ describe Settings do expect(described_class.host_without_www('http://foo.com')).to eq 'foo.com' expect(described_class.host_without_www('http://www.foo.com')).to eq 'foo.com' expect(described_class.host_without_www('http://secure.foo.com')).to eq 'secure.foo.com' - expect(described_class.host_without_www('http://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'gravatar.com' + expect(described_class.host_without_www('https://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'gravatar.com' expect(described_class.host_without_www('https://foo.com')).to eq 'foo.com' expect(described_class.host_without_www('https://www.foo.com')).to eq 'foo.com' diff --git a/spec/javascripts/api_spec.js b/spec/javascripts/api_spec.js index 2aa4fb1f6c6..cc5fa42aafe 100644 --- a/spec/javascripts/api_spec.js +++ b/spec/javascripts/api_spec.js @@ -262,9 +262,9 @@ describe('Api', () => { it('fetches an issue template', (done) => { const namespace = 'some namespace'; const project = 'some project'; - const templateKey = 'template key'; + const templateKey = ' template #%?.key '; const templateType = 'template type'; - const expectedUrl = `${dummyUrlRoot}/${namespace}/${project}/templates/${templateType}/${templateKey}`; + const expectedUrl = `${dummyUrlRoot}/${namespace}/${project}/templates/${templateType}/${encodeURIComponent(templateKey)}`; spyOn(jQuery, 'ajax').and.callFake((request) => { expect(request.url).toEqual(expectedUrl); return sendDummyResponse(); diff --git a/spec/javascripts/behaviors/secret_values_spec.js b/spec/javascripts/behaviors/secret_values_spec.js index 9eeae474e7d..38d9bba6868 100644 --- a/spec/javascripts/behaviors/secret_values_spec.js +++ b/spec/javascripts/behaviors/secret_values_spec.js @@ -1,16 +1,24 @@ import SecretValues from '~/behaviors/secret_values'; -function generateFixtureMarkup(secrets, isRevealed) { +function generateValueMarkup( + secret, + valueClass = 'js-secret-value', + placeholderClass = 'js-secret-value-placeholder', +) { + return ` + <div class="${placeholderClass}"> + *** + </div> + <div class="hide ${valueClass}"> + ${secret} + </div> + `; +} + +function generateFixtureMarkup(secrets, isRevealed, valueClass, placeholderClass) { return ` <div class="js-secret-container"> - ${secrets.map(secret => ` - <div class="js-secret-value-placeholder"> - *** - </div> - <div class="hide js-secret-value"> - ${secret} - </div> - `).join('')} + ${secrets.map(secret => generateValueMarkup(secret, valueClass, placeholderClass)).join('')} <button class="js-secret-value-reveal-button" data-secret-reveal-status="${isRevealed}" @@ -21,11 +29,25 @@ function generateFixtureMarkup(secrets, isRevealed) { `; } -function setupSecretFixture(secrets, isRevealed) { +function setupSecretFixture( + secrets, + isRevealed, + valueClass = 'js-secret-value', + placeholderClass = 'js-secret-value-placeholder', +) { const wrapper = document.createElement('div'); - wrapper.innerHTML = generateFixtureMarkup(secrets, isRevealed); - - const secretValues = new SecretValues(wrapper.querySelector('.js-secret-container')); + wrapper.innerHTML = generateFixtureMarkup( + secrets, + isRevealed, + valueClass, + placeholderClass, + ); + + const secretValues = new SecretValues({ + container: wrapper.querySelector('.js-secret-container'), + valueSelector: `.${valueClass}`, + placeholderSelector: `.${placeholderClass}`, + }); secretValues.init(); return wrapper; @@ -49,7 +71,7 @@ describe('setupSecretValues', () => { expect(revealButton.textContent).toEqual('Hide value'); }); - it('should value hidden initially', () => { + it('should have value hidden initially', () => { const wrapper = setupSecretFixture(secrets, false); const values = wrapper.querySelectorAll('.js-secret-value'); const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder'); @@ -143,4 +165,64 @@ describe('setupSecretValues', () => { }); }); }); + + describe('with dynamic secrets', () => { + const secrets = ['mysecret123', 'happygoat456', 'tanuki789']; + + it('should toggle values and placeholders', () => { + const wrapper = setupSecretFixture(secrets, false); + // Insert the new dynamic row + wrapper.querySelector('.js-secret-container').insertAdjacentHTML('afterbegin', generateValueMarkup('foobarbazdynamic')); + + const revealButton = wrapper.querySelector('.js-secret-value-reveal-button'); + const values = wrapper.querySelectorAll('.js-secret-value'); + const placeholders = wrapper.querySelectorAll('.js-secret-value-placeholder'); + + revealButton.click(); + + expect(values.length).toEqual(4); + values.forEach((value) => { + expect(value.classList.contains('hide')).toEqual(false); + }); + expect(placeholders.length).toEqual(4); + placeholders.forEach((placeholder) => { + expect(placeholder.classList.contains('hide')).toEqual(true); + }); + + revealButton.click(); + + expect(values.length).toEqual(4); + values.forEach((value) => { + expect(value.classList.contains('hide')).toEqual(true); + }); + expect(placeholders.length).toEqual(4); + placeholders.forEach((placeholder) => { + expect(placeholder.classList.contains('hide')).toEqual(false); + }); + }); + }); + + describe('selector options', () => { + const secrets = ['mysecret123']; + + it('should respect `valueSelector` and `placeholderSelector` options', () => { + const valueClass = 'js-some-custom-placeholder-selector'; + const placeholderClass = 'js-some-custom-value-selector'; + + const wrapper = setupSecretFixture(secrets, false, valueClass, placeholderClass); + const values = wrapper.querySelectorAll(`.${valueClass}`); + const placeholders = wrapper.querySelectorAll(`.${placeholderClass}`); + const revealButton = wrapper.querySelector('.js-secret-value-reveal-button'); + + expect(values.length).toEqual(1); + expect(placeholders.length).toEqual(1); + + revealButton.click(); + + expect(values.length).toEqual(1); + expect(values[0].classList.contains('hide')).toEqual(false); + expect(placeholders.length).toEqual(1); + expect(placeholders[0].classList.contains('hide')).toEqual(true); + }); + }); }); diff --git a/spec/javascripts/environments/environment_item_spec.js b/spec/javascripts/environments/environment_item_spec.js index 0e141adb628..7a34126eef7 100644 --- a/spec/javascripts/environments/environment_item_spec.js +++ b/spec/javascripts/environments/environment_item_spec.js @@ -68,7 +68,7 @@ describe('Environment item', () => { username: 'root', id: 1, state: 'active', - avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', web_url: 'http://localhost:3000/root', }, commit: { @@ -84,7 +84,7 @@ describe('Environment item', () => { username: 'root', id: 1, state: 'active', - avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', web_url: 'http://localhost:3000/root', }, commit_path: '/root/ci-folders/tree/500aabcb17c97bdcf2d0c410b70cb8556f0362dd', diff --git a/spec/javascripts/fixtures/projects.json b/spec/javascripts/fixtures/projects.json index 1339ee00870..68a150f602a 100644 --- a/spec/javascripts/fixtures/projects.json +++ b/spec/javascripts/fixtures/projects.json @@ -14,7 +14,7 @@ "username": "root", "id": 1, "state": "active", - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon", + "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon", "web_url": "http://localhost:3000/u/root" }, "name": "test", diff --git a/spec/javascripts/helpers/user_mock_data_helper.js b/spec/javascripts/helpers/user_mock_data_helper.js index a9783ea065c..323fee3767e 100644 --- a/spec/javascripts/helpers/user_mock_data_helper.js +++ b/spec/javascripts/helpers/user_mock_data_helper.js @@ -4,7 +4,7 @@ export default { for (let i = 0; i < numberUsers; i = i += 1) { users.push( { - avatar: 'http://gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + avatar: 'https://gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', id: (i + 1), name: `GitLab User ${i}`, username: `gitlab${i}`, diff --git a/spec/javascripts/jobs/mock_data.js b/spec/javascripts/jobs/mock_data.js index 43532275121..43589d54be4 100644 --- a/spec/javascripts/jobs/mock_data.js +++ b/spec/javascripts/jobs/mock_data.js @@ -37,7 +37,7 @@ export default { username: 'root', id: 1, state: 'active', - avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', web_url: 'http://localhost:3000/root', }, erase_path: '/root/ci-mock/-/jobs/4757/erase', @@ -54,7 +54,7 @@ export default { username: 'root', id: 1, state: 'active', - avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', web_url: 'http://localhost:3000/root', }, active: false, @@ -107,10 +107,10 @@ export default { username: 'root', id: 1, state: 'active', - avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', web_url: 'http://localhost:3000/root', }, - author_gravatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', + author_gravatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon', commit_url: 'http://localhost:3000/root/ci-mock/commit/c58647773a6b5faf066d4ad6ff2c9fbba5f180f6', commit_path: '/root/ci-mock/commit/c58647773a6b5faf066d4ad6ff2c9fbba5f180f6', }, diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js index b020a1020df..f0c800c759d 100644 --- a/spec/javascripts/notes/mock_data.js +++ b/spec/javascripts/notes/mock_data.js @@ -107,7 +107,7 @@ export const note = { "name": "Administrator", "username": "root", "state": "active", - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", "path": "/root" }, "created_at": "2017-08-10T15:24:03.087Z", diff --git a/spec/javascripts/sidebar/mock_data.js b/spec/javascripts/sidebar/mock_data.js index 7bc591d2d47..d9e84e35f69 100644 --- a/spec/javascripts/sidebar/mock_data.js +++ b/spec/javascripts/sidebar/mock_data.js @@ -27,7 +27,7 @@ const RESPONSE_MAP = { username: 'user0', id: 22, state: 'active', - avatar_url: 'http: //www.gravatar.com/avatar/52e4ce24a915fb7e51e1ad3b57f4b00a?s=80\u0026d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/52e4ce24a915fb7e51e1ad3b57f4b00a?s=80\u0026d=identicon', web_url: 'http: //localhost:3001/user0', }, { @@ -35,7 +35,7 @@ const RESPONSE_MAP = { username: 'tajuana', id: 18, state: 'active', - avatar_url: 'http: //www.gravatar.com/avatar/4852a41fb41616bf8f140d3701673f53?s=80\u0026d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/4852a41fb41616bf8f140d3701673f53?s=80\u0026d=identicon', web_url: 'http: //localhost:3001/tajuana', }, { @@ -43,7 +43,7 @@ const RESPONSE_MAP = { username: 'michaele.will', id: 16, state: 'active', - avatar_url: 'http: //www.gravatar.com/avatar/e301827eb03be955c9c172cb9a8e4e8a?s=80\u0026d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/e301827eb03be955c9c172cb9a8e4e8a?s=80\u0026d=identicon', web_url: 'http: //localhost:3001/michaele.will', }, ], @@ -72,24 +72,24 @@ const RESPONSE_MAP = { username: 'user0', id: 22, state: 'active', - avatar_url: 'http: //www.gravatar.com/avatar/52e4ce24a915fb7e51e1ad3b57f4b00a?s=80\u0026d=identicon', - web_url: 'http: //localhost:3001/user0', + avatar_url: 'https://www.gravatar.com/avatar/52e4ce24a915fb7e51e1ad3b57f4b00a?s=80\u0026d=identicon', + web_url: 'http://localhost:3001/user0', }, { name: 'Marguerite Bartell', username: 'tajuana', id: 18, state: 'active', - avatar_url: 'http: //www.gravatar.com/avatar/4852a41fb41616bf8f140d3701673f53?s=80\u0026d=identicon', - web_url: 'http: //localhost:3001/tajuana', + avatar_url: 'https://www.gravatar.com/avatar/4852a41fb41616bf8f140d3701673f53?s=80\u0026d=identicon', + web_url: 'http://localhost:3001/tajuana', }, { name: 'Laureen Ritchie', username: 'michaele.will', id: 16, state: 'active', - avatar_url: 'http: //www.gravatar.com/avatar/e301827eb03be955c9c172cb9a8e4e8a?s=80\u0026d=identicon', - web_url: 'http: //localhost:3001/michaele.will', + avatar_url: 'https://www.gravatar.com/avatar/e301827eb03be955c9c172cb9a8e4e8a?s=80\u0026d=identicon', + web_url: 'http://localhost:3001/michaele.will', }, ], human_time_estimate: null, @@ -100,24 +100,24 @@ const RESPONSE_MAP = { username: 'user0', id: 22, state: 'active', - avatar_url: 'http: //www.gravatar.com/avatar/52e4ce24a915fb7e51e1ad3b57f4b00a?s=80\u0026d=identicon', - web_url: 'http: //localhost:3001/user0', + avatar_url: 'https://www.gravatar.com/avatar/52e4ce24a915fb7e51e1ad3b57f4b00a?s=80\u0026d=identicon', + web_url: 'http://localhost:3001/user0', }, { name: 'Marguerite Bartell', username: 'tajuana', id: 18, state: 'active', - avatar_url: 'http: //www.gravatar.com/avatar/4852a41fb41616bf8f140d3701673f53?s=80\u0026d=identicon', - web_url: 'http: //localhost:3001/tajuana', + avatar_url: 'https://www.gravatar.com/avatar/4852a41fb41616bf8f140d3701673f53?s=80\u0026d=identicon', + web_url: 'http://localhost:3001/tajuana', }, { name: 'Laureen Ritchie', username: 'michaele.will', id: 16, state: 'active', - avatar_url: 'http: //www.gravatar.com/avatar/e301827eb03be955c9c172cb9a8e4e8a?s=80\u0026d=identicon', - web_url: 'http: //localhost:3001/michaele.will', + avatar_url: 'https://www.gravatar.com/avatar/e301827eb03be955c9c172cb9a8e4e8a?s=80\u0026d=identicon', + web_url: 'http://localhost:3001/michaele.will', }, ], subscribed: true, @@ -182,7 +182,7 @@ const mockData = { id: 1, name: 'Administrator', username: 'root', - avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', }, rootPath: '/', fullPath: '/gitlab-org/gitlab-shell', @@ -194,7 +194,7 @@ const mockData = { human_total_time_spent: null, }, user: { - avatar: 'http://gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + avatar: 'https://gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', id: 1, name: 'Administrator', username: 'root', diff --git a/spec/javascripts/sidebar/sidebar_store_spec.js b/spec/javascripts/sidebar/sidebar_store_spec.js index ea4eae1e23f..3591f96ff87 100644 --- a/spec/javascripts/sidebar/sidebar_store_spec.js +++ b/spec/javascripts/sidebar/sidebar_store_spec.js @@ -6,14 +6,14 @@ const ASSIGNEE = { id: 2, name: 'gitlab user 2', username: 'gitlab2', - avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', }; const ANOTHER_ASSINEE = { id: 3, name: 'gitlab user 3', username: 'gitlab3', - avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', }; const PARTICIPANT = { @@ -38,7 +38,7 @@ describe('Sidebar store', () => { id: 1, name: 'Administrator', username: 'root', - avatar_url: 'http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + avatar_url: 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', }, editable: true, rootPath: '/', diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index ae494267659..3dd75307484 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -38,7 +38,7 @@ export default { "username": "root", "id": 1, "state": "active", - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", "web_url": "http://localhost:3000/root" }, "merged_at": "2017-04-07T15:39:25.696Z", @@ -50,7 +50,7 @@ export default { "username": "root", "id": 1, "state": "active", - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", "web_url": "http://localhost:3000/root" }, "merge_user": null, @@ -64,7 +64,7 @@ export default { "username": "root", "id": 1, "state": "active", - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", "web_url": "http://localhost:3000/root" }, "active": false, @@ -159,10 +159,10 @@ export default { "username": "root", "id": 1, "state": "active", - "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", "web_url": "http://localhost:3000/root" }, - "author_gravatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", + "author_gravatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon", "commit_url": "http://localhost:3000/root/acets-app/commit/104096c51715e12e7ae41f9333e9fa35b73f385d", "commit_path": "/root/acets-app/commit/104096c51715e12e7ae41f9333e9fa35b73f385d" }, diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index 168207552ff..8ac960133c5 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -268,6 +268,21 @@ describe Gitlab::Git::Blob, seed_helper: true do expect(blobs).to all( be_a(Gitlab::Git::Blob) ) end + it 'accepts blob IDs as a lazy enumerator' do + blobs = described_class.batch_lfs_pointers(repository, [lfs_blob.id].lazy) + + expect(blobs.count).to eq(1) + expect(blobs).to all( be_a(Gitlab::Git::Blob) ) + end + + it 'handles empty list of IDs gracefully' do + blobs_1 = described_class.batch_lfs_pointers(repository, [].lazy) + blobs_2 = described_class.batch_lfs_pointers(repository, []) + + expect(blobs_1).to eq([]) + expect(blobs_2).to eq([]) + end + it 'silently ignores tree objects' do blobs = described_class.batch_lfs_pointers(repository, [tree_object.oid]) diff --git a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb index d72572cd510..44695acbe7d 100644 --- a/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb @@ -244,7 +244,7 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do it 'returns true when a commit exists' do expect(project.repository) - .to receive(:lookup) + .to receive(:commit) .with('123') .and_return(double(:commit)) @@ -253,9 +253,9 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do it 'returns false when a commit does not exist' do expect(project.repository) - .to receive(:lookup) + .to receive(:commit) .with('123') - .and_raise(Rugged::OdbError) + .and_return(nil) expect(importer.commit_exists?('123')).to eq(false) end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index c76f32b3989..429b6615131 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1127,9 +1127,19 @@ describe MergeRequest do end end - context 'when the revert commit is mentioned in a note before the MR was merged' do + context 'when the revert commit is mentioned in a note just before the MR was merged' do before do - subject.notes.last.update!(created_at: subject.metrics.merged_at - 1.second) + subject.notes.last.update!(created_at: subject.metrics.merged_at - 30.seconds) + end + + it 'returns false' do + expect(subject.can_be_reverted?(current_user)).to be_falsey + end + end + + context 'when the revert commit is mentioned in a note long before the MR was merged' do + before do + subject.notes.last.update!(created_at: subject.metrics.merged_at - 2.minutes) end it 'returns true' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 8f406253f39..7c61c6b7299 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2356,7 +2356,7 @@ describe Repository do let(:commit) { repository.commit } let(:ancestor) { commit.parents.first } - context 'with Gitaly enabled' do + shared_examples '#ancestor?' do it 'it is an ancestor' do expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true) end @@ -2370,27 +2370,19 @@ describe Repository do expect(repository.ancestor?(ancestor.id, nil)).to eq(false) expect(repository.ancestor?(nil, nil)).to eq(false) end - end - - context 'with Gitaly disabled' do - before do - allow(Gitlab::GitalyClient).to receive(:enabled?).and_return(false) - allow(Gitlab::GitalyClient).to receive(:feature_enabled?).with(:is_ancestor).and_return(false) - end - it 'it is an ancestor' do - expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true) + it 'returns false for invalid commit IDs' do + expect(repository.ancestor?(commit.id, Gitlab::Git::BLANK_SHA)).to eq(false) + expect(repository.ancestor?( Gitlab::Git::BLANK_SHA, commit.id)).to eq(false) end + end - it 'it is not an ancestor' do - expect(repository.ancestor?(commit.id, ancestor.id)).to eq(false) - end + context 'with Gitaly enabled' do + it_behaves_like('#ancestor?') + end - it 'returns false on nil-values' do - expect(repository.ancestor?(nil, commit.id)).to eq(false) - expect(repository.ancestor?(ancestor.id, nil)).to eq(false) - expect(repository.ancestor?(nil, nil)).to eq(false) - end + context 'with Gitaly disabled', :skip_gitaly_mock do + it_behaves_like('#ancestor?') end end |