Commit 2b2f2821 authored by Shinya Maeda's avatar Shinya Maeda

Squashed commit of the following:

commit 7acc01f59c2954cfaa7de1cba93ddf160c938009
Author: Your Name <you@example.com>
Date:   Wed Oct 23 18:37:02 2019 +0700

    Edit release page - Back-end

    Squashed commit of the following:

    commit 0dce2c9ac97a6f2f588daeb0b81dde59590bb171
    Author: Etienne Baqué <ebaque@gitlab.com>
    Date:   Sun Oct 20 21:36:25 2019 +0300

        Removed unused links from the Release API doc

    commit d8438b837b8a164596aaa86ce3922eb3cd356a54
    Author: Etienne Baqué <ebaque@gitlab.com>
    Date:   Wed Oct 16 15:32:19 2019 +0300

        More updates based on code review

        Moved edit_url to Release entity class.
        Preloaded namspaces in Release finder.

    commit 2fad9190a3aa701f13265b37adf8c14e165191ee
    Author: Etienne Baqué <ebaque@gitlab.com>
    Date:   Tue Oct 15 13:14:40 2019 +0300

        Modified release route constraint

    commit 91038cb485e8884c2b83e672b86bd0abe9c867f9
    Author: Etienne Baqué <ebaque@gitlab.com>
    Date:   Fri Oct 11 10:51:06 2019 +0300

        Fixed various files based on code review

        Changed edit to edit_url in Release API.
        Added feature flag in ReleaseController.
        Refactored method into Release model.

    commit 2e0a9cf588232bad03aa2893938852551e32c2f2
    Author: Etienne Baqué <ebaque@gitlab.com>
    Date:   Thu Oct 10 09:48:43 2019 +0300

        Removed access to release edit page

    commit 57896ac535dc881e0b7f28a93c678ec87911dd50
    Author: Etienne Baqué <ebaque@gitlab.com>
    Date:   Wed Oct 9 12:56:10 2019 +0300

        Added tests for _links in release payload

        Added rspecs for ReleaseController#edit.
        Adjusted API schemas for testing.
        Updated Release API documentation.

    commit d9314c150ff4a09e4733b86a5203e7110f784a7f
    Author: Etienne Baqué <ebaque@gitlab.com>
    Date:   Sun Sep 29 23:16:17 2019 +0300

        Added routes for Release edit and update
