Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-07-06

app/controllers/concerns/preview_markdown.rb

@@ -10,6 +10,7 @@ module PreviewMarkdown
       when 'wikis'    then { pipeline: :wiki, project_wiki: @project_wiki, page_slug: params[:id] }
       when 'snippets' then { skip_project_check: true }
       when 'groups'   then { group: group }
+      when 'projects' then { issuable_state_filter_enabled: true }
       else {}
       end
 

app/controllers/dashboard/projects_controller.rb

@@ -7,7 +7,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
   skip_cross_project_access_check :index, :starred
 
   def index
-    @projects = load_projects(params.merge(non_public: true)).page(params[:page])
+    @projects = load_projects(params.merge(non_public: true))
 
     respond_to do |format|
       format.html
@@ -25,7 +25,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
 
   def starred
     @projects = load_projects(params.merge(starred: true))
-      .includes(:forked_from_project, :tags).page(params[:page])
+      .includes(:forked_from_project, :tags)
 
     @groups = []
 
@@ -51,6 +51,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
                 .new(params: finder_params, current_user: current_user)
                 .execute
                 .includes(:route, :creator, namespace: [:route, :owner])
+                .page(finder_params[:page])
 
     prepare_projects_for_rendering(projects)
   end

app/helpers/projects_helper.rb

@@ -179,6 +179,7 @@ module ProjectsHelper
       controller.action_name,
       Gitlab::CurrentSettings.cache_key,
       "cross-project:#{can?(current_user, :read_cross_project)}",
+      max_project_member_access_cache_key(project),
       'v2.6'
     ]
 

app/helpers/users_helper.rb

@@ -31,6 +31,14 @@ module UsersHelper
     current_user_menu_items.include?(item)
   end
 
+  def max_project_member_access(project)
+    current_user&.max_member_access_for_project(project.id) || Gitlab::Access::NO_ACCESS
+  end
+
+  def max_project_member_access_cache_key(project)
+    "access:#{max_project_member_access(project)}"
+  end
+
   private
 
   def get_profile_tabs

app/views/shared/projects/_project.html.haml

@@ -4,7 +4,7 @@
 - ci = false unless local_assigns[:ci] == true
 - skip_namespace = false unless local_assigns[:skip_namespace] == true
 - user = local_assigns[:user]
-- access = user&.max_member_access_for_project(project.id) unless user.nil?
+- access = max_project_member_access(project)
 - css_class = '' unless local_assigns[:css_class]
 - show_last_commit_as_description = false unless local_assigns[:show_last_commit_as_description] == true && can_show_last_commit_in_list?(project)
 - css_class += " no-description" if project.description.blank? && !show_last_commit_as_description

app/workers/archive_trace_worker.rb

@@ -5,7 +5,7 @@ class ArchiveTraceWorker
   include PipelineBackgroundQueue
 
   def perform(job_id)
-    Ci::Build.find_by(id: job_id).try do |job|
+    Ci::Build.without_archived_trace.find_by(id: job_id).try do |job|
       job.trace.archive!
     end
   end

app/workers/ci/archive_traces_cron_worker.rb

@@ -12,6 +12,7 @@ module Ci
       Ci::Build.finished.with_live_trace.find_each(batch_size: 100) do |build|
         begin
           build.trace.archive!
+        rescue ::Gitlab::Ci::Trace::AlreadyArchivedError
         rescue => e
           failed_archive_counter.increment
           Rails.logger.error "Failed to archive stale live trace. id: #{build.id} message: #{e.message}"

changelogs/unreleased/31583-osw-gfm-complete-status-indication.yml

+---
+title: Present state indication on GFM preview
+merge_request:
+author:
+type: added

changelogs/unreleased/fix-trace-archive-cron-worker-race-condition.yml

+---
+title: Check if archived trace exist before archive it
+merge_request: 20297
+author:
+type: fixed

changelogs/unreleased/fj-43565-wrong-role-displayed.yml

+---
+title: Fix wrong role badge displayed in projects dashboard
+merge_request: 20374
+author:
+type: fixed

lib/gitlab/ci/trace.rb

@@ -6,6 +6,7 @@ module Gitlab
       LEASE_TIMEOUT = 1.hour
 
       ArchiveError = Class.new(StandardError)
