Merge branch '41766-create-releases-page' into 'master'

Creates index page for releases behind a feature flag

See merge request gitlab-org/gitlab-ce!23687

14 files changed, 195 insertions, 68 deletions

diff --git a/app/assets/javascripts/behaviors/shortcuts/shortcuts_navigation.js b/app/assets/javascripts/behaviors/shortcuts/shortcuts_navigation.js
index fa9b2c9f755..bef1553703b 100644
--- a/app/assets/javascripts/behaviors/shortcuts/shortcuts_navigation.js
+++ b/app/assets/javascripts/behaviors/shortcuts/shortcuts_navigation.js
@@ -8,6 +8,7 @@ export default class ShortcutsNavigation extends Shortcuts {
 
     Mousetrap.bind('g p', () => findAndFollowLink('.shortcuts-project'));
     Mousetrap.bind('g v', () => findAndFollowLink('.shortcuts-project-activity'));
+    Mousetrap.bind('g r', () => findAndFollowLink('.shortcuts-project-releases'));
     Mousetrap.bind('g f', () => findAndFollowLink('.shortcuts-tree'));
     Mousetrap.bind('g c', () => findAndFollowLink('.shortcuts-commits'));
     Mousetrap.bind('g j', () => findAndFollowLink('.shortcuts-builds'));
diff --git a/app/controllers/projects/releases_controller.rb b/app/controllers/projects/releases_controller.rb
index 55827075896..58d5ea4762f 100644
--- a/app/controllers/projects/releases_controller.rb
+++ b/app/controllers/projects/releases_controller.rb
@@ -4,39 +4,16 @@ class Projects::ReleasesController < Projects::ApplicationController
   # Authorize
   before_action :require_non_empty_project
   before_action :authorize_download_code!
-  before_action :authorize_push_code!
-  before_action :tag
-  before_action :release
+  before_action :check_releases_page_feature_flag
 
-  def edit
-  end
-
-  def update
-    # Release belongs to Tag which is not active record object,
-    # it exists only to save a description to each Tag.
-    # If description is empty we should destroy the existing record.
-    if release_params[:description].present?
-      release.update(release_params)
-    else
-      release.destroy
-    end
-
-    redirect_to project_tag_path(@project, @tag.name)
+  def index
   end
 
   private
 
-  def tag
-    @tag ||= @repository.find_tag(params[:tag_id])
-  end
-
-  # rubocop: disable CodeReuse/ActiveRecord
-  def release
-    @release ||= @project.releases.find_or_initialize_by(tag: @tag.name)
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
+  def check_releases_page_feature_flag
+    return render_404 unless Feature.enabled?(:releases_page)
 
-  def release_params
-    params.require(:release).permit(:description)
+    push_frontend_feature_flag(:releases_page)
   end
 end
diff --git a/app/controllers/projects/tags/releases_controller.rb b/app/controllers/projects/tags/releases_controller.rb
new file mode 100644
index 00000000000..334e1847cc8
--- /dev/null
+++ b/app/controllers/projects/tags/releases_controller.rb
@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+class Projects::Tags::ReleasesController < Projects::ApplicationController
+  # Authorize
+  before_action :require_non_empty_project
+  before_action :authorize_download_code!
+  before_action :authorize_push_code!
+  before_action :tag
+  before_action :release
+
+  def edit
+  end
+
+  def update
+    # Release belongs to Tag which is not active record object,
+    # it exists only to save a description to each Tag.
+    # If description is empty we should destroy the existing record.
+    if release_params[:description].present?
+      release.update(release_params)
+    else
+      release.destroy
+    end
+
+    redirect_to project_tag_path(@project, @tag.name)
+  end
+
+  private
+
+  def tag
+    @tag ||= @repository.find_tag(params[:tag_id])
+  end
+
+  # rubocop: disable CodeReuse/ActiveRecord
+  def release
+    @release ||= @project.releases.find_or_initialize_by(tag: @tag.name)
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
+
+  def release_params
+    params.require(:release).permit(:description)
+  end
+end
diff --git a/app/controllers/projects/tags_controller.rb b/app/controllers/projects/tags_controller.rb
index 686d66b10a3..a50a1475eb2 100644
--- a/app/controllers/projects/tags_controller.rb
+++ b/app/controllers/projects/tags_controller.rb
@@ -42,7 +42,7 @@ class Projects::TagsController < Projects::ApplicationController
   # rubocop: enable CodeReuse/ActiveRecord
 
   def create
-    result = Tags::CreateService.new(@project, current_user)
+    result = ::Tags::CreateService.new(@project, current_user)
       .execute(params[:tag_name], params[:ref], params[:message], params[:release_description])
 
     if result[:status] == :success
@@ -58,7 +58,7 @@ class Projects::TagsController < Projects::ApplicationController
   end
 
   def destroy
-    result = Tags::DestroyService.new(project, current_user).execute(params[:id])
+    result = ::Tags::DestroyService.new(project, current_user).execute(params[:id])
 
     respond_to do |format|
       if result[:status] == :success
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 1186eb3ddcc..aa54172e108 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -277,7 +277,7 @@ module ProjectsHelper
     nav_tabs = [:home]
 
     if !project.empty_repo? && can?(current_user, :download_code, project)
-      nav_tabs << [:files, :commits, :network, :graphs, :forks]
+      nav_tabs << [:files, :commits, :network, :graphs, :forks, :releases]
     end
 
     if project.repo_exists? && can?(current_user, :read_merge_request, project)
@@ -533,6 +533,7 @@ module ProjectsHelper
     %w[
       projects#show
       projects#activity
+      releases#index
       cycle_analytics#show
     ]
   end
@@ -564,7 +565,6 @@ module ProjectsHelper
       projects/repositories
       tags
       branches
-      releases
       graphs
       network
     ]
diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml
index bdd0108db0d..59557c70904 100644
--- a/app/views/layouts/nav/sidebar/_project.html.haml
+++ b/app/views/layouts/nav/sidebar/_project.html.haml
@@ -29,6 +29,11 @@
             = link_to activity_project_path(@project), title: _('Activity'), class: 'shortcuts-project-activity' do
               %span= _('Activity')
 
+          - if project_nav_tab?(:releases) && Feature.enabled?(:releases_page)
+            = nav_link(controller: :releases) do
+              = link_to project_releases_path(@project), title: _('Releases'), class: 'shortcuts-project-releases' do
+                %span= _('Releases')
+
           = render_if_exists 'projects/sidebar/security_dashboard'
 
           - if can?(current_user, :read_cycle_analytics, @project)
@@ -62,7 +67,7 @@
               = link_to project_branches_path(@project) do
                 = _('Branches')
 
-            = nav_link(controller: [:tags, :releases]) do
+            = nav_link(controller: [:tags]) do
               = link_to project_tags_path(@project) do
                 = _('Tags')
 
diff --git a/app/views/projects/releases/index.html.haml b/app/views/projects/releases/index.html.haml
new file mode 100644
index 00000000000..7bc942a3c3c
--- /dev/null
+++ b/app/views/projects/releases/index.html.haml
@@ -0,0 +1,5 @@
+- @no_container = true
+- page_title _('Releases')
+
+%div{ 'class' => container_class }
+  #js-releases-page
diff --git a/app/views/projects/releases/edit.html.haml b/app/views/projects/tags/releases/edit.html.haml
index 52c6c7ec424..52c6c7ec424 100644
--- a/app/views/projects/releases/edit.html.haml
+++ b/app/views/projects/tags/releases/edit.html.haml
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 7d0623cb904..03c95b61e51 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -95,6 +95,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
         end
       end
 
+      resources :releases, only: [:index]
       resources :forks, only: [:index, :new, :create]
       resource :import, only: [:new, :create, :show]
 
diff --git a/config/routes/repository.rb b/config/routes/repository.rb
index d439cb9acbd..96975759709 100644
--- a/config/routes/repository.rb
+++ b/config/routes/repository.rb
@@ -55,7 +55,7 @@ scope format: false do
     resources :branches, only: [:index, :new, :create, :destroy]
     delete :merged_branches, controller: 'branches', action: :destroy_all_merged
     resources :tags, only: [:index, :show, :new, :create, :destroy] do
-      resource :release, only: [:edit, :update]
+      resource :release, controller: 'tags/releases', only: [:edit, :update]
     end
 
     resources :protected_branches, only: [:index, :show, :create, :update, :destroy]
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 8a7db438add..9f9844e2140 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -5506,6 +5506,9 @@ msgstr ""
 msgid "Related merge requests"
 msgstr ""
 
+msgid "Releases"
+msgstr ""
+
 msgid "Remind later"
 msgstr ""
 
diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb
index 20a6beb3df8..d9fb6e0d838 100644
--- a/spec/controllers/projects/releases_controller_spec.rb
+++ b/spec/controllers/projects/releases_controller_spec.rb
@@ -1,55 +1,65 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe Projects::ReleasesController do
-  let!(:project) { create(:project, :repository) }
+  let!(:project) { create(:project, :repository, :public) }
   let!(:user)    { create(:user) }
-  let!(:release) { create(:release, project: project) }
-  let!(:tag)     { release.tag }
 
   before do
-    project.add_developer(user)
-    sign_in(user)
+    stub_feature_flags(releases_page: true)
   end
 