parent 4a65acd7
...@@ -3,10 +3,12 @@ ...@@ -3,10 +3,12 @@
class Projects::ReleasesController < Projects::ApplicationController class Projects::ReleasesController < Projects::ApplicationController
# Authorize # Authorize
before_action :require_non_empty_project, except: [:index] before_action :require_non_empty_project, except: [:index]
before_action :release, only: %i[edit update]
before_action :authorize_read_release! before_action :authorize_read_release!
before_action do before_action do
push_frontend_feature_flag(:release_edit_page, project) push_frontend_feature_flag(:release_edit_page, project)
end end
before_action :authorize_update_release!, only: %i[edit update]
def index def index
respond_to do |format| respond_to do |format|
...@@ -22,4 +24,25 @@ class Projects::ReleasesController < Projects::ApplicationController ...@@ -22,4 +24,25 @@ class Projects::ReleasesController < Projects::ApplicationController
def releases def releases
ReleasesFinder.new(@project, current_user).execute ReleasesFinder.new(@project, current_user).execute
end end
def edit
respond_to do |format|
format.html { render 'edit' }
end
end
private
def authorize_update_release!
access_denied! unless Feature.enabled?(:release_edit_page, project)
access_denied! unless can?(current_user, :update_release, release)
end
def release
@release ||= project.releases.find_by_tag!(sanitized_tag_name)
end
def sanitized_tag_name
CGI.unescape(params[:tag])
end
end end
...@@ -6,9 +6,11 @@ class ReleasesFinder ...@@ -6,9 +6,11 @@ class ReleasesFinder
@current_user = current_user @current_user = current_user
end end
def execute def execute(preload: true)
return Release.none unless Ability.allowed?(@current_user, :read_release, @project) return Release.none unless Ability.allowed?(@current_user, :read_release, @project)
@project.releases.sorted releases = @project.releases
releases = releases.preloaded if preload
releases.sorted
end end
end end
...@@ -27,13 +27,17 @@ class Release < ApplicationRecord ...@@ -27,13 +27,17 @@ class Release < ApplicationRecord
validates_associated :milestone_releases, message: -> (_, obj) { obj[:value].map(&:errors).map(&:full_messages).join(",") } validates_associated :milestone_releases, message: -> (_, obj) { obj[:value].map(&:errors).map(&:full_messages).join(",") }
scope :sorted, -> { order(released_at: :desc) } scope :sorted, -> { order(released_at: :desc) }
scope :with_project_and_namespace, -> { includes(project: :namespace) } scope :preloaded, -> { includes(project: :namespace) }
delegate :repository, to: :project delegate :repository, to: :project
after_commit :create_evidence!, on: :create after_commit :create_evidence!, on: :create
after_commit :notify_new_release, on: :create after_commit :notify_new_release, on: :create
def to_param
CGI.escape(tag)
end
def commit def commit
strong_memoize(:commit) do strong_memoize(:commit) do
repository.commit(actual_sha) repository.commit(actual_sha)
......
...@@ -31,6 +31,12 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated ...@@ -31,6 +31,12 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated
project_issues_url(project, params_for_issues_and_mrs) project_issues_url(project, params_for_issues_and_mrs)
end end
def edit_url
return unless release_edit_page_available?
edit_project_release_url(project, release)
end
private private
def can_download_code? def can_download_code?
...@@ -44,4 +50,8 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated ...@@ -44,4 +50,8 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated
def release_mr_issue_urls_available? def release_mr_issue_urls_available?
::Feature.enabled?(:release_mr_issue_urls, project) ::Feature.enabled?(:release_mr_issue_urls, project)
end end
def release_edit_page_available?
::Feature.enabled?(:release_edit_page, project)
end
end end
...@@ -7,7 +7,7 @@ class NewReleaseWorker ...@@ -7,7 +7,7 @@ class NewReleaseWorker
feature_category :release_orchestration feature_category :release_orchestration
def perform(release_id) def perform(release_id)
release = Release.with_project_and_namespace.find_by_id(release_id) release = Release.preloaded.find_by_id(release_id)
return unless release return unless release
NotificationService.new.send_new_release_notifications(release) NotificationService.new.send_new_release_notifications(release)
......
...@@ -179,7 +179,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -179,7 +179,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end end
end end
resources :releases, only: [:index] resources :releases, only: [:index, :edit], param: :tag, constraints: { tag: %r{[^/]+} }
resources :starrers, only: [:index] resources :starrers, only: [:index]
resources :forks, only: [:index, :new, :create] resources :forks, only: [:index, :new, :create]
resources :group_links, only: [:index, :create, :update, :destroy], constraints: { id: /\d+/ } resources :group_links, only: [:index, :create, :update, :destroy], constraints: { id: /\d+/ }
......
...@@ -1324,6 +1324,7 @@ module API ...@@ -1324,6 +1324,7 @@ module API
expose :_links do expose :_links do
expose :merge_requests_url, expose_nil: false expose :merge_requests_url, expose_nil: false
expose :issues_url, expose_nil: false expose :issues_url, expose_nil: false
expose :edit_url, expose_nil: false
end end
private private
......
...@@ -5,10 +5,33 @@ require 'spec_helper' ...@@ -5,10 +5,33 @@ require 'spec_helper'
describe Projects::ReleasesController do describe Projects::ReleasesController do
let!(:project) { create(:project, :repository, :public) } let!(:project) { create(:project, :repository, :public) }
let!(:private_project) { create(:project, :repository, :private) } let!(:private_project) { create(:project, :repository, :private) }
let!(:user) { create(:user) } let(:user) { developer }
let(:developer) { create(:user) }
let(:reporter) { create(:user) }
let!(:release_1) { create(:release, project: project, released_at: Time.zone.parse('2018-10-18')) } let!(:release_1) { create(:release, project: project, released_at: Time.zone.parse('2018-10-18')) }
let!(:release_2) { create(:release, project: project, released_at: Time.zone.parse('2019-10-19')) } let!(:release_2) { create(:release, project: project, released_at: Time.zone.parse('2019-10-19')) }
before do
project.add_developer(developer)
project.add_reporter(reporter)
end
shared_examples_for 'successful request' do
it 'renders a 200' do
subject
expect(response).to have_gitlab_http_status(:success)
end
end
shared_examples_for 'not found' do
it 'renders 404' do
subject
expect(response).to have_gitlab_http_status(:not_found)
end
end
shared_examples 'common access controls' do shared_examples 'common access controls' do
it 'renders a 200' do it 'renders a 200' do
get_index get_index
...@@ -23,21 +46,31 @@ describe Projects::ReleasesController do ...@@ -23,21 +46,31 @@ describe Projects::ReleasesController do
sign_in(user) sign_in(user)
end end
context 'when user is a developer' do
let(:user) { developer }
it 'renders a 200 for a logged in developer' do it 'renders a 200 for a logged in developer' do
project.add_developer(user) sign_in(user)
get_index get_index
expect(response.status).to eq(200) expect(response.status).to eq(200)
end end
end
context 'when user is an external user' do
let(:user) { create(:user) }
it 'renders a 404 when logged in but not in the project' do it 'renders a 404 when logged in but not in the project' do
sign_in(user)
get_index get_index
expect(response.status).to eq(404) expect(response.status).to eq(404)
end end
end end
end end
end
describe 'GET #index' do describe 'GET #index' do
before do before do
...@@ -85,6 +118,54 @@ describe Projects::ReleasesController do ...@@ -85,6 +118,54 @@ describe Projects::ReleasesController do
end end
end end
describe 'GET #edit' do
subject do
get :edit, params: { namespace_id: project.namespace, project_id: project, tag: tag }
end
before do
sign_in(user)
end
let!(:release) { create(:release, project: project) }
let(:tag) { CGI.escape(release.tag) }
it_behaves_like 'successful request'
context 'when tag name contains slash' do
let!(:release) { create(:release, project: project, tag: 'awesome/v1.0') }
let(:tag) { CGI.escape(release.tag) }
it_behaves_like 'successful request'
it 'is accesible at a URL encoded path' do
expect(edit_project_release_path(project, release))
.to eq("/#{project.namespace.path}/#{project.name}/-/releases/awesome%252Fv1.0/edit")
end
end
context 'when feature flag `release_edit_page` is disabled' do
before do
stub_feature_flags(release_edit_page: false)
end
it_behaves_like 'not found'
end
context 'when release does not exist' do
let!(:release) { }
let(:tag) { 'non-existent-tag' }
it_behaves_like 'not found'
end
context 'when user is a reporter' do
let(:user) { reporter }
it_behaves_like 'not found'
end
end
private private
def get_index def get_index
......
...@@ -8,8 +8,7 @@ describe ReleasesFinder do ...@@ -8,8 +8,7 @@ describe ReleasesFinder do
let(:repository) { project.repository } let(:repository) { project.repository }
let(:v1_0_0) { create(:release, project: project, tag: 'v1.0.0') } let(:v1_0_0) { create(:release, project: project, tag: 'v1.0.0') }
let(:v1_1_0) { create(:release, project: project, tag: 'v1.1.0') } let(:v1_1_0) { create(:release, project: project, tag: 'v1.1.0') }
let(:finder) { described_class.new(project, user) }
subject { described_class.new(project, user)}
before do before do
v1_0_0.update_attribute(:released_at, 2.days.ago) v1_0_0.update_attribute(:released_at, 2.days.ago)
...@@ -17,11 +16,13 @@ describe ReleasesFinder do ...@@ -17,11 +16,13 @@ describe ReleasesFinder do
end end
describe '#execute' do describe '#execute' do
subject { finder.execute(**args) }
let(:args) { {} }
context 'when the user is not part of the project' do context 'when the user is not part of the project' do
it 'returns no releases' do it 'returns no releases' do
releases = subject.execute is_expected.to be_empty
expect(releases).to be_empty
end end
end end
...@@ -31,11 +32,25 @@ describe ReleasesFinder do ...@@ -31,11 +32,25 @@ describe ReleasesFinder do
end end
it 'sorts by release date' do it 'sorts by release date' do
releases = subject.execute is_expected.to be_present
expect(subject.size).to eq(2)
expect(subject).to eq([v1_1_0, v1_0_0])
end
it 'preloads associations' do
expect(Release).to receive(:preloaded).once.and_call_original
expect(releases).to be_present subject
expect(releases.size).to eq(2) end
expect(releases).to eq([v1_1_0, v1_0_0])
context 'when preload is false' do
let(:args) { { preload: false } }
it 'does not preload associations' do
expect(Release).not_to receive(:preloaded)
subject
end
end end
end end
end end
......
...@@ -38,10 +38,11 @@ ...@@ -38,10 +38,11 @@
"additionalProperties": false "additionalProperties": false
}, },
"_links": { "_links": {
"required": ["merge_requests_url", "issues_url"], "required": ["merge_requests_url", "issues_url", "edit_url"],
"properties": { "properties": {
"merge_requests_url": { "type": "string" }, "merge_requests_url": { "type": "string" },
"issues_url": { "type": "string" } "issues_url": { "type": "string" },
"edit_url": { "type": "string"}
} }
} }
}, },
......
...@@ -26,10 +26,11 @@ ...@@ -26,10 +26,11 @@
"additionalProperties": false "additionalProperties": false
}, },
"_links": { "_links": {
"required": ["merge_requests_url", "issues_url"], "required": ["merge_requests_url", "issues_url", "edit_url"],
"properties": { "properties": {
"merge_requests_url": { "type": "string" }, "merge_requests_url": { "type": "string" },
"issues_url": { "type": "string" } "issues_url": { "type": "string" },
"edit_url": { "type": "string"}
} }
} }
}, },
......
...@@ -82,4 +82,20 @@ describe ReleasePresenter do ...@@ -82,4 +82,20 @@ describe ReleasePresenter do
it { is_expected.to be_nil } it { is_expected.to be_nil }
end end
end end
describe '#edit_url' do
subject { presenter.edit_url }
it 'returns release edit url' do
is_expected.to match /#{edit_project_release_url(project, release)}/
end
context 'when release_edit_page feature flag is disabled' do
before do
stub_feature_flags(release_edit_page: false)
end
it { is_expected.to be_nil }
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment