From 05732d90da0c556d6857bb4c1acd46d676dd07ba Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Mon, 2 Apr 2018 18:30:49 +0200
Subject: [PATCH 01/18] Rename delete_protected_branch ability to
 push_to_delete_protected_branch to prevent confusion with
 destroy_protected_branch

---
 app/policies/project_policy.rb                | 4 ++--
 app/views/projects/branches/_branch.html.haml | 2 +-
 lib/gitlab/user_access.rb                     | 2 +-
 spec/policies/project_policy_spec.rb          | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 21bb0934dee..b495132d693 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -208,7 +208,7 @@ class ProjectPolicy < BasePolicy
   end
 
   rule { can?(:master_access) }.policy do
-    enable :delete_protected_branch
+    enable :push_to_delete_protected_branch
     enable :update_project_snippet
     enable :update_environment
     enable :update_deployment
@@ -232,8 +232,8 @@ class ProjectPolicy < BasePolicy
 
   rule { archived }.policy do
     prevent :create_merge_request
+    prevent :push_to_delete_protected_branch
     prevent :push_code
-    prevent :delete_protected_branch
     prevent :update_merge_request
     prevent :admin_merge_request
   end
diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml
index 883dfb3e6c8..8bc146cbaf4 100644
--- a/app/views/projects/branches/_branch.html.haml
+++ b/app/views/projects/branches/_branch.html.haml
@@ -61,7 +61,7 @@
           title: s_('Branches|The default branch cannot be deleted') }
           = icon("trash-o")
       - elsif protected_branch?(@project, branch)
