diff options
Diffstat (limited to 'spec')
32 files changed, 442 insertions, 157 deletions
diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 84db26a958a..c29b2fe8946 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -22,4 +22,28 @@ describe Admin::GroupsController do expect(response).to redirect_to(admin_groups_path) end end + + describe 'PUT #members_update' do + let(:group_user) { create(:user) } + + it 'adds user to members' do + put :members_update, id: group, + user_ids: group_user.id, + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'Users were successfully added.' + expect(response).to redirect_to(admin_group_path(group)) + expect(group.users).to include group_user + end + + it 'adds no user to members' do + put :members_update, id: group, + user_ids: '', + access_level: Gitlab::Access::GUEST + + expect(response).to set_flash.to 'No users specified.' + expect(response).to redirect_to(admin_group_path(group)) + expect(group.users).not_to include group_user + end + end end diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index 6a6e9bf378a..05999431d8f 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -127,7 +127,7 @@ describe Projects::LabelsController do context 'group owner' do before do - GroupMember.add_users_to_group(group, [user], :owner) + GroupMember.add_users(group, [user], :owner) end it 'gives access' do diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 362d167befa..55eca187f6c 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -414,7 +414,8 @@ describe 'Issues', feature: true do it 'will not send ajax request when no data is changed' do page.within '.labels' do click_link 'Edit' - first('.dropdown-menu-close').click + + find('.dropdown-menu-close', match: :first).click expect(page).not_to have_selector('.block-loading') end diff --git a/spec/features/merge_requests/create_new_mr_spec.rb b/spec/features/merge_requests/create_new_mr_spec.rb index 3a4ec07b2b0..16b09933bda 100644 --- a/spec/features/merge_requests/create_new_mr_spec.rb +++ b/spec/features/merge_requests/create_new_mr_spec.rb @@ -20,13 +20,13 @@ feature 'Create New Merge Request', feature: true, js: true do expect(page).to have_content('Target branch') first('.js-source-branch').click - first('.dropdown-source-branch .dropdown-content a', text: 'v1.1.0').click + find('.dropdown-source-branch .dropdown-content a', match: :first).click expect(page).to have_content "b83d6e3" end it 'selects the target branch sha when a tag with the same name exists' do - visit namespace_project_merge_requests_path(project.namespace, project) + visit namespace_project_merge_requests_path(project.namespace, project) click_link 'New merge request' @@ -46,8 +46,8 @@ feature 'Create New Merge Request', feature: true, js: true do expect(page).to have_content('Source branch') expect(page).to have_content('Target branch') - first('.js-source-branch').click - first('.dropdown-source-branch .dropdown-content a', text: 'orphaned-branch').click + find('.js-source-branch', match: :first).click + find('.dropdown-source-branch .dropdown-content a', text: 'orphaned-branch', match: :first).click click_button "Compare branches" click_link "Changes" diff --git a/spec/features/merge_requests/versions_spec.rb b/spec/features/merge_requests/versions_spec.rb index 68a68f5d3f3..7a2da623c58 100644 --- a/spec/features/merge_requests/versions_spec.rb +++ b/spec/features/merge_requests/versions_spec.rb @@ -107,14 +107,13 @@ feature 'Merge Request versions', js: true, feature: true do it 'should have 0 chages between versions' do page.within '.mr-version-compare-dropdown' do - expect(page).to have_content 'version 1' + expect(find('.dropdown-toggle')).to have_content 'version 1' end page.within '.mr-version-dropdown' do find('.btn-default').click - find(:link, 'version 1').trigger('click') + click_link 'version 1' end - expect(page).to have_content '0 changed files' end end @@ -129,12 +128,12 @@ feature 'Merge Request versions', js: true, feature: true do it 'should set the compared versions to be the same' do page.within '.mr-version-compare-dropdown' do - expect(page).to have_content 'version 2' + expect(find('.dropdown-toggle')).to have_content 'version 2' end page.within '.mr-version-dropdown' do find('.btn-default').click - find(:link, 'version 1').trigger('click') + click_link 'version 1' end page.within '.mr-version-compare-dropdown' do diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb index 62d0aedda48..6cdca0f114b 100644 --- a/spec/features/projects/issuable_templates_spec.rb +++ b/spec/features/projects/issuable_templates_spec.rb @@ -163,12 +163,14 @@ feature 'issuable templates', feature: true, js: true do end def select_template(name) - first('.js-issuable-selector').click - first('.js-issuable-selector-wrap .dropdown-content a', text: name).click + find('.js-issuable-selector').click + + find('.js-issuable-selector-wrap .dropdown-content a', text: name, match: :first).click end def select_option(name) - first('.js-issuable-selector').click - first('.js-issuable-selector-wrap .dropdown-footer-list a', text: name).click + find('.js-issuable-selector').click + + find('.js-issuable-selector-wrap .dropdown-footer-list a', text: name, match: :first).click end end diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb index 28b8def331d..345bc33a67b 100644 --- a/spec/helpers/submodule_helper_spec.rb +++ b/spec/helpers/submodule_helper_spec.rb @@ -70,10 +70,12 @@ describe SubmoduleHelper do expect(submodule_links(submodule_item)).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash']) end - it 'returns original with non-standard url' do + it 'handles urls with no .git on the end' do stub_url('http://github.com/gitlab-org/gitlab-ce') - expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil]) + expect(submodule_links(submodule_item)).to eq(['https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash']) + end + it 'returns original with non-standard url' do stub_url('http://github.com/another/gitlab-org/gitlab-ce.git') expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil]) end @@ -95,10 +97,12 @@ describe SubmoduleHelper do expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash']) end - it 'returns original with non-standard url' do + it 'handles urls with no .git on the end' do stub_url('http://gitlab.com/gitlab-org/gitlab-ce') - expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil]) + expect(submodule_links(submodule_item)).to eq(['https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash']) + end + it 'returns original with non-standard url' do stub_url('http://gitlab.com/another/gitlab-org/gitlab-ce.git') expect(submodule_links(submodule_item)).to eq([repo.submodule_url_for, nil]) end diff --git a/spec/javascripts/blob/blob_fork_suggestion_spec.js b/spec/javascripts/blob/blob_fork_suggestion_spec.js new file mode 100644 index 00000000000..d0d64d75957 --- /dev/null +++ b/spec/javascripts/blob/blob_fork_suggestion_spec.js @@ -0,0 +1,37 @@ +import BlobForkSuggestion from '~/blob/blob_fork_suggestion'; + +describe('BlobForkSuggestion', () => { + let blobForkSuggestion; + + const openButtons = [document.createElement('div')]; + const forkButtons = [document.createElement('a')]; + const cancelButtons = [document.createElement('div')]; + const suggestionSections = [document.createElement('div')]; + const actionTextPieces = [document.createElement('div')]; + + beforeEach(() => { + blobForkSuggestion = new BlobForkSuggestion({ + openButtons, + forkButtons, + cancelButtons, + suggestionSections, + actionTextPieces, + }); + }); + + afterEach(() => { + blobForkSuggestion.destroy(); + }); + + it('showSuggestionSection', () => { + blobForkSuggestion.showSuggestionSection('/foo', 'foo'); + expect(suggestionSections[0].classList.contains('hidden')).toEqual(false); + expect(forkButtons[0].getAttribute('href')).toEqual('/foo'); + expect(actionTextPieces[0].textContent).toEqual('foo'); + }); + + it('hideSuggestionSection', () => { + blobForkSuggestion.hideSuggestionSection(); + expect(suggestionSections[0].classList.contains('hidden')).toEqual(true); + }); +}); diff --git a/spec/javascripts/commit/pipelines/pipelines_spec.js b/spec/javascripts/commit/pipelines/pipelines_spec.js index 8cac3cad232..ad31448f81c 100644 --- a/spec/javascripts/commit/pipelines/pipelines_spec.js +++ b/spec/javascripts/commit/pipelines/pipelines_spec.js @@ -36,6 +36,7 @@ describe('Pipelines table in Commits and Merge requests', () => { setTimeout(() => { expect(this.component.$el.querySelector('.empty-state')).toBeDefined(); expect(this.component.$el.querySelector('.realtime-loading')).toBe(null); + expect(this.component.$el.querySelector('.js-pipelines-error-state')).toBe(null); done(); }, 1); }); @@ -67,6 +68,8 @@ describe('Pipelines table in Commits and Merge requests', () => { setTimeout(() => { expect(this.component.$el.querySelectorAll('table > tbody > tr').length).toEqual(1); expect(this.component.$el.querySelector('.realtime-loading')).toBe(null); + expect(this.component.$el.querySelector('.empty-state')).toBe(null); + expect(this.component.$el.querySelector('.js-pipelines-error-state')).toBe(null); done(); }, 0); }); @@ -95,10 +98,12 @@ describe('Pipelines table in Commits and Merge requests', () => { this.component.$destroy(); }); - it('should render empty state', function (done) { + it('should render error state', function (done) { setTimeout(() => { expect(this.component.$el.querySelector('.js-pipelines-error-state')).toBeDefined(); expect(this.component.$el.querySelector('.realtime-loading')).toBe(null); + expect(this.component.$el.querySelector('.js-empty-state')).toBe(null); + expect(this.component.$el.querySelector('table')).toBe(null); done(); }, 0); }); diff --git a/spec/javascripts/environments/environment_actions_spec.js b/spec/javascripts/environments/environment_actions_spec.js index 6348d97b0a5..676bf61cfd9 100644 --- a/spec/javascripts/environments/environment_actions_spec.js +++ b/spec/javascripts/environments/environment_actions_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import actionsComp from '~/environments/components/environment_actions'; +import actionsComp from '~/environments/components/environment_actions.vue'; describe('Actions Component', () => { let ActionsComponent; diff --git a/spec/javascripts/environments/environment_item_spec.js b/spec/javascripts/environments/environment_item_spec.js index 4d42de4d549..0e141adb628 100644 --- a/spec/javascripts/environments/environment_item_spec.js +++ b/spec/javascripts/environments/environment_item_spec.js @@ -1,6 +1,6 @@ import 'timeago.js'; import Vue from 'vue'; -import environmentItemComp from '~/environments/components/environment_item'; +import environmentItemComp from '~/environments/components/environment_item.vue'; describe('Environment item', () => { let EnvironmentItem; diff --git a/spec/javascripts/environments/environment_table_spec.js b/spec/javascripts/environments/environment_table_spec.js index 3df967848a7..effbc6c3ee1 100644 --- a/spec/javascripts/environments/environment_table_spec.js +++ b/spec/javascripts/environments/environment_table_spec.js @@ -1,5 +1,5 @@ import Vue from 'vue'; -import environmentTableComp from '~/environments/components/environments_table'; +import environmentTableComp from '~/environments/components/environments_table.vue'; describe('Environment item', () => { preloadFixtures('static/environments/element.html.raw'); diff --git a/spec/javascripts/vue_pipelines_index/async_button_spec.js b/spec/javascripts/pipelines/async_button_spec.js index 28c9c7ab282..28c9c7ab282 100644 --- a/spec/javascripts/vue_pipelines_index/async_button_spec.js +++ b/spec/javascripts/pipelines/async_button_spec.js diff --git a/spec/javascripts/vue_pipelines_index/empty_state_spec.js b/spec/javascripts/pipelines/empty_state_spec.js index bb47a28d9fe..bb47a28d9fe 100644 --- a/spec/javascripts/vue_pipelines_index/empty_state_spec.js +++ b/spec/javascripts/pipelines/empty_state_spec.js diff --git a/spec/javascripts/vue_pipelines_index/error_state_spec.js b/spec/javascripts/pipelines/error_state_spec.js index f667d351f72..f667d351f72 100644 --- a/spec/javascripts/vue_pipelines_index/error_state_spec.js +++ b/spec/javascripts/pipelines/error_state_spec.js diff --git a/spec/javascripts/vue_pipelines_index/mock_data.js b/spec/javascripts/pipelines/mock_data.js index 2365a662b9f..2365a662b9f 100644 --- a/spec/javascripts/vue_pipelines_index/mock_data.js +++ b/spec/javascripts/pipelines/mock_data.js diff --git a/spec/javascripts/vue_pipelines_index/nav_controls_spec.js b/spec/javascripts/pipelines/nav_controls_spec.js index 601eebce38a..601eebce38a 100644 --- a/spec/javascripts/vue_pipelines_index/nav_controls_spec.js +++ b/spec/javascripts/pipelines/nav_controls_spec.js diff --git a/spec/javascripts/vue_pipelines_index/pipeline_url_spec.js b/spec/javascripts/pipelines/pipeline_url_spec.js index 53931d67ad7..53931d67ad7 100644 --- a/spec/javascripts/vue_pipelines_index/pipeline_url_spec.js +++ b/spec/javascripts/pipelines/pipeline_url_spec.js diff --git a/spec/javascripts/vue_pipelines_index/pipelines_actions_spec.js b/spec/javascripts/pipelines/pipelines_actions_spec.js index c89dacbcd93..c89dacbcd93 100644 --- a/spec/javascripts/vue_pipelines_index/pipelines_actions_spec.js +++ b/spec/javascripts/pipelines/pipelines_actions_spec.js diff --git a/spec/javascripts/vue_pipelines_index/pipelines_artifacts_spec.js b/spec/javascripts/pipelines/pipelines_artifacts_spec.js index 9724b63d957..9724b63d957 100644 --- a/spec/javascripts/vue_pipelines_index/pipelines_artifacts_spec.js +++ b/spec/javascripts/pipelines/pipelines_artifacts_spec.js diff --git a/spec/javascripts/vue_pipelines_index/pipelines_spec.js b/spec/javascripts/pipelines/pipelines_spec.js index e9c05f74ce6..e9c05f74ce6 100644 --- a/spec/javascripts/vue_pipelines_index/pipelines_spec.js +++ b/spec/javascripts/pipelines/pipelines_spec.js diff --git a/spec/javascripts/vue_pipelines_index/pipelines_store_spec.js b/spec/javascripts/pipelines/pipelines_store_spec.js index 10ff0c6bb84..10ff0c6bb84 100644 --- a/spec/javascripts/vue_pipelines_index/pipelines_store_spec.js +++ b/spec/javascripts/pipelines/pipelines_store_spec.js diff --git a/spec/javascripts/vue_pipelines_index/stage_spec.js b/spec/javascripts/pipelines/stage_spec.js index 66b57a82363..66b57a82363 100644 --- a/spec/javascripts/vue_pipelines_index/stage_spec.js +++ b/spec/javascripts/pipelines/stage_spec.js diff --git a/spec/javascripts/shortcuts_spec.js b/spec/javascripts/shortcuts_spec.js new file mode 100644 index 00000000000..9b8373df29e --- /dev/null +++ b/spec/javascripts/shortcuts_spec.js @@ -0,0 +1,45 @@ +/* global Shortcuts */ +describe('Shortcuts', () => { + const fixtureName = 'issues/issue_with_comment.html.raw'; + const createEvent = (type, target) => $.Event(type, { + target, + }); + + preloadFixtures(fixtureName); + + describe('toggleMarkdownPreview', () => { + let sc; + + beforeEach(() => { + loadFixtures(fixtureName); + + spyOnEvent('.js-new-note-form .js-md-preview-button', 'focus'); + spyOnEvent('.edit-note .js-md-preview-button', 'focus'); + + sc = new Shortcuts(); + }); + + it('focuses preview button in form', () => { + sc.toggleMarkdownPreview( + createEvent('KeyboardEvent', document.querySelector('.js-new-note-form .js-note-text'), + )); + + expect('focus').toHaveBeenTriggeredOn('.js-new-note-form .js-md-preview-button'); + }); + + it('focues preview button inside edit comment form', (done) => { + document.querySelector('.js-note-edit').click(); + + setTimeout(() => { + sc.toggleMarkdownPreview( + createEvent('KeyboardEvent', document.querySelector('.edit-note .js-note-text'), + )); + + expect('focus').not.toHaveBeenTriggeredOn('.js-new-note-form .js-md-preview-button'); + expect('focus').toHaveBeenTriggeredOn('.edit-note .js-md-preview-button'); + + done(); + }); + }); + }); +}); diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb index 4817fcd031a..dd2674f9f20 100644 --- a/spec/lib/banzai/object_renderer_spec.rb +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -4,13 +4,13 @@ describe Banzai::ObjectRenderer do let(:project) { create(:empty_project) } let(:user) { project.owner } let(:renderer) { described_class.new(project, user, custom_value: 'value') } - let(:object) { Note.new(note: 'hello', note_html: '<p>hello</p>') } + let(:object) { Note.new(note: 'hello', note_html: '<p dir="auto">hello</p>', cached_markdown_version: CacheMarkdownField::CACHE_VERSION) } describe '#render' do it 'renders and redacts an Array of objects' do renderer.render([object], :note) - expect(object.redacted_note_html).to eq '<p>hello</p>' + expect(object.redacted_note_html).to eq '<p dir="auto">hello</p>' expect(object.user_visible_reference_count).to eq 0 end diff --git a/spec/lib/banzai/redactor_spec.rb b/spec/lib/banzai/redactor_spec.rb index 6d2c141e18b..e6f2963193c 100644 --- a/spec/lib/banzai/redactor_spec.rb +++ b/spec/lib/banzai/redactor_spec.rb @@ -42,6 +42,31 @@ describe Banzai::Redactor do end end + context 'when project is in pending delete' do + let!(:issue) { create(:issue, project: project) } + let(:redactor) { described_class.new(project, user) } + + before do + project.update(pending_delete: true) + end + + it 'redacts an issue attached' do + doc = Nokogiri::HTML.fragment("<a class='gfm' data-reference-type='issue' data-issue='#{issue.id}'>foo</a>") + + redactor.redact([doc]) + + expect(doc.to_html).to eq('foo') + end + + it 'redacts an external issue' do + doc = Nokogiri::HTML.fragment("<a class='gfm' data-reference-type='issue' data-external-issue='#{issue.id}' data-project='#{project.id}'>foo</a>") + + redactor.redact([doc]) + + expect(doc.to_html).to eq('foo') + end + end + context 'when reference visible to user' do it 'does not redact an array of documents' do doc1_html = '<a class="gfm" data-reference-type="issue">foo</a>' diff --git a/spec/lib/banzai/renderer_spec.rb b/spec/lib/banzai/renderer_spec.rb index aaa6b12e67e..e6f8d2a1fed 100644 --- a/spec/lib/banzai/renderer_spec.rb +++ b/spec/lib/banzai/renderer_spec.rb @@ -1,73 +1,36 @@ require 'spec_helper' describe Banzai::Renderer do - def expect_render(project = :project) - expected_context = { project: project } - expect(renderer).to receive(:cacheless_render) { :html }.with(:markdown, expected_context) - end - - def expect_cache_update - expect(object).to receive(:update_column).with("field_html", :html) - end - - def fake_object(*features) - markdown = :markdown if features.include?(:markdown) - html = :html if features.include?(:html) - - object = double( - "object", - banzai_render_context: { project: :project }, - field: markdown, - field_html: html - ) + def fake_object(fresh:) + object = double('object') - allow(object).to receive(:markdown_cache_field_for).with(:field).and_return("field_html") - allow(object).to receive(:new_record?).and_return(features.include?(:new)) - allow(object).to receive(:destroyed?).and_return(features.include?(:destroyed)) + allow(object).to receive(:cached_html_up_to_date?).with(:field).and_return(fresh) + allow(object).to receive(:cached_html_for).with(:field).and_return('field_html') object end - describe "#render_field" do + describe '#render_field' do let(:renderer) { Banzai::Renderer } - let(:subject) { renderer.render_field(object, :field) } + subject { renderer.render_field(object, :field) } - context "with an empty cache" do - let(:object) { fake_object(:markdown) } - it "caches and returns the result" do - expect_render - expect_cache_update - expect(subject).to eq(:html) - end - end + context 'with a stale cache' do + let(:object) { fake_object(fresh: false) } - context "with a filled cache" do - let(:object) { fake_object(:markdown, :html) } + it 'caches and returns the result' do + expect(object).to receive(:refresh_markdown_cache!).with(do_update: true) - it "uses the cache" do - expect_render.never - expect_cache_update.never - should eq(:html) + is_expected.to eq('field_html') end end - context "new object" do - let(:object) { fake_object(:new, :markdown) } - - it "doesn't cache the result" do - expect_render - expect_cache_update.never - expect(subject).to eq(:html) - end - end + context 'with an up-to-date cache' do + let(:object) { fake_object(fresh: true) } - context "destroyed object" do - let(:object) { fake_object(:destroyed, :markdown) } + it 'uses the cache' do + expect(object).to receive(:refresh_markdown_cache!).never - it "doesn't cache the result" do - expect_render - expect_cache_update.never - expect(subject).to eq(:html) + is_expected.to eq('field_html') end end end diff --git a/spec/lib/gitlab/git/encoding_helper_spec.rb b/spec/lib/gitlab/git/encoding_helper_spec.rb index 27bcc241b82..f6ac7b23d1d 100644 --- a/spec/lib/gitlab/git/encoding_helper_spec.rb +++ b/spec/lib/gitlab/git/encoding_helper_spec.rb @@ -56,6 +56,10 @@ describe Gitlab::Git::EncodingHelper do expect(r.encoding.name).to eq('UTF-8') end end + + it 'returns empty string on conversion errors' do + expect { ext_class.encode_utf8('') }.not_to raise_error(ArgumentError) + end end describe '#clean' do diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 6151d53cd91..de0069bdcac 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -1,9 +1,6 @@ require 'spec_helper' describe CacheMarkdownField do - caching_classes = CacheMarkdownField::CACHING_CLASSES - CacheMarkdownField::CACHING_CLASSES = ["ThingWithMarkdownFields"].freeze - # The minimum necessary ActiveModel to test this concern class ThingWithMarkdownFields include ActiveModel::Model @@ -27,18 +24,19 @@ describe CacheMarkdownField do cache_markdown_field :foo cache_markdown_field :baz, pipeline: :single_line - def self.add_attr(attr_name) - self.attribute_names += [attr_name] - define_attribute_methods(attr_name) - attr_reader(attr_name) - define_method("#{attr_name}=") do |val| - send("#{attr_name}_will_change!") unless val == send(attr_name) - instance_variable_set("@#{attr_name}", val) + def self.add_attr(name) + self.attribute_names += [name] + define_attribute_methods(name) + attr_reader(name) + define_method("#{name}=") do |value| + write_attribute(name, value) end end - [:foo, :foo_html, :bar, :baz, :baz_html].each do |attr_name| - add_attr(attr_name) + add_attr :cached_markdown_version + + [:foo, :foo_html, :bar, :baz, :baz_html].each do |name| + add_attr(name) end def initialize(*) @@ -48,6 +46,15 @@ describe CacheMarkdownField do clear_changes_information end + def read_attribute(name) + instance_variable_get("@#{name}") + end + + def write_attribute(name, value) + send("#{name}_will_change!") unless value == read_attribute(name) + instance_variable_set("@#{name}", value) + end + def save run_callbacks :save do changes_applied @@ -55,127 +62,236 @@ describe CacheMarkdownField do end end - CacheMarkdownField::CACHING_CLASSES = caching_classes - def thing_subclass(new_attr) Class.new(ThingWithMarkdownFields) { add_attr(new_attr) } end - let(:markdown) { "`Foo`" } - let(:html) { "<p><code>Foo</code></p>" } + let(:markdown) { '`Foo`' } + let(:html) { '<p dir="auto"><code>Foo</code></p>' } - let(:updated_markdown) { "`Bar`" } - let(:updated_html) { "<p dir=\"auto\"><code>Bar</code></p>" } + let(:updated_markdown) { '`Bar`' } + let(:updated_html) { '<p dir="auto"><code>Bar</code></p>' } - subject { ThingWithMarkdownFields.new(foo: markdown, foo_html: html) } + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: CacheMarkdownField::CACHE_VERSION) } - describe ".attributes" do - it "excludes cache attributes" do - expect(thing_subclass(:qux).new.attributes.keys.sort).to eq(%w[bar baz foo qux]) + describe '.attributes' do + it 'excludes cache attributes' do + expect(thing.attributes.keys.sort).to eq(%w[bar baz foo]) end end - describe ".cache_markdown_field" do - it "refuses to allow untracked classes" do - expect { thing_subclass(:qux).__send__(:cache_markdown_field, :qux) }.to raise_error(RuntimeError) + context 'an unchanged markdown field' do + before do + thing.foo = thing.foo + thing.save end + + it { expect(thing.foo).to eq(markdown) } + it { expect(thing.foo_html).to eq(html) } + it { expect(thing.foo_html_changed?).not_to be_truthy } + it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } end - context "an unchanged markdown field" do + context 'a changed markdown field' do before do - subject.foo = subject.foo - subject.save + thing.foo = updated_markdown + thing.save end - it { expect(subject.foo).to eq(markdown) } - it { expect(subject.foo_html).to eq(html) } - it { expect(subject.foo_html_changed?).not_to be_truthy } + it { expect(thing.foo_html).to eq(updated_html) } + it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } end - context "a changed markdown field" do + context 'a non-markdown field changed' do + before do + thing.bar = 'OK' + thing.save + end + + it { expect(thing.bar).to eq('OK') } + it { expect(thing.foo).to eq(markdown) } + it { expect(thing.foo_html).to eq(html) } + it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } + end + + context 'version is out of date' do + let(:thing) { ThingWithMarkdownFields.new(foo: updated_markdown, foo_html: html, cached_markdown_version: nil) } + before do - subject.foo = updated_markdown - subject.save + thing.save + end + + it { expect(thing.foo_html).to eq(updated_html) } + it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } + end + + describe '#cached_html_up_to_date?' do + subject { thing.cached_html_up_to_date?(:foo) } + + it 'returns false when the version is absent' do + thing.cached_markdown_version = nil + + is_expected.to be_falsy + end + + it 'returns false when the version is too early' do + thing.cached_markdown_version -= 1 + + is_expected.to be_falsy + end + + it 'returns false when the version is too late' do + thing.cached_markdown_version += 1 + + is_expected.to be_falsy + end + + it 'returns true when the version is just right' do + thing.cached_markdown_version = CacheMarkdownField::CACHE_VERSION + + is_expected.to be_truthy end - it { expect(subject.foo_html).to eq(updated_html) } + it 'returns false if markdown has been changed but html has not' do + thing.foo = updated_html + + is_expected.to be_falsy + end + + it 'returns true if markdown has not been changed but html has' do + thing.foo_html = updated_html + + is_expected.to be_truthy + end + + it 'returns true if markdown and html have both been changed' do + thing.foo = updated_markdown + thing.foo_html = updated_html + + is_expected.to be_truthy + end end - context "a non-markdown field changed" do + describe '#refresh_markdown_cache!' do before do - subject.bar = "OK" - subject.save + thing.foo = updated_markdown + end + + context 'do_update: false' do + it 'fills all html fields' do + thing.refresh_markdown_cache! + + expect(thing.foo_html).to eq(updated_html) + expect(thing.foo_html_changed?).to be_truthy + expect(thing.baz_html_changed?).to be_truthy + end + + it 'does not save the result' do + expect(thing).not_to receive(:update_columns) + + thing.refresh_markdown_cache! + end + + it 'updates the markdown cache version' do + thing.cached_markdown_version = nil + thing.refresh_markdown_cache! + + expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) + end end - it { expect(subject.bar).to eq("OK") } - it { expect(subject.foo).to eq(markdown) } - it { expect(subject.foo_html).to eq(html) } + context 'do_update: true' do + it 'fills all html fields' do + thing.refresh_markdown_cache!(do_update: true) + + expect(thing.foo_html).to eq(updated_html) + expect(thing.foo_html_changed?).to be_truthy + expect(thing.baz_html_changed?).to be_truthy + end + + it 'skips saving if not persisted' do + expect(thing).to receive(:persisted?).and_return(false) + expect(thing).not_to receive(:update_columns) + + thing.refresh_markdown_cache!(do_update: true) + end + + it 'saves the changes using #update_columns' do + expect(thing).to receive(:persisted?).and_return(true) + expect(thing).to receive(:update_columns) + .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => CacheMarkdownField::CACHE_VERSION) + + thing.refresh_markdown_cache!(do_update: true) + end + end end describe '#banzai_render_context' do - it "sets project to nil if the object lacks a project" do - context = subject.banzai_render_context(:foo) - expect(context).to have_key(:project) + subject(:context) { thing.banzai_render_context(:foo) } + + it 'sets project to nil if the object lacks a project' do + is_expected.to have_key(:project) expect(context[:project]).to be_nil end - it "excludes author if the object lacks an author" do - context = subject.banzai_render_context(:foo) - expect(context).not_to have_key(:author) + it 'excludes author if the object lacks an author' do + is_expected.not_to have_key(:author) end - it "raises if the context for an unrecognised field is requested" do - expect{subject.banzai_render_context(:not_found)}.to raise_error(ArgumentError) + it 'raises if the context for an unrecognised field is requested' do + expect { thing.banzai_render_context(:not_found) }.to raise_error(ArgumentError) end - it "includes the pipeline" do - context = subject.banzai_render_context(:baz) - expect(context[:pipeline]).to eq(:single_line) + it 'includes the pipeline' do + baz = thing.banzai_render_context(:baz) + + expect(baz[:pipeline]).to eq(:single_line) end - it "returns copies of the context template" do - template = subject.cached_markdown_fields[:baz] - copy = subject.banzai_render_context(:baz) + it 'returns copies of the context template' do + template = thing.cached_markdown_fields[:baz] + copy = thing.banzai_render_context(:baz) + expect(copy).not_to be(template) end - context "with a project" do - subject { thing_subclass(:project).new(foo: markdown, foo_html: html, project: :project) } + context 'with a project' do + let(:thing) { thing_subclass(:project).new(foo: markdown, foo_html: html, project: :project_value) } - it "sets the project in the context" do - context = subject.banzai_render_context(:foo) - expect(context).to have_key(:project) - expect(context[:project]).to eq(:project) + it 'sets the project in the context' do + is_expected.to have_key(:project) + expect(context[:project]).to eq(:project_value) end - it "invalidates the cache when project changes" do - subject.project = :new_project + it 'invalidates the cache when project changes' do + thing.project = :new_project allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html) - subject.save + thing.save - expect(subject.foo_html).to eq(updated_html) - expect(subject.baz_html).to eq(updated_html) + expect(thing.foo_html).to eq(updated_html) + expect(thing.baz_html).to eq(updated_html) + expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) end end - context "with an author" do - subject { thing_subclass(:author).new(foo: markdown, foo_html: html, author: :author) } + context 'with an author' do + let(:thing) { thing_subclass(:author).new(foo: markdown, foo_html: html, author: :author_value) } - it "sets the author in the context" do - context = subject.banzai_render_context(:foo) - expect(context).to have_key(:author) - expect(context[:author]).to eq(:author) + it 'sets the author in the context' do + is_expected.to have_key(:author) + expect(context[:author]).to eq(:author_value) end - it "invalidates the cache when author changes" do - subject.author = :new_author + it 'invalidates the cache when author changes' do + thing.author = :new_author allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html) - subject.save + thing.save - expect(subject.foo_html).to eq(updated_html) - expect(subject.baz_html).to eq(updated_html) + expect(thing.foo_html).to eq(updated_html) + expect(thing.baz_html).to eq(updated_html) + expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) end end end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index c720cc9f2c2..b0f3657d3b5 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -386,6 +386,31 @@ describe Member, models: true do end end + describe '.add_users' do + %w[project group].each do |source_type| + context "when source is a #{source_type}" do + let!(:source) { create(source_type, :public, :access_requestable) } + let!(:user) { create(:user) } + let!(:admin) { create(:admin) } + + it 'returns a <Source>Member objects' do + members = described_class.add_users(source, [user], :master) + + expect(members).to be_a Array + expect(members.first).to be_a "#{source_type.classify}Member".constantize + expect(members.first).to be_persisted + end + + it 'returns an empty array' do + members = described_class.add_users(source, [], :master) + + expect(members).to be_a Array + expect(members).to be_empty + end + end + end + end + describe '#accept_request' do let(:member) { create(:project_member, requested_at: Time.now.utc) } diff --git a/spec/models/members/group_member_spec.rb b/spec/models/members/group_member_spec.rb index 024380b7ebb..17765b25856 100644 --- a/spec/models/members/group_member_spec.rb +++ b/spec/models/members/group_member_spec.rb @@ -13,12 +13,12 @@ describe GroupMember, models: true do end end - describe '.add_users_to_group' do + describe '.add_users' do it 'adds the given users to the given group' do group = create(:group) users = create_list(:user, 2) - described_class.add_users_to_group( + described_class.add_users( group, [users.first.id, users.second], described_class::MASTER diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 74bc4847247..40365585a56 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -24,6 +24,7 @@ describe API::Projects, :api do namespace: user.namespace, merge_requests_enabled: false, issues_enabled: false, wiki_enabled: false, + builds_enabled: false, snippets_enabled: false) end let(:project_member3) do @@ -342,6 +343,7 @@ describe API::Projects, :api do project = attributes_for(:project, { path: 'camelCasePath', issues_enabled: false, + jobs_enabled: false, merge_requests_enabled: false, wiki_enabled: false, only_allow_merge_if_pipeline_succeeds: false, @@ -351,6 +353,8 @@ describe API::Projects, :api do post api('/projects', user), project + expect(response).to have_http_status(201) + project.each_pair do |k, v| next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k) expect(json_response[k.to_s]).to eq(v) @@ -1078,7 +1082,9 @@ describe API::Projects, :api do it 'returns 400 when nothing sent' do project_param = {} + put api("/projects/#{project.id}", user), project_param + expect(response).to have_http_status(400) expect(json_response['error']).to match('at least one parameter must be provided') end @@ -1086,7 +1092,9 @@ describe API::Projects, :api do context 'when unauthenticated' do it 'returns authentication error' do project_param = { name: 'bar' } + put api("/projects/#{project.id}"), project_param + expect(response).to have_http_status(401) end end @@ -1094,8 +1102,11 @@ describe API::Projects, :api do context 'when authenticated as project owner' do it 'updates name' do project_param = { name: 'bar' } + put api("/projects/#{project.id}", user), project_param + expect(response).to have_http_status(200) + project_param.each_pair do |k, v| expect(json_response[k.to_s]).to eq(v) end @@ -1103,8 +1114,11 @@ describe API::Projects, :api do it 'updates visibility_level' do project_param = { visibility: 'public' } + put api("/projects/#{project3.id}", user), project_param + expect(response).to have_http_status(200) + project_param.each_pair do |k, v| expect(json_response[k.to_s]).to eq(v) end @@ -1113,17 +1127,23 @@ describe API::Projects, :api do it 'updates visibility_level from public to private' do project3.update_attributes({ visibility_level: Gitlab::VisibilityLevel::PUBLIC }) project_param = { visibility: 'private' } + put api("/projects/#{project3.id}", user), project_param + expect(response).to have_http_status(200) + project_param.each_pair do |k, v| expect(json_response[k.to_s]).to eq(v) end + expect(json_response['visibility']).to eq('private') end it 'does not update name to existing name' do project_param = { name: project3.name } + put api("/projects/#{project.id}", user), project_param + expect(response).to have_http_status(400) expect(json_response['message']['name']).to eq(['has already been taken']) end @@ -1139,8 +1159,23 @@ describe API::Projects, :api do it 'updates path & name to existing path & name in different namespace' do project_param = { path: project4.path, name: project4.name } + put api("/projects/#{project3.id}", user), project_param + expect(response).to have_http_status(200) + + project_param.each_pair do |k, v| + expect(json_response[k.to_s]).to eq(v) + end + end + + it 'updates jobs_enabled' do + project_param = { jobs_enabled: true } + + put api("/projects/#{project3.id}", user), project_param + + expect(response).to have_http_status(200) + project_param.each_pair do |k, v| expect(json_response[k.to_s]).to eq(v) end |