diff options
25 files changed, 301 insertions, 155 deletions
diff --git a/CHANGELOG b/CHANGELOG index 890c154fede..f92f486064f 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 7.11.0 (unreleased) + - Fix broken view when viewing history of a file that includes a path that used to be another file (Stan Hu) - Don't show duplicate deploy keys - Fix commit time being displayed in the wrong timezone in some cases (Hannes Rosenögger) - Make the first branch pushed to an empty repository the default HEAD (Stan Hu) @@ -42,6 +43,9 @@ v 7.11.0 (unreleased) - Task lists are now usable in comments, and will show up in Markdown previews. - Fix bug where avatar filenames were not actually deleted from the database during removal (Stan Hu) - Fix bug where Slack service channel was not saved in admin template settings. (Stan Hu) + - Protect OmniAuth request phase against CSRF. + - + - - Move snippets UI to fluid layout - Improve UI for sidebar. Increase separation between navigation and content - Improve new project command options (Ben Bodenmiller) @@ -58,6 +62,9 @@ v 7.11.0 (unreleased) - Fix automatic blocking of auto-created users from Active Directory. - Call merge request web hook for each new commits (Arthur Gautier) - Use SIGKILL by default in Sidekiq::MemoryKiller + - Fix mentioning of private groups. + - Add style for <kbd> element in markdown + - Spin spinner icon next to "Checking for CI status..." on MR page. - Fix reference links in dashboard activity and ATOM feeds. v 7.10.2 @@ -23,7 +23,7 @@ gem "pg", group: :postgres # Auth gem "devise", '3.2.4' gem "devise-async", '0.9.0' -gem 'omniauth', "~> 1.1.3" +gem 'omniauth', "~> 1.2.2" gem 'omniauth-google-oauth2' gem 'omniauth-twitter' gem 'omniauth-github' @@ -44,7 +44,7 @@ gem "browser" # Extracting information from a git repository # Provide access to Gitlab::Git library -gem "gitlab_git", '~> 7.1.11' +gem "gitlab_git", '~> 7.1.12' # Ruby/Rack Git Smart-HTTP Server Handler gem 'gitlab-grack', '~> 2.0.2', require: 'grack' diff --git a/Gemfile.lock b/Gemfile.lock index 9940ab15242..571ab27ea71 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -225,7 +225,7 @@ GEM mime-types (~> 1.19) gitlab_emoji (0.1.0) gemojione (~> 2.0) - gitlab_git (7.1.11) + gitlab_git (7.1.12) activesupport (~> 4.0) charlock_holmes (~> 0.6) gitlab-linguist (~> 3.0) @@ -362,9 +362,9 @@ GEM rack (~> 1.2) octokit (3.7.0) sawyer (~> 0.6.0, >= 0.5.3) - omniauth (1.1.4) - hashie (>= 1.2, < 3) - rack + omniauth (1.2.2) + hashie (>= 1.2, < 4) + rack (~> 1.0) omniauth-bitbucket (0.0.2) multi_json (~> 1.7) omniauth (~> 1.1) @@ -723,7 +723,7 @@ DEPENDENCIES gitlab-grack (~> 2.0.2) gitlab-linguist (~> 3.0.1) gitlab_emoji (~> 0.1) - gitlab_git (~> 7.1.11) + gitlab_git (~> 7.1.12) gitlab_meta (= 7.0) gitlab_omniauth-ldap (= 1.2.1) gollum-lib (~> 4.0.2) @@ -751,7 +751,7 @@ DEPENDENCIES newrelic_rpm nprogress-rails octokit (= 3.7.0) - omniauth (~> 1.1.3) + omniauth (~> 1.2.2) omniauth-bitbucket omniauth-github omniauth-gitlab diff --git a/app/assets/stylesheets/base/mixins.scss b/app/assets/stylesheets/base/mixins.scss index a0794e7825a..08cbe911672 100644 --- a/app/assets/stylesheets/base/mixins.scss +++ b/app/assets/stylesheets/base/mixins.scss @@ -73,6 +73,22 @@ padding: 0; } + kbd { + display: inline-block; + padding: 3px 5px; + font-size: 11px; + line-height: 10px; + color: #555; + vertical-align: middle; + background-color: #FCFCFC; + border-width: 1px; + border-style: solid; + border-color: #CCC #CCC #BBB; + border-image: none; + border-radius: 3px; + box-shadow: 0px -1px 0px #BBB inset; + } + h1 { margin-top: 45px; font-size: 2.5em; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 61b907e39be..e943be67dbf 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -33,7 +33,16 @@ ul.notes { &:before { content: "\00b7"; } + font-size: 13px; + + a { + @extend .cgray; + + &:hover { + text-decoration: underline; + } + } } .author { color: #333; @@ -47,7 +56,7 @@ ul.notes { .note-role { float: right; - margin-top: 2px; + margin-top: 1px; border: 1px solid #bbb; background-color: transparent; color: #999; @@ -142,31 +151,23 @@ ul.notes { .discussion, .note { - &.note:hover { - .note-actions { display: block; } - .note-actions + .note-role { display: none; } - } - .discussion-header:hover { - .discussion-actions { display: block; } - } - .discussion-actions, .note-actions { - display: none; float: right; - - i.fa { - font-size: 16px; - line-height: 16px; - vertical-align: middle; - } + margin-left: 10px; a { margin-left: 5px; - @extend .cgray; + color: #999; + + i.fa { + font-size: 16px; + line-height: 16px; + } &:hover { + @extend .cgray; &.danger { @extend .cred; } } } diff --git a/app/controllers/concerns/authenticates_with_two_factor.rb b/app/controllers/concerns/authenticates_with_two_factor.rb new file mode 100644 index 00000000000..d5918a7af3b --- /dev/null +++ b/app/controllers/concerns/authenticates_with_two_factor.rb @@ -0,0 +1,30 @@ +# == AuthenticatesWithTwoFactor +# +# Controller concern to handle two-factor authentication +# +# Upon inclusion, skips `require_no_authentication` on `:create`. +module AuthenticatesWithTwoFactor + extend ActiveSupport::Concern + + included do + # This action comes from DeviseController, but because we call `sign_in` + # manually, not skipping this action would cause a "You are already signed + # in." error message to be shown upon successful login. + skip_before_action :require_no_authentication, only: [:create] + end + + # Store the user's ID in the session for later retrieval and render the + # two factor code prompt + # + # The user must have been authenticated with a valid login and password + # before calling this method! + # + # user - User record + # + # Returns nil + def prompt_for_two_factor(user) + session[:otp_user_id] = user.id + + render 'devise/sessions/two_factor' and return + end +end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index d4ff0d97561..b89b4c27350 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,11 +1,7 @@ class SessionsController < Devise::SessionsController - prepend_before_action :authenticate_with_two_factor, only: [:create] + include AuthenticatesWithTwoFactor - # This action comes from DeviseController, but because we call `sign_in` - # manually inside `authenticate_with_two_factor`, not skipping this action - # would cause a "You are already signed in." error message to be shown upon - # successful login. - skip_before_action :require_no_authentication, only: [:create] + prepend_before_action :authenticate_with_two_factor, only: [:create] def new redirect_path = @@ -74,9 +70,7 @@ class SessionsController < Devise::SessionsController end else if user && user.valid_password?(user_params[:password]) - # Save the user's ID to session so we can ask for a one-time password - session[:otp_user_id] = user.id - render :two_factor and return + prompt_for_two_factor(user) end end end diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 672be54e66f..ea9722b9bef 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -324,12 +324,20 @@ module ApplicationHelper count = if project.nil? - "" + nil elsif current_controller?(:issues) - " (#{project.issues.send(entity).count})" + project.issues.send(entity).count elsif current_controller?(:merge_requests) - " (#{project.merge_requests.send(entity).count})" + project.merge_requests.send(entity).count end - "#{entity_title}#{count}" + + html = content_tag :span, entity_title + + if count.present? + html += " " + html += content_tag :span, number_with_delimiter(count), class: 'badge' + end + + html.html_safe end end diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index a5957391bb7..b7c39df885d 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -42,10 +42,10 @@ module Mentionable SystemNoteService.cross_reference_exists?(target, local_reference) end - def mentioned_users(current_user = nil, p = project) + def mentioned_users(current_user = nil) return [] if mentionable_text.blank? - ext = Gitlab::ReferenceExtractor.new(p, current_user) + ext = Gitlab::ReferenceExtractor.new(self.project, current_user) ext.analyze(mentionable_text) ext.users.uniq end diff --git a/app/models/concerns/participable.rb b/app/models/concerns/participable.rb index 7a5e4876ff2..a4832204f7b 100644 --- a/app/models/concerns/participable.rb +++ b/app/models/concerns/participable.rb @@ -40,7 +40,7 @@ module Participable meth = method(attr) value = - if meth.arity == 1 + if meth.arity == 1 || meth.arity == -1 meth.call(current_user) else meth.call diff --git a/app/views/devise/shared/_omniauth_box.html.haml b/app/views/devise/shared/_omniauth_box.html.haml index 8dce0b16936..f8ba9d80ae8 100644 --- a/app/views/devise/shared/_omniauth_box.html.haml +++ b/app/views/devise/shared/_omniauth_box.html.haml @@ -5,6 +5,6 @@ - providers.each do |provider| %span.light - if default_providers.include?(provider) - = link_to oauth_image_tag(provider), omniauth_authorize_path(resource_name, provider), class: 'oauth-image-link' + = link_to oauth_image_tag(provider), omniauth_authorize_path(resource_name, provider), method: :post, class: 'oauth-image-link' - else - = link_to provider.to_s.titleize, omniauth_authorize_path(resource_name, provider), class: "btn", "data-no-turbolink" => "true" + = link_to provider.to_s.titleize, omniauth_authorize_path(resource_name, provider), method: :post, class: "btn", "data-no-turbolink" => "true" diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index 6ac60b01f85..06bad7dd84a 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -62,7 +62,7 @@ - enabled_social_providers.each do |provider| .btn-group = link_to oauth_image_tag(provider), omniauth_authorize_path(User, provider), - class: "btn btn-lg #{'active' if oauth_active?(provider)}" + method: :post, class: "btn btn-lg #{'active' if oauth_active?(provider)}" - if oauth_active?(provider) = link_to unlink_profile_account_path(provider: provider), method: :delete, class: 'btn btn-lg' do = icon('close') diff --git a/app/views/projects/_aside.html.haml b/app/views/projects/_aside.html.haml index c2f56996ba8..333a1e6156d 100644 --- a/app/views/projects/_aside.html.haml +++ b/app/views/projects/_aside.html.haml @@ -10,17 +10,14 @@ - if @repository.changelog %li.hidden-xs = link_to changelog_url(@project) do - = icon("list-alt fw") Changelog - if @repository.license %li = link_to license_url(@project) do - = icon("check-circle-o fw") License - if @repository.contribution_guide %li = link_to contribution_guide_url(@project) do - = icon("info-circle fw") Contribution guide .actions @@ -73,15 +70,12 @@ %ul.nav.nav-pills %li = link_to namespace_project_commits_path(@project.namespace, @project, @ref || @repository.root_ref) do - = icon("history fw") = pluralize(number_with_delimiter(@repository.commit_count), 'commit') %li = link_to namespace_project_branches_path(@project.namespace, @project) do - = icon("code-fork fw") = pluralize(number_with_delimiter(@repository.branch_names.count), 'branch') %li = link_to namespace_project_tags_path(@project.namespace, @project) do - = icon("tags fw") = pluralize(number_with_delimiter(@repository.tag_names.count), 'tag') .actions diff --git a/app/views/projects/commits/_head.html.haml b/app/views/projects/commits/_head.html.haml index 66261c7336d..e3d8cd0fdd5 100644 --- a/app/views/projects/commits/_head.html.haml +++ b/app/views/projects/commits/_head.html.haml @@ -3,7 +3,7 @@ = link_to namespace_project_commits_path(@project.namespace, @project, @ref || @repository.root_ref) do = icon("history") Commits - %span.badge= number_with_precision(@repository.commit_count, precision: 0, delimiter: ',') + %span.badge= number_with_delimiter(@repository.commit_count) = nav_link(controller: :compare) do = link_to namespace_project_compare_index_path(@project.namespace, @project, from: @repository.root_ref, to: @ref || @repository.root_ref) do = icon("exchange") diff --git a/app/views/projects/merge_requests/show/_mr_ci.html.haml b/app/views/projects/merge_requests/show/_mr_ci.html.haml index ffa3f7b0e36..3b1cd53df37 100644 --- a/app/views/projects/merge_requests/show/_mr_ci.html.haml +++ b/app/views/projects/merge_requests/show/_mr_ci.html.haml @@ -1,34 +1,34 @@ - if @commits.any? .ci_widget.ci-success{style: "display:none"} - %i.fa.fa-check + = icon("check") %span CI build passed for #{@merge_request.last_commit_short_sha}. = link_to "View build page", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink" .ci_widget.ci-failed{style: "display:none"} - %i.fa.fa-times + = icon("times") %span CI build failed for #{@merge_request.last_commit_short_sha}. = link_to "View build page", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink" - [:running, :pending].each do |status| .ci_widget{class: "ci-#{status}", style: "display:none"} - %i.fa.fa-clock-o + = icon("clock-o") %span CI build #{status} for #{@merge_request.last_commit_short_sha}. = link_to "View build page", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink" .ci_widget - %i.fa.fa-spinner + = icon("spinner spin") Checking for CI status for #{@merge_request.last_commit_short_sha} .ci_widget.ci-canceled{style: "display:none"} - %i.fa.fa-times + = icon("times") %span CI build canceled for #{@merge_request.last_commit_short_sha}. = link_to "View build page", ci_build_details_path(@merge_request), :"data-no-turbolink" => "data-no-turbolink" .ci_widget.ci-error{style: "display:none"} - %i.fa.fa-times + = icon("times") %span Cannot connect to the CI server. Please check your settings and try again. diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 2f3c407d6e9..0a77f200f56 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -8,19 +8,13 @@ = image_tag avatar_icon(note.author_email), class: 'avatar s40', alt: '' .timeline-content .note-header - .note-actions - = link_to "##{dom_id(note)}", name: dom_id(note), title: "Link here" do - = icon('link fw') - Link here - - - if note_editable?(note) + - if note_editable?(note) + .note-actions = link_to '#', title: 'Edit comment', class: 'js-note-edit' do - = icon('pencil-square-o fw') - Edit + = icon('pencil-square-o') - = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'danger js-note-delete' do - = icon('trash-o fw', class: 'cred') - Remove + = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'js-note-delete danger' do + = icon('trash-o') - unless note.system - member = note.project.team.find_member(note.author.id) @@ -33,10 +27,13 @@ = image_tag avatar_icon(note.author_email), class: 'avatar s16', alt: '' = link_to_member(note.project, note.author, avatar: false) + %span.author-username = '@' + note.author.username + %span.note-last-update - = note_timestamp(note) + = link_to "##{dom_id(note)}", name: dom_id(note), title: "Link here" do + = note_timestamp(note) - if note.superceded?(@notes) - if note.upvote? diff --git a/app/views/projects/notes/discussions/_active.html.haml b/app/views/projects/notes/discussions/_active.html.haml index 7c6f7243173..e7a3854701c 100644 --- a/app/views/projects/notes/discussions/_active.html.haml +++ b/app/views/projects/notes/discussions/_active.html.haml @@ -14,7 +14,9 @@ - last_note = discussion_notes.last last updated by = link_to_member(@project, last_note.author, avatar: false) + %span.discussion-last-update #{time_ago_with_tooltip(last_note.updated_at, 'bottom', 'discussion_updated_ago')} + .discussion-body.js-toggle-content = render "projects/notes/discussions/diff", discussion_notes: discussion_notes, note: note diff --git a/config/initializers/7_omniauth.rb b/config/initializers/7_omniauth.rb index 8f6c5673103..103aa06ca32 100644 --- a/config/initializers/7_omniauth.rb +++ b/config/initializers/7_omniauth.rb @@ -10,3 +10,8 @@ if Gitlab::LDAP::Config.enabled? alias_method server['provider_name'], :ldap end end + +OmniAuth.config.allowed_request_methods = [:post] +OmniAuth.config.before_request_phase do |env| + OmniAuth::RequestForgeryProtection.new(env).call +end diff --git a/doc/api/projects.md b/doc/api/projects.md index 971fe96fb8e..17c014019ea 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -32,6 +32,7 @@ Parameters: - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria +- `ci_enabled_first` - Return projects ordered by ci_enabled flag. Projects with enabled GitLab CI go first ```json [ @@ -134,6 +135,7 @@ Parameters: - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria +- `ci_enabled_first` - Return projects ordered by ci_enabled flag. Projects with enabled GitLab CI go first ### List ALL projects @@ -149,6 +151,7 @@ Parameters: - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria +- `ci_enabled_first` - Return projects ordered by ci_enabled flag. Projects with enabled GitLab CI go first ### Get single project diff --git a/lib/api/projects.rb b/lib/api/projects.rb index e3fff79d68f..1f2251c9b9c 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -22,7 +22,12 @@ module API projects = projects.search(params[:search]) end - projects.reorder(project_order_by => project_sort) + if params[:ci_enabled_first].present? + projects.includes(:gitlab_ci_service). + reorder("services.active DESC, projects.#{project_order_by} #{project_sort}") + else + projects.reorder(project_order_by => project_sort) + end end def project_order_by diff --git a/lib/gitlab/markdown.rb b/lib/gitlab/markdown.rb index 4b45f239ce5..133010adcaf 100644 --- a/lib/gitlab/markdown.rb +++ b/lib/gitlab/markdown.rb @@ -70,9 +70,9 @@ module Gitlab reference_class: html_options[:class], # RelativeLinkFilter - ref: @ref, + ref: @ref, requested_path: @path, - project_wiki: @project_wiki + project_wiki: @project_wiki } result = pipeline.call(text, context) diff --git a/lib/gitlab/markdown/relative_link_filter.rb b/lib/gitlab/markdown/relative_link_filter.rb index deb302c88e1..9de2b24a9da 100644 --- a/lib/gitlab/markdown/relative_link_filter.rb +++ b/lib/gitlab/markdown/relative_link_filter.rb @@ -9,19 +9,18 @@ module Gitlab # :commit # :project # :project_wiki - # :requested_path # :ref + # :requested_path class RelativeLinkFilter < HTML::Pipeline::Filter - def call - if linkable_files? - doc.search('a').each do |el| - process_link_attr el.attribute('href') - end - - doc.search('img').each do |el| - process_link_attr el.attribute('src') - end + return doc unless linkable_files? + + doc.search('a').each do |el| + process_link_attr el.attribute('href') + end + + doc.search('img').each do |el| + process_link_attr el.attribute('src') end doc @@ -40,6 +39,8 @@ module Gitlab if uri.relative? && uri.path.present? html_attr.value = rebuild_relative_uri(uri).to_s end + rescue URI::Error + # noop end def rebuild_relative_uri(uri) @@ -85,12 +86,25 @@ module Gitlab repository.tree(current_sha, path).entries.any? end - # Check if the path is pointing to a directory(tree) or a file(blob) - # eg. doc/api is directory and doc/README.md is file. + # Get the type of the given path + # + # path - String path to check + # + # Examples: + # + # path_type('doc/README.md') # => 'blob' + # path_type('doc/logo.png') # => 'raw' + # path_type('doc/api') # => 'tree' + # + # Returns a String def path_type(path) - return 'tree' if repository.tree(current_sha, path).entries.any? - return 'raw' if repository.blob_at(current_sha, path).try(:image?) - 'blob' + if repository.tree(current_sha, path).entries.any? + 'tree' + elsif repository.blob_at(current_sha, path).try(:image?) + 'raw' + else + 'blob' + end end def current_sha diff --git a/lib/omni_auth/request_forgery_protection.rb b/lib/omni_auth/request_forgery_protection.rb new file mode 100644 index 00000000000..3557522d3c9 --- /dev/null +++ b/lib/omni_auth/request_forgery_protection.rb @@ -0,0 +1,66 @@ +# Protects OmniAuth request phase against CSRF. + +module OmniAuth + # Based on ActionController::RequestForgeryProtection. + class RequestForgeryProtection + def initialize(env) + @env = env + end + + def request + @request ||= ActionDispatch::Request.new(@env) + end + + def session + request.session + end + + def reset_session + request.reset_session + end + + def params + request.params + end + + def call + verify_authenticity_token + end + + def verify_authenticity_token + if !verified_request? + Rails.logger.warn "Can't verify CSRF token authenticity" if Rails.logger + handle_unverified_request + end + end + + private + + def protect_against_forgery? + ApplicationController.allow_forgery_protection + end + + def request_forgery_protection_token + ApplicationController.request_forgery_protection_token + end + + def forgery_protection_strategy + ApplicationController.forgery_protection_strategy + end + + def verified_request? + !protect_against_forgery? || request.get? || request.head? || + form_authenticity_token == params[request_forgery_protection_token] || + form_authenticity_token == request.headers['X-CSRF-Token'] + end + + def handle_unverified_request + forgery_protection_strategy.new(self).handle_unverified_request + end + + # Sets the token value for the current session. + def form_authenticity_token + session[:_csrf_token] ||= SecureRandom.base64(32) + end + end +end diff --git a/spec/lib/gitlab/markdown/relative_link_filter_spec.rb b/spec/lib/gitlab/markdown/relative_link_filter_spec.rb index 38cf567d73f..5ee5310825d 100644 --- a/spec/lib/gitlab/markdown/relative_link_filter_spec.rb +++ b/spec/lib/gitlab/markdown/relative_link_filter_spec.rb @@ -2,119 +2,114 @@ require 'spec_helper' module Gitlab::Markdown describe RelativeLinkFilter do - include ActionView::Helpers::TagHelper - - let!(:project) { create(:project) } + def filter(doc) + described_class.call(doc, { + commit: project.commit, + project: project, + project_wiki: project_wiki, + ref: ref, + requested_path: requested_path + }) + end - let(:commit) { project.commit } - let(:project_path) { project.path_with_namespace } - let(:repository) { project.repository } - let(:ref) { 'markdown' } + def image(path) + %(<img src="#{path}" />) + end - let(:project_wiki) { nil } - let(:requested_path) { '/' } - let(:blob) { RepoHelpers.sample_blob } - - let(:context) do - { - commit: commit, - project: project, - project_wiki: project_wiki, - requested_path: requested_path, - ref: ref - } + def link(path) + %(<a href="#{path}">#{path}</a>) end + let(:project) { create(:project) } + let(:project_path) { project.path_with_namespace } + let(:ref) { 'markdown' } + let(:project_wiki) { nil } + let(:requested_path) { '/' } shared_examples :preserve_unchanged do - - it "should not modify any relative url in anchor" do - doc = tag(:a, href: 'README.md') - expect( filter(doc) ).to match '"README.md"' + it 'does not modify any relative URL in anchor' do + doc = filter(link('README.md')) + expect(doc.at_css('a')['href']).to eq 'README.md' end - it "should not modify any relative url in image" do - doc = tag(:img, src: 'files/images/logo-black.png') - expect( filter(doc) ).to match '"files/images/logo-black.png"' + it 'does not modify any relative URL in image' do + doc = filter(image('files/images/logo-black.png')) + expect(doc.at_css('img')['src']).to eq 'files/images/logo-black.png' end end shared_examples :relative_to_requested do - - it "should rebuild url relative to the requested path" do - expect( filter(tag(:a, href: 'users.md')) ).to \ - match %("/#{project_path}/blob/#{ref}/doc/api/users.md") + it 'rebuilds URL relative to the requested path' do + doc = filter(link('users.md')) + expect(doc.at_css('a')['href']). + to eq "/#{project_path}/blob/#{ref}/doc/api/users.md" end end - - context "with a project_wiki" do + context 'with a project_wiki' do let(:project_wiki) { double('ProjectWiki') } - include_examples :preserve_unchanged end - context "without a repository" do - let!(:project) { create(:empty_project) } - + context 'without a repository' do + let(:project) { create(:empty_project) } include_examples :preserve_unchanged end - context "with an empty repository" do - let!(:project) { create(:project_empty_repo) } - + context 'with an empty repository' do + let(:project) { create(:project_empty_repo) } include_examples :preserve_unchanged end + it 'does not raise an exception on invalid URIs' do + act = link("://foo") + expect { filter(act) }.not_to raise_error + end - context "with a valid repository" do - - it "should rebuild relative url for a file in the repo" do - expect( filter(tag(:a, href: 'doc/api/README.md')) ).to \ - match %("/#{project_path}/blob/#{ref}/doc/api/README.md") + context 'with a valid repository' do + it 'rebuilds relative URL for a file in the repo' do + doc = filter(link('doc/api/README.md')) + expect(doc.at_css('a')['href']). + to eq "/#{project_path}/blob/#{ref}/doc/api/README.md" end - it "should rebuild relative url for a file in the repo with an anchor" do - expect( filter(tag(:a, href: 'README.md#section')) ).to \ - match %("/#{project_path}/blob/#{ref}/README.md#section") + it 'rebuilds relative URL for a file in the repo with an anchor' do + doc = filter(link('README.md#section')) + expect(doc.at_css('a')['href']). + to eq "/#{project_path}/blob/#{ref}/README.md#section" end - it "should rebuild relative url for a directory in the repo" do - expect( filter(tag(:a, href: 'doc/api/')) ).to \ - match %("/#{project_path}/tree/#{ref}/doc/api") + it 'rebuilds relative URL for a directory in the repo' do + doc = filter(link('doc/api/')) + expect(doc.at_css('a')['href']). + to eq "/#{project_path}/tree/#{ref}/doc/api" end - it "should rebuild relative url for an image in the repo" do - expect( filter(tag(:img, src: 'files/images/logo-black.png')) ).to \ - match %("/#{project_path}/raw/#{ref}/files/images/logo-black.png") + it 'rebuilds relative URL for an image in the repo' do + doc = filter(link('files/images/logo-black.png')) + expect(doc.at_css('a')['href']). + to eq "/#{project_path}/raw/#{ref}/files/images/logo-black.png" end - it "should not modify relative url with an anchor only" do - doc = tag(:a, href: '#section-1') - expect( filter(doc) ).to match %("#section-1") + it 'does not modify relative URL with an anchor only' do + doc = filter(link('#section-1')) + expect(doc.at_css('a')['href']).to eq '#section-1' end - it "should not modify absolute url" do - expect( filter(tag(:a, href: 'http://example.org')) ).to \ - match %("http://example.org") + it 'does not modify absolute URL' do + doc = filter(link('http://example.com')) + expect(doc.at_css('a')['href']).to eq 'http://example.com' end - context "when requested path is a file in the repo" do + context 'when requested path is a file in the repo' do let(:requested_path) { 'doc/api/README.md' } - include_examples :relative_to_requested end - context "when requested path is a directory in the repo" do + context 'when requested path is a directory in the repo' do let(:requested_path) { 'doc/api' } - include_examples :relative_to_requested end end - - - def filter(doc) - described_class.call(doc, context).to_s - end end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index cc387378d3a..aada7febf6c 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -86,6 +86,15 @@ describe API::API, api: true do expect(json_response).to be_an Array expect(json_response.first['id']).to eq(project3.id) end + + it 'returns projects in the correct order when ci_enabled_first parameter is passed' do + [project, project2, project3].each{ |project| project.build_missing_services } + project2.gitlab_ci_service.update(active: true, token: "token", project_url: "url") + get api('/projects', user), { ci_enabled_first: 'true'} + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.first['id']).to eq(project2.id) + end end end end |