diff options
author | Filipa Lacerda <filipa@gitlab.com> | 2017-03-10 23:11:50 +0000 |
---|---|---|
committer | Filipa Lacerda <filipa@gitlab.com> | 2017-03-10 23:11:50 +0000 |
commit | 6b1b616ea9e30055f8fdab7c7ea012ac2d5c7269 (patch) | |
tree | c7394d00329721530d73423d7e3895482bf996dd | |
parent | e8d525d07c30329d7cbe7b418296d0d9b9a391c0 (diff) | |
parent | e78aed49cd81e2873d7a536bc13e2e77435e290e (diff) | |
download | gitlab-ce-6b1b616ea9e30055f8fdab7c7ea012ac2d5c7269.tar.gz |
Merge branch 'master' into 29326-update-documentation
* master:
Using guard clause and added more specs
Changelog
Fix GitHub Import for open PRs from a fork
Add frequently used emojis back to awards menu
-rw-r--r-- | app/assets/javascripts/awards_handler.js | 30 | ||||
-rw-r--r-- | changelogs/unreleased/29046-fix-github-importer-open-prs.yml | 4 | ||||
-rw-r--r-- | changelogs/unreleased/add-frequently-used-emojis-back-to-menu.yml | 4 | ||||
-rw-r--r-- | lib/gitlab/github_import/importer.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/github_import/pull_request_formatter.rb | 4 | ||||
-rw-r--r-- | spec/javascripts/awards_handler_spec.js | 50 | ||||
-rw-r--r-- | spec/lib/gitlab/github_import/importer_spec.rb | 120 | ||||
-rw-r--r-- | spec/lib/gitlab/github_import/pull_request_formatter_spec.rb | 8 |
8 files changed, 166 insertions, 56 deletions
diff --git a/app/assets/javascripts/awards_handler.js b/app/assets/javascripts/awards_handler.js index 54836efdf29..8a077f0081a 100644 --- a/app/assets/javascripts/awards_handler.js +++ b/app/assets/javascripts/awards_handler.js @@ -45,12 +45,12 @@ function buildCategoryMap() { }); } -function renderCategory(name, emojiList) { +function renderCategory(name, emojiList, opts = {}) { return ` <h5 class="emoji-menu-title"> ${name} </h5> - <ul class="clearfix emoji-menu-list"> + <ul class="clearfix emoji-menu-list ${opts.menuListClass}"> ${emojiList.map(emojiName => ` <li class="emoji-menu-list-item"> <button class="emoji-menu-btn text-center js-emoji-btn" type="button"> @@ -140,9 +140,6 @@ AwardsHandler.prototype.showEmojiMenu = function showEmojiMenu($addBtn) { const $createdMenu = $('.emoji-menu'); $addBtn.removeClass('is-loading'); this.positionMenu($createdMenu, $addBtn); - if (!this.frequentEmojiBlockRendered) { - this.renderFrequentlyUsedBlock(); - } return setTimeout(() => { $createdMenu.addClass('is-visible'); $('#emoji_search').focus(); @@ -165,11 +162,21 @@ AwardsHandler.prototype.createEmojiMenu = function createEmojiMenu(callback) { const emojisInCategory = categoryMap[categoryNameKey]; const firstCategory = renderCategory(categoryLabelMap[categoryNameKey], emojisInCategory); + // Render the frequently used + const frequentlyUsedEmojis = this.getFrequentlyUsedEmojis(); + let frequentlyUsedCatgegory = ''; + if (frequentlyUsedEmojis.length > 0) { + frequentlyUsedCatgegory = renderCategory('Frequently used', frequentlyUsedEmojis, { + menuListClass: 'frequent-emojis', + }); + } + const emojiMenuMarkup = ` <div class="emoji-menu"> <input type="text" name="emoji_search" id="emoji_search" value="" class="emoji-search search-input form-control" placeholder="Search emoji" /> <div class="emoji-menu-content"> + ${frequentlyUsedCatgegory} ${firstCategory} </div> </div> @@ -457,19 +464,6 @@ AwardsHandler.prototype.getFrequentlyUsedEmojis = function getFrequentlyUsedEmoj return _.compact(_.uniq(frequentlyUsedEmojis)); }; -AwardsHandler.prototype.renderFrequentlyUsedBlock = function renderFrequentlyUsedBlock() { - if (Cookies.get('frequently_used_emojis')) { - const frequentlyUsedEmojis = this.getFrequentlyUsedEmojis(); - const ul = $('<ul class="clearfix emoji-menu-list frequent-emojis">'); - for (let i = 0, len = frequentlyUsedEmojis.length; i < len; i += 1) { - const emoji = frequentlyUsedEmojis[i]; - $(`.emoji-menu-content [data-name="${emoji}"]`).closest('li').clone().appendTo(ul); - } - $('.emoji-menu-content').prepend(ul).prepend($('<h5>').text('Frequently used')); - } - this.frequentEmojiBlockRendered = true; -}; - AwardsHandler.prototype.setupSearch = function setupSearch() { this.registerEventListener('on', $('input.emoji-search'), 'input', (e) => { const term = $(e.target).val().trim(); diff --git a/changelogs/unreleased/29046-fix-github-importer-open-prs.yml b/changelogs/unreleased/29046-fix-github-importer-open-prs.yml new file mode 100644 index 00000000000..d279c269f94 --- /dev/null +++ b/changelogs/unreleased/29046-fix-github-importer-open-prs.yml @@ -0,0 +1,4 @@ +--- +title: Fix GitHub Import deleting branches for open PRs from a fork +merge_request: 9758 +author: diff --git a/changelogs/unreleased/add-frequently-used-emojis-back-to-menu.yml b/changelogs/unreleased/add-frequently-used-emojis-back-to-menu.yml new file mode 100644 index 00000000000..66d5bb63734 --- /dev/null +++ b/changelogs/unreleased/add-frequently-used-emojis-back-to-menu.yml @@ -0,0 +1,4 @@ +--- +title: Add frequently used emojis back to awards menu +merge_request: +author: diff --git a/lib/gitlab/github_import/importer.rb b/lib/gitlab/github_import/importer.rb index dc73cad93a5..eea4a91f17d 100644 --- a/lib/gitlab/github_import/importer.rb +++ b/lib/gitlab/github_import/importer.rb @@ -171,6 +171,8 @@ module Gitlab end def clean_up_restored_branches(pull_request) + return if pull_request.opened? + remove_branch(pull_request.source_branch_name) unless pull_request.source_branch_exists? remove_branch(pull_request.target_branch_name) unless pull_request.target_branch_exists? end diff --git a/lib/gitlab/github_import/pull_request_formatter.rb b/lib/gitlab/github_import/pull_request_formatter.rb index 28812fd0cb9..add7236e339 100644 --- a/lib/gitlab/github_import/pull_request_formatter.rb +++ b/lib/gitlab/github_import/pull_request_formatter.rb @@ -60,6 +60,10 @@ module Gitlab source_branch.repo.id != target_branch.repo.id end + def opened? + state == 'opened' + end + private def state diff --git a/spec/javascripts/awards_handler_spec.js b/spec/javascripts/awards_handler_spec.js index dc0a62ade50..9a2978006aa 100644 --- a/spec/javascripts/awards_handler_spec.js +++ b/spec/javascripts/awards_handler_spec.js @@ -1,6 +1,7 @@ /* eslint-disable space-before-function-paren, no-var, one-var, one-var-declaration-per-line, no-unused-expressions, comma-dangle, new-parens, no-unused-vars, quotes, jasmine/no-spec-dupes, prefer-template, max-len */ import promisePolyfill from 'es6-promise'; +import Cookies from 'js-cookie'; import AwardsHandler from '~/awards_handler'; promisePolyfill.polyfill(); @@ -208,8 +209,8 @@ promisePolyfill.polyfill(); expect($('[data-name=alien]').is(':visible')).toBe(true); }) .then(done) - .catch(() => { - done.fail('Failed to open and build emoji menu'); + .catch((err) => { + done.fail(`Failed to open and build emoji menu: ${err.message}`); }); }); }); @@ -232,8 +233,8 @@ promisePolyfill.polyfill(); it('should add selected emoji to awards block', function(done) { return openEmojiMenuAndAddEmoji() .then(done) - .catch(() => { - done.fail('Failed to open and build emoji menu'); + .catch((err) => { + done.fail(`Failed to open and build emoji menu: ${err.message}`); }); }); it('should remove already selected emoji', function(done) { @@ -247,7 +248,46 @@ promisePolyfill.polyfill(); }) .then(done) .catch((err) => { - done.fail('Failed to open and build emoji menu'); + done.fail(`Failed to open and build emoji menu: ${err.message}`); + }); + }); + }); + + describe('frequently used emojis', function() { + beforeEach(() => { + // Clear it out + Cookies.set('frequently_used_emojis', ''); + }); + + it('shouldn\'t have any "Frequently used" heading if no frequently used emojis', function(done) { + return openAndWaitForEmojiMenu() + .then(() => { + const emojiMenu = document.querySelector('.emoji-menu'); + Array.prototype.forEach.call(emojiMenu.querySelectorAll('.emoji-menu-title'), (title) => { + expect(title.textContent.trim().toLowerCase()).not.toBe('frequently used'); + }); + }) + .then(done) + .catch((err) => { + done.fail(`Failed to open and build emoji menu: ${err.message}`); + }); + }); + + it('should have any frequently used section when there are frequently used emojis', function(done) { + awardsHandler.addEmojiToFrequentlyUsedList('8ball'); + + return openAndWaitForEmojiMenu() + .then(() => { + const emojiMenu = document.querySelector('.emoji-menu'); + const hasFrequentlyUsedHeading = Array.prototype.some.call(emojiMenu.querySelectorAll('.emoji-menu-title'), title => + title.textContent.trim().toLowerCase() === 'frequently used' + ); + + expect(hasFrequentlyUsedHeading).toBe(true); + }) + .then(done) + .catch((err) => { + done.fail(`Failed to open and build emoji menu: ${err.message}`); }); }); }); diff --git a/spec/lib/gitlab/github_import/importer_spec.rb b/spec/lib/gitlab/github_import/importer_spec.rb index 3f080de99dd..8b867fbe322 100644 --- a/spec/lib/gitlab/github_import/importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer_spec.rb @@ -55,9 +55,6 @@ describe Gitlab::GithubImport::Importer, lib: true do allow_any_instance_of(Octokit::Client).to receive(:releases).and_return([release1, release2]) end - 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 double( name: 'Bug', @@ -127,32 +124,6 @@ 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: 'branch-merged', repo: repository, sha: source_sha) } - let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } - let(:target_branch) { double(ref: 'master', repo: repository, sha: target_sha) } - let(:pull_request) do - double( - number: 1347, - milestone: nil, - state: 'open', - title: 'New feature', - body: 'Please pull these awesome changes', - head: source_branch, - base: target_branch, - assignee: nil, - user: octocat, - created_at: created_at, - updated_at: updated_at, - closed_at: nil, - merged_at: nil, - url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", - labels: [double(name: 'Label #2')] - ) - end - let(:release1) do double( tag_name: 'v1.0.0', @@ -177,12 +148,14 @@ describe Gitlab::GithubImport::Importer, lib: true do ) end + subject { described_class.new(project) } + it 'returns true' do - expect(described_class.new(project).execute).to eq true + expect(subject.execute).to eq true end it 'does not raise an error' do - expect { described_class.new(project).execute }.not_to raise_error + expect { subject.execute }.not_to raise_error end it 'stores error messages' do @@ -205,15 +178,93 @@ describe Gitlab::GithubImport::Importer, lib: true do end end + shared_examples 'Gitlab::GithubImport unit-testing' do + describe '#clean_up_restored_branches' do + subject { described_class.new(project) } + + before do + allow(gh_pull_request).to receive(:source_branch_exists?).at_least(:once) { false } + allow(gh_pull_request).to receive(:target_branch_exists?).at_least(:once) { false } + end + + context 'when pull request stills open' do + let(:gh_pull_request) { Gitlab::GithubImport::PullRequestFormatter.new(project, pull_request) } + + it 'does not remove branches' do + expect(subject).not_to receive(:remove_branch) + subject.send(:clean_up_restored_branches, gh_pull_request) + end + end + + context 'when pull request is closed' do + let(:gh_pull_request) { Gitlab::GithubImport::PullRequestFormatter.new(project, closed_pull_request) } + + it 'does remove branches' do + expect(subject).to receive(:remove_branch).at_least(2).times + subject.send(:clean_up_restored_branches, gh_pull_request) + end + end + end + end + let(:project) { create(:project, :wiki_disabled, import_url: "#{repo_root}/octocat/Hello-World.git") } + let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } let(:credentials) { { user: 'joe' } } + let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } + let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } + let(:repository) { double(id: 1, fork: false) } + let(:source_sha) { create(:commit, project: project).id } + let(:source_branch) { double(ref: 'branch-merged', repo: repository, sha: source_sha) } + let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } + let(:target_branch) { double(ref: 'master', repo: repository, sha: target_sha) } + let(:pull_request) do + double( + number: 1347, + milestone: nil, + state: 'open', + title: 'New feature', + body: 'Please pull these awesome changes', + head: source_branch, + base: target_branch, + assignee: nil, + user: octocat, + created_at: created_at, + updated_at: updated_at, + closed_at: nil, + merged_at: nil, + url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", + labels: [double(name: 'Label #2')] + ) + end + let(:closed_pull_request) do + double( + number: 1347, + milestone: nil, + state: 'closed', + title: 'New feature', + body: 'Please pull these awesome changes', + head: source_branch, + base: target_branch, + assignee: nil, + user: octocat, + created_at: created_at, + updated_at: updated_at, + closed_at: updated_at, + merged_at: nil, + url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", + labels: [double(name: 'Label #2')] + ) + end + context 'when importing a GitHub project' do let(:api_root) { 'https://api.github.com' } let(:repo_root) { 'https://github.com' } + subject { described_class.new(project) } it_behaves_like 'Gitlab::GithubImport::Importer#execute' it_behaves_like 'Gitlab::GithubImport::Importer#execute an error occurs' + it_behaves_like 'Gitlab::GithubImport unit-testing' describe '#client' do it 'instantiates a Client' do @@ -223,7 +274,7 @@ describe Gitlab::GithubImport::Importer, lib: true do {} ) - described_class.new(project).client + subject.client end end end @@ -231,6 +282,8 @@ describe Gitlab::GithubImport::Importer, lib: true do context 'when importing a Gitea project' do let(:api_root) { 'https://try.gitea.io/api/v1' } let(:repo_root) { 'https://try.gitea.io' } + subject { described_class.new(project) } + before do project.update(import_type: 'gitea', import_url: "#{repo_root}/foo/group/project.git") end @@ -239,6 +292,7 @@ describe Gitlab::GithubImport::Importer, lib: true do let(:expected_not_called) { [:import_releases] } end it_behaves_like 'Gitlab::GithubImport::Importer#execute an error occurs' + it_behaves_like 'Gitlab::GithubImport unit-testing' describe '#client' do it 'instantiates a Client' do @@ -248,7 +302,7 @@ describe Gitlab::GithubImport::Importer, lib: true do { host: "#{repo_root}:443/foo", api_version: 'v1' } ) - described_class.new(project).client + subject.client end end end 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 951cbea7857..44423917944 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -306,4 +306,12 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do expect(pull_request.url).to eq 'https://api.github.com/repos/octocat/Hello-World/pulls/1347' end end + + describe '#opened?' do + let(:raw_data) { double(base_data.merge(state: 'open')) } + + it 'returns true when state is "open"' do + expect(pull_request.opened?).to be_truthy + end + end end |