diff options
6 files changed, 64 insertions, 6 deletions
diff --git a/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js b/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js index cd67ba5fab8..1074ce0e744 100644 --- a/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js +++ b/app/assets/javascripts/cycle_analytics/cycle_analytics_bundle.js @@ -58,10 +58,16 @@ export default () => { service: this.createCycleAnalyticsService(cycleAnalyticsEl.dataset.requestPath), }; }, + defaultNumberOfSummaryItems: 3, computed: { currentStage() { return this.store.currentActiveStage(); }, + summaryTableColumnClass() { + return this.state.summary.length === this.$options.defaultNumberOfSummaryItems + ? 'col-sm-3' + : 'col-sm-4'; + }, }, created() { // Conditional check placed here to prevent this method from being called on the diff --git a/app/views/projects/cycle_analytics/show.html.haml b/app/views/projects/cycle_analytics/show.html.haml index 7fedd1ab785..1691af9dfdd 100644 --- a/app/views/projects/cycle_analytics/show.html.haml +++ b/app/views/projects/cycle_analytics/show.html.haml @@ -13,10 +13,10 @@ .content-block .container-fluid .row - .col-sm-3.col-12.column{ "v-for" => "item in state.summary" } + .col-12.column{ "v-for" => "item in state.summary", ":class" => "summaryTableColumnClass" } %h3.header {{ item.value }} %p.text {{ item.title }} - .col-sm-3.col-12.column + .col-12.column{ ":class" => "summaryTableColumnClass" } .dropdown.inline.js-ca-dropdown %button.dropdown-menu-toggle{ "data-toggle" => "dropdown", :type => "button" } %span.dropdown-label {{ n__('Last %d day', 'Last %d days', 30) }} diff --git a/changelogs/unreleased/security-ag-cycle-analytics-guest-permissions.yml b/changelogs/unreleased/security-ag-cycle-analytics-guest-permissions.yml new file mode 100644 index 00000000000..c7a3b8923cd --- /dev/null +++ b/changelogs/unreleased/security-ag-cycle-analytics-guest-permissions.yml @@ -0,0 +1,5 @@ +--- +title: Hide commit counts from guest users in Cycle Analytics. +merge_request: +author: +type: security diff --git a/lib/gitlab/cycle_analytics/stage_summary.rb b/lib/gitlab/cycle_analytics/stage_summary.rb index ea440c441b7..9c75d4bb455 100644 --- a/lib/gitlab/cycle_analytics/stage_summary.rb +++ b/lib/gitlab/cycle_analytics/stage_summary.rb @@ -11,13 +11,29 @@ module Gitlab end def data - [serialize(Summary::Issue.new(project: @project, from: @from, to: @to, current_user: @current_user)), - serialize(Summary::Commit.new(project: @project, from: @from, to: @to)), - serialize(Summary::Deploy.new(project: @project, from: @from, to: @to))] + summary = [issue_stats] + summary << commit_stats if user_has_sufficient_access? + summary << deploy_stats end private + def issue_stats + serialize(Summary::Issue.new(project: @project, from: @from, to: @to, current_user: @current_user)) + end + + def commit_stats + serialize(Summary::Commit.new(project: @project, from: @from, to: @to)) + end + + def deploy_stats + serialize(Summary::Deploy.new(project: @project, from: @from, to: @to)) + end + + def user_has_sufficient_access? + @project.team.member?(@current_user, Gitlab::Access::REPORTER) + end + def serialize(summary_object) AnalyticsSummarySerializer.new.represent(summary_object) end diff --git a/spec/features/cycle_analytics_spec.rb b/spec/features/cycle_analytics_spec.rb index 07f0864fb3b..df8d5124f36 100644 --- a/spec/features/cycle_analytics_spec.rb +++ b/spec/features/cycle_analytics_spec.rb @@ -108,6 +108,10 @@ describe 'Cycle Analytics', :js do wait_for_requests end + it 'does not show the commit stats' do + expect(page).to have_no_selector(:xpath, commits_counter_selector) + end + it 'needs permissions to see restricted stages' do expect(find('.stage-events')).to have_content(issue.title) @@ -123,8 +127,12 @@ describe 'Cycle Analytics', :js do find(:xpath, "//p[contains(text(),'New Issue')]/preceding-sibling::h3") end + def commits_counter_selector + "//p[contains(text(),'Commits')]/preceding-sibling::h3" + end + def commits_counter - find(:xpath, "//p[contains(text(),'Commits')]/preceding-sibling::h3") + find(:xpath, commits_counter_selector) end def deploys_counter diff --git a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb index 8f9dac6d281..94edef20296 100644 --- a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb @@ -6,6 +6,11 @@ describe Gitlab::CycleAnalytics::StageSummary do let(:project) { create(:project, :repository) } let(:options) { { from: 1.day.ago, current_user: user } } let(:user) { create(:user, :admin) } + + before do + project.add_maintainer(user) + end + let(:stage_summary) { described_class.new(project, options).data } describe "#new_issues" do @@ -86,6 +91,24 @@ describe Gitlab::CycleAnalytics::StageSummary do expect(subject).to eq(2) end end + + context 'when a guest user is signed in' do + let(:guest_user) { create(:user) } + + before do + project.add_guest(guest_user) + options.merge!({ current_user: guest_user }) + end + + it 'does not include commit stats' do + data = described_class.new(project, options).data + expect(includes_commits?(data)).to be_falsy + end + + def includes_commits?(data) + data.any? { |h| h["title"] == 'Commits' } + end + end end describe "#deploys" do |