summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFilipa Lacerda <filipa@gitlab.com>2017-03-10 23:11:50 +0000
committerFilipa Lacerda <filipa@gitlab.com>2017-03-10 23:11:50 +0000
commit6b1b616ea9e30055f8fdab7c7ea012ac2d5c7269 (patch)
treec7394d00329721530d73423d7e3895482bf996dd
parente8d525d07c30329d7cbe7b418296d0d9b9a391c0 (diff)
parente78aed49cd81e2873d7a536bc13e2e77435e290e (diff)
downloadgitlab-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.js30
-rw-r--r--changelogs/unreleased/29046-fix-github-importer-open-prs.yml4
-rw-r--r--changelogs/unreleased/add-frequently-used-emojis-back-to-menu.yml4
-rw-r--r--lib/gitlab/github_import/importer.rb2
-rw-r--r--lib/gitlab/github_import/pull_request_formatter.rb4
-rw-r--r--spec/javascripts/awards_handler_spec.js50
-rw-r--r--spec/lib/gitlab/github_import/importer_spec.rb120
-rw-r--r--spec/lib/gitlab/github_import/pull_request_formatter_spec.rb8
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