From 633e64382d30674fecb1aaf138d18c73827c9a40 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 09:51:36 +0100 Subject: Added Ci::Status::Build --- app/models/ci/build.rb | 4 +++ app/models/commit_status.rb | 4 +++ lib/gitlab/ci/status/build/common.rb | 54 +++++++++++++++++++++++++++++++++++ lib/gitlab/ci/status/build/factory.rb | 15 ++++++++++ lib/gitlab/ci/status/core.rb | 10 +++++-- 5 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 lib/gitlab/ci/status/build/common.rb create mode 100644 lib/gitlab/ci/status/build/factory.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 88c46076df6..0f4c498c266 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -100,6 +100,10 @@ module Ci end end + def detailed_status + Gitlab::Ci::Status::Build::Factory.new(self).fabricate! + end + def manual? self.when == 'manual' end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index cf90475f4d4..fce16174e22 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -131,4 +131,8 @@ class CommitStatus < ActiveRecord::Base def has_trace? false end + + def detailed_status + Gitlab::Ci::Status::Factory.new(self).fabricate! + end end diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb new file mode 100644 index 00000000000..d3d7e03ee3f --- /dev/null +++ b/lib/gitlab/ci/status/build/common.rb @@ -0,0 +1,54 @@ +module Gitlab + module Ci + module Status + module Build + module Common + def has_details? + true + end + + def details_path + namespace_project_build_path(@subject.project.namespace, + @subject.project, + @subject.pipeline) + end + + def action_type + case + when @subject.playable? then :playable + when @subject.active? then :cancel + when @subject.retryable? then :retry + end + end + + def has_action?(current_user) + action_type && can?(current_user, :update_build, @subject) + end + + def action_icon + case action_type + when :playable then 'remove' + when :cancel then 'icon_play' + when :retry then 'repeat' + end + end + + def action_path + case action_type + when :playable + play_namespace_project_build_path(subject.project.namespace, subject.project, subject) + when :cancel + cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject) + when :retry + retry_namespace_project_build_path(subject.project.namespace, subject.project, subject) + end + end + + def action_method + :post + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb new file mode 100644 index 00000000000..dd38e1418b6 --- /dev/null +++ b/lib/gitlab/ci/status/build/factory.rb @@ -0,0 +1,15 @@ +module Gitlab + module Ci + module Status + module Build + class Factory < Status::Factory + private + + def core_status + super.extend(Status::Build::Common) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index ce4108fdcf2..60c559248aa 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -34,15 +34,15 @@ module Gitlab end def has_details? - raise NotImplementedError + false end def details_path raise NotImplementedError end - def has_action? - raise NotImplementedError + def has_action?(_user = nil) + false end def action_icon @@ -52,6 +52,10 @@ module Gitlab def action_path raise NotImplementedError end + + def action_method + raise NotImplementedError + end end end end -- cgit v1.2.1 From 516dc7a5be3624f1866fa46f19f6472e2f9fae22 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 10:40:56 +0100 Subject: Improve actions --- app/models/ci/build.rb | 4 +++ lib/gitlab/ci/status/build/common.rb | 26 ++++++------------- lib/gitlab/ci/status/build/factory.rb | 4 +++ lib/gitlab/ci/status/build/play.rb | 47 +++++++++++++++++++++++++++++++++++ lib/gitlab/ci/status/build/stop.rb | 47 +++++++++++++++++++++++++++++++++++ 5 files changed, 110 insertions(+), 18 deletions(-) create mode 100644 lib/gitlab/ci/status/build/play.rb create mode 100644 lib/gitlab/ci/status/build/stop.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 0f4c498c266..73564dd2aa0 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -127,6 +127,10 @@ module Ci end end + def cancelable? + active? + end + def retryable? project.builds_enabled? && commands.present? && complete? end diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb index d3d7e03ee3f..2bed68d1a11 100644 --- a/lib/gitlab/ci/status/build/common.rb +++ b/lib/gitlab/ci/status/build/common.rb @@ -13,33 +13,23 @@ module Gitlab @subject.pipeline) end - def action_type - case - when @subject.playable? then :playable - when @subject.active? then :cancel - when @subject.retryable? then :retry - end - end - def has_action?(current_user) - action_type && can?(current_user, :update_build, @subject) + (subject.cancelable? || subject.retryable?) && + can?(current_user, :update_build, @subject) end def action_icon - case action_type - when :playable then 'remove' - when :cancel then 'icon_play' - when :retry then 'repeat' + case + when subject.cancelable? then 'icon_play' + when subject.retryable? then 'repeat' end end def action_path - case action_type - when :playable - play_namespace_project_build_path(subject.project.namespace, subject.project, subject) - when :cancel + case + when subject.cancelable? cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject) - when :retry + when subject.retryable? retry_namespace_project_build_path(subject.project.namespace, subject.project, subject) end end diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index dd38e1418b6..d8a9f53f236 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -5,6 +5,10 @@ module Gitlab class Factory < Status::Factory private + def extended_statuses + [Stop, Play] + end + def core_status super.extend(Status::Build::Common) end diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb new file mode 100644 index 00000000000..581c81d0175 --- /dev/null +++ b/lib/gitlab/ci/status/build/play.rb @@ -0,0 +1,47 @@ +module Gitlab + module Ci + module Status + module Status + class Play < SimpleDelegator + extend Status::Extended + + def text + 'play' + end + + def label + 'play' + end + + def icon + 'icon_status_skipped' + end + + def to_s + 'play' + end + + def has_action?(current_user) + can?(current_user, :update_build, subject) + end + + def action_icon + :play + end + + def action_path + play_namespace_project_build_path(subject.project.namespace, subject.project, subject) + end + + def action_method + :post + end + + def self.matches?(build) + build.playable? && !build.stops_environment? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb new file mode 100644 index 00000000000..261de9695c5 --- /dev/null +++ b/lib/gitlab/ci/status/build/stop.rb @@ -0,0 +1,47 @@ +module Gitlab + module Ci + module Status + module Status + class Play < SimpleDelegator + extend Status::Extended + + def text + 'stop' + end + + def label + 'stop' + end + + def icon + 'icon_status_skipped' + end + + def to_s + 'stop' + end + + def has_action?(current_user) + can?(current_user, :update_build, subject) + end + + def action_icon + :play + end + + def action_path + play_namespace_project_build_path(subject.project.namespace, subject.project, subject) + end + + def action_method + :post + end + + def self.matches?(build) + build.playable? && build.stops_environment? + end + end + end + end + end +end -- cgit v1.2.1 From 1b6c2c3c0a38bed733d861902eb8c9397ab76cd3 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 10:48:08 +0100 Subject: Introduce `cancelable` and `returnable` [ci skip] --- lib/gitlab/ci/status/build/cancelable.rb | 31 +++++++++++++++++++++++++++++++ lib/gitlab/ci/status/build/common.rb | 25 ------------------------- lib/gitlab/ci/status/build/factory.rb | 2 +- lib/gitlab/ci/status/build/play.rb | 10 +--------- lib/gitlab/ci/status/build/retryable.rb | 31 +++++++++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 35 deletions(-) create mode 100644 lib/gitlab/ci/status/build/cancelable.rb create mode 100644 lib/gitlab/ci/status/build/retryable.rb diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb new file mode 100644 index 00000000000..bff0464ef0c --- /dev/null +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -0,0 +1,31 @@ +module Gitlab + module Ci + module Status + module Status + class Cancelable < SimpleDelegator + extend Status::Extended + + def has_action?(current_user) + can?(current_user, :update_build, subject) + end + + def action_icon + 'remove' + end + + def action_path + cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject) + end + + def action_method + :post + end + + def self.matches?(build) + build.cancelable? + end + end + end + end + end +end diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb index 2bed68d1a11..3e47d7dfd20 100644 --- a/lib/gitlab/ci/status/build/common.rb +++ b/lib/gitlab/ci/status/build/common.rb @@ -12,31 +12,6 @@ module Gitlab @subject.project, @subject.pipeline) end - - def has_action?(current_user) - (subject.cancelable? || subject.retryable?) && - can?(current_user, :update_build, @subject) - end - - def action_icon - case - when subject.cancelable? then 'icon_play' - when subject.retryable? then 'repeat' - end - end - - def action_path - case - when subject.cancelable? - cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject) - when subject.retryable? - retry_namespace_project_build_path(subject.project.namespace, subject.project, subject) - end - end - - def action_method - :post - end end end end diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index d8a9f53f236..8f420a93954 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -6,7 +6,7 @@ module Gitlab private def extended_statuses - [Stop, Play] + [Stop, Play, Cancelable, Retryable] end def core_status diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 581c81d0175..d295850137b 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -13,20 +13,12 @@ module Gitlab 'play' end - def icon - 'icon_status_skipped' - end - - def to_s - 'play' - end - def has_action?(current_user) can?(current_user, :update_build, subject) end def action_icon - :play + 'play' end def action_path diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb new file mode 100644 index 00000000000..b3c0eedadf8 --- /dev/null +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -0,0 +1,31 @@ +module Gitlab + module Ci + module Status + module Status + class Retryable < SimpleDelegator + extend Status::Extended + + def has_action?(current_user) + can?(current_user, :update_build, subject) + end + + def action_icon + 'repeat' + end + + def action_path + retry_namespace_project_build_path(subject.project.namespace, subject.project, subject) + end + + def action_method + :post + end + + def self.matches?(build) + build.retryable? + end + end + end + end + end +end -- cgit v1.2.1 From a83a80edb368d9b5697493123c2f13d8b7c6531e Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 10:52:44 +0100 Subject: Check permission of details --- lib/gitlab/ci/status/build/common.rb | 4 ++-- lib/gitlab/ci/status/core.rb | 2 +- lib/gitlab/ci/status/pipeline/common.rb | 4 ++-- lib/gitlab/ci/status/stage/common.rb | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb index 3e47d7dfd20..2fb79afa3d3 100644 --- a/lib/gitlab/ci/status/build/common.rb +++ b/lib/gitlab/ci/status/build/common.rb @@ -3,8 +3,8 @@ module Gitlab module Status module Build module Common - def has_details? - true + def has_details?(current_user) + can?(current_user, :read_build, subject) end def details_path diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index 60c559248aa..6b47096f811 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -33,7 +33,7 @@ module Gitlab self.class.name.demodulize.downcase.underscore end - def has_details? + def has_details?(_user = nil) false end diff --git a/lib/gitlab/ci/status/pipeline/common.rb b/lib/gitlab/ci/status/pipeline/common.rb index 25e52bec3da..5f79044a496 100644 --- a/lib/gitlab/ci/status/pipeline/common.rb +++ b/lib/gitlab/ci/status/pipeline/common.rb @@ -3,8 +3,8 @@ module Gitlab module Status module Pipeline module Common - def has_details? - true + def has_details?(current_user) + can?(current_user, :read_pipeline, subject) end def details_path diff --git a/lib/gitlab/ci/status/stage/common.rb b/lib/gitlab/ci/status/stage/common.rb index 14c437d2b98..e6ee2f92341 100644 --- a/lib/gitlab/ci/status/stage/common.rb +++ b/lib/gitlab/ci/status/stage/common.rb @@ -3,8 +3,8 @@ module Gitlab module Status module Stage module Common - def has_details? - true + def has_details?(current_user) + can?(current_user, :read_pipeline, subject) end def details_path -- cgit v1.2.1 From feaf01802c092be8f55994c910f2975376cbd20f Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 11:03:01 +0100 Subject: Remove ci_status_with_icon helper and replace it with partial [ci skip] --- app/helpers/ci_status_helper.rb | 20 +------------------- app/views/admin/runners/show.html.haml | 2 +- app/views/ci/status/_icon_with_label.html.haml | 10 ++++++++++ app/views/projects/builds/_header.html.haml | 2 +- app/views/projects/ci/builds/_build.html.haml | 5 +---- app/views/projects/ci/pipelines/_pipeline.html.haml | 5 +---- .../_generic_commit_status.html.haml | 5 +---- app/views/projects/pipelines/_info.html.haml | 2 +- 8 files changed, 17 insertions(+), 34 deletions(-) create mode 100644 app/views/ci/status/_icon_with_label.html.haml diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index 8e19752a8a1..d9f5e01f0dc 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -4,25 +4,7 @@ module CiStatusHelper builds_namespace_project_commit_path(project.namespace, project, pipeline.sha) end - def ci_status_with_icon(status, target = nil) - content = ci_icon_for_status(status) + ci_text_for_status(status) - klass = "ci-status ci-#{status}" - - if target - link_to content, target, class: klass - else - content_tag :span, content, class: klass - end - end - - def ci_text_for_status(status) - if detailed_status?(status) - status.text - else - status - end - end - + # Is used by Commit and Merge Request Widget def ci_label_for_status(status) if detailed_status?(status) return status.label diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index 73038164056..fa8be25ffa8 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -91,7 +91,7 @@ %strong ##{build.id} %td.status - = ci_status_with_icon(build.status) + = render "ci/status/icon_with_label", subject: build %td.status - if project diff --git a/app/views/ci/status/_icon_with_label.html.haml b/app/views/ci/status/_icon_with_label.html.haml new file mode 100644 index 00000000000..65a74e88444 --- /dev/null +++ b/app/views/ci/status/_icon_with_label.html.haml @@ -0,0 +1,10 @@ +- details_path = subject.details_path if subject.has_details?(current_user) +- klass = "ci-status ci-#{subject.status}" +- if details_path + = link_to details_path, class: klass do + = custom_icon(status.icon) + = status.text +- else + %span{ class: klass } + = custom_icon(status.icon) + = status.text diff --git a/app/views/projects/builds/_header.html.haml b/app/views/projects/builds/_header.html.haml index f6aa20c4579..5e4e30f08d5 100644 --- a/app/views/projects/builds/_header.html.haml +++ b/app/views/projects/builds/_header.html.haml @@ -1,6 +1,6 @@ .content-block.build-header .header-content - = ci_status_with_icon(@build.status) + = render "ci/status/icon_with_label", subject: build Build %strong ##{@build.id} in pipeline diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 18b3b04154f..6b0cd3e49a0 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -9,10 +9,7 @@ %tr.build.commit{class: ('retried' if retried)} %td.status - - if can?(current_user, :read_build, build) - = ci_status_with_icon(build.status, namespace_project_build_url(build.project.namespace, build.project, build)) - - else - = ci_status_with_icon(build.status) + = render "ci/status/icon_with_label", subject: build %td.branch-commit - if can?(current_user, :read_build, build) diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index b58dceb58c9..84243e4306d 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -1,13 +1,10 @@ - status = pipeline.status -- detailed_status = pipeline.detailed_status - show_commit = local_assigns.fetch(:show_commit, true) - show_branch = local_assigns.fetch(:show_branch, true) %tr.commit %td.commit-link - = link_to namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id), class: "ci-status ci-#{detailed_status}" do - = ci_icon_for_status(detailed_status) - = ci_text_for_status(detailed_status) + = render "ci/status/icon_with_label", subject: pipeline %td = link_to namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id) do diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml index 7f751d9ae2e..69cb1631ee8 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml @@ -8,10 +8,7 @@ %tr.generic_commit_status{class: ('retried' if retried)} %td.status - - if can?(current_user, :read_commit_status, generic_commit_status) && generic_commit_status.target_url - = ci_status_with_icon(generic_commit_status.status, generic_commit_status.target_url) - - else - = ci_status_with_icon(generic_commit_status.status) + = render "ci/status/icon_with_label", subject: generic_commit_status %td.generic_commit_status-link - if can?(current_user, :read_commit_status, generic_commit_status) && generic_commit_status.target_url diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index 229bdfb0e8d..f7385184a2b 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -1,6 +1,6 @@ .page-content-header .header-main-content - = ci_status_with_icon(@pipeline.detailed_status) + = render "ci/status/icon_with_label", subject: @pipeline %strong Pipeline ##{@commit.pipelines.last.id} triggered #{time_ago_with_tooltip(@commit.authored_date)} by = author_avatar(@commit, size: 24) -- cgit v1.2.1 From e0ce97fb7d7d995fa76df57bfaac6d3601800190 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 8 Dec 2016 13:51:06 +0100 Subject: Refactor ci status factories to DRY code a little --- lib/gitlab/ci/status/build/factory.rb | 11 +++++------ lib/gitlab/ci/status/extended.rb | 2 +- lib/gitlab/ci/status/factory.rb | 30 +++++++++++++++++------------- lib/gitlab/ci/status/pipeline/factory.rb | 8 +++----- lib/gitlab/ci/status/stage/factory.rb | 6 ++---- 5 files changed, 28 insertions(+), 29 deletions(-) diff --git a/lib/gitlab/ci/status/build/factory.rb b/lib/gitlab/ci/status/build/factory.rb index 8f420a93954..eee9a64120b 100644 --- a/lib/gitlab/ci/status/build/factory.rb +++ b/lib/gitlab/ci/status/build/factory.rb @@ -3,14 +3,13 @@ module Gitlab module Status module Build class Factory < Status::Factory - private - - def extended_statuses - [Stop, Play, Cancelable, Retryable] + def self.extended_statuses + [Status::Build::Stop, Status::Build::Play, + Status::Build::Cancelable, Status::Build::Retryable] end - def core_status - super.extend(Status::Build::Common) + def self.common_helpers + Status::Build::Common end end end diff --git a/lib/gitlab/ci/status/extended.rb b/lib/gitlab/ci/status/extended.rb index 6bfb5d38c1f..93e6eff1c94 100644 --- a/lib/gitlab/ci/status/extended.rb +++ b/lib/gitlab/ci/status/extended.rb @@ -2,7 +2,7 @@ module Gitlab module Ci module Status module Extended - def matches?(_subject) + def matches?(_subject, _user) raise NotImplementedError end end diff --git a/lib/gitlab/ci/status/factory.rb b/lib/gitlab/ci/status/factory.rb index b2f896f2211..944e0fdde2d 100644 --- a/lib/gitlab/ci/status/factory.rb +++ b/lib/gitlab/ci/status/factory.rb @@ -2,10 +2,9 @@ module Gitlab module Ci module Status class Factory - attr_reader :subject - - def initialize(subject) + def initialize(subject, user = nil) @subject = subject + @user = user end def fabricate! @@ -16,27 +15,32 @@ module Gitlab end end + def self.extended_statuses + [] + end + + def self.common_helpers + Module.new + end + private - def subject_status - @subject_status ||= subject.status + def simple_status + @simple_status ||= @subject.status || :created end def core_status Gitlab::Ci::Status - .const_get(subject_status.capitalize) - .new(subject) + .const_get(simple_status.capitalize) + .new(@subject) + .extend(self.class.common_helpers) end def extended_status - @extended ||= extended_statuses.find do |status| - status.matches?(subject) + @extended ||= self.class.extended_statuses.find do |status| + status.matches?(@subject, @user) end end - - def extended_statuses - [] - end end end end diff --git a/lib/gitlab/ci/status/pipeline/factory.rb b/lib/gitlab/ci/status/pipeline/factory.rb index 4ac4ec671d0..16dcb326be9 100644 --- a/lib/gitlab/ci/status/pipeline/factory.rb +++ b/lib/gitlab/ci/status/pipeline/factory.rb @@ -3,14 +3,12 @@ module Gitlab module Status module Pipeline class Factory < Status::Factory - private - - def extended_statuses + def self.extended_statuses [Pipeline::SuccessWithWarnings] end - def core_status - super.extend(Status::Pipeline::Common) + def self.common_helpers + Status::Pipeline::Common end end end diff --git a/lib/gitlab/ci/status/stage/factory.rb b/lib/gitlab/ci/status/stage/factory.rb index c6522d5ada1..689a5dd45bc 100644 --- a/lib/gitlab/ci/status/stage/factory.rb +++ b/lib/gitlab/ci/status/stage/factory.rb @@ -3,10 +3,8 @@ module Gitlab module Status module Stage class Factory < Status::Factory - private - - def core_status - super.extend(Status::Stage::Common) + def self.common_helpers + Status::Stage::Common end end end -- cgit v1.2.1 From 5059d0b834eeea22ada4b6ac98cfddc2123691e9 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 8 Dec 2016 14:28:49 +0100 Subject: Incorporate permission checks into new CI statuses [ci skip] --- lib/gitlab/ci/status/build/cancelable.rb | 12 +++++++----- lib/gitlab/ci/status/build/common.rb | 10 +++++----- lib/gitlab/ci/status/build/play.rb | 12 +++++++----- lib/gitlab/ci/status/build/retryable.rb | 12 +++++++----- lib/gitlab/ci/status/build/stop.rb | 12 +++++++----- lib/gitlab/ci/status/core.rb | 13 ++++++------- lib/gitlab/ci/status/extended.rb | 8 ++++++-- lib/gitlab/ci/status/factory.rb | 4 ++-- lib/gitlab/ci/status/pipeline/common.rb | 10 +++++----- lib/gitlab/ci/status/pipeline/success_with_warnings.rb | 4 ++-- lib/gitlab/ci/status/stage/common.rb | 12 ++++++------ 11 files changed, 60 insertions(+), 49 deletions(-) diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index bff0464ef0c..a8830b04715 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -3,10 +3,10 @@ module Gitlab module Status module Status class Cancelable < SimpleDelegator - extend Status::Extended + include Status::Extended - def has_action?(current_user) - can?(current_user, :update_build, subject) + def has_action? + can?(user, :update_build, subject) end def action_icon @@ -14,14 +14,16 @@ module Gitlab end def action_path - cancel_namespace_project_build_path(subject.project.namespace, subject.project, subject) + cancel_namespace_project_build_path(subject.project.namespace, + subject.project, + subject) end def action_method :post end - def self.matches?(build) + def self.matches?(build, user) build.cancelable? end end diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb index 2fb79afa3d3..2b602f1e247 100644 --- a/lib/gitlab/ci/status/build/common.rb +++ b/lib/gitlab/ci/status/build/common.rb @@ -3,14 +3,14 @@ module Gitlab module Status module Build module Common - def has_details?(current_user) - can?(current_user, :read_build, subject) + def has_details? + can?(user, :read_build, subject) end def details_path - namespace_project_build_path(@subject.project.namespace, - @subject.project, - @subject.pipeline) + namespace_project_build_path(subject.project.namespace, + subject.project, + subject.pipeline) end end end diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index d295850137b..70c08197ea1 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -3,7 +3,7 @@ module Gitlab module Status module Status class Play < SimpleDelegator - extend Status::Extended + include Status::Extended def text 'play' @@ -13,8 +13,8 @@ module Gitlab 'play' end - def has_action?(current_user) - can?(current_user, :update_build, subject) + def has_action? + can?(user, :update_build, subject) end def action_icon @@ -22,14 +22,16 @@ module Gitlab end def action_path - play_namespace_project_build_path(subject.project.namespace, subject.project, subject) + play_namespace_project_build_path(subject.project.namespace, + subject.project, + subject) end def action_method :post end - def self.matches?(build) + def self.matches?(build, user) build.playable? && !build.stops_environment? end end diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index b3c0eedadf8..3309e8808e1 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -3,10 +3,10 @@ module Gitlab module Status module Status class Retryable < SimpleDelegator - extend Status::Extended + include Status::Extended - def has_action?(current_user) - can?(current_user, :update_build, subject) + def has_action? + can?(user, :update_build, subject) end def action_icon @@ -14,14 +14,16 @@ module Gitlab end def action_path - retry_namespace_project_build_path(subject.project.namespace, subject.project, subject) + retry_namespace_project_build_path(subject.project.namespace, + subject.project, + subject) end def action_method :post end - def self.matches?(build) + def self.matches?(build, user) build.retryable? end end diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index 261de9695c5..6fb51890bec 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -3,7 +3,7 @@ module Gitlab module Status module Status class Play < SimpleDelegator - extend Status::Extended + include Status::Extended def text 'stop' @@ -21,8 +21,8 @@ module Gitlab 'stop' end - def has_action?(current_user) - can?(current_user, :update_build, subject) + def has_action? + can?(user, :update_build, subject) end def action_icon @@ -30,14 +30,16 @@ module Gitlab end def action_path - play_namespace_project_build_path(subject.project.namespace, subject.project, subject) + play_namespace_project_build_path(subject.project.namespace, + subject.project, + subject) end def action_method :post end - def self.matches?(build) + def self.matches?(build, user) build.playable? && build.stops_environment? end end diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index 6b47096f811..df371363736 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -6,8 +6,11 @@ module Gitlab class Core include Gitlab::Routing.url_helpers - def initialize(subject) + attr_reader :subject, :user + + def initialize(subject, user) @subject = subject + @user = user end def icon @@ -18,10 +21,6 @@ module Gitlab raise NotImplementedError end - def title - "#{@subject.class.name.demodulize}: #{label}" - end - # Deprecation warning: this method is here because we need to maintain # backwards compatibility with legacy statuses. We often do something # like "ci-status ci-status-#{status}" to set CSS class. @@ -33,7 +32,7 @@ module Gitlab self.class.name.demodulize.downcase.underscore end - def has_details?(_user = nil) + def has_details? false end @@ -41,7 +40,7 @@ module Gitlab raise NotImplementedError end - def has_action?(_user = nil) + def has_action? false end diff --git a/lib/gitlab/ci/status/extended.rb b/lib/gitlab/ci/status/extended.rb index 93e6eff1c94..d367c9bda69 100644 --- a/lib/gitlab/ci/status/extended.rb +++ b/lib/gitlab/ci/status/extended.rb @@ -2,8 +2,12 @@ module Gitlab module Ci module Status module Extended - def matches?(_subject, _user) - raise NotImplementedError + extend ActiveSupport::Concern + + class_methods do + def matches?(_subject, _user) + raise NotImplementedError + end end end end diff --git a/lib/gitlab/ci/status/factory.rb b/lib/gitlab/ci/status/factory.rb index 944e0fdde2d..ae9ef895df4 100644 --- a/lib/gitlab/ci/status/factory.rb +++ b/lib/gitlab/ci/status/factory.rb @@ -2,7 +2,7 @@ module Gitlab module Ci module Status class Factory - def initialize(subject, user = nil) + def initialize(subject, user) @subject = subject @user = user end @@ -32,7 +32,7 @@ module Gitlab def core_status Gitlab::Ci::Status .const_get(simple_status.capitalize) - .new(@subject) + .new(@subject, @user) .extend(self.class.common_helpers) end diff --git a/lib/gitlab/ci/status/pipeline/common.rb b/lib/gitlab/ci/status/pipeline/common.rb index 5f79044a496..76bfd18bf40 100644 --- a/lib/gitlab/ci/status/pipeline/common.rb +++ b/lib/gitlab/ci/status/pipeline/common.rb @@ -3,14 +3,14 @@ module Gitlab module Status module Pipeline module Common - def has_details?(current_user) - can?(current_user, :read_pipeline, subject) + def has_details? + can?(user, :read_pipeline, subject) end def details_path - namespace_project_pipeline_path(@subject.project.namespace, - @subject.project, - @subject) + namespace_project_pipeline_path(subject.project.namespace, + subject.project, + subject) end def has_action? diff --git a/lib/gitlab/ci/status/pipeline/success_with_warnings.rb b/lib/gitlab/ci/status/pipeline/success_with_warnings.rb index 4b040d60df8..a7c98f9e909 100644 --- a/lib/gitlab/ci/status/pipeline/success_with_warnings.rb +++ b/lib/gitlab/ci/status/pipeline/success_with_warnings.rb @@ -3,7 +3,7 @@ module Gitlab module Status module Pipeline class SuccessWithWarnings < SimpleDelegator - extend Status::Extended + include Status::Extended def text 'passed' @@ -21,7 +21,7 @@ module Gitlab 'success_with_warnings' end - def self.matches?(pipeline) + def self.matches?(pipeline, user) pipeline.success? && pipeline.has_warnings? end end diff --git a/lib/gitlab/ci/status/stage/common.rb b/lib/gitlab/ci/status/stage/common.rb index e6ee2f92341..6851ceda317 100644 --- a/lib/gitlab/ci/status/stage/common.rb +++ b/lib/gitlab/ci/status/stage/common.rb @@ -3,15 +3,15 @@ module Gitlab module Status module Stage module Common - def has_details?(current_user) - can?(current_user, :read_pipeline, subject) + def has_details? + can?(user, :read_pipeline, subject) end def details_path - namespace_project_pipeline_path(@subject.project.namespace, - @subject.project, - @subject.pipeline, - anchor: @subject.name) + namespace_project_pipeline_path(subject.project.namespace, + subject.project, + subject.pipeline, + anchor: subject.name) end def has_action? -- cgit v1.2.1 From 23feb6a773a49123c3ece0ff2ed675fd294d8817 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 8 Dec 2016 14:40:21 +0100 Subject: Fix tests related to detailed statuses and permissions [ci skip] --- spec/lib/gitlab/ci/status/canceled_spec.rb | 4 +++- spec/lib/gitlab/ci/status/created_spec.rb | 8 +++----- spec/lib/gitlab/ci/status/extended_spec.rb | 2 +- spec/lib/gitlab/ci/status/factory_spec.rb | 6 ++++-- spec/lib/gitlab/ci/status/failed_spec.rb | 8 +++----- spec/lib/gitlab/ci/status/pending_spec.rb | 8 +++----- spec/lib/gitlab/ci/status/pipeline/common_spec.rb | 4 +++- spec/lib/gitlab/ci/status/pipeline/factory_spec.rb | 4 +++- spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb | 2 +- spec/lib/gitlab/ci/status/running_spec.rb | 8 +++----- spec/lib/gitlab/ci/status/skipped_spec.rb | 8 +++----- spec/lib/gitlab/ci/status/stage/common_spec.rb | 8 ++++++-- spec/lib/gitlab/ci/status/stage/factory_spec.rb | 8 ++++++-- spec/lib/gitlab/ci/status/success_spec.rb | 8 +++----- 14 files changed, 45 insertions(+), 41 deletions(-) diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb index 619ecbcba67..eaf974bb953 100644 --- a/spec/lib/gitlab/ci/status/canceled_spec.rb +++ b/spec/lib/gitlab/ci/status/canceled_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Canceled do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'canceled' } diff --git a/spec/lib/gitlab/ci/status/created_spec.rb b/spec/lib/gitlab/ci/status/created_spec.rb index 157302c65a8..2ce176a29d6 100644 --- a/spec/lib/gitlab/ci/status/created_spec.rb +++ b/spec/lib/gitlab/ci/status/created_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Created do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'created' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Created do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_created' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: created' } - end end diff --git a/spec/lib/gitlab/ci/status/extended_spec.rb b/spec/lib/gitlab/ci/status/extended_spec.rb index 120e121aae5..864121dec4b 100644 --- a/spec/lib/gitlab/ci/status/extended_spec.rb +++ b/spec/lib/gitlab/ci/status/extended_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Ci::Status::Extended do end it 'requires subclass to implement matcher' do - expect { subject.matches?(double) } + expect { subject.matches?(double, double) } .to raise_error(NotImplementedError) end end diff --git a/spec/lib/gitlab/ci/status/factory_spec.rb b/spec/lib/gitlab/ci/status/factory_spec.rb index d5bd7f7102b..f92a1c149bf 100644 --- a/spec/lib/gitlab/ci/status/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/factory_spec.rb @@ -2,15 +2,17 @@ require 'spec_helper' describe Gitlab::Ci::Status::Factory do subject do - described_class.new(object) + described_class.new(resource, user) end + let(:user) { create(:user) } + let(:status) { subject.fabricate! } context 'when object has a core status' do HasStatus::AVAILABLE_STATUSES.each do |core_status| context "when core status is #{core_status}" do - let(:object) { double(status: core_status) } + let(:resource) { double(status: core_status) } it "fabricates a core status #{core_status}" do expect(status).to be_a( diff --git a/spec/lib/gitlab/ci/status/failed_spec.rb b/spec/lib/gitlab/ci/status/failed_spec.rb index 0b3cb8168e6..9d527e6a7ef 100644 --- a/spec/lib/gitlab/ci/status/failed_spec.rb +++ b/spec/lib/gitlab/ci/status/failed_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Failed do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'failed' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Failed do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_failed' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: failed' } - end end diff --git a/spec/lib/gitlab/ci/status/pending_spec.rb b/spec/lib/gitlab/ci/status/pending_spec.rb index 57c901c1202..d03f595d3c7 100644 --- a/spec/lib/gitlab/ci/status/pending_spec.rb +++ b/spec/lib/gitlab/ci/status/pending_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pending do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'pending' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Pending do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_pending' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: pending' } - end end diff --git a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb index 21adee3f8e7..4f32ae5d809 100644 --- a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb @@ -1,11 +1,13 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::Common do + let(:user) { create(:user) } let(:pipeline) { create(:ci_pipeline) } subject do Class.new(Gitlab::Ci::Status::Core) - .new(pipeline).extend(described_class) + .new(pipeline, user) + .extend(described_class) end it 'does not have action' do diff --git a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb index d6243940f2e..c6b2582652d 100644 --- a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb @@ -1,8 +1,10 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::Factory do + let(:user) { create(:user) } + subject do - described_class.new(pipeline) + described_class.new(pipeline, user) end let(:status) do diff --git a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb index 02e526e3de2..634f80088d5 100644 --- a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::SuccessWithWarnings do subject do - described_class.new(double('status')) + described_class.new(double('status'), double('user')) end describe '#test' do diff --git a/spec/lib/gitlab/ci/status/running_spec.rb b/spec/lib/gitlab/ci/status/running_spec.rb index c023f1872cc..9f47090d396 100644 --- a/spec/lib/gitlab/ci/status/running_spec.rb +++ b/spec/lib/gitlab/ci/status/running_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Running do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'running' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Running do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_running' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: running' } - end end diff --git a/spec/lib/gitlab/ci/status/skipped_spec.rb b/spec/lib/gitlab/ci/status/skipped_spec.rb index d4f7f4b3b70..94601648a8d 100644 --- a/spec/lib/gitlab/ci/status/skipped_spec.rb +++ b/spec/lib/gitlab/ci/status/skipped_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Skipped do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'skipped' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Skipped do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_skipped' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: skipped' } - end end diff --git a/spec/lib/gitlab/ci/status/stage/common_spec.rb b/spec/lib/gitlab/ci/status/stage/common_spec.rb index f3259c6f23e..9b7e6777dc1 100644 --- a/spec/lib/gitlab/ci/status/stage/common_spec.rb +++ b/spec/lib/gitlab/ci/status/stage/common_spec.rb @@ -1,12 +1,16 @@ require 'spec_helper' describe Gitlab::Ci::Status::Stage::Common do + let(:user) { create(:user) } let(:pipeline) { create(:ci_empty_pipeline) } - let(:stage) { build(:ci_stage, pipeline: pipeline, name: 'test') } + + let(:stage) do + build(:ci_stage, pipeline: pipeline, name: 'test') + end subject do Class.new(Gitlab::Ci::Status::Core) - .new(stage).extend(described_class) + .new(stage, user).extend(described_class) end it 'does not have action' do diff --git a/spec/lib/gitlab/ci/status/stage/factory_spec.rb b/spec/lib/gitlab/ci/status/stage/factory_spec.rb index 17929665c83..5a281564415 100644 --- a/spec/lib/gitlab/ci/status/stage/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/stage/factory_spec.rb @@ -1,11 +1,15 @@ require 'spec_helper' describe Gitlab::Ci::Status::Stage::Factory do + let(:user) { create(:user) } let(:pipeline) { create(:ci_empty_pipeline) } - let(:stage) { build(:ci_stage, pipeline: pipeline, name: 'test') } + + let(:stage) do + build(:ci_stage, pipeline: pipeline, name: 'test') + end subject do - described_class.new(stage) + described_class.new(stage, user) end let(:status) do diff --git a/spec/lib/gitlab/ci/status/success_spec.rb b/spec/lib/gitlab/ci/status/success_spec.rb index 9e261a3aa5f..90f9f615e0d 100644 --- a/spec/lib/gitlab/ci/status/success_spec.rb +++ b/spec/lib/gitlab/ci/status/success_spec.rb @@ -1,7 +1,9 @@ require 'spec_helper' describe Gitlab::Ci::Status::Success do - subject { described_class.new(double('subject')) } + subject do + described_class.new(double('subject'), double('user')) + end describe '#text' do it { expect(subject.label).to eq 'passed' } @@ -14,8 +16,4 @@ describe Gitlab::Ci::Status::Success do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_success' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: passed' } - end end -- cgit v1.2.1 From f0cd73bfadbe9fa27b25473dab61d8c566292392 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 8 Dec 2016 14:51:38 +0100 Subject: Fix some detailed statuses specs related to abilities --- app/models/ability.rb | 6 ++++++ lib/gitlab/ci/status/core.rb | 1 + lib/gitlab/ci/status/stage/common.rb | 2 +- spec/lib/gitlab/ci/status/canceled_spec.rb | 4 ---- spec/lib/gitlab/ci/status/extended_spec.rb | 2 +- spec/lib/gitlab/ci/status/pipeline/common_spec.rb | 7 ++++++- spec/lib/gitlab/ci/status/pipeline/factory_spec.rb | 5 +++++ .../status/pipeline/success_with_warnings_spec.rb | 10 +++++----- spec/lib/gitlab/ci/status/stage/common_spec.rb | 23 +++++++++++++++++----- spec/lib/gitlab/ci/status/stage/factory_spec.rb | 7 ++++++- 10 files changed, 49 insertions(+), 18 deletions(-) diff --git a/app/models/ability.rb b/app/models/ability.rb index fa8f8bc3a5f..ce461caf686 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,4 +1,10 @@ class Ability + module Allowable + def can?(user, action, subject) + Ability.allowed?(user, action, subject) + end + end + class << self # Given a list of users and a project this method returns the users that can # read the given project. diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index df371363736..7e9f6e35012 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -5,6 +5,7 @@ module Gitlab # class Core include Gitlab::Routing.url_helpers + include Ability::Allowable attr_reader :subject, :user diff --git a/lib/gitlab/ci/status/stage/common.rb b/lib/gitlab/ci/status/stage/common.rb index 6851ceda317..7852f492e1d 100644 --- a/lib/gitlab/ci/status/stage/common.rb +++ b/lib/gitlab/ci/status/stage/common.rb @@ -4,7 +4,7 @@ module Gitlab module Stage module Common def has_details? - can?(user, :read_pipeline, subject) + can?(user, :read_pipeline, subject.pipeline) end def details_path diff --git a/spec/lib/gitlab/ci/status/canceled_spec.rb b/spec/lib/gitlab/ci/status/canceled_spec.rb index eaf974bb953..4639278ad45 100644 --- a/spec/lib/gitlab/ci/status/canceled_spec.rb +++ b/spec/lib/gitlab/ci/status/canceled_spec.rb @@ -16,8 +16,4 @@ describe Gitlab::Ci::Status::Canceled do describe '#icon' do it { expect(subject.icon).to eq 'icon_status_canceled' } end - - describe '#title' do - it { expect(subject.title).to eq 'Double: canceled' } - end end diff --git a/spec/lib/gitlab/ci/status/extended_spec.rb b/spec/lib/gitlab/ci/status/extended_spec.rb index 864121dec4b..c2d74ca5cde 100644 --- a/spec/lib/gitlab/ci/status/extended_spec.rb +++ b/spec/lib/gitlab/ci/status/extended_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Extended do subject do - Class.new.extend(described_class) + Class.new.include(described_class) end it 'requires subclass to implement matcher' do diff --git a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb index 4f32ae5d809..2df9d574677 100644 --- a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::Common do let(:user) { create(:user) } - let(:pipeline) { create(:ci_pipeline) } + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_pipeline, project: project) } subject do Class.new(Gitlab::Ci::Status::Core) @@ -10,6 +11,10 @@ describe Gitlab::Ci::Status::Pipeline::Common do .extend(described_class) end + before do + project.team << [user, :developer] + end + it 'does not have action' do expect(subject).not_to have_action end diff --git a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb index c6b2582652d..d4a2dc7fcc1 100644 --- a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::Factory do let(:user) { create(:user) } + let(:project) { pipeline.project } subject do described_class.new(pipeline, user) @@ -11,6 +12,10 @@ describe Gitlab::Ci::Status::Pipeline::Factory do subject.fabricate! end + before do + project.team << [user, :developer] + end + context 'when pipeline has a core status' do HasStatus::AVAILABLE_STATUSES.each do |core_status| context "when core status is #{core_status}" do diff --git a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb index 634f80088d5..7e3383c307f 100644 --- a/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/success_with_warnings_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::SuccessWithWarnings do subject do - described_class.new(double('status'), double('user')) + described_class.new(double('status')) end describe '#test' do @@ -29,13 +29,13 @@ describe Gitlab::Ci::Status::Pipeline::SuccessWithWarnings do end it 'is a correct match' do - expect(described_class.matches?(pipeline)).to eq true + expect(described_class.matches?(pipeline, double)).to eq true end end context 'when pipeline does not have warnings' do it 'does not match' do - expect(described_class.matches?(pipeline)).to eq false + expect(described_class.matches?(pipeline, double)).to eq false end end end @@ -51,13 +51,13 @@ describe Gitlab::Ci::Status::Pipeline::SuccessWithWarnings do end it 'does not match' do - expect(described_class.matches?(pipeline)).to eq false + expect(described_class.matches?(pipeline, double)).to eq false end end context 'when pipeline does not have warnings' do it 'does not match' do - expect(described_class.matches?(pipeline)).to eq false + expect(described_class.matches?(pipeline, double)).to eq false end end end diff --git a/spec/lib/gitlab/ci/status/stage/common_spec.rb b/spec/lib/gitlab/ci/status/stage/common_spec.rb index 9b7e6777dc1..8814a7614a0 100644 --- a/spec/lib/gitlab/ci/status/stage/common_spec.rb +++ b/spec/lib/gitlab/ci/status/stage/common_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Stage::Common do let(:user) { create(:user) } - let(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:stage) do build(:ci_stage, pipeline: pipeline, name: 'test') @@ -17,14 +18,26 @@ describe Gitlab::Ci::Status::Stage::Common do expect(subject).not_to have_action end - it 'has details' do - expect(subject).to have_details - end - it 'links to the pipeline details page' do expect(subject.details_path) .to include "pipelines/#{pipeline.id}" expect(subject.details_path) .to include "##{stage.name}" end + + context 'when user has permission to read pipeline' do + before do + project.team << [user, :master] + end + + it 'has details' do + expect(subject).to have_details + end + end + + context 'when user does not have permission to read pipeline' do + it 'does not have details' do + expect(subject).not_to have_details + end + end end diff --git a/spec/lib/gitlab/ci/status/stage/factory_spec.rb b/spec/lib/gitlab/ci/status/stage/factory_spec.rb index 5a281564415..6f8721d30c2 100644 --- a/spec/lib/gitlab/ci/status/stage/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/stage/factory_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' describe Gitlab::Ci::Status::Stage::Factory do let(:user) { create(:user) } - let(:pipeline) { create(:ci_empty_pipeline) } + let(:project) { create(:empty_project) } + let(:pipeline) { create(:ci_empty_pipeline, project: project) } let(:stage) do build(:ci_stage, pipeline: pipeline, name: 'test') @@ -16,6 +17,10 @@ describe Gitlab::Ci::Status::Stage::Factory do subject.fabricate! end + before do + project.team << [user, :developer] + end + context 'when stage has a core status' do HasStatus::AVAILABLE_STATUSES.each do |core_status| context "when core status is #{core_status}" do -- cgit v1.2.1 From 980009e6e85562c9ee8026878929d09905b2a0a9 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 17:52:24 +0100 Subject: Fix auto loading of constants for Ci Statuses --- app/models/ci/build.rb | 6 +++--- app/models/ci/pipeline.rb | 4 ++-- app/models/ci/stage.rb | 4 ++-- app/models/commit_status.rb | 4 ++-- app/views/ci/status/_icon_with_label.html.haml | 13 +++++++------ lib/gitlab/ci/status/build/cancelable.rb | 2 +- lib/gitlab/ci/status/build/play.rb | 2 +- lib/gitlab/ci/status/build/retryable.rb | 2 +- lib/gitlab/ci/status/build/stop.rb | 4 ++-- 9 files changed, 21 insertions(+), 20 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 73564dd2aa0..65ee327a8e5 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -100,8 +100,8 @@ module Ci end end - def detailed_status - Gitlab::Ci::Status::Build::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Build::Factory.new(self, current_user).fabricate! end def manual? @@ -156,7 +156,7 @@ module Ci end def environment_action - self.options.fetch(:environment, {}).fetch(:action, 'start') + self.options.fetch(:environment, {}).fetch(:action, 'start') if self.options end def outdated_deployment? diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index fda8228a1e9..1f33106d358 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -336,8 +336,8 @@ module Ci .select { |merge_request| merge_request.head_pipeline.try(:id) == self.id } end - def detailed_status - Gitlab::Ci::Status::Pipeline::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Pipeline::Factory.new(self, current_user).fabricate! end private diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index d2a37c0a827..be52cce20f1 100644 --- a/app/models/ci/stage.rb +++ b/app/models/ci/stage.rb @@ -22,8 +22,8 @@ module Ci @status ||= statuses.latest.status end - def detailed_status - Gitlab::Ci::Status::Stage::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Stage::Factory.new(self, current_user).fabricate! end def statuses diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index fce16174e22..6548a7dda2c 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -132,7 +132,7 @@ class CommitStatus < ActiveRecord::Base false end - def detailed_status - Gitlab::Ci::Status::Factory.new(self).fabricate! + def detailed_status(current_user) + Gitlab::Ci::Status::Factory.new(self, current_user).fabricate! end end diff --git a/app/views/ci/status/_icon_with_label.html.haml b/app/views/ci/status/_icon_with_label.html.haml index 65a74e88444..d3fe332cc78 100644 --- a/app/views/ci/status/_icon_with_label.html.haml +++ b/app/views/ci/status/_icon_with_label.html.haml @@ -1,10 +1,11 @@ -- details_path = subject.details_path if subject.has_details?(current_user) -- klass = "ci-status ci-#{subject.status}" +- detailed_status = subject.detailed_status(current_user) +- details_path = detailed_status.details_path if detailed_status.has_details? +- klass = "ci-status ci-#{detailed_status}" - if details_path = link_to details_path, class: klass do - = custom_icon(status.icon) - = status.text + = custom_icon(detailed_status.icon) + = detailed_status.text - else %span{ class: klass } - = custom_icon(status.icon) - = status.text + = custom_icon(detailed_status.icon) + = detailed_status.text diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index a8830b04715..88be0cd924b 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -1,7 +1,7 @@ module Gitlab module Ci module Status - module Status + module Build class Cancelable < SimpleDelegator include Status::Extended diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 70c08197ea1..57c7058fe84 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -1,7 +1,7 @@ module Gitlab module Ci module Status - module Status + module Build class Play < SimpleDelegator include Status::Extended diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index 3309e8808e1..69f2ad1d277 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -1,7 +1,7 @@ module Gitlab module Ci module Status - module Status + module Build class Retryable < SimpleDelegator include Status::Extended diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index 6fb51890bec..cd9bd959a7c 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -1,8 +1,8 @@ module Gitlab module Ci module Status - module Status - class Play < SimpleDelegator + module Build + class Stop < SimpleDelegator include Status::Extended def text -- cgit v1.2.1 From d1dd89356c4cd5e70f2b81ef416e52b745486293 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 18:16:23 +0100 Subject: Rename icon_with_label to icon_with_description --- app/views/admin/runners/show.html.haml | 2 +- app/views/ci/status/_icon_with_description.html.haml | 12 ++++++++++++ app/views/ci/status/_icon_with_label.html.haml | 11 ----------- app/views/projects/builds/_header.html.haml | 2 +- app/views/projects/ci/builds/_build.html.haml | 2 +- app/views/projects/ci/pipelines/_pipeline.html.haml | 2 +- .../generic_commit_statuses/_generic_commit_status.html.haml | 2 +- app/views/projects/pipelines/_info.html.haml | 2 +- app/views/projects/stage/_graph.html.haml | 4 ++-- 9 files changed, 20 insertions(+), 19 deletions(-) create mode 100644 app/views/ci/status/_icon_with_description.html.haml delete mode 100644 app/views/ci/status/_icon_with_label.html.haml diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index fa8be25ffa8..badeb11b208 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -91,7 +91,7 @@ %strong ##{build.id} %td.status - = render "ci/status/icon_with_label", subject: build + = render "ci/status/icon_with_description", subject: build %td.status - if project diff --git a/app/views/ci/status/_icon_with_description.html.haml b/app/views/ci/status/_icon_with_description.html.haml new file mode 100644 index 00000000000..34c923440d0 --- /dev/null +++ b/app/views/ci/status/_icon_with_description.html.haml @@ -0,0 +1,12 @@ +- detailed_status = subject.detailed_status(current_user) +- details_path = detailed_status.details_path if detailed_status.has_details? +- klass = "ci-status ci-#{detailed_status}" + +- if details_path + = link_to details_path, class: klass do + = custom_icon(detailed_status.icon) + = detailed_status.text +- else + %span{ class: klass } + = custom_icon(detailed_status.icon) + = detailed_status.text diff --git a/app/views/ci/status/_icon_with_label.html.haml b/app/views/ci/status/_icon_with_label.html.haml deleted file mode 100644 index d3fe332cc78..00000000000 --- a/app/views/ci/status/_icon_with_label.html.haml +++ /dev/null @@ -1,11 +0,0 @@ -- detailed_status = subject.detailed_status(current_user) -- details_path = detailed_status.details_path if detailed_status.has_details? -- klass = "ci-status ci-#{detailed_status}" -- if details_path - = link_to details_path, class: klass do - = custom_icon(detailed_status.icon) - = detailed_status.text -- else - %span{ class: klass } - = custom_icon(detailed_status.icon) - = detailed_status.text diff --git a/app/views/projects/builds/_header.html.haml b/app/views/projects/builds/_header.html.haml index 5e4e30f08d5..85d1793ecb9 100644 --- a/app/views/projects/builds/_header.html.haml +++ b/app/views/projects/builds/_header.html.haml @@ -1,6 +1,6 @@ .content-block.build-header .header-content - = render "ci/status/icon_with_label", subject: build + = render "ci/status/icon_with_description", subject: build Build %strong ##{@build.id} in pipeline diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 6b0cd3e49a0..4257bb86859 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -9,7 +9,7 @@ %tr.build.commit{class: ('retried' if retried)} %td.status - = render "ci/status/icon_with_label", subject: build + = render "ci/status/icon_with_description", subject: build %td.branch-commit - if can?(current_user, :read_build, build) diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index 84243e4306d..6dff955ea3d 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -4,7 +4,7 @@ %tr.commit %td.commit-link - = render "ci/status/icon_with_label", subject: pipeline + = render "ci/status/icon_with_description", subject: pipeline %td = link_to namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id) do diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml index 69cb1631ee8..1dd07ae1a2a 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml @@ -8,7 +8,7 @@ %tr.generic_commit_status{class: ('retried' if retried)} %td.status - = render "ci/status/icon_with_label", subject: generic_commit_status + = render "ci/status/icon_with_description", subject: generic_commit_status %td.generic_commit_status-link - if can?(current_user, :read_commit_status, generic_commit_status) && generic_commit_status.target_url diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index f7385184a2b..d05697b4ee3 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -1,6 +1,6 @@ .page-content-header .header-main-content - = render "ci/status/icon_with_label", subject: @pipeline + = render "ci/status/icon_with_description", subject: @pipeline %strong Pipeline ##{@commit.pipelines.last.id} triggered #{time_ago_with_tooltip(@commit.authored_date)} by = author_avatar(@commit, size: 24) diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index 1d8fa10db0c..745b6d143f4 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -14,9 +14,9 @@ %li.build{ class: ("playable" if is_playable) } .curve .build-content - = render "projects/#{status.to_partial_path}_pipeline", subject: status + = render 'ci/status/icon_with_name_and_action', subject: status - else %li.build .curve .dropdown.inline.build-content - = render "projects/stage/in_stage_group", name: group_name, subject: grouped_statuses + = render 'projects/stage/in_stage_group', name: group_name, subject: grouped_statuses -- cgit v1.2.1 From d9a979c84f2d91b25ccde9131f9b4b03b40c4ef7 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 18:18:30 +0100 Subject: Add action_class/action_title --- lib/gitlab/ci/status/build/cancelable.rb | 6 +++++- lib/gitlab/ci/status/build/play.rb | 8 ++++++++ lib/gitlab/ci/status/build/retryable.rb | 6 +++++- lib/gitlab/ci/status/build/stop.rb | 6 +++++- lib/gitlab/ci/status/core.rb | 7 +++++++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/ci/status/build/cancelable.rb b/lib/gitlab/ci/status/build/cancelable.rb index 88be0cd924b..a979fe7d573 100644 --- a/lib/gitlab/ci/status/build/cancelable.rb +++ b/lib/gitlab/ci/status/build/cancelable.rb @@ -10,7 +10,7 @@ module Gitlab end def action_icon - 'remove' + 'ban' end def action_path @@ -23,6 +23,10 @@ module Gitlab :post end + def action_title + 'Cancel' + end + def self.matches?(build, user) build.cancelable? end diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 57c7058fe84..e3066d40a37 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -17,10 +17,18 @@ module Gitlab can?(user, :update_build, subject) end + def action_title + 'Play' + end + def action_icon 'play' end + def action_class + 'ci-play-icon' + end + def action_path play_namespace_project_build_path(subject.project.namespace, subject.project, diff --git a/lib/gitlab/ci/status/build/retryable.rb b/lib/gitlab/ci/status/build/retryable.rb index 69f2ad1d277..8e38d6a8523 100644 --- a/lib/gitlab/ci/status/build/retryable.rb +++ b/lib/gitlab/ci/status/build/retryable.rb @@ -10,7 +10,11 @@ module Gitlab end def action_icon - 'repeat' + 'refresh' + end + + def action_title + 'Retry' end def action_path diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index cd9bd959a7c..487fd033960 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -26,7 +26,11 @@ module Gitlab end def action_icon - :play + 'stop' + end + + def action_title + 'Stop' end def action_path diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index 7e9f6e35012..dd3a824e486 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -49,6 +49,9 @@ module Gitlab raise NotImplementedError end + def action_class + end + def action_path raise NotImplementedError end @@ -56,6 +59,10 @@ module Gitlab def action_method raise NotImplementedError end + + def action_title + raise NotImplementedError + end end end end -- cgit v1.2.1 From 23f02681c036b150966ce4459410c94694167b34 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 8 Dec 2016 18:21:11 +0100 Subject: Revert some unneeded changes --- app/views/projects/stage/_graph.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/stage/_graph.html.haml b/app/views/projects/stage/_graph.html.haml index 745b6d143f4..bf8c75b6e5c 100644 --- a/app/views/projects/stage/_graph.html.haml +++ b/app/views/projects/stage/_graph.html.haml @@ -14,7 +14,7 @@ %li.build{ class: ("playable" if is_playable) } .curve .build-content - = render 'ci/status/icon_with_name_and_action', subject: status + = render "projects/#{status.to_partial_path}_pipeline", subject: status - else %li.build .curve -- cgit v1.2.1 From dc67554c08ac6cfe23809af9f086336ca9ca740e Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 12 Dec 2016 13:28:02 +0100 Subject: Improve detailed status badge partial --- app/views/admin/runners/show.html.haml | 2 +- app/views/ci/status/_badge.html.haml | 8 ++++++++ app/views/ci/status/_icon_with_description.html.haml | 12 ------------ app/views/projects/builds/_header.html.haml | 2 +- app/views/projects/ci/builds/_build.html.haml | 2 +- app/views/projects/ci/pipelines/_pipeline.html.haml | 2 +- .../generic_commit_statuses/_generic_commit_status.html.haml | 2 +- app/views/projects/pipelines/_info.html.haml | 2 +- 8 files changed, 14 insertions(+), 18 deletions(-) create mode 100644 app/views/ci/status/_badge.html.haml delete mode 100644 app/views/ci/status/_icon_with_description.html.haml diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml index badeb11b208..ca503e35623 100644 --- a/app/views/admin/runners/show.html.haml +++ b/app/views/admin/runners/show.html.haml @@ -91,7 +91,7 @@ %strong ##{build.id} %td.status - = render "ci/status/icon_with_description", subject: build + = render 'ci/status/badge', status: build.detailed_status(current_user) %td.status - if project diff --git a/app/views/ci/status/_badge.html.haml b/app/views/ci/status/_badge.html.haml new file mode 100644 index 00000000000..b1b6e9c2b05 --- /dev/null +++ b/app/views/ci/status/_badge.html.haml @@ -0,0 +1,8 @@ +- if status.has_details? + = link_to status.details_path, class: "ci-status ci-#{status}" do + = custom_icon(status.icon) + = status.text +- else + %span{ class: "ci-status ci-#{status}" } + = custom_icon(status.icon) + = detailed_status.text diff --git a/app/views/ci/status/_icon_with_description.html.haml b/app/views/ci/status/_icon_with_description.html.haml deleted file mode 100644 index 34c923440d0..00000000000 --- a/app/views/ci/status/_icon_with_description.html.haml +++ /dev/null @@ -1,12 +0,0 @@ -- detailed_status = subject.detailed_status(current_user) -- details_path = detailed_status.details_path if detailed_status.has_details? -- klass = "ci-status ci-#{detailed_status}" - -- if details_path - = link_to details_path, class: klass do - = custom_icon(detailed_status.icon) - = detailed_status.text -- else - %span{ class: klass } - = custom_icon(detailed_status.icon) - = detailed_status.text diff --git a/app/views/projects/builds/_header.html.haml b/app/views/projects/builds/_header.html.haml index 85d1793ecb9..057a720a54a 100644 --- a/app/views/projects/builds/_header.html.haml +++ b/app/views/projects/builds/_header.html.haml @@ -1,6 +1,6 @@ .content-block.build-header .header-content - = render "ci/status/icon_with_description", subject: build + = render 'ci/status/badge', status: @build.detailed_status(current_user) Build %strong ##{@build.id} in pipeline diff --git a/app/views/projects/ci/builds/_build.html.haml b/app/views/projects/ci/builds/_build.html.haml index 4257bb86859..f1cb0201032 100644 --- a/app/views/projects/ci/builds/_build.html.haml +++ b/app/views/projects/ci/builds/_build.html.haml @@ -9,7 +9,7 @@ %tr.build.commit{class: ('retried' if retried)} %td.status - = render "ci/status/icon_with_description", subject: build + = render "ci/status/badge", status: build.detailed_status(current_user) %td.branch-commit - if can?(current_user, :read_build, build) diff --git a/app/views/projects/ci/pipelines/_pipeline.html.haml b/app/views/projects/ci/pipelines/_pipeline.html.haml index 6dff955ea3d..3f05a21990f 100644 --- a/app/views/projects/ci/pipelines/_pipeline.html.haml +++ b/app/views/projects/ci/pipelines/_pipeline.html.haml @@ -4,7 +4,7 @@ %tr.commit %td.commit-link - = render "ci/status/icon_with_description", subject: pipeline + = render 'ci/status/badge', status: pipeline.detailed_status(current_user) %td = link_to namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id) do diff --git a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml index 1dd07ae1a2a..9f444f076c0 100644 --- a/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml +++ b/app/views/projects/generic_commit_statuses/_generic_commit_status.html.haml @@ -8,7 +8,7 @@ %tr.generic_commit_status{class: ('retried' if retried)} %td.status - = render "ci/status/icon_with_description", subject: generic_commit_status + = render 'ci/status/badge', status: generic_commit_status.detailed_status(current_user) %td.generic_commit_status-link - if can?(current_user, :read_commit_status, generic_commit_status) && generic_commit_status.target_url diff --git a/app/views/projects/pipelines/_info.html.haml b/app/views/projects/pipelines/_info.html.haml index d05697b4ee3..b00ba2d5307 100644 --- a/app/views/projects/pipelines/_info.html.haml +++ b/app/views/projects/pipelines/_info.html.haml @@ -1,6 +1,6 @@ .page-content-header .header-main-content - = render "ci/status/icon_with_description", subject: @pipeline + = render 'ci/status/badge', status: @pipeline.detailed_status(current_user) %strong Pipeline ##{@commit.pipelines.last.id} triggered #{time_ago_with_tooltip(@commit.authored_date)} by = author_avatar(@commit, size: 24) -- cgit v1.2.1 From d60820146f87863a1ef93ccdc678fd617029950a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 12 Dec 2016 13:28:20 +0100 Subject: Fix path to build status details in common helpers --- lib/gitlab/ci/status/build/common.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/gitlab/ci/status/build/common.rb b/lib/gitlab/ci/status/build/common.rb index 2b602f1e247..3fec2c5d4db 100644 --- a/lib/gitlab/ci/status/build/common.rb +++ b/lib/gitlab/ci/status/build/common.rb @@ -10,7 +10,7 @@ module Gitlab def details_path namespace_project_build_path(subject.project.namespace, subject.project, - subject.pipeline) + subject) end end end -- cgit v1.2.1 From ffafd0973100a53efc46011ca4415e8385e0cc07 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 12 Dec 2016 14:14:35 +0100 Subject: Fix build stop extended status CSS class --- lib/gitlab/ci/status/build/stop.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index 487fd033960..76c66fab323 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -17,10 +17,6 @@ module Gitlab 'icon_status_skipped' end - def to_s - 'stop' - end - def has_action? can?(user, :update_build, subject) end -- cgit v1.2.1 From f91e8269c18ac80d6f43fdd7b87362e5c9ccbc94 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 12 Dec 2016 14:53:05 +0100 Subject: Add tests for build play extended detailed status --- spec/factories/ci/builds.rb | 13 ++++-- spec/lib/gitlab/ci/status/build/play_spec.rb | 59 ++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 spec/lib/gitlab/ci/status/build/play_spec.rb diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index c443af09075..62466c06194 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -12,12 +12,14 @@ FactoryGirl.define do started_at 'Di 29. Okt 09:51:28 CET 2013' finished_at 'Di 29. Okt 09:53:28 CET 2013' commands 'ls -a' + options do { image: "ruby:2.1", services: ["postgres"] } end + yaml_variables do [ { key: :DB_NAME, value: 'postgres', public: true } @@ -60,15 +62,20 @@ FactoryGirl.define do end trait :teardown_environment do - options do - { environment: { action: 'stop' } } - end + environment 'staging' + options environment: { name: 'staging', + action: 'stop' } end trait :allowed_to_fail do allow_failure true end + trait :playable do + skipped + manual + end + after(:build) do |build, evaluator| build.project = build.pipeline.project end diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb new file mode 100644 index 00000000000..ae103d8993d --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/play_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Play do + let(:core_status) { double('core status') } + let(:user) { double('user') } + + subject do + described_class.new(core_status) + end + + describe '#text' do + it { expect(subject.text).to eq 'play' } + end + + describe '#label' do + it { expect(subject.label).to eq 'play' } + end + + describe '#icon' do + it 'does not override core status icon' do + expect(core_status).to receive(:icon) + + subject.icon + end + end + + describe '.matches?' do + context 'build is playable' do + context 'when build stops an environment' do + let(:build) do + create(:ci_build, :playable, :teardown_environment) + end + + it 'does not match' do + expect(described_class.matches?(build, user)) + .to be false + end + end + + context 'when build does not stop an environment' do + let(:build) { create(:ci_build, :playable) } + + it 'is a correct match' do + expect(described_class.matches?(build, user)) + .to be true + end + end + end + + context 'when build is not playable' do + let(:build) { create(:ci_build) } + + it 'does not match' do + expect(described_class.matches?(build, user)) + .to be false + end + end + end +end -- cgit v1.2.1 From ab37be2dda8188c56d6e76a23b46ccc88c42baa8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 12 Dec 2016 14:59:46 +0100 Subject: Add specs for build stop extended detailed status --- lib/gitlab/ci/status/build/stop.rb | 4 -- spec/lib/gitlab/ci/status/build/stop_spec.rb | 59 ++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 spec/lib/gitlab/ci/status/build/stop_spec.rb diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index 76c66fab323..a4966a55892 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -13,10 +13,6 @@ module Gitlab 'stop' end - def icon - 'icon_status_skipped' - end - def has_action? can?(user, :update_build, subject) end diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb new file mode 100644 index 00000000000..f2121210417 --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Stop do + let(:core_status) { double('core status') } + let(:user) { double('user') } + + subject do + described_class.new(core_status) + end + + describe '#text' do + it { expect(subject.text).to eq 'stop' } + end + + describe '#label' do + it { expect(subject.label).to eq 'stop' } + end + + describe '#icon' do + it 'does not override core status icon' do + expect(core_status).to receive(:icon) + + subject.icon + end + end + + describe '.matches?' do + context 'build is playable' do + context 'when build stops an environment' do + let(:build) do + create(:ci_build, :playable, :teardown_environment) + end + + it 'is a correct match' do + expect(described_class.matches?(build, user)) + .to be true + end + end + + context 'when build does not stop an environment' do + let(:build) { create(:ci_build, :playable) } + + it 'does not match' do + expect(described_class.matches?(build, user)) + .to be false + end + end + end + + context 'when build is not playable' do + let(:build) { create(:ci_build) } + + it 'does not match' do + expect(described_class.matches?(build, user)) + .to be false + end + end + end +end -- cgit v1.2.1 From 48ea142819f206bb005184a050e812966cd23157 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 12 Dec 2016 15:14:31 +0100 Subject: Fix pipeline specs for detailed statuses [ci skip] --- spec/models/ci/pipeline_spec.rb | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 8158e71dd55..e78ae14b737 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -442,11 +442,15 @@ describe Ci::Pipeline, models: true do end describe '#detailed_status' do + let(:user) { create(:user) } + + subject { pipeline.detailed_status(user) } + context 'when pipeline is created' do let(:pipeline) { create(:ci_pipeline, status: :created) } it 'returns detailed status for created pipeline' do - expect(pipeline.detailed_status.text).to eq 'created' + expect(subject.text).to eq 'created' end end @@ -454,7 +458,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :pending) } it 'returns detailed status for pending pipeline' do - expect(pipeline.detailed_status.text).to eq 'pending' + expect(subject.text).to eq 'pending' end end @@ -462,7 +466,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :running) } it 'returns detailed status for running pipeline' do - expect(pipeline.detailed_status.text).to eq 'running' + expect(subject.text).to eq 'running' end end @@ -470,7 +474,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :success) } it 'returns detailed status for successful pipeline' do - expect(pipeline.detailed_status.text).to eq 'passed' + expect(subject.text).to eq 'passed' end end @@ -478,7 +482,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :failed) } it 'returns detailed status for failed pipeline' do - expect(pipeline.detailed_status.text).to eq 'failed' + expect(subject.text).to eq 'failed' end end @@ -486,7 +490,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :canceled) } it 'returns detailed status for canceled pipeline' do - expect(pipeline.detailed_status.text).to eq 'canceled' + expect(subject.text).to eq 'canceled' end end @@ -494,7 +498,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :skipped) } it 'returns detailed status for skipped pipeline' do - expect(pipeline.detailed_status.text).to eq 'skipped' + expect(subject.text).to eq 'skipped' end end @@ -506,7 +510,7 @@ describe Ci::Pipeline, models: true do end it 'retruns detailed status for successful pipeline with warnings' do - expect(pipeline.detailed_status.label).to eq 'passed with warnings' + expect(subject.label).to eq 'passed with warnings' end end end -- cgit v1.2.1 From be13fc68e74cd6820a5b175365f72f12de339e57 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 12 Dec 2016 15:17:18 +0100 Subject: Fix detailed status badge for generic commit status [ci skip] --- app/views/ci/status/_badge.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/ci/status/_badge.html.haml b/app/views/ci/status/_badge.html.haml index b1b6e9c2b05..c00ddd183fb 100644 --- a/app/views/ci/status/_badge.html.haml +++ b/app/views/ci/status/_badge.html.haml @@ -5,4 +5,4 @@ - else %span{ class: "ci-status ci-#{status}" } = custom_icon(status.icon) - = detailed_status.text + = status.text -- cgit v1.2.1 From 7d36c88916717646f583a95118a47a095ab449a0 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 12 Dec 2016 15:20:21 +0100 Subject: Fix detailed status specs for pipeline stage model --- spec/models/ci/stage_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index f232761dba2..8fff38f7cda 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -68,7 +68,9 @@ describe Ci::Stage, models: true do end describe '#detailed_status' do - subject { stage.detailed_status } + let(:user) { create(:user) } + + subject { stage.detailed_status(user) } context 'when build is created' do let!(:stage_build) { create_job(:ci_build, status: :created) } -- cgit v1.2.1 From 605715067767572ff96964370d78e7b31083ddde Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 28 Nov 2016 11:34:02 +0000 Subject: Adds manual action icon and case to show it --- app/helpers/ci_status_helper.rb | 2 ++ app/views/shared/icons/_icon_status_manual.svg | 1 + 2 files changed, 3 insertions(+) create mode 100755 app/views/shared/icons/_icon_status_manual.svg diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index d9f5e01f0dc..eb2aeaa4628 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -48,6 +48,8 @@ module CiStatusHelper 'icon_status_created' when 'skipped' 'icon_status_skipped' + when 'manual' + 'icon_status_manual' else 'icon_status_canceled' end diff --git a/app/views/shared/icons/_icon_status_manual.svg b/app/views/shared/icons/_icon_status_manual.svg new file mode 100755 index 00000000000..ffb00c6f443 --- /dev/null +++ b/app/views/shared/icons/_icon_status_manual.svg @@ -0,0 +1 @@ + -- cgit v1.2.1 From 2011f8f1c2b4788df04fd76dcab816ab337e9e08 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 11:53:36 +0100 Subject: Use manual build icon in play/stop build statuses --- app/helpers/ci_status_helper.rb | 2 -- lib/gitlab/ci/status/build/play.rb | 8 ++++++-- lib/gitlab/ci/status/build/stop.rb | 8 ++++++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/helpers/ci_status_helper.rb b/app/helpers/ci_status_helper.rb index eb2aeaa4628..d9f5e01f0dc 100644 --- a/app/helpers/ci_status_helper.rb +++ b/app/helpers/ci_status_helper.rb @@ -48,8 +48,6 @@ module CiStatusHelper 'icon_status_created' when 'skipped' 'icon_status_skipped' - when 'manual' - 'icon_status_manual' else 'icon_status_canceled' end diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index e3066d40a37..974b9e9b1f9 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -6,11 +6,15 @@ module Gitlab include Status::Extended def text - 'play' + 'manual' end def label - 'play' + 'manual play action' + end + + def icon + 'icon_status_manual' end def has_action? diff --git a/lib/gitlab/ci/status/build/stop.rb b/lib/gitlab/ci/status/build/stop.rb index a4966a55892..f8ffa95cde4 100644 --- a/lib/gitlab/ci/status/build/stop.rb +++ b/lib/gitlab/ci/status/build/stop.rb @@ -6,11 +6,15 @@ module Gitlab include Status::Extended def text - 'stop' + 'manual' end def label - 'stop' + 'manual stop action' + end + + def icon + 'icon_status_manual' end def has_action? -- cgit v1.2.1 From 48d43608b8e9118033c1701c98f330aa10f9eb54 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 12:27:01 +0100 Subject: Refine build stop/play extended status specs --- lib/gitlab/ci/status/build/play.rb | 8 +++---- spec/lib/gitlab/ci/status/build/play_spec.rb | 27 ++++++++------------- spec/lib/gitlab/ci/status/build/stop_spec.rb | 35 +++++++++++++++++----------- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/lib/gitlab/ci/status/build/play.rb b/lib/gitlab/ci/status/build/play.rb index 974b9e9b1f9..5c506e6d59f 100644 --- a/lib/gitlab/ci/status/build/play.rb +++ b/lib/gitlab/ci/status/build/play.rb @@ -21,14 +21,14 @@ module Gitlab can?(user, :update_build, subject) end - def action_title - 'Play' - end - def action_icon 'play' end + def action_title + 'Play' + end + def action_class 'ci-play-icon' end diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb index ae103d8993d..180a2808a42 100644 --- a/spec/lib/gitlab/ci/status/build/play_spec.rb +++ b/spec/lib/gitlab/ci/status/build/play_spec.rb @@ -1,30 +1,26 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Play do - let(:core_status) { double('core status') } + let(:status) { double('core') } let(:user) { double('user') } - subject do - described_class.new(core_status) - end + subject { described_class.new(status) } describe '#text' do - it { expect(subject.text).to eq 'play' } + it { expect(subject.text).to eq 'manual' } end describe '#label' do - it { expect(subject.label).to eq 'play' } + it { expect(subject.label).to eq 'manual play action' } end describe '#icon' do - it 'does not override core status icon' do - expect(core_status).to receive(:icon) - - subject.icon - end + it { expect(subject.icon).to eq 'icon_status_manual' } end describe '.matches?' do + subject { described_class.matches?(build, user) } + context 'build is playable' do context 'when build stops an environment' do let(:build) do @@ -32,8 +28,7 @@ describe Gitlab::Ci::Status::Build::Play do end it 'does not match' do - expect(described_class.matches?(build, user)) - .to be false + expect(subject).to be false end end @@ -41,8 +36,7 @@ describe Gitlab::Ci::Status::Build::Play do let(:build) { create(:ci_build, :playable) } it 'is a correct match' do - expect(described_class.matches?(build, user)) - .to be true + expect(subject).to be true end end end @@ -51,8 +45,7 @@ describe Gitlab::Ci::Status::Build::Play do let(:build) { create(:ci_build) } it 'does not match' do - expect(described_class.matches?(build, user)) - .to be false + expect(subject).to be false end end end diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb index f2121210417..bc1939fb493 100644 --- a/spec/lib/gitlab/ci/status/build/stop_spec.rb +++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb @@ -1,30 +1,40 @@ require 'spec_helper' describe Gitlab::Ci::Status::Build::Stop do - let(:core_status) { double('core status') } + let(:status) { double('core status') } let(:user) { double('user') } subject do - described_class.new(core_status) + described_class.new(status) end describe '#text' do - it { expect(subject.text).to eq 'stop' } + it { expect(subject.text).to eq 'manual' } end describe '#label' do - it { expect(subject.label).to eq 'stop' } + it { expect(subject.label).to eq 'manual stop action' } end describe '#icon' do - it 'does not override core status icon' do - expect(core_status).to receive(:icon) + it { expect(subject.icon).to eq 'icon_status_manual' } + end - subject.icon - end + describe '#has_action?' do + end + + describe '#action_icon' do + end + + describe '#action_path' do + end + + describe '#action_title' do end describe '.matches?' do + subject { described_class.matches?(build, user) } + context 'build is playable' do context 'when build stops an environment' do let(:build) do @@ -32,8 +42,7 @@ describe Gitlab::Ci::Status::Build::Stop do end it 'is a correct match' do - expect(described_class.matches?(build, user)) - .to be true + expect(subject).to be true end end @@ -41,8 +50,7 @@ describe Gitlab::Ci::Status::Build::Stop do let(:build) { create(:ci_build, :playable) } it 'does not match' do - expect(described_class.matches?(build, user)) - .to be false + expect(subject).to be false end end end @@ -51,8 +59,7 @@ describe Gitlab::Ci::Status::Build::Stop do let(:build) { create(:ci_build) } it 'does not match' do - expect(described_class.matches?(build, user)) - .to be false + expect(subject).to be false end end end -- cgit v1.2.1 From 65c3bfe68a3a0599187219ba49a1a46f23b45312 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 13:13:09 +0100 Subject: Extend specs for build play/stop detailed statuses --- spec/lib/gitlab/ci/status/build/play_spec.rb | 30 ++++++++++++++++++++++++++ spec/lib/gitlab/ci/status/build/stop_spec.rb | 32 ++++++++++++++++++++++------ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb index 180a2808a42..0fa4405c604 100644 --- a/spec/lib/gitlab/ci/status/build/play_spec.rb +++ b/spec/lib/gitlab/ci/status/build/play_spec.rb @@ -18,6 +18,36 @@ describe Gitlab::Ci::Status::Build::Play do it { expect(subject.icon).to eq 'icon_status_manual' } end + describe 'action details' do + let(:user) { create(:user) } + let(:build) { create(:ci_build) } + let(:status) { Gitlab::Ci::Status::Core.new(build, user) } + + describe '#has_action?' do + context 'when user is allowed to update build' do + before { build.project.team << [user, :developer] } + + it { is_expected.to have_action } + end + + context 'when user is not allowed to update build' do + it { is_expected.not_to have_action } + end + end + + describe '#action_path' do + it { expect(subject.action_path).to include "#{build.id}/play" } + end + + describe '#action_icon' do + it { expect(subject.action_icon).to eq 'play' } + end + + describe '#action_title' do + it { expect(subject.action_title).to eq 'Play' } + end + end + describe '.matches?' do subject { described_class.matches?(build, user) } diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb index bc1939fb493..984cd1e5007 100644 --- a/spec/lib/gitlab/ci/status/build/stop_spec.rb +++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb @@ -20,16 +20,34 @@ describe Gitlab::Ci::Status::Build::Stop do it { expect(subject.icon).to eq 'icon_status_manual' } end - describe '#has_action?' do - end + describe 'action details' do + let(:user) { create(:user) } + let(:build) { create(:ci_build) } + let(:status) { Gitlab::Ci::Status::Core.new(build, user) } - describe '#action_icon' do - end + describe '#has_action?' do + context 'when user is allowed to update build' do + before { build.project.team << [user, :developer] } - describe '#action_path' do - end + it { is_expected.to have_action } + end + + context 'when user is not allowed to update build' do + it { is_expected.not_to have_action } + end + end + + describe '#action_path' do + it { expect(subject.action_path).to include "#{build.id}/play" } + end - describe '#action_title' do + describe '#action_icon' do + it { expect(subject.action_icon).to eq 'stop' } + end + + describe '#action_title' do + it { expect(subject.action_title).to eq 'Stop' } + end end describe '.matches?' do -- cgit v1.2.1 From a91a95b2327a3570db16afa1fde91c899d79d5a5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 13:22:40 +0100 Subject: Add tests for build cancelable/retryable statuses --- spec/lib/gitlab/ci/status/build/cancelable_spec.rb | 86 ++++++++++++++++++++++ spec/lib/gitlab/ci/status/build/retryable_spec.rb | 86 ++++++++++++++++++++++ 2 files changed, 172 insertions(+) create mode 100644 spec/lib/gitlab/ci/status/build/cancelable_spec.rb create mode 100644 spec/lib/gitlab/ci/status/build/retryable_spec.rb diff --git a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb new file mode 100644 index 00000000000..989328c0719 --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb @@ -0,0 +1,86 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Cancelable do + let(:status) { double('core status') } + let(:user) { double('user') } + + subject do + described_class.new(status) + end + + describe '#text' do + it 'does not override status text' do + expect(status).to receive(:text) + + subject.text + end + end + + describe '#icon' do + it 'does not override status icon' do + expect(status).to receive(:icon) + + subject.icon + end + end + + describe '#label' do + it 'does not override status label' do + expect(status).to receive(:label) + + subject.label + end + end + + describe 'action details' do + let(:user) { create(:user) } + let(:build) { create(:ci_build) } + let(:status) { Gitlab::Ci::Status::Core.new(build, user) } + + describe '#has_action?' do + context 'when user is allowed to update build' do + before { build.project.team << [user, :developer] } + + it { is_expected.to have_action } + end + + context 'when user is not allowed to update build' do + it { is_expected.not_to have_action } + end + end + + describe '#action_path' do + it { expect(subject.action_path).to include "#{build.id}/cancel" } + end + + describe '#action_icon' do + it { expect(subject.action_icon).to eq 'ban' } + end + + describe '#action_title' do + it { expect(subject.action_title).to eq 'Cancel' } + end + end + + describe '.matches?' do + subject { described_class.matches?(build, user) } + + context 'build is cancelable' do + let(:build) do + create(:ci_build, :running) + end + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when build is not cancelable' do + let(:build) { create(:ci_build, :success) } + + it 'does not match' do + expect(subject).to be false + end + end + end +end diff --git a/spec/lib/gitlab/ci/status/build/retryable_spec.rb b/spec/lib/gitlab/ci/status/build/retryable_spec.rb new file mode 100644 index 00000000000..59f6b78f99d --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/retryable_spec.rb @@ -0,0 +1,86 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Retryable do + let(:status) { double('core status') } + let(:user) { double('user') } + + subject do + described_class.new(status) + end + + describe '#text' do + it 'does not override status text' do + expect(status).to receive(:text) + + subject.text + end + end + + describe '#icon' do + it 'does not override status icon' do + expect(status).to receive(:icon) + + subject.icon + end + end + + describe '#label' do + it 'does not override status label' do + expect(status).to receive(:label) + + subject.label + end + end + + describe 'action details' do + let(:user) { create(:user) } + let(:build) { create(:ci_build) } + let(:status) { Gitlab::Ci::Status::Core.new(build, user) } + + describe '#has_action?' do + context 'when user is allowed to update build' do + before { build.project.team << [user, :developer] } + + it { is_expected.to have_action } + end + + context 'when user is not allowed to update build' do + it { is_expected.not_to have_action } + end + end + + describe '#action_path' do + it { expect(subject.action_path).to include "#{build.id}/retry" } + end + + describe '#action_icon' do + it { expect(subject.action_icon).to eq 'refresh' } + end + + describe '#action_title' do + it { expect(subject.action_title).to eq 'Retry' } + end + end + + describe '.matches?' do + subject { described_class.matches?(build, user) } + + context 'build is retryable' do + let(:build) do + create(:ci_build, :success) + end + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when build is not retryable' do + let(:build) { create(:ci_build, :running) } + + it 'does not match' do + expect(subject).to be false + end + end + end +end -- cgit v1.2.1 From 5f590a71fd26b637501f8751bc6f9adff4d9c8db Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 13:24:25 +0100 Subject: Improve readability in methods for detailed status --- app/models/ci/build.rb | 4 +++- app/models/ci/pipeline.rb | 4 +++- app/models/ci/stage.rb | 4 +++- app/models/commit_status.rb | 4 +++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 65ee327a8e5..63a4a075f8e 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -101,7 +101,9 @@ module Ci end def detailed_status(current_user) - Gitlab::Ci::Status::Build::Factory.new(self, current_user).fabricate! + Gitlab::Ci::Status::Build::Factory + .new(self, current_user) + .fabricate! end def manual? diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 1f33106d358..54f73171fd4 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -337,7 +337,9 @@ module Ci end def detailed_status(current_user) - Gitlab::Ci::Status::Pipeline::Factory.new(self, current_user).fabricate! + Gitlab::Ci::Status::Pipeline::Factory + .new(self, current_user) + .fabricate! end private diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index be52cce20f1..7ef59445d77 100644 --- a/app/models/ci/stage.rb +++ b/app/models/ci/stage.rb @@ -23,7 +23,9 @@ module Ci end def detailed_status(current_user) - Gitlab::Ci::Status::Stage::Factory.new(self, current_user).fabricate! + Gitlab::Ci::Status::Stage::Factory + .new(self, current_user) + .fabricate! end def statuses diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 6548a7dda2c..31cd381dcd2 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -133,6 +133,8 @@ class CommitStatus < ActiveRecord::Base end def detailed_status(current_user) - Gitlab::Ci::Status::Factory.new(self, current_user).fabricate! + Gitlab::Ci::Status::Factory + .new(self, current_user) + .fabricate! end end -- cgit v1.2.1 From a9ec4ec07e64d5f823c30d9bcc4c4bb1be7f2d75 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 13:57:18 +0100 Subject: Make build retryable only if complete and executed --- app/models/ci/build.rb | 3 ++- spec/models/build_spec.rb | 38 ++++++++++++++++++++++++++++++-------- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 63a4a075f8e..89b0cae25ed 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -134,7 +134,8 @@ module Ci end def retryable? - project.builds_enabled? && commands.present? && complete? + project.builds_enabled? && commands.present? && + (success? || failed?) end def retried? diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index d4970e38f7c..2b678355ee5 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -900,20 +900,42 @@ describe Ci::Build, models: true do end describe '#retryable?' do - context 'when build is running' do - before do - build.run! + subject { build } + + context 'when build is retryable' do + context 'when build is successful' do + before do + build.success! + end + + it { is_expected.to be_retryable } end - it { expect(build).not_to be_retryable } + context 'when build is failed' do + before do + build.drop! + end + + it { is_expected.to be_retryable } + end end - context 'when build is finished' do - before do - build.success! + context 'when build is not retryable' do + context 'when build is running' do + before do + build.run! + end + + it { is_expected.not_to be_retryable } end - it { expect(build).to be_retryable } + context 'when build is skipped' do + before do + build.skip! + end + + it { is_expected.not_to be_retryable } + end end end -- cgit v1.2.1 From f4513d5c1981922e71a00b9e4b135605eb674c6f Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 14:01:17 +0100 Subject: Make it possible to retry build that was canceled --- app/models/ci/build.rb | 2 +- spec/models/build_spec.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 89b0cae25ed..e7cf606a7ae 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -135,7 +135,7 @@ module Ci def retryable? project.builds_enabled? && commands.present? && - (success? || failed?) + (success? || failed? || canceled?) end def retried? diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index 2b678355ee5..e9b4cac5fef 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -918,6 +918,14 @@ describe Ci::Build, models: true do it { is_expected.to be_retryable } end + + context 'when build is canceled' do + before do + build.cancel! + end + + it { is_expected.to be_retryable } + end end context 'when build is not retryable' do -- cgit v1.2.1 From d7a774320e9e79bebb57dbbb927ff6af6d09e0e2 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 14:07:51 +0100 Subject: Add tests for detailed build statuses factory --- spec/lib/gitlab/ci/status/build/factory_spec.rb | 141 ++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 spec/lib/gitlab/ci/status/build/factory_spec.rb diff --git a/spec/lib/gitlab/ci/status/build/factory_spec.rb b/spec/lib/gitlab/ci/status/build/factory_spec.rb new file mode 100644 index 00000000000..dccb29b5ef6 --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/factory_spec.rb @@ -0,0 +1,141 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Factory do + let(:user) { create(:user) } + let(:project) { build.project } + + subject { described_class.new(build, user) } + let(:status) { subject.fabricate! } + + before { project.team << [user, :developer] } + + context 'when build is successful' do + let(:build) { create(:ci_build, :success) } + + it 'fabricates a retryable build status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Retryable + end + + it 'fabricates status with correct details' do + expect(status.text).to eq 'passed' + expect(status.icon).to eq 'icon_status_success' + expect(status.label).to eq 'passed' + expect(status).to have_details + expect(status).to have_action + end + end + + context 'when build is failed' do + let(:build) { create(:ci_build, :failed) } + + it 'fabricates a retryable build status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Retryable + end + + it 'fabricates status with correct details' do + expect(status.text).to eq 'failed' + expect(status.icon).to eq 'icon_status_failed' + expect(status.label).to eq 'failed' + expect(status).to have_details + expect(status).to have_action + end + end + + context 'when build is a canceled' do + let(:build) { create(:ci_build, :canceled) } + + it 'fabricates a retryable build status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Retryable + end + + it 'fabricates status with correct details' do + expect(status.text).to eq 'canceled' + expect(status.icon).to eq 'icon_status_canceled' + expect(status.label).to eq 'canceled' + expect(status).to have_details + expect(status).to have_action + end + end + + context 'when build is running' do + let(:build) { create(:ci_build, :running) } + + it 'fabricates a canceable build status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Cancelable + end + + it 'fabricates status with correct details' do + expect(status.text).to eq 'running' + expect(status.icon).to eq 'icon_status_running' + expect(status.label).to eq 'running' + expect(status).to have_details + expect(status).to have_action + end + end + + context 'when build is pending' do + let(:build) { create(:ci_build, :pending) } + + it 'fabricates a cancelable build status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Cancelable + end + + it 'fabricates status with correct details' do + expect(status.text).to eq 'pending' + expect(status.icon).to eq 'icon_status_pending' + expect(status.label).to eq 'pending' + expect(status).to have_details + expect(status).to have_action + end + end + + context 'when build is skipped' do + let(:build) { create(:ci_build, :skipped) } + + it 'fabricates a core skipped status' do + expect(status).to be_a Gitlab::Ci::Status::Skipped + end + + it 'fabricates status with correct details' do + expect(status.text).to eq 'skipped' + expect(status.icon).to eq 'icon_status_skipped' + expect(status.label).to eq 'skipped' + expect(status).to have_details + expect(status).not_to have_action + end + end + + context 'when build is a manual action' do + context 'when build is a play action' do + let(:build) { create(:ci_build, :playable) } + + it 'fabricates a core skipped status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Play + end + + it 'fabricates status with correct details' do + expect(status.text).to eq 'manual' + expect(status.icon).to eq 'icon_status_manual' + expect(status.label).to eq 'manual play action' + expect(status).to have_details + expect(status).to have_action + end + end + + context 'when build is an environment stop action' do + let(:build) { create(:ci_build, :playable, :teardown_environment) } + + it 'fabricates a core skipped status' do + expect(status).to be_a Gitlab::Ci::Status::Build::Stop + end + + it 'fabricates status with correct details' do + expect(status.text).to eq 'manual' + expect(status.icon).to eq 'icon_status_manual' + expect(status.label).to eq 'manual stop action' + expect(status).to have_details + expect(status).to have_action + end + end + end +end -- cgit v1.2.1 From 7f0ecf3a97a20a0b274ebfd46e762afad05870fc Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 14:18:34 +0100 Subject: Add missing tests for build `cancelable?` method --- spec/models/build_spec.rb | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index e9b4cac5fef..ac596ce4a91 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -899,6 +899,42 @@ describe Ci::Build, models: true do end end + describe '#cancelable?' do + subject { build } + + context 'when build is cancelable' do + context 'when build is pending' do + it { is_expected.to be_cancelable } + end + + context 'when build is running' do + before do + build.run! + end + + it { is_expected.to be_cancelable } + end + end + + context 'when build is not cancelable' do + context 'when build is successful' do + before do + build.success! + end + + it { is_expected.not_to be_cancelable } + end + + context 'when build is failed' do + before do + build.drop! + end + + it { is_expected.not_to be_cancelable } + end + end + end + describe '#retryable?' do subject { build } -- cgit v1.2.1 From 8f743edee14374a6d39a3cf5aa0fc884a0d536b8 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 14:26:44 +0100 Subject: Add tests for common build detailed status helpers --- spec/lib/gitlab/ci/status/build/common_spec.rb | 37 ++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 spec/lib/gitlab/ci/status/build/common_spec.rb diff --git a/spec/lib/gitlab/ci/status/build/common_spec.rb b/spec/lib/gitlab/ci/status/build/common_spec.rb new file mode 100644 index 00000000000..40b96b1807b --- /dev/null +++ b/spec/lib/gitlab/ci/status/build/common_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Build::Common do + let(:user) { create(:user) } + let(:build) { create(:ci_build) } + let(:project) { build.project } + + subject do + Gitlab::Ci::Status::Core + .new(build, user) + .extend(described_class) + end + + describe '#has_action?' do + it { is_expected.not_to have_action } + end + + describe '#has_details?' do + context 'when user has access to read build' do + before { project.team << [user, :developer] } + + it { is_expected.to have_details } + end + + context 'when user does not have access to read build' do + before { project.update(public_builds: false) } + + it { is_expected.not_to have_details } + end + end + + describe '#details_path' do + it 'links to the build details page' do + expect(subject.details_path).to include "builds/#{build.id}" + end + end +end -- cgit v1.2.1 From 24dd70d37834e55a7ead23ae14125e5e12f64d8b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 14:29:48 +0100 Subject: Extend tests for pipeline detailed status helpers --- spec/lib/gitlab/ci/status/pipeline/common_spec.rb | 30 ++++++++++++++--------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb index 2df9d574677..d665674bf70 100644 --- a/spec/lib/gitlab/ci/status/pipeline/common_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/common_spec.rb @@ -2,29 +2,35 @@ require 'spec_helper' describe Gitlab::Ci::Status::Pipeline::Common do let(:user) { create(:user) } - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :private) } let(:pipeline) { create(:ci_pipeline, project: project) } subject do - Class.new(Gitlab::Ci::Status::Core) + Gitlab::Ci::Status::Core .new(pipeline, user) .extend(described_class) end - before do - project.team << [user, :developer] + describe '#has_action?' do + it { is_expected.not_to have_action } end - it 'does not have action' do - expect(subject).not_to have_action - end + describe '#has_details?' do + context 'when user has access to read pipeline' do + before { project.team << [user, :developer] } + + it { is_expected.to have_details } + end - it 'has details' do - expect(subject).to have_details + context 'when user does not have access to read pipeline' do + it { is_expected.not_to have_details } + end end - it 'links to the pipeline details page' do - expect(subject.details_path) - .to include "pipelines/#{pipeline.id}" + describe '#details_path' do + it 'links to the pipeline details page' do + expect(subject.details_path) + .to include "pipelines/#{pipeline.id}" + end end end -- cgit v1.2.1 From 00970606d78486a8b6672659e9ea28521a102e44 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 14:44:43 +0100 Subject: Extract abilities checking module from ability model --- app/models/ability.rb | 6 ------ lib/gitlab/allowable.rb | 8 ++++++++ lib/gitlab/ci/status/core.rb | 2 +- spec/lib/gitlab/allowable_spec.rb | 27 +++++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 7 deletions(-) create mode 100644 lib/gitlab/allowable.rb create mode 100644 spec/lib/gitlab/allowable_spec.rb diff --git a/app/models/ability.rb b/app/models/ability.rb index ce461caf686..fa8f8bc3a5f 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,10 +1,4 @@ class Ability - module Allowable - def can?(user, action, subject) - Ability.allowed?(user, action, subject) - end - end - class << self # Given a list of users and a project this method returns the users that can # read the given project. diff --git a/lib/gitlab/allowable.rb b/lib/gitlab/allowable.rb new file mode 100644 index 00000000000..3cc218f9db4 --- /dev/null +++ b/lib/gitlab/allowable.rb @@ -0,0 +1,8 @@ +module Gitlab + module Allowable + def can?(user, action, subject) + Ability.allowed?(user, action, subject) + end + end +end + diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index dd3a824e486..12c0ca1d6d5 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -5,7 +5,7 @@ module Gitlab # class Core include Gitlab::Routing.url_helpers - include Ability::Allowable + include Gitlab::Allowable attr_reader :subject, :user diff --git a/spec/lib/gitlab/allowable_spec.rb b/spec/lib/gitlab/allowable_spec.rb new file mode 100644 index 00000000000..87733d53e92 --- /dev/null +++ b/spec/lib/gitlab/allowable_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe Gitlab::Allowable do + subject do + Class.new.include(described_class).new + end + + describe '#can?' do + let(:user) { create(:user) } + + context 'when user is allowed to do something' do + let(:project) { create(:empty_project, :public) } + + it 'reports correct ability to perform action' do + expect(subject.can?(user, :read_project, project)).to be true + end + end + + context 'when user is not allowed to do something' do + let(:project) { create(:empty_project, :private) } + + it 'reports correct ability to perform action' do + expect(subject.can?(user, :read_project, project)).to be false + end + end + end +end -- cgit v1.2.1 From 84290a452dcdd2271337e2301b846c9498b74c86 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 14:51:23 +0100 Subject: Make it possible to mix `Gitlab::Routing` in --- lib/gitlab/ci/status/core.rb | 2 +- lib/gitlab/routing.rb | 6 ++++++ spec/lib/gitlab/routing_spec.rb | 23 +++++++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 spec/lib/gitlab/routing_spec.rb diff --git a/lib/gitlab/ci/status/core.rb b/lib/gitlab/ci/status/core.rb index 12c0ca1d6d5..46fef8262c1 100644 --- a/lib/gitlab/ci/status/core.rb +++ b/lib/gitlab/ci/status/core.rb @@ -4,7 +4,7 @@ module Gitlab # Base abstract class fore core status # class Core - include Gitlab::Routing.url_helpers + include Gitlab::Routing include Gitlab::Allowable attr_reader :subject, :user diff --git a/lib/gitlab/routing.rb b/lib/gitlab/routing.rb index 5132177de51..632e2d87500 100644 --- a/lib/gitlab/routing.rb +++ b/lib/gitlab/routing.rb @@ -1,5 +1,11 @@ module Gitlab module Routing + extend ActiveSupport::Concern + + included do + include Gitlab::Routing.url_helpers + end + # Returns the URL helpers Module. # # This method caches the output as Rails' "url_helpers" method creates an diff --git a/spec/lib/gitlab/routing_spec.rb b/spec/lib/gitlab/routing_spec.rb new file mode 100644 index 00000000000..01d5acfc15b --- /dev/null +++ b/spec/lib/gitlab/routing_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe Gitlab::Routing do + context 'when module is included' do + subject do + Class.new.include(described_class).new + end + + it 'makes it possible to access url helpers' do + expect(subject).to respond_to(:namespace_project_path) + end + end + + context 'when module is not included' do + subject do + Class.new.include(described_class.url_helpers).new + end + + it 'exposes url helpers module through a method' do + expect(subject).to respond_to(:namespace_project_path) + end + end +end -- cgit v1.2.1 From f62e6081fd114641e41ffa1614d16c30a742cbaa Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 13 Dec 2016 14:58:56 +0100 Subject: Update manual build icon SVG --- app/views/shared/icons/_icon_status_manual.svg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/shared/icons/_icon_status_manual.svg b/app/views/shared/icons/_icon_status_manual.svg index ffb00c6f443..c98839f51a9 100755 --- a/app/views/shared/icons/_icon_status_manual.svg +++ b/app/views/shared/icons/_icon_status_manual.svg @@ -1 +1 @@ - + -- cgit v1.2.1 From 309580bd41a178a02894d399e3cd45659a8ba7a7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 14 Dec 2016 08:36:03 +0100 Subject: Remove trailing blank line from Allowable module --- lib/gitlab/allowable.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/gitlab/allowable.rb b/lib/gitlab/allowable.rb index 3cc218f9db4..f48abcc86d5 100644 --- a/lib/gitlab/allowable.rb +++ b/lib/gitlab/allowable.rb @@ -5,4 +5,3 @@ module Gitlab end end end - -- cgit v1.2.1 From ac115a9e1f106863112ca9daf463a8f5b8db7d0a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 14 Dec 2016 10:21:16 +0100 Subject: Add some missing tests for detailed status methods --- spec/models/build_spec.rb | 9 +++++++++ spec/models/commit_status_spec.rb | 9 +++++++++ spec/models/generic_commit_status_spec.rb | 16 ++++++++++++++-- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index ac596ce4a91..7f39aff7639 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -1246,4 +1246,13 @@ describe Ci::Build, models: true do it { is_expected.to eq('review/master') } end end + + describe '#detailed_status' do + let(:user) { create(:user) } + + it 'returns a detailed status' do + expect(build.detailed_status(user)) + .to be_a Gitlab::Ci::Status::Build::Cancelable + end + end end diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 1ec08c2a9d0..701f3323c0f 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -234,4 +234,13 @@ describe CommitStatus, models: true do end end end + + describe '#detailed_status' do + let(:user) { create(:user) } + + it 'returns a detailed status' do + expect(commit_status.detailed_status(user)) + .to be_a Gitlab::Ci::Status::Success + end + end end diff --git a/spec/models/generic_commit_status_spec.rb b/spec/models/generic_commit_status_spec.rb index 615cfe3142b..6004bfdb7b7 100644 --- a/spec/models/generic_commit_status_spec.rb +++ b/spec/models/generic_commit_status_spec.rb @@ -1,8 +1,11 @@ require 'spec_helper' describe GenericCommitStatus, models: true do - let(:pipeline) { FactoryGirl.create :ci_pipeline } - let(:generic_commit_status) { FactoryGirl.create :generic_commit_status, pipeline: pipeline } + let(:pipeline) { create(:ci_pipeline) } + + let(:generic_commit_status) do + create(:generic_commit_status, pipeline: pipeline) + end describe '#context' do subject { generic_commit_status.context } @@ -17,6 +20,15 @@ describe GenericCommitStatus, models: true do it { is_expected.to eq([:external]) } end + describe '#detailed_status' do + let(:user) { create(:user) } + + it 'returns detailed status object' do + expect(generic_commit_status.detailed_status(user)) + .to be_a Gitlab::Ci::Status::Success + end + end + describe 'set_default_values' do before do generic_commit_status.context = nil -- cgit v1.2.1 From 6757aaa120c375f15e09f4c428286e5dc1d95f84 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Wed, 14 Dec 2016 11:36:19 +0100 Subject: Improve build status specs contexts descriptions --- app/views/ci/status/_badge.html.haml | 2 ++ spec/lib/gitlab/ci/status/build/cancelable_spec.rb | 2 +- spec/lib/gitlab/ci/status/build/play_spec.rb | 2 +- spec/lib/gitlab/ci/status/build/retryable_spec.rb | 2 +- spec/lib/gitlab/ci/status/build/stop_spec.rb | 2 +- 5 files changed, 6 insertions(+), 4 deletions(-) diff --git a/app/views/ci/status/_badge.html.haml b/app/views/ci/status/_badge.html.haml index c00ddd183fb..f2135af2686 100644 --- a/app/views/ci/status/_badge.html.haml +++ b/app/views/ci/status/_badge.html.haml @@ -1,3 +1,5 @@ +- status = local_assigns.fetch(:status) + - if status.has_details? = link_to status.details_path, class: "ci-status ci-#{status}" do = custom_icon(status.icon) diff --git a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb index 989328c0719..9376bce17a1 100644 --- a/spec/lib/gitlab/ci/status/build/cancelable_spec.rb +++ b/spec/lib/gitlab/ci/status/build/cancelable_spec.rb @@ -65,7 +65,7 @@ describe Gitlab::Ci::Status::Build::Cancelable do describe '.matches?' do subject { described_class.matches?(build, user) } - context 'build is cancelable' do + context 'when build is cancelable' do let(:build) do create(:ci_build, :running) end diff --git a/spec/lib/gitlab/ci/status/build/play_spec.rb b/spec/lib/gitlab/ci/status/build/play_spec.rb index 0fa4405c604..4ddf04a8e11 100644 --- a/spec/lib/gitlab/ci/status/build/play_spec.rb +++ b/spec/lib/gitlab/ci/status/build/play_spec.rb @@ -51,7 +51,7 @@ describe Gitlab::Ci::Status::Build::Play do describe '.matches?' do subject { described_class.matches?(build, user) } - context 'build is playable' do + context 'when build is playable' do context 'when build stops an environment' do let(:build) do create(:ci_build, :playable, :teardown_environment) diff --git a/spec/lib/gitlab/ci/status/build/retryable_spec.rb b/spec/lib/gitlab/ci/status/build/retryable_spec.rb index 59f6b78f99d..d61e5bbaa6b 100644 --- a/spec/lib/gitlab/ci/status/build/retryable_spec.rb +++ b/spec/lib/gitlab/ci/status/build/retryable_spec.rb @@ -65,7 +65,7 @@ describe Gitlab::Ci::Status::Build::Retryable do describe '.matches?' do subject { described_class.matches?(build, user) } - context 'build is retryable' do + context 'when build is retryable' do let(:build) do create(:ci_build, :success) end diff --git a/spec/lib/gitlab/ci/status/build/stop_spec.rb b/spec/lib/gitlab/ci/status/build/stop_spec.rb index 984cd1e5007..59a85b55f90 100644 --- a/spec/lib/gitlab/ci/status/build/stop_spec.rb +++ b/spec/lib/gitlab/ci/status/build/stop_spec.rb @@ -53,7 +53,7 @@ describe Gitlab::Ci::Status::Build::Stop do describe '.matches?' do subject { described_class.matches?(build, user) } - context 'build is playable' do + context 'when build is playable' do context 'when build stops an environment' do let(:build) do create(:ci_build, :playable, :teardown_environment) -- cgit v1.2.1