+      AlreadyArchivedError = Class.new(StandardError)
 
       attr_reader :job
 
@@ -81,7 +82,9 @@ module Gitlab
 
       def write(mode)
         stream = Gitlab::Ci::Trace::Stream.new do
-          if current_path
+          if trace_artifact
+            raise AlreadyArchivedError, 'Could not write to the archived trace'
+          elsif current_path
             File.open(current_path, mode)
           elsif Feature.enabled?('ci_enable_live_trace')
             Gitlab::Ci::Trace::ChunkedIO.new(job)
@@ -117,7 +120,7 @@ module Gitlab
       private
 
       def unsafe_archive!
-        raise ArchiveError, 'Already archived' if trace_artifact
+        raise AlreadyArchivedError, 'Could not archive again' if trace_artifact
         raise ArchiveError, 'Job is not finished yet' unless job.complete?
 
         if job.trace_chunks.any?

spec/controllers/projects_controller_spec.rb

@@ -661,13 +661,40 @@ describe ProjectsController do
   end
 
   describe 'POST #preview_markdown' do
-    it 'renders json in a correct format' do
+    before do
       sign_in(user)
+    end
 
+    it 'renders json in a correct format' do
       post :preview_markdown, namespace_id: public_project.namespace, id: public_project, text: '*Markdown* text'
 
       expect(JSON.parse(response.body).keys).to match_array(%w(body references))
     end
