diff options
author | Kamil Trzcinski <ayufan@ayufan.eu> | 2016-08-15 20:36:06 +0200 |
---|---|---|
committer | Kamil Trzcinski <ayufan@ayufan.eu> | 2016-08-15 20:36:06 +0200 |
commit | d626c1d3729f500a891a6934ea779136671ef8b2 (patch) | |
tree | 3ffe8ae1d06ccecac566b370b81df85fa16bc2de | |
parent | 4d4ef89cdcb9beca66a13ab2b64e9eb45f8d8256 (diff) | |
parent | 640e485c6aa19f8fca1be7fc45e7f65da4469fbd (diff) | |
download | gitlab-ce-d626c1d3729f500a891a6934ea779136671ef8b2.tar.gz |
Merge remote-tracking branch 'origin/master' into pipeline-hooks-without-slack
58 files changed, 619 insertions, 131 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e8d54e768d3..be5614520a5 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -15,6 +15,7 @@ variables: USE_DB: "true" USE_BUNDLE_INSTALL: "true" GIT_DEPTH: "20" + PHANTOMJS_VERSION: "2.1.1" before_script: - source ./scripts/prepare_build.sh diff --git a/CHANGELOG b/CHANGELOG index 48850b5728c..47cae4909e9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.11.0 (unreleased) - Remove the http_parser.rb dependency by removing the tinder gem. !5758 (tbalthazar) + - Ability to specify branches for Pivotal Tracker integration (Egor Lynko) - Fix don't pass a local variable called `i` to a partial. !20510 (herminiotorres) - Fix rename `add_users_into_project` and `projects_ids`. !20512 (herminiotorres) - Fix the title of the toggle dropdown button. !5515 (herminiotorres) @@ -32,12 +33,14 @@ v 8.11.0 (unreleased) - Add "No one can push" as an option for protected branches. !5081 - Improve performance of AutolinkFilter#text_parse by using XPath - Add experimental Redis Sentinel support !1877 + - Rendering of SVGs as blobs is now limited to SVGs with a size smaller or equal to 2MB - Fix branches page dropdown sort initial state (ClemMakesApps) - Environments have an url to link to - Various redundant database indexes have been removed - Update `timeago` plugin to use multiple string/locale settings - Remove unused images (ClemMakesApps) - Limit git rev-list output count to one in forced push check + - Show deployment status on merge requests with external URLs - Clean up unused routes (Josef Strzibny) - Fix issue on empty project to allow developers to only push to protected branches if given permission - Add green outline to New Branch button. !5447 (winniehell) @@ -54,6 +57,7 @@ v 8.11.0 (unreleased) - Optimize checking if a user has read access to a list of issues !5370 - Store all DB secrets in secrets.yml, under descriptive names !5274 - Nokogiri's various parsing methods are now instrumented + - Add archived badge to project list !5798 - Add simple identifier to public SSH keys (muteor) - Admin page now references docs instead of a specific file !5600 (AnAverageHuman) - Add a way to send an email and create an issue based on private personal token. Find the email address from issues page. !3363 @@ -108,6 +112,8 @@ v 8.11.0 (unreleased) - Sort folders with submodules in Files view !5521 - Each `File::exists?` replaced to `File::exist?` because of deprecate since ruby version 2.2.0 - Add auto-completition in pipeline (Katarzyna Kobierska Ula Budziszewska) + - Fix a memory leak caused by Banzai::Filter::SanitizationFilter + - Speed up todos queries by limiting the projects set we join with v 8.10.5 - Add a data migration to fix some missing timestamps in the members table. !5670 @@ -272,6 +278,7 @@ v 8.10.0 - Fix new snippet style bug (elliotec) - Instrument Rinku usage - Be explicit to define merge request discussion variables + - Use cache for todos counter calling TodoService - Metrics for Rouge::Plugins::Redcarpet and Rouge::Formatters::HTMLGitlab - RailsCache metris now includes fetch_hit/fetch_miss and read_hit/read_miss info. - Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w) diff --git a/app/assets/javascripts/issuable.js b/app/assets/javascripts/issuable.js index f27f1bad1f7..d0305c6c6a1 100644 --- a/app/assets/javascripts/issuable.js +++ b/app/assets/javascripts/issuable.js @@ -5,13 +5,10 @@ this.Issuable = { init: function() { - if (!issuable_created) { - issuable_created = true; - Issuable.initTemplates(); - Issuable.initSearch(); - Issuable.initChecks(); - return Issuable.initLabelFilterRemove(); - } + Issuable.initTemplates(); + Issuable.initSearch(); + Issuable.initChecks(); + return Issuable.initLabelFilterRemove(); }, initTemplates: function() { return Issuable.labelRow = _.template('<% _.each(labels, function(label){ %> <span class="label-row btn-group" role="group" aria-label="<%- label.title %>" style="color: <%- label.text_color %>;"> <a href="#" class="btn btn-transparent has-tooltip" style="background-color: <%- label.color %>;" title="<%- label.description %>" data-container="body"> <%- label.title %> </a> <button type="button" class="btn btn-transparent label-remove js-label-filter-remove" style="background-color: <%- label.color %>;" data-label="<%- label.title %>"> <i class="fa fa-times"></i> </button> </span> <% }); %>'); diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 0a661e529f0..b4636269518 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -69,6 +69,10 @@ &.ci-success { color: $gl-success; + + a.environment { + color: inherit; + } } &.ci-success_with_warnings { @@ -126,7 +130,6 @@ &.has-conflicts .fa-exclamation-triangle { color: $gl-warning; } - } p:last-child { diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index 19a76a5b5d8..1243bb96d4d 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -37,8 +37,8 @@ class Dashboard::TodosController < Dashboard::ApplicationController def todos_counts { - count: TodosFinder.new(current_user, state: :pending).execute.count, - done_count: TodosFinder.new(current_user, state: :done).execute.count + count: current_user.todos_pending_count, + done_count: current_user.todos_done_count } end end diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index 2f0a9659d15..c7911736812 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -1,6 +1,7 @@ class ProjectsFinder < UnionFinder - def execute(current_user = nil, options = {}) + def execute(current_user = nil, project_ids_relation = nil) segments = all_projects(current_user) + segments.map! { |s| s.where(id: project_ids_relation) } if project_ids_relation find_union(segments, Project) end diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index ff866c2faa5..4fe0070552e 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -27,9 +27,11 @@ class TodosFinder items = by_action_id(items) items = by_action(items) items = by_author(items) - items = by_project(items) items = by_state(items) items = by_type(items) + # Filtering by project HAS TO be the last because we use + # the project IDs yielded by the todos query thus far + items = by_project(items) items.reorder(id: :desc) end @@ -91,14 +93,9 @@ class TodosFinder @project end - def projects - return @projects if defined?(@projects) - - if project? - @projects = project - else - @projects = ProjectsFinder.new.execute(current_user) - end + def projects(items) + item_project_ids = items.reorder(nil).select(:project_id) + ProjectsFinder.new.execute(current_user, item_project_ids) end def type? @@ -136,8 +133,9 @@ class TodosFinder def by_project(items) if project? items = items.where(project: project) - elsif projects - items = items.merge(projects).joins(:project) + else + item_projects = projects(items) + items = items.merge(item_projects).joins(:project) end items diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index e3a208f826a..0465327060e 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -1,10 +1,10 @@ module TodosHelper def todos_pending_count - @todos_pending_count ||= TodosFinder.new(current_user, state: :pending).execute.count + @todos_pending_count ||= current_user.todos_pending_count end def todos_done_count - @todos_done_count ||= TodosFinder.new(current_user, state: :done).execute.count + @todos_done_count ||= current_user.todos_done_count end def todo_action_name(todo) diff --git a/app/models/blob.rb b/app/models/blob.rb index 0df2805e448..12cc5aaafba 100644 --- a/app/models/blob.rb +++ b/app/models/blob.rb @@ -3,6 +3,9 @@ class Blob < SimpleDelegator CACHE_TIME = 60 # Cache raw blobs referred to by a (mutable) ref for 1 minute CACHE_TIME_IMMUTABLE = 3600 # Cache blobs referred to by an immutable reference for 1 hour + # The maximum size of an SVG that can be displayed. + MAXIMUM_SVG_SIZE = 2.megabytes + # Wrap a Gitlab::Git::Blob object, or return nil when given nil # # This method prevents the decorated object from evaluating to "truthy" when @@ -31,6 +34,10 @@ class Blob < SimpleDelegator text? && language && language.name == 'SVG' end + def size_within_svg_limits? + size <= MAXIMUM_SVG_SIZE + end + def video? UploaderHelper::VIDEO_EXT.include?(extname.downcase.delete('.')) end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 1a7cd60817e..1e338889714 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -36,4 +36,10 @@ class Deployment < ActiveRecord::Base def manual_actions deployable.try(:other_actions) end + + def includes_commit?(commit) + return false unless commit + + project.repository.is_ancestor?(commit.id, sha) + end end diff --git a/app/models/environment.rb b/app/models/environment.rb index baed106e8c8..75e6f869786 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -25,4 +25,10 @@ class Environment < ActiveRecord::Base def nullify_external_url self.external_url = nil if self.external_url.blank? end + + def includes_commit?(commit) + return false unless last_deployment + + last_deployment.includes_commit?(commit) + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index f6d0d0c98f5..fe799382fd0 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -591,6 +591,14 @@ class MergeRequest < ActiveRecord::Base !pipeline || pipeline.success? end + def environments + return unless diff_head_commit + + target_project.environments.select do |environment| + environment.includes_commit?(diff_head_commit) + end + end + def state_human_name if merged? "Merged" diff --git a/app/models/project_services/pivotaltracker_service.rb b/app/models/project_services/pivotaltracker_service.rb index ad19b7795da..5301f9fa0ff 100644 --- a/app/models/project_services/pivotaltracker_service.rb +++ b/app/models/project_services/pivotaltracker_service.rb @@ -1,7 +1,9 @@ class PivotaltrackerService < Service include HTTParty - prop_accessor :token + API_ENDPOINT = 'https://www.pivotaltracker.com/services/v5/source_commits' + + prop_accessor :token, :restrict_to_branch validates :token, presence: true, if: :activated? def title @@ -18,7 +20,17 @@ class PivotaltrackerService < Service def fields [ - { type: 'text', name: 'token', placeholder: '' } + { + type: 'text', + name: 'token', + placeholder: 'Pivotal Tracker API token.' + }, + { + type: 'text', + name: 'restrict_to_branch', + placeholder: 'Comma-separated list of branches which will be ' \ + 'automatically inspected. Leave blank to include all branches.' + } ] end @@ -28,8 +40,8 @@ class PivotaltrackerService < Service def execute(data) return unless supported_events.include?(data[:object_kind]) + return unless allowed_branch?(data[:ref]) - url = 'https://www.pivotaltracker.com/services/v5/source_commits' data[:commits].each do |commit| message = { 'source_commit' => { @@ -40,7 +52,7 @@ class PivotaltrackerService < Service } } PivotaltrackerService.post( - url, + API_ENDPOINT, body: message.to_json, headers: { 'Content-Type' => 'application/json', @@ -49,4 +61,15 @@ class PivotaltrackerService < Service ) end end + + private + + def allowed_branch?(ref) + return true unless ref.present? && restrict_to_branch.present? + + branch = Gitlab::Git.ref_name(ref) + allowed_branches = restrict_to_branch.split(',').map(&:strip) + + branch.present? && allowed_branches.include?(branch) + end end diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index a255710f577..46f70da2452 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -56,6 +56,10 @@ class ProjectWiki end end + def repository_exists? + !!repository.exists? + end + def empty? pages.empty? end diff --git a/app/models/user.rb b/app/models/user.rb index 73368be7b1b..87a2d999843 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -809,13 +809,13 @@ class User < ActiveRecord::Base def todos_done_count(force: false) Rails.cache.fetch(['users', id, 'todos_done_count'], force: force) do - todos.done.count + TodosFinder.new(self, state: :done).execute.count end end def todos_pending_count(force: false) Rails.cache.fetch(['users', id, 'todos_pending_count'], force: force) do - todos.pending.count + TodosFinder.new(self, state: :pending).execute.count end end diff --git a/app/services/merge_requests/get_urls_service.rb b/app/services/merge_requests/get_urls_service.rb index 501fd135e16..08c1f72d65a 100644 --- a/app/services/merge_requests/get_urls_service.rb +++ b/app/services/merge_requests/get_urls_service.rb @@ -30,10 +30,21 @@ module MergeRequests end def get_branches(changes) + return [] if project.empty_repo? + return [] unless project.merge_requests_enabled + changes_list = Gitlab::ChangesList.new(changes) changes_list.map do |change| next unless Gitlab::Git.branch_ref?(change[:ref]) - Gitlab::Git.branch_name(change[:ref]) + + # Deleted branch + next if Gitlab::Git.blank_ref?(change[:newrev]) + + # Default branch + branch_name = Gitlab::Git.branch_name(change[:ref]) + next if branch_name == project.default_branch + + branch_name end.compact end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 6b48d68cccb..eb833dd82ac 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -144,8 +144,9 @@ class TodoService def mark_todos_as_done(todos, current_user) todos = current_user.todos.where(id: todos.map(&:id)) unless todos.respond_to?(:update_all) - todos.update_all(state: :done) + marked_todos = todos.update_all(state: :done) current_user.update_todos_count_cache + marked_todos end # When user marks an issue as todo diff --git a/app/views/projects/blob/_image.html.haml b/app/views/projects/blob/_image.html.haml index 18caddabd39..4c356d1f07f 100644 --- a/app/views/projects/blob/_image.html.haml +++ b/app/views/projects/blob/_image.html.haml @@ -1,9 +1,15 @@ .file-content.image_file - if blob.svg? - - # We need to scrub SVG but we cannot do so in the RawController: it would - - # be wrong/strange if RawController modified the data. - - blob.load_all_data!(@repository) - - blob = sanitize_svg(blob) - %img{src: "data:#{blob.mime_type};base64,#{Base64.encode64(blob.data)}"} + - if blob.size_within_svg_limits? + - # We need to scrub SVG but we cannot do so in the RawController: it would + - # be wrong/strange if RawController modified the data. + - blob.load_all_data!(@repository) + - blob = sanitize_svg(blob) + %img{src: "data:#{blob.mime_type};base64,#{Base64.encode64(blob.data)}"} + - else + .nothing-here-block + The SVG could not be displayed as it is too large, you can + #{link_to('view the raw file', namespace_project_raw_path(@project.namespace, @project, @id), target: '_blank')} + instead. - else %img{src: namespace_project_raw_path(@project.namespace, @project, tree_join(@commit.id, blob.path))} diff --git a/app/views/projects/merge_requests/widget/_heading.html.haml b/app/views/projects/merge_requests/widget/_heading.html.haml index 6ef640bb654..494695a03a5 100644 --- a/app/views/projects/merge_requests/widget/_heading.html.haml +++ b/app/views/projects/merge_requests/widget/_heading.html.haml @@ -42,3 +42,16 @@ .ci_widget.ci-error{style: "display:none"} = icon("times-circle") Could not connect to the CI server. Please check your settings and try again. + +- @merge_request.environments.each do |environment| + .mr-widget-heading + .ci_widget.ci-success + = ci_icon_for_status("success") + %span.hidden-sm + Deployed to + = succeed '.' do + = link_to environment.name, namespace_project_environment_path(@project.namespace, @project, environment), class: 'environment' + - external_url = environment.external_url + - if external_url + = link_to external_url, target: '_blank' do + = icon('external-link', text: "View on #{external_url.gsub(/\A.*?:\/\//, '')}", right: true) diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index 92803838d02..281ec728e41 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -12,6 +12,8 @@ %li.project-row{ class: css_class } = cache(cache_key) do .controls + - if project.archived + %span.label.label-warning archived - if project.commit.try(:status) %span = render_commit_status(project.commit) diff --git a/db/schema.rb b/db/schema.rb index 7154ce54160..bf1d42db8ae 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -589,12 +589,12 @@ ActiveRecord::Schema.define(version: 20160810142633) do t.datetime "locked_at" t.integer "updated_by_id" t.string "merge_error" - t.text "merge_params" t.boolean "merge_when_build_succeeds", default: false, null: false t.integer "merge_user_id" t.string "merge_commit_sha" t.datetime "deleted_at" t.string "in_progress_merge_commit_sha" + t.text "merge_params" end add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree diff --git a/doc/api/services.md b/doc/api/services.md index f821a614047..579fdc0c8c9 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -355,7 +355,7 @@ PUT /projects/:id/services/gemnasium Parameters: -- `api_key` (**required**) - Your personal API KEY on gemnasium.com +- `api_key` (**required**) - Your personal API KEY on gemnasium.com - `token` (**required**) - The project's slug on gemnasium.com ### Delete Gemnasium service @@ -503,6 +503,7 @@ PUT /projects/:id/services/pivotaltracker Parameters: - `token` (**required**) +- `restrict_to_branch` (optional) - Comma-separated list of branches which will be automatically inspected. Leave blank to include all branches. ### Delete PivotalTracker service @@ -661,4 +662,3 @@ Get JetBrains TeamCity CI service settings for a project. ``` GET /projects/:id/services/teamcity ``` - diff --git a/features/steps/dashboard/dashboard.rb b/features/steps/dashboard/dashboard.rb index 80ed4c6d64c..a7d61bc28e0 100644 --- a/features/steps/dashboard/dashboard.rb +++ b/features/steps/dashboard/dashboard.rb @@ -26,6 +26,7 @@ class Spinach::Features::Dashboard < Spinach::FeatureSteps end step 'I see prefilled new Merge Request page' do + expect(page).to have_selector('.merge-request-form') expect(current_path).to eq new_namespace_project_merge_request_path(@project.namespace, @project) expect(find("#merge_request_target_project_id").value).to eq @project.id.to_s expect(find("input#merge_request_source_branch").value).to eq "fix" diff --git a/features/steps/dashboard/event_filters.rb b/features/steps/dashboard/event_filters.rb index 726b37cfde5..ca3cd0ecc4e 100644 --- a/features/steps/dashboard/event_filters.rb +++ b/features/steps/dashboard/event_filters.rb @@ -1,4 +1,5 @@ class Spinach::Features::EventFilters < Spinach::FeatureSteps + include WaitForAjax include SharedAuthentication include SharedPaths include SharedProject @@ -72,14 +73,20 @@ class Spinach::Features::EventFilters < Spinach::FeatureSteps end When 'I click "push" event filter' do - click_link("push_event_filter") + wait_for_ajax + click_link("Push events") + wait_for_ajax end When 'I click "team" event filter' do - click_link("team_event_filter") + wait_for_ajax + click_link("Team") + wait_for_ajax end When 'I click "merge" event filter' do - click_link("merged_event_filter") + wait_for_ajax + click_link("Merge events") + wait_for_ajax end end diff --git a/features/steps/dashboard/issues.rb b/features/steps/dashboard/issues.rb index 8706f0e8e78..39c65bb6cde 100644 --- a/features/steps/dashboard/issues.rb +++ b/features/steps/dashboard/issues.rb @@ -43,9 +43,14 @@ class Spinach::Features::DashboardIssues < Spinach::FeatureSteps step 'I click "All" link' do find(".js-author-search").click + expect(page).to have_selector(".dropdown-menu-author li a") find(".dropdown-menu-author li a", match: :first).click + expect(page).not_to have_selector(".dropdown-menu-author li a") + find(".js-assignee-search").click + expect(page).to have_selector(".dropdown-menu-assignee li a") find(".dropdown-menu-assignee li a", match: :first).click + expect(page).not_to have_selector(".dropdown-menu-assignee li a") end def should_see(issue) diff --git a/features/steps/dashboard/merge_requests.rb b/features/steps/dashboard/merge_requests.rb index 06db36c7014..6777101fb15 100644 --- a/features/steps/dashboard/merge_requests.rb +++ b/features/steps/dashboard/merge_requests.rb @@ -47,9 +47,14 @@ class Spinach::Features::DashboardMergeRequests < Spinach::FeatureSteps step 'I click "All" link' do find(".js-author-search").click + expect(page).to have_selector(".dropdown-menu-author li a") find(".dropdown-menu-author li a", match: :first).click + expect(page).not_to have_selector(".dropdown-menu-author li a") + find(".js-assignee-search").click + expect(page).to have_selector(".dropdown-menu-assignee li a") find(".dropdown-menu-assignee li a", match: :first).click + expect(page).not_to have_selector(".dropdown-menu-assignee li a") end def should_see(merge_request) diff --git a/features/steps/dashboard/new_project.rb b/features/steps/dashboard/new_project.rb index 727a6a71373..dcfa88f69fc 100644 --- a/features/steps/dashboard/new_project.rb +++ b/features/steps/dashboard/new_project.rb @@ -29,6 +29,7 @@ class Spinach::Features::NewProject < Spinach::FeatureSteps end step 'I am redirected to the GitHub import page' do + expect(page).to have_content('Import Projects from GitHub') expect(current_path).to eq new_import_github_path end @@ -47,6 +48,7 @@ class Spinach::Features::NewProject < Spinach::FeatureSteps end step 'I redirected to Google Code import page' do + expect(page).to have_content('Import projects from Google Code') expect(current_path).to eq new_import_google_code_path end end diff --git a/features/steps/project/builds/artifacts.rb b/features/steps/project/builds/artifacts.rb index b4a32ed2e38..055fca036d3 100644 --- a/features/steps/project/builds/artifacts.rb +++ b/features/steps/project/builds/artifacts.rb @@ -10,6 +10,7 @@ class Spinach::Features::ProjectBuildsArtifacts < Spinach::FeatureSteps step 'I click artifacts browse button' do click_link 'Browse' + expect(page).not_to have_selector('.build-sidebar') end step 'I should see content of artifacts archive' do diff --git a/features/steps/project/forked_merge_requests.rb b/features/steps/project/forked_merge_requests.rb index 6b56a77b832..dacab6c7977 100644 --- a/features/steps/project/forked_merge_requests.rb +++ b/features/steps/project/forked_merge_requests.rb @@ -34,6 +34,9 @@ class Spinach::Features::ProjectForkedMergeRequests < Spinach::FeatureSteps end step 'I fill out a "Merge Request On Forked Project" merge request' do + expect(page).to have_content('Source branch') + expect(page).to have_content('Target branch') + first('.js-source-project').click first('.dropdown-source-project a', text: @forked_project.path_with_namespace) diff --git a/features/steps/project/issues/issues.rb b/features/steps/project/issues/issues.rb index 35f166c7c08..daee90b3767 100644 --- a/features/steps/project/issues/issues.rb +++ b/features/steps/project/issues/issues.rb @@ -354,6 +354,8 @@ class Spinach::Features::ProjectIssues < Spinach::FeatureSteps end def filter_issue(text) + sleep 1 fill_in 'issue_search', with: text + sleep 1 end end diff --git a/features/steps/project/merge_requests.rb b/features/steps/project/merge_requests.rb index a02a54923a5..53d1aedf27f 100644 --- a/features/steps/project/merge_requests.rb +++ b/features/steps/project/merge_requests.rb @@ -489,10 +489,12 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps end step 'I fill in merge request search with "Fe"' do + sleep 1 fill_in 'issue_search', with: "Fe" end step 'I click the "Target branch" dropdown' do + expect(page).to have_content('Target branch') first('.target_branch').click end diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb index 9a8896acb15..841d191d55b 100644 --- a/features/steps/project/source/browse_files.rb +++ b/features/steps/project/source/browse_files.rb @@ -69,6 +69,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps end step 'I edit code' do + expect(page).to have_selector('.file-editor') set_new_content end @@ -131,6 +132,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps step 'I click on "New file" link in repo' do find('.add-to-tree').click click_link 'New file' + expect(page).to have_selector('.file-editor') end step 'I click on "Upload file" link in repo' do diff --git a/features/steps/project/wiki.rb b/features/steps/project/wiki.rb index 732dc5d0b93..07a955b1a14 100644 --- a/features/steps/project/wiki.rb +++ b/features/steps/project/wiki.rb @@ -142,7 +142,9 @@ class Spinach::Features::ProjectWiki < Spinach::FeatureSteps end step 'I edit the Wiki page with a path' do + expect(page).to have_content('three') click_on 'three' + expect(find('.nav-text')).to have_content('Three') click_on 'Edit' end diff --git a/features/steps/shared/issuable.rb b/features/steps/shared/issuable.rb index b5fd24d246f..aa666a954bc 100644 --- a/features/steps/shared/issuable.rb +++ b/features/steps/shared/issuable.rb @@ -133,9 +133,7 @@ module SharedIssuable end step 'The list should be sorted by "Oldest updated"' do - page.within('.content div.dropdown.inline.prepend-left-10') do - expect(page.find('button.dropdown-toggle.btn')).to have_content('Oldest updated') - end + expect(find('.issues-filters')).to have_content('Oldest updated') end step 'I click link "Next" in the sidebar' do diff --git a/features/support/wait_for_ajax.rb b/features/support/wait_for_ajax.rb new file mode 100644 index 00000000000..b90fc112671 --- /dev/null +++ b/features/support/wait_for_ajax.rb @@ -0,0 +1,11 @@ +module WaitForAjax + def wait_for_ajax + Timeout.timeout(Capybara.default_max_wait_time) do + loop until finished_all_ajax_requests? + end + end + + def finished_all_ajax_requests? + page.evaluate_script('jQuery.active').zero? + end +end diff --git a/lib/api/todos.rb b/lib/api/todos.rb index 26c24c3baff..19df13d8aac 100644 --- a/lib/api/todos.rb +++ b/lib/api/todos.rb @@ -61,9 +61,9 @@ module API # delete ':id' do todo = current_user.todos.find(params[:id]) - todo.done + TodoService.new.mark_todos_as_done([todo], current_user) - present todo, with: Entities::Todo, current_user: current_user + present todo.reload, with: Entities::Todo, current_user: current_user end # Mark all todos as done @@ -73,9 +73,7 @@ module API # delete do todos = find_todos - todos.each(&:done) - - todos.length + TodoService.new.mark_todos_as_done(todos, current_user) end end end diff --git a/lib/banzai/filter/sanitization_filter.rb b/lib/banzai/filter/sanitization_filter.rb index ca80aac5a08..6e13282d5f4 100644 --- a/lib/banzai/filter/sanitization_filter.rb +++ b/lib/banzai/filter/sanitization_filter.rb @@ -7,7 +7,7 @@ module Banzai UNSAFE_PROTOCOLS = %w(data javascript vbscript).freeze def whitelist - whitelist = super + whitelist = super.dup customize_whitelist(whitelist) @@ -42,6 +42,8 @@ module Banzai # Allow any protocol in `a` elements... whitelist[:protocols].delete('a') + whitelist[:transformers] = whitelist[:transformers].dup + # ...but then remove links with unsafe protocols whitelist[:transformers].push(remove_unsafe_links) diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb index 52f117e963b..4b32eb966aa 100644 --- a/lib/gitlab/checks/change_access.rb +++ b/lib/gitlab/checks/change_access.rb @@ -11,7 +11,7 @@ module Gitlab end def exec - error = protected_branch_checks || tag_checks || push_checks + error = push_checks || tag_checks || protected_branch_checks if error GitAccessStatus.new(false, error) diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index 7e71a030901..76b2178c79c 100755 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -20,10 +20,11 @@ if [ -f /.dockerenv ] || [ -f ./dockerinit ]; then # Install phantomjs package pushd vendor/apt - if [ ! -e phantomjs_1.9.8-0jessie_amd64.deb ]; then - wget -q https://gitlab.com/axil/phantomjs-debian/raw/master/phantomjs_1.9.8-0jessie_amd64.deb + PHANTOMJS_FILE="phantomjs-$PHANTOMJS_VERSION-linux-x86_64" + if [ ! -d "$PHANTOMJS_FILE" ]; then + curl -q -L "https://s3.amazonaws.com/gitlab-build-helpers/$PHANTOMJS_FILE.tar.bz2" | tar jx fi - dpkg -i phantomjs_1.9.8-0jessie_amd64.deb + cp "$PHANTOMJS_FILE/bin/phantomjs" "/usr/bin/" popd # Try to install packages diff --git a/spec/features/issuables/default_sort_order_spec.rb b/spec/features/issuables/default_sort_order_spec.rb index 0d495cd04aa..9114f751b55 100644 --- a/spec/features/issuables/default_sort_order_spec.rb +++ b/spec/features/issuables/default_sort_order_spec.rb @@ -55,7 +55,7 @@ describe 'Projects > Issuables > Default sort order', feature: true do it 'is "last updated"' do visit_merge_requests_with_state(project, 'merged') - expect(selected_sort_order).to eq('last updated') + expect(find('.issues-other-filters')).to have_content('Last updated') expect(first_merge_request).to include(last_updated_issuable.title) expect(last_merge_request).to include(first_updated_issuable.title) end @@ -67,7 +67,7 @@ describe 'Projects > Issuables > Default sort order', feature: true do it 'is "last updated"' do visit_merge_requests_with_state(project, 'closed') - expect(selected_sort_order).to eq('last updated') + expect(find('.issues-other-filters')).to have_content('Last updated') expect(first_merge_request).to include(last_updated_issuable.title) expect(last_merge_request).to include(first_updated_issuable.title) end @@ -79,7 +79,7 @@ describe 'Projects > Issuables > Default sort order', feature: true do it 'is "last created"' do visit_merge_requests_with_state(project, 'all') - expect(selected_sort_order).to eq('last created') + expect(find('.issues-other-filters')).to have_content('Last created') expect(first_merge_request).to include(last_created_issuable.title) expect(last_merge_request).to include(first_created_issuable.title) end @@ -108,7 +108,7 @@ describe 'Projects > Issuables > Default sort order', feature: true do it 'is "last created"' do visit_issues project - expect(selected_sort_order).to eq('last created') + expect(find('.issues-other-filters')).to have_content('Last created') expect(first_issue).to include(last_created_issuable.title) expect(last_issue).to include(first_created_issuable.title) end @@ -120,7 +120,7 @@ describe 'Projects > Issuables > Default sort order', feature: true do it 'is "last created"' do visit_issues_with_state(project, 'open') - expect(selected_sort_order).to eq('last created') + expect(find('.issues-other-filters')).to have_content('Last created') expect(first_issue).to include(last_created_issuable.title) expect(last_issue).to include(first_created_issuable.title) end @@ -132,7 +132,7 @@ describe 'Projects > Issuables > Default sort order', feature: true do it 'is "last updated"' do visit_issues_with_state(project, 'closed') - expect(selected_sort_order).to eq('last updated') + expect(find('.issues-other-filters')).to have_content('Last updated') expect(first_issue).to include(last_updated_issuable.title) expect(last_issue).to include(first_updated_issuable.title) end @@ -144,7 +144,7 @@ describe 'Projects > Issuables > Default sort order', feature: true do it 'is "last created"' do visit_issues_with_state(project, 'all') - expect(selected_sort_order).to eq('last created') + expect(find('.issues-other-filters')).to have_content('Last created') expect(first_issue).to include(last_created_issuable.title) expect(last_issue).to include(first_created_issuable.title) end diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index ea81ee54c90..e262f285868 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -117,7 +117,7 @@ describe 'Filter issues', feature: true do find('.dropdown-menu-user-link', text: user.username).click - wait_for_ajax + expect(page).not_to have_selector('.issues-list .issue') find('.js-label-select').click diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb index e296078bad8..11c9de3c4bf 100644 --- a/spec/features/merge_requests/create_new_mr_spec.rb +++ b/spec/features/merge_requests/create_new_mr_spec.rb @@ -13,6 +13,8 @@ feature 'Create New Merge Request', feature: true, js: true do it 'generates a diff for an orphaned branch' do click_link 'New Merge Request' + expect(page).to have_content('Source branch') + expect(page).to have_content('Target branch') first('.js-source-branch').click first('.dropdown-source-branch .dropdown-content a', text: 'orphaned-branch').click diff --git a/spec/features/profiles/preferences_spec.rb b/spec/features/profiles/preferences_spec.rb index 787bf42d048..d14a1158b67 100644 --- a/spec/features/profiles/preferences_spec.rb +++ b/spec/features/profiles/preferences_spec.rb @@ -68,10 +68,14 @@ describe 'Profile > Preferences', feature: true do allowing_for_delay do find('#logo').click + + expect(page).to have_content("You don't have starred projects yet") expect(page.current_path).to eq starred_dashboard_projects_path end click_link 'Your Projects' + + expect(page).not_to have_content("You don't have starred projects yet") expect(page.current_path).to eq dashboard_projects_path end end diff --git a/spec/features/projects/files/project_owner_creates_license_file_spec.rb b/spec/features/projects/files/project_owner_creates_license_file_spec.rb index e1e105e6bbe..dbd07464444 100644 --- a/spec/features/projects/files/project_owner_creates_license_file_spec.rb +++ b/spec/features/projects/files/project_owner_creates_license_file_spec.rb @@ -39,6 +39,7 @@ feature 'project owner creates a license file', feature: true, js: true do scenario 'project master creates a license file from the "Add license" link' do click_link 'Add License' + expect(page).to have_content('New File') expect(current_path).to eq( namespace_project_new_blob_path(project.namespace, project, 'master')) expect(find('#file_name').value).to eq('LICENSE') diff --git a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb index 67aac25e427..45bf0c0d038 100644 --- a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb +++ b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb @@ -14,6 +14,7 @@ feature 'project owner sees a link to create a license file in empty project', f visit namespace_project_path(project.namespace, project) click_link 'Create empty bare repository' click_on 'LICENSE' + expect(page).to have_content('New File') expect(current_path).to eq( namespace_project_new_blob_path(project.namespace, project, 'master')) diff --git a/spec/features/u2f_spec.rb b/spec/features/u2f_spec.rb index 9335f5bf120..d370f90f7d9 100644 --- a/spec/features/u2f_spec.rb +++ b/spec/features/u2f_spec.rb @@ -1,8 +1,16 @@ require 'spec_helper' feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: true, js: true do + include WaitForAjax + before { allow_any_instance_of(U2fHelper).to receive(:inject_u2f_api?).and_return(true) } + def manage_two_factor_authentication + click_on 'Manage Two-Factor Authentication' + expect(page).to have_content("Setup New U2F Device") + wait_for_ajax + end + def register_u2f_device(u2f_device = nil) u2f_device ||= FakeU2fDevice.new(page) u2f_device.respond_to_u2f_registration @@ -34,7 +42,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: describe 'when 2FA via OTP is enabled' do it 'allows registering a new device' do visit profile_account_path - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication expect(page.body).to match("You've already enabled two-factor authentication using mobile") register_u2f_device @@ -46,15 +54,15 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: visit profile_account_path # First device - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication register_u2f_device expect(page.body).to match('Your U2F device was registered') # Second device - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication register_u2f_device expect(page.body).to match('Your U2F device was registered') - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication expect(page.body).to match('You have 2 U2F devices registered') end end @@ -62,7 +70,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: it 'allows the same device to be registered for multiple users' do # First user visit profile_account_path - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication u2f_device = register_u2f_device expect(page.body).to match('Your U2F device was registered') logout @@ -71,7 +79,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: user = login_as(:user) user.update_attribute(:otp_required_for_login, true) visit profile_account_path - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication register_u2f_device(u2f_device) expect(page.body).to match('Your U2F device was registered') @@ -81,7 +89,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: context "when there are form errors" do it "doesn't register the device if there are errors" do visit profile_account_path - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication # Have the "u2f device" respond with bad data page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };") @@ -96,7 +104,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: it "allows retrying registration" do visit profile_account_path - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication # Failed registration page.execute_script("u2f.register = function(_,_,_,callback) { callback('bad response'); };") @@ -122,7 +130,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: login_as(user) user.update_attribute(:otp_required_for_login, true) visit profile_account_path - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication @u2f_device = register_u2f_device logout end @@ -161,7 +169,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: current_user = login_as(:user) current_user.update_attribute(:otp_required_for_login, true) visit profile_account_path - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication register_u2f_device logout @@ -182,7 +190,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: current_user = login_as(:user) current_user.update_attribute(:otp_required_for_login, true) visit profile_account_path - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication register_u2f_device(@u2f_device) logout @@ -248,7 +256,7 @@ feature 'Using U2F (Universal 2nd Factor) Devices for Authentication', feature: user = login_as(:user) user.update_attribute(:otp_required_for_login, true) visit profile_account_path - click_on 'Manage Two-Factor Authentication' + manage_two_factor_authentication expect(page).to have_content("Your U2F device needs to be set up.") register_u2f_device end diff --git a/spec/features/variables_spec.rb b/spec/features/variables_spec.rb index 61f2bc61e0c..d7880d5778f 100644 --- a/spec/features/variables_spec.rb +++ b/spec/features/variables_spec.rb @@ -42,6 +42,7 @@ describe 'Project variables', js: true do find('.btn-variable-edit').click end + expect(page).to have_content('Update variable') fill_in('variable_key', with: 'key') fill_in('variable_value', with: 'key value') click_button('Save variable') diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index 0a1cc3b3df7..7a3a74335e8 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -23,73 +23,36 @@ describe ProjectsFinder do let(:finder) { described_class.new } - describe 'without a group' do - describe 'without a user' do - subject { finder.execute } + describe 'without a user' do + subject { finder.execute } - it { is_expected.to eq([public_project]) } - end - - describe 'with a user' do - subject { finder.execute(user) } - - describe 'without private projects' do - it { is_expected.to eq([public_project, internal_project]) } - end - - describe 'with private projects' do - before do - private_project.team.add_user(user, Gitlab::Access::MASTER) - end - - it do - is_expected.to eq([public_project, internal_project, - private_project]) - end - end - end + it { is_expected.to eq([public_project]) } end - describe 'with a group' do - describe 'without a user' do - subject { finder.execute(nil, group: group) } + describe 'with a user' do + subject { finder.execute(user) } - it { is_expected.to eq([public_project]) } + describe 'without private projects' do + it { is_expected.to eq([public_project, internal_project]) } end - describe 'with a user' do - subject { finder.execute(user, group: group) } - - describe 'without shared projects' do - it { is_expected.to eq([public_project, internal_project]) } + describe 'with private projects' do + before do + private_project.team.add_user(user, Gitlab::Access::MASTER) end - describe 'with shared projects and group membership' do - before do - group.add_user(user, Gitlab::Access::DEVELOPER) - - shared_project.project_group_links. - create(group_access: Gitlab::Access::MASTER, group: group) - end - - it do - is_expected.to eq([shared_project, public_project, internal_project]) - end + it do + is_expected.to eq([public_project, internal_project, private_project]) end + end + end - describe 'with shared projects and project membership' do - before do - shared_project.team.add_user(user, Gitlab::Access::DEVELOPER) + describe 'with project_ids_relation' do + let(:project_ids_relation) { Project.where(id: internal_project.id) } - shared_project.project_group_links. - create(group_access: Gitlab::Access::MASTER, group: group) - end + subject { finder.execute(user, project_ids_relation) } - it do - is_expected.to eq([shared_project, public_project, internal_project]) - end - end - end + it { is_expected.to eq([internal_project]) } end end end diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb new file mode 100644 index 00000000000..39069b49978 --- /dev/null +++ b/spec/lib/gitlab/checks/change_access_spec.rb @@ -0,0 +1,99 @@ +require 'spec_helper' + +describe Gitlab::Checks::ChangeAccess, lib: true do + describe '#exec' do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:user_access) { Gitlab::UserAccess.new(user, project: project) } + let(:changes) do + { + oldrev: 'be93687618e4b132087f430a4d8fc3a609c9b77c', + newrev: '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51', + ref: 'refs/heads/master' + } + end + + subject { described_class.new(changes, project: project, user_access: user_access).exec } + + before { allow(user_access).to receive(:can_do_action?).with(:push_code).and_return(true) } + + context 'without failed checks' do + it "doesn't return any error" do + expect(subject.status).to be(true) + end + end + + context 'when the user is not allowed to push code' do + it 'returns an error' do + expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) + + expect(subject.status).to be(false) + expect(subject.message).to eq('You are not allowed to push code to this project.') + end + end + + context 'tags check' do + let(:changes) do + { + oldrev: 'be93687618e4b132087f430a4d8fc3a609c9b77c', + newrev: '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51', + ref: 'refs/tags/v1.0.0' + } + end + + it 'returns an error if the user is not allowed to update tags' do + expect(user_access).to receive(:can_do_action?).with(:admin_project).and_return(false) + + expect(subject.status).to be(false) + expect(subject.message).to eq('You are not allowed to change existing tags on this project.') + end + end + + context 'protected branches check' do + before do + allow(project).to receive(:protected_branch?).with('master').and_return(true) + end + + it 'returns an error if the user is not allowed to do forced pushes to protected branches' do + expect(Gitlab::Checks::ForcePush).to receive(:force_push?).and_return(true) + expect(user_access).to receive(:can_do_action?).with(:force_push_code_to_protected_branches).and_return(false) + + expect(subject.status).to be(false) + expect(subject.message).to eq('You are not allowed to force push code to a protected branch on this project.') + end + + it 'returns an error if the user is not allowed to merge to protected branches' do + expect_any_instance_of(Gitlab::Checks::MatchingMergeRequest).to receive(:match?).and_return(true) + expect(user_access).to receive(:can_merge_to_branch?).and_return(false) + expect(user_access).to receive(:can_push_to_branch?).and_return(false) + + expect(subject.status).to be(false) + expect(subject.message).to eq('You are not allowed to merge code into protected branches on this project.') + end + + it 'returns an error if the user is not allowed to push to protected branches' do + expect(user_access).to receive(:can_push_to_branch?).and_return(false) + + expect(subject.status).to be(false) + expect(subject.message).to eq('You are not allowed to push code to protected branches on this project.') + end + + context 'branch deletion' do + let(:changes) do + { + oldrev: 'be93687618e4b132087f430a4d8fc3a609c9b77c', + newrev: '0000000000000000000000000000000000000000', + ref: 'refs/heads/master' + } + end + + it 'returns an error if the user is not allowed to delete protected branches' do + expect(user_access).to receive(:can_do_action?).with(:remove_protected_branches).and_return(false) + + expect(subject.status).to be(false) + expect(subject.message).to eq('You are not allowed to delete protected branches from this project.') + end + end + end + end +end diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 1e5d6a34f83..cee20234e1f 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -94,4 +94,26 @@ describe Blob do expect(blob.to_partial_path).to eq 'download' end end + + describe '#size_within_svg_limits?' do + let(:blob) { described_class.decorate(double(:blob)) } + + it 'returns true when the blob size is smaller than the SVG limit' do + expect(blob).to receive(:size).and_return(42) + + expect(blob.size_within_svg_limits?).to eq(true) + end + + it 'returns true when the blob size is equal to the SVG limit' do + expect(blob).to receive(:size).and_return(Blob::MAXIMUM_SVG_SIZE) + + expect(blob.size_within_svg_limits?).to eq(true) + end + + it 'returns false when the blob size is larger than the SVG limit' do + expect(blob).to receive(:size).and_return(1.terabyte) + + expect(blob.size_within_svg_limits?).to eq(false) + end + end end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 7df3df4bb9e..bfff639ad78 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -15,4 +15,28 @@ describe Deployment, models: true do it { is_expected.to validate_presence_of(:ref) } it { is_expected.to validate_presence_of(:sha) } + + describe '#includes_commit?' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + let(:deployment) do + create(:deployment, environment: environment, sha: project.commit.id) + end + + context 'when there is no project commit' do + it 'returns false' do + commit = project.commit('feature') + + expect(deployment.includes_commit?(commit)).to be false + end + end + + context 'when they share the same tree branch' do + it 'returns true' do + commit = project.commit + + expect(deployment.includes_commit?(commit)).to be true + end + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 8a84ac0a7c7..c881897926e 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -30,4 +30,37 @@ describe Environment, models: true do expect(env.external_url).to be_nil end end + + describe '#includes_commit?' do + context 'without a last deployment' do + it "returns false" do + expect(environment.includes_commit?('HEAD')).to be false + end + end + + context 'with a last deployment' do + let(:project) { create(:project) } + let(:environment) { create(:environment, project: project) } + + let!(:deployment) do + create(:deployment, environment: environment, sha: project.commit('master').id) + end + + context 'in the same branch' do + it 'returns true' do + expect(environment.includes_commit?(RepoHelpers.sample_commit)).to be true + end + end + + context 'not in the same branch' do + before do + deployment.update(sha: project.commit('feature').id) + end + + it 'returns false' do + expect(environment.includes_commit?(RepoHelpers.sample_commit)).to be false + end + end + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 3270b877c1a..35a4418ebb3 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -674,6 +674,21 @@ describe MergeRequest, models: true do end end + describe "#environments" do + let(:project) { create(:project) } + let!(:environment) { create(:environment, project: project) } + let!(:environment1) { create(:environment, project: project) } + let!(:environment2) { create(:environment, project: project) } + let(:merge_request) { create(:merge_request, source_project: project) } + + it 'selects deployed environments' do + create(:deployment, environment: environment, sha: project.commit('master').id) + create(:deployment, environment: environment1, sha: project.commit('feature').id) + + expect(merge_request.environments).to eq [environment] + end + end + describe "#reload_diff" do let(:note) { create(:diff_note_on_merge_request, project: subject.project, noteable: subject) } diff --git a/spec/models/project_services/pivotaltracker_service_spec.rb b/spec/models/project_services/pivotaltracker_service_spec.rb index f37edd4d970..d098d988521 100644 --- a/spec/models/project_services/pivotaltracker_service_spec.rb +++ b/spec/models/project_services/pivotaltracker_service_spec.rb @@ -39,4 +39,75 @@ describe PivotaltrackerService, models: true do it { is_expected.not_to validate_presence_of(:token) } end end + + describe 'Execute' do + let(:service) do + PivotaltrackerService.new.tap do |service| + service.token = 'secret_api_token' + end + end + + let(:url) { PivotaltrackerService::API_ENDPOINT } + + def push_data(branch: 'master') + { + object_kind: 'push', + ref: "refs/heads/#{branch}", + commits: [ + { + id: '21c12ea', + author: { + name: 'Some User' + }, + url: 'https://example.com/commit', + message: 'commit message', + } + ] + } + end + + before do + WebMock.stub_request(:post, url) + end + + it 'should post correct message' do + service.execute(push_data) + expect(WebMock).to have_requested(:post, url).with( + body: { + 'source_commit' => { + 'commit_id' => '21c12ea', + 'author' => 'Some User', + 'url' => 'https://example.com/commit', + 'message' => 'commit message' + } + }, + headers: { + 'Content-Type' => 'application/json', + 'X-TrackerToken' => 'secret_api_token' + } + ).once + end + + context 'when allowed branches is specified' do + let(:service) do + super().tap do |service| + service.restrict_to_branch = 'master,v10' + end + end + + it 'should post message if branch is in the list' do + service.execute(push_data(branch: 'master')) + service.execute(push_data(branch: 'v10')) + + expect(WebMock).to have_requested(:post, url).twice + end + + it 'should not post message if branch is not in the list' do + service.execute(push_data(branch: 'mas')) + service.execute(push_data(branch: 'v11')) + + expect(WebMock).not_to have_requested(:post, url) + end + end + end end diff --git a/spec/requests/api/todos_spec.rb b/spec/requests/api/todos_spec.rb index 3ccd0af652f..887a2ba5b84 100644 --- a/spec/requests/api/todos_spec.rb +++ b/spec/requests/api/todos_spec.rb @@ -117,6 +117,12 @@ describe API::Todos, api: true do expect(response.status).to eq(200) expect(pending_1.reload).to be_done end + + it 'updates todos cache' do + expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original + + delete api("/todos/#{pending_1.id}", john_doe) + end end end @@ -139,6 +145,12 @@ describe API::Todos, api: true do expect(pending_2.reload).to be_done expect(pending_3.reload).to be_done end + + it 'updates todos cache' do + expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original + + delete api("/todos", john_doe) + end end end diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index ec26770c3eb..8a4b76367e3 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -7,7 +7,9 @@ describe MergeRequests::GetUrlsService do let(:new_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" } let(:show_merge_request_url) { "http://localhost/#{project.namespace.name}/#{project.path}/merge_requests/#{merge_request.iid}" } let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } + let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" } let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } + let(:default_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master" } describe "#execute" do shared_examples 'new_merge_request_link' do @@ -32,6 +34,28 @@ describe MergeRequests::GetUrlsService do end end + shared_examples 'no_merge_request_url' do + it 'returns no URL' do + result = service.execute(changes) + expect(result).to be_empty + end + end + + context 'pushing to default branch' do + let(:changes) { default_branch_changes } + it_behaves_like 'no_merge_request_url' + end + + context 'pushing to project with MRs disabled' do + let(:changes) { new_branch_changes } + + before do + project.merge_requests_enabled = false + end + + it_behaves_like 'no_merge_request_url' + end + context 'pushing one completely new branch' do let(:changes) { new_branch_changes } it_behaves_like 'new_merge_request_link' @@ -42,6 +66,11 @@ describe MergeRequests::GetUrlsService do it_behaves_like 'new_merge_request_link' end + context 'pushing to deleted branch' do + let(:changes) { deleted_branch_changes } + it_behaves_like 'no_merge_request_url' + end + context 'pushing to existing branch and merge request opened' do let!(:merge_request) { create(:merge_request, source_project: project, source_branch: source_branch) } let(:changes) { existing_branch_changes } @@ -61,6 +90,11 @@ describe MergeRequests::GetUrlsService do let(:changes) { existing_branch_changes } # Source project is now the forked one let(:service) { MergeRequests::GetUrlsService.new(forked_project) } + + before do + allow(forked_project).to receive(:empty_repo?).and_return(false) + end + it_behaves_like 'show_merge_request_url' end diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 34d8ea9090e..6c3cbeae13c 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -472,6 +472,42 @@ describe TodoService, services: true do expect(john_doe.todos_pending_count).to eq(1) end + describe '#mark_todos_as_done' do + let(:issue) { create(:issue, project: project, author: author, assignee: john_doe) } + + it 'marks a relation of todos as done' do + create(:todo, :mentioned, user: john_doe, target: issue, project: project) + + todos = TodosFinder.new(john_doe, {}).execute + expect { TodoService.new.mark_todos_as_done(todos, john_doe) } + .to change { john_doe.todos.done.count }.from(0).to(1) + end + + it 'marks an array of todos as done' do + todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project) + + expect { TodoService.new.mark_todos_as_done([todo], john_doe) } + .to change { todo.reload.state }.from('pending').to('done') + end + + it 'returns the number of updated todos' do # Needed on API + todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project) + + expect(TodoService.new.mark_todos_as_done([todo], john_doe)).to eq(1) + end + + it 'caches the number of todos of a user', :caching do + create(:todo, :mentioned, user: john_doe, target: issue, project: project) + todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project) + TodoService.new.mark_todos_as_done([todo], john_doe) + + expect_any_instance_of(TodosFinder).not_to receive(:execute) + + expect(john_doe.todos_done_count).to eq(1) + expect(john_doe.todos_pending_count).to eq(1) + end + end + def should_create_todo(attributes = {}) attributes.reverse_merge!( project: project, diff --git a/spec/views/projects/merge_requests/_heading.html.haml_spec.rb b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb new file mode 100644 index 00000000000..733b2dfa7ff --- /dev/null +++ b/spec/views/projects/merge_requests/_heading.html.haml_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe 'projects/merge_requests/widget/_heading' do + include Devise::TestHelpers + + context 'when released to an environment' do + let(:project) { merge_request.target_project } + let(:merge_request) { create(:merge_request, :merged) } + let(:environment) { create(:environment, project: project) } + let!(:deployment) do + create(:deployment, environment: environment, sha: project.commit('master').id) + end + + before do + assign(:merge_request, merge_request) + assign(:project, project) + + render + end + + it 'displays that the environment is deployed' do + expect(rendered).to match("Deployed to") + expect(rendered).to match("#{environment.name}") + end + end +end |