diff options
83 files changed, 838 insertions, 246 deletions
diff --git a/PROCESS.md b/PROCESS.md index 99af3be7f14..c24210341e0 100644 --- a/PROCESS.md +++ b/PROCESS.md @@ -71,11 +71,15 @@ star, smile, etc.). Some good tips about code reviews can be found in our ## Feature freeze on the 7th for the release on the 22nd -After the 7th (Pacific Standard Time Zone) of each month, RC1 of the upcoming release (to be shipped on the 22nd) is created and deployed to GitLab.com and the stable branch for this release is frozen, which means master is no longer merged into it. +After 7th at 23:59 (Pacific Standard Time Zone) of each month, RC1 of the upcoming release (to be shipped on the 22nd) is created and deployed to GitLab.com and the stable branch for this release is frozen, which means master is no longer merged into it. Merge requests may still be merged into master during this period, but they will go into the _next_ release, unless they are manually cherry-picked into the stable branch. + By freezing the stable branches 2 weeks prior to a release, we reduce the risk of a last minute merge request potentially breaking things. +Any release candidate that gets created after this date can become a final release, +hence the name release candidate. + ### Between the 1st and the 7th These types of merge requests for the upcoming release need special consideration: @@ -193,11 +197,10 @@ to be backported down to the `9.5` release, you will need to assign it the ### Asking for an exception If you think a merge request should go into an RC or patch even though it does not meet these requirements, -you can ask for an exception to be made. Exceptions require sign-off from 3 people besides the developer: +you can ask for an exception to be made. -1. a Release Manager -2. an Engineering Lead -3. an Engineering Director, the VP of Engineering, or the CTO +Go to [Release tasks issue tracker](https://gitlab.com/gitlab-org/release/tasks/issues/new) and create an issue +using the `Exception-request` issue template. You can find who is who on the [team page](https://about.gitlab.com/team/). diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index 98831f5be4a..83245aadf6e 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -21,7 +21,7 @@ class IssuesFinder < IssuableFinder CONFIDENTIAL_ACCESS_LEVEL = Gitlab::Access::REPORTER def klass - Issue + Issue.includes(:author) end def with_confidentiality_access_check diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index 12157818bcd..33ee1e975b9 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -57,7 +57,7 @@ class NotesFinder types = %w(commit issue merge_request snippet) note_relations = types.map { |t| notes_for_type(t) } note_relations.map! { |notes| search(notes) } - UnionFinder.new.find_union(note_relations, Note) + UnionFinder.new.find_union(note_relations, Note.includes(:author)) end def noteables_for_type(noteable_type) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6530327698b..a6011eb9f30 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -68,18 +68,32 @@ module ApplicationHelper end end - def avatar_icon(user_or_email = nil, size = nil, scale = 2, only_path: true) - user = - if user_or_email.is_a?(User) - user_or_email - else - User.find_by_any_email(user_or_email.try(:downcase)) - end + # Takes both user and email and returns the avatar_icon by + # user (preferred) or email. + def avatar_icon_for(user = nil, email = nil, size = nil, scale = 2, only_path: true) + if user + avatar_icon_for_user(user, size, scale, only_path: only_path) + elsif email + avatar_icon_for_email(email, size, scale, only_path: only_path) + else + default_avatar + end + end + + def avatar_icon_for_email(email = nil, size = nil, scale = 2, only_path: true) + user = User.find_by_any_email(email.try(:downcase)) + if user + avatar_icon_for_user(user, size, scale, only_path: only_path) + else + gravatar_icon(email, size, scale) + end + end + def avatar_icon_for_user(user = nil, size = nil, scale = 2, only_path: true) if user user.avatar_url(size: size, only_path: only_path) || default_avatar else - gravatar_icon(user_or_email, size, scale) + gravatar_icon(nil, size, scale) end end diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb index be11d453898..21b6c0a8ad5 100644 --- a/app/helpers/avatars_helper.rb +++ b/app/helpers/avatars_helper.rb @@ -8,10 +8,22 @@ module AvatarsHelper })) end + def user_avatar_url_for(options = {}) + if options[:url] + options[:url] + elsif options[:user] + avatar_icon_for_user(options[:user], options[:size]) + else + avatar_icon_for_email(options[:user_email], options[:size]) + end + end + def user_avatar_without_link(options = {}) avatar_size = options[:size] || 16 user_name = options[:user].try(:name) || options[:user_name] - avatar_url = options[:url] || avatar_icon(options[:user] || options[:user_email], avatar_size) + + avatar_url = user_avatar_url_for(options.merge(size: avatar_size)) + has_tooltip = options[:has_tooltip].nil? ? true : options[:has_tooltip] data_attributes = options[:data] || {} css_class = %W[avatar s#{avatar_size}].push(*options[:css_class]) diff --git a/app/helpers/namespaces_helper.rb b/app/helpers/namespaces_helper.rb index b78d3072186..40ca666f1bf 100644 --- a/app/helpers/namespaces_helper.rb +++ b/app/helpers/namespaces_helper.rb @@ -33,7 +33,7 @@ module NamespacesHelper if namespace.is_a?(Group) group_icon(namespace) else - avatar_icon(namespace.owner.email, size) + avatar_icon_for_user(namespace.owner, size) end end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 43c8b1b612b..b97b72d62c3 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -19,7 +19,7 @@ module ProjectsHelper classes = %W[avatar avatar-inline s#{opts[:size]}] classes << opts[:avatar_class] if opts[:avatar_class] - avatar = avatar_icon(author, opts[:size]) + avatar = avatar_icon_for_user(author, opts[:size]) src = opts[:lazy_load] ? nil : avatar image_tag(src, width: opts[:size], class: classes, alt: '', "data-src" => avatar) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index ee987949080..490edf4ac57 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -41,12 +41,41 @@ module Ci scope :unstarted, ->() { where(runner_id: nil) } scope :ignore_failures, ->() { where(allow_failure: false) } + + # This convoluted mess is because we need to handle two cases of + # artifact files during the migration. And a simple OR clause + # makes it impossible to optimize. + + # Instead we want to use UNION ALL and do two carefully + # constructed disjoint queries. But Rails cannot handle UNION or + # UNION ALL queries so we do the query in a subquery and wrap it + # in an otherwise redundant WHERE IN query (IN is fine for + # non-null columns). + + # This should all be ripped out when the migration is finished and + # replaced with just the new storage to avoid the extra work. + scope :with_artifacts, ->() do - where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)', - '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id')) + old = Ci::Build.select(:id).where(%q[artifacts_file <> '']) + new = Ci::Build.select(:id).where(%q[(artifacts_file IS NULL OR artifacts_file = '') AND EXISTS (?)], + Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id')) + where('ci_builds.id IN (? UNION ALL ?)', old, new) end - scope :with_artifacts_not_expired, ->() { with_artifacts.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) } - scope :with_expired_artifacts, ->() { with_artifacts.where('artifacts_expire_at < ?', Time.now) } + + scope :with_artifacts_not_expired, ->() do + old = Ci::Build.select(:id).where(%q[artifacts_file <> '' AND (artifacts_expire_at IS NULL OR artifacts_expire_at > ?)], Time.now) + new = Ci::Build.select(:id).where(%q[(artifacts_file IS NULL OR artifacts_file = '') AND EXISTS (?)], + Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id AND (expire_at IS NULL OR expire_at > ?)', Time.now)) + where('ci_builds.id IN (? UNION ALL ?)', old, new) + end + + scope :with_expired_artifacts, ->() do + old = Ci::Build.select(:id).where(%q[artifacts_file <> '' AND artifacts_expire_at < ?], Time.now) + new = Ci::Build.select(:id).where(%q[(artifacts_file IS NULL OR artifacts_file = '') AND EXISTS (?)], + Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id AND expire_at < ?', Time.now)) + where('ci_builds.id IN (? UNION ALL ?)', old, new) + end + scope :last_month, ->() { where('created_at > ?', Date.today - 1.month) } scope :manual_actions, ->() { where(when: :manual, status: COMPLETED_STATUSES + [:manual]) } scope :ref_protected, -> { where(protected: true) } diff --git a/app/models/event.rb b/app/models/event.rb index 8a79100de5a..75538ba196c 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -96,10 +96,6 @@ class Event < ActiveRecord::Base self.inheritance_column = 'action' - # "data" will be removed in 10.0 but it may be possible that JOINs happen that - # include this column, hence we're ignoring it as well. - ignore_column :data - class << self def model_name ActiveModel::Name.new(self, nil, 'event') diff --git a/app/models/member.rb b/app/models/member.rb index c47145667b5..2d17795e62d 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -314,7 +314,7 @@ class Member < ActiveRecord::Base end def notification_setting - @notification_setting ||= user.notification_settings_for(source) + @notification_setting ||= user&.notification_settings_for(source) end def notifiable?(type, opts = {}) diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb index de3a252d6c6..2e89f00dad8 100644 --- a/app/services/members/authorized_destroy_service.rb +++ b/app/services/members/authorized_destroy_service.rb @@ -11,6 +11,7 @@ module Members Member.transaction do unassign_issues_and_merge_requests(member) unless member.invite? + member.notification_setting&.destroy member.destroy end diff --git a/app/services/projects/create_from_template_service.rb b/app/services/projects/create_from_template_service.rb index 87d9ed7a0e6..a549cfbabea 100644 --- a/app/services/projects/create_from_template_service.rb +++ b/app/services/projects/create_from_template_service.rb @@ -5,11 +5,15 @@ module Projects end def execute - params[:file] = Gitlab::ProjectTemplate.find(params[:template_name]).file + template_name = params.delete(:template_name) + file = Gitlab::ProjectTemplate.find(template_name).file + + params[:file] = file + + GitlabProjectsImportService.new(current_user, params).execute - GitlabProjectsImportService.new(@current_user, @params).execute ensure - params[:file]&.close + file&.close end end end diff --git a/app/services/projects/gitlab_projects_import_service.rb b/app/services/projects/gitlab_projects_import_service.rb index a3d7f5cbed5..a68ecb4abe1 100644 --- a/app/services/projects/gitlab_projects_import_service.rb +++ b/app/services/projects/gitlab_projects_import_service.rb @@ -11,12 +11,14 @@ module Projects def execute FileUtils.mkdir_p(File.dirname(import_upload_path)) + + file = params.delete(:file) FileUtils.copy_entry(file.path, import_upload_path) - Gitlab::ImportExport::ProjectCreator.new(params[:namespace_id], - current_user, - import_upload_path, - params[:path]).execute + params[:import_type] = 'gitlab_project' + params[:import_source] = import_upload_path + + ::Projects::CreateService.new(current_user, params).execute end private @@ -28,9 +30,5 @@ module Projects def tmp_filename SecureRandom.hex end - - def file - params[:file] - end end end diff --git a/app/views/admin/users/_user.html.haml b/app/views/admin/users/_user.html.haml index 3b85586e45b..bbfeceff5b9 100644 --- a/app/views/admin/users/_user.html.haml +++ b/app/views/admin/users/_user.html.haml @@ -1,6 +1,6 @@ %li.flex-row .user-avatar - = image_tag avatar_icon(user), class: "avatar", alt: '' + = image_tag avatar_icon_for_user(user), class: "avatar", alt: '' .row-main-content .user-name.row-title.str-truncated-100 = link_to user.name, [:admin, user] diff --git a/app/views/admin/users/show.html.haml b/app/views/admin/users/show.html.haml index 101667508a9..ec3be869797 100644 --- a/app/views/admin/users/show.html.haml +++ b/app/views/admin/users/show.html.haml @@ -10,7 +10,7 @@ = @user.name %ul.well-list %li - = image_tag avatar_icon(@user, 60), class: "avatar s60" + = image_tag avatar_icon_for_user(@user, 60), class: "avatar s60" %li %span.light Profile page: %strong diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml index 205320ed87c..8b9fa3d6b05 100644 --- a/app/views/discussions/_discussion.html.haml +++ b/app/views/discussions/_discussion.html.haml @@ -3,7 +3,7 @@ .timeline-entry-inner .timeline-icon = link_to user_path(discussion.author) do - = image_tag avatar_icon(discussion.author), class: "avatar s40" + = image_tag avatar_icon_for_user(discussion.author), class: "avatar s40" .timeline-content .discussion.js-toggle-container{ data: { discussion_id: discussion.id } } .discussion-header diff --git a/app/views/events/_event.atom.builder b/app/views/events/_event.atom.builder index 38741fe6662..d56234e6c1a 100644 --- a/app/views/events/_event.atom.builder +++ b/app/views/events/_event.atom.builder @@ -10,7 +10,7 @@ xml.entry do # eager-loaded. This allows us to re-use the user object's Email address, # instead of having to run additional queries to figure out what Email to use # for the avatar. - xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(event.author)) + xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon_for_user(event.author)) xml.author do xml.username event.author_username diff --git a/app/views/help/ui.html.haml b/app/views/help/ui.html.haml index 445f0dffbcc..1c4d67a8d2c 100644 --- a/app/views/help/ui.html.haml +++ b/app/views/help/ui.html.haml @@ -68,7 +68,7 @@ .example .cover-block .avatar-holder - = image_tag avatar_icon('admin@example.com', 90), class: "avatar s90", alt: '' + = image_tag avatar_icon_for_email('admin@example.com', 90), class: "avatar s90", alt: '' .cover-title John Smith diff --git a/app/views/issues/_issue.atom.builder b/app/views/issues/_issue.atom.builder index 0c113c08526..21cf6d0dd65 100644 --- a/app/views/issues/_issue.atom.builder +++ b/app/views/issues/_issue.atom.builder @@ -3,7 +3,7 @@ xml.entry do xml.link href: project_issue_url(issue.project, issue) xml.title truncate(issue.title, length: 80) xml.updated issue.updated_at.xmlschema - xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(issue.author_email)) + xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon_for_user(issue.author)) xml.author do xml.name issue.author_name diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml index e7fc83a8d04..1d00ae928f6 100644 --- a/app/views/layouts/header/_default.html.haml +++ b/app/views/layouts/header/_default.html.haml @@ -45,7 +45,7 @@ = todos_count_format(todos_pending_count) %li.header-user.dropdown = link_to current_user, class: user_dropdown_class, data: { toggle: "dropdown" } do - = image_tag avatar_icon(current_user, 23), width: 23, height: 23, class: "header-user-avatar qa-user-avatar" + = image_tag avatar_icon_for_user(current_user, 23), width: 23, height: 23, class: "header-user-avatar qa-user-avatar" = sprite_icon('angle-down', css_class: 'caret-down') .dropdown-menu-nav.dropdown-menu-align-right %ul diff --git a/app/views/notify/pipeline_failed_email.html.haml b/app/views/notify/pipeline_failed_email.html.haml index 8eb3f2d5192..38dab104eb5 100644 --- a/app/views/notify/pipeline_failed_email.html.haml +++ b/app/views/notify/pipeline_failed_email.html.haml @@ -60,7 +60,7 @@ %tbody %tr %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } - %img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for(commit.author, commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } - if commit.author %a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" } @@ -76,7 +76,7 @@ %tbody %tr %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } - %img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for(commit.committer, commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } - if commit.committer %a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" } @@ -100,7 +100,7 @@ triggered by - if @pipeline.user %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;padding-left:5px", width: "24" } - %img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for_user(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;font-weight:500;line-height:1.4;vertical-align:baseline;" } %a.muted{ href: user_url(@pipeline.user), style: "color:#333333;text-decoration:none;" } = @pipeline.user.name diff --git a/app/views/notify/pipeline_success_email.html.haml b/app/views/notify/pipeline_success_email.html.haml index bae37292d62..7b06e8afa0b 100644 --- a/app/views/notify/pipeline_success_email.html.haml +++ b/app/views/notify/pipeline_success_email.html.haml @@ -60,7 +60,7 @@ %tbody %tr %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } - %img.avatar{ height: "24", src: avatar_icon(commit.author || commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for(commit.author, commit.author_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } - if commit.author %a.muted{ href: user_url(commit.author), style: "color:#333333;text-decoration:none;" } @@ -76,7 +76,7 @@ %tbody %tr %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;" } - %img.avatar{ height: "24", src: avatar_icon(commit.committer || commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for(commit.committer, commit.committer_email, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;" } - if commit.committer %a.muted{ href: user_url(commit.committer), style: "color:#333333;text-decoration:none;" } @@ -100,7 +100,7 @@ triggered by - if @pipeline.user %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;line-height:1.4;vertical-align:middle;padding-right:5px;padding-left:5px", width: "24" } - %img.avatar{ height: "24", src: avatar_icon(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ + %img.avatar{ height: "24", src: avatar_icon_for_user(@pipeline.user, 24, only_path: false), style: "display:block;border-radius:12px;margin:-2px 0;", width: "24", alt: "" }/ %td{ style: "font-family:'Helvetica Neue',Helvetica,Arial,sans-serif;font-size:15px;font-weight:500;line-height:1.4;vertical-align:baseline;" } %a.muted{ href: user_url(@pipeline.user), style: "color:#333333;text-decoration:none;" } = @pipeline.user.name diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 5c76d2d8f51..110736dc557 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -20,8 +20,8 @@ or change it at #{link_to Gitlab.config.gravatar.host, 'https://' + Gitlab.config.gravatar.host} .col-lg-8 .clearfix.avatar-image.append-bottom-default - = link_to avatar_icon(@user, 400), target: '_blank', rel: 'noopener noreferrer' do - = image_tag avatar_icon(@user, 160), alt: '', class: 'avatar s160' + = link_to avatar_icon_for_user(@user, 400), target: '_blank', rel: 'noopener noreferrer' do + = image_tag avatar_icon_for_user(@user, 160), alt: '', class: 'avatar s160' %h5.prepend-top-0= _("Upload new avatar") .prepend-top-5.append-bottom-10 %button.btn.js-choose-user-avatar-button{ type: 'button' }= _("Choose file...") diff --git a/app/views/projects/commits/_commit.atom.builder b/app/views/projects/commits/_commit.atom.builder index 04914888763..50f7e7a3a33 100644 --- a/app/views/projects/commits/_commit.atom.builder +++ b/app/views/projects/commits/_commit.atom.builder @@ -3,7 +3,7 @@ xml.entry do xml.link href: project_commit_url(@project, id: commit.id) xml.title truncate(commit.title, length: 80, escape: false) xml.updated commit.committed_date.xmlschema - xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon(commit.author_email)) + xml.media :thumbnail, width: "40", height: "40", url: image_url(avatar_icon_for_email(commit.author_email)) xml.author do |author| xml.name commit.author_name diff --git a/app/views/projects/jobs/_user.html.haml b/app/views/projects/jobs/_user.html.haml index 83f299da651..461d503f95d 100644 --- a/app/views/projects/jobs/_user.html.haml +++ b/app/views/projects/jobs/_user.html.haml @@ -1,7 +1,7 @@ by %a{ href: user_path(@build.user) } %span.hidden-xs - = image_tag avatar_icon(@build.user, 24), class: "avatar s24" + = image_tag avatar_icon_for_user(@build.user, 24), class: "avatar s24" %strong{ data: { toggle: 'tooltip', placement: 'top', title: @build.user.to_reference } } = @build.user.name %strong.visible-xs-inline= @build.user.to_reference diff --git a/app/views/projects/network/show.json.erb b/app/views/projects/network/show.json.erb index 7491b37310d..a0e82e891ff 100644 --- a/app/views/projects/network/show.json.erb +++ b/app/views/projects/network/show.json.erb @@ -9,7 +9,7 @@ author: { name: c.author_name, email: c.author_email, - icon: image_path(avatar_icon(c.author_email, 20)) + icon: image_path(avatar_icon_for_email(c.author_email, 20)) }, time: c.time, space: c.spaces.first, diff --git a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml index a8692b83b07..55d0e8bb7f9 100644 --- a/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml +++ b/app/views/projects/pipeline_schedules/_pipeline_schedule.html.haml @@ -21,7 +21,7 @@ = s_("PipelineSchedules|Inactive") %td - if pipeline_schedule.owner - = image_tag avatar_icon(pipeline_schedule.owner, 20), class: "avatar s20" + = image_tag avatar_icon_for_user(pipeline_schedule.owner, 20), class: "avatar s20" = link_to user_path(pipeline_schedule.owner) do = pipeline_schedule.owner&.name %td diff --git a/app/views/projects/repositories/_feed.html.haml b/app/views/projects/repositories/_feed.html.haml index 170f9e259df..87895a15239 100644 --- a/app/views/projects/repositories/_feed.html.haml +++ b/app/views/projects/repositories/_feed.html.haml @@ -11,7 +11,7 @@ %div = link_to project_commits_path(@project, commit.id) do %code= commit.short_id - = image_tag avatar_icon(commit.author_email), class: "", width: 16, alt: '' + = image_tag avatar_icon_for_email(commit.author_email), class: "", width: 16, alt: '' = markdown(truncate(commit.title, length: 40), pipeline: :single_line, author: commit.author) %td %span.pull-right.cgray diff --git a/app/views/search/results/_snippet_blob.html.haml b/app/views/search/results/_snippet_blob.html.haml index c4a5131c1a7..57a0b64bfd5 100644 --- a/app/views/search/results/_snippet_blob.html.haml +++ b/app/views/search/results/_snippet_blob.html.haml @@ -7,7 +7,7 @@ = snippet.title by = link_to user_snippets_path(snippet.author) do - = image_tag avatar_icon(snippet.author), class: "avatar avatar-inline s16", alt: '' + = image_tag avatar_icon_for_user(snippet.author), class: "avatar avatar-inline s16", alt: '' = snippet.author_name %span.light= time_ago_with_tooltip(snippet.created_at) %h4.snippet-title diff --git a/app/views/search/results/_snippet_title.html.haml b/app/views/search/results/_snippet_title.html.haml index aef825691e0..65710c09a89 100644 --- a/app/views/search/results/_snippet_title.html.haml +++ b/app/views/search/results/_snippet_title.html.haml @@ -18,6 +18,6 @@ %span by = link_to user_snippets_path(snippet_title.author) do - = image_tag avatar_icon(snippet_title.author), class: "avatar avatar-inline s16", alt: '' + = image_tag avatar_icon_for_user(snippet_title.author), class: "avatar avatar-inline s16", alt: '' = snippet_title.author_name %span.light= time_ago_with_tooltip(snippet_title.created_at) diff --git a/app/views/shared/members/_member.html.haml b/app/views/shared/members/_member.html.haml index 71878e93255..ba57d922c6d 100644 --- a/app/views/shared/members/_member.html.haml +++ b/app/views/shared/members/_member.html.haml @@ -8,7 +8,7 @@ %li.member{ class: dom_class(member), id: dom_id(member) } %span.list-item-name - if user - = image_tag avatar_icon(user, 40), class: "avatar s40", alt: '' + = image_tag avatar_icon_for_user(user, 40), class: "avatar s40", alt: '' .user-info = link_to user.name, user_path(user), class: 'member' %span.cgray= user.to_reference @@ -36,7 +36,7 @@ Expires in #{distance_of_time_in_words_to_now(member.expires_at)} - else - = image_tag avatar_icon(member.invite_email, 40), class: "avatar s40", alt: '' + = image_tag avatar_icon_for_email(member.invite_email, 40), class: "avatar s40", alt: '' .user-info .member= member.invite_email .cgray diff --git a/app/views/shared/milestones/_issuable.html.haml b/app/views/shared/milestones/_issuable.html.haml index 14395bcc661..479b7270b28 100644 --- a/app/views/shared/milestones/_issuable.html.haml +++ b/app/views/shared/milestones/_issuable.html.haml @@ -28,4 +28,4 @@ - assignees.each do |assignee| = link_to polymorphic_path(issuable_type_args, { milestone_title: @milestone.title, assignee_id: assignee.id, state: 'all' }), class: 'has-tooltip', title: "Assigned to #{assignee.name}", data: { container: 'body' } do - - image_tag(avatar_icon(assignee, 16), class: "avatar s16", alt: '') + - image_tag(avatar_icon_for_user(assignee, 16), class: "avatar s16", alt: '') diff --git a/app/views/shared/milestones/_participants_tab.html.haml b/app/views/shared/milestones/_participants_tab.html.haml index 1615871385e..fe83040c168 100644 --- a/app/views/shared/milestones/_participants_tab.html.haml +++ b/app/views/shared/milestones/_participants_tab.html.haml @@ -2,7 +2,7 @@ - users.each do |user| %li = link_to user, title: user.name, class: "darken" do - = image_tag avatar_icon(user, 32), class: "avatar s32" + = image_tag avatar_icon_for_user(user, 32), class: "avatar s32" %strong= truncate(user.name, length: 40) %div %small.cgray= user.username diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index 98e0161f7d1..bf359774ead 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -16,7 +16,7 @@ = icon_for_system_note(note) - else %a.image-diff-avatar-link{ href: user_path(note.author) } - = image_tag avatar_icon(note.author), alt: '', class: 'avatar s40' + = image_tag avatar_icon_for_user(note.author), alt: '', class: 'avatar s40' - if note.is_a?(DiffNote) && note.on_image? - if show_image_comment_badge && note_counter == 0 -# Only show this for the first comment in the discussion diff --git a/app/views/shared/notes/_notes_with_form.html.haml b/app/views/shared/notes/_notes_with_form.html.haml index e11f778adf5..b3f865c5b47 100644 --- a/app/views/shared/notes/_notes_with_form.html.haml +++ b/app/views/shared/notes/_notes_with_form.html.haml @@ -14,7 +14,7 @@ .timeline-icon.hidden-xs.hidden-sm %a.author_link{ href: user_path(current_user) } - = image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40' + = image_tag avatar_icon_for_user(current_user), alt: current_user.to_reference, class: 'avatar s40' .timeline-content.timeline-content-form = render "shared/notes/form", view: diff_view, supports_autocomplete: autocomplete - elsif !current_user diff --git a/app/views/shared/projects/_project.html.haml b/app/views/shared/projects/_project.html.haml index 2a75b46d376..33435216c14 100644 --- a/app/views/shared/projects/_project.html.haml +++ b/app/views/shared/projects/_project.html.haml @@ -17,7 +17,7 @@ .avatar-container.s40 = link_to project_path(project), class: dom_class(project) do - if project.creator && use_creator_avatar - = image_tag avatar_icon(project.creator.email, 40), class: "avatar s40", alt:'' + = image_tag avatar_icon_for_user(project.creator, 40), class: "avatar s40", alt:'' - else = project_icon(project, alt: '', class: 'avatar project-avatar s40') .project-details diff --git a/app/views/shared/snippets/_snippet.html.haml b/app/views/shared/snippets/_snippet.html.haml index 57b445321e2..491a8a41090 100644 --- a/app/views/shared/snippets/_snippet.html.haml +++ b/app/views/shared/snippets/_snippet.html.haml @@ -1,7 +1,7 @@ - link_project = local_assigns.fetch(:link_project, false) %li.snippet-row - = image_tag avatar_icon(snippet.author), class: "avatar s40 hidden-xs", alt: '' + = image_tag avatar_icon_for_user(snippet.author), class: "avatar s40 hidden-xs", alt: '' .title = link_to reliable_snippet_path(snippet) do diff --git a/app/views/users/show.html.haml b/app/views/users/show.html.haml index 90aa1be30ac..c9a77d668a2 100644 --- a/app/views/users/show.html.haml +++ b/app/views/users/show.html.haml @@ -35,8 +35,8 @@ .profile-header .avatar-holder - = link_to avatar_icon(@user, 400), target: '_blank', rel: 'noopener noreferrer' do - = image_tag avatar_icon(@user, 90), class: "avatar s90", alt: '' + = link_to avatar_icon_for_user(@user, 400), target: '_blank', rel: 'noopener noreferrer' do + = image_tag avatar_icon_for_user(@user, 90), class: "avatar s90", alt: '' .user-info .cover-title diff --git a/changelogs/unreleased/42481-remove-notification-settings-left-projects.yml b/changelogs/unreleased/42481-remove-notification-settings-left-projects.yml new file mode 100644 index 00000000000..ea99649131b --- /dev/null +++ b/changelogs/unreleased/42481-remove-notification-settings-left-projects.yml @@ -0,0 +1,5 @@ +--- +title: Remove user notification settings for groups and projects when user leaves +merge_request: 16906 +author: Jacopo Beschi @jacopo-beschi +type: fixed diff --git a/changelogs/unreleased/42800-change-usage-of-avatar_icon.yml b/changelogs/unreleased/42800-change-usage-of-avatar_icon.yml new file mode 100644 index 00000000000..00f4b7436a7 --- /dev/null +++ b/changelogs/unreleased/42800-change-usage-of-avatar_icon.yml @@ -0,0 +1,6 @@ +--- +title: Use a user object in ApplicationHelper#avatar_icon where possible to avoid + N+1 queries. +merge_request: 42800 +author: +type: performance diff --git a/changelogs/unreleased/expired-ci-artifacts.yml b/changelogs/unreleased/expired-ci-artifacts.yml new file mode 100644 index 00000000000..2fcbdb02f84 --- /dev/null +++ b/changelogs/unreleased/expired-ci-artifacts.yml @@ -0,0 +1,5 @@ +--- +title: Change SQL for expired artifacts to use new ci_job_artifacts.expire_at +merge_request: 16578 +author: +type: performance diff --git a/changelogs/unreleased/feature-include-custom-attributes-in-api.yml b/changelogs/unreleased/feature-include-custom-attributes-in-api.yml new file mode 100644 index 00000000000..f1087d7f7cc --- /dev/null +++ b/changelogs/unreleased/feature-include-custom-attributes-in-api.yml @@ -0,0 +1,5 @@ +--- +title: Allow including custom attributes in API responses +merge_request: 16526 +author: Markus Koller +type: changed diff --git a/changelogs/unreleased/fix-template-project-visibility.yml b/changelogs/unreleased/fix-template-project-visibility.yml new file mode 100644 index 00000000000..6576097822b --- /dev/null +++ b/changelogs/unreleased/fix-template-project-visibility.yml @@ -0,0 +1,5 @@ +--- +title: Respect description and visibility when creating project from template +merge_request: 16820 +author: George Tsiolis +type: fixed diff --git a/db/migrate/20180119160751_optimize_ci_job_artifacts.rb b/db/migrate/20180119160751_optimize_ci_job_artifacts.rb new file mode 100644 index 00000000000..9b4340ed7b7 --- /dev/null +++ b/db/migrate/20180119160751_optimize_ci_job_artifacts.rb @@ -0,0 +1,23 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class OptimizeCiJobArtifacts < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + disable_ddl_transaction! + + def up + # job_id is just here to be a covering index for index only scans + # since we'll almost always be joining against ci_builds on job_id + add_concurrent_index(:ci_job_artifacts, [:expire_at, :job_id]) + add_concurrent_index(:ci_builds, [:artifacts_expire_at], where: "artifacts_file <> ''") + end + + def down + remove_concurrent_index(:ci_job_artifacts, [:expire_at, :job_id]) + remove_concurrent_index(:ci_builds, [:artifacts_expire_at], where: "artifacts_file <> ''") + end +end diff --git a/db/schema.rb b/db/schema.rb index b281be110da..6b43fc8403c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -293,6 +293,7 @@ ActiveRecord::Schema.define(version: 20180208183958) do t.integer "failure_reason" end + add_index "ci_builds", ["artifacts_expire_at"], name: "index_ci_builds_on_artifacts_expire_at", where: "(artifacts_file <> ''::text)", using: :btree add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree add_index "ci_builds", ["commit_id", "status", "type"], name: "index_ci_builds_on_commit_id_and_status_and_type", using: :btree @@ -333,6 +334,7 @@ ActiveRecord::Schema.define(version: 20180208183958) do t.string "file" end + add_index "ci_job_artifacts", ["expire_at", "job_id"], name: "index_ci_job_artifacts_on_expire_at_and_job_id", using: :btree add_index "ci_job_artifacts", ["job_id", "file_type"], name: "index_ci_job_artifacts_on_job_id_and_file_type", unique: true, using: :btree add_index "ci_job_artifacts", ["project_id"], name: "index_ci_job_artifacts_on_project_id", using: :btree diff --git a/doc/api/groups.md b/doc/api/groups.md index de730cdd869..f50558b58a6 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -15,6 +15,7 @@ Parameters: | `order_by` | string | no | Order groups by `name` or `path`. Default is `name` | | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` | | `statistics` | boolean | no | Include group statistics (admins only) | +| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | | `owned` | boolean | no | Limit to groups owned by the current user | ``` @@ -98,6 +99,7 @@ Parameters: | `order_by` | string | no | Order groups by `name` or `path`. Default is `name` | | `sort` | string | no | Order groups in `asc` or `desc` order. Default is `asc` | | `statistics` | boolean | no | Include group statistics (admins only) | +| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | | `owned` | boolean | no | Limit to groups owned by the current user | ``` @@ -145,6 +147,7 @@ Parameters: | `simple` | boolean | no | Return only the ID, URL, name, and path of each project | | `owned` | boolean | no | Limit by projects owned by the current user | | `starred` | boolean | no | Limit by projects starred by the current user | +| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | Example response: @@ -204,6 +207,7 @@ Parameters: | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `id` | integer/string | yes | The ID or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user | +| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | ```bash curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/groups/4 diff --git a/doc/api/projects.md b/doc/api/projects.md index 46f5de5aa0e..05d2f2af00b 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -37,6 +37,7 @@ GET /projects | `membership` | boolean | no | Limit by projects that the current user is a member of | | `starred` | boolean | no | Limit by projects starred by the current user | | `statistics` | boolean | no | Include project statistics | +| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | | `with_issues_enabled` | boolean | no | Limit by enabled issues feature | | `with_merge_requests_enabled` | boolean | no | Limit by enabled merge requests feature | @@ -220,6 +221,7 @@ GET /users/:user_id/projects | `membership` | boolean | no | Limit by projects that the current user is a member of | | `starred` | boolean | no | Limit by projects starred by the current user | | `statistics` | boolean | no | Include project statistics | +| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | | `with_issues_enabled` | boolean | no | Limit by enabled issues feature | | `with_merge_requests_enabled` | boolean | no | Limit by enabled merge requests feature | @@ -388,6 +390,7 @@ GET /projects/:id | --------- | ---- | -------- | ----------- | | `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) | | `statistics` | boolean | no | Include project statistics | +| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | ```json { @@ -664,6 +667,7 @@ GET /projects/:id/forks | `membership` | boolean | no | Limit by projects that the current user is a member of | | `starred` | boolean | no | Limit by projects starred by the current user | | `statistics` | boolean | no | Include project statistics | +| `with_custom_attributes` | boolean | no | Include [custom attributes](custom_attributes.md) in response (admins only) | | `with_issues_enabled` | boolean | no | Limit by enabled issues feature | | `with_merge_requests_enabled` | boolean | no | Limit by enabled merge requests feature | diff --git a/doc/api/users.md b/doc/api/users.md index 2082e45756a..a4447e32908 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -165,6 +165,12 @@ You can filter by [custom attributes](custom_attributes.md) with: GET /users?custom_attributes[key]=value&custom_attributes[other_key]=other_value ``` +You can include the users' [custom attributes](custom_attributes.md) in the response with: + +``` +GET /users?with_custom_attributes=true +``` + ## Single user Get a single user. @@ -245,6 +251,12 @@ Parameters: } ``` +You can include the user's [custom attributes](custom_attributes.md) in the response with: + +``` +GET /users/:id?with_custom_attributes=true +``` + ## User creation Creates a new user. Note only administrators can create new users. Either `password` or `reset_password` should be specified (`reset_password` takes priority). diff --git a/doc/development/feature_flags.md b/doc/development/feature_flags.md index 59e8a087e02..5d1f657015c 100644 --- a/doc/development/feature_flags.md +++ b/doc/development/feature_flags.md @@ -1,6 +1,6 @@ # Manage feature flags -Starting from GitLab 9.3 we support feature flags via +Starting from GitLab 9.3 we support feature flags for features in GitLab via [Flipper](https://github.com/jnunemaker/flipper/). You should use the `Feature` class (defined in `lib/feature.rb`) in your code to get, set and list feature flags. @@ -19,3 +19,8 @@ dynamic (querying the DB etc.). Once defined in `lib/feature.rb`, you will be able to activate a feature for a given feature group via the [`feature_group` param of the features API](../api/features.md#set-or-create-a-feature) + +## Feature flags for user applications + +GitLab does not yet support the use of feature flags in deployed user applications. +You can follow the progress on that [in the issue on our issue tracker](https://gitlab.com/gitlab-org/gitlab-ee/issues/779).
\ No newline at end of file diff --git a/doc/development/i18n/index.md b/doc/development/i18n/index.md index 8aa0462d213..7290a175501 100644 --- a/doc/development/i18n/index.md +++ b/doc/development/i18n/index.md @@ -40,37 +40,12 @@ See [Translation guidelines](translation.md). ### Proof reading -Proof reading helps ensure the accuracy and consistency of translations. -All translations are proof read before being accepted. -If a translations requires changes, you will be notified with a comment explaining why. - -Community assistance proof reading translations is encouraged and appreciated. -Requests to become a proof reader will be considered on the merits of previous translations. - -- Bulgarian -- Chinese Simplified - - [Huang Tao](https://crowdin.com/profile/htve) -- Chinese Traditional - - [Huang Tao](https://crowdin.com/profile/htve) -- Chinese Traditional, Hong Kong - - [Huang Tao](https://crowdin.com/profile/htve) -- Dutch -- Esperanto -- French -- German -- Italian - - [Paolo Falomo](https://crowdin.com/profile/paolo.falomo) -- Japanese -- Korean - - [Huang Tao](https://crowdin.com/profile/htve) -- Portuguese, Brazilian -- Russian - - [Alexy Lustin](https://crowdin.com/profile/lustin) - - [Nikita Grylov](https://crowdin.com/profile/nixel2007) -- Spanish -- Ukrainian - -If you would like to be added as a proof reader, please [open an issue](https://gitlab.com/gitlab-org/gitlab-ce/issues). +Proof reading helps ensure the accuracy and consistency of translations. All +translations are proof read before being accepted. If a translations requires +changes, you will be notified with a comment explaining why. + +See [Proofreading Translations](proofreader.md) for more information on who's +able to proofread and instructions on becoming a proofreader yourself. ## Release diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md new file mode 100644 index 00000000000..795e1e83105 --- /dev/null +++ b/doc/development/i18n/proofreader.md @@ -0,0 +1,48 @@ +# Proofread Translations + +Most translations are contributed, reviewed, and accepted by the community. We +are very appreciative of the work done by translators and proofreaders! + +## Proofreaders + +- Bulgarian +- Chinese Simplified + - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve) +- Chinese Traditional + - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve) +- Chinese Traditional, Hong Kong + - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve) +- Dutch +- Esperanto +- French +- German +- Italian + - Paolo Falomo - [GitLab](https://gitlab.com/paolofalomo), [Crowdin](https://crowdin.com/profile/paolo.falomo) +- Japanese +- Korean + - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve) +- Portuguese, Brazilian + - Paulo George Gomes Bezerra - [GitLab](https://gitlab.com/paulobezerra), [Crowdin](https://crowdin.com/profile/paulogomes.rep) +- Russian + - Nikita Grylov - [GitLab](https://gitlab.com/nixel2007), [Crowdin](https://crowdin.com/profile/nixel2007) + - Alexy Lustin - [GitLab](https://gitlab.com/allustin), [Crowdin](https://crowdin.com/profile/lustin) +- Spanish +- Ukrainian + - Volodymyr Sobotovych - [GitLab](https://gitlab.com/wheleph), [Crowdin](https://crowdin.com/profile/wheleph) + - Andrew Vityuk - [GitLab](https://gitlab.com/3_1_3_u), [Crowdin](https://crowdin.com/profile/andruwa13) + +## Become a proofreader + +> **Note:** Before requesting Proofreader permissions in Crowdin please make +> sure that you have a history of contributing translations to the GitLab +> project. + +1. Once your translations have been accepted, + [open a merge request](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/new) + to request Proofreader permissions and add yourself to the list above. + + In the merge request description, please include links to any projects you + have previously translated. + +1. Your request to become a proofreader will be considered on the merits of + your previous translations. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index e9e32ac76fe..03abc1b95c5 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -22,6 +22,7 @@ module API end expose :avatar_path, if: ->(user, options) { options.fetch(:only_path, false) && user.avatar_path } + expose :custom_attributes, using: 'API::Entities::CustomAttribute', if: :with_custom_attributes expose :web_url do |user, options| Gitlab::Routing.url_helpers.user_url(user) @@ -109,6 +110,8 @@ module API expose :star_count, :forks_count expose :last_activity_at + expose :custom_attributes, using: 'API::Entities::CustomAttribute', if: :with_custom_attributes + def self.preload_relation(projects_relation, options = {}) projects_relation.preload(:project_feature, :route) .preload(namespace: [:route, :owner], @@ -230,6 +233,8 @@ module API expose :parent_id end + expose :custom_attributes, using: 'API::Entities::CustomAttribute', if: :with_custom_attributes + expose :statistics, if: :statistics do with_options format_with: -> (value) { value.to_i } do expose :storage_size diff --git a/lib/api/groups.rb b/lib/api/groups.rb index b81f07a1770..4a4df1b8b9e 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -1,6 +1,7 @@ module API class Groups < Grape::API include PaginationParams + include Helpers::CustomAttributes before { authenticate_non_get! } @@ -67,6 +68,8 @@ module API } groups = groups.with_statistics if options[:statistics] + groups, options = with_custom_attributes(groups, options) + present paginate(groups), options end end @@ -79,6 +82,7 @@ module API end params do use :group_list_params + use :with_custom_attributes end get do groups = find_groups(params) @@ -142,9 +146,20 @@ module API desc 'Get a single group, with containing projects.' do success Entities::GroupDetail end + params do + use :with_custom_attributes + end get ":id" do group = find_group!(params[:id]) - present group, with: Entities::GroupDetail, current_user: current_user + + options = { + with: Entities::GroupDetail, + current_user: current_user + } + + group, options = with_custom_attributes(group, options) + + present group, options end desc 'Remove a group.' @@ -175,12 +190,19 @@ module API optional :starred, type: Boolean, default: false, desc: 'Limit by starred status' use :pagination + use :with_custom_attributes end get ":id/projects" do projects = find_group_projects(params) - entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project - present entity.prepare_relation(projects), with: entity, current_user: current_user + options = { + with: params[:simple] ? Entities::BasicProjectDetails : Entities::Project, + current_user: current_user + } + + projects, options = with_custom_attributes(projects, options) + + present options[:with].prepare_relation(projects), options end desc 'Get a list of subgroups in this group.' do @@ -188,6 +210,7 @@ module API end params do use :group_list_params + use :with_custom_attributes end get ":id/subgroups" do groups = find_groups(params) diff --git a/lib/api/helpers/custom_attributes.rb b/lib/api/helpers/custom_attributes.rb new file mode 100644 index 00000000000..70e4eda95f8 --- /dev/null +++ b/lib/api/helpers/custom_attributes.rb @@ -0,0 +1,28 @@ +module API + module Helpers + module CustomAttributes + extend ActiveSupport::Concern + + included do + helpers do + params :with_custom_attributes do + optional :with_custom_attributes, type: Boolean, default: false, desc: 'Include custom attributes in the response' + end + + def with_custom_attributes(collection_or_resource, options = {}) + options = options.merge( + with_custom_attributes: params[:with_custom_attributes] && + can?(current_user, :read_custom_attribute) + ) + + if options[:with_custom_attributes] && collection_or_resource.is_a?(ActiveRecord::Relation) + collection_or_resource = collection_or_resource.includes(:custom_attributes) + end + + [collection_or_resource, options] + end + end + end + end + end +end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 5b481121a10..e90892a90f7 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -3,6 +3,7 @@ require_dependency 'declarative_policy' module API class Projects < Grape::API include PaginationParams + include Helpers::CustomAttributes before { authenticate_non_get! } @@ -80,6 +81,7 @@ module API projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled] projects = projects.with_statistics if params[:statistics] projects = paginate(projects) + projects, options = with_custom_attributes(projects, options) if current_user project_members = current_user.project_members.preload(:source, user: [notification_settings: :source]) @@ -107,6 +109,7 @@ module API requires :user_id, type: String, desc: 'The ID or username of the user' use :collection_params use :statistics_params + use :with_custom_attributes end get ":user_id/projects" do user = find_user(params[:user_id]) @@ -127,6 +130,7 @@ module API params do use :collection_params use :statistics_params + use :with_custom_attributes end get do present_projects load_projects @@ -196,11 +200,19 @@ module API end params do use :statistics_params + use :with_custom_attributes end get ":id" do - entity = current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails - present user_project, with: entity, current_user: current_user, - user_can_admin_project: can?(current_user, :admin_project, user_project), statistics: params[:statistics] + options = { + with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails, + current_user: current_user, + user_can_admin_project: can?(current_user, :admin_project, user_project), + statistics: params[:statistics] + } + + project, options = with_custom_attributes(user_project, options) + + present project, options end desc 'Fork new project for the current user or provided namespace.' do @@ -242,6 +254,7 @@ module API end params do use :collection_params + use :with_custom_attributes end get ':id/forks' do forks = ForkProjectsFinder.new(user_project, params: project_finder_params, current_user: current_user).execute diff --git a/lib/api/users.rb b/lib/api/users.rb index 3cc12724b8a..3920171205f 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -2,6 +2,7 @@ module API class Users < Grape::API include PaginationParams include APIGuard + include Helpers::CustomAttributes allow_access_with_scope :read_user, if: -> (request) { request.get? } @@ -70,6 +71,7 @@ module API use :sort_params use :pagination + use :with_custom_attributes end get do authenticated_as_admin! if params[:external].present? || (params[:extern_uid].present? && params[:provider].present?) @@ -94,8 +96,9 @@ module API entity = current_user&.admin? ? Entities::UserWithAdmin : Entities::UserBasic users = users.preload(:identities, :u2f_registrations) if entity == Entities::UserWithAdmin + users, options = with_custom_attributes(users, with: entity) - present paginate(users), with: entity + present paginate(users), options end desc 'Get a single user' do @@ -103,12 +106,16 @@ module API end params do requires :id, type: Integer, desc: 'The ID of the user' + + use :with_custom_attributes end get ":id" do user = User.find_by(id: params[:id]) not_found!('User') unless user && can?(current_user, :read_user, user) opts = current_user&.admin? ? { with: Entities::UserWithAdmin } : { with: Entities::User } + user, opts = with_custom_attributes(user, opts) + present user, opts end diff --git a/lib/gitlab/background_migration/populate_untracked_uploads.rb b/lib/gitlab/background_migration/populate_untracked_uploads.rb index 8a8e770940e..ee55fabd6f0 100644 --- a/lib/gitlab/background_migration/populate_untracked_uploads.rb +++ b/lib/gitlab/background_migration/populate_untracked_uploads.rb @@ -249,7 +249,7 @@ module Gitlab end def drop_temp_table_if_finished - if UntrackedFile.all.empty? + if UntrackedFile.all.empty? && !Rails.env.test? # Dropping a table intermittently breaks test cleanup UntrackedFile.connection.drop_table(:untracked_files_for_uploads, if_exists: true) end diff --git a/lib/gitlab/background_migration/prepare_untracked_uploads.rb b/lib/gitlab/background_migration/prepare_untracked_uploads.rb index 298de005b9b..914a9e48a2f 100644 --- a/lib/gitlab/background_migration/prepare_untracked_uploads.rb +++ b/lib/gitlab/background_migration/prepare_untracked_uploads.rb @@ -171,8 +171,10 @@ module Gitlab end def drop_temp_table - UntrackedFile.connection.drop_table(:untracked_files_for_uploads, - if_exists: true) + unless Rails.env.test? # Dropping a table intermittently breaks test cleanup + UntrackedFile.connection.drop_table(:untracked_files_for_uploads, + if_exists: true) + end end end end diff --git a/lib/gitlab/import_export/project_creator.rb b/lib/gitlab/import_export/project_creator.rb deleted file mode 100644 index 77bb3ca6581..00000000000 --- a/lib/gitlab/import_export/project_creator.rb +++ /dev/null @@ -1,23 +0,0 @@ -module Gitlab - module ImportExport - class ProjectCreator - def initialize(namespace_id, current_user, file, project_path) - @namespace_id = namespace_id - @current_user = current_user - @file = file - @project_path = project_path - end - - def execute - ::Projects::CreateService.new( - @current_user, - name: @project_path, - path: @project_path, - namespace_id: @namespace_id, - import_type: "gitlab_project", - import_source: @file - ).execute - end - end - end -end @@ -117,6 +117,10 @@ module QA autoload :Show, 'qa/page/project/pipeline/show' end + module Job + autoload :Show, 'qa/page/project/job/show' + end + module Settings autoload :Common, 'qa/page/project/settings/common' autoload :Advanced, 'qa/page/project/settings/advanced' @@ -165,6 +169,7 @@ module QA # module Git autoload :Repository, 'qa/git/repository' + autoload :Location, 'qa/git/location' end ## diff --git a/qa/qa/factory/resource/runner.rb b/qa/qa/factory/resource/runner.rb index 5f37f8ac2e9..03b69eb1bdf 100644 --- a/qa/qa/factory/resource/runner.rb +++ b/qa/qa/factory/resource/runner.rb @@ -4,7 +4,7 @@ module QA module Factory module Resource class Runner < Factory::Base - attr_writer :name, :tags + attr_writer :name, :tags, :image dependency Factory::Resource::Project, as: :project do |project| project.name = 'project-with-ci-cd' @@ -19,6 +19,10 @@ module QA @tags || %w[qa e2e] end + def image + @image || 'gitlab/gitlab-runner:alpine' + end + def fabricate! project.visit! @@ -31,6 +35,7 @@ module QA runner.token = runners.registration_token runner.address = runners.coordinator_address runner.tags = tags + runner.image = image runner.register! end end diff --git a/qa/qa/git/location.rb b/qa/qa/git/location.rb new file mode 100644 index 00000000000..30538388530 --- /dev/null +++ b/qa/qa/git/location.rb @@ -0,0 +1,32 @@ +require 'uri' +require 'forwardable' + +module QA + module Git + class Location + extend Forwardable + + attr_reader :git_uri, :uri + def_delegators :@uri, :user, :host, :path + + # See: config/initializers/1_settings.rb + # Settings#build_gitlab_shell_ssh_path_prefix + def initialize(git_uri) + @git_uri = git_uri + @uri = + if git_uri.start_with?('ssh://') + URI.parse(git_uri) + else + *rest, path = git_uri.split(':') + # Host cannot have : so we'll need to escape it + user_host = rest.join('%3A').sub(/\A\[(.+)\]\z/, '\1') + URI.parse("ssh://#{user_host}/#{path}") + end + end + + def port + uri.port || 22 + end + end + end +end diff --git a/qa/qa/git/repository.rb b/qa/qa/git/repository.rb index 8f999511d58..8eb7031f609 100644 --- a/qa/qa/git/repository.rb +++ b/qa/qa/git/repository.rb @@ -1,3 +1,4 @@ +require 'cgi' require 'uri' module QA diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index 5c3af4b9115..7924479e2a1 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -17,7 +17,8 @@ module QA start = Time.now while Time.now - start < max - return true if yield + result = yield + return result if result sleep(time) diff --git a/qa/qa/page/project/job/show.rb b/qa/qa/page/project/job/show.rb new file mode 100644 index 00000000000..21bda74efb2 --- /dev/null +++ b/qa/qa/page/project/job/show.rb @@ -0,0 +1,19 @@ +module QA::Page + module Project::Job + class Show < QA::Page::Base + view 'app/views/projects/jobs/show.html.haml' do + element :build_output, '.js-build-output' + end + + def output + css = '.js-build-output' + + wait(reload: false) do + has_css?(css) + end + + find(css).text + end + end + end +end diff --git a/qa/qa/page/project/pipeline/index.rb b/qa/qa/page/project/pipeline/index.rb index 32c108393b9..ce430a2a6ee 100644 --- a/qa/qa/page/project/pipeline/index.rb +++ b/qa/qa/page/project/pipeline/index.rb @@ -6,7 +6,13 @@ module QA::Page end def go_to_latest_pipeline - first('.js-pipeline-url-link').click + css = '.js-pipeline-url-link' + + link = wait(reload: false) do + first(css) + end + + link.click end end end diff --git a/qa/qa/page/project/pipeline/show.rb b/qa/qa/page/project/pipeline/show.rb index 0835173f1cd..b183552d46c 100644 --- a/qa/qa/page/project/pipeline/show.rb +++ b/qa/qa/page/project/pipeline/show.rb @@ -11,6 +11,7 @@ module QA::Page view 'app/assets/javascripts/pipelines/components/graph/job_component.vue' do element :job_component, /class.*ci-job-component.*/ + element :job_link, /class.*js-pipeline-graph-job-link.*/ end view 'app/assets/javascripts/vue_shared/components/ci_icon.vue' do @@ -30,6 +31,16 @@ module QA::Page end end end + + def go_to_first_job + css = '.js-pipeline-graph-job-link' + + wait(reload: false) do + has_css?(css) + end + + first(css).click + end end end end diff --git a/qa/qa/page/project/show.rb b/qa/qa/page/project/show.rb index 9d2a84ea644..b603557f59c 100644 --- a/qa/qa/page/project/show.rb +++ b/qa/qa/page/project/show.rb @@ -22,22 +22,24 @@ module QA end def choose_repository_clone_http - wait(reload: false) do - click_element :clone_dropdown - - page.within('.clone-options-dropdown') do - click_link('HTTP') - end + choose_repository_clone('HTTP', 'http') + end - # Ensure git clone textbox was updated to http URI - repository_location.include?('http') - end + def choose_repository_clone_ssh + # It's not always beginning with ssh:// so detecting with @ + # would be more reliable because ssh would always contain it. + # We can't use .git because HTTP also contain that part. + choose_repository_clone('SSH', '@') end def repository_location find('#project_clone').value end + def repository_location_uri + Git::Location.new(repository_location) + end + def project_name find('.qa-project-name').text end @@ -56,6 +58,21 @@ module QA click_link 'New issue' end + + private + + def choose_repository_clone(kind, detect_text) + wait(reload: false) do + click_element :clone_dropdown + + page.within('.clone-options-dropdown') do + click_link(kind) + end + + # Ensure git clone textbox was updated + repository_location.include?(detect_text) + end + end end end end diff --git a/qa/qa/runtime/rsa_key.rb b/qa/qa/runtime/rsa_key.rb index d456062bce7..fcd7dcc4f02 100644 --- a/qa/qa/runtime/rsa_key.rb +++ b/qa/qa/runtime/rsa_key.rb @@ -7,7 +7,7 @@ module QA extend Forwardable attr_reader :key - def_delegators :@key, :fingerprint + def_delegators :@key, :fingerprint, :to_pem def initialize(bits = 4096) @key = OpenSSL::PKey::RSA.new(bits) diff --git a/qa/qa/specs/features/project/deploy_key_clone_spec.rb b/qa/qa/specs/features/project/deploy_key_clone_spec.rb new file mode 100644 index 00000000000..19d3c83758a --- /dev/null +++ b/qa/qa/specs/features/project/deploy_key_clone_spec.rb @@ -0,0 +1,81 @@ +require 'digest/sha1' + +module QA + feature 'cloning code using a deploy key', :core, :docker do + let(:runner_name) { "qa-runner-#{Time.now.to_i}" } + let(:key) { Runtime::RSAKey.new } + + given(:project) do + Factory::Resource::Project.fabricate! do |resource| + resource.name = 'deploy-key-clone-project' + end + end + + after do + Service::Runner.new(runner_name).remove! + end + + scenario 'user sets up a deploy key to clone code using pipelines' do + Runtime::Browser.visit(:gitlab, Page::Main::Login) + Page::Main::Login.act { sign_in_using_credentials } + + Factory::Resource::Runner.fabricate! do |resource| + resource.project = project + resource.name = runner_name + resource.tags = %w[qa docker] + resource.image = 'gitlab/gitlab-runner:ubuntu' + end + + Factory::Resource::DeployKey.fabricate! do |resource| + resource.project = project + resource.title = 'deploy key title' + resource.key = key.public_key + end + + Factory::Resource::SecretVariable.fabricate! do |resource| + resource.project = project + resource.key = 'DEPLOY_KEY' + resource.value = key.to_pem + end + + project.visit! + + repository_uri = Page::Project::Show.act do + choose_repository_clone_ssh + repository_location_uri + end + + gitlab_ci = <<~YAML + cat-config: + script: + - mkdir -p ~/.ssh + - ssh-keyscan -p #{repository_uri.port} #{repository_uri.host} >> ~/.ssh/known_hosts + - eval $(ssh-agent -s) + - echo "$DEPLOY_KEY" | ssh-add - + - git clone #{repository_uri.git_uri} + - sha1sum #{project.name}/.gitlab-ci.yml + tags: + - qa + - docker + YAML + + Factory::Repository::Push.fabricate! do |resource| + resource.project = project + resource.file_name = '.gitlab-ci.yml' + resource.commit_message = 'Add .gitlab-ci.yml' + resource.file_content = gitlab_ci + end + + sha1sum = Digest::SHA1.hexdigest(gitlab_ci) + + Page::Project::Show.act { wait_for_push } + Page::Menu::Side.act { click_ci_cd_pipelines } + Page::Project::Pipeline::Index.act { go_to_latest_pipeline } + Page::Project::Pipeline::Show.act { go_to_first_job } + + Page::Project::Job::Show.perform do |job| + expect(job.output).to include(sha1sum) + end + end + end +end diff --git a/qa/spec/git/location_spec.rb b/qa/spec/git/location_spec.rb new file mode 100644 index 00000000000..aef906ee836 --- /dev/null +++ b/qa/spec/git/location_spec.rb @@ -0,0 +1,55 @@ +describe QA::Git::Location do + describe '.new' do + context 'when URI starts with ssh://' do + context 'when URI has port' do + it 'parses correctly' do + uri = described_class + .new('ssh://git@qa.test:2222/sandbox/qa/repo.git') + + expect(uri.user).to eq('git') + expect(uri.host).to eq('qa.test') + expect(uri.port).to eq(2222) + expect(uri.path).to eq('/sandbox/qa/repo.git') + end + end + + context 'when URI does not have port' do + it 'parses correctly' do + uri = described_class + .new('ssh://git@qa.test/sandbox/qa/repo.git') + + expect(uri.user).to eq('git') + expect(uri.host).to eq('qa.test') + expect(uri.port).to eq(22) + expect(uri.path).to eq('/sandbox/qa/repo.git') + end + end + end + + context 'when URI does not start with ssh://' do + context 'when host does not have colons' do + it 'parses correctly' do + uri = described_class + .new('git@qa.test:sandbox/qa/repo.git') + + expect(uri.user).to eq('git') + expect(uri.host).to eq('qa.test') + expect(uri.port).to eq(22) + expect(uri.path).to eq('/sandbox/qa/repo.git') + end + end + + context 'when host has a colon' do + it 'parses correctly' do + uri = described_class + .new('[git@qa:test]:sandbox/qa/repo.git') + + expect(uri.user).to eq('git') + expect(uri.host).to eq('qa%3Atest') + expect(uri.port).to eq(22) + expect(uri.path).to eq('/sandbox/qa/repo.git') + end + end + end + end +end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 6ba599cdf83..f6ba3a581ca 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -180,8 +180,8 @@ FactoryBot.define do trait :artifacts do after(:create) do |build| - create(:ci_job_artifact, :archive, job: build) - create(:ci_job_artifact, :metadata, job: build) + create(:ci_job_artifact, :archive, job: build, expire_at: build.artifacts_expire_at) + create(:ci_job_artifact, :metadata, job: build, expire_at: build.artifacts_expire_at) build.reload end end diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index f7a4a7afced..43cb0dfe163 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -63,13 +63,29 @@ describe ApplicationHelper do end end - describe 'avatar_icon' do + describe 'avatar_icon_for' do + let!(:user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: 'bar@example.com') } + let(:email) { 'foo@example.com' } + let!(:another_user) { create(:user, avatar: File.open(uploaded_image_temp_path), email: email) } + + it 'prefers the user to retrieve the avatar_url' do + expect(helper.avatar_icon_for(user, email).to_s) + .to eq(user.avatar.url) + end + + it 'falls back to email lookup if no user given' do + expect(helper.avatar_icon_for(nil, email).to_s) + .to eq(another_user.avatar.url) + end + end + + describe 'avatar_icon_for_email' do let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) } context 'using an email' do context 'when there is a matching user' do it 'returns a relative URL for the avatar' do - expect(helper.avatar_icon(user.email).to_s) + expect(helper.avatar_icon_for_email(user.email).to_s) .to eq(user.avatar.url) end end @@ -78,17 +94,37 @@ describe ApplicationHelper do it 'calls gravatar_icon' do expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2) - helper.avatar_icon('foo@example.com', 20, 2) + helper.avatar_icon_for_email('foo@example.com', 20, 2) + end + end + + context 'without an email passed' do + it 'calls gravatar_icon' do + expect(helper).to receive(:gravatar_icon).with(nil, 20, 2) + + helper.avatar_icon_for_email(nil, 20, 2) end end end + end + + describe 'avatar_icon_for_user' do + let(:user) { create(:user, avatar: File.open(uploaded_image_temp_path)) } - describe 'using a user' do + context 'with a user object passed' do it 'returns a relative URL for the avatar' do - expect(helper.avatar_icon(user).to_s) + expect(helper.avatar_icon_for_user(user).to_s) .to eq(user.avatar.url) end end + + context 'without a user object passed' do + it 'calls gravatar_icon' do + expect(helper).to receive(:gravatar_icon).with(nil, 20, 2) + + helper.avatar_icon_for_user(nil, 20, 2) + end + end end describe 'gravatar_icon' do diff --git a/spec/helpers/avatars_helper_spec.rb b/spec/helpers/avatars_helper_spec.rb index f44e7ef6843..04c6d259135 100644 --- a/spec/helpers/avatars_helper_spec.rb +++ b/spec/helpers/avatars_helper_spec.rb @@ -29,7 +29,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, 16), + src: avatar_icon_for_user(user, 16), data: { container: 'body' }, class: 'avatar s16 has-tooltip', title: user.name @@ -43,7 +43,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, 16), + src: avatar_icon_for_user(user, 16), data: { container: 'body' }, class: "avatar s16 #{options[:css_class]} has-tooltip", title: user.name @@ -58,7 +58,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, options[:size]), + src: avatar_icon_for_user(user, options[:size]), data: { container: 'body' }, class: "avatar s#{options[:size]} has-tooltip", title: user.name @@ -89,7 +89,7 @@ describe AvatarsHelper do :img, alt: "#{user.name}'s avatar", src: LazyImageTagHelper.placeholder_image, - data: { container: 'body', src: avatar_icon(user, 16) }, + data: { container: 'body', src: avatar_icon_for_user(user, 16) }, class: "avatar s16 has-tooltip lazy", title: user.name ) @@ -104,7 +104,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, 16), + src: avatar_icon_for_user(user, 16), data: { container: 'body' }, class: "avatar s16 has-tooltip", title: user.name @@ -119,7 +119,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, 16), + src: avatar_icon_for_user(user, 16), class: "avatar s16", title: user.name ) @@ -137,7 +137,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{user.name}'s avatar", - src: avatar_icon(user, 16), + src: avatar_icon_for_user(user, 16), data: { container: 'body' }, class: "avatar s16 has-tooltip", title: user.name @@ -149,7 +149,7 @@ describe AvatarsHelper do is_expected.to eq tag( :img, alt: "#{options[:user_name]}'s avatar", - src: avatar_icon(options[:user_email], 16), + src: avatar_icon_for_email(options[:user_email], 16), data: { container: 'body' }, class: "avatar s16 has-tooltip", title: options[:user_name] diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index c0251bf7dc0..b67fee2fcc0 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -215,7 +215,7 @@ describe ProjectsHelper do let(:expected) { double } before do - expect(helper).to receive(:avatar_icon).with(user, 16).and_return(expected) + expect(helper).to receive(:avatar_icon_for_user).with(user, 16).and_return(expected) end it 'returns image tag for member avatar' do diff --git a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb index fb3f29ff4c9..c8fa252439a 100644 --- a/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/populate_untracked_uploads_spec.rb @@ -14,16 +14,10 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :migra let!(:uploads) { described_class::Upload } before do - DatabaseCleaner.clean - drop_temp_table_if_exists ensure_temporary_tracking_table_exists uploads.delete_all end - after(:all) do - drop_temp_table_if_exists - end - context 'with untracked files and tracked files in untracked_files_for_uploads' do let!(:appearance) { create_or_update_appearance(logo: uploaded_file, header_logo: uploaded_file) } let!(:user1) { create(:user, :with_avatar) } @@ -122,9 +116,9 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads, :sidekiq, :migra end it 'drops the temporary tracking table after processing the batch, if there are no untracked rows left' do - subject.perform(1, untracked_files_for_uploads.last.id) + expect(subject).to receive(:drop_temp_table_if_finished) - expect(ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey + subject.perform(1, untracked_files_for_uploads.last.id) end it 'does not block a whole batch because of one bad path' do @@ -260,10 +254,6 @@ describe Gitlab::BackgroundMigration::PopulateUntrackedUploads::UntrackedFile do ensure_temporary_tracking_table_exists end - after(:all) do - drop_temp_table_if_exists - end - describe '#upload_path' do def assert_upload_path(file_path, expected_upload_path) untracked_file = create_untracked_file(file_path) diff --git a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb index 43f3548eadc..ca77e64ae40 100644 --- a/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb +++ b/spec/lib/gitlab/background_migration/prepare_untracked_uploads_spec.rb @@ -1,19 +1,11 @@ require 'spec_helper' -describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do +describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq, :migration, schema: 20180129193323 do include TrackUntrackedUploadsHelpers include MigrationsHelpers let!(:untracked_files_for_uploads) { described_class::UntrackedFile } - before do - DatabaseCleaner.clean - end - - after do - drop_temp_table_if_exists - end - around do |example| # Especially important so the follow-up migration does not get run Sidekiq::Testing.fake! do @@ -76,7 +68,8 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do it 'correctly schedules the follow-up background migration jobs' do described_class.new.perform - expect(described_class::FOLLOW_UP_MIGRATION).to be_scheduled_migration(1, 5) + ids = described_class::UntrackedFile.all.order(:id).pluck(:id) + expect(described_class::FOLLOW_UP_MIGRATION).to be_scheduled_migration(ids.first, ids.last) expect(BackgroundMigrationWorker.jobs.size).to eq(1) end @@ -150,9 +143,11 @@ describe Gitlab::BackgroundMigration::PrepareUntrackedUploads, :sidekiq do # may not have an upload directory because they have no uploads. context 'when no files were ever uploaded' do it 'deletes the `untracked_files_for_uploads` table (and does not raise error)' do - described_class.new.perform + background_migration = described_class.new + + expect(background_migration).to receive(:drop_temp_table) - expect(untracked_files_for_uploads.connection.table_exists?(:untracked_files_for_uploads)).to be_falsey + background_migration.perform end end end diff --git a/spec/migrations/README.md b/spec/migrations/README.md index 45cf25b96de..49760fa62b8 100644 --- a/spec/migrations/README.md +++ b/spec/migrations/README.md @@ -89,5 +89,5 @@ end ## Best practices 1. Note that this type of tests do not run within the transaction, we use -a truncation database cleanup strategy. Do not depend on transaction being +a deletion database cleanup strategy. Do not depend on transaction being present. diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 5d349f45a33..de829011e58 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -81,7 +81,7 @@ describe 'OpenID Connect requests' do it 'includes all user information and group memberships' do request_user_info - expect(json_response).to eq({ + expect(json_response).to match(a_hash_including({ 'sub' => hashed_subject, 'name' => 'Alice', 'nickname' => 'alice', @@ -90,13 +90,12 @@ describe 'OpenID Connect requests' do 'website' => 'https://example.com', 'profile' => 'http://localhost/alice', 'picture' => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png", - 'groups' => - if Group.supports_nested_groups? - ['group1', 'group2/group3', 'group2/group3/group4'] - else - ['group1', 'group2/group3'] - end - }) + 'groups' => anything + })) + + expected_groups = %w[group1 group2/group3] + expected_groups << 'group2/group3/group4' if Group.supports_nested_groups? + expect(json_response['groups']).to match_array(expected_groups) end end diff --git a/spec/services/members/authorized_destroy_service_spec.rb b/spec/services/members/authorized_destroy_service_spec.rb index 757c45708b9..9cf6f64a078 100644 --- a/spec/services/members/authorized_destroy_service_spec.rb +++ b/spec/services/members/authorized_destroy_service_spec.rb @@ -21,6 +21,15 @@ describe Members::AuthorizedDestroyService do .to change { Member.count }.from(3).to(2) end + it "doesn't destroy invited project member notification_settings" do + project.add_developer(member_user) + + member = create :project_member, :invited, project: project + + expect { described_class.new(member, member_user).execute } + .not_to change { NotificationSetting.count } + end + it 'destroys invited group member' do group.add_developer(member_user) @@ -29,38 +38,73 @@ describe Members::AuthorizedDestroyService do expect { described_class.new(member, member_user).execute } .to change { Member.count }.from(2).to(1) end + + it "doesn't destroy invited group member notification_settings" do + group.add_developer(member_user) + + member = create :group_member, :invited, group: group + + expect { described_class.new(member, member_user).execute } + .not_to change { NotificationSetting.count } + end + end + + context 'Requested user' do + it "doesn't destroy member notification_settings" do + member = create(:project_member, user: member_user, requested_at: Time.now) + + expect { described_class.new(member, member_user).execute } + .not_to change { NotificationSetting.count } + end end context 'Group member' do - it "unassigns issues and merge requests" do + let(:member) { group.members.find_by(user_id: member_user.id) } + + before do group.add_developer(member_user) + end + it "unassigns issues and merge requests" do issue = create :issue, project: group_project, assignees: [member_user] create :issue, assignees: [member_user] merge_request = create :merge_request, target_project: group_project, source_project: group_project, assignee: member_user create :merge_request, target_project: project, source_project: project, assignee: member_user - member = group.members.find_by(user_id: member_user.id) - expect { described_class.new(member, member_user).execute } .to change { number_of_assigned_issuables(member_user) }.from(4).to(2) expect(issue.reload.assignee_ids).to be_empty expect(merge_request.reload.assignee_id).to be_nil end + + it 'destroys member notification_settings' do + group.add_developer(member_user) + member = group.members.find_by(user_id: member_user.id) + + expect { described_class.new(member, member_user).execute } + .to change { member_user.notification_settings.count }.by(-1) + end end context 'Project member' do - it "unassigns issues and merge requests" do + let(:member) { project.members.find_by(user_id: member_user.id) } + + before do project.add_developer(member_user) + end + it "unassigns issues and merge requests" do create :issue, project: project, assignees: [member_user] create :merge_request, target_project: project, source_project: project, assignee: member_user - member = project.members.find_by(user_id: member_user.id) - expect { described_class.new(member, member_user).execute } .to change { number_of_assigned_issuables(member_user) }.from(2).to(0) end + + it 'destroys member notification_settings' do + expect { described_class.new(member, member_user).execute } + .to change { member_user.notification_settings.count }.by(-1) + end end end diff --git a/spec/services/projects/create_from_template_service_spec.rb b/spec/services/projects/create_from_template_service_spec.rb index 9919ec254c6..609d678caea 100644 --- a/spec/services/projects/create_from_template_service_spec.rb +++ b/spec/services/projects/create_from_template_service_spec.rb @@ -4,8 +4,10 @@ describe Projects::CreateFromTemplateService do let(:user) { create(:user) } let(:project_params) do { - path: user.to_param, - template_name: 'rails' + path: user.to_param, + template_name: 'rails', + description: 'project description', + visibility_level: Gitlab::VisibilityLevel::PRIVATE } end @@ -22,5 +24,7 @@ describe Projects::CreateFromTemplateService do expect(project).to be_saved expect(project.scheduled?).to be(true) + expect(project.description).to match('project description') + expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) end end diff --git a/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb b/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb index 4e18804b937..9fc2fbef449 100644 --- a/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/custom_attributes_shared_examples.rb @@ -17,12 +17,88 @@ shared_examples 'custom attributes endpoints' do |attributable_name| end end - it 'filters by custom attributes' do - get api("/#{attributable_name}", admin), custom_attributes: { foo: 'foo', bar: 'bar' } + context 'with an authorized user' do + it 'filters by custom attributes' do + get api("/#{attributable_name}", admin), custom_attributes: { foo: 'foo', bar: 'bar' } - expect(response).to have_gitlab_http_status(200) - expect(json_response.size).to be 1 - expect(json_response.first['id']).to eq attributable.id + expect(response).to have_gitlab_http_status(200) + expect(json_response.size).to be 1 + expect(json_response.first['id']).to eq attributable.id + end + end + end + + describe "GET /#{attributable_name} with custom attributes" do + before do + other_attributable + end + + context 'with an unauthorized user' do + it 'does not include custom attributes' do + get api("/#{attributable_name}", user), with_custom_attributes: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response.size).to be 2 + expect(json_response.first).not_to include 'custom_attributes' + end + end + + context 'with an authorized user' do + it 'does not include custom attributes by default' do + get api("/#{attributable_name}", admin) + + expect(response).to have_gitlab_http_status(200) + expect(json_response.size).to be 2 + expect(json_response.first).not_to include 'custom_attributes' + expect(json_response.second).not_to include 'custom_attributes' + end + + it 'includes custom attributes if requested' do + get api("/#{attributable_name}", admin), with_custom_attributes: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response.size).to be 2 + + attributable_response = json_response.find { |r| r['id'] == attributable.id } + other_attributable_response = json_response.find { |r| r['id'] == other_attributable.id } + + expect(attributable_response['custom_attributes']).to contain_exactly( + { 'key' => 'foo', 'value' => 'foo' }, + { 'key' => 'bar', 'value' => 'bar' } + ) + + expect(other_attributable_response['custom_attributes']).to eq [] + end + end + end + + describe "GET /#{attributable_name}/:id with custom attributes" do + context 'with an unauthorized user' do + it 'does not include custom attributes' do + get api("/#{attributable_name}/#{attributable.id}", user), with_custom_attributes: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response).not_to include 'custom_attributes' + end + end + + context 'with an authorized user' do + it 'does not include custom attributes by default' do + get api("/#{attributable_name}/#{attributable.id}", admin) + + expect(response).to have_gitlab_http_status(200) + expect(json_response).not_to include 'custom_attributes' + end + + it 'includes custom attributes if requested' do + get api("/#{attributable_name}/#{attributable.id}", admin), with_custom_attributes: true + + expect(response).to have_gitlab_http_status(200) + expect(json_response['custom_attributes']).to contain_exactly( + { 'key' => 'foo', 'value' => 'foo' }, + { 'key' => 'bar', 'value' => 'bar' } + ) + end end end @@ -33,14 +109,16 @@ shared_examples 'custom attributes endpoints' do |attributable_name| it_behaves_like 'an unauthorized API user' end - it 'returns all custom attributes' do - get api("/#{attributable_name}/#{attributable.id}/custom_attributes", admin) + context 'with an authorized user' do + it 'returns all custom attributes' do + get api("/#{attributable_name}/#{attributable.id}/custom_attributes", admin) - expect(response).to have_gitlab_http_status(200) - expect(json_response).to contain_exactly( - { 'key' => 'foo', 'value' => 'foo' }, - { 'key' => 'bar', 'value' => 'bar' } - ) + expect(response).to have_gitlab_http_status(200) + expect(json_response).to contain_exactly( + { 'key' => 'foo', 'value' => 'foo' }, + { 'key' => 'bar', 'value' => 'bar' } + ) + end end end @@ -51,11 +129,13 @@ shared_examples 'custom attributes endpoints' do |attributable_name| it_behaves_like 'an unauthorized API user' end - it 'returns a single custom attribute' do - get api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin) + context 'with an authorized user' do + it'returns a single custom attribute' do + get api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin) - expect(response).to have_gitlab_http_status(200) - expect(json_response).to eq({ 'key' => 'foo', 'value' => 'foo' }) + expect(response).to have_gitlab_http_status(200) + expect(json_response).to eq({ 'key' => 'foo', 'value' => 'foo' }) + end end end @@ -66,24 +146,26 @@ shared_examples 'custom attributes endpoints' do |attributable_name| it_behaves_like 'an unauthorized API user' end - it 'creates a new custom attribute' do - expect do - put api("/#{attributable_name}/#{attributable.id}/custom_attributes/new", admin), value: 'new' - end.to change { attributable.custom_attributes.count }.by(1) + context 'with an authorized user' do + it 'creates a new custom attribute' do + expect do + put api("/#{attributable_name}/#{attributable.id}/custom_attributes/new", admin), value: 'new' + end.to change { attributable.custom_attributes.count }.by(1) - expect(response).to have_gitlab_http_status(200) - expect(json_response).to eq({ 'key' => 'new', 'value' => 'new' }) - expect(attributable.custom_attributes.find_by(key: 'new').value).to eq 'new' - end + expect(response).to have_gitlab_http_status(200) + expect(json_response).to eq({ 'key' => 'new', 'value' => 'new' }) + expect(attributable.custom_attributes.find_by(key: 'new').value).to eq 'new' + end - it 'updates an existing custom attribute' do - expect do - put api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin), value: 'new' - end.not_to change { attributable.custom_attributes.count } + it 'updates an existing custom attribute' do + expect do + put api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin), value: 'new' + end.not_to change { attributable.custom_attributes.count } - expect(response).to have_gitlab_http_status(200) - expect(json_response).to eq({ 'key' => 'foo', 'value' => 'new' }) - expect(custom_attribute1.reload.value).to eq 'new' + expect(response).to have_gitlab_http_status(200) + expect(json_response).to eq({ 'key' => 'foo', 'value' => 'new' }) + expect(custom_attribute1.reload.value).to eq 'new' + end end end @@ -94,13 +176,15 @@ shared_examples 'custom attributes endpoints' do |attributable_name| it_behaves_like 'an unauthorized API user' end - it 'deletes an existing custom attribute' do - expect do - delete api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin) - end.to change { attributable.custom_attributes.count }.by(-1) + context 'with an authorized user' do + it 'deletes an existing custom attribute' do + expect do + delete api("/#{attributable_name}/#{attributable.id}/custom_attributes/foo", admin) + end.to change { attributable.custom_attributes.count }.by(-1) - expect(response).to have_gitlab_http_status(204) - expect(attributable.custom_attributes.find_by(key: 'foo')).to be_nil + expect(response).to have_gitlab_http_status(204) + expect(attributable.custom_attributes.find_by(key: 'foo')).to be_nil + end end end end diff --git a/spec/support/track_untracked_uploads_helpers.rb b/spec/support/track_untracked_uploads_helpers.rb index 5752078d2a0..a8b3ed1f41c 100644 --- a/spec/support/track_untracked_uploads_helpers.rb +++ b/spec/support/track_untracked_uploads_helpers.rb @@ -8,10 +8,6 @@ module TrackUntrackedUploadsHelpers Gitlab::BackgroundMigration::PrepareUntrackedUploads.new.send(:ensure_temporary_tracking_table_exists) end - def drop_temp_table_if_exists - ActiveRecord::Base.connection.drop_table(:untracked_files_for_uploads) if ActiveRecord::Base.connection.table_exists?(:untracked_files_for_uploads) - end - def create_or_update_appearance(attrs) a = Appearance.first_or_initialize(title: 'foo', description: 'bar') a.update!(attrs) |