Updates based on MR feedback

app/controllers/groups/analytics_controller.rb

 class Groups::AnalyticsController < Groups::ApplicationController
+  include LicenseHelper
+
   before_action :group
+  before_action :check_contribution_analytics_available! && !LicenseHelper.show_promotions?
 
   layout 'group'
 

app/helpers/license_helper.rb

@@ -86,7 +86,7 @@ module LicenseHelper
     end
   end
 
-  def show_project_feature_promotion?(project_feature, callout_id)
+  def show_project_feature_promotion?(project_feature, callout_id = '')
     !@project.feature_available?(project_feature) && show_promotions? && show_callout?(callout_id)
   end
 

app/views/layouts/nav/_new_group_sidebar.html.haml

@@ -25,7 +25,7 @@
               %span
                 Activity
 
-          - if @group.feature_available?(:contribution_analytics)
+          - if @group.feature_available?(:contribution_analytics) || show_promotions?
             = nav_link(path: 'analytics#show') do
               = link_to group_analytics_path(@group), title: 'Contribution Analytics', data: {placement: 'right'} do
                 %span

app/views/projects/audit_events/index.html.haml

@@ -2,7 +2,7 @@
 
 = render "projects/settings/head"
 
-- unless @project.feature_available?(:audit_events) && show_promotions?
+- if show_project_feature_promotion?(:audit_events)
   = render 'shared/promotions/promote_audit_events'
 
 %h3.page-title Project Audit Events

app/views/projects/edit.html.haml

@@ -163,7 +163,7 @@
       %p
         Customize your merge request restrictions.
     .settings-content.no-animate{ class: ('expanded' if expanded) }
-      - if show_project_feature_promotion?(:fast_forward_merge, 'promote_mr_approval_dismissed')
+      - if show_project_feature_promotion?(:merge_request_approvers, 'promote_mr_approval_dismissed')
         = render 'shared/promotions/promote_mr_approval'
       = form_for [@project.namespace.becomes(Namespace), @project], remote: true, html: { multipart: true, class: "merge-request-settings-form" }, authenticity_token: true do |f|
         = render 'merge_request_settings', form: f

app/views/projects/settings/repository/show.html.haml

@@ -26,5 +26,4 @@
 
 = render @deploy_keys
 
-- if show_promotions? && show_callout?('promote_repository_features_dismissed')
-  = render 'shared/promotions/promote_repository_features'
+= render 'shared/promotions/promote_repository_features'

ee/app/views/projects/settings/ee/_nav.html.haml

-= nav_link(controller: :audit_events) do
-  = link_to project_audit_events_path(@project), title: "Audit Events" do
-    %span
-      Audit Events
+- if @project.feature_available?(:audit_events) || show_promotions?
+  = nav_link(controller: :audit_events) do
+    = link_to project_audit_events_path(@project), title: "Audit Events" do
+      %span
+        Audit Events

ee/app/views/shared/promotions/_promote_burndown_charts.html.haml

@@ -8,14 +8,14 @@
       %h4
         - if current_application_settings.should_check_namespace_plan?
           - if @project.group ? can?(current_user, :admin_group, @project.group) : @project.owner == current_user
-            Upgrade your plan to improve milestone with Burndown Charts.
+            Upgrade your plan to improve milestones with Burndown Charts.
           - elsif @project.group
-            Upgrade { @project.group.name }'s plan to improve milestone with Burndown Charts.
+            Upgrade the plan for #{ @project.group.name } to improve milestones with Burndown Charts.
           - else
             - owner = @project.namespace.owner
-            Upgrade { @project.group.name }'s plan to improve milestone with Burndown Charts.
+            Upgrade #{ owner.name }'s plan to improve milestones with Burndown Charts.
         - else
-          Improve milestone with Burndown Charts.
+          Improve milestones with Burndown Charts.
       %p
         Burndown Charts are visual representations of the progress of completing a milestone. At a glance, you see the current state for the completion a given milestone. Without them, you would have to organize the data from the milestone and plot it yourself to have the same sense of progress.
         = link_to 'Read more', help_page_path('user/project/milestones/burndown_charts.html'), target: '_blank'

ee/app/views/shared/promotions/_promote_contribution_analytics.html.haml

@@ -4,7 +4,13 @@
   .user-callout-copy
     %h4
       - if current_application_settings.should_check_namespace_plan?
-        Upgrade your plan to activate Contribution Analytics.
+        - if @project.group ? can?(current_user, :admin_group, @project.group) : @project.owner == current_user
+          Upgrade your plan to activate Contribution Analytics.
+        - elsif @project.group
+          Upgrade the plan for #{ @project.group.name } to activate Contribution Analytics.
+        - else
+          - owner = @project.namespace.owner
+          Upgrade #{ owner.name }'s plan to activate Contribution Analytics.
       - else
         Track activity with Contribution Analytics.
     %p