-  describe 'GET #edit' do
-    it 'initializes a new release' do
-      tag_id = release.tag
-      project.releases.destroy_all # rubocop: disable DestroyAll
-
-      get :edit, namespace_id: project.namespace, project_id: project, tag_id: tag_id
+  describe 'GET #index' do
+    it 'renders a 200' do
+      get_index
 
-      release = assigns(:release)
-      expect(release).not_to be_nil
-      expect(release).not_to be_persisted
+      expect(response.status).to eq(200)
     end
 
-    it 'retrieves an existing release' do
-      get :edit, namespace_id: project.namespace, project_id: project, tag_id: release.tag
+    context 'when the project is private' do
+      let!(:project) { create(:project, :repository, :private) }
 
-      release = assigns(:release)
-      expect(release).not_to be_nil
-      expect(release).to be_persisted
-    end
-  end
+      it 'renders a 302' do
+        get_index
+
+        expect(response.status).to eq(302)
+      end
+
+      it 'renders a 200 for a logged in developer' do
+        project.add_developer(user)
+        sign_in(user)
 
-  describe 'PUT #update' do
-    it 'updates release note description' do
-      update_release('description updated')
+        get_index
 
-      release = project.releases.find_by_tag(tag)
-      expect(release.description).to eq("description updated")
+        expect(response.status).to eq(200)
+      end
+
+      it 'renders a 404 when logged in but not in the project' do
+        sign_in(user)
+
+        get_index
+
+        expect(response.status).to eq(404)
+      end
     end
 
-    it 'deletes release note when description is null' do
-      expect { update_release('') }.to change(project.releases, :count).by(-1)
+    context 'when releases_page feature flag is disabled' do
+      before do
+        stub_feature_flags(releases_page: false)
+      end
+
+      it 'renders a 404' do
+        get_index
+
+        expect(response.status).to eq(404)
+      end
     end
   end
 
-  def update_release(description)
-    put :update,
-      namespace_id: project.namespace.to_param,
-      project_id: project,
-      tag_id: release.tag,
-      release: { description: description }
+  private
+
+  def get_index
+    get :index, namespace_id: project.namespace, project_id: project
   end
 end
diff --git a/spec/controllers/projects/tags/releases_controller_spec.rb b/spec/controllers/projects/tags/releases_controller_spec.rb
new file mode 100644
index 00000000000..6bf4ac65a45
--- /dev/null
+++ b/spec/controllers/projects/tags/releases_controller_spec.rb
@@ -0,0 +1,57 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Projects::Tags::ReleasesController do
+  let!(:project) { create(:project, :repository) }
+  let!(:user)    { create(:user) }
+  let!(:release) { create(:release, project: project) }
+  let!(:tag)     { release.tag }
+
+  before do
+    project.add_developer(user)
+    sign_in(user)
+  end
+
+  describe 'GET #edit' do
+    it 'initializes a new release' do
+      tag_id = release.tag
+      project.releases.destroy_all # rubocop: disable DestroyAll
+
+      get :edit, namespace_id: project.namespace, project_id: project, tag_id: tag_id
+
+      release = assigns(:release)
+      expect(release).not_to be_nil
+      expect(release).not_to be_persisted
+    end
+
+    it 'retrieves an existing release' do
+      get :edit, namespace_id: project.namespace, project_id: project, tag_id: release.tag
+
+      release = assigns(:release)
+      expect(release).not_to be_nil
+      expect(release).to be_persisted
+    end
+  end
+
+  describe 'PUT #update' do
+    it 'updates release note description' do
+      update_release('description updated')
+
+      release = project.releases.find_by_tag(tag)
+      expect(release.description).to eq("description updated")
+    end
+
+    it 'deletes release note when description is null' do
+      expect { update_release('') }.to change(project.releases, :count).by(-1)
+    end
+  end
+
+  def update_release(description)
+    put :update,
+      namespace_id: project.namespace.to_param,
+      project_id: project,
+      tag_id: release.tag,
+      release: { description: description }
+  end
+end
diff --git a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb
index 98d4456b277..ec20c346234 100644
--- a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb
+++ b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb
@@ -49,4 +49,30 @@ describe 'layouts/nav/sidebar/_project' do
       expect(rendered).to have_css('.sidebar-top-level-items > li.active', text: 'Registry')
     end
   end
+
+  describe 'releases entry' do
+    describe 'when releases feature flag is disabled' do
+      before do
+        stub_feature_flags(releases_page: false)
+      end
+
+      it 'does not render releases link' do
+        render
+
+        expect(rendered).not_to have_link('Releases', href: project_releases_path(project))
+      end
+    end
+
+    describe 'when releases feature flags is enabled' do
+      before do
+        stub_feature_flags(releases_page: true)
+      end
+
+      it 'renders releases link' do
+        render
+
+        expect(rendered).to have_link('Releases', href: project_releases_path(project))
+      end
+    end
+  end
 end