diff options
45 files changed, 743 insertions, 375 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 9f179efa3ce..ccceea45963 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -937,10 +937,9 @@ Lint/Void: ##################### Performance ############################ -# TODO: Enable Casecmp Cop. # Use `casecmp` rather than `downcase ==`. Performance/Casecmp: - Enabled: false + Enabled: true # TODO: Enable DoubleStartEndWith Cop. # Use `str.{start,end}_with?(x, ..., y, ...)` instead of @@ -990,11 +989,12 @@ Performance/RedundantSortBy: # string. Performance/StartWith: Enabled: false + # Use `tr` instead of `gsub` when you are replacing the same number of # characters. Use `delete` instead of `gsub` when you are deleting # characters. Performance/StringReplacement: - Enabled: false + Enabled: true # TODO: Enable TimesMap Cop. # Checks for `.times.map` calls. diff --git a/CHANGELOG b/CHANGELOG index 7d5f424eaec..4841361482d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.8.0 (unreleased) + - Fix error when using link to uploads in global snippets - Assign labels and milestone to target project when moving issue. !3934 (Long Nguyen) - Use a case-insensitive comparison in sanitizing URI schemes - Project#open_branches has been cleaned up and no longer loads entire records into memory. @@ -20,6 +21,7 @@ v 8.8.0 (unreleased) - Fix error when visiting commit builds page before build was updated - Add 'l' shortcut to open Label dropdown on issuables and 'i' to create new issue on a project - Update SVG sanitizer to conform to SVG 1.1 + - Speed up push emails with multiple recipients by only generating the email once - Updated search UI - Display informative message when new milestone is created - Sanitize milestones and labels titles @@ -31,6 +33,7 @@ v 8.8.0 (unreleased) - API: Expose Issue#user_notes_count. !3126 (Anton Popov) - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718 - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes) + - Add eager load paths to help prevent dependency load issues in Sidekiq workers. !3724 - Added multiple colors for labels in dropdowns when dups happen. - Improve description for the Two-factor Authentication sign-in screen. (Connor Shea) - API support for the 'since' and 'until' operators on commit requests (Paco Guzman) @@ -38,10 +41,16 @@ v 8.8.0 (unreleased) - Expire repository exists? and has_visible_content? caches after a push if necessary - Fix unintentional filtering bug in issues sorted by milestone due (Takuya Noguchi) - Fix adding a todo for private group members (Ahmad Sherif) + - Bump ace-rails-ap gem version from 2.0.1 to 4.0.2 which upgrades Ace Editor from 1.1.2 to 1.2.3 v 8.7.4 - - Fix always showing build notification message when switching between merge requests - - Links for Redmine issue references are generated correctly again (Benedikt Huss) + - Links for Redmine issue references are generated correctly again !4048 (Benedikt Huss) + - Fix setting trusted proxies !3970 + - Fix BitBucket importer bug when throwing exceptions !3941 + - Use sign out path only if not empty !3989 + - Running rake gitlab:db:drop_tables now drops tables with cascade !4020 + - Running rake gitlab:db:drop_tables uses "IF EXISTS" as a precaution !4100 + - Use a case-insensitive comparison in sanitizing URI schemes v 8.7.3 - Emails, Gitlab::Email::Message, Gitlab::Diff, and Premailer::Adapter::Nokogiri are now instrumented @@ -197,7 +197,7 @@ gem 'licensee', '~> 8.0.0' gem "rack-attack", '~> 4.3.1' # Ace editor -gem 'ace-rails-ap', '~> 2.0.1' +gem 'ace-rails-ap', '~> 4.0.2' # Keyboard shortcuts gem 'mousetrap-rails', '~> 1.4.6' @@ -218,7 +218,6 @@ gem 'gitlab_emoji', '~> 0.3.0' gem 'gon', '~> 6.0.1' gem 'jquery-atwho-rails', '~> 1.3.2' gem 'jquery-rails', '~> 4.1.0' -gem 'jquery-scrollto-rails', '~> 1.4.3' gem 'jquery-ui-rails', '~> 5.0.0' gem 'raphael-rails', '~> 2.1.2' gem 'request_store', '~> 1.3.0' diff --git a/Gemfile.lock b/Gemfile.lock index 86b9142ef27..bc47533e5bb 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -3,7 +3,7 @@ GEM specs: CFPropertyList (2.3.2) RedCloth (4.2.9) - ace-rails-ap (2.0.1) + ace-rails-ap (4.0.2) actionmailer (4.2.6) actionpack (= 4.2.6) actionview (= 4.2.6) @@ -432,8 +432,6 @@ GEM rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) - jquery-scrollto-rails (1.4.3) - railties (> 3.1, < 5.0) jquery-turbolinks (2.1.0) railties (>= 3.1.0) turbolinks @@ -882,7 +880,7 @@ PLATFORMS DEPENDENCIES RedCloth (~> 4.2.9) - ace-rails-ap (~> 2.0.1) + ace-rails-ap (~> 4.0.2) activerecord-deprecated_finders (~> 1.0.3) activerecord-session_store (~> 0.1.0) acts-as-taggable-on (~> 3.4) @@ -953,7 +951,6 @@ DEPENDENCIES influxdb (~> 0.2) jquery-atwho-rails (~> 1.3.2) jquery-rails (~> 4.1.0) - jquery-scrollto-rails (~> 1.4.3) jquery-turbolinks (~> 2.1.0) jquery-ui-rails (~> 5.0.0) kaminari (~> 0.16.3) diff --git a/app/assets/javascripts/ci/application.js.coffee b/app/assets/javascripts/ci/application.js.coffee index 05aa0f366bb..ca24c1d759f 100644 --- a/app/assets/javascripts/ci/application.js.coffee +++ b/app/assets/javascripts/ci/application.js.coffee @@ -1,34 +1,6 @@ -# This is a manifest file that'll be compiled into application.js, which will include all the files -# listed below. -# -# Any JavaScript/Coffee file within this directory, lib/assets/javascripts, vendor/assets/javascripts, -# or vendor/assets/javascripts of plugins, if any, can be referenced here using a relative path. -# -# It's not advisable to add code directly here, but if you do, it'll appear at the bottom of the -# the compiled file. -# -# WARNING: THE FIRST BLANK LINE MARKS THE END OF WHAT'S TO BE PROCESSED, ANY BLANK LINE SHOULD -# GO AFTER THE REQUIRES BELOW. -# #= require pager #= require jquery_nested_form #= require_tree . -# -$(document).on 'click', '.edit-runner-link', (event) -> - event.preventDefault() - - descr = $(this).closest('.runner-description').first() - descr.addClass('hide') - form = descr.next('.runner-description-form') - descrInput = form.find('input.description') - originalValue = descrInput.val() - form.removeClass('hide') - form.find('.cancel').on 'click', (event) -> - event.preventDefault() - - form.addClass('hide') - descrInput.val(originalValue) - descr.removeClass('hide') $(document).on 'click', '.assign-all-runner', -> $(this).replaceWith('<i class="fa fa-refresh fa-spin"></i> Assign in progress..') diff --git a/app/assets/javascripts/shortcuts_issuable.coffee b/app/assets/javascripts/shortcuts_issuable.coffee index ad9b3c1c6bf..ccb42ab2168 100644 --- a/app/assets/javascripts/shortcuts_issuable.coffee +++ b/app/assets/javascripts/shortcuts_issuable.coffee @@ -6,6 +6,10 @@ class @ShortcutsIssuable extends ShortcutsNavigation super() Mousetrap.bind('a', @openSidebarDropdown.bind(@, 'assignee')) Mousetrap.bind('m', @openSidebarDropdown.bind(@, 'milestone')) + Mousetrap.bind('r', => + @replyWithSelectedText() + return false + ) Mousetrap.bind('j', => @prevIssue() return false diff --git a/app/controllers/dashboard/labels_controller.rb b/app/controllers/dashboard/labels_controller.rb index 23a4ef21ea2..2a88350a4ca 100644 --- a/app/controllers/dashboard/labels_controller.rb +++ b/app/controllers/dashboard/labels_controller.rb @@ -1,6 +1,6 @@ class Dashboard::LabelsController < Dashboard::ApplicationController def index - labels = Label.where(project_id: projects).select(:title, :color).uniq(:title) + labels = Label.where(project_id: projects).select(:id, :title, :color).uniq(:title) respond_to do |format| format.json { render json: labels } diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb index 71acc244a91..c08eb811532 100644 --- a/app/controllers/dashboard/projects_controller.rb +++ b/app/controllers/dashboard/projects_controller.rb @@ -28,7 +28,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController end def starred - @projects = current_user.starred_projects.sorted_by_activity + @projects = current_user.viewable_starred_projects.sorted_by_activity @projects = filter_projects(@projects) @projects = @projects.includes(:namespace, :forked_from_project, :tags) @projects = @projects.sort(@sort = params[:sort]) diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 1dce4a21729..4dda4e51f6a 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -25,7 +25,7 @@ class DashboardController < Dashboard::ApplicationController def load_events projects = if params[:filter] == "starred" - current_user.starred_projects + current_user.viewable_starred_projects else current_user.authorized_projects end diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb index dfa9bd259e8..47524b1cf0b 100644 --- a/app/controllers/projects/hooks_controller.rb +++ b/app/controllers/projects/hooks_controller.rb @@ -27,8 +27,10 @@ class Projects::HooksController < Projects::ApplicationController if !@project.empty_repo? status, message = TestHookService.new.execute(hook, current_user) - if status - flash[:notice] = 'Hook successfully executed.' + if status && status >= 200 && status < 400 + flash[:notice] = "Hook executed successfully: HTTP #{status}" + elsif status + flash[:alert] = "Hook executed successfully but returned HTTP #{status} #{message}" else flash[:alert] = "Hook execution failed: #{message}" end diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index 377c2999d6c..5489283432b 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -59,9 +59,9 @@ module Emails subject: subject("Project was moved")) end - def repository_push_email(project_id, recipient, opts = {}) + def repository_push_email(project_id, opts = {}) @message = - Gitlab::Email::Message::RepositoryPush.new(self, project_id, recipient, opts) + Gitlab::Email::Message::RepositoryPush.new(self, project_id, opts) # used in notify layout @target_url = @message.target_url @@ -72,7 +72,6 @@ module Emails mail(from: sender(@message.author_id, @message.send_from_committer_email?), reply_to: @message.reply_to, - to: @message.recipient, subject: @message.subject) end end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index fde05f729dc..8b87b6c3d64 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -38,7 +38,7 @@ class WebHook < ActiveRecord::Base basic_auth: auth) end - [(response.code >= 200 && response.code < 300), ActionView::Base.full_sanitizer.sanitize(response.to_s)] + [response.code, response.to_s] rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e logger.error("WebHook Error => #{e}") [false, e.to_s] diff --git a/app/models/repository.rb b/app/models/repository.rb index 7aebfe279fb..de7e163078d 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -81,7 +81,7 @@ class Repository def commit(id = 'HEAD') return nil unless exists? commit = Gitlab::Git::Commit.find(raw_repository, id) - commit = Commit.new(commit, @project) if commit + commit = ::Commit.new(commit, @project) if commit commit rescue Rugged::OdbError nil @@ -453,7 +453,7 @@ class Repository def version cache.fetch(:version) do tree(:head).blobs.find do |file| - file.name.downcase == 'version' + file.name.casecmp('version').zero? end end end diff --git a/app/models/user.rb b/app/models/user.rb index 37179c10788..5ca53e7c64a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -382,6 +382,11 @@ class User < ActiveRecord::Base Project.where("projects.id IN (#{projects_union.to_sql})") end + def viewable_starred_projects + starred_projects.where("projects.visibility_level IN (?) OR projects.id IN (#{projects_union.to_sql})", + [Project::PUBLIC, Project::INTERNAL]) + end + def owned_projects @owned_projects ||= Project.where('namespace_id IN (?) OR namespace_id = ?', diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 19aab999e00..48a6131b444 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -35,7 +35,7 @@ module Projects end end - log_info("Project \"#{project.name}\" was removed") + log_info("Project \"#{project.path_with_namespace}\" was removed") system_hook_service.execute_hooks_for(project, :destroy) true end diff --git a/app/views/admin/runners/_runner.html.haml b/app/views/admin/runners/_runner.html.haml index 6745e58deca..36b21eefdee 100644 --- a/app/views/admin/runners/_runner.html.haml +++ b/app/views/admin/runners/_runner.html.haml @@ -11,18 +11,10 @@ = link_to admin_runner_path(runner) do = runner.short_sha %td - .runner-description - = runner.description - %span (#{link_to 'edit', '#', class: 'edit-runner-link'}) - .runner-description-form.hide - = form_for [:admin, runner], remote: true, html: { class: 'form-inline' } do |f| - .form-group - = f.text_field :description, class: 'form-control' - = f.submit 'Save', class: 'btn' - %span (#{link_to 'cancel', '#', class: 'cancel'}) + = runner.description %td - if runner.shared? - \- + n/a - else = runner.projects.count(:all) %td diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index 172579dafda..c33740e23fa 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -27,8 +27,9 @@ %li = link_to dashboard_todos_path, title: 'Todos', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do = icon('bell fw') - %span.badge.todos-pending-count - = todos_pending_count + - unless todos_pending_count == 0 + %span.badge.todos-pending-count + = todos_pending_count - if current_user.can_create_project? %li = link_to new_project_path, title: 'New project', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb index c4d8595d45d..6ebcba5f39b 100644 --- a/app/workers/emails_on_push_worker.rb +++ b/app/workers/emails_on_push_worker.rb @@ -1,6 +1,8 @@ class EmailsOnPushWorker include Sidekiq::Worker + attr_reader :email, :skip_premailer + def perform(project_id, recipients, push_data, options = {}) options.symbolize_keys! options.reverse_merge!( @@ -41,11 +43,11 @@ class EmailsOnPushWorker end end - recipients.split(" ").each do |recipient| + recipients.split.each do |recipient| begin - Notify.repository_push_email( - project_id, + send_email( recipient, + project_id, author_id: author_id, ref: ref, action: action, @@ -53,14 +55,29 @@ class EmailsOnPushWorker reverse_compare: reverse_compare, send_from_committer_email: send_from_committer_email, disable_diffs: disable_diffs - ).deliver_now + ) + # These are input errors and won't be corrected even if Sidekiq retries rescue Net::SMTPFatalError, Net::SMTPSyntaxError => e logger.info("Failed to send e-mail for project '#{project.name_with_namespace}' to #{recipient}: #{e}") end end ensure + @email = nil compare = nil GC.start end + + private + + def send_email(recipient, project_id, options) + # Generating the body of this email can be expensive, so only do it once + @skip_premailer ||= email.present? + @email ||= Notify.repository_push_email(project_id, options) + + email.to = recipient + email.add_message_id + email.header[:skip_premailer] = true if skip_premailer + email.deliver_now + end end diff --git a/config/application.rb b/config/application.rb index b602e2b6168..cba80f38f1f 100644 --- a/config/application.rb +++ b/config/application.rb @@ -1,23 +1,30 @@ require File.expand_path('../boot', __FILE__) require 'rails/all' -require 'devise' -I18n.config.enforce_available_locales = false + Bundler.require(:default, Rails.env) -require_relative '../lib/gitlab/redis' module Gitlab class Application < Rails::Application + require_dependency Rails.root.join('lib/gitlab/redis') + # Settings in config/environments/* take precedence over those specified here. # Application configuration should go into files in config/initializers # -- all .rb files in that directory are automatically loaded. - # Custom directories with classes and modules you want to be autoloadable. - config.autoload_paths.push(*%W(#{config.root}/lib - #{config.root}/app/models/hooks - #{config.root}/app/models/concerns - #{config.root}/app/models/project_services - #{config.root}/app/models/members)) + # Sidekiq uses eager loading, but directories not in the standard Rails + # directories must be added to the eager load paths: + # https://github.com/mperham/sidekiq/wiki/FAQ#why-doesnt-sidekiq-autoload-my-rails-application-code + # Also, there is no need to add `lib` to autoload_paths since autoloading is + # configured to check for eager loaded paths: + # https://github.com/rails/rails/blob/v4.2.6/railties/lib/rails/engine.rb#L687 + # This is a nice reference article on autoloading/eager loading: + # http://blog.arkency.com/2014/11/dont-forget-about-eager-load-when-extending-autoload + config.eager_load_paths.push(*%W(#{config.root}/lib + #{config.root}/app/models/ci + #{config.root}/app/models/hooks + #{config.root}/app/models/members + #{config.root}/app/models/project_services)) # Only load the plugins named here, in the order given (default is alphabetical). # :all can be used as a placeholder for all plugins not explicitly named. diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index 8db2c05fe45..23c8cea038a 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -1,4 +1,4 @@ -require 'gitlab' # Load lib/gitlab.rb as soon as possible +require_dependency Rails.root.join('lib/gitlab') # Load Gitlab as soon as possible class Settings < Settingslogic source ENV.fetch('GITLAB_CONFIG') { "#{Rails.root}/config/gitlab.yml" } diff --git a/config/initializers/5_backend.rb b/config/initializers/5_backend.rb index 80d641d73a3..e026151a032 100644 --- a/config/initializers/5_backend.rb +++ b/config/initializers/5_backend.rb @@ -1,11 +1,11 @@ # GIT over HTTP -require Rails.root.join("lib", "gitlab", "backend", "grack_auth") +require_dependency Rails.root.join('lib/gitlab/backend/grack_auth') # GIT over SSH -require Rails.root.join("lib", "gitlab", "backend", "shell") +require_dependency Rails.root.join('lib/gitlab/backend/shell') # GitLab shell adapter -require Rails.root.join("lib", "gitlab", "backend", "shell_adapter") +require_dependency Rails.root.join('lib/gitlab/backend/shell_adapter') required_version = Gitlab::VersionInfo.parse(Gitlab::Shell.version_required) current_version = Gitlab::VersionInfo.parse(Gitlab::Shell.new.version) diff --git a/config/initializers/monkey_patch.rb b/config/initializers/monkey_patch.rb deleted file mode 100644 index 62b05a55285..00000000000 --- a/config/initializers/monkey_patch.rb +++ /dev/null @@ -1,48 +0,0 @@ -## This patch is from rails 4.2-stable. Remove it when 4.2.6 is released -## https://github.com/rails/rails/issues/21108 - -module ActiveRecord - module ConnectionAdapters - class AbstractMysqlAdapter < AbstractAdapter - # SHOW VARIABLES LIKE 'name' - def show_variable(name) - variables = select_all("select @@#{name} as 'Value'", 'SCHEMA') - variables.first['Value'] unless variables.empty? - rescue ActiveRecord::StatementInvalid - nil - end - - - # MySQL is too stupid to create a temporary table for use subquery, so we have - # to give it some prompting in the form of a subsubquery. Ugh! - def subquery_for(key, select) - subsubselect = select.clone - subsubselect.projections = [key] - - subselect = Arel::SelectManager.new(select.engine) - subselect.project Arel.sql(key.name) - # Materialized subquery by adding distinct - # to work with MySQL 5.7.6 which sets optimizer_switch='derived_merge=on' - subselect.from subsubselect.distinct.as('__active_record_temp') - end - end - end -end - -module ActiveRecord - module ConnectionAdapters - class MysqlAdapter < AbstractMysqlAdapter - ADAPTER_NAME = 'MySQL'.freeze - - # Get the client encoding for this database - def client_encoding - return @client_encoding if @client_encoding - - result = exec_query( - "select @@character_set_client", - 'SCHEMA') - @client_encoding = ENCODINGS[result.rows.last.last] - end - end - end -end diff --git a/doc/web_hooks/web_hooks.md b/doc/web_hooks/web_hooks.md index c1c51302e79..45506ac1d7c 100644 --- a/doc/web_hooks/web_hooks.md +++ b/doc/web_hooks/web_hooks.md @@ -13,6 +13,19 @@ You can configure webhooks to listen for specific events like pushes, issues or Webhooks can be used to update an external issue tracker, trigger CI builds, update a backup mirror, or even deploy to your production server. +## Webhook endpoint tips + +If you are writing your own endpoint (web server) that will receive +GitLab webhooks keep in mind the following things: + +- Your endpoint should send its HTTP response as fast as possible. If + you wait too long, GitLab may decide the hook failed and retry it. +- Your endpoint should ALWAYS return a valid HTTP response. If you do + not do this then GitLab will think the hook failed and retry it. + Most HTTP libraries take care of this for you automatically but if + you are writing a low-level hook this is important to remember. +- GitLab ignores the HTTP status code returned by your endpoint. + ## SSL Verification By default, the SSL certificate of the webhook endpoint is verified based on diff --git a/features/steps/project/hooks.rb b/features/steps/project/hooks.rb index b1ffe7f7b4c..13c0713669a 100644 --- a/features/steps/project/hooks.rb +++ b/features/steps/project/hooks.rb @@ -59,7 +59,7 @@ class Spinach::Features::ProjectHooks < Spinach::FeatureSteps step 'hook should be triggered' do expect(current_path).to eq namespace_project_hooks_path(current_project.namespace, current_project) expect(page).to have_selector '.flash-notice', - text: 'Hook successfully executed.' + text: 'Hook executed successfully: HTTP 200' end step 'I should see hook error message' do diff --git a/lib/api/api.rb b/lib/api/api.rb index cc1004f8005..5fd9c30cb42 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -1,5 +1,3 @@ -Dir["#{Rails.root}/lib/api/*.rb"].each {|file| require file} - module API class API < Grape::API include APIGuard @@ -25,38 +23,39 @@ module API format :json content_type :txt, "text/plain" - helpers Helpers - - mount Groups - mount GroupMembers - mount Users - mount Projects - mount Repositories - mount Issues - mount Milestones - mount Session - mount MergeRequests - mount Notes - mount Internal - mount SystemHooks - mount ProjectSnippets - mount ProjectMembers - mount DeployKeys - mount ProjectHooks - mount Services - mount Files - mount Commits - mount CommitStatus - mount Namespaces - mount Branches - mount Labels - mount Settings - mount Keys - mount Tags - mount Triggers - mount Builds - mount Variables - mount Runners - mount Licenses + # Ensure the namespace is right, otherwise we might load Grape::API::Helpers + helpers ::API::Helpers + + mount ::API::Groups + mount ::API::GroupMembers + mount ::API::Users + mount ::API::Projects + mount ::API::Repositories + mount ::API::Issues + mount ::API::Milestones + mount ::API::Session + mount ::API::MergeRequests + mount ::API::Notes + mount ::API::Internal + mount ::API::SystemHooks + mount ::API::ProjectSnippets + mount ::API::ProjectMembers + mount ::API::DeployKeys + mount ::API::ProjectHooks + mount ::API::Services + mount ::API::Files + mount ::API::Commits + mount ::API::CommitStatuses + mount ::API::Namespaces + mount ::API::Branches + mount ::API::Labels + mount ::API::Settings + mount ::API::Keys + mount ::API::Tags + mount ::API::Triggers + mount ::API::Builds + mount ::API::Variables + mount ::API::Runners + mount ::API::Licenses end end diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index b9994fcefda..7e67edb203a 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -2,171 +2,175 @@ require 'rack/oauth2' -module APIGuard - extend ActiveSupport::Concern +module API + module APIGuard + extend ActiveSupport::Concern - included do |base| - # OAuth2 Resource Server Authentication - use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| - # The authenticator only fetches the raw token string + included do |base| + # OAuth2 Resource Server Authentication + use Rack::OAuth2::Server::Resource::Bearer, 'The API' do |request| + # The authenticator only fetches the raw token string - # Must yield access token to store it in the env - request.access_token - end + # Must yield access token to store it in the env + request.access_token + end - helpers HelperMethods + helpers HelperMethods - install_error_responders(base) - end + install_error_responders(base) + end - # Helper Methods for Grape Endpoint - module HelperMethods - # Invokes the doorkeeper guard. - # - # If token is presented and valid, then it sets @current_user. - # - # If the token does not have sufficient scopes to cover the requred scopes, - # then it raises InsufficientScopeError. - # - # If the token is expired, then it raises ExpiredError. - # - # If the token is revoked, then it raises RevokedError. - # - # If the token is not found (nil), then it raises TokenNotFoundError. - # - # Arguments: - # - # scopes: (optional) scopes required for this guard. - # Defaults to empty array. - # - def doorkeeper_guard!(scopes: []) - if (access_token = find_access_token).nil? - raise TokenNotFoundError - - else - case validate_access_token(access_token, scopes) - when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE - raise InsufficientScopeError.new(scopes) - when Oauth2::AccessTokenValidationService::EXPIRED - raise ExpiredError - when Oauth2::AccessTokenValidationService::REVOKED - raise RevokedError - when Oauth2::AccessTokenValidationService::VALID - @current_user = User.find(access_token.resource_owner_id) + # Helper Methods for Grape Endpoint + module HelperMethods + # Invokes the doorkeeper guard. + # + # If token is presented and valid, then it sets @current_user. + # + # If the token does not have sufficient scopes to cover the requred scopes, + # then it raises InsufficientScopeError. + # + # If the token is expired, then it raises ExpiredError. + # + # If the token is revoked, then it raises RevokedError. + # + # If the token is not found (nil), then it raises TokenNotFoundError. + # + # Arguments: + # + # scopes: (optional) scopes required for this guard. + # Defaults to empty array. + # + def doorkeeper_guard!(scopes: []) + if (access_token = find_access_token).nil? + raise TokenNotFoundError + + else + case validate_access_token(access_token, scopes) + when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE + raise InsufficientScopeError.new(scopes) + when Oauth2::AccessTokenValidationService::EXPIRED + raise ExpiredError + when Oauth2::AccessTokenValidationService::REVOKED + raise RevokedError + when Oauth2::AccessTokenValidationService::VALID + @current_user = User.find(access_token.resource_owner_id) + end end end - end - def doorkeeper_guard(scopes: []) - if access_token = find_access_token - case validate_access_token(access_token, scopes) - when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE - raise InsufficientScopeError.new(scopes) + def doorkeeper_guard(scopes: []) + if access_token = find_access_token + case validate_access_token(access_token, scopes) + when Oauth2::AccessTokenValidationService::INSUFFICIENT_SCOPE + raise InsufficientScopeError.new(scopes) - when Oauth2::AccessTokenValidationService::EXPIRED - raise ExpiredError + when Oauth2::AccessTokenValidationService::EXPIRED + raise ExpiredError - when Oauth2::AccessTokenValidationService::REVOKED - raise RevokedError + when Oauth2::AccessTokenValidationService::REVOKED + raise RevokedError - when Oauth2::AccessTokenValidationService::VALID - @current_user = User.find(access_token.resource_owner_id) + when Oauth2::AccessTokenValidationService::VALID + @current_user = User.find(access_token.resource_owner_id) + end end end - end - def current_user - @current_user - end + def current_user + @current_user + end - private - def find_access_token - @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods) - end + private - def doorkeeper_request - @doorkeeper_request ||= ActionDispatch::Request.new(env) - end + def find_access_token + @access_token ||= Doorkeeper.authenticate(doorkeeper_request, Doorkeeper.configuration.access_token_methods) + end - def validate_access_token(access_token, scopes) - Oauth2::AccessTokenValidationService.validate(access_token, scopes: scopes) - end - end + def doorkeeper_request + @doorkeeper_request ||= ActionDispatch::Request.new(env) + end - module ClassMethods - # Installs the doorkeeper guard on the whole Grape API endpoint. - # - # Arguments: - # - # scopes: (optional) scopes required for this guard. - # Defaults to empty array. - # - def guard_all!(scopes: []) - before do - guard! scopes: scopes + def validate_access_token(access_token, scopes) + Oauth2::AccessTokenValidationService.validate(access_token, scopes: scopes) end end - private - def install_error_responders(base) - error_classes = [ MissingTokenError, TokenNotFoundError, - ExpiredError, RevokedError, InsufficientScopeError] + module ClassMethods + # Installs the doorkeeper guard on the whole Grape API endpoint. + # + # Arguments: + # + # scopes: (optional) scopes required for this guard. + # Defaults to empty array. + # + def guard_all!(scopes: []) + before do + guard! scopes: scopes + end + end - base.send :rescue_from, *error_classes, oauth2_bearer_token_error_handler - end + private - def oauth2_bearer_token_error_handler - Proc.new do |e| - response = - case e - when MissingTokenError - Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new - - when TokenNotFoundError - Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( - :invalid_token, - "Bad Access Token.") - - when ExpiredError - Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( - :invalid_token, - "Token is expired. You can either do re-authorization or token refresh.") - - when RevokedError - Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( - :invalid_token, - "Token was revoked. You have to re-authorize from the user.") - - when InsufficientScopeError - # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2) - # does not include WWW-Authenticate header, which breaks the standard. - Rack::OAuth2::Server::Resource::Bearer::Forbidden.new( - :insufficient_scope, - Rack::OAuth2::Server::Resource::ErrorMethods::DEFAULT_DESCRIPTION[:insufficient_scope], - { scope: e.scopes }) - end + def install_error_responders(base) + error_classes = [ MissingTokenError, TokenNotFoundError, + ExpiredError, RevokedError, InsufficientScopeError] - response.finish + base.send :rescue_from, *error_classes, oauth2_bearer_token_error_handler + end + + def oauth2_bearer_token_error_handler + Proc.new do |e| + response = + case e + when MissingTokenError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new + + when TokenNotFoundError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( + :invalid_token, + "Bad Access Token.") + + when ExpiredError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( + :invalid_token, + "Token is expired. You can either do re-authorization or token refresh.") + + when RevokedError + Rack::OAuth2::Server::Resource::Bearer::Unauthorized.new( + :invalid_token, + "Token was revoked. You have to re-authorize from the user.") + + when InsufficientScopeError + # FIXME: ForbiddenError (inherited from Bearer::Forbidden of Rack::Oauth2) + # does not include WWW-Authenticate header, which breaks the standard. + Rack::OAuth2::Server::Resource::Bearer::Forbidden.new( + :insufficient_scope, + Rack::OAuth2::Server::Resource::ErrorMethods::DEFAULT_DESCRIPTION[:insufficient_scope], + { scope: e.scopes }) + end + + response.finish + end end end - end - # - # Exceptions - # + # + # Exceptions + # - class MissingTokenError < StandardError; end + class MissingTokenError < StandardError; end - class TokenNotFoundError < StandardError; end + class TokenNotFoundError < StandardError; end - class ExpiredError < StandardError; end + class ExpiredError < StandardError; end - class RevokedError < StandardError; end + class RevokedError < StandardError; end - class InsufficientScopeError < StandardError - attr_reader :scopes - def initialize(scopes) - @scopes = scopes + class InsufficientScopeError < StandardError + attr_reader :scopes + def initialize(scopes) + @scopes = scopes + end end end end diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb index 7388ed2f4ea..9bcd33ff19e 100644 --- a/lib/api/commit_statuses.rb +++ b/lib/api/commit_statuses.rb @@ -2,7 +2,7 @@ require 'mime/types' module API # Project commit statuses API - class CommitStatus < Grape::API + class CommitStatuses < Grape::API resource :projects do before { authenticate! } diff --git a/lib/api/projects.rb b/lib/api/projects.rb index cc2c7a0c503..9b595772675 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -44,7 +44,7 @@ module API # Example Request: # GET /projects/starred get '/starred' do - @projects = current_user.starred_projects + @projects = current_user.viewable_starred_projects @projects = filter_projects(@projects) @projects = paginate @projects present @projects, with: Entities::Project diff --git a/lib/banzai/filter/upload_link_filter.rb b/lib/banzai/filter/upload_link_filter.rb index 7edfe5ade2d..c0f503c9af3 100644 --- a/lib/banzai/filter/upload_link_filter.rb +++ b/lib/banzai/filter/upload_link_filter.rb @@ -8,6 +8,8 @@ module Banzai # class UploadLinkFilter < HTML::Pipeline::Filter def call + return doc unless project + doc.search('a').each do |el| process_link_attr el.attribute('href') end @@ -31,7 +33,11 @@ module Banzai end def build_url(uri) - File.join(Gitlab.config.gitlab.url, context[:project].path_with_namespace, uri) + File.join(Gitlab.config.gitlab.url, project.path_with_namespace, uri) + end + + def project + context[:project] end # Ensure that a :project key exists in context diff --git a/lib/ci/api/api.rb b/lib/ci/api/api.rb index 353c4ddebf8..17bb99a2ae5 100644 --- a/lib/ci/api/api.rb +++ b/lib/ci/api/api.rb @@ -1,9 +1,7 @@ -Dir["#{Rails.root}/lib/ci/api/*.rb"].each {|file| require file} - module Ci module API class API < Grape::API - include APIGuard + include ::API::APIGuard version 'v1', using: :path rescue_from ActiveRecord::RecordNotFound do @@ -31,9 +29,9 @@ module Ci helpers ::API::Helpers helpers Gitlab::CurrentSettings - mount Builds - mount Runners - mount Triggers + mount ::Ci::API::Builds + mount ::Ci::API::Runners + mount ::Ci::API::Triggers end end end diff --git a/lib/gitlab.rb b/lib/gitlab.rb index 7479e729db1..37f4c34054f 100644 --- a/lib/gitlab.rb +++ b/lib/gitlab.rb @@ -1,4 +1,4 @@ -require 'gitlab/git' +require_dependency 'gitlab/git' module Gitlab def self.com? diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 6f9da69983a..42bec913a45 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -5,11 +5,11 @@ module Gitlab end def self.mysql? - adapter_name.downcase == 'mysql2' + adapter_name.casecmp('mysql2').zero? end def self.postgresql? - adapter_name.downcase == 'postgresql' + adapter_name.casecmp('postgresql').zero? end def self.version diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb index d0815fc7eea..6fe7faa547a 100644 --- a/lib/gitlab/diff/parser.rb +++ b/lib/gitlab/diff/parser.rb @@ -18,7 +18,7 @@ module Gitlab @lines.each do |line| next if filename?(line) - full_line = line.gsub(/\n/, '') + full_line = line.delete("\n") if line.match(/^@@ -/) type = "match" diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index 8f9be6cd9a3..2c91a0487c3 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -2,7 +2,6 @@ module Gitlab module Email module Message class RepositoryPush - attr_accessor :recipient attr_reader :author_id, :ref, :action include Gitlab::Routing.url_helpers @@ -11,13 +10,12 @@ module Gitlab delegate :name, to: :author, prefix: :author delegate :username, to: :author, prefix: :author - def initialize(notify, project_id, recipient, opts = {}) + def initialize(notify, project_id, opts = {}) raise ArgumentError, 'Missing options: author_id, ref, action' unless opts[:author_id] && opts[:ref] && opts[:action] @notify = notify @project_id = project_id - @recipient = recipient @opts = opts.dup @author_id = @opts.delete(:author_id) diff --git a/lib/gitlab/markup_helper.rb b/lib/gitlab/markup_helper.rb index a5f767b134d..dda371e6554 100644 --- a/lib/gitlab/markup_helper.rb +++ b/lib/gitlab/markup_helper.rb @@ -40,7 +40,7 @@ module Gitlab # Returns boolean def plain?(filename) filename.downcase.end_with?('.txt') || - filename.downcase == 'readme' + filename.casecmp('readme').zero? end def previewable?(filename) diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb new file mode 100644 index 00000000000..cc7f78e7325 --- /dev/null +++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb @@ -0,0 +1,137 @@ +require 'spec_helper' + +describe 'Projects > Merge requests > User lists merge requests', feature: true do + include SortingHelper + + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + + before do + @fix = create(:merge_request, + title: 'fix', + source_project: project, + source_branch: 'fix', + assignee: user, + milestone: create(:milestone, due_date: '2013-12-11'), + created_at: 1.minute.ago, + updated_at: 1.minute.ago) + create(:merge_request, + title: 'markdown', + source_project: project, + source_branch: 'markdown', + assignee: user, + milestone: create(:milestone, due_date: '2013-12-12'), + created_at: 2.minutes.ago, + updated_at: 2.minutes.ago) + create(:merge_request, + title: 'lfs', + source_project: project, + source_branch: 'lfs', + created_at: 3.minutes.ago, + updated_at: 10.seconds.ago) + end + + it 'filters on no assignee' do + visit_merge_requests(project, assignee_id: IssuableFinder::NONE) + + expect(current_path).to eq(namespace_project_merge_requests_path(project.namespace, project)) + expect(page).to have_content 'lfs' + expect(page).not_to have_content 'fix' + expect(page).not_to have_content 'markdown' + end + + it 'filters on a specific assignee' do + visit_merge_requests(project, assignee_id: user.id) + + expect(page).not_to have_content 'lfs' + expect(page).to have_content 'fix' + expect(page).to have_content 'markdown' + end + + it 'sorts by newest' do + visit_merge_requests(project, sort: sort_value_recently_created) + + expect(first_merge_request).to include('lfs') + expect(last_merge_request).to include('fix') + end + + it 'sorts by oldest' do + visit_merge_requests(project, sort: sort_value_oldest_created) + + expect(first_merge_request).to include('fix') + expect(last_merge_request).to include('lfs') + end + + it 'sorts by last updated' do + visit_merge_requests(project, sort: sort_value_recently_updated) + + expect(first_merge_request).to include('lfs') + end + + it 'sorts by oldest updated' do + visit_merge_requests(project, sort: sort_value_oldest_updated) + + expect(first_merge_request).to include('markdown') + end + + it 'sorts by milestone due soon' do + visit_merge_requests(project, sort: sort_value_milestone_soon) + + expect(first_merge_request).to include('fix') + end + + it 'sorts by milestone due later' do + visit_merge_requests(project, sort: sort_value_milestone_later) + + expect(first_merge_request).to include('markdown') + end + + it 'filters on one label and sorts by due soon' do + label = create(:label, project: project) + create(:label_link, label: label, target: @fix) + + visit_merge_requests(project, label_name: [label.name], + sort: sort_value_due_date_soon) + + expect(first_merge_request).to include('fix') + end + + context 'while filtering on two labels' do + let(:label) { create(:label, project: project) } + let(:label2) { create(:label, project: project) } + + before do + create(:label_link, label: label, target: @fix) + create(:label_link, label: label2, target: @fix) + end + + it 'sorts by due soon' do + visit_merge_requests(project, label_name: [label.name, label2.name], + sort: sort_value_due_date_soon) + + expect(first_merge_request).to include('fix') + end + + context 'filter on assignee and' do + it 'sorts by due soon' do + visit_merge_requests(project, label_name: [label.name, label2.name], + assignee_id: user.id, + sort: sort_value_due_date_soon) + + expect(first_merge_request).to include('fix') + end + end + end + + def visit_merge_requests(project, opts = {}) + visit namespace_project_merge_requests_path(project.namespace, project, opts) + end + + def first_merge_request + page.all('ul.mr-list > li').first.text + end + + def last_merge_request + page.all('ul.mr-list > li').last.text + end +end diff --git a/spec/lib/banzai/filter/upload_link_filter_spec.rb b/spec/lib/banzai/filter/upload_link_filter_spec.rb index 3b073a90a95..b83be54746c 100644 --- a/spec/lib/banzai/filter/upload_link_filter_spec.rb +++ b/spec/lib/banzai/filter/upload_link_filter_spec.rb @@ -8,6 +8,10 @@ describe Banzai::Filter::UploadLinkFilter, lib: true do project: project }) + raw_filter(doc, contexts) + end + + def raw_filter(doc, contexts = {}) described_class.call(doc, contexts) end @@ -70,4 +74,18 @@ describe Banzai::Filter::UploadLinkFilter, lib: true do expect(doc.at_css('img')['src']).to match "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/%ED%95%9C%EA%B8%80.png" end end + + context 'when project context does not exist' do + let(:upload_link) { link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg') } + + it 'does not raise error' do + expect { raw_filter(upload_link, project: nil) }.not_to raise_error + end + + it 'does not rewrite link' do + doc = raw_filter(upload_link, project: nil) + + expect(doc.to_html).to eq upload_link + end + end end diff --git a/spec/lib/gitlab/email/message/repository_push_spec.rb b/spec/lib/gitlab/email/message/repository_push_spec.rb index b2d7a799810..7d6cce6daec 100644 --- a/spec/lib/gitlab/email/message/repository_push_spec.rb +++ b/spec/lib/gitlab/email/message/repository_push_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::Email::Message::RepositoryPush do let!(:author) { create(:author, name: 'Author') } let(:message) do - described_class.new(Notify, project.id, 'recipient@example.com', opts) + described_class.new(Notify, project.id, opts) end context 'new commits have been pushed to repository' do diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 495c5cbac00..5f7e4a526e6 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -593,7 +593,7 @@ describe Notify do let(:user) { create(:user) } let(:tree_path) { namespace_project_tree_path(project.namespace, project, "master") } - subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :create) } + subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :create) } it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" @@ -606,10 +606,6 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'is sent to recipient' do - is_expected.to deliver_to 'devs@company.name' - end - it 'has the correct subject' do is_expected.to have_subject /Pushed new branch master/ end @@ -624,7 +620,7 @@ describe Notify do let(:user) { create(:user) } let(:tree_path) { namespace_project_tree_path(project.namespace, project, "v1.0") } - subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/tags/v1.0', action: :create) } + subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/tags/v1.0', action: :create) } it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" @@ -637,10 +633,6 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'is sent to recipient' do - is_expected.to deliver_to 'devs@company.name' - end - it 'has the correct subject' do is_expected.to have_subject /Pushed new tag v1\.0/ end @@ -654,7 +646,7 @@ describe Notify do let(:example_site_path) { root_path } let(:user) { create(:user) } - subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :delete) } + subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :delete) } it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" @@ -667,10 +659,6 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'is sent to recipient' do - is_expected.to deliver_to 'devs@company.name' - end - it 'has the correct subject' do is_expected.to have_subject /Deleted branch master/ end @@ -680,7 +668,7 @@ describe Notify do let(:example_site_path) { root_path } let(:user) { create(:user) } - subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) } + subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/tags/v1.0', action: :delete) } it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" @@ -693,10 +681,6 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'is sent to recipient' do - is_expected.to deliver_to 'devs@company.name' - end - it 'has the correct subject' do is_expected.to have_subject /Deleted tag v1\.0/ end @@ -710,7 +694,7 @@ describe Notify do let(:diff_path) { namespace_project_compare_path(project.namespace, project, from: Commit.new(compare.base, project), to: Commit.new(compare.head, project)) } let(:send_from_committer_email) { false } - subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, send_from_committer_email: send_from_committer_email) } + subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare, reverse_compare: false, send_from_committer_email: send_from_committer_email) } it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" @@ -723,10 +707,6 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'is sent to recipient' do - is_expected.to deliver_to 'devs@company.name' - end - it 'has the correct subject' do is_expected.to have_subject /\[#{project.path_with_namespace}\]\[master\] #{commits.length} commits:/ end @@ -818,7 +798,7 @@ describe Notify do let(:commits) { Commit.decorate(compare.commits, nil) } let(:diff_path) { namespace_project_commit_path(project.namespace, project, commits.first) } - subject { Notify.repository_push_email(project.id, 'devs@company.name', author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare) } + subject { Notify.repository_push_email(project.id, author_id: user.id, ref: 'refs/heads/master', action: :push, compare: compare) } it_behaves_like 'it should show Gmail Actions View Commit link' it_behaves_like "a user cannot unsubscribe through footer link" @@ -831,10 +811,6 @@ describe Notify do expect(sender.address).to eq(gitlab_sender) end - it 'is sent to recipient' do - is_expected.to deliver_to 'devs@company.name' - end - it 'has the correct subject' do is_expected.to have_subject /#{commits.first.title}/ end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index 37a27d73aab..f9bab487b96 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -95,13 +95,13 @@ describe WebHook, models: true do it "handles 200 status code" do WebMock.stub_request(:post, project_hook.url).to_return(status: 200, body: "Success") - expect(project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success']) + expect(project_hook.execute(@data, 'push_hooks')).to eq([200, 'Success']) end it "handles 2xx status codes" do WebMock.stub_request(:post, project_hook.url).to_return(status: 201, body: "Success") - expect(project_hook.execute(@data, 'push_hooks')).to eq([true, 'Success']) + expect(project_hook.execute(@data, 'push_hooks')).to eq([201, 'Success']) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b60725756cc..e170cc85a1e 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -783,4 +783,23 @@ describe User, models: true do it { is_expected.to eq([private_project]) } end + + describe '#viewable_starred_projects' do + let(:user) { create(:user) } + let(:public_project) { create(:empty_project, :public) } + let(:private_project) { create(:empty_project, :private) } + let(:private_viewable_project) { create(:empty_project, :private) } + + before do + private_viewable_project.team << [user, Gitlab::Access::MASTER] + + [public_project, private_project, private_viewable_project].each do |project| + user.toggle_star(project) + end + end + + it 'returns only starred projects the user can view' do + expect(user.viewable_starred_projects).not_to include(private_project) + end + end end diff --git a/spec/requests/api/commit_status_spec.rb b/spec/requests/api/commit_statuses_spec.rb index f3785b19362..633927c8c3e 100644 --- a/spec/requests/api/commit_status_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe API::CommitStatus, api: true do +describe API::CommitStatuses, api: true do include ApiHelpers let!(:project) { create(:project) } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 66193eac051..f167813e07d 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -10,20 +10,20 @@ describe API::API, api: true do let(:admin) { create(:admin) } let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } let(:project2) { create(:project, path: 'project2', creator_id: user.id, namespace: user.namespace) } - let(:project3) { create(:project, path: 'project3', creator_id: user.id, namespace: user.namespace) } let(:snippet) { create(:project_snippet, :public, author: user, project: project, title: 'example') } let(:project_member) { create(:project_member, :master, user: user, project: project) } let(:project_member2) { create(:project_member, :developer, user: user3, project: project) } let(:user4) { create(:user) } let(:project3) do create(:project, + :private, name: 'second_project', path: 'second_project', creator_id: user.id, namespace: user.namespace, merge_requests_enabled: false, issues_enabled: false, wiki_enabled: false, - snippets_enabled: false, visibility_level: 0) + snippets_enabled: false) end let(:project_member3) do create(:project_member, @@ -164,21 +164,18 @@ describe API::API, api: true do end describe 'GET /projects/starred' do + let(:public_project) { create(:project, :public) } + before do - admin.starred_projects << project - admin.save! + project_member2 + user3.update_attributes(starred_projects: [project, project2, project3, public_project]) end - it 'should return the starred projects' do - get api('/projects/all', admin) + it 'should return the starred projects viewable by the user' do + get api('/projects/starred', user3) expect(response.status).to eq(200) expect(json_response).to be_an Array - - expect(json_response).to satisfy do |response| - response.one? do |entry| - entry['name'] == project.name - end - end + expect(json_response.map { |project| project['id'] }).to contain_exactly(project.id, public_project.id) end end diff --git a/spec/workers/emails_on_push_worker_spec.rb b/spec/workers/emails_on_push_worker_spec.rb index 3600c771075..439da765c2c 100644 --- a/spec/workers/emails_on_push_worker_spec.rb +++ b/spec/workers/emails_on_push_worker_spec.rb @@ -6,29 +6,66 @@ describe EmailsOnPushWorker do let(:project) { create(:project) } let(:user) { create(:user) } let(:data) { Gitlab::PushDataBuilder.build_sample(project, user) } + let(:recipients) { user.email } + let(:perform) { subject.perform(project.id, recipients, data.stringify_keys) } subject { EmailsOnPushWorker.new } - before do - allow(Project).to receive(:find).and_return(project) - end - describe "#perform" do - it "sends mail" do - subject.perform(project.id, user.email, data.stringify_keys) + context "when there are no errors in sending" do + let(:email) { ActionMailer::Base.deliveries.last } + + before { perform } - email = ActionMailer::Base.deliveries.last - expect(email.subject).to include('Change some files') - expect(email.to).to eq([user.email]) + it "sends a mail with the correct subject" do + expect(email.subject).to include('Change some files') + end + + it "sends the mail to the correct recipient" do + expect(email.to).to eq([user.email]) + end end - it "gracefully handles an input SMTP error" do - ActionMailer::Base.deliveries.clear - allow(Notify).to receive(:repository_push_email).and_raise(Net::SMTPFatalError) + context "when there is an SMTP error" do + before do + ActionMailer::Base.deliveries.clear + allow(Notify).to receive(:repository_push_email).and_raise(Net::SMTPFatalError) + perform + end + + it "gracefully handles an input SMTP error" do + expect(ActionMailer::Base.deliveries.count).to eq(0) + end + end + + context "when there are multiple recipients" do + let(:recipients) do + 1.upto(5).map { |i| user.email.sub('@', "+#{i}@") }.join("\n") + end + + before do + # This is a hack because we modify the mail object before sending, for efficency, + # but the TestMailer adapter just appends the objects to an array. To clone a mail + # object, create a new one! + # https://github.com/mikel/mail/issues/314#issuecomment-12750108 + allow_any_instance_of(Mail::TestMailer).to receive(:deliver!).and_wrap_original do |original, mail| + original.call(Mail.new(mail.encoded)) + end + + ActionMailer::Base.deliveries.clear + end - subject.perform(project.id, user.email, data.stringify_keys) + it "sends the mail to each of the recipients" do + perform + expect(ActionMailer::Base.deliveries.count).to eq(5) + expect(ActionMailer::Base.deliveries.map(&:to).flatten).to contain_exactly(*recipients.split) + end - expect(ActionMailer::Base.deliveries.count).to eq(0) + it "only generates the mail once" do + expect(Notify).to receive(:repository_push_email).once.and_call_original + expect(Premailer::Rails::CustomizedPremailer).to receive(:new).once.and_call_original + perform + end end end end diff --git a/vendor/assets/javascripts/jquery.scrollTo.js b/vendor/assets/javascripts/jquery.scrollTo.js new file mode 100755 index 00000000000..7ba17766b70 --- /dev/null +++ b/vendor/assets/javascripts/jquery.scrollTo.js @@ -0,0 +1,210 @@ +/*! + * jQuery.scrollTo + * Copyright (c) 2007-2015 Ariel Flesler - aflesler<a>gmail<d>com | http://flesler.blogspot.com + * Licensed under MIT + * http://flesler.blogspot.com/2007/10/jqueryscrollto.html + * @projectDescription Lightweight, cross-browser and highly customizable animated scrolling with jQuery + * @author Ariel Flesler + * @version 2.1.2 + */ +;(function(factory) { + 'use strict'; + if (typeof define === 'function' && define.amd) { + // AMD + define(['jquery'], factory); + } else if (typeof module !== 'undefined' && module.exports) { + // CommonJS + module.exports = factory(require('jquery')); + } else { + // Global + factory(jQuery); + } +})(function($) { + 'use strict'; + + var $scrollTo = $.scrollTo = function(target, duration, settings) { + return $(window).scrollTo(target, duration, settings); + }; + + $scrollTo.defaults = { + axis:'xy', + duration: 0, + limit:true + }; + + function isWin(elem) { + return !elem.nodeName || + $.inArray(elem.nodeName.toLowerCase(), ['iframe','#document','html','body']) !== -1; + } + + $.fn.scrollTo = function(target, duration, settings) { + if (typeof duration === 'object') { + settings = duration; + duration = 0; + } + if (typeof settings === 'function') { + settings = { onAfter:settings }; + } + if (target === 'max') { + target = 9e9; + } + + settings = $.extend({}, $scrollTo.defaults, settings); + // Speed is still recognized for backwards compatibility + duration = duration || settings.duration; + // Make sure the settings are given right + var queue = settings.queue && settings.axis.length > 1; + if (queue) { + // Let's keep the overall duration + duration /= 2; + } + settings.offset = both(settings.offset); + settings.over = both(settings.over); + + return this.each(function() { + // Null target yields nothing, just like jQuery does + if (target === null) return; + + var win = isWin(this), + elem = win ? this.contentWindow || window : this, + $elem = $(elem), + targ = target, + attr = {}, + toff; + + switch (typeof targ) { + // A number will pass the regex + case 'number': + case 'string': + if (/^([+-]=?)?\d+(\.\d+)?(px|%)?$/.test(targ)) { + targ = both(targ); + // We are done + break; + } + // Relative/Absolute selector + targ = win ? $(targ) : $(targ, elem); + /* falls through */ + case 'object': + if (targ.length === 0) return; + // DOMElement / jQuery + if (targ.is || targ.style) { + // Get the real position of the target + toff = (targ = $(targ)).offset(); + } + } + + var offset = $.isFunction(settings.offset) && settings.offset(elem, targ) || settings.offset; + + $.each(settings.axis.split(''), function(i, axis) { + var Pos = axis === 'x' ? 'Left' : 'Top', + pos = Pos.toLowerCase(), + key = 'scroll' + Pos, + prev = $elem[key](), + max = $scrollTo.max(elem, axis); + + if (toff) {// jQuery / DOMElement + attr[key] = toff[pos] + (win ? 0 : prev - $elem.offset()[pos]); + + // If it's a dom element, reduce the margin + if (settings.margin) { + attr[key] -= parseInt(targ.css('margin'+Pos), 10) || 0; + attr[key] -= parseInt(targ.css('border'+Pos+'Width'), 10) || 0; + } + + attr[key] += offset[pos] || 0; + + if (settings.over[pos]) { + // Scroll to a fraction of its width/height + attr[key] += targ[axis === 'x'?'width':'height']() * settings.over[pos]; + } + } else { + var val = targ[pos]; + // Handle percentage values + attr[key] = val.slice && val.slice(-1) === '%' ? + parseFloat(val) / 100 * max + : val; + } + + // Number or 'number' + if (settings.limit && /^\d+$/.test(attr[key])) { + // Check the limits + attr[key] = attr[key] <= 0 ? 0 : Math.min(attr[key], max); + } + + // Don't waste time animating, if there's no need. + if (!i && settings.axis.length > 1) { + if (prev === attr[key]) { + // No animation needed + attr = {}; + } else if (queue) { + // Intermediate animation + animate(settings.onAfterFirst); + // Don't animate this axis again in the next iteration. + attr = {}; + } + } + }); + + animate(settings.onAfter); + + function animate(callback) { + var opts = $.extend({}, settings, { + // The queue setting conflicts with animate() + // Force it to always be true + queue: true, + duration: duration, + complete: callback && function() { + callback.call(elem, targ, settings); + } + }); + $elem.animate(attr, opts); + } + }); + }; + + // Max scrolling position, works on quirks mode + // It only fails (not too badly) on IE, quirks mode. + $scrollTo.max = function(elem, axis) { + var Dim = axis === 'x' ? 'Width' : 'Height', + scroll = 'scroll'+Dim; + + if (!isWin(elem)) + return elem[scroll] - $(elem)[Dim.toLowerCase()](); + + var size = 'client' + Dim, + doc = elem.ownerDocument || elem.document, + html = doc.documentElement, + body = doc.body; + + return Math.max(html[scroll], body[scroll]) - Math.min(html[size], body[size]); + }; + + function both(val) { + return $.isFunction(val) || $.isPlainObject(val) ? val : { top:val, left:val }; + } + + // Add special hooks so that window scroll properties can be animated + $.Tween.propHooks.scrollLeft = + $.Tween.propHooks.scrollTop = { + get: function(t) { + return $(t.elem)[t.prop](); + }, + set: function(t) { + var curr = this.get(t); + // If interrupt is true and user scrolled, stop animating + if (t.options.interrupt && t._last && t._last !== curr) { + return $(t.elem).stop(); + } + var next = Math.round(t.now); + // Don't waste CPU + // Browsers don't render floating point scroll + if (curr !== next) { + $(t.elem)[t.prop](next); + t._last = this.get(t); + } + } + }; + + // AMD requirement + return $scrollTo; +}); |