Commit 3a871bb3 authored by Etienne Baqué's avatar Etienne Baqué

Updated review app button based on code review

Update ProjectPresenter.
Added rspec accordingly.
Removed changelog file.
parent f74eb8b4
...@@ -276,8 +276,8 @@ class ProjectPresenter < Gitlab::View::Presenter::Delegated ...@@ -276,8 +276,8 @@ class ProjectPresenter < Gitlab::View::Presenter::Delegated
end end
def kubernetes_cluster_anchor_data def kubernetes_cluster_anchor_data
if can_cluster_be_created? if can_instantiate_cluster?
if clusters_empty? if clusters.empty?
AnchorData.new(false, AnchorData.new(false,
statistic_icon + _('Add Kubernetes cluster'), statistic_icon + _('Add Kubernetes cluster'),
new_project_cluster_path(project)) new_project_cluster_path(project))
...@@ -327,26 +327,26 @@ class ProjectPresenter < Gitlab::View::Presenter::Delegated ...@@ -327,26 +327,26 @@ class ProjectPresenter < Gitlab::View::Presenter::Delegated
def can_setup_review_app? def can_setup_review_app?
strong_memoize(:can_setup_review_app) do strong_memoize(:can_setup_review_app) do
cicd_missing? || (can_cluster_be_created? && clusters_empty?) (can_instantiate_cluster? && all_clusters_empty?) || cicd_missing?
end end
end end
def can_cluster_be_created? def all_clusters_empty?
current_user && can?(current_user, :create_cluster, project) strong_memoize(:all_clusters_empty) do
project.all_clusters.empty?
end
end end
private
def cicd_missing? def cicd_missing?
current_user && can_current_user_push_code? && repository.gitlab_ci_yml.blank? && !auto_devops_enabled? current_user && can_current_user_push_code? && repository.gitlab_ci_yml.blank? && !auto_devops_enabled?
end end
def clusters_empty? def can_instantiate_cluster?
strong_memoize(:cluster_missing) do current_user && can?(current_user, :create_cluster, project)
project.clusters.empty?
end
end end
private
def filename_path(filename) def filename_path(filename)
if blob = repository.public_send(filename) # rubocop:disable GitlabSecurity/PublicSend if blob = repository.public_send(filename) # rubocop:disable GitlabSecurity/PublicSend
project_blob_path( project_blob_path(
......
...@@ -3,14 +3,14 @@ ...@@ -3,14 +3,14 @@
class ReviewAppSetupEntity < Grape::Entity class ReviewAppSetupEntity < Grape::Entity
include RequestAwareEntity include RequestAwareEntity
expose :can_setup_review_app? expose :can_setup_review_app?, as: :can_setup_review_app
expose :clusters_empty?, if: -> (_, _) { project.can_setup_review_app? } do |project| expose :all_clusters_empty?, as: :all_clusters_empty, if: -> (_, _) { project.can_setup_review_app? } do |project|
project.clusters_empty? project.all_clusters_empty?
end end
expose :review_snippet, if: -> (_, _) { project.can_setup_review_app? } do |_| expose :review_snippet, if: -> (_, _) { project.can_setup_review_app? } do |_|
YAML.safe_load(File.read(Rails.root.join('lib', 'gitlab', 'ci', 'snippets', 'review_app_default.yml'))).inspect YAML.safe_load(File.read(Rails.root.join('lib', 'gitlab', 'ci', 'snippets', 'review_app_default.yml'))).to_s
end end
private private
......
---
title: Provide backend support to show button for review app creation
merge_request: 22085
author:
type: added
{ {
"type": "object", "type": "object",
"required": ["can_setup_review_app?"], "required": ["can_setup_review_app"],
"properties": { "properties": {
"can_setup_review_app?": { "type": "boolean" }, "can_setup_review_app": { "type": "boolean" },
"clusters_empty?": { "type": "boolean" }, "all_clusters_empty": { "type": "boolean" },
"review_snippet": { "type": "string "} "review_snippet": { "type": "string "}
}, },
"additionalProperties": false "additionalProperties": false
......
...@@ -4,11 +4,10 @@ require 'spec_helper' ...@@ -4,11 +4,10 @@ require 'spec_helper'
describe ProjectPresenter do describe ProjectPresenter do
let(:user) { create(:user) } let(:user) { create(:user) }
describe '#license_short_name' do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:presenter) { described_class.new(project, current_user: user) } let(:presenter) { described_class.new(project, current_user: user) }
describe '#license_short_name' do
context 'when project.repository has a license_key' do context 'when project.repository has a license_key' do
it 'returns the nickname of the license if present' do it 'returns the nickname of the license if present' do
allow(project.repository).to receive(:license_key).and_return('agpl-3.0') allow(project.repository).to receive(:license_key).and_return('agpl-3.0')
...@@ -33,8 +32,6 @@ describe ProjectPresenter do ...@@ -33,8 +32,6 @@ describe ProjectPresenter do
end end
describe '#default_view' do describe '#default_view' do
let(:presenter) { described_class.new(project, current_user: user) }
context 'user not signed in' do context 'user not signed in' do
let(:user) { nil } let(:user) { nil }
...@@ -125,7 +122,6 @@ describe ProjectPresenter do ...@@ -125,7 +122,6 @@ describe ProjectPresenter do
describe '#can_current_user_push_code?' do describe '#can_current_user_push_code?' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:presenter) { described_class.new(project, current_user: user) }
context 'empty repo' do context 'empty repo' do
let(:project) { create(:project) } let(:project) { create(:project) }
...@@ -163,7 +159,6 @@ describe ProjectPresenter do ...@@ -163,7 +159,6 @@ describe ProjectPresenter do
context 'statistics anchors (empty repo)' do context 'statistics anchors (empty repo)' do
let(:project) { create(:project, :empty_repo) } let(:project) { create(:project, :empty_repo) }
let(:presenter) { described_class.new(project, current_user: user) }
describe '#files_anchor_data' do describe '#files_anchor_data' do
it 'returns files data' do it 'returns files data' do
...@@ -200,7 +195,6 @@ describe ProjectPresenter do ...@@ -200,7 +195,6 @@ describe ProjectPresenter do
context 'statistics anchors' do context 'statistics anchors' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
let(:presenter) { described_class.new(project, current_user: user) }
describe '#files_anchor_data' do describe '#files_anchor_data' do
it 'returns files data' do it 'returns files data' do
...@@ -416,7 +410,6 @@ describe ProjectPresenter do ...@@ -416,7 +410,6 @@ describe ProjectPresenter do
describe '#statistics_buttons' do describe '#statistics_buttons' do
let(:project) { build(:project) } let(:project) { build(:project) }
let(:presenter) { described_class.new(project, current_user: user) }
it 'orders the items correctly' do it 'orders the items correctly' do
allow(project.repository).to receive(:readme).and_return(double(name: 'readme')) allow(project.repository).to receive(:readme).and_return(double(name: 'readme'))
...@@ -435,8 +428,6 @@ describe ProjectPresenter do ...@@ -435,8 +428,6 @@ describe ProjectPresenter do
end end
describe '#repo_statistics_buttons' do describe '#repo_statistics_buttons' do
let(:presenter) { described_class.new(project, current_user: user) }
subject(:empty_repo_statistics_buttons) { presenter.empty_repo_statistics_buttons } subject(:empty_repo_statistics_buttons) { presenter.empty_repo_statistics_buttons }
before do before do
...@@ -485,4 +476,73 @@ describe ProjectPresenter do ...@@ -485,4 +476,73 @@ describe ProjectPresenter do
end end
end end
end end
describe '#can_setup_review_app?' do
subject { presenter.can_setup_review_app? }
context 'when the ci/cd file is missing' do
before do
allow(presenter).to receive(:cicd_missing?).and_return(true)
end
it { is_expected.to be_truthy }
end
context 'when the ci/cd file is not missing' do
before do
allow(presenter).to receive(:cicd_missing?).and_return(false)
end
context 'and the user can create a cluster' do
before do
allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?).with(user, :create_cluster, project).and_return(true)
end
context 'and there is no cluster associated to this project' do
let(:project) { create(:project, clusters: []) }
it { is_expected.to be_truthy }
end
context 'and there is already a cluster associated to this project' do
let(:project) { create(:project, clusters: [build(:cluster, :providing_by_gcp)]) }
it { is_expected.to be_falsey }
end
context 'when a group cluster is instantiated' do
let_it_be(:cluster) { create(:cluster, :group) }
let_it_be(:group) { cluster.group }
context 'and the project belongs to this group' do
let!(:project) { create(:project, group: group) }
it { is_expected.to be_falsey }
end
context 'and the project does not belong to this group' do
it { is_expected.to be_truthy }
end
end
context 'and there is already an instance cluster' do
it 'is false' do
create(:cluster, :instance)
is_expected.to be_falsey
end
end
end
context 'and the user cannot create a cluster' do
before do
allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?).with(user, :create_cluster, project).and_return(false)
end
it { is_expected.to be_falsey }
end
end
end
end end
...@@ -17,8 +17,8 @@ describe ReviewAppSetupEntity do ...@@ -17,8 +17,8 @@ describe ReviewAppSetupEntity do
subject { entity.as_json } subject { entity.as_json }
describe '#as_json' do describe '#as_json' do
it 'contains can_setup_review_app?' do it 'contains can_setup_review_app' do
expect(subject).to include(:can_setup_review_app?) expect(subject).to include(:can_setup_review_app)
end end
context 'when the user can setup a review app' do context 'when the user can setup a review app' do
...@@ -27,19 +27,17 @@ describe ReviewAppSetupEntity do ...@@ -27,19 +27,17 @@ describe ReviewAppSetupEntity do
end end
it 'contains relevant fields' do it 'contains relevant fields' do
expect(subject.keys).to include(:clusters_empty?, :review_snippet) expect(subject.keys).to include(:all_clusters_empty, :review_snippet)
end end
it 'exposes the relevant review snippet' do it 'exposes the relevant review snippet' do
review_app_snippet = YAML.safe_load(File.read(Rails.root.join('lib', 'gitlab', 'ci', 'snippets', 'review_app_default.yml'))).inspect review_app_snippet = YAML.safe_load(File.read(Rails.root.join('lib', 'gitlab', 'ci', 'snippets', 'review_app_default.yml'))).to_s
expect(subject[:review_snippet]).to eq(review_app_snippet) expect(subject[:review_snippet]).to eq(review_app_snippet)
end end
it 'exposes whether the project has associated clusters' do it 'exposes whether the project has associated clusters' do
allow(project).to receive(:clusters).and_return([]) expect(subject[:all_clusters_empty]).to be_truthy
expect(subject[:clusters_empty?]).to be_truthy
end end
end end
...@@ -49,7 +47,7 @@ describe ReviewAppSetupEntity do ...@@ -49,7 +47,7 @@ describe ReviewAppSetupEntity do
end end
it 'does not expose certain fields' do it 'does not expose certain fields' do
expect(subject.keys).not_to include(:clusters_empty?, :review_snippet) expect(subject.keys).not_to include(:all_clusters_empty, :review_snippet)
end end
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