Commit b42dc1a5 authored by Eric Eastwood's avatar Eric Eastwood Committed by Alfredo Sumaran

Linking to edit file directly

parent 3f60fe1a
function BlobForkSuggestion(openButton, cancelButton, suggestionSection) {
if (openButton) {
openButton.addEventListener('click', () => {
suggestionSection.classList.remove('hidden');
});
}
if (cancelButton) {
cancelButton.addEventListener('click', () => {
suggestionSection.classList.add('hidden');
});
}
}
export default BlobForkSuggestion;
...@@ -43,6 +43,7 @@ import GroupsList from './groups_list'; ...@@ -43,6 +43,7 @@ import GroupsList from './groups_list';
import ProjectsList from './projects_list'; import ProjectsList from './projects_list';
import MiniPipelineGraph from './mini_pipeline_graph_dropdown'; import MiniPipelineGraph from './mini_pipeline_graph_dropdown';
import BlobLinePermalinkUpdater from './blob/blob_line_permalink_updater'; import BlobLinePermalinkUpdater from './blob/blob_line_permalink_updater';
import BlobForkSuggestion from './blob/blob_fork_suggestion';
import UserCallout from './user_callout'; import UserCallout from './user_callout';
const ShortcutsBlob = require('./shortcuts_blob'); const ShortcutsBlob = require('./shortcuts_blob');
...@@ -86,6 +87,12 @@ const ShortcutsBlob = require('./shortcuts_blob'); ...@@ -86,6 +87,12 @@ const ShortcutsBlob = require('./shortcuts_blob');
skipResetBindings: true, skipResetBindings: true,
fileBlobPermalinkUrl, fileBlobPermalinkUrl,
}); });
new BlobForkSuggestion(
document.querySelector('.js-edit-blob-link-fork-toggler'),
document.querySelector('.js-cancel-fork-suggestion'),
document.querySelector('.js-file-fork-suggestion-section'),
);
} }
switch (page) { switch (page) {
......
...@@ -281,3 +281,16 @@ span.idiff { ...@@ -281,3 +281,16 @@ span.idiff {
display: none; display: none;
} }
} }
.file-fork-suggestion {
display: flex;
align-items: center;
justify-content: flex-end;
background-color: $gray-light;
border-bottom: 1px solid $border-color;
padding: 5px $gl-padding;
}
.file-fork-suggestion-note {
margin-right: 1.5em;
}
...@@ -7,9 +7,11 @@ class Projects::BlobController < Projects::ApplicationController ...@@ -7,9 +7,11 @@ class Projects::BlobController < Projects::ApplicationController
# Raised when given an invalid file path # Raised when given an invalid file path
InvalidPathError = Class.new(StandardError) InvalidPathError = Class.new(StandardError)
prepend_before_action :authenticate_user!, only: [:edit]
before_action :require_non_empty_project, except: [:new, :create] before_action :require_non_empty_project, except: [:new, :create]
before_action :authorize_download_code! before_action :authorize_download_code!
before_action :authorize_edit_tree!, only: [:new, :create, :edit, :update, :destroy] before_action :authorize_edit_tree!, only: [:new, :create, :update, :destroy]
before_action :assign_blob_vars before_action :assign_blob_vars
before_action :commit, except: [:new, :create] before_action :commit, except: [:new, :create]
before_action :blob, except: [:new, :create] before_action :blob, except: [:new, :create]
...@@ -37,7 +39,11 @@ class Projects::BlobController < Projects::ApplicationController ...@@ -37,7 +39,11 @@ class Projects::BlobController < Projects::ApplicationController
end end
def edit def edit
if can_collaborate_with_project?
blob.load_all_data!(@repository) blob.load_all_data!(@repository)
else
redirect_to action: 'show'
end
end end
def update def update
......
...@@ -8,31 +8,36 @@ module BlobHelper ...@@ -8,31 +8,36 @@ module BlobHelper
%w(credits changelog news copying copyright license authors) %w(credits changelog news copying copyright license authors)
end end
def edit_blob_link(project = @project, ref = @ref, path = @path, options = {}) def edit_path(project = @project, ref = @ref, path = @path, options = {})
return unless current_user namespace_project_edit_blob_path(project.namespace, project,
blob = options.delete(:blob)
blob ||= project.repository.blob_at(ref, path) rescue nil
return unless blob
edit_path = namespace_project_edit_blob_path(project.namespace, project,
tree_join(ref, path), tree_join(ref, path),
options[:link_opts]) options[:link_opts])
end
if !on_top_of_branch?(project, ref) def fork_path(project = @project, ref = @ref, path = @path, options = {})
button_tag "Edit", class: "btn disabled has-tooltip", title: "You can only edit files when you are on a branch", data: { container: 'body' }
elsif can_edit_blob?(blob, project, ref)
link_to "Edit", edit_path, class: 'btn btn-sm'
elsif can?(current_user, :fork_project, project)
continue_params = { continue_params = {
to: edit_path, to: edit_path,
notice: edit_in_new_fork_notice, notice: edit_in_new_fork_notice,
notice_now: edit_in_new_fork_notice_now notice_now: edit_in_new_fork_notice_now
} }
fork_path = namespace_project_forks_path(project.namespace, project, namespace_key: current_user.namespace.id, continue: continue_params) namespace_project_forks_path(project.namespace, project, namespace_key: current_user.namespace.id, continue: continue_params)
end
link_to "Edit", fork_path, class: 'btn', method: :post def edit_blob_link(project = @project, ref = @ref, path = @path, options = {})
blob = options.delete(:blob)
blob ||= project.repository.blob_at(ref, path) rescue nil
return unless blob
common_classes = "btn js-edit-blob #{options[:extra_class]}"
if !on_top_of_branch?(project, ref)
button_tag 'Edit', class: "#{common_classes} disabled has-tooltip", title: "You can only edit files when you are on a branch", data: { container: 'body' }
# This condition applies to anonymous or users who can edit directly
elsif !current_user || (current_user && can_edit_blob?(blob, project, ref))
link_to 'Edit', edit_path(project, ref, path, options), class: "#{common_classes} btn-sm"
elsif current_user && can?(current_user, :fork_project, project)
button_tag 'Edit', class: "#{common_classes} js-edit-blob-link-fork-toggler"
end end
end end
......
...@@ -25,4 +25,11 @@ ...@@ -25,4 +25,11 @@
#blob-content-holder.blob-content-holder #blob-content-holder.blob-content-holder
%article.file-holder %article.file-holder
= render "projects/blob/header", blob: blob = render "projects/blob/header", blob: blob
- if current_user
.js-file-fork-suggestion-section.file-fork-suggestion.hidden
%span.file-fork-suggestion-note
You don't have permission to edit this file. Try forking this project to edit the file.
= link_to 'Fork', fork_path, method: :post, class: 'btn btn-grouped btn-inverted btn-new'
%button.js-cancel-fork-suggestion.btn.btn-grouped{ type: 'button' }
Cancel
= render blob.to_partial_path(@project), blob: blob = render blob.to_partial_path(@project), blob: blob
...@@ -32,8 +32,8 @@ ...@@ -32,8 +32,8 @@
= link_to 'Permalink', namespace_project_blob_path(@project.namespace, @project, = link_to 'Permalink', namespace_project_blob_path(@project.namespace, @project,
tree_join(@commit.sha, @path)), class: 'btn btn-sm js-data-file-blob-permalink-url' tree_join(@commit.sha, @path)), class: 'btn btn-sm js-data-file-blob-permalink-url'
- if current_user
.btn-group{ role: "group" }< .btn-group{ role: "group" }<
= edit_blob_link if blob_text_viewable?(blob) = edit_blob_link if blob_text_viewable?(blob)
- if current_user
= replace_blob_link = replace_blob_link
= delete_blob_link = delete_blob_link
---
title: Linking to blob edit page handles anonymous users and users without enough permissions
to edit directly
merge_request:
author:
...@@ -158,6 +158,8 @@ Feature: Project Source Browse Files ...@@ -158,6 +158,8 @@ Feature: Project Source Browse Files
Given I don't have write access Given I don't have write access
And I click on ".gitignore" file in repo And I click on ".gitignore" file in repo
And I click button "Edit" And I click button "Edit"
Then I should see a Fork/Cancel combo
And I click button "Fork"
Then I should see a notice about a new fork having been created Then I should see a notice about a new fork having been created
And I can edit code And I can edit code
...@@ -180,6 +182,8 @@ Feature: Project Source Browse Files ...@@ -180,6 +182,8 @@ Feature: Project Source Browse Files
Given I don't have write access Given I don't have write access
And I click on ".gitignore" file in repo And I click on ".gitignore" file in repo
And I click button "Edit" And I click button "Edit"
Then I should see a Fork/Cancel combo
And I click button "Fork"
And I edit code And I edit code
And I fill the commit message And I fill the commit message
And I click on "Commit Changes" And I click on "Commit Changes"
......
...@@ -56,13 +56,17 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps ...@@ -56,13 +56,17 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
end end
step 'I click button "Edit"' do step 'I click button "Edit"' do
click_link 'Edit' find('.js-edit-blob').click
end end
step 'I cannot see the edit button' do step 'I cannot see the edit button' do
expect(page).not_to have_link 'edit' expect(page).not_to have_link 'edit'
end end
step 'I click button "Fork"' do
click_link 'Fork'
end
step 'I can edit code' do step 'I can edit code' do
set_new_content set_new_content
expect(evaluate_script('ace.edit("editor").getValue()')).to eq new_gitignore_content expect(evaluate_script('ace.edit("editor").getValue()')).to eq new_gitignore_content
...@@ -366,6 +370,12 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps ...@@ -366,6 +370,12 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
end end
end end
step 'I should see a Fork/Cancel combo' do
expect(page).to have_link 'Fork'
expect(page).to have_button 'Cancel'
expect(page).to have_content 'You don\'t have permission to edit this file. Try forking this project to edit the file.'
end
step 'I should see a notice about a new fork having been created' do step 'I should see a notice about a new fork having been created' do
expect(page).to have_content "You're not allowed to make changes to this project directly. A fork of this project has been created that you can make changes in, so you can submit a merge request." expect(page).to have_content "You're not allowed to make changes to this project directly. A fork of this project has been created that you can make changes in, so you can submit a merge request."
end end
......
...@@ -2,15 +2,10 @@ require 'rails_helper' ...@@ -2,15 +2,10 @@ require 'rails_helper'
describe Projects::BlobController do describe Projects::BlobController do
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
let(:user) { create(:user) }
before do
project.team << [user, :master]
sign_in(user)
end
describe 'GET diff' do describe 'GET diff' do
let(:user) { create(:user) }
render_views render_views
def do_get(opts = {}) def do_get(opts = {})
...@@ -20,6 +15,12 @@ describe Projects::BlobController do ...@@ -20,6 +15,12 @@ describe Projects::BlobController do
get :diff, params.merge(opts) get :diff, params.merge(opts)
end end
before do
project.team << [user, :master]
sign_in(user)
end
context 'when essential params are missing' do context 'when essential params are missing' do
it 'renders nothing' do it 'renders nothing' do
do_get do_get
...@@ -37,7 +38,69 @@ describe Projects::BlobController do ...@@ -37,7 +38,69 @@ describe Projects::BlobController do
end end
end end
describe 'GET edit' do
let(:default_params) do
{
namespace_id: project.namespace,
project_id: project,
id: 'master/CHANGELOG'
}
end
context 'anonymous' do
before do
get :edit, default_params
end
it 'redirects to sign in and returns' do
expect(response).to redirect_to(new_user_session_path)
end
end
context 'as guest' do
let(:guest) { create(:user) }
before do
sign_in(guest)
get :edit, default_params
end
it 'redirects to blob show' do
expect(response).to redirect_to(namespace_project_blob_path(project.namespace, project, 'master/CHANGELOG'))
end
end
context 'as developer' do
let(:developer) { create(:user) }
before do
project.team << [developer, :developer]
sign_in(developer)
get :edit, default_params
end
it 'redirects to blob show' do
expect(response).to have_http_status(200)
end
end
context 'as master' do
let(:master) { create(:user) }
before do
project.team << [master, :master]
sign_in(master)
get :edit, default_params
end
it 'redirects to blob show' do
expect(response).to have_http_status(200)
end
end
end
describe 'PUT update' do describe 'PUT update' do
let(:user) { create(:user) }
let(:default_params) do let(:default_params) do
{ {
namespace_id: project.namespace, namespace_id: project.namespace,
...@@ -53,6 +116,12 @@ describe Projects::BlobController do ...@@ -53,6 +116,12 @@ describe Projects::BlobController do
namespace_project_blob_path(project.namespace, project, 'master/CHANGELOG') namespace_project_blob_path(project.namespace, project, 'master/CHANGELOG')
end end
before do
project.team << [user, :master]
sign_in(user)
end
it 'redirects to blob' do it 'redirects to blob' do
put :update, default_params put :update, default_params
......
require 'spec_helper'
feature 'File blob', feature: true do
include WaitForAjax
include TreeHelper
let(:project) { create(:project, :public, :test_repo) }
let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') }
let(:branch) { 'master' }
let(:file_path) { project.repository.ls_files(project.repository.root_ref)[1] }
context 'anonymous' do
context 'from blob file path' do
before do
visit namespace_project_blob_path(project.namespace, project, tree_join(branch, file_path))
end
it 'updates content' do
expect(page).to have_link 'Edit'
end
end
end
end
...@@ -2,20 +2,25 @@ require 'spec_helper' ...@@ -2,20 +2,25 @@ require 'spec_helper'
feature 'Editing file blob', feature: true, js: true do feature 'Editing file blob', feature: true, js: true do
include WaitForAjax include WaitForAjax
include TreeHelper
given(:user) { create(:user) } let(:project) { create(:project, :public, :test_repo) }
given(:role) { :developer } let(:merge_request) { create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master') }
given(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') } let(:branch) { 'master' }
given(:project) { merge_request.target_project } let(:file_path) { project.repository.ls_files(project.repository.root_ref)[1] }
background do context 'as a developer' do
login_as(user) let(:user) { create(:user) }
let(:role) { :developer }
before do
project.team << [user, role] project.team << [user, role]
login_as(user)
end end
def edit_and_commit def edit_and_commit
wait_for_ajax wait_for_ajax
first('.file-actions').click_link 'Edit' find('.js-edit-blob').click
execute_script('ace.edit("editor").setValue("class NextFeature\nend\n")') execute_script('ace.edit("editor").setValue("class NextFeature\nend\n")')
click_button 'Commit Changes' click_button 'Commit Changes'
end end
...@@ -26,20 +31,106 @@ feature 'Editing file blob', feature: true, js: true do ...@@ -26,20 +31,106 @@ feature 'Editing file blob', feature: true, js: true do
edit_and_commit edit_and_commit
end end
scenario 'returns me to the mr' do it 'returns me to the mr' do
expect(page).to have_content(merge_request.title) expect(page).to have_content(merge_request.title)
end end
end end
context 'from blob file path' do context 'from blob file path' do
before do before do
visit namespace_project_blob_path(project.namespace, project, '/feature/files/ruby/feature.rb') visit namespace_project_blob_path(project.namespace, project, tree_join(branch, file_path))
edit_and_commit edit_and_commit
end end
scenario 'updates content' do it 'updates content' do
expect(page).to have_content 'successfully committed' expect(page).to have_content 'successfully committed'
expect(page).to have_content 'NextFeature' expect(page).to have_content 'NextFeature'
end end
end end
end
context 'visit blob edit' do
context 'redirects to sign in and returns' do
context 'as developer' do
let(:user) { create(:user) }
before do
project.team << [user, :developer]
visit namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path))
end
it 'redirects to sign in and returns' do
expect(page).to have_current_path(new_user_session_path)
login_as(user)
expect(page).to have_current_path(namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path)))
end
end
context 'as guest' do
let(:user) { create(:user) }
before do
visit namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path))
end
it 'redirects to sign in and returns' do
expect(page).to have_current_path(new_user_session_path)
login_as(user)
expect(page).to have_current_path(namespace_project_blob_path(project.namespace, project, tree_join(branch, file_path)))
end
end
end
context 'as developer' do
let(:user) { create(:user) }
let(:protected_branch) { 'protected-branch' }
before do
project.team << [user, :developer]
project.repository.add_branch(user, protected_branch, 'master')
create(:protected_branch, project: project, name: protected_branch)
login_as(user)
end
context 'on some branch' do
before do
visit namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path))
end
it 'shows blob editor with same branch' do
expect(page).to have_current_path(namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path)))
expect(find('.js-target-branch .dropdown-toggle-text').text).to eq(branch)
end
end
context 'with protected branch' do
before do
visit namespace_project_edit_blob_path(project.namespace, project, tree_join(protected_branch, file_path))
end
it 'shows blob editor with patch branch' do
expect(find('.js-target-branch .dropdown-toggle-text').text).to eq('patch-1')
end
end
end
context 'as master' do
let(:user) { create(:user) }
before do
project.team << [user, :master]
login_as(user)
visit namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path))
end
it 'shows blob editor with same branch' do
expect(page).to have_current_path(namespace_project_edit_blob_path(project.namespace, project, tree_join(branch, file_path)))
expect(find('.js-target-branch .dropdown-toggle-text').text).to eq(branch)
end
end
end
end end
...@@ -73,7 +73,7 @@ describe BlobHelper do ...@@ -73,7 +73,7 @@ describe BlobHelper do
let(:project) { create(:project, :repository, namespace: namespace) } let(:project) { create(:project, :repository, namespace: namespace) }
before do before do
allow(self).to receive(:current_user).and_return(double) allow(self).to receive(:current_user).and_return(nil)
allow(self).to receive(:can_collaborate_with_project?).and_return(true) allow(self).to receive(:can_collaborate_with_project?).and_return(true)
end end
......
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