diff options
33 files changed, 390 insertions, 201 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index aa49f41ebf4..e5fe527e611 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -75,6 +75,7 @@ Naming/FileName: - 'qa/spec/**/*' - 'qa/qa/specs/**/*' - 'qa/bin/*' + - 'ee/bin/*' - 'config/**/*' - 'ee/config/**/*' - 'lib/generators/**/*' @@ -158,7 +158,7 @@ gem 'state_machines-activerecord', '~> 0.5.1' gem 'acts-as-taggable-on', '~> 6.0' # Background jobs -gem 'sidekiq', '~> 5.2.1' +gem 'sidekiq', '~> 5.2.7' gem 'sidekiq-cron', '~> 1.0' gem 'redis-namespace', '~> 1.6.0' gem 'gitlab-sidekiq-fetcher', '~> 0.4.0', require: 'sidekiq-reliable-fetch' diff --git a/Gemfile.lock b/Gemfile.lock index d214892eed7..faf5af5c7e7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -846,7 +846,7 @@ GEM rack shoulda-matchers (3.1.2) activesupport (>= 4.0.0) - sidekiq (5.2.5) + sidekiq (5.2.7) connection_pool (~> 2.2, >= 2.2.2) rack (>= 1.5.0) rack-protection (>= 1.5.0) @@ -1183,7 +1183,7 @@ DEPENDENCIES settingslogic (~> 2.0.9) sham_rack (~> 1.3.6) shoulda-matchers (~> 3.1.2) - sidekiq (~> 5.2.1) + sidekiq (~> 5.2.7) sidekiq-cron (~> 1.0) simple_po_parser (~> 1.1.2) simplecov (~> 0.14.0) diff --git a/app/assets/javascripts/ide/components/new_dropdown/upload.vue b/app/assets/javascripts/ide/components/new_dropdown/upload.vue index ec759043efc..188518dd419 100644 --- a/app/assets/javascripts/ide/components/new_dropdown/upload.vue +++ b/app/assets/javascripts/ide/components/new_dropdown/upload.vue @@ -57,6 +57,8 @@ export default { type: 'blob', content: result, base64: !isText, + binary: !isText, + rawPath: !isText ? target.result : '', }); }, readFile(file) { diff --git a/app/assets/javascripts/ide/components/repo_editor.vue b/app/assets/javascripts/ide/components/repo_editor.vue index c7798ad0cd7..e15b2a6f76b 100644 --- a/app/assets/javascripts/ide/components/repo_editor.vue +++ b/app/assets/javascripts/ide/components/repo_editor.vue @@ -1,5 +1,6 @@ <script> import { mapState, mapGetters, mapActions } from 'vuex'; +import { viewerInformationForPath } from '~/vue_shared/components/content_viewer/lib/viewer_utils'; import flash from '~/flash'; import ContentViewer from '~/vue_shared/components/content_viewer/content_viewer.vue'; import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue'; @@ -56,6 +57,10 @@ export default { active: this.file.viewMode === 'preview', }; }, + fileType() { + const info = viewerInformationForPath(this.file.path); + return (info && info.id) || ''; + }, }, watch: { file(newVal, oldVal) { @@ -258,6 +263,7 @@ export default { :path="file.rawPath || file.path" :file-size="file.size" :project-path="file.projectId" + :type="fileType" /> <diff-viewer v-if="showDiffViewer" diff --git a/app/assets/javascripts/ide/lib/files.js b/app/assets/javascripts/ide/lib/files.js index df100f753d7..b8abaa41f23 100644 --- a/app/assets/javascripts/ide/lib/files.js +++ b/app/assets/javascripts/ide/lib/files.js @@ -22,6 +22,8 @@ export const decorateFiles = ({ tempFile = false, content = '', base64 = false, + binary = false, + rawPath = '', }) => { const treeList = []; const entries = {}; @@ -90,6 +92,8 @@ export const decorateFiles = ({ changed: tempFile, content, base64, + binary, + rawPath, previewMode: viewerInformationForPath(name), parentPath, }); diff --git a/app/assets/javascripts/ide/stores/actions.js b/app/assets/javascripts/ide/stores/actions.js index 7b660bda081..fd678e6e10c 100644 --- a/app/assets/javascripts/ide/stores/actions.js +++ b/app/assets/javascripts/ide/stores/actions.js @@ -53,7 +53,7 @@ export const setResizingStatus = ({ commit }, resizing) => { export const createTempEntry = ( { state, commit, dispatch }, - { name, type, content = '', base64 = false }, + { name, type, content = '', base64 = false, binary = false, rawPath = '' }, ) => new Promise(resolve => { const fullName = name.slice(-1) !== '/' && type === 'tree' ? `${name}/` : name; @@ -79,8 +79,10 @@ export const createTempEntry = ( branchId: state.currentBranchId, type, tempFile: true, - base64, content, + base64, + binary, + rawPath, }); const { file, parentPath } = data; diff --git a/app/assets/javascripts/ide/stores/modules/file_templates/actions.js b/app/assets/javascripts/ide/stores/modules/file_templates/actions.js index b7090e09daf..59ead8a3dcf 100644 --- a/app/assets/javascripts/ide/stores/modules/file_templates/actions.js +++ b/app/assets/javascripts/ide/stores/modules/file_templates/actions.js @@ -23,22 +23,27 @@ export const receiveTemplateTypesError = ({ commit, dispatch }) => { export const receiveTemplateTypesSuccess = ({ commit }, templates) => commit(types.RECEIVE_TEMPLATE_TYPES_SUCCESS, templates); -export const fetchTemplateTypes = ({ dispatch, state, rootState }, page = 1) => { +export const fetchTemplateTypes = ({ dispatch, state, rootState }) => { if (!Object.keys(state.selectedTemplateType).length) return Promise.reject(); dispatch('requestTemplateTypes'); - return Api.projectTemplates(rootState.currentProjectId, state.selectedTemplateType.key, { page }) - .then(({ data, headers }) => { - const nextPage = parseInt(normalizeHeaders(headers)['X-NEXT-PAGE'], 10); + const fetchPages = (page = 1, prev = []) => + Api.projectTemplates(rootState.currentProjectId, state.selectedTemplateType.key, { + page, + per_page: 100, + }) + .then(({ data, headers }) => { + const nextPage = parseInt(normalizeHeaders(headers)['X-NEXT-PAGE'], 10); + const nextData = prev.concat(data); - dispatch('receiveTemplateTypesSuccess', data); + dispatch('receiveTemplateTypesSuccess', nextData); - if (nextPage) { - dispatch('fetchTemplateTypes', nextPage); - } - }) - .catch(() => dispatch('receiveTemplateTypesError')); + return nextPage ? fetchPages(nextPage, nextData) : nextData; + }) + .catch(() => dispatch('receiveTemplateTypesError')); + + return fetchPages(); }; export const setSelectedTemplateType = ({ commit, dispatch, rootGetters }, type) => { diff --git a/app/assets/javascripts/ide/stores/modules/file_templates/mutations.js b/app/assets/javascripts/ide/stores/modules/file_templates/mutations.js index 25a65b047f1..7fc1c9134a7 100644 --- a/app/assets/javascripts/ide/stores/modules/file_templates/mutations.js +++ b/app/assets/javascripts/ide/stores/modules/file_templates/mutations.js @@ -3,13 +3,14 @@ import * as types from './mutation_types'; export default { [types.REQUEST_TEMPLATE_TYPES](state) { state.isLoading = true; + state.templates = []; }, [types.RECEIVE_TEMPLATE_TYPES_ERROR](state) { state.isLoading = false; }, [types.RECEIVE_TEMPLATE_TYPES_SUCCESS](state, templates) { state.isLoading = false; - state.templates = state.templates.concat(templates); + state.templates = templates; }, [types.SET_SELECTED_TEMPLATE_TYPE](state, type) { state.selectedTemplateType = type; diff --git a/app/assets/javascripts/ide/stores/utils.js b/app/assets/javascripts/ide/stores/utils.js index 3ab8f3f11be..66c5180b782 100644 --- a/app/assets/javascripts/ide/stores/utils.js +++ b/app/assets/javascripts/ide/stores/utils.js @@ -69,6 +69,8 @@ export const decorateData = entity => { changed = false, parentTreeUrl = '', base64 = false, + binary = false, + rawPath = '', previewMode, file_lock, html, @@ -92,6 +94,8 @@ export const decorateData = entity => { renderError, content, base64, + binary, + rawPath, previewMode, file_lock, html, diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 89563711bcd..1e47bef7b61 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -454,8 +454,13 @@ Please check your network connection and try again.`; </component> </template> </ul> + <draft-note + v-if="showDraft(discussion.reply_id)" + :key="`draft_${discussion.id}`" + :draft="draftForDiscussion(discussion.reply_id)" + /> <div - v-if="isExpanded || !hasReplies" + v-else-if="isExpanded || !hasReplies" :class="{ 'is-replying': isReplying }" class="discussion-reply-holder" > diff --git a/app/assets/javascripts/vue_shared/components/content_viewer/content_viewer.vue b/app/assets/javascripts/vue_shared/components/content_viewer/content_viewer.vue index 4155e1bab9c..1e6f4c376c1 100644 --- a/app/assets/javascripts/vue_shared/components/content_viewer/content_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/content_viewer/content_viewer.vue @@ -1,5 +1,4 @@ <script> -import { viewerInformationForPath } from './lib/viewer_utils'; import MarkdownViewer from './viewers/markdown_viewer.vue'; import ImageViewer from './viewers/image_viewer.vue'; import DownloadViewer from './viewers/download_viewer.vue'; @@ -24,15 +23,18 @@ export default { required: false, default: '', }, + type: { + type: String, + required: false, + default: '', + }, }, computed: { viewer() { if (!this.path) return null; + if (!this.type) return DownloadViewer; - const previewInfo = viewerInformationForPath(this.path); - if (!previewInfo) return DownloadViewer; - - switch (previewInfo.id) { + switch (this.type) { case 'markdown': return MarkdownViewer; case 'image': diff --git a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue index ad3b3b81ac5..8d77b156aa4 100644 --- a/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue +++ b/app/assets/javascripts/vue_shared/components/diff_viewer/viewers/image_diff/swipe_viewer.vue @@ -58,12 +58,11 @@ export default { const moveX = e.pageX || e.touches[0].pageX; let leftValue = moveX - this.$refs.swipeFrame.getBoundingClientRect().left; - const spaceLeft = 20; const { clientWidth } = this.$refs.swipeFrame; if (leftValue <= 0) { leftValue = 0; - } else if (leftValue > clientWidth - spaceLeft) { - leftValue = clientWidth - spaceLeft; + } else if (leftValue > clientWidth) { + leftValue = clientWidth; } this.swipeWrapWidth = (leftValue / clientWidth) * 100; @@ -80,7 +79,7 @@ export default { document.body.removeEventListener('touchmove', this.dragMove); }, prepareSwipe() { - if (this.swipeOldImgInfo && this.swipeNewImgInfo) { + if (this.swipeOldImgInfo && this.swipeNewImgInfo && this.swipeOldImgInfo.renderedWidth > 0) { // Add 2 for border width this.swipeMaxWidth = Math.max(this.swipeOldImgInfo.renderedWidth, this.swipeNewImgInfo.renderedWidth) + 2; @@ -101,6 +100,8 @@ export default { }, resize: _.throttle(function throttledResize() { this.swipeBarPos = 0; + this.swipeWrapWidth = 0; + this.prepareSwipe(); }, 400), }, }; @@ -111,6 +112,8 @@ export default { <div ref="swipeFrame" :style="{ + width: swipeMaxPixelWidth, + height: swipeMaxPixelHeight, 'user-select': dragging ? 'none' : null, }" class="swipe-frame" diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb index 3adf8a0c1a1..3f0aedfbfb2 100644 --- a/app/services/compare_service.rb +++ b/app/services/compare_service.rb @@ -3,7 +3,7 @@ require 'securerandom' # Compare 2 refs for one repo or between repositories -# and return Gitlab::Git::Compare object that responds to commits and diffs +# and return Compare object that responds to commits and diffs class CompareService attr_reader :start_project, :start_ref_name @@ -15,7 +15,7 @@ class CompareService def execute(target_project, target_ref, base_sha: nil, straight: false) raw_compare = target_project.repository.compare_source_branch(target_ref, start_project.repository, start_ref_name, straight: straight) - return unless raw_compare + return unless raw_compare && raw_compare.base && raw_compare.head Compare.new(raw_compare, target_project, diff --git a/app/views/projects/issues/new.html.haml b/app/views/projects/issues/new.html.haml index 9a081a42b6f..c6ff0d50ef4 100644 --- a/app/views/projects/issues/new.html.haml +++ b/app/views/projects/issues/new.html.haml @@ -1,9 +1,9 @@ -- add_to_breadcrumbs "Issues", project_issues_path(@project) -- breadcrumb_title "New" -- page_title "New Issue" +- add_to_breadcrumbs _("Issues"), project_issues_path(@project) +- breadcrumb_title _("New") +- page_title _("New Issue") %h3.page-title - New Issue + _("New Issue") %hr = render "form" diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 4bf1d8702af..57d13b99b21 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -1,7 +1,7 @@ - @content_class = "limit-container-width" unless fluid_layout -- add_to_breadcrumbs "Issues", project_issues_path(@project) +- add_to_breadcrumbs _("Issues"), project_issues_path(@project) - breadcrumb_title @issue.to_reference -- page_title "#{@issue.title} (#{@issue.to_reference})", "Issues" +- page_title "#{@issue.title} (#{@issue.to_reference})", _("Issues") - page_description @issue.description - page_card_attributes @issue.card_attributes diff --git a/changelogs/unreleased/58252-web-ide-dropdown-duplicates.yml b/changelogs/unreleased/58252-web-ide-dropdown-duplicates.yml new file mode 100644 index 00000000000..48b03994586 --- /dev/null +++ b/changelogs/unreleased/58252-web-ide-dropdown-duplicates.yml @@ -0,0 +1,5 @@ +--- +title: Resolve Web IDE template dropdown showing duplicates +merge_request: 27237 +author: +type: fixed diff --git a/changelogs/unreleased/58850-fix-misplaced-swipe-view-26969.yml b/changelogs/unreleased/58850-fix-misplaced-swipe-view-26969.yml new file mode 100644 index 00000000000..fa3e81df4e0 --- /dev/null +++ b/changelogs/unreleased/58850-fix-misplaced-swipe-view-26969.yml @@ -0,0 +1,5 @@ +--- +title: "Fix misaligned image diff swipe view" +merge_request: 26969 +author: ftab +type: fixed diff --git a/changelogs/unreleased/59514-uploading-images-base64.yml b/changelogs/unreleased/59514-uploading-images-base64.yml new file mode 100644 index 00000000000..905b00db06a --- /dev/null +++ b/changelogs/unreleased/59514-uploading-images-base64.yml @@ -0,0 +1,5 @@ +--- +title: Show proper preview for uploaded images in Web IDE +merge_request: 27471 +author: +type: fixed diff --git a/changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml b/changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml new file mode 100644 index 00000000000..7511543f7f6 --- /dev/null +++ b/changelogs/unreleased/sh-avoid-fetching-temp-refs-within-project.yml @@ -0,0 +1,5 @@ +--- +title: Don't create a temp reference for branch comparisons within project +merge_request: 24038 +author: +type: fixed diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md index 87853b95294..f6396f1f946 100644 --- a/doc/development/i18n/proofreader.md +++ b/doc/development/i18n/proofreader.md @@ -22,6 +22,7 @@ are very appreciative of the work done by translators and proofreaders! - Victor Wu - [GitLab](https://gitlab.com/victorwuky), [Crowdin](https://crowdin.com/profile/victorwu) - Chinese Traditional, Hong Kong 繁體中文 (香港) - Victor Wu - [GitLab](https://gitlab.com/victorwuky), [Crowdin](https://crowdin.com/profile/victorwu) + - Ivan Ip - [GitLab](https://gitlab.com/lifehome), [Crowdin](https://crowdin.com/profile/lifehome) - Czech - Proofreaders needed. - Danish diff --git a/lib/gitlab/checks/branch_check.rb b/lib/gitlab/checks/branch_check.rb index 1dbd564fb6f..4ddc1c718c7 100644 --- a/lib/gitlab/checks/branch_check.rb +++ b/lib/gitlab/checks/branch_check.rb @@ -48,7 +48,7 @@ module Gitlab if project.empty_repo? protected_branch_push_checks - elsif creation? && protected_branch_creation_enabled? + elsif creation? protected_branch_creation_checks elsif deletion? protected_branch_deletion_checks @@ -124,10 +124,6 @@ module Gitlab Gitlab::Routing.url_helpers.project_project_members_url(project) end - def protected_branch_creation_enabled? - Feature.enabled?(:protected_branch_creation, project, default_enabled: true) - end - def matching_merge_request? Checks::MatchingMergeRequest.new(newrev, branch_name, project).match? end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index a22e3c4b9dd..c12cb6a6434 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -732,18 +732,29 @@ module Gitlab end def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:) + reachable_ref = + if source_repository == self + source_branch_name + else + # If a tmp ref was created before for a separate repo comparison (forks), + # we're able to short-circuit the tmp ref re-creation: + # 1. Take the SHA from the source repo + # 2. Read that in the current "target" repo + # 3. If that SHA is still known (readable), it means GC hasn't + # cleaned it up yet, so we can use it instead re-writing the tmp ref. + source_commit_id = source_repository.commit(source_branch_name)&.sha + commit(source_commit_id)&.sha if source_commit_id + end + + return compare(target_branch_name, reachable_ref, straight: straight) if reachable_ref + tmp_ref = "refs/tmp/#{SecureRandom.hex}" return unless fetch_source_branch!(source_repository, source_branch_name, tmp_ref) - Gitlab::Git::Compare.new( - self, - target_branch_name, - tmp_ref, - straight: straight - ) + compare(target_branch_name, tmp_ref, straight: straight) ensure - delete_refs(tmp_ref) + delete_refs(tmp_ref) if tmp_ref end def write_ref(ref_path, ref, old_ref: nil) @@ -999,6 +1010,13 @@ module Gitlab private + def compare(base_ref, head_ref, straight:) + Gitlab::Git::Compare.new(self, + base_ref, + head_ref, + straight: straight) + end + def empty_diff_stats Gitlab::Git::DiffStatsCollection.new([]) end diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 1818809518d..92380a2bf09 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -82,7 +82,7 @@ describe Projects::CompareController do show_request expect(response).to be_success - expect(assigns(:diffs).diff_files.to_a).to eq([]) + expect(assigns(:diffs)).to eq([]) expect(assigns(:commits)).to eq([]) end end diff --git a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb index 65de0afae0c..5db54f42264 100644 --- a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb +++ b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb @@ -191,29 +191,119 @@ describe 'Merge request > User creates image diff notes', :js do end end - describe 'image view modes' do - before do - visit project_commit_path(project, '2f63565e7aac07bcdadb654e253078b727143ec4') + shared_examples 'swipe view' do + it 'moves the swipe handle' do + # Simulate dragging swipe view slider + expect { drag_and_drop_by(find('.swipe-bar'), 20, 0) } + .to change { find('.swipe-bar')['style'] } + .from(a_string_matching('left: 1px')) end - it 'resizes image in onion skin view mode' do - find('.view-modes-menu .onion-skin').click + it 'shows both images at the same position' do + drag_and_drop_by(find('.swipe-bar'), 40, 0) - expect(find('.onion-skin-frame')['style']).to match('width: 228px; height: 240px;') + expect(left_position('.frame.added img')) + .to eq(left_position('.frame.deleted img')) end + end - it 'resets onion skin view mode opacity when toggling between view modes' do - find('.view-modes-menu .onion-skin').click - + shared_examples 'onion skin' do + it 'resets opacity when toggling between view modes' do # Simulate dragging onion-skin slider drag_and_drop_by(find('.dragger'), -30, 0) expect(find('.onion-skin-frame .frame.added', visible: false)['style']).not_to match('opacity: 1;') + switch_to_swipe_view + switch_to_onion_skin + + expect(find('.onion-skin-frame .frame.added', visible: false)['style']).to match('opacity: 1;') + end + end + + describe 'changes tab image diff' do + let(:merge_request) { create(:merge_request, source_project: project, target_project: project, target_branch: 'master', source_branch: 'deleted-image-test', author: user) } + + before do + visit diffs_project_merge_request_path(project, merge_request) + click_link "Changes" + end + + def set_image_diff_sources + # set path of added and deleted images to something the spec can view + page.execute_script("document.querySelector('.frame.added img').src = '/apple-touch-icon.png';") + page.execute_script("document.querySelector('.frame.deleted img').src = '/favicon.png';") + + wait_for_requests + + expect(find('.frame.added img', visible: false)['src']).to match('/apple-touch-icon.png') + expect(find('.frame.deleted img', visible: false)['src']).to match('/favicon.png') + end + + def switch_to_swipe_view + # it isn't given the .swipe class in the merge request diff + find('.view-modes-menu li:nth-child(2)').click + expect(find('.view-modes-menu li.active')).to have_content('Swipe') + + set_image_diff_sources + end + + def switch_to_onion_skin + # it isn't given the .onion-skin class in the merge request diff + find('.view-modes-menu li:nth-child(3)').click + expect(find('.view-modes-menu li.active')).to have_content('Onion skin') + + set_image_diff_sources + end + + describe 'onion skin' do + before do + switch_to_onion_skin + end + + it_behaves_like 'onion skin' + end + + describe 'swipe view' do + before do + switch_to_swipe_view + end + + it_behaves_like 'swipe view' + end + end + + describe 'image view modes' do + before do + visit project_commit_path(project, '2f63565e7aac07bcdadb654e253078b727143ec4') + end + + def switch_to_swipe_view find('.view-modes-menu .swipe').click + end + + def switch_to_onion_skin find('.view-modes-menu .onion-skin').click + end - expect(find('.onion-skin-frame .frame.added', visible: false)['style']).to match('opacity: 1;') + describe 'onion skin' do + before do + switch_to_onion_skin + end + + it 'resizes image' do + expect(find('.onion-skin-frame')['style']).to match('width: 228px; height: 240px;') + end + + it_behaves_like 'onion skin' + end + + describe 'swipe view' do + before do + switch_to_swipe_view + end + + it_behaves_like 'swipe view' end end @@ -232,4 +322,8 @@ describe 'Merge request > User creates image diff notes', :js do click_button 'Comment' wait_for_requests end + + def left_position(element) + page.evaluate_script("document.querySelectorAll('#{element}')[0].getBoundingClientRect().left;") + end end diff --git a/spec/frontend/ide/stores/modules/file_templates/mutations_spec.js b/spec/frontend/ide/stores/modules/file_templates/mutations_spec.js index 8e8b7f06ca2..6a1a826093c 100644 --- a/spec/frontend/ide/stores/modules/file_templates/mutations_spec.js +++ b/spec/frontend/ide/stores/modules/file_templates/mutations_spec.js @@ -2,6 +2,9 @@ import createState from '~/ide/stores/modules/file_templates/state'; import * as types from '~/ide/stores/modules/file_templates/mutation_types'; import mutations from '~/ide/stores/modules/file_templates/mutations'; +const mockFileTemplates = [['MIT'], ['CC']]; +const mockTemplateType = 'test'; + describe('IDE file templates mutations', () => { let state; @@ -10,11 +13,21 @@ describe('IDE file templates mutations', () => { }); describe(`${types.REQUEST_TEMPLATE_TYPES}`, () => { - it('sets isLoading', () => { + it('sets loading to true', () => { + state.isLoading = false; + mutations[types.REQUEST_TEMPLATE_TYPES](state); expect(state.isLoading).toBe(true); }); + + it('sets templates to an empty array', () => { + state.templates = mockFileTemplates; + + mutations[types.REQUEST_TEMPLATE_TYPES](state); + + expect(state.templates).toEqual([]); + }); }); describe(`${types.RECEIVE_TEMPLATE_TYPES_ERROR}`, () => { @@ -31,29 +44,33 @@ describe('IDE file templates mutations', () => { it('sets isLoading to false', () => { state.isLoading = true; - mutations[types.RECEIVE_TEMPLATE_TYPES_SUCCESS](state, []); + mutations[types.RECEIVE_TEMPLATE_TYPES_SUCCESS](state, mockFileTemplates); expect(state.isLoading).toBe(false); }); - it('sets templates', () => { - mutations[types.RECEIVE_TEMPLATE_TYPES_SUCCESS](state, ['test']); + it('sets templates to payload', () => { + state.templates = ['test']; + + mutations[types.RECEIVE_TEMPLATE_TYPES_SUCCESS](state, mockFileTemplates); - expect(state.templates).toEqual(['test']); + expect(state.templates).toEqual(mockFileTemplates); }); }); describe(`${types.SET_SELECTED_TEMPLATE_TYPE}`, () => { - it('sets selectedTemplateType', () => { - mutations[types.SET_SELECTED_TEMPLATE_TYPE](state, 'type'); + it('sets templates type to selected type', () => { + state.selectedTemplateType = ''; - expect(state.selectedTemplateType).toBe('type'); + mutations[types.SET_SELECTED_TEMPLATE_TYPE](state, mockTemplateType); + + expect(state.selectedTemplateType).toBe(mockTemplateType); }); - it('clears templates', () => { - state.templates = ['test']; + it('sets templates to empty array', () => { + state.templates = mockFileTemplates; - mutations[types.SET_SELECTED_TEMPLATE_TYPE](state, 'type'); + mutations[types.SET_SELECTED_TEMPLATE_TYPE](state, mockTemplateType); expect(state.templates).toEqual([]); }); @@ -61,6 +78,8 @@ describe('IDE file templates mutations', () => { describe(`${types.SET_UPDATE_SUCCESS}`, () => { it('sets updateSuccess', () => { + state.updateSuccess = false; + mutations[types.SET_UPDATE_SUCCESS](state, true); expect(state.updateSuccess).toBe(true); diff --git a/spec/javascripts/ide/components/new_dropdown/upload_spec.js b/spec/javascripts/ide/components/new_dropdown/upload_spec.js index 878e17ac805..d19af6af2d7 100644 --- a/spec/javascripts/ide/components/new_dropdown/upload_spec.js +++ b/spec/javascripts/ide/components/new_dropdown/upload_spec.js @@ -78,6 +78,8 @@ describe('new dropdown upload', () => { type: 'blob', content: 'plain text', base64: false, + binary: false, + rawPath: '', }); }); @@ -89,6 +91,8 @@ describe('new dropdown upload', () => { type: 'blob', content: binaryTarget.result.split('base64,')[1], base64: true, + binary: true, + rawPath: binaryTarget.result, }); }); }); diff --git a/spec/javascripts/ide/stores/modules/file_templates/actions_spec.js b/spec/javascripts/ide/stores/modules/file_templates/actions_spec.js index 734233100ab..548962c7a92 100644 --- a/spec/javascripts/ide/stores/modules/file_templates/actions_spec.js +++ b/spec/javascripts/ide/stores/modules/file_templates/actions_spec.js @@ -69,18 +69,16 @@ describe('IDE file templates actions', () => { describe('fetchTemplateTypes', () => { describe('success', () => { - let nextPage; + const pages = [[{ name: 'MIT' }], [{ name: 'Apache' }], [{ name: 'CC' }]]; beforeEach(() => { - mock.onGet(/api\/(.*)\/templates\/licenses/).replyOnce(() => [ - 200, - [ - { - name: 'MIT', - }, - ], - { 'X-NEXT-PAGE': nextPage }, - ]); + mock.onGet(/api\/(.*)\/templates\/licenses/).reply(({ params }) => { + const pageNum = params.page; + const page = pages[pageNum - 1]; + const hasNextPage = pageNum < pages.length; + + return [200, page, hasNextPage ? { 'X-NEXT-PAGE': pageNum + 1 } : {}]; + }); }); it('rejects if selectedTemplateType is empty', done => { @@ -112,43 +110,15 @@ describe('IDE file templates actions', () => { }, { type: 'receiveTemplateTypesSuccess', - payload: [ - { - name: 'MIT', - }, - ], - }, - ], - done, - ); - }); - - it('dispatches actions for next page', done => { - nextPage = '2'; - state.selectedTemplateType = { - key: 'licenses', - }; - - testAction( - actions.fetchTemplateTypes, - null, - state, - [], - [ - { - type: 'requestTemplateTypes', + payload: pages[0], }, { type: 'receiveTemplateTypesSuccess', - payload: [ - { - name: 'MIT', - }, - ], + payload: pages[0].concat(pages[1]), }, { - type: 'fetchTemplateTypes', - payload: 2, + type: 'receiveTemplateTypesSuccess', + payload: pages[0].concat(pages[1]).concat(pages[2]), }, ], done, diff --git a/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js b/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js index 4da8c6196b1..bdf802052b9 100644 --- a/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js +++ b/spec/javascripts/vue_shared/components/content_viewer/content_viewer_spec.js @@ -4,6 +4,7 @@ import axios from '~/lib/utils/axios_utils'; import contentViewer from '~/vue_shared/components/content_viewer/content_viewer.vue'; import mountComponent from 'spec/helpers/vue_mount_component_helper'; import { GREEN_BOX_IMAGE_URL } from 'spec/test_constants'; +import '~/behaviors/markdown/render_gfm'; describe('ContentViewer', () => { let vm; @@ -29,6 +30,7 @@ describe('ContentViewer', () => { path: 'test.md', content: '* Test', projectPath: 'testproject', + type: 'markdown', }); const previewContainer = vm.$el.querySelector('.md-previewer'); @@ -44,6 +46,7 @@ describe('ContentViewer', () => { createComponent({ path: GREEN_BOX_IMAGE_URL, fileSize: 1024, + type: 'image', }); setTimeout(() => { diff --git a/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js b/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js index 7f2e246d656..97c870f27d9 100644 --- a/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js +++ b/spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js @@ -138,22 +138,6 @@ describe('ImageDiffViewer', () => { done(); }); }); - - it('drag handler is working', done => { - vm.$el.querySelector('.view-modes-menu li:nth-child(2)').click(); - - vm.$nextTick(() => { - expect(vm.$el.querySelector('.swipe-bar').style.left).toBe('1px'); - expect(vm.$el.querySelector('.top-handle')).not.toBeNull(); - - dragSlider(vm.$el.querySelector('.swipe-bar'), 40); - - vm.$nextTick(() => { - expect(vm.$el.querySelector('.swipe-bar').style.left).toBe('-20px'); - done(); - }); - }); - }); }); describe('onionSkin', () => { diff --git a/spec/lib/gitlab/checks/branch_check_spec.rb b/spec/lib/gitlab/checks/branch_check_spec.rb index 8d5ab27a17c..71b64a3b9df 100644 --- a/spec/lib/gitlab/checks/branch_check_spec.rb +++ b/spec/lib/gitlab/checks/branch_check_spec.rb @@ -77,117 +77,85 @@ describe Gitlab::Checks::BranchCheck do let(:oldrev) { '0000000000000000000000000000000000000000' } let(:ref) { 'refs/heads/feature' } - context 'protected branch creation feature is disabled' do + context 'user can push to branch' do before do - stub_feature_flags(protected_branch_creation: false) + allow(user_access) + .to receive(:can_push_to_branch?) + .with('feature') + .and_return(true) end - context 'user is not allowed to push to protected branch' do - before do - allow(user_access) - .to receive(:can_push_to_branch?) - .and_return(false) - end - - it 'raises an error' do - expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to protected branches on this project.') - end + it 'does not raise an error' do + expect { subject.validate! }.not_to raise_error end + end - context 'user is allowed to push to protected branch' do - before do - allow(user_access) - .to receive(:can_push_to_branch?) - .and_return(true) - end - - it 'does not raise an error' do - expect { subject.validate! }.not_to raise_error - end + context 'user cannot push to branch' do + before do + allow(user_access) + .to receive(:can_push_to_branch?) + .with('feature') + .and_return(false) end - end - context 'protected branch creation feature is enabled' do - context 'user can push to branch' do + context 'user cannot merge to branch' do before do allow(user_access) - .to receive(:can_push_to_branch?) + .to receive(:can_merge_to_branch?) .with('feature') - .and_return(true) + .and_return(false) end - it 'does not raise an error' do - expect { subject.validate! }.not_to raise_error + it 'raises an error' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to create protected branches on this project.') end end - context 'user cannot push to branch' do + context 'user can merge to branch' do before do allow(user_access) - .to receive(:can_push_to_branch?) + .to receive(:can_merge_to_branch?) .with('feature') - .and_return(false) + .and_return(true) + + allow(project.repository) + .to receive(:branch_names_contains_sha) + .with(newrev) + .and_return(['branch']) end - context 'user cannot merge to branch' do + context "newrev isn't in any protected branches" do before do - allow(user_access) - .to receive(:can_merge_to_branch?) - .with('feature') + allow(ProtectedBranch) + .to receive(:any_protected?) + .with(project, ['branch']) .and_return(false) end it 'raises an error' do - expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to create protected branches on this project.') + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only use an existing protected branch ref as the basis of a new protected branch.') end end - context 'user can merge to branch' do + context 'newrev is included in a protected branch' do before do - allow(user_access) - .to receive(:can_merge_to_branch?) - .with('feature') + allow(ProtectedBranch) + .to receive(:any_protected?) + .with(project, ['branch']) .and_return(true) - - allow(project.repository) - .to receive(:branch_names_contains_sha) - .with(newrev) - .and_return(['branch']) end - context "newrev isn't in any protected branches" do - before do - allow(ProtectedBranch) - .to receive(:any_protected?) - .with(project, ['branch']) - .and_return(false) - end + context 'via web interface' do + let(:protocol) { 'web' } - it 'raises an error' do - expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only use an existing protected branch ref as the basis of a new protected branch.') + it 'allows branch creation' do + expect { subject.validate! }.not_to raise_error end end - context 'newrev is included in a protected branch' do - before do - allow(ProtectedBranch) - .to receive(:any_protected?) - .with(project, ['branch']) - .and_return(true) - end - - context 'via web interface' do - let(:protocol) { 'web' } - - it 'allows branch creation' do - expect { subject.validate! }.not_to raise_error - end - end - - context 'via SSH' do - it 'raises an error' do - expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only create protected branches using the web interface and API.') - end + context 'via SSH' do + it 'raises an error' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You can only create protected branches using the web interface and API.') end end end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 778950c95e4..45fe5d72937 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -1966,6 +1966,70 @@ describe Gitlab::Git::Repository, :seed_helper do end end + describe '#compare_source_branch' do + let(:repository) { Gitlab::Git::Repository.new('default', TEST_GITATTRIBUTES_REPO_PATH, '', 'group/project') } + + context 'within same repository' do + it 'does not create a temp ref' do + expect(repository).not_to receive(:fetch_source_branch!) + expect(repository).not_to receive(:delete_refs) + + compare = repository.compare_source_branch('master', repository, 'feature', straight: false) + expect(compare).to be_a(Gitlab::Git::Compare) + expect(compare.commits.count).to be > 0 + end + + it 'returns empty commits when source ref does not exist' do + compare = repository.compare_source_branch('master', repository, 'non-existent-branch', straight: false) + + expect(compare.commits).to be_empty + end + end + + context 'with different repositories' do + context 'when ref is known by source repo, but not by target' do + before do + mutable_repository.write_ref('another-branch', 'feature') + end + + it 'creates temp ref' do + expect(repository).not_to receive(:fetch_source_branch!) + expect(repository).not_to receive(:delete_refs) + + compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false) + expect(compare).to be_a(Gitlab::Git::Compare) + expect(compare.commits.count).to be > 0 + end + end + + context 'when ref is known by source and target repos' do + before do + mutable_repository.write_ref('another-branch', 'feature') + repository.write_ref('another-branch', 'feature') + end + + it 'does not create a temp ref' do + expect(repository).not_to receive(:fetch_source_branch!) + expect(repository).not_to receive(:delete_refs) + + compare = repository.compare_source_branch('master', mutable_repository, 'another-branch', straight: false) + expect(compare).to be_a(Gitlab::Git::Compare) + expect(compare.commits.count).to be > 0 + end + end + + context 'when ref is unknown by source repo' do + it 'returns nil when source ref does not exist' do + expect(repository).to receive(:fetch_source_branch!).and_call_original + expect(repository).to receive(:delete_refs).and_call_original + + compare = repository.compare_source_branch('master', mutable_repository, 'non-existent-branch', straight: false) + expect(compare).to be_nil + end + end + end + end + describe '#checksum' do it 'calculates the checksum for non-empty repo' do expect(repository.checksum).to eq '51d0a9662681f93e1fee547a6b7ba2bcaf716059' diff --git a/spec/services/compare_service_spec.rb b/spec/services/compare_service_spec.rb index 0e4ef69ec19..fadd43635a6 100644 --- a/spec/services/compare_service_spec.rb +++ b/spec/services/compare_service_spec.rb @@ -19,5 +19,18 @@ describe CompareService do it { expect(subject.diffs.size).to eq(3) } end + + context 'compare with target branch that does not exist' do + subject { service.execute(project, 'non-existent-ref') } + + it { expect(subject).to be_nil } + end + + context 'compare with source branch that does not exist' do + let(:service) { described_class.new(project, 'non-existent-branch') } + subject { service.execute(project, 'non-existent-ref') } + + it { expect(subject).to be_nil } + end end end |