diff options
30 files changed, 336 insertions, 77 deletions
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index 0d91a54c7d4..9e11b32fcaa 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -0.3.0 +0.3.1 @@ -34,7 +34,7 @@ gem 'omniauth-saml', '~> 1.7.0' gem 'omniauth-shibboleth', '~> 1.2.0' gem 'omniauth-twitter', '~> 1.2.0' gem 'omniauth_crowd', '~> 2.2.0' -gem 'omniauth-authentiq', '~> 0.2.0' +gem 'omniauth-authentiq', '~> 0.3.0' gem 'rack-oauth2', '~> 1.2.1' gem 'jwt', '~> 1.5.6' diff --git a/Gemfile.lock b/Gemfile.lock index a3c2fad41ba..4f98dc9d085 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -448,7 +448,7 @@ GEM rack (>= 1.0, < 3) omniauth-auth0 (1.4.1) omniauth-oauth2 (~> 1.1) - omniauth-authentiq (0.2.2) + omniauth-authentiq (0.3.0) omniauth-oauth2 (~> 1.3, >= 1.3.1) omniauth-azure-oauth2 (0.0.6) jwt (~> 1.0) @@ -925,7 +925,7 @@ DEPENDENCIES oj (~> 2.17.4) omniauth (~> 1.3.2) omniauth-auth0 (~> 1.4.1) - omniauth-authentiq (~> 0.2.0) + omniauth-authentiq (~> 0.3.0) omniauth-azure-oauth2 (~> 0.0.6) omniauth-cas3 (~> 1.1.2) omniauth-facebook (~> 4.0.0) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index f54c79c2e37..3ab7e6e0658 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -78,6 +78,13 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController handle_omniauth end + def authentiq + if params['sid'] + handle_service_ticket oauth['provider'], params['sid'] + end + handle_omniauth + end + private def handle_omniauth diff --git a/changelogs/unreleased/26315-unify-labels-filter-behavior.yml b/changelogs/unreleased/26315-unify-labels-filter-behavior.yml new file mode 100644 index 00000000000..cd2f40c94fe --- /dev/null +++ b/changelogs/unreleased/26315-unify-labels-filter-behavior.yml @@ -0,0 +1,4 @@ +--- +title: Unify issues search behavior by always filtering when ALL labels matches +merge_request: 8849 +author: diff --git a/changelogs/unreleased/28204-option-to-disable-webpack-dev-server-livereload.yml b/changelogs/unreleased/28204-option-to-disable-webpack-dev-server-livereload.yml new file mode 100644 index 00000000000..df2478a3f28 --- /dev/null +++ b/changelogs/unreleased/28204-option-to-disable-webpack-dev-server-livereload.yml @@ -0,0 +1,4 @@ +--- +title: Pick up option from GDK to disable webpack dev server livereload +merge_request: +author: diff --git a/changelogs/unreleased/9381-authentiq-backchannel-logout.yml b/changelogs/unreleased/9381-authentiq-backchannel-logout.yml new file mode 100644 index 00000000000..4dbf36cd096 --- /dev/null +++ b/changelogs/unreleased/9381-authentiq-backchannel-logout.yml @@ -0,0 +1,4 @@ +--- +title: Adds remote logout functionality to the Authentiq OAuth provider +merge_request: 9381 +author: Alexandros Keramidas diff --git a/changelogs/unreleased/artifactsdoc.yml b/changelogs/unreleased/artifactsdoc.yml new file mode 100644 index 00000000000..4ef32d5256f --- /dev/null +++ b/changelogs/unreleased/artifactsdoc.yml @@ -0,0 +1,4 @@ +--- +title: Added documentation for permalinks to most recent build artifacts. +merge_request: 8934 +author: Christian Godenschwager diff --git a/changelogs/unreleased/feature-github-find-users-by-email.yml b/changelogs/unreleased/feature-github-find-users-by-email.yml new file mode 100644 index 00000000000..1503cf2b9f7 --- /dev/null +++ b/changelogs/unreleased/feature-github-find-users-by-email.yml @@ -0,0 +1,4 @@ +--- +title: GitHub Importer - Find users based on GitHub email address +merge_request: 8958 +author: diff --git a/changelogs/unreleased/updated-pages-0-3-1.yml b/changelogs/unreleased/updated-pages-0-3-1.yml new file mode 100644 index 00000000000..8622b823c86 --- /dev/null +++ b/changelogs/unreleased/updated-pages-0-3-1.yml @@ -0,0 +1,4 @@ +--- +title: Update GitLab Pages to v0.3.1 +merge_request: +author: diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index a8afc36fc78..738dbeefc11 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -240,6 +240,17 @@ Devise.setup do |config| true end end + if provider['name'] == 'authentiq' + provider['args'][:remote_sign_out_handler] = lambda do |request| + authentiq_session = request.params['sid'] + if Gitlab::OAuth::Session.valid?(:authentiq, authentiq_session) + Gitlab::OAuth::Session.destroy(:authentiq, authentiq_session) + true + else + false + end + end + end if provider['name'] == 'shibboleth' provider['args'][:fail_with_empty_uid] = true diff --git a/config/webpack.config.js b/config/webpack.config.js index 515569b2e97..15899993874 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -10,6 +10,7 @@ var ROOT_PATH = path.resolve(__dirname, '..'); var IS_PRODUCTION = process.env.NODE_ENV === 'production'; var IS_DEV_SERVER = process.argv[1].indexOf('webpack-dev-server') !== -1; var DEV_SERVER_PORT = parseInt(process.env.DEV_SERVER_PORT, 10) || 3808; +var DEV_SERVER_LIVERELOAD = process.env.DEV_SERVER_LIVERELOAD !== 'false'; var config = { context: path.join(ROOT_PATH, 'app/assets/javascripts'), @@ -114,6 +115,7 @@ if (IS_DEV_SERVER) { port: DEV_SERVER_PORT, headers: { 'Access-Control-Allow-Origin': '*' }, stats: 'errors-only', + inline: DEV_SERVER_LIVERELOAD }; config.output.publicPath = '//localhost:' + DEV_SERVER_PORT + config.output.publicPath; } diff --git a/doc/administration/auth/authentiq.md b/doc/administration/auth/authentiq.md index 3f39539da95..fb1a16b0f96 100644 --- a/doc/administration/auth/authentiq.md +++ b/doc/administration/auth/authentiq.md @@ -54,7 +54,7 @@ Authentiq will generate a Client ID and the accompanying Client Secret for you t 5. The `scope` is set to request the user's name, email (required and signed), and permission to send push notifications to sign in on subsequent visits. See [OmniAuth Authentiq strategy](https://github.com/AuthentiqID/omniauth-authentiq#scopes-and-redirect-uri-configuration) for more information on scopes and modifiers. -6. Change 'YOUR_CLIENT_ID' and 'YOUR_CLIENT_SECRET' to the Client credentials you received in step 1. +6. Change `YOUR_CLIENT_ID` and `YOUR_CLIENT_SECRET` to the Client credentials you received in step 1. 7. Save the configuration file. diff --git a/doc/api/issues.md b/doc/api/issues.md index 7c0a444d4fa..f40e0938b0f 100644 --- a/doc/api/issues.md +++ b/doc/api/issues.md @@ -30,7 +30,7 @@ GET /issues?milestone=1.0.0&state=opened | Attribute | Type | Required | Description | | --------- | ---- | -------- | ----------- | | `state` | string | no | Return all issues or just those that are `opened` or `closed`| -| `labels` | string | no | Comma-separated list of label names, issues with any of the labels will be returned | +| `labels` | string | no | Comma-separated list of label names, issues must have all labels to be returned | | `milestone` | string| no | The milestone title | | `order_by`| string | no | Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` | | `sort` | string | no | Return requests sorted in `asc` or `desc` order. Default is `desc` | @@ -188,7 +188,7 @@ GET /projects/:id/issues?milestone=1.0.0&state=opened | `id` | integer | yes | The ID of a project | | `iid` | integer | no | Return the issue having the given `iid` | | `state` | string | no | Return all issues or just those that are `opened` or `closed`| -| `labels` | string | no | Comma-separated list of label names, issues with any of the labels will be returned | +| `labels` | string | no | Comma-separated list of label names, issues must have all labels to be returned | | `milestone` | string| no | The milestone title | | `order_by`| string | no | Return requests ordered by `created_at` or `updated_at` fields. Default is `created_at` | | `sort` | string | no | Return requests sorted in `asc` or `desc` order. Default is `desc` | diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md index 49f140a37f6..3f58c098b43 100644 --- a/doc/api/v3_to_v4.md +++ b/doc/api/v3_to_v4.md @@ -28,3 +28,5 @@ changes are in V4: - Return pagination headers for all endpoints that return an array - Removed `DELETE projects/:id/deploy_keys/:key_id/disable`. Use `DELETE projects/:id/deploy_keys/:key_id` instead - Moved `PUT /users/:id/(block|unblock)` to `POST /users/:id/(block|unblock)` +- Labels filter on `projects/:id/issues` and `/issues` now matches only issues containing all labels (i.e.: Logical AND, not OR) + diff --git a/doc/development/testing.md b/doc/development/testing.md index 500cbefcffb..9b545d7f0f1 100644 --- a/doc/development/testing.md +++ b/doc/development/testing.md @@ -95,6 +95,25 @@ so we need to set some guidelines for their use going forward: [lets-not]: https://robots.thoughtbot.com/lets-not +### Time-sensitive tests + +[Timecop](https://github.com/travisjeffery/timecop) is available in our +Ruby-based tests for verifying things that are time-sensitive. Any test that +exercises or verifies something time-sensitive should make use of Timecop to +prevent transient test failures. + +Example: + +```ruby +it 'is overdue' do + issue = build(:issue, due_date: Date.tomorrow) + + Timecop.freeze(3.days.from_now) do + expect(issue).to be_overdue + end +end +``` + ### Test speed GitLab has a massive test suite that, without parallelization, can take more diff --git a/doc/user/project/pipelines/job_artifacts.md b/doc/user/project/pipelines/job_artifacts.md index f85f4bf8e1e..5ce99843301 100644 --- a/doc/user/project/pipelines/job_artifacts.md +++ b/doc/user/project/pipelines/job_artifacts.md @@ -90,18 +90,43 @@ inside GitLab that make that possible. It is possible to download the latest artifacts of a job via a well known URL so you can use it for scripting purposes. -The structure of the URL is the following: +The structure of the URL to download the whole artifacts archive is the following: ``` https://example.com/<namespace>/<project>/builds/artifacts/<ref>/download?job=<job_name> ``` -For example, to download the latest artifacts of the job named `rspec 6 20` of +To download a single file from the artifacts use the following URL: + +``` +https://example.com/<namespace>/<project>/builds/artifacts/<ref>/file/<path_to_file>?job=<job_name> +``` + +For example, to download the latest artifacts of the job named `coverage` of the `master` branch of the `gitlab-ce` project that belongs to the `gitlab-org` namespace, the URL would be: ``` -https://gitlab.com/gitlab-org/gitlab-ce/builds/artifacts/master/download?job=rspec+6+20 +https://gitlab.com/gitlab-org/gitlab-ce/builds/artifacts/master/download?job=coverage +``` + +To download the file `coverage/index.html` from the same +artifacts use the following URL: + +``` +https://gitlab.com/gitlab-org/gitlab-ce/builds/artifacts/master/file/coverage/index.html?job=coverage +``` + +There is also a URL to browse the latest job artifacts: + +``` +https://example.com/<namespace>/<project>/builds/artifacts/<ref>/browse?job=<job_name> +``` + +For example: + +``` +https://gitlab.com/gitlab-org/gitlab-ce/builds/artifacts/master/browse?job=coverage ``` The latest builds are also exposed in the UI in various places. Specifically, diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 90fca20d4fa..2b946bfd349 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -10,17 +10,9 @@ module API args.delete(:id) args[:milestone_title] = args.delete(:milestone) + args[:label_name] = args.delete(:labels) - match_all_labels = args.delete(:match_all_labels) - labels = args.delete(:labels) - args[:label_name] = labels if match_all_labels - - issues = IssuesFinder.new(current_user, args).execute.inc_notes_with_associations - - # TODO: Remove in 9.0 pass `label_name: args.delete(:labels)` to IssuesFinder - if !match_all_labels && labels.present? - issues = issues.includes(:labels).where('labels.title' => labels.split(',')) - end + issues = IssuesFinder.new(current_user, args).execute issues.reorder(args[:order_by] => args[:sort]) end @@ -77,7 +69,7 @@ module API get ":id/issues" do group = find_group!(params[:id]) - issues = find_issues(group_id: group.id, state: params[:state] || 'opened', match_all_labels: true) + issues = find_issues(group_id: group.id, state: params[:state] || 'opened') present paginate(issues), with: Entities::Issue, current_user: current_user end diff --git a/lib/gitlab/github_import/base_formatter.rb b/lib/gitlab/github_import/base_formatter.rb index 95dba9a327b..8c80791e7c9 100644 --- a/lib/gitlab/github_import/base_formatter.rb +++ b/lib/gitlab/github_import/base_formatter.rb @@ -1,11 +1,12 @@ module Gitlab module GithubImport class BaseFormatter - attr_reader :formatter, :project, :raw_data + attr_reader :client, :formatter, :project, :raw_data - def initialize(project, raw_data) + def initialize(project, raw_data, client = nil) @project = project @raw_data = raw_data + @client = client @formatter = Gitlab::ImportFormatter.new end @@ -18,19 +19,6 @@ module Gitlab def url raw_data.url || '' end - - private - - def gitlab_user_id(github_id) - User.joins(:identities). - find_by("identities.extern_uid = ? AND identities.provider = 'github'", github_id.to_s). - try(:id) - end - - def gitlab_author_id - return @gitlab_author_id if defined?(@gitlab_author_id) - @gitlab_author_id = gitlab_user_id(raw_data.user.id) - end end end end diff --git a/lib/gitlab/github_import/client.rb b/lib/gitlab/github_import/client.rb index ba869faa92e..7dbeec5b010 100644 --- a/lib/gitlab/github_import/client.rb +++ b/lib/gitlab/github_import/client.rb @@ -10,6 +10,7 @@ module Gitlab @access_token = access_token @host = host.to_s.sub(%r{/+\z}, '') @api_version = api_version + @users = {} if access_token ::Octokit.auto_paginate = false @@ -64,6 +65,13 @@ module Gitlab api.respond_to?(method) || super end + def user(login) + return nil unless login.present? + return @users[login] if @users.key?(login) + + @users[login] = api.user(login) + end + private def api_endpoint diff --git a/lib/gitlab/github_import/comment_formatter.rb b/lib/gitlab/github_import/comment_formatter.rb index 2bddcde2b7c..e21922070c1 100644 --- a/lib/gitlab/github_import/comment_formatter.rb +++ b/lib/gitlab/github_import/comment_formatter.rb @@ -1,6 +1,8 @@ module Gitlab module GithubImport class CommentFormatter < BaseFormatter + attr_writer :author_id + def attributes { project: project, @@ -17,11 +19,11 @@ module Gitlab private def author - raw_data.user.login + @author ||= UserFormatter.new(client, raw_data.user) end def author_id - gitlab_author_id || project.creator_id + author.gitlab_id || project.creator_id end def body @@ -52,10 +54,10 @@ module Gitlab end def note - if gitlab_author_id + if author.gitlab_id body else - formatter.author_line(author) + body + formatter.author_line(author.login) + body end end diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index 9a4ffd28438..d95ff4fd104 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -110,7 +110,7 @@ module Gitlab def import_issues fetch_resources(:issues, repo, state: :all, sort: :created, direction: :asc, per_page: 100) do |issues| issues.each do |raw| - gh_issue = IssueFormatter.new(project, raw) + gh_issue = IssueFormatter.new(project, raw, client) begin issuable = @@ -131,7 +131,8 @@ module Gitlab def import_pull_requests fetch_resources(:pull_requests, repo, state: :all, sort: :created, direction: :asc, per_page: 100) do |pull_requests| pull_requests.each do |raw| - gh_pull_request = PullRequestFormatter.new(project, raw) + gh_pull_request = PullRequestFormatter.new(project, raw, client) + next unless gh_pull_request.valid? begin @@ -209,14 +210,16 @@ module Gitlab ActiveRecord::Base.no_touching do comments.each do |raw| begin - comment = CommentFormatter.new(project, raw) + comment = CommentFormatter.new(project, raw, client) + # GH does not return info about comment's parent, so we guess it by checking its URL! *_, parent, iid = URI(raw.html_url).path.split('/') - if parent == 'issues' - issuable = Issue.find_by(project_id: project.id, iid: iid) - else - issuable = MergeRequest.find_by(target_project_id: project.id, iid: iid) - end + + issuable = if parent == 'issues' + Issue.find_by(project_id: project.id, iid: iid) + else + MergeRequest.find_by(target_project_id: project.id, iid: iid) + end next unless issuable diff --git a/lib/gitlab/github_import/issuable_formatter.rb b/lib/gitlab/github_import/issuable_formatter.rb index 256f360efc7..29fb0f9d333 100644 --- a/lib/gitlab/github_import/issuable_formatter.rb +++ b/lib/gitlab/github_import/issuable_formatter.rb @@ -1,6 +1,8 @@ module Gitlab module GithubImport class IssuableFormatter < BaseFormatter + attr_writer :assignee_id, :author_id + def project_association raise NotImplementedError end @@ -23,18 +25,24 @@ module Gitlab raw_data.assignee.present? end - def assignee_id + def author + @author ||= UserFormatter.new(client, raw_data.user) + end + + def author_id + @author_id ||= author.gitlab_id || project.creator_id + end + + def assignee if assigned? - gitlab_user_id(raw_data.assignee.id) + @assignee ||= UserFormatter.new(client, raw_data.assignee) end end - def author - raw_data.user.login - end + def assignee_id + return @assignee_id if defined?(@assignee_id) - def author_id - gitlab_author_id || project.creator_id + @assignee_id = assignee.try(:gitlab_id) end def body @@ -42,10 +50,10 @@ module Gitlab end def description - if gitlab_author_id + if author.gitlab_id body else - formatter.author_line(author) + body + formatter.author_line(author.login) + body end end diff --git a/lib/gitlab/github_import/user_formatter.rb b/lib/gitlab/github_import/user_formatter.rb new file mode 100644 index 00000000000..04c2964da20 --- /dev/null +++ b/lib/gitlab/github_import/user_formatter.rb @@ -0,0 +1,45 @@ +module Gitlab + module GithubImport + class UserFormatter + attr_reader :client, :raw + + delegate :id, :login, to: :raw, allow_nil: true + + def initialize(client, raw) + @client = client + @raw = raw + end + + def gitlab_id + return @gitlab_id if defined?(@gitlab_id) + + @gitlab_id = find_by_external_uid || find_by_email + end + + private + + def email + @email ||= client.user(raw.login).try(:email) + end + + def find_by_email + return nil unless email + + User.find_by_any_email(email) + .try(:id) + end + + def find_by_external_uid + return nil unless id + + identities = ::Identity.arel_table + + User.select(:id) + .joins(:identities).where(identities[:provider].eq(:github) + .and(identities[:extern_uid].eq(id))) + .first + .try(:id) + end + end + end +end diff --git a/spec/lib/gitlab/github_import/comment_formatter_spec.rb b/spec/lib/gitlab/github_import/comment_formatter_spec.rb index e6e33d3686a..cc38872e426 100644 --- a/spec/lib/gitlab/github_import/comment_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/comment_formatter_spec.rb @@ -1,8 +1,9 @@ require 'spec_helper' describe Gitlab::GithubImport::CommentFormatter, lib: true do + let(:client) { double } let(:project) { create(:empty_project) } - let(:octocat) { double(id: 123456, login: 'octocat') } + let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } let(:created_at) { DateTime.strptime('2013-04-10T20:09:31Z') } let(:updated_at) { DateTime.strptime('2014-03-03T18:58:10Z') } let(:base) do @@ -16,7 +17,11 @@ describe Gitlab::GithubImport::CommentFormatter, lib: true do } end - subject(:comment) { described_class.new(project, raw)} + subject(:comment) { described_class.new(project, raw, client) } + + before do + allow(client).to receive(:user).and_return(octocat) + end describe '#attributes' do context 'when do not reference a portion of the diff' do @@ -69,8 +74,15 @@ describe Gitlab::GithubImport::CommentFormatter, lib: true do context 'when author is a GitLab user' do let(:raw) { double(base.merge(user: octocat)) } - it 'returns GitLab user id as author_id' do + it 'returns GitLab user id associated with GitHub id as author_id' do gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(comment.attributes.fetch(:author_id)).to eq gl_user.id + end + + it 'returns GitLab user id associated with GitHub email as author_id' do + gl_user = create(:user, email: octocat.email) + expect(comment.attributes.fetch(:author_id)).to eq gl_user.id end diff --git a/spec/lib/gitlab/github_import/importer_spec.rb b/spec/lib/gitlab/github_import/importer_spec.rb index afd78abdc9b..33d83d6d2f1 100644 --- a/spec/lib/gitlab/github_import/importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer_spec.rb @@ -44,6 +44,7 @@ describe Gitlab::GithubImport::Importer, lib: true do allow_any_instance_of(Octokit::Client).to receive(:rate_limit!).and_raise(Octokit::NotFound) allow_any_instance_of(Gitlab::Shell).to receive(:import_repository).and_raise(Gitlab::Shell::Error) + allow_any_instance_of(Octokit::Client).to receive(:user).and_return(octocat) allow_any_instance_of(Octokit::Client).to receive(:labels).and_return([label1, label2]) allow_any_instance_of(Octokit::Client).to receive(:milestones).and_return([milestone, milestone]) allow_any_instance_of(Octokit::Client).to receive(:issues).and_return([issue1, issue2]) @@ -53,7 +54,8 @@ describe Gitlab::GithubImport::Importer, lib: true do allow_any_instance_of(Octokit::Client).to receive(:last_response).and_return(double(rels: { next: nil })) allow_any_instance_of(Octokit::Client).to receive(:releases).and_return([release1, release2]) end - let(:octocat) { double(id: 123456, login: 'octocat') } + + let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } let(:label1) do @@ -125,6 +127,7 @@ describe Gitlab::GithubImport::Importer, lib: true do ) end + let!(:user) { create(:user, email: octocat.email) } let(:repository) { double(id: 1, fork: false) } let(:source_sha) { create(:commit, project: project).id } let(:source_branch) { double(ref: 'feature', repo: repository, sha: source_sha) } diff --git a/spec/lib/gitlab/github_import/issue_formatter_spec.rb b/spec/lib/gitlab/github_import/issue_formatter_spec.rb index eec1fabab54..f34d09f2c1d 100644 --- a/spec/lib/gitlab/github_import/issue_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/issue_formatter_spec.rb @@ -1,8 +1,9 @@ require 'spec_helper' describe Gitlab::GithubImport::IssueFormatter, lib: true do + let(:client) { double } let!(:project) { create(:empty_project, namespace: create(:namespace, path: 'octocat')) } - let(:octocat) { double(id: 123456, login: 'octocat') } + let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } @@ -23,7 +24,11 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do } end - subject(:issue) { described_class.new(project, raw_data) } + subject(:issue) { described_class.new(project, raw_data, client) } + + before do + allow(client).to receive(:user).and_return(octocat) + end shared_examples 'Gitlab::GithubImport::IssueFormatter#attributes' do context 'when issue is open' do @@ -75,11 +80,17 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do expect(issue.attributes.fetch(:assignee_id)).to be_nil end - it 'returns GitLab user id as assignee_id when is a GitLab user' do + it 'returns GitLab user id associated with GitHub id as assignee_id' do gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') expect(issue.attributes.fetch(:assignee_id)).to eq gl_user.id end + + it 'returns GitLab user id associated with GitHub email as assignee_id' do + gl_user = create(:user, email: octocat.email) + + expect(issue.attributes.fetch(:assignee_id)).to eq gl_user.id + end end context 'when it has a milestone' do @@ -100,16 +111,22 @@ describe Gitlab::GithubImport::IssueFormatter, lib: true do context 'when author is a GitLab user' do let(:raw_data) { double(base_data.merge(user: octocat)) } - it 'returns project#creator_id as author_id when is not a GitLab user' do + it 'returns project creator_id as author_id when is not a GitLab user' do expect(issue.attributes.fetch(:author_id)).to eq project.creator_id end - it 'returns GitLab user id as author_id when is a GitLab user' do + it 'returns GitLab user id associated with GitHub id as author_id' do gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') expect(issue.attributes.fetch(:author_id)).to eq gl_user.id end + it 'returns GitLab user id associated with GitHub email as author_id' do + gl_user = create(:user, email: octocat.email) + + expect(issue.attributes.fetch(:author_id)).to eq gl_user.id + end + it 'returns description without created at tag line' do create(:omniauth_user, extern_uid: octocat.id, provider: 'github') diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index 90947ff4707..e46be18aa99 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' describe Gitlab::GithubImport::PullRequestFormatter, lib: true do + let(:client) { double } let(:project) { create(:project, :repository) } let(:source_sha) { create(:commit, project: project).id } let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } @@ -10,7 +11,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do let(:target_repo) { repository } let(:target_branch) { double(ref: 'master', repo: target_repo, sha: target_sha) } let(:removed_branch) { double(ref: 'removed-branch', repo: source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') } - let(:octocat) { double(id: 123456, login: 'octocat') } + let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } let(:base_data) do @@ -32,7 +33,11 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do } end - subject(:pull_request) { described_class.new(project, raw_data) } + subject(:pull_request) { described_class.new(project, raw_data, client) } + + before do + allow(client).to receive(:user).and_return(octocat) + end shared_examples 'Gitlab::GithubImport::PullRequestFormatter#attributes' do context 'when pull request is open' do @@ -121,26 +126,38 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do expect(pull_request.attributes.fetch(:assignee_id)).to be_nil end - it 'returns GitLab user id as assignee_id when is a GitLab user' do + it 'returns GitLab user id associated with GitHub id as assignee_id' do gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') expect(pull_request.attributes.fetch(:assignee_id)).to eq gl_user.id end + + it 'returns GitLab user id associated with GitHub email as assignee_id' do + gl_user = create(:user, email: octocat.email) + + expect(pull_request.attributes.fetch(:assignee_id)).to eq gl_user.id + end end context 'when author is a GitLab user' do let(:raw_data) { double(base_data.merge(user: octocat)) } - it 'returns project#creator_id as author_id when is not a GitLab user' do + it 'returns project creator_id as author_id when is not a GitLab user' do expect(pull_request.attributes.fetch(:author_id)).to eq project.creator_id end - it 'returns GitLab user id as author_id when is a GitLab user' do + it 'returns GitLab user id associated with GitHub id as author_id' do gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') expect(pull_request.attributes.fetch(:author_id)).to eq gl_user.id end + it 'returns GitLab user id associated with GitHub email as author_id' do + gl_user = create(:user, email: octocat.email) + + expect(pull_request.attributes.fetch(:author_id)).to eq gl_user.id + end + it 'returns description without created at tag line' do create(:omniauth_user, extern_uid: octocat.id, provider: 'github') diff --git a/spec/lib/gitlab/github_import/user_formatter_spec.rb b/spec/lib/gitlab/github_import/user_formatter_spec.rb new file mode 100644 index 00000000000..db792233657 --- /dev/null +++ b/spec/lib/gitlab/github_import/user_formatter_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe Gitlab::GithubImport::UserFormatter, lib: true do + let(:client) { double } + let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } + + subject(:user) { described_class.new(client, octocat) } + + before do + allow(client).to receive(:user).and_return(octocat) + end + + describe '#gitlab_id' do + context 'when GitHub user is a GitLab user' do + it 'return GitLab user id when user associated their account with GitHub' do + gl_user = create(:omniauth_user, extern_uid: octocat.id, provider: 'github') + + expect(user.gitlab_id).to eq gl_user.id + end + + it 'returns GitLab user id when user primary email matches GitHub email' do + gl_user = create(:user, email: octocat.email) + + expect(user.gitlab_id).to eq gl_user.id + end + + it 'returns GitLab user id when any of user linked emails matches GitHub email' do + gl_user = create(:user, email: 'johndoe@example.com') + create(:email, user: gl_user, email: octocat.email) + + expect(user.gitlab_id).to eq gl_user.id + end + end + + it 'returns nil when GitHub user is not a GitLab user' do + expect(user.gitlab_id).to be_nil + end + end +end diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 74ac7955cb8..ece1b43567d 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -117,14 +117,20 @@ describe API::Issues, api: true do expect(json_response.first['labels']).to eq([label.title]) end - it 'returns an array of labeled issues when at least one label matches' do - get api("/issues?labels=#{label.title},foo,bar", user) + it 'returns an array of labeled issues when all labels matches' do + label_b = create(:label, title: 'foo', project: project) + label_c = create(:label, title: 'bar', project: project) + + create(:label_link, label: label_b, target: issue) + create(:label_link, label: label_c, target: issue) + + get api("/issues", user), labels: "#{label.title},#{label_b.title},#{label_c.title}" expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) - expect(json_response.first['labels']).to eq([label.title]) + expect(json_response.first['labels']).to eq([label_c.title, label_b.title, label.title]) end it 'returns an empty array if no issue matches labels' do @@ -356,6 +362,21 @@ describe API::Issues, api: true do expect(json_response.length).to eq(0) end + it 'returns an array of labeled issues when all labels matches' do + label_b = create(:label, title: 'foo', project: group_project) + label_c = create(:label, title: 'bar', project: group_project) + + create(:label_link, label: label_b, target: group_issue) + create(:label_link, label: label_c, target: group_issue) + + get api("#{base_url}", user), labels: "#{group_label.title},#{label_b.title},#{label_c.title}" + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + expect(json_response.first['labels']).to eq([label_c.title, label_b.title, group_label.title]) + end + it 'returns an empty array if no group issue matches labels' do get api("#{base_url}?labels=foo,bar", user) @@ -549,14 +570,28 @@ describe API::Issues, api: true do expect(json_response.first['labels']).to eq([label.title]) end - it 'returns an array of labeled project issues where all labels match' do - get api("#{base_url}/issues?labels=#{label.title},foo,bar", user) + it 'returns an array of labeled issues when all labels matches' do + label_b = create(:label, title: 'foo', project: project) + label_c = create(:label, title: 'bar', project: project) + + create(:label_link, label: label_b, target: issue) + create(:label_link, label: label_c, target: issue) + + get api("#{base_url}/issues", user), labels: "#{label.title},#{label_b.title},#{label_c.title}" expect(response).to have_http_status(200) expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.length).to eq(1) - expect(json_response.first['labels']).to eq([label.title]) + expect(json_response.first['labels']).to eq([label_c.title, label_b.title, label.title]) + end + + it 'returns an empty array if not all labels matches' do + get api("#{base_url}/issues?labels=#{label.title},foo", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(0) end it 'returns an empty array if no project issue matches labels' do |