-        - if can?(current_user, :delete_protected_branch, @project)
+        - if can?(current_user, :push_to_delete_protected_branch, @project)
           %button{ class: "btn btn-remove remove-row js-ajax-loading-spinner has-tooltip",
             title: s_('Branches|Delete protected branch'),
             data: { toggle: "modal",
diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index 24393f96d96..69952cbb47c 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -51,7 +51,7 @@ module Gitlab
       return false unless can_access_git?
 
       if protected?(ProtectedBranch, project, ref)
-        user.can?(:delete_protected_branch, project)
+        user.can?(:push_to_delete_protected_branch, project)
       else
         user.can?(:push_code, project)
       end
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 905d82b3bb1..7843af839df 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -43,7 +43,7 @@ describe ProjectPolicy do
 
   let(:base_master_permissions) do
     %i[
-      delete_protected_branch update_project_snippet update_environment
+      push_to_delete_protected_branch update_project_snippet update_environment
       update_deployment admin_project_snippet
       admin_project_member admin_note admin_wiki admin_project
       admin_commit_status admin_build admin_container_image

From d18285257304115a43defc82202d76b0f498f4c3 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Mon, 2 Apr 2018 18:31:15 +0200
Subject: [PATCH 02/18] Update phrasing around archived feature in UI

---
 app/views/admin/projects/show.html.haml |  2 +-
 app/views/projects/edit.html.haml       | 11 ++++++-----
 app/views/projects/show.html.haml       |  2 +-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/app/views/admin/projects/show.html.haml b/app/views/admin/projects/show.html.haml
index c47b8a88f56..aeba9788fda 100644
--- a/app/views/admin/projects/show.html.haml
+++ b/app/views/admin/projects/show.html.haml
@@ -101,7 +101,7 @@
         - if @project.archived?
           %li
             %span.light archived:
-            %strong repository is read-only
+            %strong project is read-only
 
         %li
           %span.light access:
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index 99eeb9551e3..0994498c6be 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -114,17 +114,18 @@
               Archive project
           - if @project.archived?
             %p
-              Unarchiving the project will mark its repository as active. The project can be committed to.
+              Unarchiving the project will restore people's ability to make changes to it.
+              The repository can be committed to, and issues, comments and other entities can be created.
               %strong Once active this project shows up in the search and on the dashboard.
             = link_to 'Unarchive project', unarchive_project_path(@project),
-                data: { confirm: "Are you sure that you want to unarchive this project?\nWhen this project is unarchived it is active and can be committed to again." },
+                data: { confirm: "Are you sure that you want to unarchive this project?" },
                 method: :post, class: "btn btn-success"
           - else
             %p
-              Archiving the project will mark its repository as read-only. It is hidden from the dashboard and doesn't show up in searches.
-              %strong Archived projects cannot be committed to!
+              Archiving the project will make it entirely read-only. It is hidden from the dashboard and doesn't show up in searches.
+              %strong The repository cannot be committed to, and no issues, comments or other entities can be created.
             = link_to 'Archive project', archive_project_path(@project),
-                data: { confirm: "Are you sure that you want to archive this project?\nAn archived project cannot be committed to." },
+                data: { confirm: "Are you sure that you want to archive this project?" },
                 method: :post, class: "btn btn-warning"
       .sub-section.rename-respository
         %h4.warning-title
diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml
index 94331a16abd..e28accd5b43 100644
--- a/app/views/projects/show.html.haml
+++ b/app/views/projects/show.html.haml
@@ -24,7 +24,7 @@
     .text-warning.center.prepend-top-20
       %p
         = icon("exclamation-triangle fw")
-        #{ _('Archived project! Repository is read-only') }
+        #{ _('Archived project! Repository and other project resources are read-only') }
 
   - view_path = @project.default_view
 

From 267a909600e02f0728fec1765adf817acc03d813 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Mon, 2 Apr 2018 19:05:47 +0200
Subject: [PATCH 03/18] Remove edit_note and update_note abilities in favor of
 admin_note

---
 app/helpers/notes_helper.rb            |  4 --
 app/models/ability.rb                  |  4 --
 app/policies/issuable_policy.rb        |  2 -
 app/policies/note_policy.rb            |  6 +--
 app/serializers/note_entity.rb         |  2 +-
 app/views/shared/notes/_note.html.haml |  2 +-
 spec/models/ability_spec.rb            | 56 --------------------------
 spec/policies/note_policy_spec.rb      |  4 --
 8 files changed, 3 insertions(+), 77 deletions(-)

diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 27ed48fdbc7..7f67574a428 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -6,10 +6,6 @@ module NotesHelper
     end
   end
 
-  def note_editable?(note)
-    Ability.can_edit_note?(current_user, note)
-  end
-
   def note_supports_quick_actions?(note)
     Notes::QuickActionsService.supported?(note)
   end
diff --git a/app/models/ability.rb b/app/models/ability.rb
index 6dae49f38dc..618d4af4272 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -46,10 +46,6 @@ class Ability
       end
     end
 
-    def can_edit_note?(user, note)
-      allowed?(user, :edit_note, note)
-    end
-
     def allowed?(user, action, subject = :global, opts = {})
       if subject.is_a?(Hash)
         opts, subject = subject, :global
diff --git a/app/policies/issuable_policy.rb b/app/policies/issuable_policy.rb
index e86d1c8f98e..b431d376e3d 100644
--- a/app/policies/issuable_policy.rb
+++ b/app/policies/issuable_policy.rb
@@ -18,9 +18,7 @@ class IssuablePolicy < BasePolicy
 
   rule { locked & ~is_project_member }.policy do
     prevent :create_note
-    prevent :update_note
     prevent :admin_note
     prevent :resolve_note
-    prevent :edit_note
   end
 end
diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb
index d4cb5a77e63..2f6c579906c 100644
--- a/app/policies/note_policy.rb
+++ b/app/policies/note_policy.rb
@@ -8,14 +8,10 @@ class NotePolicy < BasePolicy
 
   condition(:editable, scope: :subject) { @subject.editable? }
 
-  rule { ~editable | anonymous }.prevent :edit_note
-
-  rule { is_author | admin }.enable :edit_note
-  rule { can?(:master_access) }.enable :edit_note
+  rule { ~editable }.prevent :admin_note
 
   rule { is_author }.policy do
     enable :read_note
-    enable :update_note
     enable :admin_note
     enable :resolve_note
   end
diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb
index c964aa9c99b..fa516fee6ec 100644
--- a/app/serializers/note_entity.rb
+++ b/app/serializers/note_entity.rb
@@ -15,7 +15,7 @@ class NoteEntity < API::Entities::Note
 
   expose :current_user do
     expose :can_edit do |note|
-      Ability.can_edit_note?(request.current_user, note)
+      Ability.allowed?(request.current_user, :admin_note, note)
     end
   end
 
diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml
index bf359774ead..893a7f26ebd 100644
--- a/app/views/shared/notes/_note.html.haml
+++ b/app/views/shared/notes/_note.html.haml
@@ -2,7 +2,7 @@
 - return if note.cross_reference_not_visible_for?(current_user)
 
 - show_image_comment_badge = local_assigns.fetch(:show_image_comment_badge, false)
-- note_editable = note_editable?(note)
+- note_editable = can?(current_user, :admin_note, note)
 - note_counter = local_assigns.fetch(:note_counter, 0)
 
 %li.timeline-entry{ id: dom_id(note),
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index cd175dba6da..199f49d0bf2 100644
--- a/spec/models/ability_spec.rb
+++ b/spec/models/ability_spec.rb
@@ -7,62 +7,6 @@ describe Ability do
     end
   end
 
-  describe '.can_edit_note?' do
-    let(:project) { create(:project) }
-    let(:note) { create(:note_on_issue, project: project) }
-
-    context 'using an anonymous user' do
-      it 'returns false' do
-        expect(described_class.can_edit_note?(nil, note)).to be_falsy
-      end
-    end
-
-    context 'using a system note' do
-      it 'returns false' do
-        system_note = create(:note, system: true)
-        user = create(:user)
-
-        expect(described_class.can_edit_note?(user, system_note)).to be_falsy
-      end
-    end
-
-    context 'using users with different access levels' do
-      let(:user) { create(:user) }
-
-      it 'returns true for the author' do
-        expect(described_class.can_edit_note?(note.author, note)).to be_truthy
-      end
-
-      it 'returns false for a guest user' do
-        project.add_guest(user)
-
-        expect(described_class.can_edit_note?(user, note)).to be_falsy
-      end
-
-      it 'returns false for a developer' do
-        project.add_developer(user)
-
-        expect(described_class.can_edit_note?(user, note)).to be_falsy
-      end
-
-      it 'returns true for a master' do
-        project.add_master(user)
-
-        expect(described_class.can_edit_note?(user, note)).to be_truthy
-      end
-
-      it 'returns true for a group owner' do
-        group = create(:group)
-        project.project_group_links.create(
-          group: group,
-          group_access: Gitlab::Access::MASTER)
-        group.add_owner(user)
-
-        expect(described_class.can_edit_note?(user, note)).to be_truthy
-      end
-    end
-  end
-
   describe '.users_that_can_read_project' do
     context 'using a public project' do
       it 'returns all the users' do
diff --git a/spec/policies/note_policy_spec.rb b/spec/policies/note_policy_spec.rb
index 58d36a2c84e..e8096358f7d 100644
--- a/spec/policies/note_policy_spec.rb
+++ b/spec/policies/note_policy_spec.rb
@@ -18,7 +18,6 @@ describe NotePolicy, mdoels: true do
     context 'when the project is public' do
       context 'when the note author is not a project member' do
         it 'can edit a note' do
-          expect(policies).to be_allowed(:update_note)
           expect(policies).to be_allowed(:admin_note)
           expect(policies).to be_allowed(:resolve_note)
           expect(policies).to be_allowed(:read_note)
@@ -29,7 +28,6 @@ describe NotePolicy, mdoels: true do
         it 'can edit note' do
           policies = policies(create(:project_snippet, project: project))
 
-          expect(policies).to be_allowed(:update_note)
           expect(policies).to be_allowed(:admin_note)
           expect(policies).to be_allowed(:resolve_note)
           expect(policies).to be_allowed(:read_note)
@@ -47,7 +45,6 @@ describe NotePolicy, mdoels: true do
           end
 
           it 'can edit a note' do
-            expect(policies).to be_allowed(:update_note)
             expect(policies).to be_allowed(:admin_note)
             expect(policies).to be_allowed(:resolve_note)
             expect(policies).to be_allowed(:read_note)
@@ -56,7 +53,6 @@ describe NotePolicy, mdoels: true do
 
         context 'when the note author is not a project member' do
           it 'can not edit a note' do
-            expect(policies).to be_disallowed(:update_note)
             expect(policies).to be_disallowed(:admin_note)
             expect(policies).to be_disallowed(:resolve_note)
           end

From 8272ec9a7683863c43d217c97bdf7bf165cb3cf2 Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Mon, 2 Apr 2018 20:38:47 +0200
Subject: [PATCH 04/18] Update policies to make archived projects completely
 read-only

---
 app/policies/ci/build_policy.rb             |  4 +-
 app/policies/ci/pipeline_schedule_policy.rb | 14 +---
 app/policies/note_policy.rb                 |  3 +-
 app/policies/project_policy.rb              | 89 ++++++++++++++-------
 spec/policies/project_policy_spec.rb        | 67 +++++++++++++++-
 5 files changed, 131 insertions(+), 46 deletions(-)

diff --git a/app/policies/ci/build_policy.rb b/app/policies/ci/build_policy.rb
index 1ab391a5a9d..808a81cbbf9 100644
--- a/app/policies/ci/build_policy.rb
+++ b/app/policies/ci/build_policy.rb
@@ -11,7 +11,7 @@ module Ci
     end
 
     condition(:owner_of_job) do
-      can?(:developer_access) && @subject.triggered_by?(@user)
+      @subject.triggered_by?(@user)
     end
 
     rule { protected_ref }.policy do
@@ -19,6 +19,6 @@ module Ci
       prevent :erase_build
     end
 
-    rule { can?(:master_access) | owner_of_job }.enable :erase_build
+    rule { can?(:admin_build) | (can?(:update_build) & owner_of_job) }.enable :erase_build
   end
 end
diff --git a/app/policies/ci/pipeline_schedule_policy.rb b/app/policies/ci/pipeline_schedule_policy.rb
index dc7a4aed577..ecba0488d3c 100644
--- a/app/policies/ci/pipeline_schedule_policy.rb
+++ b/app/policies/ci/pipeline_schedule_policy.rb
@@ -7,23 +7,17 @@ module Ci
     end
 
     condition(:owner_of_schedule) do
-      can?(:developer_access) && pipeline_schedule.owned_by?(@user)
+      pipeline_schedule.owned_by?(@user)
     end
 
-    condition(:non_owner_of_schedule) do
-      !pipeline_schedule.owned_by?(@user)
-    end
+    rule { can?(:create_pipeline) }.enable :play_pipeline_schedule
 
-    rule { can?(:developer_access) }.policy do
-      enable :play_pipeline_schedule
-    end
-
-    rule { can?(:master_access) | owner_of_schedule }.policy do
+    rule { can?(:admin_pipeline) | (can?(:update_build) & owner_of_schedule) }.policy do
       enable :update_pipeline_schedule
       enable :admin_pipeline_schedule
     end
 
-    rule { can?(:master_access) & non_owner_of_schedule }.policy do
+    rule { can?(:admin_pipeline_schedule) & ~owner_of_schedule }.policy do
       enable :take_ownership_pipeline_schedule
     end
 
diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb
index 2f6c579906c..6659bf062c4 100644
--- a/app/policies/note_policy.rb
+++ b/app/policies/note_policy.rb
@@ -3,7 +3,6 @@ class NotePolicy < BasePolicy
   delegate { @subject.noteable if @subject.noteable.lockable? }
 
   condition(:is_author) { @user && @subject.author == @user }
-  condition(:for_merge_request, scope: :subject) { @subject.for_merge_request? }
   condition(:is_noteable_author) { @user && @subject.noteable.author_id == @user.id }
 
   condition(:editable, scope: :subject) { @subject.editable? }
@@ -16,7 +15,7 @@ class NotePolicy < BasePolicy
     enable :resolve_note
   end
 
-  rule { for_merge_request & is_noteable_author }.policy do
+  rule { is_noteable_author }.policy do
     enable :resolve_note
   end
 end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index b495132d693..91dd89a8de1 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -1,10 +1,38 @@
 class ProjectPolicy < BasePolicy
-  def self.create_read_update_admin(name)
+  READONLY_FEATURES_WHEN_ARCHIVED = %i[
+    issue
+    list
+    merge_request
+    label
+    milestone
+    project_snippet
+    wiki
+    note
+    pipeline
+    pipeline_schedule
+    build
+    trigger
+    environment
+    deployment
+    commit_status
+    container_image
+    pages
+    cluster
+  ].freeze
+
+  def self.create_read_update_admin_destroy(name)
+    [
+      :"read_#{name}",
+      *create_update_admin_destroy(name)
+    ]
+  end
+
+  def self.create_update_admin_destroy(name)
     [
       :"create_#{name}",
-      :"read_#{name}",
       :"update_#{name}",
-      :"admin_#{name}"
+      :"admin_#{name}",
+      :"destroy_#{name}"
     ]
   end
 
@@ -15,7 +43,7 @@ class ProjectPolicy < BasePolicy
   end
 
   desc "Project has public builds enabled"
-  condition(:public_builds, scope: :subject) { project.public_builds? }
+  condition(:public_builds, scope: :subject, score: 0) { project.public_builds? }
 
   # For guest access we use #team_member? so we can use
   # project.members, which gets cached in subject scope.
@@ -35,7 +63,7 @@ class ProjectPolicy < BasePolicy
   condition(:master) { team_access_level >= Gitlab::Access::MASTER }
 
   desc "Project is public"
-  condition(:public_project, scope: :subject) { project.public? }
+  condition(:public_project, scope: :subject, score: 0) { project.public? }
 
   desc "Project is visible to internal users"
   condition(:internal_access) do
@@ -46,7 +74,7 @@ class ProjectPolicy < BasePolicy
   condition(:group_member, scope: :subject) { project_group_member? }
 
   desc "Project is archived"
-  condition(:archived, scope: :subject) { project.archived? }
+  condition(:archived, scope: :subject, score: 0) { project.archived? }
 
   condition(:default_issues_tracker, scope: :subject) { project.default_issues_tracker? }
 
@@ -56,10 +84,10 @@ class ProjectPolicy < BasePolicy
   end
 
   desc "Project has an external wiki"
-  condition(:has_external_wiki, scope: :subject) { project.has_external_wiki? }
+  condition(:has_external_wiki, scope: :subject, score: 0) { project.has_external_wiki? }
 
   desc "Project has request access enabled"
-  condition(:request_access_enabled, scope: :subject) { project.request_access_enabled }
+  condition(:request_access_enabled, scope: :subject, score: 0) { project.request_access_enabled }
 
   desc "Has merge requests allowing pushes to user"
   condition(:has_merge_requests_allowing_pushes, scope: :subject) do
@@ -231,37 +259,45 @@ class ProjectPolicy < BasePolicy
   end
 
   rule { archived }.policy do
-    prevent :create_merge_request
-    prevent :push_to_delete_protected_branch
     prevent :push_code
-    prevent :update_merge_request
-    prevent :admin_merge_request
+    prevent :push_to_delete_protected_branch
+    prevent :request_access
+    prevent :upload_file
+    prevent :resolve_note
+
+    READONLY_FEATURES_WHEN_ARCHIVED.each do |feature|
+      prevent(*create_update_admin_destroy(feature))
+    end
+  end
+
+  rule { issues_disabled }.policy do
+    prevent(*create_read_update_admin_destroy(:issue))
   end
 
   rule { merge_requests_disabled | repository_disabled }.policy do
-    prevent(*create_read_update_admin(:merge_request))
+    prevent(*create_read_update_admin_destroy(:merge_request))
   end
 
   rule { issues_disabled & merge_requests_disabled }.policy do
-    prevent(*create_read_update_admin(:label))
-    prevent(*create_read_update_admin(:milestone))
+    prevent(*create_read_update_admin_destroy(:label))
+    prevent(*create_read_update_admin_destroy(:milestone))
   end
 
   rule { snippets_disabled }.policy do
-    prevent(*create_read_update_admin(:project_snippet))
+    prevent(*create_read_update_admin_destroy(:project_snippet))
   end
 
   rule { wiki_disabled & ~has_external_wiki }.policy do
-    prevent(*create_read_update_admin(:wiki))
+    prevent(*create_read_update_admin_destroy(:wiki))
     prevent(:download_wiki_code)
   end
 
   rule { builds_disabled | repository_disabled }.policy do
-    prevent(*create_read_update_admin(:build))
-    prevent(*(create_read_update_admin(:pipeline) - [:read_pipeline]))
-    prevent(*create_read_update_admin(:pipeline_schedule))
-    prevent(*create_read_update_admin(:environment))
-    prevent(*create_read_update_admin(:deployment))
+    prevent(*create_update_admin_destroy(:pipeline))
+    prevent(*create_read_update_admin_destroy(:build))
+    prevent(*create_read_update_admin_destroy(:pipeline_schedule))
+    prevent(*create_read_update_admin_destroy(:environment))
+    prevent(*create_read_update_admin_destroy(:deployment))
   end
 
   rule { repository_disabled }.policy do
@@ -272,7 +308,7 @@ class ProjectPolicy < BasePolicy
   end
 
   rule { container_registry_disabled }.policy do
-    prevent(*create_read_update_admin(:container_image))
+    prevent(*create_read_update_admin_destroy(:container_image))
   end
 
   rule { anonymous & ~public_project }.prevent_all
@@ -314,13 +350,6 @@ class ProjectPolicy < BasePolicy
     enable :read_pipeline_schedule
   end
 
-  rule { issues_disabled }.policy do
-    prevent :create_issue
-    prevent :update_issue
-    prevent :admin_issue
-    prevent :read_issue
-  end
-
   # These rules are included to allow maintainers of projects to push to certain
   # to run pipelines for the branches they have access to.
   rule { can?(:public_access) & has_merge_requests_allowing_pushes }.policy do
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 7843af839df..f20e4a021da 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -136,13 +136,49 @@ describe ProjectPolicy do
     end
   end
 
+  shared_examples 'archived project policies' do
+    let(:feature_write_abilities) do
+      described_class::READONLY_FEATURES_WHEN_ARCHIVED.flat_map do |feature|
+        described_class.create_update_admin_destroy(feature)
+      end
+    end
+
+    let(:other_write_abilities) do
+      %i[
+        push_to_delete_protected_branch
+        push_code
+        request_access
+        upload_file
+        resolve_note
+      ]
+    end
+
+    context 'when the project is archived' do
+      before do
+        project.archived = true
+      end
+
+      it 'disables write actions on all relevant project features' do
+        expect_disallowed(*feature_write_abilities)
+      end
+
+      it 'disables some other important write actions' do
+        expect_disallowed(*other_write_abilities)
+      end
+
+      it 'does not disable other other abilities' do
+        expect_allowed(*(regular_abilities - feature_write_abilities - other_write_abilities))
+      end
+    end
+  end
+
   shared_examples 'project policies as anonymous' do
     context 'abilities for public projects' do
       context 'when a project has pending invites' do
         let(:group) { create(:group, :public) }
         let(:project) { create(:project, :public, namespace: group) }
         let(:user_permissions) { [:create_project, :create_issue, :create_note, :upload_file] }
-        let(:anonymous_permissions) { guest_permissions - user_permissions }
+        let(:anonymous_permissions) { guest_permissions - user_permissions  }
 
         subject { described_class.new(nil, project) }
 
@@ -154,6 +190,10 @@ describe ProjectPolicy do
           expect_allowed(*anonymous_permissions)
           expect_disallowed(*user_permissions)
         end
+
+        it_behaves_like 'archived project policies' do
+          let(:regular_abilities) { anonymous_permissions }
+        end
       end
     end
 
@@ -184,6 +224,10 @@ describe ProjectPolicy do
         expect_disallowed(*owner_permissions)
       end
 
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { guest_permissions }
+      end
+
       context 'public builds enabled' do
         it do
           expect_allowed(*guest_permissions)
@@ -224,12 +268,15 @@ describe ProjectPolicy do
       it do
         expect_allowed(*guest_permissions)
         expect_allowed(*reporter_permissions)
-        expect_allowed(*reporter_permissions)
         expect_allowed(*team_member_reporter_permissions)
         expect_disallowed(*developer_permissions)
         expect_disallowed(*master_permissions)
         expect_disallowed(*owner_permissions)
       end
+
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { reporter_permissions }
+      end
     end
   end
 
@@ -247,6 +294,10 @@ describe ProjectPolicy do
         expect_disallowed(*master_permissions)
         expect_disallowed(*owner_permissions)
       end
+
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { developer_permissions }
+      end
     end
   end
 
@@ -264,6 +315,10 @@ describe ProjectPolicy do
         expect_allowed(*master_permissions)
         expect_disallowed(*owner_permissions)
       end
+
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { master_permissions }
+      end
     end
   end
 
@@ -281,6 +336,10 @@ describe ProjectPolicy do
         expect_allowed(*master_permissions)
         expect_allowed(*owner_permissions)
       end
+
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { owner_permissions }
+      end
     end
   end
 
@@ -298,6 +357,10 @@ describe ProjectPolicy do
         expect_allowed(*master_permissions)
         expect_allowed(*owner_permissions)
       end
+
+      it_behaves_like 'archived project policies' do
+        let(:regular_abilities) { owner_permissions }
+      end
     end
   end
 

From bdd7600de71c9490be4ba4ddc27999b490b7cf8a Mon Sep 17 00:00:00 2001
From: Douwe Maan <douwe@selenight.nl>
Date: Mon, 2 Apr 2018 20:40:11 +0200
Subject: [PATCH 05/18] Add ability checks in views where they were previously
 missing

---
 .../notes/components/comment_form.vue         |  4 +--
 .../projects/clusters/_empty_state.html.haml  |  5 ++--
 .../projects/commit/_commit_box.html.haml     |  5 ++--
 app/views/projects/issues/_nav_btns.html.haml | 13 +++++-----
 app/views/projects/tags/_tag.html.haml        |  6 ++---
 app/views/projects/tags/show.html.haml        |  2 +-
 .../projects/tree/_tree_header.html.haml      | 19 +++++++-------
 app/views/shared/_label.html.haml             | 26 +++++++++----------
 .../commit/_commit_box.html.haml_spec.rb      |  9 +++----
 9 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue
index 648fa6ff804..396a675b4ac 100644
--- a/app/assets/javascripts/notes/components/comment_form.vue
+++ b/app/assets/javascripts/notes/components/comment_form.vue
@@ -317,10 +317,10 @@ Please check your network connection and try again.`;
     <note-signed-out-widget v-if="!isLoggedIn" />
     <discussion-locked-widget
       issuable-type="issue"
-      v-else-if="!canCreateNote"
+      v-else-if="isLocked(getNoteableData) && !canCreateNote"
     />
     <ul
-      v-else
+      v-else-if="canCreateNote"
       class="notes notes-form timeline">
       <li class="timeline-entry">
         <div class="timeline-entry-inner">
diff --git a/app/views/projects/clusters/_empty_state.html.haml b/app/views/projects/clusters/_empty_state.html.haml
index 112dde66ff7..5f49d03b1bb 100644
--- a/app/views/projects/clusters/_empty_state.html.haml
+++ b/app/views/projects/clusters/_empty_state.html.haml
@@ -7,5 +7,6 @@
       - link_to_help_page = link_to(_('Learn more about Kubernetes'), help_page_path('user/project/clusters/index'), target: '_blank', rel: 'noopener noreferrer')
       %p= s_('ClusterIntegration|Kubernetes clusters allow you to use review apps, deploy your applications, run your pipelines, and much more in an easy way. %{link_to_help_page}').html_safe % { link_to_help_page: link_to_help_page}
 
-      .text-center
-        = link_to s_('ClusterIntegration|Add Kubernetes cluster'), new_project_cluster_path(@project), class: 'btn btn-success'
+      - if can?(current_user, :create_cluster, @project)
+        .text-center
+          = link_to s_('ClusterIntegration|Add Kubernetes cluster'), new_project_cluster_path(@project), class: 'btn btn-success'
diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml
index 74c5317428c..aec58a91a71 100644
--- a/app/views/projects/commit/_commit_box.html.haml
+++ b/app/views/projects/commit/_commit_box.html.haml
@@ -35,9 +35,10 @@
         - unless @commit.has_been_reverted?(current_user)
           %li.clearfix
             = revert_commit_link(@commit, project_commit_path(@project, @commit.id), has_tooltip: false)
-        %li.clearfix
-          = cherry_pick_commit_link(@commit, project_commit_path(@project, @commit.id), has_tooltip: false)
         - if can_collaborate_with_project?
+          %li.clearfix
+            = cherry_pick_commit_link(@commit, project_commit_path(@project, @commit.id), has_tooltip: false)
+        - if can?(current_user, :push_code, @project)
           %li.clearfix
             = link_to s_("CreateTag|Tag"), new_project_tag_path(@project, ref: @commit)
         %li.divider
diff --git a/app/views/projects/issues/_nav_btns.html.haml b/app/views/projects/issues/_nav_btns.html.haml
index 0d39edb7bfd..bc672286d90 100644
--- a/app/views/projects/issues/_nav_btns.html.haml
+++ b/app/views/projects/issues/_nav_btns.html.haml
@@ -2,9 +2,10 @@
   = icon('rss')
 - if @can_bulk_update
   = button_tag "Edit issues", class: "btn btn-default append-right-10 js-bulk-update-toggle"
-= link_to "New issue", new_project_issue_path(@project,
-                                              issue: { assignee_id: finder.assignee.try(:id),
-                                                       milestone_id: finder.milestones.first.try(:id) }),
-                                              class: "btn btn-new",
-                                              title: "New issue",
-                                              id: "new_issue_link"
+- if !current_user || can?(current_user, :create_issue, @project)
+  = link_to "New issue", new_project_issue_path(@project,
+                                                issue: { assignee_id: finder.assignee.try(:id),
+                                                         milestone_id: finder.milestones.first.try(:id) }),
+                                                class: "btn btn-new",
+                                                title: "New issue",
+                                                id: "new_issue_link"
diff --git a/app/views/projects/tags/_tag.html.haml b/app/views/projects/tags/_tag.html.haml
index 3d5f92f9aaa..98b4d6339da 100644
--- a/app/views/projects/tags/_tag.html.haml
+++ b/app/views/projects/tags/_tag.html.haml
@@ -31,6 +31,6 @@
       = link_to edit_project_tag_release_path(@project, tag.name), class: 'btn has-tooltip', title: s_('TagsPage|Edit release notes'), data: { container: "body" } do
         = icon("pencil")
 
-    - if can?(current_user, :admin_project, @project)
-      = link_to project_tag_path(@project, tag.name), class: "btn btn-remove remove-row has-tooltip prepend-left-10 #{protected_tag?(@project, tag) ? 'disabled' : ''}", title: s_('TagsPage|Delete tag'), method: :delete, data: { confirm: s_('TagsPage|Deleting the %{tag_name} tag cannot be undone. Are you sure?') % { tag_name: tag.name }, container: 'body' }, remote: true do
-        = icon("trash-o")
+      - if can?(current_user, :admin_project, @project)
+        = link_to project_tag_path(@project, tag.name), class: "btn btn-remove remove-row has-tooltip prepend-left-10 #{protected_tag?(@project, tag) ? 'disabled' : ''}", title: s_('TagsPage|Delete tag'), method: :delete, data: { confirm: s_('TagsPage|Deleting the %{tag_name} tag cannot be undone. Are you sure?') % { tag_name: tag.name }, container: 'body' }, remote: true do
+          = icon("trash-o")
diff --git a/app/views/projects/tags/show.html.haml b/app/views/projects/tags/show.html.haml
index dfe2c37ed8e..7a3469cdd26 100644
--- a/app/views/projects/tags/show.html.haml
+++ b/app/views/projects/tags/show.html.haml
@@ -28,7 +28,7 @@
         = icon('history')
       .btn-container.controls-item
         = render 'projects/buttons/download', project: @project, ref: @tag.name
-      - if can?(current_user, :admin_project, @project)
+      - if can?(current_user, :push_code, @project) && can?(current_user, :admin_project, @project)
         .btn-container.controls-item-full
           = link_to project_tag_path(@project, @tag.name), class: "btn btn-remove remove-row has-tooltip #{protected_tag?(@project, @tag) ? 'disabled' : ''}", title: s_('TagsPage|Delete tag'), method: :delete, data: { confirm: s_('TagsPage|Deleting the %{tag_name} tag cannot be undone. Are you sure?') % { tag_name: @tag.name } } do
             %i.fa.fa-trash-o
diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml
index 5ef5e9c09a2..59cf78420fb 100644
--- a/app/views/projects/tree/_tree_header.html.haml
+++ b/app/views/projects/tree/_tree_header.html.haml
@@ -61,15 +61,16 @@
                   = link_to fork_path, method: :post do
                     #{ _('New directory') }
 
-              %li.divider
-              %li.dropdown-header
-                #{ _('This repository') }
-              %li
-                = link_to new_project_branch_path(@project) do
-                  #{ _('New branch') }
-              %li
-                = link_to new_project_tag_path(@project) do
-                  #{ _('New tag') }
+              - if can?(current_user, :push_code, @project)
+                %li.divider
+                %li.dropdown-header
+                  #{ _('This repository') }
+                %li
+                  = link_to new_project_branch_path(@project) do
+                    #{ _('New branch') }
+                %li
+                  = link_to new_project_tag_path(@project) do
+                    #{ _('New tag') }
 
 .tree-controls
   = link_to s_('Commits|History'), project_commits_path(@project, @id), class: 'btn'
diff --git a/app/views/shared/_label.html.haml b/app/views/shared/_label.html.haml
index 56403907844..836df57a3a2 100644
--- a/app/views/shared/_label.html.haml
+++ b/app/views/shared/_label.html.haml
@@ -47,20 +47,20 @@
               class: 'text-danger'
 
   .pull-right.hidden-xs.hidden-sm
-    - if label.is_a?(ProjectLabel) && label.project.group && can?(current_user, :admin_label, label.project.group)
-      %button.js-promote-project-label-button.btn.btn-transparent.btn-action.has-tooltip{ title: _('Promote to Group Label'),
-        disabled: true,
-        type: 'button',
-        data: { url: promote_project_label_path(label.project, label),
-                label_title: label.title,
-                label_color: label.color,
-                label_text_color: label.text_color,
-                group_name: label.project.group.name,
-                target: '#promote-label-modal',
-                container: 'body',
-                toggle: 'modal' } }
-        = sprite_icon('level-up')
     - if can?(current_user, :admin_label, label)
+      - if label.is_a?(ProjectLabel) && label.project.group && can?(current_user, :admin_label, label.project.group)
+        %button.js-promote-project-label-button.btn.btn-transparent.btn-action.has-tooltip{ title: _('Promote to Group Label'),
+          disabled: true,
+          type: 'button',
+          data: { url: promote_project_label_path(label.project, label),
+                  label_title: label.title,
+                  label_color: label.color,
+                  label_text_color: label.text_color,
+                  group_name: label.project.group.name,
+                  target: '#promote-label-modal',
+                  container: 'body',
+                  toggle: 'modal' } }
+          = sprite_icon('level-up')
       = link_to edit_label_path(label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do
         %span.sr-only Edit
         = sprite_icon('pencil')
diff --git a/spec/views/projects/commit/_commit_box.html.haml_spec.rb b/spec/views/projects/commit/_commit_box.html.haml_spec.rb
index 448b925cf34..fb5087f5090 100644
--- a/spec/views/projects/commit/_commit_box.html.haml_spec.rb
+++ b/spec/views/projects/commit/_commit_box.html.haml_spec.rb
@@ -7,6 +7,8 @@ describe 'projects/commit/_commit_box.html.haml' do
   before do
     assign(:project, project)
     assign(:commit, project.commit)
+    assign(:current_user, user)
+    allow(view).to receive(:current_user).and_return(user)
     allow(view).to receive(:can_collaborate_with_project?).and_return(false)
   end
 
@@ -47,7 +49,8 @@ describe 'projects/commit/_commit_box.html.haml' do
   context 'viewing a commit' do
     context 'as a developer' do
       before do
-        expect(view).to receive(:can_collaborate_with_project?).and_return(true)
+        project.add_developer(user)
+        allow(view).to receive(:can_collaborate_with_project?).and_return(true)
       end
 
       it 'has a link to create a new tag' do
@@ -58,10 +61,6 @@ describe 'projects/commit/_commit_box.html.haml' do
     end
 
     context 'as a non-developer' do
-      before do
-        expect(view).to receive(:can_collaborate_with_project?).and_return(false)
-      end
-
       it 'does not have a link to create a new tag' do
         render
 

From 6114c40fca764602addc911bc58bc05c42e03747 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Thu, 5 Apr 2018 18:42:57 +0200
Subject: [PATCH 06/18] Move `ProjectPolicy`-class methods into module

That way the ProjectPolicy class can be extended with this module
before we prepend the EE::ProjectPolicy. This makes the classmethods
available for rules defined in the EE::ProjectPolicy.
---
 app/policies/project_policy.rb               | 18 ++----------------
 app/policies/project_policy/class_methods.rb | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 16 deletions(-)
 create mode 100644 app/policies/project_policy/class_methods.rb

diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 91dd89a8de1..b4970b605ca 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -1,4 +1,6 @@
 class ProjectPolicy < BasePolicy
+  extend ClassMethods
+
   READONLY_FEATURES_WHEN_ARCHIVED = %i[
     issue
     list
@@ -20,22 +22,6 @@ class ProjectPolicy < BasePolicy
     cluster
   ].freeze
 
-  def self.create_read_update_admin_destroy(name)
-    [
-      :"read_#{name}",
-      *create_update_admin_destroy(name)
-    ]
-  end
-
-  def self.create_update_admin_destroy(name)
-    [
-      :"create_#{name}",
-      :"update_#{name}",
-      :"admin_#{name}",
-      :"destroy_#{name}"
-    ]
-  end
-
   desc "User is a project owner"
   condition :owner do
     (project.owner.present? && project.owner == @user) ||
diff --git a/app/policies/project_policy/class_methods.rb b/app/policies/project_policy/class_methods.rb
new file mode 100644
index 00000000000..60e5aba00ba
--- /dev/null
+++ b/app/policies/project_policy/class_methods.rb
@@ -0,0 +1,19 @@
+class ProjectPolicy
+  module ClassMethods
+    def create_read_update_admin_destroy(name)
+      [
+        :"read_#{name}",
+        *create_update_admin_destroy(name)
+      ]
+    end
+
+    def create_update_admin_destroy(name)
+      [
+        :"create_#{name}",
+        :"update_#{name}",
+        :"admin_#{name}",
+        :"destroy_#{name}"
+      ]
+    end
+  end
+end

From 267dba0ae56c6236b49907ad77fd572578c7b30e Mon Sep 17 00:00:00 2001
From: James Ramsay <james@jramsay.com.au>
Date: Thu, 5 Apr 2018 14:34:27 -0400
Subject: [PATCH 07/18] Update documentation

---
 doc/user/project/settings/index.md | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/doc/user/project/settings/index.md b/doc/user/project/settings/index.md
index 888dd0e143a..a387c1e443e 100644
--- a/doc/user/project/settings/index.md
+++ b/doc/user/project/settings/index.md
@@ -57,15 +57,20 @@ Here you can run housekeeping, archive, rename, transfer, or remove a project.
 NOTE: **Note:**
 Only project Owners and Admin users have the [permissions] to archive a project.
 
-An archived project will be hidden by default in the project listings.
+Archiving a project makes it read-only for all users and indicates that it is
+no longer actively maintained. Projects that have been archived can also be
+unarchived.
+
+When a project is archived, the repository, issues, merge requests and all
+other features are read-only. Archived projects are also hidden
+in project listings.
+
+To archive a project:
 
 1. Navigate to your project's **Settings > General > Advanced settings**.
-1. Under "Archive project", hit the **Archive project** button.
+1. In the Archive project section, click the **Archive project** button.
 1. Confirm the action when asked to.
 
-An archived project can be fully restored and will therefore retain its
-repository and all associated resources whilst in an archived state.
-
 #### Renaming a repository
 
 NOTE: **Note:**

From ec43e3644436e0ae90f6fb6a64256c38ceb68cc9 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Fri, 6 Apr 2018 12:47:52 +0200
Subject: [PATCH 08/18] Prevent new merge requests for archived projects

This prevents creating merge requests targeting archived projects.

This could happen when a project was already forked, but then the
source was archived.
---
 .../projects/application_controller.rb        |  6 ++++-
 .../merge_request_target_project_finder.rb    |  1 +
 app/helpers/blob_helper.rb                    |  4 +--
 app/policies/project_policy.rb                |  4 +++
 app/presenters/merge_request_presenter.rb     |  6 ++++-
 app/services/merge_requests/create_service.rb |  2 +-
 .../merge_request/user_cherry_picks_spec.rb   |  8 ++++++
 .../projects/files/user_edits_files_spec.rb   | 27 +++++++++++++++++++
 .../projects/merge_request_button_spec.rb     | 12 +++++++++
 .../user_reverts_merge_request_spec.rb        |  8 ++++++
 ...erge_request_target_project_finder_spec.rb |  6 +++++
 spec/policies/project_policy_spec.rb          | 17 +++++++++++-
 .../merge_requests/create_service_spec.rb     | 24 ++++++++++++++++-
 13 files changed, 118 insertions(+), 7 deletions(-)

diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index 6d9b42a2c04..cbb14b55399 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -34,8 +34,12 @@ class Projects::ApplicationController < ApplicationController
   def can_collaborate_with_project?(project = nil, ref: nil)
     project ||= @project
 
+    can_create_merge_request =
+      can?(current_user, :create_merge_request_in_project, project) &&
+      current_user.already_forked?(project)
+
     can?(current_user, :push_code, project) ||
-      (current_user && current_user.already_forked?(project)) ||
+      can_create_merge_request ||
       user_access(project).can_push_to_branch?(ref)
   end
 
diff --git a/app/finders/merge_request_target_project_finder.rb b/app/finders/merge_request_target_project_finder.rb
index f358938344e..188ec447a94 100644
--- a/app/finders/merge_request_target_project_finder.rb
+++ b/app/finders/merge_request_target_project_finder.rb
@@ -12,6 +12,7 @@ class MergeRequestTargetProjectFinder
     if @source_project.fork_network
       @source_project.fork_network.projects
         .public_or_visible_to_user(current_user)
+        .non_archived
         .with_feature_available_for_user(:merge_requests, current_user)
     else
       Project.where(id: source_project)
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index 2b440e4d584..ac7b8d6672e 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -59,7 +59,7 @@ module BlobHelper
       button_tag label, class: "#{common_classes} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' }
     elsif can_modify_blob?(blob, project, ref)
       button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal'
-    elsif can?(current_user, :fork_project, project)
+    elsif can?(current_user, :create_merge_request_in_project, project)
       edit_fork_button_tag(common_classes, project, label, edit_modify_file_fork_params(action), action)
     end
   end
@@ -334,7 +334,7 @@ module BlobHelper
       # Web IDE (Beta) requires the user to have this feature enabled
     elsif !current_user || (current_user && can_modify_blob?(blob, project, ref))
       edit_link_tag(text, edit_path, common_classes)
-    elsif current_user && can?(current_user, :fork_project, project)
+    elsif can?(current_user, :create_merge_request_in_project, project)
       edit_fork_button_tag(common_classes, project, text, edit_blob_fork_params(edit_path))
     end
   end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index b4970b605ca..72e30c932a9 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -140,6 +140,7 @@ class ProjectPolicy < BasePolicy
 
   rule { can?(:guest_access) }.policy do
     enable :read_project
+    enable :create_merge_request_in_project
     enable :read_board
     enable :read_list
     enable :read_wiki
@@ -250,6 +251,7 @@ class ProjectPolicy < BasePolicy
     prevent :request_access
     prevent :upload_file
     prevent :resolve_note
+    prevent :create_merge_request_in_project
 
     READONLY_FEATURES_WHEN_ARCHIVED.each do |feature|
       prevent(*create_update_admin_destroy(feature))
@@ -261,6 +263,7 @@ class ProjectPolicy < BasePolicy
   end
 
   rule { merge_requests_disabled | repository_disabled }.policy do
+    prevent :create_merge_request_in_project
     prevent(*create_read_update_admin_destroy(:merge_request))
   end
 
@@ -306,6 +309,7 @@ class ProjectPolicy < BasePolicy
 
   rule { can?(:public_access) }.policy do
     enable :read_project
+    enable :create_merge_request_in_project
     enable :read_board
     enable :read_list
     enable :read_wiki
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index 9f3f2637183..950d3fde2ea 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -196,8 +196,12 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
   end
 
   def user_can_collaborate_with_project?
+    can_create_merge_request =
+      can?(current_user, :create_merge_request_in_project, project) &&
+      current_user.already_forked?(project)
+
     can?(current_user, :push_code, project) ||
-      (current_user && current_user.already_forked?(project)) ||
+      can_create_merge_request ||
       can_push_to_source_branch?
   end
 
diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb
index c57a2445341..4c2c8398461 100644
--- a/app/services/merge_requests/create_service.rb
+++ b/app/services/merge_requests/create_service.rb
@@ -72,7 +72,7 @@ module MergeRequests
       params.delete(:target_project_id)
 
       unless can?(current_user, :read_project, @source_project) &&
-          can?(current_user, :read_project, @project)
+          can?(current_user, :create_merge_request_in_project, @project)
 
         raise Gitlab::Access::AccessDeniedError
       end
diff --git a/spec/features/merge_request/user_cherry_picks_spec.rb b/spec/features/merge_request/user_cherry_picks_spec.rb
index 494096b21c0..61d1bdaa95a 100644
--- a/spec/features/merge_request/user_cherry_picks_spec.rb
+++ b/spec/features/merge_request/user_cherry_picks_spec.rb
@@ -40,6 +40,14 @@ describe 'Merge request > User cherry-picks', :js do
 
         expect(page).to have_link 'Cherry-pick'
       end
+
+      it 'hides the cherry pick button for an archived project' do
+        project.update!(archived: true)
+
+        visit project_merge_request_path(project, merge_request)
+
+        expect(page).not_to have_link 'Cherry-pick'
+      end
     end
   end
 end
diff --git a/spec/features/projects/files/user_edits_files_spec.rb b/spec/features/projects/files/user_edits_files_spec.rb
index 523a9f3f4fe..ee9deb30890 100644
--- a/spec/features/projects/files/user_edits_files_spec.rb
+++ b/spec/features/projects/files/user_edits_files_spec.rb
@@ -12,6 +12,27 @@ describe 'Projects > Files > User edits files' do
     sign_in(user)
   end
 
+  shared_examples 'unavailable for an archived project' do
+    it 'does not show the edit link for an archived project', :js do
+      project.update!(archived: true)
+      visit project_tree_path(project, project.repository.root_ref)
+
+      click_link('.gitignore')
+
+      aggregate_failures 'available edit buttons' do
+        # We're showing a link, if the user can edit directly, this is becomes a
+        # button when the user can fork the project.
+        expect(page).not_to have_link('Edit')
+        expect(page).not_to have_button('Edit')
+        expect(page).not_to have_link('Web IDE')
+        expect(page).not_to have_button('Web IDE')
+
+        expect(page).not_to have_button('Replace')
+        expect(page).not_to have_button('Delete')
+      end
+    end
+  end
+
   context 'when an user has write access' do
     before do
       project.add_master(user)
@@ -85,6 +106,8 @@ describe 'Projects > Files > User edits files' do
 
       expect(page).to have_css('.line_holder.new')
     end
+
+    it_behaves_like 'unavailable for an archived project'
   end
 
   context 'when an user does not have write access' do
@@ -168,6 +191,10 @@ describe 'Projects > Files > User edits files' do
         expect(page).to have_content("From #{forked_project.full_path}")
         expect(page).to have_content("into #{project2.full_path}")
       end
+
+      it_behaves_like 'unavailable for an archived project' do
+        let(:project) { project2 }
+      end
     end
   end
 end
diff --git a/spec/features/projects/merge_request_button_spec.rb b/spec/features/projects/merge_request_button_spec.rb
index 40689964b91..b571d5a0e26 100644
--- a/spec/features/projects/merge_request_button_spec.rb
+++ b/spec/features/projects/merge_request_button_spec.rb
@@ -45,6 +45,18 @@ feature 'Merge Request button' do
           end
         end
       end
+
+      context 'when the project is archived' do
+        it 'hides the link' do
+          project.update!(archived: true)
+
+          visit url
+
+          within("#content-body") do
+            expect(page).not_to have_link(label)
+          end
+        end
+      end
     end
 
     context 'logged in as non-member' do
diff --git a/spec/features/projects/merge_requests/user_reverts_merge_request_spec.rb b/spec/features/projects/merge_requests/user_reverts_merge_request_spec.rb
index a41d683dbbb..f3e97bc9eb2 100644
--- a/spec/features/projects/merge_requests/user_reverts_merge_request_spec.rb
+++ b/spec/features/projects/merge_requests/user_reverts_merge_request_spec.rb
@@ -56,4 +56,12 @@ describe 'User reverts a merge request', :js do
 
     expect(page).to have_content('The merge request has been successfully reverted. You can now submit a merge request to get this change into the original branch.')
   end
+
+  it 'cannot revert a merge requests for an archived project' do
+    project.update!(archived: true)
+
+    visit(merge_request_path(merge_request))
+
+    expect(page).not_to have_link('Revert')
+  end
 end
diff --git a/spec/finders/merge_request_target_project_finder_spec.rb b/spec/finders/merge_request_target_project_finder_spec.rb
index c81bfd7932c..f302cf80ce8 100644
--- a/spec/finders/merge_request_target_project_finder_spec.rb
+++ b/spec/finders/merge_request_target_project_finder_spec.rb
@@ -19,6 +19,12 @@ describe MergeRequestTargetProjectFinder do
 
       expect(finder.execute).to contain_exactly(forked_project)
     end
+
+    it 'does not contain archived projects' do
+      base_project.update!(archived: true)
+
+      expect(finder.execute).to contain_exactly(other_fork, forked_project)
+    end
   end
 
   context 'public projects' do
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index f20e4a021da..4c01c320841 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -14,7 +14,7 @@ describe ProjectPolicy do
       read_project read_board read_list read_wiki read_issue
       read_project_for_iids read_issue_iid read_merge_request_iid read_label
       read_milestone read_project_snippet read_project_member read_note
-      create_project create_issue create_note upload_file
+      create_project create_issue create_note upload_file create_merge_request_in_project
     ]
   end
 
@@ -136,6 +136,20 @@ describe ProjectPolicy do
     end
   end
 
+  context 'merge requests feature' do
+    subject { described_class.new(owner, project) }
+
+    it 'disallows all permissions when the feature is disabled' do
+      project.project_feature.update(merge_requests_access_level: ProjectFeature::DISABLED)
+
+      mr_permissions = [:create_merge_request, :read_merge_request,
+                        :update_merge_request, :admin_merge_request,
+                        :create_merge_request_in_project]
+
+      expect_disallowed(*mr_permissions)
+    end
+  end
+
   shared_examples 'archived project policies' do
     let(:feature_write_abilities) do
       described_class::READONLY_FEATURES_WHEN_ARCHIVED.flat_map do |feature|
@@ -145,6 +159,7 @@ describe ProjectPolicy do
 
     let(:other_write_abilities) do
       %i[
+        create_merge_request_in_project
         push_to_delete_protected_branch
         push_code
         request_access
diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb
index 44a83c436cb..736a50b2c15 100644
--- a/spec/services/merge_requests/create_service_spec.rb
+++ b/spec/services/merge_requests/create_service_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe MergeRequests::CreateService do
+  include ProjectForksHelper
+
   let(:project) { create(:project, :repository) }
   let(:user) { create(:user) }
   let(:assignee) { create(:user) }
@@ -300,7 +302,7 @@ describe MergeRequests::CreateService do
     end
 
     context 'when source and target projects are different' do
-      let(:target_project) { create(:project) }
+      let(:target_project) { fork_project(project, nil, repository: true) }
 
       let(:opts) do
         {
@@ -334,6 +336,26 @@ describe MergeRequests::CreateService do
             .to raise_error Gitlab::Access::AccessDeniedError
         end
       end
+
+      context 'when the user has access to both projects' do
+        before do
+          target_project.add_developer(user)
+          project.add_developer(user)
+        end
+
+        it 'creates the merge request' do
+          merge_request = described_class.new(project, user, opts).execute
+
+          expect(merge_request).to be_persisted
+        end
+
+        it 'does not create the merge request when the target project is archived' do
+          target_project.update!(archived: true)
+
+          expect { described_class.new(project, user, opts).execute }
+            .to raise_error Gitlab::Access::AccessDeniedError
+        end
+      end
     end
 
     context 'when user sets source project id' do

From 8ad9c4e873f2185e8bbfa6f363db32dbbba27141 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Fri, 6 Apr 2018 14:18:58 +0200
Subject: [PATCH 09/18] Rename `create_merge_request` permissions

So we can distinguish between the permissions on the source and the
target project.

- `create_merge_request_from` indicates a user can create a merge
  request with the project as a source_project
- `create_merge_request_in` indicates a user can create a merge
  request with the project as a target_project
---
 app/controllers/projects/application_controller.rb  |  2 +-
 app/controllers/projects/issues_controller.rb       |  2 +-
 .../projects/merge_requests/creations_controller.rb |  2 +-
 app/helpers/blob_helper.rb                          |  6 +++---
 app/helpers/compare_helper.rb                       |  2 +-
 app/policies/project_policy.rb                      | 11 ++++++-----
 app/presenters/merge_request_presenter.rb           |  2 +-
 app/services/merge_requests/create_service.rb       |  4 ++--
 app/views/projects/branches/_branch.html.haml       |  2 +-
 app/views/projects/issues/_new_branch.html.haml     |  2 +-
 app/views/projects/merge_requests/index.html.haml   |  2 +-
 db/fixtures/development/10_merge_requests.rb        |  8 ++++++--
 spec/policies/project_policy_spec.rb                | 13 +++++++------
 13 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index cbb14b55399..53a0c712e49 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -35,7 +35,7 @@ class Projects::ApplicationController < ApplicationController
     project ||= @project
 
     can_create_merge_request =
-      can?(current_user, :create_merge_request_in_project, project) &&
+      can?(current_user, :create_merge_request_in, project) &&
       current_user.already_forked?(project)
 
     can?(current_user, :push_code, project) ||
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index b14939c4216..9dcd13b9385 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -20,7 +20,7 @@ class Projects::IssuesController < Projects::ApplicationController
   before_action :authorize_update_issuable!, only: [:edit, :update, :move]
 
   # Allow create a new branch and empty WIP merge request from current issue
-  before_action :authorize_create_merge_request!, only: [:create_merge_request]
+  before_action :authorize_create_merge_request_in!, only: [:create_merge_request]
 
   respond_to :html
 
diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb
index a90030a8312..4a377fefc62 100644
--- a/app/controllers/projects/merge_requests/creations_controller.rb
+++ b/app/controllers/projects/merge_requests/creations_controller.rb
@@ -5,7 +5,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
 
   skip_before_action :merge_request
   before_action :whitelist_query_limiting, only: [:create]
-  before_action :authorize_create_merge_request!
+  before_action :authorize_create_merge_request_from!
   before_action :apply_diff_view_cookie!, only: [:diffs, :diff_for_path]
   before_action :build_merge_request, except: [:create]
 
diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index ac7b8d6672e..a5f7a7204df 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -59,7 +59,7 @@ module BlobHelper
       button_tag label, class: "#{common_classes} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' }
     elsif can_modify_blob?(blob, project, ref)
       button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal'
-    elsif can?(current_user, :create_merge_request_in_project, project)
+    elsif can?(current_user, :create_merge_request_in, project)
       edit_fork_button_tag(common_classes, project, label, edit_modify_file_fork_params(action), action)
     end
   end
@@ -280,7 +280,7 @@ module BlobHelper
       options << link_to("submit an issue", new_project_issue_path(project))
     end
 
-    merge_project = can?(current_user, :create_merge_request, project) ? project : (current_user && current_user.fork_of(project))
+    merge_project = can?(current_user, :create_merge_request_from, project) ? project : (current_user && current_user.fork_of(project))
     if merge_project
       options << link_to("create a merge request", project_new_merge_request_path(project))
     end
@@ -334,7 +334,7 @@ module BlobHelper
       # Web IDE (Beta) requires the user to have this feature enabled
     elsif !current_user || (current_user && can_modify_blob?(blob, project, ref))
       edit_link_tag(text, edit_path, common_classes)
-    elsif can?(current_user, :create_merge_request_in_project, project)
+    elsif can?(current_user, :fork_project, project) && can?(current_user, :create_merge_request_in, project)
       edit_fork_button_tag(common_classes, project, text, edit_blob_fork_params(edit_path))
     end
   end
diff --git a/app/helpers/compare_helper.rb b/app/helpers/compare_helper.rb
index 8bf96c0905f..2df5b5d1695 100644
--- a/app/helpers/compare_helper.rb
+++ b/app/helpers/compare_helper.rb
@@ -3,7 +3,7 @@ module CompareHelper
     from.present? &&
       to.present? &&
       from != to &&
-      can?(current_user, :create_merge_request, project) &&
+      can?(current_user, :create_merge_request_from, project) &&
       project.repository.branch_exists?(from) &&
       project.repository.branch_exists?(to)
   end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 72e30c932a9..ac154270236 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -140,7 +140,7 @@ class ProjectPolicy < BasePolicy
 
   rule { can?(:guest_access) }.policy do
     enable :read_project
-    enable :create_merge_request_in_project
+    enable :create_merge_request_in
     enable :read_board
     enable :read_list
     enable :read_wiki
@@ -212,7 +212,7 @@ class ProjectPolicy < BasePolicy
     enable :create_pipeline
     enable :update_pipeline
     enable :create_pipeline_schedule
-    enable :create_merge_request
+    enable :create_merge_request_from
     enable :create_wiki
     enable :push_code
     enable :resolve_note
@@ -251,7 +251,8 @@ class ProjectPolicy < BasePolicy
     prevent :request_access
     prevent :upload_file
     prevent :resolve_note
-    prevent :create_merge_request_in_project
+    prevent :create_merge_request_from
+    prevent :create_merge_request_in
 
     READONLY_FEATURES_WHEN_ARCHIVED.each do |feature|
       prevent(*create_update_admin_destroy(feature))
@@ -263,7 +264,8 @@ class ProjectPolicy < BasePolicy
   end
 
   rule { merge_requests_disabled | repository_disabled }.policy do
-    prevent :create_merge_request_in_project
+    prevent :create_merge_request_in
+    prevent :create_merge_request_from
     prevent(*create_read_update_admin_destroy(:merge_request))
   end
 
@@ -309,7 +311,6 @@ class ProjectPolicy < BasePolicy
 
   rule { can?(:public_access) }.policy do
     enable :read_project
-    enable :create_merge_request_in_project
     enable :read_board
     enable :read_list
     enable :read_wiki
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index 950d3fde2ea..a55af1422e8 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -197,7 +197,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
 
   def user_can_collaborate_with_project?
     can_create_merge_request =
-      can?(current_user, :create_merge_request_in_project, project) &&
+      can?(current_user, :create_merge_request_in, project) &&
       current_user.already_forked?(project)
 
     can?(current_user, :push_code, project) ||
diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb
index 4c2c8398461..fe1ac70781e 100644
--- a/app/services/merge_requests/create_service.rb
+++ b/app/services/merge_requests/create_service.rb
@@ -71,8 +71,8 @@ module MergeRequests
       params.delete(:source_project_id)
       params.delete(:target_project_id)
 
-      unless can?(current_user, :read_project, @source_project) &&
-          can?(current_user, :create_merge_request_in_project, @project)
+      unless can?(current_user, :create_merge_request_from, @source_project) &&
+          can?(current_user, :create_merge_request_in, @project)
 
         raise Gitlab::Access::AccessDeniedError
       end
diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml
index 8bc146cbaf4..6ca33c7524d 100644
--- a/app/views/projects/branches/_branch.html.haml
+++ b/app/views/projects/branches/_branch.html.haml
@@ -4,7 +4,7 @@
 - diverging_commit_counts = @repository.diverging_commit_counts(branch)
 - number_commits_behind = diverging_commit_counts[:behind]
 - number_commits_ahead = diverging_commit_counts[:ahead]
-- merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project))
+- merge_project = can?(current_user, :create_merge_request_from, @project) ? @project : (current_user && current_user.fork_of(@project))
 %li{ class: "branch-item js-branch-#{branch.name}" }
   .branch-info
     .branch-title
diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml
index 36e24037214..02ea3dc74fe 100644
--- a/app/views/projects/issues/_new_branch.html.haml
+++ b/app/views/projects/issues/_new_branch.html.haml
@@ -1,4 +1,4 @@
-- can_create_merge_request = can?(current_user, :create_merge_request, @project)
+- can_create_merge_request = can?(current_user, :create_merge_request_from, @project)
 - data_action = can_create_merge_request ? 'create-mr' : 'create-branch'
 - value = can_create_merge_request ? 'Create merge request' : 'Create branch'
 
diff --git a/app/views/projects/merge_requests/index.html.haml b/app/views/projects/merge_requests/index.html.haml
index b2c0d9e1cfa..f9ab0f5709b 100644
--- a/app/views/projects/merge_requests/index.html.haml
+++ b/app/views/projects/merge_requests/index.html.haml
@@ -1,6 +1,6 @@
 - @no_container = true
 - @can_bulk_update = can?(current_user, :admin_merge_request, @project)
-- merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project))
+- merge_project = can?(current_user, :create_merge_request_from, @project) ? @project : (current_user && current_user.fork_of(@project))
 - new_merge_request_path = project_new_merge_request_path(merge_project) if merge_project
 
 - page_title "Merge Requests"
diff --git a/db/fixtures/development/10_merge_requests.rb b/db/fixtures/development/10_merge_requests.rb
index 30244ee4431..bcfdd058a1c 100644
--- a/db/fixtures/development/10_merge_requests.rb
+++ b/db/fixtures/development/10_merge_requests.rb
@@ -4,7 +4,7 @@ Gitlab::Seeder.quiet do
   # Limit the number of merge requests per project to avoid long seeds
   MAX_NUM_MERGE_REQUESTS = 10
 
-  Project.all.reject(&:empty_repo?).each do |project|
+  Project.non_archived.with_merge_requests_enabled.reject(&:empty_repo?).each do |project|
     branches = project.repository.branch_names.sample(MAX_NUM_MERGE_REQUESTS * 2)
 
     branches.each do |branch_name|
@@ -21,7 +21,11 @@ Gitlab::Seeder.quiet do
         assignee: project.team.users.sample
       }
 
-      MergeRequests::CreateService.new(project, project.team.users.sample, params).execute
+      # Only create MRs with users that are allowed to create MRs
+      developer = project.team.developers.sample
+      break unless developer
+
+      MergeRequests::CreateService.new(project, developer, params).execute
       print '.'
     end
   end
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 4c01c320841..33d5a61c563 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -14,7 +14,7 @@ describe ProjectPolicy do
       read_project read_board read_list read_wiki read_issue
       read_project_for_iids read_issue_iid read_merge_request_iid read_label
       read_milestone read_project_snippet read_project_member read_note
-      create_project create_issue create_note upload_file create_merge_request_in_project
+      create_project create_issue create_note upload_file create_merge_request_in
     ]
   end
 
@@ -35,7 +35,7 @@ describe ProjectPolicy do
     %i[
       admin_milestone admin_merge_request update_merge_request create_commit_status
       update_commit_status create_build update_build create_pipeline
-      update_pipeline create_merge_request create_wiki push_code
+      update_pipeline create_merge_request_from create_wiki push_code
       resolve_note create_container_image update_container_image
       create_environment create_deployment
     ]
@@ -142,9 +142,9 @@ describe ProjectPolicy do
     it 'disallows all permissions when the feature is disabled' do
       project.project_feature.update(merge_requests_access_level: ProjectFeature::DISABLED)
 
-      mr_permissions = [:create_merge_request, :read_merge_request,
+      mr_permissions = [:create_merge_request_from, :read_merge_request,
                         :update_merge_request, :admin_merge_request,
-                        :create_merge_request_in_project]
+                        :create_merge_request_in]
 
       expect_disallowed(*mr_permissions)
     end
@@ -159,7 +159,8 @@ describe ProjectPolicy do
 
     let(:other_write_abilities) do
       %i[
-        create_merge_request_in_project
+        create_merge_request_in
+        create_merge_request_from
         push_to_delete_protected_branch
         push_code
         request_access
@@ -192,7 +193,7 @@ describe ProjectPolicy do
       context 'when a project has pending invites' do
         let(:group) { create(:group, :public) }
         let(:project) { create(:project, :public, namespace: group) }
-        let(:user_permissions) { [:create_project, :create_issue, :create_note, :upload_file] }
+        let(:user_permissions) { [:create_merge_request_in, :create_project, :create_issue, :create_note, :upload_file] }
         let(:anonymous_permissions) { guest_permissions - user_permissions  }
 
         subject { described_class.new(nil, project) }

From 12dd2b0cc0cf6dd8dc43ff8b8df8687268ba4af5 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Fri, 6 Apr 2018 16:02:36 +0200
Subject: [PATCH 10/18] Share collaboration check between view and presenter

---
 .../concerns/checks_collaboration.rb          | 21 +++++++
 .../projects/application_controller.rb        | 18 +-----
 app/helpers/commits_helper.rb                 |  2 +-
 app/presenters/merge_request_presenter.rb     | 15 +----
 .../projects/commit/_commit_box.html.haml     |  2 +-
 app/views/projects/commit/show.html.haml      |  2 +-
 .../concerns/checks_collaboration_spec.rb     | 55 +++++++++++++++++++
 .../projects/tree/show.html.haml_spec.rb      |  1 +
 8 files changed, 84 insertions(+), 32 deletions(-)
 create mode 100644 app/controllers/concerns/checks_collaboration.rb
 create mode 100644 spec/controllers/concerns/checks_collaboration_spec.rb

diff --git a/app/controllers/concerns/checks_collaboration.rb b/app/controllers/concerns/checks_collaboration.rb
new file mode 100644
index 00000000000..81367663a06
--- /dev/null
+++ b/app/controllers/concerns/checks_collaboration.rb
@@ -0,0 +1,21 @@
+module ChecksCollaboration
+  def can_collaborate_with_project?(project, ref: nil)
+    return true if can?(current_user, :push_code, project)
+
+    can_create_merge_request =
+      can?(current_user, :create_merge_request_in, project) &&
+      current_user.already_forked?(project)
+
+    can_create_merge_request ||
+      user_access(project).can_push_to_branch?(ref)
+  end
+
+  # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  # enabling this so we can easily cache the user access value as it might be
+  # used across multiple calls in the view
+  def user_access(project)
+    @user_access ||= {}
+    @user_access[project] ||= Gitlab::UserAccess.new(current_user, project: project)
+  end
+  # rubocop:enable Gitlab/ModuleWithInstanceVariables
+end
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index 53a0c712e49..032bb2267e7 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -1,5 +1,6 @@
 class Projects::ApplicationController < ApplicationController
   include RoutableActions
+  include ChecksCollaboration
 
   skip_before_action :authenticate_user!
   before_action :project
@@ -31,18 +32,6 @@ class Projects::ApplicationController < ApplicationController
     @repository ||= project.repository
   end
 
-  def can_collaborate_with_project?(project = nil, ref: nil)
-    project ||= @project
-
-    can_create_merge_request =
-      can?(current_user, :create_merge_request_in, project) &&
-      current_user.already_forked?(project)
-
-    can?(current_user, :push_code, project) ||
-      can_create_merge_request ||
-      user_access(project).can_push_to_branch?(ref)
-  end
-
   def authorize_action!(action)
     unless can?(current_user, action, project)
       return access_denied!
@@ -95,9 +84,4 @@ class Projects::ApplicationController < ApplicationController
   def check_issues_available!
     return render_404 unless @project.feature_available?(:issues, current_user)
   end
-
-  def user_access(project)
-    @user_access ||= {}
-    @user_access[project] ||= Gitlab::UserAccess.new(current_user, project: project)
-  end
 end
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index 7cc56de24e4..98894b86551 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -163,7 +163,7 @@ module CommitsHelper
     tooltip = "#{action.capitalize} this #{commit.change_type_title(current_user)} in a new merge request" if has_tooltip
     btn_class = "btn btn-#{btn_class}" unless btn_class.nil?
 
-    if can_collaborate_with_project?
+    if can_collaborate_with_project?(@project)
       link_to action.capitalize, "#modal-#{action}-commit", 'data-toggle' => 'modal', 'data-container' => 'body', title: (tooltip if has_tooltip), class: "#{btn_class} #{'has-tooltip' if has_tooltip}"
     elsif can?(current_user, :fork_project, @project)
       continue_params = {
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index a55af1422e8..4b4132af2d0 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -3,6 +3,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
   include GitlabRoutingHelper
   include MarkupHelper
   include TreeHelper
+  include ChecksCollaboration
   include Gitlab::Utils::StrongMemoize
 
   presents :merge_request
@@ -152,11 +153,11 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
   end
 
   def can_revert_on_current_merge_request?
-    user_can_collaborate_with_project? && cached_can_be_reverted?
+    can_collaborate_with_project?(project) && cached_can_be_reverted?
   end
 
   def can_cherry_pick_on_current_merge_request?
-    user_can_collaborate_with_project? && can_be_cherry_picked?
+    can_collaborate_with_project?(project) && can_be_cherry_picked?
   end
 
   def can_push_to_source_branch?
@@ -195,16 +196,6 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
     end.sort.to_sentence
   end
 
-  def user_can_collaborate_with_project?
-    can_create_merge_request =
-      can?(current_user, :create_merge_request_in, project) &&
-      current_user.already_forked?(project)
-
-    can?(current_user, :push_code, project) ||
-      can_create_merge_request ||
-      can_push_to_source_branch?
-  end
-
   def user_can_fork_project?
     can?(current_user, :fork_project, project)
   end
diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml
index aec58a91a71..e391f08acca 100644
--- a/app/views/projects/commit/_commit_box.html.haml
+++ b/app/views/projects/commit/_commit_box.html.haml
@@ -35,7 +35,7 @@
         - unless @commit.has_been_reverted?(current_user)
           %li.clearfix
             = revert_commit_link(@commit, project_commit_path(@project, @commit.id), has_tooltip: false)
-        - if can_collaborate_with_project?
+        - if can_collaborate_with_project?(@project)
           %li.clearfix
             = cherry_pick_commit_link(@commit, project_commit_path(@project, @commit.id), has_tooltip: false)
         - if can?(current_user, :push_code, @project)
diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml
index abb292f8f27..541ae905246 100644
--- a/app/views/projects/commit/show.html.haml
+++ b/app/views/projects/commit/show.html.haml
@@ -17,6 +17,6 @@
 
   .limited-width-notes
     = render "shared/notes/notes_with_form", :autocomplete => true
-    - if can_collaborate_with_project?
+    - if can_collaborate_with_project?(@project)
       - %w(revert cherry-pick).each do |type|
         = render "projects/commit/change", type: type, commit: @commit, title: @commit.title
diff --git a/spec/controllers/concerns/checks_collaboration_spec.rb b/spec/controllers/concerns/checks_collaboration_spec.rb
new file mode 100644
index 00000000000..1bd764290ae
--- /dev/null
+++ b/spec/controllers/concerns/checks_collaboration_spec.rb
@@ -0,0 +1,55 @@
+require 'spec_helper'
+
+describe ChecksCollaboration do
+  include ProjectForksHelper
+
+  let(:helper) do
+    fake_class = Class.new(ApplicationController) do
+      include ChecksCollaboration
+    end
+
+    fake_class.new
+  end
+
+  describe '#can_collaborate_with_project?' do
+    let(:user) { create(:user) }
+    let(:project) { create(:project, :public) }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+      allow(helper).to receive(:can?) do |user, ability, subject|
+        Ability.allowed?(user, ability, subject)
+      end
+    end
+
+    it 'is true if the user can push to the project'  do
+      project.add_developer(user)
+
+      expect(helper.can_collaborate_with_project?(project)).to be_truthy
+    end
+
+    it 'is true when the user can push to a branch of the project' do
+      fake_access = double('Gitlab::UserAccess')
+      expect(fake_access).to receive(:can_push_to_branch?).with('a-branch').and_return(true)
+      expect(Gitlab::UserAccess).to receive(:new).with(user, project: project).and_return(fake_access)
+
+      expect(helper.can_collaborate_with_project?(project, ref: 'a-branch')).to be_truthy
+    end
+
+    context 'when the user has forked the project' do
+      before do
+        fork_project(project, user, namespace: user.namespace)
+      end
+
+      it 'is true' do
+        expect(helper.can_collaborate_with_project?(project)).to be_truthy
+      end
+
+      it 'is false when the project is archived' do
+        project.archived = true
+
+        expect(helper.can_collaborate_with_project?(project)).to be_falsy
+      end
+    end
+  end
+end
diff --git a/spec/views/projects/tree/show.html.haml_spec.rb b/spec/views/projects/tree/show.html.haml_spec.rb
index 3b098320ad7..6fa878eceaf 100644
--- a/spec/views/projects/tree/show.html.haml_spec.rb
+++ b/spec/views/projects/tree/show.html.haml_spec.rb
@@ -12,6 +12,7 @@ describe 'projects/tree/show' do
     assign(:lfs_blob_ids, [])
 
     allow(view).to receive(:can?).and_return(true)
+    allow(view).to receive(:can_contribute_to_project?).and_return(true)
     allow(view).to receive(:can_collaborate_with_project?).and_return(true)
     allow(view).to receive_message_chain('user_access.can_push_to_branch?').and_return(true)
     allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings)

From fe9389ba6c10e9addefc6e647f1476ff765236d9 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Fri, 6 Apr 2018 16:04:48 +0200
Subject: [PATCH 11/18] Hide new mr from dropdown for archived projects

---
 app/helpers/merge_requests_helper.rb          | 12 +++
 app/views/layouts/header/_new_dropdown.haml   |  2 +-
 .../user_sees_collaboration_links_spec.rb     | 87 +++++++++++++++++++
 3 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 spec/features/projects/show/user_sees_collaboration_links_spec.rb

diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index fb4fe1c40b7..c19c5b9cc82 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -138,6 +138,18 @@ module MergeRequestsHelper
     end
   end
 
+  def merge_request_source_project_for_project(project = @project)
+    unless can?(current_user, :create_merge_request_in, project)
+      return nil
+    end
+
+    if can?(current_user, :create_merge_request_from, project)
+      project
+    else
+      current_user.fork_of(project)
+    end
+  end
+
   def merge_params_ee(merge_request)
     {}
   end
diff --git a/app/views/layouts/header/_new_dropdown.haml b/app/views/layouts/header/_new_dropdown.haml
index eb32f393310..7373df25e78 100644
--- a/app/views/layouts/header/_new_dropdown.haml
+++ b/app/views/layouts/header/_new_dropdown.haml
@@ -20,7 +20,7 @@
 
       - if @project&.persisted?
         - create_project_issue = can?(current_user, :create_issue, @project)
-        - merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project))
+        - merge_project = merge_request_source_project_for_project(@project)
         - create_project_snippet = can?(current_user, :create_project_snippet, @project)
         - if create_project_issue || merge_project || create_project_snippet
           %li.dropdown-bold-header This project
diff --git a/spec/features/projects/show/user_sees_collaboration_links_spec.rb b/spec/features/projects/show/user_sees_collaboration_links_spec.rb
new file mode 100644
index 00000000000..7b3711531c6
--- /dev/null
+++ b/spec/features/projects/show/user_sees_collaboration_links_spec.rb
@@ -0,0 +1,87 @@
+require 'spec_helper'
+
+describe 'Projects > Show > Collaboration links' do
+  let(:project) { create(:project, :repository) }
+  let(:user) { create(:user) }
+
+  before do
+    project.add_developer(user)
+    sign_in(user)
+  end
+
+  it 'shows all the expected links' do
+    visit project_path(project)
+
+    # The navigation bar
+    page.within('.header-new') do
+      aggregate_failures 'dropdown links in the navigation bar' do
+        expect(page).to have_link('New issue')
+        expect(page).to have_link('New merge request')
+        expect(page).to have_link('New snippet', href: new_project_snippet_path(project))
+      end
+    end
+
+    # The project header
+    page.within('.project-home-panel') do
+      aggregate_failures 'dropdown links in the project home panel' do
+        expect(page).to have_link('New issue')
+        expect(page).to have_link('New merge request')
+        expect(page).to have_link('New snippet')
+        expect(page).to have_link('New file')
+        expect(page).to have_link('New branch')
+        expect(page).to have_link('New tag')
+      end
+    end
+
+    # The dropdown above the tree
+    page.within('.repo-breadcrumb') do
+      aggregate_failures 'dropdown links above the repo tree' do
+        expect(page).to have_link('New file')
+        expect(page).to have_link('Upload file')
+        expect(page).to have_link('New directory')
+        expect(page).to have_link('New branch')
+        expect(page).to have_link('New tag')
+      end
+    end
+
+    # The Web IDE
+    expect(page).to have_link('Web IDE')
+  end
+
+  it 'hides the links when the project is archived' do
+    project.update!(archived: true)
+
+    visit project_path(project)
+
+    page.within('.header-new') do
+      aggregate_failures 'dropdown links' do
+        expect(page).not_to have_link('New issue')
+        expect(page).not_to have_link('New merge request')
+        expect(page).not_to have_link('New snippet', href: new_project_snippet_path(project))
+      end
+    end
+
+    page.within('.project-home-panel') do
+      aggregate_failures 'dropdown links' do
+        expect(page).not_to have_link('New issue')
+        expect(page).not_to have_link('New merge request')
+        expect(page).not_to have_link('New snippet')
+        expect(page).not_to have_link('New file')
+        expect(page).not_to have_link('New branch')
+        expect(page).not_to have_link('New tag')
+      end
+    end
+
+    page.within('.repo-breadcrumb') do
+      aggregate_failures 'dropdown links' do
+        expect(page).not_to have_link('New file')
+        expect(page).not_to have_link('Upload file')
+        expect(page).not_to have_link('New directory')
+        expect(page).not_to have_link('New branch')
+        expect(page).not_to have_link('New tag')
+      end
+    end
+
+    expect(page).not_to have_link('Web IDE')
+  end
+end

From 1747a573dff40ab51ba892bf9f5293e3c9b794d6 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Fri, 6 Apr 2018 17:07:26 +0200
Subject: [PATCH 12/18] Hide `new merge request` button from index page

When a project is archived, don't show the button on the index page
of merge requests
---
 app/views/projects/merge_requests/index.html.haml    |  2 +-
 .../user_views_open_merge_requests_spec.rb           | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/app/views/projects/merge_requests/index.html.haml b/app/views/projects/merge_requests/index.html.haml
index f9ab0f5709b..623380c9c61 100644
--- a/app/views/projects/merge_requests/index.html.haml
+++ b/app/views/projects/merge_requests/index.html.haml
@@ -1,6 +1,6 @@
 - @no_container = true
 - @can_bulk_update = can?(current_user, :admin_merge_request, @project)
-- merge_project = can?(current_user, :create_merge_request_from, @project) ? @project : (current_user && current_user.fork_of(@project))
+- merge_project = merge_request_source_project_for_project(@project)
 - new_merge_request_path = project_new_merge_request_path(merge_project) if merge_project
 
 - page_title "Merge Requests"
diff --git a/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb b/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb
index bf95dbb7d09..08826625842 100644
--- a/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb
+++ b/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb
@@ -94,6 +94,18 @@ describe 'User views open merge requests' do
       end
 
       include_examples 'shows merge requests'
+
+      it 'shows the new merge request button' do
+        expect(page).to have_link('New merge request')
+      end
+
+      context 'when the project is archived' do
+        let(:project) { create(:project, :public, :repository, archived: true) }
+
+        it 'hides the new merge request button' do
+          expect(page).not_to have_link('New merge request')
+        end
+      end
     end
   end
 

From 083b0a9b034a0e4004c012444f1a8811bd96ed29 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Fri, 6 Apr 2018 18:27:12 +0200
Subject: [PATCH 13/18] Hide file/directory creation buttons

---
 app/helpers/blob_helper.rb                    |  4 ++--
 app/views/projects/_last_push.html.haml       |  5 +++--
 app/views/projects/branches/_branch.html.haml |  2 +-
 .../projects/buttons/_dropdown.html.haml      | 22 +++++++++++--------
 .../projects/commit/_commit_box.html.haml     |  6 +++--
 .../projects/issues/_new_branch.html.haml     |  8 +++----
 .../projects/tree/_tree_header.html.haml      | 14 +++++++-----
 spec/features/projects/branches_spec.rb       | 22 ++++++++++++++++++-
 .../buttons/_dropdown.html.haml_spec.rb       |  3 ++-
 .../projects/tree/show.html.haml_spec.rb      |  1 -
 10 files changed, 59 insertions(+), 28 deletions(-)

diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb
index a5f7a7204df..866b8773db6 100644
--- a/app/helpers/blob_helper.rb
+++ b/app/helpers/blob_helper.rb
@@ -59,7 +59,7 @@ module BlobHelper
       button_tag label, class: "#{common_classes} disabled has-tooltip", title: "It is not possible to #{action} files that are stored in LFS using the web interface", data: { container: 'body' }
     elsif can_modify_blob?(blob, project, ref)
       button_tag label, class: "#{common_classes}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal'
-    elsif can?(current_user, :create_merge_request_in, project)
+    elsif can?(current_user, :fork_project, project) && can?(current_user, :create_merge_request_in, project)
       edit_fork_button_tag(common_classes, project, label, edit_modify_file_fork_params(action), action)
     end
   end
@@ -280,7 +280,7 @@ module BlobHelper
       options << link_to("submit an issue", new_project_issue_path(project))
     end
 
-    merge_project = can?(current_user, :create_merge_request_from, project) ? project : (current_user && current_user.fork_of(project))
+    merge_project = merge_request_source_project_for_project(@project)
     if merge_project
       options << link_to("create a merge request", project_new_merge_request_path(project))
     end
diff --git a/app/views/projects/_last_push.html.haml b/app/views/projects/_last_push.html.haml
index 6a1035d2dc7..e9a8eae4375 100644
--- a/app/views/projects/_last_push.html.haml
+++ b/app/views/projects/_last_push.html.haml
@@ -14,5 +14,6 @@
         #{time_ago_with_tooltip(event.created_at)}
 
       .flex-right
-        = link_to new_mr_path_from_push_event(event), title: _("New merge request"), class: "btn btn-info btn-sm qa-create-merge-request" do
-          #{ _('Create merge request') }
+        - if can?(current_user, :create_merge_request_in, @project)
+          = link_to new_mr_path_from_push_event(event), title: _("New merge request"), class: "btn btn-info btn-sm qa-create-merge-request" do
+            #{ _('Create merge request') }
diff --git a/app/views/projects/branches/_branch.html.haml b/app/views/projects/branches/_branch.html.haml
index 6ca33c7524d..71176acd12d 100644
--- a/app/views/projects/branches/_branch.html.haml
+++ b/app/views/projects/branches/_branch.html.haml
@@ -4,7 +4,7 @@
 - diverging_commit_counts = @repository.diverging_commit_counts(branch)
 - number_commits_behind = diverging_commit_counts[:behind]
 - number_commits_ahead = diverging_commit_counts[:ahead]
-- merge_project = can?(current_user, :create_merge_request_from, @project) ? @project : (current_user && current_user.fork_of(@project))
+- merge_project = merge_request_source_project_for_project(@project)
 %li{ class: "branch-item js-branch-#{branch.name}" }
   .branch-info
     .branch-title
diff --git a/app/views/projects/buttons/_dropdown.html.haml b/app/views/projects/buttons/_dropdown.html.haml
index 18e948ce35a..268c8eba9c5 100644
--- a/app/views/projects/buttons/_dropdown.html.haml
+++ b/app/views/projects/buttons/_dropdown.html.haml
@@ -1,13 +1,17 @@
-- if current_user
+- can_create_issue = show_new_issue_link?
+- can_create_project_snippet = can?(current_user, :create_project_snippet, @project)
+- can_push_code = can?(current_user, :push_code, @project)
+- create_mr_from_new_fork = can?(current_user, :fork_project, @project) && can?(current_user, :create_merge_request_in, @project)
+- merge_project = merge_request_source_project_for_project(@project)
+
+- show_menu = can_create_issue || can_create_project_snippet || can_push_code || create_mr_from_new_fork || merge_project
+
+- if show_menu
   .project-action-button.dropdown.inline
     %a.btn.dropdown-toggle.has-tooltip{ href: '#', title: _('Create new...'), 'data-toggle' => 'dropdown', 'data-container' => 'body', 'aria-label' => _('Create new...') }
       = icon('plus')
       = icon("caret-down")
     %ul.dropdown-menu.dropdown-menu-align-right.project-home-dropdown
-      - can_create_issue = can?(current_user, :create_issue, @project)
-      - merge_project = can?(current_user, :create_merge_request, @project) ? @project : (current_user && current_user.fork_of(@project))
-      - can_create_project_snippet = can?(current_user, :create_project_snippet, @project)
-
       - if can_create_issue || merge_project || can_create_project_snippet
         %li.dropdown-header= _('This project')
 
@@ -20,17 +24,17 @@
       - if can_create_project_snippet
         %li= link_to _('New snippet'), new_project_snippet_path(@project)
 
-      - if can?(current_user, :push_code, @project)
+      - if can_push_code
         %li.dropdown-header= _('This repository')
 
-      - if can?(current_user, :push_code, @project)
+      - if can_push_code
         %li= link_to _('New file'), project_new_blob_path(@project, @project.default_branch || 'master')
         - unless @project.empty_repo?
           %li= link_to _('New branch'), new_project_branch_path(@project)
           %li= link_to _('New tag'), new_project_tag_path(@project)
-      - elsif current_user && current_user.already_forked?(@project)
+      - elsif can_collaborate_with_project?(@project)
         %li= link_to _('New file'), project_new_blob_path(@project, @project.default_branch || 'master')
-      - elsif can?(current_user, :fork_project, @project)
+      - elsif create_mr_from_new_fork
         - continue_params = { to:         project_new_blob_path(@project, @project.default_branch || 'master'),
                               notice:     edit_in_new_fork_notice,
                               notice_now: edit_in_new_fork_notice_now }
diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml
index e391f08acca..213c4c90a0e 100644
--- a/app/views/projects/commit/_commit_box.html.haml
+++ b/app/views/projects/commit/_commit_box.html.haml
@@ -1,3 +1,5 @@
+- can_collaborate = can_collaborate_with_project?(@project)
+
 .page-content-header.js-commit-box{ 'data-commit-path' => branches_project_commit_path(@project, @commit.id) }
   .header-main-content
     = render partial: 'signature', object: @commit.signature
@@ -32,10 +34,10 @@
         %li.visible-xs-block.visible-sm-block
           = link_to project_tree_path(@project, @commit) do
             #{ _('Browse Files') }
-        - unless @commit.has_been_reverted?(current_user)
+        - if can_collaborate && !@commit.has_been_reverted?(current_user)
           %li.clearfix
             = revert_commit_link(@commit, project_commit_path(@project, @commit.id), has_tooltip: false)
-        - if can_collaborate_with_project?(@project)
+        - if can_collaborate
           %li.clearfix
             = cherry_pick_commit_link(@commit, project_commit_path(@project, @commit.id), has_tooltip: false)
         - if can?(current_user, :push_code, @project)
diff --git a/app/views/projects/issues/_new_branch.html.haml b/app/views/projects/issues/_new_branch.html.haml
index 02ea3dc74fe..4b8bf578b28 100644
--- a/app/views/projects/issues/_new_branch.html.haml
+++ b/app/views/projects/issues/_new_branch.html.haml
@@ -1,8 +1,8 @@
-- can_create_merge_request = can?(current_user, :create_merge_request_from, @project)
-- data_action = can_create_merge_request ? 'create-mr' : 'create-branch'
-- value = can_create_merge_request ? 'Create merge request' : 'Create branch'
-
 - if can?(current_user, :push_code, @project)
+  - can_create_merge_request = can?(current_user, :create_merge_request_in, @project)
+  - data_action = can_create_merge_request ? 'create-mr' : 'create-branch'
+  - value = can_create_merge_request ? 'Create merge request' : 'Create branch'
+
   - can_create_path = can_create_branch_project_issue_path(@project, @issue)
   - create_mr_path = create_merge_request_project_issue_path(@project, @issue, branch_name: @issue.to_branch_name, ref: @project.default_branch)
   - create_branch_path = project_branches_path(@project, branch_name: @issue.to_branch_name, ref: @project.default_branch, issue_iid: @issue.iid)
diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml
index 59cf78420fb..8587d3b0c0d 100644
--- a/app/views/projects/tree/_tree_header.html.haml
+++ b/app/views/projects/tree/_tree_header.html.haml
@@ -1,3 +1,6 @@
+- can_collaborate = can_collaborate_with_project?(@project)
+- can_create_mr_from_fork = can?(current_user, :fork_project, @project) && can?(current_user, :create_merge_request_in, @project)
+
 .tree-ref-container
   .tree-ref-holder
     = render 'shared/ref_switcher', destination: 'tree', path: @path, show_create: true
@@ -15,7 +18,7 @@
       %li
         = link_to truncate(title, length: 40), project_tree_path(@project, tree_join(@ref, path))
 
-    - if current_user
+    - if can_collaborate || can_create_mr_from_fork
       %li
         %a.btn.add-to-tree{ addtotree_toggle_attributes }
           = sprite_icon('plus', size: 16, css_class: 'pull-left')
@@ -35,7 +38,7 @@
                 %li
                   = link_to '#modal-create-new-dir', { 'data-target' => '#modal-create-new-dir', 'data-toggle' => 'modal' } do
                     #{ _('New directory') }
-              - elsif can?(current_user, :fork_project, @project)
+              - elsif can?(current_user, :fork_project, @project) && can?(current_user, :create_merge_request_in, @project)
                 %li
                   - continue_params = { to:         project_new_blob_path(@project, @id),
                                         notice:     edit_in_new_fork_notice,
@@ -77,8 +80,9 @@
 
   = render 'projects/find_file_link'
 
-  = succeed " " do
-    = link_to ide_edit_path(@project, @id, ""), class: 'btn btn-default' do
-      = _('Web IDE')
+  - if can_collaborate
+    = succeed " " do
+      = link_to ide_edit_path(@project, @id, ""), class: 'btn btn-default' do
+        = _('Web IDE')
 
   = render 'projects/buttons/download', project: @project, ref: @ref
diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb
index 2a9d9e6416c..acf478cd117 100644
--- a/spec/features/projects/branches_spec.rb
+++ b/spec/features/projects/branches_spec.rb
@@ -195,6 +195,26 @@ describe 'Branches' do
         expect(page).to have_content("Protected branches can be managed in project settings")
       end
     end
+
+    it 'shows the merge request button' do
+      visit project_branches_path(project)
+
+      page.within first('.all-branches li') do
+        expect(page).to have_content 'Merge request'
+      end
+    end
+
+    context 'when the project is archived' do
+      let(:project) { create(:project, :public, :repository, archived: true) }
+
+      it 'does not show the merge request button when the project is archived' do
+        visit project_branches_path(project)
+
+        page.within first('.all-branches li') do
+          expect(page).not_to have_content 'Merge request'
+        end
+      end
+    end
   end
 
   context 'logged out' do
@@ -204,7 +224,7 @@ describe 'Branches' do
 
     it 'does not show merge request button' do
       page.within first('.all-branches li') do
-        expect(page).not_to have_content 'Merge Request'
+        expect(page).not_to have_content 'Merge request'
       end
     end
   end
diff --git a/spec/views/projects/buttons/_dropdown.html.haml_spec.rb b/spec/views/projects/buttons/_dropdown.html.haml_spec.rb
index d0e692635b9..8b9aab30286 100644
--- a/spec/views/projects/buttons/_dropdown.html.haml_spec.rb
+++ b/spec/views/projects/buttons/_dropdown.html.haml_spec.rb
@@ -8,7 +8,8 @@ describe 'projects/buttons/_dropdown' do
       assign(:project, project)
 
       allow(view).to receive(:current_user).and_return(user)
-      allow(view).to receive(:can?).and_return(true)
+      allow(view).to receive(:can?).with(user, :push_code, project).and_return(true)
+      allow(view).to receive(:can_collaborate_with_project?).and_return(true)
     end
 
     context 'empty repository' do
diff --git a/spec/views/projects/tree/show.html.haml_spec.rb b/spec/views/projects/tree/show.html.haml_spec.rb
index 6fa878eceaf..3b098320ad7 100644
--- a/spec/views/projects/tree/show.html.haml_spec.rb
+++ b/spec/views/projects/tree/show.html.haml_spec.rb
@@ -12,7 +12,6 @@ describe 'projects/tree/show' do
     assign(:lfs_blob_ids, [])
 
     allow(view).to receive(:can?).and_return(true)
-    allow(view).to receive(:can_contribute_to_project?).and_return(true)
     allow(view).to receive(:can_collaborate_with_project?).and_return(true)
     allow(view).to receive_message_chain('user_access.can_push_to_branch?').and_return(true)
     allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings)

From 71ccfde322b633e9245bee6acba1e64cb7640f19 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Fri, 6 Apr 2018 19:58:53 +0200
Subject: [PATCH 14/18] Correct permissions for creating merge requests from
 issues

This could only be possible for users that can create merge requests
within a project.

So they need to be a allowed to create a branch and create a merge request.
---
 app/controllers/projects/issues_controller.rb  |  2 +-
 app/views/projects/_last_push.html.haml        |  4 ++--
 lib/api/merge_requests.rb                      |  2 +-
 lib/api/v3/merge_requests.rb                   |  2 +-
 .../handler/create_merge_request_handler.rb    |  3 ++-
 .../projects/issues_controller_spec.rb         | 18 +++++++++++++++++-
 spec/requests/api/merge_requests_spec.rb       |  4 ++--
 spec/requests/api/v3/merge_requests_spec.rb    |  4 ++--
 8 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 9dcd13b9385..767e492f566 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -20,7 +20,7 @@ class Projects::IssuesController < Projects::ApplicationController
   before_action :authorize_update_issuable!, only: [:edit, :update, :move]
 
   # Allow create a new branch and empty WIP merge request from current issue
-  before_action :authorize_create_merge_request_in!, only: [:create_merge_request]
+  before_action :authorize_create_merge_request_from!, only: [:create_merge_request]
 
   respond_to :html
 
diff --git a/app/views/projects/_last_push.html.haml b/app/views/projects/_last_push.html.haml
index e9a8eae4375..f6d396c8127 100644
--- a/app/views/projects/_last_push.html.haml
+++ b/app/views/projects/_last_push.html.haml
@@ -13,7 +13,7 @@
 
         #{time_ago_with_tooltip(event.created_at)}
 
-      .flex-right
-        - if can?(current_user, :create_merge_request_in, @project)
+      - if can?(current_user, :create_merge_request_in, event.project.default_merge_request_target)
+        .flex-right
           = link_to new_mr_path_from_push_event(event), title: _("New merge request"), class: "btn btn-info btn-sm qa-create-merge-request" do
             #{ _('Create merge request') }
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 3264a26f7d2..d4cc18f622b 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -189,7 +189,7 @@ module API
       post ":id/merge_requests" do
         Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42316')
 
-        authorize! :create_merge_request, user_project
+        authorize! :create_merge_request_from, user_project
 
         mr_params = declared_params(include_missing: false)
         mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch)
diff --git a/lib/api/v3/merge_requests.rb b/lib/api/v3/merge_requests.rb
index ce216497996..9b0f70e2bfe 100644
--- a/lib/api/v3/merge_requests.rb
+++ b/lib/api/v3/merge_requests.rb
@@ -93,7 +93,7 @@ module API
         post ":id/merge_requests" do
           Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42126')
 
-          authorize! :create_merge_request, user_project
+          authorize! :create_merge_request_from, user_project
 
           mr_params = declared_params(include_missing: false)
           mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) if mr_params[:remove_source_branch].present?
diff --git a/lib/gitlab/email/handler/create_merge_request_handler.rb b/lib/gitlab/email/handler/create_merge_request_handler.rb
index 3436306e122..2f864f2082b 100644
--- a/lib/gitlab/email/handler/create_merge_request_handler.rb
+++ b/lib/gitlab/email/handler/create_merge_request_handler.rb
@@ -23,7 +23,8 @@ module Gitlab
         def execute
           raise ProjectNotFound unless project
 
-          validate_permission!(:create_merge_request)
+          validate_permission!(:create_merge_request_in)
+          validate_permission!(:create_merge_request_from)
 
           verify_record!(
             record: create_merge_request,
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 01b5506b64b..6c6b1e4a500 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -938,7 +938,7 @@ describe Projects::IssuesController do
   end
 
   describe 'POST create_merge_request' do
-    let(:project) { create(:project, :repository) }
+    let(:project) { create(:project, :repository, :public) }
 
     before do
       project.add_developer(user)
@@ -955,6 +955,22 @@ describe Projects::IssuesController do
       expect(response).to match_response_schema('merge_request')
     end
 
+    it 'is not available when the project is archived' do
+      project.update(archived: true)
+
+      create_merge_request
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    it 'is not available for users who cannot create merge requests' do
+      sign_in(create(:user))
+
+      create_merge_request
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
     def create_merge_request
       post :create_merge_request, namespace_id: project.namespace.to_param,
                                   project_id: project.to_param,
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 3764aec0c71..f64623d7018 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -861,7 +861,7 @@ describe API::MergeRequests do
         expect(json_response['title']).to eq('Test merge_request')
       end
 
-      it 'returns 422 when target project has disabled merge requests' do
+      it 'returns 403 when target project has disabled merge requests' do
         project.project_feature.update(merge_requests_access_level: 0)
 
         post api("/projects/#{forked_project.id}/merge_requests", user2),
@@ -871,7 +871,7 @@ describe API::MergeRequests do
              author: user2,
              target_project_id: project.id
 
-        expect(response).to have_gitlab_http_status(422)
+        expect(response).to have_gitlab_http_status(403)
       end
 
       it "returns 400 when source_branch is missing" do
diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb
index 6b748369f0d..be70cb24dce 100644
--- a/spec/requests/api/v3/merge_requests_spec.rb
+++ b/spec/requests/api/v3/merge_requests_spec.rb
@@ -340,7 +340,7 @@ describe API::MergeRequests do
         expect(json_response['title']).to eq('Test merge_request')
       end
 
-      it "returns 422 when target project has disabled merge requests" do
+      it "returns 403 when target project has disabled merge requests" do
         project.project_feature.update(merge_requests_access_level: 0)
 
         post v3_api("/projects/#{forked_project.id}/merge_requests", user2),
@@ -350,7 +350,7 @@ describe API::MergeRequests do
              author: user2,
              target_project_id: project.id
 
-        expect(response).to have_gitlab_http_status(422)
+        expect(response).to have_gitlab_http_status(403)
       end
 
       it "returns 400 when source_branch is missing" do

From 04c7d0d55500e6f118bd17153f3af11e83fce826 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Fri, 6 Apr 2018 20:19:37 +0200
Subject: [PATCH 15/18] Prevent awarding emoji when a project is archived

This prevents performing the requests, and disables all emoji reaction buttons
---
 .../notes/components/note_actions.vue         |  9 +--
 .../notes/components/note_awards_list.vue     | 11 +--
 .../notes/components/note_body.vue            |  1 +
 .../notes/components/noteable_note.vue        |  1 +
 app/helpers/issues_helper.rb                  |  4 +-
 app/models/concerns/awardable.rb              | 14 ++--
 app/policies/note_policy.rb                   |  2 +-
 app/policies/personal_snippet_policy.rb       |  2 +
 app/policies/project_policy.rb                |  2 +
 app/serializers/issue_entity.rb               |  4 ++
 app/serializers/note_entity.rb                |  4 ++
 app/views/award_emoji/_awards_block.html.haml |  4 +-
 app/views/projects/notes/_actions.html.haml   |  2 +-
 spec/factories/award_emoji.rb                 |  4 ++
 .../merge_request/user_awards_emoji_spec.rb   |  8 +++
 ...ser_interacts_with_awards_in_issue_spec.rb | 68 +++++++++++++++++++
 spec/helpers/issues_helper_spec.rb            | 23 ++++++-
 .../notes/components/note_actions_spec.js     |  4 +-
 .../notes/components/note_awards_list_spec.js | 34 +++++++++-
 .../notes/components/note_body_spec.js        |  1 +
 spec/javascripts/notes/mock_data.js           | 15 +++-
 spec/models/concerns/awardable_spec.rb        | 25 +++++++
 spec/policies/personal_snippet_policy_spec.rb | 11 +++
 spec/policies/project_policy_spec.rb          |  4 +-
 24 files changed, 231 insertions(+), 26 deletions(-)

diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue
index a7e2d857013..626b0799581 100644
--- a/app/assets/javascripts/notes/components/note_actions.vue
+++ b/app/assets/javascripts/notes/components/note_actions.vue
@@ -40,6 +40,10 @@ export default {
       type: Boolean,
       required: true,
     },
+    canAwardEmoji: {
+      type: Boolean,
+      required: true,
+    },
     canDelete: {
       type: Boolean,
       required: true,
@@ -74,9 +78,6 @@ export default {
     shouldShowActionsDropdown() {
       return this.currentUserId && (this.canEdit || this.canReportAsAbuse);
     },
-    canAddAwardEmoji() {
-      return this.currentUserId;
-    },
     isAuthoredByCurrentUser() {
       return this.authorId === this.currentUserId;
     },
@@ -149,7 +150,7 @@ export default {
       </button>
     </div>
     <div
-      v-if="canAddAwardEmoji"
+      v-if="canAwardEmoji"
       class="note-actions-item">
       <a
         v-tooltip
diff --git a/app/assets/javascripts/notes/components/note_awards_list.vue b/app/assets/javascripts/notes/components/note_awards_list.vue
index 6cb8229e268..e8fd155a1ee 100644
--- a/app/assets/javascripts/notes/components/note_awards_list.vue
+++ b/app/assets/javascripts/notes/components/note_awards_list.vue
@@ -28,6 +28,10 @@ export default {
       type: Number,
       required: true,
     },
+    canAwardEmoji: {
+      type: Boolean,
+      required: true,
+    },
   },
   computed: {
     ...mapGetters(['getUserData']),
@@ -67,9 +71,6 @@ export default {
     isAuthoredByMe() {
       return this.noteAuthorId === this.getUserData.id;
     },
-    isLoggedIn() {
-      return this.getUserData.id;
-    },
   },
   created() {
     this.emojiSmiling = emojiSmiling;
@@ -156,7 +157,7 @@ export default {
       return title;
     },
     handleAward(awardName) {
-      if (!this.isLoggedIn) {
+      if (!this.canAwardEmoji) {
         return;
       }
 
@@ -208,7 +209,7 @@ export default {
         </span>
       </button>
       <div
-        v-if="isLoggedIn"
+        v-if="canAwardEmoji"
         class="award-menu-holder">
         <button
           v-tooltip
diff --git a/app/assets/javascripts/notes/components/note_body.vue b/app/assets/javascripts/notes/components/note_body.vue
index 069f94c5845..0cb626c14f4 100644
--- a/app/assets/javascripts/notes/components/note_body.vue
+++ b/app/assets/javascripts/notes/components/note_body.vue
@@ -112,6 +112,7 @@ export default {
       :note-author-id="note.author.id"
       :awards="note.award_emoji"
       :toggle-award-path="note.toggle_award_path"
+      :can-award-emoji="note.current_user.can_award_emoji"
     />
     <note-attachment
       v-if="note.attachment"
diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue
index 3554027d2b4..566f5c68e66 100644
--- a/app/assets/javascripts/notes/components/noteable_note.vue
+++ b/app/assets/javascripts/notes/components/noteable_note.vue
@@ -177,6 +177,7 @@ export default {
             :note-id="note.id"
             :access-level="note.human_access"
             :can-edit="note.current_user.can_edit"
+            :can-award-emoji="note.current_user.can_award_emoji"
             :can-delete="note.current_user.can_edit"
             :can-report-as-abuse="canReportAsAbuse"
             :report-abuse-path="note.report_abuse_path"
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 0f25d401406..5a2f7bb01c7 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -82,8 +82,8 @@ module IssuesHelper
     names.to_sentence
   end
 
-  def award_state_class(awards, current_user)
-    if !current_user
+  def award_state_class(awardable, awards, current_user)
+    if !can?(current_user, :award_emoji, awardable)
       "disabled"
     elsif current_user && awards.find { |a| a.user_id == current_user.id }
       "active"
diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb
index d8394415362..fce37e7f78e 100644
--- a/app/models/concerns/awardable.rb
+++ b/app/models/concerns/awardable.rb
@@ -79,11 +79,7 @@ module Awardable
   end
 
   def user_can_award?(current_user, name)
-    if user_authored?(current_user)
-      !awardable_votes?(normalize_name(name))
-    else
-      true
-    end
+    awardable_by_user?(current_user, name) && Ability.allowed?(current_user, :award_emoji, self)
   end
 
   def user_authored?(current_user)
@@ -119,4 +115,12 @@ module Awardable
   def normalize_name(name)
     Gitlab::Emoji.normalize_emoji_name(name)
   end
+
+  def awardable_by_user?(current_user, name)
+    if user_authored?(current_user)
+      !awardable_votes?(normalize_name(name))
+    else
+      true
+    end
+  end
 end
diff --git a/app/policies/note_policy.rb b/app/policies/note_policy.rb
index 6659bf062c4..077a6761ee6 100644
--- a/app/policies/note_policy.rb
+++ b/app/policies/note_policy.rb
@@ -1,6 +1,6 @@
 class NotePolicy < BasePolicy
   delegate { @subject.project }
-  delegate { @subject.noteable if @subject.noteable.lockable? }
+  delegate { @subject.noteable if DeclarativePolicy.has_policy?(@subject.noteable) }
 
   condition(:is_author) { @user && @subject.author == @user }
   condition(:is_noteable_author) { @user && @subject.noteable.author_id == @user.id }
diff --git a/app/policies/personal_snippet_policy.rb b/app/policies/personal_snippet_policy.rb
index cac0530b9f7..c1a84727cfa 100644
--- a/app/policies/personal_snippet_policy.rb
+++ b/app/policies/personal_snippet_policy.rb
@@ -25,4 +25,6 @@ class PersonalSnippetPolicy < BasePolicy
   end
 
   rule { anonymous }.prevent :comment_personal_snippet
+
+  rule { can?(:comment_personal_snippet) }.enable :award_emoji
 end
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index ac154270236..3529d0aa60c 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -155,6 +155,7 @@ class ProjectPolicy < BasePolicy
     enable :create_note
     enable :upload_file
     enable :read_cycle_analytics
+    enable :award_emoji
   end
 
   # These abilities are not allowed to admins that are not members of the project,
@@ -253,6 +254,7 @@ class ProjectPolicy < BasePolicy
     prevent :resolve_note
     prevent :create_merge_request_from
     prevent :create_merge_request_in
+    prevent :award_emoji
 
     READONLY_FEATURES_WHEN_ARCHIVED.each do |feature|
       prevent(*create_update_admin_destroy(feature))
diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb
index b5e2334b6e3..840fdbcbf14 100644
--- a/app/serializers/issue_entity.rb
+++ b/app/serializers/issue_entity.rb
@@ -29,6 +29,10 @@ class IssueEntity < IssuableEntity
     expose :can_update do |issue|
       can?(request.current_user, :update_issue, issue)
     end
+
+    expose :can_award_emoji do |issue|
+      can?(request.current_user, :award_emoji, issue)
+    end
   end
 
   expose :create_note_path do |issue|
diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb
index fa516fee6ec..06d603b277e 100644
--- a/app/serializers/note_entity.rb
+++ b/app/serializers/note_entity.rb
@@ -17,6 +17,10 @@ class NoteEntity < API::Entities::Note
     expose :can_edit do |note|
       Ability.allowed?(request.current_user, :admin_note, note)
     end
+
+    expose :can_award_emoji do |note|
+      Ability.allowed?(request.current_user, :award_emoji, note)
+    end
   end
 
   expose :resolved?, as: :resolved
diff --git a/app/views/award_emoji/_awards_block.html.haml b/app/views/award_emoji/_awards_block.html.haml
index 5f07d2720c2..4b3c52af16a 100644
--- a/app/views/award_emoji/_awards_block.html.haml
+++ b/app/views/award_emoji/_awards_block.html.haml
@@ -3,13 +3,13 @@
 .awards.js-awards-block{ class: ("hidden" if !inline && grouped_emojis.empty?), data: { award_url: toggle_award_url(awardable) } }
   - awards_sort(grouped_emojis).each do |emoji, awards|
     %button.btn.award-control.js-emoji-btn.has-tooltip{ type: "button",
-      class: [(award_state_class(awards, current_user)), (award_user_authored_class(emoji) if user_authored)],
+      class: [(award_state_class(awardable, awards, current_user)), (award_user_authored_class(emoji) if user_authored)],
       data: { placement: "bottom", title: award_user_list(awards, current_user) } }
       = emoji_icon(emoji)
       %span.award-control-text.js-counter
         = awards.count
 
-  - if current_user
+  - if can?(current_user, :award_emoji, awardable)
     .award-menu-holder.js-award-holder
       %button.btn.award-control.has-tooltip.js-add-award{ type: 'button',
         'aria-label': 'Add reaction',
diff --git a/app/views/projects/notes/_actions.html.haml b/app/views/projects/notes/_actions.html.haml
index 5ea653ccad5..b4fe1cabdfd 100644
--- a/app/views/projects/notes/_actions.html.haml
+++ b/app/views/projects/notes/_actions.html.haml
@@ -36,7 +36,7 @@
           %template{ 'v-else' => '' }
             = render 'shared/icons/icon_resolve_discussion.svg'
 
-- if current_user
+- if can?(current_user, :award_emoji, note)
   - if note.emoji_awardable?
     - user_authored = note.user_authored?(current_user)
     .note-actions-item
diff --git a/spec/factories/award_emoji.rb b/spec/factories/award_emoji.rb
index a0abbbce686..d37e2bf511e 100644
--- a/spec/factories/award_emoji.rb
+++ b/spec/factories/award_emoji.rb
@@ -4,6 +4,10 @@ FactoryBot.define do
     user
     awardable factory: :issue
 
+    after(:create) do |award, evaluator|
+      award.awardable.project.add_guest(evaluator.user)
+    end
+
     trait :upvote
     trait :downvote do
       name "thumbsdown"
diff --git a/spec/features/merge_request/user_awards_emoji_spec.rb b/spec/features/merge_request/user_awards_emoji_spec.rb
index 2f24cfbd9e3..fa254e8c6c5 100644
--- a/spec/features/merge_request/user_awards_emoji_spec.rb
+++ b/spec/features/merge_request/user_awards_emoji_spec.rb
@@ -35,6 +35,14 @@ describe 'Merge request > User awards emoji', :js do
 
       expect(page).to have_selector('.emoji-menu', count: 1)
     end
+
+    describe 'the project is archived' do
+      let(:project) { create(:project, :public, :repository, archived: true) }
+
+      it 'does not see award menu button' do
+        expect(page).not_to have_selector('.js-award-holder')
+      end
+    end
   end
 
   describe 'logged out' do
diff --git a/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb b/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb
index adff0a10f0e..8a3d83da93f 100644
--- a/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb
+++ b/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb
@@ -101,4 +101,72 @@ describe 'User interacts with awards in an issue', :js do
 
     expect(page).to have_selector('gl-emoji[data-name="smile"]')
   end
+
+  context 'when a project is archived' do
+    let(:project) { create(:project, archived: true) }
+
+    it 'hides the add award button' do
+      page.within('.awards') do
+        expect(page).not_to have_css('.js-add-award')
+      end
+    end
+  end
+
+  context 'awards on a note' do
+    let!(:note) { create(:note, noteable: issue, project: issue.project) }
+    let!(:award_emoji) { create(:award_emoji, awardable: note, name: '100') }
+
+    it 'shows the award on the note' do
+      page.within('.note-awards') do
+        expect(page).to have_selector('gl-emoji[data-name="100"]')
+      end
+    end
+
+    it 'allows adding a vote to an award' do
+      page.within('.note-awards') do
+        find('gl-emoji[data-name="100"]').click
+      end
+      wait_for_requests
+
+      expect(note.reload.award_emoji.size).to eq(2)
+    end
+
+    it 'allows adding a new emoji' do
+      page.within('.note-actions') do
+        find('a.js-add-award').click
+      end
+      page.within('.emoji-menu-content') do
+        find('gl-emoji[data-name="8ball"]').click
+      end
+      wait_for_requests
+
+      page.within('.note-awards') do
+        expect(page).to have_selector('gl-emoji[data-name="8ball"]')
+      end
+      expect(note.reload.award_emoji.size).to eq(2)
+    end
+
+    context 'when the project is archived' do
+      let(:project) { create(:project, archived: true) }
+
+      it 'hides the buttons for adding new emoji' do
+        page.within('.note-awards') do
+          expect(page).not_to have_css('.award-menu-holder')
+        end
+
+        page.within('.note-actions') do
+          expect(page).not_to have_css('a.js-add-award')
+        end
+      end
+
+      it 'does not allow toggling existing emoji' do
+        page.within('.note-awards') do
+          find('gl-emoji[data-name="100"]').click
+        end
+        wait_for_requests
+
+        expect(note.reload.award_emoji.size).to eq(1)
+      end
+    end
+  end
 end
diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb
index aeef5352333..95da9a6f3d4 100644
--- a/spec/helpers/issues_helper_spec.rb
+++ b/spec/helpers/issues_helper_spec.rb
@@ -96,13 +96,32 @@ describe IssuesHelper do
 
   describe '#award_state_class' do
     let!(:upvote) { create(:award_emoji) }
+    let(:awardable) { upvote.awardable }
+    let(:user) { upvote.user }
+
+    before do
+      allow(helper).to receive(:can?) do |*args|
+        Ability.allowed?(*args)
+      end
+    end
 
     it "returns disabled string for unauthenticated user" do
-      expect(award_state_class(AwardEmoji.all, nil)).to eq("disabled")
+      expect(helper.award_state_class(awardable, AwardEmoji.all, nil)).to eq("disabled")
+    end
+
+    it "returns disabled for a user that does not have access to the awardable" do
+      expect(helper.award_state_class(awardable, AwardEmoji.all, build(:user))).to eq("disabled")
     end
 
     it "returns active string for author" do
-      expect(award_state_class(AwardEmoji.all, upvote.user)).to eq("active")
+      expect(helper.award_state_class(awardable, AwardEmoji.all, upvote.user)).to eq("active")
+    end
+
+    it "is blank for a user that has access to the awardable" do
+      user = build(:user)
+      expect(helper).to receive(:can?).with(user, :award_emoji, awardable).and_return(true)
+
+      expect(helper.award_state_class(awardable, AwardEmoji.all, user)).to be_blank
     end
   end
 
diff --git a/spec/javascripts/notes/components/note_actions_spec.js b/spec/javascripts/notes/components/note_actions_spec.js
index ab81aabb992..1dfe890e05e 100644
--- a/spec/javascripts/notes/components/note_actions_spec.js
+++ b/spec/javascripts/notes/components/note_actions_spec.js
@@ -3,7 +3,7 @@ import store from '~/notes/stores';
 import noteActions from '~/notes/components/note_actions.vue';
 import { userDataMock } from '../mock_data';
 
-describe('issse_note_actions component', () => {
+describe('issue_note_actions component', () => {
   let vm;
   let Component;
 
@@ -24,6 +24,7 @@ describe('issse_note_actions component', () => {
         authorId: 26,
         canDelete: true,
         canEdit: true,
+        canAwardEmoji: true,
         canReportAsAbuse: true,
         noteId: 539,
         reportAbusePath: '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26',
@@ -70,6 +71,7 @@ describe('issse_note_actions component', () => {
         authorId: 26,
         canDelete: false,
         canEdit: false,
+        canAwardEmoji: false,
         canReportAsAbuse: false,
         noteId: 539,
         reportAbusePath: '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26',
diff --git a/spec/javascripts/notes/components/note_awards_list_spec.js b/spec/javascripts/notes/components/note_awards_list_spec.js
index 15995ec5a05..1c30d8691b1 100644
--- a/spec/javascripts/notes/components/note_awards_list_spec.js
+++ b/spec/javascripts/notes/components/note_awards_list_spec.js
@@ -29,6 +29,7 @@ describe('note_awards_list component', () => {
         awards: awardsMock,
         noteAuthorId: 2,
         noteId: 545,
+        canAwardEmoji: true,
         toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji',
       },
     }).$mount();
@@ -43,14 +44,45 @@ describe('note_awards_list component', () => {
     expect(vm.$el.querySelector('.js-awards-block button [data-name="cartwheel_tone3"]')).toBeDefined();
   });
 
-  it('should be possible to remove awareded emoji', () => {
+  it('should be possible to remove awarded emoji', () => {
     spyOn(vm, 'handleAward').and.callThrough();
+    spyOn(vm, 'toggleAwardRequest').and.callThrough();
     vm.$el.querySelector('.js-awards-block button').click();
 
     expect(vm.handleAward).toHaveBeenCalledWith('flag_tz');
+    expect(vm.toggleAwardRequest).toHaveBeenCalled();
   });
 
   it('should be possible to add new emoji', () => {
     expect(vm.$el.querySelector('.js-add-award')).toBeDefined();
   });
+
+  describe('when the user cannot award emoji', () => {
+    beforeEach(() => {
+      const Component = Vue.extend(awardsNote);
+
+      vm = new Component({
+        store,
+        propsData: {
+          awards: awardsMock,
+          noteAuthorId: 2,
+          noteId: 545,
+          canAwardEmoji: false,
+          toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji',
+        },
+      }).$mount();
+    });
+
+    it('should not be possible to remove awarded emoji', () => {
+      spyOn(vm, 'toggleAwardRequest').and.callThrough();
+
+      vm.$el.querySelector('.js-awards-block button').click();
+
+      expect(vm.toggleAwardRequest).not.toHaveBeenCalled();
+    });
+
+    it('should not be possible to add new emoji', () => {
+      expect(vm.$el.querySelector('.js-add-award')).toBeNull();
+    });
+  });
 });
diff --git a/spec/javascripts/notes/components/note_body_spec.js b/spec/javascripts/notes/components/note_body_spec.js
index 0ff804f0e55..4e551496ff0 100644
--- a/spec/javascripts/notes/components/note_body_spec.js
+++ b/spec/javascripts/notes/components/note_body_spec.js
@@ -18,6 +18,7 @@ describe('issue_note_body component', () => {
       propsData: {
         note,
         canEdit: true,
+        canAwardEmoji: true,
       },
     }).$mount();
   });
diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js
index 24388fba219..bfe3a65feee 100644
--- a/spec/javascripts/notes/mock_data.js
+++ b/spec/javascripts/notes/mock_data.js
@@ -9,6 +9,7 @@ export const notesDataMock = {
   totalNotes: 1,
   closePath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=close',
   reopenPath: '/twitter/flight/issues/9.json?issue%5Bstate_event%5D=reopen',
+  canAwardEmoji: true,
 };
 
 export const userDataMock = {
@@ -30,6 +31,7 @@ export const noteableDataMock = {
   current_user: {
     can_create_note: true,
     can_update: true,
+    can_award_emoji: true,
   },
   description: '',
   due_date: null,
@@ -86,7 +88,10 @@ export const individualNote = {
       human_access: 'Owner',
       note: 'sdfdsaf',
       note_html: "<p dir='auto'>sdfdsaf</p>",
-      current_user: { can_edit: true },
+      current_user: {
+        can_edit: true,
+        can_award_emoji: true,
+      },
       discussion_id: '0fb4e0e3f9276e55ff32eb4195add694aece4edd',
       emoji_awardable: true,
       award_emoji: [
@@ -129,6 +134,7 @@ export const note = {
   note_html: '<p dir="auto">Vel id placeat reprehenderit sit numquam.</p>',
   current_user: {
     can_edit: true,
+    can_award_emoji: true,
   },
   discussion_id: 'd3842a451b7f3d9a5dfce329515127b2d29a4cd0',
   emoji_awardable: true,
@@ -187,6 +193,7 @@ export const discussionMock = {
       note_html: "<p dir='auto'>THIS IS A DICUSSSION!</p>",
       current_user: {
         can_edit: true,
+        can_award_emoji: true,
       },
       discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
       emoji_awardable: true,
@@ -231,6 +238,7 @@ export const discussionMock = {
       },
       current_user: {
         can_edit: true,
+        can_award_emoji: true,
       },
       discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
       emoji_awardable: true,
@@ -275,6 +283,7 @@ export const discussionMock = {
       },
       current_user: {
         can_edit: true,
+        can_award_emoji: true,
       },
       discussion_id: '9e3bd2f71a01de45fd166e6719eb380ad9f270b1',
       emoji_awardable: true,
@@ -365,6 +374,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = {
             note_html: '\u003cp dir="auto"\u003esdfdsaf\u003c/p\u003e',
             current_user: {
               can_edit: true,
+              can_award_emoji: true,
             },
             discussion_id: '0fb4e0e3f9276e55ff32eb4195add694aece4edd',
             emoji_awardable: true,
@@ -425,6 +435,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = {
             note_html: '\u003cp dir="auto"\u003eNew note!\u003c/p\u003e',
             current_user: {
               can_edit: true,
+              can_award_emoji: true,
             },
             discussion_id: '70d5c92a4039a36c70100c6691c18c27e4b0a790',
             emoji_awardable: true,
@@ -478,6 +489,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = {
       },
       current_user: {
         can_edit: true,
+        can_award_emoji: true,
       },
       discussion_id: 'a3ed36e29b1957efb3b68c53e2d7a2b24b1df052',
       emoji_awardable: true,
@@ -527,6 +539,7 @@ export const DISCUSSION_NOTE_RESPONSE_MAP = {
             note_html: '\u003cp dir="auto"\u003eAdding a comment\u003c/p\u003e',
             current_user: {
               can_edit: true,
+              can_award_emoji: true,
             },
             discussion_id: 'a3ed36e29b1957efb3b68c53e2d7a2b24b1df052',
             emoji_awardable: true,
diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb
index 34f923d3f0c..a980cff28fb 100644
--- a/spec/models/concerns/awardable_spec.rb
+++ b/spec/models/concerns/awardable_spec.rb
@@ -46,6 +46,31 @@ describe Awardable do
     end
   end
 
+  describe '#user_can_award?' do
+    let(:user) { create(:user) }
+
+    before do
+      issue.project.add_guest(user)
+    end
+
+    it 'does not allow upvoting or downvoting your own issue' do
+      issue.update!(author: user)
+
+      expect(issue.user_can_award?(user, AwardEmoji::DOWNVOTE_NAME)).to be_falsy
+      expect(issue.user_can_award?(user, AwardEmoji::UPVOTE_NAME)).to be_falsy
+    end
+
+    it 'is truthy when the user is allowed to award emoji' do
+      expect(issue.user_can_award?(user, AwardEmoji::UPVOTE_NAME)).to be_truthy
+    end
+
+    it 'is falsy when the project is archived' do
+      issue.project.update!(archived: true)
+
+      expect(issue.user_can_award?(user, AwardEmoji::UPVOTE_NAME)).to be_falsy
+    end
+  end
+
   describe "#toggle_award_emoji" do
     it "adds an emoji if it isn't awarded yet" do
       expect { issue.toggle_award_emoji("thumbsup", award_emoji.user) }.to change { AwardEmoji.count }.by(1)
diff --git a/spec/policies/personal_snippet_policy_spec.rb b/spec/policies/personal_snippet_policy_spec.rb
index 50bb0899eba..3809692b373 100644
--- a/spec/policies/personal_snippet_policy_spec.rb
+++ b/spec/policies/personal_snippet_policy_spec.rb
@@ -27,6 +27,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_allowed(:read_personal_snippet)
         is_expected.to be_disallowed(:comment_personal_snippet)
+        is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
     end
@@ -37,6 +38,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_allowed(:read_personal_snippet)
         is_expected.to be_allowed(:comment_personal_snippet)
+        is_expected.to be_allowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
     end
@@ -47,6 +49,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_allowed(:read_personal_snippet)
         is_expected.to be_allowed(:comment_personal_snippet)
+        is_expected.to be_allowed(:award_emoji)
         is_expected.to be_allowed(*author_permissions)
       end
     end
@@ -61,6 +64,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_disallowed(:read_personal_snippet)
         is_expected.to be_disallowed(:comment_personal_snippet)
+        is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
     end
@@ -71,6 +75,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_allowed(:read_personal_snippet)
         is_expected.to be_allowed(:comment_personal_snippet)
+        is_expected.to be_allowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
     end
@@ -81,6 +86,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_disallowed(:read_personal_snippet)
         is_expected.to be_disallowed(:comment_personal_snippet)
+        is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
     end
@@ -91,6 +97,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_allowed(:read_personal_snippet)
         is_expected.to be_allowed(:comment_personal_snippet)
+        is_expected.to be_allowed(:award_emoji)
         is_expected.to be_allowed(*author_permissions)
       end
     end
@@ -105,6 +112,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_disallowed(:read_personal_snippet)
         is_expected.to be_disallowed(:comment_personal_snippet)
+        is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
     end
@@ -115,6 +123,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_disallowed(:read_personal_snippet)
         is_expected.to be_disallowed(:comment_personal_snippet)
+        is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
     end
@@ -125,6 +134,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_disallowed(:read_personal_snippet)
         is_expected.to be_disallowed(:comment_personal_snippet)
+        is_expected.to be_disallowed(:award_emoji)
         is_expected.to be_disallowed(*author_permissions)
       end
     end
@@ -135,6 +145,7 @@ describe PersonalSnippetPolicy do
       it do
         is_expected.to be_allowed(:read_personal_snippet)
         is_expected.to be_allowed(:comment_personal_snippet)
+        is_expected.to be_allowed(:award_emoji)
         is_expected.to be_allowed(*author_permissions)
       end
     end
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 33d5a61c563..8b9c4ac0b4b 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -15,6 +15,7 @@ describe ProjectPolicy do
       read_project_for_iids read_issue_iid read_merge_request_iid read_label
       read_milestone read_project_snippet read_project_member read_note
       create_project create_issue create_note upload_file create_merge_request_in
+      award_emoji
     ]
   end
 
@@ -166,6 +167,7 @@ describe ProjectPolicy do
         request_access
         upload_file
         resolve_note
+        award_emoji
       ]
     end
 
@@ -193,7 +195,7 @@ describe ProjectPolicy do
       context 'when a project has pending invites' do
         let(:group) { create(:group, :public) }
         let(:project) { create(:project, :public, namespace: group) }
-        let(:user_permissions) { [:create_merge_request_in, :create_project, :create_issue, :create_note, :upload_file] }
+        let(:user_permissions) { [:create_merge_request_in, :create_project, :create_issue, :create_note, :upload_file, :award_emoji] }
         let(:anonymous_permissions) { guest_permissions - user_permissions  }
 
         subject { described_class.new(nil, project) }

From c39e3f22ce12a302e294deb3523414e4a684b6fb Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Tue, 10 Apr 2018 19:49:26 +0200
Subject: [PATCH 16/18] Show `New Issue` link for projects

When a user is not logged in, we want to show the `New Issue` link so
he gets directed to the login flow first.

When a project is archived, we never want to show the link.
---
 app/helpers/issues_helper.rb                  | 11 ++++++++++
 app/views/layouts/header/_new_dropdown.haml   |  2 +-
 .../projects/buttons/_dropdown.html.haml      |  2 +-
 app/views/projects/issues/_nav_btns.html.haml |  2 +-
 app/views/projects/issues/show.html.haml      | 13 ++++++-----
 .../shared/milestones/_sidebar.html.haml      |  2 +-
 .../projects/commit/cherry_pick_spec.rb       | 10 +++++++++
 .../commit/user_reverts_commit_spec.rb        | 17 ++++++++++++++
 .../projects/issues/user_views_issue_spec.rb  | 18 ++++++++++++++-
 spec/helpers/issues_helper_spec.rb            | 22 +++++++++++++++++++
 10 files changed, 89 insertions(+), 10 deletions(-)

diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 5a2f7bb01c7..96dc7ae1185 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -126,6 +126,17 @@ module IssuesHelper
     link_to link_text, path
   end
 
+  def show_new_issue_link?(project)
+    return false unless project
+    return false if project.archived?
+
+    # We want to show the link to users that are not signed in, that way they
+    # get directed to the sign-in/sign-up flow and afterwards to the new issue page.
+    return true unless current_user
+
+    can?(current_user, :create_issue, project)
+  end
+
   # Required for Banzai::Filter::IssueReferenceFilter
   module_function :url_for_issue
   module_function :url_for_internal_issue
diff --git a/app/views/layouts/header/_new_dropdown.haml b/app/views/layouts/header/_new_dropdown.haml
index 7373df25e78..6f53f5ac1ae 100644
--- a/app/views/layouts/header/_new_dropdown.haml
+++ b/app/views/layouts/header/_new_dropdown.haml
@@ -19,7 +19,7 @@
           %li.dropdown-bold-header GitLab
 
       - if @project&.persisted?
-        - create_project_issue = can?(current_user, :create_issue, @project)
+        - create_project_issue = show_new_issue_link?(@project)
         - merge_project = merge_request_source_project_for_project(@project)
         - create_project_snippet = can?(current_user, :create_project_snippet, @project)
         - if create_project_issue || merge_project || create_project_snippet
diff --git a/app/views/projects/buttons/_dropdown.html.haml b/app/views/projects/buttons/_dropdown.html.haml
index 268c8eba9c5..2e86a7d36d7 100644
--- a/app/views/projects/buttons/_dropdown.html.haml
+++ b/app/views/projects/buttons/_dropdown.html.haml
@@ -1,4 +1,4 @@
-- can_create_issue = show_new_issue_link?
+- can_create_issue = show_new_issue_link?(@project)
 - can_create_project_snippet = can?(current_user, :create_project_snippet, @project)
 - can_push_code = can?(current_user, :push_code, @project)
 - create_mr_from_new_fork = can?(current_user, :fork_project, @project) && can?(current_user, :create_merge_request_in, @project)
diff --git a/app/views/projects/issues/_nav_btns.html.haml b/app/views/projects/issues/_nav_btns.html.haml
index bc672286d90..dd1a836fa20 100644
--- a/app/views/projects/issues/_nav_btns.html.haml
+++ b/app/views/projects/issues/_nav_btns.html.haml
@@ -2,7 +2,7 @@
   = icon('rss')
 - if @can_bulk_update
   = button_tag "Edit issues", class: "btn btn-default append-right-10 js-bulk-update-toggle"
-- if !current_user || can?(current_user, :create_issue, @project)
+- if show_new_issue_link?(@project)
   = link_to "New issue", new_project_issue_path(@project,
                                                 issue: { assignee_id: finder.assignee.try(:id),
                                                          milestone_id: finder.milestones.first.try(:id) }),
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index ec7e87219f5..f1fc1c2316d 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -7,6 +7,7 @@
 
 - can_update_issue = can?(current_user, :update_issue, @issue)
 - can_report_spam = @issue.submittable_as_spam_by?(current_user)
+- can_create_issue = show_new_issue_link?(@project)
 
 .detail-page-header
   .detail-page-header-body
@@ -42,16 +43,18 @@
             %li= link_to 'Reopen issue', issue_path(@issue, issue: { state_event: :reopen }, format: 'json'), class: "btn-reopen js-btn-issue-action #{issue_button_visibility(@issue, false)}", title: 'Reopen issue'
           - if can_report_spam
             %li= link_to 'Submit as spam', mark_as_spam_project_issue_path(@project, @issue), method: :post, class: 'btn-spam', title: 'Submit as spam'
-          - if can_update_issue || can_report_spam
-            %li.divider
-          %li= link_to 'New issue', new_project_issue_path(@project), title: 'New issue', id: 'new_issue_link'
+          - if can_create_issue
+            - if can_update_issue || can_report_spam
+              %li.divider
+            %li= link_to 'New issue', new_project_issue_path(@project), title: 'New issue', id: 'new_issue_link'
 
     = render 'shared/issuable/close_reopen_button', issuable: @issue, can_update: can_update_issue
 
     - if can_report_spam
       = link_to 'Submit as spam', mark_as_spam_project_issue_path(@project, @issue), method: :post, class: 'hidden-xs hidden-sm btn btn-grouped btn-spam', title: 'Submit as spam'
-    = link_to new_project_issue_path(@project), class: 'hidden-xs hidden-sm btn btn-grouped new-issue-link btn-new btn-inverted', title: 'New issue', id: 'new_issue_link' do
-      New issue
+    - if can_create_issue
+      = link_to new_project_issue_path(@project), class: 'hidden-xs hidden-sm btn btn-grouped new-issue-link btn-new btn-inverted', title: 'New issue', id: 'new_issue_link' do
+        New issue
 
 .issue-details.issuable-details
   .detail-page-description.content-block
diff --git a/app/views/shared/milestones/_sidebar.html.haml b/app/views/shared/milestones/_sidebar.html.haml
index a942ebc328b..ee134480705 100644
--- a/app/views/shared/milestones/_sidebar.html.haml
+++ b/app/views/shared/milestones/_sidebar.html.haml
@@ -72,7 +72,7 @@
         .title.hide-collapsed
           Issues
           %span.badge= milestone.issues_visible_to_user(current_user).count
-          - if project && can?(current_user, :create_issue, project)
+          - if show_new_issue_link?(project)
             = link_to new_project_issue_path(project, issue: { milestone_id: milestone.id }), class: "pull-right", title: "New Issue" do
               New issue
         .value.hide-collapsed.bold
diff --git a/spec/features/projects/commit/cherry_pick_spec.rb b/spec/features/projects/commit/cherry_pick_spec.rb
index c4c399e3058..2f9d4f67488 100644
--- a/spec/features/projects/commit/cherry_pick_spec.rb
+++ b/spec/features/projects/commit/cherry_pick_spec.rb
@@ -89,4 +89,14 @@ describe 'Cherry-pick Commits' do
       expect(page).to have_content('The commit has been successfully cherry-picked.')
     end
   end
+
+  context 'when the project is archived' do
+    let(:project) { create(:project, :repository, namespace: group, archived: true) }
+
+    it 'does not show the cherry-pick link' do
+      find('.header-action-buttons a.dropdown-toggle').click
+
+      expect(page).not_to have_css("a[href='#modal-cherry-pick-commit']")
+    end
+  end
 end
diff --git a/spec/features/projects/commit/user_reverts_commit_spec.rb b/spec/features/projects/commit/user_reverts_commit_spec.rb
index 221f1d7757e..5a09e2380b6 100644
--- a/spec/features/projects/commit/user_reverts_commit_spec.rb
+++ b/spec/features/projects/commit/user_reverts_commit_spec.rb
@@ -10,13 +10,16 @@ describe 'User reverts a commit', :js do
     sign_in(user)
 
     visit(project_commit_path(project, sample_commit.id))
+  end
 
+  def click_revert
     find('.header-action-buttons .dropdown').click
     find('a[href="#modal-revert-commit"]').click
   end
 
   context 'without creating a new merge request' do
     before do
+      click_revert
       page.within('#modal-revert-commit') do
         uncheck('create_merge_request')
         click_button('Revert')
@@ -44,6 +47,10 @@ describe 'User reverts a commit', :js do
   end
 
   context 'with creating a new merge request' do
+    before do
+      click_revert
+    end
+
     it 'reverts a commit' do
       page.within('#modal-revert-commit') do
         click_button('Revert')
@@ -53,4 +60,14 @@ describe 'User reverts a commit', :js do
       expect(page).to have_content("From revert-#{Commit.truncate_sha(sample_commit.id)} into master")
     end
   end
+
+  context 'when the project is archived' do
+    let(:project) { create(:project, :repository, namespace: user.namespace, archived: true) }
+
+    it 'does not show the revert link' do
+      find('.header-action-buttons .dropdown').click
+
+      expect(page).not_to have_link('Revert')
+    end
+  end
 end
diff --git a/spec/features/projects/issues/user_views_issue_spec.rb b/spec/features/projects/issues/user_views_issue_spec.rb
index f7f2cde3d64..d17ce636f3b 100644
--- a/spec/features/projects/issues/user_views_issue_spec.rb
+++ b/spec/features/projects/issues/user_views_issue_spec.rb
@@ -6,11 +6,27 @@ describe "User views issue" do
   set(:issue) { create(:issue, project: project, description: "# Description header", author: user) }
 
   before do
-    project.add_guest(user)
+    project.add_developer(user)
     sign_in(user)
 
     visit(project_issue_path(project, issue))
   end
 
   it { expect(page).to have_header_with_correct_id_and_link(1, "Description header", "description-header") }
+
+  it { expect(page).to have_link('New issue') }
+
+  it { expect(page).to have_button('Create merge request') }
+
+  it { expect(page).to have_link('Close issue') }
+
+  context 'when the project is archived' do
+    let(:project) { create(:project, :public, archived: true) }
+
+    it { expect(page).not_to have_link('New issue') }
+
+    it { expect(page).not_to have_button('Create merge request') }
+
+    it { expect(page).not_to have_link('Close issue') }
+  end
 end
diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb
index 95da9a6f3d4..8bb2e234e9a 100644
--- a/spec/helpers/issues_helper_spec.rb
+++ b/spec/helpers/issues_helper_spec.rb
@@ -163,4 +163,26 @@ describe IssuesHelper do
       end
     end
   end
+
+  describe '#show_new_issue_link?' do
+    before do
+      allow(helper).to receive(:current_user)
+    end
+
+    it 'is false when no project there is no project' do
+      expect(helper.show_new_issue_link?(nil)).to be_falsey
+    end
+
+    it 'is true when there is a project and no logged in user' do
+      expect(helper.show_new_issue_link?(build(:project))).to be_truthy
+    end
+
+    it 'is true when the current user does not have access to the project' do
+      project = build(:project)
+      allow(helper).to receive(:current_user).and_return(project.owner)
+
+      expect(helper).to receive(:can?).with(project.owner, :create_issue, project).and_return(true)
+      expect(helper.show_new_issue_link?(project)).to be_truthy
+    end
+  end
 end

From 2bf3846adcac843958837a32fafdeb63d067a532 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Wed, 11 Apr 2018 10:34:42 +0200
Subject: [PATCH 17/18] Add matcher for emoji

---
 spec/features/admin/admin_broadcast_messages_spec.rb        | 2 +-
 spec/features/groups/show_spec.rb                           | 2 +-
 .../awards/user_interacts_with_awards_in_issue_spec.rb      | 6 +++---
 spec/support/matchers/have_emoji.rb                         | 5 +++++
 4 files changed, 10 insertions(+), 5 deletions(-)
 create mode 100644 spec/support/matchers/have_emoji.rb

diff --git a/spec/features/admin/admin_broadcast_messages_spec.rb b/spec/features/admin/admin_broadcast_messages_spec.rb
index 9cb351282a0..430a8d22b0f 100644
--- a/spec/features/admin/admin_broadcast_messages_spec.rb
+++ b/spec/features/admin/admin_broadcast_messages_spec.rb
@@ -45,7 +45,7 @@ feature 'Admin Broadcast Messages' do
 
     page.within('.broadcast-message-preview') do
       expect(page).to have_selector('strong', text: 'Markdown')
-      expect(page).to have_selector('gl-emoji[data-name="tada"]')
+      expect(page).to have_emoji('tada')
     end
   end
 end
diff --git a/spec/features/groups/show_spec.rb b/spec/features/groups/show_spec.rb
index 4ffadbbcd35..3a0424d60f8 100644
--- a/spec/features/groups/show_spec.rb
+++ b/spec/features/groups/show_spec.rb
@@ -98,7 +98,7 @@ feature 'Group show page' do
 
     it 'shows the project info' do
       expect(page).to have_content(project.title)
-      expect(page).to have_selector('gl-emoji[data-name="smile"]')
+      expect(page).to have_emoji('smile')
     end
   end
 end
diff --git a/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb b/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb
index 8a3d83da93f..a4890b34208 100644
--- a/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb
+++ b/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb
@@ -99,7 +99,7 @@ describe 'User interacts with awards in an issue', :js do
       click_button('Comment')
     end
 
-    expect(page).to have_selector('gl-emoji[data-name="smile"]')
+    expect(page).to have_emoji('smile')
   end
 
   context 'when a project is archived' do
@@ -118,7 +118,7 @@ describe 'User interacts with awards in an issue', :js do
 
     it 'shows the award on the note' do
       page.within('.note-awards') do
-        expect(page).to have_selector('gl-emoji[data-name="100"]')
+        expect(page).to have_emoji('100')
       end
     end
 
@@ -141,7 +141,7 @@ describe 'User interacts with awards in an issue', :js do
       wait_for_requests
 
       page.within('.note-awards') do
-        expect(page).to have_selector('gl-emoji[data-name="8ball"]')
+        expect(page).to have_emoji('8ball')
       end
       expect(note.reload.award_emoji.size).to eq(2)
     end
diff --git a/spec/support/matchers/have_emoji.rb b/spec/support/matchers/have_emoji.rb
new file mode 100644
index 00000000000..23fb8e9c1c4
--- /dev/null
+++ b/spec/support/matchers/have_emoji.rb
@@ -0,0 +1,5 @@
+RSpec::Matchers.define :have_emoji do |emoji_name|
+  match do |actual|
+    expect(actual).to have_selector("gl-emoji[data-name='#{emoji_name}']")
+  end
+end

From eae2ed3366ac13832c56aefe965513b986d7e8b1 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Wed, 11 Apr 2018 23:14:02 +0200
Subject: [PATCH 18/18] Spec improvements for speed, reliability and
 readability

---
 .../projects/issues_controller_spec.rb        |  2 +-
 .../merge_request/user_awards_emoji_spec.rb   |  2 +-
 ...ser_interacts_with_awards_in_issue_spec.rb |  4 ++--
 spec/features/projects/branches_spec.rb       |  2 +-
 .../projects/commit/cherry_pick_spec.rb       |  3 ++-
 .../commit/user_reverts_commit_spec.rb        |  2 +-
 .../projects/files/user_edits_files_spec.rb   | 12 ++++------
 .../projects/issues/user_views_issue_spec.rb  | 22 +++++++++----------
 .../user_views_open_merge_requests_spec.rb    |  2 +-
 .../commit/_commit_box.html.haml_spec.rb      |  1 -
 10 files changed, 24 insertions(+), 28 deletions(-)

diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 6c6b1e4a500..ca86b0bc737 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -956,7 +956,7 @@ describe Projects::IssuesController do
     end
 
     it 'is not available when the project is archived' do
-      project.update(archived: true)
+      project.update!(archived: true)
 
       create_merge_request
 
diff --git a/spec/features/merge_request/user_awards_emoji_spec.rb b/spec/features/merge_request/user_awards_emoji_spec.rb
index fa254e8c6c5..859a4c65562 100644
--- a/spec/features/merge_request/user_awards_emoji_spec.rb
+++ b/spec/features/merge_request/user_awards_emoji_spec.rb
@@ -37,7 +37,7 @@ describe 'Merge request > User awards emoji', :js do
     end
 
     describe 'the project is archived' do
-      let(:project) { create(:project, :public, :repository, archived: true) }
+      let(:project) { create(:project, :public, :repository, :archived) }
 
       it 'does not see award menu button' do
         expect(page).not_to have_selector('.js-award-holder')
diff --git a/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb b/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb
index a4890b34208..12e07647ecd 100644
--- a/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb
+++ b/spec/features/projects/awards/user_interacts_with_awards_in_issue_spec.rb
@@ -103,7 +103,7 @@ describe 'User interacts with awards in an issue', :js do
   end
 
   context 'when a project is archived' do
-    let(:project) { create(:project, archived: true) }
+    let(:project) { create(:project, :archived) }
 
     it 'hides the add award button' do
       page.within('.awards') do
@@ -147,7 +147,7 @@ describe 'User interacts with awards in an issue', :js do
     end
 
     context 'when the project is archived' do
-      let(:project) { create(:project, archived: true) }
+      let(:project) { create(:project, :archived) }
 
       it 'hides the buttons for adding new emoji' do
         page.within('.note-awards') do
diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb
index acf478cd117..b7ce1b9993a 100644
--- a/spec/features/projects/branches_spec.rb
+++ b/spec/features/projects/branches_spec.rb
@@ -205,7 +205,7 @@ describe 'Branches' do
     end
 
     context 'when the project is archived' do
-      let(:project) { create(:project, :public, :repository, archived: true) }
+      let(:project) { create(:project, :public, :repository, :archived) }
 
       it 'does not show the merge request button when the project is archived' do
         visit project_branches_path(project)
diff --git a/spec/features/projects/commit/cherry_pick_spec.rb b/spec/features/projects/commit/cherry_pick_spec.rb
index 2f9d4f67488..1df45865d6f 100644
--- a/spec/features/projects/commit/cherry_pick_spec.rb
+++ b/spec/features/projects/commit/cherry_pick_spec.rb
@@ -91,11 +91,12 @@ describe 'Cherry-pick Commits' do
   end
 
   context 'when the project is archived' do
-    let(:project) { create(:project, :repository, namespace: group, archived: true) }
+    let(:project) { create(:project, :repository, :archived, namespace: group) }
 
     it 'does not show the cherry-pick link' do
       find('.header-action-buttons a.dropdown-toggle').click
 
+      expect(page).not_to have_text("Cherry-pick")
       expect(page).not_to have_css("a[href='#modal-cherry-pick-commit']")
     end
   end
diff --git a/spec/features/projects/commit/user_reverts_commit_spec.rb b/spec/features/projects/commit/user_reverts_commit_spec.rb
index 5a09e2380b6..42844a03ea6 100644
--- a/spec/features/projects/commit/user_reverts_commit_spec.rb
+++ b/spec/features/projects/commit/user_reverts_commit_spec.rb
@@ -62,7 +62,7 @@ describe 'User reverts a commit', :js do
   end
 
   context 'when the project is archived' do
-    let(:project) { create(:project, :repository, namespace: user.namespace, archived: true) }
+    let(:project) { create(:project, :repository, :archived, namespace: user.namespace) }
 
     it 'does not show the revert link' do
       find('.header-action-buttons .dropdown').click
diff --git a/spec/features/projects/files/user_edits_files_spec.rb b/spec/features/projects/files/user_edits_files_spec.rb
index ee9deb30890..dc6e4fd27cb 100644
--- a/spec/features/projects/files/user_edits_files_spec.rb
+++ b/spec/features/projects/files/user_edits_files_spec.rb
@@ -20,15 +20,11 @@ describe 'Projects > Files > User edits files' do
       click_link('.gitignore')
 
       aggregate_failures 'available edit buttons' do
-        # We're showing a link, if the user can edit directly, this is becomes a
-        # button when the user can fork the project.
-        expect(page).not_to have_link('Edit')
-        expect(page).not_to have_button('Edit')
-        expect(page).not_to have_link('Web IDE')
-        expect(page).not_to have_button('Web IDE')
+        expect(page).not_to have_text('Edit')
+        expect(page).not_to have_text('Web IDE')
 
-        expect(page).not_to have_button('Replace')
-        expect(page).not_to have_button('Delete')
+        expect(page).not_to have_text('Replace')
+        expect(page).not_to have_text('Delete')
       end
     end
   end
diff --git a/spec/features/projects/issues/user_views_issue_spec.rb b/spec/features/projects/issues/user_views_issue_spec.rb
index d17ce636f3b..4093876c289 100644
--- a/spec/features/projects/issues/user_views_issue_spec.rb
+++ b/spec/features/projects/issues/user_views_issue_spec.rb
@@ -14,19 +14,19 @@ describe "User views issue" do
 
   it { expect(page).to have_header_with_correct_id_and_link(1, "Description header", "description-header") }
 
-  it { expect(page).to have_link('New issue') }
-
-  it { expect(page).to have_button('Create merge request') }
-
-  it { expect(page).to have_link('Close issue') }
+  it 'shows the merge request and issue actions', :aggregate_failures do
+    expect(page).to have_link('New issue')
+    expect(page).to have_button('Create merge request')
+    expect(page).to have_link('Close issue')
+  end
 
   context 'when the project is archived' do
-    let(:project) { create(:project, :public, archived: true) }
+    let(:project) { create(:project, :public, :archived) }
 
-    it { expect(page).not_to have_link('New issue') }
-
-    it { expect(page).not_to have_button('Create merge request') }
-
-    it { expect(page).not_to have_link('Close issue') }
+    it 'hides the merge request and issue actions', :aggregate_failures do
+      expect(page).not_to have_link('New issue')
+      expect(page).not_to have_button('Create merge request')
+      expect(page).not_to have_link('Close issue')
+    end
   end
 end
diff --git a/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb b/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb
index 08826625842..115e548b691 100644
--- a/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb
+++ b/spec/features/projects/merge_requests/user_views_open_merge_requests_spec.rb
@@ -100,7 +100,7 @@ describe 'User views open merge requests' do
       end
 
       context 'when the project is archived' do
-        let(:project) { create(:project, :public, :repository, archived: true) }
+        let(:project) { create(:project, :public, :repository, :archived) }
 
         it 'hides the new merge request button' do
           expect(page).not_to have_link('New merge request')
diff --git a/spec/views/projects/commit/_commit_box.html.haml_spec.rb b/spec/views/projects/commit/_commit_box.html.haml_spec.rb
index fb5087f5090..2fdd28a3be4 100644
--- a/spec/views/projects/commit/_commit_box.html.haml_spec.rb
+++ b/spec/views/projects/commit/_commit_box.html.haml_spec.rb
@@ -7,7 +7,6 @@ describe 'projects/commit/_commit_box.html.haml' do
   before do
     assign(:project, project)
     assign(:commit, project.commit)
-    assign(:current_user, user)
     allow(view).to receive(:current_user).and_return(user)
     allow(view).to receive(:can_collaborate_with_project?).and_return(false)
   end