+
+    context 'state filter on references' do
+      let(:issue) { create(:issue, :closed, project: public_project) }
+      let(:merge_request) { create(:merge_request, :closed, target_project: public_project) }
+
+      it 'renders JSON body with state filter for issues' do
+        post :preview_markdown, namespace_id: public_project.namespace,
+                                id: public_project,
+                                text: issue.to_reference
+
+        json_response = JSON.parse(response.body)
+
+        expect(json_response['body']).to match(/\##{issue.iid} \(closed\)/)
+      end
+
+      it 'renders JSON body with state filter for MRs' do
+        post :preview_markdown, namespace_id: public_project.namespace,
+                                id: public_project,
+                                text: merge_request.to_reference
+
+        json_response = JSON.parse(response.body)
+
+        expect(json_response['body']).to match(/\!#{merge_request.iid} \(closed\)/)
+      end
+    end
   end
 
   describe '#ensure_canonical_path' do

spec/features/dashboard/projects_spec.rb

@@ -29,6 +29,34 @@ describe 'Dashboard Projects' do
     end
   end
 
+  context 'when user has access to the project' do
+    it 'shows role badge' do
+      visit dashboard_projects_path
+
+      page.within '.user-access-role' do
+        expect(page).to have_content('Developer')
+      end
+    end
+
+    context 'when role changes', :use_clean_rails_memory_store_fragment_caching do
+      it 'displays the right role' do
+        visit dashboard_projects_path
+
+        page.within '.user-access-role' do
+          expect(page).to have_content('Developer')
+        end
+
+        project.members.last.update(access_level: 40)
+
+        visit dashboard_projects_path
+
+        page.within '.user-access-role' do
+          expect(page).to have_content('Maintainer')
+        end
+      end
+    end
+  end
+
   context 'when last_repository_updated_at, last_activity_at and update_at are present' do
     it 'shows the last_repository_updated_at attribute as the update date' do
       project.update_attributes!(last_repository_updated_at: Time.now, last_activity_at: 1.hour.ago)

spec/helpers/projects_helper_spec.rb

@@ -80,6 +80,7 @@ describe ProjectsHelper do
     before do
       allow(helper).to receive(:current_user).and_return(user)
       allow(helper).to receive(:can?).with(user, :read_cross_project) { true }
+      allow(user).to receive(:max_member_access_for_project).and_return(40)
     end
 
     it "includes the route" do
@@ -125,6 +126,10 @@ describe ProjectsHelper do
 
       expect(helper.project_list_cache_key(project)).to include("pipeline-status/#{project.commit.sha}-success")
     end
+
+    it "includes the user max member access" do
+      expect(helper.project_list_cache_key(project)).to include('access:40')
+    end
   end
 
   describe '#load_pipeline_status' do

spec/requests/api/runner_spec.rb

@@ -856,6 +856,7 @@ describe API::Runner, :clean_gitlab_redis_shared_state do
         it 'does not update job status and job trace' do
           update_job(state: 'success', trace: 'BUILD TRACE UPDATED')
 
+          job.reload
           expect(response).to have_gitlab_http_status(403)
           expect(response.header['Job-Status']).to eq 'failed'
           expect(job.trace.raw).to eq 'Job failed'

spec/spec_helper.rb

@@ -172,6 +172,17 @@ RSpec.configure do |config|
     redis_queues_cleanup!
   end
 
+  config.around(:each, :use_clean_rails_memory_store_fragment_caching) do |example|
+    caching_store = ActionController::Base.cache_store
+    ActionController::Base.cache_store = ActiveSupport::Cache::MemoryStore.new
+    ActionController::Base.perform_caching = true
+
+    example.run
+
+    ActionController::Base.perform_caching = false
+    ActionController::Base.cache_store = caching_store
+  end
+
   # The :each scope runs "inside" the example, so this hook ensures the DB is in the
   # correct state before any examples' before hooks are called. This prevents a
   # problem where `ScheduleIssuesClosedAtTypeChange` (or any migration that depends

spec/support/shared_examples/ci_trace_shared_examples.rb

@@ -138,6 +138,28 @@ shared_examples_for 'common trace features' do
     end
   end
 
+  describe '#write' do
+    subject { trace.send(:write, mode) { } }
+
+    let(:mode) { 'wb' }
+
+    context 'when arhicved trace does not exist yet' do
+      it 'does not raise an error' do
+        expect { subject }.not_to raise_error
+      end
+    end
+
+    context 'when arhicved trace already exists' do
+      before do
+        create(:ci_job_artifact, :trace, job: build)
+      end
+
+      it 'raises an error' do
+        expect { subject }.to raise_error(Gitlab::Ci::Trace::AlreadyArchivedError)
+      end
+    end
+  end
+
   describe '#set' do
     before do
       trace.set("12")
@@ -574,7 +596,7 @@ shared_examples_for 'trace with disabled live trace feature' do
 
       it 'does not archive' do
         expect_any_instance_of(described_class).not_to receive(:archive_stream!)
-        expect { subject }.to raise_error('Already archived')
+        expect { subject }.to raise_error(Gitlab::Ci::Trace::AlreadyArchivedError)
         expect(build.job_artifacts_trace.file.exists?).to be_truthy
       end
     end
@@ -761,7 +783,7 @@ shared_examples_for 'trace with enabled live trace feature' do
 
       it 'does not archive' do
         expect_any_instance_of(described_class).not_to receive(:archive_stream!)
-        expect { subject }.to raise_error('Already archived')
+        expect { subject }.to raise_error(Gitlab::Ci::Trace::AlreadyArchivedError)
         expect(build.job_artifacts_trace.file.exists?).to be_truthy
       end
     end

spec/workers/ci/archive_traces_cron_worker_spec.rb

@@ -25,24 +25,36 @@ describe Ci::ArchiveTracesCronWorker do
     end
   end
 
-  context 'when a job was succeeded' do
+  context 'when a job succeeded' do
     let!(:build) { create(:ci_build, :success, :trace_live) }
 
     it_behaves_like 'archives trace'
 
-    context 'when archive raised an exception' do
-      let!(:build) { create(:ci_build, :success, :trace_artifact, :trace_live) }
+    context 'when a trace had already been archived' do
+      let!(:build) { create(:ci_build, :success, :trace_live, :trace_artifact) }
       let!(:build2) { create(:ci_build, :success, :trace_live) }
 
-      it 'archives valid targets' do
-        expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Already archived")
-
+      it 'continues to archive live traces' do
         subject
 
         build2.reload
         expect(build2.job_artifacts_trace).to be_exist
       end
     end
+
+    context 'when an unexpected exception happened during archiving' do
+      let!(:build) { create(:ci_build, :success, :trace_live) }
+
+      before do
+        allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive!).and_raise('Unexpected error')
+      end
+
+      it 'puts a log' do
+        expect(Rails.logger).to receive(:error).with("Failed to archive stale live trace. id: #{build.id} message: Unexpected error")
+
+        subject
+      end
+    end
   end
 
   context 'when a job was cancelled' do