Commit 6d7078e7 authored by Bala Kumar's avatar Bala Kumar Committed by Shinya Maeda

Resolve "Use Same Endpoint for HTML and JSON in OperationsController"

parent b8376928
# frozen_string_literal: true
# Note: Both Operations dashboard (https://docs.gitlab.com/ee/user/operations_dashboard/) and Environments dashboard (https://docs.gitlab.com/ee/ci/environments/environments_dashboard.html) features are co-existing in the same controller.
class OperationsController < ApplicationController
before_action :authorize_read_operations_dashboard!
respond_to :json, only: [:list]
feature_category :release_orchestration
POLLING_INTERVAL = 120_000
# Used by Operations dashboard.
def index
end
def environments
end
respond_to do |format|
format.html
def list
Gitlab::PollingInterval.set_header(response, interval: POLLING_INTERVAL)
format.json do
set_polling_interval_header
projects = load_projects
render json: { projects: serialize_as_json(projects) }
end
end
end
def environments_list
Gitlab::PollingInterval.set_header(response, interval: POLLING_INTERVAL)
# Used by Environments dashboard.
def environments
respond_to do |format|
format.html
format.json do
set_polling_interval_header
projects = load_environments_projects
render json: { projects: serialize_as_json_for_environments(projects) }
end
end
end
# Used by Operations and Environments dashboard.
def create
respond_to do |format|
format.json do
project_ids = params['project_ids']
result = add_projects(project_ids)
......@@ -40,7 +50,10 @@ class OperationsController < ApplicationController
invalid: result.invalid_project_ids
}
end
end
end
# Used by Operations and Environments dashboard.
def destroy
project_id = params['project_id']
......@@ -57,6 +70,10 @@ class OperationsController < ApplicationController
render_404 unless can?(current_user, :read_operations_dashboard)
end
def set_polling_interval_header
Gitlab::PollingInterval.set_header(response, interval: POLLING_INTERVAL)
end
def load_projects
Dashboard::Operations::ListService.new(current_user).execute
end
......
......@@ -6,8 +6,8 @@ module EE
def operations_data
{
'add-path' => add_operations_project_path,
'list-path' => operations_list_path,
'add-path' => add_operations_project_path(format: :json),
'list-path' => operations_path(format: :json),
'empty-dashboard-svg-path' => image_path('illustrations/operations-dashboard_empty.svg'),
'empty-dashboard-help-path' => help_page_path('user/operations_dashboard/index.md')
}
......@@ -15,8 +15,8 @@ module EE
def environments_data
{
'add-path' => add_operations_project_path,
'list-path' => operations_environments_list_path,
'add-path' => add_operations_environments_project_path(format: :json),
'list-path' => operations_environments_path(format: :json),
'empty-dashboard-svg-path' => image_path('illustrations/operations-dashboard_empty.svg'),
'empty-dashboard-help-path' => help_page_path('ci/environments/environments_dashboard.md'),
'environments-dashboard-help-path' => help_page_path('ci/environments/environments_dashboard.md')
......
......@@ -9,7 +9,7 @@ class DashboardEnvironmentsProjectEntity < Grape::Entity
expose :web_url
expose :remove_path do |project|
remove_operations_project_path(project_id: project.id)
remove_operations_environments_project_path(project_id: project.id)
end
expose :namespace, using: API::Entities::NamespaceBasic
......
# frozen_string_literal: true
# Used by Operations dashboard
get 'operations' => 'operations#index'
get 'operations/environments' => 'operations#environments'
get 'operations/list' => 'operations#list'
get 'operations/environments_list' => 'operations#environments_list'
post 'operations' => 'operations#create', as: :add_operations_project
delete 'operations' => 'operations#destroy', as: :remove_operations_project
# Used by Environments dashboard
get 'operations/environments' => 'operations#environments'
post 'operations/environments' => 'operations#create', as: :add_operations_environments_project
delete 'operations/environments' => 'operations#destroy', as: :remove_operations_environments_project
......@@ -14,49 +14,31 @@ RSpec.describe OperationsController do
sign_in(user)
end
shared_examples 'unlicensed' do |http_method, action|
shared_examples 'unlicensed' do |http_method, action, format = :html|
before do
stub_licensed_features(operations_dashboard: false)
end
it 'renders 404' do
public_send(http_method, action)
public_send(http_method, action, format: format)
expect(response).to have_gitlab_http_status(:not_found)
end
end
describe 'GET #index' do
it_behaves_like 'unlicensed', :get, :index
it 'renders index with 200 status code' do
get :index
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template(:index)
def get_index(format)
get :index, format: format
end
context 'with an anonymous user' do
before do
sign_out(user)
end
it 'redirects to sign-in page' do
get :index
describe 'format html' do
it_behaves_like 'unlicensed', :get, :index, :html
expect(response).to redirect_to(new_user_session_path)
end
end
end
describe 'GET #environments' do
it_behaves_like 'unlicensed', :get, :environments
it 'renders the view' do
get :environments
it 'renders index with 200 status code' do
get_index(:html)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template(:environments)
expect(response).to render_template(:index)
end
context 'with an anonymous user' do
......@@ -65,25 +47,25 @@ RSpec.describe OperationsController do
end
it 'redirects to sign-in page' do
get :environments
get_index(:html)
expect(response).to redirect_to(new_user_session_path)
end
end
end
describe 'GET #list' do
describe 'format json' do
let(:now) { Time.current.change(usec: 0) }
let(:project) { create(:project, :repository) }
let(:commit) { project.commit }
let!(:environment) { create(:environment, name: 'production', project: project) }
let!(:deployment) { create(:deployment, :success, environment: environment, sha: commit.id, created_at: now, project: project) }
it_behaves_like 'unlicensed', :get, :list
it_behaves_like 'unlicensed', :get, :index, :json
shared_examples 'empty project list' do
it 'returns an empty list' do
get :list
get_index(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to match_schema('dashboard/operations/list', dir: 'ee')
......@@ -124,7 +106,7 @@ RSpec.describe OperationsController do
end
it 'returns a list of added projects' do
get :list
get_index(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/list', dir: 'ee')
......@@ -148,7 +130,7 @@ RSpec.describe OperationsController do
end
user.update!(ops_dashboard_projects: projects)
get :list
get_index(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/list', dir: 'ee')
......@@ -157,7 +139,7 @@ RSpec.describe OperationsController do
end
it 'returns a list of added projects' do
get :list
get_index(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/list', dir: 'ee')
......@@ -183,16 +165,30 @@ RSpec.describe OperationsController do
sign_out(user)
end
it 'redirects to sign-in page' do
get :list
it 'returns unauthorized response' do
get_index(:json)
expect(response).to redirect_to(new_user_session_path)
expect(response).to have_gitlab_http_status(:unauthorized)
expect(json_response['error']).to include('sign in or sign up before continuing')
end
end
end
end
describe 'GET #environments_list' do
it_behaves_like 'unlicensed', :get, :environments_list
describe 'GET #environments' do
def get_environments(format, params = nil)
get :environments, params: params, format: format
end
describe 'format html' do
it_behaves_like 'unlicensed', :get, :environments, :html
it 'renders the view' do
get_environments(:html)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to render_template(:environments)
end
context 'with an anonymous user' do
before do
......@@ -200,11 +196,28 @@ RSpec.describe OperationsController do
end
it 'redirects to sign-in page' do
get :environments_list
get_environments(:html)
expect(response).to redirect_to(new_user_session_path)
end
end
end
describe 'format json' do
it_behaves_like 'unlicensed', :get, :environments, :json
context 'with an anonymous user' do
before do
sign_out(user)
end
it 'returns unauthorized response' do
get_environments(:json)
expect(response).to have_gitlab_http_status(:unauthorized)
expect(json_response['error']).to include('sign in or sign up before continuing')
end
end
context 'with an authenticated user without sufficient access_level' do
it 'returns an empty project list' do
......@@ -212,7 +225,7 @@ RSpec.describe OperationsController do
project.add_reporter(user)
user.update!(ops_dashboard_projects: [project])
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['projects']).to eq([])
......@@ -221,14 +234,14 @@ RSpec.describe OperationsController do
context 'with an authenticated developer' do
it 'returns an empty project list' do
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['projects']).to eq([])
end
it 'sets the polling interval header' do
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response.headers[Gitlab::PollingInterval::HEADER_NAME]).to eq('120000')
......@@ -239,7 +252,7 @@ RSpec.describe OperationsController do
project.add_developer(user)
user.update!(ops_dashboard_projects: [])
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['projects']).to eq([])
......@@ -254,10 +267,10 @@ RSpec.describe OperationsController do
end
it 'returns a project without an environment' do
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
......@@ -271,10 +284,10 @@ RSpec.describe OperationsController do
it 'returns one project with one environment' do
environment = create(:environment, project: project, name: 'staging')
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
......@@ -295,10 +308,10 @@ RSpec.describe OperationsController do
environment2 = create(:environment, project: project)
environment3 = create(:environment, project: project2)
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
expect(json_response['projects'].count).to eq(2)
expect(json_response['projects'].map { |p| p['id'] }.sort).to eq([project.id, project2.id])
......@@ -313,10 +326,10 @@ RSpec.describe OperationsController do
create(:environment, project: project, name: 'review/test-feature')
create(:environment, project: project, name: 'review/another-feature')
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
......@@ -326,10 +339,10 @@ RSpec.describe OperationsController do
it 'does not return environments that would be grouped into a folder even when there is only a single environment' do
create(:environment, project: project, name: 'staging/test-feature')
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
......@@ -339,10 +352,10 @@ RSpec.describe OperationsController do
it 'returns an environment not in a folder' do
environment = create(:environment, project: project, name: 'production')
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
......@@ -354,10 +367,10 @@ RSpec.describe OperationsController do
environment = create(:environment, project: project)
deployment = create(:deployment, :success, project: project, environment: environment)
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
environment_json = project_json['environments'].first
......@@ -371,10 +384,10 @@ RSpec.describe OperationsController do
ci_build = create(:ci_build, project: project)
create(:deployment, :success, project: project, environment: environment, deployable: ci_build)
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
environment_json = project_json['environments'].first
......@@ -388,10 +401,10 @@ RSpec.describe OperationsController do
environment = create(:environment, project: project)
deployment = create(:deployment, :failed, project: project, environment: environment)
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
environment_json = project_json['environments'].first
......@@ -403,10 +416,10 @@ RSpec.describe OperationsController do
context 'with environments pagination' do
shared_examples_for 'environments pagination' do |params, projects_count|
specify do
get :environments_list, params: params
get_environments(:json, params)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
expect(json_response['projects'].count).to eq(projects_count)
expect(response).to include_pagination_headers
end
......@@ -434,7 +447,7 @@ RSpec.describe OperationsController do
end
context 'N+1 queries' do
subject { get :environments_list }
subject { get_environments(:json) }
it 'avoids N+1 database queries' do
control_count = ActiveRecord::QueryRecorder.new { subject }.count
......@@ -456,10 +469,10 @@ RSpec.describe OperationsController do
unavailable_project.add_developer(user)
user.update!(ops_dashboard_projects: [unavailable_project])
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
expect(json_response['projects'].count).to eq(0)
end
......@@ -480,10 +493,10 @@ RSpec.describe OperationsController do
all_projects = [private_project] + public_projects
user.update!(ops_dashboard_projects: all_projects)
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
expect(json_response['projects'].count).to eq(7)
actual_ids = json_response['projects'].map { |p| p['id'].to_i }
......@@ -498,10 +511,10 @@ RSpec.describe OperationsController do
create(:environment, project: project)
create(:environment, project: project)
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
......@@ -521,10 +534,10 @@ RSpec.describe OperationsController do
create(:environment, project: project_b)
user.update!(ops_dashboard_projects: [project, project_b])
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].find { |p| p['id'] == project.id }
project_b_json = json_response['projects'].find { |p| p['id'] == project_b.id }
......@@ -548,10 +561,10 @@ RSpec.describe OperationsController do
ci_build = create(:ci_build, project: project, pipeline: pipeline)
create(:deployment, :success, project: project, environment: environment, deployable: ci_build, sha: commit.sha)
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
environment_json = project_json['environments'].first
......@@ -567,10 +580,10 @@ RSpec.describe OperationsController do
ci_build = create(:ci_build, project: project, pipeline: pipeline)
create(:deployment, :canceled, project: project, environment: environment, deployable: ci_build, sha: commit.sha)
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
environment_json = project_json['environments'].first
......@@ -593,10 +606,10 @@ RSpec.describe OperationsController do
create(:deployment, :success, project: project, environment: environment, deployable: ci_build, sha: commit.sha)
create(:ci_sources_pipeline, project: project, pipeline: pipeline, source_project: project_b, source_pipeline: pipeline_b, source_job: ci_build_b)
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
environment_json = project_json['environments'].first
......@@ -622,10 +635,10 @@ RSpec.describe OperationsController do
create(:deployment, :success, project: project, environment: environment, deployable: ci_build, sha: commit.sha)
create(:ci_sources_pipeline, project: project_b, pipeline: pipeline_b, source_project: project, source_pipeline: pipeline, source_job: ci_build)
get :environments_list
get_environments(:json)
expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('dashboard/operations/environments_list', dir: 'ee')
expect(response).to match_response_schema('dashboard/operations/environments', dir: 'ee')
project_json = json_response['projects'].first
environment_json = project_json['environments'].first
......@@ -647,9 +660,15 @@ RSpec.describe OperationsController do
end
end
end
end
describe 'POST #create' do
it_behaves_like 'unlicensed', :post, :create
describe 'format json' do
it_behaves_like 'unlicensed', :post, :create, :json
def post_create(params)
post :create, params: params, format: :json
end
context 'without added projects' do
let(:project_a) { create(:project) }
......@@ -661,7 +680,7 @@ RSpec.describe OperationsController do
end
it 'adds projects to the dashboard' do
post :create, params: { project_ids: [project_a.id, project_b.id.to_s] }
post_create({ project_ids: [project_a.id, project_b.id.to_s] })
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to match_schema('dashboard/operations/add', dir: 'ee')
......@@ -674,7 +693,7 @@ RSpec.describe OperationsController do
end
it 'cannot add a project twice' do
post :create, params: { project_ids: [project_a.id, project_a.id] }
post_create({ project_ids: [project_a.id, project_a.id] })
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to match_schema('dashboard/operations/add', dir: 'ee')
......@@ -687,7 +706,7 @@ RSpec.describe OperationsController do
end
it 'does not add invalid project ids' do
post :create, params: { project_ids: ['', -1, '-2'] }
post_create({ project_ids: ['', -1, '-2'] })
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to match_schema('dashboard/operations/add', dir: 'ee')
......@@ -709,7 +728,7 @@ RSpec.describe OperationsController do
end
it 'does not add already added project' do
post :create, params: { project_ids: [project.id] }
post_create({ project_ids: [project.id] })
expect(response).to have_gitlab_http_status(:ok)
expect(json_response).to match_schema('dashboard/operations/add', dir: 'ee')
......@@ -734,6 +753,7 @@ RSpec.describe OperationsController do
end
end
end
end
describe 'DELETE #destroy' do
it_behaves_like 'unlicensed', :delete, :destroy
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Operations routing', 'routing' do
describe '/-/operations' do
it 'routes to the operations index action' do
expect(get("#{operations_path}.html")).to route_to(
controller: 'operations',
action: 'index',
format: 'html')
expect(get("#{operations_path}.json")).to route_to(
controller: 'operations',
action: 'index',
format: 'json')
end
it 'routes to the operations create action' do
expect(post("#{add_operations_project_path}.json")).to route_to(
controller: 'operations',
action: 'create',
format: 'json')
end
it 'routes to operations destroy action' do
expect(delete("#{remove_operations_project_path}.json")).to route_to(
controller: 'operations',
action: 'destroy',
format: 'json')
end
end
describe '/-/operations/environments' do
it 'routes to the environments list action' do
expect(get("#{operations_environments_path}.html")).to route_to(
controller: 'operations',
action: 'environments',
format: 'html')
expect(get("#{operations_environments_path}.json")).to route_to(
controller: 'operations',
action: 'environments',
format: 'json')
end
it 'routes to the environments create action' do
expect(post("#{add_operations_environments_project_path}.json")).to route_to(
controller: 'operations',
action: 'create',
format: 'json')
end
it 'routes to environments destroy action' do
expect(delete("#{remove_operations_environments_project_path}.json")).to route_to(
controller: 'operations',
action: 'destroy',
format: 'json')
end
end
end
......@@ -6,8 +6,8 @@ RSpec.describe 'operations/environments.html.haml' do
it 'renders the frontend configuration' do
render
expect(rendered).to match %r{data-add-path="/-/operations"}
expect(rendered).to match %r{data-list-path="/-/operations/environments_list"}
expect(rendered).to match %r{data-add-path="/-/operations/environments.json"}
expect(rendered).to match %r{data-list-path="/-/operations/environments.json"}
expect(rendered).to match %r{data-empty-dashboard-svg-path="/assets/illustrations/operations-dashboard_empty.*\.svg"}
expect(rendered).to match %r{data-empty-dashboard-help-path="/help/ci/environments/environments_dashboard.md"}
expect(rendered).to match %r{data-environments-dashboard-help-path="/help/ci/environments/environments_dashboard.md"}
......
......@@ -6,8 +6,8 @@ RSpec.describe 'operations/index.html.haml' do
it 'renders the frontend configuration' do
render
expect(rendered).to match %r{data-add-path="/-/operations"}
expect(rendered).to match %r{data-list-path="/-/operations/list"}
expect(rendered).to match %r{data-add-path="/-/operations.json"}
expect(rendered).to match %r{data-list-path="/-/operations.json"}
expect(rendered).to match %{data-empty-dashboard-svg-path="/assets/illustrations/operations-dashboard_empty.*\.svg"}
expect(rendered).to match %r{data-empty-dashboard-help-path="/help/user/operations_dashboard/index.md"}
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