Merge branch 'mw-pa-deep-links' into 'master'

Productivity Analytics: Add deep links (BE only)

See merge request gitlab-org/gitlab!21031

ee/app/controllers/analytics/productivity_analytics_controller.rb

@@ -7,6 +7,7 @@ class Analytics::ProductivityAnalyticsController < Analytics::ApplicationControl
 
   before_action :load_group
   before_action :load_project
+  before_action :build_request_params
   before_action -> {
     check_feature_availability!(:productivity_analytics)
   }, if: -> { request.format.json? }
@@ -18,6 +19,8 @@ class Analytics::ProductivityAnalyticsController < Analytics::ApplicationControl
     push_frontend_feature_flag(:productivity_analytics_scatterplot_enabled, default_enabled: true)
   }
 
+  before_action :validate_params, only: :show, if: -> { request.format.json? }
+
   include IssuableCollections
 
   def show
@@ -67,6 +70,29 @@ class Analytics::ProductivityAnalyticsController < Analytics::ApplicationControl
     'merged'
   end
 
+  def validate_params
+    if @request_params.invalid?
+      render(
+        json: { message: 'Invalid parameters', errors: @request_params.errors },
+        status: :unprocessable_entity
+      )
+    end
+  end
+
+  def build_request_params
+    @request_params ||= Analytics::ProductivityAnalyticsRequestParams.new(allowed_request_params.merge(group: @group, project: @project))
+  end
+
+  def allowed_request_params
+    params.permit(
+      :merged_at_after,
+      :merged_at_before,
+      :author_username,
+      :milestone_title,
+      label_name: []
+    )
+  end
+
   def productivity_analytics
     @productivity_analytics ||= ProductivityAnalytics.new(merge_requests: finder.execute, sort: params[:sort])
   end

ee/app/views/analytics/productivity_analytics/show.html.haml

 - page_title _('Productivity Analytics')
 
-#js-productivity-analytics
+- data_attributes = @request_params.valid? ? @request_params.to_data_attributes : @request_params.to_default_data_attributes
+
+#js-productivity-analytics{ data: data_attributes }
   .page-title-holder.d-flex.align-items-center
     .page-title
       = _('Productivity Analytics')

ee/lib/analytics/productivity_analytics_request_params.rb

+# frozen_string_literal: true
+
+module Analytics
+  class ProductivityAnalyticsRequestParams
+    include ActiveModel::Model
+    include ActiveModel::Validations
+    include ActiveModel::Attributes
+
+    DEFAULT_DATE_RANGE = 30.days
+
+    attr_writer :label_name
+
+    attribute :merged_at_after, :datetime
+    attribute :merged_at_before, :datetime
+    attribute :author_username, :string
+    attribute :milestone_title, :string
+
+    attr_accessor :group, :project
+
+    validates :group, presence: true
+    validates :merged_at_after, presence: true
+    validates :merged_at_before, presence: true
+
+    validate :validate_merged_at_after_is_earlier_than_merged_at_before
+    validate :validate_merged_at_before
+    validate :validate_merged_at_after
+
+    def initialize(params = {})
+      params[:merged_at_before] ||= Date.today.at_end_of_day
+      params[:merged_at_after] ||= default_merged_at_after
+
+      super(params)
+    end
+
+    def label_name
+      Array(@label_name)
+    end
+
+    def to_data_attributes
+      {}.tap do |attrs|
+        attrs[:group] = group_data_attributes if group
+        attrs[:project] = project_data_attributes if project
+        attrs[:author_username] = author_username
+        attrs[:label_name] = label_name.any? ? label_name.join(',') : nil
+        attrs[:milestone_title] = milestone_title
+        attrs[:merged_at_after] = merged_at_after.iso8601
+        attrs[:merged_at_before] = merged_at_before.iso8601
+      end.compact
+    end
+
+    def to_default_data_attributes
+      { merged_at_after: merged_at_after.iso8601, merged_at_before: merged_at_before.iso8601 }
+    end
+
+    private
+
+    def group_data_attributes
+      {
+        id: group.id,
+        name: group.name,
+        full_path: group.full_path,
+        avatar_url: group.avatar_url
+      }
+    end
+
+    def project_data_attributes
+      {
+        id: project.id,
+        name: project.name,
+        path_with_namespace: project.path_with_namespace,
+        avatar_url: project.avatar_url
+      }
+    end
+
+    def validate_merged_at_after_is_earlier_than_merged_at_before
+      return if merged_at_after.nil? || merged_at_before.nil?
+      return if merged_at_after <= merged_at_before
+
+      errors.add(:merged_at_before, s_('ProductivityAnalytics|is earlier than the given merged at after date'))
+    end
+
+    def validate_merged_at_before
+      return unless merged_at_before
+
+      validate_against_productivity_analytics_start_date(:merged_at_before, merged_at_before)
+    end
+
+    def validate_merged_at_after
+      return unless merged_at_after
+
+      validate_against_productivity_analytics_start_date(:merged_at_after, merged_at_after)
+    end
+
+    def validate_against_productivity_analytics_start_date(attribute_name, value)
+      return unless productivity_analytics_start_date
+      return if value >= productivity_analytics_start_date
+
+      errors.add(attribute_name, s_('ProductivityAanalytics|is earlier than the allowed minimum date'))
+    end
+
+    def productivity_analytics_start_date
+      @productivity_analytics_start_date ||= ApplicationSetting.current&.productivity_analytics_start_date&.beginning_of_day
+    end
+
+    # Providing default value for `merged_at_after` and prevent setting the value to a datetime where we don't have data (`productivity_analytics_start_date`).
+    def default_merged_at_after
+      default_value = DEFAULT_DATE_RANGE.ago.to_time.utc.beginning_of_day
+
+      if productivity_analytics_start_date && productivity_analytics_start_date > default_value
+        productivity_analytics_start_date
+      else
+        default_value
+      end
+    end
+  end
+end

ee/spec/controllers/analytics/productivity_analytics_controller_spec.rb

@@ -88,6 +88,21 @@ describe Analytics::ProductivityAnalyticsController do
       expect(response).to have_gitlab_http_status(403)
     end
 
+    context 'when invalid params are given' do
+      let(:params) { { group_id: group, merged_at_before: 10.days.ago, merged_at_after: 5.days.ago } }
+
+      before do
+        group.add_owner(current_user)
+      end
+
+      it 'returns 422, unprocessable_entity' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:unprocessable_entity)
+        expect(response).to match_response_schema('analytics/cycle_analytics/validation_error', dir: 'ee')
+      end
+    end
+
     context 'without group_id specified' do
       it 'returns 403' do
         subject

ee/spec/features/analytics/productivity_analytics_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'ProductivityAnalytics' do
+  let(:user) { create(:user) }
+  let(:group) { create(:group) }
+  let(:project) { create(:project, group: group) }
+
+  let(:params) do
+    {
+      author_username: 'user',
+      label_name: %w[label1 label2],
+      milestone_title: 'user',
+      merged_at_after: Date.yesterday.to_time,
+      merged_at_before: Date.today.to_time,
+      group_id: group,
+      project_id: project.full_path
+    }
+  end
+
+  before do
+    stub_licensed_features(productivity_analytics: true)
+
+    sign_in(user)
+
+    group.add_reporter(user)
+  end
+
+  it 'exposes valid url params in data attributes' do
+    visit analytics_productivity_analytics_path(params)
+
+    element = page.find('#js-productivity-analytics')
+
+    expect(element['data-project-id']).to eq(project.id.to_s)
+    expect(element['data-project-name']).to eq(project.name)
+    expect(element['data-project-path-with-namespace']).to eq(project.path_with_namespace)
+    expect(element['data-project-avatar-url']).to eq(project.avatar_url)
+
+    expect(element['data-group-id']).to eq(group.id.to_s)
+    expect(element['data-group-name']).to eq(group.name)
+    expect(element['data-group-full-path']).to eq(group.full_path)
+    expect(element['data-group-avatar-url']).to eq(group.avatar_url)
+
+    expect(element['data-author-username']).to eq(params[:author_username])
+    expect(element['data-label-name']).to eq(params[:label_name].join(','))
+    expect(element['data-milestone-title']).to eq(params[:milestone_title])
+
+    expect(element['data-merged-at-after']).to eq(params[:merged_at_after].utc.iso8601)
+    expect(element['data-merged-at-before']).to eq(params[:merged_at_before].utc.iso8601)
+  end
+
+  context 'when params are invalid' do
+    before do
+      params[:merged_at_before] = params[:merged_at_after] - 5.days # invalid
+    end
+
+    it 'does not expose params in data attributes' do
+      visit analytics_productivity_analytics_path(params)
+
+      element = page.find('#js-productivity-analytics')
+
+      expect(element['data-project-id']).to be_nil
+      expect(element['data-group-id']).to be_nil
+      expect(element['data-author-username']).to be_nil
+
+      expect(element['data-merged-at-before']).not_to be_nil
+      expect(element['data-merged-at-after']).not_to be_nil
+    end
+  end
+end

ee/spec/lib/analytics/productivity_analytics_request_params_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Analytics::ProductivityAnalyticsRequestParams do
+  let(:params) do
+    {
+      author_username: 'user',
+      label_name: %w[label1 label2],
+      milestone_title: 'user',
+      merged_at_after: 5.days.ago.to_time,
+      merged_at_before: Date.today.to_time,
+      group: Group.new
+    }
+  end
+
+  subject { described_class.new(params) }
+
+  describe 'validations' do
+    it 'is valid' do
+      expect(subject).to be_valid
+    end
+
+    describe '`merged_at` params' do
+      context 'when `merged_at_before` is earlier than `merged_at_after`' do
+        before do
+          params[:merged_at_after] = Date.today.to_time
+          params[:merged_at_before] = 5.days.ago.to_time
+        end
+
+        it 'is invalid' do
+          expect(subject).to be_invalid
+          expect(subject.errors.messages[:merged_at_before]).not_to be_empty
+        end
+      end
+
+      context 'when `merged_at_after` is earlier than `productivity_analytics_start_date`' do
+        before do
+          params[:merged_at_after] = 5.days.ago.to_time
+
+          allow(ApplicationSetting)
+            .to receive(:current)
+            .and_return(ApplicationSetting.build_from_defaults(productivity_analytics_start_date: Date.today.to_time))
+        end
+
+        it 'is invalid' do
+          expect(subject).to be_invalid
+          expect(subject.errors.messages[:merged_at_after]).not_to be_empty
+        end
+      end
+
+      context 'when `merged_at_before` is earlier than `productivity_analytics_start_date`' do
+        before do
+          params[:merged_at_before] = 5.days.ago.to_time
+
+          allow(ApplicationSetting)
+            .to receive(:current)
+            .and_return(ApplicationSetting.build_from_defaults(productivity_analytics_start_date: Date.today.to_time))
+        end
+
+        it 'is invalid' do
+          expect(subject).to be_invalid
+          expect(subject.errors.messages[:merged_at_before]).not_to be_empty
+        end
+      end
+    end
+  end
+
+  describe 'default values' do
+    around do |example|
+      Timecop.freeze { example.run }
+    end
+
+    describe '`merged_at_before`' do
+      it 'defaults to today date' do
+        expect(described_class.new.merged_at_before).to eq(Date.today.at_end_of_day)
+      end
+    end
+
+    describe '`merged_at_after`' do
+      context 'when `productivity_analytics_start_date` is within the last 30 days' do
+        before do
+          allow(ApplicationSetting)
+            .to receive(:current)
+            .and_return(ApplicationSetting.build_from_defaults(productivity_analytics_start_date: 15.days.ago.to_time))
+        end
+
+        it 'defaults to `productivity_analytics_start_date`' do
+          expect(described_class.new.merged_at_after).to eq(ApplicationSetting.current.productivity_analytics_start_date.beginning_of_day)
+        end
+      end
+
+      context 'when `productivity_analytics_start_date` older than 30 days' do
+        before do
+          allow(ApplicationSetting)
+            .to receive(:current)
+            .and_return(ApplicationSetting.build_from_defaults(productivity_analytics_start_date: 45.days.ago.to_time))
+        end
+
+        it 'defaults to 30 days ago' do
+          expect(described_class.new.merged_at_after).to eq(30.days.ago.to_time.utc.beginning_of_day)
+        end
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -13328,6 +13328,9 @@ msgstr ""
 msgid "ProductivityAanalytics|Merge requests"
 msgstr ""
 
+msgid "ProductivityAanalytics|is earlier than the allowed minimum date"
+msgstr ""
+
 msgid "ProductivityAnalytics|Ascending"
 msgstr ""
 
@@ -13361,6 +13364,9 @@ msgstr ""
 msgid "ProductivityAnalytics|Trendline"
 msgstr ""
 
+msgid "ProductivityAnalytics|is earlier than the given merged at after date"
+msgstr ""
+
 msgid "Profile"
 msgstr ""