ee/app/views/shared/promotions/_promote_mr_approval.html.haml

@@ -9,13 +9,15 @@
         - else
           Improve Merge Requests and customer support with GitLab Enterprise Edition.
       %ul
-        %li
-          = link_to 'Merge Request Approvals', help_page_path('user/project/merge_requests/merge_request_approvals.html'), target: '_blank'
-          %p
-            Merge request approvals allow you to set the number of necessary approvals and predefine a list of approvers that will need to approve every merge request in a project.
-        %li
-          = link_to 'Fast-forward Merge', help_page_path('user/project/merge_requests/fast_forward_merge.html'), target: '_blank'
-          %p
-            If you prefer a linear Git history and a way to accept merge requests without creating merge commits, you can configure this on a per-project basis.
+        - if !@project.feature_available?(:merge_request_approvers)
+          %li
+            = link_to 'Merge Request Approvals', help_page_path('user/project/merge_requests/merge_request_approvals.html'), target: '_blank'
+            %p
+              Merge request approvals allow you to set the number of necessary approvals and predefine a list of approvers that will need to approve every merge request in a project.
+        - if !@project.feature_available?(:fast_forward_merge)
+          %li
+            = link_to 'Fast-forward Merge', help_page_path('user/project/merge_requests/fast_forward_merge.html'), target: '_blank'
+            %p
+              If you prefer a linear Git history and a way to accept merge requests without creating merge commits, you can configure this on a per-project basis.
 
       = render 'shared/promotions/promotion_link_project'

ee/app/views/shared/promotions/_promote_repository_features.html.haml

-.user-callout.promotion-callout{ id: 'promote_repository_features', data: { uid: 'promote_repository_features_dismissed' } }
-  .bordered-box.content-block
-    %button.btn.btn-default.close.js-close-callout{ type: 'button', 'aria-label' => 'Dismiss repository features promotion' }
-      = icon('times', class: 'dismiss-icon', 'aria-hidden' => 'true')
-    .svg-container
-      = custom_icon('icon_push_rules')
-    .user-callout-copy
-      %h4
-        - if current_application_settings.should_check_namespace_plan?
-          Upgrade your plan to improve repositories.
-        - else
-          Improve repositories with GitLab Enterprise Edition.
-      %ul
-        - unless @project.feature_available?(:push_rules)
-          %li
-            = link_to 'Push Rules', help_page_path('push_rules/push_rules.md#push-rules'), target: '_blank'
-            %p
-              Push Rules are defined per project so you can have different rules applied to different projects depends on your needs.
-        - unless @project.feature_available?(:repository_mirrors)
-          %li
-            = link_to 'Repository Mirroring', help_page_path('workflow/repository_mirroring.html'), target: '_blank'
-            %p
-              Repository Mirroring is a way to mirror repositories from external sources. It can be used to mirror all branches, tags, and commits that you have in your repository.
-        - unless @project.feature_available?(:protected_refs_for_users)
-          %li
-            = link_to 'Better Protected Branches', help_page_path('user/project/protected_branches.html'), target: '_blank'
-            %p
-              You can restrict access to protected branches by choosing a role (Masters, Developers) as well as certain users.
+- if show_promotions? && show_callout?('promote_repository_features_dismissed') && (!@project.feature_available?(:push_rules) || !@project.feature_available?(:repository_mirrors) || !@project.feature_available?(:protected_refs_for_users))
+  .user-callout.promotion-callout{ id: 'promote_repository_features', data: { uid: 'promote_repository_features_dismissed' } }
+    .bordered-box.content-block
+      %button.btn.btn-default.close.js-close-callout{ type: 'button', 'aria-label' => 'Dismiss repository features promotion' }
+        = icon('times', class: 'dismiss-icon', 'aria-hidden' => 'true')
+      .svg-container
+        = custom_icon('icon_push_rules')
+      .user-callout-copy
+        %h4
+          - if current_application_settings.should_check_namespace_plan?
+            Upgrade your plan to improve repositories.
+          - else
+            Improve repositories with GitLab Enterprise Edition.
+        %ul
+          - unless @project.feature_available?(:push_rules)
+            %li
+              = link_to 'Push Rules', help_page_path('push_rules/push_rules.md#push-rules'), target: '_blank'
+              %p
+                Push Rules are defined per project so you can have different rules applied to different projects depends on your needs.
+          - unless @project.feature_available?(:repository_mirrors)
+            %li
+              = link_to 'Repository Mirroring', help_page_path('workflow/repository_mirroring.html'), target: '_blank'
+              %p
+                Repository Mirroring is a way to mirror repositories from external sources. It can be used to mirror all branches, tags, and commits that you have in your repository.
+          - unless @project.feature_available?(:protected_refs_for_users)
+            %li
+              = link_to 'Better Protected Branches', help_page_path('user/project/protected_branches.html'), target: '_blank'
+              %p
+                You can restrict access to protected branches by choosing a role (Masters, Developers) as well as certain users.
 
