summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/controllers/projects/issues_controller.rb10
-rw-r--r--app/views/projects/issues/edit.html.haml7
-rw-r--r--changelogs/unreleased/36670-remove-edit-form.yml5
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb36
-rw-r--r--spec/features/issues/form_spec.rb49
-rw-r--r--spec/features/issues_spec.rb108
-rw-r--r--spec/features/projects/issuable_templates_spec.rb46
-rw-r--r--spec/features/security/project/internal_access_spec.rb15
-rw-r--r--spec/features/security/project/private_access_spec.rb15
-rw-r--r--spec/features/security/project/public_access_spec.rb15
-rw-r--r--spec/support/update_invalid_issuable.rb27
11 files changed, 62 insertions, 271 deletions
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index a3ec79a56d9..ee6e6f80cdd 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -16,7 +16,7 @@ class Projects::IssuesController < Projects::ApplicationController
before_action :authorize_create_issue!, only: [:new, :create]
# Allow modify issue
- before_action :authorize_update_issue!, only: [:edit, :update, :move]
+ before_action :authorize_update_issue!, only: [:update, :move]
# Allow create a new branch and empty WIP merge request from current issue
before_action :authorize_create_merge_request!, only: [:create_merge_request]
@@ -63,10 +63,6 @@ class Projects::IssuesController < Projects::ApplicationController
respond_with(@issue)
end
- def edit
- respond_with(@issue)
- end
-
def show
@noteable = @issue
@note = @project.notes.new(noteable: @issue)
@@ -126,10 +122,6 @@ class Projects::IssuesController < Projects::ApplicationController
@issue = Issues::UpdateService.new(project, current_user, update_params).execute(issue)
respond_to do |format|
- format.html do
- recaptcha_check_with_fallback { render :edit }
- end
-
format.json do
render_issue_json
end
diff --git a/app/views/projects/issues/edit.html.haml b/app/views/projects/issues/edit.html.haml
deleted file mode 100644
index 1b7d878c38c..00000000000
--- a/app/views/projects/issues/edit.html.haml
+++ /dev/null
@@ -1,7 +0,0 @@
-- page_title "Edit", "#{@issue.title} (#{@issue.to_reference})", "Issues"
-
-%h3.page-title
- Edit Issue ##{@issue.iid}
-%hr
-
-= render "form"
diff --git a/changelogs/unreleased/36670-remove-edit-form.yml b/changelogs/unreleased/36670-remove-edit-form.yml
new file mode 100644
index 00000000000..4e80b685f67
--- /dev/null
+++ b/changelogs/unreleased/36670-remove-edit-form.yml
@@ -0,0 +1,5 @@
+---
+title: Remove the ability to visit the issue edit form directly
+merge_request: 14523
+author:
+type: removed
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index b4a22a46b51..166a299fc80 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -259,7 +259,7 @@ describe Projects::IssuesController do
context 'when captcha is not verified' do
def update_spam_issue
- update_issue(title: 'Spam Title', description: 'Spam lives here')
+ update_issue({ title: 'Spam Title', description: 'Spam lives here' }, format: :json)
end
before do
@@ -267,7 +267,6 @@ describe Projects::IssuesController do
end
it 'rejects an issue recognized as a spam' do
- expect(Gitlab::Recaptcha).to receive(:load_configurations!).and_return(true)
expect { update_spam_issue }.not_to change { issue.reload.title }
end
@@ -287,14 +286,6 @@ describe Projects::IssuesController do
expect(spam_logs.first.recaptcha_verified).to be_falsey
end
- context 'as HTML' do
- it 'renders verify template' do
- update_spam_issue
-
- expect(response).to render_template(:verify)
- end
- end
-
context 'as JSON' do
before do
update_issue({ title: 'Spam Title', description: 'Spam lives here' }, format: :json)
@@ -318,7 +309,7 @@ describe Projects::IssuesController do
def update_verified_issue
update_issue({ title: spammy_title },
{ spam_log_id: spam_logs.last.id,
- recaptcha_verification: true })
+ recaptcha_verification: true, format: :json })
end
before do
@@ -326,11 +317,8 @@ describe Projects::IssuesController do
.and_return(true)
end
- it 'redirect to issue page' do
- update_verified_issue
-
- expect(response)
- .to redirect_to(project_issue_path(project, issue))
+ it 'returns 200 status' do
+ expect(response).to have_http_status(200)
end
it 'accepts an issue after recaptcha is verified' do
@@ -574,26 +562,16 @@ describe Projects::IssuesController do
end
end
- describe 'GET #edit' do
- it_behaves_like 'restricted action', success: 200
-
- def go(id:)
- get :edit,
- namespace_id: project.namespace.to_param,
- project_id: project,
- id: id
- end
- end
-
describe 'PUT #update' do
- it_behaves_like 'restricted action', success: 302
+ it_behaves_like 'restricted action', success: 200
def go(id:)
put :update,
namespace_id: project.namespace.to_param,
project_id: project,
id: id,
- issue: { title: 'New title' }
+ issue: { title: 'New title' },
+ format: :json
end
end
end
diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb
index 2db6f9a2982..8ce470fc288 100644
--- a/spec/features/issues/form_spec.rb
+++ b/spec/features/issues/form_spec.rb
@@ -218,54 +218,15 @@ describe 'New/edit issue', :js do
context 'edit issue' do
before do
- visit edit_project_issue_path(project, issue)
- end
-
- it 'allows user to update issue' do
- expect(find('input[name="issue[assignee_ids][]"]', visible: false).value).to match(user.id.to_s)
- expect(find('input[name="issue[milestone_id]"]', visible: false).value).to match(milestone.id.to_s)
- expect(find('a', text: 'Assign to me', visible: false)).not_to be_visible
-
- page.within '.js-user-search' do
- expect(page).to have_content user.name
- end
-
- page.within '.js-milestone-select' do
- expect(page).to have_content milestone.title
- end
-
- click_button 'Labels'
- page.within '.dropdown-menu-labels' do
- click_link label.title
- click_link label2.title
- end
- page.within '.js-label-select' do
- expect(page).to have_content label.title
- end
- expect(page.all('input[name="issue[label_ids][]"]', visible: false)[1].value).to match(label.id.to_s)
- expect(page.all('input[name="issue[label_ids][]"]', visible: false)[2].value).to match(label2.id.to_s)
-
- click_button 'Save changes'
-
- page.within '.issuable-sidebar' do
- page.within '.assignee' do
- expect(page).to have_content user.name
- end
-
- page.within '.milestone' do
- expect(page).to have_content milestone.title
- end
-
- page.within '.labels' do
- expect(page).to have_content label.title
- expect(page).to have_content label2.title
- end
+ visit project_issue_path(project, issue)
+ page.within('.content .issuable-actions') do
+ click_on 'Edit'
end
end
it 'description has autocomplete' do
- find('#issue_description').native.send_keys('')
- fill_in 'issue_description', with: '@'
+ find_field('issue-description').native.send_keys('')
+ fill_in 'issue-description', with: '@'
expect(page).to have_selector('.atwho-view')
end
diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb
index fb763c93c66..30310055dc5 100644
--- a/spec/features/issues_spec.rb
+++ b/spec/features/issues_spec.rb
@@ -1,6 +1,6 @@
require 'spec_helper'
-describe 'Issues' do
+describe 'Issues', :js do
include DropzoneHelper
include IssueHelpers
include SortingHelper
@@ -24,109 +24,15 @@ describe 'Issues' do
end
before do
- visit edit_project_issue_path(project, issue)
- find('.js-zen-enter').click
- end
-
- it 'opens new issue popup' do
- expect(page).to have_content("Issue ##{issue.iid}")
- end
- end
-
- describe 'Editing issue assignee' do
- let!(:issue) do
- create(:issue,
- author: user,
- assignees: [user],
- project: project)
- end
-
- it 'allows user to select unassigned', js: true do
- visit edit_project_issue_path(project, issue)
-
- expect(page).to have_content "Assignee #{user.name}"
-
- first('.js-user-search').click
- click_link 'Unassigned'
-
- click_button 'Save changes'
-
- page.within('.assignee') do
- expect(page).to have_content 'No assignee - assign yourself'
- end
-
- expect(issue.reload.assignees).to be_empty
- end
- end
-
- describe 'due date', js: true do
- context 'on new form' do
- before do
- visit new_project_issue_path(project)
- end
-
- it 'saves with due date' do
- date = Date.today.at_beginning_of_month
-
- fill_in 'issue_title', with: 'bug 345'
- fill_in 'issue_description', with: 'bug description'
- find('#issuable-due-date').click
-
- page.within '.pika-single' do
- click_button date.day
- end
-
- expect(find('#issuable-due-date').value).to eq date.to_s
-
- click_button 'Submit issue'
-
- page.within '.issuable-sidebar' do
- expect(page).to have_content date.to_s(:medium)
- end
+ visit project_issue_path(project, issue)
+ page.within('.content .issuable-actions') do
+ find('.issuable-edit').click
end
+ find('.issue-details .content-block .js-zen-enter').click
end
- context 'on edit form' do
- let(:issue) { create(:issue, author: user, project: project, due_date: Date.today.at_beginning_of_month.to_s) }
-
- before do
- visit edit_project_issue_path(project, issue)
- end
-
- it 'saves with due date' do
- date = Date.today.at_beginning_of_month
-
- expect(find('#issuable-due-date').value).to eq date.to_s
-
- date = date.tomorrow
-
- fill_in 'issue_title', with: 'bug 345'
- fill_in 'issue_description', with: 'bug description'
- find('#issuable-due-date').click
-
- page.within '.pika-single' do
- click_button date.day
- end
-
- expect(find('#issuable-due-date').value).to eq date.to_s
-
- click_button 'Save changes'
-
- page.within '.issuable-sidebar' do
- expect(page).to have_content date.to_s(:medium)
- end
- end
-
- it 'warns about version conflict' do
- issue.update(title: "New title")
-
- fill_in 'issue_title', with: 'bug 345'
- fill_in 'issue_description', with: 'bug description'
-
- click_button 'Save changes'
-
- expect(page).to have_content 'Someone edited the issue the same time you did'
- end
+ it 'opens new issue popup' do
+ expect(page).to have_content(issue.description)
end
end
diff --git a/spec/features/projects/issuable_templates_spec.rb b/spec/features/projects/issuable_templates_spec.rb
index d2789d0aa52..1f9b52dd998 100644
--- a/spec/features/projects/issuable_templates_spec.rb
+++ b/spec/features/projects/issuable_templates_spec.rb
@@ -3,6 +3,7 @@ require 'spec_helper'
feature 'issuable templates', js: true do
let(:user) { create(:user) }
let(:project) { create(:project, :public, :repository) }
+ let(:issue_form_location) { '#content-body .issuable-details .detail-page-description' }
before do
project.team << [user, :master]
@@ -28,14 +29,17 @@ feature 'issuable templates', js: true do
longtemplate_content,
message: 'added issue template',
branch_name: 'master')
- visit edit_project_issue_path project, issue
- fill_in :'issue[title]', with: 'test issue title'
+ visit project_issue_path project, issue
+ page.within('.content .issuable-actions') do
+ click_on 'Edit'
+ end
+ fill_in :'issue-title', with: 'test issue title'
end
scenario 'user selects "bug" template' do
select_template 'bug'
wait_for_requests
- assert_template
+ assert_template(page_part: issue_form_location)
save_changes
end
@@ -43,30 +47,19 @@ feature 'issuable templates', js: true do
select_template 'bug'
wait_for_requests
select_option 'No template'
- assert_template('')
+ assert_template(expected_content: '', page_part: issue_form_location)
save_changes('')
end
scenario 'user selects "bug" template, edits description and then selects "reset template"' do
select_template 'bug'
wait_for_requests
- find_field('issue_description').send_keys(description_addition)
- assert_template(template_content + description_addition)
+ find_field('issue-description').send_keys(description_addition)
+ assert_template(expected_content: template_content + description_addition, page_part: issue_form_location)
select_option 'Reset template'
- assert_template
+ assert_template(page_part: issue_form_location)
save_changes
end
-
- it 'updates height of markdown textarea' do
- start_height = page.evaluate_script('$(".markdown-area").outerHeight()')
-
- select_template 'test'
- wait_for_requests
-
- end_height = page.evaluate_script('$(".markdown-area").outerHeight()')
-
- expect(end_height).not_to eq(start_height)
- end
end
context 'user creates an issue using templates, with a prior description' do
@@ -81,15 +74,18 @@ feature 'issuable templates', js: true do
template_content,
message: 'added issue template',
branch_name: 'master')
- visit edit_project_issue_path project, issue
- fill_in :'issue[title]', with: 'test issue title'
- fill_in :'issue[description]', with: prior_description
+ visit project_issue_path project, issue
+ page.within('.content .issuable-actions') do
+ click_on 'Edit'
+ end
+ fill_in :'issue-title', with: 'test issue title'
+ fill_in :'issue-description', with: prior_description
end
scenario 'user selects "bug" template' do
select_template 'bug'
wait_for_requests
- assert_template("#{template_content}")
+ assert_template(page_part: issue_form_location)
save_changes
end
end
@@ -154,8 +150,10 @@ feature 'issuable templates', js: true do
end
end
- def assert_template(expected_content = template_content)
- expect(find('textarea')['value']).to eq(expected_content)
+ def assert_template(expected_content: template_content, page_part: '#content-body')
+ page.within(page_part) do
+ expect(find('textarea')['value']).to eq(expected_content)
+ end
end
def save_changes(expected_content = template_content)
diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb
index a7928857b7d..d70cf1527e7 100644
--- a/spec/features/security/project/internal_access_spec.rb
+++ b/spec/features/security/project/internal_access_spec.rb
@@ -181,21 +181,6 @@ describe "Internal Project Access" do
it { is_expected.to be_denied_for(:visitor) }
end
- describe "GET /:project_path/issues/:id/edit" do
- let(:issue) { create(:issue, project: project) }
- subject { edit_project_issue_path(project, issue) }
-
- it { is_expected.to be_allowed_for(:admin) }
- it { is_expected.to be_allowed_for(:owner).of(project) }
- it { is_expected.to be_allowed_for(:master).of(project) }
- it { is_expected.to be_allowed_for(:developer).of(project) }
- it { is_expected.to be_allowed_for(:reporter).of(project) }
- it { is_expected.to be_denied_for(:guest).of(project) }
- it { is_expected.to be_denied_for(:user) }
- it { is_expected.to be_denied_for(:external) }
- it { is_expected.to be_denied_for(:visitor) }
- end
-
describe "GET /:project_path/snippets" do
subject { project_snippets_path(project) }
diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb
index a4396b20afd..ea130606545 100644
--- a/spec/features/security/project/private_access_spec.rb
+++ b/spec/features/security/project/private_access_spec.rb
@@ -181,21 +181,6 @@ describe "Private Project Access" do
it { is_expected.to be_denied_for(:visitor) }
end
- describe "GET /:project_path/issues/:id/edit" do
- let(:issue) { create(:issue, project: project) }
- subject { edit_project_issue_path(project, issue) }
-
- it { is_expected.to be_allowed_for(:admin) }
- it { is_expected.to be_allowed_for(:owner).of(project) }
- it { is_expected.to be_allowed_for(:master).of(project) }
- it { is_expected.to be_allowed_for(:developer).of(project) }
- it { is_expected.to be_allowed_for(:reporter).of(project) }
- it { is_expected.to be_denied_for(:guest).of(project) }
- it { is_expected.to be_denied_for(:user) }
- it { is_expected.to be_denied_for(:external) }
- it { is_expected.to be_denied_for(:visitor) }
- end
-
describe "GET /:project_path/snippets" do
subject { project_snippets_path(project) }
diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb
index fccdeb0e5b7..d15f5af66c9 100644
--- a/spec/features/security/project/public_access_spec.rb
+++ b/spec/features/security/project/public_access_spec.rb
@@ -394,21 +394,6 @@ describe "Public Project Access" do
it { is_expected.to be_allowed_for(:visitor) }
end
- describe "GET /:project_path/issues/:id/edit" do
- let(:issue) { create(:issue, project: project) }
- subject { edit_project_issue_path(project, issue) }
-
- it { is_expected.to be_allowed_for(:admin) }
- it { is_expected.to be_allowed_for(:owner).of(project) }
- it { is_expected.to be_allowed_for(:master).of(project) }
- it { is_expected.to be_allowed_for(:developer).of(project) }
- it { is_expected.to be_allowed_for(:reporter).of(project) }
- it { is_expected.to be_denied_for(:guest).of(project) }
- it { is_expected.to be_denied_for(:user) }
- it { is_expected.to be_denied_for(:external) }
- it { is_expected.to be_denied_for(:visitor) }
- end
-
describe "GET /:project_path/snippets" do
subject { project_snippets_path(project) }
diff --git a/spec/support/update_invalid_issuable.rb b/spec/support/update_invalid_issuable.rb
index 1490287681b..50a1d4a56e2 100644
--- a/spec/support/update_invalid_issuable.rb
+++ b/spec/support/update_invalid_issuable.rb
@@ -25,11 +25,13 @@ shared_examples 'update invalid issuable' do |klass|
.and_raise(ActiveRecord::StaleObjectError.new(issuable, :save))
end
- it 'renders edit when format is html' do
- put :update, params
+ if klass == MergeRequest
+ it 'renders edit when format is html' do
+ put :update, params
- expect(response).to render_template(:edit)
- expect(assigns[:conflict]).to be_truthy
+ expect(response).to render_template(:edit)
+ expect(assigns[:conflict]).to be_truthy
+ end
end
it 'renders json error message when format is json' do
@@ -42,16 +44,17 @@ shared_examples 'update invalid issuable' do |klass|
end
end
- context 'when updating an invalid issuable' do
- before do
- key = klass == Issue ? :issue : :merge_request
- params[key][:title] = ""
- end
+ if klass == MergeRequest
+ context 'when updating an invalid issuable' do
+ before do
+ params[:merge_request][:title] = ""
+ end
- it 'renders edit when merge request is invalid' do
- put :update, params
+ it 'renders edit when merge request is invalid' do
+ put :update, params
- expect(response).to render_template(:edit)
+ expect(response).to render_template(:edit)
+ end
end
end
end