diff options
Diffstat (limited to 'spec')
104 files changed, 2697 insertions, 862 deletions
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 6ec84f5c528..1db1963476c 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -252,7 +252,7 @@ describe Projects::NotesController do before do service_params = ActionController::Parameters.new({ note: 'some note', - noteable_id: merge_request.id.to_s, + noteable_id: merge_request.id, noteable_type: 'MergeRequest', commit_id: nil, merge_request_diff_head_sha: 'sha' diff --git a/spec/controllers/projects/settings/repository_controller_spec.rb b/spec/controllers/projects/settings/repository_controller_spec.rb index b34053fc993..7f67f67e775 100644 --- a/spec/controllers/projects/settings/repository_controller_spec.rb +++ b/spec/controllers/projects/settings/repository_controller_spec.rb @@ -32,4 +32,24 @@ describe Projects::Settings::RepositoryController do expect(RepositoryCleanupWorker).to have_received(:perform_async).once end end + + describe 'POST create_deploy_token' do + let(:deploy_token_params) do + { + name: 'deployer_token', + expires_at: 1.month.from_now.to_date.to_s, + username: 'deployer', + read_repository: '1' + } + end + + subject(:create_deploy_token) { post :create_deploy_token, params: { namespace_id: project.namespace, project_id: project, deploy_token: deploy_token_params } } + + it 'creates deploy token' do + expect { create_deploy_token }.to change { DeployToken.active.count }.by(1) + + expect(response).to have_gitlab_http_status(200) + expect(response).to render_template(:show) + end + end end diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb index bebf17728c0..9e7d34b10c0 100644 --- a/spec/controllers/projects/templates_controller_spec.rb +++ b/spec/controllers/projects/templates_controller_spec.rb @@ -3,49 +3,101 @@ require 'spec_helper' describe Projects::TemplatesController do - let(:project) { create(:project, :repository) } + let(:project) { create(:project, :repository, :private) } let(:user) { create(:user) } - let(:user2) { create(:user) } - let(:file_path_1) { '.gitlab/issue_templates/bug.md' } + let(:file_path_1) { '.gitlab/issue_templates/issue_template.md' } + let(:file_path_2) { '.gitlab/merge_request_templates/merge_request_template.md' } let(:body) { JSON.parse(response.body) } + let!(:file_1) { project.repository.create_file(user, file_path_1, 'issue content', message: 'message', branch_name: 'master') } + let!(:file_2) { project.repository.create_file(user, file_path_2, 'merge request content', message: 'message', branch_name: 'master') } - before do - project.add_developer(user) - sign_in(user) - end + describe '#show' do + shared_examples 'renders issue templates as json' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json) - before do - project.add_user(user, Gitlab::Access::MAINTAINER) - project.repository.create_file(user, file_path_1, 'something valid', - message: 'test 3', branch_name: 'master') - end + expect(response.status).to eq(200) + expect(body['name']).to eq('issue_template') + expect(body['content']).to eq('issue content') + end + end - describe '#show' do - it 'renders template name and content as json' do - get(:show, params: { namespace_id: project.namespace.to_param, template_type: "issue", key: "bug", project_id: project }, format: :json) + shared_examples 'renders merge request templates as json' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json) + + expect(response.status).to eq(200) + expect(body['name']).to eq('merge_request_template') + expect(body['content']).to eq('merge request content') + end + end + + shared_examples 'renders 404 when requesting an issue template' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json) + + expect(response.status).to eq(404) + end + end + + shared_examples 'renders 404 when requesting a merge request template' do + it do + get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json) - expect(response.status).to eq(200) - expect(body["name"]).to eq("bug") - expect(body["content"]).to eq("something valid") + expect(response.status).to eq(404) + end end - it 'renders 404 when unauthorized' do - sign_in(user2) - get(:show, params: { namespace_id: project.namespace.to_param, template_type: "issue", key: "bug", project_id: project }, format: :json) + shared_examples 'renders 404 when params are invalid' do + it 'does not route when the template type is invalid' do + expect do + get(:show, params: { namespace_id: project.namespace, template_type: 'invalid_type', key: 'issue_template', project_id: project }, format: :json) + end.to raise_error(ActionController::UrlGenerationError) + end + + it 'renders 404 when the format type is invalid' do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :html) + + expect(response.status).to eq(404) + end + + it 'renders 404 when the key is unknown' do + get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'unknown_template', project_id: project }, format: :json) - expect(response.status).to eq(404) + expect(response.status).to eq(404) + end end - it 'renders 404 when template type is not found' do - sign_in(user) - get(:show, params: { namespace_id: project.namespace.to_param, template_type: "dont_exist", key: "bug", project_id: project }, format: :json) + context 'when the user is not a member of the project' do + before do + sign_in(user) + end - expect(response.status).to eq(404) + include_examples 'renders 404 when requesting an issue template' + include_examples 'renders 404 when requesting a merge request template' + include_examples 'renders 404 when params are invalid' end - it 'renders 404 without errors' do - sign_in(user) - expect { get(:show, params: { namespace_id: project.namespace.to_param, template_type: "dont_exist", key: "bug", project_id: project }, format: :json) }.not_to raise_error + context 'when user is a member of the project' do + before do + project.add_developer(user) + sign_in(user) + end + + include_examples 'renders issue templates as json' + include_examples 'renders merge request templates as json' + include_examples 'renders 404 when params are invalid' + end + + context 'when user is a guest of the project' do + before do + project.add_guest(user) + sign_in(user) + end + + include_examples 'renders issue templates as json' + include_examples 'renders 404 when requesting a merge request template' + include_examples 'renders 404 when params are invalid' end end end diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 936d7c7dae4..586d59c2d09 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -119,6 +119,119 @@ describe Snippets::NotesController do end end + describe 'POST create' do + context 'when a snippet is public' do + let(:request_params) do + { + note: attributes_for(:note_on_personal_snippet, noteable: public_snippet), + snippet_id: public_snippet.id + } + end + + before do + sign_in user + end + + it 'returns status 302' do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(302) + end + + it 'creates the note' do + expect { post :create, params: request_params }.to change { Note.count }.by(1) + end + end + + context 'when a snippet is internal' do + let(:request_params) do + { + note: attributes_for(:note_on_personal_snippet, noteable: internal_snippet), + snippet_id: internal_snippet.id + } + end + + before do + sign_in user + end + + it 'returns status 302' do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(302) + end + + it 'creates the note' do + expect { post :create, params: request_params }.to change { Note.count }.by(1) + end + end + + context 'when a snippet is private' do + let(:request_params) do + { + note: attributes_for(:note_on_personal_snippet, noteable: private_snippet), + snippet_id: private_snippet.id + } + end + + before do + sign_in user + end + + context 'when user is not the author' do + before do + sign_in(user) + end + + it 'returns status 404' do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(404) + end + + it 'does not create the note' do + expect { post :create, params: request_params }.not_to change { Note.count } + end + + context 'when user sends a snippet_id for a public snippet' do + let(:request_params) do + { + note: attributes_for(:note_on_personal_snippet, noteable: private_snippet), + snippet_id: public_snippet.id + } + end + + it 'returns status 302' do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(302) + end + + it 'creates the note on the public snippet' do + expect { post :create, params: request_params }.to change { Note.count }.by(1) + expect(Note.last.noteable).to eq public_snippet + end + end + end + + context 'when user is the author' do + before do + sign_in(private_snippet.author) + end + + it 'returns status 302' do + post :create, params: request_params + + expect(response).to have_gitlab_http_status(302) + end + + it 'creates the note' do + expect { post :create, params: request_params }.to change { Note.count }.by(1) + end + end + end + end + describe 'DELETE destroy' do let(:request_params) do { diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index f8666a1986f..3aba02bf3ff 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -209,8 +209,8 @@ describe SnippetsController do context 'when the snippet description contains a file' do include FileMoverHelpers - let(:picture_file) { '/-/system/temp/secret56/picture.jpg' } - let(:text_file) { '/-/system/temp/secret78/text.txt' } + let(:picture_file) { "/-/system/user/#{user.id}/secret56/picture.jpg" } + let(:text_file) { "/-/system/user/#{user.id}/secret78/text.txt" } let(:description) do "Description with picture:  and "\ "text: [text.txt](/uploads#{text_file})" diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index d27658e02cb..0876502a899 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -24,121 +24,160 @@ describe UploadsController do let!(:user) { create(:user, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) } describe 'POST create' do - let(:model) { 'personal_snippet' } - let(:snippet) { create(:personal_snippet, :public) } let(:jpg) { fixture_file_upload('spec/fixtures/rails_sample.jpg', 'image/jpg') } let(:txt) { fixture_file_upload('spec/fixtures/doc_sample.txt', 'text/plain') } - context 'when a user does not have permissions to upload a file' do - it "returns 401 when the user is not logged in" do - post :create, params: { model: model, id: snippet.id }, format: :json + context 'snippet uploads' do + let(:model) { 'personal_snippet' } + let(:snippet) { create(:personal_snippet, :public) } - expect(response).to have_gitlab_http_status(401) - end + context 'when a user does not have permissions to upload a file' do + it "returns 401 when the user is not logged in" do + post :create, params: { model: model, id: snippet.id }, format: :json - it "returns 404 when user can't comment on a snippet" do - private_snippet = create(:personal_snippet, :private) + expect(response).to have_gitlab_http_status(401) + end - sign_in(user) - post :create, params: { model: model, id: private_snippet.id }, format: :json + it "returns 404 when user can't comment on a snippet" do + private_snippet = create(:personal_snippet, :private) - expect(response).to have_gitlab_http_status(404) - end - end + sign_in(user) + post :create, params: { model: model, id: private_snippet.id }, format: :json - context 'when a user is logged in' do - before do - sign_in(user) + expect(response).to have_gitlab_http_status(404) + end end - it "returns an error without file" do - post :create, params: { model: model, id: snippet.id }, format: :json + context 'when a user is logged in' do + before do + sign_in(user) + end - expect(response).to have_gitlab_http_status(422) - end + it "returns an error without file" do + post :create, params: { model: model, id: snippet.id }, format: :json - it "returns an error with invalid model" do - expect { post :create, params: { model: 'invalid', id: snippet.id }, format: :json } - .to raise_error(ActionController::UrlGenerationError) - end + expect(response).to have_gitlab_http_status(422) + end - it "returns 404 status when object not found" do - post :create, params: { model: model, id: 9999 }, format: :json + it "returns an error with invalid model" do + expect { post :create, params: { model: 'invalid', id: snippet.id }, format: :json } + .to raise_error(ActionController::UrlGenerationError) + end - expect(response).to have_gitlab_http_status(404) - end + it "returns 404 status when object not found" do + post :create, params: { model: model, id: 9999 }, format: :json - context 'with valid image' do - before do - post :create, params: { model: 'personal_snippet', id: snippet.id, file: jpg }, format: :json + expect(response).to have_gitlab_http_status(404) end - it 'returns a content with original filename, new link, and correct type.' do - expect(response.body).to match '\"alt\":\"rails_sample\"' - expect(response.body).to match "\"url\":\"/uploads" + context 'with valid image' do + before do + post :create, params: { model: 'personal_snippet', id: snippet.id, file: jpg }, format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + expect(response.body).to match '\"alt\":\"rails_sample\"' + expect(response.body).to match "\"url\":\"/uploads" + end + + it 'creates a corresponding Upload record' do + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq snippet + end + end end - it 'creates a corresponding Upload record' do - upload = Upload.last + context 'with valid non-image file' do + before do + post :create, params: { model: 'personal_snippet', id: snippet.id, file: txt }, format: :json + end - aggregate_failures do - expect(upload).to exist - expect(upload.model).to eq snippet + it 'returns a content with original filename, new link, and correct type.' do + expect(response.body).to match '\"alt\":\"doc_sample.txt\"' + expect(response.body).to match "\"url\":\"/uploads" + end + + it 'creates a corresponding Upload record' do + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq snippet + end end end end + end + + context 'user uploads' do + let(:model) { 'user' } + + it 'returns 401 when the user has no access' do + post :create, params: { model: 'user', id: user.id }, format: :json - context 'with valid non-image file' do + expect(response).to have_gitlab_http_status(401) + end + + context 'when user is logged in' do before do - post :create, params: { model: 'personal_snippet', id: snippet.id, file: txt }, format: :json + sign_in(user) + end + + subject do + post :create, params: { model: model, id: user.id, file: jpg }, format: :json end it 'returns a content with original filename, new link, and correct type.' do - expect(response.body).to match '\"alt\":\"doc_sample.txt\"' - expect(response.body).to match "\"url\":\"/uploads" + subject + + expect(response.body).to match '\"alt\":\"rails_sample\"' + expect(response.body).to match "\"url\":\"/uploads/-/system/user/#{user.id}/" end it 'creates a corresponding Upload record' do + expect { subject }.to change { Upload.count } + upload = Upload.last aggregate_failures do expect(upload).to exist - expect(upload.model).to eq snippet + expect(upload.model).to eq user end end - end - context 'temporal with valid image' do - subject do - post :create, params: { model: 'personal_snippet', file: jpg }, format: :json - end + context 'with valid non-image file' do + subject do + post :create, params: { model: model, id: user.id, file: txt }, format: :json + end - it 'returns a content with original filename, new link, and correct type.' do - subject + it 'returns a content with original filename, new link, and correct type.' do + subject - expect(response.body).to match '\"alt\":\"rails_sample\"' - expect(response.body).to match "\"url\":\"/uploads/-/system/temp" - end + expect(response.body).to match '\"alt\":\"doc_sample.txt\"' + expect(response.body).to match "\"url\":\"/uploads/-/system/user/#{user.id}/" + end - it 'does not create an Upload record' do - expect { subject }.not_to change { Upload.count } - end - end + it 'creates a corresponding Upload record' do + expect { subject }.to change { Upload.count } - context 'temporal with valid non-image file' do - subject do - post :create, params: { model: 'personal_snippet', file: txt }, format: :json + upload = Upload.last + + aggregate_failures do + expect(upload).to exist + expect(upload.model).to eq user + end + end end - it 'returns a content with original filename, new link, and correct type.' do - subject + it 'returns 404 when given user is not the logged in one' do + another_user = create(:user) - expect(response.body).to match '\"alt\":\"doc_sample.txt\"' - expect(response.body).to match "\"url\":\"/uploads/-/system/temp" - end + post :create, params: { model: model, id: another_user.id, file: txt }, format: :json - it 'does not create an Upload record' do - expect { subject }.not_to change { Upload.count } + expect(response).to have_gitlab_http_status(404) end end end diff --git a/spec/factories/releases.rb b/spec/factories/releases.rb index cab6b4a811f..4cacc77c182 100644 --- a/spec/factories/releases.rb +++ b/spec/factories/releases.rb @@ -6,6 +6,7 @@ FactoryBot.define do description "Awesome release" project author + released_at { Time.zone.parse('2018-10-20T18:00:00Z') } trait :legacy do sha nil diff --git a/spec/factories/services.rb b/spec/factories/services.rb index 763909f30bd..ecb481ed84a 100644 --- a/spec/factories/services.rb +++ b/spec/factories/services.rb @@ -6,8 +6,6 @@ FactoryBot.define do factory :custom_issue_tracker_service, class: CustomIssueTrackerService do project - type 'CustomIssueTrackerService' - category 'issue_tracker' active true properties( project_url: 'https://project.url.com', @@ -54,6 +52,38 @@ FactoryBot.define do ) end + factory :bugzilla_service do + project + active true + issue_tracker + end + + factory :redmine_service do + project + active true + issue_tracker + end + + factory :youtrack_service do + project + active true + issue_tracker + end + + factory :gitlab_issue_tracker_service do + project + active true + issue_tracker + end + + trait :issue_tracker do + properties( + project_url: 'http://issue-tracker.example.com', + issues_url: 'http://issue-tracker.example.com', + new_issue_url: 'http://issue-tracker.example.com' + ) + end + factory :jira_cloud_service, class: JiraService do project active true diff --git a/spec/features/discussion_comments/commit_spec.rb b/spec/features/discussion_comments/commit_spec.rb index ea720cee74e..b3f1731ec95 100644 --- a/spec/features/discussion_comments/commit_spec.rb +++ b/spec/features/discussion_comments/commit_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Discussion Comments Commit', :js do +describe 'Thread Comments Commit', :js do include RepoHelpers let(:user) { create(:user) } @@ -16,7 +16,7 @@ describe 'Discussion Comments Commit', :js do visit project_commit_path(project, sample_commit.id) end - it_behaves_like 'discussion comments', 'commit' + it_behaves_like 'thread comments', 'commit' it 'has class .js-note-emoji' do expect(page).to have_css('.js-note-emoji') diff --git a/spec/features/discussion_comments/issue_spec.rb b/spec/features/discussion_comments/issue_spec.rb index 5ec19460bbd..d71a1ee4731 100644 --- a/spec/features/discussion_comments/issue_spec.rb +++ b/spec/features/discussion_comments/issue_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Discussion Comments Issue', :js do +describe 'Thread Comments Issue', :js do let(:user) { create(:user) } let(:project) { create(:project) } let(:issue) { create(:issue, project: project) } @@ -12,5 +12,5 @@ describe 'Discussion Comments Issue', :js do visit project_issue_path(project, issue) end - it_behaves_like 'discussion comments', 'issue' + it_behaves_like 'thread comments', 'issue' end diff --git a/spec/features/discussion_comments/merge_request_spec.rb b/spec/features/discussion_comments/merge_request_spec.rb index f940e973923..86e3507a3ee 100644 --- a/spec/features/discussion_comments/merge_request_spec.rb +++ b/spec/features/discussion_comments/merge_request_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Discussion Comments Merge Request', :js do +describe 'Thread Comments Merge Request', :js do let(:user) { create(:user) } let(:project) { create(:project, :repository) } let(:merge_request) { create(:merge_request, source_project: project) } @@ -12,5 +12,5 @@ describe 'Discussion Comments Merge Request', :js do visit project_merge_request_path(project, merge_request) end - it_behaves_like 'discussion comments', 'merge request' + it_behaves_like 'thread comments', 'merge request' end diff --git a/spec/features/discussion_comments/snippets_spec.rb b/spec/features/discussion_comments/snippets_spec.rb index d330e89505e..29aa3e4366c 100644 --- a/spec/features/discussion_comments/snippets_spec.rb +++ b/spec/features/discussion_comments/snippets_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe 'Discussion Comments Snippet', :js do +describe 'Thread Comments Snippet', :js do let(:user) { create(:user) } let(:project) { create(:project) } let(:snippet) { create(:project_snippet, :private, project: project, author: user) } @@ -12,5 +12,5 @@ describe 'Discussion Comments Snippet', :js do visit project_snippet_path(project, snippet) end - it_behaves_like 'discussion comments', 'snippet' + it_behaves_like 'thread comments', 'snippet' end diff --git a/spec/features/groups/issues_spec.rb b/spec/features/groups/issues_spec.rb index c000165ccd9..0ada530781c 100644 --- a/spec/features/groups/issues_spec.rb +++ b/spec/features/groups/issues_spec.rb @@ -150,6 +150,25 @@ describe 'Group issues page' do check_issue_order end + it 'issues should not be draggable when user is not logged in', :js do + sign_out(user_in_group) + + visit issues_group_path(group, sort: 'relative_position') + + drag_to(selector: '.manual-ordering', + from_index: 0, + to_index: 2) + + wait_for_requests + + # Issue order should remain the same + page.within('.manual-ordering') do + expect(find('.issue:nth-child(1) .title')).to have_content('Issue #1') + expect(find('.issue:nth-child(2) .title')).to have_content('Issue #2') + expect(find('.issue:nth-child(3) .title')).to have_content('Issue #3') + end + end + def check_issue_order page.within('.manual-ordering') do expect(find('.issue:nth-child(1) .title')).to have_content('Issue #2') diff --git a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb index ada57285abf..f6dccb5e98a 100644 --- a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb +++ b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb @@ -1,13 +1,13 @@ require 'rails_helper' -describe 'Resolving all open discussions in a merge request from an issue', :js do +describe 'Resolving all open threads in a merge request from an issue', :js do let(:user) { create(:user) } let(:project) { create(:project, :repository) } let(:merge_request) { create(:merge_request, source_project: project) } let!(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion } def resolve_all_discussions_link_selector - text = "Resolve all discussions in new issue" + text = "Resolve all threads in new issue" url = new_project_issue_path(project, merge_request_to_resolve_discussions_of: merge_request.iid) %Q{a[data-original-title="#{text}"][href="#{url}"]} end @@ -19,15 +19,15 @@ describe 'Resolving all open discussions in a merge request from an issue', :js visit project_merge_request_path(project, merge_request) end - it 'shows a button to resolve all discussions by creating a new issue' do + it 'shows a button to resolve all threads by creating a new issue' do within('.line-resolve-all-container') do expect(page).to have_selector resolve_all_discussions_link_selector end end - context 'resolving the discussion' do + context 'resolving the thread' do before do - click_button 'Resolve discussion' + click_button 'Resolve thread' end it 'hides the link for creating a new issue' do @@ -35,15 +35,15 @@ describe 'Resolving all open discussions in a merge request from an issue', :js end end - context 'creating an issue for discussions' do + context 'creating an issue for threads' do before do find(resolve_all_discussions_link_selector).click end - it_behaves_like 'creating an issue for a discussion' + it_behaves_like 'creating an issue for a thread' end - context 'for a project where all discussions need to be resolved before merging' do + context 'for a project where all threads need to be resolved before merging' do before do project.update_attribute(:only_allow_merge_if_all_discussions_are_resolved, true) end @@ -59,27 +59,27 @@ describe 'Resolving all open discussions in a merge request from an issue', :js end end - context 'merge request has discussions that need to be resolved' do + context 'merge request has threads that need to be resolved' do before do visit project_merge_request_path(project, merge_request) end - it 'shows a warning that the merge request contains unresolved discussions' do - expect(page).to have_content 'There are unresolved discussions.' + it 'shows a warning that the merge request contains unresolved threads' do + expect(page).to have_content 'There are unresolved threads.' end - it 'has a link to resolve all discussions by creating an issue' do + it 'has a link to resolve all threads by creating an issue' do page.within '.mr-widget-body' do expect(page).to have_link 'Create an issue to resolve them later', href: new_project_issue_path(project, merge_request_to_resolve_discussions_of: merge_request.iid) end end - context 'creating an issue for discussions' do + context 'creating an issue for threads' do before do page.click_link 'Create an issue to resolve them later', href: new_project_issue_path(project, merge_request_to_resolve_discussions_of: merge_request.iid) end - it_behaves_like 'creating an issue for a discussion' + it_behaves_like 'creating an issue for a thread' end end end @@ -92,8 +92,8 @@ describe 'Resolving all open discussions in a merge request from an issue', :js visit new_project_issue_path(project, merge_request_to_resolve_discussions_of: merge_request.iid) end - it 'Shows a notice to ask someone else to resolve the discussions' do - expect(page).to have_content("The discussions at #{merge_request.to_reference} will stay unresolved. Ask someone with permission to resolve them.") + it 'Shows a notice to ask someone else to resolve the threads' do + expect(page).to have_content("The threads at #{merge_request.to_reference} will stay unresolved. Ask someone with permission to resolve them.") end end end diff --git a/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb index b20730bdb22..1b1a31d0723 100644 --- a/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb +++ b/spec/features/issues/create_issue_for_single_discussion_in_merge_request_spec.rb @@ -1,13 +1,13 @@ require 'rails_helper' -describe 'Resolve an open discussion in a merge request by creating an issue', :js do +describe 'Resolve an open thread in a merge request by creating an issue', :js do let(:user) { create(:user) } let(:project) { create(:project, :repository, only_allow_merge_if_all_discussions_are_resolved: true) } let(:merge_request) { create(:merge_request, source_project: project) } let!(:discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion } def resolve_discussion_selector - title = 'Resolve this discussion in a new issue' + title = 'Resolve this thread in a new issue' url = new_project_issue_path(project, discussion_to_resolve: discussion.id, merge_request_to_resolve_discussions_of: merge_request.iid) "a[data-original-title=\"#{title}\"][href=\"#{url}\"]" end @@ -30,40 +30,40 @@ describe 'Resolve an open discussion in a merge request by creating an issue', : end end - context 'resolving the discussion' do + context 'resolving the thread' do before do - click_button 'Resolve discussion' + click_button 'Resolve thread' end it 'hides the link for creating a new issue' do expect(page).not_to have_css resolve_discussion_selector end - it 'shows the link for creating a new issue when unresolving a discussion' do + it 'shows the link for creating a new issue when unresolving a thread' do page.within '.diff-content' do - click_button 'Unresolve discussion' + click_button 'Unresolve thread' end expect(page).to have_css resolve_discussion_selector end end - it 'has a link to create a new issue for a discussion' do + it 'has a link to create a new issue for a thread' do expect(page).to have_css resolve_discussion_selector end context 'creating the issue' do before do - find(resolve_discussion_selector).click + find(resolve_discussion_selector, match: :first).click end - it 'has a hidden field for the discussion' do + it 'has a hidden field for the thread' do discussion_field = find('#discussion_to_resolve', visible: false) expect(discussion_field.value).to eq(discussion.id.to_s) end - it_behaves_like 'creating an issue for a discussion' + it_behaves_like 'creating an issue for a thread' end end @@ -75,8 +75,8 @@ describe 'Resolve an open discussion in a merge request by creating an issue', : discussion_to_resolve: discussion.id) end - it 'Shows a notice to ask someone else to resolve the discussions' do - expect(page).to have_content("The discussion at #{merge_request.to_reference}"\ + it 'Shows a notice to ask someone else to resolve the threads' do + expect(page).to have_content("The thread at #{merge_request.to_reference}"\ " (discussion #{discussion.first_note.id}) will stay unresolved."\ " Ask someone with permission to resolve it.") end diff --git a/spec/features/merge_request/user_comments_on_merge_request_spec.rb b/spec/features/merge_request/user_comments_on_merge_request_spec.rb index 69bdab85d81..e6bc3780b5c 100644 --- a/spec/features/merge_request/user_comments_on_merge_request_spec.rb +++ b/spec/features/merge_request/user_comments_on_merge_request_spec.rb @@ -37,7 +37,7 @@ describe 'User comments on a merge request', :js do wait_for_requests page.within('.notes .discussion') do - expect(page).to have_content("#{user.name} #{user.to_reference} started a discussion") + expect(page).to have_content("#{user.name} #{user.to_reference} started a thread") expect(page).to have_content(sample_commit.line_code_path) expect(page).to have_content('Line is wrong') end diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index 260eec7a9ed..10fe60cb075 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe 'Merge request > User resolves diff notes and discussions', :js do +describe 'Merge request > User resolves diff notes and threads', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:guest) { create(:user) } @@ -17,7 +17,7 @@ describe 'Merge request > User resolves diff notes and discussions', :js do ) end - context 'no discussions' do + context 'no threads' do before do project.add_maintainer(user) sign_in(user) @@ -25,8 +25,8 @@ describe 'Merge request > User resolves diff notes and discussions', :js do visit_merge_request end - it 'displays no discussion resolved data' do - expect(page).not_to have_content('discussion resolved') + it 'displays no thread resolved data' do + expect(page).not_to have_content('thread resolved') expect(page).not_to have_selector('.discussion-next-btn') end end @@ -38,10 +38,10 @@ describe 'Merge request > User resolves diff notes and discussions', :js do visit_merge_request end - context 'single discussion' do - it 'shows text with how many discussions' do + context 'single thread' do + it 'shows text with how many threads' do page.within '.line-resolve-all-container' do - expect(page).to have_content('0/1 discussion resolved') + expect(page).to have_content('0/1 thread resolved') end end @@ -54,18 +54,18 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.within '.diff-content' do - expect(page).to have_selector('.btn', text: 'Unresolve discussion') + expect(page).to have_selector('.btn', text: 'Unresolve thread') end page.within '.line-resolve-all-container' do - expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_content('1/1 thread resolved') expect(page).to have_selector('.line-resolve-btn.is-active') end end - it 'allows user to mark discussion as resolved' do + it 'allows user to mark thread as resolved' do page.within '.diff-content' do - click_button 'Resolve discussion' + click_button 'Resolve thread' end expect(page).to have_selector('.discussion-body', visible: false) @@ -75,38 +75,38 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.within '.line-resolve-all-container' do - expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_content('1/1 thread resolved') expect(page).to have_selector('.line-resolve-btn.is-active') end end - it 'allows user to unresolve discussion' do + it 'allows user to unresolve thread' do page.within '.diff-content' do - click_button 'Resolve discussion' - click_button 'Unresolve discussion' + click_button 'Resolve thread' + click_button 'Unresolve thread' end page.within '.line-resolve-all-container' do - expect(page).to have_content('0/1 discussion resolved') + expect(page).to have_content('0/1 thread resolved') end end - describe 'resolved discussion' do + describe 'resolved thread' do before do page.within '.diff-content' do - click_button 'Resolve discussion' + click_button 'Resolve thread' end visit_merge_request end describe 'timeline view' do - it 'hides when resolve discussion is clicked' do + it 'hides when resolve thread is clicked' do expect(page).to have_selector('.discussion-header') expect(page).not_to have_selector('.discussion-body') end - it 'shows resolved discussion when toggled' do + it 'shows resolved thread when toggled' do find(".timeline-content .discussion[data-discussion-id='#{note.discussion_id}'] .discussion-toggle-button").click expect(page.find(".line-holder-placeholder")).to be_visible @@ -130,11 +130,11 @@ describe 'Merge request > User resolves diff notes and discussions', :js do page.find('#parallel-diff-btn').click end - it 'hides when resolve discussion is clicked' do + it 'hides when resolve thread is clicked' do expect(page).not_to have_selector('.diffs .diff-file .notes_holder') end - it 'shows resolved discussion when toggled' do + it 'shows resolved thread when toggled' do find('.diff-comment-avatar-holders').click expect(find('.diffs .diff-file .notes_holder')).to be_visible @@ -143,7 +143,7 @@ describe 'Merge request > User resolves diff notes and discussions', :js do describe 'reply form' do before do - click_button 'Toggle discussion' + click_button 'Toggle thread' page.within '.diff-content' do click_button 'Reply...' @@ -160,34 +160,34 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.within '.line-resolve-all-container' do - expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_content('1/1 thread resolved') end end it 'allows user to unresolve from reply form without a comment' do page.within '.diff-content' do - click_button 'Unresolve discussion' + click_button 'Unresolve thread' wait_for_requests end page.within '.line-resolve-all-container' do - expect(page).to have_content('0/1 discussion resolved') + expect(page).to have_content('0/1 thread resolved') expect(page).not_to have_selector('.line-resolve-btn.is-active') end end - it 'allows user to comment & unresolve discussion' do + it 'allows user to comment & unresolve thread' do page.within '.diff-content' do find('.js-note-text').set 'testing' - click_button 'Comment & unresolve discussion' + click_button 'Comment & unresolve thread' wait_for_requests end page.within '.line-resolve-all-container' do - expect(page).to have_content('0/1 discussion resolved') + expect(page).to have_content('0/1 thread resolved') end end end @@ -197,31 +197,31 @@ describe 'Merge request > User resolves diff notes and discussions', :js do page.within '.diff-content' do click_button 'Reply...' - click_button 'Resolve discussion' + click_button 'Resolve thread' end page.within '.line-resolve-all-container' do - expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_content('1/1 thread resolved') expect(page).to have_selector('.line-resolve-btn.is-active') end end - it 'allows user to comment & resolve discussion' do + it 'allows user to comment & resolve thread' do page.within '.diff-content' do click_button 'Reply...' find('.js-note-text').set 'testing' - click_button 'Comment & resolve discussion' + click_button 'Comment & resolve thread' end page.within '.line-resolve-all-container' do - expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_content('1/1 thread resolved') expect(page).to have_selector('.line-resolve-btn.is-active') end end - it 'allows user to quickly scroll to next unresolved discussion' do + it 'allows user to quickly scroll to next unresolved thread' do page.within '.line-resolve-all-container' do page.find('.discussion-next-btn').click end @@ -231,7 +231,7 @@ describe 'Merge request > User resolves diff notes and discussions', :js do it 'hides jump to next button when all resolved' do page.within '.diff-content' do - click_button 'Resolve discussion' + click_button 'Resolve thread' end expect(page).to have_selector('.discussion-next-btn', visible: false) @@ -248,7 +248,7 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end end - it 'hides jump to next discussion button' do + it 'hides jump to next thread button' do page.within '.discussion-reply-holder' do expect(page).not_to have_selector('.discussion-next-btn') end @@ -261,7 +261,7 @@ describe 'Merge request > User resolves diff notes and discussions', :js do visit_merge_request end - it 'does not mark discussion as resolved when resolving single note' do + it 'does not mark thread as resolved when resolving single note' do page.within("#note_#{note.id}") do first('.line-resolve-btn').click @@ -273,11 +273,11 @@ describe 'Merge request > User resolves diff notes and discussions', :js do expect(page).to have_content('Last updated') page.within '.line-resolve-all-container' do - expect(page).to have_content('0/1 discussion resolved') + expect(page).to have_content('0/1 thread resolved') end end - it 'resolves discussion' do + it 'resolves thread' do resolve_buttons = page.all('.note .line-resolve-btn', count: 2) resolve_buttons.each do |button| button.click @@ -290,28 +290,28 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.within '.line-resolve-all-container' do - expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_content('1/1 thread resolved') end end end - context 'muliple discussions' do + context 'muliple threads' do before do create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) visit_merge_request end - it 'shows text with how many discussions' do + it 'shows text with how many threads' do page.within '.line-resolve-all-container' do - expect(page).to have_content('0/2 discussions resolved') + expect(page).to have_content('0/2 threads resolved') end end it 'allows user to mark a single note as resolved' do - click_button('Resolve discussion', match: :first) + click_button('Resolve thread', match: :first) page.within '.line-resolve-all-container' do - expect(page).to have_content('1/2 discussions resolved') + expect(page).to have_content('1/2 threads resolved') end end @@ -321,27 +321,27 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.within '.line-resolve-all-container' do - expect(page).to have_content('2/2 discussions resolved') + expect(page).to have_content('2/2 threads resolved') expect(page).to have_selector('.line-resolve-btn.is-active') end end - it 'allows user to mark all discussions as resolved' do + it 'allows user to mark all threads as resolved' do page.all('.discussion-reply-holder', count: 2).each do |reply_holder| page.within reply_holder do - click_button 'Resolve discussion' + click_button 'Resolve thread' end end page.within '.line-resolve-all-container' do - expect(page).to have_content('2/2 discussions resolved') + expect(page).to have_content('2/2 threads resolved') expect(page).to have_selector('.line-resolve-btn.is-active') end end - it 'allows user to quickly scroll to next unresolved discussion' do + it 'allows user to quickly scroll to next unresolved thread' do page.within('.discussion-reply-holder', match: :first) do - click_button 'Resolve discussion' + click_button 'Resolve thread' end page.within '.line-resolve-all-container' do @@ -362,25 +362,25 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end end - it 'shows jump to next discussion button on all discussions' do + it 'shows jump to next thread button except on last thread' do wait_for_requests all_discussion_replies = page.all('.discussion-reply-holder') expect(all_discussion_replies.count).to eq(2) - expect(all_discussion_replies.first.all('.discussion-next-btn').count).to eq(1) - expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(1) + expect(all_discussion_replies.first.all('.discussion-next-btn').count).to eq(2) + expect(all_discussion_replies.last.all('.discussion-next-btn').count).to eq(2) end - it 'displays next discussion even if hidden' do + it 'displays next thread even if hidden' do page.all('.note-discussion', count: 2).each do |discussion| page.within discussion do - click_button 'Toggle discussion' + click_button 'Toggle thread' end end page.within('.issuable-discussion #notes') do - expect(page).not_to have_selector('.btn', text: 'Resolve discussion') + expect(page).not_to have_selector('.btn', text: 'Resolve thread') end page.within '.line-resolve-all-container' do @@ -388,19 +388,19 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.all('.note-discussion').first do - expect(page.find('.discussion-with-resolve-btn')).to have_selector('.btn', text: 'Resolve discussion') + expect(page.find('.discussion-with-resolve-btn')).to have_selector('.btn', text: 'Resolve thread') end page.all('.note-discussion').last do - expect(page.find('.discussion-with-resolve-btn')).not.to have_selector('.btn', text: 'Resolve discussion') + expect(page.find('.discussion-with-resolve-btn')).not.to have_selector('.btn', text: 'Resolve thread') end end end context 'changes tab' do - it 'shows text with how many discussions' do + it 'shows text with how many threads' do page.within '.line-resolve-all-container' do - expect(page).to have_content('0/1 discussion resolved') + expect(page).to have_content('0/1 thread resolved') end end @@ -412,18 +412,18 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.within '.diff-content' do - expect(page).to have_selector('.btn', text: 'Unresolve discussion') + expect(page).to have_selector('.btn', text: 'Unresolve thread') end page.within '.line-resolve-all-container' do - expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_content('1/1 thread resolved') expect(page).to have_selector('.line-resolve-btn.is-active') end end - it 'allows user to mark discussion as resolved' do + it 'allows user to mark thread as resolved' do page.within '.diff-content' do - click_button 'Resolve discussion' + click_button 'Resolve thread' end page.within '.diff-content .note' do @@ -431,50 +431,50 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.within '.line-resolve-all-container' do - expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_content('1/1 thread resolved') expect(page).to have_selector('.line-resolve-btn.is-active') end end - it 'allows user to unresolve discussion' do + it 'allows user to unresolve thread' do page.within '.diff-content' do - click_button 'Resolve discussion' - click_button 'Unresolve discussion' + click_button 'Resolve thread' + click_button 'Unresolve thread' end page.within '.line-resolve-all-container' do - expect(page).to have_content('0/1 discussion resolved') + expect(page).to have_content('0/1 thread resolved') end end - it 'allows user to comment & resolve discussion' do + it 'allows user to comment & resolve thread' do page.within '.diff-content' do click_button 'Reply...' find('.js-note-text').set 'testing' - click_button 'Comment & resolve discussion' + click_button 'Comment & resolve thread' end page.within '.line-resolve-all-container' do - expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_content('1/1 thread resolved') expect(page).to have_selector('.line-resolve-btn.is-active') end end - it 'allows user to comment & unresolve discussion' do + it 'allows user to comment & unresolve thread' do page.within '.diff-content' do - click_button 'Resolve discussion' + click_button 'Resolve thread' click_button 'Reply...' find('.js-note-text').set 'testing' - click_button 'Comment & unresolve discussion' + click_button 'Comment & unresolve thread' end page.within '.line-resolve-all-container' do - expect(page).to have_content('0/1 discussion resolved') + expect(page).to have_content('0/1 thread resolved') end end end @@ -497,13 +497,13 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.within '.line-resolve-all-container' do - expect(page).to have_content('0/1 discussion resolved') + expect(page).to have_content('0/1 thread resolved') end end - it 'does not allow user to mark discussion as resolved' do + it 'does not allow user to mark thread as resolved' do page.within '.diff-content .note' do - expect(page).not_to have_selector('.btn', text: 'Resolve discussion') + expect(page).not_to have_selector('.btn', text: 'Resolve thread') end end end @@ -523,11 +523,11 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.within '.diff-content' do - expect(page).to have_selector('.btn', text: 'Unresolve discussion') + expect(page).to have_selector('.btn', text: 'Unresolve thread') end page.within '.line-resolve-all-container' do - expect(page).to have_content('1/1 discussion resolved') + expect(page).to have_content('1/1 thread resolved') expect(page).to have_selector('.line-resolve-btn.is-active') end end @@ -546,7 +546,7 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end page.within '.line-resolve-all-container' do - expect(page).to have_content('0/1 discussion resolved') + expect(page).to have_content('0/1 thread resolved') end end end @@ -558,15 +558,15 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end it 'shows resolved icon' do - expect(page).to have_content '1/1 discussion resolved' + expect(page).to have_content '1/1 thread resolved' - click_button 'Toggle discussion' + click_button 'Toggle thread' expect(page).to have_selector('.line-resolve-btn.is-active') end it 'does not allow user to click resolve button' do expect(page).to have_selector('.line-resolve-btn.is-disabled') - click_button 'Toggle discussion' + click_button 'Toggle thread' expect(page).to have_selector('.line-resolve-btn.is-disabled') end diff --git a/spec/features/merge_request/user_sees_discussions_spec.rb b/spec/features/merge_request/user_sees_discussions_spec.rb index 57be1d06708..39258b48295 100644 --- a/spec/features/merge_request/user_sees_discussions_spec.rb +++ b/spec/features/merge_request/user_sees_discussions_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe 'Merge request > User sees discussions', :js do +describe 'Merge request > User sees threads', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } let(:merge_request) { create(:merge_request, source_project: project) } @@ -30,7 +30,7 @@ describe 'Merge request > User sees discussions', :js do visit project_merge_request_path(project, merge_request) end - context 'active discussions' do + context 'active threads' do it 'shows a link to the diff' do within(".discussion[data-discussion-id='#{active_discussion.id}']") do path = diffs_project_merge_request_path(project, merge_request, anchor: active_discussion.line_code) @@ -39,7 +39,7 @@ describe 'Merge request > User sees discussions', :js do end end - context 'outdated discussions' do + context 'outdated threads' do it 'shows a link to the outdated diff' do within(".discussion[data-discussion-id='#{outdated_discussion.id}']") do path = diffs_project_merge_request_path(project, merge_request, diff_id: old_merge_request_diff.id, anchor: outdated_discussion.line_code) @@ -85,7 +85,7 @@ describe 'Merge request > User sees discussions', :js do it_behaves_like 'a functional discussion' it 'displays correct header' do - expect(page).to have_content "started a discussion on commit #{note.commit_id[0...7]}" + expect(page).to have_content "started a thread on commit #{note.commit_id[0...7]}" end end diff --git a/spec/features/merge_request/user_sees_merge_button_depending_on_unresolved_discussions_spec.rb b/spec/features/merge_request/user_sees_merge_button_depending_on_unresolved_discussions_spec.rb index f6b771facf8..cf398a7df18 100644 --- a/spec/features/merge_request/user_sees_merge_button_depending_on_unresolved_discussions_spec.rb +++ b/spec/features/merge_request/user_sees_merge_button_depending_on_unresolved_discussions_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -describe 'Merge request > User sees merge button depending on unresolved discussions', :js do +describe 'Merge request > User sees merge button depending on unresolved threads', :js do let(:project) { create(:project, :repository) } let(:user) { project.creator } let!(:merge_request) { create(:merge_request_with_diff_notes, source_project: project, author: user) } @@ -16,14 +16,14 @@ describe 'Merge request > User sees merge button depending on unresolved discuss visit project_merge_request_path(project, merge_request) end - context 'with unresolved discussions' do + context 'with unresolved threads' do it 'does not allow to merge' do expect(page).not_to have_button 'Merge' - expect(page).to have_content('There are unresolved discussions.') + expect(page).to have_content('There are unresolved threads.') end end - context 'with all discussions resolved' do + context 'with all threads resolved' do before do merge_request.discussions.each { |d| d.resolve!(user) } visit project_merge_request_path(project, merge_request) @@ -41,13 +41,13 @@ describe 'Merge request > User sees merge button depending on unresolved discuss visit project_merge_request_path(project, merge_request) end - context 'with unresolved discussions' do + context 'with unresolved threads' do it 'does not allow to merge' do expect(page).to have_button 'Merge' end end - context 'with all discussions resolved' do + context 'with all threads resolved' do before do merge_request.discussions.each { |d| d.resolve!(user) } visit project_merge_request_path(project, merge_request) diff --git a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb index e8b4fc8f160..4363b359038 100644 --- a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb +++ b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb @@ -186,12 +186,12 @@ describe 'User comments on a diff', :js do it 'resolves discussion when applied' do page.within('.diff-discussions') do - expect(page).not_to have_content('Unresolve discussion') + expect(page).not_to have_content('Unresolve thread') click_button('Apply suggestion') wait_for_requests - expect(page).to have_content('Unresolve discussion') + expect(page).to have_content('Unresolve thread') end end end diff --git a/spec/features/projects/releases/user_views_releases_spec.rb b/spec/features/projects/releases/user_views_releases_spec.rb index 317ffb6a2ff..725d7173bce 100644 --- a/spec/features/projects/releases/user_views_releases_spec.rb +++ b/spec/features/projects/releases/user_views_releases_spec.rb @@ -16,6 +16,7 @@ describe 'User views releases', :js do expect(page).to have_content(release.name) expect(page).to have_content(release.tag) + expect(page).not_to have_content('Upcoming Release') end context 'when there is a link as an asset' do @@ -43,4 +44,15 @@ describe 'User views releases', :js do end end end + + context 'with an upcoming release' do + let(:tomorrow) { Time.zone.now + 1.day } + let!(:release) { create(:release, project: project, released_at: tomorrow ) } + + it 'sees the upcoming tag' do + visit project_releases_path(project) + + expect(page).to have_content('Upcoming Release') + end + end end diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb index 8c7bc192c50..1edfee705c8 100644 --- a/spec/features/projects/settings/repository_settings_spec.rb +++ b/spec/features/projects/settings/repository_settings_spec.rb @@ -112,11 +112,17 @@ describe 'Projects > Settings > Repository settings' do it 'add a new deploy token' do fill_in 'deploy_token_name', with: 'new_deploy_key' fill_in 'deploy_token_expires_at', with: (Date.today + 1.month).to_s + fill_in 'deploy_token_username', with: 'deployer' check 'deploy_token_read_repository' check 'deploy_token_read_registry' click_button 'Create deploy token' expect(page).to have_content('Your new project deploy token has been created') + + within('.created-deploy-token-container') do + expect(page).to have_selector("input[name='deploy-token-user'][value='deployer']") + expect(page).to have_selector("input[name='deploy-token'][readonly='readonly']") + end end end diff --git a/spec/features/snippets/user_creates_snippet_spec.rb b/spec/features/snippets/user_creates_snippet_spec.rb index 1c97d5ec5b4..93d77d5b5ce 100644 --- a/spec/features/snippets/user_creates_snippet_spec.rb +++ b/spec/features/snippets/user_creates_snippet_spec.rb @@ -41,7 +41,7 @@ describe 'User creates snippet', :js do expect(page).to have_content('My Snippet') link = find('a.no-attachment-icon img[alt="banana_sample"]')['src'] - expect(link).to match(%r{/uploads/-/system/temp/\h{32}/banana_sample\.gif\z}) + expect(link).to match(%r{/uploads/-/system/user/#{user.id}/\h{32}/banana_sample\.gif\z}) reqs = inspect_requests { visit(link) } expect(reqs.first.status_code).to eq(200) diff --git a/spec/finders/releases_finder_spec.rb b/spec/finders/releases_finder_spec.rb index 32ee15134a2..5ffb8c74bf5 100644 --- a/spec/finders/releases_finder_spec.rb +++ b/spec/finders/releases_finder_spec.rb @@ -12,8 +12,8 @@ describe ReleasesFinder do subject { described_class.new(project, user)} before do - v1_0_0.update_attribute(:created_at, 2.days.ago) - v1_1_0.update_attribute(:created_at, 1.day.ago) + v1_0_0.update_attribute(:released_at, 2.days.ago) + v1_1_0.update_attribute(:released_at, 1.day.ago) end describe '#execute' do @@ -30,7 +30,7 @@ describe ReleasesFinder do project.add_developer(user) end - it 'sorts by creation date' do + it 'sorts by release date' do releases = subject.execute expect(releases).to be_present diff --git a/spec/fixtures/api/schemas/public_api/v4/release.json b/spec/fixtures/api/schemas/public_api/v4/release.json index 6ea0781c1ed..ec3fa59cdb1 100644 --- a/spec/fixtures/api/schemas/public_api/v4/release.json +++ b/spec/fixtures/api/schemas/public_api/v4/release.json @@ -1,12 +1,14 @@ { "type": "object", - "required": ["name", "tag_name", "commit"], + "required": ["name", "tag_name", "commit", "released_at"], "properties": { "name": { "type": "string" }, "tag_name": { "type": "string" }, "description": { "type": "string" }, "description_html": { "type": "string" }, "created_at": { "type": "date" }, + "released_at": { "type": "date" }, + "upcoming_release": { "type": "boolean" }, "commit": { "oneOf": [{ "type": "null" }, { "$ref": "commit/basic.json" }] }, diff --git a/spec/fixtures/api/schemas/public_api/v4/release/release_for_guest.json b/spec/fixtures/api/schemas/public_api/v4/release/release_for_guest.json index e78398ad1d5..0c1e8fd5fb3 100644 --- a/spec/fixtures/api/schemas/public_api/v4/release/release_for_guest.json +++ b/spec/fixtures/api/schemas/public_api/v4/release/release_for_guest.json @@ -1,11 +1,13 @@ { "type": "object", - "required": ["name"], + "required": ["name", "released_at"], "properties": { "name": { "type": "string" }, "description": { "type": "string" }, "description_html": { "type": "string" }, "created_at": { "type": "date" }, + "released_at": { "type": "date" }, + "upcoming_release": { "type": "boolean" }, "author": { "oneOf": [{ "type": "null" }, { "$ref": "../user/basic.json" }] }, diff --git a/spec/frontend/diffs/components/diff_discussion_reply_spec.js b/spec/frontend/diffs/components/diff_discussion_reply_spec.js new file mode 100644 index 00000000000..28689ab07de --- /dev/null +++ b/spec/frontend/diffs/components/diff_discussion_reply_spec.js @@ -0,0 +1,90 @@ +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import Vuex from 'vuex'; +import DiffDiscussionReply from '~/diffs/components/diff_discussion_reply.vue'; +import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue'; +import NoteSignedOutWidget from '~/notes/components/note_signed_out_widget.vue'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('DiffDiscussionReply', () => { + let wrapper; + let getters; + let store; + + const createComponent = (props = {}, slots = {}) => { + wrapper = shallowMount(DiffDiscussionReply, { + store, + localVue, + sync: false, + propsData: { + ...props, + }, + slots: { + ...slots, + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + describe('if user can reply', () => { + beforeEach(() => { + getters = { + userCanReply: () => true, + getUserData: () => ({ + path: 'test-path', + avatar_url: 'avatar_url', + name: 'John Doe', + }), + }; + + store = new Vuex.Store({ + getters, + }); + }); + + it('should render a form if component has form', () => { + createComponent( + { + renderReplyPlaceholder: false, + hasForm: true, + }, + { + form: `<div id="test-form"></div>`, + }, + ); + + expect(wrapper.find('#test-form').exists()).toBe(true); + }); + + it('should render a reply placeholder if there is no form', () => { + createComponent({ + renderReplyPlaceholder: true, + hasForm: false, + }); + + expect(wrapper.find(ReplyPlaceholder).exists()).toBe(true); + }); + }); + + it('renders a signed out widget when user is not logged in', () => { + getters = { + userCanReply: () => false, + getUserData: () => null, + }; + + store = new Vuex.Store({ + getters, + }); + + createComponent({ + renderReplyPlaceholder: false, + hasForm: false, + }); + + expect(wrapper.find(NoteSignedOutWidget).exists()).toBe(true); + }); +}); diff --git a/spec/frontend/diffs/components/diff_gutter_avatars_spec.js b/spec/frontend/diffs/components/diff_gutter_avatars_spec.js new file mode 100644 index 00000000000..48ee5c63f35 --- /dev/null +++ b/spec/frontend/diffs/components/diff_gutter_avatars_spec.js @@ -0,0 +1,113 @@ +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import DiffGutterAvatars from '~/diffs/components/diff_gutter_avatars.vue'; +import discussionsMockData from '../mock_data/diff_discussions'; + +const localVue = createLocalVue(); +const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; + +describe('DiffGutterAvatars', () => { + let wrapper; + + const findCollapseButton = () => wrapper.find('.diff-notes-collapse'); + const findMoreCount = () => wrapper.find('.diff-comments-more-count'); + const findUserAvatars = () => wrapper.findAll('.diff-comment-avatar'); + + const createComponent = (props = {}) => { + wrapper = shallowMount(DiffGutterAvatars, { + localVue, + sync: false, + propsData: { + ...props, + }, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + describe('when expanded', () => { + beforeEach(() => { + createComponent({ + discussions: getDiscussionsMockData(), + discussionsExpanded: true, + }); + }); + + it('renders a collapse button when discussions are expanded', () => { + expect(findCollapseButton().exists()).toBe(true); + }); + + it('should emit toggleDiscussions event on button click', () => { + findCollapseButton().trigger('click'); + + expect(wrapper.emitted().toggleLineDiscussions).toBeTruthy(); + }); + }); + + describe('when collapsed', () => { + beforeEach(() => { + createComponent({ + discussions: getDiscussionsMockData(), + discussionsExpanded: false, + }); + }); + + it('renders user avatars and moreCount text', () => { + expect(findUserAvatars().exists()).toBe(true); + expect(findMoreCount().exists()).toBe(true); + }); + + it('renders correct amount of user avatars', () => { + expect(findUserAvatars().length).toBe(3); + }); + + it('renders correct moreCount number', () => { + expect(findMoreCount().text()).toBe('+2'); + }); + + it('should emit toggleDiscussions event on avatars click', () => { + findUserAvatars() + .at(0) + .trigger('click'); + + expect(wrapper.emitted().toggleLineDiscussions).toBeTruthy(); + }); + + it('should emit toggleDiscussions event on more count text click', () => { + findMoreCount().trigger('click'); + + expect(wrapper.emitted().toggleLineDiscussions).toBeTruthy(); + }); + }); + + it('renders an empty more count string if there are no discussions', () => { + createComponent({ + discussions: [], + discussionsExpanded: false, + }); + + expect(findMoreCount().exists()).toBe(false); + }); + + describe('tooltip text', () => { + beforeEach(() => { + createComponent({ + discussions: getDiscussionsMockData(), + discussionsExpanded: false, + }); + }); + + it('returns original comment if it is shorter than max length', () => { + const note = wrapper.vm.discussions[0].notes[0]; + + expect(wrapper.vm.getTooltipText(note)).toEqual('Administrator: comment 1'); + }); + + it('returns truncated version of comment if it is longer than max length', () => { + const note = wrapper.vm.discussions[0].notes[1]; + + expect(wrapper.vm.getTooltipText(note)).toEqual('Fatih Acet: comment 2 is r...'); + }); + }); +}); diff --git a/spec/frontend/diffs/mock_data/diff_discussions.js b/spec/frontend/diffs/mock_data/diff_discussions.js new file mode 100644 index 00000000000..711ab543411 --- /dev/null +++ b/spec/frontend/diffs/mock_data/diff_discussions.js @@ -0,0 +1,529 @@ +export default { + id: '6b232e05bea388c6b043ccc243ba505faac04ea8', + reply_id: '6b232e05bea388c6b043ccc243ba505faac04ea8', + position: { + old_line: null, + new_line: 2, + old_path: 'CHANGELOG', + new_path: 'CHANGELOG', + base_sha: 'e63f41fe459e62e1228fcef60d7189127aeba95a', + start_sha: 'd9eaefe5a676b820c57ff18cf5b68316025f7962', + head_sha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13', + }, + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2', + expanded: true, + notes: [ + { + id: '1749', + type: 'DiffNote', + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-04-03T21:06:21.521Z', + updated_at: '2018-04-08T08:50:41.762Z', + system: false, + noteable_id: 51, + noteable_type: 'MergeRequest', + noteable_iid: 20, + human_access: 'Owner', + note: 'comment 1', + note_html: '<p dir="auto">comment 1</p>', + last_edited_at: '2018-04-08T08:50:41.762Z', + last_edited_by: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + current_user: { + can_edit: true, + can_award_emoji: true, + }, + resolved: false, + resolvable: true, + resolved_by: null, + discussion_id: '6b232e05bea388c6b043ccc243ba505faac04ea8', + emoji_awardable: true, + award_emoji: [], + toggle_award_path: '/gitlab-org/gitlab-test/notes/1749/toggle_award_emoji', + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-test%2Fmerge_requests%2F20%23note_1749&user_id=1', + path: '/gitlab-org/gitlab-test/notes/1749', + noteable_note_url: 'http://localhost:3000/gitlab-org/gitlab-test/merge_requests/20#note_1749', + resolve_path: + '/gitlab-org/gitlab-test/merge_requests/20/discussions/6b232e05bea388c6b043ccc243ba505faac04ea8/resolve', + resolve_with_issue_path: + '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', + }, + { + id: '1753', + type: 'DiffNote', + attachment: null, + author: { + id: 1, + name: 'Fatih Acet', + username: 'fatihacet', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/fatihacevt', + }, + created_at: '2018-04-08T08:49:35.804Z', + updated_at: '2018-04-08T08:50:45.915Z', + system: false, + noteable_id: 51, + noteable_type: 'MergeRequest', + noteable_iid: 20, + human_access: 'Owner', + note: 'comment 2 is really long one', + note_html: '<p dir="auto">comment 2 is really long one</p>', + last_edited_at: '2018-04-08T08:50:45.915Z', + last_edited_by: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + current_user: { + can_edit: true, + can_award_emoji: true, + }, + resolved: false, + resolvable: true, + resolved_by: null, + discussion_id: '6b232e05bea388c6b043ccc243ba505faac04ea8', + emoji_awardable: true, + award_emoji: [], + toggle_award_path: '/gitlab-org/gitlab-test/notes/1753/toggle_award_emoji', + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-test%2Fmerge_requests%2F20%23note_1753&user_id=1', + path: '/gitlab-org/gitlab-test/notes/1753', + noteable_note_url: 'http://localhost:3000/gitlab-org/gitlab-test/merge_requests/20#note_1753', + resolve_path: + '/gitlab-org/gitlab-test/merge_requests/20/discussions/6b232e05bea388c6b043ccc243ba505faac04ea8/resolve', + resolve_with_issue_path: + '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', + }, + { + id: '1754', + type: 'DiffNote', + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-04-08T08:50:48.294Z', + updated_at: '2018-04-08T08:50:48.294Z', + system: false, + noteable_id: 51, + noteable_type: 'MergeRequest', + noteable_iid: 20, + human_access: 'Owner', + note: 'comment 3', + note_html: '<p dir="auto">comment 3</p>', + current_user: { + can_edit: true, + can_award_emoji: true, + }, + resolved: false, + resolvable: true, + resolved_by: null, + discussion_id: '6b232e05bea388c6b043ccc243ba505faac04ea8', + emoji_awardable: true, + award_emoji: [], + toggle_award_path: '/gitlab-org/gitlab-test/notes/1754/toggle_award_emoji', + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-test%2Fmerge_requests%2F20%23note_1754&user_id=1', + path: '/gitlab-org/gitlab-test/notes/1754', + noteable_note_url: 'http://localhost:3000/gitlab-org/gitlab-test/merge_requests/20#note_1754', + resolve_path: + '/gitlab-org/gitlab-test/merge_requests/20/discussions/6b232e05bea388c6b043ccc243ba505faac04ea8/resolve', + resolve_with_issue_path: + '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', + }, + { + id: '1755', + type: 'DiffNote', + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-04-08T08:50:50.911Z', + updated_at: '2018-04-08T08:50:50.911Z', + system: false, + noteable_id: 51, + noteable_type: 'MergeRequest', + noteable_iid: 20, + human_access: 'Owner', + note: 'comment 4', + note_html: '<p dir="auto">comment 4</p>', + current_user: { + can_edit: true, + can_award_emoji: true, + }, + resolved: false, + resolvable: true, + resolved_by: null, + discussion_id: '6b232e05bea388c6b043ccc243ba505faac04ea8', + emoji_awardable: true, + award_emoji: [], + toggle_award_path: '/gitlab-org/gitlab-test/notes/1755/toggle_award_emoji', + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-test%2Fmerge_requests%2F20%23note_1755&user_id=1', + path: '/gitlab-org/gitlab-test/notes/1755', + noteable_note_url: 'http://localhost:3000/gitlab-org/gitlab-test/merge_requests/20#note_1755', + resolve_path: + '/gitlab-org/gitlab-test/merge_requests/20/discussions/6b232e05bea388c6b043ccc243ba505faac04ea8/resolve', + resolve_with_issue_path: + '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', + }, + { + id: '1756', + type: 'DiffNote', + attachment: null, + author: { + id: 1, + name: 'Administrator', + username: 'root', + state: 'active', + avatar_url: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + path: '/root', + }, + created_at: '2018-04-08T08:50:53.895Z', + updated_at: '2018-04-08T08:50:53.895Z', + system: false, + noteable_id: 51, + noteable_type: 'MergeRequest', + noteable_iid: 20, + human_access: 'Owner', + note: 'comment 5', + note_html: '<p dir="auto">comment 5</p>', + current_user: { + can_edit: true, + can_award_emoji: true, + }, + resolved: false, + resolvable: true, + resolved_by: null, + discussion_id: '6b232e05bea388c6b043ccc243ba505faac04ea8', + emoji_awardable: true, + award_emoji: [], + toggle_award_path: '/gitlab-org/gitlab-test/notes/1756/toggle_award_emoji', + report_abuse_path: + '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-test%2Fmerge_requests%2F20%23note_1756&user_id=1', + path: '/gitlab-org/gitlab-test/notes/1756', + noteable_note_url: 'http://localhost:3000/gitlab-org/gitlab-test/merge_requests/20#note_1756', + resolve_path: + '/gitlab-org/gitlab-test/merge_requests/20/discussions/6b232e05bea388c6b043ccc243ba505faac04ea8/resolve', + resolve_with_issue_path: + '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', + }, + ], + individual_note: false, + resolvable: true, + resolved: false, + resolve_path: + '/gitlab-org/gitlab-test/merge_requests/20/discussions/6b232e05bea388c6b043ccc243ba505faac04ea8/resolve', + resolve_with_issue_path: + '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', + diff_file: { + submodule: false, + submodule_link: null, + blob: { + id: '9e10516ca50788acf18c518a231914a21e5f16f7', + path: 'CHANGELOG', + name: 'CHANGELOG', + mode: '100644', + readable_text: true, + icon: 'file-text-o', + }, + blob_path: 'CHANGELOG', + blob_name: 'CHANGELOG', + blob_icon: '<i aria-hidden="true" data-hidden="true" class="fa fa-file-text-o fa-fw"></i>', + file_hash: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a', + file_path: 'CHANGELOG.rb', + new_file: false, + deleted_file: false, + renamed_file: false, + old_path: 'CHANGELOG', + new_path: 'CHANGELOG', + mode_changed: false, + a_mode: '100644', + b_mode: '100644', + text: true, + added_lines: 2, + removed_lines: 0, + diff_refs: { + base_sha: 'e63f41fe459e62e1228fcef60d7189127aeba95a', + start_sha: 'd9eaefe5a676b820c57ff18cf5b68316025f7962', + head_sha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13', + }, + content_sha: 'c48ee0d1bf3b30453f5b32250ce03134beaa6d13', + stored_externally: null, + external_storage: null, + old_path_html: 'CHANGELOG_OLD', + new_path_html: 'CHANGELOG', + is_fully_expanded: true, + context_lines_path: + '/gitlab-org/gitlab-test/blob/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG/diff', + highlighted_diff_lines: [ + { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1', + type: 'new', + old_line: null, + new_line: 1, + text: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', + rich_text: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', + meta_data: null, + }, + { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2', + type: 'new', + old_line: null, + new_line: 2, + text: '<span id="LC2" class="line" lang="plaintext"></span>\n', + rich_text: '<span id="LC2" class="line" lang="plaintext"></span>\n', + meta_data: null, + }, + { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', + type: null, + old_line: 1, + new_line: 3, + text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', + rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', + meta_data: null, + }, + { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', + type: null, + old_line: 2, + new_line: 4, + text: '<span id="LC4" class="line" lang="plaintext"></span>\n', + rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n', + meta_data: null, + }, + { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', + type: null, + old_line: 3, + new_line: 5, + text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', + rich_text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', + meta_data: null, + }, + { + line_code: null, + type: 'match', + old_line: null, + new_line: null, + text: '', + rich_text: '', + meta_data: { + old_pos: 3, + new_pos: 5, + }, + }, + { + line_code: null, + type: 'match', + old_line: null, + new_line: null, + text: '', + rich_text: '', + meta_data: { + old_pos: 3, + new_pos: 5, + }, + }, + { + line_code: null, + type: 'match', + old_line: null, + new_line: null, + text: '', + rich_text: '', + meta_data: { + old_pos: 3, + new_pos: 5, + }, + }, + ], + parallel_diff_lines: [ + { + left: null, + right: { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1', + type: 'new', + old_line: null, + new_line: 1, + text: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', + rich_text: '<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', + meta_data: null, + }, + }, + { + left: null, + right: { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_2', + type: 'new', + old_line: null, + new_line: 2, + text: '<span id="LC2" class="line" lang="plaintext"></span>\n', + rich_text: '<span id="LC2" class="line" lang="plaintext"></span>\n', + meta_data: null, + }, + }, + { + left: { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', + type: null, + old_line: 1, + new_line: 3, + text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', + rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', + meta_data: null, + }, + right: { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_3', + type: null, + old_line: 1, + new_line: 3, + text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', + rich_text: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n', + meta_data: null, + }, + }, + { + left: { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', + type: null, + old_line: 2, + new_line: 4, + text: '<span id="LC4" class="line" lang="plaintext"></span>\n', + rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n', + meta_data: null, + }, + right: { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4', + type: null, + old_line: 2, + new_line: 4, + text: '<span id="LC4" class="line" lang="plaintext"></span>\n', + rich_text: '<span id="LC4" class="line" lang="plaintext"></span>\n', + meta_data: null, + }, + }, + { + left: { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', + type: null, + old_line: 3, + new_line: 5, + text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', + rich_text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', + meta_data: null, + }, + right: { + line_code: '1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_3_5', + type: null, + old_line: 3, + new_line: 5, + text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', + rich_text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n', + meta_data: null, + }, + }, + { + left: { + line_code: null, + type: 'match', + old_line: null, + new_line: null, + text: '', + rich_text: '', + meta_data: { + old_pos: 3, + new_pos: 5, + }, + }, + right: { + line_code: null, + type: 'match', + old_line: null, + new_line: null, + text: '', + rich_text: '', + meta_data: { + old_pos: 3, + new_pos: 5, + }, + }, + }, + ], + viewer: { + name: 'text', + error: null, + }, + }, + diff_discussion: true, + truncated_diff_lines: [ + { + text: 'line', + rich_text: + '<tr class="line_holder new" id="">\n<td class="diff-line-num new old_line" data-linenumber="1">\n \n</td>\n<td class="diff-line-num new new_line" data-linenumber="1">\n1\n</td>\n<td class="line_content new"><span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n</td>\n</tr>\n<tr class="line_holder new" id="">\n<td class="diff-line-num new old_line" data-linenumber="1">\n \n</td>\n<td class="diff-line-num new new_line" data-linenumber="2">\n2\n</td>\n<td class="line_content new"><span id="LC2" class="line" lang="plaintext"></span>\n</td>\n</tr>\n', + can_receive_suggestion: true, + line_code: '6f209374f7e565f771b95720abf46024c41d1885_1_1', + type: 'new', + old_line: null, + new_line: 1, + meta_data: null, + }, + ], +}; + +export const imageDiffDiscussions = [ + { + id: '1', + position: { + x: 10, + y: 10, + width: 100, + height: 200, + }, + }, + { + id: '2', + position: { + x: 5, + y: 5, + width: 100, + height: 200, + }, + }, +]; diff --git a/spec/javascripts/error_tracking_settings/components/app_spec.js b/spec/frontend/error_tracking_settings/components/app_spec.js index 2e52a45fd34..022f12ef191 100644 --- a/spec/javascripts/error_tracking_settings/components/app_spec.js +++ b/spec/frontend/error_tracking_settings/components/app_spec.js @@ -4,7 +4,7 @@ import ErrorTrackingSettings from '~/error_tracking_settings/components/app.vue' import ErrorTrackingForm from '~/error_tracking_settings/components/error_tracking_form.vue'; import ProjectDropdown from '~/error_tracking_settings/components/project_dropdown.vue'; import createStore from '~/error_tracking_settings/store'; -import { TEST_HOST } from 'spec/test_constants'; +import { TEST_HOST } from 'helpers/test_constants'; const localVue = createLocalVue(); localVue.use(Vuex); diff --git a/spec/javascripts/error_tracking_settings/components/error_tracking_form_spec.js b/spec/frontend/error_tracking_settings/components/error_tracking_form_spec.js index 23e57c4bbf1..23e57c4bbf1 100644 --- a/spec/javascripts/error_tracking_settings/components/error_tracking_form_spec.js +++ b/spec/frontend/error_tracking_settings/components/error_tracking_form_spec.js diff --git a/spec/javascripts/error_tracking_settings/components/project_dropdown_spec.js b/spec/frontend/error_tracking_settings/components/project_dropdown_spec.js index 8e5dbe28452..8e5dbe28452 100644 --- a/spec/javascripts/error_tracking_settings/components/project_dropdown_spec.js +++ b/spec/frontend/error_tracking_settings/components/project_dropdown_spec.js diff --git a/spec/javascripts/error_tracking_settings/mock.js b/spec/frontend/error_tracking_settings/mock.js index 32cdba33c14..42233f82d54 100644 --- a/spec/javascripts/error_tracking_settings/mock.js +++ b/spec/frontend/error_tracking_settings/mock.js @@ -1,5 +1,5 @@ import createStore from '~/error_tracking_settings/store'; -import { TEST_HOST } from 'spec/test_constants'; +import { TEST_HOST } from '../helpers/test_constants'; const defaultStore = createStore(); diff --git a/spec/javascripts/error_tracking_settings/store/actions_spec.js b/spec/frontend/error_tracking_settings/store/actions_spec.js index 0255b3a7aa4..1eab0f7470b 100644 --- a/spec/javascripts/error_tracking_settings/store/actions_spec.js +++ b/spec/frontend/error_tracking_settings/store/actions_spec.js @@ -1,13 +1,16 @@ import MockAdapter from 'axios-mock-adapter'; -import testAction from 'spec/helpers/vuex_action_helper'; -import { TEST_HOST } from 'spec/test_constants'; +import testAction from 'helpers/vuex_action_helper'; +import { TEST_HOST } from 'helpers/test_constants'; import axios from '~/lib/utils/axios_utils'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; -import actionsDefaultExport, * as actions from '~/error_tracking_settings/store/actions'; +import { refreshCurrentPage } from '~/lib/utils/url_utility'; +import * as actions from '~/error_tracking_settings/store/actions'; import * as types from '~/error_tracking_settings/store/mutation_types'; import defaultState from '~/error_tracking_settings/store/state'; import { projectList } from '../mock'; +jest.mock('~/lib/utils/url_utility'); + describe('error tracking settings actions', () => { let state; @@ -21,6 +24,7 @@ describe('error tracking settings actions', () => { afterEach(() => { mock.restore(); + refreshCurrentPage.mockClear(); }); it('should request and transform the project list', done => { @@ -111,7 +115,6 @@ describe('error tracking settings actions', () => { }); it('should save the page', done => { - const refreshCurrentPage = spyOnDependency(actionsDefaultExport, 'refreshCurrentPage'); mock.onPatch(TEST_HOST).reply(200); testAction(actions.updateSettings, null, state, [], [{ type: 'requestSettings' }], () => { expect(mock.history.patch.length).toBe(1); diff --git a/spec/javascripts/error_tracking_settings/store/getters_spec.js b/spec/frontend/error_tracking_settings/store/getters_spec.js index 2c5ff084b8a..2c5ff084b8a 100644 --- a/spec/javascripts/error_tracking_settings/store/getters_spec.js +++ b/spec/frontend/error_tracking_settings/store/getters_spec.js diff --git a/spec/javascripts/error_tracking_settings/store/mutation_spec.js b/spec/frontend/error_tracking_settings/store/mutation_spec.js index bb1f1da784e..fa188462c3f 100644 --- a/spec/javascripts/error_tracking_settings/store/mutation_spec.js +++ b/spec/frontend/error_tracking_settings/store/mutation_spec.js @@ -1,4 +1,4 @@ -import { TEST_HOST } from 'spec/test_constants'; +import { TEST_HOST } from 'helpers/test_constants'; import mutations from '~/error_tracking_settings/store/mutations'; import defaultState from '~/error_tracking_settings/store/state'; import * as types from '~/error_tracking_settings/store/mutation_types'; diff --git a/spec/javascripts/error_tracking_settings/utils_spec.js b/spec/frontend/error_tracking_settings/utils_spec.js index 4b144f7daf1..4b144f7daf1 100644 --- a/spec/javascripts/error_tracking_settings/utils_spec.js +++ b/spec/frontend/error_tracking_settings/utils_spec.js diff --git a/spec/frontend/lib/utils/text_utility_spec.js b/spec/frontend/lib/utils/text_utility_spec.js index 9e920d59093..dc886d0db3b 100644 --- a/spec/frontend/lib/utils/text_utility_spec.js +++ b/spec/frontend/lib/utils/text_utility_spec.js @@ -55,9 +55,24 @@ describe('text_utility', () => { }); }); - describe('slugifyWithHyphens', () => { + describe('slugify', () => { + it('should remove accents and convert to lower case', () => { + expect(textUtils.slugify('João')).toEqual('jo-o'); + }); it('should replaces whitespaces with hyphens and convert to lower case', () => { - expect(textUtils.slugifyWithHyphens('My Input String')).toEqual('my-input-string'); + expect(textUtils.slugify('My Input String')).toEqual('my-input-string'); + }); + it('should remove trailing whitespace and replace whitespaces within string with a hyphen', () => { + expect(textUtils.slugify(' a new project ')).toEqual('a-new-project'); + }); + it('should only remove non-allowed special characters', () => { + expect(textUtils.slugify('test!_pro-ject~')).toEqual('test-_pro-ject-'); + }); + it('should squash multiple hypens', () => { + expect(textUtils.slugify('test!!!!_pro-ject~')).toEqual('test-_pro-ject-'); + }); + it('should return empty string if only non-allowed characters', () => { + expect(textUtils.slugify('здрасти')).toEqual(''); }); }); diff --git a/spec/frontend/notes/components/discussion_notes_spec.js b/spec/frontend/notes/components/discussion_notes_spec.js index 394666403ee..58d367077e8 100644 --- a/spec/frontend/notes/components/discussion_notes_spec.js +++ b/spec/frontend/notes/components/discussion_notes_spec.js @@ -87,7 +87,7 @@ describe('DiscussionNotes', () => { discussion.notes[0], ]; discussion.notes = notesData; - createComponent({ discussion }); + createComponent({ discussion, shouldRenderDiffs: true }); const notes = wrapper.findAll('.notes > li'); expect(notes.at(0).is(PlaceholderSystemNote)).toBe(true); diff --git a/spec/frontend/notes/components/discussion_reply_placeholder_spec.js b/spec/frontend/notes/components/discussion_reply_placeholder_spec.js index 07a366cf339..e008f4ed093 100644 --- a/spec/frontend/notes/components/discussion_reply_placeholder_spec.js +++ b/spec/frontend/notes/components/discussion_reply_placeholder_spec.js @@ -2,13 +2,19 @@ import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vu import { shallowMount, createLocalVue } from '@vue/test-utils'; const localVue = createLocalVue(); +const buttonText = 'Test Button Text'; describe('ReplyPlaceholder', () => { let wrapper; + const findButton = () => wrapper.find({ ref: 'button' }); + beforeEach(() => { wrapper = shallowMount(ReplyPlaceholder, { localVue, + propsData: { + buttonText, + }, }); }); @@ -17,9 +23,7 @@ describe('ReplyPlaceholder', () => { }); it('emits onClick even on button click', () => { - const button = wrapper.find({ ref: 'button' }); - - button.trigger('click'); + findButton().trigger('click'); expect(wrapper.emitted()).toEqual({ onClick: [[]], @@ -27,8 +31,6 @@ describe('ReplyPlaceholder', () => { }); it('should render reply button', () => { - const button = wrapper.find({ ref: 'button' }); - - expect(button.text()).toEqual('Reply...'); + expect(findButton().text()).toEqual(buttonText); }); }); diff --git a/spec/javascripts/vue_shared/components/markdown/header_spec.js b/spec/frontend/vue_shared/components/markdown/header_spec.js index af92e5f5ae2..aa0b544f948 100644 --- a/spec/javascripts/vue_shared/components/markdown/header_spec.js +++ b/spec/frontend/vue_shared/components/markdown/header_spec.js @@ -53,7 +53,7 @@ describe('Markdown field header component', () => { }); it('emits toggle markdown event when clicking preview', () => { - spyOn(vm, '$emit'); + jest.spyOn(vm, '$emit').mockImplementation(); vm.$el.querySelector('.js-preview-link').click(); @@ -65,7 +65,7 @@ describe('Markdown field header component', () => { }); it('does not emit toggle markdown event when triggered from another form', () => { - spyOn(vm, '$emit'); + jest.spyOn(vm, '$emit').mockImplementation(); $(document).triggerHandler('markdown-preview:show', [ $( @@ -76,17 +76,13 @@ describe('Markdown field header component', () => { expect(vm.$emit).not.toHaveBeenCalled(); }); - it('blurs preview link after click', done => { + it('blurs preview link after click', () => { const link = vm.$el.querySelector('li:nth-child(2) button'); - spyOn(HTMLElement.prototype, 'blur'); + jest.spyOn(HTMLElement.prototype, 'blur').mockImplementation(); link.click(); - setTimeout(() => { - expect(link.blur).toHaveBeenCalled(); - - done(); - }); + expect(link.blur).toHaveBeenCalled(); }); it('renders markdown table template', () => { diff --git a/spec/graphql/types/label_type_spec.rb b/spec/graphql/types/label_type_spec.rb index f498b32f9ed..8e7b2c69eff 100644 --- a/spec/graphql/types/label_type_spec.rb +++ b/spec/graphql/types/label_type_spec.rb @@ -7,4 +7,6 @@ describe GitlabSchema.types['Label'] do is_expected.to have_graphql_fields(*expected_fields) end + + it { is_expected.to require_graphql_authorizations(:read_label) } end diff --git a/spec/graphql/types/metadata_type_spec.rb b/spec/graphql/types/metadata_type_spec.rb index 55205bf5b6a..5236380e477 100644 --- a/spec/graphql/types/metadata_type_spec.rb +++ b/spec/graphql/types/metadata_type_spec.rb @@ -2,4 +2,5 @@ require 'spec_helper' describe GitlabSchema.types['Metadata'] do it { expect(described_class.graphql_name).to eq('Metadata') } + it { is_expected.to require_graphql_authorizations(:read_instance_metadata) } end diff --git a/spec/graphql/types/namespace_type_spec.rb b/spec/graphql/types/namespace_type_spec.rb index 77fd590586e..e1153832cc9 100644 --- a/spec/graphql/types/namespace_type_spec.rb +++ b/spec/graphql/types/namespace_type_spec.rb @@ -13,4 +13,6 @@ describe GitlabSchema.types['Namespace'] do is_expected.to have_graphql_fields(*expected_fields) end + + it { is_expected.to require_graphql_authorizations(:read_namespace) } end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb index af1972a2513..bc3b8a42392 100644 --- a/spec/graphql/types/query_type_spec.rb +++ b/spec/graphql/types/query_type_spec.rb @@ -34,9 +34,5 @@ describe GitlabSchema.types['Query'] do is_expected.to have_graphql_type(Types::MetadataType) is_expected.to have_graphql_resolver(Resolvers::MetadataResolver) end - - it 'authorizes with read_instance_metadata' do - is_expected.to require_graphql_authorizations(:read_instance_metadata) - end end end diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js index 596a1ba5ad2..d4280d3ec2c 100644 --- a/spec/javascripts/diffs/components/diff_file_header_spec.js +++ b/spec/javascripts/diffs/components/diff_file_header_spec.js @@ -521,7 +521,7 @@ describe('diff_file_header', () => { }); describe('with discussions', () => { - it('dispatches toggleFileDiscussions when user clicks on toggle discussions button', () => { + it('dispatches toggleFileDiscussionWrappers when user clicks on toggle discussions button', () => { const propsCopy = Object.assign({}, props); propsCopy.diffFile.submodule = false; propsCopy.diffFile.blob = { @@ -552,11 +552,11 @@ describe('diff_file_header', () => { }), }); - spyOn(vm, 'toggleFileDiscussions'); + spyOn(vm, 'toggleFileDiscussionWrappers'); vm.$el.querySelector('.js-btn-vue-toggle-comments').click(); - expect(vm.toggleFileDiscussions).toHaveBeenCalled(); + expect(vm.toggleFileDiscussionWrappers).toHaveBeenCalled(); }); }); }); diff --git a/spec/javascripts/diffs/components/diff_gutter_avatars_spec.js b/spec/javascripts/diffs/components/diff_gutter_avatars_spec.js deleted file mode 100644 index cdd30919b09..00000000000 --- a/spec/javascripts/diffs/components/diff_gutter_avatars_spec.js +++ /dev/null @@ -1,146 +0,0 @@ -import Vue from 'vue'; -import DiffGutterAvatarsComponent from '~/diffs/components/diff_gutter_avatars.vue'; -import { COUNT_OF_AVATARS_IN_GUTTER } from '~/diffs/constants'; -import store from '~/mr_notes/stores'; -import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; -import discussionsMockData from '../mock_data/diff_discussions'; - -describe('DiffGutterAvatars', () => { - let component; - const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; - - beforeEach(() => { - component = createComponentWithStore(Vue.extend(DiffGutterAvatarsComponent), store, { - discussions: getDiscussionsMockData(), - }).$mount(); - }); - - describe('computed', () => { - describe('discussionsExpanded', () => { - it('should return true when all discussions are expanded', () => { - expect(component.discussionsExpanded).toEqual(true); - }); - - it('should return false when all discussions are not expanded', () => { - component.discussions[0].expanded = false; - - expect(component.discussionsExpanded).toEqual(false); - }); - }); - - describe('allDiscussions', () => { - it('should return an array of notes', () => { - expect(component.allDiscussions).toEqual([...component.discussions[0].notes]); - }); - }); - - describe('notesInGutter', () => { - it('should return a subset of discussions to show in gutter', () => { - expect(component.notesInGutter.length).toEqual(COUNT_OF_AVATARS_IN_GUTTER); - expect(component.notesInGutter[0]).toEqual({ - note: component.discussions[0].notes[0].note, - author: component.discussions[0].notes[0].author, - }); - }); - }); - - describe('moreCount', () => { - it('should return count of remaining discussions from gutter', () => { - expect(component.moreCount).toEqual(2); - }); - }); - - describe('moreText', () => { - it('should return proper text if moreCount > 0', () => { - expect(component.moreText).toEqual('2 more comments'); - }); - - it('should return empty string if there is no discussion', () => { - component.discussions = []; - - expect(component.moreText).toEqual(''); - }); - }); - }); - - describe('methods', () => { - describe('getTooltipText', () => { - it('should return original comment if it is shorter than max length', () => { - const note = component.discussions[0].notes[0]; - - expect(component.getTooltipText(note)).toEqual('Administrator: comment 1'); - }); - - it('should return truncated version of comment', () => { - const note = component.discussions[0].notes[1]; - - expect(component.getTooltipText(note)).toEqual('Fatih Acet: comment 2 is r...'); - }); - }); - - describe('toggleDiscussions', () => { - it('should toggle all discussions', () => { - expect(component.discussions[0].expanded).toEqual(true); - - component.$store.dispatch('setInitialNotes', getDiscussionsMockData()); - component.discussions = component.$store.state.notes.discussions; - component.toggleDiscussions(); - - expect(component.discussions[0].expanded).toEqual(false); - component.$store.dispatch('setInitialNotes', []); - }); - - it('forces expansion of all discussions', () => { - spyOn(component.$store, 'dispatch'); - - component.discussions[0].expanded = true; - component.discussions.push({ - ...component.discussions[0], - id: '123test', - expanded: false, - }); - - component.toggleDiscussions(); - - expect(component.$store.dispatch.calls.argsFor(0)).toEqual([ - 'toggleDiscussion', - { - discussionId: component.discussions[0].id, - forceExpanded: true, - }, - ]); - - expect(component.$store.dispatch.calls.argsFor(1)).toEqual([ - 'toggleDiscussion', - { - discussionId: component.discussions[1].id, - forceExpanded: true, - }, - ]); - }); - }); - }); - - describe('template', () => { - const buttonSelector = '.js-diff-comment-button'; - const svgSelector = `${buttonSelector} svg`; - const avatarSelector = '.js-diff-comment-avatar'; - const plusCountSelector = '.js-diff-comment-plus'; - - it('should have button to collapse discussions when the discussions expanded', () => { - expect(component.$el.querySelector(buttonSelector)).toBeDefined(); - expect(component.$el.querySelector(svgSelector)).toBeDefined(); - }); - - it('should have user avatars when discussions collapsed', () => { - component.discussions[0].expanded = false; - - Vue.nextTick(() => { - expect(component.$el.querySelector(buttonSelector)).toBeNull(); - expect(component.$el.querySelectorAll(avatarSelector).length).toEqual(4); - expect(component.$el.querySelector(plusCountSelector)).toBeDefined(); - expect(component.$el.querySelector(plusCountSelector).textContent).toEqual('+2'); - }); - }); - }); -}); diff --git a/spec/javascripts/diffs/components/inline_diff_view_spec.js b/spec/javascripts/diffs/components/inline_diff_view_spec.js index 4452106580a..0b3890b68d6 100644 --- a/spec/javascripts/diffs/components/inline_diff_view_spec.js +++ b/spec/javascripts/diffs/components/inline_diff_view_spec.js @@ -36,10 +36,11 @@ describe('InlineDiffView', () => { it('should render discussions', done => { const el = component.$el; component.diffLines[1].discussions = getDiscussionsMockData(); + component.diffLines[1].discussionsExpanded = true; Vue.nextTick(() => { expect(el.querySelectorAll('.notes_holder').length).toEqual(1); - expect(el.querySelectorAll('.notes_holder .note-discussion li').length).toEqual(5); + expect(el.querySelectorAll('.notes_holder .note-discussion li').length).toEqual(6); expect(el.innerText.indexOf('comment 5')).toBeGreaterThan(-1); component.$store.dispatch('setInitialNotes', []); diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index f973728cfe1..f8872a3eb13 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -206,6 +206,7 @@ describe('DiffsStoreActions', () => { position_type: 'text', }, }, + hash: 'diff-content-1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a', }, }, ], diff --git a/spec/javascripts/environments/environment_terminal_button_spec.js b/spec/javascripts/environments/environment_terminal_button_spec.js index 56e18db59c5..fc98e656efe 100644 --- a/spec/javascripts/environments/environment_terminal_button_spec.js +++ b/spec/javascripts/environments/environment_terminal_button_spec.js @@ -12,36 +12,24 @@ describe('Stop Component', () => { }).$mount(); }; - describe('enabled', () => { - beforeEach(() => { - mountWithProps({ terminalPath }); - }); - - describe('computed', () => { - it('title', () => { - expect(component.title).toEqual('Terminal'); - }); - }); - - it('should render a link to open a web terminal with the provided path', () => { - expect(component.$el.tagName).toEqual('A'); - expect(component.$el.getAttribute('data-original-title')).toEqual('Terminal'); - expect(component.$el.getAttribute('aria-label')).toEqual('Terminal'); - expect(component.$el.getAttribute('href')).toEqual(terminalPath); - }); + beforeEach(() => { + mountWithProps({ terminalPath }); + }); - it('should render a non-disabled button', () => { - expect(component.$el.classList).not.toContain('disabled'); + describe('computed', () => { + it('title', () => { + expect(component.title).toEqual('Terminal'); }); }); - describe('disabled', () => { - beforeEach(() => { - mountWithProps({ terminalPath, disabled: true }); - }); + it('should render a link to open a web terminal with the provided path', () => { + expect(component.$el.tagName).toEqual('A'); + expect(component.$el.getAttribute('data-original-title')).toEqual('Terminal'); + expect(component.$el.getAttribute('aria-label')).toEqual('Terminal'); + expect(component.$el.getAttribute('href')).toEqual(terminalPath); + }); - it('should render a disabled button', () => { - expect(component.$el.classList).toContain('disabled'); - }); + it('should render a non-disabled button', () => { + expect(component.$el.classList).not.toContain('disabled'); }); }); diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js index efa864e7d00..74805ca8c00 100644 --- a/spec/javascripts/notes/components/noteable_discussion_spec.js +++ b/spec/javascripts/notes/components/noteable_discussion_spec.js @@ -36,14 +36,20 @@ describe('noteable_discussion component', () => { }); it('should render user avatar', () => { + const discussion = { ...discussionMock }; + discussion.diff_file = mockDiffFile; + discussion.diff_discussion = true; + + wrapper.setProps({ discussion, renderDiffFile: true }); + expect(wrapper.find('.user-avatar-link').exists()).toBe(true); }); - it('should not render discussion header for non diff discussions', () => { + it('should not render thread header for non diff threads', () => { expect(wrapper.find('.discussion-header').exists()).toBe(false); }); - it('should render discussion header', done => { + it('should render thread header', done => { const discussion = { ...discussionMock }; discussion.diff_file = mockDiffFile; discussion.diff_discussion = true; @@ -90,16 +96,16 @@ describe('noteable_discussion component', () => { .catch(done.fail); }); - it('does not render jump to discussion button', () => { - expect( - wrapper.find('*[data-original-title="Jump to next unresolved discussion"]').exists(), - ).toBe(false); + it('does not render jump to thread button', () => { + expect(wrapper.find('*[data-original-title="Jump to next unresolved thread"]').exists()).toBe( + false, + ); }); }); describe('methods', () => { describe('jumpToNextDiscussion', () => { - it('expands next unresolved discussion', done => { + it('expands next unresolved thread', done => { const discussion2 = getJSONFixture(discussionWithTwoUnresolvedNotes)[0]; discussion2.resolved = false; discussion2.active = true; @@ -114,9 +120,7 @@ describe('noteable_discussion component', () => { const nextDiscussionId = discussion2.id; - setFixtures(` - <div class="discussion" data-discussion-id="${nextDiscussionId}"></div> - `); + setFixtures(`<div class="discussion" data-discussion-id="${nextDiscussionId}"></div>`); wrapper.vm.jumpToNextDiscussion(); @@ -162,20 +166,20 @@ describe('noteable_discussion component', () => { .catch(done.fail); }); - describe('for commit discussions', () => { - it('should display a monospace started a discussion on commit', () => { - expect(wrapper.text()).toContain(`started a discussion on commit ${truncatedCommitId}`); + describe('for commit threads', () => { + it('should display a monospace started a thread on commit', () => { + expect(wrapper.text()).toContain(`started a thread on commit ${truncatedCommitId}`); expect(commitElement.exists()).toBe(true); expect(commitElement.text()).toContain(truncatedCommitId); }); }); - describe('for diff discussion with a commit id', () => { - it('should display started discussion on commit header', done => { + describe('for diff thread with a commit id', () => { + it('should display started thread on commit header', done => { wrapper.vm.discussion.for_commit = false; wrapper.vm.$nextTick(() => { - expect(wrapper.text()).toContain(`started a discussion on commit ${truncatedCommitId}`); + expect(wrapper.text()).toContain(`started a thread on commit ${truncatedCommitId}`); expect(commitElement).not.toBe(null); @@ -189,7 +193,7 @@ describe('noteable_discussion component', () => { wrapper.vm.$nextTick(() => { expect(wrapper.text()).toContain( - `started a discussion on an outdated change in commit ${truncatedCommitId}`, + `started a thread on an outdated change in commit ${truncatedCommitId}`, ); expect(commitElement).not.toBe(null); @@ -199,21 +203,21 @@ describe('noteable_discussion component', () => { }); }); - describe('for diff discussions without a commit id', () => { - it('should show started a discussion on the diff text', done => { + describe('for diff threads without a commit id', () => { + it('should show started a thread on the diff text', done => { Object.assign(wrapper.vm.discussion, { for_commit: false, commit_id: null, }); wrapper.vm.$nextTick(() => { - expect(wrapper.text()).toContain('started a discussion on the diff'); + expect(wrapper.text()).toContain('started a thread on the diff'); done(); }); }); - it('should show discussion on older version text', done => { + it('should show thread on older version text', done => { Object.assign(wrapper.vm.discussion, { for_commit: false, commit_id: null, @@ -221,7 +225,7 @@ describe('noteable_discussion component', () => { }); wrapper.vm.$nextTick(() => { - expect(wrapper.text()).toContain('started a discussion on an old version of the diff'); + expect(wrapper.text()).toContain('started a thread on an old version of the diff'); done(); }); @@ -229,7 +233,7 @@ describe('noteable_discussion component', () => { }); }); - describe('for resolved discussion', () => { + describe('for resolved thread', () => { beforeEach(() => { const discussion = getJSONFixture(discussionWithTwoUnresolvedNotes)[0]; wrapper.setProps({ discussion }); @@ -242,7 +246,7 @@ describe('noteable_discussion component', () => { }); }); - describe('for unresolved discussion', () => { + describe('for unresolved thread', () => { beforeEach(done => { const discussion = { ...getJSONFixture(discussionWithTwoUnresolvedNotes)[0], diff --git a/spec/javascripts/notes/stores/actions_spec.js b/spec/javascripts/notes/stores/actions_spec.js index 7a9f32ddcff..65f72a135aa 100644 --- a/spec/javascripts/notes/stores/actions_spec.js +++ b/spec/javascripts/notes/stores/actions_spec.js @@ -373,7 +373,7 @@ describe('Actions Notes Store', () => { type: 'updateMergeRequestWidget', }, { - type: 'updateResolvableDiscussonsCounts', + type: 'updateResolvableDiscussionsCounts', }, ], done, @@ -400,7 +400,7 @@ describe('Actions Notes Store', () => { type: 'updateMergeRequestWidget', }, { - type: 'updateResolvableDiscussonsCounts', + type: 'updateResolvableDiscussionsCounts', }, { type: 'diffs/removeDiscussionsFromDiff', @@ -452,7 +452,7 @@ describe('Actions Notes Store', () => { type: 'startTaskList', }, { - type: 'updateResolvableDiscussonsCounts', + type: 'updateResolvableDiscussionsCounts', }, ], done, @@ -527,7 +527,7 @@ describe('Actions Notes Store', () => { ], [ { - type: 'updateResolvableDiscussonsCounts', + type: 'updateResolvableDiscussionsCounts', }, { type: 'updateMergeRequestWidget', @@ -552,7 +552,7 @@ describe('Actions Notes Store', () => { ], [ { - type: 'updateResolvableDiscussonsCounts', + type: 'updateResolvableDiscussionsCounts', }, { type: 'updateMergeRequestWidget', @@ -587,10 +587,10 @@ describe('Actions Notes Store', () => { }); }); - describe('updateResolvableDiscussonsCounts', () => { + describe('updateResolvableDiscussionsCounts', () => { it('commits UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS', done => { testAction( - actions.updateResolvableDiscussonsCounts, + actions.updateResolvableDiscussionsCounts, null, {}, [{ type: 'UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS' }], @@ -712,7 +712,7 @@ describe('Actions Notes Store', () => { [ { type: 'updateMergeRequestWidget' }, { type: 'startTaskList' }, - { type: 'updateResolvableDiscussonsCounts' }, + { type: 'updateResolvableDiscussionsCounts' }, ], done, ); diff --git a/spec/javascripts/performance_bar/components/simple_metric_spec.js b/spec/javascripts/performance_bar/components/simple_metric_spec.js deleted file mode 100644 index 98b843e9711..00000000000 --- a/spec/javascripts/performance_bar/components/simple_metric_spec.js +++ /dev/null @@ -1,47 +0,0 @@ -import Vue from 'vue'; -import simpleMetric from '~/performance_bar/components/simple_metric.vue'; -import mountComponent from 'spec/helpers/vue_mount_component_helper'; - -describe('simpleMetric', () => { - let vm; - - afterEach(() => { - vm.$destroy(); - }); - - describe('when the current request has no details', () => { - beforeEach(() => { - vm = mountComponent(Vue.extend(simpleMetric), { - currentRequest: {}, - metric: 'gitaly', - }); - }); - - it('does not display details', () => { - expect(vm.$el.innerText).not.toContain('/'); - }); - - it('displays the metric name', () => { - expect(vm.$el.innerText).toContain('gitaly'); - }); - }); - - describe('when the current request has details', () => { - beforeEach(() => { - vm = mountComponent(Vue.extend(simpleMetric), { - currentRequest: { - details: { gitaly: { duration: '123ms', calls: '456' } }, - }, - metric: 'gitaly', - }); - }); - - it('diplays details', () => { - expect(vm.$el.innerText.replace(/\s+/g, ' ')).toContain('123ms / 456'); - }); - - it('displays the metric name', () => { - expect(vm.$el.innerText).toContain('gitaly'); - }); - }); -}); diff --git a/spec/javascripts/releases/components/release_block_spec.js b/spec/javascripts/releases/components/release_block_spec.js index e98c665f99d..f761a18e326 100644 --- a/spec/javascripts/releases/components/release_block_spec.js +++ b/spec/javascripts/releases/components/release_block_spec.js @@ -14,7 +14,7 @@ describe('Release block', () => { description_html: '<div><h2>changelog</h2><ul><li>line1</li<li>line 2</li></ul></div>', author_name: 'Release bot', author_email: 'release-bot@example.com', - created_at: '2012-05-28T05:00:00-07:00', + released_at: '2012-05-28T05:00:00-07:00', author: { avatar_url: 'uploads/-/system/user/avatar/johndoe/avatar.png', id: 482476, @@ -101,7 +101,7 @@ describe('Release block', () => { }); it('renders release date', () => { - expect(vm.$el.textContent).toContain(timeagoMixin.methods.timeFormated(release.created_at)); + expect(vm.$el.textContent).toContain(timeagoMixin.methods.timeFormated(release.released_at)); }); it('renders number of assets provided', () => { @@ -152,13 +152,13 @@ describe('Release block', () => { }); }); - describe('with pre_release flag', () => { + describe('with upcoming_release flag', () => { beforeEach(() => { - vm = factory(Object.assign({}, release, { pre_release: true })); + vm = factory(Object.assign({}, release, { upcoming_release: true })); }); - it('renders pre-release badge', () => { - expect(vm.$el.textContent).toContain('Pre-release'); + it('renders upcoming release badge', () => { + expect(vm.$el.textContent).toContain('Upcoming Release'); }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_unresolved_discussions_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_unresolved_discussions_spec.js index bd64d7b2926..5bd1af56bcc 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_unresolved_discussions_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_unresolved_discussions_spec.js @@ -10,7 +10,7 @@ describe('UnresolvedDiscussions', () => { vm.$destroy(); }); - describe('with discussions path', () => { + describe('with threads path', () => { beforeEach(() => { vm = mountComponent(Component, { mr: { @@ -21,7 +21,7 @@ describe('UnresolvedDiscussions', () => { it('should have correct elements', () => { expect(vm.$el.innerText).toContain( - 'There are unresolved discussions. Please resolve these discussions', + 'There are unresolved threads. Please resolve these threads', ); expect(vm.$el.innerText).toContain('Create an issue to resolve them later'); @@ -29,14 +29,14 @@ describe('UnresolvedDiscussions', () => { }); }); - describe('without discussions path', () => { + describe('without threads path', () => { beforeEach(() => { vm = mountComponent(Component, { mr: {} }); }); it('should not show create issue link if user cannot create issue', () => { expect(vm.$el.innerText).toContain( - 'There are unresolved discussions. Please resolve these discussions', + 'There are unresolved threads. Please resolve these threads', ); expect(vm.$el.querySelector('.js-create-issue')).toEqual(null); diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb index 8ff971114d6..a714fa50f5f 100644 --- a/spec/lib/banzai/filter/relative_link_filter_spec.rb +++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb @@ -83,6 +83,11 @@ describe Banzai::Filter::RelativeLinkFilter do expect { filter(act) }.not_to raise_error end + it 'does not explode with an escaped null byte' do + act = link("/%00") + expect { filter(act) }.not_to raise_error + end + it 'does not raise an exception with a space in the path' do act = link("/uploads/d18213acd3732630991986120e167e3d/Landscape_8.jpg \nBut here's some more unexpected text :smile:)") expect { filter(act) }.not_to raise_error diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index 6f05914f915..403e0785d1b 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -40,7 +40,7 @@ describe Feature do .once .and_call_original - expect(Rails.cache) + expect(Gitlab::ThreadMemoryCache.cache_backend) .to receive(:fetch) .once .with('flipper:persisted_names', expires_in: 1.minute) diff --git a/spec/lib/gitlab/asciidoc_spec.rb b/spec/lib/gitlab/asciidoc_spec.rb index 0f933ac5464..8f2434acd26 100644 --- a/spec/lib/gitlab/asciidoc_spec.rb +++ b/spec/lib/gitlab/asciidoc_spec.rb @@ -56,7 +56,7 @@ module Gitlab }, 'pre' => { input: '```mypre"><script>alert(3)</script>', - output: "<div>\n<div>\n<pre lang=\"mypre\">\"><code></code></pre>\n</div>\n</div>" + output: "<div>\n<div>\n<pre class=\"code highlight js-syntax-highlight plaintext\" lang=\"plaintext\" v-pre=\"true\"><code><span id=\"LC1\" class=\"line\" lang=\"plaintext\">\"></span></code></pre>\n</div>\n</div>" } } @@ -67,6 +67,72 @@ module Gitlab end end + context 'with fenced block' do + it 'highlights syntax' do + input = <<~ADOC + ```js + console.log('hello world') + ``` + ADOC + + output = <<~HTML + <div> + <div> + <pre class="code highlight js-syntax-highlight javascript" lang="javascript" v-pre="true"><code><span id="LC1" class="line" lang="javascript"><span class="nx">console</span><span class="p">.</span><span class="nx">log</span><span class="p">(</span><span class="dl">'</span><span class="s1">hello world</span><span class="dl">'</span><span class="p">)</span></span></code></pre> + </div> + </div> + HTML + + expect(render(input, context)).to include(output.strip) + end + end + + context 'with listing block' do + it 'highlights syntax' do + input = <<~ADOC + [source,c++] + .class.cpp + ---- + #include <stdio.h> + + for (int i = 0; i < 5; i++) { + std::cout<<"*"<<std::endl; + } + ---- + ADOC + + output = <<~HTML + <div> + <div>class.cpp</div> + <div> + <pre class="code highlight js-syntax-highlight cpp" lang="cpp" v-pre="true"><code><span id="LC1" class="line" lang="cpp"><span class="cp">#include <stdio.h></span></span> + <span id="LC2" class="line" lang="cpp"></span> + <span id="LC3" class="line" lang="cpp"><span class="k">for</span> <span class="p">(</span><span class="kt">int</span> <span class="n">i</span> <span class="o">=</span> <span class="mi">0</span><span class="p">;</span> <span class="n">i</span> <span class="o"><</span> <span class="mi">5</span><span class="p">;</span> <span class="n">i</span><span class="o">++</span><span class="p">)</span> <span class="p">{</span></span> + <span id="LC4" class="line" lang="cpp"> <span class="n">std</span><span class="o">::</span><span class="n">cout</span><span class="o"><<</span><span class="s">"*"</span><span class="o"><<</span><span class="n">std</span><span class="o">::</span><span class="n">endl</span><span class="p">;</span></span> + <span id="LC5" class="line" lang="cpp"><span class="p">}</span></span></code></pre> + </div> + </div> + HTML + + expect(render(input, context)).to include(output.strip) + end + end + + context 'with stem block' do + it 'does not apply syntax highlighting' do + input = <<~ADOC + [stem] + ++++ + \sqrt{4} = 2 + ++++ + ADOC + + output = "<div>\n<div>\n\\$ qrt{4} = 2\\$\n</div>\n</div>" + + expect(render(input, context)).to include(output) + end + end + context 'external links' do it 'adds the `rel` attribute to the link' do output = render('link:https://google.com[Google]', context) diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index 3b5ca7c950c..d9c73cff01e 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -309,6 +309,15 @@ describe Gitlab::Auth do .to eq(auth_success) end + it 'succeeds when custom login and token are valid' do + deploy_token = create(:deploy_token, username: 'deployer', read_registry: false, projects: [project]) + auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, [:download_code]) + + expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'deployer') + expect(gl_auth.find_for_git_client('deployer', deploy_token.token, project: project, ip: 'ip')) + .to eq(auth_success) + end + it 'fails when login is not valid' do expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'random_login') expect(gl_auth.find_for_git_client('random_login', deploy_token.token, project: project, ip: 'ip')) diff --git a/spec/lib/gitlab/background_migration/migrate_system_uploads_to_new_folder_spec.rb b/spec/lib/gitlab/background_migration/migrate_system_uploads_to_new_folder_spec.rb deleted file mode 100644 index ea8bdd48e72..00000000000 --- a/spec/lib/gitlab/background_migration/migrate_system_uploads_to_new_folder_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'spec_helper' - -# rubocop:disable RSpec/FactoriesInMigrationSpecs -describe Gitlab::BackgroundMigration::MigrateSystemUploadsToNewFolder, :delete do - let(:migration) { described_class.new } - - before do - allow(migration).to receive(:logger).and_return(Logger.new(nil)) - end - - describe '#perform' do - it 'renames the path of system-uploads' do - upload = create(:upload, model: create(:project), path: 'uploads/system/project/avatar.jpg') - - migration.perform('uploads/system/', 'uploads/-/system/') - - expect(upload.reload.path).to eq('uploads/-/system/project/avatar.jpg') - end - end -end -# rubocop:enable RSpec/FactoriesInMigrationSpecs diff --git a/spec/lib/gitlab/background_migration/move_personal_snippet_files_spec.rb b/spec/lib/gitlab/background_migration/move_personal_snippet_files_spec.rb deleted file mode 100644 index 593486fc56c..00000000000 --- a/spec/lib/gitlab/background_migration/move_personal_snippet_files_spec.rb +++ /dev/null @@ -1,74 +0,0 @@ -require 'spec_helper' - -# rubocop:disable RSpec/FactoriesInMigrationSpecs -describe Gitlab::BackgroundMigration::MovePersonalSnippetFiles do - let(:test_dir) { File.join(Rails.root, 'tmp', 'tests', 'move_snippet_files_test') } - let(:old_uploads_dir) { File.join('uploads', 'system', 'personal_snippet') } - let(:new_uploads_dir) { File.join('uploads', '-', 'system', 'personal_snippet') } - let(:snippet) do - snippet = create(:personal_snippet) - create_upload_for_snippet(snippet) - snippet.update!(description: markdown_linking_file(snippet)) - snippet - end - - let(:migration) { described_class.new } - - before do - allow(migration).to receive(:base_directory) { test_dir } - end - - describe '#perform' do - it 'moves the file on the disk' do - expected_path = File.join(test_dir, new_uploads_dir, snippet.id.to_s, "secret#{snippet.id}", 'upload.txt') - - migration.perform(old_uploads_dir, new_uploads_dir) - - expect(File.exist?(expected_path)).to be_truthy - end - - it 'updates the markdown of the snippet' do - expected_path = File.join(new_uploads_dir, snippet.id.to_s, "secret#{snippet.id}", 'upload.txt') - expected_markdown = "[an upload](#{expected_path})" - - migration.perform(old_uploads_dir, new_uploads_dir) - - expect(snippet.reload.description).to eq(expected_markdown) - end - - it 'updates the markdown of notes' do - expected_path = File.join(new_uploads_dir, snippet.id.to_s, "secret#{snippet.id}", 'upload.txt') - expected_markdown = "with [an upload](#{expected_path})" - - note = create(:note_on_personal_snippet, noteable: snippet, note: "with #{markdown_linking_file(snippet)}") - - migration.perform(old_uploads_dir, new_uploads_dir) - - expect(note.reload.note).to eq(expected_markdown) - end - end - - def create_upload_for_snippet(snippet) - snippet_path = path_for_file_in_snippet(snippet) - path = File.join(old_uploads_dir, snippet.id.to_s, snippet_path) - absolute_path = File.join(test_dir, path) - - FileUtils.mkdir_p(File.dirname(absolute_path)) - FileUtils.touch(absolute_path) - - create(:upload, model: snippet, path: snippet_path, uploader: PersonalFileUploader) - end - - def path_for_file_in_snippet(snippet) - secret = "secret#{snippet.id}" - filename = 'upload.txt' - - File.join(secret, filename) - end - - def markdown_linking_file(snippet) - path = File.join(old_uploads_dir, snippet.id.to_s, path_for_file_in_snippet(snippet)) - "[an upload](#{path})" - end -end -# rubocop:enable RSpec/FactoriesInMigrationSpecs diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index 7f336ee853e..4e8bff3d738 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -90,6 +90,27 @@ describe Gitlab::Ci::Config do end end + context 'when yml is too big' do + let(:yml) do + <<~YAML + --- &1 + - hi + - *1 + YAML + end + + describe '.new' do + it 'raises error' do + expect(Gitlab::Sentry).to receive(:track_exception) + + expect { config }.to raise_error( + described_class::ConfigError, + /The parsed YAML is too big/ + ) + end + end + end + context 'when config logic is incorrect' do let(:yml) { 'before_script: "ls"' } diff --git a/spec/lib/gitlab/config/loader/yaml_spec.rb b/spec/lib/gitlab/config/loader/yaml_spec.rb index 44c9a3896a8..0affeb01f6a 100644 --- a/spec/lib/gitlab/config/loader/yaml_spec.rb +++ b/spec/lib/gitlab/config/loader/yaml_spec.rb @@ -8,7 +8,7 @@ describe Gitlab::Config::Loader::Yaml do describe '#valid?' do it 'returns true' do - expect(loader.valid?).to be true + expect(loader).to be_valid end end @@ -24,7 +24,7 @@ describe Gitlab::Config::Loader::Yaml do describe '#valid?' do it 'returns false' do - expect(loader.valid?).to be false + expect(loader).not_to be_valid end end @@ -43,7 +43,10 @@ describe Gitlab::Config::Loader::Yaml do describe '#initialize' do it 'raises FormatError' do - expect { loader }.to raise_error(Gitlab::Config::Loader::FormatError, 'Unknown alias: bad_alias') + expect { loader }.to raise_error( + Gitlab::Config::Loader::FormatError, + 'Unknown alias: bad_alias' + ) end end end @@ -53,7 +56,68 @@ describe Gitlab::Config::Loader::Yaml do describe '#valid?' do it 'returns false' do - expect(loader.valid?).to be false + expect(loader).not_to be_valid + end + end + end + + # Prevent Billion Laughs attack: https://gitlab.com/gitlab-org/gitlab-ce/issues/56018 + context 'when yaml size is too large' do + let(:yml) do + <<~YAML + a: &a ["lol","lol","lol","lol","lol","lol","lol","lol","lol"] + b: &b [*a,*a,*a,*a,*a,*a,*a,*a,*a] + c: &c [*b,*b,*b,*b,*b,*b,*b,*b,*b] + d: &d [*c,*c,*c,*c,*c,*c,*c,*c,*c] + e: &e [*d,*d,*d,*d,*d,*d,*d,*d,*d] + f: &f [*e,*e,*e,*e,*e,*e,*e,*e,*e] + g: &g [*f,*f,*f,*f,*f,*f,*f,*f,*f] + h: &h [*g,*g,*g,*g,*g,*g,*g,*g,*g] + i: &i [*h,*h,*h,*h,*h,*h,*h,*h,*h] + YAML + end + + describe '#valid?' do + it 'returns false' do + expect(loader).not_to be_valid + end + + it 'returns true if "ci_yaml_limit_size" feature flag is disabled' do + stub_feature_flags(ci_yaml_limit_size: false) + + expect(loader).to be_valid + end + end + + describe '#load!' do + it 'raises FormatError' do + expect { loader.load! }.to raise_error( + Gitlab::Config::Loader::FormatError, + 'The parsed YAML is too big' + ) + end + end + end + + # Prevent Billion Laughs attack: https://gitlab.com/gitlab-org/gitlab-ce/issues/56018 + context 'when yaml has cyclic data structure' do + let(:yml) do + <<~YAML + --- &1 + - hi + - *1 + YAML + end + + describe '#valid?' do + it 'returns false' do + expect(loader.valid?).to be(false) + end + end + + describe '#load!' do + it 'raises FormatError' do + expect { loader.load! }.to raise_error(Gitlab::Config::Loader::FormatError, 'The parsed YAML is too big') end end end diff --git a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb index aec9c4baf0a..d60d1b7559a 100644 --- a/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb +++ b/spec/lib/gitlab/graphql/authorize/authorize_field_service_spec.rb @@ -7,35 +7,39 @@ require 'spec_helper' describe Gitlab::Graphql::Authorize::AuthorizeFieldService do def type(type_authorizations = []) Class.new(Types::BaseObject) do - graphql_name "TestType" + graphql_name 'TestType' authorize type_authorizations end end - def type_with_field(field_type, field_authorizations = [], resolved_value = "Resolved value") + def type_with_field(field_type, field_authorizations = [], resolved_value = 'Resolved value', **options) Class.new(Types::BaseObject) do - graphql_name "TestTypeWithField" - field :test_field, field_type, null: true, authorize: field_authorizations, resolve: -> (_, _, _) { resolved_value} + graphql_name 'TestTypeWithField' + options.reverse_merge!(null: true) + field :test_field, field_type, + authorize: field_authorizations, + resolve: -> (_, _, _) { resolved_value }, + **options end end let(:current_user) { double(:current_user) } subject(:service) { described_class.new(field) } - describe "#authorized_resolve" do - let(:presented_object) { double("presented object") } - let(:presented_type) { double("parent type", object: presented_object) } + describe '#authorized_resolve' do + let(:presented_object) { double('presented object') } + let(:presented_type) { double('parent type', object: presented_object) } subject(:resolved) { service.authorized_resolve.call(presented_type, {}, { current_user: current_user }) } - context "scalar types" do - shared_examples "checking permissions on the presented object" do - it "checks the abilities on the object being presented and returns the value" do + context 'scalar types' do + shared_examples 'checking permissions on the presented object' do + it 'checks the abilities on the object being presented and returns the value' do expected_permissions.each do |permission| spy_ability_check_for(permission, presented_object, passed: true) end - expect(resolved).to eq("Resolved value") + expect(resolved).to eq('Resolved value') end it "returns nil if the value wasn't authorized" do @@ -45,61 +49,71 @@ describe Gitlab::Graphql::Authorize::AuthorizeFieldService do end end - context "when the field is a built-in scalar type" do - let(:field) { type_with_field(GraphQL::STRING_TYPE, :read_field).fields["testField"].to_graphql } + context 'when the field is a built-in scalar type' do + let(:field) { type_with_field(GraphQL::STRING_TYPE, :read_field).fields['testField'].to_graphql } let(:expected_permissions) { [:read_field] } - it_behaves_like "checking permissions on the presented object" + it_behaves_like 'checking permissions on the presented object' end - context "when the field is a list of scalar types" do - let(:field) { type_with_field([GraphQL::STRING_TYPE], :read_field).fields["testField"].to_graphql } + context 'when the field is a list of scalar types' do + let(:field) { type_with_field([GraphQL::STRING_TYPE], :read_field).fields['testField'].to_graphql } let(:expected_permissions) { [:read_field] } - it_behaves_like "checking permissions on the presented object" + it_behaves_like 'checking permissions on the presented object' end - context "when the field is sub-classed scalar type" do - let(:field) { type_with_field(Types::TimeType, :read_field).fields["testField"].to_graphql } + context 'when the field is sub-classed scalar type' do + let(:field) { type_with_field(Types::TimeType, :read_field).fields['testField'].to_graphql } let(:expected_permissions) { [:read_field] } - it_behaves_like "checking permissions on the presented object" + it_behaves_like 'checking permissions on the presented object' end - context "when the field is a list of sub-classed scalar types" do - let(:field) { type_with_field([Types::TimeType], :read_field).fields["testField"].to_graphql } + context 'when the field is a list of sub-classed scalar types' do + let(:field) { type_with_field([Types::TimeType], :read_field).fields['testField'].to_graphql } let(:expected_permissions) { [:read_field] } - it_behaves_like "checking permissions on the presented object" + it_behaves_like 'checking permissions on the presented object' end end - context "when the field is a specific type" do + context 'when the field is a specific type' do let(:custom_type) { type(:read_type) } - let(:object_in_field) { double("presented in field") } - let(:field) { type_with_field(custom_type, :read_field, object_in_field).fields["testField"].to_graphql } + let(:object_in_field) { double('presented in field') } + let(:field) { type_with_field(custom_type, :read_field, object_in_field).fields['testField'].to_graphql } - it "checks both field & type permissions" do + it 'checks both field & type permissions' do spy_ability_check_for(:read_field, object_in_field, passed: true) spy_ability_check_for(:read_type, object_in_field, passed: true) expect(resolved).to eq(object_in_field) end - it "returns nil if viewing was not allowed" do + it 'returns nil if viewing was not allowed' do spy_ability_check_for(:read_field, object_in_field, passed: false) spy_ability_check_for(:read_type, object_in_field, passed: true) expect(resolved).to be_nil end - context "when the field is a list" do - let(:object_1) { double("presented in field 1") } - let(:object_2) { double("presented in field 2") } + context 'when the field is not nullable' do + let(:field) { type_with_field(custom_type, [], object_in_field, null: false).fields['testField'].to_graphql } + + it 'returns nil when viewing is not allowed' do + spy_ability_check_for(:read_type, object_in_field, passed: false) + + expect(resolved).to be_nil + end + end + + context 'when the field is a list' do + let(:object_1) { double('presented in field 1') } + let(:object_2) { double('presented in field 2') } let(:presented_types) { [double(object: object_1), double(object: object_2)] } - let(:field) { type_with_field([custom_type], :read_field, presented_types).fields["testField"].to_graphql } + let(:field) { type_with_field([custom_type], :read_field, presented_types).fields['testField'].to_graphql } - it "checks all permissions" do + it 'checks all permissions' do allow(Ability).to receive(:allowed?) { true } spy_ability_check_for(:read_field, object_1, passed: true) @@ -110,7 +124,7 @@ describe Gitlab::Graphql::Authorize::AuthorizeFieldService do expect(resolved).to eq(presented_types) end - it "filters out objects that the user cannot see" do + it 'filters out objects that the user cannot see' do allow(Ability).to receive(:allowed?) { true } spy_ability_check_for(:read_type, object_1, passed: false) diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index a406c25b1d8..28b187c3676 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -123,6 +123,7 @@ Release: - project_id - created_at - updated_at +- released_at Releases::Link: - id - release_id @@ -429,6 +430,7 @@ Service: - confidential_issues_events - confidential_note_events - deployment_events +- description ProjectHook: - id - url diff --git a/spec/lib/gitlab/issuable_metadata_spec.rb b/spec/lib/gitlab/issuable_metadata_spec.rb index 916f3876a8e..032467b8b4e 100644 --- a/spec/lib/gitlab/issuable_metadata_spec.rb +++ b/spec/lib/gitlab/issuable_metadata_spec.rb @@ -7,11 +7,11 @@ describe Gitlab::IssuableMetadata do subject { Class.new { include Gitlab::IssuableMetadata }.new } it 'returns an empty Hash if an empty collection is provided' do - expect(subject.issuable_meta_data(Issue.none, 'Issue')).to eq({}) + expect(subject.issuable_meta_data(Issue.none, 'Issue', user)).to eq({}) end it 'raises an error when given a collection with no limit' do - expect { subject.issuable_meta_data(Issue.all, 'Issue') }.to raise_error(/must have a limit/) + expect { subject.issuable_meta_data(Issue.all, 'Issue', user) }.to raise_error(/must have a limit/) end context 'issues' do @@ -23,7 +23,7 @@ describe Gitlab::IssuableMetadata do let!(:closing_issues) { create(:merge_requests_closing_issues, issue: issue, merge_request: merge_request) } it 'aggregates stats on issues' do - data = subject.issuable_meta_data(Issue.all.limit(10), 'Issue') + data = subject.issuable_meta_data(Issue.all.limit(10), 'Issue', user) expect(data.count).to eq(2) expect(data[issue.id].upvotes).to eq(1) @@ -46,7 +46,7 @@ describe Gitlab::IssuableMetadata do let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } it 'aggregates stats on merge requests' do - data = subject.issuable_meta_data(MergeRequest.all.limit(10), 'MergeRequest') + data = subject.issuable_meta_data(MergeRequest.all.limit(10), 'MergeRequest', user) expect(data.count).to eq(2) expect(data[merge_request.id].upvotes).to eq(1) diff --git a/spec/lib/gitlab/legacy_github_import/importer_spec.rb b/spec/lib/gitlab/legacy_github_import/importer_spec.rb index a0c664da185..9163019514b 100644 --- a/spec/lib/gitlab/legacy_github_import/importer_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/importer_spec.rb @@ -132,6 +132,7 @@ describe Gitlab::LegacyGithubImport::Importer do body: 'Release v1.0.0', draft: false, created_at: created_at, + published_at: created_at, updated_at: updated_at, url: "#{api_root}/repos/octocat/Hello-World/releases/1" ) @@ -144,6 +145,7 @@ describe Gitlab::LegacyGithubImport::Importer do body: nil, draft: false, created_at: created_at, + published_at: created_at, updated_at: updated_at, url: "#{api_root}/repos/octocat/Hello-World/releases/2" ) diff --git a/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb b/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb index c57b96fb00d..534cf219520 100644 --- a/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb +++ b/spec/lib/gitlab/legacy_github_import/release_formatter_spec.rb @@ -4,6 +4,7 @@ describe Gitlab::LegacyGithubImport::ReleaseFormatter do let!(:project) { create(:project, namespace: create(:namespace, path: 'octocat')) } let(:octocat) { double(id: 123456, login: 'octocat') } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } + let(:published_at) { DateTime.strptime('2011-01-26T20:00:00Z') } let(:base_data) do { @@ -11,7 +12,7 @@ describe Gitlab::LegacyGithubImport::ReleaseFormatter do name: 'First release', draft: false, created_at: created_at, - published_at: created_at, + published_at: published_at, body: 'Release v1.0.0' } end @@ -28,6 +29,7 @@ describe Gitlab::LegacyGithubImport::ReleaseFormatter do name: 'First release', description: 'Release v1.0.0', created_at: created_at, + released_at: published_at, updated_at: created_at } diff --git a/spec/lib/gitlab/performance_bar_spec.rb b/spec/lib/gitlab/performance_bar_spec.rb index f480376acb4..ee3c571c9c0 100644 --- a/spec/lib/gitlab/performance_bar_spec.rb +++ b/spec/lib/gitlab/performance_bar_spec.rb @@ -3,17 +3,42 @@ require 'spec_helper' describe Gitlab::PerformanceBar do shared_examples 'allowed user IDs are cached' do before do - # Warm the Redis cache + # Warm the caches described_class.enabled?(user) end it 'caches the allowed user IDs in cache', :use_clean_rails_memory_store_caching do expect do + expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original + expect(described_class.l2_cache_backend).not_to receive(:fetch) expect(described_class.enabled?(user)).to be_truthy end.not_to exceed_query_limit(0) end + + it 'caches the allowed user IDs in L1 cache for 1 minute', :use_clean_rails_memory_store_caching do + Timecop.travel 2.minutes do + expect do + expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original + expect(described_class.l2_cache_backend).to receive(:fetch).and_call_original + expect(described_class.enabled?(user)).to be_truthy + end.not_to exceed_query_limit(0) + end + end + + it 'caches the allowed user IDs in L2 cache for 5 minutes', :use_clean_rails_memory_store_caching do + Timecop.travel 6.minutes do + expect do + expect(described_class.l1_cache_backend).to receive(:fetch).and_call_original + expect(described_class.l2_cache_backend).to receive(:fetch).and_call_original + expect(described_class.enabled?(user)).to be_truthy + end.not_to exceed_query_limit(2) + end + end end + it { expect(described_class.l1_cache_backend).to eq(Gitlab::ThreadMemoryCache.cache_backend) } + it { expect(described_class.l2_cache_backend).to eq(Rails.cache) } + describe '.enabled?' do let(:user) { create(:user) } diff --git a/spec/lib/gitlab/utils/deep_size_spec.rb b/spec/lib/gitlab/utils/deep_size_spec.rb new file mode 100644 index 00000000000..1e619a15980 --- /dev/null +++ b/spec/lib/gitlab/utils/deep_size_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' + +describe Gitlab::Utils::DeepSize do + let(:data) do + { + a: [1, 2, 3], + b: { + c: [4, 5], + d: [ + { e: [[6], [7]] } + ] + } + } + end + + let(:max_size) { 1.kilobyte } + let(:max_depth) { 10 } + let(:deep_size) { described_class.new(data, max_size: max_size, max_depth: max_depth) } + + describe '#evaluate' do + context 'when data within size and depth limits' do + it 'returns true' do + expect(deep_size).to be_valid + end + end + + context 'when data not within size limit' do + let(:max_size) { 200.bytes } + + it 'returns false' do + expect(deep_size).not_to be_valid + end + end + + context 'when data not within depth limit' do + let(:max_depth) { 2 } + + it 'returns false' do + expect(deep_size).not_to be_valid + end + end + end +end diff --git a/spec/migrations/backfill_and_add_not_null_constraint_to_released_at_column_on_releases_table_spec.rb b/spec/migrations/backfill_and_add_not_null_constraint_to_released_at_column_on_releases_table_spec.rb new file mode 100644 index 00000000000..9cae1daacea --- /dev/null +++ b/spec/migrations/backfill_and_add_not_null_constraint_to_released_at_column_on_releases_table_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'migrate', '20190628185004_backfill_and_add_not_null_constraint_to_released_at_column_on_releases_table.rb') + +describe BackfillAndAddNotNullConstraintToReleasedAtColumnOnReleasesTable, :migration do + let(:releases) { table(:releases) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + + subject(:migration) { described_class.new } + + it 'fills released_at with the value of created_at' do + created_at_a = Time.zone.parse('2019-02-10T08:00:00Z') + created_at_b = Time.zone.parse('2019-03-10T18:00:00Z') + namespace = namespaces.create(name: 'foo', path: 'foo') + project = projects.create!(namespace_id: namespace.id) + release_a = releases.create!(project_id: project.id, created_at: created_at_a) + release_b = releases.create!(project_id: project.id, created_at: created_at_b) + + disable_migrations_output { migration.up } + + release_a.reload + release_b.reload + expect(release_a.released_at).to eq(created_at_a) + expect(release_b.released_at).to eq(created_at_b) + end +end diff --git a/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb b/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb index 34f4a36d63d..65a918d5440 100644 --- a/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb +++ b/spec/migrations/backfill_store_project_full_path_in_repo_spec.rb @@ -13,7 +13,7 @@ describe BackfillStoreProjectFullPathInRepo, :migration do subject(:migration) { described_class.new } around do |example| - Sidekiq::Testing.inline! do + perform_enqueued_jobs do example.run end end diff --git a/spec/migrations/remove_project_labels_group_id_spec.rb b/spec/migrations/remove_project_labels_group_id_spec.rb deleted file mode 100644 index 01b09e71d83..00000000000 --- a/spec/migrations/remove_project_labels_group_id_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -# encoding: utf-8 - -require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20180202111106_remove_project_labels_group_id.rb') - -describe RemoveProjectLabelsGroupId, :delete do - let(:migration) { described_class.new } - let(:group) { create(:group) } # rubocop:disable RSpec/FactoriesInMigrationSpecs - let!(:project_label) { create(:label, group_id: group.id) } # rubocop:disable RSpec/FactoriesInMigrationSpecs - let!(:group_label) { create(:group_label) } # rubocop:disable RSpec/FactoriesInMigrationSpecs - - describe '#up' do - it 'updates the project labels group ID' do - expect { migration.up }.to change { project_label.reload.group_id }.to(nil) - end - - it 'keeps the group labels group ID' do - expect { migration.up }.not_to change { group_label.reload.group_id } - end - end -end diff --git a/spec/models/deploy_token_spec.rb b/spec/models/deploy_token_spec.rb index 2fe82eaa778..8d951ab6f0f 100644 --- a/spec/models/deploy_token_spec.rb +++ b/spec/models/deploy_token_spec.rb @@ -8,6 +8,15 @@ describe DeployToken do it { is_expected.to have_many :project_deploy_tokens } it { is_expected.to have_many(:projects).through(:project_deploy_tokens) } + describe 'validations' do + let(:username_format_message) { "can contain only letters, digits, '_', '-', '+', and '.'" } + + it { is_expected.to validate_length_of(:username).is_at_most(255) } + it { is_expected.to allow_value('GitLab+deploy_token-3.14').for(:username) } + it { is_expected.not_to allow_value('<script>').for(:username).with_message(username_format_message) } + it { is_expected.not_to allow_value('').for(:username).with_message(username_format_message) } + end + describe '#ensure_token' do it 'ensures a token' do deploy_token.token = nil @@ -87,8 +96,30 @@ describe DeployToken do end describe '#username' do - it 'returns a harcoded username' do - expect(deploy_token.username).to eq("gitlab+deploy-token-#{deploy_token.id}") + context 'persisted records' do + it 'returns a default username if none is set' do + expect(deploy_token.username).to eq("gitlab+deploy-token-#{deploy_token.id}") + end + + it 'returns the username provided if one is set' do + deploy_token = create(:deploy_token, username: 'deployer') + + expect(deploy_token.username).to eq('deployer') + end + end + + context 'new records' do + it 'returns nil if no username is set' do + deploy_token = build(:deploy_token) + + expect(deploy_token.username).to be_nil + end + + it 'returns the username provided if one is set' do + deploy_token = build(:deploy_token, username: 'deployer') + + expect(deploy_token.username).to eq('deployer') + end end end diff --git a/spec/models/project_services/bugzilla_service_spec.rb b/spec/models/project_services/bugzilla_service_spec.rb index 6818db48fee..d5b0f94f461 100644 --- a/spec/models/project_services/bugzilla_service_spec.rb +++ b/spec/models/project_services/bugzilla_service_spec.rb @@ -32,4 +32,49 @@ describe BugzillaService do it { is_expected.not_to validate_presence_of(:new_issue_url) } end end + + context 'overriding properties' do + let(:default_title) { 'JIRA' } + let(:default_description) { 'JiraService|Jira issue tracker' } + let(:url) { 'http://bugzilla.example.com' } + let(:access_params) do + { project_url: url, issues_url: url, new_issue_url: url } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) { create(:bugzilla_service, properties: properties) } + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:bugzilla_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:bugzilla_service, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:bugzilla_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('Bugzilla') + expect(service.description).to eq('Bugzilla issue tracker') + end + end + end end diff --git a/spec/models/project_services/custom_issue_tracker_service_spec.rb b/spec/models/project_services/custom_issue_tracker_service_spec.rb index f0e7551693d..56b0bda6626 100644 --- a/spec/models/project_services/custom_issue_tracker_service_spec.rb +++ b/spec/models/project_services/custom_issue_tracker_service_spec.rb @@ -48,4 +48,47 @@ describe CustomIssueTrackerService do end end end + + context 'overriding properties' do + let(:url) { 'http://custom.example.com' } + let(:access_params) do + { project_url: url, issues_url: url, new_issue_url: url } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) { create(:custom_issue_tracker_service, properties: properties) } + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:custom_issue_tracker_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:custom_issue_tracker_service, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:custom_issue_tracker_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('Custom Issue Tracker') + expect(service.description).to eq('Custom issue tracker') + end + end + end end diff --git a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb index 11f96c03d46..a3726f09dc5 100644 --- a/spec/models/project_services/gitlab_issue_tracker_service_spec.rb +++ b/spec/models/project_services/gitlab_issue_tracker_service_spec.rb @@ -51,4 +51,47 @@ describe GitlabIssueTrackerService do end end end + + context 'overriding properties' do + let(:url) { 'http://gitlab.example.com' } + let(:access_params) do + { project_url: url, issues_url: url, new_issue_url: url } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) { create(:gitlab_issue_tracker_service, properties: properties) } + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:gitlab_issue_tracker_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:gitlab_issue_tracker_service, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:gitlab_issue_tracker_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('GitLab') + expect(service.description).to eq('GitLab issue tracker') + end + end + end end diff --git a/spec/models/project_services/jira_service_spec.rb b/spec/models/project_services/jira_service_spec.rb index fc08457a3c5..9b122d85293 100644 --- a/spec/models/project_services/jira_service_spec.rb +++ b/spec/models/project_services/jira_service_spec.rb @@ -115,6 +115,70 @@ describe JiraService do end end + describe '#create' do + let(:params) do + { + project: create(:project), title: 'custom title', description: 'custom description' + } + end + + subject { described_class.create(params) } + + it 'does not store title & description into properties' do + expect(subject.properties.keys).not_to include('title', 'description') + end + + it 'sets title & description correctly' do + service = subject + + expect(service.title).to eq('custom title') + expect(service.description).to eq('custom description') + end + end + + context 'overriding properties' do + let(:url) { 'http://issue_tracker.example.com' } + let(:access_params) do + { url: url, username: 'username', password: 'password' } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) { create(:jira_service, properties: properties) } + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:jira_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:jira_service, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:jira_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('Jira') + expect(service.description).to eq('Jira issue tracker') + end + end + end + describe '#close_issue' do let(:custom_base_url) { 'http://custom_url' } let(:user) { create(:user) } @@ -450,36 +514,54 @@ describe JiraService do end describe 'description and title' do - let(:project) { create(:project) } + let(:title) { 'Jira One' } + let(:description) { 'Jira One issue tracker' } + let(:properties) do + { + url: 'http://jira.example.com/web', + username: 'mic', + password: 'password', + title: title, + description: description + } + end context 'when it is not set' do - before do - @service = project.create_jira_service(active: true) - end + it 'default values are returned' do + service = create(:jira_service) - after do - @service.destroy! + expect(service.title).to eq('Jira') + expect(service.description).to eq('Jira issue tracker') end + end - it 'is initialized' do - expect(@service.title).to eq('Jira') - expect(@service.description).to eq('Jira issue tracker') + context 'when it is set in properties' do + it 'values from properties are returned' do + service = create(:jira_service, properties: properties) + + expect(service.title).to eq(title) + expect(service.description).to eq(description) end end - context 'when it is set' do - before do - properties = { 'title' => 'Jira One', 'description' => 'Jira One issue tracker' } - @service = project.create_jira_service(active: true, properties: properties) - end + context 'when it is in title & description fields' do + it 'values from title and description fields are returned' do + service = create(:jira_service, title: title, description: description) - after do - @service.destroy! + expect(service.title).to eq(title) + expect(service.description).to eq(description) end + end + + context 'when it is in both properites & title & description fields' do + it 'values from title and description fields are returned' do + title2 = 'Jira 2' + description2 = 'Jira description 2' - it 'is correct' do - expect(@service.title).to eq('Jira One') - expect(@service.description).to eq('Jira One issue tracker') + service = create(:jira_service, title: title2, description: description2, properties: properties) + + expect(service.title).to eq(title2) + expect(service.description).to eq(description2) end end end @@ -505,29 +587,21 @@ describe JiraService do end describe 'project and issue urls' do - let(:project) { create(:project) } - context 'when gitlab.yml was initialized' do - before do + it 'is prepopulated with the settings' do settings = { 'jira' => { - 'title' => 'Jira', 'url' => 'http://jira.sample/projects/project_a', 'api_url' => 'http://jira.sample/api' } } allow(Gitlab.config).to receive(:issues_tracker).and_return(settings) - @service = project.create_jira_service(active: true) - end - after do - @service.destroy! - end + project = create(:project) + service = project.create_jira_service(active: true) - it 'is prepopulated with the settings' do - expect(@service.properties['title']).to eq('Jira') - expect(@service.properties['url']).to eq('http://jira.sample/projects/project_a') - expect(@service.properties['api_url']).to eq('http://jira.sample/api') + expect(service.properties['url']).to eq('http://jira.sample/projects/project_a') + expect(service.properties['api_url']).to eq('http://jira.sample/api') end end end diff --git a/spec/models/project_services/redmine_service_spec.rb b/spec/models/project_services/redmine_service_spec.rb index ac570ac27e1..806e3695962 100644 --- a/spec/models/project_services/redmine_service_spec.rb +++ b/spec/models/project_services/redmine_service_spec.rb @@ -40,4 +40,47 @@ describe RedmineService do expect(described_class.reference_pattern.match('#123')[:issue]).to eq('123') end end + + context 'overriding properties' do + let(:url) { 'http://redmine.example.com' } + let(:access_params) do + { project_url: url, issues_url: url, new_issue_url: url } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) { create(:redmine_service, properties: properties) } + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:redmine_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:redmine_service, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:redmine_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('Redmine') + expect(service.description).to eq('Redmine issue tracker') + end + end + end end diff --git a/spec/models/project_services/youtrack_service_spec.rb b/spec/models/project_services/youtrack_service_spec.rb index bf9d892f66c..b47ef6702b4 100644 --- a/spec/models/project_services/youtrack_service_spec.rb +++ b/spec/models/project_services/youtrack_service_spec.rb @@ -37,4 +37,47 @@ describe YoutrackService do expect(described_class.reference_pattern.match('YT-123')[:issue]).to eq('YT-123') end end + + context 'overriding properties' do + let(:url) { 'http://youtrack.example.com' } + let(:access_params) do + { project_url: url, issues_url: url, new_issue_url: url } + end + + # this will be removed as part of https://gitlab.com/gitlab-org/gitlab-ce/issues/63084 + context 'when data are stored in properties' do + let(:properties) { access_params.merge(title: title, description: description) } + let(:service) { create(:youtrack_service, properties: properties) } + + include_examples 'issue tracker fields' + end + + context 'when data are stored in separated fields' do + let(:service) do + create(:youtrack_service, title: title, description: description, properties: access_params) + end + + include_examples 'issue tracker fields' + end + + context 'when data are stored in both properties and separated fields' do + let(:properties) { access_params.merge(title: 'wrong title', description: 'wrong description') } + let(:service) do + create(:youtrack_service, title: title, description: description, properties: properties) + end + + include_examples 'issue tracker fields' + end + + context 'when no title & description are set' do + let(:service) do + create(:youtrack_service, properties: access_params) + end + + it 'returns default values' do + expect(service.title).to eq('YouTrack') + expect(service.description).to eq('YouTrack issue tracker') + end + end + end end diff --git a/spec/models/release_spec.rb b/spec/models/release_spec.rb index 7c106ce6b85..e9d846e7291 100644 --- a/spec/models/release_spec.rb +++ b/spec/models/release_spec.rb @@ -64,4 +64,14 @@ RSpec.describe Release do is_expected.to all(be_a(Releases::Source)) end end + + describe '#upcoming_release?' do + context 'during the backfill migration when released_at could be nil' do + it 'handles a nil released_at value and returns false' do + allow(release).to receive(:released_at).and_return nil + + expect(release.upcoming_release?).to eq(false) + end + end + end end diff --git a/spec/models/service_spec.rb b/spec/models/service_spec.rb index d442c73c118..0797b9a9d83 100644 --- a/spec/models/service_spec.rb +++ b/spec/models/service_spec.rb @@ -244,7 +244,8 @@ describe Service do let(:service) do GitlabIssueTrackerService.create( project: create(:project), - title: 'random title' + title: 'random title', + project_url: 'http://gitlab.example.com' ) end @@ -252,8 +253,12 @@ describe Service do expect { service }.not_to raise_error end + it 'sets title correctly' do + expect(service.title).to eq('random title') + end + it 'creates the properties' do - expect(service.properties).to eq({ "title" => "random title" }) + expect(service.properties).to eq({ "project_url" => "http://gitlab.example.com" }) end end diff --git a/spec/requests/api/graphql/namespace/projects_spec.rb b/spec/requests/api/graphql/namespace/projects_spec.rb index de1cd9586b6..63fa16c79ca 100644 --- a/spec/requests/api/graphql/namespace/projects_spec.rb +++ b/spec/requests/api/graphql/namespace/projects_spec.rb @@ -58,9 +58,7 @@ describe 'getting projects', :nested_groups do it 'finds only public projects' do post_graphql(query, current_user: nil) - expect(graphql_data['namespace']['projects']['edges'].size).to eq(1) - project = graphql_data['namespace']['projects']['edges'][0]['node'] - expect(project['id']).to eq(public_project.to_global_id.to_s) + expect(graphql_data['namespace']).to be_nil end end end diff --git a/spec/requests/api/graphql/project/repository_spec.rb b/spec/requests/api/graphql/project/repository_spec.rb index 67af612a4a0..261433a3d6a 100644 --- a/spec/requests/api/graphql/project/repository_spec.rb +++ b/spec/requests/api/graphql/project/repository_spec.rb @@ -34,4 +34,28 @@ describe 'getting a repository in a project' do expect(graphql_data['project']).to be(nil) end end + + context 'when the repository is only accessible to members' do + let(:project) do + create(:project, :public, :repository, repository_access_level: ProjectFeature::PRIVATE) + end + + it 'returns a repository for the owner' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']['repository']).not_to be_nil + end + + it 'returns nil for the repository for other users' do + post_graphql(query, current_user: create(:user)) + + expect(graphql_data['project']['repository']).to be_nil + end + + it 'returns nil for the repository for other users' do + post_graphql(query, current_user: nil) + + expect(graphql_data['project']['repository']).to be_nil + end + end end diff --git a/spec/requests/api/issues/get_group_issues_spec.rb b/spec/requests/api/issues/get_group_issues_spec.rb index 8b02cf56e9f..9a41d790945 100644 --- a/spec/requests/api/issues/get_group_issues_spec.rb +++ b/spec/requests/api/issues/get_group_issues_spec.rb @@ -23,7 +23,11 @@ describe API::Issues do describe 'GET /groups/:id/issues' do let!(:group) { create(:group) } - let!(:group_project) { create(:project, :public, creator_id: user.id, namespace: group) } + let!(:group_project) { create(:project, :public, :repository, creator_id: user.id, namespace: group) } + let!(:private_mrs_project) do + create(:project, :public, :repository, creator_id: user.id, namespace: group, merge_requests_access_level: ProjectFeature::PRIVATE) + end + let!(:group_closed_issue) do create :closed_issue, author: user, @@ -234,6 +238,30 @@ describe API::Issues do it_behaves_like 'group issues statistics' end end + + context "when returns issue merge_requests_count for different access levels" do + let!(:merge_request1) do + create(:merge_request, + :simple, + author: user, + source_project: private_mrs_project, + target_project: private_mrs_project, + description: "closes #{group_issue.to_reference(private_mrs_project)}") + end + let!(:merge_request2) do + create(:merge_request, + :simple, + author: user, + source_project: group_project, + target_project: group_project, + description: "closes #{group_issue.to_reference}") + end + + it_behaves_like 'accessible merge requests count' do + let(:api_url) { base_url } + let(:target_issue) { group_issue } + end + end end end diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb index 0b0f754ab57..f7ca6fd1e0a 100644 --- a/spec/requests/api/issues/get_project_issues_spec.rb +++ b/spec/requests/api/issues/get_project_issues_spec.rb @@ -4,8 +4,9 @@ require 'spec_helper' describe API::Issues do set(:user) { create(:user) } - set(:project) do - create(:project, :public, creator_id: user.id, namespace: user.namespace) + set(:project) { create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace) } + set(:private_mrs_project) do + create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace, merge_requests_access_level: ProjectFeature::PRIVATE) end let(:user2) { create(:user) } @@ -60,9 +61,28 @@ describe API::Issues do let(:no_milestone_title) { 'None' } let(:any_milestone_title) { 'Any' } + let!(:merge_request1) do + create(:merge_request, + :simple, + author: user, + source_project: project, + target_project: project, + description: "closes #{issue.to_reference}") + end + let!(:merge_request2) do + create(:merge_request, + :simple, + author: user, + source_project: private_mrs_project, + target_project: private_mrs_project, + description: "closes #{issue.to_reference(private_mrs_project)}") + end + before(:all) do project.add_reporter(user) project.add_guest(guest) + private_mrs_project.add_reporter(user) + private_mrs_project.add_guest(guest) end before do @@ -257,6 +277,11 @@ describe API::Issues do expect_paginated_array_response(issue.id) end + it_behaves_like 'accessible merge requests count' do + let(:api_url) { "/projects/#{project.id}/issues" } + let(:target_issue) { issue } + end + context 'with labeled issues' do let(:label_b) { create(:label, title: 'foo', project: project) } let(:label_c) { create(:label, title: 'bar', project: project) } @@ -636,34 +661,26 @@ describe API::Issues do expect(json_response['iid']).to eq(confidential_issue.iid) end end - end - - describe 'GET :id/issues/:issue_iid/closed_by' do - let(:merge_request) do - create(:merge_request, - :simple, - author: user, - source_project: project, - target_project: project, - description: "closes #{issue.to_reference}") - end - before do - create(:merge_requests_closing_issues, issue: issue, merge_request: merge_request) + it_behaves_like 'accessible merge requests count' do + let(:api_url) { "/projects/#{project.id}/issues/#{issue.iid}" } + let(:target_issue) { issue } end + end + describe 'GET :id/issues/:issue_iid/closed_by' do context 'when unauthenticated' do it 'return public project issues' do get api("/projects/#{project.id}/issues/#{issue.iid}/closed_by") - expect_paginated_array_response(merge_request.id) + expect_paginated_array_response(merge_request1.id) end end it 'returns merge requests that will close issue on merge' do get api("/projects/#{project.id}/issues/#{issue.iid}/closed_by", user) - expect_paginated_array_response(merge_request.id) + expect_paginated_array_response(merge_request1.id) end context 'when no merge requests will close issue' do @@ -721,13 +738,6 @@ describe API::Issues do end it 'returns merge requests that mentioned a issue' do - create(:merge_request, - :simple, - author: user, - source_project: project, - target_project: project, - description: 'Some description') - get_related_merge_requests(project.id, issue.iid, user) expect_paginated_array_response(related_mr.id) diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index f32ffd1c77b..d195f54be11 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -4,8 +4,9 @@ require 'spec_helper' describe API::Issues do set(:user) { create(:user) } - set(:project) do - create(:project, :public, creator_id: user.id, namespace: user.namespace) + set(:project) { create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace) } + set(:private_mrs_project) do + create(:project, :public, :repository, creator_id: user.id, namespace: user.namespace, merge_requests_access_level: ProjectFeature::PRIVATE) end let(:user2) { create(:user) } @@ -63,6 +64,8 @@ describe API::Issues do before(:all) do project.add_reporter(user) project.add_guest(guest) + private_mrs_project.add_reporter(user) + private_mrs_project.add_guest(guest) end before do @@ -725,6 +728,30 @@ describe API::Issues do end end end + + context "when returns issue merge_requests_count for different access levels" do + let!(:merge_request1) do + create(:merge_request, + :simple, + author: user, + source_project: private_mrs_project, + target_project: private_mrs_project, + description: "closes #{issue.to_reference(private_mrs_project)}") + end + let!(:merge_request2) do + create(:merge_request, + :simple, + author: user, + source_project: project, + target_project: project, + description: "closes #{issue.to_reference}") + end + + it_behaves_like 'accessible merge requests count' do + let(:api_url) { "/issues" } + let(:target_issue) { issue } + end + end end describe 'DELETE /projects/:id/issues/:issue_iid' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 76d093e0774..a82ecb4fd63 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -834,6 +834,31 @@ describe API::MergeRequests do end end + context 'head_pipeline' do + before do + merge_request.update(head_pipeline: create(:ci_pipeline)) + merge_request.project.project_feature.update(builds_access_level: 10) + end + + context 'when user can read the pipeline' do + it 'exposes pipeline information' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) + + expect(json_response).to include('head_pipeline') + end + end + + context 'when user can not read the pipeline' do + let(:guest) { create(:user) } + + it 'does not expose pipeline information' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", guest) + + expect(json_response).not_to include('head_pipeline') + end + end + end + it 'returns the commits behind the target branch when include_diverged_commits_count is present' do allow_any_instance_of(merge_request.class).to receive(:diverged_commits_count).and_return(1) diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 8603fa2a73d..206e898381d 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -24,7 +24,7 @@ describe API::Releases do project: project, tag: 'v0.1', author: maintainer, - created_at: 2.days.ago) + released_at: 2.days.ago) end let!(:release_2) do @@ -32,7 +32,7 @@ describe API::Releases do project: project, tag: 'v0.2', author: maintainer, - created_at: 1.day.ago) + released_at: 1.day.ago) end it 'returns 200 HTTP status' do @@ -41,7 +41,7 @@ describe API::Releases do expect(response).to have_gitlab_http_status(:ok) end - it 'returns releases ordered by created_at' do + it 'returns releases ordered by released_at' do get api("/projects/#{project.id}/releases", maintainer) expect(json_response.count).to eq(2) @@ -56,6 +56,26 @@ describe API::Releases do end end + it 'returns an upcoming_release status for a future release' do + tomorrow = Time.now.utc + 1.day + create(:release, project: project, tag: 'v0.1', author: maintainer, released_at: tomorrow) + + get api("/projects/#{project.id}/releases", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.first['upcoming_release']).to eq(true) + end + + it 'returns an upcoming_release status for a past release' do + yesterday = Time.now.utc - 1.day + create(:release, project: project, tag: 'v0.1', author: maintainer, released_at: yesterday) + + get api("/projects/#{project.id}/releases", maintainer) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.first['upcoming_release']).to eq(false) + end + context 'when tag does not exist in git repository' do let!(:release) { create(:release, project: project, tag: 'v1.1.5') } @@ -316,6 +336,51 @@ describe API::Releases do expect(project.releases.last.description).to eq('Super nice release') end + it 'sets the released_at to the current time if the released_at parameter is not provided' do + now = Time.zone.parse('2015-08-25 06:00:00Z') + Timecop.freeze(now) do + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq(now) + end + end + + it 'sets the released_at to the value in the parameters if specified' do + params = { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release', + released_at: '2019-03-20T10:00:00Z' + } + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2019-03-20T10:00:00Z') + end + + it 'assumes the utc timezone for released_at if the timezone is not provided' do + params = { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release', + released_at: '2019-03-25 10:00:00' + } + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2019-03-25T10:00:00Z') + end + + it 'allows specifying a released_at with a local time zone' do + params = { + name: 'New release', + tag_name: 'v0.1', + description: 'Super nice release', + released_at: '2019-03-25T10:00:00+09:00' + } + post api("/projects/#{project.id}/releases", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2019-03-25T01:00:00Z') + end + context 'when description is empty' do let(:params) do { @@ -540,6 +605,7 @@ describe API::Releases do project: project, tag: 'v0.1', name: 'New release', + released_at: '2018-03-01T22:00:00Z', description: 'Super nice release') end @@ -560,6 +626,7 @@ describe API::Releases do expect(project.releases.last.tag).to eq('v0.1') expect(project.releases.last.name).to eq('New release') + expect(project.releases.last.released_at).to eq('2018-03-01T22:00:00Z') end it 'matches response schema' do @@ -568,6 +635,14 @@ describe API::Releases do expect(response).to match_response_schema('public_api/v4/release') end + it 'updates released_at' do + params = { released_at: '2015-10-10T05:00:00Z' } + + put api("/projects/#{project.id}/releases/v0.1", maintainer), params: params + + expect(project.releases.last.released_at).to eq('2015-10-10T05:00:00Z') + end + context 'when user tries to update sha' do let(:params) { { sha: 'xxx' } } diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index 89adbc77a7f..d832963292c 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -102,6 +102,27 @@ describe 'Rack Attack global throttles' do expect_rejection { get(*get_args) } end + + it 'logs RackAttack info into structured logs' do + requests_per_period.times do + get(*get_args) + expect(response).to have_http_status 200 + end + + arguments = { + message: 'Rack_Attack', + env: :throttle, + ip: '127.0.0.1', + request_method: 'GET', + fullpath: get_args.first, + user_id: user.id, + username: user.username + } + + expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once + + expect_rejection { get(*get_args) } + end end context 'when the throttle is disabled' do @@ -189,7 +210,15 @@ describe 'Rack Attack global throttles' do expect(response).to have_http_status 200 end - expect(Gitlab::AuthLogger).to receive(:error).once + arguments = { + message: 'Rack_Attack', + env: :throttle, + ip: '127.0.0.1', + request_method: 'GET', + fullpath: '/users/sign_in' + } + + expect(Gitlab::AuthLogger).to receive(:error).with(arguments) get url_that_does_not_require_authentication end @@ -345,7 +374,17 @@ describe 'Rack Attack global throttles' do expect(response).to have_http_status 200 end - expect(Gitlab::AuthLogger).to receive(:error).once + arguments = { + message: 'Rack_Attack', + env: :throttle, + ip: '127.0.0.1', + request_method: 'GET', + fullpath: '/dashboard/snippets', + user_id: user.id, + username: user.username + } + + expect(Gitlab::AuthLogger).to receive(:error).with(arguments).once get url_that_requires_authentication end diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 83775b1040e..6dde40d1cb6 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -693,4 +693,24 @@ describe 'project routing' do it_behaves_like 'redirecting a legacy project path', "/gitlab/gitlabhq/settings/repository", "/gitlab/gitlabhq/-/settings/repository" end + + describe Projects::TemplatesController, 'routing' do + describe '#show' do + def show_with_template_type(template_type) + "/gitlab/gitlabhq/templates/#{template_type}/template_name" + end + + it 'routes when :template_type is `merge_request`' do + expect(get(show_with_template_type('merge_request'))).to route_to('projects/templates#show', namespace_id: 'gitlab', project_id: 'gitlabhq', template_type: 'merge_request', key: 'template_name', format: 'json') + end + + it 'routes when :template_type is `issue`' do + expect(get(show_with_template_type('issue'))).to route_to('projects/templates#show', namespace_id: 'gitlab', project_id: 'gitlabhq', template_type: 'issue', key: 'template_name', format: 'json') + end + + it 'routes to application#route_not_found when :template_type is unknown' do + expect(get(show_with_template_type('invalid'))).to route_to('application#route_not_found', unmatched_route: 'gitlab/gitlabhq/templates/invalid/template_name') + end + end + end end diff --git a/spec/routing/uploads_routing_spec.rb b/spec/routing/uploads_routing_spec.rb index 6a041ffdd6c..42e84774088 100644 --- a/spec/routing/uploads_routing_spec.rb +++ b/spec/routing/uploads_routing_spec.rb @@ -12,10 +12,19 @@ describe 'Uploads', 'routing' do ) end + it 'allows creating uploads for users' do + expect(post('/uploads/user?id=1')).to route_to( + controller: 'uploads', + action: 'create', + model: 'user', + id: '1' + ) + end + it 'does not allow creating uploads for other models' do - UploadsController::MODEL_CLASSES.keys.compact.each do |model| - next if model == 'personal_snippet' + unroutable_models = UploadsController::MODEL_CLASSES.keys.compact - %w(personal_snippet user) + unroutable_models.each do |model| expect(post("/uploads/#{model}?id=1")).not_to be_routable end end diff --git a/spec/rubocop/cop/graphql/authorize_types_spec.rb b/spec/rubocop/cop/graphql/authorize_types_spec.rb new file mode 100644 index 00000000000..eae3e176d64 --- /dev/null +++ b/spec/rubocop/cop/graphql/authorize_types_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rubocop' +require 'rubocop/rspec/support' +require_relative '../../../../rubocop/cop/graphql/authorize_types' + +describe RuboCop::Cop::Graphql::AuthorizeTypes do + include RuboCop::RSpec::ExpectOffense + include CopHelper + + subject(:cop) { described_class.new } + + context 'when in a type folder' do + before do + allow(cop).to receive(:in_type?).and_return(true) + end + + it 'adds an offense when there is no authorize call' do + inspect_source(<<~TYPE) + module Types + class AType < BaseObject + field :a_thing + field :another_thing + end + end + TYPE + + expect(cop.offenses.size).to eq 1 + end + + it 'does not add an offense for classes that have an authorize call' do + expect_no_offenses(<<~TYPE.strip) + module Types + class AType < BaseObject + graphql_name 'ATypeName' + + authorize :an_ability, :second_ability + + field :a_thing + end + end + TYPE + end + + it 'does not add an offense for classes that only have an authorize call' do + expect_no_offenses(<<~TYPE.strip) + module Types + class AType < SuperClassWithFields + authorize :an_ability + end + end + TYPE + end + + it 'does not add an offense for base types' do + expect_no_offenses(<<~TYPE) + module Types + class AType < BaseEnum + field :a_thing + end + end + TYPE + end + end +end diff --git a/spec/services/branches/diverging_commit_counts_service_spec.rb b/spec/services/branches/diverging_commit_counts_service_spec.rb index bfdbebdb7c1..370da773ab2 100644 --- a/spec/services/branches/diverging_commit_counts_service_spec.rb +++ b/spec/services/branches/diverging_commit_counts_service_spec.rb @@ -19,34 +19,13 @@ describe Branches::DivergingCommitCountsService do expect(result).to eq(behind: 29, ahead: 2) end - context 'when gitaly_count_diverging_commits_no_max is enabled' do - before do - stub_feature_flags(gitaly_count_diverging_commits_no_max: true) - end - - it 'calls diverging_commit_count without max count' do - expect(repository.raw_repository) - .to receive(:diverging_commit_count) - .with(root_ref_sha, diverged_branch_sha) - .and_return([29, 2]) - - service.call(diverged_branch) - end - end - - context 'when gitaly_count_diverging_commits_no_max is disabled' do - before do - stub_feature_flags(gitaly_count_diverging_commits_no_max: false) - end - - it 'calls diverging_commit_count with max count' do - expect(repository.raw_repository) - .to receive(:diverging_commit_count) - .with(root_ref_sha, diverged_branch_sha, max_count: Repository::MAX_DIVERGING_COUNT) - .and_return([29, 2]) + it 'calls diverging_commit_count without max count' do + expect(repository.raw_repository) + .to receive(:diverging_commit_count) + .with(root_ref_sha, diverged_branch_sha) + .and_return([29, 2]) - service.call(diverged_branch) - end + service.call(diverged_branch) end end end diff --git a/spec/services/deploy_tokens/create_service_spec.rb b/spec/services/deploy_tokens/create_service_spec.rb index 886ffd4593d..fbb66fe4cb7 100644 --- a/spec/services/deploy_tokens/create_service_spec.rb +++ b/spec/services/deploy_tokens/create_service_spec.rb @@ -32,6 +32,22 @@ describe DeployTokens::CreateService do end end + context 'when username is empty string' do + let(:deploy_token_params) { attributes_for(:deploy_token, username: '') } + + it 'converts it to nil' do + expect(subject.read_attribute(:username)).to be_nil + end + end + + context 'when username is provided' do + let(:deploy_token_params) { attributes_for(:deploy_token, username: 'deployer') } + + it 'keeps the provided username' do + expect(subject.read_attribute(:username)).to eq('deployer') + end + end + context 'when the deploy token is invalid' do let(:deploy_token_params) { attributes_for(:deploy_token, read_repository: false, read_registry: false) } diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 97377c2f560..2a2547f9400 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -1148,7 +1148,7 @@ describe SystemNoteService do end it 'sets the note text' do - expect(subject.note).to eq 'resolved all discussions' + expect(subject.note).to eq 'resolved all threads' end end diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb index 0f8af2c5d6d..7c8e57702ae 100644 --- a/spec/support/features/discussion_comments_shared_example.rb +++ b/spec/support/features/discussion_comments_shared_example.rb @@ -1,4 +1,4 @@ -shared_examples 'discussion comments' do |resource_name| +shared_examples 'thread comments' do |resource_name| let(:form_selector) { '.js-main-target-form' } let(:dropdown_selector) { "#{form_selector} .comment-type-dropdown" } let(:toggle_selector) { "#{dropdown_selector} .dropdown-toggle" } @@ -48,7 +48,7 @@ shared_examples 'discussion comments' do |resource_name| find(toggle_selector).click end - it 'has a "Comment" item (selected by default) and "Start discussion" item' do + it 'has a "Comment" item (selected by default) and "Start thread" item' do expect(page).to have_selector menu_selector find("#{menu_selector} li", match: :first) @@ -59,7 +59,7 @@ shared_examples 'discussion comments' do |resource_name| expect(items.first).to have_selector '.fa-check' expect(items.first['class']).to match 'droplab-item-selected' - expect(items.last).to have_content 'Start discussion' + expect(items.last).to have_content 'Start thread' expect(items.last).to have_content "Discuss a specific suggestion or question#{' that needs to be resolved' if resource_name == 'merge request'}." expect(items.last).not_to have_selector '.fa-check' expect(items.last['class']).not_to match 'droplab-item-selected' @@ -103,7 +103,7 @@ shared_examples 'discussion comments' do |resource_name| expect(find(dropdown_selector)).to have_content 'Comment' end - describe 'when selecting "Start discussion"' do + describe 'when selecting "Start thread"' do before do find("#{menu_selector} li", match: :first) all("#{menu_selector} li").last.click @@ -114,9 +114,9 @@ shared_examples 'discussion comments' do |resource_name| # on issues page, the submit input is a <button>, on other pages it is <input> if button.tag_name == 'button' - expect(find(submit_selector)).to have_content 'Start discussion' + expect(find(submit_selector)).to have_content 'Start thread' else - expect(find(submit_selector).value).to eq 'Start discussion' + expect(find(submit_selector).value).to eq 'Start thread' end expect(page).not_to have_selector menu_selector @@ -124,17 +124,17 @@ shared_examples 'discussion comments' do |resource_name| if resource_name =~ /(issue|merge request)/ it 'updates the close button text' do - expect(find(close_selector)).to have_content "Start discussion & close #{resource_name}" + expect(find(close_selector)).to have_content "Start thread & close #{resource_name}" end it 'typing does not change the close button text' do find("#{form_selector} .note-textarea").send_keys('b') - expect(find(close_selector)).to have_content "Start discussion & close #{resource_name}" + expect(find(close_selector)).to have_content "Start thread & close #{resource_name}" end end - describe 'creating a discussion' do + describe 'creating a thread' do before do find(submit_selector).click wait_for_requests @@ -150,7 +150,7 @@ shared_examples 'discussion comments' do |resource_name| wait_for_requests end - it 'clicking "Start discussion" will post a discussion' do + it 'clicking "Start thread" will post a thread' do new_comment = all(comments_selector).last expect(new_comment).to have_content 'a' @@ -176,10 +176,10 @@ shared_examples 'discussion comments' do |resource_name| if resource_name == 'merge request' let(:note_id) { find("#{comments_selector} .note:first-child", match: :first)['data-note-id'] } - let(:reply_id) { find("#{comments_selector} .note:last-child", match: :first)['data-note-id'] } + let(:reply_id) { find("#{comments_selector} .note:last-of-type", match: :first)['data-note-id'] } it 'can be replied to after resolving' do - click_button "Resolve discussion" + click_button "Resolve thread" wait_for_requests refresh @@ -188,10 +188,10 @@ shared_examples 'discussion comments' do |resource_name| submit_reply('to reply or not reply') end - it 'shows resolved discussion when toggled' do + it 'shows resolved thread when toggled' do submit_reply('a') - click_button "Resolve discussion" + click_button "Resolve thread" wait_for_requests expect(page).to have_selector(".note-row-#{note_id}", visible: true) @@ -205,7 +205,7 @@ shared_examples 'discussion comments' do |resource_name| end if resource_name == 'issue' - it "clicking 'Start discussion & close #{resource_name}' will post a discussion and close the #{resource_name}" do + it "clicking 'Start thread & close #{resource_name}' will post a thread and close the #{resource_name}" do find(close_selector).click find(comments_selector, match: :first) @@ -224,7 +224,7 @@ shared_examples 'discussion comments' do |resource_name| find(toggle_selector).click end - it 'has "Start discussion" selected' do + it 'has "Start thread" selected' do find("#{menu_selector} li", match: :first) items = all("#{menu_selector} li") @@ -232,7 +232,7 @@ shared_examples 'discussion comments' do |resource_name| expect(items.first).not_to have_selector '.fa-check' expect(items.first['class']).not_to match 'droplab-item-selected' - expect(items.last).to have_content 'Start discussion' + expect(items.last).to have_content 'Start thread' expect(items.last).to have_selector '.fa-check' expect(items.last['class']).to match 'droplab-item-selected' end @@ -277,7 +277,7 @@ shared_examples 'discussion comments' do |resource_name| expect(items.first).to have_selector '.fa-check' expect(items.first['class']).to match 'droplab-item-selected' - expect(items.last).to have_content 'Start discussion' + expect(items.last).to have_content 'Start thread' expect(items.last).not_to have_selector '.fa-check' expect(items.last['class']).not_to match 'droplab-item-selected' end @@ -299,13 +299,13 @@ shared_examples 'discussion comments' do |resource_name| expect(find("#{form_selector} .js-note-target-reopen")).to have_content "Comment & reopen #{resource_name}" end - it "should show a 'Start discussion & reopen #{resource_name}' button when 'Start discussion' is selected" do + it "should show a 'Start thread & reopen #{resource_name}' button when 'Start thread' is selected" do find(toggle_selector).click find("#{menu_selector} li", match: :first) all("#{menu_selector} li").last.click - expect(find("#{form_selector} .js-note-target-reopen")).to have_content "Start discussion & reopen #{resource_name}" + expect(find("#{form_selector} .js-note-target-reopen")).to have_content "Start thread & reopen #{resource_name}" end end end diff --git a/spec/support/features/resolving_discussions_in_issues_shared_examples.rb b/spec/support/features/resolving_discussions_in_issues_shared_examples.rb index 38e5fb155a4..8d0e03134d0 100644 --- a/spec/support/features/resolving_discussions_in_issues_shared_examples.rb +++ b/spec/support/features/resolving_discussions_in_issues_shared_examples.rb @@ -1,4 +1,4 @@ -shared_examples 'creating an issue for a discussion' do +shared_examples 'creating an issue for a thread' do it 'shows an issue with the title filled in' do title_field = page.find_field('issue[title]') diff --git a/spec/support/matchers/gitaly_matchers.rb b/spec/support/matchers/gitaly_matchers.rb index ebfabcd8f24..933ed22b5d0 100644 --- a/spec/support/matchers/gitaly_matchers.rb +++ b/spec/support/matchers/gitaly_matchers.rb @@ -9,6 +9,6 @@ end RSpec::Matchers.define :gitaly_request_with_params do |params| match do |actual| - params.reduce(true) { |r, (key, val)| r && actual.send(key) == val } + params.reduce(true) { |r, (key, val)| r && actual[key.to_s] == val } end end diff --git a/spec/support/shared_examples/features/comments_on_merge_request_files_shared_examples.rb b/spec/support/shared_examples/features/comments_on_merge_request_files_shared_examples.rb index 221926aaf7e..2b36955a3c4 100644 --- a/spec/support/shared_examples/features/comments_on_merge_request_files_shared_examples.rb +++ b/spec/support/shared_examples/features/comments_on_merge_request_files_shared_examples.rb @@ -16,7 +16,7 @@ shared_examples 'comment on merge request file' do visit(merge_request_path(merge_request)) page.within('.notes .discussion') do - expect(page).to have_content("#{user.name} #{user.to_reference} started a discussion") + expect(page).to have_content("#{user.name} #{user.to_reference} started a thread") expect(page).to have_content(sample_commit.line_code_path) expect(page).to have_content('Line is wrong') end diff --git a/spec/support/shared_examples/models/services_fields_shared_examples.rb b/spec/support/shared_examples/models/services_fields_shared_examples.rb new file mode 100644 index 00000000000..6fbd0da9383 --- /dev/null +++ b/spec/support/shared_examples/models/services_fields_shared_examples.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +shared_examples 'issue tracker fields' do + let(:title) { 'custom title' } + let(:description) { 'custom description' } + let(:url) { 'http://issue_tracker.example.com' } + + context 'when data are stored in the properties' do + describe '#update' do + before do + service.update(title: 'new_title', description: 'new description') + end + + it 'removes title and description from properties' do + expect(service.reload.properties).not_to include('title', 'description') + end + + it 'stores title & description in services table' do + expect(service.read_attribute(:title)).to eq('new_title') + expect(service.read_attribute(:description)).to eq('new description') + end + end + + describe 'reading fields' do + it 'returns correct values' do + expect(service.title).to eq(title) + expect(service.description).to eq(description) + end + end + end +end diff --git a/spec/support/shared_examples/requests/api/issues/merge_requests_count_shared_examples.rb b/spec/support/shared_examples/requests/api/issues/merge_requests_count_shared_examples.rb new file mode 100644 index 00000000000..5f4e178f2e5 --- /dev/null +++ b/spec/support/shared_examples/requests/api/issues/merge_requests_count_shared_examples.rb @@ -0,0 +1,37 @@ +def get_issue + json_response.is_a?(Array) ? json_response.detect {|issue| issue['id'] == target_issue.id} : json_response +end + +shared_examples 'accessible merge requests count' do + it 'returns anonymous accessible merge requests count' do + get api(api_url), params: { scope: 'all' } + + issue = get_issue + expect(issue).not_to be_nil + expect(issue['merge_requests_count']).to eq(1) + end + + it 'returns guest accessible merge requests count' do + get api(api_url, guest), params: { scope: 'all' } + + issue = get_issue + expect(issue).not_to be_nil + expect(issue['merge_requests_count']).to eq(1) + end + + it 'returns reporter accessible merge requests count' do + get api(api_url, user), params: { scope: 'all' } + + issue = get_issue + expect(issue).not_to be_nil + expect(issue['merge_requests_count']).to eq(2) + end + + it 'returns admin accessible merge requests count' do + get api(api_url, admin), params: { scope: 'all' } + + issue = get_issue + expect(issue).not_to be_nil + expect(issue['merge_requests_count']).to eq(2) + end +end diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb index e474a714b10..5ee0a10f38d 100644 --- a/spec/uploaders/file_mover_spec.rb +++ b/spec/uploaders/file_mover_spec.rb @@ -3,79 +3,139 @@ require 'spec_helper' describe FileMover do include FileMoverHelpers + let(:user) { create(:user) } let(:filename) { 'banana_sample.gif' } - let(:temp_file_path) { File.join('uploads/-/system/temp', 'secret55', filename) } + let(:secret) { 'secret55' } + let(:temp_file_path) { File.join("uploads/-/system/user/#{user.id}", secret, filename) } let(:temp_description) do "test  "\ "same  " end - let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, 'secret55', filename) } + let(:file_path) { File.join('uploads/-/system/personal_snippet', snippet.id.to_s, secret, filename) } let(:snippet) { create(:personal_snippet, description: temp_description) } - subject { described_class.new(temp_file_path, snippet).execute } + let(:tmp_uploader) do + PersonalFileUploader.new(user, secret: secret) + end + + let(:file) { fixture_file_upload('spec/fixtures/banana_sample.gif') } + subject { described_class.new(temp_file_path, from_model: user, to_model: snippet).execute } describe '#execute' do - before do - expect(FileUtils).to receive(:mkdir_p).with(a_string_including(File.dirname(file_path))) - expect(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path)) - allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true) - allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10) + let(:tmp_upload) { tmp_uploader.upload } - stub_file_mover(temp_file_path) + before do + tmp_uploader.store!(file) end - context 'when move and field update successful' do - it 'updates the description correctly' do - subject + context 'local storage' do + before do + allow(FileUtils).to receive(:mkdir_p).with(a_string_including(File.dirname(file_path))) + allow(FileUtils).to receive(:move).with(a_string_including(temp_file_path), a_string_including(file_path)) + allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:exists?).and_return(true) + allow_any_instance_of(CarrierWave::SanitizedFile).to receive(:size).and_return(10) - expect(snippet.reload.description) - .to eq( - "test  "\ - "same  " - ) + stub_file_mover(temp_file_path) end - it 'creates a new update record' do - expect { subject }.to change { Upload.count }.by(1) + context 'when move and field update successful' do + it 'updates the description correctly' do + subject + + expect(snippet.reload.description) + .to eq("test  "\ + "same  ") + end + + it 'updates existing upload record' do + expect { subject } + .to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') } + .from([user.id, 'User']).to([snippet.id, 'Snippet']) + end + + it 'schedules a background migration' do + expect_any_instance_of(PersonalFileUploader).to receive(:schedule_background_upload).once + + subject + end end - it 'schedules a background migration' do - expect_any_instance_of(PersonalFileUploader).to receive(:schedule_background_upload).once + context 'when update_markdown fails' do + before do + expect(FileUtils).to receive(:move).with(a_string_including(file_path), a_string_including(temp_file_path)) + end - subject + subject { described_class.new(file_path, :non_existing_field, from_model: user, to_model: snippet).execute } + + it 'does not update the description' do + subject + + expect(snippet.reload.description) + .to eq("test  "\ + "same  ") + end + + it 'does not change the upload record' do + expect { subject } + .not_to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') } + end end end - context 'when update_markdown fails' do + context 'when tmp uploader is not local storage' do before do - expect(FileUtils).to receive(:move).with(a_string_including(file_path), a_string_including(temp_file_path)) + stub_uploads_object_storage(uploader: PersonalFileUploader) + allow_any_instance_of(PersonalFileUploader).to receive(:file_storage?) { false } end - subject { described_class.new(file_path, snippet, :non_existing_field).execute } + after do + FileUtils.rm_f(File.join('personal_snippet', snippet.id.to_s, secret, filename)) + end - it 'does not update the description' do - subject + context 'when move and field update successful' do + it 'updates the description correctly' do + subject - expect(snippet.reload.description) - .to eq( - "test  "\ - "same  " - ) + expect(snippet.reload.description) + .to eq("test  "\ + "same  ") + end + + it 'creates new target upload record an delete the old upload' do + expect { subject } + .to change { Upload.last.attributes.values_at('model_id', 'model_type') } + .from([user.id, 'User']).to([snippet.id, 'Snippet']) + + expect(Upload.count).to eq(1) + end end - it 'does not create a new update record' do - expect { subject }.not_to change { Upload.count } + context 'when update_markdown fails' do + subject { described_class.new(file_path, :non_existing_field, from_model: user, to_model: snippet).execute } + + it 'does not update the description' do + subject + + expect(snippet.reload.description) + .to eq("test  "\ + "same  ") + end + + it 'does not change the upload record' do + expect { subject } + .to change { Upload.last.attributes.values_at('model_id', 'model_type') }.from([user.id, 'User']) + end end end end context 'security' do context 'when relative path is involved' do - let(:temp_file_path) { File.join('uploads/-/system/temp', '..', 'another_subdir_of_temp') } + let(:temp_file_path) { File.join("uploads/-/system/user/#{user.id}", '..', 'another_subdir_of_temp') } it 'does not trigger move if path is outside designated directory' do - stub_file_mover('uploads/-/system/another_subdir_of_temp') + stub_file_mover("uploads/-/system/user/#{user.id}/another_subdir_of_temp") expect(FileUtils).not_to receive(:move) subject diff --git a/spec/validators/color_validator_spec.rb b/spec/validators/color_validator_spec.rb new file mode 100644 index 00000000000..e5a38ac9372 --- /dev/null +++ b/spec/validators/color_validator_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ColorValidator do + using RSpec::Parameterized::TableSyntax + + subject do + Class.new do + include ActiveModel::Model + include ActiveModel::Validations + attr_accessor :color + validates :color, color: true + end.new + end + + where(:color, :is_valid) do + '#000abc' | true + '#aaa' | true + '#BBB' | true + '#cCc' | true + '#ffff' | false + '#000111222' | false + 'invalid' | false + '000' | false + end + + with_them do + it 'only accepts valid colors' do + subject.color = color + + expect(subject.valid?).to eq(is_valid) + end + end + + it 'fails fast for long invalid string' do + subject.color = '#' + ('0' * 50_000) + 'xxx' + + expect do + Timeout.timeout(5.seconds) { subject.valid? } + end.not_to raise_error + end +end |