Skip to content
Snippets Groups Projects
Commit cd8cc880 authored by Eduardo Bonet's avatar Eduardo Bonet :speech_balloon:
Browse files

Refactors Controllers to use can?

Instead of calling directly the feature flag, uses can? to validate
whether user has access to feature

Changelog: changed
parent 5f239e59
No related branches found
No related tags found
1 merge request!121661Use can? on ::Ml Controllers
......@@ -3,7 +3,7 @@
module Projects
module Ml
class CandidatesController < ApplicationController
before_action :check_feature_flag, :set_candidate
before_action :check_feature_enabled, :set_candidate
feature_category :mlops
......@@ -26,8 +26,8 @@ def set_candidate
render_404 unless @candidate.present?
end
def check_feature_flag
render_404 unless Feature.enabled?(:ml_experiment_tracking, @project)
def check_feature_enabled
render_404 unless can?(current_user, :read_model_experiments, @project)
end
end
end
......
......@@ -5,7 +5,7 @@ module Ml
class ExperimentsController < ::Projects::ApplicationController
include Projects::Ml::ExperimentsHelper
before_action :check_feature_flag
before_action :check_feature_enabled
before_action :set_experiment, only: [:show, :destroy]
feature_category :mlops
......@@ -55,8 +55,8 @@ def destroy
private
def check_feature_flag
render_404 unless Feature.enabled?(:ml_experiment_tracking, @project)
def check_feature_enabled
render_404 unless can?(current_user, :read_model_experiments, @project)
end
def set_experiment
......
......@@ -10,11 +10,13 @@
let(:ff_value) { true }
let(:candidate_iid) { candidate.iid }
let(:model_experiments_enabled) { true }
before do
stub_feature_flags(ml_experiment_tracking: false)
stub_feature_flags(ml_experiment_tracking: project) if ff_value
allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?)
.with(user, :read_model_experiments, project)
.and_return(model_experiments_enabled)
sign_in(user)
end
......@@ -32,9 +34,9 @@
end
end
shared_examples '404 if feature flag disabled' do
context 'when :ml_experiment_tracking disabled' do
let(:ff_value) { false }
shared_examples '404 when model experiments is unavailable' do
context 'when user does not have access' do
let(:model_experiments_enabled) { false }
it_behaves_like 'renders 404'
end
......@@ -59,7 +61,7 @@
end
it_behaves_like '404 if candidate does not exist'
it_behaves_like '404 if feature flag disabled'
it_behaves_like '404 when model experiments is unavailable'
end
describe 'DELETE #destroy' do
......@@ -81,7 +83,7 @@
end
it_behaves_like '404 if candidate does not exist'
it_behaves_like '404 if feature flag disabled'
it_behaves_like '404 when model experiments is unavailable'
end
private
......
......@@ -3,27 +3,25 @@
require 'spec_helper'
RSpec.describe Projects::Ml::ExperimentsController, feature_category: :mlops do
let_it_be(:project_with_feature) { create(:project, :repository) }
let_it_be(:user) { project_with_feature.first_owner }
let_it_be(:project_without_feature) do
create(:project, :repository).tap { |p| p.add_developer(user) }
end
let_it_be(:project) { create(:project, :repository) }
let_it_be(:user) { project.first_owner }
let_it_be(:experiment) do
create(:ml_experiments, project: project_with_feature, user: user).tap do |e|
create(:ml_experiments, project: project, user: user).tap do |e|
create(:ml_candidates, experiment: e, user: user)
end
end
let(:params) { basic_params }
let(:ff_value) { true }
let(:project) { project_with_feature }
let(:basic_params) { { namespace_id: project.namespace.to_param, project_id: project } }
let(:experiment_iid) { experiment.iid }
let(:model_experiments_enabled) { true }
before do
stub_feature_flags(ml_experiment_tracking: false)
stub_feature_flags(ml_experiment_tracking: project_with_feature) if ff_value
allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?)
.with(user, :read_model_experiments, project)
.and_return(model_experiments_enabled)
sign_in(user)
end
......@@ -42,9 +40,9 @@
end
end
shared_examples '404 if feature flag disabled' do
context 'when :ml_experiment_tracking disabled' do
let(:ff_value) { false }
shared_examples '404 when model experiments is unavailable' do
context 'when user does not have access' do
let(:model_experiments_enabled) { false }
it_behaves_like 'renders 404'
end
......@@ -71,7 +69,7 @@
describe 'pagination' do
let_it_be(:experiments) do
create_list(:ml_experiments, 3, project: project_with_feature)
create_list(:ml_experiments, 3, project: project)
end
let(:params) { basic_params.merge(id: experiment.iid) }
......@@ -102,19 +100,7 @@
end
end
context 'when :ml_experiment_tracking is disabled for the project' do
let(:project) { project_without_feature }
before do
list_experiments
end
it 'responds with a 404' do
expect(response).to have_gitlab_http_status(:not_found)
end
end
it_behaves_like '404 if feature flag disabled' do
it_behaves_like '404 when model experiments is unavailable' do
before do
list_experiments
end
......@@ -225,7 +211,7 @@
end
it_behaves_like '404 if experiment does not exist'
it_behaves_like '404 if feature flag disabled'
it_behaves_like '404 when model experiments is unavailable'
end
end
......@@ -257,14 +243,14 @@
end
it_behaves_like '404 if experiment does not exist'
it_behaves_like '404 if feature flag disabled'
it_behaves_like '404 when model experiments is unavailable'
end
end
end
describe 'DELETE #destroy' do
let_it_be(:experiment_for_deletion) do
create(:ml_experiments, project: project_with_feature, user: user).tap do |e|
create(:ml_experiments, project: project, user: user).tap do |e|
create(:ml_candidates, experiment: e, user: user)
end
end
......@@ -282,7 +268,7 @@
end
it_behaves_like '404 if experiment does not exist'
it_behaves_like '404 if feature flag disabled'
it_behaves_like '404 when model experiments is unavailable'
end
private
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment