diff options
author | Clement Ho <ClemMakesApps@gmail.com> | 2018-05-17 09:58:36 -0400 |
---|---|---|
committer | Clement Ho <ClemMakesApps@gmail.com> | 2018-05-17 09:58:36 -0400 |
commit | fd5fdb2c1aba95b8dc851e80cb7216551052122d (patch) | |
tree | 745d4a60a5f029644767cee9e8284b514fcb5a5a | |
parent | 42189e9185e59758deb93ea4ea625e2dc6f99524 (diff) | |
parent | ec7163ae1db54f4adce7942673abc8a4a073fbd6 (diff) | |
download | gitlab-ce-fd5fdb2c1aba95b8dc851e80cb7216551052122d.tar.gz |
Merge branch 'master' into bootstrap4
53 files changed, 613 insertions, 252 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index b1445feee58..84d8e69b84e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -189,7 +189,7 @@ stages: <<: *dedicated-no-docs-and-no-qa-pull-cache-job <<: *use-pg variables: - CREATE_DB_USER: "true" + SETUP_DB: "false" script: # Manually clone gitlab-test and only seed this project in # db/fixtures/development/04_project.rb thanks to SIZE=1 below @@ -233,7 +233,7 @@ stages: .migration-paths: &migration-paths <<: *dedicated-no-docs-and-no-qa-pull-cache-job variables: - CREATE_DB_USER: "true" + SETUP_DB: "false" script: - git fetch https://gitlab.com/gitlab-org/gitlab-ce.git v9.3.0 - git checkout -f FETCH_HEAD @@ -242,7 +242,7 @@ stages: - cp config/gitlab.yml.example config/gitlab.yml - bundle exec rake db:drop db:create db:schema:load db:seed_fu - date - - git checkout $CI_COMMIT_SHA + - git checkout -f $CI_COMMIT_SHA - bundle install $BUNDLE_INSTALL_FLAGS - date - . scripts/prepare_build.sh diff --git a/app/assets/javascripts/notes/components/note_header.vue b/app/assets/javascripts/notes/components/note_header.vue index 7183d0b50b2..a4081957207 100644 --- a/app/assets/javascripts/notes/components/note_header.vue +++ b/app/assets/javascripts/notes/components/note_header.vue @@ -93,10 +93,13 @@ export default { v-html="actionTextHtml" class="system-note-message"> </span> + <span class="system-note-separator"> + · + </span> <a :href="noteTimestampLink" @click="updateTargetNoteHash" - class="note-timestamp"> + class="note-timestamp system-note-separator"> <time-ago-tooltip :time="createdAt" tooltip-placement="bottom" diff --git a/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js b/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js index a99ce0f1c36..5316d3e9f3c 100644 --- a/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js +++ b/app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js @@ -1,4 +1,4 @@ -/* eslint-disable func-names, space-before-function-paren, no-var, prefer-rest-params, max-len, no-restricted-syntax, vars-on-top, no-use-before-define, no-param-reassign, new-cap, no-underscore-dangle, wrap-iife, comma-dangle, no-return-assign, prefer-arrow-callback, quotes, prefer-template, newline-per-chained-call, no-else-return, no-shadow */ +/* eslint-disable func-names, space-before-function-paren, prefer-rest-params, max-len, no-restricted-syntax, vars-on-top, no-use-before-define, no-param-reassign, new-cap, no-underscore-dangle, wrap-iife, comma-dangle, no-return-assign, prefer-arrow-callback, quotes, prefer-template, newline-per-chained-call, no-else-return, no-shadow */ import $ from 'jquery'; import _ from 'underscore'; @@ -13,17 +13,17 @@ import { dateTickFormat } from '~/lib/utils/tick_formats'; const d3 = { extent, max, select, scaleTime, scaleLinear, axisLeft, axisBottom, area, brushX, timeParse }; -const extend = function(child, parent) { for (var key in parent) { if (hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; }; const hasProp = {}.hasOwnProperty; +const extend = function(child, parent) { for (const key in parent) { if (hasProp.call(parent, key)) child[key] = parent[key]; } function ctor() { this.constructor = child; } ctor.prototype = parent.prototype; child.prototype = new ctor(); child.__super__ = parent.prototype; return child; }; export const ContributorsGraph = (function() { function ContributorsGraph() {} ContributorsGraph.prototype.MARGIN = { top: 20, - right: 20, + right: 10, bottom: 30, - left: 50 + left: 40 }; ContributorsGraph.prototype.x_domain = null; @@ -32,6 +32,12 @@ export const ContributorsGraph = (function() { ContributorsGraph.prototype.dates = []; + ContributorsGraph.prototype.determine_width = function(baseWidth, $parentElement) { + const parentPaddingWidth = parseFloat($parentElement.css('padding-left')) + parseFloat($parentElement.css('padding-right')); + const marginWidth = this.MARGIN.left + this.MARGIN.right; + return baseWidth - parentPaddingWidth - marginWidth; + }; + ContributorsGraph.set_x_domain = function(data) { return ContributorsGraph.prototype.x_domain = data; }; @@ -105,11 +111,10 @@ export const ContributorsMasterGraph = (function(superClass) { function ContributorsMasterGraph(data1) { const $parentElement = $('#contributors-master'); - const parentPadding = parseFloat($parentElement.css('padding-left')) + parseFloat($parentElement.css('padding-right')); this.data = data1; this.update_content = this.update_content.bind(this); - this.width = $('.content').width() - parentPadding - (this.MARGIN.left + this.MARGIN.right); + this.width = this.determine_width($('.js-graphs-show').width(), $parentElement); this.height = 200; this.x = null; this.y = null; @@ -122,8 +127,7 @@ export const ContributorsMasterGraph = (function(superClass) { } ContributorsMasterGraph.prototype.process_dates = function(data) { - var dates; - dates = this.get_dates(data); + const dates = this.get_dates(data); this.parse_dates(data); return ContributorsGraph.set_dates(dates); }; @@ -133,8 +137,7 @@ export const ContributorsMasterGraph = (function(superClass) { }; ContributorsMasterGraph.prototype.parse_dates = function(data) { - var parseDate; - parseDate = d3.timeParse("%Y-%m-%d"); + const parseDate = d3.timeParse("%Y-%m-%d"); return data.forEach(function(d) { return d.date = parseDate(d.date); }); @@ -152,7 +155,14 @@ export const ContributorsMasterGraph = (function(superClass) { }; ContributorsMasterGraph.prototype.create_svg = function() { - return this.svg = d3.select("#contributors-master").append("svg").attr("width", this.width + this.MARGIN.left + this.MARGIN.right).attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom).attr("class", "tint-box").append("g").attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")"); + this.svg = d3.select("#contributors-master") + .append("svg") + .attr("width", this.width + this.MARGIN.left + this.MARGIN.right) + .attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom) + .attr("class", "tint-box") + .append("g") + .attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")"); + return this.svg; }; ContributorsMasterGraph.prototype.create_area = function(x, y) { @@ -218,12 +228,14 @@ export const ContributorsAuthorGraph = (function(superClass) { extend(ContributorsAuthorGraph, superClass); function ContributorsAuthorGraph(data1) { + const $parentElements = $('.person'); + this.data = data1; // Don't split graph size in half for mobile devices. - if ($(window).width() < 768) { - this.width = $('.content').width() - 80; + if ($(window).width() < 790) { + this.width = this.determine_width($('.js-graphs-show').width(), $parentElements); } else { - this.width = ($('.content').width() / 2) - 100; + this.width = this.determine_width($('.js-graphs-show').width() / 2, $parentElements); } this.height = 200; this.x = null; @@ -249,8 +261,7 @@ export const ContributorsAuthorGraph = (function(superClass) { ContributorsAuthorGraph.prototype.create_area = function(x, y) { return this.area = d3.area().x(function(d) { - var parseDate; - parseDate = d3.timeParse("%Y-%m-%d"); + const parseDate = d3.timeParse("%Y-%m-%d"); return x(parseDate(d)); }).y0(this.height).y1((function(_this) { return function(d) { @@ -264,9 +275,16 @@ export const ContributorsAuthorGraph = (function(superClass) { }; ContributorsAuthorGraph.prototype.create_svg = function() { - var persons = document.querySelectorAll('.person'); + const persons = document.querySelectorAll('.person'); this.list_item = persons[persons.length - 1]; - return this.svg = d3.select(this.list_item).append("svg").attr("width", this.width + this.MARGIN.left + this.MARGIN.right).attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom).attr("class", "spark").append("g").attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")"); + this.svg = d3.select(this.list_item) + .append("svg") + .attr("width", this.width + this.MARGIN.left + this.MARGIN.right) + .attr("height", this.height + this.MARGIN.top + this.MARGIN.bottom) + .attr("class", "spark") + .append("g") + .attr("transform", "translate(" + this.MARGIN.left + "," + this.MARGIN.top + ")"); + return this.svg; }; ContributorsAuthorGraph.prototype.draw_path = function(data) { diff --git a/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue b/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue index bec4e7c99b6..368eeb6c453 100644 --- a/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue +++ b/app/assets/javascripts/vue_shared/components/time_ago_tooltip.vue @@ -40,7 +40,7 @@ export default { :class="cssClass" :title="tooltipTitle(time)" :data-placement="tooltipPlacement" - data-container="body"> - {{ timeFormated(time) }} + data-container="body" + v-text="timeFormated(time)"> </time> </template> diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index 135cc5de0a5..1c3d312f7ac 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -1,5 +1,3 @@ -@import './issues/issue_count_badge'; - [v-cloak] { display: none; } diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss index 86c3dec17ec..19fb99bfa93 100644 --- a/app/assets/stylesheets/pages/issues.scss +++ b/app/assets/stylesheets/pages/issues.scss @@ -1,5 +1,3 @@ -@import "./issues/issue_count_badge"; - .issues-list { .issue { padding: 10px 0 10px $gl-padding; diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index d865dbc828f..299eda53140 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -455,6 +455,10 @@ ul.notes { white-space: normal; } + .system-note-separator { + color: $gl-text-color-disabled; + } + a:hover { text-decoration: underline; } diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb index e85cdcb8db7..d6a6bc7d4a1 100644 --- a/app/controllers/admin/dashboard_controller.rb +++ b/app/controllers/admin/dashboard_controller.rb @@ -1,4 +1,6 @@ class Admin::DashboardController < Admin::ApplicationController + include CountHelper + def index @projects = Project.order_id_desc.without_deleted.with_route.limit(10) @users = User.order_id_desc.limit(10) diff --git a/app/controllers/concerns/accepts_pending_invitations.rb b/app/controllers/concerns/accepts_pending_invitations.rb new file mode 100644 index 00000000000..6e8aef52b52 --- /dev/null +++ b/app/controllers/concerns/accepts_pending_invitations.rb @@ -0,0 +1,15 @@ +module AcceptsPendingInvitations + extend ActiveSupport::Concern + + def accept_pending_invitations + return unless resource.active_for_authentication? + + clear_stored_location_for_resource if resource.accept_pending_invitations!.any? + end + + def clear_stored_location_for_resource + session_key = stored_location_key_for(resource) + + session.delete(session_key) + end +end diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index 6d9c38d9581..7bc46a6ccc0 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -1,4 +1,6 @@ class ConfirmationsController < Devise::ConfirmationsController + include AcceptsPendingInvitations + def almost_there flash[:notice] = nil render layout: "devise_empty" @@ -11,6 +13,8 @@ class ConfirmationsController < Devise::ConfirmationsController end def after_confirmation_path_for(resource_name, resource) + accept_pending_invitations + # incoming resource can either be a :user or an :email if signed_in?(:user) after_sign_in(resource) diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index b7f548e0e63..1d1184d46d1 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -23,8 +23,12 @@ class Projects::CommitController < Projects::ApplicationController respond_to do |format| format.html { render } - format.diff { render text: @commit.to_diff } - format.patch { render text: @commit.to_patch } + format.diff do + send_git_diff(@project.repository, @commit.diff_refs) + end + format.patch do + send_git_patch(@project.repository, @commit.diff_refs) + end end end diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index 177c8a54099..1d850baf012 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -69,7 +69,7 @@ module Projects @project_runners = @project.runners.ordered @assignable_runners = current_user - .ci_authorized_runners + .ci_owned_runners .assignable_for(project) .ordered .page(params[:page]).per(20) diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index 1848c806c41..f5a222b3a48 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -1,5 +1,6 @@ class RegistrationsController < Devise::RegistrationsController include Recaptcha::Verify + include AcceptsPendingInvitations before_action :whitelist_query_limiting, only: [:destroy] @@ -16,6 +17,7 @@ class RegistrationsController < Devise::RegistrationsController end if !Gitlab::Recaptcha.load_configurations! || verify_recaptcha + accept_pending_invitations super else flash[:alert] = 'There was an error with the reCAPTCHA. Please solve the reCAPTCHA again.' @@ -60,7 +62,7 @@ class RegistrationsController < Devise::RegistrationsController def after_sign_up_path_for(user) Gitlab::AppLogger.info("User Created: username=#{user.username} email=#{user.email} ip=#{request.remote_ip} confirmed:#{user.confirmed?}") - user.confirmed? ? dashboard_projects_path : users_almost_there_path + user.confirmed? ? stored_location_for(user) || dashboard_projects_path : users_almost_there_path end def after_inactive_sign_up_path_for(resource) diff --git a/app/helpers/count_helper.rb b/app/helpers/count_helper.rb new file mode 100644 index 00000000000..24ee62e68ba --- /dev/null +++ b/app/helpers/count_helper.rb @@ -0,0 +1,5 @@ +module CountHelper + def approximate_count_with_delimiters(model) + number_with_delimiter(Gitlab::Database::Count.approximate_count(model)) + end +end diff --git a/app/models/appearance.rb b/app/models/appearance.rb index fb66dd0b766..f8713138a93 100644 --- a/app/models/appearance.rb +++ b/app/models/appearance.rb @@ -2,6 +2,7 @@ class Appearance < ActiveRecord::Base include CacheMarkdownField include AfterCommitQueue include ObjectStorage::BackgroundMove + include WithUploads cache_markdown_field :description cache_markdown_field :new_project_guidelines @@ -14,8 +15,6 @@ class Appearance < ActiveRecord::Base mount_uploader :logo, AttachmentUploader mount_uploader :header_logo, AttachmentUploader - has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - CACHE_KEY = "current_appearance:#{Gitlab::VERSION}".freeze after_commit :flush_redis_cache diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index bda69f85a78..e6f1ed519be 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -52,7 +52,7 @@ module Ci # Without that, placeholders would miss one and couldn't match. where(locked: false) .where.not("ci_runners.id IN (#{project.runners.select(:id).to_sql})") - .specific + .project_type end validate :tag_constraints diff --git a/app/models/concerns/with_uploads.rb b/app/models/concerns/with_uploads.rb new file mode 100644 index 00000000000..e7cfffb775b --- /dev/null +++ b/app/models/concerns/with_uploads.rb @@ -0,0 +1,39 @@ +# Mounted uploaders are destroyed by carrierwave's after_commit +# hook. This hook fetches upload location (local vs remote) from +# Upload model. So it's neccessary to make sure that during that +# after_commit hook model's associated uploads are not deleted yet. +# IOW we can not use dependent: :destroy : +# has_many :uploads, as: :model, dependent: :destroy +# +# And because not-mounted uploads require presence of upload's +# object model when destroying them (FileUploader's `build_upload` method +# references `model` on delete), we can not use after_commit hook for these +# uploads. +# +# Instead FileUploads are destroyed in before_destroy hook and remaining uploads +# are destroyed by the carrierwave's after_commit hook. + +module WithUploads + extend ActiveSupport::Concern + + # Currently there is no simple way how to select only not-mounted + # uploads, it should be all FileUploaders so we select them by + # `uploader` class + FILE_UPLOADERS = %w(PersonalFileUploader NamespaceFileUploader FileUploader).freeze + + included do + has_many :uploads, as: :model + + before_destroy :destroy_file_uploads + end + + # mounted uploads are deleted in carrierwave's after_commit hook, + # but FileUploaders which are not mounted must be deleted explicitly and + # it can not be done in after_commit because FileUploader requires loads + # associated model on destroy (which is already deleted in after_commit) + def destroy_file_uploads + self.uploads.where(uploader: FILE_UPLOADERS).find_each do |upload| + upload.destroy + end + end +end diff --git a/app/models/group.rb b/app/models/group.rb index cefca316399..8fb77a7869d 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -10,6 +10,7 @@ class Group < Namespace include LoadedInGroupList include GroupDescendant include TokenAuthenticatable + include WithUploads has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent alias_method :members, :group_members @@ -30,8 +31,6 @@ class Group < Namespace has_many :variables, class_name: 'Ci::GroupVariable' has_many :custom_attributes, class_name: 'GroupCustomAttribute' - has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - has_many :boards has_many :badges, class_name: 'GroupBadge' diff --git a/app/models/project.rb b/app/models/project.rb index 534a0e630af..0975e64e995 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -23,6 +23,7 @@ class Project < ActiveRecord::Base include ::Gitlab::Utils::StrongMemoize include ChronicDurationAttribute include FastDestroyAll::Helpers + include WithUploads extend Gitlab::ConfigHelper @@ -301,8 +302,6 @@ class Project < ActiveRecord::Base inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } } validates :variables, variable_duplicates: { scope: :environment_scope } - has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - # Scopes scope :pending_delete, -> { where(pending_delete: true) } scope :without_deleted, -> { where(pending_delete: false) } diff --git a/app/models/user.rb b/app/models/user.rb index 173ab38e20c..8ef3c3ceff0 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -17,6 +17,7 @@ class User < ActiveRecord::Base include IgnorableColumn include BulkMemberAccessLoad include BlocksJsonSerialization + include WithUploads DEFAULT_NOTIFICATION_LEVEL = :participating @@ -137,7 +138,6 @@ class User < ActiveRecord::Base has_many :custom_attributes, class_name: 'UserCustomAttribute' has_many :callouts, class_name: 'UserCallout' - has_many :uploads, as: :model, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :term_agreements belongs_to :accepted_term, class_name: 'ApplicationSetting::Term' @@ -860,6 +860,16 @@ class User < ActiveRecord::Base confirmed? && !temp_oauth_email? end + def accept_pending_invitations! + pending_invitations.select do |member| + member.accept_invite!(self) + end + end + + def pending_invitations + Member.where(invite_email: verified_emails).invite + end + def all_emails all_emails = [] all_emails << email unless temp_oauth_email? @@ -999,12 +1009,19 @@ class User < ActiveRecord::Base !solo_owned_groups.present? end - def ci_authorized_runners - @ci_authorized_runners ||= begin - runner_ids = Ci::RunnerProject + def ci_owned_runners + @ci_owned_runners ||= begin + project_runner_ids = Ci::RunnerProject .where(project: authorized_projects(Gitlab::Access::MASTER)) .select(:runner_id) - Ci::Runner.specific.where(id: runner_ids) + + group_runner_ids = Ci::RunnerNamespace + .where(namespace_id: owned_or_masters_groups.select(:id)) + .select(:runner_id) + + union = Gitlab::SQL::Union.new([project_runner_ids, group_runner_ids]) + + Ci::Runner.specific.where("ci_runners.id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection end end @@ -1205,6 +1222,11 @@ class User < ActiveRecord::Base !terms_accepted? end + def owned_or_masters_groups + union = Gitlab::SQL::Union.new([owned_groups, masters_groups]) + Group.from("(#{union.to_sql}) namespaces") + end + protected # override, from Devise::Validatable diff --git a/app/policies/ci/runner_policy.rb b/app/policies/ci/runner_policy.rb index 7dff8470e23..895abe87d86 100644 --- a/app/policies/ci/runner_policy.rb +++ b/app/policies/ci/runner_policy.rb @@ -1,16 +1,19 @@ module Ci class RunnerPolicy < BasePolicy with_options scope: :subject, score: 0 - condition(:shared) { @subject.is_shared? } - - with_options scope: :subject, score: 0 condition(:locked, scope: :subject) { @subject.locked? } - condition(:authorized_runner) { @user.ci_authorized_runners.include?(@subject) } + condition(:owned_runner) { @user.ci_owned_runners.exists?(@subject.id) } rule { anonymous }.prevent_all - rule { admin | authorized_runner }.enable :assign_runner - rule { ~admin & shared }.prevent :assign_runner + + rule { admin | owned_runner }.policy do + enable :assign_runner + enable :read_runner + enable :update_runner + enable :delete_runner + end + rule { ~admin & locked }.prevent :assign_runner end end diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml index 78706492ba3..afb65f6438f 100644 --- a/app/views/admin/dashboard/index.html.haml +++ b/app/views/admin/dashboard/index.html.haml @@ -10,7 +10,7 @@ = link_to admin_projects_path do %h3.text-center Projects: - = number_with_delimiter(Project.cached_count) + = approximate_count_with_delimiters(Project) %hr = link_to('New project', new_project_path, class: "btn btn-new") .col-sm-4 @@ -19,7 +19,7 @@ = link_to admin_users_path do %h3.text-center Users: - = number_with_delimiter(User.count) + = approximate_count_with_delimiters(User) %hr = link_to 'New user', new_admin_user_path, class: "btn btn-new" .col-sm-4 @@ -28,7 +28,7 @@ = link_to admin_groups_path do %h3.text-center Groups: - = number_with_delimiter(Group.count) + = approximate_count_with_delimiters(Group) %hr = link_to 'New group', new_admin_group_path, class: "btn btn-new" .row @@ -39,31 +39,31 @@ %p Forks %span.light.float-right - = number_with_delimiter(ForkedProjectLink.count) + = approximate_count_with_delimiters(ForkedProjectLink) %p Issues %span.light.float-right - = number_with_delimiter(Issue.count) + = approximate_count_with_delimiters(Issue) %p Merge Requests %span.light.float-right - = number_with_delimiter(MergeRequest.count) + = approximate_count_with_delimiters(MergeRequest) %p Notes %span.light.float-right - = number_with_delimiter(Note.count) + = approximate_count_with_delimiters(Note) %p Snippets %span.light.float-right - = number_with_delimiter(Snippet.count) + = approximate_count_with_delimiters(Snippet) %p SSH Keys %span.light.float-right - = number_with_delimiter(Key.count) + = approximate_count_with_delimiters(Key) %p Milestones %span.light.float-right - = number_with_delimiter(Milestone.count) + = approximate_count_with_delimiters(Milestone) %p Active Users %span.light.float-right diff --git a/app/views/notify/member_invited_email.html.haml b/app/views/notify/member_invited_email.html.haml index b8b75da3f2f..6730172242b 100644 --- a/app/views/notify/member_invited_email.html.haml +++ b/app/views/notify/member_invited_email.html.haml @@ -4,7 +4,7 @@ by = link_to member.created_by.name, user_url(member.created_by) to join the - = link_to member_source.human_name, member_source.web_url + = link_to member_source.human_name, member_source.public? ? member_source.web_url : invite_url(@token) #{member_source.model_name.singular} as #{member.human_access}. %p diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index ed0167dbf42..ca6e3602f05 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -41,8 +41,9 @@ - if note.system %span.system-note-message = markdown_field(note, :note) - %a{ href: "##{dom_id(note)}" } - = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago') + %span.system-note-separator + · + %a.system-note-separator{ href: "##{dom_id(note)}" }= time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago') - unless note.system? .note-actions - if note.for_personal_snippet? diff --git a/changelogs/unreleased/22647-width-contributors-graphs.yml b/changelogs/unreleased/22647-width-contributors-graphs.yml new file mode 100644 index 00000000000..87be3a25d8a --- /dev/null +++ b/changelogs/unreleased/22647-width-contributors-graphs.yml @@ -0,0 +1,5 @@ +--- +title: Fix width of contributors graphs +merge_request: 18639 +author: Paul Vorbach +type: fixed diff --git a/changelogs/unreleased/42531-open-invite-404.yml b/changelogs/unreleased/42531-open-invite-404.yml new file mode 100644 index 00000000000..73729f4a929 --- /dev/null +++ b/changelogs/unreleased/42531-open-invite-404.yml @@ -0,0 +1,5 @@ +--- +title: Automatically accepts project/group invite by email after user signup +merge_request: 17634 +author: Jacopo Beschi @jacopo-beschi +type: changed diff --git a/changelogs/unreleased/jivl-add-dot-system-notes.yml b/changelogs/unreleased/jivl-add-dot-system-notes.yml new file mode 100644 index 00000000000..2246bab1464 --- /dev/null +++ b/changelogs/unreleased/jivl-add-dot-system-notes.yml @@ -0,0 +1,5 @@ +--- +title: Add dot to separate system notes content +merge_request: 18864 +author: +type: changed diff --git a/changelogs/unreleased/jprovazn-remote-upload-destroy.yml b/changelogs/unreleased/jprovazn-remote-upload-destroy.yml new file mode 100644 index 00000000000..22e55920fa3 --- /dev/null +++ b/changelogs/unreleased/jprovazn-remote-upload-destroy.yml @@ -0,0 +1,5 @@ +--- +title: Fix deletion of Object Store uploads +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/zj-workhorse-commit-patch-diff.yml b/changelogs/unreleased/zj-workhorse-commit-patch-diff.yml new file mode 100644 index 00000000000..bce68692d98 --- /dev/null +++ b/changelogs/unreleased/zj-workhorse-commit-patch-diff.yml @@ -0,0 +1,5 @@ +--- +title: Workhorse to send raw diff and patch for commits +merge_request: +author: +type: other diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 92e3d5cc10a..0d125cd7831 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -165,6 +165,7 @@ module API group = find_group!(params[:id]) authorize! :admin_group, group + Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/46285') destroy_conditionally!(group) do |group| ::Groups::DestroyService.new(group, current_user).execute end diff --git a/lib/api/runners.rb b/lib/api/runners.rb index 5f2a9567605..5cb96d467c0 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -14,7 +14,7 @@ module API use :pagination end get do - runners = filter_runners(current_user.ci_authorized_runners, params[:scope], without: %w(specific shared)) + runners = filter_runners(current_user.ci_owned_runners, params[:scope], without: %w(specific shared)) present paginate(runners), with: Entities::Runner end @@ -184,40 +184,35 @@ module API def authenticate_show_runner!(runner) return if runner.is_shared || current_user.admin? - forbidden!("No access granted") unless user_can_access_runner?(runner) + forbidden!("No access granted") unless can?(current_user, :read_runner, runner) end def authenticate_update_runner!(runner) return if current_user.admin? - forbidden!("Runner is shared") if runner.is_shared? - forbidden!("No access granted") unless user_can_access_runner?(runner) + forbidden!("No access granted") unless can?(current_user, :update_runner, runner) end def authenticate_delete_runner!(runner) return if current_user.admin? - forbidden!("Runner is shared") if runner.is_shared? forbidden!("Runner associated with more than one project") if runner.projects.count > 1 - forbidden!("No access granted") unless user_can_access_runner?(runner) + forbidden!("No access granted") unless can?(current_user, :delete_runner, runner) end def authenticate_enable_runner!(runner) - forbidden!("Runner is shared") if runner.is_shared? - forbidden!("Runner is locked") if runner.locked? + forbidden!("Runner is a group runner") if runner.group_type? + return if current_user.admin? - forbidden!("No access granted") unless user_can_access_runner?(runner) + forbidden!("Runner is locked") if runner.locked? + forbidden!("No access granted") unless can?(current_user, :assign_runner, runner) end def authenticate_list_runners_jobs!(runner) return if current_user.admin? - forbidden!("No access granted") unless user_can_access_runner?(runner) - end - - def user_can_access_runner?(runner) - current_user.ci_authorized_runners.exists?(runner.id) + forbidden!("No access granted") unless can?(current_user, :read_runner, runner) end end end diff --git a/lib/api/v3/groups.rb b/lib/api/v3/groups.rb index 2c52d21fa1c..3844fd4810d 100644 --- a/lib/api/v3/groups.rb +++ b/lib/api/v3/groups.rb @@ -131,6 +131,7 @@ module API delete ":id" do group = find_group!(params[:id]) authorize! :admin_group, group + Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/46285') present ::Groups::DestroyService.new(group, current_user).execute, with: Entities::GroupDetail, current_user: current_user end diff --git a/lib/api/v3/runners.rb b/lib/api/v3/runners.rb index c6d9957d452..8a5c46805bd 100644 --- a/lib/api/v3/runners.rb +++ b/lib/api/v3/runners.rb @@ -58,7 +58,7 @@ module API end def user_can_access_runner?(runner) - current_user.ci_authorized_runners.exists?(runner.id) + current_user.ci_owned_runners.exists?(runner.id) end end end diff --git a/lib/gitlab/database/count.rb b/lib/gitlab/database/count.rb new file mode 100644 index 00000000000..3374203960e --- /dev/null +++ b/lib/gitlab/database/count.rb @@ -0,0 +1,48 @@ +# For large tables, PostgreSQL can take a long time to count rows due to MVCC. +# We can optimize this by using the reltuples count as described in https://wiki.postgresql.org/wiki/Slow_Counting. +module Gitlab + module Database + module Count + CONNECTION_ERRORS = + if defined?(PG) + [ + ActionView::Template::Error, + ActiveRecord::StatementInvalid, + PG::Error + ].freeze + else + [ + ActionView::Template::Error, + ActiveRecord::StatementInvalid + ].freeze + end + + def self.approximate_count(model) + return model.count unless Gitlab::Database.postgresql? + + execute_estimate_if_updated_recently(model) || model.count + end + + def self.execute_estimate_if_updated_recently(model) + ActiveRecord::Base.connection.select_value(postgresql_estimate_query(model)).to_i if reltuples_updated_recently?(model) + rescue *CONNECTION_ERRORS + end + + def self.reltuples_updated_recently?(model) + time = "to_timestamp(#{1.hour.ago.to_i})" + query = <<~SQL + SELECT 1 FROM pg_stat_user_tables WHERE relname = '#{model.table_name}' AND + (last_vacuum > #{time} OR last_autovacuum > #{time} OR last_analyze > #{time} OR last_autoanalyze > #{time}) + SQL + + ActiveRecord::Base.connection.select_all(query).count > 0 + rescue *CONNECTION_ERRORS + false + end + + def self.postgresql_estimate_query(model) + "SELECT reltuples::bigint AS estimate FROM pg_class where relname = '#{model.table_name}'" + end + end + end +end diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index fabcd46c8e9..d79a4dbeee4 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -342,21 +342,6 @@ module Gitlab parent_ids.first end - # Shows the diff between the commit's parent and the commit. - # - # Cuts out the header and stats from #to_patch and returns only the diff. - # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/324 - def to_diff - Gitlab::GitalyClient.migrate(:commit_patch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled| - if is_enabled - @repository.gitaly_commit_client.patch(id) - else - rugged_diff_from_parent.patch - end - end - end - # Returns a diff object for the changes from this commit's first parent. # If there is no parent, then the diff is between this commit and an # empty repo. See Repository#diff for keys allowed in the +options+ @@ -432,16 +417,6 @@ module Gitlab Gitlab::Git::CommitStats.new(@repository, self) end - def to_patch(options = {}) - begin - rugged_commit.to_mbox(options) - rescue Rugged::InvalidError => ex - if ex.message =~ /commit \w+ is a merge commit/i - 'Patch format is not currently supported for merge commits.' - end - end - end - # Get ref names collection # # Ex. diff --git a/scripts/create_mysql_user.sh b/scripts/create_mysql_user.sh index 286b1325f1d..35f68c581f3 100644 --- a/scripts/create_mysql_user.sh +++ b/scripts/create_mysql_user.sh @@ -1,7 +1,6 @@ #!/bin/bash mysql --user=root --host=mysql <<EOF -CREATE DATABASE IF NOT EXISTS gitlabhq_test DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci; CREATE USER IF NOT EXISTS 'gitlab'@'%'; GRANT ALL PRIVILEGES ON gitlabhq_test.* TO 'gitlab'@'%'; FLUSH PRIVILEGES; diff --git a/scripts/create_postgres_user.sh b/scripts/create_postgres_user.sh index 8a744df3226..8a049bcd7fb 100644 --- a/scripts/create_postgres_user.sh +++ b/scripts/create_postgres_user.sh @@ -1,8 +1,6 @@ #!/bin/bash psql -h postgres -U postgres postgres <<EOF -DROP DATABASE IF EXISTS gitlabhq_test; -CREATE DATABASE gitlabhq_test; CREATE USER gitlab; -GRANT ALL PRIVILEGES ON DATABASE gitlabhq_test TO gitlab; +GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA public TO gitlab; EOF diff --git a/scripts/prepare_build.sh b/scripts/prepare_build.sh index 206d62dbc78..d8bcc9f8191 100644 --- a/scripts/prepare_build.sh +++ b/scripts/prepare_build.sh @@ -49,20 +49,8 @@ sed -i 's/localhost/redis/g' config/redis.queues.yml cp config/redis.shared_state.yml.example config/redis.shared_state.yml sed -i 's/localhost/redis/g' config/redis.shared_state.yml -# Some tasks (e.g. db:seed_fu) need to have a properly-configured database -# user but not necessarily a full schema loaded -if [ "$CREATE_DB_USER" != "false" ]; then - if [ "$GITLAB_DATABASE" = 'postgresql' ]; then - . scripts/create_postgres_user.sh - else - . scripts/create_mysql_user.sh - fi -fi - if [ "$SETUP_DB" != "false" ]; then - bundle exec rake db:drop db:create db:schema:load db:migrate - - if [ "$GITLAB_DATABASE" = "mysql" ]; then - bundle exec rake add_limits_mysql - fi + setup_db +elif getent hosts postgres || getent hosts mysql; then + setup_db_user_only fi diff --git a/scripts/utils.sh b/scripts/utils.sh index 6faa701f0ce..2d2ba115563 100644 --- a/scripts/utils.sh +++ b/scripts/utils.sh @@ -12,3 +12,21 @@ retry() { done return 1 } + +setup_db_user_only() { + if [ "$GITLAB_DATABASE" = "postgresql" ]; then + . scripts/create_postgres_user.sh + else + . scripts/create_mysql_user.sh + fi +} + +setup_db() { + setup_db_user_only + + bundle exec rake db:drop db:create db:schema:load db:migrate + + if [ "$GITLAB_DATABASE" = "mysql" ]; then + bundle exec rake add_limits_mysql + fi +} diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 694c64ae1ad..003fec8ac68 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -79,41 +79,18 @@ describe Projects::CommitController do end describe "as diff" do - include_examples "export as", :diff - let(:format) { :diff } + it "triggers workhorse to serve the request" do + go(id: commit.id, format: :diff) - it "should really only be a git diff" do - go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format) - - expect(response.body).to start_with("diff --git") - end - - it "is only be a git diff without whitespace changes" do - go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format, w: 1) - - expect(response.body).to start_with("diff --git") - - # without whitespace option, there are more than 2 diff_splits for other formats - diff_splits = assigns(:diffs).diff_files.first.diff.diff.split("\n") - expect(diff_splits.length).to be <= 2 + expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-diff:") end end describe "as patch" do - include_examples "export as", :patch - let(:format) { :patch } - let(:commit2) { project.commit('498214de67004b1da3d820901307bed2a68a8ef6') } - - it "is a git email patch" do - go(id: commit2.id, format: format) - - expect(response.body).to start_with("From #{commit2.id}") - end - it "contains a git diff" do - go(id: commit2.id, format: format) + go(id: commit.id, format: :patch) - expect(response.body).to match(/^diff --git/) + expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-format-patch:") end end diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb index a91c868cbaf..f1810763d2d 100644 --- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb @@ -19,11 +19,11 @@ describe Projects::Settings::CiCdController do end context 'with group runners' do - let(:group_runner) { create(:ci_runner) } + let(:group_runner) { create(:ci_runner, runner_type: :group_type) } let(:parent_group) { create(:group) } let(:group) { create(:group, runners: [group_runner], parent: parent_group) } let(:other_project) { create(:project, group: group) } - let!(:project_runner) { create(:ci_runner, projects: [other_project]) } + let!(:project_runner) { create(:ci_runner, projects: [other_project], runner_type: :project_type) } let!(:shared_runner) { create(:ci_runner, :shared) } it 'sets assignable project runners only' do @@ -31,7 +31,7 @@ describe Projects::Settings::CiCdController do get :show, namespace_id: project.namespace, project_id: project - expect(assigns(:assignable_runners)).to eq [project_runner] + expect(assigns(:assignable_runners)).to contain_exactly(project_runner) end end end diff --git a/spec/features/invites_spec.rb b/spec/features/invites_spec.rb index e4be6193b8b..a986ddc4abc 100644 --- a/spec/features/invites_spec.rb +++ b/spec/features/invites_spec.rb @@ -5,18 +5,41 @@ describe 'Invites' do let(:owner) { create(:user, name: 'John Doe') } let(:group) { create(:group, name: 'Owned') } let(:project) { create(:project, :repository, namespace: group) } - let(:invite) { group.group_members.invite.last } + let(:group_invite) { group.group_members.invite.last } before do project.add_master(owner) group.add_user(owner, Gitlab::Access::OWNER) group.add_developer('user@example.com', owner) - invite.generate_invite_token! + group_invite.generate_invite_token! + end + + def confirm_email_and_sign_in(new_user) + new_user_token = User.find_by_email(new_user.email).confirmation_token + + visit user_confirmation_path(confirmation_token: new_user_token) + fill_in_sign_in_form(new_user) + end + + def fill_in_sign_up_form(new_user) + fill_in 'new_user_name', with: new_user.name + fill_in 'new_user_username', with: new_user.username + fill_in 'new_user_email', with: new_user.email + fill_in 'new_user_email_confirmation', with: new_user.email + fill_in 'new_user_password', with: new_user.password + click_button "Register" + end + + def fill_in_sign_in_form(user) + fill_in 'user_login', with: user.email + fill_in 'user_password', with: user.password + check 'user_remember_me' + click_button 'Sign in' end context 'when signed out' do before do - visit invite_path(invite.raw_invite_token) + visit invite_path(group_invite.raw_invite_token) end it 'renders sign in page with sign in notice' do @@ -25,12 +48,9 @@ describe 'Invites' do end it 'sign in and redirects to invitation page' do - fill_in 'user_login', with: user.email - fill_in 'user_password', with: user.password - check 'user_remember_me' - click_button 'Sign in' + fill_in_sign_in_form(user) - expect(current_path).to eq(invite_path(invite.raw_invite_token)) + expect(current_path).to eq(invite_path(group_invite.raw_invite_token)) expect(page).to have_content( 'You have been invited by John Doe to join group Owned as Developer.' ) @@ -45,7 +65,7 @@ describe 'Invites' do end it 'shows message user already a member' do - visit invite_path(invite.raw_invite_token) + visit invite_path(group_invite.raw_invite_token) expect(page).to have_content('However, you are already a member of this group.') end end @@ -53,7 +73,7 @@ describe 'Invites' do describe 'accepting the invitation' do before do sign_in(user) - visit invite_path(invite.raw_invite_token) + visit invite_path(group_invite.raw_invite_token) end it 'grants access and redirects to group page' do @@ -69,7 +89,7 @@ describe 'Invites' do context 'when signed in' do before do sign_in(user) - visit invite_path(invite.raw_invite_token) + visit invite_path(group_invite.raw_invite_token) end it 'declines application and redirects to dashboard' do @@ -83,7 +103,7 @@ describe 'Invites' do context 'when signed out' do before do - visit decline_invite_path(invite.raw_invite_token) + visit decline_invite_path(group_invite.raw_invite_token) end it 'declines application and redirects to sign in page' do @@ -94,4 +114,72 @@ describe 'Invites' do end end end + + describe 'invite an user using their email address' do + let(:new_user) { build_stubbed(:user) } + let(:invite_email) { new_user.email } + let(:group_invite) { create(:group_member, :invited, group: group, invite_email: invite_email) } + let!(:project_invite) { create(:project_member, :invited, project: project, invite_email: invite_email) } + + before do + stub_application_setting(send_user_confirmation_email: send_email_confirmation) + visit invite_path(group_invite.raw_invite_token) + end + + context 'email confirmation disabled' do + let(:send_email_confirmation) { false } + + it 'signs up and redirects to the dashboard page with all the projects/groups invitations automatically accepted' do + fill_in_sign_up_form(new_user) + + expect(current_path).to eq(dashboard_projects_path) + expect(page).to have_content(project.full_name) + visit group_path(group) + expect(page).to have_content(group.full_name) + end + + context 'the user sign-up using a different email address' do + let(:invite_email) { build_stubbed(:user).email } + + it 'signs up and redirects to the invitation page' do + fill_in_sign_up_form(new_user) + + expect(current_path).to eq(invite_path(group_invite.raw_invite_token)) + end + end + end + + context 'email confirmation enabled' do + let(:send_email_confirmation) { true } + + it 'signs up and redirects to root page with all the project/groups invitation automatically accepted' do + fill_in_sign_up_form(new_user) + confirm_email_and_sign_in(new_user) + + expect(current_path).to eq(root_path) + expect(page).to have_content(project.full_name) + visit group_path(group) + expect(page).to have_content(group.full_name) + end + + it "doesn't accept invitations until the user confirm his email" do + fill_in_sign_up_form(new_user) + sign_in(owner) + + visit project_project_members_path(project) + expect(page).to have_content 'Invited' + end + + context 'the user sign-up using a different email address' do + let(:invite_email) { build_stubbed(:user).email } + + it 'signs up and redirects to the invitation page' do + fill_in_sign_up_form(new_user) + confirm_email_and_sign_in(new_user) + + expect(current_path).to eq(invite_path(group_invite.raw_invite_token)) + end + end + end + end end diff --git a/spec/lib/gitlab/database/count_spec.rb b/spec/lib/gitlab/database/count_spec.rb new file mode 100644 index 00000000000..9d9caaabe16 --- /dev/null +++ b/spec/lib/gitlab/database/count_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +describe Gitlab::Database::Count do + before do + create_list(:project, 3) + end + + describe '.execute_estimate_if_updated_recently', :postgresql do + context 'when reltuples have not been updated' do + before do + expect(described_class).to receive(:reltuples_updated_recently?).and_return(false) + end + + it 'returns nil' do + expect(described_class.execute_estimate_if_updated_recently(Project)).to be nil + end + end + + context 'when reltuples have been updated' do + before do + ActiveRecord::Base.connection.execute('ANALYZE projects') + end + + it 'calls postgresql_estimate_query' do + expect(described_class).to receive(:postgresql_estimate_query).with(Project).and_call_original + expect(described_class.execute_estimate_if_updated_recently(Project)).to eq(3) + end + end + end + + describe '.approximate_count' do + context 'when reltuples have not been updated' do + it 'counts all projects the normal way' do + allow(described_class).to receive(:reltuples_updated_recently?).and_return(false) + + expect(Project).to receive(:count).and_call_original + expect(described_class.approximate_count(Project)).to eq(3) + end + end + + context 'no permission' do + it 'falls back to standard query' do + allow(described_class).to receive(:reltuples_updated_recently?).and_raise(PG::InsufficientPrivilege) + + expect(Project).to receive(:count).and_call_original + expect(described_class.approximate_count(Project)).to eq(3) + end + end + + describe 'when reltuples have been updated', :postgresql do + before do + ActiveRecord::Base.connection.execute('ANALYZE projects') + end + + it 'counts all projects in the fast way' do + expect(described_class).to receive(:postgresql_estimate_query).with(Project).and_call_original + + expect(described_class.approximate_count(Project)).to eq(3) + end + end + end +end diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index a05feaac1ca..2e068584c2e 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -554,24 +554,10 @@ describe Gitlab::Git::Commit, seed_helper: true do it_should_behave_like '#stats' end - describe '#to_diff' do - subject { commit.to_diff } - - it { is_expected.not_to include "From #{SeedRepo::Commit::ID}" } - it { is_expected.to include 'diff --git a/files/ruby/popen.rb b/files/ruby/popen.rb'} - end - describe '#has_zero_stats?' do it { expect(commit.has_zero_stats?).to eq(false) } end - describe '#to_patch' do - subject { commit.to_patch } - - it { is_expected.to include "From #{SeedRepo::Commit::ID}" } - it { is_expected.to include 'diff --git a/files/ruby/popen.rb b/files/ruby/popen.rb'} - end - describe '#to_hash' do let(:hash) { commit.to_hash } subject { hash } diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 84ddbbbf2ee..8a52c151cc4 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -594,7 +594,7 @@ describe Notify do it 'contains all the useful information' do is_expected.to have_subject "Invitation to join the #{project.full_name} project" is_expected.to have_html_escaped_body_text project.full_name - is_expected.to have_body_text project.web_url + is_expected.to have_body_text project.full_name is_expected.to have_body_text project_member.human_access is_expected.to have_body_text project_member.invite_token end diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 56b5d616284..5489c17bd82 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -5,7 +5,7 @@ describe Appearance do it { is_expected.to be_valid } - it { is_expected.to have_many(:uploads).dependent(:destroy) } + it { is_expected.to have_many(:uploads) } describe '.current', :use_clean_rails_memory_store_caching do let!(:appearance) { create(:appearance) } @@ -41,4 +41,12 @@ describe Appearance do expect(new_row.valid?).to eq(false) end end + + context 'with uploads' do + it_behaves_like 'model with mounted uploader', false do + let(:model_object) { create(:appearance, :with_logo) } + let(:upload_attribute) { :logo } + let(:uploader_class) { AttachmentUploader } + end + end end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index e2b212f4f4c..0fbc934f669 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -626,62 +626,26 @@ describe Ci::Runner do end describe '.assignable_for' do - let(:runner) { create(:ci_runner) } + let!(:unlocked_project_runner) { create(:ci_runner, runner_type: :project_type, projects: [project]) } + let!(:locked_project_runner) { create(:ci_runner, runner_type: :project_type, locked: true, projects: [project]) } + let!(:group_runner) { create(:ci_runner, runner_type: :group_type) } + let!(:instance_runner) { create(:ci_runner, :shared) } let(:project) { create(:project) } let(:another_project) { create(:project) } - before do - project.runners << runner - end - - context 'with shared runners' do - before do - runner.update(is_shared: true) - end - - context 'does not give owned runner' do - subject { described_class.assignable_for(project) } - - it { is_expected.to be_empty } - end - - context 'does not give shared runner' do - subject { described_class.assignable_for(another_project) } - - it { is_expected.to be_empty } - end - end - - context 'with unlocked runner' do - context 'does not give owned runner' do - subject { described_class.assignable_for(project) } - - it { is_expected.to be_empty } - end + context 'with already assigned project' do + subject { described_class.assignable_for(project) } - context 'does give a specific runner' do - subject { described_class.assignable_for(another_project) } - - it { is_expected.to contain_exactly(runner) } - end + it { is_expected.to be_empty } end - context 'with locked runner' do - before do - runner.update(locked: true) - end - - context 'does not give owned runner' do - subject { described_class.assignable_for(project) } - - it { is_expected.to be_empty } - end - - context 'does not give a locked runner' do - subject { described_class.assignable_for(another_project) } + context 'with a different project' do + subject { described_class.assignable_for(another_project) } - it { is_expected.to be_empty } - end + it { is_expected.to include(unlocked_project_runner) } + it { is_expected.not_to include(group_runner) } + it { is_expected.not_to include(locked_project_runner) } + it { is_expected.not_to include(instance_runner) } end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index 4e6b037a720..448b813c2e1 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -182,7 +182,6 @@ eos it { is_expected.to respond_to(:date) } it { is_expected.to respond_to(:diffs) } it { is_expected.to respond_to(:id) } - it { is_expected.to respond_to(:to_patch) } end describe '#closes_issues' do diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 0907d28d33b..f83b52e8975 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -15,7 +15,7 @@ describe Group do it { is_expected.to have_many(:notification_settings).dependent(:destroy) } it { is_expected.to have_many(:labels).class_name('GroupLabel') } it { is_expected.to have_many(:variables).class_name('Ci::GroupVariable') } - it { is_expected.to have_many(:uploads).dependent(:destroy) } + it { is_expected.to have_many(:uploads) } it { is_expected.to have_one(:chat_team) } it { is_expected.to have_many(:custom_attributes).class_name('GroupCustomAttribute') } it { is_expected.to have_many(:badges).class_name('GroupBadge') } @@ -691,4 +691,12 @@ describe Group do end end end + + context 'with uploads' do + it_behaves_like 'model with mounted uploader', true do + let(:model_object) { create(:group, :with_avatar) } + let(:upload_attribute) { :avatar } + let(:uploader_class) { AttachmentUploader } + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 5b452f17979..39625b559eb 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -76,7 +76,7 @@ describe Project do it { is_expected.to have_many(:project_group_links) } it { is_expected.to have_many(:notification_settings).dependent(:delete_all) } it { is_expected.to have_many(:forks).through(:forked_project_links) } - it { is_expected.to have_many(:uploads).dependent(:destroy) } + it { is_expected.to have_many(:uploads) } it { is_expected.to have_many(:pipeline_schedules) } it { is_expected.to have_many(:members_and_requesters) } it { is_expected.to have_many(:clusters) } @@ -3739,4 +3739,12 @@ describe Project do it { is_expected.to be_nil } end end + + context 'with uploads' do + it_behaves_like 'model with mounted uploader', true do + let(:model_object) { create(:project, :with_avatar) } + let(:upload_attribute) { :avatar } + let(:uploader_class) { AttachmentUploader } + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index bb5308221f0..684fa030baf 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -39,7 +39,7 @@ describe User do it { is_expected.to have_many(:builds).dependent(:nullify) } it { is_expected.to have_many(:pipelines).dependent(:nullify) } it { is_expected.to have_many(:chat_names).dependent(:destroy) } - it { is_expected.to have_many(:uploads).dependent(:destroy) } + it { is_expected.to have_many(:uploads) } it { is_expected.to have_many(:reported_abuse_reports).dependent(:destroy).class_name('AbuseReport') } it { is_expected.to have_many(:custom_attributes).class_name('UserCustomAttribute') } @@ -1223,6 +1223,24 @@ describe User do end end + describe '#accept_pending_invitations!' do + let(:user) { create(:user, email: 'user@email.com') } + let!(:project_member_invite) { create(:project_member, :invited, invite_email: user.email) } + let!(:group_member_invite) { create(:group_member, :invited, invite_email: user.email) } + let!(:external_project_member_invite) { create(:project_member, :invited, invite_email: 'external@email.com') } + let!(:external_group_member_invite) { create(:group_member, :invited, invite_email: 'external@email.com') } + + it 'accepts all the user members pending invitations and returns the accepted_members' do + accepted_members = user.accept_pending_invitations! + + expect(accepted_members).to match_array([project_member_invite, group_member_invite]) + expect(group_member_invite.reload).not_to be_invite + expect(project_member_invite.reload).not_to be_invite + expect(external_project_member_invite.reload).to be_invite + expect(external_group_member_invite.reload).to be_invite + end + end + describe '#all_emails' do let(:user) { create(:user) } @@ -1786,28 +1804,54 @@ describe User do end end - describe '#ci_authorized_runners' do + describe '#ci_owned_runners' do let(:user) { create(:user) } - let(:runner) { create(:ci_runner) } + let(:runner_1) { create(:ci_runner) } + let(:runner_2) { create(:ci_runner) } - before do - project.runners << runner - end - - context 'without any projects' do - let(:project) { create(:project) } + context 'without any projects nor groups' do + let!(:project) { create(:project, runners: [runner_1]) } + let!(:group) { create(:group) } it 'does not load' do - expect(user.ci_authorized_runners).to be_empty + expect(user.ci_owned_runners).to be_empty end end context 'with personal projects runners' do let(:namespace) { create(:namespace, owner: user) } - let(:project) { create(:project, namespace: namespace) } + let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } it 'loads' do - expect(user.ci_authorized_runners).to contain_exactly(runner) + expect(user.ci_owned_runners).to contain_exactly(runner_1) + end + end + + context 'with personal group runner' do + let!(:project) { create(:project, runners: [runner_1]) } + let!(:group) do + create(:group, runners: [runner_2]).tap do |group| + group.add_owner(user) + end + end + + it 'loads' do + expect(user.ci_owned_runners).to contain_exactly(runner_2) + end + end + + context 'with personal project and group runner' do + let(:namespace) { create(:namespace, owner: user) } + let!(:project) { create(:project, namespace: namespace, runners: [runner_1]) } + + let!(:group) do + create(:group, runners: [runner_2]).tap do |group| + group.add_owner(user) + end + end + + it 'loads' do + expect(user.ci_owned_runners).to contain_exactly(runner_1, runner_2) end end @@ -1818,7 +1862,7 @@ describe User do end it 'loads' do - expect(user.ci_authorized_runners).to contain_exactly(runner) + expect(user.ci_owned_runners).to contain_exactly(runner_1) end end @@ -1828,14 +1872,28 @@ describe User do end it 'does not load' do - expect(user.ci_authorized_runners).to be_empty + expect(user.ci_owned_runners).to be_empty end end end context 'with groups projects runners' do let(:group) { create(:group) } - let(:project) { create(:project, group: group) } + let!(:project) { create(:project, group: group, runners: [runner_1]) } + + def add_user(access) + group.add_user(user, access) + end + + it_behaves_like :member + end + + context 'with groups runners' do + let!(:group) do + create(:group, runners: [runner_1]).tap do |group| + group.add_owner(user) + end + end def add_user(access) group.add_user(user, access) @@ -1845,7 +1903,7 @@ describe User do end context 'with other projects runners' do - let(:project) { create(:project) } + let!(:project) { create(:project, runners: [runner_1]) } def add_user(access) project.add_role(user, access) @@ -1858,7 +1916,7 @@ describe User do let(:group) { create(:group) } let(:another_user) { create(:user) } let(:subgroup) { create(:group, parent: group) } - let(:project) { create(:project, group: subgroup) } + let!(:project) { create(:project, group: subgroup, runners: [runner_1]) } def add_user(access) group.add_user(user, access) @@ -2769,4 +2827,12 @@ describe User do expect { user.increment_failed_attempts! }.not_to change(user, :failed_attempts) end end + + context 'with uploads' do + it_behaves_like 'model with mounted uploader', false do + let(:model_object) { create(:user, :with_avatar) } + let(:upload_attribute) { :avatar } + let(:uploader_class) { AttachmentUploader } + end + end end diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index 981ac768e3a..c7587c877fc 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -27,7 +27,7 @@ describe API::Runners do end end - let!(:group_runner) { create(:ci_runner, description: 'Group runner', groups: [group]) } + let!(:group_runner) { create(:ci_runner, description: 'Group runner', groups: [group], runner_type: :group_type) } before do # Set project access for users @@ -48,7 +48,7 @@ describe API::Runners do expect(json_response).to be_an Array expect(json_response[0]).to have_key('ip_address') expect(descriptions).to contain_exactly( - 'Project runner', 'Two projects runner' + 'Project runner', 'Two projects runner', 'Group runner' ) expect(shared).to be_falsey end @@ -592,6 +592,15 @@ describe API::Runners do end.to change { project.runners.count }.by(+1) expect(response).to have_gitlab_http_status(201) end + + it 'enables a shared runner' do + expect do + post api("/projects/#{project.id}/runners", admin), runner_id: shared_runner.id + end.to change { project.runners.count }.by(1) + + expect(shared_runner.reload).not_to be_shared + expect(response).to have_gitlab_http_status(201) + end end context 'user is not admin' do diff --git a/spec/support/shared_examples/models/with_uploads_shared_examples.rb b/spec/support/shared_examples/models/with_uploads_shared_examples.rb new file mode 100644 index 00000000000..47ad0c6345d --- /dev/null +++ b/spec/support/shared_examples/models/with_uploads_shared_examples.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +shared_examples_for 'model with mounted uploader' do |supports_fileuploads| + describe '.destroy' do + before do + stub_uploads_object_storage(uploader_class) + + model_object.public_send(upload_attribute).migrate!(ObjectStorage::Store::REMOTE) + end + + it 'deletes remote uploads' do + expect_any_instance_of(CarrierWave::Storage::Fog::File).to receive(:delete).and_call_original + + expect { model_object.destroy }.to change { Upload.count }.by(-1) + end + + it 'deletes any FileUploader uploads which are not mounted', skip: !supports_fileuploads do + create(:upload, uploader: FileUploader, model: model_object) + + expect { model_object.destroy }.to change { Upload.count }.by(-2) + end + end +end |