Follow-up from "Add read attribute for ci setting to restrict pipeline cancellation"
The following discussions from !139414 (merged) should be addressed:
-
@drew started a discussion: attrs << :restrict_pipeline_cancellation_role if project.ci_cancellation_restriction.feature_available?
On the safe-nav operators:
- We don't use one in
#show
forproject.feature_available?
, do we need one here forproject.ci_cancellation_restriction
? - Project#ci_cancellation_restriction returns a
::Ci::ProjectCancellationRestriction
in all cases, do we need to do&.feature_available?
on it?
# ee/app/models/ee/project.rb def ci_cancellation_restriction ::Ci::ProjectCancellationRestriction.new(self) end
- We don't use one in
-
@drew started a discussion: return false unless @ci_settings
CI settings are contingent on the project:
def initialize(project) @project = project @ci_settings = project&.ci_cd_settings end
we could probably get rid of the first check?
-
@drew started a discussion: - return unless @project&.ci_cancellation_restriction.feature_available?
If we have the Project, in an EE view, this is guaranteed right?
-
@drew started a discussion: let(:current_user) { create(:user).tap { |u| project.add_developer(u) }
I checked and the test still passes this way, and just gets closer to the actual logic of the permission
Edited by Allison Browne