diff options
| author | James Lopez <james@jameslopez.es> | 2016-07-13 13:44:07 +0200 |
|---|---|---|
| committer | James Lopez <james@jameslopez.es> | 2016-07-13 13:44:07 +0200 |
| commit | 0b2469a4e5ee5d8deec072be3426c28133927740 (patch) | |
| tree | f8815236640f584ae3b710ed9a9ea8a71857fdd1 | |
| parent | c607b1c941fbfaca22dd78dae2fd67d873b56828 (diff) | |
| parent | 9ca633eb4c62231e4ddff5466c723cf8e2bdb25d (diff) | |
| download | gitlab-ce-0b2469a4e5ee5d8deec072be3426c28133927740.tar.gz | |
Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into fix/persistent-import-data
170 files changed, 1734 insertions, 1034 deletions
diff --git a/.hound.yml b/.hound.yml deleted file mode 100644 index 3bde29fb2bf..00000000000 --- a/.hound.yml +++ /dev/null @@ -1,4 +0,0 @@ -# Prefer single quotes -StringLiterals: - EnforcedStyle: single_quotes - Enabled: true diff --git a/.teatro.yml b/.teatro.yml deleted file mode 100644 index 30054361981..00000000000 --- a/.teatro.yml +++ /dev/null @@ -1,8 +0,0 @@ -stage: - before: - - cp config/gitlab.teatro.yml config/gitlab.yml - - mkdir /apps/gitlab-satellites - - mkdir /apps/repositories - - database: - - RAILS_ENV=development force=yes bundle exec rake db:create gitlab:setup
\ No newline at end of file diff --git a/CHANGELOG b/CHANGELOG index 523fd2d6ba3..3af947be599 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,11 +1,14 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.10.0 (unreleased) + - Expose {should,force}_remove_source_branch (Ben Boeckel) - Fix commit builds API, return all builds for all pipelines for given commit. !4849 - Replace Haml with Hamlit to make view rendering faster. !3666 + - Expire the branch cache after `git gc` runs - Refactor repository paths handling to allow multiple git mount points - Optimize system note visibility checking by memoizing the visible reference count !5070 - Add Application Setting to configure default Repository Path for new projects + - Delete award emoji when deleting a user - Remove pinTo from Flash and make inline flash messages look nicer !4854 (winniehell) - Wrap code blocks on Activies and Todos page. !4783 (winniehell) - Align flash messages with left side of page content !4959 (winniehell) @@ -30,7 +33,9 @@ v 8.10.0 (unreleased) - Exclude email check from the standard health check - Updated layout for Projects, Groups, Users on Admin area !4424 - Fix changing issue state columns in milestone view + - Update health_check gem to version 2.1.0 - Add notification settings dropdown for groups + - Render inline diffs for multiple changed lines following eachother - Wildcards for protected branches. !4665 - Allow importing from Github using Personal Access Tokens. (Eric K Idema) - API: Todos !3188 (Robert Schilling) @@ -38,6 +43,7 @@ v 8.10.0 (unreleased) - Add "Enabled Git access protocols" to Application Settings - Diffs will create button/diff form on demand no on server side - Reduce size of HTML used by diff comment forms + - Protected branches have a "Developers can Merge" setting. !4892 (original implementation by Mathias Vestergaard) - Fix user creation with stronger minimum password requirements !4054 (nathan-pmt) - Only show New Snippet button to users that can create snippets. - PipelinesFinder uses git cache data @@ -46,14 +52,17 @@ v 8.10.0 (unreleased) - Collapse large diffs by default (!4990) - Check for conflicts with existing Project's wiki path when creating a new project. - Show last push widget in upstream after push to fork + - Cache todos pending/done dashboard query counts. - Don't instantiate a git tree on Projects show default view - Bump Rinku to 2.0.0 - Remove unused front-end variable -> default_issues_tracker + - ObjectRenderer retrieve renderer content using Rails.cache.read_multi - Better caching of git calls on ProjectsController#show. - Avoid to retrieve MR closes_issues as much as possible. - Add API endpoint for a group issues !4520 (mahcsig) - Add Bugzilla integration !4930 (iamtjg) - Instrument Rinku usage + - Be explicit to define merge request discussion variables - 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) @@ -67,12 +76,25 @@ v 8.10.0 (unreleased) - Allow '?', or '&' for label names - Fix importer for GitHub Pull Requests when a branch was reused across Pull Requests - Add date when user joined the team on the member page - - Fix 404 redirect after validation fails importing a GitLab project + - Fix 404 redirect after validation fails importing a GitLab project - Fix 404 redirect after validation fails importing a GitLab project - Added setting to set new users by default as external !4545 (Dravere) - Add min value for project limit field on user's form !3622 (jastkand) + - Reset project pushes_since_gc when we enqueue the git gc call - Add reminder to not paste private SSH keys !4399 (Ingo Blechschmidt) - Remove duplicate `description` field in `MergeRequest` entities (Ben Boeckel) + - Style of import project buttons were fixed in the new project page. !5183 (rdemirbay) + - Fix GitHub client requests when rate limit is disabled + - Optimistic locking for Issues and Merge Requests (Title and description overriding prevention) + - Redesign Builds and Pipelines pages + - Change status color and icon for running builds + +v 8.9.6 + - Fix importing of events under notes for GitLab projects. !5154 + - Fix log statements in import/export. !5129 + - Fix commit avatar alignment in compare view. !5128 + - Fix broken migration in MySQL. !5005 + - Overwrite Host and X-Forwarded-Host headers in NGINX !5213 v 8.9.7 (unreleased) - Fix import_data wrongly saved as a result of an invalid import_url diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 879be8a98fc..e7c7d3cc3c8 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -0.7.7 +0.7.8 @@ -344,7 +344,7 @@ gem 'oauth2', '~> 1.2.0' gem 'paranoia', '~> 2.0' # Health check -gem 'health_check', '~> 1.5.1' +gem 'health_check', '~> 2.1.0' # System information gem 'vmstat', '~> 2.1.0' diff --git a/Gemfile.lock b/Gemfile.lock index 721ab9ddc5d..f8018e58a5e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -322,8 +322,8 @@ GEM thor tilt hashie (3.4.3) - health_check (1.5.1) - rails (>= 2.3.0) + health_check (2.1.0) + rails (>= 4.0) hipchat (1.5.2) httparty mimemagic @@ -870,7 +870,7 @@ DEPENDENCIES grape (~> 0.13.0) grape-entity (~> 0.4.2) hamlit (~> 2.5) - health_check (~> 1.5.1) + health_check (~> 2.1.0) hipchat (~> 1.5.0) html-pipeline (~> 1.11.0) httparty (~> 0.13.3) diff --git a/app/assets/javascripts/diff.js.coffee b/app/assets/javascripts/diff.js.coffee index 83516f97552..c132cc8c542 100644 --- a/app/assets/javascripts/diff.js.coffee +++ b/app/assets/javascripts/diff.js.coffee @@ -1,6 +1,7 @@ class @Diff UNFOLD_COUNT = 20 constructor: -> + $('.files .diff-file').singleFileDiff() @filesCommentButton = $('.files .diff-file').filesCommentButton() $(document).off('click', '.js-unfold') diff --git a/app/assets/javascripts/protected_branches.js.coffee b/app/assets/javascripts/protected_branches.js.coffee index 79c2306e4d2..14afef2e2ee 100644 --- a/app/assets/javascripts/protected_branches.js.coffee +++ b/app/assets/javascripts/protected_branches.js.coffee @@ -1,18 +1,18 @@ $ -> $(".protected-branches-list :checkbox").change (e) -> name = $(this).attr("name") - if name == "developers_can_push" + if name == "developers_can_push" || name == "developers_can_merge" id = $(this).val() - checked = $(this).is(":checked") + can_push = $(this).is(":checked") url = $(this).data("url") $.ajax - type: "PUT" + type: "PATCH" url: url dataType: "json" data: id: id protected_branch: - developers_can_push: checked + "#{name}": can_push success: -> row = $(e.target) diff --git a/app/assets/stylesheets/framework/avatar.scss b/app/assets/stylesheets/framework/avatar.scss index bb8d71fbae8..8b6ddf8ba18 100644 --- a/app/assets/stylesheets/framework/avatar.scss +++ b/app/assets/stylesheets/framework/avatar.scss @@ -20,6 +20,7 @@ } &.s16 { width: 16px; height: 16px; margin-right: 6px; } + &.s20 { width: 20px; height: 20px; margin-right: 7px; } &.s24 { width: 24px; height: 24px; margin-right: 8px; } &.s26 { width: 26px; height: 26px; margin-right: 8px; } &.s32 { width: 32px; height: 32px; margin-right: 10px; } diff --git a/app/assets/stylesheets/framework/gitlab-theme.scss b/app/assets/stylesheets/framework/gitlab-theme.scss index d1ff63bd099..3673b81f183 100644 --- a/app/assets/stylesheets/framework/gitlab-theme.scss +++ b/app/assets/stylesheets/framework/gitlab-theme.scss @@ -11,7 +11,6 @@ .toggle-nav-collapse, .pin-nav-btn { color: $color-light; - background: $color; &:hover { color: $white-light; diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index 1a2220f3b40..cec52678495 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -17,7 +17,7 @@ height: 100%; overflow: hidden; transition: width $sidebar-transition-duration; - @include box-shadow(2px 0 16px 0 #bbb); + @include box-shadow(2px 0 16px 0 $black-transparent); } } @@ -76,7 +76,7 @@ } a { - padding: 7px 15px 7px 12px; + padding: 7px 16px; font-size: $gl-font-size; line-height: 24px; display: block; @@ -108,7 +108,7 @@ } } -.toggle-nav-collapse { +.sidebar-action-buttons { width: $sidebar_width; position: absolute; top: 0; @@ -117,12 +117,37 @@ padding: 5px 0; font-size: 18px; line-height: 30px; + + .toggle-nav-collapse { + left: 0; + } + + .pin-nav-btn { + right: 0; + display: none; + + @media (min-width: $sidebar-breakpoint) { + display: block; + } + + .fa { + transition: transform .15s; + } + + &.is-active { + .fa { + transform: rotate(90deg); + } + } + } } .nav-header-btn { - padding: 10px 5px; + padding: 10px 16px; color: inherit; transition-duration: .3s; + position: absolute; + top: 0; &:hover, &:focus { @@ -131,30 +156,6 @@ } } -.pin-nav-btn { - display: none; - position: absolute; - left: 0; - bottom: 0; - height: 50px; - width: $sidebar_width; - line-height: 30px; - - @media (min-width: $sidebar-breakpoint) { - display: block; - } - - .fa { - transition: transform .15s; - } - - &.is-active { - .fa { - transform: rotate(90deg); - } - } -} - .page-sidebar-collapsed { padding-left: 0; diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 4337fab5d87..09d3caa0e6a 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -17,6 +17,7 @@ $focus-border-color: #3aabf0; $table-border-color: #f0f0f0; $background-color: #fafafa; $dark-background-color: #f7f7f7; +$table-text-gray: #8f8f8f; /* * Text diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss index e8f1935d239..99a2cd306cf 100644 --- a/app/assets/stylesheets/pages/builds.scss +++ b/app/assets/stylesheets/pages/builds.scss @@ -83,14 +83,6 @@ } } -table.builds { - .build-link { - a { - color: $gl-dark-link-color; - } - } -} - .build-trace { background: $ci-output-bg; color: $ci-text-color; diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss index 47bfd144930..3b1e38fc07d 100644 --- a/app/assets/stylesheets/pages/labels.scss +++ b/app/assets/stylesheets/pages/labels.scss @@ -162,9 +162,15 @@ } .filtered-labels { + font-size: 0; + padding: 12px 16px; + .label-row { + margin-top: 4px; + margin-bottom: 4px; + &:not(:last-child) { - margin-right: 5px; + margin-right: 8px; } } diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index d9756b66af0..15c6c9f231a 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -73,11 +73,14 @@ color: #888; } - &.ci-pending, - &.ci-running { + &.ci-pending { color: $gl-warning; } + &.ci-running { + color: $blue-normal; + } + &.ci-failed, &.ci-error { color: $gl-danger; diff --git a/app/assets/stylesheets/pages/pipelines.scss b/app/assets/stylesheets/pages/pipelines.scss index 6128868b670..cbf8297f387 100644 --- a/app/assets/stylesheets/pages/pipelines.scss +++ b/app/assets/stylesheets/pages/pipelines.scss @@ -1,15 +1,12 @@ .pipelines { .stage { - max-width: 100px; + max-width: 80px; + width: 80px; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; } - .duration, .finished_at { - margin: 4px 0; - } - .commit-title { margin: 0; } @@ -22,3 +19,136 @@ margin: 4px; } } + +.content-list { + + &.pipelines, + &.builds-content-list { + width: 100%; + overflow: auto; + } +} + +.table.builds { + min-width: 1100px; + + tr { + th { + padding: 16px; + border: none; + } + } + + tbody { + border-top-width: 1px; + } + + .branch-commit { + + .branch-name { + margin-left: 8px; + font-weight: bold; + max-width: 180px; + overflow: hidden; + display: inline-block; + white-space: nowrap; + vertical-align: top; + text-overflow: ellipsis; + } + + svg { + margin: 0 6px; + height: 14px; + width: auto; + vertical-align: middle; + } + + .commit-id { + color: $gl-link-color; + margin-right: 8px; + } + + .commit-title { + margin-top: 4px; + max-width: 320px; + overflow: hidden; + white-space: nowrap; + text-overflow: ellipsis; + } + + .avatar { + margin-left: 0; + } + + .label-container { + + .label { + margin-top: 5px; + } + } + } + + .duration, + .finished-at { + color: $table-text-gray; + margin: 4px 0; + + .fa { + font-size: 12px; + } + + svg { + height: 12px; + width: auto; + vertical-align: middle; + } + + .fa, + svg { + margin-right: 5px; + } + } + + .pipeline-actions { + + .btn { + margin: 0; + color: $table-text-gray; + } + + .dropdown-toggle, + .dropdown-menu { + color: $table-text-gray; + + .fa { + color: $table-text-gray; + margin-right: 6px; + font-size: 14px; + } + } + + .btn-remove { + color: $white-light; + } + + .btn-group { + &.open { + .btn-default { + background-color: $white-normal; + border-color: $border-white-normal; + } + } + } + } + + .build-link { + + a { + color: $gl-dark-link-color; + } + } + + .btn-group.open .dropdown-toggle { + box-shadow: none; + } +} diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index bce4aac3334..5be911dc562 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -340,23 +340,30 @@ a.deploy-project-label { .project-import { .form-group { - margin-bottom: 0; + margin-bottom: 5px; } + .import-buttons { padding-left: 0; display: -webkit-flex; display: flex; -webkit-flex-wrap: wrap; flex-wrap: wrap; + .btn { - margin-right: 10px; - padding: 8px 12px; + margin: 0 10px 10px 0; + padding: 8px; } - &> div { - margin-bottom: 14px; + + > div { padding-left: 0; + &:last-child { margin-bottom: 0; + + .btn { + margin-right: 0; + } } } } diff --git a/app/assets/stylesheets/pages/status.scss b/app/assets/stylesheets/pages/status.scss index 2370d35924e..c6b053150be 100644 --- a/app/assets/stylesheets/pages/status.scss +++ b/app/assets/stylesheets/pages/status.scss @@ -32,11 +32,15 @@ border-color: $gl-gray; } - &.ci-pending, - &.ci-running { + &.ci-pending { color: $gl-warning; border-color: $gl-warning; } + + &.ci-running { + color: $blue-normal; + border-color: $blue-normal; + } } .ci-status-icon-success { @@ -45,10 +49,12 @@ .ci-status-icon-failed { color: $gl-danger; } - .ci-status-icon-running, .ci-status-icon-pending { color: $gl-warning; } + .ci-status-icon-running { + color: $blue-normal; + } .ci-status-icon-canceled, .ci-status-icon-disabled, .ci-status-icon-not-found, diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb index 3a2db3e6eeb..19a76a5b5d8 100644 --- a/app/controllers/dashboard/todos_controller.rb +++ b/app/controllers/dashboard/todos_controller.rb @@ -1,6 +1,4 @@ class Dashboard::TodosController < Dashboard::ApplicationController - include TodosHelper - before_action :find_todos, only: [:index, :destroy_all] def index @@ -13,7 +11,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController respond_to do |format| format.html { redirect_to dashboard_todos_path, notice: 'Todo was successfully marked as done.' } format.js { head :ok } - format.json { render json: { count: todos_pending_count, done_count: todos_done_count } } + format.json { render json: todos_counts } end end @@ -23,7 +21,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController respond_to do |format| format.html { redirect_to dashboard_todos_path, notice: 'All todos were marked as done.' } format.js { head :ok } - format.json { render json: { count: todos_pending_count, done_count: todos_done_count } } + format.json { render json: todos_counts } end end @@ -36,4 +34,11 @@ class Dashboard::TodosController < Dashboard::ApplicationController def find_todos @todos ||= TodosFinder.new(current_user, params).execute end + + def todos_counts + { + count: TodosFinder.new(current_user, state: :pending).execute.count, + done_count: TodosFinder.new(current_user, state: :done).execute.count + } + end end diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index f35d631df0c..f54c79c2e37 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -107,7 +107,11 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController # Only allow properly saved users to login. if @user.persisted? && @user.valid? log_audit_event(@user, with: oauth['provider']) - sign_in_and_redirect(@user) + if @user.two_factor_enabled? + prompt_for_two_factor(@user) + else + sign_in_and_redirect(@user) + end else error_message = @user.errors.full_messages.to_sentence diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index b6e80762e3c..f7ada5cfee4 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -119,6 +119,10 @@ class Projects::IssuesController < Projects::ApplicationController render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }) end end + + rescue ActiveRecord::StaleObjectError + @conflict = true + render :edit end def referenced_merge_requests @@ -216,7 +220,7 @@ class Projects::IssuesController < Projects::ApplicationController def issue_params params.require(:issue).permit( :title, :assignee_id, :position, :description, :confidential, - :milestone_id, :due_date, :state_event, :task_num, label_ids: [] + :milestone_id, :due_date, :state_event, :task_num, :lock_version, label_ids: [] ) end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 941d68cda17..2deb7959700 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -56,7 +56,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController def show respond_to do |format| - format.html + format.html { define_discussion_vars } format.json do render json: @merge_request @@ -82,7 +82,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request_diff = @merge_request.merge_request_diff respond_to do |format| - format.html + format.html { define_discussion_vars } format.json { render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") } } end end @@ -108,7 +108,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController def commits respond_to do |format| - format.html { render 'show' } + format.html do + define_discussion_vars + + render 'show' + end format.json do # Get commits from repository # or from cache if already merged @@ -123,7 +127,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController def builds respond_to do |format| - format.html { render 'show' } + format.html do + define_discussion_vars + + render 'show' + end format.json { render json: { html: view_to_html_string('projects/merge_requests/show/_builds') } } end end @@ -188,6 +196,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController else render "edit" end + rescue ActiveRecord::StaleObjectError + @conflict = true + render :edit end def remove_wip @@ -353,14 +364,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request.unlock_mr @merge_request.close end - - if request.format == :html || action_name == 'show' - define_show_html_vars - end end - # Discussion tab data is only required on html requests - def define_show_html_vars + # Discussion tab data is rendered on html responses of actions + # :show, :diff, :commits, :builds. but not when request the data through AJAX + def define_discussion_vars # Build a note object for comment form @note = @project.notes.new(noteable: @noteable) @@ -419,7 +427,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController :title, :assignee_id, :source_project_id, :source_branch, :target_project_id, :target_branch, :milestone_id, :state_event, :description, :task_num, :force_remove_source_branch, - label_ids: [] + :lock_version, label_ids: [] ) end diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb index 80dad758afa..10dca47fded 100644 --- a/app/controllers/projects/protected_branches_controller.rb +++ b/app/controllers/projects/protected_branches_controller.rb @@ -50,6 +50,6 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController end def protected_branch_params - params.require(:protected_branch).permit(:name, :developers_can_push) + params.require(:protected_branch).permit(:name, :developers_can_push, :developers_can_merge) end end diff --git a/app/controllers/projects/todos_controller.rb b/app/controllers/projects/todos_controller.rb index 23868d986e9..5685d0f4e7c 100644 --- a/app/controllers/projects/todos_controller.rb +++ b/app/controllers/projects/todos_controller.rb @@ -5,7 +5,7 @@ class Projects::TodosController < Projects::ApplicationController todo = TodoService.new.mark_todo(issuable, current_user) render json: { - count: current_user.todos_pending_count, + count: TodosFinder.new(current_user, state: :pending).execute.count, delete_path: dashboard_todo_path(todo) } end diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb index 7806d9e4cc5..ff866c2faa5 100644 --- a/app/finders/todos_finder.rb +++ b/app/finders/todos_finder.rb @@ -8,7 +8,7 @@ # action_id: integer # author_id: integer # project_id; integer -# state: 'pending' or 'done' +# state: 'pending' (default) or 'done' # type: 'Issue' or 'MergeRequest' # @@ -37,7 +37,7 @@ class TodosFinder private def action_id? - action_id.present? && [Todo::ASSIGNED, Todo::MENTIONED, Todo::BUILD_FAILED, Todo::MARKED].include?(action_id.to_i) + action_id.present? && Todo::ACTION_NAMES.has_key?(action_id.to_i) end def action_id diff --git a/app/helpers/appearances_helper.rb b/app/helpers/appearances_helper.rb index 950f323e383..e12a1052988 100644 --- a/app/helpers/appearances_helper.rb +++ b/app/helpers/appearances_helper.rb @@ -31,7 +31,7 @@ module AppearancesHelper end end - def navbar_icon(icon_name, size: 16) + def custom_icon(icon_name, size: 16) render "shared/icons/#{icon_name}.svg", size: size end end diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb index 601df5c18df..bfd23aa4e04 100644 --- a/app/helpers/branches_helper.rb +++ b/app/helpers/branches_helper.rb @@ -12,7 +12,7 @@ module BranchesHelper def can_push_branch?(project, branch_name) return false unless project.repository.branch_exists?(branch_name) - ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(branch_name) + ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch_name) end def project_branches diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index 8e4ae1e6aec..e6c99c9959e 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -29,8 +29,10 @@ module CiStatusHelper 'check' when 'failed' 'close' - when 'running', 'pending' + when 'pending' 'clock-o' + when 'running' + 'spinner' else 'circle' end diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index a832a6c8df7..e3a208f826a 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -1,10 +1,10 @@ module TodosHelper def todos_pending_count - TodosFinder.new(current_user, state: :pending).execute.count + @todos_pending_count ||= TodosFinder.new(current_user, state: :pending).execute.count end def todos_done_count - TodosFinder.new(current_user, state: :done).execute.count + @todos_done_count ||= TodosFinder.new(current_user, state: :done).execute.count end def todo_action_name(todo) @@ -13,6 +13,7 @@ module TodosHelper when Todo::MENTIONED then 'mentioned you on' when Todo::BUILD_FAILED then 'The build failed for your' when Todo::MARKED then 'added a todo for' + when Todo::APPROVAL_REQUIRED then 'set you as an approver for' end end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index acb6f5a2998..fb49bd7dd64 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -87,6 +87,12 @@ module Issuable User.find(assignee_id_was).update_cache_counts if assignee_id_was assignee.update_cache_counts if assignee end + + # We want to use optimistic lock for cases when only title or description are involved + # http://api.rubyonrails.org/classes/ActiveRecord/Locking/Optimistic.html + def locking_enabled? + title_changed? || description_changed? + end end module ClassMethods diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 157901378d3..471e32f3b60 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -552,7 +552,13 @@ class MergeRequest < ActiveRecord::Base end def can_be_merged_by?(user) - ::Gitlab::GitAccess.new(user, project, 'web').can_push_to_branch?(target_branch) + access = ::Gitlab::UserAccess.new(user, project: project) + access.can_push_to_branch?(target_branch) || access.can_merge_to_branch?(target_branch) + end + + def can_be_merged_via_command_line_by?(user) + access = ::Gitlab::UserAccess.new(user, project: project) + access.can_push_to_branch?(target_branch) end def mergeable_ci_state? diff --git a/app/models/project.rb b/app/models/project.rb index 3306fb86282..72c4f591420 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -836,6 +836,10 @@ class Project < ActiveRecord::Base protected_branches.matching(branch_name).any?(&:developers_can_push) end + def developers_can_merge_to_protected_branch?(branch_name) + protected_branches.matching(branch_name).any?(&:developers_can_merge) + end + def forked? !(forked_project_link.nil? || forked_project_link.forked_from_project.nil?) end diff --git a/app/models/repository.rb b/app/models/repository.rb index 5b670cb4b8f..09487b62f98 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -769,9 +769,9 @@ class Repository end end - def merge(user, source_sha, target_branch, options = {}) - our_commit = rugged.branches[target_branch].target - their_commit = rugged.lookup(source_sha) + def merge(user, merge_request, options = {}) + our_commit = rugged.branches[merge_request.target_branch].target + their_commit = rugged.lookup(merge_request.diff_head_sha) raise "Invalid merge target" if our_commit.nil? raise "Invalid merge source" if their_commit.nil? @@ -779,14 +779,16 @@ class Repository merge_index = rugged.merge_commits(our_commit, their_commit) return false if merge_index.conflicts? - commit_with_hooks(user, target_branch) do |ref| + commit_with_hooks(user, merge_request.target_branch) do |tmp_ref| actual_options = options.merge( parents: [our_commit, their_commit], tree: merge_index.write_tree(rugged), - update_ref: ref + update_ref: tmp_ref ) - Rugged::Commit.create(rugged, actual_options) + commit_id = Rugged::Commit.create(rugged, actual_options) + merge_request.update(in_progress_merge_commit_sha: commit_id) + commit_id end end diff --git a/app/models/todo.rb b/app/models/todo.rb index ac3fdbc7f3b..8d7a5965aa1 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -1,14 +1,16 @@ class Todo < ActiveRecord::Base - ASSIGNED = 1 - MENTIONED = 2 - BUILD_FAILED = 3 - MARKED = 4 + ASSIGNED = 1 + MENTIONED = 2 + BUILD_FAILED = 3 + MARKED = 4 + APPROVAL_REQUIRED = 5 # This is an EE-only feature ACTION_NAMES = { ASSIGNED => :assigned, MENTIONED => :mentioned, BUILD_FAILED => :build_failed, - MARKED => :marked + MARKED => :marked, + APPROVAL_REQUIRED => :approval_required } belongs_to :author, class_name: "User" diff --git a/app/models/user.rb b/app/models/user.rb index 79c670cb35a..7a72c202150 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -87,7 +87,7 @@ class User < ActiveRecord::Base has_many :builds, dependent: :nullify, class_name: 'Ci::Build' has_many :todos, dependent: :destroy has_many :notification_settings, dependent: :destroy - has_many :award_emoji, as: :awardable, dependent: :destroy + has_many :award_emoji, dependent: :destroy # # Validations diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb index c578097376a..ed73d8cb8c2 100644 --- a/app/services/commits/change_service.rb +++ b/app/services/commits/change_service.rb @@ -23,7 +23,7 @@ module Commits private def check_push_permissions - allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch) + allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch) unless allowed raise ValidationError.new('You are not allowed to push into this branch') @@ -31,7 +31,7 @@ module Commits true end - + def create_target_branch(new_branch) # Temporary branch exists and contains the change commit return success if repository.find_branch(new_branch) diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 37c5e321b39..55da949f56a 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -42,7 +42,7 @@ module Files end def validate - allowed = ::Gitlab::GitAccess.new(current_user, project, 'web').can_push_to_branch?(@target_branch) + allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch) unless allowed raise_error("You are not allowed to push into this branch") diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index a886f35981f..e02b50ff9a2 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -89,7 +89,8 @@ class GitPushService < BaseService # Set protection on the default branch if configured if current_application_settings.default_branch_protection != PROTECTION_NONE developers_can_push = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? true : false - @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push }) + developers_can_merge = current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? true : false + @project.protected_branches.create({ name: @project.default_branch, developers_can_push: developers_can_push, developers_can_merge: developers_can_merge }) end end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index f1b1d90c457..0dac0614141 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -34,7 +34,7 @@ module MergeRequests committer: committer } - commit_id = repository.merge(current_user, merge_request.diff_head_sha, merge_request.target_branch, options) + commit_id = repository.merge(current_user, merge_request, options) merge_request.update(merge_commit_sha: commit_id) rescue GitHooksService::PreReceiveError => e merge_request.update(merge_error: e.message) @@ -43,6 +43,8 @@ module MergeRequests merge_request.update(merge_error: "Something went wrong during merge") Rails.logger.error(e.message) false + ensure + merge_request.update(in_progress_merge_commit_sha: nil) end def after_merge diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index b11ecd97a57..1daf6bbf553 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -48,7 +48,7 @@ module MergeRequests end def force_push? - Gitlab::ForcePushCheck.force_push?(@project, @oldrev, @newrev) + Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev) end # Refresh merge request diff if we push to source or target branch of merge request diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb index 752c11d7ae6..29b3981f49f 100644 --- a/app/services/projects/housekeeping_service.rb +++ b/app/services/projects/housekeeping_service.rb @@ -7,8 +7,6 @@ # module Projects class HousekeepingService < BaseService - include Gitlab::ShellAdapter - LEASE_TIMEOUT = 3600 class LeaseTaken < StandardError @@ -24,11 +22,7 @@ module Projects def execute raise LeaseTaken unless try_obtain_lease - GitlabShellOneShotWorker.perform_async(:gc, @project.repository_storage_path, @project.path_with_namespace) - ensure - Gitlab::Metrics.measure(:reset_pushes_since_gc) do - update_pushes_since_gc(0) - end + execute_gitlab_shell_gc end def needed? @@ -36,19 +30,27 @@ module Projects end def increment! - Gitlab::Metrics.measure(:increment_pushes_since_gc) do - update_pushes_since_gc(@project.pushes_since_gc + 1) + if Gitlab::ExclusiveLease.new("project_housekeeping:increment!:#{@project.id}", timeout: 60).try_obtain + Gitlab::Metrics.measure(:increment_pushes_since_gc) do + update_pushes_since_gc(@project.pushes_since_gc + 1) + end end end private - def update_pushes_since_gc(new_value) - if Gitlab::ExclusiveLease.new("project_housekeeping:update_pushes_since_gc:#{project.id}", timeout: 60).try_obtain - @project.update_column(:pushes_since_gc, new_value) + def execute_gitlab_shell_gc + GitGarbageCollectWorker.perform_async(@project.id) + ensure + Gitlab::Metrics.measure(:reset_pushes_since_gc) do + update_pushes_since_gc(0) end end + def update_pushes_since_gc(new_value) + @project.update_column(:pushes_since_gc, new_value) + end + def try_obtain_lease Gitlab::Metrics.measure(:obtain_housekeeping_lease) do lease = ::Gitlab::ExclusiveLease.new("project_housekeeping:#{@project.id}", timeout: LEASE_TIMEOUT) diff --git a/app/views/admin/builds/_build.html.haml b/app/views/admin/builds/_build.html.haml index 967151bc33b..ce818c30c30 100644 --- a/app/views/admin/builds/_build.html.haml +++ b/app/views/admin/builds/_build.html.haml @@ -1,30 +1,40 @@ - project = build.project -%tr.build +%tr.build.commit %td.status = ci_status_with_icon(build.status) - %td.build-link - - if can?(current_user, :read_build, build.project) - = link_to namespace_project_build_url(build.project.namespace, build.project, build) do - %strong Build ##{build.id} - - else - %strong Build ##{build.id} + %td + .branch-commit + - if can?(current_user, :read_build, build.project) + = link_to namespace_project_build_url(build.project.namespace, build.project, build) do + %span.build-link ##{build.id} + - else + %span.build-link ##{build.id} - - if build.stuck? - %i.fa.fa-warning.text-warning + - if build.stuck? + %i.fa.fa-warning.text-warning - %td - - if project - = link_to project.name_with_namespace, admin_namespace_project_path(project.namespace, project) + - if build.ref + = link_to build.ref, namespace_project_commits_path(build.project.namespace, build.project, build.ref), class: "monospace branch-name" + - else + .light none + = custom_icon("icon_commit") - %td - = link_to build.short_sha, namespace_project_commit_path(build.project.namespace, build.project, build.sha), class: "monospace" + = link_to build.short_sha, namespace_project_commit_path(build.project.namespace, build.project, build.sha), class: "monospace commit-id" + + .label-container + - if build.tags.any? + - build.tags.each do |tag| + %span.label.label-primary + = tag + - if build.try(:trigger_request) + %span.label.label-info triggered + - if build.try(:allow_failure) + %span.label.label-danger allowed to fail %td - - if build.ref - = link_to build.ref, namespace_project_commits_path(build.project.namespace, build.project, build.ref) - - else - .light none + - if project + = link_to project.name_with_namespace, admin_namespace_project_path(project.namespace, project) %td - if build.try(:runner) @@ -36,22 +46,15 @@ #{build.stage} / #{build.name} %td - - if build.tags.any? - - build.tags.each do |tag| - %span.label.label-primary - = tag - - if build.try(:trigger_request) - %span.label.label-info triggered - - if build.try(:allow_failure) - %span.label.label-danger allowed to fail - - %td.duration - if build.duration - #{duration_in_words(build.finished_at, build.started_at)} + %p.duration + = custom_icon("icon_timer") + = duration_in_numbers(build.finished_at, build.started_at) - %td.timestamp - if build.finished_at - %span #{time_ago_with_tooltip(build.finished_at)} + %p.finished-at + = icon("calendar") + %span #{time_ago_with_tooltip(build.finished_at)} - if defined?(coverage) && coverage %td.coverage diff --git a/app/views/admin/builds/index.html.haml b/app/views/admin/builds/index.html.haml index 1e60205f91a..9ea3cca0ecb 100644 --- a/app/views/admin/builds/index.html.haml +++ b/app/views/admin/builds/index.html.haml @@ -27,7 +27,7 @@ .row-content-block.second-block #{(@scope || 'all').capitalize} builds - %ul.content-list + %ul.content-list.builds-content-list - if @builds.blank? %li .nothing-here-block No builds to show @@ -37,15 +37,11 @@ %thead %tr %th Status - %th Build ID - %th Project %th Commit - %th Ref + %th Project %th Runner %th Name - %th Tags - %th Duration - %th Finished at + %th %th - @builds.each do |build| diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index fc42e5dcc66..4e340b6ec16 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -9,14 +9,14 @@ %span To do %span.badge - = todos_pending_count + = number_with_delimiter(todos_pending_count) - todo_done_active = ('active' if params[:state] == 'done') %li{class: "todos-done #{todo_done_active}"} = link_to todos_filter_path(state: 'done') do %span Done %span.badge - = todos_done_count + = number_with_delimiter(todos_done_count) .nav-controls - if @todos.any?(&:pending?) diff --git a/app/views/groups/milestones/index.html.haml b/app/views/groups/milestones/index.html.haml index 121a7de3ad7..a8fdbd8c426 100644 --- a/app/views/groups/milestones/index.html.haml +++ b/app/views/groups/milestones/index.html.haml @@ -6,7 +6,6 @@ .nav-controls - if can?(current_user, :admin_milestones, @group) = link_to new_group_milestone_path(@group), class: "btn btn-new" do - = icon('plus') New Milestone .row-content-block diff --git a/app/views/groups/projects.html.haml b/app/views/groups/projects.html.haml index c2f2d9912f7..33fee334d93 100644 --- a/app/views/groups/projects.html.haml +++ b/app/views/groups/projects.html.haml @@ -7,7 +7,6 @@ - if can? current_user, :admin_group, @group .controls = link_to new_project_path(namespace_id: @group.id), class: "btn btn-sm btn-success" do - = icon('plus') New Project %ul.well-list - @projects.each do |project| diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index a83eb7e88bb..eddeae98bc4 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -6,8 +6,7 @@ .cover-block.groups-cover-block %div{ class: container_class } - = link_to group_icon(@group), target: '_blank' do - = image_tag group_icon(@group), class: "avatar group-avatar s70" + = image_tag group_icon(@group), class: "avatar group-avatar s70" .group-info .cover-title %h1 diff --git a/app/views/layouts/_collapse_button.html.haml b/app/views/layouts/_collapse_button.html.haml deleted file mode 100644 index 8c140a5943e..00000000000 --- a/app/views/layouts/_collapse_button.html.haml +++ /dev/null @@ -1,3 +0,0 @@ -= link_to '#', class: 'nav-header-btn text-center toggle-nav-collapse', title: "Open/Close" do - %span.sr-only Toggle navigation - = icon('bars') diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index 8596bbfdef6..90c1fa4c87c 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -1,6 +1,13 @@ .page-with-sidebar{ class: "#{page_sidebar_class} #{page_gutter_class}" } .sidebar-wrapper.nicescroll{ class: nav_sidebar_class } - = render partial: 'layouts/collapse_button' + .sidebar-action-buttons + = link_to '#', class: 'nav-header-btn toggle-nav-collapse', title: "Open/Close" do + %span.sr-only Toggle navigation + = icon('bars') + = link_to '#', class: "nav-header-btn pin-nav-btn has-tooltip #{'is-active' if pinned_nav?} js-nav-pin", title: pinned_nav? ? "Unpin navigation" : "Pin Navigation", data: {placement: 'right', container: 'body'} do + %span.sr-only Toggle navigation pinning + = icon('thumb-tack') + - if defined?(sidebar) && sidebar = render "layouts/nav/#{sidebar}" - elsif current_user @@ -8,9 +15,6 @@ - else = render 'layouts/nav/explore' - = link_to '#', class: "nav-header-btn text-center pin-nav-btn has-tooltip #{'is-active' if pinned_nav?} js-nav-pin", title: pinned_nav? ? "Unpin navigation" : "Pin Navigation", data: {placement: 'right', container: 'body'} do - %span.sr-only Toggle navigation pinning - = icon('thumb-tack') - if defined?(nav) && nav .layout-nav .container-fluid diff --git a/app/views/layouts/nav/_dashboard.html.haml b/app/views/layouts/nav/_dashboard.html.haml index 52e41b1a857..21668698814 100644 --- a/app/views/layouts/nav/_dashboard.html.haml +++ b/app/views/layouts/nav/_dashboard.html.haml @@ -1,64 +1,44 @@ %ul.nav.nav-sidebar = nav_link(path: ['root#index', 'projects#trending', 'projects#starred', 'dashboard/projects#index'], html_options: {class: "#{project_tab_class} home"}) do = link_to dashboard_projects_path, title: 'Projects', class: 'dashboard-shortcuts-projects' do - .icon-container - = navbar_icon('project') %span Projects = nav_link(controller: :todos) do = link_to dashboard_todos_path, title: 'Todos' do - .icon-container - = icon('bell fw') %span Todos %span.count= number_with_delimiter(todos_pending_count) = nav_link(path: 'dashboard#activity') do = link_to activity_dashboard_path, class: 'dashboard-shortcuts-activity', title: 'Activity' do - .icon-container - = navbar_icon('activity') %span Activity = nav_link(controller: [:groups, 'groups/milestones', 'groups/group_members']) do = link_to dashboard_groups_path, title: 'Groups' do - .icon-container - = navbar_icon('group') %span Groups = nav_link(controller: 'dashboard/milestones') do = link_to dashboard_milestones_path, title: 'Milestones' do - .icon-container - = navbar_icon('milestones') %span Milestones = nav_link(path: 'dashboard#issues') do = link_to assigned_issues_dashboard_path, title: 'Issues', class: 'dashboard-shortcuts-issues' do - .icon-container - = navbar_icon('issues') %span Issues %span.count= number_with_delimiter(current_user.assigned_issues.opened.count) = nav_link(path: 'dashboard#merge_requests') do = link_to assigned_mrs_dashboard_path, title: 'Merge Requests', class: 'dashboard-shortcuts-merge_requests' do - .icon-container - = navbar_icon('mr') %span Merge Requests %span.count= number_with_delimiter(current_user.assigned_merge_requests.opened.count) = nav_link(controller: :snippets) do = link_to dashboard_snippets_path, title: 'Snippets' do - .icon-container - = icon('clipboard fw') %span Snippets = nav_link(controller: :help) do = link_to help_path, title: 'Help' do - .icon-container - = icon('question-circle fw') %span Help = nav_link(html_options: {class: profile_tab_class}) do = link_to profile_path, title: 'Profile Settings', data: {placement: 'bottom'} do - .icon-container - = icon('user fw') %span Profile Settings diff --git a/app/views/layouts/nav/_profile.html.haml b/app/views/layouts/nav/_profile.html.haml index 96fe62c39c3..6d514f669db 100644 --- a/app/views/layouts/nav/_profile.html.haml +++ b/app/views/layouts/nav/_profile.html.haml @@ -18,9 +18,9 @@ %span Applications = nav_link(controller: :personal_access_tokens) do - = link_to profile_personal_access_tokens_path, title: 'Personal Access Tokens' do + = link_to profile_personal_access_tokens_path, title: 'Access Tokens' do %span - Personal Access Tokens + Access Tokens = nav_link(controller: :emails) do = link_to profile_emails_path, title: 'Emails' do %span diff --git a/app/views/projects/builds/index.html.haml b/app/views/projects/builds/index.html.haml index 381b3754cd5..85c31dfd918 100644 --- a/app/views/projects/builds/index.html.haml +++ b/app/views/projects/builds/index.html.haml @@ -36,7 +36,7 @@ = link_to ci_lint_path, class: 'btn btn-default' do %span CI Lint - %ul.content-list + %ul.content-list.builds-content-list - if @builds.blank? %li .nothing-here-block No builds to show @@ -46,14 +46,10 @@ %thead %tr %th Status - %th Build ID %th Commit - %th Ref %th Stage %th Name - %th Tags - %th Duration - %th Finished at + %th - if @project.build_coverage_enabled? %th Coverage %th diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 5bd6e3f0ebc..e1b42b2cfa5 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -1,32 +1,45 @@ -%tr.build +%tr.build.commit %td.status - if can?(current_user, :read_build, build) = ci_status_with_icon(build.status, namespace_project_build_url(build.project.namespace, build.project, build)) - else = ci_status_with_icon(build.status) - %td.build-link - - if can?(current_user, :read_build, build) - = link_to namespace_project_build_url(build.project.namespace, build.project, build) do - %strong ##{build.id} - - else - %strong ##{build.id} + %td + .branch-commit + - if can?(current_user, :read_build, build) + = link_to namespace_project_build_url(build.project.namespace, build.project, build) do + %span ##{build.id} + - else + %span ##{build.id} - - if build.stuck? - = icon('warning', class: 'text-warning has-tooltip', title: 'Build is stuck. Check runners.') - - if defined?(retried) && retried - = icon('warning', class: 'text-warning has-tooltip', title: 'Build was retried.') + - if build.stuck? + = icon('warning', class: 'text-warning has-tooltip', title: 'Build is stuck. Check runners.') + - if defined?(retried) && retried + = icon('warning', class: 'text-warning has-tooltip', title: 'Build was retried.') - - if defined?(commit_sha) && commit_sha - %td - = link_to build.short_sha, namespace_project_commit_path(build.project.namespace, build.project, build.sha), class: "monospace" + - if defined?(ref) && ref + - if build.ref + = link_to build.ref, namespace_project_commits_path(build.project.namespace, build.project, build.ref), class: "monospace branch-name" + - else + .light none + = custom_icon("icon_commit") + + - if defined?(commit_sha) && commit_sha + = link_to build.short_sha, namespace_project_commit_path(build.project.namespace, build.project, build.sha), class: "commit-id monospace" + + .label-container + - if build.tags.any? + - build.tags.each do |tag| + %span.label.label-primary + = tag + - if build.try(:trigger_request) + %span.label.label-info triggered + - if build.try(:allow_failure) + %span.label.label-danger allowed to fail + - if defined?(retried) && retried + %span.label.label-warning retried - - if defined?(ref) && ref - %td - - if build.ref - = link_to build.ref, namespace_project_commits_path(build.project.namespace, build.project, build.ref) - - else - .light none - if defined?(runner) && runner %td @@ -43,25 +56,14 @@ = build.name %td - .label-container - - if build.tags.any? - - build.tags.each do |tag| - %span.label.label-primary - = tag - - if build.try(:trigger_request) - %span.label.label-info triggered - - if build.try(:allow_failure) - %span.label.label-danger allowed to fail - - if defined?(retried) && retried - %span.label.label-warning retried - - %td.duration - if build.duration - #{duration_in_words(build.finished_at, build.started_at)} - - %td.timestamp + %p.duration + = custom_icon("icon_timer") + = duration_in_numbers(build.finished_at, build.started_at) - if build.finished_at - %span #{time_ago_with_tooltip(build.finished_at)} + %p.finished-at + = icon("calendar") + %span #{time_ago_with_tooltip(build.finished_at)} - if defined?(coverage) && coverage %td.coverage @@ -79,4 +81,4 @@ = icon('remove', class: 'cred') - elsif defined?(allow_retry) && allow_retry && build.retryable? = link_to retry_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Retry', class: 'btn btn-build' do - = icon('refresh') + = icon('repeat') diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index af8dd5cd02c..4ef72ff5d2a 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -1,17 +1,18 @@ - status = pipeline.status %tr.commit %td.commit-link - = link_to namespace_project_pipeline_path(@project.namespace, @project, pipeline.id), class: "ci-status ci-#{status}" do - = ci_icon_for_status(status) - %strong ##{pipeline.id} + = link_to namespace_project_pipeline_path(@project.namespace, @project, pipeline.id) do + = ci_status_with_icon(status) + %td - %div.branch-commit + .branch-commit + = link_to namespace_project_pipeline_path(@project.namespace, @project, pipeline.id) do + %span ##{pipeline.id} - if pipeline.ref - = link_to pipeline.ref, namespace_project_commits_path(@project.namespace, @project, pipeline.ref), class: "monospace" - · + = link_to pipeline.ref, namespace_project_commits_path(@project.namespace, @project, pipeline.ref), class: "monospace branch-name" + = custom_icon("icon_commit") = link_to pipeline.short_sha, namespace_project_commit_path(@project.namespace, @project, pipeline.sha), class: "commit-id monospace" - - if pipeline.tag? %span.label.label-primary tag - elsif pipeline.latest? @@ -25,6 +26,7 @@ %p.commit-title - if commit = pipeline.commit + = commit_author_avatar(commit, size: 20) = link_to_gfm truncate(commit.title, length: 60), namespace_project_commit_path(@project.namespace, @project, commit.id), class: "commit-row-message" - else Cant find HEAD commit for this branch @@ -45,22 +47,37 @@ %td - if pipeline.started_at && pipeline.finished_at %p.duration + = custom_icon("icon_timer") = duration_in_numbers(pipeline.finished_at, pipeline.started_at) + - if pipeline.finished_at + %p.finished-at + = icon("calendar") + #{time_ago_with_tooltip(pipeline.finished_at)} - %td + %td.pipeline-actions .controls.hidden-xs.pull-right - artifacts = pipeline.builds.latest.select { |b| b.artifacts? } - if artifacts.present? - .dropdown.inline.build-artifacts - %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} - = icon('download') - %b.caret - %ul.dropdown-menu.dropdown-menu-align-right - - artifacts.each do |build| + .btn-group.inline + .btn-group + %a.dropdown-toggle.btn.btn-default{type: 'button', 'data-toggle' => 'dropdown'} + = icon("play") + %b.caret + %ul.dropdown-menu.dropdown-menu-align-right %li - = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, build), rel: 'nofollow' do - = icon("download") - %span Download '#{build.name}' artifacts + = link_to '#' do + = icon("play") + %span Deploy to production + .btn-group + %a.dropdown-toggle.btn.btn-default.build-artifacts{type: 'button', 'data-toggle' => 'dropdown'} + = icon("download") + %b.caret + %ul.dropdown-menu.dropdown-menu-align-right + - artifacts.each do |build| + %li + = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, build), rel: 'nofollow' do + = icon("download") + %span Download '#{build.name}' artifacts - if can?(current_user, :update_pipeline, @project) - if pipeline.retryable? diff --git a/app/views/projects/commit/_pipeline.html.haml b/app/views/projects/commit/_pipeline.html.haml index 0411137b7c6..41fd5459429 100644 --- a/app/views/projects/commit/_pipeline.html.haml +++ b/app/views/projects/commit/_pipeline.html.haml @@ -42,9 +42,7 @@ %th Status %th Build ID %th Name - %th Tags - %th Duration - %th Finished at + %th - if pipeline.project.build_coverage_enabled? %th Coverage %th diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml index 5bc5c71283e..542827b2f15 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml @@ -50,10 +50,12 @@ %td.duration - if generic_commit_status.duration + = icon("clock-o") #{duration_in_words(generic_commit_status.finished_at, generic_commit_status.started_at)} %td.timestamp - if generic_commit_status.finished_at + = icon("calendar") %span #{time_ago_with_tooltip(generic_commit_status.finished_at)} - if defined?(coverage) && coverage diff --git a/app/views/projects/issues/index.html.haml b/app/views/projects/issues/index.html.haml index 312bd86ed04..7612fe3719a 100644 --- a/app/views/projects/issues/index.html.haml +++ b/app/views/projects/issues/index.html.haml @@ -32,7 +32,7 @@ Code, test, and deploy together .blank-state .blank-state-icon - = navbar_icon("issues", size: 50) + = custom_icon("issues", size: 50) %h3.blank-state-title You don't have any issues right now. %p.blank-state-text diff --git a/app/views/projects/merge_requests/widget/open/_conflicts.html.haml b/app/views/projects/merge_requests/widget/open/_conflicts.html.haml index 06ab0a3fa00..f000cc38a65 100644 --- a/app/views/projects/merge_requests/widget/open/_conflicts.html.haml +++ b/app/views/projects/merge_requests/widget/open/_conflicts.html.haml @@ -4,7 +4,7 @@ %p Please resolve these conflicts or - - if @merge_request.can_be_merged_by?(current_user) + - if @merge_request.can_be_merged_via_command_line_by?(current_user) #{link_to "merge this request manually", "#modal_merge_info", class: "how_to_merge_link vlink", "data-toggle" => "modal"}. - else ask someone with write access to this repository to merge this request manually. diff --git a/app/views/projects/pipelines/index.html.haml b/app/views/projects/pipelines/index.html.haml index 7c225e2b282..5f466bdbac2 100644 --- a/app/views/projects/pipelines/index.html.haml +++ b/app/views/projects/pipelines/index.html.haml @@ -28,7 +28,7 @@ .nav-controls - if can? current_user, :create_pipeline, @project = link_to new_namespace_project_pipeline_path(@project.namespace, @project), class: 'btn btn-create' do - New pipeline + Run pipeline - unless @repository.gitlab_ci_yml = link_to 'Get started with Pipelines', help_page_path('ci/quick_start/README'), class: 'btn btn-info' @@ -45,13 +45,13 @@ .table-holder %table.table.builds %tbody - %th ID + %th Status %th Commit - stages.each do |stage| %th.stage %span.has-tooltip{ title: "#{stage.titleize}" } = stage.titleize - %th Duration + %th %th = render @pipelines, commit_sha: true, stage: true, allow_retry: true, stages: stages diff --git a/app/views/projects/protected_branches/_branches_list.html.haml b/app/views/projects/protected_branches/_branches_list.html.haml index 97cb1a9052b..720d67dff7c 100644 --- a/app/views/projects/protected_branches/_branches_list.html.haml +++ b/app/views/projects/protected_branches/_branches_list.html.haml @@ -8,6 +8,7 @@ .table-responsive %table.table.protected-branches-list %colgroup + %col{ width: "20%" } %col{ width: "30%" } %col{ width: "25%" } %col{ width: "25%" } @@ -18,6 +19,7 @@ %th Protected Branch %th Commit %th Developers Can Push + %th Developers Can Merge - if can_admin_project %th %tbody diff --git a/app/views/projects/protected_branches/_protected_branch.html.haml b/app/views/projects/protected_branches/_protected_branch.html.haml index 474aec3a97c..7fda7f96047 100644 --- a/app/views/projects/protected_branches/_protected_branch.html.haml +++ b/app/views/projects/protected_branches/_protected_branch.html.haml @@ -16,6 +16,8 @@ (branch was removed from repository) %td = check_box_tag("developers_can_push", protected_branch.id, protected_branch.developers_can_push, data: { url: url }) + %td + = check_box_tag("developers_can_merge", protected_branch.id, protected_branch.developers_can_merge, data: { url: url }) - if can_admin_project %td = link_to 'Unprotect', [@project.namespace.becomes(Namespace), @project, protected_branch], data: { confirm: 'Branch will be writable for developers. Are you sure?' }, method: :delete, class: "btn btn-warning btn-sm pull-right" diff --git a/app/views/projects/protected_branches/index.html.haml b/app/views/projects/protected_branches/index.html.haml index 3fab95751e0..101b3f3b452 100644 --- a/app/views/projects/protected_branches/index.html.haml +++ b/app/views/projects/protected_branches/index.html.haml @@ -36,6 +36,14 @@ = f.label :developers_can_push, "Developers can push", class: "label-light append-bottom-0" %p.light.append-bottom-0 Allow developers to push to this branch + + .form-group + = f.check_box :developers_can_merge, class: "pull-left" + .prepend-left-20 + = f.label :developers_can_merge, "Developers can merge", class: "label-light append-bottom-0" + %p.light.append-bottom-0 + Allow developers to accept merge requests to this branch = f.submit "Protect", class: "btn-create btn protect-branch-btn", disabled: true + %hr = render "branches_list" diff --git a/app/views/projects/snippets/index.html.haml b/app/views/projects/snippets/index.html.haml index 6c994ae486b..1646bcf4b8a 100644 --- a/app/views/projects/snippets/index.html.haml +++ b/app/views/projects/snippets/index.html.haml @@ -1,6 +1,6 @@ - page_title "Snippets" -.row-content-block.top-block +.sub-header-block .pull-right - if can?(current_user, :create_project_snippet, @project) = link_to new_namespace_project_snippet_path(@project.namespace, @project), class: "btn btn-new", title: "New Snippet" do diff --git a/app/views/shared/icons/_icon_commit.svg b/app/views/shared/icons/_icon_commit.svg new file mode 100644 index 00000000000..0e96035b7b7 --- /dev/null +++ b/app/views/shared/icons/_icon_commit.svg @@ -0,0 +1,3 @@ +<svg xmlns="http://www.w3.org/2000/svg" width="40" height="40" viewBox="0 0 40 40"> + <path fill="#8F8F8F" fill-rule="evenodd" d="M28.7769836,18 C27.8675252,13.9920226 24.2831748,11 20,11 C15.7168252,11 12.1324748,13.9920226 11.2230164,18 L4.0085302,18 C2.90195036,18 2,18.8954305 2,20 C2,21.1122704 2.8992496,22 4.0085302,22 L11.2230164,22 C12.1324748,26.0079774 15.7168252,29 20,29 C24.2831748,29 27.8675252,26.0079774 28.7769836,22 L35.9914698,22 C37.0980496,22 38,21.1045695 38,20 C38,18.8877296 37.1007504,18 35.9914698,18 L28.7769836,18 L28.7769836,18 Z M20,25 C22.7614237,25 25,22.7614237 25,20 C25,17.2385763 22.7614237,15 20,15 C17.2385763,15 15,17.2385763 15,20 C15,22.7614237 17.2385763,25 20,25 L20,25 Z"/> +</svg> diff --git a/app/views/shared/icons/_icon_timer.svg b/app/views/shared/icons/_icon_timer.svg new file mode 100644 index 00000000000..0b1e5804427 --- /dev/null +++ b/app/views/shared/icons/_icon_timer.svg @@ -0,0 +1 @@ +<svg xmlns="http://www.w3.org/2000/svg" width="40" height="40" viewBox="0 0 40 40"><g fill="#8F8F8F" fill-rule="evenodd"><path d="M29.513 10.134A15.922 15.922 0 0 0 23 7.28V6h2.993C26.55 6 27 5.552 27 5V2a1 1 0 0 0-1.007-1H14.007C13.45 1 13 1.448 13 2v3a1 1 0 0 0 1.007 1H17v1.28C9.597 8.686 4 15.19 4 23c0 8.837 7.163 16 16 16s16-7.163 16-16c0-3.461-1.099-6.665-2.967-9.283l1.327-1.58a2.498 2.498 0 0 0-.303-3.53 2.499 2.499 0 0 0-3.528.315l-1.016 1.212zM20 34c6.075 0 11-4.925 11-11s-4.925-11-11-11S9 16.925 9 23s4.925 11 11 11z"/><path d="M19 21h-4.002c-.552 0-.998.452-.998 1.01v1.98c0 .567.447 1.01.998 1.01h7.004c.274 0 .521-.111.701-.291a.979.979 0 0 0 .297-.704v-8.01c0-.54-.452-.995-1.01-.995h-1.98a.997.997 0 0 0-1.01.995V21z"/></g></svg>
\ No newline at end of file diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index c30bdb0ae91..98bbb12eaec 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -1,5 +1,12 @@ = form_errors(issuable) +- if @conflict + .alert.alert-danger + Someone edited the #{issuable.class.model_name.human.downcase} the same time you did. + Please check out + = link_to "the #{issuable.class.model_name.human.downcase}", polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), target: "_blank" + and make sure your changes will not unintentionally remove theirs + .form-group = f.label :title, class: 'control-label' .col-sm-10 @@ -149,3 +156,5 @@ = link_to 'Delete', polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), data: { confirm: "#{issuable.class.name.titleize} will be removed! Are you sure?" }, method: :delete, class: 'btn btn-danger btn-grouped' = link_to 'Cancel', polymorphic_path([@project.namespace.becomes(Namespace), @project, issuable]), class: 'btn btn-grouped btn-cancel' + += f.hidden_field :lock_version diff --git a/app/views/shared/milestones/_summary.html.haml b/app/views/shared/milestones/_summary.html.haml index 975c74f4ea6..dee2472fa79 100644 --- a/app/views/shared/milestones/_summary.html.haml +++ b/app/views/shared/milestones/_summary.html.haml @@ -26,7 +26,6 @@ %span.pull-right.tab-issues-buttons - if project && can?(current_user, :create_issue, project) = link_to new_namespace_project_issue_path(project.namespace, project, issue: { milestone_id: milestone.id }), class: "btn btn-grouped", title: "New Issue" do - %i.fa.fa-plus New Issue = link_to 'Browse Issues', milestones_browse_issuables_path(milestone, type: :issues), class: "btn btn-grouped" %span.pull-right.tab-merge-requests-buttons.hidden diff --git a/app/workers/git_garbage_collect_worker.rb b/app/workers/git_garbage_collect_worker.rb new file mode 100644 index 00000000000..2fa3c838f55 --- /dev/null +++ b/app/workers/git_garbage_collect_worker.rb @@ -0,0 +1,14 @@ +class GitGarbageCollectWorker + include Sidekiq::Worker + include Gitlab::ShellAdapter + + sidekiq_options queue: :gitlab_shell, retry: false + + def perform(project_id) + project = Project.find(project_id) + + gitlab_shell.gc(project.repository_storage_path, project.path_with_namespace) + # Expire the branch cache in case garbage collection caused a ref lookup to fail + project.repository.after_create_branch + end +end diff --git a/app/workers/gitlab_shell_one_shot_worker.rb b/app/workers/gitlab_shell_one_shot_worker.rb deleted file mode 100644 index 4ddbcf574d5..00000000000 --- a/app/workers/gitlab_shell_one_shot_worker.rb +++ /dev/null @@ -1,10 +0,0 @@ -class GitlabShellOneShotWorker - include Sidekiq::Worker - include Gitlab::ShellAdapter - - sidekiq_options queue: :gitlab_shell, retry: false - - def perform(action, *arg) - gitlab_shell.send(action, *arg) - end -end diff --git a/config/gitlab.teatro.yml b/config/gitlab.teatro.yml deleted file mode 100644 index 75b79b837e0..00000000000 --- a/config/gitlab.teatro.yml +++ /dev/null @@ -1,87 +0,0 @@ - -production: &base - gitlab: - host: localhost - port: 80 - https: false - - user: root - - email_from: example@example.com - - support_email: support@example.com - - default_projects_features: - issues: true - merge_requests: true - wiki: true - snippets: false - visibility_level: "private" # can be "private" | "internal" | "public" - - issues_tracker: - - gravatar: - enabled: true # Use user avatar image from Gravatar.com (default: true) - - ldap: - enabled: false - host: '_your_ldap_server' - port: 636 - uid: 'sAMAccountName' - method: 'ssl' # "tls" or "ssl" or "plain" - bind_dn: '_the_full_dn_of_the_user_you_will_bind_with' - password: '_the_password_of_the_bind_user' - allow_username_or_email_login: true - - base: '' - - user_filter: '' - - omniauth: - enabled: false - - satellites: - # Relative paths are relative to Rails.root (default: tmp/repo_satellites/) - path: /apps/gitlab-satellites/ - - backup: - path: "tmp/backups" # Relative paths are relative to Rails.root (default: tmp/backups/) - - repositories: - storages: # REPO PATHS MUST NOT BE A SYMLINK!!! - default: /apps/repositories/ - - gitlab_shell: - path: /apps/gitlab-shell/ - - hooks_path: /apps/gitlab-shell/hooks/ - - upload_pack: true - receive_pack: true - - git: - bin_path: /usr/bin/git - max_size: 5242880 # 5.megabytes - timeout: 10 - - extra: - -development: - <<: *base - -test: - <<: *base - gravatar: - enabled: true - gitlab: - host: localhost - port: 80 - issues_tracker: - redmine: - title: "Redmine" - project_url: "http://redmine/projects/:issues_tracker_id" - issues_url: "http://redmine/:project_id/:issues_tracker_id/:id" - new_issue_url: "http://redmine/projects/:issues_tracker_id/issues/new" - -staging: - <<: *base diff --git a/config/initializers/health_check.rb b/config/initializers/health_check.rb index 6796407d4e6..4c91a61fb4a 100644 --- a/config/initializers/health_check.rb +++ b/config/initializers/health_check.rb @@ -1,16 +1,3 @@ -# Email forcibly included in the standard checks, but the email health check -# doesn't support the full range of SMTP options, which can result in failures -# for valid SMTP configurations. -# Overwrite the HealthCheck's detection of whether email is configured -# in order to avoid the email check during standard checks -module HealthCheck - class Utils - def self.mailer_configured? - false - end - end -end - HealthCheck.setup do |config| config.standard_checks = ['database', 'migrations', 'cache'] config.full_checks = ['database', 'migrations', 'cache'] diff --git a/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb b/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb new file mode 100644 index 00000000000..15ad8e8bcbb --- /dev/null +++ b/db/migrate/20160519203051_add_developers_can_merge_to_protected_branches.rb @@ -0,0 +1,9 @@ +class AddDevelopersCanMergeToProtectedBranches < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + disable_ddl_transaction! + + def change + add_column_with_default :protected_branches, :developers_can_merge, :boolean, default: false, allow_null: false + end +end diff --git a/db/migrate/20160629025435_add_column_in_progress_merge_commit_sha_to_merge_requests.rb b/db/migrate/20160629025435_add_column_in_progress_merge_commit_sha_to_merge_requests.rb new file mode 100644 index 00000000000..7c5f76572ef --- /dev/null +++ b/db/migrate/20160629025435_add_column_in_progress_merge_commit_sha_to_merge_requests.rb @@ -0,0 +1,8 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddColumnInProgressMergeCommitShaToMergeRequests < ActiveRecord::Migration + def change + add_column :merge_requests, :in_progress_merge_commit_sha, :string + end +end diff --git a/db/migrate/20160707104333_add_lock_to_issuables.rb b/db/migrate/20160707104333_add_lock_to_issuables.rb new file mode 100644 index 00000000000..cb516672800 --- /dev/null +++ b/db/migrate/20160707104333_add_lock_to_issuables.rb @@ -0,0 +1,17 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddLockToIssuables < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + def up + add_column_with_default :issues, :lock_version, :integer, default: 0 + add_column_with_default :merge_requests, :lock_version, :integer, default: 0 + end + + def down + remove_column :issues, :lock_version + remove_column :merge_requests, :lock_version + end +end diff --git a/db/migrate/20160712171823_remove_award_emojis_with_no_user.rb b/db/migrate/20160712171823_remove_award_emojis_with_no_user.rb new file mode 100644 index 00000000000..668c22bb51c --- /dev/null +++ b/db/migrate/20160712171823_remove_award_emojis_with_no_user.rb @@ -0,0 +1,21 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveAwardEmojisWithNoUser < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # When using the methods "add_concurrent_index" or "add_column_with_default" + # you must disable the use of transactions as these methods can not run in an + # existing transaction. When using "add_concurrent_index" make sure that this + # method is the _only_ method called in the migration, any other changes + # should go in a separate migration. This ensures that upon failure _only_ the + # index creation fails and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def up + AwardEmoji.joins('LEFT JOIN users ON users.id = user_id').where('users.id IS NULL').destroy_all + end +end diff --git a/db/schema.rb b/db/schema.rb index a5eea3a697c..64019cf79bb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20160705163108) do +ActiveRecord::Schema.define(version: 20160712171823) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -70,11 +70,11 @@ ActiveRecord::Schema.define(version: 20160705163108) do t.string "recaptcha_site_key" t.string "recaptcha_private_key" t.integer "metrics_port", default: 8089 - t.boolean "akismet_enabled", default: false - t.string "akismet_api_key" t.integer "metrics_sample_interval", default: 15 t.boolean "sentry_enabled", default: false t.string "sentry_dsn" + t.boolean "akismet_enabled", default: false + t.string "akismet_api_key" t.boolean "email_author_in_body", default: false t.integer "default_group_visibility" t.boolean "repository_checks_enabled", default: false @@ -84,10 +84,10 @@ ActiveRecord::Schema.define(version: 20160705163108) do t.string "health_check_access_token" t.boolean "send_user_confirmation_email", default: false t.integer "container_registry_token_expire_delay", default: 5 - t.boolean "user_default_external", default: false, null: false t.text "after_sign_up_text" t.string "repository_storage", default: "default" t.string "enabled_git_access_protocol" + t.boolean "user_default_external", default: false, null: false end create_table "audit_events", force: :cascade do |t| @@ -165,8 +165,8 @@ ActiveRecord::Schema.define(version: 20160705163108) do t.text "artifacts_metadata" t.integer "erased_by_id" t.datetime "erased_at" - t.string "environment" t.datetime "artifacts_expire_at" + t.string "environment" t.integer "artifacts_size" end @@ -481,10 +481,11 @@ ActiveRecord::Schema.define(version: 20160705163108) do t.string "state" t.integer "iid" t.integer "updated_by_id" + t.integer "moved_to_id" t.boolean "confidential", default: false t.datetime "deleted_at" t.date "due_date" - t.integer "moved_to_id" + t.integer "lock_version", default: 0, null: false end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree @@ -624,6 +625,8 @@ ActiveRecord::Schema.define(version: 20160705163108) do t.integer "merge_user_id" t.string "merge_commit_sha" t.datetime "deleted_at" + t.integer "lock_version", default: 0, null: false + t.string "in_progress_merge_commit_sha" end add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree @@ -773,10 +776,10 @@ ActiveRecord::Schema.define(version: 20160705163108) do t.integer "user_id", null: false t.string "token", null: false t.string "name", null: false - t.datetime "created_at", null: false - t.datetime "updated_at", null: false t.boolean "revoked", default: false t.datetime "expires_at" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false end add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree @@ -858,11 +861,12 @@ ActiveRecord::Schema.define(version: 20160705163108) do add_index "projects", ["visibility_level"], name: "index_projects_on_visibility_level", using: :btree create_table "protected_branches", force: :cascade do |t| - t.integer "project_id", null: false - t.string "name", null: false + t.integer "project_id", null: false + t.string "name", null: false t.datetime "created_at" t.datetime "updated_at" - t.boolean "developers_can_push", default: false, null: false + t.boolean "developers_can_push", default: false, null: false + t.boolean "developers_can_merge", default: false, null: false end add_index "protected_branches", ["project_id"], name: "index_protected_branches_on_project_id", using: :btree @@ -896,9 +900,9 @@ ActiveRecord::Schema.define(version: 20160705163108) do t.string "type" t.string "title" t.integer "project_id" - t.datetime "created_at" - t.datetime "updated_at" - t.boolean "active", default: false, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.boolean "active", null: false t.text "properties" t.boolean "template", default: false t.boolean "push_events", default: true diff --git a/doc/README.md b/doc/README.md index cf7a828d91e..cc0b6e0c1e5 100644 --- a/doc/README.md +++ b/doc/README.md @@ -11,7 +11,7 @@ - [Importing and exporting projects between instances](user/project/settings/import_export.md). - [Markdown](markdown/markdown.md) GitLab's advanced formatting system. - [Migrating from SVN](workflow/importing/migrating_from_svn.md) Convert a SVN repository to Git and GitLab. -- [Permissions](permissions/permissions.md) Learn what each role in a project (external/guest/reporter/developer/master/owner) can do. +- [Permissions](user/permissions.md) Learn what each role in a project (external/guest/reporter/developer/master/owner) can do. - [Profile Settings](profile/README.md) - [Project Services](project_services/project_services.md) Integrate a project with external services, such as CI and chat. - [Public access](public_access/public_access.md) Learn how you can allow public and internal access to projects. diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md index 816f09e1007..a8c3b068d22 100644 --- a/doc/api/merge_requests.md +++ b/doc/api/merge_requests.md @@ -68,7 +68,9 @@ Parameters: "merge_when_build_succeeds": true, "merge_status": "can_be_merged", "subscribed" : false, - "user_notes_count": 1 + "user_notes_count": 1, + "should_remove_source_branch": true, + "force_remove_source_branch": false } ] ``` @@ -132,7 +134,9 @@ Parameters: "merge_when_build_succeeds": true, "merge_status": "can_be_merged", "subscribed" : true, - "user_notes_count": 1 + "user_notes_count": 1, + "should_remove_source_branch": true, + "force_remove_source_branch": false } ``` @@ -233,6 +237,8 @@ Parameters: "merge_status": "can_be_merged", "subscribed" : true, "user_notes_count": 1, + "should_remove_source_branch": true, + "force_remove_source_branch": false, "changes": [ { "old_path": "VERSION", @@ -312,7 +318,9 @@ Parameters: "merge_when_build_succeeds": true, "merge_status": "can_be_merged", "subscribed" : true, - "user_notes_count": 0 + "user_notes_count": 0, + "should_remove_source_branch": true, + "force_remove_source_branch": false } ``` @@ -383,7 +391,9 @@ Parameters: "merge_when_build_succeeds": true, "merge_status": "can_be_merged", "subscribed" : true, - "user_notes_count": 1 + "user_notes_count": 1, + "should_remove_source_branch": true, + "force_remove_source_branch": false } ``` @@ -481,7 +491,9 @@ Parameters: "merge_when_build_succeeds": true, "merge_status": "can_be_merged", "subscribed" : true, - "user_notes_count": 1 + "user_notes_count": 1, + "should_remove_source_branch": true, + "force_remove_source_branch": false } ``` @@ -547,7 +559,9 @@ Parameters: "merge_when_build_succeeds": true, "merge_status": "can_be_merged", "subscribed" : true, - "user_notes_count": 1 + "user_notes_count": 1, + "should_remove_source_branch": true, + "force_remove_source_branch": false } ``` @@ -866,7 +880,9 @@ Example response: "merge_when_build_succeeds": false, "merge_status": "unchecked", "subscribed": true, - "user_notes_count": 7 + "user_notes_count": 7, + "should_remove_source_branch": true, + "force_remove_source_branch": false }, "target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/merge_requests/7", "body": "Et voluptas laudantium minus nihil recusandae ut accusamus earum aut non.", diff --git a/doc/api/todos.md b/doc/api/todos.md index 29e73664410..23f6e35f2a4 100644 --- a/doc/api/todos.md +++ b/doc/api/todos.md @@ -15,7 +15,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | -| `action` | string | no | The action to be filtered. Can be `assigned`, `mentioned`, `build_failed`, or `marked`. | +| `action` | string | no | The action to be filtered. Can be `assigned`, `mentioned`, `build_failed`, `marked`, or `approval_required`. | | `author_id` | integer | no | The ID of an author | | `project_id` | integer | no | The ID of a project | | `state` | string | no | The state of the todo. Can be either `pending` or `done` | diff --git a/doc/ci/README.md b/doc/ci/README.md index a9d407528e8..0833027f91d 100644 --- a/doc/ci/README.md +++ b/doc/ci/README.md @@ -15,6 +15,6 @@ - [Use SSH keys in your build environment](ssh_keys/README.md) - [Trigger builds through the API](triggers/README.md) - [Build artifacts](build_artifacts/README.md) -- [User permissions](permissions/README.md) +- [User permissions](../user/permissions.md#gitlab-ci) - [API](../api/ci/README.md) - [CI services (linked docker containers)](services/README.md) diff --git a/doc/ci/permissions/README.md b/doc/ci/permissions/README.md index d77061c14cd..42eb59f84c8 100644 --- a/doc/ci/permissions/README.md +++ b/doc/ci/permissions/README.md @@ -1,24 +1,3 @@ # Users Permissions -GitLab CI relies on user's role on the GitLab. There are three permissions levels on GitLab CI: admin, master, developer, other. - -Admin user can perform any actions on GitLab CI in scope of instance and project. Also user with admin permission can use admin interface. - - - - -| Action | Guest, Reporter | Developer | Master | Admin | -|---------------------------------------|-----------------|-------------|----------|--------| -| See commits and builds | ✓ | ✓ | ✓ | ✓ | -| Retry or cancel build | | ✓ | ✓ | ✓ | -| Remove project | | | ✓ | ✓ | -| Create project | | | ✓ | ✓ | -| Change project configuration | | | ✓ | ✓ | -| Add specific runners | | | ✓ | ✓ | -| Add shared runners | | | | ✓ | -| See events in the system | | | | ✓ | -| Admin interface | | | | ✓ | - - - - +This document was moved to [user/permissions.md](../../user/permissions.md#gitlab-ci). diff --git a/doc/development/ui_guide.md b/doc/development/ui_guide.md index ce0aaa2fd25..8d02c52c477 100644 --- a/doc/development/ui_guide.md +++ b/doc/development/ui_guide.md @@ -27,6 +27,8 @@ We try to keep the amount of tabs in the header navigation between 5 and 10 so t tab should represent separate functionality. Everything related to the issue tracker should be under the 'Issues' tab while everything related to the wiki should be under 'Wiki' tab and so on and so forth. +When adding a new tab to the header don't use more than 2 words for text in the link. +We want to keep links short and easy to remember and fit all of them in the small screen. ## Mobile screen size diff --git a/doc/integration/saml.md b/doc/integration/saml.md index 8a7205caaa4..f3b2a288776 100644 --- a/doc/integration/saml.md +++ b/doc/integration/saml.md @@ -138,7 +138,7 @@ This setting is only available on GitLab 8.7 and above. SAML login includes support for external groups. You can define in the SAML settings which groups, to which your users belong in your IdP, you wish to be -marked as [external](../permissions/permissions.md). +marked as [external](../user/permissions.md). ### Requirements @@ -306,4 +306,4 @@ For this you need take the following into account: validators are optional Make sure that one of the above described scenarios is valid, or the requests will -fail with one of the mentioned errors.
\ No newline at end of file +fail with one of the mentioned errors. diff --git a/doc/permissions/permissions.md b/doc/permissions/permissions.md index 44f3f6d3b12..78d67aeec78 100644 --- a/doc/permissions/permissions.md +++ b/doc/permissions/permissions.md @@ -1,104 +1,3 @@ # Permissions -Users have different abilities depending on the access level they have in a particular group or project. - -If a user is both in a project group and in the project itself, the highest permission level is used. - -If a user is a GitLab administrator they receive all permissions. - -On public and internal projects the Guest role is not enforced. -All users will be able to create issues, leave comments, and pull or download the project code. - -To add or import a user, you can follow the [project users and members -documentation](../workflow/add-user/add-user.md). - -## Project - -| Action | Guest | Reporter | Developer | Master | Owner | -|---------------------------------------|---------|------------|-------------|----------|--------| -| Create new issue | ✓ | ✓ | ✓ | ✓ | ✓ | -| Leave comments | ✓ | ✓ | ✓ | ✓ | ✓ | -| See a list of builds | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | -| See a build log | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | -| Download and browse build artifacts | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | -| Pull project code | | ✓ | ✓ | ✓ | ✓ | -| Download project | | ✓ | ✓ | ✓ | ✓ | -| Create code snippets | | ✓ | ✓ | ✓ | ✓ | -| Manage issue tracker | | ✓ | ✓ | ✓ | ✓ | -| Manage labels | | ✓ | ✓ | ✓ | ✓ | -| See a commit status | | ✓ | ✓ | ✓ | ✓ | -| See a container registry | | ✓ | ✓ | ✓ | ✓ | -| See environments | | ✓ | ✓ | ✓ | ✓ | -| Manage merge requests | | | ✓ | ✓ | ✓ | -| Create new merge request | | | ✓ | ✓ | ✓ | -| Create new branches | | | ✓ | ✓ | ✓ | -| Push to non-protected branches | | | ✓ | ✓ | ✓ | -| Force push to non-protected branches | | | ✓ | ✓ | ✓ | -| Remove non-protected branches | | | ✓ | ✓ | ✓ | -| Add tags | | | ✓ | ✓ | ✓ | -| Write a wiki | | | ✓ | ✓ | ✓ | -| Cancel and retry builds | | | ✓ | ✓ | ✓ | -| Create or update commit status | | | ✓ | ✓ | ✓ | -| Update a container registry | | | ✓ | ✓ | ✓ | -| Remove a container registry image | | | ✓ | ✓ | ✓ | -| Create new environments | | | ✓ | ✓ | ✓ | -| Create new milestones | | | | ✓ | ✓ | -| Add new team members | | | | ✓ | ✓ | -| Push to protected branches | | | | ✓ | ✓ | -| Enable/disable branch protection | | | | ✓ | ✓ | -| Turn on/off prot. branch push for devs| | | | ✓ | ✓ | -| Rewrite/remove git tags | | | | ✓ | ✓ | -| Edit project | | | | ✓ | ✓ | -| Add deploy keys to project | | | | ✓ | ✓ | -| Configure project hooks | | | | ✓ | ✓ | -| Manage runners | | | | ✓ | ✓ | -| Manage build triggers | | | | ✓ | ✓ | -| Manage variables | | | | ✓ | ✓ | -| Delete environments | | | | ✓ | ✓ | -| Switch visibility level | | | | | ✓ | -| Transfer project to another namespace | | | | | ✓ | -| Remove project | | | | | ✓ | -| Force push to protected branches [^2] | | | | | | -| Remove protected branches [^2] | | | | | | - -[^1]: If **Allow guest to access builds** is enabled in CI settings -[^2]: Not allowed for Guest, Reporter, Developer, Master, or Owner - -## Group - -In order for a group to appear as public and be browsable, it must contain at -least one public project. - -Any user can remove themselves from a group, unless they are the last Owner of the group. - -| Action | Guest | Reporter | Developer | Master | Owner | -|-------------------------|-------|----------|-----------|--------|-------| -| Browse group | ✓ | ✓ | ✓ | ✓ | ✓ | -| Edit group | | | | | ✓ | -| Create project in group | | | | ✓ | ✓ | -| Manage group members | | | | | ✓ | -| Remove group | | | | | ✓ | - -## External Users - -In cases where it is desired that a user has access only to some internal or -private projects, there is the option of creating **External Users**. This -feature may be useful when for example a contractor is working on a given -project and should only have access to that project. - -External users can only access projects to which they are explicitly granted -access, thus hiding all other internal or private ones from them. Access can be -granted by adding the user as member to the project or group. - -They will, like usual users, receive a role in the project or group with all -the abilities that are mentioned in the table above. They cannot however create -groups or projects, and they have the same access as logged out users in all -other cases. - -An administrator can flag a user as external [through the API](../api/users.md) -or by checking the checkbox on the admin panel. As an administrator, navigate -to **Admin > Users** to create a new user or edit an existing one. There, you -will find the option to flag the user as external. - -By default new users are not set as external users. This behavior can be changed -by an administrator under **Admin > Application Settings**.
\ No newline at end of file +This document was moved to [user/permissions.md](../user/permissions.md). diff --git a/doc/public_access/public_access.md b/doc/public_access/public_access.md index 9a5c5a5c92a..a3921f1b89f 100644 --- a/doc/public_access/public_access.md +++ b/doc/public_access/public_access.md @@ -17,7 +17,7 @@ Public projects can be cloned **without any** authentication. They will also be listed on the public access directory (`/public`). -**Any logged in user** will have [Guest](../permissions/permissions.md) +**Any logged in user** will have [Guest](../user/permissions.md) permissions on the repository. ### Internal projects @@ -27,7 +27,7 @@ Internal projects can be cloned by any logged in user. They will also be listed on the public access directory (`/public`) for logged in users. -Any logged in user will have [Guest](../permissions/permissions.md) permissions +Any logged in user will have [Guest](../user/permissions.md) permissions on the repository. ### How to change project visibility diff --git a/doc/user/permissions.md b/doc/user/permissions.md new file mode 100644 index 00000000000..66542861781 --- /dev/null +++ b/doc/user/permissions.md @@ -0,0 +1,131 @@ +# Permissions + +Users have different abilities depending on the access level they have in a +particular group or project. If a user is both in a group's project and the +project itself, the highest permission level is used. + +On public and internal projects the Guest role is not enforced. All users will +be able to create issues, leave comments, and pull or download the project code. + +GitLab administrators receive all permissions. + +To add or import a user, you can follow the [project users and members +documentation](../workflow/add-user/add-user.md). + +## Project + +The following table depicts the various user permission levels in a project. + +| Action | Guest | Reporter | Developer | Master | Owner | +|---------------------------------------|---------|------------|-------------|----------|--------| +| Create new issue | ✓ | ✓ | ✓ | ✓ | ✓ | +| Leave comments | ✓ | ✓ | ✓ | ✓ | ✓ | +| See a list of builds | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | +| See a build log | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | +| Download and browse build artifacts | ✓ [^1] | ✓ | ✓ | ✓ | ✓ | +| Pull project code | | ✓ | ✓ | ✓ | ✓ | +| Download project | | ✓ | ✓ | ✓ | ✓ | +| Create code snippets | | ✓ | ✓ | ✓ | ✓ | +| Manage issue tracker | | ✓ | ✓ | ✓ | ✓ | +| Manage labels | | ✓ | ✓ | ✓ | ✓ | +| See a commit status | | ✓ | ✓ | ✓ | ✓ | +| See a container registry | | ✓ | ✓ | ✓ | ✓ | +| See environments | | ✓ | ✓ | ✓ | ✓ | +| Manage/Accept merge requests | | | ✓ | ✓ | ✓ | +| Create new merge request | | | ✓ | ✓ | ✓ | +| Create new branches | | | ✓ | ✓ | ✓ | +| Push to non-protected branches | | | ✓ | ✓ | ✓ | +| Force push to non-protected branches | | | ✓ | ✓ | ✓ | +| Remove non-protected branches | | | ✓ | ✓ | ✓ | +| Add tags | | | ✓ | ✓ | ✓ | +| Write a wiki | | | ✓ | ✓ | ✓ | +| Cancel and retry builds | | | ✓ | ✓ | ✓ | +| Create or update commit status | | | ✓ | ✓ | ✓ | +| Update a container registry | | | ✓ | ✓ | ✓ | +| Remove a container registry image | | | ✓ | ✓ | ✓ | +| Create new environments | | | ✓ | ✓ | ✓ | +| Create new milestones | | | | ✓ | ✓ | +| Add new team members | | | | ✓ | ✓ | +| Push to protected branches | | | | ✓ | ✓ | +| Enable/disable branch protection | | | | ✓ | ✓ | +| Turn on/off protected branch push for devs| | | | ✓ | ✓ | +| Rewrite/remove Git tags | | | | ✓ | ✓ | +| Edit project | | | | ✓ | ✓ | +| Add deploy keys to project | | | | ✓ | ✓ | +| Configure project hooks | | | | ✓ | ✓ | +| Manage runners | | | | ✓ | ✓ | +| Manage build triggers | | | | ✓ | ✓ | +| Manage variables | | | | ✓ | ✓ | +| Delete environments | | | | ✓ | ✓ | +| Switch visibility level | | | | | ✓ | +| Transfer project to another namespace | | | | | ✓ | +| Remove project | | | | | ✓ | +| Force push to protected branches [^2] | | | | | | +| Remove protected branches [^2] | | | | | | + +[^1]: If **Allow guest to access builds** is enabled in CI settings +[^2]: Not allowed for Guest, Reporter, Developer, Master, or Owner + +## Group + +Any user can remove themselves from a group, unless they are the last Owner of +the group. The following table depicts the various user permission levels in a +group. + +| Action | Guest | Reporter | Developer | Master | Owner | +|-------------------------|-------|----------|-----------|--------|-------| +| Browse group | ✓ | ✓ | ✓ | ✓ | ✓ | +| Edit group | | | | | ✓ | +| Create project in group | | | | ✓ | ✓ | +| Manage group members | | | | | ✓ | +| Remove group | | | | | ✓ | + +## External Users + +In cases where it is desired that a user has access only to some internal or +private projects, there is the option of creating **External Users**. This +feature may be useful when for example a contractor is working on a given +project and should only have access to that project. + +External users can only access projects to which they are explicitly granted +access, thus hiding all other internal or private ones from them. Access can be +granted by adding the user as member to the project or group. + +They will, like usual users, receive a role in the project or group with all +the abilities that are mentioned in the table above. They cannot however create +groups or projects, and they have the same access as logged out users in all +other cases. + +An administrator can flag a user as external [through the API](../api/users.md) +or by checking the checkbox on the admin panel. As an administrator, navigate +to **Admin > Users** to create a new user or edit an existing one. There, you +will find the option to flag the user as external. + +By default new users are not set as external users. This behavior can be changed +by an administrator under **Admin > Application Settings**. + +## GitLab CI + +GitLab CI permissions rely on the role the user has in GitLab. There are four +permission levels it total: + +- admin +- master +- developer +- guest/reporter + +The admin user can perform any action on GitLab CI in scope of the GitLab +instance and project. In addition, all admins can use the admin interface under +`/admin/runners`. + +| Action | Guest, Reporter | Developer | Master | Admin | +|---------------------------------------|-----------------|-------------|----------|--------| +| See commits and builds | ✓ | ✓ | ✓ | ✓ | +| Retry or cancel build | | ✓ | ✓ | ✓ | +| Remove project | | | ✓ | ✓ | +| Create project | | | ✓ | ✓ | +| Change project configuration | | | ✓ | ✓ | +| Add specific runners | | | ✓ | ✓ | +| Add shared runners | | | | ✓ | +| See events in the system | | | | ✓ | +| Admin interface | | | | ✓ | diff --git a/doc/workflow/add-user/add-user.md b/doc/workflow/add-user/add-user.md index 4b551130255..0537ce0bcd4 100644 --- a/doc/workflow/add-user/add-user.md +++ b/doc/workflow/add-user/add-user.md @@ -23,7 +23,7 @@ want to add. --- -Select the user and the [permission level](../../permissions/permissions.md) +Select the user and the [permission level](../../user/permissions.md) that you'd like to give the user. Note that you can select more than one user.  diff --git a/doc/workflow/forking_workflow.md b/doc/workflow/forking_workflow.md index 217a4a4012f..733d079bd4a 100644 --- a/doc/workflow/forking_workflow.md +++ b/doc/workflow/forking_workflow.md @@ -38,7 +38,7 @@ Forking a project is in most cases a two-step process. --- After the forking is done, you can start working on the newly created -repository. There, you will have full [Owner](../permissions/permissions.md) +repository. There, you will have full [Owner](../user/permissions.md) access, so you can set it up as you please. ## Merging upstream diff --git a/doc/workflow/protected_branches.md b/doc/workflow/protected_branches.md index 67adfc2f43a..5c1c7b47c8a 100644 --- a/doc/workflow/protected_branches.md +++ b/doc/workflow/protected_branches.md @@ -12,7 +12,7 @@ A protected branch does three simple things: You can make any branch a protected branch. GitLab makes the master branch a protected branch by default. -To protect a branch, user needs to have at least a Master permission level, see [permissions document](../permissions/permissions.md). +To protect a branch, user needs to have at least a Master permission level, see [permissions document](../user/permissions.md).  diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature index 21768c15c17..8176ec5ab45 100644 --- a/features/project/merge_requests.feature +++ b/features/project/merge_requests.feature @@ -89,7 +89,7 @@ Feature: Project Merge Requests Then The list should be sorted by "Oldest updated" @javascript - Scenario: Visiting Merge Requests from a differente Project after sorting + Scenario: Visiting Merge Requests from a different Project after sorting Given I visit project "Shop" merge requests page And I sort the list by "Oldest updated" And I visit dashboard merge requests page diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 8edb80177da..301dbb688a7 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -207,6 +207,8 @@ module API merge_request.subscribed?(options[:current_user]) end expose :user_notes_count + expose :should_remove_source_branch?, as: :should_remove_source_branch + expose :force_remove_source_branch?, as: :force_remove_source_branch end class MergeRequestChanges < MergeRequest diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 77e407b54c5..73557cf7db6 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -17,7 +17,7 @@ module API def current_user @current_user ||= (find_user_by_private_token || doorkeeper_guard) - unless @current_user && Gitlab::UserAccess.allowed?(@current_user) + unless @current_user && Gitlab::UserAccess.new(@current_user).allowed? return nil end diff --git a/lib/banzai.rb b/lib/banzai.rb index 093382261ae..9ebe379f454 100644 --- a/lib/banzai.rb +++ b/lib/banzai.rb @@ -3,6 +3,10 @@ module Banzai Renderer.render(text, context) end + def self.cache_collection_render(texts_and_contexts) + Renderer.cache_collection_render(texts_and_contexts) + end + def self.render_result(text, context = {}) Renderer.render_result(text, context) end diff --git a/lib/banzai/object_renderer.rb b/lib/banzai/object_renderer.rb index dc83b87a6c1..9aef807c152 100644 --- a/lib/banzai/object_renderer.rb +++ b/lib/banzai/object_renderer.rb @@ -39,9 +39,7 @@ module Banzai # Renders the attribute of every given object. def render_objects(objects, attribute) - objects.map do |object| - render_attribute(object, attribute) - end + render_attributes(objects, attribute) end # Redacts the list of documents. @@ -64,16 +62,21 @@ module Banzai context end - # Renders the attribute of an object. + # Renders the attributes of a set of objects. # - # Returns a `Nokogiri::HTML::Document`. - def render_attribute(object, attribute) - context = context_for(object, attribute) + # Returns an Array of `Nokogiri::HTML::Document`. + def render_attributes(objects, attribute) + strings_and_contexts = objects.map do |object| + context = context_for(object, attribute) + + string = object.__send__(attribute) - string = object.__send__(attribute) - html = Banzai.render(string, context) + { text: string, context: context } + end - Banzai::Pipeline[:relative_link].to_document(html, context) + Banzai.cache_collection_render(strings_and_contexts).each_with_index.map do |html, index| + Banzai::Pipeline[:relative_link].to_document(html, strings_and_contexts[index][:context]) + end end def base_context diff --git a/lib/banzai/renderer.rb b/lib/banzai/renderer.rb index 6718acdef7e..910687a7b6a 100644 --- a/lib/banzai/renderer.rb +++ b/lib/banzai/renderer.rb @@ -10,7 +10,7 @@ module Banzai # requiring XHTML, such as Atom feeds, need to call `post_process` on the # result, providing the appropriate `pipeline` option. # - # markdown - Markdown String + # text - Markdown String # context - Hash of context options passed to our HTML Pipeline # # Returns an HTML-safe String @@ -29,6 +29,58 @@ module Banzai end end + # Perform multiple render from an Array of Markdown String into an + # Array of HTML-safe String of HTML. + # + # As the rendered Markdown String can be already cached read all the data + # from the cache using Rails.cache.read_multi operation. If the Markdown String + # is not in the cache or it's not cacheable (no cache_key entry is provided in + # the context) the Markdown String is rendered and stored in the cache so the + # next render call gets the rendered HTML-safe String from the cache. + # + # For further explanation see #render method comments. + # + # texts_and_contexts - An Array of Hashes that contains the Markdown String (:text) + # an options passed to our HTML Pipeline (:context) + # + # If on the :context you specify a :cache_key entry will be used to retrieve it + # and cache the result of rendering the Markdown String. + # + # Returns an Array containing HTML-safe String instances. + # + # Example: + # texts_and_contexts + # => [{ text: '### Hello', + # context: { cache_key: [note, :note] } }] + def self.cache_collection_render(texts_and_contexts) + items_collection = texts_and_contexts.each_with_index do |item, index| + context = item[:context] + cache_key = full_cache_multi_key(context.delete(:cache_key), context[:pipeline]) + + item[:cache_key] = cache_key if cache_key + end + + cacheable_items, non_cacheable_items = items_collection.partition { |item| item.key?(:cache_key) } + + items_in_cache = [] + items_not_in_cache = [] + + unless cacheable_items.empty? + items_in_cache = Rails.cache.read_multi(*cacheable_items.map { |item| item[:cache_key] }) + items_not_in_cache = cacheable_items.reject do |item| + item[:rendered] = items_in_cache[item[:cache_key]] + items_in_cache.key?(item[:cache_key]) + end + end + + (items_not_in_cache + non_cacheable_items).each do |item| + item[:rendered] = render(item[:text], item[:context]) + Rails.cache.write(item[:cache_key], item[:rendered]) if item[:cache_key] + end + + items_collection.map { |item| item[:rendered] } + end + def self.render_result(text, context = {}) text = Pipeline[:pre_process].to_html(text, context) if text @@ -78,5 +130,13 @@ module Banzai return unless cache_key ["banzai", *cache_key, pipeline_name || :full] end + + # To map Rails.cache.read_multi results we need to know the Rails.cache.expanded_key. + # Other option will be to generate stringified keys on our side and don't delegate to Rails.cache.expanded_key + # method. + def self.full_cache_multi_key(cache_key, pipeline_name) + return unless cache_key + Rails.cache.send(:expanded_key, full_cache_key(cache_key, pipeline_name)) + end end end diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index 831f1e635ba..de41ea415a6 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -14,9 +14,10 @@ module Gitlab OWNER = 50 # Branch protection settings - PROTECTION_NONE = 0 - PROTECTION_DEV_CAN_PUSH = 1 - PROTECTION_FULL = 2 + PROTECTION_NONE = 0 + PROTECTION_DEV_CAN_PUSH = 1 + PROTECTION_FULL = 2 + PROTECTION_DEV_CAN_MERGE = 3 class << self def values @@ -54,6 +55,7 @@ module Gitlab def protection_options { "Not protected: Both developers and masters can push new commits, force push, or delete the branch." => PROTECTION_NONE, + "Protected against pushes: Developers cannot push new commits, but are allowed to accept merge requests to the branch." => PROTECTION_DEV_CAN_MERGE, "Partially protected: Developers can push new commits, but cannot force push or delete the branch. Masters can do all of those." => PROTECTION_DEV_CAN_PUSH, "Fully protected: Developers cannot push new commits, force push, or delete the branch. Only masters can do any of those." => PROTECTION_FULL, } diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb new file mode 100644 index 00000000000..5551fac4b8b --- /dev/null +++ b/lib/gitlab/checks/change_access.rb @@ -0,0 +1,96 @@ +module Gitlab + module Checks + class ChangeAccess + attr_reader :user_access, :project + + def initialize(change, user_access:, project:) + @oldrev, @newrev, @ref = change.split(' ') + @branch_name = branch_name(@ref) + @user_access = user_access + @project = project + end + + def exec + error = protected_branch_checks || tag_checks || push_checks + + if error + GitAccessStatus.new(false, error) + else + GitAccessStatus.new(true) + end + end + + protected + + def protected_branch_checks + return unless project.protected_branch?(@branch_name) + + if forced_push? && user_access.cannot_do_action?(:force_push_code_to_protected_branches) + return "You are not allowed to force push code to a protected branch on this project." + elsif Gitlab::Git.blank_ref?(@newrev) && user_access.cannot_do_action?(:remove_protected_branches) + return "You are not allowed to delete protected branches from this project." + end + + if matching_merge_request? + if user_access.can_merge_to_branch?(@branch_name) || user_access.can_push_to_branch?(@branch_name) + return + else + "You are not allowed to merge code into protected branches on this project." + end + else + if user_access.can_push_to_branch?(@branch_name) + return + else + "You are not allowed to push code to protected branches on this project." + end + end + end + + def tag_checks + tag_ref = tag_name(@ref) + + if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project) + "You are not allowed to change existing tags on this project." + end + end + + def push_checks + if user_access.cannot_do_action?(:push_code) + "You are not allowed to push code to this project." + end + end + + private + + def protected_tag?(tag_name) + project.repository.tag_exists?(tag_name) + end + + def forced_push? + Gitlab::Checks::ForcePush.force_push?(@project, @oldrev, @newrev) + end + + def matching_merge_request? + Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match? + end + + def branch_name(ref) + ref = @ref.to_s + if Gitlab::Git.branch_ref?(ref) + Gitlab::Git.ref_name(ref) + else + nil + end + end + + def tag_name(ref) + ref = @ref.to_s + if Gitlab::Git.tag_ref?(ref) + Gitlab::Git.ref_name(ref) + else + nil + end + end + end + end +end diff --git a/lib/gitlab/checks/force_push.rb b/lib/gitlab/checks/force_push.rb new file mode 100644 index 00000000000..dfa83a0eab3 --- /dev/null +++ b/lib/gitlab/checks/force_push.rb @@ -0,0 +1,17 @@ +module Gitlab + module Checks + class ForcePush + def self.force_push?(project, oldrev, newrev) + return false if project.empty_repo? + + # Created or deleted branch + if Gitlab::Git.blank_ref?(oldrev) || Gitlab::Git.blank_ref?(newrev) + false + else + missed_refs, _ = Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} --git-dir=#{project.repository.path_to_repo} rev-list #{oldrev} ^#{newrev})) + missed_refs.split("\n").size > 0 + end + end + end + end +end diff --git a/lib/gitlab/checks/matching_merge_request.rb b/lib/gitlab/checks/matching_merge_request.rb new file mode 100644 index 00000000000..849848515da --- /dev/null +++ b/lib/gitlab/checks/matching_merge_request.rb @@ -0,0 +1,18 @@ +module Gitlab + module Checks + class MatchingMergeRequest + def initialize(newrev, branch_name, project) + @newrev = newrev + @branch_name = branch_name + @project = project + end + + def match? + @project.merge_requests + .with_state(:locked) + .where(in_progress_merge_commit_sha: @newrev, target_branch: @branch_name) + .exists? + end + end + end +end diff --git a/lib/gitlab/diff/inline_diff.rb b/lib/gitlab/diff/inline_diff.rb index 789c14518b0..28ad637fda4 100644 --- a/lib/gitlab/diff/inline_diff.rb +++ b/lib/gitlab/diff/inline_diff.rb @@ -1,16 +1,30 @@ module Gitlab module Diff class InlineDiff + # Regex to find a run of deleted lines followed by the same number of added lines + LINE_PAIRS_PATTERN = %r{ + # Runs start at the beginning of the string (the first line) or after a space (for an unchanged line) + (?:\A|\s) + + # This matches a number of `-`s followed by the same number of `+`s through recursion + (?<del_ins> + - + \g<del_ins>? + \+ + ) + + # Runs end at the end of the string (the last line) or before a space (for an unchanged line) + (?=\s|\z) + }x.freeze + attr_accessor :old_line, :new_line, :offset def self.for_lines(lines) - local_edit_indexes = self.find_local_edits(lines) + changed_line_pairs = self.find_changed_line_pairs(lines) inline_diffs = [] - local_edit_indexes.each do |index| - old_index = index - new_index = index + 1 + changed_line_pairs.each do |old_index, new_index| old_line = lines[old_index] new_line = lines[new_index] @@ -51,18 +65,28 @@ module Gitlab private - def self.find_local_edits(lines) - line_prefixes = lines.map { |line| line.match(/\A([+-])/) ? $1 : ' ' } - joined_line_prefixes = " #{line_prefixes.join} " - - offset = 0 - local_edit_indexes = [] - while index = joined_line_prefixes.index(" -+ ", offset) - local_edit_indexes << index - offset = index + 1 + # Finds pairs of old/new line pairs that represent the same line that changed + def self.find_changed_line_pairs(lines) + # Prefixes of all diff lines, indicating their types + # For example: `" - + -+ ---+++ --+ -++"` + line_prefixes = lines.each_with_object("") { |line, s| s << line[0] }.gsub(/[^ +-]/, ' ') + + changed_line_pairs = [] + line_prefixes.scan(LINE_PAIRS_PATTERN) do + # For `"---+++"`, `begin_index == 0`, `end_index == 6` + begin_index, end_index = Regexp.last_match.offset(:del_ins) + + # For `"---+++"`, `changed_line_count == 3` + changed_line_count = (end_index - begin_index) / 2 + + halfway_index = begin_index + changed_line_count + (begin_index...halfway_index).each do |i| + # For `"---+++"`, index 1 maps to 1 + 3 = 4 + changed_line_pairs << [i, i + changed_line_count] + end end - local_edit_indexes + changed_line_pairs end def longest_common_prefix(a, b) diff --git a/lib/gitlab/force_push_check.rb b/lib/gitlab/force_push_check.rb deleted file mode 100644 index 93c6a5bb7f5..00000000000 --- a/lib/gitlab/force_push_check.rb +++ /dev/null @@ -1,15 +0,0 @@ -module Gitlab - class ForcePushCheck - def self.force_push?(project, oldrev, newrev) - return false if project.empty_repo? - - # Created or deleted branch - if Gitlab::Git.blank_ref?(oldrev) || Gitlab::Git.blank_ref?(newrev) - false - else - missed_refs, _ = Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} --git-dir=#{project.repository.path_to_repo} rev-list #{oldrev} ^#{newrev})) - missed_refs.split("\n").size > 0 - end - end - end -end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index 7679c7e4bb8..308f23bc9bc 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -1,52 +1,17 @@ +# Check a user's access to perform a git action. All public methods in this +# class return an instance of `GitlabAccessStatus` module Gitlab class GitAccess DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive } PUSH_COMMANDS = %w{ git-receive-pack } - attr_reader :actor, :project, :protocol + attr_reader :actor, :project, :protocol, :user_access def initialize(actor, project, protocol) @actor = actor @project = project @protocol = protocol - end - - def user - return @user if defined?(@user) - - @user = - case actor - when User - actor - when DeployKey - nil - when Key - actor.user - end - end - - def deploy_key - actor if actor.is_a?(DeployKey) - end - - def can_push_to_branch?(ref) - return false unless user - - if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref) - user.can?(:push_code_to_protected_branches, project) - else - user.can?(:push_code, project) - end - end - - def can_read_project? - if user - user.can?(:read_project, project) - elsif deploy_key - deploy_key.projects.include?(project) - else - false - end + @user_access = UserAccess.new(user, project: project) end def check(cmd, changes = nil) @@ -56,11 +21,11 @@ module Gitlab return build_status_object(false, "No user or key was provided.") end - if user && !user_allowed? + if user && !user_access.allowed? return build_status_object(false, "Your account has been blocked.") end - unless project && can_read_project? + unless project && (user_access.can_read_project? || deploy_key_can_read_project?) return build_status_object(false, 'The project you were looking for could not be found.') end @@ -95,7 +60,7 @@ module Gitlab end def user_download_access_check - unless user.can?(:download_code, project) + unless user_access.can_do_action?(:download_code) return build_status_object(false, "You are not allowed to download code from this project.") end @@ -125,46 +90,8 @@ module Gitlab build_status_object(true) end - def can_user_do_action?(action) - @permission_cache ||= {} - @permission_cache[action] ||= user.can?(action, project) - end - def change_access_check(change) - oldrev, newrev, ref = change.split(' ') - - action = - if project.protected_branch?(branch_name(ref)) - protected_branch_action(oldrev, newrev, branch_name(ref)) - elsif (tag_ref = tag_name(ref)) && protected_tag?(tag_ref) - # Prevent any changes to existing git tag unless user has permissions - :admin_project - else - :push_code - end - - unless can_user_do_action?(action) - status = - case action - when :force_push_code_to_protected_branches - build_status_object(false, "You are not allowed to force push code to a protected branch on this project.") - when :remove_protected_branches - build_status_object(false, "You are not allowed to deleted protected branches from this project.") - when :push_code_to_protected_branches - build_status_object(false, "You are not allowed to push code to protected branches on this project.") - when :admin_project - build_status_object(false, "You are not allowed to change existing tags on this project.") - else # :push_code - build_status_object(false, "You are not allowed to push code to this project.") - end - return status - end - - build_status_object(true) - end - - def forced_push?(oldrev, newrev) - Gitlab::ForcePushCheck.force_push?(project, oldrev, newrev) + Checks::ChangeAccess.new(change, user_access: user_access, project: project).exec end def protocol_allowed? @@ -173,48 +100,38 @@ module Gitlab private - def protected_branch_action(oldrev, newrev, branch_name) - # we dont allow force push to protected branch - if forced_push?(oldrev, newrev) - :force_push_code_to_protected_branches - elsif Gitlab::Git.blank_ref?(newrev) - # and we dont allow remove of protected branch - :remove_protected_branches - elsif project.developers_can_push_to_protected_branch?(branch_name) - :push_code - else - :push_code_to_protected_branches - end + def matching_merge_request?(newrev, branch_name) + Checks::MatchingMergeRequest.new(newrev, branch_name, project).match? end - def protected_tag?(tag_name) - project.repository.tag_exists?(tag_name) - end - - def user_allowed? - Gitlab::UserAccess.allowed?(user) - end - - def branch_name(ref) - ref = ref.to_s - if Gitlab::Git.branch_ref?(ref) - Gitlab::Git.ref_name(ref) - else - nil - end + def deploy_key + actor if actor.is_a?(DeployKey) end - def tag_name(ref) - ref = ref.to_s - if Gitlab::Git.tag_ref?(ref) - Gitlab::Git.ref_name(ref) + def deploy_key_can_read_project? + if deploy_key + deploy_key.projects.include?(project) else - nil + false end end protected + def user + return @user if defined?(@user) + + @user = + case actor + when User + actor + when DeployKey + nil + when Key + actor.user + end + end + def build_status_object(status, message = '') GitAccessStatus.new(status, message) end diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb index 8672cbc0ec4..f71d3575909 100644 --- a/lib/gitlab/git_access_wiki.rb +++ b/lib/gitlab/git_access_wiki.rb @@ -1,7 +1,7 @@ module Gitlab class GitAccessWiki < GitAccess def change_access_check(change) - if user.can?(:create_wiki, project) + if user_access.can_do_action?(:create_wiki) build_status_object(true) else build_status_object(false, "You are not allowed to write to this project's wiki.") diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb index 043f10d96a9..084e514492c 100644 --- a/lib/gitlab/github_import/client.rb +++ b/lib/gitlab/github_import/client.rb @@ -78,10 +78,21 @@ module Gitlab def rate_limit api.rate_limit! + # GitHub Rate Limit API returns 404 when the rate limit is + # disabled. In this case we just want to return gracefully + # instead of spitting out an error. + rescue Octokit::NotFound + nil + end + + def has_rate_limit? + return @has_rate_limit if defined?(@has_rate_limit) + + @has_rate_limit = rate_limit.present? end def rate_limit_exceed? - rate_limit.remaining <= GITHUB_SAFE_REMAINING_REQUESTS + has_rate_limit? && rate_limit.remaining <= GITHUB_SAFE_REMAINING_REQUESTS end def rate_limit_sleep_time diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb index d1b42c1f9b9..c0f85e9b3a8 100644 --- a/lib/gitlab/user_access.rb +++ b/lib/gitlab/user_access.rb @@ -1,7 +1,23 @@ module Gitlab - module UserAccess - def self.allowed?(user) - return false if user.blocked? + class UserAccess + attr_reader :user, :project + + def initialize(user, project: nil) + @user = user + @project = project + end + + def can_do_action?(action) + @permission_cache ||= {} + @permission_cache[action] ||= user.can?(action, project) + end + + def cannot_do_action?(action) + !can_do_action?(action) + end + + def allowed? + return false if user.blank? || user.blocked? if user.requires_ldap_check? && user.try_obtain_ldap_lease return false unless Gitlab::LDAP::Access.allowed?(user) @@ -9,5 +25,31 @@ module Gitlab true end + + def can_push_to_branch?(ref) + return false unless user + + if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref) + user.can?(:push_code_to_protected_branches, project) + else + user.can?(:push_code, project) + end + end + + def can_merge_to_branch?(ref) + return false unless user + + if project.protected_branch?(ref) && !project.developers_can_merge_to_protected_branch?(ref) + user.can?(:push_code_to_protected_branches, project) + else + user.can?(:push_code, project) + end + end + + def can_read_project? + return false unless user + + user.can?(:read_project, project) + end end end diff --git a/lib/support/nginx/gitlab b/lib/support/nginx/gitlab index d521de28e8a..4a4892a2e07 100644 --- a/lib/support/nginx/gitlab +++ b/lib/support/nginx/gitlab @@ -49,7 +49,12 @@ server { proxy_http_version 1.1; - proxy_set_header Host $http_host; + ## By overwriting Host and clearing X-Forwarded-Host we ensure that + ## internal HTTP redirects generated by GitLab always send users to + ## YOUR_SERVER_FQDN. + proxy_set_header Host YOUR_SERVER_FQDN; + proxy_set_header X-Forwarded-Host ""; + proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; diff --git a/lib/support/nginx/gitlab-ssl b/lib/support/nginx/gitlab-ssl index bf014b56cf6..0b93d7f292f 100644 --- a/lib/support/nginx/gitlab-ssl +++ b/lib/support/nginx/gitlab-ssl @@ -93,7 +93,12 @@ server { proxy_http_version 1.1; - proxy_set_header Host $http_host; + ## By overwriting Host and clearing X-Forwarded-Host we ensure that + ## internal HTTP redirects generated by GitLab always send users to + ## YOUR_SERVER_FQDN. + proxy_set_header Host YOUR_SERVER_FQDN; + proxy_set_header X-Forwarded-Host ""; + proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-Ssl on; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; diff --git a/spec/controllers/projects/todo_controller_spec.rb b/spec/controllers/projects/todo_controller_spec.rb index 5a8bba28594..936320a3709 100644 --- a/spec/controllers/projects/todo_controller_spec.rb +++ b/spec/controllers/projects/todo_controller_spec.rb @@ -1,6 +1,8 @@ require('spec_helper') describe Projects::TodosController do + include ApiHelpers + let(:user) { create(:user) } let(:project) { create(:project) } let(:issue) { create(:issue, project: project) } @@ -8,43 +10,51 @@ describe Projects::TodosController do context 'Issues' do describe 'POST create' do + def go + post :create, + namespace_id: project.namespace.path, + project_id: project.path, + issuable_id: issue.id, + issuable_type: 'issue', + format: 'html' + end + context 'when authorized' do before do sign_in(user) project.team << [user, :developer] end - it 'should create todo for issue' do + it 'creates todo for issue' do expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: issue.id, - issuable_type: 'issue') + go end.to change { user.todos.count }.by(1) expect(response).to have_http_status(200) end + + it 'returns todo path and pending count' do + go + + expect(response).to have_http_status(200) + expect(json_response['count']).to eq 1 + expect(json_response['delete_path']).to match(/\/dashboard\/todos\/\d{1}/) + end end context 'when not authorized' do - it 'should not create todo for issue that user has no access to' do + it 'does not create todo for issue that user has no access to' do sign_in(user) expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: issue.id, - issuable_type: 'issue') + go end.to change { user.todos.count }.by(0) expect(response).to have_http_status(404) end - it 'should not create todo for issue when user not logged in' do + it 'does not create todo for issue when user not logged in' do expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: issue.id, - issuable_type: 'issue') + go end.to change { user.todos.count }.by(0) expect(response).to have_http_status(302) @@ -55,43 +65,51 @@ describe Projects::TodosController do context 'Merge Requests' do describe 'POST create' do + def go + post :create, + namespace_id: project.namespace.path, + project_id: project.path, + issuable_id: merge_request.id, + issuable_type: 'merge_request', + format: 'html' + end + context 'when authorized' do before do sign_in(user) project.team << [user, :developer] end - it 'should create todo for merge request' do + it 'creates todo for merge request' do expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: merge_request.id, - issuable_type: 'merge_request') + go end.to change { user.todos.count }.by(1) expect(response).to have_http_status(200) end + + it 'returns todo path and pending count' do + go + + expect(response).to have_http_status(200) + expect(json_response['count']).to eq 1 + expect(json_response['delete_path']).to match(/\/dashboard\/todos\/\d{1}/) + end end context 'when not authorized' do - it 'should not create todo for merge request user has no access to' do + it 'does not create todo for merge request user has no access to' do sign_in(user) expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: merge_request.id, - issuable_type: 'merge_request') + go end.to change { user.todos.count }.by(0) expect(response).to have_http_status(404) end - it 'should not create todo for merge request user has no access to' do + it 'does not create todo for merge request user has no access to' do expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: merge_request.id, - issuable_type: 'merge_request') + go end.to change { user.todos.count }.by(0) expect(response).to have_http_status(302) diff --git a/spec/factories/todos.rb b/spec/factories/todos.rb index 7fc20cd5555..866e663f026 100644 --- a/spec/factories/todos.rb +++ b/spec/factories/todos.rb @@ -23,6 +23,10 @@ FactoryGirl.define do action { Todo::BUILD_FAILED } end + trait :approval_required do + action { Todo::APPROVAL_REQUIRED } + end + trait :done do state :done end diff --git a/spec/features/expand_collapse_diffs_spec.rb b/spec/features/expand_collapse_diffs_spec.rb index 7cff196c8d9..78bc888f2a6 100644 --- a/spec/features/expand_collapse_diffs_spec.rb +++ b/spec/features/expand_collapse_diffs_spec.rb @@ -106,7 +106,7 @@ feature 'Expand and collapse diffs', js: true, feature: true do let(:comment_text) { 'A comment' } before do - large_diff.find('.line_holder', match: :prefer_exact).hover + large_diff.find('.diff-line-num', match: :prefer_exact).hover large_diff.find('.add-diff-note').click large_diff.find('.note-textarea').send_keys comment_text large_diff.find_button('Comment').click @@ -161,7 +161,7 @@ feature 'Expand and collapse diffs', js: true, feature: true do end it 'does not make a new HTTP request' do - expect(evaluate_script('ajaxUris')).to be_empty + expect(evaluate_script('ajaxUris')).not_to include(a_string_matching('small_diff.md')) end end end @@ -199,7 +199,7 @@ feature 'Expand and collapse diffs', js: true, feature: true do end it 'does not make a new HTTP request' do - expect(evaluate_script('ajaxUris')).to be_empty + expect(evaluate_script('ajaxUris')).not_to include(a_string_matching('small_diff.md')) end end end diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index d51c9abea19..cfe6349a1a1 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -121,6 +121,17 @@ describe 'Issues', feature: true do expect(page).to have_content date.to_s(:medium) end end + + it 'warns about version conflict' do + issue.update(title: "New title") + + fill_in 'issue_title', with: 'bug 345' + fill_in 'issue_description', with: 'bug description' + + click_button 'Save changes' + + expect(page).to have_content 'Someone edited the issue the same time you did' + end end end diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index 72b5ff231f7..58753ff21f6 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -28,6 +28,11 @@ feature 'Login', feature: true do end describe 'with two-factor authentication' do + def enter_code(code) + fill_in 'Two-Factor Authentication code', with: code + click_button 'Verify code' + end + context 'with valid username/password' do let(:user) { create(:user, :two_factor) } @@ -36,11 +41,6 @@ feature 'Login', feature: true do expect(page).to have_content('Two-Factor Authentication') end - def enter_code(code) - fill_in 'Two-Factor Authentication code', with: code - click_button 'Verify code' - end - it 'does not show a "You are already signed in." error message' do enter_code(user.current_otp) expect(page).not_to have_content('You are already signed in.') @@ -108,6 +108,39 @@ feature 'Login', feature: true do end end end + + context 'logging in via OAuth' do + def saml_config + OpenStruct.new(name: 'saml', label: 'saml', args: { + assertion_consumer_service_url: 'https://localhost:3443/users/auth/saml/callback', + idp_cert_fingerprint: '26:43:2C:47:AF:F0:6B:D0:07:9C:AD:A3:74:FE:5D:94:5F:4E:9E:52', + idp_sso_target_url: 'https://idp.example.com/sso/saml', + issuer: 'https://localhost:3443/', + name_identifier_format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:transient' + }) + end + + def stub_omniauth_config(messages) + Rails.application.env_config['devise.mapping'] = Devise.mappings[:user] + Rails.application.routes.disable_clear_and_finalize = true + Rails.application.routes.draw do + post '/users/auth/saml' => 'omniauth_callbacks#saml' + end + allow(Gitlab::OAuth::Provider).to receive_messages(providers: [:saml], config_for: saml_config) + allow(Gitlab.config.omniauth).to receive_messages(messages) + allow_any_instance_of(Object).to receive(:user_omniauth_authorize_path).with('saml').and_return('/users/auth/saml') + end + + it 'should show 2FA prompt after OAuth login' do + stub_omniauth_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [saml_config]) + user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml') + login_via('saml', user, 'my-uid') + + expect(page).to have_content('Two-Factor Authentication') + enter_code(user.current_otp) + expect(current_path).to eq root_path + end + end end describe 'without two-factor authentication' do diff --git a/spec/features/merge_requests/diffs_spec.rb b/spec/features/merge_requests/diffs_spec.rb new file mode 100644 index 00000000000..c9a0059645d --- /dev/null +++ b/spec/features/merge_requests/diffs_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +feature 'Diffs URL', js: true, feature: true do + before do + login_as :admin + @merge_request = create(:merge_request) + @project = @merge_request.source_project + end + + context 'when visit with */* as accept header' do + before(:each) do + page.driver.add_header('Accept', '*/*') + end + + it 'renders the notes' do + create :note_on_merge_request, project: @project, noteable: @merge_request, note: 'Rebasing with master' + + visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request) + + # Load notes and diff through AJAX + expect(page).to have_css('.note-text', visible: false, text: 'Rebasing with master') + expect(page).to have_css('.diffs.tab-pane.active') + end + end +end diff --git a/spec/features/merge_requests/edit_mr_spec.rb b/spec/features/merge_requests/edit_mr_spec.rb index 9e007ab7635..8ad884492d1 100644 --- a/spec/features/merge_requests/edit_mr_spec.rb +++ b/spec/features/merge_requests/edit_mr_spec.rb @@ -17,5 +17,16 @@ feature 'Edit Merge Request', feature: true do it 'form should have class js-quick-submit' do expect(page).to have_selector('.js-quick-submit') end + + it 'warns about version conflict' do + merge_request.update(title: "New title") + + fill_in 'merge_request_title', with: 'bug 345' + fill_in 'merge_request_description', with: 'bug description' + + click_button 'Save changes' + + expect(page).to have_content 'Someone edited the merge request the same time you did' + end end end diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb index 1bd354815e4..8db897b1646 100644 --- a/spec/finders/notes_finder_spec.rb +++ b/spec/finders/notes_finder_spec.rb @@ -11,7 +11,7 @@ describe NotesFinder do project.team << [user, :master] end - describe :execute do + describe '#execute' do let(:params) { { target_id: commit.id, target_type: 'commit', last_fetched_at: 1.hour.ago.to_i } } before do diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb index d99175967af..bcdb95250ca 100644 --- a/spec/lib/banzai/object_renderer_spec.rb +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -29,7 +29,7 @@ describe Banzai::ObjectRenderer do renderer = described_class.new(project, user) - expect(renderer).to receive(:render_attribute).with(object, :note). + expect(renderer).to receive(:render_attributes).with([object], :note). and_call_original rendered = renderer.render_objects([object], :note) @@ -89,14 +89,36 @@ describe Banzai::ObjectRenderer do end end - describe '#render_attribute' do - it 'renders the attribute of an object' do - object = double(:doc, note: 'hello') + describe '#render_attributes' do + it 'renders the attribute of a list of objects' do + objects = [double(:doc, note: 'hello'), double(:doc, note: 'bye')] renderer = described_class.new(project, user, pipeline: :note) - doc = renderer.render_attribute(object, :note) - expect(doc).to be_an_instance_of(Nokogiri::HTML::DocumentFragment) - expect(doc.to_html).to eq('<p>hello</p>') + expect(Banzai).to receive(:cache_collection_render). + with([ + { text: 'hello', context: renderer.context_for(objects[0], :note) }, + { text: 'bye', context: renderer.context_for(objects[1], :note) } + ]). + and_call_original + + docs = renderer.render_attributes(objects, :note) + + expect(docs[0]).to be_an_instance_of(Nokogiri::HTML::DocumentFragment) + expect(docs[0].to_html).to eq('<p>hello</p>') + + expect(docs[1]).to be_an_instance_of(Nokogiri::HTML::DocumentFragment) + expect(docs[1].to_html).to eq('<p>bye</p>') + end + + it 'returns when no objects to render' do + objects = [] + renderer = described_class.new(project, user, pipeline: :note) + + expect(Banzai).to receive(:cache_collection_render). + with([]). + and_call_original + + expect(renderer.render_attributes(objects, :note)).to eq([]) end end diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index bad439bc489..bcbf409c8b0 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -31,7 +31,7 @@ module Ci }) end - describe :only do + describe 'only' do it "does not return builds if only has another branch" do config = YAML.dump({ before_script: ["pwd"], @@ -187,7 +187,7 @@ module Ci end end - describe :except do + describe 'except' do it "returns builds if except has another branch" do config = YAML.dump({ before_script: ["pwd"], diff --git a/spec/lib/gitlab/build_data_builder_spec.rb b/spec/lib/gitlab/build_data_builder_spec.rb index 38be9448794..23ae5cfacc4 100644 --- a/spec/lib/gitlab/build_data_builder_spec.rb +++ b/spec/lib/gitlab/build_data_builder_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe 'Gitlab::BuildDataBuilder' do let(:build) { create(:ci_build) } - describe :build do + describe '.build' do let(:data) do Gitlab::BuildDataBuilder.build(build) end diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index 1cb513d5229..0460dcf4658 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -8,14 +8,14 @@ describe Gitlab::Diff::File, lib: true do let(:diff) { commit.diffs.first } let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) } - describe :diff_lines do + describe '#diff_lines' do let(:diff_lines) { diff_file.diff_lines } it { expect(diff_lines.size).to eq(30) } it { expect(diff_lines.first).to be_kind_of(Gitlab::Diff::Line) } end - describe :mode_changed? do + describe '#mode_changed?' do it { expect(diff_file.mode_changed?).to be_falsey } end diff --git a/spec/lib/gitlab/diff/inline_diff_spec.rb b/spec/lib/gitlab/diff/inline_diff_spec.rb index 95a993d26cf..8ca3f73509e 100644 --- a/spec/lib/gitlab/diff/inline_diff_spec.rb +++ b/spec/lib/gitlab/diff/inline_diff_spec.rb @@ -3,14 +3,19 @@ require 'spec_helper' describe Gitlab::Diff::InlineDiff, lib: true do describe '.for_lines' do let(:diff) do - <<eos - class Test -- def initialize(test = true) -+ def initialize(test = false) - @test = test - end - end -eos + <<-EOF.strip_heredoc + class Test + - def initialize(test = true) + + def initialize(test = false) + @test = test + - if true + - @foo = "bar" + + unless false + + @foo = "baz" + end + end + end + EOF end let(:subject) { described_class.for_lines(diff.lines) } @@ -20,8 +25,11 @@ eos expect(subject[1]).to eq([25..27]) expect(subject[2]).to eq([25..28]) expect(subject[3]).to be_nil - expect(subject[4]).to be_nil - expect(subject[5]).to be_nil + expect(subject[4]).to eq([5..10]) + expect(subject[5]).to eq([17..17]) + expect(subject[6]).to eq([5..15]) + expect(subject[7]).to eq([17..17]) + expect(subject[8]).to be_nil end end diff --git a/spec/lib/gitlab/diff/parser_spec.rb b/spec/lib/gitlab/diff/parser_spec.rb index cdff063a9ed..c3359627652 100644 --- a/spec/lib/gitlab/diff/parser_spec.rb +++ b/spec/lib/gitlab/diff/parser_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::Diff::Parser, lib: true do let(:diff) { commit.diffs.first } let(:parser) { Gitlab::Diff::Parser.new } - describe :parse do + describe '#parse' do let(:diff) do <<eos --- a/files/ruby/popen.rb diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb index 08312e60f4a..c268f84c759 100644 --- a/spec/lib/gitlab/diff/position_tracer_spec.rb +++ b/spec/lib/gitlab/diff/position_tracer_spec.rb @@ -1639,7 +1639,8 @@ describe Gitlab::Diff::PositionTracer, lib: true do committer: committer } - repository.merge(current_user, second_create_file_commit.sha, branch_name, options) + merge_request = create(:merge_request, source_branch: second_create_file_commit.sha, target_branch: branch_name, source_project: project) + repository.merge(current_user, merge_request, options) project.commit(branch_name) end diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index c79ba11f782..db33c7a22bb 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -6,67 +6,6 @@ describe Gitlab::GitAccess, lib: true do let(:user) { create(:user) } let(:actor) { user } - describe 'can_push_to_branch?' do - describe 'push to none protected branch' do - it "returns true if user is a master" do - project.team << [user, :master] - expect(access.can_push_to_branch?("random_branch")).to be_truthy - end - - it "returns true if user is a developer" do - project.team << [user, :developer] - expect(access.can_push_to_branch?("random_branch")).to be_truthy - end - - it "returns false if user is a reporter" do - project.team << [user, :reporter] - expect(access.can_push_to_branch?("random_branch")).to be_falsey - end - end - - describe 'push to protected branch' do - before do - @branch = create :protected_branch, project: project - end - - it "returns true if user is a master" do - project.team << [user, :master] - expect(access.can_push_to_branch?(@branch.name)).to be_truthy - end - - it "returns false if user is a developer" do - project.team << [user, :developer] - expect(access.can_push_to_branch?(@branch.name)).to be_falsey - end - - it "returns false if user is a reporter" do - project.team << [user, :reporter] - expect(access.can_push_to_branch?(@branch.name)).to be_falsey - end - end - - describe 'push to protected branch if allowed for developers' do - before do - @branch = create :protected_branch, project: project, developers_can_push: true - end - - it "returns true if user is a master" do - project.team << [user, :master] - expect(access.can_push_to_branch?(@branch.name)).to be_truthy - end - - it "returns true if user is a developer" do - project.team << [user, :developer] - expect(access.can_push_to_branch?(@branch.name)).to be_truthy - end - - it "returns false if user is a reporter" do - project.team << [user, :reporter] - expect(access.can_push_to_branch?(@branch.name)).to be_falsey - end - end - end - describe '#check with single protocols allowed' do def disable_protocol(protocol) settings = ::ApplicationSetting.create_from_defaults @@ -160,96 +99,46 @@ describe Gitlab::GitAccess, lib: true do end describe 'push_access_check' do - def protect_feature_branch - create(:protected_branch, name: 'feature', project: project) - end + before { merge_into_protected_branch } + let(:unprotected_branch) { FFaker::Internet.user_name } - def changes - { - push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow", + let(:changes) do + { push_new_branch: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/heads/wow", push_master: '6f6d7e7ed 570e7b2ab refs/heads/master', push_protected_branch: '6f6d7e7ed 570e7b2ab refs/heads/feature', push_remove_protected_branch: "570e7b2ab #{Gitlab::Git::BLANK_SHA} "\ 'refs/heads/feature', push_tag: '6f6d7e7ed 570e7b2ab refs/tags/v1.0.0', push_new_tag: "#{Gitlab::Git::BLANK_SHA} 570e7b2ab refs/tags/v7.8.9", - push_all: ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'] - } + push_all: ['6f6d7e7ed 570e7b2ab refs/heads/master', '6f6d7e7ed 570e7b2ab refs/heads/feature'], + merge_into_protected_branch: "0b4bc9a #{merge_into_protected_branch} refs/heads/feature" } end - def self.permissions_matrix - { - master: { - push_new_branch: true, - push_master: true, - push_protected_branch: true, - push_remove_protected_branch: false, - push_tag: true, - push_new_tag: true, - push_all: true, - }, - - developer: { - push_new_branch: true, - push_master: true, - push_protected_branch: false, - push_remove_protected_branch: false, - push_tag: false, - push_new_tag: true, - push_all: false, - }, - - reporter: { - push_new_branch: false, - push_master: false, - push_protected_branch: false, - push_remove_protected_branch: false, - push_tag: false, - push_new_tag: false, - push_all: false, - }, - - guest: { - push_new_branch: false, - push_master: false, - push_protected_branch: false, - push_remove_protected_branch: false, - push_tag: false, - push_new_tag: false, - push_all: false, - } - } - end - - def self.updated_permissions_matrix - updated_permissions_matrix = permissions_matrix.dup - updated_permissions_matrix[:developer][:push_protected_branch] = true - updated_permissions_matrix[:developer][:push_all] = true - updated_permissions_matrix + def stub_git_hooks + # Running the `pre-receive` hook is expensive, and not necessary for this test. + allow_any_instance_of(GitHooksService).to receive(:execute).and_yield end - permissions_matrix.keys.each do |role| - describe "#{role} access" do - before { protect_feature_branch } - before { project.team << [user, role] } - - permissions_matrix[role].each do |action, allowed| - context action do - subject { access.push_access_check(changes[action]) } + def merge_into_protected_branch + @protected_branch_merge_commit ||= begin + stub_git_hooks + project.repository.add_branch(user, unprotected_branch, 'feature') + target_branch = project.repository.lookup('feature') + source_branch = project.repository.commit_file(user, FFaker::InternetSE.login_user_name, FFaker::HipsterIpsum.paragraph, FFaker::HipsterIpsum.sentence, unprotected_branch, false) + rugged = project.repository.rugged + author = { email: "email@example.com", time: Time.now, name: "Example Git User" } - it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey } - end - end + merge_index = rugged.merge_commits(target_branch, source_branch) + Rugged::Commit.create(rugged, author: author, committer: author, message: "commit message", parents: [target_branch, source_branch], tree: merge_index.write_tree(rugged)) end end - context "with enabled developers push to protected branches " do - updated_permissions_matrix.keys.each do |role| + def self.run_permission_checks(permissions_matrix) + permissions_matrix.keys.each do |role| describe "#{role} access" do - before { create(:protected_branch, name: 'feature', developers_can_push: true, project: project) } before { project.team << [user, role] } - updated_permissions_matrix[role].each do |action, allowed| + permissions_matrix[role].each do |action, allowed| context action do subject { access.push_access_check(changes[action]) } @@ -259,5 +148,97 @@ describe Gitlab::GitAccess, lib: true do end end end + + permissions_matrix = { + master: { + push_new_branch: true, + push_master: true, + push_protected_branch: true, + push_remove_protected_branch: false, + push_tag: true, + push_new_tag: true, + push_all: true, + merge_into_protected_branch: true + }, + + developer: { + push_new_branch: true, + push_master: true, + push_protected_branch: false, + push_remove_protected_branch: false, + push_tag: false, + push_new_tag: true, + push_all: false, + merge_into_protected_branch: false + }, + + reporter: { + push_new_branch: false, + push_master: false, + push_protected_branch: false, + push_remove_protected_branch: false, + push_tag: false, + push_new_tag: false, + push_all: false, + merge_into_protected_branch: false + }, + + guest: { + push_new_branch: false, + push_master: false, + push_protected_branch: false, + push_remove_protected_branch: false, + push_tag: false, + push_new_tag: false, + push_all: false, + merge_into_protected_branch: false + } + } + + [['feature', 'exact'], ['feat*', 'wildcard']].each do |protected_branch_name, protected_branch_type| + context do + before { create(:protected_branch, name: protected_branch_name, project: project) } + + run_permission_checks(permissions_matrix) + end + + context "when 'developers can push' is turned on for the #{protected_branch_type} protected branch" do + before { create(:protected_branch, name: protected_branch_name, developers_can_push: true, project: project) } + + run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true })) + end + + context "when 'developers can merge' is turned on for the #{protected_branch_type} protected branch" do + before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, project: project) } + + context "when a merge request exists for the given source/target branch" do + context "when the merge request is in progress" do + before do + create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', state: 'locked', in_progress_merge_commit_sha: merge_into_protected_branch) + end + + run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: true })) + end + + context "when the merge request is not in progress" do + before do + create(:merge_request, source_project: project, source_branch: unprotected_branch, target_branch: 'feature', in_progress_merge_commit_sha: nil) + end + + run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false })) + end + end + + context "when a merge request does not exist for the given source/target branch" do + run_permission_checks(permissions_matrix.deep_merge(developer: { merge_into_protected_branch: false })) + end + end + + context "when 'developers can merge' and 'developers can push' are turned on for the #{protected_branch_type} protected branch" do + before { create(:protected_branch, name: protected_branch_name, developers_can_merge: true, developers_can_push: true, project: project) } + + run_permission_checks(permissions_matrix.deep_merge(developer: { push_protected_branch: true, push_all: true, merge_into_protected_branch: true })) + end + end end end diff --git a/spec/lib/gitlab/github_import/client_spec.rb b/spec/lib/gitlab/github_import/client_spec.rb index 3b023a35446..613c47d55f1 100644 --- a/spec/lib/gitlab/github_import/client_spec.rb +++ b/spec/lib/gitlab/github_import/client_spec.rb @@ -61,4 +61,11 @@ describe Gitlab::GithubImport::Client, lib: true do expect(client.api.api_endpoint).to eq 'https://github.company.com/' end end + + it 'does not raise error when rate limit is disabled' do + stub_request(:get, /api.github.com/) + allow(client.api).to receive(:rate_limit!).and_raise(Octokit::NotFound) + + expect { client.issues }.not_to raise_error + end end diff --git a/spec/lib/gitlab/ldap/access_spec.rb b/spec/lib/gitlab/ldap/access_spec.rb index f5b66b8156f..acd5394382c 100644 --- a/spec/lib/gitlab/ldap/access_spec.rb +++ b/spec/lib/gitlab/ldap/access_spec.rb @@ -4,7 +4,7 @@ describe Gitlab::LDAP::Access, lib: true do let(:access) { Gitlab::LDAP::Access.new user } let(:user) { create(:omniauth_user) } - describe :allowed? do + describe '#allowed?' do subject { access.allowed? } context 'when the user cannot be found' do diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb index 03199a2523e..949f6e2b19a 100644 --- a/spec/lib/gitlab/ldap/user_spec.rb +++ b/spec/lib/gitlab/ldap/user_spec.rb @@ -25,7 +25,7 @@ describe Gitlab::LDAP::User, lib: true do OmniAuth::AuthHash.new(uid: 'my-uid', provider: 'ldapmain', info: info_upper_case) end - describe :changed? do + describe '#changed?' do it "marks existing ldap user as changed" do create(:omniauth_user, extern_uid: 'my-uid', provider: 'ldapmain') expect(ldap_user.changed?).to be_truthy diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb new file mode 100644 index 00000000000..aa9ec243498 --- /dev/null +++ b/spec/lib/gitlab/user_access_spec.rb @@ -0,0 +1,88 @@ +require 'spec_helper' + +describe Gitlab::UserAccess, lib: true do + let(:access) { Gitlab::UserAccess.new(user, project: project) } + let(:project) { create(:project) } + let(:user) { create(:user) } + + describe 'can_push_to_branch?' do + describe 'push to none protected branch' do + it 'returns true if user is a master' do + project.team << [user, :master] + expect(access.can_push_to_branch?('random_branch')).to be_truthy + end + + it 'returns true if user is a developer' do + project.team << [user, :developer] + expect(access.can_push_to_branch?('random_branch')).to be_truthy + end + + it 'returns false if user is a reporter' do + project.team << [user, :reporter] + expect(access.can_push_to_branch?('random_branch')).to be_falsey + end + end + + describe 'push to protected branch' do + let(:branch) { create :protected_branch, project: project } + + it 'returns true if user is a master' do + project.team << [user, :master] + expect(access.can_push_to_branch?(branch.name)).to be_truthy + end + + it 'returns false if user is a developer' do + project.team << [user, :developer] + expect(access.can_push_to_branch?(branch.name)).to be_falsey + end + + it 'returns false if user is a reporter' do + project.team << [user, :reporter] + expect(access.can_push_to_branch?(branch.name)).to be_falsey + end + end + + describe 'push to protected branch if allowed for developers' do + before do + @branch = create :protected_branch, project: project, developers_can_push: true + end + + it 'returns true if user is a master' do + project.team << [user, :master] + expect(access.can_push_to_branch?(@branch.name)).to be_truthy + end + + it 'returns true if user is a developer' do + project.team << [user, :developer] + expect(access.can_push_to_branch?(@branch.name)).to be_truthy + end + + it 'returns false if user is a reporter' do + project.team << [user, :reporter] + expect(access.can_push_to_branch?(@branch.name)).to be_falsey + end + end + + describe 'merge to protected branch if allowed for developers' do + before do + @branch = create :protected_branch, project: project, developers_can_merge: true + end + + it 'returns true if user is a master' do + project.team << [user, :master] + expect(access.can_merge_to_branch?(@branch.name)).to be_truthy + end + + it 'returns true if user is a developer' do + project.team << [user, :developer] + expect(access.can_merge_to_branch?(@branch.name)).to be_truthy + end + + it 'returns false if user is a reporter' do + project.team << [user, :reporter] + expect(access.can_merge_to_branch?(@branch.name)).to be_falsey + end + end + + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 34507cf5083..4e5481f9154 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -15,7 +15,7 @@ describe Ci::Pipeline, models: true do it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :short_sha } - describe :valid_commit_sha do + describe '#valid_commit_sha' do context 'commit.sha can not start with 00000000' do before do pipeline.sha = '0' * 40 @@ -26,7 +26,7 @@ describe Ci::Pipeline, models: true do end end - describe :short_sha do + describe '#short_sha' do subject { pipeline.short_sha } it 'has 8 items' do @@ -35,10 +35,10 @@ describe Ci::Pipeline, models: true do it { expect(pipeline.sha).to start_with(subject) } end - describe :create_next_builds do + describe '#create_next_builds' do end - describe :retried do + describe '#retried' do subject { pipeline.retried } before do @@ -51,7 +51,7 @@ describe Ci::Pipeline, models: true do end end - describe :create_builds do + describe '#create_builds' do let!(:pipeline) { FactoryGirl.create :ci_pipeline, project: project, ref: 'master', tag: false } def create_builds(trigger_request = nil) diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index 98f60087cf5..4e7833c3162 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -9,7 +9,7 @@ describe Ci::Variable, models: true do subject.value = secret_value end - describe :value do + describe '#value' do it 'stores the encrypted value' do expect(subject.encrypted_value).not_to be_nil end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 96397d7c8a9..05f22c7a9eb 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -24,14 +24,14 @@ describe CommitStatus, models: true do it { is_expected.to respond_to :running? } it { is_expected.to respond_to :pending? } - describe :author do + describe '#author' do subject { commit_status.author } before { commit_status.author = User.new } it { is_expected.to eq(commit_status.user) } end - describe :started? do + describe '#started?' do subject { commit_status.started? } context 'without started_at' do @@ -57,7 +57,7 @@ describe CommitStatus, models: true do end end - describe :active? do + describe '#active?' do subject { commit_status.active? } %w(pending running).each do |state| @@ -77,7 +77,7 @@ describe CommitStatus, models: true do end end - describe :complete? do + describe '#complete?' do subject { commit_status.complete? } %w(success failed canceled).each do |state| @@ -97,7 +97,7 @@ describe CommitStatus, models: true do end end - describe :duration do + describe '#duration' do subject { commit_status.duration } it { is_expected.to eq(120.0) } @@ -122,7 +122,7 @@ describe CommitStatus, models: true do end end - describe :latest do + describe '.latest' do subject { CommitStatus.latest.order(:id) } before do @@ -138,7 +138,7 @@ describe CommitStatus, models: true do end end - describe :running_or_pending do + describe '.running_or_pending' do subject { CommitStatus.running_or_pending.order(:id) } before do diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 0344dae8b5d..5e652660e2c 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -7,7 +7,7 @@ describe Mentionable do nil end - describe :references do + describe 'references' do let(:project) { create(:project) } it 'excludes JIRA references' do diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb index fa1a0d4e0c7..f94987dcaff 100644 --- a/spec/models/forked_project_link_spec.rb +++ b/spec/models/forked_project_link_spec.rb @@ -18,7 +18,7 @@ describe ForkedProjectLink, "add link on fork" do end end -describe :forked_from_project do +describe '#forked?' do let(:forked_project_link) { build(:forked_project_link) } let(:project_from) { create(:project) } let(:project_to) { create(:project, forked_project_link: forked_project_link) } diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb index c4e781dd1dc..615cfe3142b 100644 --- a/spec/models/generic_commit_status_spec.rb +++ b/spec/models/generic_commit_status_spec.rb @@ -4,33 +4,33 @@ describe GenericCommitStatus, models: true do let(:pipeline) { FactoryGirl.create :ci_pipeline } let(:generic_commit_status) { FactoryGirl.create :generic_commit_status, pipeline: pipeline } - describe :context do + describe '#context' do subject { generic_commit_status.context } before { generic_commit_status.context = 'my_context' } it { is_expected.to eq(generic_commit_status.name) } end - describe :tags do + describe '#tags' do subject { generic_commit_status.tags } it { is_expected.to eq([:external]) } end - describe :set_default_values do + describe 'set_default_values' do before do generic_commit_status.context = nil generic_commit_status.stage = nil generic_commit_status.save end - describe :context do + describe '#context' do subject { generic_commit_status.context } it { is_expected.not_to be_nil } end - describe :stage do + describe '#stage' do subject { generic_commit_status.stage } it { is_expected.not_to be_nil } diff --git a/spec/models/global_milestone_spec.rb b/spec/models/global_milestone_spec.rb index 197c99cd007..ae77ec5b348 100644 --- a/spec/models/global_milestone_spec.rb +++ b/spec/models/global_milestone_spec.rb @@ -14,7 +14,7 @@ describe GlobalMilestone, models: true do let(:milestone2_project2) { create(:milestone, title: "VD-123", project: project2) } let(:milestone2_project3) { create(:milestone, title: "VD-123", project: project3) } - describe :build_collection do + describe '.build_collection' do before do milestones = [ @@ -42,7 +42,7 @@ describe GlobalMilestone, models: true do end end - describe :initialize do + describe '#initialize' do before do milestones = [ @@ -63,7 +63,7 @@ describe GlobalMilestone, models: true do end end - describe :safe_title do + describe '#safe_title' do let(:milestone) { create(:milestone, title: "git / test", project: project1) } it 'should strip out slashes and spaces' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index a878ff1b227..266c46213a6 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -97,22 +97,22 @@ describe Group, models: true do end end - describe :users do + describe '#users' do it { expect(group.users).to eq(group.owners) } end - describe :human_name do + describe '#human_name' do it { expect(group.human_name).to eq(group.name) } end - describe :add_users do + describe '#add_user' do let(:user) { create(:user) } before { group.add_user(user, GroupMember::MASTER) } it { expect(group.group_members.masters.map(&:user)).to include(user) } end - describe :add_users do + describe '#add_users' do let(:user) { create(:user) } before { group.add_users([user.id], GroupMember::GUEST) } @@ -124,7 +124,7 @@ describe Group, models: true do end end - describe :avatar_type do + describe '#avatar_type' do let(:user) { create(:user) } before { group.add_user(user, GroupMember::MASTER) } diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb index 4c103462433..ba622dfb9be 100644 --- a/spec/models/members/project_member_spec.rb +++ b/spec/models/members/project_member_spec.rb @@ -101,7 +101,7 @@ describe ProjectMember, models: true do end end - describe :add_users_into_projects do + describe '.add_users_into_projects' do before do @project_1 = create :project @project_2 = create :project @@ -123,7 +123,7 @@ describe ProjectMember, models: true do it { expect(@project_2.users).to include(@user_2) } end - describe :truncate_teams do + describe '.truncate_teams' do before do @project_1 = create :project @project_2 = create :project diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 1e18c788b50..d661dc0e59a 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -70,7 +70,7 @@ describe Milestone, models: true do end end - describe :expired? do + describe '#expired?' do context "expired" do before do allow(milestone).to receive(:due_date).and_return(Date.today.prev_year) @@ -88,7 +88,7 @@ describe Milestone, models: true do end end - describe :percent_complete do + describe '#percent_complete' do before do allow(milestone).to receive_messages( closed_items_count: 3, @@ -111,11 +111,11 @@ describe Milestone, models: true do it { expect(milestone.is_empty?(user)).to be_falsey } end - describe :can_be_closed? do + describe '#can_be_closed?' do it { expect(milestone.can_be_closed?).to be_truthy } end - describe :total_items_count do + describe '#total_items_count' do before do create :closed_issue, milestone: milestone create :merge_request, milestone: milestone @@ -126,7 +126,7 @@ describe Milestone, models: true do end end - describe :can_be_closed? do + describe '#can_be_closed?' do before do milestone = create :milestone create :closed_issue, milestone: milestone diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 5f68cd2b066..a162da0208e 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -18,11 +18,11 @@ describe Namespace, models: true do it { is_expected.to respond_to(:to_param) } end - describe :to_param do + describe '#to_param' do it { expect(namespace.to_param).to eq(namespace.path) } end - describe :human_name do + describe '#human_name' do it { expect(namespace.human_name).to eq(namespace.owner_name) } end @@ -54,7 +54,7 @@ describe Namespace, models: true do end end - describe :move_dir do + describe '#move_dir' do before do @namespace = create :namespace @project = create :project, namespace: @namespace @@ -98,7 +98,7 @@ describe Namespace, models: true do end end - describe :find_by_path_or_name do + describe '.find_by_path_or_name' do before do @namespace = create(:namespace, name: 'WoW', path: 'woW') end diff --git a/spec/models/project_security_spec.rb b/spec/models/project_security_spec.rb index e12258c0874..2142c7c13ef 100644 --- a/spec/models/project_security_spec.rb +++ b/spec/models/project_security_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Project, models: true do - describe :authorization do + describe 'authorization' do before do @p1 = create(:project) diff --git a/spec/models/project_services/buildkite_service_spec.rb b/spec/models/project_services/buildkite_service_spec.rb index 60364df2015..0866e1532dd 100644 --- a/spec/models/project_services/buildkite_service_spec.rb +++ b/spec/models/project_services/buildkite_service_spec.rb @@ -57,7 +57,7 @@ describe BuildkiteService, models: true do ) end - describe :webhook_url do + describe '#webhook_url' do it 'returns the webhook url' do expect(@service.webhook_url).to eq( 'https://webhook.buildkite.com/deliver/secret-sauce-webhook-token' @@ -65,7 +65,7 @@ describe BuildkiteService, models: true do end end - describe :commit_status_path do + describe '#commit_status_path' do it 'returns the correct status page' do expect(@service.commit_status_path('2ab7834c')).to eq( 'https://gitlab.buildkite.com/status/secret-sauce-status-token.json?commit=2ab7834c' @@ -73,7 +73,7 @@ describe BuildkiteService, models: true do end end - describe :build_page do + describe '#build_page' do it 'returns the correct build page' do expect(@service.build_page('2ab7834c', nil)).to eq( 'https://buildkite.com/account-name/example-project/builds?commit=2ab7834c' diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d6bb8d0d54f..e842c58dd82 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -314,7 +314,7 @@ describe Project, models: true do end end - describe :update_merge_requests do + describe '#update_merge_requests' do let(:project) { create(:project) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:key) { create(:key, user_id: project.owner.id) } @@ -363,7 +363,7 @@ describe Project, models: true do end end - describe :to_param do + describe '#to_param' do context 'with namespace' do before do @group = create :group, name: 'gitlab' @@ -374,7 +374,7 @@ describe Project, models: true do end end - describe :repository do + describe '#repository' do let(:project) { create(:project) } it 'should return valid repo' do @@ -382,7 +382,7 @@ describe Project, models: true do end end - describe :default_issues_tracker? do + describe '#default_issues_tracker?' do let(:project) { create(:project) } let(:ext_project) { create(:redmine_project) } @@ -395,7 +395,7 @@ describe Project, models: true do end end - describe :external_issue_tracker do + describe '#external_issue_tracker' do let(:project) { create(:project) } let(:ext_project) { create(:redmine_project) } @@ -436,7 +436,7 @@ describe Project, models: true do end end - describe :cache_has_external_issue_tracker do + describe '#cache_has_external_issue_tracker' do let(:project) { create(:project) } it 'stores true if there is any external_issue_tracker' do @@ -458,7 +458,7 @@ describe Project, models: true do end end - describe :open_branches do + describe '#open_branches' do let(:project) { create(:project) } before do @@ -535,7 +535,7 @@ describe Project, models: true do end end - describe :avatar_type do + describe '#avatar_type' do let(:project) { create(:project) } it 'should be true if avatar is image' do @@ -549,7 +549,7 @@ describe Project, models: true do end end - describe :avatar_url do + describe '#avatar_url' do subject { project.avatar_url } let(:project) { create(:project) } @@ -586,7 +586,7 @@ describe Project, models: true do end end - describe :pipeline do + describe '#pipeline' do let(:project) { create :project } let(:pipeline) { create :ci_pipeline, project: project, ref: 'master' } @@ -606,7 +606,7 @@ describe Project, models: true do end end - describe :builds_enabled do + describe '#builds_enabled' do let(:project) { create :project } before { project.builds_enabled = true } @@ -708,7 +708,7 @@ describe Project, models: true do end end - describe :any_runners do + describe '#any_runners' do let(:project) { create(:empty_project, shared_runners_enabled: shared_runners_enabled) } let(:specific_runner) { create(:ci_runner) } let(:shared_runner) { create(:ci_runner, :shared) } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 24e49c8def3..e14cec589fe 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -4,19 +4,20 @@ describe Repository, models: true do include RepoHelpers TestBlob = Struct.new(:name) - let(:repository) { create(:project).repository } + let(:project) { create(:project) } + let(:repository) { project.repository } let(:user) { create(:user) } let(:commit_options) do author = repository.user_to_committer(user) { message: 'Test message', committer: author, author: author } end let(:merge_commit) do - source_sha = repository.find_branch('feature').target - merge_commit_sha = repository.merge(user, source_sha, 'master', commit_options) - repository.commit(merge_commit_sha) + merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project) + merge_commit_id = repository.merge(user, merge_request, commit_options) + repository.commit(merge_commit_id) end - describe :branch_names_contains do + describe '#branch_names_contains' do subject { repository.branch_names_contains(sample_commit.id) } it { is_expected.to include('master') } @@ -24,7 +25,7 @@ describe Repository, models: true do it { is_expected.not_to include('fix') } end - describe :tag_names_contains do + describe '#tag_names_contains' do subject { repository.tag_names_contains(sample_commit.id) } it { is_expected.to include('v1.1.0') } @@ -72,13 +73,13 @@ describe Repository, models: true do end end - describe :last_commit_for_path do + describe '#last_commit_for_path' do subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id } it { is_expected.to eq('c1acaa58bbcbc3eafe538cb8274ba387047b69f8') } end - describe :find_commits_by_message do + describe '#find_commits_by_message' do subject { repository.find_commits_by_message('submodule').map{ |k| k.id } } it { is_expected.to include('5937ac0a7beb003549fc5fd26fc247adbce4a52e') } @@ -87,7 +88,7 @@ describe Repository, models: true do it { is_expected.not_to include('913c66a37b4a45b9769037c55c2d238bd0942d2e') } end - describe :blob_at do + describe '#blob_at' do context 'blank sha' do subject { repository.blob_at(Gitlab::Git::BLANK_SHA, '.gitignore') } @@ -95,7 +96,7 @@ describe Repository, models: true do end end - describe :merged_to_root_ref? do + describe '#merged_to_root_ref?' do context 'merged branch' do subject { repository.merged_to_root_ref?('improve/awesome') } @@ -103,7 +104,7 @@ describe Repository, models: true do end end - describe :can_be_merged? do + describe '#can_be_merged?' do context 'mergeable branches' do subject { repository.can_be_merged?('0b4bc9a49b562e85de7cc9e834518ea6828729b9', 'master') } @@ -305,7 +306,7 @@ describe Repository, models: true do end end - describe :add_branch do + describe '#add_branch' do context 'when pre hooks were successful' do it 'should run without errors' do hook = double(trigger: [true, nil]) @@ -349,7 +350,7 @@ describe Repository, models: true do end end - describe :rm_branch do + describe '#rm_branch' do context 'when pre hooks were successful' do it 'should run without errors' do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) @@ -386,7 +387,7 @@ describe Repository, models: true do end end - describe :commit_with_hooks do + describe '#commit_with_hooks' do context 'when pre hooks were successful' do before do expect_any_instance_of(GitHooksService).to receive(:execute). diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index 96bbbec9ea1..67b3783d514 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -22,11 +22,11 @@ describe Service, models: true do @testable = @service.can_test? end - describe :can_test do + describe '#can_test?' do it { expect(@testable).to eq(true) } end - describe :test do + describe '#test' do let(:data) { 'test' } it 'test runs execute' do @@ -45,7 +45,7 @@ describe Service, models: true do @testable = @service.can_test? end - describe :can_test do + describe '#can_test?' do it { expect(@testable).to eq(true) } end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 3984b30ddf8..ff39f187759 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -427,7 +427,7 @@ describe User, models: true do end end - describe :not_in_project do + describe '.not_in_project' do before do User.delete_all @user = create :user @@ -598,7 +598,7 @@ describe User, models: true do end end - describe :avatar_type do + describe '#avatar_type' do let(:user) { create(:user) } it "should be true if avatar is image" do @@ -612,7 +612,7 @@ describe User, models: true do end end - describe :requires_ldap_check? do + describe '#requires_ldap_check?' do let(:user) { User.new } it 'is false when LDAP is disabled' do @@ -651,7 +651,7 @@ describe User, models: true do end context 'ldap synchronized user' do - describe :ldap_user? do + describe '#ldap_user?' do it 'is true if provider name starts with ldap' do user = create(:omniauth_user, provider: 'ldapmain') expect(user.ldap_user?).to be_truthy @@ -668,7 +668,7 @@ describe User, models: true do end end - describe :ldap_identity do + describe '#ldap_identity' do it 'returns ldap identity' do user = create :omniauth_user expect(user.ldap_identity.provider).not_to be_empty @@ -825,7 +825,7 @@ describe User, models: true do end end - describe :can_be_removed? do + describe '#can_be_removed?' do subject { create(:user) } context 'no owned groups' do diff --git a/spec/requests/api/api_helpers_spec.rb b/spec/requests/api/api_helpers_spec.rb index 83025953889..3d5c19aeff3 100644 --- a/spec/requests/api/api_helpers_spec.rb +++ b/spec/requests/api/api_helpers_spec.rb @@ -49,7 +49,7 @@ describe API::Helpers, api: true do it "should return nil for a user without access" do env[API::Helpers::PRIVATE_TOKEN_HEADER] = user.private_token - allow(Gitlab::UserAccess).to receive(:allowed?).and_return(false) + allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) expect(current_user).to be_nil end @@ -73,7 +73,7 @@ describe API::Helpers, api: true do it "should return nil for a user without access" do env[API::Helpers::PRIVATE_TOKEN_HEADER] = personal_access_token.token - allow(Gitlab::UserAccess).to receive(:allowed?).and_return(false) + allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false) expect(current_user).to be_nil end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4a1b5600bdf..651b91e9f68 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -138,6 +138,8 @@ describe API::API, api: true do expect(json_response['work_in_progress']).to be_falsy expect(json_response['merge_when_build_succeeds']).to be_falsy expect(json_response['merge_status']).to eq('can_be_merged') + expect(json_response['should_close_merge_request']).to be_falsy + expect(json_response['force_close_merge_request']).to be_falsy end it "should return merge_request" do @@ -147,6 +149,8 @@ describe API::API, api: true do expect(json_response['iid']).to eq(merge_request.iid) expect(json_response['work_in_progress']).to eq(false) expect(json_response['merge_status']).to eq('can_be_merged') + expect(json_response['should_close_merge_request']).to be_falsy + expect(json_response['force_close_merge_request']).to be_falsy end it 'should return merge_request by iid' do diff --git a/spec/services/ci/create_trigger_request_service_spec.rb b/spec/services/ci/create_trigger_request_service_spec.rb index ae4b7aca820..b72e0bd3dbe 100644 --- a/spec/services/ci/create_trigger_request_service_spec.rb +++ b/spec/services/ci/create_trigger_request_service_spec.rb @@ -9,7 +9,7 @@ describe Ci::CreateTriggerRequestService, services: true do stub_ci_pipeline_to_return_yaml_file end - describe :execute do + describe '#execute' do context 'valid params' do subject { service.execute(project, trigger, 'master') } diff --git a/spec/services/ci/image_for_build_service_spec.rb b/spec/services/ci/image_for_build_service_spec.rb index 476a888e394..3a3e3efe709 100644 --- a/spec/services/ci/image_for_build_service_spec.rb +++ b/spec/services/ci/image_for_build_service_spec.rb @@ -8,7 +8,7 @@ module Ci let(:commit) { project.ensure_pipeline(commit_sha, 'master') } let(:build) { FactoryGirl.create(:ci_build, pipeline: commit) } - describe :execute do + describe '#execute' do before { build } context 'branch name' do diff --git a/spec/services/ci/register_build_service_spec.rb b/spec/services/ci/register_build_service_spec.rb index f28f2f1438d..026d0ca6534 100644 --- a/spec/services/ci/register_build_service_spec.rb +++ b/spec/services/ci/register_build_service_spec.rb @@ -13,7 +13,7 @@ module Ci specific_runner.assign_to(project) end - describe :execute do + describe '#execute' do context 'runner follow tag list' do it "picks build with the same tag" do pending_build.tag_list = ["linux"] diff --git a/spec/services/create_commit_builds_service_spec.rb b/spec/services/create_commit_builds_service_spec.rb index 309213bd44c..4d09bc5fb12 100644 --- a/spec/services/create_commit_builds_service_spec.rb +++ b/spec/services/create_commit_builds_service_spec.rb @@ -9,7 +9,7 @@ describe CreateCommitBuildsService, services: true do stub_ci_pipeline_to_return_yaml_file end - describe :execute do + describe '#execute' do context 'valid params' do let(:pipeline) do service.execute(project, user, diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index f6dc9d4008f..789836f71bb 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -4,7 +4,7 @@ describe EventCreateService, services: true do let(:service) { EventCreateService.new } describe 'Issues' do - describe :open_issue do + describe '#open_issue' do let(:issue) { create(:issue) } it { expect(service.open_issue(issue, issue.author)).to be_truthy } @@ -14,7 +14,7 @@ describe EventCreateService, services: true do end end - describe :close_issue do + describe '#close_issue' do let(:issue) { create(:issue) } it { expect(service.close_issue(issue, issue.author)).to be_truthy } @@ -24,7 +24,7 @@ describe EventCreateService, services: true do end end - describe :reopen_issue do + describe '#reopen_issue' do let(:issue) { create(:issue) } it { expect(service.reopen_issue(issue, issue.author)).to be_truthy } @@ -36,7 +36,7 @@ describe EventCreateService, services: true do end describe 'Merge Requests' do - describe :open_mr do + describe '#open_mr' do let(:merge_request) { create(:merge_request) } it { expect(service.open_mr(merge_request, merge_request.author)).to be_truthy } @@ -46,7 +46,7 @@ describe EventCreateService, services: true do end end - describe :close_mr do + describe '#close_mr' do let(:merge_request) { create(:merge_request) } it { expect(service.close_mr(merge_request, merge_request.author)).to be_truthy } @@ -56,7 +56,7 @@ describe EventCreateService, services: true do end end - describe :merge_mr do + describe '#merge_mr' do let(:merge_request) { create(:merge_request) } it { expect(service.merge_mr(merge_request, merge_request.author)).to be_truthy } @@ -66,7 +66,7 @@ describe EventCreateService, services: true do end end - describe :reopen_mr do + describe '#reopen_mr' do let(:merge_request) { create(:merge_request) } it { expect(service.reopen_mr(merge_request, merge_request.author)).to be_truthy } @@ -80,7 +80,7 @@ describe EventCreateService, services: true do describe 'Milestone' do let(:user) { create :user } - describe :open_milestone do + describe '#open_milestone' do let(:milestone) { create(:milestone) } it { expect(service.open_milestone(milestone, user)).to be_truthy } @@ -90,7 +90,7 @@ describe EventCreateService, services: true do end end - describe :close_mr do + describe '#close_mr' do let(:milestone) { create(:milestone) } it { expect(service.close_milestone(milestone, user)).to be_truthy } @@ -100,7 +100,7 @@ describe EventCreateService, services: true do end end - describe :destroy_mr do + describe '#destroy_mr' do let(:milestone) { create(:milestone) } it { expect(service.destroy_milestone(milestone, user)).to be_truthy } diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index afabeed4a80..47c0580e0f0 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -224,7 +224,7 @@ describe GitPushService, services: true do it "when pushing a branch for the first time" do expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") - expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false }) + expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false, developers_can_merge: false }) execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) end @@ -242,7 +242,17 @@ describe GitPushService, services: true do expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") - expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: true }) + expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: true, developers_can_merge: false }) + + execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master') + end + + it "when pushing a branch for the first time with default branch protection set to 'developers can merge'" do + stub_application_setting(default_branch_protection: Gitlab::Access::PROTECTION_DEV_CAN_MERGE) + + expect(project).to receive(:execute_hooks) + expect(project.default_branch).to eq("master") + expect(project.protected_branches).to receive(:create).with({ name: "master", developers_can_push: false, developers_can_merge: true }) execute_service(project, user, @blankrev, 'newrev', 'refs/heads/master' ) end diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 62b25709a5d..67a919ba8ee 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -12,7 +12,7 @@ describe Issues::CloseService, services: true do project.team << [user2, :developer] end - describe :execute do + describe '#execute' do context "valid params" do before do perform_enqueued_jobs do diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 8443a00e70c..c1db4f3284b 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -12,7 +12,7 @@ describe MergeRequests::CloseService, services: true do project.team << [user2, :developer] end - describe :execute do + describe '#execute' do context 'valid params' do let(:service) { MergeRequests::CloseService.new(project, user, {}) } diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index e433f49872d..d0b55d2d509 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -5,7 +5,7 @@ describe MergeRequests::CreateService, services: true do let(:user) { create(:user) } let(:assignee) { create(:user) } - describe :execute do + describe '#execute' do context 'valid params' do let(:opts) do { diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 2f72cd60071..f5bf3c1e367 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -11,7 +11,7 @@ describe MergeRequests::MergeService, services: true do project.team << [user2, :developer] end - describe :execute do + describe '#execute' do context 'valid params' do let(:service) { MergeRequests::MergeService.new(project, user, commit_message: 'Awesome message') } diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 7d5cb876063..ce643b3f860 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -5,7 +5,7 @@ describe MergeRequests::RefreshService, services: true do let(:user) { create(:user) } let(:service) { MergeRequests::RefreshService } - describe :execute do + describe '#execute' do before do @user = create(:user) group = create(:group) @@ -88,8 +88,7 @@ describe MergeRequests::RefreshService, services: true do # Merge master -> feature branch author = { email: 'test@gitlab.com', time: Time.now, name: "Me" } commit_options = { message: 'Test message', committer: author, author: author } - master_commit = @project.repository.commit('master') - @project.repository.merge(@user, master_commit.id, 'feature', commit_options) + @project.repository.merge(@user, @merge_request, commit_options) commit = @project.repository.commit('feature') service.new(@project, @user).execute(@oldrev, commit.id, 'refs/heads/feature') reload_mrs diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index ac0221998f5..88c9c640514 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -11,7 +11,7 @@ describe MergeRequests::ReopenService, services: true do project.team << [user2, :developer] end - describe :execute do + describe '#execute' do context 'valid params' do let(:service) { MergeRequests::ReopenService.new(project, user, {}) } diff --git a/spec/services/milestones/close_service_spec.rb b/spec/services/milestones/close_service_spec.rb index 1cd6eb2ab38..5d400299be0 100644 --- a/spec/services/milestones/close_service_spec.rb +++ b/spec/services/milestones/close_service_spec.rb @@ -9,7 +9,7 @@ describe Milestones::CloseService, services: true do project.team << [user, :master] end - describe :execute do + describe '#execute' do before do Milestones::CloseService.new(project, user, {}).execute(milestone) end diff --git a/spec/services/milestones/create_service_spec.rb b/spec/services/milestones/create_service_spec.rb index c793026e300..6d29edb449a 100644 --- a/spec/services/milestones/create_service_spec.rb +++ b/spec/services/milestones/create_service_spec.rb @@ -4,7 +4,7 @@ describe Milestones::CreateService, services: true do let(:project) { create(:empty_project) } let(:user) { create(:user) } - describe :execute do + describe '#execute' do context "valid params" do before do project.team << [user, :master] diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 35f576874b8..32753e84b31 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -5,7 +5,7 @@ describe Notes::CreateService, services: true do let(:issue) { create(:issue, project: project) } let(:user) { create(:user) } - describe :execute do + describe '#execute' do context "valid params" do before do project.team << [user, :master] diff --git a/spec/services/notes/post_process_service_spec.rb b/spec/services/notes/post_process_service_spec.rb index d4c50f824c1..e33a611929b 100644 --- a/spec/services/notes/post_process_service_spec.rb +++ b/spec/services/notes/post_process_service_spec.rb @@ -5,7 +5,7 @@ describe Notes::PostProcessService, services: true do let(:issue) { create(:issue, project: project) } let(:user) { create(:user) } - describe :execute do + describe '#execute' do before do project.team << [user, :master] note_opts = { diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index d3dddfb4817..9fc93f325f7 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -50,7 +50,7 @@ describe NotificationService, services: true do update_custom_notification(:new_note, @u_custom_global) end - describe :new_note do + describe '#new_note' do it do add_users_with_subscription(note.project, issue) @@ -306,7 +306,7 @@ describe NotificationService, services: true do project.team << [merge_request.assignee, :master] end - describe :new_note do + describe '#new_note' do it "records sent notifications" do # Ensure create SentNotification by noteable = merge_request 6 times, not noteable = note expect(SentNotification).to receive(:record_note).with(note, any_args).exactly(4).times.and_call_original diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb index bd4dc6a0f79..ad0d58672b3 100644 --- a/spec/services/projects/housekeeping_service_spec.rb +++ b/spec/services/projects/housekeeping_service_spec.rb @@ -12,18 +12,28 @@ describe Projects::HousekeepingService do it 'enqueues a sidekiq job' do expect(subject).to receive(:try_obtain_lease).and_return(true) - expect(GitlabShellOneShotWorker).to receive(:perform_async).with(:gc, project.repository_storage_path, project.path_with_namespace) + expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id) subject.execute - expect(project.pushes_since_gc).to eq(0) + expect(project.reload.pushes_since_gc).to eq(0) end - it 'does not enqueue a job when no lease can be obtained' do - expect(subject).to receive(:try_obtain_lease).and_return(false) - expect(GitlabShellOneShotWorker).not_to receive(:perform_async) + context 'when no lease can be obtained' do + before(:each) do + expect(subject).to receive(:try_obtain_lease).and_return(false) + end - expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken) - expect(project.pushes_since_gc).to eq(0) + it 'does not enqueue a job' do + expect(GitGarbageCollectWorker).not_to receive(:perform_async) + + expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken) + end + + it 'does not reset pushes_since_gc' do + expect do + expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken) + end.not_to change { project.pushes_since_gc }.from(3) + end end end @@ -39,10 +49,24 @@ describe Projects::HousekeepingService do end describe 'increment!' do + let(:lease_key) { "project_housekeeping:increment!:#{project.id}" } + it 'increments the pushes_since_gc counter' do - expect(project.pushes_since_gc).to eq(0) - subject.increment! - expect(project.pushes_since_gc).to eq(1) + lease = double(:lease, try_obtain: true) + expect(Gitlab::ExclusiveLease).to receive(:new).with(lease_key, anything).and_return(lease) + + expect do + subject.increment! + end.to change { project.pushes_since_gc }.from(0).to(1) + end + + it 'does not increment when no lease can be obtained' do + lease = double(:lease, try_obtain: false) + expect(Gitlab::ExclusiveLease).to receive(:new).with(lease_key, anything).and_return(lease) + + expect do + subject.increment! + end.not_to change { project.pushes_since_gc } end end end diff --git a/spec/services/test_hook_service_spec.rb b/spec/services/test_hook_service_spec.rb index f034f251ba4..4f47e89b4b5 100644 --- a/spec/services/test_hook_service_spec.rb +++ b/spec/services/test_hook_service_spec.rb @@ -5,7 +5,7 @@ describe TestHookService, services: true do let(:project) { create :project } let(:hook) { create :project_hook, project: project } - describe :execute do + describe '#execute' do it "should execute successfully" do stub_request(:post, hook.url).to_return(status: 200) expect(TestHookService.new.execute(hook, user)).to be_truthy diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 606da1b7605..3638dcbb2d3 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -33,7 +33,6 @@ RSpec.configure do |config| config.include LoginHelpers, type: :request config.include StubConfiguration config.include EmailHelpers - config.include RelativeUrl, type: feature config.include TestEnv config.include ActiveJob::TestHelper config.include StubGitlabCalls diff --git a/spec/support/login_helpers.rb b/spec/support/login_helpers.rb index ffdf2bb0a8a..e5f76afbfc0 100644 --- a/spec/support/login_helpers.rb +++ b/spec/support/login_helpers.rb @@ -37,6 +37,40 @@ module LoginHelpers Thread.current[:current_user] = user end + def login_via(provider, user, uid) + mock_auth_hash(provider, uid, user.email) + visit new_user_session_path + click_link provider + end + + def mock_auth_hash(provider, uid, email) + # The mock_auth configuration allows you to set per-provider (or default) + # authentication hashes to return during integration testing. + OmniAuth.config.mock_auth[provider.to_sym] = OmniAuth::AuthHash.new({ + provider: provider, + uid: uid, + info: { + name: 'mockuser', + email: email, + image: 'mock_user_thumbnail_url' + }, + credentials: { + token: 'mock_token', + secret: 'mock_secret' + }, + extra: { + raw_info: { + info: { + name: 'mockuser', + email: email, + image: 'mock_user_thumbnail_url' + } + } + } + }) + Rails.application.env_config['omniauth.auth'] = OmniAuth.config.mock_auth[:saml] + end + # Requires Javascript driver. def logout find(".header-user-dropdown-toggle").click diff --git a/spec/support/omni_auth.rb b/spec/support/omni_auth.rb new file mode 100644 index 00000000000..0b1af4052ff --- /dev/null +++ b/spec/support/omni_auth.rb @@ -0,0 +1 @@ +OmniAuth.config.test_mode = true diff --git a/spec/support/relative_url.rb b/spec/support/relative_url.rb deleted file mode 100644 index 72e3ccce75b..00000000000 --- a/spec/support/relative_url.rb +++ /dev/null @@ -1,8 +0,0 @@ -# Fix route helpers in tests (e.g. root_path, ...) -module RelativeUrl - extend ActiveSupport::Concern - - included do - default_url_options[:script_name] = Rails.application.config.relative_url_root - end -end diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb new file mode 100644 index 00000000000..a9cce8b8b59 --- /dev/null +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe GitGarbageCollectWorker do + let(:project) { create(:project) } + let(:shell) { Gitlab::Shell.new } + + subject { GitGarbageCollectWorker.new } + + before do + allow(subject).to receive(:gitlab_shell).and_return(shell) + end + + describe "#perform" do + it "runs `git gc`" do + expect(shell).to receive(:gc).with( + project.repository_storage_path, + project.path_with_namespace). + and_return(true) + expect_any_instance_of(Repository).to receive(:after_create_branch) + + subject.perform(project.id) + end + end +end |