-      = render 'shared/promotions/promotion_link_project'
+        = render 'shared/promotions/promotion_link_project'

ee/app/views/shared/promotions/_promotion_link_project.html.haml

@@ -6,7 +6,7 @@
   - else
     - owner = @project.namespace.owner
     %p Contact owner #{ link_to(owner.name, user_path(owner)) } to upgrade the plan.
-- elsif current_user && current_user.admin?
+- elsif current_user&.admin?
   - if License.current&.expired?
     = link_to 'Buy GitLab Enterprise Edition', Gitlab::SUBSCRIPTIONS_PLANS_URL, class: 'btn btn-primary'
   - else

spec/features/promotion_spec.rb

@@ -79,7 +79,6 @@ describe 'Promotions', js: true do
     it 'should appear in project edit page' do
       visit edit_project_path(project)
       expect(find('#promote_service_desk')).to have_content 'Improve customer support with GitLab Service Desk.'
-      expect(find('#promote_service_desk')).to have_content 'GitLab Service Desk is a simple way to allow people to create issues in your GitLab instance without needing their own user account.'
     end
 
     it 'does not show when cookie is set' do
@@ -106,7 +105,6 @@ describe 'Promotions', js: true do
     it 'should appear in project edit page' do
       visit edit_project_path(project)
       expect(find('#promote_mr_approval')).to have_content 'Improve Merge Requests and customer support'
-      expect(find('#promote_mr_approval')).to have_content 'Merge request approvals allow you to set the number of necessary approvals and predefine a list of approvers that will need to approve every merge request in a project.'
     end
 
     it 'does not show when cookie is set' do
@@ -134,7 +132,6 @@ describe 'Promotions', js: true do
       visit project_settings_repository_path(project)
 
       expect(find('#promote_repository_features')).to have_content 'Improve repositories with GitLab Enterprise Edition'
-      expect(find('#promote_repository_features')).to have_content 'Push Rules are defined per project so you can have different rules applied to different projects depends on your needs.'
     end
 
     it 'does not show when cookie is set' do
@@ -161,7 +158,6 @@ describe 'Promotions', js: true do
     it 'should appear in new MR page' do
       visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature' })
       expect(find('#promote_squash_commits')).to have_content 'Improve Merge Requests with Squash Commit and GitLab Enterprise Edition.'
-      expect(find('#promote_squash_commits')).to have_content 'Squashing lets you tidy up the commit history of a branch when accepting a merge request.'
     end
 
     it 'does not show when cookie is set' do
@@ -188,7 +184,6 @@ describe 'Promotions', js: true do
     it 'should appear in milestone page' do
       visit project_milestone_path(project, milestone)
       expect(find('#promote_burndown_charts')).to have_content 'Improve milestone with Burndown Charts.'
-      expect(find('#promote_burndown_charts')).to have_content 'Burndown Charts are visual representations of the progress of completing a milestone.'
     end
 
     it 'does not show when cookie is set' do
@@ -216,7 +211,6 @@ describe 'Promotions', js: true do
       visit project_issues_path(project)
       click_on 'Export as CSV'
       expect(find('.issues-export-modal')).to have_content 'Export issues with GitLab Enterprise Edition.'
-      expect(find('.issues-export-modal')).to have_content 'Export Issues to CSV enables you and your team to export all the data collected from issues into a comma-separated values (CSV) file'
     end
   end
 
@@ -231,7 +225,6 @@ describe 'Promotions', js: true do
     it 'should appear on the page' do
       visit project_audit_events_path(project)
       expect(find('.user-callout-copy')).to have_content 'Track your project with Audit Events'
-      expect(find('.user-callout-copy')).to have_content 'Audit Events is a way to keep track of important events that happened in GitLab.'
     end
   end
 
@@ -246,7 +239,6 @@ describe 'Promotions', js: true do
     it 'should appear on the page' do
       visit group_analytics_path(group)
       expect(find('.user-callout-copy')).to have_content 'Track activity with Contribution Analytics.'
-      expect(find('.user-callout-copy')).to have_content 'With contribution analytics you can have an overview for the activity of issues, merge requests and push events of your organization and its members.'
     end
   end
 end