Commit 541f7675 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'fix-diff-patch-public-mr' into 'master'

Fix downloading of patches on public merge requests when user logged out

### What does this MR do?

This MR makes it possible to download a diff patch on a public merge request when a user is logged out.

### Why was this MR needed?

An Error 500 would result when a user attempted to click on the "Email Patches" or "Plain Diff" button:

```
NoMethodError - undefined method `id' for nil:NilClass:
  lib/gitlab/backend/shell_env.rb:9:in `set_env'
  lib/gitlab/satellite/action.rb:20:in `in_locked_and_timed_satellite'
  lib/gitlab/satellite/merge_action.rb:49:in `diff_in_satellite'
  app/models/merge_request.rb:219:in `to_diff'
  app/controllers/projects/merge_requests_controller.rb:42:in `block (2 levels) in show'
```

### What are the relevant issue numbers?

* Closes #1225
* Closes #1854 (dup)
* Closes #1858 (dup)

See merge request !872
parents 94f130cb 555fd0cf
...@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date. ...@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
v 7.13.0 (unreleased) v 7.13.0 (unreleased)
- Fix invalid timestamps in RSS feeds (Rowan Wookey) - Fix invalid timestamps in RSS feeds (Rowan Wookey)
- Fix error when deleting a user who has projects (Stan Hu) - Fix error when deleting a user who has projects (Stan Hu)
- Fix downloading of patches on public merge requests when user logged out (Stan Hu)
- Update maintenance documentation to explain no need to recompile asssets for omnibus installations (Stan Hu) - Update maintenance documentation to explain no need to recompile asssets for omnibus installations (Stan Hu)
- Support commenting on diffs in side-by-side mode (Stan Hu) - Support commenting on diffs in side-by-side mode (Stan Hu)
- Fix JavaScript error when clicking on the comment button on a diff line that has a comment already (Stan Hu) - Fix JavaScript error when clicking on the comment button on a diff line that has a comment already (Stan Hu)
......
...@@ -41,6 +41,18 @@ Feature: Project Merge Requests ...@@ -41,6 +41,18 @@ Feature: Project Merge Requests
And I submit new merge request "Wiki Feature" And I submit new merge request "Wiki Feature"
Then I should see merge request "Wiki Feature" Then I should see merge request "Wiki Feature"
Scenario: I download a diff on a public merge request
Given public project "Community"
And "John Doe" owns public project "Community"
And project "Community" has "Bug CO-01" open merge request with diffs inside
Given I logout directly
And I visit merge request page "Bug CO-01"
And I click on "Email Patches"
Then I should see a patch diff
And I visit merge request page "Bug CO-01"
And I click on "Plain Diff"
Then I should see a patch diff
@javascript @javascript
Scenario: I comment on a merge request Scenario: I comment on a merge request
Given I visit merge request page "Bug NS-04" Given I visit merge request page "Bug NS-04"
......
...@@ -6,6 +6,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps ...@@ -6,6 +6,7 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
include SharedPaths include SharedPaths
include SharedMarkdown include SharedMarkdown
include SharedDiffNote include SharedDiffNote
include SharedUser
step 'I click link "New Merge Request"' do step 'I click link "New Merge Request"' do
click_link "New Merge Request" click_link "New Merge Request"
...@@ -108,6 +109,15 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps ...@@ -108,6 +109,15 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
author: project.users.first) author: project.users.first)
end end
step 'project "Community" has "Bug CO-01" open merge request with diffs inside' do
project = Project.find_by(name: "Community")
create(:merge_request_with_diffs,
title: "Bug CO-01",
source_project: project,
target_project: project,
author: project.users.first)
end
step 'I switch to the diff tab' do step 'I switch to the diff tab' do
visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request) visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request)
end end
...@@ -326,6 +336,18 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps ...@@ -326,6 +336,18 @@ class Spinach::Features::ProjectMergeRequests < Spinach::FeatureSteps
expect(page).to have_content 'Target branch changed from master to feature' expect(page).to have_content 'Target branch changed from master to feature'
end end
step 'I click on "Email Patches"' do
click_link "Email Patches"
end
step 'I click on "Plain Diff"' do
click_link "Plain Diff"
end
step 'I should see a patch diff' do
expect(page).to have_content('diff --git')
end
def merge_request def merge_request
@merge_request ||= MergeRequest.find_by!(title: "Bug NS-05") @merge_request ||= MergeRequest.find_by!(title: "Bug NS-05")
end end
......
...@@ -357,6 +357,11 @@ module SharedPaths ...@@ -357,6 +357,11 @@ module SharedPaths
visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr) visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr)
end end
step 'I visit merge request page "Bug CO-01"' do
mr = MergeRequest.find_by(title: "Bug CO-01")
visit namespace_project_merge_request_path(mr.target_project.namespace, mr.target_project, mr)
end
step 'I visit project "Shop" merge requests page' do step 'I visit project "Shop" merge requests page' do
visit namespace_project_merge_requests_path(project.namespace, project) visit namespace_project_merge_requests_path(project.namespace, project)
end end
......
...@@ -6,8 +6,10 @@ module Gitlab ...@@ -6,8 +6,10 @@ module Gitlab
def set_env(user) def set_env(user)
# Set GL_ID env variable # Set GL_ID env variable
if user
ENV['GL_ID'] = "user-#{user.id}" ENV['GL_ID'] = "user-#{user.id}"
end end
end
def reset_env def reset_env
# Reset GL_ID env variable # Reset GL_ID env variable
......
...@@ -39,9 +39,11 @@ module Gitlab ...@@ -39,9 +39,11 @@ module Gitlab
def prepare_satellite!(repo) def prepare_satellite!(repo)
project.satellite.clear_and_update! project.satellite.clear_and_update!
if user
repo.config['user.name'] = user.name repo.config['user.name'] = user.name
repo.config['user.email'] = user.email repo.config['user.email'] = user.email
end end
end
def default_options(options = {}) def default_options(options = {})
{ raise: true, timeout: true }.merge(options) { raise: true, timeout: true }.merge(options)
......
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