Commit 46516411 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'web_editor_branch' into 'master'

Web Editor: save to new branch

Implements #1931

See merge request !1523
parents 428df28c 53e40414
...@@ -7,6 +7,7 @@ v 7.9.0 (unreleased) ...@@ -7,6 +7,7 @@ v 7.9.0 (unreleased)
- Improve UI for commits, issues and merge request lists - Improve UI for commits, issues and merge request lists
- Fix commit comments on first line of diff not rendering in Merge Request Discussion view. - Fix commit comments on first line of diff not rendering in Merge Request Discussion view.
- Improve trigger merge request hook when source project branch has been updated (Kirill Zaitsev) - Improve trigger merge request hook when source project branch has been updated (Kirill Zaitsev)
- Save web edit in new branch
v 7.8.0 v 7.8.0
- Fix access control and protection against XSS for note attachments and other uploads. - Fix access control and protection against XSS for note attachments and other uploads.
......
# Controller for viewing a file's blame # Controller for viewing a file's blame
class Projects::BlobController < Projects::ApplicationController class Projects::BlobController < Projects::ApplicationController
include ExtractsPath include ExtractsPath
include ActionView::Helpers::SanitizeHelper
# Raised when given an invalid file path # Raised when given an invalid file path
class InvalidPathError < StandardError; end class InvalidPathError < StandardError; end
...@@ -21,11 +22,18 @@ class Projects::BlobController < Projects::ApplicationController ...@@ -21,11 +22,18 @@ class Projects::BlobController < Projects::ApplicationController
def create def create
file_path = File.join(@path, File.basename(params[:file_name])) file_path = File.join(@path, File.basename(params[:file_name]))
result = Files::CreateService.new(@project, current_user, params, @ref, file_path).execute result = Files::CreateService.new(
@project,
current_user,
params.merge(new_branch: sanitized_new_branch_name),
@ref,
file_path
).execute
if result[:status] == :success if result[:status] == :success
flash[:notice] = "Your changes have been successfully committed" flash[:notice] = "Your changes have been successfully committed"
redirect_to namespace_project_blob_path(@project.namespace, @project, File.join(@ref, file_path)) ref = sanitized_new_branch_name.presence || @ref
redirect_to namespace_project_blob_path(@project.namespace, @project, File.join(ref, file_path))
else else
flash[:alert] = result[:message] flash[:alert] = result[:message]
render :new render :new
...@@ -41,7 +49,13 @@ class Projects::BlobController < Projects::ApplicationController ...@@ -41,7 +49,13 @@ class Projects::BlobController < Projects::ApplicationController
def update def update
result = Files::UpdateService. result = Files::UpdateService.
new(@project, current_user, params, @ref, @path).execute new(
@project,
current_user,
params.merge(new_branch: sanitized_new_branch_name),
@ref,
@path
).execute
if result[:status] == :success if result[:status] == :success
flash[:notice] = "Your changes have been successfully committed" flash[:notice] = "Your changes have been successfully committed"
...@@ -131,6 +145,8 @@ class Projects::BlobController < Projects::ApplicationController ...@@ -131,6 +145,8 @@ class Projects::BlobController < Projects::ApplicationController
if from_merge_request if from_merge_request
diffs_namespace_project_merge_request_path(from_merge_request.target_project.namespace, from_merge_request.target_project, from_merge_request) + diffs_namespace_project_merge_request_path(from_merge_request.target_project.namespace, from_merge_request.target_project, from_merge_request) +
"#file-path-#{hexdigest(@path)}" "#file-path-#{hexdigest(@path)}"
elsif sanitized_new_branch_name.present?
namespace_project_blob_path(@project.namespace, @project, File.join(sanitized_new_branch_name, @path))
else else
namespace_project_blob_path(@project.namespace, @project, @id) namespace_project_blob_path(@project.namespace, @project, @id)
end end
...@@ -140,4 +156,8 @@ class Projects::BlobController < Projects::ApplicationController ...@@ -140,4 +156,8 @@ class Projects::BlobController < Projects::ApplicationController
# If blob edit was initiated from merge request page # If blob edit was initiated from merge request page
@from_merge_request ||= MergeRequest.find_by(id: params[:from_merge_request_id]) @from_merge_request ||= MergeRequest.find_by(id: params[:from_merge_request_id])
end end
def sanitized_new_branch_name
@new_branch ||= sanitize(strip_tags(params[:new_branch]))
end
end end
...@@ -38,7 +38,8 @@ module Files ...@@ -38,7 +38,8 @@ module Files
created_successfully = new_file_action.commit!( created_successfully = new_file_action.commit!(
params[:content], params[:content],
params[:commit_message], params[:commit_message],
params[:encoding] params[:encoding],
params[:new_branch]
) )
if created_successfully if created_successfully
......
...@@ -23,7 +23,8 @@ module Files ...@@ -23,7 +23,8 @@ module Files
edit_file_action.commit!( edit_file_action.commit!(
params[:content], params[:content],
params[:commit_message], params[:commit_message],
params[:encoding] params[:encoding],
params[:new_branch]
) )
success success
......
...@@ -14,6 +14,13 @@ ...@@ -14,6 +14,13 @@
= render 'projects/blob/editor', ref: @ref, path: @path, blob_data: @blob.data = render 'projects/blob/editor', ref: @ref, path: @path, blob_data: @blob.data
= render 'shared/commit_message_container', params: params, = render 'shared/commit_message_container', params: params,
placeholder: "Update #{@blob.name}" placeholder: "Update #{@blob.name}"
.form-group.branch
= label_tag 'branch', class: 'control-label' do
Branch
.col-sm-10
= text_field_tag 'new_branch', @ref, class: "form-control"
= hidden_field_tag 'last_commit', @last_commit = hidden_field_tag 'last_commit', @last_commit
= hidden_field_tag 'content', '', id: "file-content" = hidden_field_tag 'content', '', id: "file-content"
= hidden_field_tag 'from_merge_request_id', params[:from_merge_request_id] = hidden_field_tag 'from_merge_request_id', params[:from_merge_request_id]
......
...@@ -4,6 +4,13 @@ ...@@ -4,6 +4,13 @@
= render 'projects/blob/editor', ref: @ref = render 'projects/blob/editor', ref: @ref
= render 'shared/commit_message_container', params: params, = render 'shared/commit_message_container', params: params,
placeholder: 'Add new file' placeholder: 'Add new file'
.form-group.branch
= label_tag 'branch', class: 'control-label' do
Branch
.col-sm-10
= text_field_tag 'new_branch', @ref, class: "form-control"
= hidden_field_tag 'content', '', id: 'file-content' = hidden_field_tag 'content', '', id: 'file-content'
= render 'projects/commit_button', ref: @ref, = render 'projects/commit_button', ref: @ref,
cancel_path: namespace_project_tree_path(@project.namespace, @project, @id) cancel_path: namespace_project_tree_path(@project.namespace, @project, @id)
......
...@@ -10,7 +10,7 @@ to create the first file and open it in the web editor. ...@@ -10,7 +10,7 @@ to create the first file and open it in the web editor.
![web editor 1](web_editor/empty_project.png) ![web editor 1](web_editor/empty_project.png)
Fill in a file name, some content, a commit message and press the commit button. Fill in a file name, some content, a commit message, branch name and press the commit button.
The file will be saved to the repository. The file will be saved to the repository.
![web editor 2](web_editor/new_file.png) ![web editor 2](web_editor/new_file.png)
...@@ -21,6 +21,6 @@ viewing the file. ...@@ -21,6 +21,6 @@ viewing the file.
![web editor 3](web_editor/show_file.png) ![web editor 3](web_editor/show_file.png)
Editing a file is almost the same as creating a new file, Editing a file is almost the same as creating a new file,
with as addition the ability to preview your changes in a separate tab. with as addition the ability to preview your changes in a separate tab. Also you can save your change to another branch by filling out field `branch`
![web editor 3](web_editor/edit_file.png) ![web editor 3](web_editor/edit_file.png)
doc/workflow/web_editor/new_file.png

