Commit ce8e4343 authored by GitLab Bot's avatar GitLab Bot

Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-07-11

parents a997bc26 0db5ccc8
......@@ -2,13 +2,16 @@ import $ from 'jquery';
import { rstrip } from './lib/utils/common_utils';
function openConfirmDangerModal($form, text) {
const $input = $('.js-confirm-danger-input');
$input.val('');
$('.js-confirm-text').text(text || '');
$('.js-confirm-danger-input').val('');
$('#modal-confirm-danger').modal('show');
const confirmTextMatch = $('.js-confirm-danger-match').text();
const $submit = $('.js-confirm-danger-submit');
$submit.disable();
$input.focus();
$('.js-confirm-danger-input').off('input').on('input', function handleInput() {
const confirmText = rstrip($(this).val());
......
......@@ -31,9 +31,6 @@ export default {
};
},
computed: {
isDiscussionsExpanded() {
return true; // TODO: @fatihacet - Fix this.
},
isCollapsed() {
return this.file.collapsed || false;
},
......@@ -131,7 +128,6 @@ export default {
:diff-file="file"
:collapsible="true"
:expanded="!isCollapsed"
:discussions-expanded="isDiscussionsExpanded"
:add-merge-request-buttons="true"
class="js-file-title file-title"
@toggleFile="handleToggle"
......
<script>
import _ from 'underscore';
import { mapActions, mapGetters } from 'vuex';
import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
import Icon from '~/vue_shared/components/icon.vue';
import FileIcon from '~/vue_shared/components/file_icon.vue';
......@@ -38,11 +39,6 @@ export default {
required: false,
default: true,
},
discussionsExpanded: {
type: Boolean,
required: false,
default: true,
},
currentUser: {
type: Object,
required: true,
......@@ -54,6 +50,10 @@ export default {
};
},
computed: {
...mapGetters('diffs', ['diffHasExpandedDiscussions']),
hasExpandedDiscussions() {
return this.diffHasExpandedDiscussions(this.diffFile);
},
icon() {
if (this.diffFile.submodule) {
return 'archive';
......@@ -88,9 +88,6 @@ export default {
collapseIcon() {
return this.expanded ? 'chevron-down' : 'chevron-right';
},
isDiscussionsExpanded() {
return this.discussionsExpanded && this.expanded;
},
viewFileButtonText() {
const truncatedContentSha = _.escape(truncateSha(this.diffFile.contentSha));
return sprintf(
......@@ -113,7 +110,8 @@ export default {
},
},
methods: {
handleToggle(e, checkTarget) {
...mapActions('diffs', ['toggleFileDiscussions']),
handleToggleFile(e, checkTarget) {
if (
!checkTarget ||
e.target === this.$refs.header ||
......@@ -125,6 +123,9 @@ export default {
showForkMessage() {
this.$emit('showForkMessage');
},
handleToggleDiscussions() {
this.toggleFileDiscussions(this.diffFile);
},
},
};
</script>
......@@ -133,7 +134,7 @@ export default {
<div
ref="header"
class="js-file-title file-title file-title-flex-parent"
@click="handleToggle($event, true)"
@click="handleToggleFile($event, true)"
>
<div class="file-header-content">
<icon
......@@ -216,10 +217,11 @@ export default {
v-if="diffFile.blob && diffFile.blob.readableText"
>
<button
:class="{ active: isDiscussionsExpanded }"
:class="{ active: hasExpandedDiscussions }"
:title="s__('MergeRequests|Toggle comments for this file')"
class="btn js-toggle-diff-comments"
class="js-btn-vue-toggle-comments btn"
type="button"
@click="handleToggleDiscussions"
>
<icon name="comment" />
</button>
......
......@@ -82,5 +82,32 @@ export const expandAllFiles = ({ commit }) => {
commit(types.EXPAND_ALL_FILES);
};
/**
* Toggles the file discussions after user clicked on the toggle discussions button.
*
* Gets the discussions for the provided diff.
*
* If all discussions are expanded, it will collapse all of them
* If all discussions are collapsed, it will expand all of them
* If some discussions are open and others closed, it will expand the closed ones.
*
* @param {Object} diff
*/
export const toggleFileDiscussions = ({ getters, dispatch }, diff) => {
const discussions = getters.getDiffFileDiscussions(diff);
const shouldCloseAll = getters.diffHasAllExpandedDiscussions(diff);
const shouldExpandAll = getters.diffHasAllCollpasedDiscussions(diff);
discussions.forEach(discussion => {
const data = { discussionId: discussion.id };
if (shouldCloseAll) {
dispatch('collapseDiscussion', data, { root: true });
} else if (shouldExpandAll || (!shouldCloseAll && !shouldExpandAll && !discussion.expanded)) {
dispatch('expandDiscussion', data, { root: true });
}
});
};
// prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {};
import _ from 'underscore';
import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '../constants';
export const isParallelView = state => state.diffViewType === PARALLEL_DIFF_VIEW_TYPE;
......@@ -8,5 +9,52 @@ export const areAllFilesCollapsed = state => state.diffFiles.every(file => file.
export const commitId = state => (state.commit && state.commit.id ? state.commit.id : null);
// prevent babel-plugin-rewire from generating an invalid default during karma tests
/**
* Checks if the diff has all discussions expanded
* @param {Object} diff
* @returns {Boolean}
*/
export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
const discussions = getters.getDiffFileDiscussions(diff);
return (discussions.length && discussions.every(discussion => discussion.expanded)) || false;
};
/**
* Checks if the diff has all discussions collpased
* @param {Object} diff
* @returns {Boolean}
*/
export const diffHasAllCollpasedDiscussions = (state, getters) => diff => {
const discussions = getters.getDiffFileDiscussions(diff);
return (discussions.length && discussions.every(discussion => !discussion.expanded)) || false;
};
/**
* Checks if the diff has any open discussions
* @param {Object} diff
* @returns {Boolean}
*/
export const diffHasExpandedDiscussions = (state, getters) => diff => {
const discussions = getters.getDiffFileDiscussions(diff);
return (
(discussions.length && discussions.find(discussion => discussion.expanded) !== undefined) ||
false
);
};
/**
* Returns an array with the discussions of the given diff
* @param {Object} diff
* @returns {Array}
*/
export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) => diff =>
rootGetters.discussions.filter(
discussion =>
discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash),
) || [];
// prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
export default () => {};
......@@ -15,6 +15,8 @@ let eTagPoll;
export const expandDiscussion = ({ commit }, data) => commit(types.EXPAND_DISCUSSION, data);
export const collapseDiscussion = ({ commit }, data) => commit(types.COLLAPSE_DISCUSSION, data);
export const setNotesData = ({ commit }, data) => commit(types.SET_NOTES_DATA, data);
export const setNoteableData = ({ commit }, data) => commit(types.SET_NOTEABLE_DATA, data);
......
export const ADD_NEW_NOTE = 'ADD_NEW_NOTE';
export const ADD_NEW_REPLY_TO_DISCUSSION = 'ADD_NEW_REPLY_TO_DISCUSSION';
export const DELETE_NOTE = 'DELETE_NOTE';
export const EXPAND_DISCUSSION = 'EXPAND_DISCUSSION';
export const REMOVE_PLACEHOLDER_NOTES = 'REMOVE_PLACEHOLDER_NOTES';
export const SET_NOTES_DATA = 'SET_NOTES_DATA';
export const SET_NOTEABLE_DATA = 'SET_NOTEABLE_DATA';
......@@ -11,12 +10,16 @@ export const SET_LAST_FETCHED_AT = 'SET_LAST_FETCHED_AT';
export const SET_TARGET_NOTE_HASH = 'SET_TARGET_NOTE_HASH';
export const SHOW_PLACEHOLDER_NOTE = 'SHOW_PLACEHOLDER_NOTE';
export const TOGGLE_AWARD = 'TOGGLE_AWARD';
export const TOGGLE_DISCUSSION = 'TOGGLE_DISCUSSION';
export const UPDATE_NOTE = 'UPDATE_NOTE';
export const UPDATE_DISCUSSION = 'UPDATE_DISCUSSION';
export const SET_DISCUSSION_DIFF_LINES = 'SET_DISCUSSION_DIFF_LINES';
export const SET_NOTES_FETCHED_STATE = 'SET_NOTES_FETCHED_STATE';
// DISCUSSION
export const COLLAPSE_DISCUSSION = 'COLLAPSE_DISCUSSION';
export const EXPAND_DISCUSSION = 'EXPAND_DISCUSSION';
export const TOGGLE_DISCUSSION = 'TOGGLE_DISCUSSION';
// Issue
export const CLOSE_ISSUE = 'CLOSE_ISSUE';
export const REOPEN_ISSUE = 'REOPEN_ISSUE';
......
......@@ -58,6 +58,11 @@ export default {
discussion.expanded = true;
},
[types.COLLAPSE_DISCUSSION](state, { discussionId }) {
const discussion = utils.findNoteObjectById(state.discussions, discussionId);
discussion.expanded = false;
},
[types.REMOVE_PLACEHOLDER_NOTES](state) {
const { discussions } = state;
......
......@@ -116,10 +116,8 @@
.modify-merge-commit-link {
padding: 0;
background-color: transparent;
border: 0;
color: $gl-text-color;
&:hover,
......@@ -501,10 +499,6 @@
}
}
.merge-request-details .content-block {
border-bottom: 0;
}
.mr-source-target {
display: flex;
flex-wrap: wrap;
......
.detail-page-description.content-block
.detail-page-description
%h2.title
= markdown_field(@merge_request, :title)
......
......@@ -23,6 +23,8 @@ module ChangelogHelpers
Abort = Class.new(StandardError)
Done = Class.new(StandardError)
MAX_FILENAME_LENGTH = 140 # ecryptfs has a limit of 140 characters
def capture_stdout(cmd)
output = IO.popen(cmd, &:read)
fail_with "command failed: #{cmd.join(' ')}" unless $?.success?
......@@ -142,7 +144,9 @@ class ChangelogEntry
def initialize(options)
@options = options
end
def execute
assert_feature_branch!
assert_title!
assert_new_file!
......@@ -221,10 +225,12 @@ class ChangelogEntry
end
def file_path
File.join(
base_path = File.join(
unreleased_path,
branch_name.gsub(/[^\w-]/, '-') << '.yml'
)
branch_name.gsub(/[^\w-]/, '-'))
# Add padding for .yml extension
base_path[0..MAX_FILENAME_LENGTH - 5] + '.yml'
end
def unreleased_path
......@@ -250,7 +256,7 @@ end
if $0 == __FILE__
begin
options = ChangelogOptionParser.parse(ARGV)
ChangelogEntry.new(options)
ChangelogEntry.new(options).execute
rescue ChangelogHelpers::Abort => ex
$stderr.puts ex.message
exit 1
......
---
title: Fixes toggle discussion button not expanding collapsed discussions
merge_request: 20452
author:
type: fixed
---
title: Improve danger confirmation modals by focusing input field
merge_request:
author: Jamie Schembri
type: added
module Gitlab
# Helper methods to do with Kubernetes network services & resources
module Kubernetes
def self.build_header_hash
Hash.new { |h, k| h[k] = [] }
end
# This is the comand that is run to start a terminal session. Kubernetes
# expects `command=foo&command=bar, not `command[]=foo&command[]=bar`
EXEC_COMMAND = URI.encode_www_form(
......@@ -37,13 +41,14 @@ module Gitlab
selectors: { pod: pod_name, container: container["name"] },
url: container_exec_url(api_url, namespace, pod_name, container["name"]),
subprotocols: ['channel.k8s.io'],
headers: Hash.new { |h, k| h[k] = [] },
headers: ::Gitlab::Kubernetes.build_header_hash,
created_at: created_at
}
end
end
def add_terminal_auth(terminal, token:, max_session_time:, ca_pem: nil)
terminal[:headers] ||= ::Gitlab::Kubernetes.build_header_hash
terminal[:headers]['Authorization'] << "Bearer #{token}"
terminal[:max_session_time] = max_session_time
terminal[:ca_pem] = ca_pem if ca_pem.present?
......
......@@ -85,6 +85,10 @@ module QA
driver.browser.save_screenshot(path)
end
Capybara::Screenshot.register_filename_prefix_formatter(:rspec) do |example|
File.join(QA::Runtime::Namespace.name, example.file_path.sub('./qa/specs/features/', ''))
end
Capybara.configure do |config|
config.default_driver = :chrome
config.javascript_driver = :chrome
......
......@@ -8,7 +8,7 @@ module QA
end
def name
'qa-test-' + time.strftime('%d-%m-%Y-%H-%M-%S')
"qa-test-#{time.strftime('%Y-%m-%d-%Y-%H-%M-%S')}"
end
def path
......
......@@ -3,6 +3,20 @@ require 'spec_helper'
load File.expand_path('../../bin/changelog', __dir__)
describe 'bin/changelog' do
let(:options) { OpenStruct.new(title: 'Test title', type: 'fixed', dry_run: true) }
describe ChangelogEntry do
it 'truncates the file path' do
entry = described_class.new(options)
allow(entry).to receive(:ee?).and_return(false)
allow(entry).to receive(:branch_name).and_return('long-branch-' * 100)
file_path = entry.send(:file_path)
expect(file_path.length).to eq(140)
end
end
describe ChangelogOptionParser do
describe '.parse' do
it 'parses --amend' do
......
......@@ -154,6 +154,12 @@ describe 'Group' do
end
end
it 'focuses confirmation field on remove group' do
click_button('Remove group')
expect(page).to have_selector '#confirm_name_input:focus'
end
it 'removes group' do
expect { remove_with_confirm('Remove group', group.path) }.to change {Group.count}.by(-1)
expect(group.members.all.count).to be_zero
......
......@@ -31,7 +31,7 @@ describe 'User comments on a diff', :js do
page.within('.files > div:nth-child(3)') do
expect(page).to have_content('Line is wrong')
find('.js-toggle-diff-comments').click
find('.js-btn-vue-toggle-comments').click
expect(page).not_to have_content('Line is wrong')
end
......@@ -64,7 +64,7 @@ describe 'User comments on a diff', :js do
# Hide the comment.
page.within('.files > div:nth-child(3)') do
find('.js-toggle-diff-comments').click
find('.js-btn-vue-toggle-comments').click
expect(page).not_to have_content('Line is wrong')
end
......@@ -77,7 +77,7 @@ describe 'User comments on a diff', :js do
# Show the comment.
page.within('.files > div:nth-child(3)') do
find('.js-toggle-diff-comments').click
find('.js-btn-vue-toggle-comments').click
end
# Now both the comments should be shown.
......
......@@ -10,7 +10,7 @@ describe 'Projects > Settings > User transfers a project', :js do
sign_in(user)
end
def transfer_project(project, group)
def transfer_project(project, group, confirm: true)
visit edit_project_path(project)
page.within('.js-project-transfer-form') do
......@@ -21,6 +21,8 @@ describe 'Projects > Settings > User transfers a project', :js do
click_button('Transfer project')
return unless confirm
fill_in 'confirm_name_input', with: project.name
click_button 'Confirm'
......@@ -28,6 +30,11 @@ describe 'Projects > Settings > User transfers a project', :js do
wait_for_requests
end
it 'focuses on the confirmation field' do
transfer_project(project, group, confirm: false)
expect(page).to have_selector '#confirm_name_input:focus'
end
it 'allows transferring a project to a group' do
old_path = project_path(project)
transfer_project(project, group)
......
......@@ -155,6 +155,12 @@ describe 'Project' do
visit edit_project_path(project)
end
it 'focuses on the confirmation field' do
click_button 'Remove project'
expect(page).to have_selector '#confirm_name_input:focus'
end
it 'removes a project' do
expect { remove_with_confirm('Remove project', project.path) }.to change { Project.count }.by(-1)
expect(page).to have_content "Project '#{project.full_name}' is in the process of being deleted."
......
......@@ -191,4 +191,48 @@ describe('DiffsStoreActions', () => {
);
});
});
describe('toggleFileDiscussions', () => {
it('should dispatch collapseDiscussion when all discussions are expanded', () => {
const getters = {
getDiffFileDiscussions: jasmine.createSpy().and.returnValue([{ id: 1 }]),
diffHasAllExpandedDiscussions: jasmine.createSpy().and.returnValue(true),
diffHasAllCollpasedDiscussions: jasmine.createSpy().and.returnValue(false),
};
const dispatch = jasmine.createSpy('dispatch');
actions.toggleFileDiscussions({ getters, dispatch });
expect(dispatch).toHaveBeenCalledWith('collapseDiscussion', { discussionId: 1 }, { root: true });
});
it('should dispatch expandDiscussion when all discussions are collapsed', () => {
const getters = {
getDiffFileDiscussions: jasmine.createSpy().and.returnValue([{ id: 1 }]),
diffHasAllExpandedDiscussions: jasmine.createSpy().and.returnValue(false),
diffHasAllCollpasedDiscussions: jasmine.createSpy().and.returnValue(true),
};
const dispatch = jasmine.createSpy();
actions.toggleFileDiscussions({ getters, dispatch });
expect(dispatch).toHaveBeenCalledWith('expandDiscussion', { discussionId: 1 }, { root: true });
});
it('should dispatch expandDiscussion when some discussions are collapsed and others are expanded for the collapsed discussion', () => {
const getters = {
getDiffFileDiscussions: jasmine.createSpy().and.returnValue([{ expanded: false, id: 1 }]),
diffHasAllExpandedDiscussions: jasmine.createSpy().and.returnValue(false),
diffHasAllCollpasedDiscussions: jasmine.createSpy().and.returnValue(false),
};
const dispatch = jasmine.createSpy();
actions.toggleFileDiscussions({ getters, dispatch });
expect(dispatch).toHaveBeenCalledWith('expandDiscussion', { discussionId: 1 }, { root: true });
});
});
});
import * as getters from '~/diffs/store/getters';
import state from '~/diffs/store/modules/diff_state';
import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants';
import discussion from '../mock_data/diff_discussions';
describe('DiffsStoreGetters', () => {
describe('Diffs Module Getters', () => {
let localState;
let discussionMock;
let discussionMock1;
const diffFileMock = {
fileHash: '9732849daca6ae818696d9575f5d1207d1a7f8bb',
};
beforeEach(() => {
localState = state();
discussionMock = Object.assign({}, discussion);
discussionMock.diff_file.file_hash = diffFileMock.fileHash;
discussionMock1 = Object.assign({}, discussion);
discussionMock1.diff_file.file_hash = diffFileMock.fileHash;
});
describe('isParallelView', () => {
......@@ -63,4 +75,113 @@ describe('DiffsStoreGetters', () => {
expect(getters.commitId(localState)).toEqual(null);
});
});
describe('diffHasAllExpandedDiscussions', () => {
it('returns true when all discussions are expanded', () => {
expect(
getters.diffHasAllExpandedDiscussions(localState, {
getDiffFileDiscussions: () => [discussionMock, discussionMock],
})(diffFileMock),
).toEqual(true);
});
it('returns false when there are no discussions', () => {
expect(
getters.diffHasAllExpandedDiscussions(localState, {
getDiffFileDiscussions: () => [],
})(diffFileMock),
).toEqual(false);
});
it('returns false when one discussions is collapsed', () => {
discussionMock1.expanded = false;
expect(
getters.diffHasAllExpandedDiscussions(localState, {
getDiffFileDiscussions: () => [discussionMock, discussionMock1],
})(diffFileMock),
).toEqual(false);
});
});
describe('diffHasAllCollpasedDiscussions', () => {
it('returns true when all discussions are collapsed', () => {
discussionMock.diff_file.file_hash = diffFileMock.fileHash;
discussionMock.expanded = false;
expect(
getters.diffHasAllCollpasedDiscussions(localState, {
getDiffFileDiscussions: () => [discussionMock],
})(diffFileMock),
).toEqual(true);
});
it('returns false when there are no discussions', () => {
expect(
getters.diffHasAllCollpasedDiscussions(localState, {
getDiffFileDiscussions: () => [],
})(diffFileMock),
).toEqual(false);
});
it('returns false when one discussions is expanded', () => {
discussionMock1.expanded = false;
expect(
getters.diffHasAllCollpasedDiscussions(localState, {
getDiffFileDiscussions: () => [discussionMock, discussionMock1],
})(diffFileMock),
).toEqual(false);
});
});
describe('diffHasExpandedDiscussions', () => {
it('returns true when one of the discussions is expanded', () => {
discussionMock1.expanded = false;
expect(
getters.diffHasExpandedDiscussions(localState, {
getDiffFileDiscussions: () => [discussionMock, discussionMock],
})(diffFileMock),
).toEqual(true);
});
it('returns false when there are no discussions', () => {
expect(
getters.diffHasExpandedDiscussions(localState, { getDiffFileDiscussions: () => [] })(
diffFileMock,
),
).toEqual(false);
});
it('returns false when no discussion is expanded', () => {
discussionMock.expanded = false;
discussionMock1.expanded = false;
expect(
getters.diffHasExpandedDiscussions(localState, {
getDiffFileDiscussions: () => [discussionMock, discussionMock1],
})(diffFileMock),
).toEqual(false);
});
});
describe('getDiffFileDiscussions', () => {
it('returns an array with discussions when fileHash matches and the discussion belongs to a diff', () => {
discussionMock.diff_file.file_hash = diffFileMock.fileHash;
expect(
getters.getDiffFileDiscussions(localState, {}, {}, { discussions: [discussionMock] })(
diffFileMock,
).length,
).toEqual(1);
});
it('returns an empty array when no discussions are found in the given diff', () => {
expect(
getters.getDiffFileDiscussions(localState, {}, {}, { discussions: [] })(diffFileMock)
.length,
).toEqual(0);
});
});
});
......@@ -128,6 +128,19 @@ describe('Actions Notes Store', () => {
});
});
describe('collapseDiscussion', () => {
it('should commit collapse discussion', done => {
testAction(
actions.collapseDiscussion,
{ discussionId: discussionMock.id },
{ notes: [discussionMock] },
[{ type: 'COLLAPSE_DISCUSSION', payload: { discussionId: discussionMock.id } }],
[],
done,
);
});
});
describe('async methods', () => {
const interceptor = (request, next) => {
next(
......
......@@ -74,6 +74,20 @@ describe('Notes Store mutations', () => {
});
});
describe('COLLAPSE_DISCUSSION', () => {
it('should collpase an expanded discussion', () => {
const discussion = Object.assign({}, discussionMock, { expanded: true });
const state = {
discussions: [discussion],
};
mutations.COLLAPSE_DISCUSSION(state, { discussionId: discussion.id });
expect(state.discussions[0].expanded).toEqual(false);
});
});
describe('REMOVE_PLACEHOLDER_NOTES', () => {
it('should remove all placeholder notes in indivudal notes and discussion', () => {
const placeholderNote = Object.assign({}, individualNote, { isPlaceholderNote: true });
......
......@@ -70,4 +70,19 @@ describe Gitlab::Kubernetes do
it { is_expected.to eq(YAML.load_file(path)) }
end
end
describe '#add_terminal_auth' do
it 'adds authentication parameters to a hash' do
terminal = { original: 'value' }
add_terminal_auth(terminal, token: 'foo', max_session_time: 0, ca_pem: 'bar')
expect(terminal).to eq(
original: 'value',
headers: { 'Authorization' => ['Bearer foo'] },
max_session_time: 0,
ca_pem: 'bar'
)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment