Merge branch '39441-bring-edit-form-back' into 'master'

Revert "Merge branch '36670-remove-edit-form' into 'master'"

See merge request gitlab-org/gitlab-ee!3208

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: [:update, :move]
+  before_action :authorize_update_issue!, only: [:edit, :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]
@@ -65,6 +65,10 @@ 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)
@@ -124,6 +128,10 @@ 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

app/views/projects/issues/edit.html.haml

+- page_title "Edit", "#{@issue.title} (#{@issue.to_reference})", "Issues"
+
+%h3.page-title
+  Edit Issue ##{@issue.iid}
+%hr
+
+= render "form"

changelogs/unreleased/39441-bring-edit-form-back.yml

+---
+title: Fix editing issue description in mobile view
+merge_request:
+author:
+type: fixed

spec/controllers/projects/issues_controller_spec.rb

@@ -557,6 +557,29 @@ describe Projects::IssuesController do
         end
       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
+
+      def go(id:)
+        put :update,
+          namespace_id: project.namespace.to_param,
+          project_id: project,
+          id: id,
+          issue: { title: 'New title' }
+      end
+    end
   end
 
   describe 'POST #create' do

spec/features/issues/form_spec.rb

@@ -222,15 +222,54 @@ describe 'New/edit issue', :js do
 
   context 'edit issue' do
     before do
-      visit project_issue_path(project, issue)
-      page.within('.content .issuable-actions') do
-        click_on 'Edit'
+      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
       end
     end
 
     it 'description has autocomplete' do
-      find_field('issue-description').native.send_keys('')
-      fill_in 'issue-description', with: '@'
+      find('#issue_description').native.send_keys('')
+      fill_in 'issue_description', with: '@'
 
       expect(page).to have_selector('.atwho-view')
     end

spec/features/issues_spec.rb

 require 'spec_helper'
 
-describe 'Issues', :js do
+describe 'Issues' do
   include DropzoneHelper
   include IssueHelpers
   include SortingHelper
@@ -24,15 +24,109 @@ describe 'Issues', :js do
     end
 
     before do
-      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
+      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.description)
+      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 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 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
+      end
+    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
     end
   end
 

spec/features/security/project/internal_access_spec.rb

@@ -192,6 +192,21 @@ 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) }
 

spec/features/security/project/private_access_spec.rb

@@ -191,6 +191,21 @@ 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) }
 

spec/features/security/project/public_access_spec.rb

@@ -413,6 +413,21 @@ 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) }
 

spec/support/update_invalid_issuable.rb

@@ -25,14 +25,14 @@ shared_examples 'update invalid issuable' do |klass|
         .and_raise(ActiveRecord::StaleObjectError.new(issuable, :save))
     end
 
-    if klass == MergeRequest
-      it 'renders edit when format is html' do
-        put :update, params
+    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
 
-        expect(assigns[:suggested_approvers]).to be_an(Array) if issuable.requires_approve?
+      if klass == MergeRequest && issuable.requires_approve?
+        expect(assigns[:suggested_approvers]).to be_an(Array)
       end
     end
 
@@ -46,17 +46,16 @@ shared_examples 'update invalid issuable' do |klass|
     end
   end
 
-  if klass == MergeRequest
-    context 'when updating an invalid issuable' do
-      before do
-        params[:merge_request][:title] = ""
-      end
+  context 'when updating an invalid issuable' do
+    before do
+      key = klass == Issue ? :issue : :merge_request
+      params[key][: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)
-      end
+      expect(response).to render_template(:edit)
     end
   end
 end