98.2 KB | W: | H:

doc/workflow/web_editor/new_file.png

83.5 KB | W: | H:

doc/workflow/web_editor/new_file.png
doc/workflow/web_editor/new_file.png
doc/workflow/web_editor/new_file.png
doc/workflow/web_editor/new_file.png
  • 2-up
  • Swipe
  • Onion skin
...@@ -34,6 +34,17 @@ Feature: Project Source Browse Files ...@@ -34,6 +34,17 @@ Feature: Project Source Browse Files
Then I am redirected to the new file Then I am redirected to the new file
And I should see its new content And I should see its new content
@javascript
Scenario: I can create and commit file and specify new branch
Given I click on "new file" link in repo
And I edit code
And I fill the new file name
And I fill the commit message
And I fill the new branch name
And I click on "Commit Changes"
Then I am redirected to the new file on new branch
And I should see its new content
@javascript @tricky @javascript @tricky
Scenario: I can create file in empty repo Scenario: I can create file in empty repo
Given I own an empty project Given I own an empty project
...@@ -83,6 +94,17 @@ Feature: Project Source Browse Files ...@@ -83,6 +94,17 @@ Feature: Project Source Browse Files
Then I am redirected to the ".gitignore" Then I am redirected to the ".gitignore"
And I should see its new content And I should see its new content
@javascript
Scenario: I can edit and commit file to new branch
Given I click on ".gitignore" file in repo
And I click button "Edit"
And I edit code
And I fill the commit message
And I fill the new branch name
And I click on "Commit Changes"
Then I am redirected to the ".gitignore" on new branch
And I should see its new content
@javascript @wip @javascript @wip
Scenario: If I don't change the content of the file I see an error message Scenario: If I don't change the content of the file I see an error message
Given I click on ".gitignore" file in repo Given I click on ".gitignore" file in repo
......
...@@ -69,6 +69,10 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps ...@@ -69,6 +69,10 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
fill_in :file_name, with: new_file_name fill_in :file_name, with: new_file_name
end end
step 'I fill the new branch name' do
fill_in :new_branch, with: 'new_branch_name'
end
step 'I fill the new file name with an illegal name' do step 'I fill the new file name with an illegal name' do
fill_in :file_name, with: '.git' fill_in :file_name, with: '.git'
end end
...@@ -148,6 +152,10 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps ...@@ -148,6 +152,10 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
expect(current_path).to eq(namespace_project_blob_path(@project.namespace, @project, 'master/.gitignore')) expect(current_path).to eq(namespace_project_blob_path(@project.namespace, @project, 'master/.gitignore'))
end end
step 'I am redirected to the ".gitignore" on new branch' do
expect(current_path).to eq(namespace_project_blob_path(@project.namespace, @project, 'new_branch_name/.gitignore'))
end
step 'I am redirected to the permalink URL' do step 'I am redirected to the permalink URL' do
expect(current_path).to( expect(current_path).to(
eq(namespace_project_blob_path(@project.namespace, @project, eq(namespace_project_blob_path(@project.namespace, @project,
...@@ -161,6 +169,11 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps ...@@ -161,6 +169,11 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
@project.namespace, @project, 'master/' + new_file_name)) @project.namespace, @project, 'master/' + new_file_name))
end end
step 'I am redirected to the new file on new branch' do
expect(current_path).to eq(namespace_project_blob_path(
@project.namespace, @project, 'new_branch_name/' + new_file_name))
end
step "I don't see the permalink link" do step "I don't see the permalink link" do
expect(page).not_to have_link('permalink') expect(page).not_to have_link('permalink')
end end
...@@ -177,7 +190,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps ...@@ -177,7 +190,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
click_link 'add a file' click_link 'add a file'
# Remove pre-receive hook so we can push without auth # Remove pre-receive hook so we can push without auth
FileUtils.rm(File.join(@project.repository.path, 'hooks', 'pre-receive')) FileUtils.rm_f(File.join(@project.repository.path, 'hooks', 'pre-receive'))
end end
private private
......
...@@ -10,7 +10,7 @@ module Gitlab ...@@ -10,7 +10,7 @@ module Gitlab
# Returns false if committing the change fails # Returns false if committing the change fails
# Returns false if pushing from the satellite to bare repo failed or was rejected # Returns false if pushing from the satellite to bare repo failed or was rejected
# Returns true otherwise # Returns true otherwise
def commit!(content, commit_message, encoding) def commit!(content, commit_message, encoding, new_branch = nil)
in_locked_and_timed_satellite do |repo| in_locked_and_timed_satellite do |repo|
prepare_satellite!(repo) prepare_satellite!(repo)
...@@ -42,10 +42,12 @@ module Gitlab ...@@ -42,10 +42,12 @@ module Gitlab
end end
target_branch = new_branch.present? ? "#{ref}:#{new_branch}" : ref
# push commit back to bare repo # push commit back to bare repo
# will raise CommandFailed when push fails # will raise CommandFailed when push fails
begin begin
repo.git.push({ raise: true, timeout: true }, :origin, ref) repo.git.push({ raise: true, timeout: true }, :origin, target_branch)
rescue Grit::Git::CommandFailed => ex rescue Grit::Git::CommandFailed => ex
log_and_raise(PushFailed, ex.message) log_and_raise(PushFailed, ex.message)
end end
......
...@@ -9,7 +9,7 @@ module Gitlab ...@@ -9,7 +9,7 @@ module Gitlab
# Returns false if committing the change fails # Returns false if committing the change fails
# Returns false if pushing from the satellite to bare repo failed or was rejected # Returns false if pushing from the satellite to bare repo failed or was rejected
# Returns true otherwise # Returns true otherwise
def commit!(content, commit_message, encoding) def commit!(content, commit_message, encoding, new_branch = nil)
in_locked_and_timed_satellite do |repo| in_locked_and_timed_satellite do |repo|
prepare_satellite!(repo) prepare_satellite!(repo)
...@@ -45,9 +45,15 @@ module Gitlab ...@@ -45,9 +45,15 @@ module Gitlab
# will raise CommandFailed when commit fails # will raise CommandFailed when commit fails
repo.git.commit(raise: true, timeout: true, a: true, m: commit_message) repo.git.commit(raise: true, timeout: true, a: true, m: commit_message)
target_branch = if new_branch.present? && !@project.empty_repo?
"#{ref}:#{new_branch}"
else
"#{current_ref}:#{ref}"
end
# push commit back to bare repo # push commit back to bare repo
# will raise CommandFailed when push fails # will raise CommandFailed when push fails
repo.git.push({ raise: true, timeout: true }, :origin, "#{current_ref}:#{ref}") repo.git.push({ raise: true, timeout: true }, :origin, target_branch)
# everything worked # everything worked
true true
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment