diff options
229 files changed, 3371 insertions, 1177 deletions
diff --git a/.gitignore b/.gitignore index 8f861d76a37..ce6a363fe35 100644 --- a/.gitignore +++ b/.gitignore @@ -4,46 +4,46 @@ .bundle .chef .directory -.envrc -.gitlab_shell_secret +/.envrc +/.gitlab_shell_secret .idea -.rbenv-version +/.rbenv-version .rbx/ -.ruby-gemset -.ruby-version -.rvmrc +/.ruby-gemset +/.ruby-version +/.rvmrc .sass-cache/ -.secret -.vagrant -.byebug_history -Vagrantfile -backups/* -config/aws.yml -config/database.yml -config/gitlab.yml -config/gitlab_ci.yml -config/initializers/rack_attack.rb -config/initializers/smtp_settings.rb -config/initializers/relative_url.rb -config/resque.yml -config/unicorn.rb -config/secrets.yml -config/sidekiq.yml -coverage/* -db/*.sqlite3 -db/*.sqlite3-journal -db/data.yml -doc/code/* -dump.rdb -log/*.log* -nohup.out -public/assets/ -public/uploads.* -public/uploads/ -shared/artifacts/ -rails_best_practices_output.html +/.secret +/.vagrant +/.byebug_history +/Vagrantfile +/backups/* +/config/aws.yml +/config/database.yml +/config/gitlab.yml +/config/gitlab_ci.yml +/config/initializers/rack_attack.rb +/config/initializers/smtp_settings.rb +/config/initializers/relative_url.rb +/config/resque.yml +/config/unicorn.rb +/config/secrets.yml +/config/sidekiq.yml +/coverage/* +/db/*.sqlite3 +/db/*.sqlite3-journal +/db/data.yml +/doc/code/* +/dump.rdb +/log/*.log* +/nohup.out +/public/assets/ +/public/uploads.* +/public/uploads/ +/shared/artifacts/ +/rails_best_practices_output.html /tags -tmp/ -vendor/bundle/* -builds/* -shared/* +/tmp/* +/vendor/bundle/* +/builds/* +/shared/* diff --git a/.rubocop.yml b/.rubocop.yml index 2d8eb4077f3..84a8015b410 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -222,7 +222,7 @@ Style/EndBlock: # Use Unix-style line endings. Style/EndOfLine: - Enabled: false + Enabled: true # Favor the use of Fixnum#even? && Fixnum#odd? Style/EvenOdd: @@ -247,7 +247,7 @@ Style/FlipFlop: # Checks use of for or each in multiline loops. Style/For: - Enabled: false + Enabled: true # Enforce the use of Kernel#sprintf, Kernel#format or String#%. Style/FormatString: @@ -286,7 +286,7 @@ Style/IdenticalConditionalBranches: # Checks the indentation of the first line of the right-hand-side of a # multi-line assignment. Style/IndentAssignment: - Enabled: false + Enabled: true # Keep indentation straight. Style/IndentationConsistency: @@ -318,7 +318,7 @@ Style/LambdaCall: # Comments should start with a space. Style/LeadingCommentSpace: - Enabled: false + Enabled: true # Use \ instead of + or << to concatenate two string literals at line end. Style/LineEndConcatenation: @@ -330,7 +330,7 @@ Style/MethodCallParentheses: # Checks if the method definitions have or don't have parentheses. Style/MethodDefParentheses: - Enabled: false + Enabled: true # Use the configured style when naming methods. Style/MethodName: @@ -362,7 +362,7 @@ Style/MultilineHashBraceLayout: # Do not use then for multi-line if/unless. Style/MultilineIfThen: - Enabled: false + Enabled: true # Checks that the closing brace in a method call is either on the same line as # the last method argument, or a new line. @@ -394,7 +394,7 @@ Style/MutableConstant: # Favor unless over if for negative conditions (or control flow or). Style/NegatedIf: - Enabled: false + Enabled: true # Favor until over while for negative conditions. Style/NegatedWhile: @@ -486,10 +486,9 @@ Style/RedundantException: Style/RedundantFreeze: Enabled: false -# TODO: Enable RedundantParentheses Cop. # Checks for parentheses that seem not to serve any purpose. Style/RedundantParentheses: - Enabled: false + Enabled: true # Don't use return where it's not required. Style/RedundantReturn: @@ -515,7 +514,7 @@ Style/SelfAssignment: # Don't use semicolons to terminate expressions. Style/Semicolon: - Enabled: false + Enabled: true # Checks for proper usage of fail and raise. Style/SignalException: @@ -541,7 +540,7 @@ Style/SpaceAfterComma: # Do not put a space between a method name and the opening parenthesis in a # method definition. Style/SpaceAfterMethodName: - Enabled: false + Enabled: true # Tracks redundant space after the ! operator. Style/SpaceAfterNot: @@ -570,11 +569,11 @@ Style/SpaceBeforeBlockBraces: # No spaces before commas. Style/SpaceBeforeComma: - Enabled: false + Enabled: true # Checks for missing space between code and a comment on the same line. Style/SpaceBeforeComment: - Enabled: false + Enabled: true # Checks that exactly one space is used between a method name and the first # argument for method calls without parentheses. @@ -705,7 +704,7 @@ Style/WhenThen: # Checks for redundant do after while or until. Style/WhileUntilDo: - Enabled: false + Enabled: true # Favor modifier while/until usage when you have a single-line body. Style/WhileUntilModifier: @@ -785,7 +784,7 @@ Lint/AssignmentInCondition: # Align block ends correctly. Lint/BlockAlignment: - Enabled: false + Enabled: true # Default values in optional keyword arguments and optional ordinal arguments # should not refer back to the name of the argument. @@ -878,7 +877,7 @@ Lint/InvalidCharacterLiteral: # Checks of literals used in conditions. Lint/LiteralInCondition: - Enabled: false + Enabled: true # Checks for literals used in interpolation. Lint/LiteralInInterpolation: @@ -1027,10 +1026,9 @@ Performance/StartWith: Performance/StringReplacement: Enabled: true -# TODO: Enable TimesMap Cop. # Checks for `.times.map` calls. Performance/TimesMap: - Enabled: false + Enabled: true ##################### Rails ################################## diff --git a/CHANGELOG b/CHANGELOG index a0194a1ba25..d1cde40c1c7 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,21 +1,38 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.9.0 (unreleased) + - Allow enabling wiki page events from Webhook management UI + - Make EmailsOnPushWorker use Sidekiq mailers queue + - Fix wiki page events' webhook to point to the wiki repository + - Fix issue todo not remove when leave project !4150 (Long Nguyen) - Allow forking projects with restricted visibility level + - Improve note validation to prevent errors when creating invalid note via API + - Remove project notification settings associated with deleted projects + - Fix 404 page when viewing TODOs that contain milestones or labels in different projects - Redesign navigation for project pages - Fix groups API to list only user's accessible projects - Redesign account and email confirmation emails - Use gitlab-shell v3.0.0 + - Add DB index on users.state - Add rake task 'gitlab:db:configure' for conditionally seeding or migrating the database - Changed the Slack build message to use the singular duration if necessary (Aran Koning) - Fix issues filter when ordering by milestone - Todos will display target state if issuable target is 'Closed' or 'Merged' + - Fix bug when sorting issues by milestone due date and filtering by two or more labels - Remove 'main language' feature + - Pipelines can be canceled only when there are running builds + - Use downcased path to container repository as this is expected path by Docker + - Projects pending deletion will render a 404 page - Measure queue duration between gitlab-workhorse and Rails + - Make authentication service for Container Registry to be compatible with < Docker 1.11 + - Add Application Setting to configure Container Registry token expire delay (default 5min) v 8.8.3 + - Fix incorrect links on pipeline page when merge request created from fork - Fix gitlab importer failing to import new projects due to missing credentials - Fix import URL migration not rescuing with the correct Error + - In search results, only show notes on confidential issues that the user has access to + - Fix health check access token changing due to old application settings being used v 8.8.2 - Added remove due date button. !4209 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9fe4cf7b0f6..a15f8c4fec7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -311,13 +311,11 @@ request is as follows: 1. Create a feature branch 1. Write [tests](https://gitlab.com/gitlab-org/gitlab-development-kit#running-the-tests) and code 1. Add your changes to the [CHANGELOG](CHANGELOG) -1. If you are changing the README, some documentation or other things which - have no effect on the tests, add `[ci skip]` somewhere in the commit message - and make sure to read the [documentation styleguide][doc-styleguide] +1. If you are writing documentation, make sure to read the [documentation styleguide][doc-styleguide] 1. If you have multiple commits please combine them into one commit by [squashing them][git-squash] 1. Push the commit(s) to your fork -1. Submit a merge request (MR) to the master branch +1. Submit a merge request (MR) to the `master` branch 1. The MR title should describe the change you want to make 1. The MR description should give a motive for your change and the method you used to achieve it, see the [merge request description format] @@ -18,9 +18,8 @@ gem "mysql2", '~> 0.3.16', group: :mysql gem "pg", '~> 0.18.2', group: :postgres # Authentication libraries -gem 'devise', '~> 3.5.4' +gem 'devise', '~> 4.0' gem 'doorkeeper', '~> 3.1' -gem 'devise-async', '~> 0.9.0' gem 'omniauth', '~> 1.3.1' gem 'omniauth-auth0', '~> 1.4.1' gem 'omniauth-azure-oauth2', '~> 0.0.6' @@ -43,9 +42,9 @@ gem 'recaptcha', require: 'recaptcha/rails' gem 'akismet', '~> 2.0' # Two-factor authentication -gem 'devise-two-factor', '~> 2.0.0' +gem 'devise-two-factor', '~> 3.0.0' gem 'rqrcode-rails3', '~> 0.1.7' -gem 'attr_encrypted', '~> 1.3.4' +gem 'attr_encrypted', '~> 3.0.0' # Browser detection gem "browser", '~> 1.0.0' @@ -73,7 +72,7 @@ gem 'grape-entity', '~> 0.4.2' gem 'rack-cors', '~> 0.4.0', require: 'rack/cors' # Pagination -gem "kaminari", "~> 0.16.3" +gem "kaminari", "~> 0.17.0" # HAML gem "haml-rails", '~> 0.9.0' diff --git a/Gemfile.lock b/Gemfile.lock index fa2b72b2524..4e000fa5b5b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -60,8 +60,8 @@ GEM oauth2 (~> 1.0) asciidoctor (1.5.3) ast (2.2.0) - attr_encrypted (1.3.4) - encryptor (>= 1.3.0) + attr_encrypted (3.0.1) + encryptor (~> 3.0.0) attr_required (1.0.0) autoprefixer-rails (6.2.3) execjs @@ -73,7 +73,7 @@ GEM thread_safe (~> 0.3, >= 0.3.1) babosa (1.0.2) base32 (0.3.2) - bcrypt (3.1.10) + bcrypt (3.1.11) benchmark-ips (2.3.0) better_errors (1.0.1) coderay (>= 1.0.0) @@ -155,21 +155,18 @@ GEM activerecord (>= 3.2.0, < 5.0) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) - devise (3.5.4) + devise (4.1.1) bcrypt (~> 3.0) orm_adapter (~> 0.1) - railties (>= 3.2.6, < 5) + railties (>= 4.1.0, < 5.1) responders - thread_safe (~> 0.1) warden (~> 1.2.3) - devise-async (0.9.0) - devise (~> 3.2) - devise-two-factor (2.0.1) + devise-two-factor (3.0.0) activesupport - attr_encrypted (~> 1.3.2) - devise (~> 3.5.0) + attr_encrypted (>= 1.3, < 4, != 2) + devise (~> 4.0) railties - rotp (~> 2) + rotp (~> 2.0) diff-lcs (1.2.5) diffy (3.0.7) docile (1.1.5) @@ -181,7 +178,7 @@ GEM email_spec (1.6.0) launchy (~> 2.1) mail (~> 2.2) - encryptor (1.3.0) + encryptor (3.0.0) equalizer (0.0.11) erubis (2.7.0) escape_utils (1.1.1) @@ -442,7 +439,7 @@ GEM railties (>= 3.2.16) json (1.8.3) jwt (1.5.2) - kaminari (0.16.3) + kaminari (0.17.0) actionpack (>= 3.0.0) activesupport (>= 3.0.0) kgio (2.10.0) @@ -656,7 +653,7 @@ GEM responders (2.1.1) railties (>= 4.2.0, < 5.1) rinku (1.7.3) - rotp (2.1.1) + rotp (2.1.2) rouge (1.10.1) rqrcode (0.7.0) chunky_png @@ -859,7 +856,7 @@ GEM coercible (~> 1.0) descendants_tracker (~> 0.0, >= 0.0.3) equalizer (~> 0.0, >= 0.0.9) - warden (1.2.4) + warden (1.2.6) rack (>= 1.0) web-console (2.3.0) activemodel (>= 4.0) @@ -894,7 +891,7 @@ DEPENDENCIES allocations (~> 1.0) asana (~> 0.4.0) asciidoctor (~> 1.5.2) - attr_encrypted (~> 1.3.4) + attr_encrypted (~> 3.0.0) awesome_print (~> 1.2.0) babosa (~> 1.0.2) base32 (~> 0.3.0) @@ -919,9 +916,8 @@ DEPENDENCIES d3_rails (~> 3.5.0) database_cleaner (~> 1.4.0) default_value_for (~> 3.0.0) - devise (~> 3.5.4) - devise-async (~> 0.9.0) - devise-two-factor (~> 2.0.0) + devise (~> 4.0) + devise-two-factor (~> 3.0.0) diffy (~> 3.0.3) doorkeeper (~> 3.1) dropzonejs-rails (~> 0.7.1) @@ -959,7 +955,7 @@ DEPENDENCIES jquery-turbolinks (~> 2.1.0) jquery-ui-rails (~> 5.0.0) jwt - kaminari (~> 0.16.3) + kaminari (~> 0.17.0) letter_opener_web (~> 1.3.0) licensee (~> 8.0.0) loofah (~> 2.0.3) diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index eaf85bb17ca..467f3b35d74 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -16,6 +16,19 @@ @include btn-default; } +@mixin btn-outline($background, $text, $border, $hover-background, $hover-text, $hover-border) { + background-color: $background; + color: $text; + border-color: $border; + + &:hover, + &:focus { + background-color: $hover-background; + color: $hover-text; + border-color: $hover-border;; + } +} + @mixin btn-color($light, $border-light, $normal, $border-normal, $dark, $border-dark, $color) { background-color: $light; border-color: $border-light; @@ -106,11 +119,14 @@ @include btn-blue; } - &.btn-close, &.btn-warning { @include btn-orange; } + &.btn-close { + @include btn-outline($white-light, $orange-normal, $orange-normal, $orange-light, $white-light, $orange-light); + } + &.btn-danger, &.btn-remove, &.btn-red { diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index c7784e15844..f253da814bc 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -63,7 +63,8 @@ $gl-padding-top: 10px; /* * Misc */ -$row-hover: #f4f8fe; +$row-hover: #f7faff; +$row-hover-border: #b2d7ff; $progress-color: #c0392b; $avatar_radius: 50%; $header-height: 50px; @@ -104,7 +105,7 @@ $blue-medium-light: #3498cb; $blue-medium: #2f8ebf; $blue-medium-dark: #2d86b4; -$orange-light: rgba(252, 109, 38, 0.80); +$orange-light: #fc8a51; $orange-normal: #e75e40; $orange-dark: #ce5237; diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 4f8a8748d3f..8046e203a99 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -41,7 +41,7 @@ margin: 0; margin-left: 20px; padding: 5px; - padding-top: 12px; + padding-top: 8px; line-height: 20px; &.right { @@ -110,6 +110,29 @@ p:last-child { margin-bottom: 0; } + + @media (max-width: $screen-sm-max) { + h4 { + font-size: 15px; + } + + p { + font-size: 13px; + } + + .btn, + .btn-group, + .accept-action { + width: 100%; + margin-bottom: 4px; + } + + .accept-control { + width: 100%; + text-align: center; + margin: 0; + } + } } .mr-widget-footer { diff --git a/app/assets/stylesheets/pages/tree.scss b/app/assets/stylesheets/pages/tree.scss index a84fc2e0318..f16fc7f388f 100644 --- a/app/assets/stylesheets/pages/tree.scss +++ b/app/assets/stylesheets/pages/tree.scss @@ -15,16 +15,23 @@ margin-bottom: 0; tr { - > td, > th { + border-bottom: 1px solid $table-border-gray; + border-top: 1px solid $table-border-gray; + + td, th { line-height: 23px; } &:hover { + cursor: pointer; + td { - background: $row-hover; + background-color: $row-hover; + border-top: 1px solid $row-hover-border; + border-bottom: 1px solid $row-hover-border; } - cursor: pointer; } + &.selected { td { background: $gray-dark; diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index ff7a5cad2fb..0a34a12e2a7 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -107,6 +107,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController :repository_checks_enabled, :metrics_packet_size, :send_user_confirmation_email, + :container_registry_token_expire_delay, restricted_visibility_levels: [], import_sources: [], disabled_oauth_sign_in_sources: [] diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 1429ee40bb7..c28d1ca9e3b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -232,7 +232,7 @@ class ApplicationController < ActionController::Base end def configure_permitted_parameters - devise_parameter_sanitizer.for(:sign_in) { |u| u.permit(:username, :email, :password, :login, :remember_me, :otp_attempt) } + devise_parameter_sanitizer.permit(:sign_in, keys: [:username, :email, :password, :login, :remember_me, :otp_attempt]) end def hexdigest(string) @@ -263,7 +263,7 @@ class ApplicationController < ActionController::Base # internal repos where you are not a member. Enable this filter # or improve current implementation to filter only issues you # created or assigned or mentioned - #@filter_params[:authorized_only] = true + # @filter_params[:authorized_only] = true end @filter_params diff --git a/app/controllers/jwt_controller.rb b/app/controllers/jwt_controller.rb index 156ab2811d6..cee3b6c43e7 100644 --- a/app/controllers/jwt_controller.rb +++ b/app/controllers/jwt_controller.rb @@ -32,7 +32,7 @@ class JwtController < ApplicationController end def auth_params - params.permit(:service, :scope, :offline_token, :account, :client_id) + params.permit(:service, :scope, :account, :client_id) end def authenticate_project(login, password) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index df98f56a1cd..f35d631df0c 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -97,7 +97,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController handle_signup_error end - def handle_service_ticket provider, ticket + def handle_service_ticket(provider, ticket) Gitlab::OAuth::Session.create provider, ticket session[:service_tickets] ||= {} session[:service_tickets][provider] = ticket diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index be872a93fee..776ba92c9ab 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -26,7 +26,7 @@ class Projects::ApplicationController < ApplicationController project_path = "#{namespace}/#{id}" @project = Project.find_with_namespace(project_path) - if @project && can?(current_user, :read_project, @project) + if can?(current_user, :read_project, @project) && !@project.pending_delete? if @project.path_with_namespace != project_path redirect_to request.original_url.gsub(project_path, @project.path_with_namespace) end diff --git a/app/controllers/projects/find_file_controller.rb b/app/controllers/projects/find_file_controller.rb index 54a0c447aee..cf53ad0a670 100644 --- a/app/controllers/projects/find_file_controller.rb +++ b/app/controllers/projects/find_file_controller.rb @@ -1,26 +1,26 @@ -# Controller for viewing a repository's file structure
-class Projects::FindFileController < Projects::ApplicationController
- include ExtractsPath
- include ActionView::Helpers::SanitizeHelper
- include TreeHelper
-
- before_action :require_non_empty_project
- before_action :assign_ref_vars
- before_action :authorize_download_code!
-
- def show
- return render_404 unless @repository.commit(@ref)
-
- respond_to do |format|
- format.html
- end
- end
-
- def list
- file_paths = @repo.ls_files(@ref)
-
- respond_to do |format|
- format.json { render json: file_paths }
- end
- end
-end
+# Controller for viewing a repository's file structure +class Projects::FindFileController < Projects::ApplicationController + include ExtractsPath + include ActionView::Helpers::SanitizeHelper + include TreeHelper + + before_action :require_non_empty_project + before_action :assign_ref_vars + before_action :authorize_download_code! + + def show + return render_404 unless @repository.commit(@ref) + + respond_to do |format| + format.html + end + end + + def list + file_paths = @repo.ls_files(@ref) + + respond_to do |format| + format.json { render json: file_paths } + end + end +end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index 47524b1cf0b..a60027ff477 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -63,7 +63,8 @@ class Projects::HooksController < Projects::ApplicationController :push_events, :tag_push_events, :token, - :url + :url, + :wiki_page_events ) end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 17ce8e2ad20..d54284d7b20 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -205,7 +205,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def branch_from - #This is always source + # This is always source @source_project = @merge_request.nil? ? @project : @merge_request.source_project @commit = @repository.commit(params[:ref]) if params[:ref].present? render layout: false diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index 0d6c32fabd2..4b404eb03fa 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -91,8 +91,8 @@ class Projects::WikisController < Projects::ApplicationController def markdown_preview text = params[:text] - ext = Gitlab::ReferenceExtractor.new(@project, current_user, current_user) - ext.analyze(text) + ext = Gitlab::ReferenceExtractor.new(@project, current_user) + ext.analyze(text, author: current_user) render json: { body: view_context.markdown(text, pipeline: :wiki, project_wiki: @project_wiki), diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 9697b88c032..f94e2a84fa2 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -197,8 +197,8 @@ class ProjectsController < Projects::ApplicationController def markdown_preview text = params[:text] - ext = Gitlab::ReferenceExtractor.new(@project, current_user, current_user) - ext.analyze(text) + ext = Gitlab::ReferenceExtractor.new(@project, current_user) + ext.analyze(text, author: current_user) render json: { body: view_context.markdown(text), diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index c29f4609e93..d68c2a708e3 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,5 +1,6 @@ class SessionsController < Devise::SessionsController include AuthenticatesWithTwoFactor + include Devise::Controllers::Rememberable include Recaptcha::ClientHelper skip_before_action :check_2fa_requirement, only: [:destroy] @@ -96,6 +97,7 @@ class SessionsController < Devise::SessionsController # Remove any lingering user data from login session.delete(:otp_user_id) + remember_me(user) if user_params[:remember_me] == '1' sign_in(user) and return else flash.now[:alert] = 'Invalid two-factor code.' diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 8ed3ccf1c02..7d8c56f4c22 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -271,7 +271,7 @@ class IssuableFinder if filter_by_no_label? items = items.without_label else - items = items.with_label(label_names) + items = items.with_label(label_names, params[:sort]) if projects items = items.where(labels: { project_id: projects }) end diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index ea383f9b0f6..cbe47176831 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -39,11 +39,11 @@ module DiffHelper end def unfold_bottom_class(bottom) - (bottom) ? 'js-unfold-bottom' : '' + bottom ? 'js-unfold-bottom' : '' end def unfold_class(unfold) - (unfold) ? 'unfold js-unfold' : '' + unfold ? 'unfold js-unfold' : '' end def diff_line_content(line, line_type = nil) diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index 551409e8855..173bdbb8654 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -147,8 +147,8 @@ module IssuesHelper def emoji_author_list(notes, current_user) list = notes.map do |note| - note.author == current_user ? "me" : note.author.name - end + note.author == current_user ? "me" : note.author.name + end list.join(", ") end diff --git a/app/models/ability.rb b/app/models/ability.rb index b354b1990c7..44515550d9e 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -23,20 +23,41 @@ class Ability end.concat(global_abilities(user)) end + # Given a list of users and a project this method returns the users that can + # read the given project. + def users_that_can_read_project(users, project) + if project.public? + users + else + users.select do |user| + if user.admin? + true + elsif project.internal? && !user.external? + true + elsif project.owner == user + true + elsif project.team.members.include?(user) + true + else + false + end + end + end + end + # List of possible abilities for anonymous user def anonymous_abilities(user, subject) - case true - when subject.is_a?(PersonalSnippet) + if subject.is_a?(PersonalSnippet) anonymous_personal_snippet_abilities(subject) - when subject.is_a?(ProjectSnippet) + elsif subject.is_a?(ProjectSnippet) anonymous_project_snippet_abilities(subject) - when subject.is_a?(CommitStatus) + elsif subject.is_a?(CommitStatus) anonymous_commit_status_abilities(subject) - when subject.is_a?(Project) || subject.respond_to?(:project) + elsif subject.is_a?(Project) || subject.respond_to?(:project) anonymous_project_abilities(subject) - when subject.is_a?(Group) || subject.respond_to?(:group) + elsif subject.is_a?(Group) || subject.respond_to?(:group) anonymous_group_abilities(subject) - when subject.is_a?(User) + elsif subject.is_a?(User) anonymous_user_abilities else [] diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 9a14954b4a7..42f908aa344 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -51,6 +51,10 @@ class ApplicationSetting < ActiveRecord::Base presence: true, numericality: { only_integer: true, greater_than: 0 } + validates :container_registry_token_expire_delay, + presence: true, + numericality: { only_integer: true, greater_than: 0 } + validates_each :restricted_visibility_levels do |record, attr, value| unless value.nil? value.each do |level| @@ -98,6 +102,10 @@ class ApplicationSetting < ActiveRecord::Base Rails.cache.delete(CACHE_KEY) end + def self.cached + Rails.cache.fetch(CACHE_KEY) + end + def self.create_from_defaults create( default_projects_limit: Settings.gitlab['default_projects_limit'], @@ -121,7 +129,8 @@ class ApplicationSetting < ActiveRecord::Base akismet_enabled: false, repository_checks_enabled: true, disabled_oauth_sign_in_sources: [], - send_user_confirmation_email: false + send_user_confirmation_email: false, + container_registry_token_expire_delay: 5, ) end diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ff7dd44c526..5e77fda70b9 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -286,7 +286,7 @@ module Ci project.runners_token end - def valid_token? token + def valid_token?(token) project.valid_runners_token? token end diff --git a/app/models/ci/commit.rb b/app/models/ci/commit.rb index 6675a3f5d53..f22b573a94c 100644 --- a/app/models/ci/commit.rb +++ b/app/models/ci/commit.rb @@ -66,6 +66,10 @@ module Ci end end + def cancelable? + builds.running_or_pending.any? + end + def cancel_running builds.running_or_pending.each(&:cancel) end diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 6829dc91cb9..adb65292208 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -60,7 +60,7 @@ module Ci end def display_name - return short_sha unless !description.blank? + return short_sha if description.blank? description end diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb index 10802f64813..f8d5d4486fd 100644 --- a/app/models/ci/variable.rb +++ b/app/models/ci/variable.rb @@ -11,6 +11,9 @@ module Ci format: { with: /\A[a-zA-Z0-9_]+\z/, message: "can contain only letters, digits and '_'." } - attr_encrypted :value, mode: :per_attribute_iv_and_salt, key: Gitlab::Application.secrets.db_key_base + attr_encrypted :value, + mode: :per_attribute_iv_and_salt, + key: Gitlab::Application.secrets.db_key_base, + algorithm: 'aes-256-cbc' end end diff --git a/app/models/commit.rb b/app/models/commit.rb index 562c3ed15b2..f96c7cb34d0 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -8,7 +8,10 @@ class Commit include StaticModel attr_mentionable :safe_message, pipeline: :single_line - participant :author, :committer, :notes + + participant :author + participant :committer + participant :notes_with_associations attr_accessor :project @@ -194,6 +197,10 @@ class Commit project.notes.for_commit_id(self.id) end + def notes_with_associations + notes.includes(:author, :project) + end + def method_missing(m, *args, &block) @raw.send(m, *args, &block) end @@ -219,7 +226,7 @@ class Commit def revert_branch_name "revert-#{short_id}" end - + def cherry_pick_branch_name project.repository.next_branch("cherry-pick-#{short_id}", mild: true) end @@ -251,11 +258,13 @@ class Commit end def has_been_reverted?(current_user = nil, noteable = self) - Gitlab::ReferenceExtractor.lazily do - noteable.notes.system.flat_map do |note| - note.all_references(current_user).commits - end - end.any? { |commit_ref| commit_ref.reverts_commit?(self) } + ext = all_references(current_user) + + noteable.notes_with_associations.system.each do |note| + note.all_references(current_user, extractor: ext) + end + + ext.commits.any? { |commit_ref| commit_ref.reverts_commit?(self) } end def change_type_title diff --git a/app/models/commit_range.rb b/app/models/commit_range.rb index 51673897d98..4066958f67c 100644 --- a/app/models/commit_range.rb +++ b/app/models/commit_range.rb @@ -62,7 +62,7 @@ class CommitRange def initialize(range_string, project) @project = project - range_string.strip! + range_string = range_string.strip unless range_string =~ /\A#{PATTERN}\z/ raise ArgumentError, "invalid CommitRange string format: #{range_string}" diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 91315b3459f..2326a395cb8 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -59,8 +59,12 @@ module Issuable prefix: true attr_mentionable :title, pipeline: :single_line - attr_mentionable :description, cache: true - participant :author, :assignee, :notes_with_associations + attr_mentionable :description + + participant :author + participant :assignee + participant :notes_with_associations + strip_attributes :title acts_as_paranoid @@ -126,13 +130,29 @@ module Issuable joins(join_clause).group(issuable_table[:id]).reorder("COUNT(notes.id) DESC") end - def with_label(title) + def with_label(title, sort = nil) if title.is_a?(Array) && title.size > 1 - joins(:labels).where(labels: { title: title }).group(arel_table[:id]).having("COUNT(DISTINCT labels.title) = #{title.size}") + joins(:labels).where(labels: { title: title }).group(*grouping_columns(sort)).having("COUNT(DISTINCT labels.title) = #{title.size}") else joins(:labels).where(labels: { title: title }) end end + + # Includes table keys in group by clause when sorting + # preventing errors in postgres + # + # Returns an array of arel columns + def grouping_columns(sort) + grouping_columns = [arel_table[:id]] + + if ["milestone_due_desc", "milestone_due_asc"].include?(sort) + milestone_table = Milestone.arel_table + grouping_columns << milestone_table[:id] + grouping_columns << milestone_table[:due_date] + end + + grouping_columns + end end def today? diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index b381d225485..f00b5b8497c 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -23,7 +23,7 @@ module Mentionable included do if self < Participable - participant ->(current_user) { mentioned_users(current_user) } + participant -> (user, ext) { all_references(user, extractor: ext) } end end @@ -43,23 +43,22 @@ module Mentionable self end - def all_references(current_user = nil, text = nil) - ext = Gitlab::ReferenceExtractor.new(self.project, current_user || self.author, self.author) + def all_references(current_user = nil, text = nil, extractor: nil) + extractor ||= Gitlab::ReferenceExtractor. + new(project, current_user || author) if text - ext.analyze(text) + extractor.analyze(text, author: author) else self.class.mentionable_attrs.each do |attr, options| - text = send(attr) + text = __send__(attr) + options = options.merge(cache_key: [self, attr], author: author) - context = options.dup - context[:cache_key] = [self, attr] if context.delete(:cache) && self.persisted? - - ext.analyze(text, context) + extractor.analyze(text, options) end end - ext + extractor end def mentioned_users(current_user = nil) diff --git a/app/models/concerns/participable.rb b/app/models/concerns/participable.rb index fc6f83b918b..9056722f45e 100644 --- a/app/models/concerns/participable.rb +++ b/app/models/concerns/participable.rb @@ -3,8 +3,6 @@ # Contains functionality related to objects that can have participants, such as # an author, an assignee and people mentioned in its description or comments. # -# Used by Issue, Note, MergeRequest, Snippet and Commit. -# # Usage: # # class Issue < ActiveRecord::Base @@ -12,22 +10,36 @@ # # # ... # -# participant :author, :assignee, :notes, ->(current_user) { mentioned_users(current_user) } +# participant :author +# participant :assignee +# participant :notes +# +# participant -> (current_user, ext) do +# ext.analyze('...') +# end # end # # issue = Issue.last # users = issue.participants -# # `users` will contain the issue's author, its assignee, -# # all users returned by its #mentioned_users method, -# # as well as all participants to all of the issue's notes, -# # since Note implements Participable as well. -# module Participable extend ActiveSupport::Concern module ClassMethods - def participant(*attrs) - participant_attrs.concat(attrs) + # Adds a list of participant attributes. Attributes can either be symbols or + # Procs. + # + # When using a Proc instead of a Symbol the Proc will be given two + # arguments: + # + # 1. The current user (as an instance of User) + # 2. An instance of `Gitlab::ReferenceExtractor` + # + # It is expected that a Proc populates the given reference extractor + # instance with data. The return value of the Proc is ignored. + # + # attr - The name of the attribute or a Proc + def participant(attr) + participant_attrs << attr end def participant_attrs @@ -35,42 +47,42 @@ module Participable end end - # Be aware that this method makes a lot of sql queries. - # Save result into variable if you are going to reuse it inside same request - def participants(current_user = self.author) - participants = - Gitlab::ReferenceExtractor.lazily do - self.class.participant_attrs.flat_map do |attr| - value = - if attr.respond_to?(:call) - instance_exec(current_user, &attr) - else - send(attr) - end + # Returns the users participating in a discussion. + # + # This method processes attributes of objects in breadth-first order. + # + # Returns an Array of User instances. + def participants(current_user = nil) + current_user ||= author + ext = Gitlab::ReferenceExtractor.new(project, current_user) + participants = Set.new + process = [self] - participants_for(value, current_user) - end.compact.uniq - end + until process.empty? + source = process.pop - unless Gitlab::ReferenceExtractor.lazy? - participants.select! do |user| - user.can?(:read_project, project) + case source + when User + participants << source + when Participable + source.class.participant_attrs.each do |attr| + if attr.respond_to?(:call) + source.instance_exec(current_user, ext, &attr) + else + process << source.__send__(attr) + end + end + when Enumerable, ActiveRecord::Relation + # This uses reverse_each so we can use "pop" to get the next value to + # process (in order). Using unshift instead of pop would require + # moving all Array values one index to the left (which can be + # expensive). + source.reverse_each { |obj| process << obj } end end - participants - end - - private + participants.merge(ext.users) - def participants_for(value, current_user = nil) - case value - when User, Banzai::LazyReference - [value] - when Enumerable, ActiveRecord::Relation - value.flat_map { |v| participants_for(v, current_user) } - when Participable - value.participants(current_user) - end + Ability.users_that_can_read_project(participants.to_a, project) end end diff --git a/app/models/issue.rb b/app/models/issue.rb index 2d4a9b9f19a..bd0fbc96d18 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -95,14 +95,13 @@ class Issue < ActiveRecord::Base end def referenced_merge_requests(current_user = nil) - @referenced_merge_requests ||= {} - @referenced_merge_requests[current_user] ||= begin - Gitlab::ReferenceExtractor.lazily do - [self, *notes].flat_map do |note| - note.all_references(current_user).merge_requests - end - end.sort_by(&:iid).uniq + ext = all_references(current_user) + + notes_with_associations.each do |object| + object.all_references(current_user, extractor: ext) end + + ext.merge_requests.sort_by(&:iid) end # All branches containing the current issue's ID, except for @@ -139,9 +138,13 @@ class Issue < ActiveRecord::Base def closed_by_merge_requests(current_user = nil) return [] unless open? - notes.system.flat_map do |note| - note.all_references(current_user).merge_requests - end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) } + ext = all_references(current_user) + + notes.system.each do |note| + note.all_references(current_user, extractor: ext) + end + + ext.merge_requests.select { |mr| mr.open? && mr.closes_issue?(self) } end def moved? diff --git a/app/models/key.rb b/app/models/key.rb index d52afda67d1..0532e84f47d 100644 --- a/app/models/key.rb +++ b/app/models/key.rb @@ -26,7 +26,7 @@ class Key < ActiveRecord::Base end def publishable_key - #Removes anything beyond the keytype and key itself + # Removes anything beyond the keytype and key itself self.key.split[0..1].join(' ') end diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb index 8dae3bb2ef2..46955b430f3 100644 --- a/app/models/members/project_member.rb +++ b/app/models/members/project_member.rb @@ -5,7 +5,6 @@ class ProjectMember < Member belongs_to :project, class_name: 'Project', foreign_key: 'source_id' - # Make sure project member points only to project as it source default_value_for :source_type, SOURCE_TYPE validates_format_of :source_type, with: /\AProject\z/ @@ -15,6 +14,8 @@ class ProjectMember < Member scope :in_projects, ->(projects) { where(source_id: projects.pluck(:id)) } scope :with_user, ->(user) { where(user_id: user.id) } + before_destroy :delete_member_todos + class << self # Add users to project teams with passed access option @@ -102,6 +103,10 @@ class ProjectMember < Member private + def delete_member_todos + user.todos.where(project_id: source_id).destroy_all if user + end + def send_invite notification_service.invite_project_member(self, @raw_invite_token) diff --git a/app/models/network/graph.rb b/app/models/network/graph.rb index 9259cb1a0fa..a2aee2f925b 100644 --- a/app/models/network/graph.rb +++ b/app/models/network/graph.rb @@ -22,9 +22,16 @@ module Network def collect_notes h = Hash.new(0) - @project.notes.where('noteable_type = ?' ,"Commit").group('notes.commit_id').select('notes.commit_id, count(notes.id) as note_count').each do |item| - h[item.commit_id] = item.note_count.to_i - end + + @project + .notes + .where('noteable_type = ?', 'Commit') + .group('notes.commit_id') + .select('notes.commit_id, count(notes.id) as note_count') + .each do |item| + h[item.commit_id] = item.note_count.to_i + end + h end @@ -89,7 +96,7 @@ module Network end end - if self.class.max_count / 2 < offset then + if self.class.max_count / 2 < offset # get max index that commit is displayed in the center. offset - self.class.max_count / 2 else @@ -130,7 +137,7 @@ module Network commit.parents(@map).each do |parent| range = commit.time..parent.time - space = if commit.space >= parent.space then + space = if commit.space >= parent.space find_free_parent_space(range, parent.space, -1, commit.space) else find_free_parent_space(range, commit.space, -1, parent.space) @@ -144,7 +151,7 @@ module Network end def find_free_parent_space(range, space_base, space_step, space_default) - if is_overlap?(range, space_default) then + if is_overlap?(range, space_default) find_free_space(range, space_step, space_base, space_default) else space_default @@ -155,9 +162,9 @@ module Network range.each do |i| if i != range.first && i != range.last && - @commits[i].spaces.include?(overlap_space) then + @commits[i].spaces.include?(overlap_space) - return true; + return true end end @@ -198,7 +205,7 @@ module Network # Visit branching chains leaves.each do |l| parents = l.parents(@map).select{|p| p.space.zero?} - for p in parents + parents.each do |p| place_chain(p, l.time) end end @@ -216,7 +223,7 @@ module Network end def mark_reserved(time_range, space) - for day in time_range + time_range.each do |day| @reserved[day].push(space) end end @@ -225,15 +232,15 @@ module Network space_default ||= space_base reserved = [] - for day in time_range + time_range.each do |day| reserved.push(*@reserved[day]) end reserved.uniq! space = space_default - while reserved.include?(space) do + while reserved.include?(space) space += space_step - if space < space_base then + if space < space_base space_step *= -1 space = space_base + space_step end diff --git a/app/models/note.rb b/app/models/note.rb index 55b98557244..c21981ead84 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -6,7 +6,7 @@ class Note < ActiveRecord::Base default_value_for :system, false - attr_mentionable :note, cache: true, pipeline: :note + attr_mentionable :note, pipeline: :note participant :author belongs_to :project @@ -29,10 +29,17 @@ class Note < ActiveRecord::Base # Attachments are deprecated and are handled by Markdown uploader validates :attachment, file_size: { maximum: :max_attachment_size } - validates :noteable_id, presence: true, if: ->(n) { n.noteable_type.present? && n.noteable_type != 'Commit' } - validates :commit_id, presence: true, if: ->(n) { n.noteable_type == 'Commit' } + validates :noteable_type, presence: true + validates :noteable_id, presence: true, unless: :for_commit? + validates :commit_id, presence: true, if: :for_commit? validates :author, presence: true + validate unless: :for_commit? do |note| + unless note.noteable.try(:project) == note.project + errors.add(:invalid_project, 'Note and noteable project mismatch') + end + end + mount_uploader :attachment, AttachmentUploader # Scopes @@ -77,14 +84,30 @@ class Note < ActiveRecord::Base # # This method uses ILIKE on PostgreSQL and LIKE on MySQL. # - # query - The search query as a String. + # query - The search query as a String. + # as_user - Limit results to those viewable by a specific user # # Returns an ActiveRecord::Relation. - def search(query) + def search(query, as_user: nil) table = arel_table pattern = "%#{query}%" - where(table[:note].matches(pattern)) + found_notes = joins('LEFT JOIN issues ON issues.id = noteable_id'). + where(table[:note].matches(pattern)) + + if as_user + found_notes.where(' + issues.confidential IS NULL + OR issues.confidential IS FALSE + OR (issues.confidential IS TRUE + AND (issues.author_id = :user_id + OR issues.assignee_id = :user_id + OR issues.project_id IN(:project_ids)))', + user_id: as_user.id, + project_ids: as_user.authorized_projects.select(:id)) + else + found_notes.where('issues.confidential IS NULL OR issues.confidential IS FALSE') + end end def grouped_awards diff --git a/app/models/project.rb b/app/models/project.rb index f3a56b86d5a..525a82c7534 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -309,21 +309,25 @@ class Project < ActiveRecord::Base @repository ||= Repository.new(path_with_namespace, self) end + def container_registry_path_with_namespace + path_with_namespace.downcase + end + def container_registry_repository return unless Gitlab.config.registry.enabled @container_registry_repository ||= begin - token = Auth::ContainerRegistryAuthenticationService.full_access_token(path_with_namespace) + token = Auth::ContainerRegistryAuthenticationService.full_access_token(container_registry_path_with_namespace) url = Gitlab.config.registry.api_url host_port = Gitlab.config.registry.host_port registry = ContainerRegistry::Registry.new(url, token: token, path: host_port) - registry.repository(path_with_namespace) + registry.repository(container_registry_path_with_namespace) end end def container_registry_repository_url if Gitlab.config.registry.enabled - "#{Gitlab.config.registry.host_port}/#{path_with_namespace}" + "#{Gitlab.config.registry.host_port}/#{container_registry_path_with_namespace}" end end @@ -946,13 +950,13 @@ class Project < ActiveRecord::Base shared_runners_enabled? && Ci::Runner.shared.active.any?(&block) end - def valid_runners_token? token + def valid_runners_token?(token) self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token) end # TODO (ayufan): For now we use runners_token (backward compatibility) # In 8.4 every build will have its own individual token valid for time of build - def valid_build_token? token + def valid_build_token?(token) self.builds_enabled? && self.runners_token && ActiveSupport::SecurityUtils.variable_size_secure_compare(token, self.runners_token) end diff --git a/app/models/project_import_data.rb b/app/models/project_import_data.rb index e2f9ffb69ac..ca8a9b4217b 100644 --- a/app/models/project_import_data.rb +++ b/app/models/project_import_data.rb @@ -6,7 +6,8 @@ class ProjectImportData < ActiveRecord::Base key: Gitlab::Application.secrets.db_key_base, marshal: true, encode: true, - mode: :per_attribute_iv_and_salt + mode: :per_attribute_iv_and_salt, + algorithm: 'aes-256-cbc' serialize :data, JSON diff --git a/app/models/project_services/irker_service.rb b/app/models/project_services/irker_service.rb index 91015e6c9b1..2e5e854fc5e 100644 --- a/app/models/project_services/irker_service.rb +++ b/app/models/project_services/irker_service.rb @@ -70,7 +70,7 @@ class IrkerService < Service private def get_channels - return true unless :activated? + return true unless activated? return true if recipients.nil? || recipients.empty? map_recipients diff --git a/app/models/project_snippet.rb b/app/models/project_snippet.rb index 5fba6baa204..25b5d777641 100644 --- a/app/models/project_snippet.rb +++ b/app/models/project_snippet.rb @@ -7,5 +7,6 @@ class ProjectSnippet < Snippet # Scopes scope :fresh, -> { order("created_at DESC") } - participant :author, :notes + participant :author + participant :notes_with_associations end diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index 339fb0b9f9d..25d82929c0b 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -27,6 +27,10 @@ class ProjectWiki @project.path_with_namespace + ".wiki" end + def web_url + Gitlab::Routing.url_helpers.namespace_project_wiki_url(@project.namespace, @project, :home) + end + def url_to_repo gitlab_shell.url_to_repo(path_with_namespace) end @@ -142,6 +146,16 @@ class ProjectWiki wiki end + def hook_attrs + { + web_url: web_url, + git_ssh_url: ssh_url_to_repo, + git_http_url: http_url_to_repo, + path_with_namespace: path_with_namespace, + default_branch: default_branch + } + end + private def init_repo(path_with_namespace) diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 0a3c3b57669..407697b745c 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -30,7 +30,8 @@ class Snippet < ActiveRecord::Base scope :public_and_internal, -> { where(visibility_level: [Snippet::PUBLIC, Snippet::INTERNAL]) } scope :fresh, -> { order("created_at DESC") } - participant :author, :notes + participant :author + participant :notes_with_associations def self.reference_prefix '$' @@ -100,6 +101,10 @@ class Snippet < ActiveRecord::Base content.lines.count > 1000 end + def notes_with_associations + notes.includes(:author, :project) + end + class << self # Searches for snippets with a matching title or file name. # diff --git a/app/models/user.rb b/app/models/user.rb index 6a09b78455b..15b6cbc2255 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -20,6 +20,11 @@ class User < ActiveRecord::Base default_value_for :hide_no_password, false default_value_for :theme_id, gitlab_config.default_theme + attr_encrypted :otp_secret, + key: Gitlab::Application.config.secret_key_base, + mode: :per_attribute_iv_and_salt, + algorithm: 'aes-256-cbc' + devise :two_factor_authenticatable, otp_secret_encryption_key: Gitlab::Application.config.secret_key_base alias_attribute :two_factor_enabled, :otp_required_for_login @@ -27,7 +32,7 @@ class User < ActiveRecord::Base devise :two_factor_backupable, otp_number_of_backup_codes: 10 serialize :otp_backup_codes, JSON - devise :lockable, :async, :recoverable, :rememberable, :trackable, + devise :lockable, :recoverable, :rememberable, :trackable, :validatable, :omniauthable, :confirmable, :registerable attr_accessor :force_random_password diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 2bbab643e69..e57b95f21ec 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -1,13 +1,13 @@ module Auth class ContainerRegistryAuthenticationService < BaseService + include Gitlab::CurrentSettings + AUDIENCE = 'container_registry' def execute return error('not found', 404) unless registry.enabled - if params[:offline_token] - return error('unauthorized', 401) unless current_user || project - else + unless current_user || project return error('forbidden', 403) unless scope end @@ -19,6 +19,7 @@ module Auth token = JSONWebToken::RSAToken.new(registry.key) token.issuer = registry.issuer token.audience = AUDIENCE + token.expire_time = token_expire_at token[:access] = names.map do |name| { type: 'repository', name: name, actions: %w(*) } end @@ -32,6 +33,7 @@ module Auth token.issuer = registry.issuer token.audience = params[:service] token.subject = current_user.try(:username) + token.expire_time = ContainerRegistryAuthenticationService.token_expire_at token[:access] = accesses.compact token end @@ -77,5 +79,9 @@ module Auth def registry Gitlab.config.registry end + + def self.token_expire_at + Time.now + current_application_settings.container_registry_token_expire_delay.minutes + end end end diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 7410442609d..299a0a967b0 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -23,7 +23,7 @@ class GitTagPushService < BaseService commits = [] message = nil - if !Gitlab::Git.blank_ref?(params[:newrev]) + unless Gitlab::Git.blank_ref?(params[:newrev]) tag_name = Gitlab::Git.ref_name(params[:ref]) tag = project.repository.find_tag(tag_name) if tag && tag.target == params[:newrev] diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 01586994813..2bb312bb252 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -5,8 +5,6 @@ module Notes note.author = current_user note.system = false - return unless valid_project?(note) - if note.save # Finish the harder work in the background NewNoteWorker.perform_in(2.seconds, note.id, params) @@ -15,14 +13,5 @@ module Notes note end - - private - - def valid_project?(note) - return false unless project - return true if note.for_commit? - - note.noteable.try(:project) == project - end end end diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb index 3b7c36f0908..43db29315a1 100644 --- a/app/services/projects/housekeeping_service.rb +++ b/app/services/projects/housekeeping_service.rb @@ -22,7 +22,7 @@ module Projects end def execute - raise LeaseTaken if !try_obtain_lease + raise LeaseTaken unless try_obtain_lease GitlabShellOneShotWorker.perform_async(:gc, @project.path_with_namespace) ensure diff --git a/app/services/wiki_pages/base_service.rb b/app/services/wiki_pages/base_service.rb index 9162f128602..4c0a2c6b4d8 100644 --- a/app/services/wiki_pages/base_service.rb +++ b/app/services/wiki_pages/base_service.rb @@ -6,9 +6,8 @@ module WikiPages object_kind: page.class.name.underscore, user: current_user.hook_attrs, project: @project.hook_attrs, - object_attributes: page.hook_attrs, - # DEPRECATED - repository: @project.hook_attrs.slice(:name, :url, :description, :homepage) + wiki: @project.wiki.hook_attrs, + object_attributes: page.hook_attrs } page_url = Gitlab::UrlBuilder.build(page) diff --git a/app/views/admin/abuse_reports/_abuse_report.html.haml b/app/views/admin/abuse_reports/_abuse_report.html.haml index 2ab01704b77..862b86d9d4a 100644 --- a/app/views/admin/abuse_reports/_abuse_report.html.haml +++ b/app/views/admin/abuse_reports/_abuse_report.html.haml @@ -16,7 +16,7 @@ .light.small = time_ago_with_tooltip(abuse_report.created_at) %td - = markdown(abuse_report.message.squish!, pipeline: :single_line) + = markdown(abuse_report.message.squish!, pipeline: :single_line, author: reporter) %td - if user = link_to 'Remove user & report', admin_abuse_report_path(abuse_report, remove_user: true), diff --git a/app/views/admin/application_settings/_form.html.haml b/app/views/admin/application_settings/_form.html.haml index df286852b97..f149f9eb431 100644 --- a/app/views/admin/application_settings/_form.html.haml +++ b/app/views/admin/application_settings/_form.html.haml @@ -178,6 +178,14 @@ .col-sm-10 = f.number_field :max_artifacts_size, class: 'form-control' + - if Gitlab.config.registry.enabled + %fieldset + %legend Container Registry + .form-group + = f.label :container_registry_token_expire_delay, 'Authorization token duration (minutes)', class: 'control-label col-sm-2' + .col-sm-10 + = f.number_field :container_registry_token_expire_delay, class: 'form-control' + %fieldset %legend Metrics %p diff --git a/app/views/devise/sessions/two_factor.html.haml b/app/views/devise/sessions/two_factor.html.haml index c9d1e454a5e..8c6a1552a53 100644 --- a/app/views/devise/sessions/two_factor.html.haml +++ b/app/views/devise/sessions/two_factor.html.haml @@ -4,6 +4,7 @@ %h3 Two-factor Authentication .login-body = form_for(resource, as: resource_name, url: session_path(resource_name), method: :post) do |f| + = f.hidden_field :remember_me, value: params[resource_name][:remember_me] = f.text_field :otp_attempt, class: 'form-control', placeholder: 'Two-factor Authentication code', required: true, autofocus: true %p.help-block.hint Enter the code from the two-factor app on your mobile device. If you've lost your device, you may enter one of your recovery codes. .prepend-top-20 diff --git a/app/views/events/_commit.html.haml b/app/views/events/_commit.html.haml index dce4081288c..1bc9f604438 100644 --- a/app/views/events/_commit.html.haml +++ b/app/views/events/_commit.html.haml @@ -2,4 +2,4 @@ .commit-row-title = link_to truncate_sha(commit[:id]), namespace_project_commit_path(project.namespace, project, commit[:id]), class: "commit_short_id", alt: '', title: truncate_sha(commit[:id]) · - = markdown event_commit_title(commit[:message]), project: project, pipeline: :single_line + = markdown event_commit_title(commit[:message]), project: project, pipeline: :single_line, author: event.author diff --git a/app/views/events/_event_issue.atom.haml b/app/views/events/_event_issue.atom.haml index fad65310021..083c3936212 100644 --- a/app/views/events/_event_issue.atom.haml +++ b/app/views/events/_event_issue.atom.haml @@ -1,2 +1,2 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - = markdown(issue.description, pipeline: :atom, project: issue.project) + = markdown(issue.description, pipeline: :atom, project: issue.project, author: issue.author) diff --git a/app/views/events/_event_merge_request.atom.haml b/app/views/events/_event_merge_request.atom.haml index 19bdc7b9ca5..d7e05600627 100644 --- a/app/views/events/_event_merge_request.atom.haml +++ b/app/views/events/_event_merge_request.atom.haml @@ -1,2 +1,2 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - = markdown(merge_request.description, pipeline: :atom, project: merge_request.project) + = markdown(merge_request.description, pipeline: :atom, project: merge_request.project, author: merge_request.author) diff --git a/app/views/events/_event_note.atom.haml b/app/views/events/_event_note.atom.haml index b730ebbd5f9..1154f982821 100644 --- a/app/views/events/_event_note.atom.haml +++ b/app/views/events/_event_note.atom.haml @@ -1,2 +1,2 @@ %div{xmlns: "http://www.w3.org/1999/xhtml"} - = markdown(note.note, pipeline: :atom, project: note.project) + = markdown(note.note, pipeline: :atom, project: note.project, author: note.author) diff --git a/app/views/events/_event_push.atom.haml b/app/views/events/_event_push.atom.haml index b271b9daff1..28bee1d0a33 100644 --- a/app/views/events/_event_push.atom.haml +++ b/app/views/events/_event_push.atom.haml @@ -6,7 +6,7 @@ %i at = commit[:timestamp].to_time.to_s(:short) - %blockquote= markdown(escape_once(commit[:message]), pipeline: :atom, project: event.project) + %blockquote= markdown(escape_once(commit[:message]), pipeline: :atom, project: event.project, author: event.author) - if event.commits_count > 15 %p %i diff --git a/app/views/events/event/_common.html.haml b/app/views/events/event/_common.html.haml index f9f623cc031..c7f29f2fc0e 100644 --- a/app/views/events/event/_common.html.haml +++ b/app/views/events/event/_common.html.haml @@ -4,7 +4,7 @@ = event_action_name(event) - if event.target - %strong= link_to event.target.reference_link_text, [event.project.namespace.becomes(Namespace), event.project, event.target], title: event.target_title + %strong= link_to event.target.reference_link_text, [event.project.namespace.becomes(Namespace), event.project, event.target], class: 'has-tooltip', title: event.target_title = event_preposition(event) diff --git a/app/views/events/event/_push.html.haml b/app/views/events/event/_push.html.haml index 235bd46107e..dc4ff17e31a 100644 --- a/app/views/events/event/_push.html.haml +++ b/app/views/events/event/_push.html.haml @@ -15,7 +15,7 @@ %ul.well-list.event_commits - few_commits = event.commits[0...2] - few_commits.each do |commit| - = render "events/commit", commit: commit, project: project + = render "events/commit", commit: commit, project: project, event: event - create_mr = event.new_ref? && create_mr_button?(event.project.default_branch, event.ref_name, event.project) - if event.commits_count > 1 diff --git a/app/views/kaminari/gitlab/_first_page.html.haml b/app/views/kaminari/gitlab/_first_page.html.haml index ada7306d98d..e7a70e3bb28 100644 --- a/app/views/kaminari/gitlab/_first_page.html.haml +++ b/app/views/kaminari/gitlab/_first_page.html.haml @@ -2,7 +2,7 @@ -# available local variables -# url: url to the first page -# current_page: a page object for the currently displayed page --# num_pages: total number of pages +-# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote %li.first diff --git a/app/views/kaminari/gitlab/_gap.html.haml b/app/views/kaminari/gitlab/_gap.html.haml index 3ffd12f8587..80ca30f36e6 100644 --- a/app/views/kaminari/gitlab/_gap.html.haml +++ b/app/views/kaminari/gitlab/_gap.html.haml @@ -1,7 +1,7 @@ -# Non-link tag that stands for skipped pages... -# available local variables -# current_page: a page object for the currently displayed page --# num_pages: total number of pages +-# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote %li{class: "page"} diff --git a/app/views/kaminari/gitlab/_last_page.html.haml b/app/views/kaminari/gitlab/_last_page.html.haml index 3431d029bcc..53f780d1d1b 100644 --- a/app/views/kaminari/gitlab/_last_page.html.haml +++ b/app/views/kaminari/gitlab/_last_page.html.haml @@ -2,7 +2,7 @@ -# available local variables -# url: url to the last page -# current_page: a page object for the currently displayed page --# num_pages: total number of pages +-# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote %li.last diff --git a/app/views/kaminari/gitlab/_next_page.html.haml b/app/views/kaminari/gitlab/_next_page.html.haml index c805914fc3f..125f09777ba 100644 --- a/app/views/kaminari/gitlab/_next_page.html.haml +++ b/app/views/kaminari/gitlab/_next_page.html.haml @@ -2,7 +2,7 @@ -# available local variables -# url: url to the next page -# current_page: a page object for the currently displayed page --# num_pages: total number of pages +-# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote - if current_page.last? diff --git a/app/views/kaminari/gitlab/_page.html.haml b/app/views/kaminari/gitlab/_page.html.haml index a52d883b9a8..522e4d1d05f 100644 --- a/app/views/kaminari/gitlab/_page.html.haml +++ b/app/views/kaminari/gitlab/_page.html.haml @@ -3,7 +3,7 @@ -# page: a page object for "this" page -# url: url to this page -# current_page: a page object for the currently displayed page --# num_pages: total number of pages +-# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote %li{class: "page#{' active' if page.current?}"} diff --git a/app/views/kaminari/gitlab/_paginator.html.haml b/app/views/kaminari/gitlab/_paginator.html.haml index a12c53bcfe7..f5e0d2ed3f3 100644 --- a/app/views/kaminari/gitlab/_paginator.html.haml +++ b/app/views/kaminari/gitlab/_paginator.html.haml @@ -1,7 +1,7 @@ -# The container tag -# available local variables -# current_page: a page object for the currently displayed page --# num_pages: total number of pages +-# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote -# paginator: the paginator that renders the pagination tags inside @@ -9,7 +9,7 @@ %div.gl-pagination %ul.pagination.clearfix - unless current_page.first? - = first_page_tag unless num_pages < 5 # As kaminari will always show the first 5 pages + = first_page_tag unless total_pages < 5 # As kaminari will always show the first 5 pages = prev_page_tag - each_page do |page| - if page.left_outer? || page.right_outer? || page.inside_window? @@ -18,5 +18,5 @@ = gap_tag = next_page_tag - unless current_page.last? - = last_page_tag unless num_pages < 5 + = last_page_tag unless total_pages < 5 diff --git a/app/views/kaminari/gitlab/_prev_page.html.haml b/app/views/kaminari/gitlab/_prev_page.html.haml index afb20455e0a..7edf10498a8 100644 --- a/app/views/kaminari/gitlab/_prev_page.html.haml +++ b/app/views/kaminari/gitlab/_prev_page.html.haml @@ -2,7 +2,7 @@ -# available local variables -# url: url to the previous page -# current_page: a page object for the currently displayed page --# num_pages: total number of pages +-# total_pages: total number of pages -# per_page: number of items to fetch per page -# remote: data-remote - if current_page.first? diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml index 087b7472701..2c9b9006668 100644 --- a/app/views/layouts/nav/_project.html.haml +++ b/app/views/layouts/nav/_project.html.haml @@ -52,15 +52,6 @@ = icon('ship fw') %span Pipelines - %span.badge.count.ci_counter= number_with_delimiter(@project.ci_commits.running_or_pending.count) - - - if project_nav_tab? :builds - = nav_link(controller: %w(builds)) do - = link_to project_builds_path(@project), title: 'Builds', class: 'shortcuts-builds' do - = icon('cubes fw') - %span - Builds - %span.badge.count.builds_counter= number_with_delimiter(@project.builds.running_or_pending.count(:all)) - if project_nav_tab? :container_registry = nav_link(controller: %w(container_registry)) do @@ -132,4 +123,10 @@ = link_to new_namespace_project_issue_path(@project.namespace, @project), class: 'shortcuts-new-issue' do Create a new issue + -# Shortcut to builds page + - if project_nav_tab? :builds + %li.hidden + = link_to project_builds_path(@project), title: 'Builds', class: 'shortcuts-builds' do + Builds + .fade-right diff --git a/app/views/notify/_note_message.html.haml b/app/views/notify/_note_message.html.haml index 12ded41fbf2..e9c66170877 100644 --- a/app/views/notify/_note_message.html.haml +++ b/app/views/notify/_note_message.html.haml @@ -2,4 +2,4 @@ %div #{link_to @note.author_name, user_url(@note.author)} wrote: %div - = markdown(@note.note, pipeline: :email) + = markdown(@note.note, pipeline: :email, author: @note.author) diff --git a/app/views/notify/new_issue_email.html.haml b/app/views/notify/new_issue_email.html.haml index ad3ab2525bb..f42b150c0d6 100644 --- a/app/views/notify/new_issue_email.html.haml +++ b/app/views/notify/new_issue_email.html.haml @@ -2,7 +2,7 @@ %div #{link_to @issue.author_name, user_url(@issue.author)} wrote: -if @issue.description - = markdown(@issue.description, pipeline: :email) + = markdown(@issue.description, pipeline: :email, author: @issue.author) - if @issue.assignee_id.present? %p diff --git a/app/views/notify/new_merge_request_email.html.haml b/app/views/notify/new_merge_request_email.html.haml index 23423e7d981..158404de396 100644 --- a/app/views/notify/new_merge_request_email.html.haml +++ b/app/views/notify/new_merge_request_email.html.haml @@ -9,4 +9,4 @@ Assignee: #{@merge_request.author_name} → #{@merge_request.assignee_name} -if @merge_request.description - = markdown(@merge_request.description, pipeline: :email) + = markdown(@merge_request.description, pipeline: :email, author: @merge_request.author) diff --git a/app/views/projects/builds/index.html.haml b/app/views/projects/builds/index.html.haml index 8fb9ebc1b8b..818d5d28f04 100644 --- a/app/views/projects/builds/index.html.haml +++ b/app/views/projects/builds/index.html.haml @@ -1,4 +1,5 @@ - page_title "Builds" += render "projects/pipelines/head" .top-area %ul.nav-links diff --git a/app/views/projects/ci/commits/_commit.html.haml b/app/views/projects/ci/commits/_commit.html.haml index 5b6b940a0c4..5e3a4123a8e 100644 --- a/app/views/projects/ci/commits/_commit.html.haml +++ b/app/views/projects/ci/commits/_commit.html.haml @@ -63,9 +63,9 @@ %span #{build.name} - if can?(current_user, :update_pipeline, @project) - - if commit.retryable? && commit.builds.failed.any? + - if commit.retryable? = link_to retry_namespace_project_pipeline_path(@project.namespace, @project, commit.id), class: 'btn has-tooltip', title: "Retry", method: :post do = icon("repeat") - - if commit.active? + - if commit.cancelable? = link_to cancel_namespace_project_pipeline_path(@project.namespace, @project, commit.id), class: 'btn btn-remove has-tooltip', title: "Cancel", method: :post do = icon("remove") diff --git a/app/views/projects/commit/_ci_commit.html.haml b/app/views/projects/commit/_ci_commit.html.haml index ce5c550b441..32ff4d30977 100644 --- a/app/views/projects/commit/_ci_commit.html.haml +++ b/app/views/projects/commit/_ci_commit.html.haml @@ -1,24 +1,24 @@ .row-content-block.build-content.middle-block .pull-right - - if can?(current_user, :update_pipeline, @project) + - if can?(current_user, :update_pipeline, ci_commit.project) - if ci_commit.builds.latest.failed.any?(&:retryable?) - = link_to "Retry failed", retry_namespace_project_pipeline_path(@project.namespace, @project, ci_commit.id), class: 'btn btn-grouped btn-primary', method: :post + = link_to "Retry failed", retry_namespace_project_pipeline_path(ci_commit.project.namespace, ci_commit.project, ci_commit.id), class: 'btn btn-grouped btn-primary', method: :post - if ci_commit.builds.running_or_pending.any? - = link_to "Cancel running", cancel_namespace_project_pipeline_path(@project.namespace, @project, ci_commit.id), data: { confirm: 'Are you sure?' }, class: 'btn btn-grouped btn-danger', method: :post + = link_to "Cancel running", cancel_namespace_project_pipeline_path(ci_commit.project.namespace, ci_commit.project, ci_commit.id), data: { confirm: 'Are you sure?' }, class: 'btn btn-grouped btn-danger', method: :post .oneline.clearfix - if defined?(pipeline_details) && pipeline_details Pipeline - = link_to "##{ci_commit.id}", namespace_project_pipeline_path(@project.namespace, @project, ci_commit.id), class: "monospace" + = link_to "##{ci_commit.id}", namespace_project_pipeline_path(ci_commit.project.namespace, ci_commit.project, ci_commit.id), class: "monospace" with = pluralize ci_commit.statuses.count(:id), "build" - if ci_commit.ref for - = link_to ci_commit.ref, namespace_project_commits_path(@project.namespace, @project, ci_commit.ref), class: "monospace" + = link_to ci_commit.ref, namespace_project_commits_path(ci_commit.project.namespace, ci_commit.project, ci_commit.ref), class: "monospace" - if defined?(link_to_commit) && link_to_commit for commit - = link_to ci_commit.short_sha, namespace_project_commit_path(@project.namespace, @project, ci_commit.sha), class: "monospace" + = link_to ci_commit.short_sha, namespace_project_commit_path(ci_commit.project.namespace, ci_commit.project, ci_commit.sha), class: "monospace" - if ci_commit.duration in = time_interval_in_words ci_commit.duration @@ -31,7 +31,7 @@ %li= error You can also test your .gitlab-ci.yml in the #{link_to "Lint", ci_lint_path} -- if @project.builds_enabled? && !ci_commit.ci_yaml_file +- if ci_commit.project.builds_enabled? && !ci_commit.ci_yaml_file .bs-callout.bs-callout-warning \.gitlab-ci.yml not found in this commit @@ -45,7 +45,7 @@ %th Tags %th Duration %th Finished at - - if @project.build_coverage_enabled? + - if ci_commit.project.build_coverage_enabled? %th Coverage %th - ci_commit.statuses.stages.each do |stage| diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 429bf041248..6674d58417b 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -63,10 +63,10 @@ .commit-box.content-block %h3.commit-title - = markdown escape_once(@commit.title), pipeline: :single_line + = markdown escape_once(@commit.title), pipeline: :single_line, author: @commit.author - if @commit.description.present? %pre.commit-description - = preserve(markdown(escape_once(@commit.description), pipeline: :single_line)) + = preserve(markdown(escape_once(@commit.description), pipeline: :single_line, author: @commit.author)) :javascript $(".commit-info.branches").load("#{branches_namespace_project_commit_path(@project.namespace, @project, @commit.id)}"); diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 655cb0ac3cb..367027182b6 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -24,7 +24,7 @@ - if commit.description? .commit-row-description.js-toggle-content %pre - = preserve(markdown(escape_once(commit.description), pipeline: :single_line)) + = preserve(markdown(escape_once(commit.description), pipeline: :single_line, author: commit.author)) .commit-row-info by diff --git a/app/views/projects/hooks/_project_hook.html.haml b/app/views/projects/hooks/_project_hook.html.haml index 62eba5888a4..8151187d499 100644 --- a/app/views/projects/hooks/_project_hook.html.haml +++ b/app/views/projects/hooks/_project_hook.html.haml @@ -3,7 +3,7 @@ .col-md-8.col-lg-7 %strong.light-header= hook.url %div - - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger| + - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events wiki_page_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray.deploy-project-label= trigger.titleize .col-md-4.col-lg-5.text-right-lg.prepend-top-5 diff --git a/app/views/projects/hooks/index.html.haml b/app/views/projects/hooks/index.html.haml index cffe9a01a96..917a0b805b1 100644 --- a/app/views/projects/hooks/index.html.haml +++ b/app/views/projects/hooks/index.html.haml @@ -64,6 +64,13 @@ Build events %p.light This url will be triggered when the build status changes + %div + = f.check_box :wiki_page_events, class: 'pull-left' + .prepend-left-20 + = f.label :wiki_page_events, class: 'label-light append-bottom-0' do + Wiki Page events + %p.light + This url will be triggered when a wiki page is created/updated .form-group = f.label :enable_ssl_verification, "SSL verification", class: "label-light" %div diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index e95f42064ac..f3b0469b7d4 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -52,12 +52,12 @@ .issue-details.issuable-details .detail-page-description.content-block %h2.title - = markdown escape_once(@issue.title), pipeline: :single_line + = markdown escape_once(@issue.title), pipeline: :single_line, author: @issue.author - if @issue.description.present? .description{ class: can?(current_user, :update_issue, @issue) ? 'js-task-list-container' : '' } .wiki = preserve do - = markdown(@issue.description, cache_key: [@issue, "description"]) + = markdown(@issue.description, cache_key: [@issue, "description"], author: @issue.author) %textarea.hidden.js-task-list-field = @issue.description = edited_time_ago_with_tooltip(@issue, placement: 'bottom', html_class: 'issue_edited_ago') diff --git a/app/views/projects/merge_requests/dropdowns/_branch.html.haml b/app/views/projects/merge_requests/dropdowns/_branch.html.haml index ba8d9a5835c..a60c445aa51 100644 --- a/app/views/projects/merge_requests/dropdowns/_branch.html.haml +++ b/app/views/projects/merge_requests/dropdowns/_branch.html.haml @@ -1,5 +1,5 @@ %ul - branches.each do |branch| %li - %a{ href: '#', class: "#{('is-active' if selected == branch)}", data: { id: branch } } + %a{ href: '#', class: "#{('is-active' if selected == branch)}", title: branch, data: { id: branch } } = branch diff --git a/app/views/projects/merge_requests/show/_mr_box.html.haml b/app/views/projects/merge_requests/show/_mr_box.html.haml index a23bd8d18d0..ebf18f6ac85 100644 --- a/app/views/projects/merge_requests/show/_mr_box.html.haml +++ b/app/views/projects/merge_requests/show/_mr_box.html.haml @@ -1,13 +1,13 @@ .detail-page-description.content-block %h2.title - = markdown escape_once(@merge_request.title), pipeline: :single_line + = markdown escape_once(@merge_request.title), pipeline: :single_line, author: @merge_request.author %div - if @merge_request.description.present? .description{class: can?(current_user, :update_merge_request, @merge_request) ? 'js-task-list-container' : ''} .wiki = preserve do - = markdown(@merge_request.description, cache_key: [@merge_request, "description"]) + = markdown(@merge_request.description, cache_key: [@merge_request, "description"], author: @merge_request.author) %textarea.hidden.js-task-list-field = @merge_request.description diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml index 55dbae598d3..13359abede7 100644 --- a/app/views/projects/merge_requests/widget/_open.html.haml +++ b/app/views/projects/merge_requests/widget/_open.html.haml @@ -26,4 +26,4 @@ %i.fa.fa-check Accepting this merge request will close #{"issue".pluralize(@closes_issues.size)} = succeed '.' do - != markdown issues_sentence(@closes_issues), pipeline: :gfm + != markdown issues_sentence(@closes_issues), pipeline: :gfm, author: @merge_request.author diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 9fbc9a45549..f1045bbd8c3 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -29,7 +29,7 @@ .note-body{class: note_editable ? 'js-task-list-container' : ''} .note-text = preserve do - = markdown(note.note, pipeline: :note, cache_key: [note, "note"]) + = markdown(note.note, pipeline: :note, cache_key: [note, "note"], author: note.author) - if note_editable = render 'projects/notes/edit_form', note: note = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) diff --git a/app/views/projects/pipelines/_head.html.haml b/app/views/projects/pipelines/_head.html.haml new file mode 100644 index 00000000000..2c8ae625e67 --- /dev/null +++ b/app/views/projects/pipelines/_head.html.haml @@ -0,0 +1,14 @@ +%ul.nav-links + - if project_nav_tab? :pipelines + = nav_link(controller: :pipelines) do + = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-pipelines' do + %span + Pipelines + %span.badge.count.ci_counter= number_with_delimiter(@project.ci_commits.running_or_pending.count) + + - if project_nav_tab? :builds + = nav_link(controller: %w(builds)) do + = link_to project_builds_path(@project), title: 'Builds', class: 'shortcuts-builds' do + %span + Builds + %span.badge.count.builds_counter= number_with_delimiter(@project.builds.running_or_pending.count(:all)) diff --git a/app/views/projects/pipelines/index.html.haml b/app/views/projects/pipelines/index.html.haml index 8788db09dbe..453767920b5 100644 --- a/app/views/projects/pipelines/index.html.haml +++ b/app/views/projects/pipelines/index.html.haml @@ -1,4 +1,5 @@ - page_title "Pipelines" += render "projects/pipelines/head" .top-area %ul.nav-links diff --git a/app/views/projects/repositories/_feed.html.haml b/app/views/projects/repositories/_feed.html.haml index 6ca919f7f80..43a6fdfd103 100644 --- a/app/views/projects/repositories/_feed.html.haml +++ b/app/views/projects/repositories/_feed.html.haml @@ -12,7 +12,7 @@ = link_to namespace_project_commits_path(@project.namespace, @project, commit.id) do %code= commit.short_id = image_tag avatar_icon(commit.author_email), class: "", width: 16, alt: '' - = markdown escape_once(truncate(commit.title, length: 40)), pipeline: :single_line + = markdown escape_once(truncate(commit.title, length: 40)), pipeline: :single_line, author: commit.author %td %span.pull-right.cgray = time_ago_with_tooltip(commit.committed_date) diff --git a/app/views/search/results/_issue.html.haml b/app/views/search/results/_issue.html.haml index 640890fbe92..8f68d6d1b87 100644 --- a/app/views/search/results/_issue.html.haml +++ b/app/views/search/results/_issue.html.haml @@ -7,7 +7,7 @@ - if issue.description.present? .description.term = preserve do - = search_md_sanitize(markdown(truncate(issue.description, length: 200, separator: " "), { project: issue.project })) + = search_md_sanitize(markdown(truncate(issue.description, length: 200, separator: " "), { project: issue.project, author: issue.author })) %span.light #{issue.project.name_with_namespace} - if issue.closed? diff --git a/app/views/search/results/_merge_request.html.haml b/app/views/search/results/_merge_request.html.haml index 333f6533213..6331c2bd6b0 100644 --- a/app/views/search/results/_merge_request.html.haml +++ b/app/views/search/results/_merge_request.html.haml @@ -6,7 +6,7 @@ - if merge_request.description.present? .description.term = preserve do - = search_md_sanitize(markdown(merge_request.description, { project: merge_request.project })) + = search_md_sanitize(markdown(merge_request.description, { project: merge_request.project, author: merge_request.author })) %span.light #{merge_request.project.name_with_namespace} .pull-right diff --git a/app/views/search/results/_note.html.haml b/app/views/search/results/_note.html.haml index d9400b1d9fa..8163aff43b6 100644 --- a/app/views/search/results/_note.html.haml +++ b/app/views/search/results/_note.html.haml @@ -19,4 +19,4 @@ .note-search-result .term = preserve do - = search_md_sanitize(markdown(note.note, {no_header_anchors: true})) + = search_md_sanitize(markdown(note.note, {no_header_anchors: true, author: note.author})) diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index 897aa5b4462..af753496260 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -4,7 +4,7 @@ = visibility_level_label(@snippet.visibility_level) = visibility_level_icon(@snippet.visibility_level, fw: false) %strong.item-title - Snippet ##{@snippet.id} + Snippet #{@snippet.to_reference} %span.creator created by #{link_to_member(@project, @snippet.author, size: 24, author_class: "author item-title")} = time_ago_with_tooltip(@snippet.created_at, placement: 'bottom', html_class: 'snippet_updated_ago') @@ -21,4 +21,4 @@ .content-block.second-block %h2.snippet-title.prepend-top-0.append-bottom-0 - = markdown escape_once(@snippet.title), pipeline: :single_line + = markdown escape_once(@snippet.title), pipeline: :single_line, author: @snippet.author diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb index fa959fc56e3..971f969e25e 100644 --- a/app/workers/emails_on_push_worker.rb +++ b/app/workers/emails_on_push_worker.rb @@ -1,6 +1,7 @@ class EmailsOnPushWorker include Sidekiq::Worker + sidekiq_options queue: :mailers attr_reader :email, :skip_premailer def perform(project_id, recipients, push_data, options = {}) diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 124d63ce3ac..436751b9d16 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -52,7 +52,7 @@ class Settings < Settingslogic # check that values in `current` (string or integer) is a contant in `modul`. def verify_constant_array(modul, current, default) values = default || [] - if !current.nil? + unless current.nil? values = [] current.each do |constant| values.push(verify_constant(modul, constant, nil)) diff --git a/config/initializers/devise_async.rb b/config/initializers/devise_async.rb deleted file mode 100644 index 05a1852cdbd..00000000000 --- a/config/initializers/devise_async.rb +++ /dev/null @@ -1 +0,0 @@ -Devise::Async.backend = :sidekiq diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 66ac88e9f4a..7bd13105045 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -52,7 +52,7 @@ Doorkeeper.configure do # For more information go to # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes default_scopes :api - #optional_scopes :write, :update + # optional_scopes :write, :update # Change the way client credentials are retrieved from the request object. # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then @@ -71,7 +71,7 @@ Doorkeeper.configure do # The value can be any string. Use nil to disable this feature. When disabled, clients must provide a valid URL # (Similar behaviour: https://developers.google.com/accounts/docs/OAuth2InstalledApp#choosingredirecturi) # - native_redirect_uri nil#'urn:ietf:wg:oauth:2.0:oob' + native_redirect_uri nil # 'urn:ietf:wg:oauth:2.0:oob' # Specify what grant flows are enabled in array of Strings. The valid # strings and the flows they enable are: diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 4c164119fff..26c30e523a7 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -13,7 +13,7 @@ end OmniAuth.config.full_host = Settings.gitlab['base_url'] OmniAuth.config.allowed_request_methods = [:post] -#In case of auto sign-in, the GET method is used (users don't get to click on a button) +# In case of auto sign-in, the GET method is used (users don't get to click on a button) OmniAuth.config.allowed_request_methods << :get if Gitlab.config.omniauth.auto_sign_in_with_provider.present? OmniAuth.config.before_request_phase do |env| OmniAuth::RequestForgeryProtection.call(env) diff --git a/config/initializers/session_store.rb b/config/initializers/session_store.rb index 599dabb9e50..0d9d87bac00 100644 --- a/config/initializers/session_store.rb +++ b/config/initializers/session_store.rb @@ -23,6 +23,6 @@ else secure: Gitlab.config.gitlab.https, httponly: true, expires_in: Settings.gitlab['session_expire_delay'] * 60, - path: (Rails.application.config.relative_url_root.nil?) ? '/' : Gitlab::Application.config.relative_url_root + path: Rails.application.config.relative_url_root.nil? ? '/' : Gitlab::Application.config.relative_url_root ) end diff --git a/config/routes.rb b/config/routes.rb index 9a5d0074a0a..428302d0fd7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -421,7 +421,11 @@ Rails.application.routes.draw do resources :projects, constraints: { id: /[^\/]+/ }, only: [:index, :new, :create] - devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks, registrations: :registrations , passwords: :passwords, sessions: :sessions, confirmations: :confirmations } + devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks, + registrations: :registrations, + passwords: :passwords, + sessions: :sessions, + confirmations: :confirmations } devise_scope :user do get '/users/auth/:provider/omniauth_error' => 'omniauth_callbacks#omniauth_error', as: :omniauth_error @@ -789,7 +793,7 @@ Rails.application.routes.draw do end # Get all keys of user - get ':username.keys' => 'profiles/keys#get_keys' , constraints: { username: /.*/ } + get ':username.keys' => 'profiles/keys#get_keys', constraints: { username: /.*/ } get ':id' => 'namespaces#show', constraints: { id: /(?:[^.]|\.(?!atom$))+/, format: /atom/ } end diff --git a/db/migrate/20160527020117_remove_notification_settings_for_deleted_projects.rb b/db/migrate/20160527020117_remove_notification_settings_for_deleted_projects.rb new file mode 100644 index 00000000000..7910120b4e0 --- /dev/null +++ b/db/migrate/20160527020117_remove_notification_settings_for_deleted_projects.rb @@ -0,0 +1,13 @@ +class RemoveNotificationSettingsForDeletedProjects < ActiveRecord::Migration + def up + execute <<-SQL + DELETE FROM notification_settings + WHERE notification_settings.source_type = 'Project' + AND NOT EXISTS ( + SELECT * + FROM projects + WHERE projects.id = notification_settings.source_id + ) + SQL + end +end diff --git a/db/migrate/20160528043124_add_users_state_index.rb b/db/migrate/20160528043124_add_users_state_index.rb new file mode 100644 index 00000000000..e77a5460737 --- /dev/null +++ b/db/migrate/20160528043124_add_users_state_index.rb @@ -0,0 +1,9 @@ +class AddUsersStateIndex < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + def change + add_concurrent_index :users, :state + end +end diff --git a/db/migrate/20160530150109_add_container_registry_token_expire_delay_to_application_settings.rb b/db/migrate/20160530150109_add_container_registry_token_expire_delay_to_application_settings.rb new file mode 100644 index 00000000000..e21376bd571 --- /dev/null +++ b/db/migrate/20160530150109_add_container_registry_token_expire_delay_to_application_settings.rb @@ -0,0 +1,9 @@ +# This is ONLINE migration + +class AddContainerRegistryTokenExpireDelayToApplicationSettings < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + def change + add_column :application_settings, :container_registry_token_expire_delay, :integer, default: 5 + end +end diff --git a/db/schema.rb b/db/schema.rb index b42eef08a00..b2af810f600 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160525205328) do +ActiveRecord::Schema.define(version: 20160530150109) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -43,46 +43,47 @@ ActiveRecord::Schema.define(version: 20160525205328) do t.datetime "created_at" t.datetime "updated_at" t.string "home_page_url" - t.integer "default_branch_protection", default: 2 + t.integer "default_branch_protection", default: 2 t.text "restricted_visibility_levels" - t.boolean "version_check_enabled", default: true - t.integer "max_attachment_size", default: 10, null: false + t.boolean "version_check_enabled", default: true + t.integer "max_attachment_size", default: 10, null: false t.integer "default_project_visibility" t.integer "default_snippet_visibility" t.text "restricted_signup_domains" - t.boolean "user_oauth_applications", default: true + t.boolean "user_oauth_applications", default: true t.string "after_sign_out_path" - t.integer "session_expire_delay", default: 10080, null: false + t.integer "session_expire_delay", default: 10080, null: false t.text "import_sources" t.text "help_page_text" t.string "admin_notification_email" - t.boolean "shared_runners_enabled", default: true, null: false - t.integer "max_artifacts_size", default: 100, null: false + t.boolean "shared_runners_enabled", default: true, null: false + t.integer "max_artifacts_size", default: 100, null: false t.string "runners_registration_token" - t.boolean "require_two_factor_authentication", default: false - t.integer "two_factor_grace_period", default: 48 - t.boolean "metrics_enabled", default: false - t.string "metrics_host", default: "localhost" - t.integer "metrics_pool_size", default: 16 - t.integer "metrics_timeout", default: 10 - t.integer "metrics_method_call_threshold", default: 10 - t.boolean "recaptcha_enabled", default: false + t.boolean "require_two_factor_authentication", default: false + t.integer "two_factor_grace_period", default: 48 + t.boolean "metrics_enabled", default: false + t.string "metrics_host", default: "localhost" + t.integer "metrics_pool_size", default: 16 + t.integer "metrics_timeout", default: 10 + t.integer "metrics_method_call_threshold", default: 10 + t.boolean "recaptcha_enabled", default: false t.string "recaptcha_site_key" t.string "recaptcha_private_key" - t.integer "metrics_port", default: 8089 - t.boolean "akismet_enabled", default: false + t.integer "metrics_port", default: 8089 + t.boolean "akismet_enabled", default: false t.string "akismet_api_key" - t.integer "metrics_sample_interval", default: 15 - t.boolean "sentry_enabled", default: false + t.integer "metrics_sample_interval", default: 15 + t.boolean "sentry_enabled", default: false t.string "sentry_dsn" - t.boolean "email_author_in_body", default: false + t.boolean "email_author_in_body", default: false t.integer "default_group_visibility" - t.boolean "repository_checks_enabled", default: false + t.boolean "repository_checks_enabled", default: false t.text "shared_runners_text" - t.integer "metrics_packet_size", default: 1 + t.integer "metrics_packet_size", default: 1 t.text "disabled_oauth_sign_in_sources" t.string "health_check_access_token" - t.boolean "send_user_confirmation_email", default: false + t.boolean "send_user_confirmation_email", default: false + t.integer "container_registry_token_expire_delay", default: 5 end create_table "audit_events", force: :cascade do |t| @@ -1000,6 +1001,7 @@ ActiveRecord::Schema.define(version: 20160525205328) do add_index "users", ["name"], name: "index_users_on_name", using: :btree add_index "users", ["name"], name: "index_users_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"} add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree + add_index "users", ["state"], name: "index_users_on_state", using: :btree add_index "users", ["username"], name: "index_users_on_username", using: :btree add_index "users", ["username"], name: "index_users_on_username_trigram", using: :gin, opclasses: {"username"=>"gin_trgm_ops"} diff --git a/doc/administration/repository_checks.md b/doc/administration/repository_checks.md index 3411e4af6a7..4172b604cec 100644 --- a/doc/administration/repository_checks.md +++ b/doc/administration/repository_checks.md @@ -5,7 +5,7 @@ This feature was [introduced][ce-3232] in GitLab 8.7. It is OFF by default because it still causes too many false alarms. Git has a built-in mechanism, [git fsck][git-fsck], to verify the -integrity of all data commited to a repository. GitLab administrators +integrity of all data committed to a repository. GitLab administrators can trigger such a check for a project via the project page under the admin panel. The checks run asynchronously so it may take a few minutes before the check result is visible on the project admin page. If the @@ -41,4 +41,4 @@ alarms you can choose to clear ALL repository check states from the --- [ce-3232]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3232 "Auto git fsck" -[git-fsck]: https://www.kernel.org/pub/software/scm/git/docs/git-fsck.html "git fsck documentation"
\ No newline at end of file +[git-fsck]: https://www.kernel.org/pub/software/scm/git/docs/git-fsck.html "git fsck documentation" diff --git a/doc/api/labels.md b/doc/api/labels.md index b857d81768e..a181c0f57a2 100644 --- a/doc/api/labels.md +++ b/doc/api/labels.md @@ -39,7 +39,7 @@ Example response: { "name" : "critical", "color" : "#d9534f", - "description": "Criticalissue. Need fix ASAP", + "description": "Critical issue. Need fix ASAP", "open_issues_count": 1, "closed_issues_count": 3, "open_merge_requests_count": 1 diff --git a/doc/api/services.md b/doc/api/services.md index 83ac7845156..ccfc0fccb7f 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -16,8 +16,8 @@ PUT /projects/:id/services/asana Parameters: -- `api_key` (**required**) - User API token. User must have access to task,all comments will be attributed to this user. -- `restrict_to_branch` (optional) - Comma-separated list of branches which will beautomatically inspected. Leave blank to include all branches. +- `api_key` (**required**) - User API token. User must have access to task, all comments will be attributed to this user. +- `restrict_to_branch` (optional) - Comma-separated list of branches which will be automatically inspected. Leave blank to include all branches. ### Delete Asana service @@ -503,6 +503,8 @@ Parameters: - `project_url` (**required**) - Project url - `issues_url` (**required**) - Issue url - `description` (optional) - Jira issue tracker +- `username` (optional) - Jira username +- `password` (optional) - Jira password ### Delete JIRA service diff --git a/doc/api/settings.md b/doc/api/settings.md index 1e745115dc8..43a0fe35e42 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -37,7 +37,8 @@ Example response: "created_at" : "2016-01-04T15:44:55.176Z", "default_project_visibility" : 0, "gravatar_enabled" : true, - "sign_in_text" : null + "sign_in_text" : null, + "container_registry_token_expire_delay": 5 } ``` @@ -64,6 +65,7 @@ PUT /application/settings | `restricted_signup_domains` | array of strings | no | Force people to use only corporate emails for sign-up. Default is null, meaning there is no restriction. | | `user_oauth_applications` | boolean | no | Allow users to register any application to use GitLab as an OAuth provider | | `after_sign_out_path` | string | no | Where to redirect users after logout | +| `container_registry_token_expire_delay` | integer | no | Container Registry token duration in minutes | ```bash curl -X PUT -H "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/application/settings?signup_enabled=false&default_project_visibility=1 @@ -90,6 +92,7 @@ Example response: "default_snippet_visibility": 0, "restricted_signup_domains": [], "user_oauth_applications": true, - "after_sign_out_path": "" + "after_sign_out_path": "", + "container_registry_token_expire_delay": 5 } ``` diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 63866d8c71c..a3481f58c6c 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -348,7 +348,7 @@ job_name: | allow_failure | no | Allow build to fail. Failed build doesn't contribute to commit status | | when | no | Define when to run build. Can be `on_success`, `on_failure` or `always` | | dependencies | no | Define other builds that a build depends on so that you can pass artifacts between them| -| artifacts | no | Define list build artifacts | +| artifacts | no | Define list of build artifacts | | cache | no | Define list of files that should be cached between subsequent runs | | before_script | no | Override a set of commands that are executed before build | | after_script | no | Override a set of commands that are executed after build | diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 1c13b094582..02e024ca15a 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -116,7 +116,7 @@ Example with Arel: users = Arel::Table.new(:users) users.group(users[:user_id]).having(users[:id].count.gt(5)) -#updtae other tables with this results +#update other tables with these results ``` Example with plain SQL and `quote_string` helper: diff --git a/doc/install/requirements.md b/doc/install/requirements.md index df8e8bdc476..09c6211b3ab 100644 --- a/doc/install/requirements.md +++ b/doc/install/requirements.md @@ -81,7 +81,7 @@ errors during usage. - More users? Run it on [multiple application servers](https://about.gitlab.com/high-availability/) We recommend having at least 1GB of swap on your server, even if you currently have -enough available RAM. Having swap will help reduce the chance of errors occuring +enough available RAM. Having swap will help reduce the chance of errors occurring if your available memory changes. Notice: The 25 workers of Sidekiq will show up as separate processes in your process overview (such as top or htop) but they share the same RAM allocation since Sidekiq is a multithreaded application. Please see the section below about Unicorn workers for information about many you need of those. @@ -150,3 +150,4 @@ On a very active server (10,000 active users) the Sidekiq process can use 1GB+ o - Safari 7+ (known problem: required fields in html5 do not work) - Opera (Latest released version) - Internet Explorer (IE) 11+ but please make sure that you have the `Compatibility View` mode disabled. +- Edge (Latest stable version) diff --git a/doc/logs/logs.md b/doc/logs/logs.md index ef5affa2ebd..f84060b8d07 100644 --- a/doc/logs/logs.md +++ b/doc/logs/logs.md @@ -1,5 +1,5 @@ ## Log system -GitLab has advanced log system so everything is logging and you can analize your instance using various system log files. +GitLab has an advanced log system where everything is logged so that you can analyze your instance using various system log files. In addition to system log files, GitLab Enterprise Edition comes with Audit Events. Find more about them [in Audit Events documentation](http://docs.gitlab.com/ee/administration/audit_events.html) System log files are typically plain text in a standard log file format. This guide talks about how to read and use these system log files. @@ -67,13 +67,13 @@ gitlab-shell is using by Gitlab for executing git commands and provide ssh acces ``` I, [2015-02-13T06:17:00.671315 #9291] INFO -- : Adding project root/example.git at </var/opt/gitlab/git-data/repositories/root/dcdcdcdcd.git>. -I, [2015-02-13T06:17:00.679433 #9291] INFO -- : Moving existing hooks directory and simlinking global hooks directory for /var/opt/gitlab/git-data/repositories/root/example.git. +I, [2015-02-13T06:17:00.679433 #9291] INFO -- : Moving existing hooks directory and symlinking global hooks directory for /var/opt/gitlab/git-data/repositories/root/example.git. ``` #### unicorn_stderr.log This file lives in `/var/log/gitlab/unicorn/unicorn_stderr.log` for omnibus package or in `/home/git/gitlab/log/unicorn_stderr.log` for installations from the source. -Unicorn is a high-performance forking Web server which is used for serving GitLab application. You can look at this log, for example, if your application does not respond. This log cantains all information about state of unicorn processes at any given time. +Unicorn is a high-performance forking Web server which is used for serving the GitLab application. You can look at this log if, for example, your application does not respond. This log contains all information about the state of unicorn processes at any given time. ``` I, [2015-02-13T06:14:46.680381 #9047] INFO -- : Refreshing Gem list diff --git a/doc/migrate_ci_to_ce/README.md b/doc/migrate_ci_to_ce/README.md index 5ec0a2069b5..8f9ef054949 100644 --- a/doc/migrate_ci_to_ce/README.md +++ b/doc/migrate_ci_to_ce/README.md @@ -355,7 +355,7 @@ sudo chown git:git /var/opt/gitlab/gitlab-ci/builds ``` #### Problems when importing CI database to GitLab -If you were migrating CI database from MySQL to PostgreSQL manually you can see errros during import about missing sequences: +If you were migrating CI database from MySQL to PostgreSQL manually you can see errors during import about missing sequences: ``` ALTER SEQUENCE ERROR: relation "ci_builds_id_seq" does not exist diff --git a/doc/operations/moving_repositories.md b/doc/operations/moving_repositories.md index 39086b7a251..54adb99386a 100644 --- a/doc/operations/moving_repositories.md +++ b/doc/operations/moving_repositories.md @@ -134,7 +134,7 @@ sudo -u git sh -c ' cat /var/opt/gitlab/transfer-logs/* | sort | uniq -u |\ /usr/bin/env JOBS=10 \ /opt/gitlab/embedded/service/gitlab-rails/bin/parallel-rsync-repos \ - /var/opt/gitlab/transfer-logs/succes-$(date +%s).log \ + /var/opt/gitlab/transfer-logs/success-$(date +%s).log \ /var/opt/gitlab/git-data/repositories \ /mnt/gitlab/repositories ' @@ -145,7 +145,7 @@ sudo -u git -H sh -c ' cat /home/git/transfer-logs/* | sort | uniq -u |\ /usr/bin/env JOBS=10 \ bin/parallel-rsync-repos \ - /home/git/transfer-logs/succes-$(date +%s).log \ + /home/git/transfer-logs/success-$(date +%s).log \ /home/git/repositories \ /mnt/gitlab/repositories ` @@ -164,7 +164,7 @@ sudo gitlab-rake gitlab:list_repos SINCE='2015-10-1 12:00 UTC' |\ sudo -u git \ /usr/bin/env JOBS=10 \ /opt/gitlab/embedded/service/gitlab-rails/bin/parallel-rsync-repos \ - succes-$(date +%s).log \ + success-$(date +%s).log \ /var/opt/gitlab/git-data/repositories \ /mnt/gitlab/repositories @@ -174,7 +174,7 @@ sudo -u git -H bundle exec rake gitlab:list_repos SINCE='2015-10-1 12:00 UTC' |\ sudo -u git -H \ /usr/bin/env JOBS=10 \ bin/parallel-rsync-repos \ - succes-$(date +%s).log \ + success-$(date +%s).log \ /home/git/repositories \ /mnt/gitlab/repositories ``` diff --git a/doc/update/8.6-to-8.7.md b/doc/update/8.6-to-8.7.md index 4a2c6ea91d2..bb463d43a7c 100644 --- a/doc/update/8.6-to-8.7.md +++ b/doc/update/8.6-to-8.7.md @@ -86,6 +86,14 @@ sudo -u git -H bundle exec rake assets:clean assets:precompile cache:clear RAILS ### 7. Update configuration files +#### New configuration options for `gitlab.yml` + +There are new configuration options available for [`gitlab.yml`](config/gitlab.yml.example). View them with the command below and apply them manually to your current `gitlab.yml`: + +```sh +git diff origin/8-6-stable:config/gitlab.yml.example origin/8-7-stable:config/gitlab.yml.example +``` + #### Git configuration Disable `git gc --auto` because GitLab runs `git gc` for us already. diff --git a/doc/update/8.7-to-8.8.md b/doc/update/8.7-to-8.8.md index b4d9212289c..32906650f6f 100644 --- a/doc/update/8.7-to-8.8.md +++ b/doc/update/8.7-to-8.8.md @@ -86,6 +86,14 @@ sudo -u git -H bundle exec rake assets:clean assets:precompile cache:clear RAILS ### 7. Update configuration files +#### New configuration options for `gitlab.yml` + +There are new configuration options available for [`gitlab.yml`](config/gitlab.yml.example). View them with the command below and apply them manually to your current `gitlab.yml`: + +```sh +git diff origin/8-7-stable:config/gitlab.yml.example origin/8-8-stable:config/gitlab.yml.example +``` + #### Git configuration Disable `git gc --auto` because GitLab runs `git gc` for us already. @@ -137,7 +145,7 @@ To make sure you didn't miss anything run a more thorough check: If all items are green, then congratulations, the upgrade is complete! -## Things went south? Revert to previous version (8.6) +## Things went south? Revert to previous version (8.7) ### 1. Revert the code to the previous version diff --git a/doc/web_hooks/web_hooks.md b/doc/web_hooks/web_hooks.md index 45506ac1d7c..8559b67af04 100644 --- a/doc/web_hooks/web_hooks.md +++ b/doc/web_hooks/web_hooks.md @@ -695,6 +695,61 @@ X-Gitlab-Event: Merge Request Hook } ``` +## Wiki Page events + +Triggered when a wiki page is created or edited. + +**Request Header**: + +``` +X-Gitlab-Event: Wiki Page Hook +``` + +**Request Body**: + +```json +{ + "object_kind": "wiki_page", + "user": { + "name": "Administrator", + "username": "root", + "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80\u0026d=identicon" + }, + "project": { + "name": "awesome-project", + "description": "This is awesome", + "web_url": "http://example.com/root/awesome-project", + "avatar_url": null, + "git_ssh_url": "git@example.com:root/awesome-project.git", + "git_http_url": "http://example.com/root/awesome-project.git", + "namespace": "root", + "visibility_level": 0, + "path_with_namespace": "root/awesome-project", + "default_branch": "master", + "homepage": "http://example.com/root/awesome-project", + "url": "git@example.com:root/awesome-project.git", + "ssh_url": "git@example.com:root/awesome-project.git", + "http_url": "http://example.com/root/awesome-project.git" + }, + "wiki": { + "web_url": "http://example.com/root/awesome-project/wikis/home", + "git_ssh_url": "git@example.com:root/awesome-project.wiki.git", + "git_http_url": "http://example.com/root/awesome-project.wiki.git", + "path_with_namespace": "root/awesome-project.wiki", + "default_branch": "master" + }, + "object_attributes": { + "title": "Awesome", + "content": "awesome content goes here", + "format": "markdown", + "message": "adding an awesome page to the wiki", + "slug": "awesome", + "url": "http://example.com/root/awesome-project/wikis/awesome", + "action": "create" + } +} +``` + #### Example webhook receiver If you want to see GitLab's webhooks in action for testing purposes you can use diff --git a/features/steps/dashboard/todos.rb b/features/steps/dashboard/todos.rb index d3b6c7f6a15..bd8a270202e 100644 --- a/features/steps/dashboard/todos.rb +++ b/features/steps/dashboard/todos.rb @@ -20,7 +20,7 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps step 'I have todos' do create(:todo, user: current_user, project: project, author: mary_jane, target: issue, action: Todo::MENTIONED) create(:todo, user: current_user, project: project, author: john_doe, target: issue, action: Todo::ASSIGNED) - note = create(:note, author: john_doe, noteable: issue, note: "#{current_user.to_reference} Wdyt?") + note = create(:note, author: john_doe, noteable: issue, note: "#{current_user.to_reference} Wdyt?", project: project) create(:todo, user: current_user, project: project, author: john_doe, target: issue, action: Todo::MENTIONED, note: note) create(:todo, user: current_user, project: project, author: john_doe, target: merge_request, action: Todo::ASSIGNED) end diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index f2c68f007ef..5cd431e05d5 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -348,7 +348,7 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps step 'another user adds a comment with text "Yay!" to issue "Release 0.4"' do issue = Issue.find_by!(title: 'Release 0.4') - create(:note_on_issue, noteable: issue, note: 'Yay!') + create(:note_on_issue, noteable: issue, project: project, note: 'Yay!') end step 'I should see a new comment with text "Yay!"' do diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index cfa586f859f..b30346790eb 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -273,7 +273,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps step 'user "John Doe" leaves a comment like "Line is wrong" on diff' do mr = MergeRequest.find_by(title: "Bug NS-05") create(:note_on_merge_request_diff, project: project, - noteable_id: mr.id, + noteable: mr, author: user_exists("John Doe"), line_code: sample_commit.line_code, note: 'Line is wrong') @@ -567,7 +567,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps click_diff_line(sample_compare.changes[1][:line_code]) end - def have_visible_content (text) + def have_visible_content(text) have_css("*", text: text, visible: true) end diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 31491cf31dd..790a1869f73 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -362,6 +362,7 @@ module API expose :restricted_signup_domains expose :user_oauth_applications expose :after_sign_out_path + expose :container_registry_token_expire_delay end class Release < Grape::Entity diff --git a/lib/api/licenses.rb b/lib/api/licenses.rb index 187d2c04703..be0e113fbcb 100644 --- a/lib/api/licenses.rb +++ b/lib/api/licenses.rb @@ -2,15 +2,15 @@ module API # Licenses API class Licenses < Grape::API PROJECT_TEMPLATE_REGEX = - /[\<\{\[] - (project|description| - one\sline\s.+\swhat\sit\sdoes\.) # matching the start and end is enough here - [\>\}\]]/xi.freeze + /[\<\{\[] + (project|description| + one\sline\s.+\swhat\sit\sdoes\.) # matching the start and end is enough here + [\>\}\]]/xi.freeze YEAR_TEMPLATE_REGEX = /[<{\[](year|yyyy)[>}\]]/i.freeze FULLNAME_TEMPLATE_REGEX = - /[\<\{\[] - (fullname|name\sof\s(author|copyright\sowner)) - [\>\}\]]/xi.freeze + /[\<\{\[] + (fullname|name\sof\s(author|copyright\sowner)) + [\>\}\]]/xi.freeze # Get the list of the available license templates # diff --git a/lib/api/users.rb b/lib/api/users.rb index ea6fa2dc8a8..8a376d3c2a3 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -76,7 +76,7 @@ module API required_attributes! [:email, :password, :name, :username] attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :bio, :location, :can_create_group, :admin, :confirm, :external] admin = attrs.delete(:admin) - confirm = !(attrs.delete(:confirm) =~ (/(false|f|no|0)$/i)) + confirm = !(attrs.delete(:confirm) =~ /(false|f|no|0)$/i) user = User.build_user(attrs) user.admin = admin unless admin.nil? user.skip_confirmation! unless confirm diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index 3e07096e6cc..660ca8c2923 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -48,7 +48,7 @@ module Backup end connection = ::Fog::Storage.new(connection_settings) - directory = connection.directories.get(remote_directory) + directory = connection.directories.create(key: remote_directory) if directory.files.create(key: tar_file, body: File.open(tar_file), public: false, multipart_chunk_size: Gitlab.config.backup.upload.multipart_chunk_size, diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index b8962379cb5..db95d7c908b 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -18,10 +18,6 @@ module Banzai @object_sym ||= object_name.to_sym end - def self.data_reference - @data_reference ||= "data-#{object_name.dasherize}" - end - def self.object_class_title @object_title ||= object_class.name.titleize end @@ -45,10 +41,6 @@ module Banzai end end - def self.referenced_by(node) - { object_sym => LazyReference.new(object_class, node.attr(data_reference)) } - end - def object_class self.class.object_class end @@ -236,7 +228,9 @@ module Banzai if cache.key?(key) cache[key] else - cache[key] = yield + value = yield + cache[key] = value if key.present? + value end end end diff --git a/lib/banzai/filter/commit_range_reference_filter.rb b/lib/banzai/filter/commit_range_reference_filter.rb index b469ea0f626..bbb88c979cc 100644 --- a/lib/banzai/filter/commit_range_reference_filter.rb +++ b/lib/banzai/filter/commit_range_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # # This filter supports cross-project references. class CommitRangeReferenceFilter < AbstractReferenceFilter + self.reference_type = :commit_range + def self.object_class CommitRange end @@ -14,34 +16,18 @@ module Banzai end end - def self.referenced_by(node) - project = Project.find(node.attr("data-project")) rescue nil - return unless project - - id = node.attr("data-commit-range") - range = find_object(project, id) - - return unless range - - { commit_range: range } - end - def initialize(*args) super @commit_map = {} end - def self.find_object(project, id) + def find_object(project, id) range = CommitRange.new(id, project) range.valid_commits? ? range : nil end - def find_object(*args) - self.class.find_object(*args) - end - def url_for_object(range, project) h = Gitlab::Routing.url_helpers h.namespace_project_compare_url(project.namespace, project, diff --git a/lib/banzai/filter/commit_reference_filter.rb b/lib/banzai/filter/commit_reference_filter.rb index bd88207326c..2ce1816672b 100644 --- a/lib/banzai/filter/commit_reference_filter.rb +++ b/lib/banzai/filter/commit_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # # This filter supports cross-project references. class CommitReferenceFilter < AbstractReferenceFilter + self.reference_type = :commit + def self.object_class Commit end @@ -14,28 +16,12 @@ module Banzai end end - def self.referenced_by(node) - project = Project.find(node.attr("data-project")) rescue nil - return unless project - - id = node.attr("data-commit") - commit = find_object(project, id) - - return unless commit - - { commit: commit } - end - - def self.find_object(project, id) + def find_object(project, id) if project && project.valid_repo? project.commit(id) end end - def find_object(*args) - self.class.find_object(*args) - end - def url_for_object(commit, project) h = Gitlab::Routing.url_helpers h.namespace_project_commit_url(project.namespace, project, commit, diff --git a/lib/banzai/filter/external_issue_reference_filter.rb b/lib/banzai/filter/external_issue_reference_filter.rb index 37344b90576..eaa702952cc 100644 --- a/lib/banzai/filter/external_issue_reference_filter.rb +++ b/lib/banzai/filter/external_issue_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # References are ignored if the project doesn't use an external issue # tracker. class ExternalIssueReferenceFilter < ReferenceFilter + self.reference_type = :external_issue + # Public: Find `JIRA-123` issue references in text # # ExternalIssueReferenceFilter.references_in(text) do |match, issue| @@ -21,18 +23,6 @@ module Banzai end end - def self.referenced_by(node) - project = Project.find(node.attr("data-project")) rescue nil - return unless project - - id = node.attr("data-external-issue") - external_issue = ExternalIssue.new(id, project) - - return unless external_issue - - { external_issue: external_issue } - end - def call # Early return if the project isn't using an external tracker return doc if project.nil? || default_issues_tracker? diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb index 59c5e89c546..2496e704002 100644 --- a/lib/banzai/filter/issue_reference_filter.rb +++ b/lib/banzai/filter/issue_reference_filter.rb @@ -5,18 +5,12 @@ module Banzai # # This filter supports cross-project references. class IssueReferenceFilter < AbstractReferenceFilter + self.reference_type = :issue + def self.object_class Issue end - def self.user_can_see_reference?(user, node, context) - # It is not possible to check access rights for external issue trackers - return true if context[:project].try(:external_issue_tracker) - - issue = Issue.find(node.attr('data-issue')) rescue nil - Ability.abilities.allowed?(user, :read_issue, issue) - end - def find_object(project, id) project.get_issue(id) end diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb index 8488a493b55..e4d3f87d0aa 100644 --- a/lib/banzai/filter/label_reference_filter.rb +++ b/lib/banzai/filter/label_reference_filter.rb @@ -2,6 +2,8 @@ module Banzai module Filter # HTML filter that replaces label references with links. class LabelReferenceFilter < AbstractReferenceFilter + self.reference_type = :label + def self.object_class Label end diff --git a/lib/banzai/filter/merge_request_reference_filter.rb b/lib/banzai/filter/merge_request_reference_filter.rb index cad38a51851..ac5216d9cfb 100644 --- a/lib/banzai/filter/merge_request_reference_filter.rb +++ b/lib/banzai/filter/merge_request_reference_filter.rb @@ -5,6 +5,8 @@ module Banzai # # This filter supports cross-project references. class MergeRequestReferenceFilter < AbstractReferenceFilter + self.reference_type = :merge_request + def self.object_class MergeRequest end diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index dad0768f51b..ca686c87d97 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -2,6 +2,8 @@ module Banzai module Filter # HTML filter that replaces milestone references with links. class MilestoneReferenceFilter < AbstractReferenceFilter + self.reference_type = :milestone + def self.object_class Milestone end diff --git a/lib/banzai/filter/redactor_filter.rb b/lib/banzai/filter/redactor_filter.rb index e589b5df6ec..c753a84a20d 100644 --- a/lib/banzai/filter/redactor_filter.rb +++ b/lib/banzai/filter/redactor_filter.rb @@ -7,8 +7,11 @@ module Banzai # class RedactorFilter < HTML::Pipeline::Filter def call - Querying.css(doc, 'a.gfm').each do |node| - unless user_can_see_reference?(node) + nodes = Querying.css(doc, 'a.gfm[data-reference-type]') + visible = nodes_visible_to_user(nodes) + + nodes.each do |node| + unless visible.include?(node) # The reference should be replaced by the original text, # which is not always the same as the rendered text. text = node.attr('data-original') || node.text @@ -21,20 +24,30 @@ module Banzai private - def user_can_see_reference?(node) - if node.has_attribute?('data-reference-filter') - reference_type = node.attr('data-reference-filter') - reference_filter = Banzai::Filter.const_get(reference_type) + def nodes_visible_to_user(nodes) + per_type = Hash.new { |h, k| h[k] = [] } + visible = Set.new + + nodes.each do |node| + per_type[node.attr('data-reference-type')] << node + end + + per_type.each do |type, nodes| + parser = Banzai::ReferenceParser[type].new(project, current_user) - reference_filter.user_can_see_reference?(current_user, node, context) - else - true + visible.merge(parser.nodes_visible_to_user(current_user, nodes)) end + + visible end def current_user context[:current_user] end + + def project + context[:project] + end end end end diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb index 31386cf851c..41ae0e1f9cc 100644 --- a/lib/banzai/filter/reference_filter.rb +++ b/lib/banzai/filter/reference_filter.rb @@ -8,24 +8,8 @@ module Banzai # :project (required) - Current project, ignored if reference is cross-project. # :only_path - Generate path-only links. class ReferenceFilter < HTML::Pipeline::Filter - def self.user_can_see_reference?(user, node, context) - if node.has_attribute?('data-project') - project_id = node.attr('data-project').to_i - return true if project_id == context[:project].try(:id) - - project = Project.find(project_id) rescue nil - Ability.abilities.allowed?(user, :read_project, project) - else - true - end - end - - def self.user_can_reference?(user, node, context) - true - end - - def self.referenced_by(node) - raise NotImplementedError, "#{self} does not implement #{__method__}" + class << self + attr_accessor :reference_type end # Returns a data attribute String to attach to a reference link @@ -43,7 +27,9 @@ module Banzai # # Returns a String def data_attribute(attributes = {}) - attributes[:reference_filter] = self.class.name.demodulize + attributes = attributes.reject { |_, v| v.nil? } + + attributes[:reference_type] = self.class.reference_type attributes.delete(:original) if context[:no_original_data] attributes.map { |key, value| %Q(data-#{key.to_s.dasherize}="#{escape_once(value)}") }.join(" ") end diff --git a/lib/banzai/filter/reference_gatherer_filter.rb b/lib/banzai/filter/reference_gatherer_filter.rb deleted file mode 100644 index 96fdb06304e..00000000000 --- a/lib/banzai/filter/reference_gatherer_filter.rb +++ /dev/null @@ -1,65 +0,0 @@ -module Banzai - module Filter - # HTML filter that gathers all referenced records that the current user has - # permission to view. - # - # Expected to be run in its own post-processing pipeline. - # - class ReferenceGathererFilter < HTML::Pipeline::Filter - def initialize(*) - super - - result[:references] ||= Hash.new { |hash, type| hash[type] = [] } - end - - def call - Querying.css(doc, 'a.gfm').each do |node| - gather_references(node) - end - - load_lazy_references unless ReferenceExtractor.lazy? - - doc - end - - private - - def gather_references(node) - return unless node.has_attribute?('data-reference-filter') - - reference_type = node.attr('data-reference-filter') - reference_filter = Banzai::Filter.const_get(reference_type) - - return if context[:reference_filter] && reference_filter != context[:reference_filter] - - return if author && !reference_filter.user_can_reference?(author, node, context) - - return unless reference_filter.user_can_see_reference?(current_user, node, context) - - references = reference_filter.referenced_by(node) - return unless references - - references.each do |type, values| - Array.wrap(values).each do |value| - result[:references][type] << value - end - end - end - - def load_lazy_references - refs = result[:references] - refs.each do |type, values| - refs[type] = ReferenceExtractor.lazily(values) - end - end - - def current_user - context[:current_user] - end - - def author - context[:author] - end - end - end -end diff --git a/lib/banzai/filter/snippet_reference_filter.rb b/lib/banzai/filter/snippet_reference_filter.rb index d507eb5ebe1..212a0bbf2a0 100644 --- a/lib/banzai/filter/snippet_reference_filter.rb +++ b/lib/banzai/filter/snippet_reference_filter.rb @@ -5,6 +5,8 @@ module Banzai # # This filter supports cross-project references. class SnippetReferenceFilter < AbstractReferenceFilter + self.reference_type = :snippet + def self.object_class Snippet end diff --git a/lib/banzai/filter/user_reference_filter.rb b/lib/banzai/filter/user_reference_filter.rb index eea3af842b6..331d8007257 100644 --- a/lib/banzai/filter/user_reference_filter.rb +++ b/lib/banzai/filter/user_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # # A special `@all` reference is also supported. class UserReferenceFilter < ReferenceFilter + self.reference_type = :user + # Public: Find `@user` user references in text # # UserReferenceFilter.references_in(text) do |match, username| @@ -21,43 +23,6 @@ module Banzai end end - def self.referenced_by(node) - if node.has_attribute?('data-group') - group = Group.find(node.attr('data-group')) rescue nil - return unless group - - { user: group.users } - elsif node.has_attribute?('data-user') - { user: LazyReference.new(User, node.attr('data-user')) } - elsif node.has_attribute?('data-project') - project = Project.find(node.attr('data-project')) rescue nil - return unless project - - { user: project.team.members.flatten } - end - end - - def self.user_can_see_reference?(user, node, context) - if node.has_attribute?('data-group') - group = Group.find(node.attr('data-group')) rescue nil - Ability.abilities.allowed?(user, :read_group, group) - else - super - end - end - - def self.user_can_reference?(user, node, context) - # Only team members can reference `@all` - if node.has_attribute?('data-project') - project = Project.find(node.attr('data-project')) rescue nil - return false unless project - - user && project.team.member?(user) - else - super - end - end - def call return doc if project.nil? @@ -114,9 +79,12 @@ module Banzai def link_to_all(link_text: nil) project = context[:project] + author = context[:author] + url = urls.namespace_project_url(project.namespace, project, only_path: context[:only_path]) - data = data_attribute(project: project.id) + + data = data_attribute(project: project.id, author: author.try(:id)) text = link_text || User.reference_prefix + 'all' link_tag(url, data, text) diff --git a/lib/banzai/lazy_reference.rb b/lib/banzai/lazy_reference.rb deleted file mode 100644 index 1095b4debc7..00000000000 --- a/lib/banzai/lazy_reference.rb +++ /dev/null @@ -1,25 +0,0 @@ -module Banzai - class LazyReference - def self.load(refs) - lazy_references, values = refs.partition { |ref| ref.is_a?(self) } - - lazy_values = lazy_references.group_by(&:klass).flat_map do |klass, refs| - ids = refs.flat_map(&:ids) - klass.where(id: ids) - end - - values + lazy_values - end - - attr_reader :klass, :ids - - def initialize(klass, ids) - @klass = klass - @ids = Array.wrap(ids).map(&:to_i) - end - - def load - self.klass.where(id: self.ids) - end - end -end diff --git a/lib/banzai/pipeline/reference_extraction_pipeline.rb b/lib/banzai/pipeline/reference_extraction_pipeline.rb deleted file mode 100644 index 919998380e4..00000000000 --- a/lib/banzai/pipeline/reference_extraction_pipeline.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Banzai - module Pipeline - class ReferenceExtractionPipeline < BasePipeline - def self.filters - FilterArray[ - Filter::ReferenceGathererFilter - ] - end - end - end -end diff --git a/lib/banzai/reference_extractor.rb b/lib/banzai/reference_extractor.rb index f4079538ec5..bf366962aef 100644 --- a/lib/banzai/reference_extractor.rb +++ b/lib/banzai/reference_extractor.rb @@ -1,28 +1,6 @@ module Banzai # Extract possible GFM references from an arbitrary String for further processing. class ReferenceExtractor - class << self - LAZY_KEY = :banzai_reference_extractor_lazy - - def lazy? - Thread.current[LAZY_KEY] - end - - def lazily(values = nil, &block) - return (values || block.call).uniq if lazy? - - begin - Thread.current[LAZY_KEY] = true - - values ||= block.call - - Banzai::LazyReference.load(values.uniq).uniq - ensure - Thread.current[LAZY_KEY] = false - end - end - end - def initialize @texts = [] end @@ -31,23 +9,21 @@ module Banzai @texts << Renderer.render(text, context) end - def references(type, context = {}) - filter = Banzai::Filter["#{type}_reference"] + def references(type, project, current_user = nil) + processor = Banzai::ReferenceParser[type]. + new(project, current_user) + + processor.process(html_documents) + end - context.merge!( - pipeline: :reference_extraction, + private - # ReferenceGathererFilter - reference_filter: filter - ) + def html_documents + # This ensures that we don't memoize anything until we have a number of + # text blobs to parse. + return [] if @texts.empty? - self.class.lazily do - @texts.flat_map do |html| - text_context = context.dup - result = Renderer.render_result(html, text_context) - result[:references][type] - end.uniq - end + @html_documents ||= @texts.map { |html| Nokogiri::HTML.fragment(html) } end end end diff --git a/lib/banzai/reference_parser.rb b/lib/banzai/reference_parser.rb new file mode 100644 index 00000000000..557bec4316e --- /dev/null +++ b/lib/banzai/reference_parser.rb @@ -0,0 +1,14 @@ +module Banzai + module ReferenceParser + # Returns the reference parser class for the given type + # + # Example: + # + # Banzai::ReferenceParser['issue'] + # + # This would return the `Banzai::ReferenceParser::IssueParser` class. + def self.[](name) + const_get("#{name.to_s.camelize}Parser") + end + end +end diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb new file mode 100644 index 00000000000..3d7b9c4a024 --- /dev/null +++ b/lib/banzai/reference_parser/base_parser.rb @@ -0,0 +1,204 @@ +module Banzai + module ReferenceParser + # Base class for reference parsing classes. + # + # Each parser should also specify its reference type by calling + # `self.reference_type = ...` in the body of the class. The value of this + # method should be a symbol such as `:issue` or `:merge_request`. For + # example: + # + # class IssueParser < BaseParser + # self.reference_type = :issue + # end + # + # The reference type is used to determine what nodes to pass to the + # `referenced_by` method. + # + # Parser classes should either implement the instance method + # `references_relation` or overwrite `referenced_by`. The + # `references_relation` method is supposed to return an + # ActiveRecord::Relation used as a base relation for retrieving the objects + # referenced in a set of HTML nodes. + # + # Each class can implement two additional methods: + # + # * `nodes_user_can_reference`: returns an Array of nodes the given user can + # refer to. + # * `nodes_visible_to_user`: returns an Array of nodes that are visible to + # the given user. + # + # You only need to overwrite these methods if you want to tweak who can see + # which references. For example, the IssueParser class defines its own + # `nodes_visible_to_user` method so it can ensure users can only see issues + # they have access to. + class BaseParser + class << self + attr_accessor :reference_type + end + + # Returns the attribute name containing the value for every object to be + # parsed by the current parser. + # + # For example, for a parser class that returns "Animal" objects this + # attribute would be "data-animal". + def self.data_attribute + @data_attribute ||= "data-#{reference_type.to_s.dasherize}" + end + + def initialize(project = nil, current_user = nil) + @project = project + @current_user = current_user + end + + # Returns all the nodes containing references that the user can refer to. + def nodes_user_can_reference(user, nodes) + nodes + end + + # Returns all the nodes that are visible to the given user. + def nodes_visible_to_user(user, nodes) + projects = lazy { projects_for_nodes(nodes) } + project_attr = 'data-project' + + nodes.select do |node| + if node.has_attribute?(project_attr) + node_id = node.attr(project_attr).to_i + + if project && project.id == node_id + true + else + can?(user, :read_project, projects[node_id]) + end + else + true + end + end + end + + # Returns an Array of objects referenced by any of the given HTML nodes. + def referenced_by(nodes) + ids = unique_attribute_values(nodes, self.class.data_attribute) + + references_relation.where(id: ids) + end + + # Returns the ActiveRecord::Relation to use for querying references in the + # DB. + def references_relation + raise NotImplementedError, + "#{self.class} does not implement #{__method__}" + end + + # Returns a Hash containing attribute values per project ID. + # + # The returned Hash uses the following format: + # + # { project id => [value1, value2, ...] } + # + # nodes - An Array of HTML nodes to process. + # attribute - The name of the attribute (as a String) for which to gather + # values. + # + # Returns a Hash. + def gather_attributes_per_project(nodes, attribute) + per_project = Hash.new { |hash, key| hash[key] = Set.new } + + nodes.each do |node| + project_id = node.attr('data-project').to_i + id = node.attr(attribute) + + per_project[project_id] << id if id + end + + per_project + end + + # Returns a Hash containing objects for an attribute grouped per their + # IDs. + # + # The returned Hash uses the following format: + # + # { id value => row } + # + # nodes - An Array of HTML nodes to process. + # + # collection - The model or ActiveRecord relation to use for retrieving + # rows from the database. + # + # attribute - The name of the attribute containing the primary key values + # for every row. + # + # Returns a Hash. + def grouped_objects_for_nodes(nodes, collection, attribute) + return {} if nodes.empty? + + ids = unique_attribute_values(nodes, attribute) + + collection.where(id: ids).each_with_object({}) do |row, hash| + hash[row.id] = row + end + end + + # Returns an Array containing all unique values of an attribute of the + # given nodes. + def unique_attribute_values(nodes, attribute) + values = Set.new + + nodes.each do |node| + if node.has_attribute?(attribute) + values << node.attr(attribute) + end + end + + values.to_a + end + + # Processes the list of HTML documents and returns an Array containing all + # the references. + def process(documents) + type = self.class.reference_type + + nodes = documents.flat_map do |document| + Querying.css(document, "a[data-reference-type='#{type}'].gfm").to_a + end + + gather_references(nodes) + end + + # Gathers the references for the given HTML nodes. + def gather_references(nodes) + nodes = nodes_user_can_reference(current_user, nodes) + nodes = nodes_visible_to_user(current_user, nodes) + + referenced_by(nodes) + end + + # Returns a Hash containing the projects for a given list of HTML nodes. + # + # The returned Hash uses the following format: + # + # { project ID => project } + # + def projects_for_nodes(nodes) + @projects_for_nodes ||= + grouped_objects_for_nodes(nodes, Project, 'data-project') + end + + def can?(user, permission, subject) + Ability.abilities.allowed?(user, permission, subject) + end + + def find_projects_for_hash_keys(hash) + Project.where(id: hash.keys) + end + + private + + attr_reader :current_user, :project + + def lazy(&block) + Gitlab::Lazy.new(&block) + end + end + end +end diff --git a/lib/banzai/reference_parser/commit_parser.rb b/lib/banzai/reference_parser/commit_parser.rb new file mode 100644 index 00000000000..0fee9d267de --- /dev/null +++ b/lib/banzai/reference_parser/commit_parser.rb @@ -0,0 +1,34 @@ +module Banzai + module ReferenceParser + class CommitParser < BaseParser + self.reference_type = :commit + + def referenced_by(nodes) + commit_ids = commit_ids_per_project(nodes) + projects = find_projects_for_hash_keys(commit_ids) + + projects.flat_map do |project| + find_commits(project, commit_ids[project.id]) + end + end + + def commit_ids_per_project(nodes) + gather_attributes_per_project(nodes, self.class.data_attribute) + end + + def find_commits(project, ids) + commits = [] + + return commits unless project.valid_repo? + + ids.each do |id| + commit = project.commit(id) + + commits << commit if commit + end + + commits + end + end + end +end diff --git a/lib/banzai/reference_parser/commit_range_parser.rb b/lib/banzai/reference_parser/commit_range_parser.rb new file mode 100644 index 00000000000..69d01f8db15 --- /dev/null +++ b/lib/banzai/reference_parser/commit_range_parser.rb @@ -0,0 +1,38 @@ +module Banzai + module ReferenceParser + class CommitRangeParser < BaseParser + self.reference_type = :commit_range + + def referenced_by(nodes) + range_ids = commit_range_ids_per_project(nodes) + projects = find_projects_for_hash_keys(range_ids) + + projects.flat_map do |project| + find_ranges(project, range_ids[project.id]) + end + end + + def commit_range_ids_per_project(nodes) + gather_attributes_per_project(nodes, self.class.data_attribute) + end + + def find_ranges(project, range_ids) + ranges = [] + + range_ids.each do |id| + range = find_object(project, id) + + ranges << range if range + end + + ranges + end + + def find_object(project, id) + range = CommitRange.new(id, project) + + range.valid_commits? ? range : nil + end + end + end +end diff --git a/lib/banzai/reference_parser/external_issue_parser.rb b/lib/banzai/reference_parser/external_issue_parser.rb new file mode 100644 index 00000000000..a1264db2111 --- /dev/null +++ b/lib/banzai/reference_parser/external_issue_parser.rb @@ -0,0 +1,25 @@ +module Banzai + module ReferenceParser + class ExternalIssueParser < BaseParser + self.reference_type = :external_issue + + def referenced_by(nodes) + issue_ids = issue_ids_per_project(nodes) + projects = find_projects_for_hash_keys(issue_ids) + issues = [] + + projects.each do |project| + issue_ids[project.id].each do |id| + issues << ExternalIssue.new(id, project) + end + end + + issues + end + + def issue_ids_per_project(nodes) + gather_attributes_per_project(nodes, self.class.data_attribute) + end + end + end +end diff --git a/lib/banzai/reference_parser/issue_parser.rb b/lib/banzai/reference_parser/issue_parser.rb new file mode 100644 index 00000000000..24076e3d9ec --- /dev/null +++ b/lib/banzai/reference_parser/issue_parser.rb @@ -0,0 +1,40 @@ +module Banzai + module ReferenceParser + class IssueParser < BaseParser + self.reference_type = :issue + + def nodes_visible_to_user(user, nodes) + # It is not possible to check access rights for external issue trackers + return nodes if project && project.external_issue_tracker + + issues = issues_for_nodes(nodes) + + nodes.select do |node| + issue = issue_for_node(issues, node) + + issue ? can?(user, :read_issue, issue) : false + end + end + + def referenced_by(nodes) + issues = issues_for_nodes(nodes) + + nodes.map { |node| issue_for_node(issues, node) }.uniq + end + + def issues_for_nodes(nodes) + @issues_for_nodes ||= grouped_objects_for_nodes( + nodes, + Issue.all.includes(:author, :assignee, :project), + self.class.data_attribute + ) + end + + private + + def issue_for_node(issues, node) + issues[node.attr(self.class.data_attribute).to_i] + end + end + end +end diff --git a/lib/banzai/reference_parser/label_parser.rb b/lib/banzai/reference_parser/label_parser.rb new file mode 100644 index 00000000000..e5d1eb11d7f --- /dev/null +++ b/lib/banzai/reference_parser/label_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class LabelParser < BaseParser + self.reference_type = :label + + def references_relation + Label + end + end + end +end diff --git a/lib/banzai/reference_parser/merge_request_parser.rb b/lib/banzai/reference_parser/merge_request_parser.rb new file mode 100644 index 00000000000..c9a9ca79c09 --- /dev/null +++ b/lib/banzai/reference_parser/merge_request_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class MergeRequestParser < BaseParser + self.reference_type = :merge_request + + def references_relation + MergeRequest.includes(:author, :assignee, :target_project) + end + end + end +end diff --git a/lib/banzai/reference_parser/milestone_parser.rb b/lib/banzai/reference_parser/milestone_parser.rb new file mode 100644 index 00000000000..a000ac61e5c --- /dev/null +++ b/lib/banzai/reference_parser/milestone_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class MilestoneParser < BaseParser + self.reference_type = :milestone + + def references_relation + Milestone + end + end + end +end diff --git a/lib/banzai/reference_parser/snippet_parser.rb b/lib/banzai/reference_parser/snippet_parser.rb new file mode 100644 index 00000000000..fa71b3c952a --- /dev/null +++ b/lib/banzai/reference_parser/snippet_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class SnippetParser < BaseParser + self.reference_type = :snippet + + def references_relation + Snippet + end + end + end +end diff --git a/lib/banzai/reference_parser/user_parser.rb b/lib/banzai/reference_parser/user_parser.rb new file mode 100644 index 00000000000..a12b0d19560 --- /dev/null +++ b/lib/banzai/reference_parser/user_parser.rb @@ -0,0 +1,92 @@ +module Banzai + module ReferenceParser + class UserParser < BaseParser + self.reference_type = :user + + def referenced_by(nodes) + group_ids = [] + user_ids = [] + project_ids = [] + + nodes.each do |node| + if node.has_attribute?('data-group') + group_ids << node.attr('data-group').to_i + elsif node.has_attribute?(self.class.data_attribute) + user_ids << node.attr(self.class.data_attribute).to_i + elsif node.has_attribute?('data-project') + project_ids << node.attr('data-project').to_i + end + end + + find_users_for_groups(group_ids) | find_users(user_ids) | + find_users_for_projects(project_ids) + end + + def nodes_visible_to_user(user, nodes) + group_attr = 'data-group' + groups = lazy { grouped_objects_for_nodes(nodes, Group, group_attr) } + visible = [] + remaining = [] + + nodes.each do |node| + if node.has_attribute?(group_attr) + node_group = groups[node.attr(group_attr).to_i] + + if node_group && + can?(user, :read_group, node_group) + visible << node + end + # Remaining nodes will be processed by the parent class' + # implementation of this method. + else + remaining << node + end + end + + visible + super(current_user, remaining) + end + + def nodes_user_can_reference(current_user, nodes) + project_attr = 'data-project' + author_attr = 'data-author' + + projects = lazy { projects_for_nodes(nodes) } + users = lazy { grouped_objects_for_nodes(nodes, User, author_attr) } + + nodes.select do |node| + project_id = node.attr(project_attr) + user_id = node.attr(author_attr) + + if project && project_id && project.id == project_id.to_i + true + elsif project_id && user_id + project = projects[project_id.to_i] + user = users[user_id.to_i] + + project && user ? project.team.member?(user) : false + else + true + end + end + end + + def find_users(ids) + return [] if ids.empty? + + User.where(id: ids).to_a + end + + def find_users_for_groups(ids) + return [] if ids.empty? + + User.joins(:group_members).where(members: { source_id: ids }).to_a + end + + def find_users_for_projects(ids) + return [] if ids.empty? + + Project.where(id: ids).flat_map { |p| p.team.members.to_a } + end + end + end +end diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index e4b4760c53b..026a5ac97ca 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -265,7 +265,7 @@ module Ci end def validate_job_dependencies!(name, job) - if !validate_array_of_strings(job[:dependencies]) + unless validate_array_of_strings(job[:dependencies]) raise ValidationError, "#{name} job: dependencies parameter should be an array of strings" end diff --git a/lib/event_filter.rb b/lib/event_filter.rb index f15b2cfd231..668d2fa41b3 100644 --- a/lib/event_filter.rb +++ b/lib/event_filter.rb @@ -27,7 +27,7 @@ class EventFilter @params = if params params.dup else - []#EventFilter.default_filter + [] # EventFilter.default_filter end end diff --git a/lib/gitlab/ci/build/artifacts/metadata.rb b/lib/gitlab/ci/build/artifacts/metadata.rb index f2020c82d40..cd2e83b4c27 100644 --- a/lib/gitlab/ci/build/artifacts/metadata.rb +++ b/lib/gitlab/ci/build/artifacts/metadata.rb @@ -56,7 +56,7 @@ module Gitlab child_pattern = '[^/]*/?$' unless @opts[:recursive] match_pattern = /^#{Regexp.escape(@path)}#{child_pattern}/ - until gz.eof? do + until gz.eof? begin path = read_string(gz).force_encoding('UTF-8') meta = read_string(gz).force_encoding('UTF-8') diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index f44d1b3a44e..92c7e8b9d88 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -1,18 +1,22 @@ module Gitlab module CurrentSettings def current_application_settings - key = :current_application_settings - - RequestStore.store[key] ||= begin - settings = nil + if RequestStore.active? + RequestStore.fetch(:current_application_settings) { ensure_application_settings! } + else + ensure_application_settings! + end + end - if connect_to_db? - settings = ::ApplicationSetting.current - settings ||= ::ApplicationSetting.create_from_defaults unless ActiveRecord::Migrator.needs_migration? - end + def ensure_application_settings! + settings = ::ApplicationSetting.cached - settings || fake_application_settings + if !settings && connect_to_db? + settings = ::ApplicationSetting.current + settings ||= ::ApplicationSetting.create_from_defaults unless ActiveRecord::Migrator.needs_migration? end + + settings || fake_application_settings end def fake_application_settings @@ -36,6 +40,7 @@ module Gitlab two_factor_grace_period: 48, akismet_enabled: false, repository_checks_enabled: true, + container_registry_token_expire_delay: 5, ) end diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb index 6fe7faa547a..522dd2b9428 100644 --- a/lib/gitlab/diff/parser.rb +++ b/lib/gitlab/diff/parser.rb @@ -17,16 +17,16 @@ module Gitlab Enumerator.new do |yielder| @lines.each do |line| next if filename?(line) - + full_line = line.delete("\n") - + if line.match(/^@@ -/) type = "match" - + line_old = line.match(/\-[0-9]*/)[0].to_i.abs rescue 0 line_new = line.match(/\+[0-9]*/)[0].to_i.abs rescue 0 - - next if line_old <= 1 && line_new <= 1 #top of file + + next if line_old <= 1 && line_new <= 1 # top of file yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) line_obj_index += 1 next @@ -39,8 +39,8 @@ module Gitlab yielder << Gitlab::Diff::Line.new(full_line, type, line_obj_index, line_old, line_new) line_obj_index += 1 end - - + + case line[0] when "+" line_new += 1 diff --git a/lib/gitlab/github_import/pull_request_formatter.rb b/lib/gitlab/github_import/pull_request_formatter.rb index 574737b31c1..a2947b56ad9 100644 --- a/lib/gitlab/github_import/pull_request_formatter.rb +++ b/lib/gitlab/github_import/pull_request_formatter.rb @@ -79,10 +79,9 @@ module Gitlab end def state - @state ||= case true - when raw_data.state == 'closed' && raw_data.merged_at.present? + @state ||= if raw_data.state == 'closed' && raw_data.merged_at.present? 'merged' - when raw_data.state == 'closed' + elsif raw_data.state == 'closed' 'closed' else 'opened' diff --git a/lib/gitlab/gitlab_import/importer.rb b/lib/gitlab/gitlab_import/importer.rb index e32ef8a44c2..3f76ec97977 100644 --- a/lib/gitlab/gitlab_import/importer.rb +++ b/lib/gitlab/gitlab_import/importer.rb @@ -17,7 +17,7 @@ module Gitlab def execute project_identifier = CGI.escape(project.import_source) - #Issues && Comments + # Issues && Comments issues = client.issues(project_identifier) issues.each do |issue| diff --git a/lib/gitlab/lazy.rb b/lib/gitlab/lazy.rb new file mode 100644 index 00000000000..2a659ae4c74 --- /dev/null +++ b/lib/gitlab/lazy.rb @@ -0,0 +1,34 @@ +module Gitlab + # A class that can be wrapped around an expensive method call so it's only + # executed when actually needed. + # + # Usage: + # + # object = Gitlab::Lazy.new { some_expensive_work_here } + # + # object['foo'] + # object.bar + class Lazy < BasicObject + def initialize(&block) + @block = block + end + + def method_missing(name, *args, &block) + __evaluate__ + + @result.__send__(name, *args, &block) + end + + def respond_to_missing?(name, include_private = false) + __evaluate__ + + @result.respond_to?(name, include_private) || super + end + + private + + def __evaluate__ + @result = @block.call unless defined?(@result) + end + end +end diff --git a/lib/gitlab/middleware/go.rb b/lib/gitlab/middleware/go.rb index 50b0dd32380..5764ab15652 100644 --- a/lib/gitlab/middleware/go.rb +++ b/lib/gitlab/middleware/go.rb @@ -39,7 +39,7 @@ module Gitlab request_url = URI.join(base_url, project_path) domain_path = strip_url(request_url.to_s) - "<!DOCTYPE html><html><head><meta content='#{domain_path} git #{request_url}.git' name='go-import'></head></html>\n"; + "<!DOCTYPE html><html><head><meta content='#{domain_path} git #{request_url}.git' name='go-import'></head></html>\n" end def strip_url(url) diff --git a/lib/gitlab/project_search_results.rb b/lib/gitlab/project_search_results.rb index 71c5b6801fb..183bd10d6a3 100644 --- a/lib/gitlab/project_search_results.rb +++ b/lib/gitlab/project_search_results.rb @@ -74,7 +74,7 @@ module Gitlab end def notes - project.notes.user.search(query).order('updated_at DESC') + project.notes.user.search(query, as_user: @current_user).order('updated_at DESC') end def commits diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 13c4d64c99b..11c0b01f0dc 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -4,10 +4,9 @@ module Gitlab REFERABLES = %i(user issue label milestone merge_request snippet commit commit_range) attr_accessor :project, :current_user, :author - def initialize(project, current_user = nil, author = nil) + def initialize(project, current_user = nil) @project = project @current_user = current_user - @author = author @references = {} @@ -18,17 +17,21 @@ module Gitlab super(text, context.merge(project: project)) end + def references(type) + super(type, project, current_user) + end + REFERABLES.each do |type| define_method("#{type}s") do - @references[type] ||= references(type, reference_context) + @references[type] ||= references(type) end end def issues if project && project.jira_tracker? - @references[:external_issue] ||= references(:external_issue, reference_context) + @references[:external_issue] ||= references(:external_issue) else - @references[:issue] ||= references(:issue, reference_context) + @references[:issue] ||= references(:issue) end end @@ -46,11 +49,5 @@ module Gitlab @pattern = Regexp.union(patterns.compact) end - - private - - def reference_context - { project: project, current_user: current_user, author: author } - end end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index c0a1f45195f..4f621a43d7e 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -63,7 +63,7 @@ describe Projects::MergeRequestsController do id: merge_request.iid, format: format) - expect(response.body).to eq((merge_request.send(:"to_#{format}")).to_s) + expect(response.body).to eq(merge_request.send(:"to_#{format}").to_s) end it "should not escape Html" do diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 91b46c4d65c..fba545560c7 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -115,6 +115,17 @@ describe ProjectsController do expect(public_project_with_dot_atom).not_to be_valid end end + + context 'when the project is pending deletions' do + it 'renders a 404 error' do + project = create(:project, pending_delete: true) + sign_in(user) + + get :show, namespace_id: project.namespace.path, id: project.path + + expect(response.status).to eq 404 + end + end end describe "#update" do diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index fbe50d10ec5..209fa37d97d 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -11,7 +11,7 @@ describe RegistrationsController do let(:user_params) { { user: { name: "new_user", username: "new_username", email: "new@user.com", password: "Any_password" } } } context 'when sending email confirmation' do - before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(false) } + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(false) } it 'logs user in directly' do post(:create, user_params) @@ -21,7 +21,7 @@ describe RegistrationsController do end context 'when not sending email confirmation' do - before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) } + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } it 'does not authenticate user and sends confirmation email' do post(:create, user_params) diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index ab57c52c7cd..5dc8724fb50 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -12,7 +12,7 @@ describe SessionsController do post(:create, user: { login: 'invalid', password: 'invalid' }) expect(response) - .to set_flash.now[:alert].to /Invalid login or password/ + .to set_flash.now[:alert].to /Invalid Login or password/ end end @@ -35,6 +35,27 @@ describe SessionsController do post(:create, { user: user_params }, { otp_user_id: user.id }) end + context 'remember_me field' do + it 'sets a remember_user_token cookie when enabled' do + allow(controller).to receive(:find_user).and_return(user) + expect(controller). + to receive(:remember_me).with(user).and_call_original + + authenticate_2fa(remember_me: '1', otp_attempt: user.current_otp) + + expect(response.cookies['remember_user_token']).to be_present + end + + it 'does nothing when disabled' do + allow(controller).to receive(:find_user).and_return(user) + expect(controller).not_to receive(:remember_me) + + authenticate_2fa(remember_me: '0', otp_attempt: user.current_otp) + + expect(response.cookies['remember_user_token']).to be_nil + end + end + ## # See #14900 issue # diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 26719f2652c..c32e205ee69 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -7,6 +7,7 @@ FactoryGirl.define do project note "Note" author + on_issue factory :note_on_commit, traits: [:on_commit] factory :note_on_commit_diff, traits: [:on_commit, :on_diff], class: LegacyDiffNote @@ -19,29 +20,26 @@ FactoryGirl.define do factory :upvote_note, traits: [:award, :upvote] trait :on_commit do - project + noteable nil + noteable_id nil + noteable_type 'Commit' commit_id RepoHelpers.sample_commit.id - noteable_type "Commit" end trait :on_diff do line_code "0_184_184" end - trait :on_merge_request do - project - noteable_id 1 - noteable_type "MergeRequest" + trait :on_issue do + noteable { create(:issue, project: project) } end - trait :on_issue do - noteable_id 1 - noteable_type "Issue" + trait :on_merge_request do + noteable { create(:merge_request, source_project: project) } end trait :on_project_snippet do - noteable_id 1 - noteable_type "Snippet" + noteable { create(:snippet, project: project) } end trait :system do diff --git a/spec/features/issues/note_polling_spec.rb b/spec/features/issues/note_polling_spec.rb index e4efdbe2421..f5cfe2d666e 100644 --- a/spec/features/issues/note_polling_spec.rb +++ b/spec/features/issues/note_polling_spec.rb @@ -9,8 +9,11 @@ feature 'Issue notes polling' do end scenario 'Another user adds a comment to an issue', js: true do - note = create(:note_on_issue, noteable: issue, note: 'Looks good!') + note = create(:note, noteable: issue, project: project, + note: 'Looks good!') + page.execute_script('notes.refresh();') + expect(page).to have_selector("#note_#{note.id}", text: 'Looks good!') end end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 749ee01890c..9271964166a 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -125,7 +125,7 @@ describe 'Issues', feature: true do describe 'Issue info' do it 'excludes award_emoji from comment count' do issue = create(:issue, author: @user, assignee: @user, project: project, title: 'foobar') - create(:upvote_note, noteable: issue) + create(:upvote_note, noteable: issue, project: project) visit namespace_project_issues_path(project.namespace, project, assignee_id: @user.id) diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index 8c38dd5b122..c1b178c3b6c 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -32,7 +32,7 @@ feature 'Login', feature: true do let(:user) { create(:user, :two_factor) } before do - login_with(user) + login_with(user, remember: true) expect(page).to have_content('Two-factor Authentication') end @@ -52,6 +52,12 @@ feature 'Login', feature: true do expect(current_path).to eq root_path end + it 'persists remember_me value via hidden field' do + field = first('input#user_remember_me', visible: false) + + expect(field.value).to eq '1' + end + it 'blocks login with invalid code' do enter_code('foo') expect(page).to have_content('Invalid two-factor code') @@ -121,7 +127,7 @@ feature 'Login', feature: true do user = create(:user, password: 'not-the-default') login_with(user) - expect(page).to have_content('Invalid login or password.') + expect(page).to have_content('Invalid Login or password.') end end diff --git a/spec/features/merge_requests/created_from_fork_spec.rb b/spec/features/merge_requests/created_from_fork_spec.rb new file mode 100644 index 00000000000..edc0bdec3db --- /dev/null +++ b/spec/features/merge_requests/created_from_fork_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +feature 'Merge request created from fork' do + given(:user) { create(:user) } + given(:project) { create(:project, :public) } + given(:fork_project) { create(:project, :public) } + + given!(:merge_request) do + create(:forked_project_link, forked_to_project: fork_project, + forked_from_project: project) + + create(:merge_request_with_diffs, source_project: fork_project, + target_project: project, + description: 'Test merge request') + end + + background do + fork_project.team << [user, :master] + login_as user + end + + scenario 'user can access merge request' do + visit_merge_request(merge_request) + + expect(page).to have_content 'Test merge request' + end + + context 'pipeline present in source project' do + include WaitForAjax + + given(:pipeline) do + create(:ci_commit_with_two_jobs, project: fork_project, + sha: merge_request.last_commit.id, + ref: merge_request.source_branch) + end + + background { pipeline.create_builds(user) } + + scenario 'user visits a pipelines page', js: true do + visit_merge_request(merge_request) + page.within('.merge-request-tabs') { click_link 'Builds' } + wait_for_ajax + + page.within('table.builds') do + expect(page).to have_content 'rspec' + expect(page).to have_content 'spinach' + end + + expect(find_link('Cancel running')[:href]) + .to include fork_project.path_with_namespace + end + end + + def visit_merge_request(mr) + visit namespace_project_merge_request_path(project.namespace, + project, mr) + end +end diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb index 2c7e1c748ad..1c130057c56 100644 --- a/spec/features/merge_requests/user_lists_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb @@ -131,6 +131,15 @@ describe 'Projects > Merge requests > User lists merge requests', feature: true expect(first_merge_request).to include('fix') expect(count_merge_requests).to eq(1) end + + it 'sorts by recently due milestone' do + visit namespace_project_merge_requests_path(project.namespace, project, + label_name: [label.name, label2.name], + assignee_id: user.id, + sort: sort_value_milestone_soon) + + expect(first_merge_request).to include('fix') + end end end diff --git a/spec/features/notes_on_merge_requests_spec.rb b/spec/features/notes_on_merge_requests_spec.rb index 9e9fec01943..2835cf44494 100644 --- a/spec/features/notes_on_merge_requests_spec.rb +++ b/spec/features/notes_on_merge_requests_spec.rb @@ -19,10 +19,14 @@ describe 'Comments', feature: true do end describe 'On a merge request', js: true, feature: true do - let!(:merge_request) { create(:merge_request) } - let!(:project) { merge_request.source_project } + let!(:project) { create(:project) } + let!(:merge_request) do + create(:merge_request, source_project: project, target_project: project) + end + let!(:note) do - create(:note_on_merge_request, :with_attachment, project: project) + create(:note_on_merge_request, :with_attachment, noteable: merge_request, + project: project) end before do diff --git a/spec/features/participants_autocomplete_spec.rb b/spec/features/participants_autocomplete_spec.rb index 1adab7e9c6c..c7c00a3266a 100644 --- a/spec/features/participants_autocomplete_spec.rb +++ b/spec/features/participants_autocomplete_spec.rb @@ -32,7 +32,8 @@ feature 'Member autocomplete', feature: true do context 'adding a new note on a Issue', js: true do before do issue = create(:issue, author: author, project: project) - create(:note, note: 'Ultralight Beam', noteable: issue, author: participant) + create(:note, note: 'Ultralight Beam', noteable: issue, + project: project, author: participant) visit_issue(project, issue) end @@ -47,7 +48,8 @@ feature 'Member autocomplete', feature: true do context 'adding a new note on a Merge Request ', js: true do before do merge = create(:merge_request, source_project: project, target_project: project, author: author) - create(:note, note: 'Ultralight Beam', noteable: merge, author: participant) + create(:note, note: 'Ultralight Beam', noteable: merge, + project: project, author: participant) visit_merge_request(project, merge) end diff --git a/spec/features/pipelines_spec.rb b/spec/features/pipelines_spec.rb index bef0578a9bb..acd6fb3538c 100644 --- a/spec/features/pipelines_spec.rb +++ b/spec/features/pipelines_spec.rb @@ -62,6 +62,36 @@ describe "Pipelines" do end end + context 'for generic statuses' do + context 'when running' do + let!(:running) { create(:generic_commit_status, status: 'running', commit: pipeline, stage: 'test') } + + before { visit namespace_project_pipelines_path(project.namespace, project) } + + it 'not be cancelable' do + expect(page).not_to have_link('Cancel') + end + + it 'pipeline is running' do + expect(page).to have_selector('.ci-running') + end + end + + context 'when failed' do + let!(:running) { create(:generic_commit_status, status: 'failed', commit: pipeline, stage: 'test') } + + before { visit namespace_project_pipelines_path(project.namespace, project) } + + it 'not be retryable' do + expect(page).not_to have_link('Retry') + end + + it 'pipeline is failed' do + expect(page).to have_selector('.ci-failed') + end + end + end + context 'downloadable pipelines' do context 'with artifacts' do let!(:with_artifacts) { create(:ci_build, :artifacts, :success, commit: pipeline, name: 'rspec tests', stage: 'test') } diff --git a/spec/features/task_lists_spec.rb b/spec/features/task_lists_spec.rb index b7368cca29d..6ed279ef9be 100644 --- a/spec/features/task_lists_spec.rb +++ b/spec/features/task_lists_spec.rb @@ -75,7 +75,10 @@ feature 'Task Lists', feature: true do describe 'for Notes' do let!(:issue) { create(:issue, author: user, project: project) } - let!(:note) { create(:note, note: markdown, noteable: issue, author: user) } + let!(:note) do + create(:note, note: markdown, noteable: issue, + project: project, author: user) + end it 'renders for note body' do visit_issue(project, issue) diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index 3354f529295..4e627753cc7 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -43,6 +43,27 @@ describe 'Dashboard Todos', feature: true do end end + context 'User has Todos with labels spanning multiple projects' do + before do + label1 = create(:label, project: project) + note1 = create(:note_on_issue, note: "Hello #{label1.to_reference(format: :name)}", noteable_id: issue.id, noteable_type: 'Issue', project: issue.project) + create(:todo, :mentioned, project: project, target: issue, user: user, note_id: note1.id) + + project2 = create(:project) + label2 = create(:label, project: project2) + issue2 = create(:issue, project: project2) + note2 = create(:note_on_issue, note: "Test #{label2.to_reference(format: :name)}", noteable_id: issue2.id, noteable_type: 'Issue', project: project2) + create(:todo, :mentioned, project: project2, target: issue2, user: user, note_id: note2.id) + + login_as(user) + visit dashboard_todos_path + end + + it 'shows page with two Todos' do + expect(page).to have_selector('.todos-list .todo', count: 2) + end + end + context 'User has multiple pages of Todos' do before do allow(Todo).to receive(:default_per_page).and_return(1) diff --git a/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb index c2a8ad36c30..593bd6d5cac 100644 --- a/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb @@ -98,11 +98,6 @@ describe Banzai::Filter::CommitRangeReferenceFilter, lib: true do expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_compare_url(project.namespace, project, from: commit1.id, to: commit2.id, only_path: true) end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit_range]).not_to be_empty - end end context 'cross-project reference' do @@ -135,11 +130,6 @@ describe Banzai::Filter::CommitRangeReferenceFilter, lib: true do exp = act = "Fixed #{project2.to_reference}@#{commit1.id}...#{commit2.id.reverse}" expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit_range]).not_to be_empty - end end context 'cross-project URL reference' do @@ -173,10 +163,5 @@ describe Banzai::Filter::CommitRangeReferenceFilter, lib: true do exp = act = "Fixed #{project2.to_reference}@#{commit1.id}...#{commit2.id.reverse}" expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit_range]).not_to be_empty - end end end diff --git a/spec/lib/banzai/filter/commit_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_reference_filter_spec.rb index 63a32d9d455..d46d3f1489e 100644 --- a/spec/lib/banzai/filter/commit_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_reference_filter_spec.rb @@ -93,11 +93,6 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_commit_url(project.namespace, project, reference, only_path: true) end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit]).not_to be_empty - end end context 'cross-project reference' do @@ -124,11 +119,6 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do exp = act = "Committed #{invalidate_reference(reference)}" expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit]).not_to be_empty - end end context 'cross-project URL reference' do @@ -154,10 +144,5 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do act = "Committed #{invalidate_reference(reference)}" expect(reference_filter(act).to_html).to match(/<a.+>#{Regexp.escape(invalidate_reference(reference))}<\/a>/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit]).not_to be_empty - end end end diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb index 266ebef33d6..8e6a264970d 100644 --- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb @@ -91,11 +91,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do expect(link).to eq helper.url_for_issue(issue.iid, project, only_path: true) end - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end - it 'does not process links containing issue numbers followed by text' do href = "#{reference}st" doc = reference_filter("<a href='#{href}'></a>") @@ -136,11 +131,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end context 'cross-project URL reference' do @@ -160,11 +150,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do doc = reference_filter("Fixed (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(issue.to_reference(project))} \(comment 123\)<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end context 'cross-project reference in link href' do @@ -184,11 +169,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do doc = reference_filter("Fixed (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>Reference<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end context 'cross-project URL in link href' do @@ -208,10 +188,5 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do doc = reference_filter("Fixed (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>Reference<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end end diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb index b0a38e7c251..f1064a701d8 100644 --- a/spec/lib/banzai/filter/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb @@ -48,11 +48,6 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do expect(link).to eq urls.namespace_project_issues_path(project.namespace, project, label_name: label.name) end - it 'adds to the results hash' do - result = reference_pipeline_result("Label #{reference}") - expect(result[:references][:label]).to eq [label] - end - describe 'label span element' do it 'includes default classes' do doc = reference_filter("Label #{reference}") @@ -170,11 +165,6 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do expect(link).to have_attribute('data-label') expect(link.attr('data-label')).to eq label.id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Label #{reference}") - expect(result[:references][:label]).to eq [label] - end end describe 'cross project label references' do diff --git a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb index 352710df307..3185e41fe5c 100644 --- a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb @@ -78,11 +78,6 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_merge_request_url(project.namespace, project, merge, only_path: true) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Merge #{reference}") - expect(result[:references][:merge_request]).to eq [merge] - end end context 'cross-project reference' do @@ -109,11 +104,6 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("Merge #{reference}") - expect(result[:references][:merge_request]).to eq [merge] - end end context 'cross-project URL reference' do @@ -133,10 +123,5 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do doc = reference_filter("Merge (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(merge.to_reference(project))} \(diffs, comment 123\)<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Merge #{reference}") - expect(result[:references][:merge_request]).to eq [merge] - end end end diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index bdf48eabb0e..9424f2363e1 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -48,11 +48,6 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do namespace_project_milestone_path(project.namespace, project, milestone) end - it 'adds to the results hash' do - result = reference_pipeline_result("Milestone #{reference}") - expect(result[:references][:milestone]).to eq [milestone] - end - context 'Integer-based references' do it 'links to a valid reference' do doc = reference_filter("See #{reference}") @@ -151,11 +146,6 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do expect(link).to have_attribute('data-milestone') expect(link.attr('data-milestone')).to eq milestone.id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Milestone #{reference}") - expect(result[:references][:milestone]).to eq [milestone] - end end describe 'cross project milestone references' do diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb index c2c2fd0eb6a..697d10bbf70 100644 --- a/spec/lib/banzai/filter/redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/redactor_filter_spec.rb @@ -16,11 +16,23 @@ describe Banzai::Filter::RedactorFilter, lib: true do end context 'with data-project' do + let(:parser_class) do + Class.new(Banzai::ReferenceParser::BaseParser) do + self.reference_type = :test + end + end + + before do + allow(Banzai::ReferenceParser).to receive(:[]). + with('test'). + and_return(parser_class) + end + it 'removes unpermitted Project references' do user = create(:user) project = create(:empty_project) - link = reference_link(project: project.id, reference_filter: 'ReferenceFilter') + link = reference_link(project: project.id, reference_type: 'test') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 @@ -31,14 +43,14 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project) project.team << [user, :master] - link = reference_link(project: project.id, reference_filter: 'ReferenceFilter') + link = reference_link(project: project.id, reference_type: 'test') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 end it 'handles invalid Project references' do - link = reference_link(project: 12345, reference_filter: 'ReferenceFilter') + link = reference_link(project: 12345, reference_type: 'test') expect { filter(link) }.not_to raise_error end @@ -51,7 +63,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: non_member) expect(doc.css('a').length).to eq 0 @@ -62,7 +74,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project, author: author) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: author) expect(doc.css('a').length).to eq 1 @@ -73,7 +85,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project, assignee: assignee) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: assignee) expect(doc.css('a').length).to eq 1 @@ -85,7 +97,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project.team << [member, :developer] issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: member) expect(doc.css('a').length).to eq 1 @@ -96,7 +108,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: admin) expect(doc.css('a').length).to eq 1 @@ -108,7 +120,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 @@ -121,7 +133,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do user = create(:user) group = create(:group, :private) - link = reference_link(group: group.id, reference_filter: 'UserReferenceFilter') + link = reference_link(group: group.id, reference_type: 'user') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 @@ -132,14 +144,14 @@ describe Banzai::Filter::RedactorFilter, lib: true do group = create(:group, :private) group.add_developer(user) - link = reference_link(group: group.id, reference_filter: 'UserReferenceFilter') + link = reference_link(group: group.id, reference_type: 'user') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 end it 'handles invalid Group references' do - link = reference_link(group: 12345, reference_filter: 'UserReferenceFilter') + link = reference_link(group: 12345, reference_type: 'user') expect { filter(link) }.not_to raise_error end @@ -149,7 +161,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do it 'allows any User reference' do user = create(:user) - link = reference_link(user: user.id, reference_filter: 'UserReferenceFilter') + link = reference_link(user: user.id, reference_type: 'user') doc = filter(link) expect(doc.css('a').length).to eq 1 diff --git a/spec/lib/banzai/filter/reference_gatherer_filter_spec.rb b/spec/lib/banzai/filter/reference_gatherer_filter_spec.rb deleted file mode 100644 index c8b1dfdf944..00000000000 --- a/spec/lib/banzai/filter/reference_gatherer_filter_spec.rb +++ /dev/null @@ -1,87 +0,0 @@ -require 'spec_helper' - -describe Banzai::Filter::ReferenceGathererFilter, lib: true do - include ActionView::Helpers::UrlHelper - include FilterSpecHelper - - def reference_link(data) - link_to('text', '', class: 'gfm', data: data) - end - - context "for issue references" do - - context 'with data-project' do - it 'removes unpermitted Project references' do - user = create(:user) - project = create(:empty_project) - issue = create(:issue, project: project) - - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') - result = pipeline_result(link, current_user: user) - - expect(result[:references][:issue]).to be_empty - end - - it 'allows permitted Project references' do - user = create(:user) - project = create(:empty_project) - issue = create(:issue, project: project) - project.team << [user, :master] - - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') - result = pipeline_result(link, current_user: user) - - expect(result[:references][:issue]).to eq([issue]) - end - - it 'handles invalid Project references' do - link = reference_link(project: 12345, issue: 12345, reference_filter: 'IssueReferenceFilter') - - expect { pipeline_result(link) }.not_to raise_error - end - end - end - - context "for user references" do - - context 'with data-group' do - it 'removes unpermitted Group references' do - user = create(:user) - group = create(:group) - - link = reference_link(group: group.id, reference_filter: 'UserReferenceFilter') - result = pipeline_result(link, current_user: user) - - expect(result[:references][:user]).to be_empty - end - - it 'allows permitted Group references' do - user = create(:user) - group = create(:group) - group.add_developer(user) - - link = reference_link(group: group.id, reference_filter: 'UserReferenceFilter') - result = pipeline_result(link, current_user: user) - - expect(result[:references][:user]).to eq([user]) - end - - it 'handles invalid Group references' do - link = reference_link(group: 12345, reference_filter: 'UserReferenceFilter') - - expect { pipeline_result(link) }.not_to raise_error - end - end - - context 'with data-user' do - it 'allows any User reference' do - user = create(:user) - - link = reference_link(user: user.id, reference_filter: 'UserReferenceFilter') - result = pipeline_result(link) - - expect(result[:references][:user]).to eq([user]) - end - end - end -end diff --git a/spec/lib/banzai/filter/snippet_reference_filter_spec.rb b/spec/lib/banzai/filter/snippet_reference_filter_spec.rb index 26466fbb180..5068ddd7faa 100644 --- a/spec/lib/banzai/filter/snippet_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/snippet_reference_filter_spec.rb @@ -77,11 +77,6 @@ describe Banzai::Filter::SnippetReferenceFilter, lib: true do expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_snippet_url(project.namespace, project, snippet, only_path: true) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Snippet #{reference}") - expect(result[:references][:snippet]).to eq [snippet] - end end context 'cross-project reference' do @@ -107,11 +102,6 @@ describe Banzai::Filter::SnippetReferenceFilter, lib: true do expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("Snippet #{reference}") - expect(result[:references][:snippet]).to eq [snippet] - end end context 'cross-project URL reference' do @@ -137,10 +127,5 @@ describe Banzai::Filter::SnippetReferenceFilter, lib: true do expect(reference_filter(act).to_html).to match(/<a.+>#{Regexp.escape(invalidate_reference(reference))}<\/a>/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Snippet #{reference}") - expect(result[:references][:snippet]).to eq [snippet] - end end end diff --git a/spec/lib/banzai/filter/user_reference_filter_spec.rb b/spec/lib/banzai/filter/user_reference_filter_spec.rb index 8bdebae1841..d7dfd6699ef 100644 --- a/spec/lib/banzai/filter/user_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/user_reference_filter_spec.rb @@ -31,28 +31,22 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do end it 'supports a special @all mention' do - doc = reference_filter("Hey #{reference}") + doc = reference_filter("Hey #{reference}", author: user) expect(doc.css('a').length).to eq 1 expect(doc.css('a').first.attr('href')) .to eq urls.namespace_project_url(project.namespace, project) end - context "when the author is a member of the project" do + it 'includes a data-author attribute when there is an author' do + doc = reference_filter(reference, author: user) - it 'adds to the results hash' do - result = reference_pipeline_result("Hey #{reference}", author: project.creator) - expect(result[:references][:user]).to eq [project.creator] - end + expect(doc.css('a').first.attr('data-author')).to eq(user.id.to_s) end - context "when the author is not a member of the project" do - - let(:other_user) { create(:user) } + it 'does not include a data-author attribute when there is no author' do + doc = reference_filter(reference) - it "doesn't add to the results hash" do - result = reference_pipeline_result("Hey #{reference}", author: other_user) - expect(result[:references][:user]).to eq [] - end + expect(doc.css('a').first.has_attribute?('data-author')).to eq(false) end end @@ -83,11 +77,6 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do expect(link).to have_attribute('data-user') expect(link.attr('data-user')).to eq user.namespace.owner_id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Hey #{reference}") - expect(result[:references][:user]).to eq [user] - end end context 'mentioning a group' do @@ -106,11 +95,6 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do expect(link).to have_attribute('data-group') expect(link.attr('data-group')).to eq group.id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Hey #{reference}") - expect(result[:references][:user]).to eq group.users - end end it 'links with adjacent text' do @@ -151,10 +135,5 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do expect(link).to have_attribute('data-user') expect(link.attr('data-user')).to eq user.namespace.owner_id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Hey #{reference}") - expect(result[:references][:user]).to eq [user] - end end end diff --git a/spec/lib/banzai/reference_parser/base_parser_spec.rb b/spec/lib/banzai/reference_parser/base_parser_spec.rb new file mode 100644 index 00000000000..543b4786d84 --- /dev/null +++ b/spec/lib/banzai/reference_parser/base_parser_spec.rb @@ -0,0 +1,237 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::BaseParser, lib: true do + include ReferenceParserHelpers + + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public) } + + subject do + klass = Class.new(described_class) do + self.reference_type = :foo + end + + klass.new(project, user) + end + + describe '.reference_type=' do + it 'sets the reference type' do + dummy = Class.new(described_class) + dummy.reference_type = :foo + + expect(dummy.reference_type).to eq(:foo) + end + end + + describe '#nodes_visible_to_user' do + let(:link) { empty_html_link } + + context 'when the link has a data-project attribute' do + it 'returns the nodes if the attribute value equals the current project ID' do + link['data-project'] = project.id.to_s + + expect(Ability.abilities).not_to receive(:allowed?) + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns the nodes if the user can read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array when the attribute value is empty' do + link['data-project'] = '' + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + + it 'returns an empty Array when the user can not read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns the nodes' do + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + end + + describe '#nodes_user_can_reference' do + it 'returns the nodes' do + link = double(:link) + + expect(subject.nodes_user_can_reference(user, [link])).to eq([link]) + end + end + + describe '#referenced_by' do + context 'when references_relation is implemented' do + it 'returns a collection of objects' do + links = Nokogiri::HTML.fragment("<a data-foo='#{user.id}'></a>"). + children + + expect(subject).to receive(:references_relation).and_return(User) + expect(subject.referenced_by(links)).to eq([user]) + end + end + + context 'when references_relation is not implemented' do + it 'raises NotImplementedError' do + links = Nokogiri::HTML.fragment('<a data-foo="1"></a>').children + + expect { subject.referenced_by(links) }. + to raise_error(NotImplementedError) + end + end + end + + describe '#references_relation' do + it 'raises NotImplementedError' do + expect { subject.references_relation }.to raise_error(NotImplementedError) + end + end + + describe '#gather_attributes_per_project' do + it 'returns a Hash containing attribute values per project' do + link = Nokogiri::HTML.fragment('<a data-project="1" data-foo="2"></a>'). + children[0] + + hash = subject.gather_attributes_per_project([link], 'data-foo') + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[1].to_a).to eq(['2']) + end + end + + describe '#grouped_objects_for_nodes' do + it 'returns a Hash grouping objects per ID' do + nodes = [double(:node)] + + expect(subject).to receive(:unique_attribute_values). + with(nodes, 'data-user'). + and_return([user.id]) + + hash = subject.grouped_objects_for_nodes(nodes, User, 'data-user') + + expect(hash).to eq({ user.id => user }) + end + + it 'returns an empty Hash when the list of nodes is empty' do + expect(subject.grouped_objects_for_nodes([], User, 'data-user')).to eq({}) + end + end + + describe '#unique_attribute_values' do + it 'returns an Array of unique values' do + link = double(:link) + + expect(link).to receive(:has_attribute?). + with('data-foo'). + twice. + and_return(true) + + expect(link).to receive(:attr). + with('data-foo'). + twice. + and_return('1') + + nodes = [link, link] + + expect(subject.unique_attribute_values(nodes, 'data-foo')).to eq(['1']) + end + end + + describe '#process' do + it 'gathers the references for every node matching the reference type' do + dummy = Class.new(described_class) do + self.reference_type = :test + end + + instance = dummy.new(project, user) + document = Nokogiri::HTML.fragment('<a class="gfm"></a><a class="gfm" data-reference-type="test"></a>') + + expect(instance).to receive(:gather_references). + with([document.children[1]]). + and_return([user]) + + expect(instance.process([document])).to eq([user]) + end + end + + describe '#gather_references' do + let(:link) { double(:link) } + + it 'does not process links a user can not reference' do + expect(subject).to receive(:nodes_user_can_reference). + with(user, [link]). + and_return([]) + + expect(subject).to receive(:referenced_by).with([]) + + subject.gather_references([link]) + end + + it 'does not process links a user can not see' do + expect(subject).to receive(:nodes_user_can_reference). + with(user, [link]). + and_return([link]) + + expect(subject).to receive(:nodes_visible_to_user). + with(user, [link]). + and_return([]) + + expect(subject).to receive(:referenced_by).with([]) + + subject.gather_references([link]) + end + + it 'returns the references if a user can reference and see a link' do + expect(subject).to receive(:nodes_user_can_reference). + with(user, [link]). + and_return([link]) + + expect(subject).to receive(:nodes_visible_to_user). + with(user, [link]). + and_return([link]) + + expect(subject).to receive(:referenced_by).with([link]) + + subject.gather_references([link]) + end + end + + describe '#can?' do + it 'delegates the permissions check to the Ability class' do + user = double(:user) + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, project) + + subject.can?(user, :read_project, project) + end + end + + describe '#find_projects_for_hash_keys' do + it 'returns a list of Projects' do + expect(subject.find_projects_for_hash_keys(project.id => project)). + to eq([project]) + end + end +end diff --git a/spec/lib/banzai/reference_parser/commit_parser_spec.rb b/spec/lib/banzai/reference_parser/commit_parser_spec.rb new file mode 100644 index 00000000000..0b76d29fce0 --- /dev/null +++ b/spec/lib/banzai/reference_parser/commit_parser_spec.rb @@ -0,0 +1,113 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::CommitParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-project attribute' do + before do + link['data-project'] = project.id.to_s + end + + context 'when the link has a data-commit attribute' do + before do + link['data-commit'] = '123' + end + + it 'returns an Array of commits' do + commit = double(:commit) + + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject).to receive(:find_commits). + with(project, ['123']). + and_return([commit]) + + expect(subject.referenced_by([link])).to eq([commit]) + end + + it 'returns an empty Array when the commit could not be found' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject).to receive(:find_commits). + with(project, ['123']). + and_return([]) + + expect(subject.referenced_by([link])).to eq([]) + end + + it 'skips projects without valid repositories' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(false) + + expect(subject.referenced_by([link])).to eq([]) + end + end + + context 'when the link does not have a data-commit attribute' do + it 'returns an empty Array' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns an empty Array' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + describe '#commit_ids_per_project' do + before do + link['data-project'] = project.id.to_s + end + + it 'returns a Hash containing commit IDs per project' do + link['data-commit'] = '123' + + hash = subject.commit_ids_per_project([link]) + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[project.id].to_a).to eq(['123']) + end + + it 'does not add a project when the data-commit attribute is empty' do + hash = subject.commit_ids_per_project([link]) + + expect(hash).to be_empty + end + end + + describe '#find_commits' do + it 'returns an Array of commit objects' do + commit = double(:commit) + + expect(project).to receive(:commit).with('123').and_return(commit) + expect(project).to receive(:valid_repo?).and_return(true) + + expect(subject.find_commits(project, %w{123})).to eq([commit]) + end + + it 'skips commit IDs for which no commit could be found' do + expect(project).to receive(:commit).with('123').and_return(nil) + expect(project).to receive(:valid_repo?).and_return(true) + + expect(subject.find_commits(project, %w{123})).to eq([]) + end + end +end diff --git a/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb b/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb new file mode 100644 index 00000000000..ba982f38542 --- /dev/null +++ b/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb @@ -0,0 +1,120 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::CommitRangeParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-project attribute' do + before do + link['data-project'] = project.id.to_s + end + + context 'when the link as a data-commit-range attribute' do + before do + link['data-commit-range'] = '123..456' + end + + it 'returns an Array of commit ranges' do + range = double(:range) + + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(range) + + expect(subject.referenced_by([link])).to eq([range]) + end + + it 'returns an empty Array when the commit range could not be found' do + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(nil) + + expect(subject.referenced_by([link])).to eq([]) + end + end + + context 'when the link does not have a data-commit-range attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + describe '#commit_range_ids_per_project' do + before do + link['data-project'] = project.id.to_s + end + + it 'returns a Hash containing range IDs per project' do + link['data-commit-range'] = '123..456' + + hash = subject.commit_range_ids_per_project([link]) + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[project.id].to_a).to eq(['123..456']) + end + + it 'does not add a project when the data-commit-range attribute is empty' do + hash = subject.commit_range_ids_per_project([link]) + + expect(hash).to be_empty + end + end + + describe '#find_ranges' do + it 'returns an Array of range objects' do + range = double(:commit) + + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(range) + + expect(subject.find_ranges(project, ['123..456'])).to eq([range]) + end + + it 'skips ranges that could not be found' do + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(nil) + + expect(subject.find_ranges(project, ['123..456'])).to eq([]) + end + end + + describe '#find_object' do + let(:range) { double(:range) } + + before do + expect(CommitRange).to receive(:new).and_return(range) + end + + context 'when the range has valid commits' do + it 'returns the commit range' do + expect(range).to receive(:valid_commits?).and_return(true) + + expect(subject.find_object(project, '123..456')).to eq(range) + end + end + + context 'when the range does not have any valid commits' do + it 'returns nil' do + expect(range).to receive(:valid_commits?).and_return(false) + + expect(subject.find_object(project, '123..456')).to be_nil + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb new file mode 100644 index 00000000000..a6ef8394fe7 --- /dev/null +++ b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::ExternalIssueParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-project attribute' do + before do + link['data-project'] = project.id.to_s + end + + context 'when the link has a data-external-issue attribute' do + it 'returns an Array of ExternalIssue instances' do + link['data-external-issue'] = '123' + + refs = subject.referenced_by([link]) + + expect(refs).to eq([ExternalIssue.new('123', project)]) + end + end + + context 'when the link does not have a data-external-issue attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + describe '#issue_ids_per_project' do + before do + link['data-project'] = project.id.to_s + end + + it 'returns a Hash containing range IDs per project' do + link['data-external-issue'] = '123' + + hash = subject.issue_ids_per_project([link]) + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[project.id].to_a).to eq(['123']) + end + + it 'does not add a project when the data-external-issue attribute is empty' do + hash = subject.issue_ids_per_project([link]) + + expect(hash).to be_empty + end + end +end diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb new file mode 100644 index 00000000000..514c752546d --- /dev/null +++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::IssueParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#nodes_visible_to_user' do + context 'when the link has a data-issue attribute' do + before do + link['data-issue'] = issue.id.to_s + end + + it 'returns the nodes when the user can read the issue' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_issue, issue). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array when the user can not read the issue' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_issue, issue). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-issue attribute' do + it 'returns an empty Array' do + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the project uses an external issue tracker' do + it 'returns all nodes' do + link = double(:link) + + expect(project).to receive(:external_issue_tracker).and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + end + + describe '#referenced_by' do + context 'when the link has a data-issue attribute' do + context 'using an existing issue ID' do + before do + link['data-issue'] = issue.id.to_s + end + + it 'returns an Array of issues' do + expect(subject.referenced_by([link])).to eq([issue]) + end + + it 'returns an empty Array when the list of nodes is empty' do + expect(subject.referenced_by([link])).to eq([issue]) + expect(subject.referenced_by([])).to eq([]) + end + end + end + end + + describe '#issues_for_nodes' do + it 'returns a Hash containing the issues for a list of nodes' do + link['data-issue'] = issue.id.to_s + nodes = [link] + + expect(subject.issues_for_nodes(nodes)).to eq({ issue.id => issue }) + end + end +end diff --git a/spec/lib/banzai/reference_parser/label_parser_spec.rb b/spec/lib/banzai/reference_parser/label_parser_spec.rb new file mode 100644 index 00000000000..77fda47f0e7 --- /dev/null +++ b/spec/lib/banzai/reference_parser/label_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::LabelParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:label) { create(:label, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-label attribute' do + context 'using an existing label ID' do + it 'returns an Array of labels' do + link['data-label'] = label.id.to_s + + expect(subject.referenced_by([link])).to eq([label]) + end + end + + context 'using a non-existing label ID' do + it 'returns an empty Array' do + link['data-label'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb new file mode 100644 index 00000000000..cf89ad598ea --- /dev/null +++ b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::MergeRequestParser, lib: true do + include ReferenceParserHelpers + + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request) } + subject { described_class.new(merge_request.target_project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-merge-request attribute' do + context 'using an existing merge request ID' do + it 'returns an Array of merge requests' do + link['data-merge-request'] = merge_request.id.to_s + + expect(subject.referenced_by([link])).to eq([merge_request]) + end + end + + context 'using a non-existing merge request ID' do + it 'returns an empty Array' do + link['data-merge-request'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/milestone_parser_spec.rb b/spec/lib/banzai/reference_parser/milestone_parser_spec.rb new file mode 100644 index 00000000000..6aa45a22cc4 --- /dev/null +++ b/spec/lib/banzai/reference_parser/milestone_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::MilestoneParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:milestone) { create(:milestone, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-milestone attribute' do + context 'using an existing milestone ID' do + it 'returns an Array of milestones' do + link['data-milestone'] = milestone.id.to_s + + expect(subject.referenced_by([link])).to eq([milestone]) + end + end + + context 'using a non-existing milestone ID' do + it 'returns an empty Array' do + link['data-milestone'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb new file mode 100644 index 00000000000..59127b7c5d1 --- /dev/null +++ b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::SnippetParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:snippet) { create(:snippet, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-snippet attribute' do + context 'using an existing snippet ID' do + it 'returns an Array of snippets' do + link['data-snippet'] = snippet.id.to_s + + expect(subject.referenced_by([link])).to eq([snippet]) + end + end + + context 'using a non-existing snippet ID' do + it 'returns an empty Array' do + link['data-snippet'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/user_parser_spec.rb b/spec/lib/banzai/reference_parser/user_parser_spec.rb new file mode 100644 index 00000000000..9a82891297d --- /dev/null +++ b/spec/lib/banzai/reference_parser/user_parser_spec.rb @@ -0,0 +1,189 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::UserParser, lib: true do + include ReferenceParserHelpers + + let(:group) { create(:group) } + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public, group: group, creator: user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-group attribute' do + context 'using an existing group ID' do + before do + link['data-group'] = project.group.id.to_s + end + + it 'returns the users of the group' do + create(:group_member, group: group, user: user) + + expect(subject.referenced_by([link])).to eq([user]) + end + + it 'returns an empty Array when the group has no users' do + expect(subject.referenced_by([link])).to eq([]) + end + end + + context 'using a non-existing group ID' do + it 'returns an empty Array' do + link['data-group'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link has a data-user attribute' do + it 'returns an Array of users' do + link['data-user'] = user.id.to_s + + expect(subject.referenced_by([link])).to eq([user]) + end + end + + context 'when the link has a data-project attribute' do + context 'using an existing project ID' do + let(:contributor) { create(:user) } + + before do + project.team << [user, :developer] + project.team << [contributor, :developer] + end + + it 'returns the members of a project' do + link['data-project'] = project.id.to_s + + # This uses an explicit sort to make sure this spec doesn't randomly + # fail when objects are returned in a different order. + refs = subject.referenced_by([link]).sort_by(&:id) + + expect(refs).to eq([user, contributor]) + end + end + + context 'using a non-existing project ID' do + it 'returns an empty Array' do + link['data-project'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end + + describe '#nodes_visible_to_use?' do + context 'when the link has a data-group attribute' do + context 'using an existing group ID' do + before do + link['data-group'] = group.id.to_s + end + + it 'returns the nodes if the user can read the group' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_group, group). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array if the user can not read the group' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_group, group). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-group attribute' do + context 'with a data-project attribute' do + it 'returns the nodes if the attribute value equals the current project ID' do + link['data-project'] = project.id.to_s + + expect(Ability.abilities).not_to receive(:allowed?) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns the nodes if the user can read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array if the user can not read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'without a data-project attribute' do + it 'returns the nodes' do + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + end + end + end + + describe '#nodes_user_can_reference' do + context 'when the link has a data-author attribute' do + it 'returns the nodes when the user is a member of the project' do + other_project = create(:project) + other_project.team << [user, :developer] + + link['data-project'] = other_project.id.to_s + link['data-author'] = user.id.to_s + + expect(subject.nodes_user_can_reference(user, [link])).to eq([link]) + end + + it 'returns an empty Array when the project could not be found' do + link['data-project'] = '' + link['data-author'] = user.id.to_s + + expect(subject.nodes_user_can_reference(user, [link])).to eq([]) + end + + it 'returns an empty Array when the user could not be found' do + other_project = create(:project) + + link['data-project'] = other_project.id.to_s + link['data-author'] = '' + + expect(subject.nodes_user_can_reference(user, [link])).to eq([]) + end + + it 'returns an empty Array when the user is not a team member' do + other_project = create(:project) + + link['data-project'] = other_project.id.to_s + link['data-author'] = user.id.to_s + + expect(subject.nodes_user_can_reference(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-author attribute' do + it 'returns the nodes' do + expect(subject.nodes_user_can_reference(user, [link])).to eq([link]) + end + end + end +end diff --git a/spec/lib/gitlab/akismet_helper_spec.rb b/spec/lib/gitlab/akismet_helper_spec.rb index 53f5d6c5c80..88a71528867 100644 --- a/spec/lib/gitlab/akismet_helper_spec.rb +++ b/spec/lib/gitlab/akismet_helper_spec.rb @@ -6,8 +6,8 @@ describe Gitlab::AkismetHelper, type: :helper do before do allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url)) - current_application_settings.akismet_enabled = true - current_application_settings.akismet_api_key = '12345' + allow_any_instance_of(ApplicationSetting).to receive(:akismet_enabled).and_return(true) + allow_any_instance_of(ApplicationSetting).to receive(:akismet_api_key).and_return('12345') end describe '#check_for_spam?' do diff --git a/spec/lib/gitlab/lazy_spec.rb b/spec/lib/gitlab/lazy_spec.rb new file mode 100644 index 00000000000..b5ca89dd242 --- /dev/null +++ b/spec/lib/gitlab/lazy_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::Lazy, lib: true do + let(:dummy) { double(:dummy) } + + context 'when not calling any methods' do + it 'does not call the supplied block' do + expect(dummy).not_to receive(:foo) + + described_class.new { dummy.foo } + end + end + + context 'when calling a method on the object' do + it 'lazy loads the value returned by the block' do + expect(dummy).to receive(:foo).and_return('foo') + + lazy = described_class.new { dummy.foo } + + expect(lazy.to_s).to eq('foo') + end + end + + describe '#respond_to?' do + it 'returns true for a method defined on the wrapped object' do + lazy = described_class.new { 'foo' } + + expect(lazy).to respond_to(:downcase) + end + + it 'returns false for a method not defined on the wrapped object' do + lazy = described_class.new { 'foo' } + + expect(lazy).not_to respond_to(:quack) + end + end +end diff --git a/spec/lib/gitlab/note_data_builder_spec.rb b/spec/lib/gitlab/note_data_builder_spec.rb index f093d0a0d8b..e848d88182f 100644 --- a/spec/lib/gitlab/note_data_builder_spec.rb +++ b/spec/lib/gitlab/note_data_builder_spec.rb @@ -9,7 +9,8 @@ describe 'Gitlab::NoteDataBuilder', lib: true do before(:each) do expect(data).to have_key(:object_attributes) expect(data[:object_attributes]).to have_key(:url) - expect(data[:object_attributes][:url]).to eq(Gitlab::UrlBuilder.build(note)) + expect(data[:object_attributes][:url]) + .to eq(Gitlab::UrlBuilder.build(note)) expect(data[:object_kind]).to eq('note') expect(data[:user]).to eq(user.hook_attrs) end @@ -37,13 +38,21 @@ describe 'Gitlab::NoteDataBuilder', lib: true do end describe 'When asking for a note on issue' do - let(:issue) { create(:issue, created_at: fixed_time, updated_at: fixed_time) } - let(:note) { create(:note_on_issue, noteable_id: issue.id, project: project) } + let(:issue) do + create(:issue, created_at: fixed_time, updated_at: fixed_time, + project: project) + end + + let(:note) do + create(:note_on_issue, noteable: issue, project: project) + end it 'returns the note and issue-specific data' do expect(data).to have_key(:issue) - expect(data[:issue].except('updated_at')).to eq(issue.hook_attrs.except('updated_at')) - expect(data[:issue]['updated_at']).to be > issue.hook_attrs['updated_at'] + expect(data[:issue].except('updated_at')) + .to eq(issue.reload.hook_attrs.except('updated_at')) + expect(data[:issue]['updated_at']) + .to be > issue.hook_attrs['updated_at'] end include_examples 'project hook data' @@ -51,13 +60,23 @@ describe 'Gitlab::NoteDataBuilder', lib: true do end describe 'When asking for a note on merge request' do - let(:merge_request) { create(:merge_request, created_at: fixed_time, updated_at: fixed_time) } - let(:note) { create(:note_on_merge_request, noteable_id: merge_request.id, project: project) } + let(:merge_request) do + create(:merge_request, created_at: fixed_time, + updated_at: fixed_time, + source_project: project) + end + + let(:note) do + create(:note_on_merge_request, noteable: merge_request, + project: project) + end it 'returns the note and merge request data' do expect(data).to have_key(:merge_request) - expect(data[:merge_request].except('updated_at')).to eq(merge_request.hook_attrs.except('updated_at')) - expect(data[:merge_request]['updated_at']).to be > merge_request.hook_attrs['updated_at'] + expect(data[:merge_request].except('updated_at')) + .to eq(merge_request.reload.hook_attrs.except('updated_at')) + expect(data[:merge_request]['updated_at']) + .to be > merge_request.hook_attrs['updated_at'] end include_examples 'project hook data' @@ -65,13 +84,22 @@ describe 'Gitlab::NoteDataBuilder', lib: true do end describe 'When asking for a note on merge request diff' do - let(:merge_request) { create(:merge_request, created_at: fixed_time, updated_at: fixed_time) } - let(:note) { create(:note_on_merge_request_diff, noteable_id: merge_request.id, project: project) } + let(:merge_request) do + create(:merge_request, created_at: fixed_time, updated_at: fixed_time, + source_project: project) + end + + let(:note) do + create(:note_on_merge_request_diff, noteable: merge_request, + project: project) + end it 'returns the note and merge request diff data' do expect(data).to have_key(:merge_request) - expect(data[:merge_request].except('updated_at')).to eq(merge_request.hook_attrs.except('updated_at')) - expect(data[:merge_request]['updated_at']).to be > merge_request.hook_attrs['updated_at'] + expect(data[:merge_request].except('updated_at')) + .to eq(merge_request.reload.hook_attrs.except('updated_at')) + expect(data[:merge_request]['updated_at']) + .to be > merge_request.hook_attrs['updated_at'] end include_examples 'project hook data' @@ -79,13 +107,22 @@ describe 'Gitlab::NoteDataBuilder', lib: true do end describe 'When asking for a note on project snippet' do - let!(:snippet) { create(:project_snippet, created_at: fixed_time, updated_at: fixed_time) } - let!(:note) { create(:note_on_project_snippet, noteable_id: snippet.id, project: project) } + let!(:snippet) do + create(:project_snippet, created_at: fixed_time, updated_at: fixed_time, + project: project) + end + + let!(:note) do + create(:note_on_project_snippet, noteable: snippet, + project: project) + end it 'returns the note and project snippet data' do expect(data).to have_key(:snippet) - expect(data[:snippet].except('updated_at')).to eq(snippet.hook_attrs.except('updated_at')) - expect(data[:snippet]['updated_at']).to be > snippet.hook_attrs['updated_at'] + expect(data[:snippet].except('updated_at')) + .to eq(snippet.reload.hook_attrs.except('updated_at')) + expect(data[:snippet]['updated_at']) + .to be > snippet.hook_attrs['updated_at'] end include_examples 'project hook data' diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index b963a3e0324..818825b1477 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -51,7 +51,7 @@ describe Notify do context 'when enabled email_author_in_body' do before do - allow(current_application_settings).to receive(:email_author_in_body).and_return(true) + allow_any_instance_of(ApplicationSetting).to receive(:email_author_in_body).and_return(true) end it 'contains a link to note author' do @@ -230,7 +230,7 @@ describe Notify do context 'when enabled email_author_in_body' do before do - allow(current_application_settings).to receive(:email_author_in_body).and_return(true) + allow_any_instance_of(ApplicationSetting).to receive(:email_author_in_body).and_return(true) end it 'contains a link to note author' do @@ -454,7 +454,7 @@ describe Notify do context 'when enabled email_author_in_body' do before do - allow(current_application_settings).to receive(:email_author_in_body).and_return(true) + allow_any_instance_of(ApplicationSetting).to receive(:email_author_in_body).and_return(true) end it 'contains a link to note author' do diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb new file mode 100644 index 00000000000..1acb5846fcf --- /dev/null +++ b/spec/models/ability_spec.rb @@ -0,0 +1,117 @@ +require 'spec_helper' + +describe Ability, lib: true do + describe '.users_that_can_read_project' do + context 'using a public project' do + it 'returns all the users' do + project = create(:project, :public) + user = build(:user) + + expect(described_class.users_that_can_read_project([user], project)). + to eq([user]) + end + end + + context 'using an internal project' do + let(:project) { create(:project, :internal) } + + it 'returns users that are administrators' do + user = build(:user, admin: true) + + expect(described_class.users_that_can_read_project([user], project)). + to eq([user]) + end + + it 'returns internal users while skipping external users' do + user1 = build(:user) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns external users if they are the project owner' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project).to receive(:owner).twice.and_return(user1) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns external users if they are project members' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project.team).to receive(:members).twice.and_return([user1]) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns an empty Array if all users are external users without access' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([]) + end + end + + context 'using a private project' do + let(:project) { create(:project, :private) } + + it 'returns users that are administrators' do + user = build(:user, admin: true) + + expect(described_class.users_that_can_read_project([user], project)). + to eq([user]) + end + + it 'returns external users if they are the project owner' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project).to receive(:owner).twice.and_return(user1) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns external users if they are project members' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project.team).to receive(:members).twice.and_return([user1]) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns an empty Array if all users are internal users without access' do + user1 = build(:user) + user2 = build(:user) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([]) + end + + it 'returns an empty Array if all users are external users without access' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([]) + end + end + end +end diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 55b7af441d6..5c6c30c20ea 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -1,18 +1,17 @@ require 'spec_helper' describe Ci::Build, models: true do - let(:project) { FactoryGirl.create :project } - let(:commit) { FactoryGirl.create :ci_commit, project: project } - let(:build) { FactoryGirl.create :ci_build, commit: commit } + let(:project) { create(:project) } + let(:commit) { create(:ci_commit, project: project) } + let(:build) { create(:ci_build, commit: commit) } it { is_expected.to validate_presence_of :ref } it { is_expected.to respond_to :trace_html } describe '#first_pending' do - let(:first) { FactoryGirl.create :ci_build, commit: commit, status: 'pending', created_at: Date.yesterday } - let(:second) { FactoryGirl.create :ci_build, commit: commit, status: 'pending' } - before { first; second } + let!(:first) { create(:ci_build, commit: commit, status: 'pending', created_at: Date.yesterday) } + let!(:second) { create(:ci_build, commit: commit, status: 'pending') } subject { Ci::Build.first_pending } it { is_expected.to be_a(Ci::Build) } @@ -219,8 +218,8 @@ describe Ci::Build, models: true do it { is_expected.to eq(predefined_variables + yaml_variables + secure_variables) } context 'and trigger variables' do - let(:trigger) { FactoryGirl.create :ci_trigger, project: project } - let(:trigger_request) { FactoryGirl.create :ci_trigger_request_with_variables, commit: commit, trigger: trigger } + let(:trigger) { create(:ci_trigger, project: project) } + let(:trigger_request) { create(:ci_trigger_request_with_variables, commit: commit, trigger: trigger) } let(:trigger_variables) do [ { key: :TRIGGER_KEY, value: 'TRIGGER_VALUE', public: false } @@ -329,7 +328,7 @@ describe Ci::Build, models: true do end context 'if there are runner' do - let(:runner) { FactoryGirl.create :ci_runner } + let(:runner) { create(:ci_runner) } before do build.project.runners << runner @@ -366,7 +365,7 @@ describe Ci::Build, models: true do it { is_expected.to be_truthy } context "and there are specific runner" do - let(:runner) { FactoryGirl.create :ci_runner, contacted_at: 1.second.ago } + let(:runner) { create(:ci_runner, contacted_at: 1.second.ago) } before do build.project.runners << runner @@ -415,7 +414,7 @@ describe Ci::Build, models: true do end describe '#repo_url' do - let(:build) { FactoryGirl.create :ci_build } + let(:build) { create(:ci_build) } let(:project) { build.project } subject { build.repo_url } @@ -429,10 +428,10 @@ describe Ci::Build, models: true do end describe '#depends_on_builds' do - let!(:build) { FactoryGirl.create :ci_build, commit: commit, name: 'build', stage_idx: 0, stage: 'build' } - let!(:rspec_test) { FactoryGirl.create :ci_build, commit: commit, name: 'rspec', stage_idx: 1, stage: 'test' } - let!(:rubocop_test) { FactoryGirl.create :ci_build, commit: commit, name: 'rubocop', stage_idx: 1, stage: 'test' } - let!(:staging) { FactoryGirl.create :ci_build, commit: commit, name: 'staging', stage_idx: 2, stage: 'deploy' } + let!(:build) { create(:ci_build, commit: commit, name: 'build', stage_idx: 0, stage: 'build') } + let!(:rspec_test) { create(:ci_build, commit: commit, name: 'rspec', stage_idx: 1, stage: 'test') } + let!(:rubocop_test) { create(:ci_build, commit: commit, name: 'rubocop', stage_idx: 1, stage: 'test') } + let!(:staging) { create(:ci_build, commit: commit, name: 'staging', stage_idx: 2, stage: 'deploy') } it 'to have no dependents if this is first build' do expect(build.depends_on_builds).to be_empty @@ -453,11 +452,10 @@ describe Ci::Build, models: true do end def create_mr(build, commit, factory: :merge_request, created_at: Time.now) - FactoryGirl.create(factory, - source_project_id: commit.gl_project_id, - target_project_id: commit.gl_project_id, - source_branch: build.ref, - created_at: created_at) + create(factory, source_project_id: commit.gl_project_id, + target_project_id: commit.gl_project_id, + source_branch: build.ref, + created_at: created_at) end describe '#merge_request' do @@ -501,8 +499,8 @@ describe Ci::Build, models: true do context 'when a Build is created after the MR' do before do @merge_request = create_mr(build, commit, factory: :merge_request_with_diffs) - commit2 = FactoryGirl.create :ci_commit, project: project - @build2 = FactoryGirl.create :ci_build, commit: commit2 + commit2 = create(:ci_commit, project: project) + @build2 = create(:ci_build, commit: commit2) commits = [double(id: commit.sha), double(id: commit2.sha)] allow(@merge_request).to receive(:commits).and_return(commits) diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index c712d211b0f..98f60087cf5 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -23,7 +23,7 @@ describe Ci::Variable, models: true do end it 'fails to decrypt if iv is incorrect' do - subject.encrypted_value_iv = nil + subject.encrypted_value_iv = SecureRandom.hex subject.instance_variable_set(:@value, nil) expect { subject.value }. to raise_error(OpenSSL::Cipher::CipherError, 'bad decrypt') diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb index 9307d97e214..384a38ebc69 100644 --- a/spec/models/commit_range_spec.rb +++ b/spec/models/commit_range_spec.rb @@ -24,6 +24,16 @@ describe CommitRange, models: true do expect { described_class.new("Foo", project) }.to raise_error(ArgumentError) end + describe '#initialize' do + it 'does not modify strings in-place' do + input = "#{sha_from}...#{sha_to} " + + described_class.new(input, project) + + expect(input).to eq("#{sha_from}...#{sha_to} ") + end + end + describe '#to_s' do it 'is correct for three-dot syntax' do expect(range.to_s).to eq "#{full_sha_from}...#{full_sha_to}" @@ -135,4 +145,28 @@ describe CommitRange, models: true do end end end + + describe '#has_been_reverted?' do + it 'returns true if the commit has been reverted' do + issue = create(:issue) + + create(:note_on_issue, + noteable: issue, + system: true, + note: commit1.revert_description, + project: issue.project) + + expect_any_instance_of(Commit).to receive(:reverts_commit?). + with(commit1). + and_return(true) + + expect(commit1.has_been_reverted?(nil, issue)).to eq(true) + end + + it 'returns false a commit has not been reverted' do + issue = create(:issue) + + expect(commit1.has_been_reverted?(nil, issue)).to eq(false) + end + end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ccb100cd96f..beca8708c9d 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Commit, models: true do - let(:project) { create(:project) } + let(:project) { create(:project, :public) } let(:commit) { project.commit } describe 'modules' do @@ -171,4 +171,40 @@ eos describe '#status' do # TODO: kamil end + + describe '#participants' do + let(:user1) { build(:user) } + let(:user2) { build(:user) } + + let!(:note1) do + create(:note_on_commit, + commit_id: commit.id, + project: project, + note: 'foo') + end + + let!(:note2) do + create(:note_on_commit, + commit_id: commit.id, + project: project, + note: 'bar') + end + + before do + allow(commit).to receive(:author).and_return(user1) + allow(commit).to receive(:committer).and_return(user2) + end + + it 'includes the commit author' do + expect(commit.participants).to include(commit.author) + end + + it 'includes the committer' do + expect(commit.participants).to include(commit.committer) + end + + it 'includes the authors of the commit notes' do + expect(commit.participants).to include(note1.author, note2.author) + end + end end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 583827cdf42..fb20578d8d3 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -118,10 +118,10 @@ describe Issue, "Issuable" do let(:project) { build_stubbed(:empty_project) } context "by milestone due date" do - #Correct order is: - #Issues/MRs with milestones ordered by date - #Issues/MRs with milestones without dates - #Issues/MRs without milestones + # Correct order is: + # Issues/MRs with milestones ordered by date + # Issues/MRs with milestones without dates + # Issues/MRs without milestones let!(:issue) { create(:issue, project: project) } let!(:early_milestone) { create(:milestone, project: project, due_date: 10.days.from_now) } @@ -189,7 +189,6 @@ describe Issue, "Issuable" do let(:data) { issue.to_hook_data(user) } let(:project) { issue.project } - it "returns correct hook data" do expect(data[:object_kind]).to eq("issue") expect(data[:user]).to eq(user.hook_attrs) @@ -229,11 +228,11 @@ describe Issue, "Issuable" do end describe "votes" do + let(:project) { issue.project } + before do - author = create :user - project = create :empty_project - issue.notes.awards.create!(note: "thumbsup", author: author, project: project) - issue.notes.awards.create!(note: "thumbsdown", author: author, project: project) + issue.notes.awards.create!(note: "thumbsup", author: user, project: project) + issue.notes.awards.create!(note: "thumbsdown", author: user, project: project) end it "returns correct values" do diff --git a/spec/models/concerns/participable_spec.rb b/spec/models/concerns/participable_spec.rb new file mode 100644 index 00000000000..7e4ea0f2d66 --- /dev/null +++ b/spec/models/concerns/participable_spec.rb @@ -0,0 +1,83 @@ +require 'spec_helper' + +describe Participable, models: true do + let(:model) do + Class.new do + include Participable + end + end + + describe '.participant' do + it 'adds the participant attributes to the existing list' do + model.participant(:foo) + model.participant(:bar) + + expect(model.participant_attrs).to eq([:foo, :bar]) + end + end + + describe '#participants' do + it 'returns the list of participants' do + model.participant(:foo) + model.participant(:bar) + + user1 = build(:user) + user2 = build(:user) + user3 = build(:user) + project = build(:project, :public) + instance = model.new + + expect(instance).to receive(:foo).and_return(user2) + expect(instance).to receive(:bar).and_return(user3) + expect(instance).to receive(:project).twice.and_return(project) + + participants = instance.participants(user1) + + expect(participants).to include(user2) + expect(participants).to include(user3) + end + + it 'supports attributes returning another Participable' do + other_model = Class.new { include Participable } + + other_model.participant(:bar) + model.participant(:foo) + + instance = model.new + other = other_model.new + user1 = build(:user) + user2 = build(:user) + project = build(:project, :public) + + expect(instance).to receive(:foo).and_return(other) + expect(other).to receive(:bar).and_return(user2) + expect(instance).to receive(:project).twice.and_return(project) + + expect(instance.participants(user1)).to eq([user2]) + end + + context 'when using a Proc as an attribute' do + it 'calls the supplied Proc' do + user1 = build(:user) + project = build(:project, :public) + + user_arg = nil + ext_arg = nil + + model.participant -> (user, ext) do + user_arg = user + ext_arg = ext + end + + instance = model.new + + expect(instance).to receive(:project).twice.and_return(project) + + instance.participants(user1) + + expect(user_arg).to eq(user1) + expect(ext_arg).to be_an_instance_of(Gitlab::ReferenceExtractor) + end + end + end +end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 6540d77fbc0..87b3d8d650a 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -231,4 +231,42 @@ describe Issue, models: true do expect(issue.to_branch_name).to match /confidential-issue\z/ end end + + describe '#participants' do + context 'using a public project' do + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project) } + + let!(:note1) do + create(:note_on_issue, noteable: issue, project: project, note: 'a') + end + + let!(:note2) do + create(:note_on_issue, noteable: issue, project: project, note: 'b') + end + + it 'includes the issue author' do + expect(issue.participants).to include(issue.author) + end + + it 'includes the authors of the notes' do + expect(issue.participants).to include(note1.author, note2.author) + end + end + + context 'using a private project' do + it 'does not include mentioned users that do not have access to the project' do + project = create(:project) + user = create(:user) + issue = create(:issue, project: project) + + create(:note_on_issue, + noteable: issue, + project: project, + note: user.to_reference) + + expect(issue.participants).not_to include(user) + end + end + end end diff --git a/spec/models/legacy_diff_note_spec.rb b/spec/models/legacy_diff_note_spec.rb index 7c29bef54e4..b2d06853886 100644 --- a/spec/models/legacy_diff_note_spec.rb +++ b/spec/models/legacy_diff_note_spec.rb @@ -63,7 +63,9 @@ describe LegacyDiffNote, models: true do code = Gitlab::Diff::LineCode.generate(diff.new_path, line.new_pos, line.old_pos) # We're persisting in order to trigger the set_diff callback - note = create(:note_on_merge_request_diff, noteable: merge, line_code: code) + note = create(:note_on_merge_request_diff, noteable: merge, + line_code: code, + project: merge.source_project) # Make sure we don't get a false positive from a guard clause expect(note).to receive(:find_noteable_diff).and_call_original diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 9f26d9eb5ce..9f13874b532 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -20,6 +20,48 @@ require 'spec_helper' describe ProjectMember, models: true do + describe 'associations' do + it { is_expected.to belong_to(:project).class_name('Project').with_foreign_key(:source_id) } + end + + describe 'validations' do + it { is_expected.to allow_value('Project').for(:source_type) } + it { is_expected.not_to allow_value('project').for(:source_type) } + end + + describe 'modules' do + it { is_expected.to include_module(Gitlab::ShellAdapter) } + end + + describe "#destroy" do + let(:owner) { create(:project_member, access_level: ProjectMember::OWNER) } + let(:project) { owner.project } + let(:master) { create(:project_member, project: project) } + + let(:owner_todos) { (0...2).map { create(:todo, user: owner.user, project: project) } } + let(:master_todos) { (0...3).map { create(:todo, user: master.user, project: project) } } + + before do + owner_todos + master_todos + end + + it "destroy itself and delete associated todos" do + expect(owner.user.todos.size).to eq(2) + expect(master.user.todos.size).to eq(3) + expect(Todo.count).to eq(5) + + master_todo_ids = master_todos.map(&:id) + master.destroy + + expect(owner.user.todos.size).to eq(2) + expect(Todo.count).to eq(2) + master_todo_ids.each do |id| + expect(Todo.exists?(id)).to eq(false) + end + end + end + describe :import_team do before do @abilities = Six.new diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index e269ff26a04..118e1e22a78 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -119,7 +119,8 @@ describe MergeRequest, models: true do before do allow(merge_request).to receive(:commits) { [merge_request.source_project.repository.commit] } - create(:note, commit_id: merge_request.commits.first.id, noteable_type: 'Commit', project: merge_request.project) + create(:note_on_commit, commit_id: merge_request.commits.first.id, + project: merge_request.project) create(:note, noteable: merge_request, project: merge_request.project) end @@ -129,7 +130,9 @@ describe MergeRequest, models: true do end it "should include notes for commits from target project as well" do - create(:note, commit_id: merge_request.commits.first.id, noteable_type: 'Commit', project: merge_request.target_project) + create(:note_on_commit, commit_id: merge_request.commits.first.id, + project: merge_request.target_project) + expect(merge_request.commits).not_to be_empty expect(merge_request.mr_and_commit_notes.count).to eq(3) end @@ -411,4 +414,28 @@ describe MergeRequest, models: true do end end end + + describe '#participants' do + let(:project) { create(:project, :public) } + + let(:mr) do + create(:merge_request, source_project: project, target_project: project) + end + + let!(:note1) do + create(:note_on_merge_request, noteable: mr, project: project, note: 'a') + end + + let!(:note2) do + create(:note_on_merge_request, noteable: mr, project: project, note: 'b') + end + + it 'includes the merge request author' do + expect(mr.participants).to include(mr.author) + end + + it 'includes the authors of the notes' do + expect(mr.participants).to include(note1.author, note2.author) + end + end end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 5d916f0e6a6..e9d89c9a847 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -12,6 +12,34 @@ describe Note, models: true do describe 'validation' do it { is_expected.to validate_presence_of(:note) } it { is_expected.to validate_presence_of(:project) } + + context 'when note is on commit' do + before { allow(subject).to receive(:for_commit?).and_return(true) } + + it { is_expected.to validate_presence_of(:commit_id) } + it { is_expected.not_to validate_presence_of(:noteable_id) } + end + + context 'when note is not on commit' do + before { allow(subject).to receive(:for_commit?).and_return(false) } + + it { is_expected.not_to validate_presence_of(:commit_id) } + it { is_expected.to validate_presence_of(:noteable_id) } + end + + context 'when noteable and note project differ' do + subject do + build(:note, noteable: build_stubbed(:issue), + project: build_stubbed(:project)) + end + + it { is_expected.to be_invalid } + end + + context 'when noteable and note project are the same' do + subject { create(:note) } + it { is_expected.to be_valid } + end end describe "Commit notes" do @@ -89,12 +117,23 @@ describe Note, models: true do end describe "#all_references" do - let!(:note1) { create(:note) } - let!(:note2) { create(:note) } + let!(:note1) { create(:note_on_issue) } + let!(:note2) { create(:note_on_issue) } it "reads the rendered note body from the cache" do - expect(Banzai::Renderer).to receive(:render).with(note1.note, pipeline: :note, cache_key: [note1, "note"], project: note1.project) - expect(Banzai::Renderer).to receive(:render).with(note2.note, pipeline: :note, cache_key: [note2, "note"], project: note2.project) + expect(Banzai::Renderer).to receive(:render). + with(note1.note, + pipeline: :note, + cache_key: [note1, "note"], + project: note1.project, + author: note1.author) + + expect(Banzai::Renderer).to receive(:render). + with(note2.note, + pipeline: :note, + cache_key: [note2, "note"], + project: note2.project, + author: note2.author) note1.all_references note2.all_references @@ -102,7 +141,7 @@ describe Note, models: true do end describe '.search' do - let(:note) { create(:note, note: 'WoW') } + let(:note) { create(:note_on_issue, note: 'WoW') } it 'returns notes with matching content' do expect(described_class.search(note.note)).to eq([note]) @@ -111,6 +150,25 @@ describe Note, models: true do it 'returns notes with matching content regardless of the casing' do expect(described_class.search('WOW')).to eq([note]) end + + context "confidential issues" do + let(:user) { create :user } + let(:confidential_issue) { create(:issue, :confidential, author: user) } + let(:confidential_note) { create :note, note: "Random", noteable: confidential_issue, project: confidential_issue.project } + + it "returns notes with matching content if user can see the issue" do + expect(described_class.search(confidential_note.note, as_user: user)).to eq([confidential_note]) + end + + it "does not return notes with matching content if user can not see the issue" do + user = create :user + expect(described_class.search(confidential_note.note, as_user: user)).to be_empty + end + + it "does not return notes with matching content for unauthenticated users" do + expect(described_class.search(confidential_note.note)).to be_empty + end + end end describe '.grouped_awards' do @@ -175,12 +233,18 @@ describe Note, models: true do let(:merge_request) { create :merge_request } it "converts aliases to actual name" do - note = create(:note, note: ":+1:", noteable: merge_request) + note = create(:note, note: ":+1:", + noteable: merge_request, + project: merge_request.project) + expect(note.reload.note).to eq("thumbsup") end it "is not an award emoji when comment is on a diff" do - note = create(:note_on_merge_request_diff, note: ":blowfish:", noteable: merge_request, line_code: "11d5d2e667e9da4f7f610f81d86c974b146b13bd_0_2") + note = create(:note_on_merge_request_diff, note: ":blowfish:", + noteable: merge_request, + project: merge_request.project, + line_code: "11d5d2e667e9da4f7f610f81d86c974b146b13bd_0_2") note = note.reload expect(note.note).to eq(":blowfish:") @@ -195,4 +259,14 @@ describe Note, models: true do expect { note.valid? }.to change(note, :line_code).to(nil) end end + + describe '#participants' do + it 'includes the note author' do + project = create(:project, :public) + issue = create(:issue, project: project) + note = create(:note_on_issue, noteable: issue, project: project) + + expect(note.participants).to include(note.author) + end + end end diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index f887ed4bafc..5f618322aab 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -176,86 +176,117 @@ describe HipchatService, models: true do context "Note events" do let(:user) { create(:user) } let(:project) { create(:project, creator_id: user.id) } - let(:issue) { create(:issue, project: project) } - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let(:snippet) { create(:project_snippet, project: project) } - let(:commit_note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') } - let(:merge_request_note) { create(:note_on_merge_request, noteable_id: merge_request.id, note: "merge request note") } - let(:issue_note) { create(:note_on_issue, noteable_id: issue.id, note: "issue note")} - let(:snippet_note) { create(:note_on_project_snippet, noteable_id: snippet.id, note: "snippet note") } - - it "should call Hipchat API for commit comment events" do - data = Gitlab::NoteDataBuilder.build(commit_note, user) - hipchat.execute(data) - expect(WebMock).to have_requested(:post, api_url).once + context 'when commit comment event triggered' do + let(:commit_note) do + create(:note_on_commit, author: user, project: project, + commit_id: project.repository.commit.id, + note: 'a comment on a commit') + end + + it "should call Hipchat API for commit comment events" do + data = Gitlab::NoteDataBuilder.build(commit_note, user) + hipchat.execute(data) - message = hipchat.send(:create_message, data) + expect(WebMock).to have_requested(:post, api_url).once - obj_attr = data[:object_attributes] - commit_id = Commit.truncate_sha(data[:commit][:id]) - title = hipchat.send(:format_title, data[:commit][:message]) + message = hipchat.send(:create_message, data) - expect(message).to eq("#{user.name} commented on " \ - "<a href=\"#{obj_attr[:url]}\">commit #{commit_id}</a> in " \ - "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ - "#{title}" \ - "<pre>a comment on a commit</pre>") + obj_attr = data[:object_attributes] + commit_id = Commit.truncate_sha(data[:commit][:id]) + title = hipchat.send(:format_title, data[:commit][:message]) + + expect(message).to eq("#{user.name} commented on " \ + "<a href=\"#{obj_attr[:url]}\">commit #{commit_id}</a> in " \ + "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ + "#{title}" \ + "<pre>a comment on a commit</pre>") + end end - it "should call Hipchat API for merge request comment events" do - data = Gitlab::NoteDataBuilder.build(merge_request_note, user) - hipchat.execute(data) + context 'when merge request comment event triggered' do + let(:merge_request) do + create(:merge_request, source_project: project, + target_project: project) + end - expect(WebMock).to have_requested(:post, api_url).once + let(:merge_request_note) do + create(:note_on_merge_request, noteable: merge_request, + project: project, + note: "merge request note") + end - message = hipchat.send(:create_message, data) + it "should call Hipchat API for merge request comment events" do + data = Gitlab::NoteDataBuilder.build(merge_request_note, user) + hipchat.execute(data) - obj_attr = data[:object_attributes] - merge_id = data[:merge_request]['iid'] - title = data[:merge_request]['title'] + expect(WebMock).to have_requested(:post, api_url).once - expect(message).to eq("#{user.name} commented on " \ - "<a href=\"#{obj_attr[:url]}\">merge request !#{merge_id}</a> in " \ - "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ - "<b>#{title}</b>" \ - "<pre>merge request note</pre>") + message = hipchat.send(:create_message, data) + + obj_attr = data[:object_attributes] + merge_id = data[:merge_request]['iid'] + title = data[:merge_request]['title'] + + expect(message).to eq("#{user.name} commented on " \ + "<a href=\"#{obj_attr[:url]}\">merge request !#{merge_id}</a> in " \ + "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ + "<b>#{title}</b>" \ + "<pre>merge request note</pre>") + end end - it "should call Hipchat API for issue comment events" do - data = Gitlab::NoteDataBuilder.build(issue_note, user) - hipchat.execute(data) + context 'when issue comment event triggered' do + let(:issue) { create(:issue, project: project) } + let(:issue_note) do + create(:note_on_issue, noteable: issue, project: project, + note: "issue note") + end - message = hipchat.send(:create_message, data) + it "should call Hipchat API for issue comment events" do + data = Gitlab::NoteDataBuilder.build(issue_note, user) + hipchat.execute(data) - obj_attr = data[:object_attributes] - issue_id = data[:issue]['iid'] - title = data[:issue]['title'] + message = hipchat.send(:create_message, data) - expect(message).to eq("#{user.name} commented on " \ - "<a href=\"#{obj_attr[:url]}\">issue ##{issue_id}</a> in " \ - "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ - "<b>#{title}</b>" \ - "<pre>issue note</pre>") + obj_attr = data[:object_attributes] + issue_id = data[:issue]['iid'] + title = data[:issue]['title'] + + expect(message).to eq("#{user.name} commented on " \ + "<a href=\"#{obj_attr[:url]}\">issue ##{issue_id}</a> in " \ + "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ + "<b>#{title}</b>" \ + "<pre>issue note</pre>") + end end - it "should call Hipchat API for snippet comment events" do - data = Gitlab::NoteDataBuilder.build(snippet_note, user) - hipchat.execute(data) + context 'when snippet comment event triggered' do + let(:snippet) { create(:project_snippet, project: project) } + let(:snippet_note) do + create(:note_on_project_snippet, noteable: snippet, + project: project, + note: "snippet note") + end - expect(WebMock).to have_requested(:post, api_url).once + it "should call Hipchat API for snippet comment events" do + data = Gitlab::NoteDataBuilder.build(snippet_note, user) + hipchat.execute(data) - message = hipchat.send(:create_message, data) + expect(WebMock).to have_requested(:post, api_url).once - obj_attr = data[:object_attributes] - snippet_id = data[:snippet]['id'] - title = data[:snippet]['title'] + message = hipchat.send(:create_message, data) - expect(message).to eq("#{user.name} commented on " \ - "<a href=\"#{obj_attr[:url]}\">snippet ##{snippet_id}</a> in " \ - "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ - "<b>#{title}</b>" \ - "<pre>snippet note</pre>") + obj_attr = data[:object_attributes] + snippet_id = data[:snippet]['id'] + title = data[:snippet]['title'] + + expect(message).to eq("#{user.name} commented on " \ + "<a href=\"#{obj_attr[:url]}\">snippet ##{snippet_id}</a> in " \ + "<a href=\"#{project.web_url}\">#{project_name}</a>: " \ + "<b>#{title}</b>" \ + "<pre>snippet note</pre>") + end end end diff --git a/spec/models/project_services/slack_service_spec.rb b/spec/models/project_services/slack_service_spec.rb index a97b7560137..155f3e74e0d 100644 --- a/spec/models/project_services/slack_service_spec.rb +++ b/spec/models/project_services/slack_service_spec.rb @@ -142,13 +142,6 @@ describe SlackService, models: true do let(:slack) { SlackService.new } let(:user) { create(:user) } let(:project) { create(:project, creator_id: user.id) } - let(:issue) { create(:issue, project: project) } - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let(:snippet) { create(:project_snippet, project: project) } - let(:commit_note) { create(:note_on_commit, author: user, project: project, commit_id: project.repository.commit.id, note: 'a comment on a commit') } - let(:merge_request_note) { create(:note_on_merge_request, noteable_id: merge_request.id, note: "merge request note") } - let(:issue_note) { create(:note_on_issue, noteable_id: issue.id, note: "issue note")} - let(:snippet_note) { create(:note_on_project_snippet, noteable_id: snippet.id, note: "snippet note") } let(:webhook_url) { 'https://hooks.slack.com/services/SVRWFV0VVAR97N/B02R25XN3/ZBqu7xMupaEEICInN685' } before do @@ -162,32 +155,61 @@ describe SlackService, models: true do WebMock.stub_request(:post, webhook_url) end - it "should call Slack API for commit comment events" do - data = Gitlab::NoteDataBuilder.build(commit_note, user) - slack.execute(data) + context 'when commit comment event executed' do + let(:commit_note) do + create(:note_on_commit, author: user, + project: project, + commit_id: project.repository.commit.id, + note: 'a comment on a commit') + end - expect(WebMock).to have_requested(:post, webhook_url).once + it "should call Slack API for commit comment events" do + data = Gitlab::NoteDataBuilder.build(commit_note, user) + slack.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end end - it "should call Slack API for merge request comment events" do - data = Gitlab::NoteDataBuilder.build(merge_request_note, user) - slack.execute(data) + context 'when merge request comment event executed' do + let(:merge_request_note) do + create(:note_on_merge_request, project: project, + note: "merge request note") + end - expect(WebMock).to have_requested(:post, webhook_url).once + it "should call Slack API for merge request comment events" do + data = Gitlab::NoteDataBuilder.build(merge_request_note, user) + slack.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end end - it "should call Slack API for issue comment events" do - data = Gitlab::NoteDataBuilder.build(issue_note, user) - slack.execute(data) + context 'when issue comment event executed' do + let(:issue_note) do + create(:note_on_issue, project: project, note: "issue note") + end - expect(WebMock).to have_requested(:post, webhook_url).once + it "should call Slack API for issue comment events" do + data = Gitlab::NoteDataBuilder.build(issue_note, user) + slack.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end end - it "should call Slack API for snippet comment events" do - data = Gitlab::NoteDataBuilder.build(snippet_note, user) - slack.execute(data) + context 'when snippet comment event executed' do + let(:snippet_note) do + create(:note_on_project_snippet, project: project, + note: "snippet note") + end - expect(WebMock).to have_requested(:post, webhook_url).once + it "should call Slack API for snippet comment events" do + data = Gitlab::NoteDataBuilder.build(snippet_note, user) + slack.execute(data) + + expect(WebMock).to have_requested(:post, webhook_url).once + end end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 6c1b0393682..338a4c3d3f0 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -784,6 +784,15 @@ describe Project, models: true do end end + describe '#container_registry_path_with_namespace' do + let(:project) { create(:empty_project, path: 'PROJECT') } + + subject { project.container_registry_path_with_namespace } + + it { is_expected.not_to eq(project.path_with_namespace) } + it { is_expected.to eq(project.path_with_namespace.downcase) } + end + describe '#container_registry_repository' do let(:project) { create(:empty_project) } diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 91ebb612baa..58b57bd4fef 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -16,6 +16,12 @@ describe ProjectWiki, models: true do end end + describe '#web_url' do + it 'returns the full web URL to the wiki' do + expect(subject.web_url).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/wikis/home") + end + end + describe "#url_to_repo" do it "returns the correct ssh url to the repo" do expect(subject.url_to_repo).to eq(gitlab_shell.url_to_repo(subject.path_with_namespace)) @@ -257,6 +263,13 @@ describe ProjectWiki, models: true do end end + describe '#hook_attrs' do + it 'returns a hash with values' do + expect(subject.hook_attrs).to be_a Hash + expect(subject.hook_attrs.keys).to contain_exactly(:web_url, :git_ssh_url, :git_http_url, :path_with_namespace, :default_branch) + end + end + private def create_temp_repo(path) diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 7a613e360d4..789816bf2c7 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -87,4 +87,31 @@ describe Snippet, models: true do expect(described_class.search_code('FOO')).to eq([snippet]) end end + + describe '#participants' do + let(:project) { create(:project, :public) } + let(:snippet) { create(:snippet, content: 'foo', project: project) } + + let!(:note1) do + create(:note_on_project_snippet, + noteable: snippet, + project: project, + note: 'a') + end + + let!(:note2) do + create(:note_on_project_snippet, + noteable: snippet, + project: project, + note: 'b') + end + + it 'includes the snippet author' do + expect(snippet.participants).to include(snippet.author) + end + + it 'includes the note authors' do + expect(snippet.participants).to include(note1.author, note2.author) + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9581990666b..548bec364f8 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -67,7 +67,7 @@ describe User, models: true do describe 'email' do context 'when no signup domains listed' do - before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return([]) } + before { allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return([]) } it 'accepts any email' do user = build(:user, email: "info@example.com") expect(user).to be_valid @@ -75,7 +75,7 @@ describe User, models: true do end context 'when a signup domain is listed and subdomains are allowed' do - before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return(['example.com', '*.example.com']) } + before { allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['example.com', '*.example.com']) } it 'accepts info@example.com' do user = build(:user, email: "info@example.com") expect(user).to be_valid @@ -93,7 +93,7 @@ describe User, models: true do end context 'when a signup domain is listed and subdomains are not allowed' do - before { allow(current_application_settings).to receive(:restricted_signup_domains).and_return(['example.com']) } + before { allow_any_instance_of(ApplicationSetting).to receive(:restricted_signup_domains).and_return(['example.com']) } it 'accepts info@example.com' do user = build(:user, email: "info@example.com") @@ -141,7 +141,7 @@ describe User, models: true do end describe '#confirm' do - before { allow(current_application_settings).to receive(:send_user_confirmation_email).and_return(true) } + before { allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true) } let(:user) { create(:user, confirmed_at: nil, unconfirmed_email: 'test@gitlab.com') } it 'returns unconfirmed' do diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index ed1ed5aeb95..beb29a68692 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -258,8 +258,8 @@ describe API::API, api: true do body: 'Hi!' end - it 'responds with 500' do - expect(response.status).to eq 500 + it 'responds with resource not found error' do + expect(response.status).to eq 404 end it 'does not create new note' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 40b24c125b5..a7690f430c4 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -20,7 +20,7 @@ describe API::API, api: true do end context "when authenticated" do - #These specs are written just in case API authentication is not required anymore + # These specs are written just in case API authentication is not required anymore context "when public level is restricted" do before do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index 98ef9d21035..67777ad48bc 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -14,9 +14,24 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do allow_any_instance_of(JSONWebToken::RSAToken).to receive(:key).and_return(rsa_key) end - shared_examples 'an authenticated' do + shared_examples 'a valid token' do it { is_expected.to include(:token) } it { expect(payload).to include('access') } + + context 'a expirable' do + let(:expires_at) { Time.at(payload['exp']) } + let(:expire_delay) { 10 } + + context 'for default configuration' do + it { expect(expires_at).not_to be_within(2.seconds).of(Time.now + expire_delay.minutes) } + end + + context 'for changed configuration' do + before { stub_application_setting(container_registry_token_expire_delay: expire_delay) } + + it { expect(expires_at).to be_within(2.seconds).of(Time.now + expire_delay.minutes) } + end + end end shared_examples 'a accessible' do @@ -28,10 +43,15 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do }] end - it_behaves_like 'an authenticated' + it_behaves_like 'a valid token' it { expect(payload).to include('access' => access) } end + shared_examples 'an inaccessible' do + it_behaves_like 'a valid token' + it { expect(payload).to include('access' => []) } + end + shared_examples 'a pullable' do it_behaves_like 'a accessible' do let(:actions) { ['pull'] } @@ -50,11 +70,6 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end end - shared_examples 'an unauthorized' do - it { is_expected.to include(http_status: 401) } - it { is_expected.not_to include(:token) } - end - shared_examples 'a forbidden' do it { is_expected.to include(http_status: 403) } it { is_expected.not_to include(:token) } @@ -75,12 +90,8 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do let(:project) { create(:project) } let(:current_user) { create(:user) } - context 'allow to use offline_token' do - let(:current_params) do - { offline_token: true } - end - - it_behaves_like 'an authenticated' + context 'allow to use scope-less authentication' do + it_behaves_like 'a valid token' end context 'allow developer to push images' do @@ -120,19 +131,15 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:pull,push" } end - it_behaves_like 'a forbidden' + it_behaves_like 'an inaccessible' end end context 'project authorization' do let(:current_project) { create(:empty_project) } - context 'allow to use offline_token' do - let(:current_params) do - { offline_token: true } - end - - it_behaves_like 'an authenticated' + context 'allow to use scope-less authentication' do + it_behaves_like 'a valid token' end context 'allow to pull and push images' do @@ -158,7 +165,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'disallow for private' do let(:project) { create(:empty_project, :private) } - it_behaves_like 'a forbidden' + it_behaves_like 'an inaccessible' end end @@ -169,7 +176,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'disallow for all' do let(:project) { create(:empty_project, :public) } - it_behaves_like 'a forbidden' + it_behaves_like 'an inaccessible' end end end @@ -184,18 +191,14 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:pull" } end - it_behaves_like 'a forbidden' + it_behaves_like 'an inaccessible' end end end context 'unauthorized' do - context 'disallow to use offline_token' do - let(:current_params) do - { offline_token: true } - end - - it_behaves_like 'an unauthorized' + context 'disallow to use scope-less authentication' do + it_behaves_like 'a forbidden' end context 'for invalid scope' do diff --git a/spec/services/groups/create_service_spec.rb b/spec/services/groups/create_service_spec.rb index a97ac3ead45..71a0b8e2a12 100644 --- a/spec/services/groups/create_service_spec.rb +++ b/spec/services/groups/create_service_spec.rb @@ -13,7 +13,7 @@ describe Groups::CreateService, services: true do end context "cannot create group with restricted visibility level" do - before { allow(current_application_settings).to receive(:restricted_visibility_levels).and_return([Gitlab::VisibilityLevel::PUBLIC]) } + before { allow_any_instance_of(ApplicationSetting).to receive(:restricted_visibility_levels).and_return([Gitlab::VisibilityLevel::PUBLIC]) } it { is_expected.not_to be_persisted } end end diff --git a/spec/services/issues/bulk_update_service_spec.rb b/spec/services/issues/bulk_update_service_spec.rb index e91906d0d49..96f050bbd9b 100644 --- a/spec/services/issues/bulk_update_service_spec.rb +++ b/spec/services/issues/bulk_update_service_spec.rb @@ -15,9 +15,7 @@ describe Issues::BulkUpdateService, services: true do describe :close_issue do before do - @issues = 5.times.collect do - create(:issue, project: @project) - end + @issues = create_list(:issue, 5, project: @project) @params = { state_event: 'close', issues_ids: @issues.map(&:id) @@ -36,11 +34,8 @@ describe Issues::BulkUpdateService, services: true do end describe :reopen_issues do - before do - @issues = 5.times.collect do - create(:closed_issue, project: @project) - end + @issues = create_list(:closed_issue, 5, project: @project) @params = { state_event: 'reopen', issues_ids: @issues.map(&:id) diff --git a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb index 82247849814..0861d74aede 100644 --- a/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb +++ b/spec/services/merge_requests/merge_when_build_succeeds_service_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe MergeRequests::MergeWhenBuildSucceedsService do - let(:user) { create(:user) } - let(:merge_request) { create(:merge_request) } + let(:user) { create(:user) } + let(:project) { create(:project) } let(:mr_merge_if_green_enabled) do create(:merge_request, merge_when_build_succeeds: true, merge_user: user, @@ -10,11 +10,15 @@ describe MergeRequests::MergeWhenBuildSucceedsService do source_project: project, target_project: project, state: "opened") end - let(:project) { create(:project) } let(:ci_commit) { create(:ci_commit_with_one_job, ref: mr_merge_if_green_enabled.source_branch, project: project) } let(:service) { MergeRequests::MergeWhenBuildSucceedsService.new(project, user, commit_message: 'Awesome message') } describe "#execute" do + let(:merge_request) do + create(:merge_request, target_project: project, source_project: project, + source_branch: "feature", target_branch: 'master') + end + context 'first time enabling' do before do allow(merge_request).to receive(:ci_commit).and_return(ci_commit) diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index ee976eb2926..29e0a63d8ce 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -208,8 +208,10 @@ describe SystemNoteService, services: true do end describe '.merge_when_build_succeeds' do - let(:ci_commit) { build :ci_commit_without_jobs } - let(:noteable) { create :merge_request } + let(:ci_commit) { build(:ci_commit_without_jobs )} + let(:noteable) do + create(:merge_request, source_project: project, target_project: project) + end subject { described_class.merge_when_build_succeeds(noteable, project, author, noteable.last_commit) } @@ -221,8 +223,10 @@ describe SystemNoteService, services: true do end describe '.cancel_merge_when_build_succeeds' do - let(:ci_commit) { build :ci_commit_without_jobs } - let(:noteable) { create :merge_request } + let(:ci_commit) { build(:ci_commit_without_jobs) } + let(:noteable) do + create(:merge_request, source_project: project, target_project: project) + end subject { described_class.cancel_merge_when_build_succeeds(noteable, project, author) } diff --git a/spec/support/filter_spec_helper.rb b/spec/support/filter_spec_helper.rb index e849a9633b9..a8e454eb09e 100644 --- a/spec/support/filter_spec_helper.rb +++ b/spec/support/filter_spec_helper.rb @@ -40,8 +40,7 @@ module FilterSpecHelper filters = [ Banzai::Filter::AutolinkFilter, - described_class, - Banzai::Filter::ReferenceGathererFilter + described_class ] HTML::Pipeline.new(filters, context) diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb index cd9fdc6f18e..7a0f078c72b 100644 --- a/spec/support/login_helpers.rb +++ b/spec/support/login_helpers.rb @@ -26,11 +26,13 @@ module LoginHelpers # Internal: Login as the specified user # - # user - User instance to login with - def login_with(user) + # user - User instance to login with + # remember - Whether or not to check "Remember me" (default: false) + def login_with(user, remember: false) visit new_user_session_path fill_in "user_login", with: user.email fill_in "user_password", with: "12345678" + check 'user_remember_me' if remember click_button "Sign in" Thread.current[:current_user] = user end diff --git a/spec/support/reference_parser_helpers.rb b/spec/support/reference_parser_helpers.rb new file mode 100644 index 00000000000..01689194eac --- /dev/null +++ b/spec/support/reference_parser_helpers.rb @@ -0,0 +1,5 @@ +module ReferenceParserHelpers + def empty_html_link + Nokogiri::HTML.fragment('<a></a>').children[0] + end +end diff --git a/spec/teaspoon_env.rb b/spec/teaspoon_env.rb index 58f45ff8610..69b2b9b6d5b 100644 --- a/spec/teaspoon_env.rb +++ b/spec/teaspoon_env.rb @@ -41,11 +41,11 @@ Teaspoon.configure do |config| suite.matcher = "{spec/javascripts,app/assets}/**/*_spec.{js,js.coffee,coffee}" # Load additional JS files, but requiring them in your spec helper is the preferred way to do this. - #suite.javascripts = [] + # suite.javascripts = [] # You can include your own stylesheets if you want to change how Teaspoon looks. # Note: Spec related CSS can and should be loaded using fixtures. - #suite.stylesheets = ["teaspoon"] + # suite.stylesheets = ["teaspoon"] # This suites spec helper, which can require additional support files. This file is loaded before any of your test # files are loaded. @@ -62,19 +62,19 @@ Teaspoon.configure do |config| # Hooks allow you to use `Teaspoon.hook("fixtures")` before, after, or during your spec run. This will make a # synchronous Ajax request to the server that will call all of the blocks you've defined for that hook name. - #suite.hook :fixtures, &proc{} + # suite.hook :fixtures, &proc{} # Determine whether specs loaded into the test harness should be embedded as individual script tags or concatenated - # into a single file. Similar to Rails' asset `debug: true` and `config.assets.debug = true` options. By default, + # into a single file. Similar to Rails' asset `debug: true` and `config.assets.debug = true` options. By default, # Teaspoon expands all assets to provide more valuable stack traces that reference individual source files. - #suite.expand_assets = true + # suite.expand_assets = true end # Example suite. Since we're just filtering to files already within the root test/javascripts, these files will also # be run in the default suite -- but can be focused into a more specific suite. - #config.suite :targeted do |suite| + # config.suite :targeted do |suite| # suite.matcher = "spec/javascripts/targeted/*_spec.{js,js.coffee,coffee}" - #end + # end # CONSOLE RUNNER SPECIFIC # @@ -94,45 +94,45 @@ Teaspoon.configure do |config| # PhantomJS: https://github.com/modeset/teaspoon/wiki/Using-PhantomJS # Selenium Webdriver: https://github.com/modeset/teaspoon/wiki/Using-Selenium-WebDriver # Capybara Webkit: https://github.com/modeset/teaspoon/wiki/Using-Capybara-Webkit - #config.driver = :phantomjs + # config.driver = :phantomjs # Specify additional options for the driver. # # PhantomJS: https://github.com/modeset/teaspoon/wiki/Using-PhantomJS # Selenium Webdriver: https://github.com/modeset/teaspoon/wiki/Using-Selenium-WebDriver # Capybara Webkit: https://github.com/modeset/teaspoon/wiki/Using-Capybara-Webkit - #config.driver_options = nil + # config.driver_options = nil # Specify the timeout for the driver. Specs are expected to complete within this time frame or the run will be # considered a failure. This is to avoid issues that can arise where tests stall. - #config.driver_timeout = 180 + # config.driver_timeout = 180 # Specify a server to use with Rack (e.g. thin, mongrel). If nil is provided Rack::Server is used. - #config.server = nil + # config.server = nil # Specify a port to run on a specific port, otherwise Teaspoon will use a random available port. - #config.server_port = nil + # config.server_port = nil # Timeout for starting the server in seconds. If your server is slow to start you may have to bump this, or you may # want to lower this if you know it shouldn't take long to start. - #config.server_timeout = 20 + # config.server_timeout = 20 # Force Teaspoon to fail immediately after a failing suite. Can be useful to make Teaspoon fail early if you have # several suites, but in environments like CI this may not be desirable. - #config.fail_fast = true + # config.fail_fast = true # Specify the formatters to use when outputting the results. # Note: Output files can be specified by using `"junit>/path/to/output.xml"`. # # Available: :dot, :clean, :documentation, :json, :junit, :pride, :rspec_html, :snowday, :swayze_or_oprah, :tap, :tap_y, :teamcity - #config.formatters = [:dot] + # config.formatters = [:dot] # Specify if you want color output from the formatters. - #config.color = true + # config.color = true # Teaspoon pipes all console[log/debug/error] to $stdout. This is useful to catch places where you've forgotten to # remove them, but in verbose applications this may not be desirable. - #config.suppress_log = false + # config.suppress_log = false # COVERAGE REPORTS / THRESHOLD ASSERTIONS # @@ -149,7 +149,7 @@ Teaspoon.configure do |config| # Specify that you always want a coverage configuration to be used. Otherwise, specify that you want coverage # on the CLI. # Set this to "true" or the name of your coverage config. - #config.use_coverage = nil + # config.use_coverage = nil # You can have multiple coverage configs by passing a name to config.coverage. # e.g. config.coverage :ci do |coverage| @@ -158,21 +158,21 @@ Teaspoon.configure do |config| # Which coverage reports Istanbul should generate. Correlates directly to what Istanbul supports. # # Available: text-summary, text, html, lcov, lcovonly, cobertura, teamcity - #coverage.reports = ["text-summary", "html"] + # coverage.reports = ["text-summary", "html"] # The path that the coverage should be written to - when there's an artifact to write to disk. # Note: Relative to `config.root`. - #coverage.output_path = "coverage" + # coverage.output_path = "coverage" # Assets to be ignored when generating coverage reports. Accepts an array of filenames or regular expressions. The # default excludes assets from vendor, gems and support libraries. - #coverage.ignore = [%r{/lib/ruby/gems/}, %r{/vendor/assets/}, %r{/support/}, %r{/(.+)_helper.}] + # coverage.ignore = [%r{/lib/ruby/gems/}, %r{/vendor/assets/}, %r{/support/}, %r{/(.+)_helper.}] # Various thresholds requirements can be defined, and those thresholds will be checked at the end of a run. If any # aren't met the run will fail with a message. Thresholds can be defined as a percentage (0-100), or nil. - #coverage.statements = nil - #coverage.functions = nil - #coverage.branches = nil - #coverage.lines = nil + # coverage.statements = nil + # coverage.functions = nil + # coverage.branches = nil + # coverage.lines = nil end end |