summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/blob/blob_fork_suggestion.js15
-rw-r--r--app/assets/javascripts/dispatcher.js7
-rw-r--r--app/assets/stylesheets/framework/files.scss13
-rw-r--r--app/controllers/projects/blob_controller.rb10
-rw-r--r--app/helpers/blob_helper.rb39
-rw-r--r--app/views/projects/blob/_blob.html.haml7
-rw-r--r--app/views/projects/blob/_header.html.haml6
-rw-r--r--changelogs/unreleased/28899-linking-to-edit-file.yml5
-rw-r--r--features/project/source/browse_files.feature4
-rw-r--r--features/steps/project/source/browse_files.rb12
-rw-r--r--spec/controllers/projects/blob_controller_spec.rb83
-rw-r--r--spec/features/projects/blobs/blob_show_spec.rb23
-rw-r--r--spec/features/projects/blobs/edit_spec.rb145
-rw-r--r--spec/helpers/blob_helper_spec.rb2
14 files changed, 313 insertions, 58 deletions
diff --git a/app/assets/javascripts/blob/blob_fork_suggestion.js b/app/assets/javascripts/blob/blob_fork_suggestion.js
new file mode 100644
index 00000000000..aa9a4e1c99a
--- /dev/null
+++ b/app/assets/javascripts/blob/blob_fork_suggestion.js
@@ -0,0 +1,15 @@
+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;
diff --git a/app/assets/javascripts/dispatcher.js b/app/assets/javascripts/dispatcher.js
index 9c7acc903d1..307c6a306d1 100644
--- a/app/assets/javascripts/dispatcher.js
+++ b/app/assets/javascripts/dispatcher.js
@@ -43,6 +43,7 @@ import GroupsList from './groups_list';
import ProjectsList from './projects_list';
import MiniPipelineGraph from './mini_pipeline_graph_dropdown';
import BlobLinePermalinkUpdater from './blob/blob_line_permalink_updater';
+import BlobForkSuggestion from './blob/blob_fork_suggestion';
import UserCallout from './user_callout';
const ShortcutsBlob = require('./shortcuts_blob');
@@ -86,6 +87,12 @@ const ShortcutsBlob = require('./shortcuts_blob');
skipResetBindings: true,
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) {
diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss
index ddea1cf540b..a5a8522739e 100644
--- a/app/assets/stylesheets/framework/files.scss
+++ b/app/assets/stylesheets/framework/files.scss
@@ -281,3 +281,16 @@ span.idiff {
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;
+}
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 80a95c6158b..73706bf8dae 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -7,9 +7,11 @@ class Projects::BlobController < Projects::ApplicationController
# Raised when given an invalid file path
InvalidPathError = Class.new(StandardError)
+ prepend_before_action :authenticate_user!, only: [:edit]
+
before_action :require_non_empty_project, except: [:new, :create]
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 :commit, except: [:new, :create]
before_action :blob, except: [:new, :create]
@@ -37,7 +39,11 @@ class Projects::BlobController < Projects::ApplicationController
end
def edit
- blob.load_all_data!(@repository)
+ if can_collaborate_with_project?
+ blob.load_all_data!(@repository)
+ else
+ redirect_to action: 'show'
+ end
end
def update
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 8631bc54509..91d6d1852cf 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -8,31 +8,36 @@ module BlobHelper
%w(credits changelog news copying copyright license authors)
end
- def edit_blob_link(project = @project, ref = @ref, path = @path, options = {})
- return unless current_user
+ def edit_path(project = @project, ref = @ref, path = @path, options = {})
+ namespace_project_edit_blob_path(project.namespace, project,
+ tree_join(ref, path),
+ options[:link_opts])
+ end
+ def fork_path(project = @project, ref = @ref, path = @path, options = {})
+ continue_params = {
+ to: edit_path,
+ notice: edit_in_new_fork_notice,
+ notice_now: edit_in_new_fork_notice_now
+ }
+ namespace_project_forks_path(project.namespace, project, namespace_key: current_user.namespace.id, continue: continue_params)
+ end
+
+ 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
- edit_path = namespace_project_edit_blob_path(project.namespace, project,
- tree_join(ref, path),
- options[:link_opts])
+ common_classes = "btn js-edit-blob #{options[:extra_class]}"
if !on_top_of_branch?(project, ref)
- 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 = {
- to: edit_path,
- notice: edit_in_new_fork_notice,
- 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)
-
- link_to "Edit", fork_path, class: 'btn', method: :post
+ 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
diff --git a/app/views/projects/blob/_blob.html.haml b/app/views/projects/blob/_blob.html.haml
index 2b2ee6ed987..aa9b852035e 100644
--- a/app/views/projects/blob/_blob.html.haml
+++ b/app/views/projects/blob/_blob.html.haml
@@ -25,4 +25,11 @@
#blob-content-holder.blob-content-holder
%article.file-holder
= 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
diff --git a/app/views/projects/blob/_header.html.haml b/app/views/projects/blob/_header.html.haml
index deeeae3d64a..6000e057ec4 100644
--- a/app/views/projects/blob/_header.html.haml
+++ b/app/views/projects/blob/_header.html.haml
@@ -32,8 +32,8 @@
= 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'
- - if current_user
- .btn-group{ role: "group" }<
- = edit_blob_link if blob_text_viewable?(blob)
+ .btn-group{ role: "group" }<
+ = edit_blob_link if blob_text_viewable?(blob)
+ - if current_user
= replace_blob_link
= delete_blob_link
diff --git a/changelogs/unreleased/28899-linking-to-edit-file.yml b/changelogs/unreleased/28899-linking-to-edit-file.yml
new file mode 100644
index 00000000000..a9f5410693b
--- /dev/null
+++ b/changelogs/unreleased/28899-linking-to-edit-file.yml
@@ -0,0 +1,5 @@
+---
+title: Linking to blob edit page handles anonymous users and users without enough permissions
+ to edit directly
+merge_request:
+author:
diff --git a/features/project/source/browse_files.feature b/features/project/source/browse_files.feature
index d4b91fec6e8..5ede935c562 100644
--- a/features/project/source/browse_files.feature
+++ b/features/project/source/browse_files.feature
@@ -158,6 +158,8 @@ Feature: Project Source Browse Files
Given I don't have write access
And I click on ".gitignore" file in repo
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
And I can edit code
@@ -180,6 +182,8 @@ Feature: Project Source Browse Files
Given I don't have write access
And I click on ".gitignore" file in repo
And I click button "Edit"
+ Then I should see a Fork/Cancel combo
+ And I click button "Fork"
And I edit code
And I fill the commit message
And I click on "Commit Changes"
diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb
index 5c47eaf0279..29f628763db 100644
--- a/features/steps/project/source/browse_files.rb
+++ b/features/steps/project/source/browse_files.rb
@@ -56,13 +56,17 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
end
step 'I click button "Edit"' do
- click_link 'Edit'
+ find('.js-edit-blob').click
end
step 'I cannot see the edit button' do
expect(page).not_to have_link 'edit'
end
+ step 'I click button "Fork"' do
+ click_link 'Fork'
+ end
+
step 'I can edit code' do
set_new_content
expect(evaluate_script('ace.edit("editor").getValue()')).to eq new_gitignore_content
@@ -366,6 +370,12 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps
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
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
diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb
index ec36a64b415..3e9f272a0d8 100644
--- a/spec/controllers/projects/blob_controller_spec.rb
+++ b/spec/controllers/projects/blob_controller_spec.rb
@@ -2,15 +2,10 @@ require 'rails_helper'
describe Projects::BlobController do
let(:project) { create(:project, :public, :repository) }
- let(:user) { create(:user) }
-
- before do
- project.team << [user, :master]
-
- sign_in(user)
- end
describe 'GET diff' do
+ let(:user) { create(:user) }
+
render_views
def do_get(opts = {})
@@ -20,6 +15,12 @@ describe Projects::BlobController do
get :diff, params.merge(opts)
end
+ before do
+ project.team << [user, :master]
+
+ sign_in(user)
+ end
+
context 'when essential params are missing' do
it 'renders nothing' do
do_get
@@ -37,7 +38,69 @@ describe Projects::BlobController do
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
+ let(:user) { create(:user) }
let(:default_params) do
{
namespace_id: project.namespace,
@@ -53,6 +116,12 @@ describe Projects::BlobController do
namespace_project_blob_path(project.namespace, project, 'master/CHANGELOG')
end
+ before do
+ project.team << [user, :master]
+
+ sign_in(user)
+ end
+
it 'redirects to blob' do
put :update, default_params
diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb
new file mode 100644
index 00000000000..01cd268ffe8
--- /dev/null
+++ b/spec/features/projects/blobs/blob_show_spec.rb
@@ -0,0 +1,23 @@
+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
diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb
index a820d07ab3b..46f2f487e0c 100644
--- a/spec/features/projects/blobs/edit_spec.rb
+++ b/spec/features/projects/blobs/edit_spec.rb
@@ -2,44 +2,135 @@ require 'spec_helper'
feature 'Editing file blob', feature: true, js: true do
include WaitForAjax
+ include TreeHelper
- given(:user) { create(:user) }
- given(:role) { :developer }
- given(:merge_request) { create(:merge_request, source_branch: 'feature', target_branch: 'master') }
- given(:project) { merge_request.target_project }
+ 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] }
- background do
- login_as(user)
- project.team << [user, role]
- end
-
- def edit_and_commit
- wait_for_ajax
- first('.file-actions').click_link 'Edit'
- execute_script('ace.edit("editor").setValue("class NextFeature\nend\n")')
- click_button 'Commit Changes'
- end
+ context 'as a developer' do
+ let(:user) { create(:user) }
+ let(:role) { :developer }
- context 'from MR diff' do
before do
- visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
- edit_and_commit
+ project.team << [user, role]
+ login_as(user)
+ end
+
+ def edit_and_commit
+ wait_for_ajax
+ find('.js-edit-blob').click
+ execute_script('ace.edit("editor").setValue("class NextFeature\nend\n")')
+ click_button 'Commit Changes'
+ end
+
+ context 'from MR diff' do
+ before do
+ visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
+ edit_and_commit
+ end
+
+ it 'returns me to the mr' do
+ expect(page).to have_content(merge_request.title)
+ end
end
- scenario 'returns me to the mr' do
- expect(page).to have_content(merge_request.title)
+ context 'from blob file path' do
+ before do
+ visit namespace_project_blob_path(project.namespace, project, tree_join(branch, file_path))
+ edit_and_commit
+ end
+
+ it 'updates content' do
+ expect(page).to have_content 'successfully committed'
+ expect(page).to have_content 'NextFeature'
+ end
end
end
- context 'from blob file path' do
- before do
- visit namespace_project_blob_path(project.namespace, project, '/feature/files/ruby/feature.rb')
- edit_and_commit
+ 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
- scenario 'updates content' do
- expect(page).to have_content 'successfully committed'
- expect(page).to have_content 'NextFeature'
+ 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
diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb
index bead7948486..508aeb7cf67 100644
--- a/spec/helpers/blob_helper_spec.rb
+++ b/spec/helpers/blob_helper_spec.rb
@@ -73,7 +73,7 @@ describe BlobHelper do
let(:project) { create(:project, :repository, namespace: namespace) }
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)
end