diff options
| author | Filipa Lacerda <filipa@gitlab.com> | 2017-12-12 11:07:53 +0000 |
|---|---|---|
| committer | Filipa Lacerda <filipa@gitlab.com> | 2017-12-12 11:07:53 +0000 |
| commit | 95414b83ddcd451fd97383510d6f34c9861096a9 (patch) | |
| tree | 5c9c491dbe2043138835fd857e2d216f2ebefdbb /spec | |
| parent | a414f548d2c2dfede96b2d3bd43d1d55a0833391 (diff) | |
| parent | 2ef39a80410540995fe3f64bf38ae0f0adb9fc0f (diff) | |
| download | gitlab-ce-95414b83ddcd451fd97383510d6f34c9861096a9.tar.gz | |
Merge branch 'master' into 40468-empty-states
* master: (54 commits)
Updated image for diff navigation docs
Milestone sidebar docs
Use app host instead of asset host when rendering image blob or diff
ActiveRecord::StaleObjectError: Attempted to update a stale object: Ci::Build
Use shared interceptor in note specs
Ignore temporary table in schema
Check for null in job tooltip title
Clarify casing of I18N placeholder names
fixes some markup issues in the js style guide docs
Fix migration that removes issues.branch_name
small change to make less conflict with EE version
Add cop for use of remove_column
Resolve merge conflicts with dev.gitlab.org/master after security release
add index for doc/administration/operations/
Remove RubySampler#sample_objects for performance as well
Bugfix: User can't change the access level of an access requester
Add spec for removing issues.assignee_id
updated imports
Keep track of storage check timings
Remove a header level in the new 'Automatic CE->EE merge' doc
...
Diffstat (limited to 'spec')
49 files changed, 749 insertions, 380 deletions
diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 9c6d584f59b..362d5cc4514 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -62,6 +62,25 @@ describe Groups::GroupMembersController do end end + describe 'PUT update' do + let(:requester) { create(:group_member, :access_request, group: group) } + + before do + group.add_owner(user) + sign_in(user) + end + + Gitlab::Access.options.each do |label, value| + it "can change the access level to #{label}" do + xhr :put, :update, group_member: { access_level: value }, + group_id: group, + id: requester + + expect(requester.reload.human_access).to eq(label) + end + end + end + describe 'DELETE destroy' do let(:member) { create(:group_member, :developer, group: group) } diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index a34dc27a5ed..290dba0610a 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -66,6 +66,26 @@ describe Projects::ProjectMembersController do end end + describe 'PUT update' do + let(:requester) { create(:project_member, :access_request, project: project) } + + before do + project.add_master(user) + sign_in(user) + end + + Gitlab::Access.options.each do |label, value| + it "can change the access level to #{label}" do + xhr :put, :update, project_member: { access_level: value }, + namespace_id: project.namespace, + project_id: project, + id: requester + + expect(requester.reload.human_access).to eq(label) + end + end + end + describe 'DELETE destroy' do let(:member) { create(:project_member, :developer, project: project) } diff --git a/spec/factories/users.rb b/spec/factories/users.rb index 4000cd085b7..8ace424f8af 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -58,6 +58,10 @@ FactoryGirl.define do end end + trait :readme do + project_view :readme + end + factory :omniauth_user do transient do extern_uid '123456' diff --git a/spec/features/groups/members/manage_members.rb b/spec/features/groups/members/manage_members.rb index da1e17225db..21f7b4999ad 100644 --- a/spec/features/groups/members/manage_members.rb +++ b/spec/features/groups/members/manage_members.rb @@ -38,6 +38,27 @@ feature 'Groups > Members > Manage members' do end end + scenario 'do not disclose email addresses', :js do + group.add_owner(user1) + create(:user, email: 'undisclosed_email@gitlab.com', name: "Jane 'invisible' Doe") + + visit group_group_members_path(group) + + find('.select2-container').click + select_input = find('.select2-input') + + select_input.send_keys('@gitlab.com') + wait_for_requests + + expect(page).to have_content('No matches found') + + select_input.native.clear + select_input.send_keys('undisclosed_email@gitlab.com') + wait_for_requests + + expect(page).to have_content("Jane 'invisible' Doe") + end + scenario 'remove user from group', :js do group.add_owner(user1) group.add_developer(user2) diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb index c31b636d67f..6a9a80235c1 100644 --- a/spec/features/issues/gfm_autocomplete_spec.rb +++ b/spec/features/issues/gfm_autocomplete_spec.rb @@ -15,7 +15,7 @@ feature 'GFM autocomplete', :js do end it 'updates issue descripton with GFM reference' do - find('.issuable-edit').click + find('.js-issuable-edit').click simulate_input('#issue-description', "@#{user.name[0...3]}") diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb index 8b8080563d3..749aa25e632 100644 --- a/spec/helpers/preferences_helper_spec.rb +++ b/spec/helpers/preferences_helper_spec.rb @@ -77,15 +77,6 @@ describe PreferencesHelper do end end - def stub_user(messages = {}) - if messages.empty? - allow(helper).to receive(:current_user).and_return(nil) - else - allow(helper).to receive(:current_user) - .and_return(double('user', messages)) - end - end - describe '#default_project_view' do context 'user not signed in' do before do @@ -125,5 +116,70 @@ describe PreferencesHelper do end end end + + context 'user signed in' do + let(:user) { create(:user, :readme) } + let(:project) { create(:project, :public, :repository) } + + before do + helper.instance_variable_set(:@project, project) + allow(helper).to receive(:current_user).and_return(user) + end + + context 'when the user is allowed to see the code' do + it 'returns the project view' do + allow(helper).to receive(:can?).with(user, :download_code, project).and_return(true) + + expect(helper.default_project_view).to eq('readme') + end + end + + context 'with wikis enabled and the right policy for the user' do + before do + project.project_feature.update_attribute(:issues_access_level, 0) + allow(helper).to receive(:can?).with(user, :download_code, project).and_return(false) + end + + it 'returns wiki if the user has the right policy' do + allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(true) + + expect(helper.default_project_view).to eq('wiki') + end + + it 'returns customize_workflow if the user does not have the right policy' do + allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(false) + + expect(helper.default_project_view).to eq('customize_workflow') + end + end + + context 'with issues as a feature available' do + it 'return issues' do + allow(helper).to receive(:can?).with(user, :download_code, project).and_return(false) + allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(false) + + expect(helper.default_project_view).to eq('projects/issues/issues') + end + end + + context 'with no activity, no wikies and no issues' do + it 'returns customize_workflow as default' do + project.project_feature.update_attribute(:issues_access_level, 0) + allow(helper).to receive(:can?).with(user, :download_code, project).and_return(false) + allow(helper).to receive(:can?).with(user, :read_wiki, project).and_return(false) + + expect(helper.default_project_view).to eq('customize_workflow') + end + end + end + end + + def stub_user(messages = {}) + if messages.empty? + allow(helper).to receive(:current_user).and_return(nil) + else + allow(helper).to receive(:current_user) + .and_return(double('user', messages)) + end end end diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js index 9e5b0bd3efe..0e656858182 100644 --- a/spec/javascripts/boards/boards_store_spec.js +++ b/spec/javascripts/boards/boards_store_spec.js @@ -9,7 +9,6 @@ import Vue from 'vue'; import Cookies from 'js-cookie'; -import '~/lib/utils/url_utility'; import '~/boards/models/issue'; import '~/boards/models/label'; import '~/boards/models/list'; diff --git a/spec/javascripts/boards/issue_spec.js b/spec/javascripts/boards/issue_spec.js index 10b88878c2a..41dcb19df3c 100644 --- a/spec/javascripts/boards/issue_spec.js +++ b/spec/javascripts/boards/issue_spec.js @@ -4,7 +4,6 @@ /* global mockBoardService */ import Vue from 'vue'; -import '~/lib/utils/url_utility'; import '~/boards/models/issue'; import '~/boards/models/label'; import '~/boards/models/list'; diff --git a/spec/javascripts/boards/list_spec.js b/spec/javascripts/boards/list_spec.js index d4627223a12..eead396ca7e 100644 --- a/spec/javascripts/boards/list_spec.js +++ b/spec/javascripts/boards/list_spec.js @@ -9,7 +9,6 @@ import Vue from 'vue'; -import '~/lib/utils/url_utility'; import '~/boards/models/issue'; import '~/boards/models/label'; import '~/boards/models/list'; diff --git a/spec/javascripts/filtered_search/filtered_search_manager_spec.js b/spec/javascripts/filtered_search/filtered_search_manager_spec.js index 230c15e5de6..5111632d681 100644 --- a/spec/javascripts/filtered_search/filtered_search_manager_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_manager_spec.js @@ -1,8 +1,8 @@ +import * as urlUtils from '~/lib/utils/url_utility'; import * as recentSearchesStoreSrc from '~/filtered_search/stores/recent_searches_store'; import RecentSearchesService from '~/filtered_search/services/recent_searches_service'; import RecentSearchesServiceError from '~/filtered_search/services/recent_searches_service_error'; import RecentSearchesRoot from '~/filtered_search/recent_searches_root'; -import '~/lib/utils/url_utility'; import '~/lib/utils/common_utils'; import '~/filtered_search/filtered_search_token_keys'; import '~/filtered_search/filtered_search_tokenizer'; @@ -162,7 +162,7 @@ describe('Filtered Search Manager', () => { it('should search with a single word', (done) => { input.value = 'searchTerm'; - spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + spyOn(urlUtils, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&search=searchTerm`); done(); }); @@ -173,7 +173,7 @@ describe('Filtered Search Manager', () => { it('should search with multiple words', (done) => { input.value = 'awesome search terms'; - spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + spyOn(urlUtils, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&search=awesome+search+terms`); done(); }); @@ -184,7 +184,7 @@ describe('Filtered Search Manager', () => { it('should search with special characters', (done) => { input.value = '~!@#$%^&*()_+{}:<>,.?/'; - spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + spyOn(urlUtils, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&search=~!%40%23%24%25%5E%26*()_%2B%7B%7D%3A%3C%3E%2C.%3F%2F`); done(); }); @@ -198,7 +198,7 @@ describe('Filtered Search Manager', () => { ${FilteredSearchSpecHelper.createFilterVisualTokenHTML('label', '~bug')} `); - spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + spyOn(urlUtils, 'visitUrl').and.callFake((url) => { expect(url).toEqual(`${defaultParams}&label_name[]=bug`); done(); }); diff --git a/spec/javascripts/fly_out_nav_spec.js b/spec/javascripts/fly_out_nav_spec.js index 4f20e31f511..a3fa07d5bc2 100644 --- a/spec/javascripts/fly_out_nav_spec.js +++ b/spec/javascripts/fly_out_nav_spec.js @@ -253,7 +253,7 @@ describe('Fly out sidebar navigation', () => { it('shows collapsed only sub-items if icon only sidebar', () => { const subItems = el.querySelector('.sidebar-sub-level-items'); const sidebar = document.createElement('div'); - sidebar.classList.add('sidebar-icons-only'); + sidebar.classList.add('sidebar-collapsed-desktop'); subItems.classList.add('is-fly-out-only'); setSidebar(sidebar); @@ -343,7 +343,7 @@ describe('Fly out sidebar navigation', () => { it('returns true when active & collapsed sidebar', () => { const sidebar = document.createElement('div'); - sidebar.classList.add('sidebar-icons-only'); + sidebar.classList.add('sidebar-collapsed-desktop'); el.classList.add('active'); setSidebar(sidebar); diff --git a/spec/javascripts/gl_dropdown_spec.js b/spec/javascripts/gl_dropdown_spec.js index ca048123bf7..b13d1bf8dff 100644 --- a/spec/javascripts/gl_dropdown_spec.js +++ b/spec/javascripts/gl_dropdown_spec.js @@ -2,7 +2,7 @@ import '~/gl_dropdown'; import '~/lib/utils/common_utils'; -import '~/lib/utils/url_utility'; +import * as urlUtils from '~/lib/utils/url_utility'; describe('glDropdown', function describeDropdown() { preloadFixtures('static/gl_dropdown.html.raw'); @@ -137,13 +137,13 @@ describe('glDropdown', function describeDropdown() { expect(this.dropdownContainerElement).toHaveClass('open'); const randomIndex = Math.floor(Math.random() * (this.projectsData.length - 1)) + 0; navigateWithKeys('down', randomIndex, () => { - spyOn(gl.utils, 'visitUrl').and.stub(); + spyOn(urlUtils, 'visitUrl').and.stub(); navigateWithKeys('enter', null, () => { expect(this.dropdownContainerElement).not.toHaveClass('open'); const link = $(`${ITEM_SELECTOR}:eq(${randomIndex}) a`, this.$dropdownMenuElement); expect(link).toHaveClass('is-active'); const linkedLocation = link.attr('href'); - if (linkedLocation && linkedLocation !== '#') expect(gl.utils.visitUrl).toHaveBeenCalledWith(linkedLocation); + if (linkedLocation && linkedLocation !== '#') expect(urlUtils.visitUrl).toHaveBeenCalledWith(linkedLocation); }); }); }); diff --git a/spec/javascripts/groups/components/app_spec.js b/spec/javascripts/groups/components/app_spec.js index 59d4f7c45c6..97e39f6411b 100644 --- a/spec/javascripts/groups/components/app_spec.js +++ b/spec/javascripts/groups/components/app_spec.js @@ -1,9 +1,9 @@ import Vue from 'vue'; +import * as utils from '~/lib/utils/url_utility'; import appComponent from '~/groups/components/app.vue'; import groupFolderComponent from '~/groups/components/group_folder.vue'; import groupItemComponent from '~/groups/components/group_item.vue'; - import eventHub from '~/groups/event_hub'; import GroupsStore from '~/groups/store/groups_store'; import GroupsService from '~/groups/service/groups_service'; @@ -176,7 +176,7 @@ describe('AppComponent', () => { it('should fetch groups for provided page details and update window state', (done) => { spyOn(vm, 'fetchGroups').and.returnValue(returnServicePromise(mockGroups)); spyOn(vm, 'updateGroups').and.callThrough(); - spyOn(gl.utils, 'mergeUrlParams').and.callThrough(); + spyOn(utils, 'mergeUrlParams').and.callThrough(); spyOn(window.history, 'replaceState'); spyOn($, 'scrollTo'); @@ -192,7 +192,7 @@ describe('AppComponent', () => { setTimeout(() => { expect(vm.isLoading).toBeFalsy(); expect($.scrollTo).toHaveBeenCalledWith(0); - expect(gl.utils.mergeUrlParams).toHaveBeenCalledWith({ page: 2 }, jasmine.any(String)); + expect(utils.mergeUrlParams).toHaveBeenCalledWith({ page: 2 }, jasmine.any(String)); expect(window.history.replaceState).toHaveBeenCalledWith({ page: jasmine.any(String), }, jasmine.any(String), jasmine.any(String)); diff --git a/spec/javascripts/groups/components/group_item_spec.js b/spec/javascripts/groups/components/group_item_spec.js index 0f4fbdae445..618d0022e4f 100644 --- a/spec/javascripts/groups/components/group_item_spec.js +++ b/spec/javascripts/groups/components/group_item_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; - +import * as urlUtils from '~/lib/utils/url_utility'; import groupItemComponent from '~/groups/components/group_item.vue'; import groupFolderComponent from '~/groups/components/group_folder.vue'; import eventHub from '~/groups/event_hub'; @@ -136,13 +136,13 @@ describe('GroupItemComponent', () => { const group = Object.assign({}, mockParentGroupItem); group.childrenCount = 0; const newVm = createComponent(group); - spyOn(gl.utils, 'visitUrl').and.stub(); + spyOn(urlUtils, 'visitUrl').and.stub(); spyOn(eventHub, '$emit'); newVm.onClickRowGroup(event); setTimeout(() => { expect(eventHub.$emit).not.toHaveBeenCalled(); - expect(gl.utils.visitUrl).toHaveBeenCalledWith(newVm.group.relativePath); + expect(urlUtils.visitUrl).toHaveBeenCalledWith(newVm.group.relativePath); done(); }, 0); }); diff --git a/spec/javascripts/image_diff/helpers/badge_helper_spec.js b/spec/javascripts/image_diff/helpers/badge_helper_spec.js index fb9c7e59031..ce3add1fd90 100644 --- a/spec/javascripts/image_diff/helpers/badge_helper_spec.js +++ b/spec/javascripts/image_diff/helpers/badge_helper_spec.js @@ -89,15 +89,8 @@ describe('badge helper', () => { }); it('should create icon comment button', () => { - const iconEl = buttonEl.querySelector('i'); + const iconEl = buttonEl.querySelector('svg'); expect(iconEl).toBeDefined(); - expect(iconEl.classList.contains('fa')).toEqual(true); - expect(iconEl.classList.contains('fa-comment-o')).toEqual(true); - }); - - it('should have .image-comment-badge.inverted in button class', () => { - expect(buttonEl.classList.contains('image-comment-badge')).toEqual(true); - expect(buttonEl.classList.contains('inverted')).toEqual(true); }); }); diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index 53b8a368d28..729c3c29f22 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -1,6 +1,7 @@ import Vue from 'vue'; import '~/render_math'; import '~/render_gfm'; +import * as urlUtils from '~/lib/utils/url_utility'; import issuableApp from '~/issue_show/components/app.vue'; import eventHub from '~/issue_show/event_hub'; import issueShowData from '../mock_data'; @@ -180,7 +181,7 @@ describe('Issuable output', () => { }); it('does not redirect if issue has not moved', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve) => { resolve({ json() { @@ -196,7 +197,7 @@ describe('Issuable output', () => { setTimeout(() => { expect( - gl.utils.visitUrl, + urlUtils.visitUrl, ).not.toHaveBeenCalled(); done(); @@ -204,7 +205,7 @@ describe('Issuable output', () => { }); it('redirects if returned web_url has changed', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn(vm.service, 'updateIssuable').and.callFake(() => new Promise((resolve) => { resolve({ json() { @@ -220,7 +221,7 @@ describe('Issuable output', () => { setTimeout(() => { expect( - gl.utils.visitUrl, + urlUtils.visitUrl, ).toHaveBeenCalledWith('/testing-issue-move'); done(); @@ -319,7 +320,7 @@ describe('Issuable output', () => { describe('deleteIssuable', () => { it('changes URL when deleted', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn(vm.service, 'deleteIssuable').and.callFake(() => new Promise((resolve) => { resolve({ json() { @@ -332,7 +333,7 @@ describe('Issuable output', () => { setTimeout(() => { expect( - gl.utils.visitUrl, + urlUtils.visitUrl, ).toHaveBeenCalledWith('/test'); done(); @@ -340,7 +341,7 @@ describe('Issuable output', () => { }); it('stops polling when deleting', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn(vm.poll, 'stop').and.callThrough(); spyOn(vm.service, 'deleteIssuable').and.callFake(() => new Promise((resolve) => { resolve({ diff --git a/spec/javascripts/job_spec.js b/spec/javascripts/job_spec.js index 20c4caa865d..4f06237deb5 100644 --- a/spec/javascripts/job_spec.js +++ b/spec/javascripts/job_spec.js @@ -1,6 +1,6 @@ import { bytesToKiB } from '~/lib/utils/number_utils'; +import * as urlUtils from '~/lib/utils/url_utility'; import '~/lib/utils/datetime_utility'; -import '~/lib/utils/url_utility'; import Job from '~/job'; import '~/breakpoints'; @@ -65,7 +65,7 @@ describe('Job', () => { const deferred2 = $.Deferred(); const deferred3 = $.Deferred(); spyOn($, 'ajax').and.returnValues(deferred1.promise(), deferred2.promise(), deferred3.promise()); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); deferred1.resolve({ html: '<span>Update<span>', @@ -103,7 +103,7 @@ describe('Job', () => { spyOn($, 'ajax').and.returnValues(deferred1.promise(), deferred2.promise(), deferred3.promise()); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); deferred1.resolve({ html: '<span>Update<span>', @@ -134,7 +134,7 @@ describe('Job', () => { describe('truncated information', () => { describe('when size is less than total', () => { it('shows information about truncated log', () => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); const deferred = $.Deferred(); spyOn($, 'ajax').and.returnValue(deferred.promise()); @@ -153,7 +153,7 @@ describe('Job', () => { it('shows the size in KiB', () => { const size = 50; - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); const deferred = $.Deferred(); spyOn($, 'ajax').and.returnValue(deferred.promise()); @@ -179,7 +179,7 @@ describe('Job', () => { spyOn($, 'ajax').and.returnValues(deferred1.promise(), deferred2.promise(), deferred3.promise()); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); deferred1.resolve({ html: '<span>Update</span>', @@ -214,7 +214,7 @@ describe('Job', () => { it('renders the raw link', () => { const deferred = $.Deferred(); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn($, 'ajax').and.returnValue(deferred.promise()); deferred.resolve({ @@ -236,7 +236,7 @@ describe('Job', () => { describe('when size is equal than total', () => { it('does not show the trunctated information', () => { const deferred = $.Deferred(); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn($, 'ajax').and.returnValue(deferred.promise()); deferred.resolve({ @@ -257,7 +257,7 @@ describe('Job', () => { describe('output trace', () => { beforeEach(() => { const deferred = $.Deferred(); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); spyOn($, 'ajax').and.returnValue(deferred.promise()); deferred.resolve({ diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index e441d1153ed..5076435e7a8 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -1,6 +1,7 @@ /* eslint-disable no-var, comma-dangle, object-shorthand */ /* global Notes */ +import * as urlUtils from '~/lib/utils/url_utility'; import '~/merge_request_tabs'; import '~/commit/pipelines/pipelines_bundle'; import '~/breakpoints'; @@ -333,7 +334,7 @@ import 'vendor/jquery.scrollTo'; describe('with note fragment hash', () => { it('should expand and scroll to linked fragment hash #note_xxx', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue(noteId); + spyOn(urlUtils, 'getLocationHash').and.returnValue(noteId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteId.length).toBeGreaterThan(0); @@ -345,7 +346,7 @@ import 'vendor/jquery.scrollTo'; }); it('should gracefully ignore non-existant fragment hash', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); + spyOn(urlUtils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(window.notes.toggleDiffNote).not.toHaveBeenCalled(); @@ -354,7 +355,7 @@ import 'vendor/jquery.scrollTo'; describe('with line number fragment hash', () => { it('should gracefully ignore line number fragment hash', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue(noteLineNumId); + spyOn(urlUtils, 'getLocationHash').and.returnValue(noteLineNumId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteLineNumId.length).toBeGreaterThan(0); @@ -387,7 +388,7 @@ import 'vendor/jquery.scrollTo'; describe('with note fragment hash', () => { it('should expand and scroll to linked fragment hash #note_xxx', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue(noteId); + spyOn(urlUtils, 'getLocationHash').and.returnValue(noteId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); @@ -400,7 +401,7 @@ import 'vendor/jquery.scrollTo'; }); it('should gracefully ignore non-existant fragment hash', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); + spyOn(urlUtils, 'getLocationHash').and.returnValue('note_something-that-does-not-exist'); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(window.notes.toggleDiffNote).not.toHaveBeenCalled(); @@ -409,7 +410,7 @@ import 'vendor/jquery.scrollTo'; describe('with line number fragment hash', () => { it('should gracefully ignore line number fragment hash', function () { - spyOn(window.gl.utils, 'getLocationHash').and.returnValue(noteLineNumId); + spyOn(urlUtils, 'getLocationHash').and.returnValue(noteLineNumId); this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); expect(noteLineNumId.length).toBeGreaterThan(0); diff --git a/spec/javascripts/notes/components/issue_comment_form_spec.js b/spec/javascripts/notes/components/comment_form_spec.js index 04a7f8e32f1..20e352dd8bd 100644 --- a/spec/javascripts/notes/components/issue_comment_form_spec.js +++ b/spec/javascripts/notes/components/comment_form_spec.js @@ -1,7 +1,7 @@ import Vue from 'vue'; import Autosize from 'autosize'; import store from '~/notes/stores'; -import issueCommentForm from '~/notes/components/issue_comment_form.vue'; +import issueCommentForm from '~/notes/components/comment_form.vue'; import { loggedOutnoteableData, notesDataMock, userDataMock, noteableDataMock } from '../mock_data'; import { keyboardDownEvent } from '../../issue_show/helpers'; diff --git a/spec/javascripts/notes/components/issue_note_app_spec.js b/spec/javascripts/notes/components/note_app_spec.js index 8e43037f356..7c8d6685ee1 100644 --- a/spec/javascripts/notes/components/issue_note_app_spec.js +++ b/spec/javascripts/notes/components/note_app_spec.js @@ -1,26 +1,15 @@ import Vue from 'vue'; -import issueNotesApp from '~/notes/components/issue_notes_app.vue'; +import notesApp from '~/notes/components/notes_app.vue'; import service from '~/notes/services/notes_service'; import * as mockData from '../mock_data'; +import getSetTimeoutPromise from '../../helpers/set_timeout_promise_helper'; -describe('issue_note_app', () => { +describe('note_app', () => { let mountComponent; let vm; - const individualNoteInterceptor = (request, next) => { - next(request.respondWith(JSON.stringify(mockData.individualNoteServerResponse), { - status: 200, - })); - }; - - const discussionNoteInterceptor = (request, next) => { - next(request.respondWith(JSON.stringify(mockData.discussionNoteServerResponse), { - status: 200, - })); - }; - beforeEach(() => { - const IssueNotesApp = Vue.extend(issueNotesApp); + const IssueNotesApp = Vue.extend(notesApp); mountComponent = (data) => { const props = data || { @@ -74,16 +63,16 @@ describe('issue_note_app', () => { describe('render', () => { beforeEach(() => { - Vue.http.interceptors.push(individualNoteInterceptor); + Vue.http.interceptors.push(mockData.individualNoteInterceptor); vm = mountComponent(); }); afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, individualNoteInterceptor); + Vue.http.interceptors = _.without(Vue.http.interceptors, mockData.individualNoteInterceptor); }); it('should render list of notes', (done) => { - const note = mockData.individualNoteServerResponse[0].notes[0]; + const note = mockData.INDIVIDUAL_NOTE_RESPONSE_MAP.GET['/gitlab-org/gitlab-ce/issues/26/discussions.json'][0].notes[0]; setTimeout(() => { expect( @@ -129,13 +118,16 @@ describe('issue_note_app', () => { describe('update note', () => { describe('individual note', () => { beforeEach(() => { - Vue.http.interceptors.push(individualNoteInterceptor); - spyOn(service, 'updateNote').and.callFake(() => Promise.resolve()); + Vue.http.interceptors.push(mockData.individualNoteInterceptor); + spyOn(service, 'updateNote').and.callThrough(); vm = mountComponent(); }); afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, individualNoteInterceptor); + Vue.http.interceptors = _.without( + Vue.http.interceptors, + mockData.individualNoteInterceptor, + ); }); it('renders edit form', (done) => { @@ -149,28 +141,36 @@ describe('issue_note_app', () => { }); it('calls the service to update the note', (done) => { - setTimeout(() => { - vm.$el.querySelector('.js-note-edit').click(); - Vue.nextTick(() => { + getSetTimeoutPromise() + .then(() => { + vm.$el.querySelector('.js-note-edit').click(); + }) + .then(Vue.nextTick) + .then(() => { vm.$el.querySelector('.js-vue-issue-note-form').value = 'this is a note'; vm.$el.querySelector('.js-vue-issue-save').click(); expect(service.updateNote).toHaveBeenCalled(); - done(); - }); - }, 0); + }) + // Wait for the requests to finish before destroying + .then(Vue.nextTick) + .then(done) + .catch(done.fail); }); }); describe('dicussion note', () => { beforeEach(() => { - Vue.http.interceptors.push(discussionNoteInterceptor); - spyOn(service, 'updateNote').and.callFake(() => Promise.resolve()); + Vue.http.interceptors.push(mockData.discussionNoteInterceptor); + spyOn(service, 'updateNote').and.callThrough(); vm = mountComponent(); }); afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, discussionNoteInterceptor); + Vue.http.interceptors = _.without( + Vue.http.interceptors, + mockData.discussionNoteInterceptor, + ); }); it('renders edit form', (done) => { @@ -184,16 +184,21 @@ describe('issue_note_app', () => { }); it('updates the note and resets the edit form', (done) => { - setTimeout(() => { - vm.$el.querySelector('.js-note-edit').click(); - Vue.nextTick(() => { + getSetTimeoutPromise() + .then(() => { + vm.$el.querySelector('.js-note-edit').click(); + }) + .then(Vue.nextTick) + .then(() => { vm.$el.querySelector('.js-vue-issue-note-form').value = 'this is a note'; vm.$el.querySelector('.js-vue-issue-save').click(); expect(service.updateNote).toHaveBeenCalled(); - done(); - }); - }, 0); + }) + // Wait for the requests to finish before destroying + .then(Vue.nextTick) + .then(done) + .catch(done.fail); }); }); }); @@ -216,12 +221,12 @@ describe('issue_note_app', () => { describe('edit form', () => { beforeEach(() => { - Vue.http.interceptors.push(individualNoteInterceptor); + Vue.http.interceptors.push(mockData.individualNoteInterceptor); vm = mountComponent(); }); afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, individualNoteInterceptor); + Vue.http.interceptors = _.without(Vue.http.interceptors, mockData.individualNoteInterceptor); }); it('should render markdown docs url', (done) => { diff --git a/spec/javascripts/notes/components/issue_note_body_spec.js b/spec/javascripts/notes/components/note_body_spec.js index 37aad50737b..b42e7943b98 100644 --- a/spec/javascripts/notes/components/issue_note_body_spec.js +++ b/spec/javascripts/notes/components/note_body_spec.js @@ -1,7 +1,7 @@ import Vue from 'vue'; import store from '~/notes/stores'; -import noteBody from '~/notes/components/issue_note_body.vue'; +import noteBody from '~/notes/components/note_body.vue'; import { noteableDataMock, notesDataMock, note } from '../mock_data'; describe('issue_note_body component', () => { diff --git a/spec/javascripts/notes/components/issue_note_form_spec.js b/spec/javascripts/notes/components/note_form_spec.js index d42ef239711..86e9e2a32a9 100644 --- a/spec/javascripts/notes/components/issue_note_form_spec.js +++ b/spec/javascripts/notes/components/note_form_spec.js @@ -1,6 +1,6 @@ import Vue from 'vue'; import store from '~/notes/stores'; -import issueNoteForm from '~/notes/components/issue_note_form.vue'; +import issueNoteForm from '~/notes/components/note_form.vue'; import { noteableDataMock, notesDataMock } from '../mock_data'; import { keyboardDownEvent } from '../../issue_show/helpers'; diff --git a/spec/javascripts/notes/components/issue_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js index b6ae55d44f5..19504e4f7c8 100644 --- a/spec/javascripts/notes/components/issue_discussion_spec.js +++ b/spec/javascripts/notes/components/noteable_discussion_spec.js @@ -1,6 +1,6 @@ import Vue from 'vue'; import store from '~/notes/stores'; -import issueDiscussion from '~/notes/components/issue_discussion.vue'; +import issueDiscussion from '~/notes/components/noteable_discussion.vue'; import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data'; describe('issue_discussion component', () => { @@ -30,7 +30,7 @@ describe('issue_discussion component', () => { it('should render discussion header', () => { expect(vm.$el.querySelector('.discussion-header')).toBeDefined(); - expect(vm.$el.querySelectorAll('.notes li').length).toEqual(discussionMock.notes.length); + expect(vm.$el.querySelector('.notes').children.length).toEqual(discussionMock.notes.length); }); describe('actions', () => { diff --git a/spec/javascripts/notes/components/issue_note_spec.js b/spec/javascripts/notes/components/noteable_note_spec.js index 73fd188dbe5..c8a6cb7e612 100644 --- a/spec/javascripts/notes/components/issue_note_spec.js +++ b/spec/javascripts/notes/components/noteable_note_spec.js @@ -1,7 +1,7 @@ import Vue from 'vue'; import store from '~/notes/stores'; -import issueNote from '~/notes/components/issue_note.vue'; +import issueNote from '~/notes/components/noteable_note.vue'; import { noteableDataMock, notesDataMock, note } from '../mock_data'; describe('issue_note', () => { @@ -41,4 +41,19 @@ describe('issue_note', () => { it('should render issue body', () => { expect(vm.$el.querySelector('.note-text').innerHTML).toEqual(note.note_html); }); + + it('prevents note preview xss', (done) => { + const imgSrc = 'data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7'; + const noteBody = `<img src="${imgSrc}" onload="alert(1)" />`; + const alertSpy = spyOn(window, 'alert'); + vm.updateNote = () => new Promise($.noop); + + vm.formUpdateHandler(noteBody, null, $.noop); + + setTimeout(() => { + expect(alertSpy).not.toHaveBeenCalled(); + expect(vm.note.note_html).toEqual(_.escape(noteBody)); + done(); + }, 0); + }); }); diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js index 42497de3c55..6b608adff15 100644 --- a/spec/javascripts/notes/mock_data.js +++ b/spec/javascripts/notes/mock_data.js @@ -312,138 +312,212 @@ export const loggedOutnoteableData = { "preview_note_path": "/gitlab-org/gitlab-ce/preview_markdown?quick_actions_target_id=98&quick_actions_target_type=Issue" } -export const individualNoteServerResponse = [{ - "id": "0fb4e0e3f9276e55ff32eb4195add694aece4edd", - "reply_id": "0fb4e0e3f9276e55ff32eb4195add694aece4edd", - "expanded": true, - "notes": [{ - "id": 1390, - "attachment": { - "url": null, - "filename": null, - "image": false - }, - "author": { - "id": 1, - "name": "Root", - "username": "root", - "state": "active", - "avatar_url": null, - "path": "/root" - }, - "created_at": "2017-08-01T17:09:33.762Z", - "updated_at": "2017-08-01T17:09:33.762Z", - "system": false, - "noteable_id": 98, - "noteable_type": "Issue", - "type": null, - "human_access": "Owner", - "note": "sdfdsaf", - "note_html": "\u003cp dir=\"auto\"\u003esdfdsaf\u003c/p\u003e", - "current_user": { - "can_edit": true +export const INDIVIDUAL_NOTE_RESPONSE_MAP = { + 'GET': { + '/gitlab-org/gitlab-ce/issues/26/discussions.json': [{ + "id": "0fb4e0e3f9276e55ff32eb4195add694aece4edd", + "reply_id": "0fb4e0e3f9276e55ff32eb4195add694aece4edd", + "expanded": true, + "notes": [{ + "id": 1390, + "attachment": { + "url": null, + "filename": null, + "image": false + }, + "author": { + "id": 1, + "name": "Root", + "username": "root", + "state": "active", + "avatar_url": null, + "path": "/root" + }, + "created_at": "2017-08-01T17:09:33.762Z", + "updated_at": "2017-08-01T17:09:33.762Z", + "system": false, + "noteable_id": 98, + "noteable_type": "Issue", + "type": null, + "human_access": "Owner", + "note": "sdfdsaf", + "note_html": "\u003cp dir=\"auto\"\u003esdfdsaf\u003c/p\u003e", + "current_user": { + "can_edit": true + }, + "discussion_id": "0fb4e0e3f9276e55ff32eb4195add694aece4edd", + "emoji_awardable": true, + "award_emoji": [{ + "name": "baseball", + "user": { + "id": 1, + "name": "Root", + "username": "root" + } + }, { + "name": "art", + "user": { + "id": 1, + "name": "Root", + "username": "root" + } + }], + "toggle_award_path": "/gitlab-org/gitlab-ce/notes/1390/toggle_award_emoji", + "report_abuse_path": "/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F26%23note_1390\u0026user_id=1", + "path": "/gitlab-org/gitlab-ce/notes/1390" + }], + "individual_note": true + }, { + "id": "70d5c92a4039a36c70100c6691c18c27e4b0a790", + "reply_id": "70d5c92a4039a36c70100c6691c18c27e4b0a790", + "expanded": true, + "notes": [{ + "id": 1391, + "attachment": { + "url": null, + "filename": null, + "image": false + }, + "author": { + "id": 1, + "name": "Root", + "username": "root", + "state": "active", + "avatar_url": null, + "path": "/root" + }, + "created_at": "2017-08-02T10:51:38.685Z", + "updated_at": "2017-08-02T10:51:38.685Z", + "system": false, + "noteable_id": 98, + "noteable_type": "Issue", + "type": null, + "human_access": "Owner", + "note": "New note!", + "note_html": "\u003cp dir=\"auto\"\u003eNew note!\u003c/p\u003e", + "current_user": { + "can_edit": true + }, + "discussion_id": "70d5c92a4039a36c70100c6691c18c27e4b0a790", + "emoji_awardable": true, + "award_emoji": [], + "toggle_award_path": "/gitlab-org/gitlab-ce/notes/1391/toggle_award_emoji", + "report_abuse_path": "/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F26%23note_1391\u0026user_id=1", + "path": "/gitlab-org/gitlab-ce/notes/1391" + }], + "individual_note": true + }], + '/gitlab-org/gitlab-ce/noteable/issue/98/notes': { + last_fetched_at: 1512900838, + notes: [], }, - "discussion_id": "0fb4e0e3f9276e55ff32eb4195add694aece4edd", - "emoji_awardable": true, - "award_emoji": [{ - "name": "baseball", - "user": { + }, + 'PUT': { + '/gitlab-org/gitlab-ce/notes/1471': { + "commands_changes": null, + "valid": true, + "id": 1471, + "attachment": null, + "author": { "id": 1, "name": "Root", - "username": "root" - } - }, { - "name": "art", - "user": { + "username": "root", + "state": "active", + "avatar_url": null, + "path": "/root" + }, + "created_at": "2017-08-08T16:53:00.666Z", + "updated_at": "2017-12-10T11:03:21.876Z", + "system": false, + "noteable_id": 124, + "noteable_type": "Issue", + "noteable_iid": 29, + "type": "DiscussionNote", + "human_access": "Owner", + "note": "Adding a comment", + "note_html": "\u003cp dir=\"auto\"\u003eAdding a comment\u003c/p\u003e", + "last_edited_at": "2017-12-10T11:03:21.876Z", + "last_edited_by": { "id": 1, - "name": "Root", - "username": "root" - } - }], - "toggle_award_path": "/gitlab-org/gitlab-ce/notes/1390/toggle_award_emoji", - "report_abuse_path": "/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F26%23note_1390\u0026user_id=1", - "path": "/gitlab-org/gitlab-ce/notes/1390" - }], - "individual_note": true - }, { - "id": "70d5c92a4039a36c70100c6691c18c27e4b0a790", - "reply_id": "70d5c92a4039a36c70100c6691c18c27e4b0a790", - "expanded": true, - "notes": [{ - "id": 1391, - "attachment": { - "url": null, - "filename": null, - "image": false - }, - "author": { - "id": 1, - "name": "Root", - "username": "root", - "state": "active", - "avatar_url": null, - "path": "/root" - }, - "created_at": "2017-08-02T10:51:38.685Z", - "updated_at": "2017-08-02T10:51:38.685Z", - "system": false, - "noteable_id": 98, - "noteable_type": "Issue", - "type": null, - "human_access": "Owner", - "note": "New note!", - "note_html": "\u003cp dir=\"auto\"\u003eNew note!\u003c/p\u003e", - "current_user": { - "can_edit": true + "name": 'Root', + "username": 'root', + "state": 'active', + "avatar_url": null, + "path": '/root', + }, + "current_user": { + "can_edit": true + }, + "discussion_id": "a3ed36e29b1957efb3b68c53e2d7a2b24b1df052", + "emoji_awardable": true, + "award_emoji": [], + "toggle_award_path": "/gitlab-org/gitlab-ce/notes/1471/toggle_award_emoji", + "report_abuse_path": "/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F29%23note_1471\u0026user_id=1", + "path": "/gitlab-org/gitlab-ce/notes/1471" }, - "discussion_id": "70d5c92a4039a36c70100c6691c18c27e4b0a790", - "emoji_awardable": true, - "award_emoji": [], - "toggle_award_path": "/gitlab-org/gitlab-ce/notes/1391/toggle_award_emoji", - "report_abuse_path": "/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F26%23note_1391\u0026user_id=1", - "path": "/gitlab-org/gitlab-ce/notes/1391" - }], - "individual_note": true -}]; + } +}; -export const discussionNoteServerResponse = [{ - "id": "a3ed36e29b1957efb3b68c53e2d7a2b24b1df052", - "reply_id": "a3ed36e29b1957efb3b68c53e2d7a2b24b1df052", - "expanded": true, - "notes": [{ - "id": 1471, - "attachment": { - "url": null, - "filename": null, - "image": false - }, - "author": { - "id": 1, - "name": "Root", - "username": "root", - "state": "active", - "avatar_url": null, - "path": "/root" - }, - "created_at": "2017-08-08T16:53:00.666Z", - "updated_at": "2017-08-08T16:53:00.666Z", - "system": false, - "noteable_id": 124, - "noteable_type": "Issue", - "noteable_iid": 29, - "type": "DiscussionNote", - "human_access": "Owner", - "note": "Adding a comment", - "note_html": "\u003cp dir=\"auto\"\u003eAdding a comment\u003c/p\u003e", - "current_user": { - "can_edit": true - }, - "discussion_id": "a3ed36e29b1957efb3b68c53e2d7a2b24b1df052", - "emoji_awardable": true, - "award_emoji": [], - "toggle_award_path": "/gitlab-org/gitlab-ce/notes/1471/toggle_award_emoji", - "report_abuse_path": "/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F29%23note_1471\u0026user_id=1", - "path": "/gitlab-org/gitlab-ce/notes/1471" - }], - "individual_note": false -}]; +export const DISCUSSION_NOTE_RESPONSE_MAP = { + ...INDIVIDUAL_NOTE_RESPONSE_MAP, + 'GET': { + ...INDIVIDUAL_NOTE_RESPONSE_MAP.GET, + '/gitlab-org/gitlab-ce/issues/26/discussions.json': [{ + "id": "a3ed36e29b1957efb3b68c53e2d7a2b24b1df052", + "reply_id": "a3ed36e29b1957efb3b68c53e2d7a2b24b1df052", + "expanded": true, + "notes": [{ + "id": 1471, + "attachment": { + "url": null, + "filename": null, + "image": false + }, + "author": { + "id": 1, + "name": "Root", + "username": "root", + "state": "active", + "avatar_url": null, + "path": "/root" + }, + "created_at": "2017-08-08T16:53:00.666Z", + "updated_at": "2017-08-08T16:53:00.666Z", + "system": false, + "noteable_id": 124, + "noteable_type": "Issue", + "noteable_iid": 29, + "type": "DiscussionNote", + "human_access": "Owner", + "note": "Adding a comment", + "note_html": "\u003cp dir=\"auto\"\u003eAdding a comment\u003c/p\u003e", + "current_user": { + "can_edit": true + }, + "discussion_id": "a3ed36e29b1957efb3b68c53e2d7a2b24b1df052", + "emoji_awardable": true, + "award_emoji": [], + "toggle_award_path": "/gitlab-org/gitlab-ce/notes/1471/toggle_award_emoji", + "report_abuse_path": "/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F29%23note_1471\u0026user_id=1", + "path": "/gitlab-org/gitlab-ce/notes/1471" + }], + "individual_note": false + }], + }, +}; + +export function individualNoteInterceptor(request, next) { + const body = INDIVIDUAL_NOTE_RESPONSE_MAP[request.method.toUpperCase()][request.url]; + + next(request.respondWith(JSON.stringify(body), { + status: 200, + })); +} + +export function discussionNoteInterceptor(request, next) { + const body = DISCUSSION_NOTE_RESPONSE_MAP[request.method.toUpperCase()][request.url]; + + next(request.respondWith(JSON.stringify(body), { + status: 200, + })); +} diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index 677a389b88f..2612c5fd7bc 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -1,6 +1,7 @@ /* eslint-disable space-before-function-paren, no-unused-expressions, no-var, object-shorthand, comma-dangle, max-len */ /* global Notes */ +import * as urlUtils from '~/lib/utils/url_utility'; import 'autosize'; import '~/gl_form'; import '~/lib/utils/text_utility'; @@ -168,8 +169,7 @@ import '~/notes'; }); it('sets target when hash matches', () => { - spyOn(gl.utils, 'getLocationHash'); - gl.utils.getLocationHash.and.returnValue(hash); + spyOn(urlUtils, 'getLocationHash').and.returnValue(hash); Notes.updateNoteTargetSelector($note); @@ -178,8 +178,7 @@ import '~/notes'; }); it('unsets target when hash does not match', () => { - spyOn(gl.utils, 'getLocationHash'); - gl.utils.getLocationHash.and.returnValue('note_doesnotexist'); + spyOn(urlUtils, 'getLocationHash').and.returnValue('note_doesnotexist'); Notes.updateNoteTargetSelector($note); @@ -187,8 +186,7 @@ import '~/notes'; }); it('unsets target when there is not a hash fragment anymore', () => { - spyOn(gl.utils, 'getLocationHash'); - gl.utils.getLocationHash.and.returnValue(null); + spyOn(urlUtils, 'getLocationHash').and.returnValue(null); Notes.updateNoteTargetSelector($note); diff --git a/spec/javascripts/pager_spec.js b/spec/javascripts/pager_spec.js index 1d3e1263371..fe3ea996eac 100644 --- a/spec/javascripts/pager_spec.js +++ b/spec/javascripts/pager_spec.js @@ -1,5 +1,6 @@ /* global fixture */ +import * as utils from '~/lib/utils/url_utility'; import '~/pager'; describe('pager', () => { @@ -30,7 +31,7 @@ describe('pager', () => { it('should use current url if data-href attribute not provided', () => { const href = `${gl.TEST_HOST}/some_list`; - spyOn(gl.utils, 'removeParams').and.returnValue(href); + spyOn(utils, 'removeParams').and.returnValue(href); Pager.init(); expect(Pager.url).toBe(href); }); @@ -44,9 +45,9 @@ describe('pager', () => { it('keeps extra query parameters from url', () => { window.history.replaceState({}, null, '?filter=test&offset=100'); const href = `${gl.TEST_HOST}/some_list?filter=test`; - spyOn(gl.utils, 'removeParams').and.returnValue(href); + spyOn(utils, 'removeParams').and.returnValue(href); Pager.init(); - expect(gl.utils.removeParams).toHaveBeenCalledWith(['limit', 'offset']); + expect(utils.removeParams).toHaveBeenCalledWith(['limit', 'offset']); expect(Pager.url).toEqual(href); }); }); diff --git a/spec/javascripts/pipelines/graph/job_component_spec.js b/spec/javascripts/pipelines/graph/job_component_spec.js index 23c87610d83..35e36e9c353 100644 --- a/spec/javascripts/pipelines/graph/job_component_spec.js +++ b/spec/javascripts/pipelines/graph/job_component_spec.js @@ -113,4 +113,35 @@ describe('pipeline graph job component', () => { component.$el.querySelector('a').classList.contains('css-class-job-name'), ).toBe(true); }); + + describe('status label', () => { + it('should not render status label when it is not provided', () => { + component = mountComponent(JobComponent, { + job: { + id: 4256, + name: 'test', + status: { + icon: 'icon_status_success', + }, + }, + }); + + expect(component.$el.querySelector('.js-job-component-tooltip').getAttribute('data-original-title')).toEqual('test'); + }); + + it('should not render status label when it is provided', () => { + component = mountComponent(JobComponent, { + job: { + id: 4256, + name: 'test', + status: { + icon: 'icon_status_success', + label: 'success', + }, + }, + }); + + expect(component.$el.querySelector('.js-job-component-tooltip').getAttribute('data-original-title')).toEqual('test - success'); + }); + }); }); diff --git a/spec/javascripts/repo/components/repo_commit_section_spec.js b/spec/javascripts/repo/components/repo_commit_section_spec.js index 1c794123095..72712e058e5 100644 --- a/spec/javascripts/repo/components/repo_commit_section_spec.js +++ b/spec/javascripts/repo/components/repo_commit_section_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import * as urlUtils from '~/lib/utils/url_utility'; import store from '~/repo/stores'; import service from '~/repo/services'; import repoCommitSection from '~/repo/components/repo_commit_section.vue'; @@ -97,7 +98,7 @@ describe('RepoCommitSection', () => { }); it('redirects to MR creation page if start new MR checkbox checked', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); vm.startNewMR = true; vm.makeCommit(); @@ -105,7 +106,7 @@ describe('RepoCommitSection', () => { getSetTimeoutPromise() .then(() => Vue.nextTick()) .then(() => { - expect(gl.utils.visitUrl).toHaveBeenCalled(); + expect(urlUtils.visitUrl).toHaveBeenCalled(); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/repo/stores/actions/tree_spec.js b/spec/javascripts/repo/stores/actions/tree_spec.js index 393a797c6a3..2bbc49d5a9f 100644 --- a/spec/javascripts/repo/stores/actions/tree_spec.js +++ b/spec/javascripts/repo/stores/actions/tree_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import * as urlUtils from '~/lib/utils/url_utility'; import store from '~/repo/stores'; import service from '~/repo/services'; import { file, resetStore } from '../../helpers'; @@ -255,7 +256,7 @@ describe('Multi-file store tree actions', () => { let row; beforeEach(() => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); row = { url: 'submoduleurl', @@ -276,7 +277,7 @@ describe('Multi-file store tree actions', () => { it('opens submodule URL', (done) => { store.dispatch('clickedTreeRow', row) .then(() => { - expect(gl.utils.visitUrl).toHaveBeenCalledWith('submoduleurl'); + expect(urlUtils.visitUrl).toHaveBeenCalledWith('submoduleurl'); done(); }).catch(done.fail); diff --git a/spec/javascripts/repo/stores/actions_spec.js b/spec/javascripts/repo/stores/actions_spec.js index f2a7a698912..21d87e46216 100644 --- a/spec/javascripts/repo/stores/actions_spec.js +++ b/spec/javascripts/repo/stores/actions_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import * as urlUtils from '~/lib/utils/url_utility'; import store from '~/repo/stores'; import service from '~/repo/services'; import { resetStore, file } from '../helpers'; @@ -10,11 +11,11 @@ describe('Multi-file store actions', () => { describe('redirectToUrl', () => { it('calls visitUrl', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); store.dispatch('redirectToUrl', 'test') .then(() => { - expect(gl.utils.visitUrl).toHaveBeenCalledWith('test'); + expect(urlUtils.visitUrl).toHaveBeenCalledWith('test'); done(); }) @@ -326,13 +327,13 @@ describe('Multi-file store actions', () => { }); it('redirects to new merge request page', (done) => { - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); store.state.endpoints.newMergeRequestUrl = 'newMergeRequestUrl?branch='; store.dispatch('commitChanges', { payload, newMr: true }) .then(() => { - expect(gl.utils.visitUrl).toHaveBeenCalledWith('newMergeRequestUrl?branch=master'); + expect(urlUtils.visitUrl).toHaveBeenCalledWith('newMergeRequestUrl?branch=master'); done(); }).catch(done.fail); diff --git a/spec/javascripts/search_autocomplete_spec.js b/spec/javascripts/search_autocomplete_spec.js index fdfc59a6f12..1e4c2c9faad 100644 --- a/spec/javascripts/search_autocomplete_spec.js +++ b/spec/javascripts/search_autocomplete_spec.js @@ -3,6 +3,7 @@ import '~/gl_dropdown'; import '~/search_autocomplete'; import '~/lib/utils/common_utils'; +import * as urlUtils from '~/lib/utils/url_utility'; (function() { var assertLinks, dashboardIssuesPath, dashboardMRsPath, groupIssuesPath, groupMRsPath, groupName, mockDashboardOptions, mockGroupOptions, mockProjectOptions, projectIssuesPath, projectMRsPath, projectName, userId, widget; @@ -121,7 +122,7 @@ import '~/lib/utils/common_utils'; loadFixtures('static/search_autocomplete.html.raw'); // Prevent turbolinks from triggering within gl_dropdown - spyOn(window.gl.utils, 'visitUrl').and.returnValue(true); + spyOn(urlUtils, 'visitUrl').and.returnValue(true); window.gon = {}; window.gon.current_user_id = userId; diff --git a/spec/javascripts/sidebar/sidebar_assignees_spec.js b/spec/javascripts/sidebar/sidebar_assignees_spec.js index 929ba75e67d..b97e24d9dcf 100644 --- a/spec/javascripts/sidebar/sidebar_assignees_spec.js +++ b/spec/javascripts/sidebar/sidebar_assignees_spec.js @@ -4,20 +4,29 @@ import SidebarMediator from '~/sidebar/sidebar_mediator'; import SidebarService from '~/sidebar/services/sidebar_service'; import SidebarStore from '~/sidebar/stores/sidebar_store'; import Mock from './mock_data'; +import mountComponent from '../helpers/vue_mount_component_helper'; describe('sidebar assignees', () => { - let component; - let SidebarAssigneeComponent; + let vm; + let mediator; + let sidebarAssigneesEl; preloadFixtures('issues/open-issue.html.raw'); beforeEach(() => { Vue.http.interceptors.push(Mock.sidebarMockInterceptor); - SidebarAssigneeComponent = Vue.extend(SidebarAssignees); - spyOn(SidebarMediator.prototype, 'saveAssignees').and.callThrough(); - spyOn(SidebarMediator.prototype, 'assignYourself').and.callThrough(); - this.mediator = new SidebarMediator(Mock.mediator); + loadFixtures('issues/open-issue.html.raw'); - this.sidebarAssigneesEl = document.querySelector('#js-vue-sidebar-assignees'); + + mediator = new SidebarMediator(Mock.mediator); + spyOn(mediator, 'saveAssignees').and.callThrough(); + spyOn(mediator, 'assignYourself').and.callThrough(); + + const SidebarAssigneeComponent = Vue.extend(SidebarAssignees); + sidebarAssigneesEl = document.querySelector('#js-vue-sidebar-assignees'); + vm = mountComponent(SidebarAssigneeComponent, { + mediator, + field: sidebarAssigneesEl.dataset.field, + }, sidebarAssigneesEl); }); afterEach(() => { @@ -28,30 +37,24 @@ describe('sidebar assignees', () => { }); it('calls the mediator when saves the assignees', () => { - component = new SidebarAssigneeComponent() - .$mount(this.sidebarAssigneesEl); - component.saveAssignees(); - - expect(SidebarMediator.prototype.saveAssignees).toHaveBeenCalled(); + vm.saveAssignees(); + expect(mediator.saveAssignees).toHaveBeenCalled(); }); it('calls the mediator when "assignSelf" method is called', () => { - component = new SidebarAssigneeComponent() - .$mount(this.sidebarAssigneesEl); - component.assignSelf(); + vm.assignSelf(); - expect(SidebarMediator.prototype.assignYourself).toHaveBeenCalled(); - expect(this.mediator.store.assignees.length).toEqual(1); + expect(mediator.assignYourself).toHaveBeenCalled(); + expect(mediator.store.assignees.length).toEqual(1); }); it('hides assignees until fetched', (done) => { - component = new SidebarAssigneeComponent().$mount(this.sidebarAssigneesEl); - const currentAssignee = this.sidebarAssigneesEl.querySelector('.value'); + const currentAssignee = sidebarAssigneesEl.querySelector('.value'); expect(currentAssignee).toBe(null); - component.store.isFetching.assignees = false; + vm.store.isFetching.assignees = false; Vue.nextTick(() => { - expect(component.$el.querySelector('.value')).toBeVisible(); + expect(vm.$el.querySelector('.value')).toBeVisible(); done(); }); }); diff --git a/spec/javascripts/sidebar/sidebar_mediator_spec.js b/spec/javascripts/sidebar/sidebar_mediator_spec.js index 14c34d5a78c..9efd109b996 100644 --- a/spec/javascripts/sidebar/sidebar_mediator_spec.js +++ b/spec/javascripts/sidebar/sidebar_mediator_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import * as urlUtils from '~/lib/utils/url_utility'; import SidebarMediator from '~/sidebar/sidebar_mediator'; import SidebarStore from '~/sidebar/stores/sidebar_store'; import SidebarService from '~/sidebar/services/sidebar_service'; @@ -85,12 +86,12 @@ describe('Sidebar mediator', () => { const moveToProjectId = 7; this.mediator.store.setMoveToProjectId(moveToProjectId); spyOn(this.mediator.service, 'moveIssue').and.callThrough(); - spyOn(gl.utils, 'visitUrl'); + spyOn(urlUtils, 'visitUrl'); this.mediator.moveIssue() .then(() => { expect(this.mediator.service.moveIssue).toHaveBeenCalledWith(moveToProjectId); - expect(gl.utils.visitUrl).toHaveBeenCalledWith('/root/some-project/issues/5'); + expect(urlUtils.visitUrl).toHaveBeenCalledWith('/root/some-project/issues/5'); }) .then(done) .catch(done.fail); diff --git a/spec/javascripts/sidebar/sidebar_subscriptions_spec.js b/spec/javascripts/sidebar/sidebar_subscriptions_spec.js index 7adf22b0f1f..a6113cb0bae 100644 --- a/spec/javascripts/sidebar/sidebar_subscriptions_spec.js +++ b/spec/javascripts/sidebar/sidebar_subscriptions_spec.js @@ -26,11 +26,14 @@ describe('Sidebar Subscriptions', function () { }); it('calls the mediator toggleSubscription on event', () => { - spyOn(SidebarMediator.prototype, 'toggleSubscription').and.returnValue(Promise.resolve()); - vm = mountComponent(SidebarSubscriptions, {}); + const mediator = new SidebarMediator(); + spyOn(mediator, 'toggleSubscription').and.returnValue(Promise.resolve()); + vm = mountComponent(SidebarSubscriptions, { + mediator, + }); eventHub.$emit('toggleSubscription'); - expect(SidebarMediator.prototype.toggleSubscription).toHaveBeenCalled(); + expect(mediator.toggleSubscription).toHaveBeenCalled(); }); }); diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js index fd7aa332d17..6897c991066 100644 --- a/spec/javascripts/test_bundle.js +++ b/spec/javascripts/test_bundle.js @@ -17,6 +17,12 @@ Vue.config.warnHandler = (msg, vm, trace) => { fail(`${msg}${trace}`); }; +let hasVueErrors = false; +Vue.config.errorHandler = function (err) { + hasVueErrors = true; + fail(err); +}; + Vue.use(VueResource); // enable test fixtures @@ -72,7 +78,7 @@ testsContext.keys().forEach(function (path) { describe('test errors', () => { beforeAll((done) => { - if (hasUnhandledPromiseRejections || hasVueWarnings) { + if (hasUnhandledPromiseRejections || hasVueWarnings || hasVueErrors) { setTimeout(done, 1000); } else { done(); @@ -86,6 +92,10 @@ describe('test errors', () => { it('has no Vue warnings', () => { expect(hasVueWarnings).toBe(false); }); + + it('has no Vue error', () => { + expect(hasVueErrors).toBe(false); + }); }); // if we're generating coverage reports, make sure to include all files so diff --git a/spec/javascripts/todos_spec.js b/spec/javascripts/todos_spec.js index 7d3c9319a11..59e16f0786e 100644 --- a/spec/javascripts/todos_spec.js +++ b/spec/javascripts/todos_spec.js @@ -1,3 +1,4 @@ +import * as urlUtils from '~/lib/utils/url_utility'; import Todos from '~/todos'; import '~/lib/utils/common_utils'; @@ -16,7 +17,7 @@ describe('Todos', () => { it('opens the todo url', (done) => { const todoLink = todoItem.dataset.url; - spyOn(gl.utils, 'visitUrl').and.callFake((url) => { + spyOn(urlUtils, 'visitUrl').and.callFake((url) => { expect(url).toEqual(todoLink); done(); }); @@ -31,7 +32,7 @@ describe('Todos', () => { beforeEach(() => { metakeyEvent = $.Event('click', { keyCode: 91, ctrlKey: true }); - visitUrlSpy = spyOn(gl.utils, 'visitUrl').and.callFake(() => {}); + visitUrlSpy = spyOn(urlUtils, 'visitUrl').and.callFake(() => {}); windowOpenSpy = spyOn(window, 'open').and.callFake(() => {}); }); diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js index 7ee998c8fce..d5b8947c86f 100644 --- a/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_deployment_spec.js @@ -1,4 +1,5 @@ import Vue from 'vue'; +import * as urlUtils from '~/lib/utils/url_utility'; import deploymentComponent from '~/vue_merge_request_widget/components/mr_widget_deployment'; import MRWidgetService from '~/vue_merge_request_widget/services/mr_widget_service'; @@ -108,13 +109,13 @@ describe('MRWidgetDeployment', () => { it('should show a confirm dialog and call service.stopEnvironment when confirmed', (done) => { spyOn(window, 'confirm').and.returnValue(true); spyOn(MRWidgetService, 'stopEnvironment').and.returnValue(returnPromise(true)); - spyOn(gl.utils, 'visitUrl').and.returnValue(true); + spyOn(urlUtils, 'visitUrl').and.returnValue(true); vm = mockStopEnvironment(); expect(window.confirm).toHaveBeenCalled(); expect(MRWidgetService.stopEnvironment).toHaveBeenCalledWith(deploymentMockData.stop_url); setTimeout(() => { - expect(gl.utils.visitUrl).toHaveBeenCalledWith(url); + expect(urlUtils.visitUrl).toHaveBeenCalledWith(url); done(); }, 333); }); diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index f19b65a5f71..e6845420f7d 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -588,12 +588,12 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe '#fetch_as_mirror_without_shell' do + describe '#fetch_repository_as_mirror' do let(:new_repository) do Gitlab::Git::Repository.new('default', 'my_project.git', '') end - subject { new_repository.fetch_as_mirror_without_shell(repository.path) } + subject { new_repository.fetch_repository_as_mirror(repository) } before do Gitlab::Shell.new.add_repository('default', 'my_project') @@ -603,7 +603,7 @@ describe Gitlab::Git::Repository, seed_helper: true do Gitlab::Shell.new.remove_repository(TestEnv.repos_path, 'my_project') end - it 'fetches a url as a mirror remote' do + it 'fetches a repository as a mirror remote' do subject expect(refs(new_repository.path)).to eq(refs(repository.path)) @@ -1662,21 +1662,6 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - describe '#fetch_remote_without_shell' do - let(:git_path) { Gitlab.config.git.bin_path } - let(:remote_name) { 'my_remote' } - - subject { repository.fetch_remote_without_shell(remote_name) } - - it 'fetches the remote and returns true if the command was successful' do - expect(repository).to receive(:popen) - .with(%W(#{git_path} fetch #{remote_name}), repository.path, {}) - .and_return(['', 0]) - - expect(subject).to be(true) - end - end - describe '#merge' do let(:repository) do Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') diff --git a/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb index 667e4747897..f66451c5188 100644 --- a/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb @@ -21,7 +21,6 @@ describe Gitlab::Metrics::Samplers::InfluxSampler do it 'samples various statistics' do expect(sampler).to receive(:sample_memory_usage) expect(sampler).to receive(:sample_file_descriptors) - expect(sampler).to receive(:sample_objects) expect(sampler).to receive(:sample_gc) expect(sampler).to receive(:flush) @@ -72,28 +71,6 @@ describe Gitlab::Metrics::Samplers::InfluxSampler do end end - if Gitlab::Metrics.mri? - describe '#sample_objects' do - it 'adds a metric containing the amount of allocated objects' do - expect(sampler).to receive(:add_metric) - .with(/object_counts/, an_instance_of(Hash), an_instance_of(Hash)) - .at_least(:once) - .and_call_original - - sampler.sample_objects - end - - it 'ignores classes without a name' do - expect(Allocations).to receive(:to_hash).and_return({ Class.new => 4 }) - - expect(sampler).not_to receive(:add_metric) - .with('object_counts', an_instance_of(Hash), type: nil) - - sampler.sample_objects - end - end - end - describe '#sample_gc' do it 'adds a metric containing garbage collection statistics' do expect(GC::Profiler).to receive(:total_time).and_return(0.24) diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb index 53699327da1..375cbf8a9ca 100644 --- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb +++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb @@ -11,7 +11,6 @@ describe Gitlab::Metrics::Samplers::RubySampler do it 'samples various statistics' do expect(Gitlab::Metrics::System).to receive(:memory_usage) expect(Gitlab::Metrics::System).to receive(:file_descriptor_count) - expect(sampler).to receive(:sample_objects) expect(sampler).to receive(:sample_gc) sampler.sample @@ -65,26 +64,4 @@ describe Gitlab::Metrics::Samplers::RubySampler do sampler.sample end end - - if Gitlab::Metrics.mri? - describe '#sample_objects' do - it 'adds a metric containing the amount of allocated objects' do - expect(sampler.metrics[:objects_total]).to receive(:set) - .with(include(class: anything), be > 0) - .at_least(:once) - .and_call_original - - sampler.sample - end - - it 'ignores classes without a name' do - expect(Allocations).to receive(:to_hash).and_return({ Class.new => 4 }) - - expect(sampler.metrics[:objects_total]).not_to receive(:set) - .with(include(class: 'object_counts'), anything) - - sampler.sample - end - end - end end diff --git a/spec/migrations/remove_assignee_id_from_issue_spec.rb b/spec/migrations/remove_assignee_id_from_issue_spec.rb new file mode 100644 index 00000000000..2c6f992d3ae --- /dev/null +++ b/spec/migrations/remove_assignee_id_from_issue_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170523073948_remove_assignee_id_from_issue.rb') + +describe RemoveAssigneeIdFromIssue, :migration do + let(:issues) { table(:issues) } + let(:issue_assignees) { table(:issue_assignees) } + let(:users) { table(:users) } + + let!(:user_1) { users.create(email: 'email1@example.com') } + let!(:user_2) { users.create(email: 'email2@example.com') } + let!(:user_3) { users.create(email: 'email3@example.com') } + + def create_issue(assignees:) + issues.create.tap do |issue| + assignees.each do |assignee| + issue_assignees.create(issue_id: issue.id, user_id: assignee.id) + end + end + end + + let!(:issue_single_assignee) { create_issue(assignees: [user_1]) } + let!(:issue_no_assignee) { create_issue(assignees: []) } + let!(:issue_multiple_assignees) { create_issue(assignees: [user_2, user_3]) } + + describe '#down' do + it 'sets the assignee_id to a random matching assignee from the assignees table' do + migrate! + disable_migrations_output { described_class.new.down } + + expect(issue_single_assignee.reload.assignee_id).to eq(user_1.id) + expect(issue_no_assignee.reload.assignee_id).to be_nil + expect(issue_multiple_assignees.reload.assignee_id).to eq(user_2.id).or(user_3.id) + + disable_migrations_output { described_class.new.up } + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index cdabd35b6ba..4687d9dfa00 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -913,11 +913,11 @@ describe User do describe 'email matching' do it 'returns users with a matching Email' do - expect(described_class.search(user.email)).to eq([user, user2]) + expect(described_class.search(user.email)).to eq([user]) end - it 'returns users with a partially matching Email' do - expect(described_class.search(user.email[0..2])).to eq([user, user2]) + it 'does not return users with a partially matching Email' do + expect(described_class.search(user.email[0..2])).not_to include(user, user2) end it 'returns users with a matching Email regardless of the casing' do @@ -973,8 +973,8 @@ describe User do expect(search_with_secondary_emails(user.email)).to eq([user]) end - it 'returns users with a partially matching email' do - expect(search_with_secondary_emails(user.email[0..2])).to eq([user]) + it 'does not return users with a partially matching email' do + expect(search_with_secondary_emails(user.email[0..2])).not_to include([user]) end it 'returns users with a matching email regardless of the casing' do @@ -997,29 +997,8 @@ describe User do expect(search_with_secondary_emails(email.email)).to eq([email.user]) end - it 'returns users with a matching part of secondary email' do - expect(search_with_secondary_emails(email.email[1..4])).to eq([email.user]) - end - - it 'return users with a matching part of secondary email regardless of case' do - expect(search_with_secondary_emails(email.email[1..4].upcase)).to eq([email.user]) - expect(search_with_secondary_emails(email.email[1..4].downcase)).to eq([email.user]) - expect(search_with_secondary_emails(email.email[1..4].capitalize)).to eq([email.user]) - end - - it 'returns multiple users with matching secondary emails' do - email1 = create(:email, email: '1_testemail@example.com') - email2 = create(:email, email: '2_testemail@example.com') - email3 = create(:email, email: 'other@email.com') - email3.user.update_attributes!(email: 'another@mail.com') - - expect( - search_with_secondary_emails('testemail@example.com').map(&:id) - ).to include(email1.user.id, email2.user.id) - - expect( - search_with_secondary_emails('testemail@example.com').map(&:id) - ).not_to include(email3.user.id) + it 'does not return users with a matching part of secondary email' do + expect(search_with_secondary_emails(email.email[1..4])).not_to include([email.user]) end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 554723d6b1e..6330c140246 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -173,6 +173,28 @@ describe API::Groups do end describe "GET /groups/:id" do + # Given a group, create one project for each visibility level + # + # group - Group to add projects to + # share_with - If provided, each project will be shared with this Group + # + # Returns a Hash of visibility_level => Project pairs + def add_projects_to_group(group, share_with: nil) + projects = { + public: create(:project, :public, namespace: group), + internal: create(:project, :internal, namespace: group), + private: create(:project, :private, namespace: group) + } + + if share_with + create(:project_group_link, project: projects[:public], group: share_with) + create(:project_group_link, project: projects[:internal], group: share_with) + create(:project_group_link, project: projects[:private], group: share_with) + end + + projects + end + context 'when unauthenticated' do it 'returns 404 for a private group' do get api("/groups/#{group2.id}") @@ -183,6 +205,26 @@ describe API::Groups do get api("/groups/#{group1.id}") expect(response).to have_gitlab_http_status(200) end + + it 'returns only public projects in the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group) + + get api("/groups/#{public_group.id}") + + expect(json_response['projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id) + end + + it 'returns only public projects shared with the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group, share_with: group1) + + get api("/groups/#{group1.id}") + + expect(json_response['shared_projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id) + end end context "when authenticated as user" do @@ -222,6 +264,26 @@ describe API::Groups do expect(response).to have_gitlab_http_status(404) end + + it 'returns only public and internal projects in the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group) + + get api("/groups/#{public_group.id}", user2) + + expect(json_response['projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id, projects[:internal].id) + end + + it 'returns only public and internal projects shared with the group' do + public_group = create(:group, :public) + projects = add_projects_to_group(public_group, share_with: group1) + + get api("/groups/#{group1.id}", user2) + + expect(json_response['shared_projects'].map { |p| p['id'].to_i }) + .to contain_exactly(projects[:public].id, projects[:internal].id) + end end context "when authenticated as admin" do diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 99525cd0a6a..3f5070a1fd2 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -860,6 +860,20 @@ describe API::Issues, :mailer do end end + context 'user does not have permissions to create issue' do + let(:not_member) { create(:user) } + + before do + project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'renders 403' do + post api("/projects/#{project.id}/issues", not_member), title: 'new issue' + + expect(response).to have_gitlab_http_status(403) + end + end + it 'creates a new project issue' do post api("/projects/#{project.id}/issues", user), title: 'new issue', labels: 'label, label2', weight: 3, diff --git a/spec/rubocop/cop/migration/remove_column_spec.rb b/spec/rubocop/cop/migration/remove_column_spec.rb new file mode 100644 index 00000000000..89112f01723 --- /dev/null +++ b/spec/rubocop/cop/migration/remove_column_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/remove_column' + +describe RuboCop::Cop::Migration::RemoveColumn do + include CopHelper + + subject(:cop) { described_class.new } + + def source(meth = 'change') + "def #{meth}; remove_column :table, :column; end" + end + + context 'in a regular migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + allow(cop).to receive(:in_post_deployment_migration?).and_return(false) + end + + it 'registers an offense when remove_column is used in the change method' do + inspect_source(cop, source('change')) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + + it 'registers an offense when remove_column is used in the up method' do + inspect_source(cop, source('up')) + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + + it 'registers no offense when remove_column is used in the down method' do + inspect_source(cop, source('down')) + + expect(cop.offenses.size).to eq(0) + end + end + + context 'in a post-deployment migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + allow(cop).to receive(:in_post_deployment_migration?).and_return(true) + end + + it 'registers no offense' do + inspect_source(cop, source) + + expect(cop.offenses.size).to eq(0) + end + end + + context 'outside of a migration' do + it 'registers no offense' do + inspect_source(cop, source) + + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/spec/services/ci/register_job_service_spec.rb b/spec/services/ci/register_job_service_spec.rb index 3ee59014b5b..22fb7ed7215 100644 --- a/spec/services/ci/register_job_service_spec.rb +++ b/spec/services/ci/register_job_service_spec.rb @@ -307,6 +307,19 @@ module Ci it_behaves_like 'not pick' end + + context 'when job object is staled' do + let!(:pre_stage_job) { create(:ci_build, :running, pipeline: pipeline, name: 'test', stage_idx: 0) } + + before do + allow_any_instance_of(Ci::Build).to receive(:drop!) + .and_raise(ActiveRecord::StaleObjectError.new(pending_job, :drop!)) + end + + it 'does not drop nor pick' do + expect(subject).to be_nil + end + end end shared_examples 'validation is not active' do diff --git a/spec/services/members/authorized_destroy_service_spec.rb b/spec/services/members/authorized_destroy_service_spec.rb index 2d04d824180..d4ef31c0c74 100644 --- a/spec/services/members/authorized_destroy_service_spec.rb +++ b/spec/services/members/authorized_destroy_service_spec.rb @@ -45,7 +45,7 @@ describe Members::AuthorizedDestroyService do expect { described_class.new(member, member_user).execute } .to change { number_of_assigned_issuables(member_user) }.from(4).to(2) - expect(issue.reload.assignee_id).to be_nil + expect(issue.reload.assignee_ids).to be_empty expect(merge_request.reload.assignee_id).to be_nil end end diff --git a/spec/services/users/keys_count_service_spec.rb b/spec/services/users/keys_count_service_spec.rb index a188cf86772..bee8380e8b7 100644 --- a/spec/services/users/keys_count_service_spec.rb +++ b/spec/services/users/keys_count_service_spec.rb @@ -15,14 +15,12 @@ describe Users::KeysCountService, :use_clean_rails_memory_store_caching do expect(service.count).to eq(1) end - it 'caches the number of keys in Redis' do + it 'caches the number of keys in Redis', :request_store do + service.delete_cache + control_count = ActiveRecord::QueryRecorder.new { service.count }.count service.delete_cache - recorder = ActiveRecord::QueryRecorder.new do - 2.times { service.count } - end - - expect(recorder.count).to eq(1) + expect { 2.times { service.count } }.not_to exceed_query_limit(control_count) end end |